fix(codex): parse config.toml structurally for hook trust detection

cmd/entire/cli/agent/codex/trust.go

@@ -4,14 +4,16 @@ import (
 	"encoding/json"
 	"os"
 	"path/filepath"
-	"regexp"
 	"strings"
+
+	"github.com/pelletier/go-toml/v2"
 )
 
 // HookTrustGaps returns the snake_case event labels declared in
 // <repoRoot>/.codex/hooks.json that don't have a matching
-// `[hooks.state."<hooks.json>:<event>:0:0"]` entry in the user's Codex
-// config.toml — i.e. events the local user hasn't approved yet.
+// `[hooks.state.<"hooks.json:event:0:0">]` entry (any TOML key quoting)
+// in the user's Codex config.toml — i.e. events the local user hasn't
+// approved yet.
 //
 // This is the structural form of the trust check: we don't recompute
 // Codex's hook hash, we only look at key presence. That misses the
@@ -20,24 +22,29 @@ import (
 // to surface fresh additions like "you trusted three hooks last month
 // but a new PostToolUse arrived" inside our SessionStart welcome.
 //
-// Returns nil when:
-//   - .codex/hooks.json doesn't exist (entire isn't installed in this repo)
-//   - The user's config.toml can't be read
-//   - Every declared event already has a state entry
-func HookTrustGaps(repoRoot string) []string {
+// The bool reports whether the check was actually performed. It is
+// false — with nil gaps — when hooks.json is missing/malformed or the
+// user's config.toml can't be read or parsed as TOML: "can't tell"
+// cases where the caller must not claim the hooks were verified.
+// Callers that only warn on gaps (the SessionStart banner) can ignore
+// it; doctor uses it to say "not verified" instead of "OK".
+func HookTrustGaps(repoRoot string) ([]string, bool) {
 	hooksJSONPath := filepath.Join(repoRoot, ".codex", "hooks.json")
 	declared, ok := declaredCodexEvents(hooksJSONPath)
-	if !ok || len(declared) == 0 {
-		return nil
+	if !ok {
+		return nil, false
+	}
+	if len(declared) == 0 {
+		return nil, true
 	}
 
 	configPath := codexConfigPath()
 	if configPath == "" {
-		return nil
+		return nil, false
 	}
 	trusted, ok := readCodexTrustedKeys(configPath)
 	if !ok {
-		return nil
+		return nil, false
 	}
 
 	var gaps []string
@@ -49,7 +56,7 @@ func HookTrustGaps(repoRoot string) []string {
 			gaps = append(gaps, ev)
 		}
 	}
-	return gaps
+	return gaps, true
 }
 
 func codexConfigPath() string {
@@ -124,20 +131,40 @@ func MissingEntireHooks(repoRoot string) []string {
 	return missing
 }
 
-// codexTrustStateHeaderRegex matches `[hooks.state."<key>"]` headers in
-// the user's Codex config.toml. Quote-only — Codex's own writer emits
-// quoted keys (codex-rs/tui/src/app/background_requests.rs:874), and
-// looser parsing would invite false matches in user-edited configs.
-var codexTrustStateHeaderRegex = regexp.MustCompile(`(?m)^\[hooks\.state\."([^"]+)"\]`)
-
+// readCodexTrustedKeys parses the user's Codex config.toml and returns
+// the set of keys under the `hooks.state` table. The parse is
+// structural (not a header regex): Codex's writer has emitted both
+// basic (double-quoted) and literal (single-quoted) TOML keys — on
+// Windows the backslashes in the hooks.json path make literal quoting
+// the natural serialization — and a real TOML parse handles every
+// quoting form, unescaping basic strings so keys compare cleanly
+// against filepath.Join output (issue #1761).
+//
+// Returns ok=false when the file is missing or not valid TOML — "can't
+// tell" cases where callers must stay silent rather than flag every
+// hook as untrusted. A parseable config without a hooks.state table
+// returns an empty set with ok=true: that's a real "nothing approved
+// yet" state and the gap warning should fire.
 func readCodexTrustedKeys(configPath string) (map[string]struct{}, bool) {
 	data, err := os.ReadFile(configPath) //nolint:gosec // path resolved from CODEX_HOME or HOME
 	if err != nil {
 		return nil, false
 	}
+	var config map[string]any
+	if err := toml.Unmarshal(data, &config); err != nil {
+		return nil, false
+	}
 	keys := make(map[string]struct{})
-	for _, m := range codexTrustStateHeaderRegex.FindAllStringSubmatch(string(data), -1) {
-		keys[m[1]] = struct{}{}
+	hooks, ok := config["hooks"].(map[string]any)
+	if !ok {
+		return keys, true
+	}
+	state, ok := hooks["state"].(map[string]any)
+	if !ok {
+		return keys, true
+	}
+	for k := range state {
+		keys[k] = struct{}{}
 	}
 	return keys, true
 }

cmd/entire/cli/agent/codex/trust_test.go

@@ -8,6 +8,19 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
+// Shared hooks.json fixtures. The two-event form exercises the
+// trusted/untrusted split; the one-event form is for tests where only
+// config.toml parsing behavior matters.
+const (
+	hooksJSONSessionStartAndPostToolUse = `{
+  "hooks": {
+    "SessionStart": [{"matcher": null, "hooks": [{"type":"command","command":"x","timeout":30}]}],
+    "PostToolUse": [{"matcher": null, "hooks": [{"type":"command","command":"x","timeout":30}]}]
+  }
+}`
+	hooksJSONSessionStartOnly = `{"hooks":{"SessionStart":[{"matcher":null,"hooks":[{"type":"command","command":"x","timeout":30}]}]}}`
+)
+
 // writeTrustFixture sets up the .codex/hooks.json fixture and points
 // CODEX_HOME at an isolated temp directory so HookTrustGaps resolves
 // the user config without touching ~/.codex on the dev machine. Tests
@@ -55,19 +68,15 @@ trusted_hash = "sha256:ccc"
 `
 	require.NoError(t, os.WriteFile(filepath.Join(os.Getenv("CODEX_HOME"), "config.toml"), []byte(configTOML), 0o600))
 
-	gaps := HookTrustGaps(repoRoot)
+	gaps, ok := HookTrustGaps(repoRoot)
+	require.True(t, ok)
 	require.Equal(t, []string{"post_tool_use"}, gaps)
 }
 
 // TestHookTrustGaps_NoGapsWhenAllTrusted returns nil when every declared
 // event has a state entry, even if extra entries exist for other paths.
 func TestHookTrustGaps_NoGapsWhenAllTrusted(t *testing.T) {
-	hooksJSON := `{
-  "hooks": {
-    "SessionStart": [{"matcher": null, "hooks": [{"type":"command","command":"x","timeout":30}]}],
-    "PostToolUse": [{"matcher": null, "hooks": [{"type":"command","command":"x","timeout":30}]}]
-  }
-}`
+	hooksJSON := hooksJSONSessionStartAndPostToolUse
 	repoRoot, hooksPath := writeTrustFixture(t, hooksJSON)
 
 	// Trust both, plus an unrelated entry from another repo to make sure
@@ -83,7 +92,8 @@ trusted_hash = "sha256:ccc"
 `
 	require.NoError(t, os.WriteFile(filepath.Join(os.Getenv("CODEX_HOME"), "config.toml"), []byte(configTOML), 0o600))
 
-	gaps := HookTrustGaps(repoRoot)
+	gaps, ok := HookTrustGaps(repoRoot)
+	require.True(t, ok)
 	require.Empty(t, gaps)
 }
 
@@ -92,23 +102,27 @@ trusted_hash = "sha256:ccc"
 func TestHookTrustGaps_NilWhenHooksJSONMissing(t *testing.T) {
 	tmp := t.TempDir()
 	t.Setenv("CODEX_HOME", tmp)
-	require.Nil(t, HookTrustGaps(tmp))
+	gaps, ok := HookTrustGaps(tmp)
+	require.Nil(t, gaps)
+	require.False(t, ok)
 }
 
 // TestHookTrustGaps_NilWhenConfigUnreadable — first-run users have no
 // config.toml yet. Codex's own startup warning still fires for them, so
 // our partial detection staying quiet is the right behavior; we'd
 // otherwise duplicate the warning.
 func TestHookTrustGaps_NilWhenConfigUnreadable(t *testing.T) {
-	hooksJSON := `{"hooks":{"SessionStart":[{"matcher":null,"hooks":[{"type":"command","command":"x","timeout":30}]}]}}`
+	hooksJSON := hooksJSONSessionStartOnly
 	tmp := t.TempDir()
 	codexHome := filepath.Join(tmp, "codex-home")
 	require.NoError(t, os.MkdirAll(filepath.Join(tmp, "repo", ".codex"), 0o750))
 	require.NoError(t, os.MkdirAll(codexHome, 0o750))
 	require.NoError(t, os.WriteFile(filepath.Join(tmp, "repo", ".codex", "hooks.json"), []byte(hooksJSON), 0o600))
 	t.Setenv("CODEX_HOME", codexHome)
 
-	require.Nil(t, HookTrustGaps(filepath.Join(tmp, "repo")))
+	gaps, ok := HookTrustGaps(filepath.Join(tmp, "repo"))
+	require.Nil(t, gaps)
+	require.False(t, ok)
 }
 
 // TestMissingEntireHooks_FlagsStaleFile — user enabled Codex on an
@@ -173,5 +187,151 @@ func TestHookTrustGaps_HandlesNonzeroHandlerIndex(t *testing.T) {
 trusted_hash = "sha256:aaa"
 `
 	require.NoError(t, os.WriteFile(filepath.Join(os.Getenv("CODEX_HOME"), "config.toml"), []byte(configTOML), 0o600))
-	require.Empty(t, HookTrustGaps(repoRoot))
+	gaps, ok := HookTrustGaps(repoRoot)
+	require.True(t, ok)
+	require.Empty(t, gaps)
+}
+
+// TestHookTrustGaps_SingleQuotedStateKeys — current Codex serializes
+// state keys as TOML literal (single-quoted) strings, the natural form
+// on Windows where backslashes need no escaping. The trust check must
+// recognize them the same as double-quoted keys (issue #1761).
+func TestHookTrustGaps_SingleQuotedStateKeys(t *testing.T) {
+	hooksJSON := hooksJSONSessionStartAndPostToolUse
+	repoRoot, hooksPath := writeTrustFixture(t, hooksJSON)
+
+	configTOML := `[hooks.state.'` + hooksPath + `:session_start:0:0']
+trusted_hash = "sha256:aaa"
+
+[hooks.state.'` + hooksPath + `:post_tool_use:0:0']
+trusted_hash = "sha256:bbb"
+`
+	require.NoError(t, os.WriteFile(filepath.Join(os.Getenv("CODEX_HOME"), "config.toml"), []byte(configTOML), 0o600))
+
+	gaps, ok := HookTrustGaps(repoRoot)
+	require.True(t, ok)
+	require.Empty(t, gaps)
+}
+
+// TestHookTrustGaps_SingleQuotedFlagsMissingEvent — literal-quoted keys
+// must not only count as trust; a genuinely missing event must still be
+// flagged when the other entries are single-quoted.
+func TestHookTrustGaps_SingleQuotedFlagsMissingEvent(t *testing.T) {
+	hooksJSON := hooksJSONSessionStartAndPostToolUse
+	repoRoot, hooksPath := writeTrustFixture(t, hooksJSON)
+
+	configTOML := `[hooks.state.'` + hooksPath + `:session_start:0:0']
+trusted_hash = "sha256:aaa"
+`
+	require.NoError(t, os.WriteFile(filepath.Join(os.Getenv("CODEX_HOME"), "config.toml"), []byte(configTOML), 0o600))
+
+	gaps, ok := HookTrustGaps(repoRoot)
+	require.True(t, ok)
+	require.Equal(t, []string{"post_tool_use"}, gaps)
+}
+
+// TestHookTrustGaps_MalformedConfigStaysSilent — a config.toml that
+// isn't valid TOML is a "can't tell" case, not a "nothing trusted"
+// case. The old regex scraper degraded it to zero trusted keys and
+// flagged every hook; structural parsing must report ok=false instead
+// (same contract as an unreadable file) so doctor can say "not
+// verified" rather than claiming OK.
+func TestHookTrustGaps_MalformedConfigStaysSilent(t *testing.T) {
+	hooksJSON := hooksJSONSessionStartOnly
+	repoRoot, _ := writeTrustFixture(t, hooksJSON)
+	require.NoError(t, os.WriteFile(filepath.Join(os.Getenv("CODEX_HOME"), "config.toml"), []byte("[hooks.state.\"unterminated\n"), 0o600))
+	gaps, ok := HookTrustGaps(repoRoot)
+	require.Nil(t, gaps)
+	require.False(t, ok)
+}
+
+// TestHookTrustGaps_ConfigWithoutStateSection — a parseable config that
+// simply has no hooks.state table is the genuine "fresh clone, nothing
+// approved yet" state: the gap warning must fire.
+func TestHookTrustGaps_ConfigWithoutStateSection(t *testing.T) {
+	hooksJSON := hooksJSONSessionStartOnly
+	repoRoot, _ := writeTrustFixture(t, hooksJSON)
+	require.NoError(t, os.WriteFile(filepath.Join(os.Getenv("CODEX_HOME"), "config.toml"), []byte("model = \"gpt-5\"\n"), 0o600))
+	gaps, ok := HookTrustGaps(repoRoot)
+	require.True(t, ok)
+	require.Equal(t, []string{"session_start"}, gaps)
+}
+
+// The readCodexTrustedKeys tests below hardcode Windows-shaped keys.
+// Full-flow HookTrustGaps tests can't exercise backslash paths on a
+// POSIX dev machine (filepath.Join emits "/"), but key parsing is
+// OS-independent — this is exactly the layer issue #1761 lives in.
+
+// TestReadCodexTrustedKeys_WindowsLiteralKeys — the reported bug: a
+// literal (single-quoted) key holding a Windows path must come back
+// verbatim, backslashes intact.
+func TestReadCodexTrustedKeys_WindowsLiteralKeys(t *testing.T) {
+	t.Parallel()
+	configPath := filepath.Join(t.TempDir(), "config.toml")
+	configTOML := `[hooks.state.'C:\repo\.codex\hooks.json:session_start:0:0']
+trusted_hash = "sha256:aaa"
+`
+	require.NoError(t, os.WriteFile(configPath, []byte(configTOML), 0o600))
+
+	keys, ok := readCodexTrustedKeys(configPath)
+	require.True(t, ok)
+	require.Contains(t, keys, `C:\repo\.codex\hooks.json:session_start:0:0`)
+}
+
+// TestReadCodexTrustedKeys_WindowsBasicEscapedKeys — the latent second
+// bug from issue #1761: a basic (double-quoted) key holding a Windows
+// path carries escaped backslashes in the raw file. The parser must
+// unescape them so the key compares equal to filepath.Join output. The
+// old regex captured the raw `C:\\repo\\...` text and could never
+// match.
+func TestReadCodexTrustedKeys_WindowsBasicEscapedKeys(t *testing.T) {
+	t.Parallel()
+	configPath := filepath.Join(t.TempDir(), "config.toml")
+	configTOML := `[hooks.state."C:\\repo\\.codex\\hooks.json:stop:0:0"]
+trusted_hash = "sha256:bbb"
+`
+	require.NoError(t, os.WriteFile(configPath, []byte(configTOML), 0o600))
+
+	keys, ok := readCodexTrustedKeys(configPath)
+	require.True(t, ok)
+	require.Contains(t, keys, `C:\repo\.codex\hooks.json:stop:0:0`)
+}
+
+// TestReadCodexTrustedKeys_DottedAssignmentForm — trust entries written
+// as dotted-key assignments under a [hooks.state] table instead of one
+// header per entry are the same data in valid TOML; structural parsing
+// handles the shape the old header regex could never see.
+func TestReadCodexTrustedKeys_DottedAssignmentForm(t *testing.T) {
+	t.Parallel()
+	configPath := filepath.Join(t.TempDir(), "config.toml")
+	configTOML := `[hooks.state]
+'C:\repo\.codex\hooks.json:post_tool_use:0:0'.trusted_hash = "sha256:ccc"
+`
+	require.NoError(t, os.WriteFile(configPath, []byte(configTOML), 0o600))
+
+	keys, ok := readCodexTrustedKeys(configPath)
+	require.True(t, ok)
+	require.Contains(t, keys, `C:\repo\.codex\hooks.json:post_tool_use:0:0`)
+}
+
+// TestReadCodexTrustedKeys_UnrelatedSectionsIgnored — keys elsewhere in
+// the config (top-level settings, other tables) must not leak into the
+// trusted set.
+func TestReadCodexTrustedKeys_UnrelatedSectionsIgnored(t *testing.T) {
+	t.Parallel()
+	configPath := filepath.Join(t.TempDir(), "config.toml")
+	configTOML := `model = "gpt-5"
+
+[projects."/some/repo"]
+trust_level = "trusted"
+
+[hooks.state."/repo/.codex/hooks.json:stop:0:0"]
+trusted_hash = "sha256:ddd"
+`
+	require.NoError(t, os.WriteFile(configPath, []byte(configTOML), 0o600))
+
+	keys, ok := readCodexTrustedKeys(configPath)
+	require.True(t, ok)
+	require.Len(t, keys, 1)
+	require.Contains(t, keys, "/repo/.codex/hooks.json:stop:0:0")
 }

cmd/entire/cli/doctor.go

@@ -448,10 +448,17 @@ func checkCodexHookTrust(cmd *cobra.Command) {
 
 	w := cmd.OutOrStdout()
 	missing := codex.MissingEntireHooks(repoRoot)
-	gaps := codex.HookTrustGaps(repoRoot)
+	gaps, checked := codex.HookTrustGaps(repoRoot)
 
 	if len(missing) == 0 && len(gaps) == 0 {
-		fmt.Fprintln(w, "✓ Codex hook trust: OK")
+		// checked=false is "can't tell" (unreadable/unparseable hooks.json
+		// or config.toml), which must not render as a verified OK.
+		if checked {
+			fmt.Fprintln(w, "✓ Codex hook trust: OK")
+		} else {
+			fmt.Fprintln(w, "Codex hook trust: NOT VERIFIED")
+			fmt.Fprintln(w, "  Could not read hook approvals from Codex's config.toml.")
+		}
 		return
 	}