fix(codex-trust): harden TOML edits, status reporting, and key parsing

Four review iterations surfaced correctness issues in the new Codex hook trust path. Notably: CRLF-terminated headers slipped past the upsert regex (Windows would accumulate duplicate blocks), readHookTrustEntries falsely matched headers inside multi-line strings, install() ignored trust-write failures and reported green status, and parseTrustKey accepted malformed keys because Number('') === 0. - TOML scanner: handle CRLF, escape-aware triple-quote counting, literal-string and basic-string quoted keys with embedded ']', and [[array.of.tables]] block boundaries. - escapeTomlString covers \b\f\n\r\t and unescapeTomlString preserves unknown escapes. - New exports: parseTrustKey (strict canonical-int validation), readHookTrustEntries, removeHookTrustEntries. - install(): catch trust-write failures and surface as 'error' state. - getStatus(): verify trust hash + enabled flag for the LAST matching managed group; surface unreadable config.toml as 'partial' so hooks.json health is still visible to the user. - remove(): clean up our [hooks.state."..."] blocks via parseTrustKey. Tests: 46 passing (up from 12), covering CRLF, multi-line strings, literal-string keys, array-of-tables boundaries, parseTrustKey edge cases, and the new read/remove APIs. Co-authored-by: Orca <help@stably.ai>
2026-10-12 16:03:03 +00:00 · 2026-05-07 16:10:30 -07:00
parent 3ea663136b
commit b5eed3341f
3 changed files with 1021 additions and 30 deletions
@@ -1,3 +1,4 @@
+/* eslint-disable max-lines -- Why: this suite keeps the hash fixture, TOML edit edge cases, and trust-state parser regressions together so Codex compatibility failures are easy to audit. */
 import { afterEach, beforeEach, describe, expect, it } from 'vitest'
 import { existsSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'fs'
 import { tmpdir } from 'os'
@@ -5,6 +6,9 @@ import { join } from 'path'
 import {
  computeTrustKey,
  computeTrustedHash,
+  parseTrustKey,
+  readHookTrustEntries,
+  removeHookTrustEntries,
  upsertHookTrustEntries,
  type CodexTrustEntry
 } from './config-toml-trust'
@@ -113,6 +117,92 @@ describe('computeTrustedHash', () => {
    })
    expect(a).toBe(b)
  })
+
+  it('produces a different hash when matcher is set', () => {
+    const a = computeTrustedHash({
+      sourcePath: '/x/hooks.json',
+      eventLabel: 'pre_tool_use',
+      groupIndex: 0,
+      handlerIndex: 0,
+      command: 'foo'
+    })
+    const b = computeTrustedHash({
+      sourcePath: '/x/hooks.json',
+      eventLabel: 'pre_tool_use',
+      groupIndex: 0,
+      handlerIndex: 0,
+      command: 'foo',
+      matcher: 'foo'
+    })
+    expect(a).not.toBe(b)
+  })
+
+  it('produces a different hash when statusMessage is set', () => {
+    const a = computeTrustedHash({
+      sourcePath: '/x/hooks.json',
+      eventLabel: 'pre_tool_use',
+      groupIndex: 0,
+      handlerIndex: 0,
+      command: 'foo'
+    })
+    const b = computeTrustedHash({
+      sourcePath: '/x/hooks.json',
+      eventLabel: 'pre_tool_use',
+      groupIndex: 0,
+      handlerIndex: 0,
+      command: 'foo',
+      statusMessage: 'msg'
+    })
+    expect(a).not.toBe(b)
+  })
+
+  it('produces a different hash when async flips from default false to true', () => {
+    const a = computeTrustedHash({
+      sourcePath: '/x/hooks.json',
+      eventLabel: 'pre_tool_use',
+      groupIndex: 0,
+      handlerIndex: 0,
+      command: 'foo',
+      async: false
+    })
+    const b = computeTrustedHash({
+      sourcePath: '/x/hooks.json',
+      eventLabel: 'pre_tool_use',
+      groupIndex: 0,
+      handlerIndex: 0,
+      command: 'foo',
+      async: true
+    })
+    expect(a).not.toBe(b)
+  })
+
+  it('clamps timeoutSec=0 to 1 (which differs from the unset default of 600)', () => {
+    const zero = computeTrustedHash({
+      sourcePath: '/x/hooks.json',
+      eventLabel: 'pre_tool_use',
+      groupIndex: 0,
+      handlerIndex: 0,
+      command: 'foo',
+      timeoutSec: 0
+    })
+    const one = computeTrustedHash({
+      sourcePath: '/x/hooks.json',
+      eventLabel: 'pre_tool_use',
+      groupIndex: 0,
+      handlerIndex: 0,
+      command: 'foo',
+      timeoutSec: 1
+    })
+    const unset = computeTrustedHash({
+      sourcePath: '/x/hooks.json',
+      eventLabel: 'pre_tool_use',
+      groupIndex: 0,
+      handlerIndex: 0,
+      command: 'foo'
+    })
+    expect(zero).toBe(one)
+    expect(zero).not.toBe(unset)
+  })
 })

 describe('computeTrustKey', () => {
@@ -256,4 +346,483 @@ describe('upsertHookTrustEntries', () => {
    expect(existsSync(`${configPath}.bak`)).toBe(false)
    expect(readFileSync(configPath, 'utf-8')).toBe(firstWrite)
  })
+
+  it('replaces a stale block written with CRLF line endings without duplicating', () => {
+    // Why: regression — Windows-style \r\n in the existing config previously
+    // caused the header pattern to miss and append a duplicate block.
+    const key = '/x/hooks.json:pre_tool_use:0:0'
+    const original = [
+      '[features]',
+      'hooks = true',
+      '',
+      `[hooks.state."${key}"]`,
+      'enabled = true',
+      'trusted_hash = "sha256:STALE"',
+      ''
+    ].join('\r\n')
+    writeFileSync(configPath, original, 'utf-8')
+
+    const entry: CodexTrustEntry = {
+      sourcePath: '/x/hooks.json',
+      eventLabel: 'pre_tool_use',
+      groupIndex: 0,
+      handlerIndex: 0,
+      command: 'echo new'
+    }
+    upsertHookTrustEntries(configPath, [entry])
+
+    const written = readFileSync(configPath, 'utf-8')
+    const occurrences = written.match(/\[hooks\.state\./g) ?? []
+    expect(occurrences).toHaveLength(1)
+    expect(written).not.toContain('STALE')
+    expect(written).toContain(`trusted_hash = "${computeTrustedHash(entry)}"`)
+  })
+
+  it('preserves an immediately-adjacent unrelated hooks.state block', () => {
+    const targetKey = '/x/hooks.json:pre_tool_use:0:0'
+    const neighborKey = '/y/hooks.json:post_tool_use:0:0'
+    const original = [
+      `[hooks.state."${targetKey}"]`,
+      'enabled = true',
+      'trusted_hash = "sha256:STALE"',
+      `[hooks.state."${neighborKey}"]`,
+      'enabled = true',
+      'trusted_hash = "sha256:NEIGHBOR"',
+      ''
+    ].join('\n')
+    writeFileSync(configPath, original, 'utf-8')
+
+    upsertHookTrustEntries(configPath, [
+      {
+        sourcePath: '/x/hooks.json',
+        eventLabel: 'pre_tool_use',
+        groupIndex: 0,
+        handlerIndex: 0,
+        command: 'echo new'
+      }
+    ])
+
+    const written = readFileSync(configPath, 'utf-8')
+    expect(written).not.toContain('STALE')
+    expect(written).toContain(`[hooks.state."${neighborKey}"]`)
+    expect(written).toContain('trusted_hash = "sha256:NEIGHBOR"')
+    // Neighbor's `enabled = true` should still be paired with NEIGHBOR's hash.
+    const neighborIdx = written.indexOf(`[hooks.state."${neighborKey}"]`)
+    expect(written.slice(neighborIdx)).toMatch(/enabled = true[\s\S]*sha256:NEIGHBOR/)
+  })
+
+  it('preserves an unrelated table whose quoted key contains a `]`', () => {
+    const original = ['[other."a]b"]', 'foo = 1', ''].join('\n')
+    writeFileSync(configPath, original, 'utf-8')
+
+    upsertHookTrustEntries(configPath, [
+      {
+        sourcePath: '/x/hooks.json',
+        eventLabel: 'pre_tool_use',
+        groupIndex: 0,
+        handlerIndex: 0,
+        command: 'echo'
+      }
+    ])
+
+    const written = readFileSync(configPath, 'utf-8')
+    expect(written).toContain('[other."a]b"]')
+    expect(written).toContain('foo = 1')
+  })
+
+  // Why: TOML supports both basic-string and literal-string quoted keys;
+  // header detection must respect `]` inside `'...'` too.
+  it('preserves an unrelated table whose literal-string key contains a `]`', () => {
+    const key = '/x/hooks.json:pre_tool_use:0:0'
+    const original = [
+      `[hooks.state."${key}"]`,
+      'enabled = true',
+      'trusted_hash = "sha256:STALE"',
+      "[other.'a]b']",
+      'foo = 1',
+      ''
+    ].join('\n')
+    writeFileSync(configPath, original, 'utf-8')
+
+    upsertHookTrustEntries(configPath, [
+      {
+        sourcePath: '/x/hooks.json',
+        eventLabel: 'pre_tool_use',
+        groupIndex: 0,
+        handlerIndex: 0,
+        command: 'echo new'
+      }
+    ])
+
+    const written = readFileSync(configPath, 'utf-8')
+    expect(written).not.toContain('STALE')
+    expect(written).toContain("[other.'a]b']")
+    expect(written).toContain('foo = 1')
+  })
+
+  it('does not treat `[fake]` inside a multi-line basic string as a header', () => {
+    const original = [
+      'model = "gpt"',
+      'description = """',
+      'This text has a fake header:',
+      '[fake]',
+      'inside it.',
+      '"""',
+      ''
+    ].join('\n')
+    writeFileSync(configPath, original, 'utf-8')
+
+    upsertHookTrustEntries(configPath, [
+      {
+        sourcePath: '/x/hooks.json',
+        eventLabel: 'pre_tool_use',
+        groupIndex: 0,
+        handlerIndex: 0,
+        command: 'echo'
+      }
+    ])
+
+    const written = readFileSync(configPath, 'utf-8')
+    expect(written).toContain(
+      ['description = """', 'This text has a fake header:', '[fake]', 'inside it.', '"""'].join(
+        '\n'
+      )
+    )
+  })
+
+  it('treats `\\"""` inside a multi-line basic string as an escaped quote, not a close', () => {
+    // Why: a basic multi-line string with `\"` escapes must not be misread as
+    // closing early — content and any following real header must survive intact.
+    const original = [
+      'prompt = """',
+      'use \\"\\"\\" carefully',
+      '"""',
+      '',
+      '[other]',
+      'x = 1',
+      ''
+    ].join('\n')
+    writeFileSync(configPath, original, 'utf-8')
+
+    upsertHookTrustEntries(configPath, [
+      {
+        sourcePath: '/x/hooks.json',
+        eventLabel: 'pre_tool_use',
+        groupIndex: 0,
+        handlerIndex: 0,
+        command: 'echo'
+      }
+    ])
+
+    const written = readFileSync(configPath, 'utf-8')
+    expect(written).toContain(['prompt = """', 'use \\"\\"\\" carefully', '"""'].join('\n'))
+    expect(written).toContain('[other]\nx = 1')
+    expect(written).toContain('[hooks.state."/x/hooks.json:pre_tool_use:0:0"]')
+  })
+
+  it('escapes literal `"` and `\\` in the source path inside the trust block header', () => {
+    const entry: CodexTrustEntry = {
+      sourcePath: '/x/with"quote\\and\\back/hooks.json',
+      eventLabel: 'pre_tool_use',
+      groupIndex: 0,
+      handlerIndex: 0,
+      command: 'echo'
+    }
+    upsertHookTrustEntries(configPath, [entry])
+
+    const written = readFileSync(configPath, 'utf-8')
+    expect(written).toContain(
+      `[hooks.state."/x/with\\"quote\\\\and\\\\back/hooks.json:pre_tool_use:0:0"]`
+    )
+  })
+})
+
+describe('removeHookTrustEntries', () => {
+  it('is a no-op (creates no file) when the config does not exist', () => {
+    removeHookTrustEntries(configPath, ['/x/hooks.json:pre_tool_use:0:0'])
+    expect(existsSync(configPath)).toBe(false)
+  })
+
+  it('does not roll a .bak forward when the requested key is not present', () => {
+    const original = ['[features]', 'hooks = true', ''].join('\n')
+    writeFileSync(configPath, original, 'utf-8')
+    removeHookTrustEntries(configPath, ['/missing/hooks.json:pre_tool_use:0:0'])
+    expect(readFileSync(configPath, 'utf-8')).toBe(original)
+    expect(existsSync(`${configPath}.bak`)).toBe(false)
+  })
+
+  it('removes a single block while leaving unrelated tables intact', () => {
+    const key = '/x/hooks.json:pre_tool_use:0:0'
+    const original = [
+      '[features]',
+      'hooks = true',
+      '',
+      `[hooks.state."${key}"]`,
+      'enabled = true',
+      'trusted_hash = "sha256:KEEP"',
+      '',
+      '[unrelated]',
+      'value = 42',
+      ''
+    ].join('\n')
+    writeFileSync(configPath, original, 'utf-8')
+
+    removeHookTrustEntries(configPath, [key])
+
+    const written = readFileSync(configPath, 'utf-8')
+    expect(written).not.toContain(`[hooks.state."${key}"]`)
+    expect(written).not.toContain('sha256:KEEP')
+    expect(written).toContain('[features]\nhooks = true')
+    expect(written).toContain('[unrelated]\nvalue = 42')
+  })
+
+  it('removes multiple blocks in a single call', () => {
+    const keyA = '/x/hooks.json:pre_tool_use:0:0'
+    const keyB = '/x/hooks.json:post_tool_use:0:0'
+    const original = [
+      `[hooks.state."${keyA}"]`,
+      'enabled = true',
+      'trusted_hash = "sha256:A"',
+      '',
+      `[hooks.state."${keyB}"]`,
+      'enabled = true',
+      'trusted_hash = "sha256:B"',
+      ''
+    ].join('\n')
+    writeFileSync(configPath, original, 'utf-8')
+
+    removeHookTrustEntries(configPath, [keyA, keyB])
+
+    const written = readFileSync(configPath, 'utf-8')
+    expect(written).not.toContain(`[hooks.state."${keyA}"]`)
+    expect(written).not.toContain(`[hooks.state."${keyB}"]`)
+    expect(written).not.toContain('sha256:A')
+    expect(written).not.toContain('sha256:B')
+  })
+})
+
+describe('readHookTrustEntries', () => {
+  it('returns an empty map when the file does not exist', () => {
+    const result = readHookTrustEntries(configPath)
+    expect(result.size).toBe(0)
+  })
+
+  it('returns key→hash entries for each [hooks.state."<key>"] block', () => {
+    const keyA = '/x/hooks.json:pre_tool_use:0:0'
+    const keyB = '/y/hooks.json:post_tool_use:1:0'
+    const original = [
+      `[hooks.state."${keyA}"]`,
+      'enabled = true',
+      'trusted_hash = "sha256:AAA"',
+      '',
+      `[hooks.state."${keyB}"]`,
+      'enabled = true',
+      'trusted_hash = "sha256:BBB"',
+      ''
+    ].join('\n')
+    writeFileSync(configPath, original, 'utf-8')
+
+    const result = readHookTrustEntries(configPath)
+    expect(result.size).toBe(2)
+    expect(result.get(keyA)?.trustedHash).toBe('sha256:AAA')
+    expect(result.get(keyA)?.enabled).toBe(true)
+    expect(result.get(keyB)?.trustedHash).toBe('sha256:BBB')
+    expect(result.get(keyB)?.enabled).toBe(true)
+  })
+
+  it('unescapes `\\\\` in the block key', () => {
+    // Why: a real Windows path on disk like C:\foo gets written escaped as
+    // `C:\\foo` inside the TOML key — the returned Map should expose the
+    // original unescaped form.
+    const original = [
+      '[hooks.state."C:\\\\foo\\\\hooks.json:pre_tool_use:0:0"]',
+      'enabled = true',
+      'trusted_hash = "sha256:WIN"',
+      ''
+    ].join('\n')
+    writeFileSync(configPath, original, 'utf-8')
+
+    const result = readHookTrustEntries(configPath)
+    expect(result.get('C:\\foo\\hooks.json:pre_tool_use:0:0')?.trustedHash).toBe('sha256:WIN')
+  })
+
+  it('reads entries from a CRLF-terminated config', () => {
+    const key = '/x/hooks.json:pre_tool_use:0:0'
+    const original = [
+      `[hooks.state."${key}"]`,
+      'enabled = true',
+      'trusted_hash = "sha256:CRLF"',
+      ''
+    ].join('\r\n')
+    writeFileSync(configPath, original, 'utf-8')
+
+    const result = readHookTrustEntries(configPath)
+    expect(result.get(key)?.trustedHash).toBe('sha256:CRLF')
+    expect(result.get(key)?.enabled).toBe(true)
+  })
+
+  it('keeps blocks that have no `trusted_hash` field so callers can see enabled-only state', () => {
+    const key = '/x/hooks.json:pre_tool_use:0:0'
+    const original = [`[hooks.state."${key}"]`, 'enabled = false', ''].join('\n')
+    writeFileSync(configPath, original, 'utf-8')
+
+    const result = readHookTrustEntries(configPath)
+    expect(result.size).toBe(1)
+    expect(result.get(key)).toEqual({ trustedHash: undefined, enabled: false })
+  })
+
+  it('reads disabled state alongside a valid trusted hash', () => {
+    const key = '/x/hooks.json:pre_tool_use:0:0'
+    const original = [
+      `[hooks.state."${key}"]`,
+      'enabled = false',
+      'trusted_hash = "sha256:DISABLED"',
+      ''
+    ].join('\n')
+    writeFileSync(configPath, original, 'utf-8')
+
+    const result = readHookTrustEntries(configPath)
+    expect(result.get(key)).toEqual({ trustedHash: 'sha256:DISABLED', enabled: false })
+  })
+
+  it('does not extract a fake [hooks.state."<key>"] header from inside a """ block', () => {
+    // Why: a header-shaped line embedded in a multi-line basic string must not
+    // be parsed as a real trust entry.
+    const original = [
+      'description = """',
+      '[hooks.state."fake-key"]',
+      'enabled = true',
+      'trusted_hash = "sha256:FAKE"',
+      '"""',
+      ''
+    ].join('\n')
+    writeFileSync(configPath, original, 'utf-8')
+
+    const result = readHookTrustEntries(configPath)
+    expect(result.size).toBe(0)
+  })
+
+  it("does not extract a fake [hooks.state.\"<key>\"] header from inside a ''' block", () => {
+    // Why: same false-positive guard for multi-line literal strings.
+    const original = [
+      "description = '''",
+      '[hooks.state."fake-key"]',
+      'enabled = true',
+      'trusted_hash = "sha256:FAKE"',
+      "'''",
+      ''
+    ].join('\n')
+    writeFileSync(configPath, original, 'utf-8')
+
+    const result = readHookTrustEntries(configPath)
+    expect(result.size).toBe(0)
+  })
+})
+
+describe('parseTrustKey', () => {
+  it('parses a typical posix-style key', () => {
+    expect(parseTrustKey('/Users/x/.codex/hooks.json:pre_tool_use:0:0')).toEqual({
+      sourcePath: '/Users/x/.codex/hooks.json',
+      eventLabel: 'pre_tool_use',
+      groupIndex: 0,
+      handlerIndex: 0
+    })
+  })
+
+  it('parses a Windows-style sourcePath whose drive letter contains a colon', () => {
+    // Why: validates the "anchor on the LAST three colons" approach so colons
+    // inside the sourcePath itself round-trip correctly.
+    expect(parseTrustKey('C:\\Users\\x\\.codex\\hooks.json:session_start:2:3')).toEqual({
+      sourcePath: 'C:\\Users\\x\\.codex\\hooks.json',
+      eventLabel: 'session_start',
+      groupIndex: 2,
+      handlerIndex: 3
+    })
+  })
+
+  it('returns null for a non-Codex event label', () => {
+    expect(parseTrustKey('/x/hooks.json:not_an_event:0:0')).toBeNull()
+  })
+
+  it('returns null for a key with too few colons', () => {
+    expect(parseTrustKey('foo:bar')).toBeNull()
+    expect(parseTrustKey('foo')).toBeNull()
+  })
+
+  it('returns null when the group index is not an integer', () => {
+    expect(parseTrustKey('/x/hooks.json:pre_tool_use:abc:0')).toBeNull()
+  })
+
+  it('returns null when the handler index is not an integer', () => {
+    expect(parseTrustKey('/x/hooks.json:pre_tool_use:0:abc')).toBeNull()
+  })
+
+  it('returns null when the source path is empty', () => {
+    expect(parseTrustKey(':pre_tool_use:0:0')).toBeNull()
+  })
+
+  it('round-trips with computeTrustKey', () => {
+    const entry: CodexTrustEntry = {
+      sourcePath: '/Users/x/.codex/hooks.json',
+      eventLabel: 'post_tool_use',
+      groupIndex: 4,
+      handlerIndex: 7,
+      command: 'irrelevant'
+    }
+    const parsed = parseTrustKey(computeTrustKey(entry))
+    expect(parsed).toEqual({
+      sourcePath: entry.sourcePath,
+      eventLabel: entry.eventLabel,
+      groupIndex: entry.groupIndex,
+      handlerIndex: entry.handlerIndex
+    })
+  })
+
+  // Why: Number('') === 0 silently passes Number.isInteger; without strict
+  // canonical-form validation, malformed keys would coerce into valid ones.
+  it('returns null for empty group/handler segments', () => {
+    expect(parseTrustKey('/x/hooks.json:pre_tool_use::0')).toBeNull()
+    expect(parseTrustKey('/x/hooks.json:pre_tool_use:0:')).toBeNull()
+    expect(parseTrustKey('/x/hooks.json:pre_tool_use::')).toBeNull()
+  })
+
+  it('returns null for exponent or whitespace numeric segments', () => {
+    expect(parseTrustKey('/x/hooks.json:pre_tool_use:1e2:0')).toBeNull()
+    expect(parseTrustKey('/x/hooks.json:pre_tool_use: 0:0')).toBeNull()
+    expect(parseTrustKey('/x/hooks.json:pre_tool_use:01:0')).toBeNull()
+  })
+})
+
+describe('upsertHookTrustEntries with array-of-tables boundaries', () => {
+  // Why: findNextTableHeader must treat `[[array.of.tables]]` as a block
+  // boundary; otherwise an upsert/remove can consume past array entries
+  // into unrelated user content.
+  it('stops the replacement at a following [[array.of.tables]] header', () => {
+    const key = '/x/hooks.json:pre_tool_use:0:0'
+    const original = [
+      `[hooks.state."${key}"]`,
+      'enabled = true',
+      'trusted_hash = "sha256:STALE"',
+      '',
+      '[[products]]',
+      'name = "thing"',
+      ''
+    ].join('\n')
+    writeFileSync(configPath, original, 'utf-8')
+
+    upsertHookTrustEntries(configPath, [
+      {
+        sourcePath: '/x/hooks.json',
+        eventLabel: 'pre_tool_use',
+        groupIndex: 0,
+        handlerIndex: 0,
+        command: 'echo'
+      }
+    ])
+
+    const written = readFileSync(configPath, 'utf-8')
+    expect(written).not.toContain('STALE')
+    expect(written).toContain('[[products]]')
+    expect(written).toContain('name = "thing"')
+  })
 })
@@ -1,3 +1,4 @@
+/* eslint-disable max-lines -- Why: Codex hook trust parsing, hashing, and byte-preserving TOML edits share one fragile file-format contract; splitting would make the compatibility shim harder to audit. */
 import {
  copyFileSync,
  existsSync,
@@ -49,6 +50,11 @@ export type CodexTrustEntry = {
  statusMessage?: string
 }

+export type CodexHookTrustState = {
+  trustedHash?: string
+  enabled?: boolean
+}
+
 // Why: matches Codex's canonical_json. Sorts object keys recursively before
 // SHA-256ing; arrays preserve order.
 function canonicalize(value: unknown): unknown {
@@ -95,6 +101,70 @@ export function computeTrustKey(entry: CodexTrustEntry): string {
  return `${entry.sourcePath}:${entry.eventLabel}:${entry.groupIndex}:${entry.handlerIndex}`
 }

+export function parseTrustKey(key: string): {
+  sourcePath: string
+  eventLabel: CodexEventLabel
+  groupIndex: number
+  handlerIndex: number
+} | null {
+  // Why: keys have shape `<sourcePath>:<eventLabel>:<groupIdx>:<handlerIdx>`.
+  // sourcePath itself may contain `:` (Windows drive letters), so anchor the
+  // parse at the LAST three colons rather than the first.
+  const lastColon = key.lastIndexOf(':')
+  if (lastColon === -1) {
+    return null
+  }
+  const handlerStr = key.slice(lastColon + 1)
+  if (!isCanonicalNonNegativeInt(handlerStr)) {
+    return null
+  }
+  const secondLast = key.lastIndexOf(':', lastColon - 1)
+  if (secondLast === -1) {
+    return null
+  }
+  const groupStr = key.slice(secondLast + 1, lastColon)
+  if (!isCanonicalNonNegativeInt(groupStr)) {
+    return null
+  }
+  const thirdLast = key.lastIndexOf(':', secondLast - 1)
+  if (thirdLast === -1) {
+    return null
+  }
+  const eventLabel = key.slice(thirdLast + 1, secondLast)
+  if (!isCodexEventLabel(eventLabel)) {
+    return null
+  }
+  const sourcePath = key.slice(0, thirdLast)
+  if (sourcePath.length === 0) {
+    return null
+  }
+  return {
+    sourcePath,
+    eventLabel,
+    groupIndex: Number(groupStr),
+    handlerIndex: Number(handlerStr)
+  }
+}
+
+// Why: Number('') === 0 and Number('1e2') === 100 both pass Number.isInteger,
+// so reject any non-canonical decimal form before numeric conversion.
+function isCanonicalNonNegativeInt(value: string): boolean {
+  return /^(0|[1-9]\d*)$/.test(value)
+}
+
+function isCodexEventLabel(value: string): value is CodexEventLabel {
+  return (
+    value === 'pre_tool_use' ||
+    value === 'permission_request' ||
+    value === 'post_tool_use' ||
+    value === 'pre_compact' ||
+    value === 'post_compact' ||
+    value === 'session_start' ||
+    value === 'user_prompt_submit' ||
+    value === 'stop'
+  )
+}
+
 // Why: regex-edit ~/.codex/config.toml rather than parse + reserialize. The
 // file is hand-edited by users (and other tools) and a round-trip through
 // any TOML library would lose comments, key ordering, and inline-table
@@ -126,8 +196,17 @@ function buildTrustBlock(key: string, hash: string): string {
  ].join('\n')
 }

+// Why: TOML basic strings forbid raw control chars; escape backslash first so
+// later substitutions don't double-escape the inserted backslashes.
 function escapeTomlString(value: string): string {
-  return value.replaceAll('\\', '\\\\').replaceAll('"', '\\"')
+  return value
+    .replaceAll('\\', '\\\\')
+    .replaceAll('"', '\\"')
+    .replaceAll('\b', '\\b')
+    .replaceAll('\f', '\\f')
+    .replaceAll('\n', '\\n')
+    .replaceAll('\r', '\\r')
+    .replaceAll('\t', '\\t')
 }

 function upsertTrustBlock(content: string, key: string, hash: string): string {
@@ -160,27 +239,52 @@ function upsertTrustBlock(content: string, key: string, hash: string): string {
 // line. Codex emits the canonical form with the key double-quoted; we never
 // share this slot with another tool, so we don't bother accepting bare
 // dotted-key variants.
+// Why: accept both LF and CRLF — Windows editors (and some user-edited files)
+// terminate the header line with \r\n.
 function buildHeaderPattern(key: string): RegExp {
  const escapedKey = escapeRegex(escapeTomlString(key))
-  return new RegExp(`(^|\\n)\\[hooks\\.state\\."${escapedKey}"\\][ \\t]*\\n`)
+  return new RegExp(`(^|\\r?\\n)\\[hooks\\.state\\."${escapedKey}"\\][ \\t]*\\r?\\n`)
 }

 function escapeRegex(value: string): string {
  return value.replaceAll(/[.*+?^${}()|[\]\\]/g, '\\$&')
 }

-// Why: scan for the next top-level `[name]` header. Skip array-of-tables
-// `[[name]]`. We only treat lines whose first non-whitespace character is
-// `[` and that close cleanly with `]` (modulo trailing comment/whitespace)
-// as headers, so `[` inside string values is ignored.
+// Why: quoted keys can contain `]` (e.g. `[hooks.state."a]b"]`) and `[` lines
+// inside multi-line strings aren't headers, so we need a stateful scanner —
+// a flat regex misclassifies both cases.
 function findNextTableHeader(text: string): number {
  let cursor = 0
+  let inMultilineBasic = false
+  let inMultilineLiteral = false
  while (cursor < text.length) {
    const newlineIdx = text.indexOf('\n', cursor)
    const lineEnd = newlineIdx === -1 ? text.length : newlineIdx
-    const line = text.slice(cursor, lineEnd).trimStart()
-    if (line.startsWith('[') && !line.startsWith('[[') && /^\[[^\]]*\]\s*(#.*)?$/.test(line)) {
-      return cursor
+    const rawLine = text.slice(cursor, lineEnd)
+    const line = rawLine.replace(/\r$/, '')
+    if (inMultilineBasic) {
+      if (countUnescapedTripleQuote(line, '"""') % 2 === 1) {
+        inMultilineBasic = false
+      }
+    } else if (inMultilineLiteral) {
+      if (countTripleQuote(line, "'''") % 2 === 1) {
+        inMultilineLiteral = false
+      }
+    } else {
+      const trimmed = line.trimStart()
+      // Why: stop at both `[table]` and `[[array.of.tables]]` — both end our
+      // block. Skipping `[[ ]]` here would let our slice consume past array
+      // entries into unrelated user content.
+      if (trimmed.startsWith('[') && isCompleteTableHeader(trimmed)) {
+        return cursor
+      }
+      // Odd count means this line opens a multi-line string without closing it.
+      if (countUnescapedTripleQuote(line, '"""') % 2 === 1) {
+        inMultilineBasic = true
+      }
+      if (countTripleQuote(line, "'''") % 2 === 1) {
+        inMultilineLiteral = true
+      }
    }
    if (newlineIdx === -1) {
      return -1
@@ -190,6 +294,96 @@ function findNextTableHeader(text: string): number {
  return -1
 }

+// Why: literal strings (`'''`) don't honor escapes, so a plain indexOf scan
+// suffices.
+function countTripleQuote(line: string, quote: string): number {
+  let count = 0
+  let i = 0
+  while ((i = line.indexOf(quote, i)) !== -1) {
+    count++
+    i += 3
+  }
+  return count
+}
+
+// Why: basic multi-line strings honor `\"` (and `\\`) escapes. Skip past
+// `\<char>` so an escaped quote doesn't count toward triple-quote scans.
+function countUnescapedTripleQuote(line: string, quote: '"""'): number {
+  let count = 0
+  let i = 0
+  while (i < line.length) {
+    if (line[i] === '\\' && i + 1 < line.length) {
+      i += 2
+      continue
+    }
+    if (line.startsWith(quote, i)) {
+      count++
+      i += 3
+      continue
+    }
+    i++
+  }
+  return count
+}
+
+// Why: walk the header byte-by-byte so `]` inside a quoted key segment
+// doesn't terminate us early. Basic strings honor `\` escapes; literal
+// strings (single quotes) don't allow escapes per TOML spec.
+// Accepts both `[table]` and `[[array.of.tables]]` since either ends a block.
+function isCompleteTableHeader(line: string): boolean {
+  if (!line.startsWith('[')) {
+    return false
+  }
+  const isArrayHeader = line.startsWith('[[')
+  let i = isArrayHeader ? 2 : 1
+  let inBasicQuote = false
+  let inLiteralQuote = false
+  while (i < line.length) {
+    const ch = line[i]
+    if (inBasicQuote) {
+      if (ch === '\\' && i + 1 < line.length) {
+        i += 2
+        continue
+      }
+      if (ch === '"') {
+        inBasicQuote = false
+      }
+      i++
+      continue
+    }
+    if (inLiteralQuote) {
+      if (ch === "'") {
+        inLiteralQuote = false
+      }
+      i++
+      continue
+    }
+    if (ch === '"') {
+      inBasicQuote = true
+      i++
+      continue
+    }
+    if (ch === "'") {
+      inLiteralQuote = true
+      i++
+      continue
+    }
+    if (ch === ']') {
+      if (isArrayHeader) {
+        if (line[i + 1] !== ']') {
+          return false
+        }
+        const tail = line.slice(i + 2)
+        return /^\s*(#.*)?$/.test(tail)
+      }
+      const tail = line.slice(i + 1)
+      return /^\s*(#.*)?$/.test(tail)
+    }
+    i++
+  }
+  return false
+}
+
 // Why: same atomic-rename + .bak rotation pattern as writeHooksJson — a
 // half-written config.toml can brick a user's Codex install, so write to
 // tmp and rename. Random-suffix tmp name avoids cross-process races on
@@ -216,3 +410,133 @@ function writeConfigAtomically(configPath: string, contents: string): void {
    }
  }
 }
+
+export function removeHookTrustEntries(configPath: string, keys: readonly string[]): void {
+  if (!existsSync(configPath)) {
+    return
+  }
+  const existing = readFileSync(configPath, 'utf-8')
+  let updated = existing
+  for (const key of keys) {
+    updated = removeTrustBlock(updated, key)
+  }
+  if (updated === existing) {
+    return
+  }
+  writeConfigAtomically(configPath, updated)
+}
+
+function removeTrustBlock(content: string, key: string): string {
+  const headerPattern = buildHeaderPattern(key)
+  const match = headerPattern.exec(content)
+  if (!match) {
+    return content
+  }
+  // Why: cut from match.index (which includes the captured leading newline)
+  // so we don't strand a blank line where our block used to live.
+  const cutStart = match.index
+  const headerLineEnd = match.index + match[0].length
+  const after = content.slice(headerLineEnd)
+  const nextHeaderRel = findNextTableHeader(after)
+  const cutEnd = nextHeaderRel === -1 ? content.length : headerLineEnd + nextHeaderRel
+  return content.slice(0, cutStart) + content.slice(cutEnd)
+}
+
+export function readHookTrustEntries(configPath: string): Map<string, CodexHookTrustState> {
+  const result = new Map<string, CodexHookTrustState>()
+  if (!existsSync(configPath)) {
+    return result
+  }
+  const content = readFileSync(configPath, 'utf-8')
+  // Why: walk line-by-line so `[hooks.state."..."]` inside a `"""..."""` or
+  // `'''...'''` multi-line string isn't mistaken for a real header.
+  const headerLineRegex = /^[ \t]*\[hooks\.state\."((?:[^"\\]|\\.)*)"\][ \t]*$/
+  let cursor = 0
+  let inMultilineBasic = false
+  let inMultilineLiteral = false
+  while (cursor < content.length) {
+    const newlineIdx = content.indexOf('\n', cursor)
+    const lineEnd = newlineIdx === -1 ? content.length : newlineIdx
+    const rawLine = content.slice(cursor, lineEnd)
+    const line = rawLine.replace(/\r$/, '')
+    const nextCursor = newlineIdx === -1 ? content.length : newlineIdx + 1
+    if (inMultilineBasic) {
+      if (countUnescapedTripleQuote(line, '"""') % 2 === 1) {
+        inMultilineBasic = false
+      }
+      cursor = nextCursor
+      continue
+    }
+    if (inMultilineLiteral) {
+      if (countTripleQuote(line, "'''") % 2 === 1) {
+        inMultilineLiteral = false
+      }
+      cursor = nextCursor
+      continue
+    }
+    const headerMatch = headerLineRegex.exec(line)
+    if (headerMatch) {
+      const escapedKey = headerMatch[1]
+      const key = unescapeTomlString(escapedKey)
+      // Why: block ends at the next *real* header (multi-line aware).
+      const after = content.slice(nextCursor)
+      const nextHeaderRel = findNextTableHeader(after)
+      const blockEnd = nextHeaderRel === -1 ? content.length : nextCursor + nextHeaderRel
+      const block = content.slice(nextCursor, blockEnd)
+      // Why: we own this block's shape (only `enabled` + `trusted_hash`), so
+      // a line scan beats pulling in a full TOML value parser.
+      const hashMatch = /^[ \t]*trusted_hash[ \t]*=[ \t]*"((?:[^"\\]|\\.)*)"/m.exec(block)
+      const enabledMatch = /^[ \t]*enabled[ \t]*=[ \t]*(true|false)[ \t\r]*(?:#.*)?$/m.exec(block)
+      result.set(key, {
+        trustedHash: hashMatch ? unescapeTomlString(hashMatch[1]) : undefined,
+        enabled: enabledMatch ? enabledMatch[1] === 'true' : undefined
+      })
+      cursor = nextCursor
+      continue
+    }
+    if (countUnescapedTripleQuote(line, '"""') % 2 === 1) {
+      inMultilineBasic = true
+    }
+    if (countTripleQuote(line, "'''") % 2 === 1) {
+      inMultilineLiteral = true
+    }
+    cursor = nextCursor
+  }
+  return result
+}
+
+function unescapeTomlString(escaped: string): string {
+  let result = ''
+  let i = 0
+  while (i < escaped.length) {
+    const ch = escaped[i]
+    if (ch === '\\' && i + 1 < escaped.length) {
+      const next = escaped[i + 1]
+      if (next === 'n') {
+        result += '\n'
+      } else if (next === 'r') {
+        result += '\r'
+      } else if (next === 't') {
+        result += '\t'
+      } else if (next === 'b') {
+        result += '\b'
+      } else if (next === 'f') {
+        result += '\f'
+      } else if (next === '"') {
+        result += '"'
+      } else if (next === '\\') {
+        result += '\\'
+      }
+      // Why: unknown escapes round-trip — preserve the backslash so we don't
+      // silently drop information.
+      else {
+        result += `\\${next}`
+      }
+      i += 2
+    } else {
+      result += ch
+      i++
+    }
+  }
+  return result
+}
@@ -12,8 +12,14 @@ import {
  type HookDefinition
 } from '../agent-hooks/installer-utils'
 import {
+  computeTrustKey,
+  computeTrustedHash,
+  parseTrustKey,
+  readHookTrustEntries,
+  removeHookTrustEntries,
  upsertHookTrustEntries,
  type CodexEventLabel,
+  type CodexHookTrustState,
  type CodexTrustEntry
 } from './config-toml-trust'

@@ -130,37 +136,91 @@ export class CodexHookService {
      }
    }

-    // Why: Report `partial` when only some managed events are registered so the
-    // sidebar surfaces a degraded install rather than a false-positive
-    // `installed`. Each CODEX_EVENTS entry must contain the managed command for
-    // the integration to function end-to-end (e.g. PreToolUse is required for
-    // permission-prompt detection per the comment above).
+    // Why: Report `partial` when managed events are missing OR when their
+    // trust entries are missing/stale. Codex 0.129+ silently drops untrusted
+    // hooks, so a green status without trust verification is misleading.
    const command = getManagedCommand(scriptPath)
+    const tomlPath = getCodexConfigTomlPath()
+    // Why: an unreadable config.toml (EACCES/EIO) is distinct from "file
+    // absent" (which returns an empty Map without throwing). Hooks.json may
+    // still be fine, so report partial with a specific reason rather than
+    // collapsing to a generic error or masking it as universally-stale trust.
+    let trustEntries: Map<string, CodexHookTrustState>
+    let trustReadError: string | null = null
+    try {
+      trustEntries = readHookTrustEntries(tomlPath)
+    } catch (error) {
+      trustEntries = new Map()
+      trustReadError = error instanceof Error ? error.message : String(error)
+    }
+
    const missing: string[] = []
+    const trustMissing: string[] = []
+    const disabled: string[] = []
    let presentCount = 0
    for (const eventName of CODEX_EVENTS) {
      const definitions = Array.isArray(config.hooks?.[eventName]) ? config.hooks![eventName]! : []
-      const hasCommand = definitions.some((definition) =>
-        (definition.hooks ?? []).some((hook) => hook.command === command)
-      )
-      if (hasCommand) {
-        presentCount += 1
-      } else {
+      // Why: install() appends our managed definition at the end, so its
+      // group index is the LAST match. Picking the first match would
+      // misreport stale duplicates as trust-missing.
+      let foundGroupIndex = -1
+      definitions.forEach((definition, idx) => {
+        const hooks = definition.hooks ?? []
+        if (hooks.some((hook) => hook.command === command)) {
+          foundGroupIndex = idx
+        }
+      })
+      if (foundGroupIndex === -1) {
        missing.push(eventName)
+        continue
+      }
+      presentCount += 1
+      // Why: a stale hash blocks firing the same as a missing entry, so
+      // compare against the canonical hash we would write.
+      const trustInput: CodexTrustEntry = {
+        sourcePath: configPath,
+        eventLabel: CODEX_EVENT_LABEL[eventName],
+        groupIndex: foundGroupIndex,
+        handlerIndex: 0,
+        command
+      }
+      const expectedHash = computeTrustedHash(trustInput)
+      const actualState = trustEntries.get(computeTrustKey(trustInput))
+      if (actualState?.trustedHash !== expectedHash) {
+        trustMissing.push(eventName)
+      } else if (actualState?.enabled === false) {
+        disabled.push(eventName)
      }
    }
    const managedHooksPresent = presentCount > 0
    let state: AgentHookInstallState
    let detail: string | null
-    if (missing.length === 0) {
-      state = 'installed'
-      detail = null
-    } else if (presentCount === 0) {
+    if (presentCount === 0) {
      state = 'not_installed'
      detail = null
+    } else if (
+      missing.length === 0 &&
+      trustMissing.length === 0 &&
+      disabled.length === 0 &&
+      trustReadError === null
+    ) {
+      state = 'installed'
+      detail = null
    } else {
      state = 'partial'
-      detail = `Managed hook missing for events: ${missing.join(', ')}`
+      const parts: string[] = []
+      if (missing.length > 0) {
+        parts.push(`Managed hook missing for events: ${missing.join(', ')}`)
+      }
+      if (trustReadError !== null) {
+        parts.push(`Trust entries unverifiable: ${trustReadError}`)
+      } else if (trustMissing.length > 0) {
+        parts.push(`Trust entry missing or stale for events: ${trustMissing.join(', ')}`)
+      }
+      if (disabled.length > 0) {
+        parts.push(`Managed hook disabled for events: ${disabled.join(', ')}`)
+      }
+      detail = parts.join('; ')
    }
    return { agent: 'codex', state, configPath, managedHooksPresent, detail }
  }
@@ -240,11 +300,20 @@ export class CodexHookService {
    config.hooks = nextHooks
    writeManagedScript(scriptPath, getManagedScript())
    writeHooksJson(configPath, config)
-    // Why: write trust entries AFTER hooks.json so a partial install (script
-    // written, hooks.json not) cannot leave a trust hash pointing at a hook
-    // that does not exist. The reverse order would be benign but this keeps
-    // the invariant clear.
-    upsertHookTrustEntries(getCodexConfigTomlPath(), trustEntries)
+    // Why: trust entries write last so a half-write can't leave a hash
+    // pointing at a hook that doesn't exist. Surface failures — without this,
+    // getStatus would report green for a hook Codex won't actually fire.
+    try {
+      upsertHookTrustEntries(getCodexConfigTomlPath(), trustEntries)
+    } catch (error) {
+      return {
+        agent: 'codex',
+        state: 'error',
+        configPath,
+        managedHooksPresent: true,
+        detail: `Hooks installed but trust entries could not be written: ${error instanceof Error ? error.message : String(error)}. Run /hooks in Codex to approve.`
+      }
+    }
    return this.getStatus()
  }

@@ -281,6 +350,35 @@ export class CodexHookService {
    }
    config.hooks = nextHooks
    writeHooksJson(configPath, config)
+
+    // Why: also drop our trust entries so config.toml doesn't accumulate dead
+    // [hooks.state."..."] blocks across install/remove cycles. Best-effort —
+    // a stale entry is harmless once hooks.json no longer references it.
+    try {
+      const tomlPath = getCodexConfigTomlPath()
+      const existingKeys = readHookTrustEntries(tomlPath)
+      // Why: only drop entries WE could have written — same configPath,
+      // recognized event label. parseTrustKey returns null for malformed or
+      // unrelated keys, keeping the shape definition centralized.
+      const ourKeys: string[] = []
+      for (const key of existingKeys.keys()) {
+        const parts = parseTrustKey(key)
+        if (parts === null) {
+          continue
+        }
+        if (parts.sourcePath !== configPath) {
+          continue
+        }
+        ourKeys.push(key)
+      }
+      if (ourKeys.length > 0) {
+        removeHookTrustEntries(tomlPath, ourKeys)
+      }
+    } catch {
+      // Best effort — stale trust entries are harmless once hooks.json no
+      // longer references the hook.
+    }
+
    return this.getStatus()
  }
 }