diff --git a/README.md b/README.md index 8c12988..51f6ff2 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,6 @@ # hookshot -A Go library for building hooks for AI coding agents like [Cursor](https://cursor.com/docs/agent/hooks), [Claude Code](https://docs.claude.com/en/docs/claude-code/hooks), [Windsurf Cascade](https://docs.codeium.com/windsurf/memories#hooks), and [Factory Droid](https://docs.factory.ai/reference/hooks-reference). +A Go library for building hooks for AI coding agents like [Cursor](https://cursor.com/docs/agent/hooks), [Claude Code](https://docs.claude.com/en/docs/claude-code/hooks), [Windsurf Cascade](https://docs.codeium.com/windsurf/memories#hooks), [Factory Droid](https://docs.factory.ai/reference/hooks-reference), and [OpenAI Codex](https://developers.openai.com/codex/hooks). Hooks are a key component of [Agentic Coding Security Management (ACSM)](https://corridor.dev/blog/introducing-acsm/) — they let you observe, control, and secure AI agent behavior in your development environment. @@ -97,16 +97,33 @@ hookshot install --binary /path/to/my-hooks } ``` +### OpenAI Codex (`~/.codex/hooks.json`) + +Codex hooks are enabled by default (the `hooks` feature flag in Codex is +stable and on). No `~/.codex/config.toml` change is required. If your +organization disabled hooks, set `[features].hooks = true` to turn them +back on. + +```json +{ + "hooks": { + "Stop": [{ "hooks": [{ "type": "command", "command": "/path/to/my-hooks codex-stop" }] }], + "PreToolUse": [{ "matcher": "Bash|apply_patch|mcp__.*", "hooks": [{ "type": "command", "command": "/path/to/my-hooks codex-pre-tool-use" }] }], + "PostToolUse": [{ "matcher": "Bash|apply_patch|mcp__.*", "hooks": [{ "type": "command", "command": "/path/to/my-hooks codex-post-tool-use" }] }] + } +} +``` + ## Unified Handlers -Write once, run on all four platforms: +Write once, run on all five platforms: -| Handler | Claude Code | Cursor | Windsurf Cascade | Factory Droid | -|---------|-------------|--------|------------------|---------------| -| `OnStop` | Stop | stop | post-cascade-response | Stop | -| `OnBeforeExecution` | PreToolUse | beforeShellExecution, beforeMCPExecution | pre-run-command, pre-mcp-tool-use | PreToolUse | -| `OnAfterFileEdit` | PostToolUse | afterFileEdit | post-write-code | PostToolUse | -| `OnPromptSubmit` | UserPromptSubmit | beforeSubmitPrompt | pre-user-prompt | UserPromptSubmit | +| Handler | Claude Code | Cursor | Windsurf Cascade | Factory Droid | OpenAI Codex | +|---------|-------------|--------|------------------|---------------|--------------| +| `OnStop` | Stop | stop | post-cascade-response | Stop | Stop | +| `OnBeforeExecution` | PreToolUse | beforeShellExecution, beforeMCPExecution | pre-run-command, pre-mcp-tool-use | PreToolUse | PreToolUse | +| `OnAfterFileEdit` | PostToolUse | afterFileEdit | post-write-code | PostToolUse | PostToolUse | +| `OnPromptSubmit` | UserPromptSubmit | beforeSubmitPrompt | pre-user-prompt | UserPromptSubmit | UserPromptSubmit | ## Platform-Specific Handlers @@ -140,6 +157,13 @@ hookshot.Register("droid-pre-tool-use", func() { return droid.PassThrough() }) }) + +// OpenAI Codex: Pre-tool use (matches Bash, apply_patch, and MCP tools) +hookshot.Register("codex-pre-tool-use", func() { + hookshot.Run(func(input codex.PreToolUseInput) codex.PreToolUseOutput { + return codex.PassThrough() + }) +}) ``` ## Documentation @@ -149,6 +173,7 @@ hookshot.Register("droid-pre-tool-use", func() { - [Cursor Reference](docs/reference-cursor.md) - [Windsurf Cascade Reference](docs/reference-cascade.md) - [Factory Droid Reference](docs/reference-droid.md) +- [OpenAI Codex Reference](docs/reference-codex.md) Full API documentation is available via godoc: @@ -158,6 +183,7 @@ go doc github.com/CorridorSecurity/hookshot/claude go doc github.com/CorridorSecurity/hookshot/cursor go doc github.com/CorridorSecurity/hookshot/cascade go doc github.com/CorridorSecurity/hookshot/droid +go doc github.com/CorridorSecurity/hookshot/codex ``` Or view online at [pkg.go.dev/github.com/CorridorSecurity/hookshot](https://pkg.go.dev/github.com/CorridorSecurity/hookshot). diff --git a/cmd/hookshot/main.go b/cmd/hookshot/main.go index 5696631..418a197 100644 --- a/cmd/hookshot/main.go +++ b/cmd/hookshot/main.go @@ -15,6 +15,7 @@ // // hookshot install --binary ./my-hooks // hookshot install --binary ./my-hooks --claude --cursor +// hookshot install --binary ./my-hooks --codex package main import ( @@ -56,7 +57,7 @@ Usage: Commands: build Build hooks binary for one or more platforms - install Install hooks to AI coding agent config files (Claude Code, Cursor, Droid, Cascade) + install Install hooks to AI coding agent config files (Claude Code, Cursor, Droid, Cascade, Codex) Run 'hookshot -h' for command-specific help.`) } @@ -199,11 +200,12 @@ func runInstall(args []string) { fs := flag.NewFlagSet("install", flag.ExitOnError) var ( - binaryPath string - claudeFlag bool - cursorFlag bool - droidFlag bool - cascadeFlag bool + binaryPath string + claudeFlag bool + cursorFlag bool + droidFlag bool + cascadeFlag bool + codexFlag bool ) fs.StringVar(&binaryPath, "binary", "", "Path to hooks binary (required)") @@ -211,6 +213,7 @@ func runInstall(args []string) { fs.BoolVar(&cursorFlag, "cursor", false, "Install to Cursor only") fs.BoolVar(&droidFlag, "droid", false, "Install to Factory Droid only") fs.BoolVar(&cascadeFlag, "cascade", false, "Install to Windsurf Cascade only") + fs.BoolVar(&codexFlag, "codex", false, "Install to OpenAI Codex only") fs.Usage = func() { fmt.Println(`Install hooks to AI coding agent config files. @@ -224,6 +227,7 @@ Examples: hookshot install --binary ./my-hooks --cursor # Cursor only hookshot install --binary ./my-hooks --droid # Factory Droid only hookshot install --binary ./my-hooks --cascade # Windsurf Cascade only + hookshot install --binary ./my-hooks --codex # OpenAI Codex only Flags:`) fs.PrintDefaults() @@ -251,11 +255,12 @@ Flags:`) } // If none specified, install to all - if !claudeFlag && !cursorFlag && !droidFlag && !cascadeFlag { + if !claudeFlag && !cursorFlag && !droidFlag && !cascadeFlag && !codexFlag { claudeFlag = true cursorFlag = true droidFlag = true cascadeFlag = true + codexFlag = true } if claudeFlag { @@ -286,11 +291,21 @@ Flags:`) } } + if codexFlag { + if err := installCodex(absPath); err != nil { + fmt.Fprintf(os.Stderr, "Error installing to OpenAI Codex: %v\n", err) + os.Exit(1) + } + } + fmt.Println("\nInstallation complete!") } func installClaude(binaryPath string) error { - homeDir, _ := os.UserHomeDir() + homeDir, err := os.UserHomeDir() + if err != nil { + return fmt.Errorf("resolving home directory: %w", err) + } configPath := filepath.Join(homeDir, ".claude", "settings.json") fmt.Printf("Installing to Claude Code (%s)...\n", configPath) @@ -355,7 +370,10 @@ func installClaude(binaryPath string) error { } func installCursor(binaryPath string) error { - homeDir, _ := os.UserHomeDir() + homeDir, err := os.UserHomeDir() + if err != nil { + return fmt.Errorf("resolving home directory: %w", err) + } configPath := filepath.Join(homeDir, ".cursor", "hooks.json") fmt.Printf("Installing to Cursor (%s)...\n", configPath) @@ -390,7 +408,10 @@ func installCursor(binaryPath string) error { } func installDroid(binaryPath string) error { - homeDir, _ := os.UserHomeDir() + homeDir, err := os.UserHomeDir() + if err != nil { + return fmt.Errorf("resolving home directory: %w", err) + } configPath := filepath.Join(homeDir, ".factory", "settings.json") fmt.Printf("Installing to Factory Droid (%s)...\n", configPath) @@ -454,8 +475,89 @@ func installDroid(binaryPath string) error { return nil } +func installCodex(binaryPath string) error { + homeDir, err := os.UserHomeDir() + if err != nil { + return fmt.Errorf("resolving home directory: %w", err) + } + configPath := filepath.Join(homeDir, ".codex", "hooks.json") + + fmt.Printf("Installing to OpenAI Codex (%s)...\n", configPath) + + // Read existing config or create new + var config map[string]any + data, err := os.ReadFile(configPath) + if err == nil { + json.Unmarshal(data, &config) + } + if config == nil { + config = make(map[string]any) + } + + // Codex hook config follows the same JSON shape as Claude Code's + // settings but lives in ~/.codex/hooks.json. Matchers include + // "mcp__.*" so MCP tool calls reach the hook binary. "apply_patch" + // alone covers Codex file edits — Codex emits "Edit" and "Write" as + // matcher aliases for apply_patch, so they're redundant here. + hooks := map[string]any{ + "Stop": []map[string]any{{ + "hooks": []map[string]any{{ + "type": "command", + "command": binaryPath + " codex-stop", + }}, + }}, + "PreToolUse": []map[string]any{{ + "matcher": "Bash|apply_patch|mcp__.*", + "hooks": []map[string]any{{ + "type": "command", + "command": binaryPath + " codex-pre-tool-use", + }}, + }}, + "PostToolUse": []map[string]any{{ + // Bash is required to catch the heredoc-style file edits + // (`apply_patch <<'PATCH' … PATCH`) and greenfield writes + // (`cat <<'EOF' > FILE … EOF`) Codex 0.130.0+ routes + // through plain Bash in addition to the apply_patch tool. The + // unified codex-post-tool-use bridge parses both shapes via + // codex.ParseApplyPatchFromBash / codex.ParseBashRedirectWrite + // — but only sees the events if the matcher itself lets + // them through. + "matcher": "Bash|apply_patch|mcp__.*", + "hooks": []map[string]any{{ + "type": "command", + "command": binaryPath + " codex-post-tool-use", + }}, + }}, + "UserPromptSubmit": []map[string]any{{ + "hooks": []map[string]any{{ + "type": "command", + "command": binaryPath + " codex-user-prompt-submit", + }}, + }}, + } + + config["hooks"] = hooks + + os.MkdirAll(filepath.Dir(configPath), 0755) + + output, err := json.MarshalIndent(config, "", " ") + if err != nil { + return err + } + + if err := os.WriteFile(configPath, output, 0644); err != nil { + return err + } + + fmt.Println(" Installed hooks: Stop, PreToolUse, PostToolUse, UserPromptSubmit") + return nil +} + func installCascade(binaryPath string) error { - homeDir, _ := os.UserHomeDir() + homeDir, err := os.UserHomeDir() + if err != nil { + return fmt.Errorf("resolving home directory: %w", err) + } configPath := filepath.Join(homeDir, ".codeium", "windsurf", "hooks.json") fmt.Printf("Installing to Windsurf Cascade (%s)...\n", configPath) @@ -463,11 +565,11 @@ func installCascade(binaryPath string) error { // Build hooks config config := map[string]any{ "hooks": map[string]any{ - "pre_run_command": []map[string]any{{"command": binaryPath + " cascade-pre-run-command"}}, - "pre_mcp_tool_use": []map[string]any{{"command": binaryPath + " cascade-pre-mcp-tool-use"}}, - "pre_user_prompt": []map[string]any{{"command": binaryPath + " cascade-pre-user-prompt"}}, - "post_write_code": []map[string]any{{"command": binaryPath + " cascade-post-write-code"}}, - "post_cascade_response": []map[string]any{{"command": binaryPath + " cascade-post-cascade-response"}}, + "pre_run_command": []map[string]any{{"command": binaryPath + " cascade-pre-run-command"}}, + "pre_mcp_tool_use": []map[string]any{{"command": binaryPath + " cascade-pre-mcp-tool-use"}}, + "pre_user_prompt": []map[string]any{{"command": binaryPath + " cascade-pre-user-prompt"}}, + "post_write_code": []map[string]any{{"command": binaryPath + " cascade-post-write-code"}}, + "post_cascade_response": []map[string]any{{"command": binaryPath + " cascade-post-cascade-response"}}, }, } diff --git a/codex/apply_patch.go b/codex/apply_patch.go new file mode 100644 index 0000000..cf9b776 --- /dev/null +++ b/codex/apply_patch.go @@ -0,0 +1,323 @@ +package codex + +import ( + "strings" + + "mvdan.cc/sh/v3/syntax" +) + +// PatchEdit captures one removed/added pair inside an apply_patch hunk. +// +// For an "add" operation the patch contains exactly one PatchEdit with +// OldString == "" and NewString set to the added contents. For an "update" +// operation each hunk in the patch becomes one PatchEdit with the removed +// lines joined as OldString and the added lines joined as NewString. For +// "delete" the Edits slice is empty (the FilePath alone is enough to drive +// policy). +type PatchEdit struct { + OldString string + NewString string +} + +// PatchFile represents one file affected by an apply_patch tool call. +// +// Codex's apply_patch carries a unified-diff-style envelope under +// tool_input.command (see https://developers.openai.com/codex/hooks). A +// single envelope may touch multiple files, which is why ParseApplyPatch +// returns a slice rather than a single FilePath/Edits pair. +type PatchFile struct { + // Operation is one of "add", "update", or "delete". + Operation string + // FilePath is the path declared in the *** {Add,Update,Delete} File: + // header. + FilePath string + // NewFilePath is set for "update" operations that include a + // "*** Move to:" line. Empty for in-place edits. + NewFilePath string + // Edits captures per-hunk old/new content. See PatchEdit for shape. + Edits []PatchEdit +} + +// ParseApplyPatch parses the unified-diff envelope from a Codex apply_patch +// tool call. The input is the raw value of tool_input.command for an +// apply_patch invocation. +// +// This is a convenience helper for handlers that want structured access to +// the patch (per-file paths, rename detection, per-hunk content). It is NOT +// invoked by the unified OnAfterFileEdit bridge — that path passes the raw +// patch text through unchanged so simple path-based policies cannot be +// silently bypassed by a parser desync against Codex's actual format. Call +// this helper only if you need the parsed view. +// +// The parser is intentionally tolerant: malformed input degrades +// gracefully (an affected file is still recorded even if its Edits list is +// incomplete) rather than returning an error. +func ParseApplyPatch(patch string) []PatchFile { + // Strip a leading "apply_patch <<'EOF'\n" wrapper if present. Some + // Codex invocations include a here-doc style wrapper around the + // "*** Begin Patch" block; this is a defensive measure and not part + // of the documented wire format. + if i := strings.Index(patch, "*** Begin Patch"); i > 0 { + patch = patch[i:] + } + + lines := strings.Split(patch, "\n") + var files []PatchFile + var current *PatchFile + var hunkOld, hunkNew []string + inHunk := false + + flushHunk := func() { + if current == nil { + return + } + if len(hunkOld) == 0 && len(hunkNew) == 0 { + hunkOld, hunkNew = nil, nil + return + } + current.Edits = append(current.Edits, PatchEdit{ + OldString: strings.Join(hunkOld, "\n"), + NewString: strings.Join(hunkNew, "\n"), + }) + hunkOld, hunkNew = nil, nil + } + + flushFile := func() { + flushHunk() + if current != nil { + files = append(files, *current) + } + current = nil + inHunk = false + } + + for _, line := range lines { + switch { + case strings.HasPrefix(line, "*** Begin Patch"), + strings.HasPrefix(line, "*** End of File"): + case strings.HasPrefix(line, "*** End Patch"): + flushFile() + case strings.HasPrefix(line, "*** Add File: "): + flushFile() + current = &PatchFile{ + Operation: "add", + FilePath: strings.TrimPrefix(line, "*** Add File: "), + } + inHunk = true + case strings.HasPrefix(line, "*** Update File: "): + flushFile() + current = &PatchFile{ + Operation: "update", + FilePath: strings.TrimPrefix(line, "*** Update File: "), + } + inHunk = false + case strings.HasPrefix(line, "*** Delete File: "): + flushFile() + current = &PatchFile{ + Operation: "delete", + FilePath: strings.TrimPrefix(line, "*** Delete File: "), + } + inHunk = false + case strings.HasPrefix(line, "*** Move to: "): + if current != nil { + current.NewFilePath = strings.TrimPrefix(line, "*** Move to: ") + } + case strings.HasPrefix(line, "@@"): + flushHunk() + inHunk = true + case current != nil: + switch current.Operation { + case "add": + if strings.HasPrefix(line, "+") { + hunkNew = append(hunkNew, line[1:]) + } else if line != "" { + hunkNew = append(hunkNew, line) + } + case "update": + if !inHunk { + continue + } + if len(line) == 0 { + flushHunk() + continue + } + switch line[0] { + case '+': + hunkNew = append(hunkNew, line[1:]) + case '-': + hunkOld = append(hunkOld, line[1:]) + case ' ': + flushHunk() + } + case "delete": + } + } + } + flushFile() + return files +} + +// ParseApplyPatchFromBash inspects a Codex Bash tool command and, if it +// invokes apply_patch with a heredoc body, returns the parsed patch +// files. The second return value reports whether at least one +// apply_patch heredoc invocation was found, so callers can short-circuit +// on plain Bash commands without parsing the patch envelope. +// +// Codex routes file edits through Bash heredocs of the form +// +// apply_patch <<'PATCH' +// *** Begin Patch +// ... unified diff envelope ... +// *** End Patch +// PATCH +// +// at least as often as it emits a first-class tool_name="apply_patch" call. +// Codex may also invoke a per-session shim binary, so the apply_patch token +// can appear with an absolute path prefix, e.g. +// `/Users/me/.codex/tmp/arg0/codex-arg0IuQk4E/apply_patch <<'PATCH' ...`. +// Without this helper, callers that only inspect tool_name silently miss +// every heredoc-style edit (which is why Codex sessions showed up in the +// dashboard with zero SecurityScanResult rows even though file edits had +// clearly happened). +// +// Implementation note: detection runs the command through a real Bash +// parser (mvdan.cc/sh/v3/syntax) and walks the AST looking for a +// CallExpr whose first Word is the literal `apply_patch` (or +// `*/apply_patch` for the per-session shim) paired with a heredoc +// redirect (`</dev/null; apply_patch <<'PATCH' ...`) +// would anchor `strings.Index` on the decoy, leaving the regex with a +// prefix that didn't contain the real `apply_patch <<` invocation — +// detection silently returned (nil, false) and the post-tool handler +// skipped dispatch entirely. +// 2. The regex required at least one space between `apply_patch` and +// `<<`, so the zero-space `apply_patch<<'PATCH'` (valid Bash) also +// evaded detection. +// +// A real parser closes both bypasses and also rules out the legacy +// false-positive case (an `apply_patch` token appearing inside a +// docs-file heredoc body), because the parser identifies the +// surrounding command (`cat > docs/apply_patch_format.md <<'EOF'`) as +// the invocation, not the body content. +// +// If a command contains multiple apply_patch heredoc invocations (in +// pipelines, `;` chains, subshells, conditionals, etc.) every body is +// parsed and the resulting PatchFile slices are concatenated. If the +// command is not valid Bash (parser error) this returns (nil, false) — +// silently treating malformed input as an edit would be unsafe. +func ParseApplyPatchFromBash(command string) ([]PatchFile, bool) { + file, err := syntax.NewParser().Parse(strings.NewReader(command), "") + if err != nil { + return nil, false + } + + var bodies []heredocBody + syntax.Walk(file, func(node syntax.Node) bool { + stmt, ok := node.(*syntax.Stmt) + if !ok { + return true + } + if !stmtInvokesApplyPatch(stmt) { + return true + } + for _, r := range stmt.Redirs { + body, ok := heredocBodyFromRedirect(command, r) + if ok { + bodies = append(bodies, body) + } + } + return true + }) + + if len(bodies) == 0 { + return nil, false + } + + var all []PatchFile + for _, b := range bodies { + all = append(all, ParseApplyPatch(b.text)...) + } + return all, true +} + +// stmtInvokesApplyPatch reports whether stmt's primary command is the +// apply_patch executable. Accepts the bare `apply_patch` token and the +// `*/apply_patch` absolute-path form Codex uses for its per-session +// shim binary. Any other shape (function definitions, command +// substitutions, builtins, etc.) returns false. +func stmtInvokesApplyPatch(stmt *syntax.Stmt) bool { + call, ok := stmt.Cmd.(*syntax.CallExpr) + if !ok || len(call.Args) == 0 { + return false + } + name := call.Args[0].Lit() + if name == "apply_patch" { + return true + } + if strings.HasSuffix(name, "/apply_patch") { + return true + } + return false +} + +// heredocBody is the extracted body of one here-doc redirect, plus a +// hint about whether tab-stripping (POSIX `<<-` semantics) was applied. +// We carry this rather than returning a raw string so test failures can +// report which variant a particular body came from. +type heredocBody struct { + text string + stripIndent bool +} + +// heredocBodyFromRedirect returns the body text for a here-doc redirect +// (`<<` or `<<-`), sliced from the original command using positional +// offsets so we preserve the bytes exactly as Codex emitted them. For +// `<<-` (DashHdoc) we strip leading TAB characters from each line, per +// POSIX, so the body handed to ParseApplyPatch matches what Bash would +// have piped to apply_patch's stdin. +// +// The Hdoc Word's End() points just past the trailing delimiter token +// (e.g. `PATCH`), so the returned body includes that trailing line. +// ParseApplyPatch tolerates the extra line (it stops dispatching state +// at `*** End Patch`) so we don't try to trim it here — keeping the +// slice byte-exact makes the helper easier to reason about than one +// that does ad-hoc post-processing. +func heredocBodyFromRedirect(command string, r *syntax.Redirect) (heredocBody, bool) { + if r.Op != syntax.Hdoc && r.Op != syntax.DashHdoc { + return heredocBody{}, false + } + if r.Hdoc == nil { + return heredocBody{}, false + } + start := int(r.Hdoc.Pos().Offset()) + end := int(r.Hdoc.End().Offset()) + if start < 0 || end > len(command) || start >= end { + return heredocBody{}, false + } + body := command[start:end] + stripIndent := r.Op == syntax.DashHdoc + if stripIndent { + body = stripLeadingTabsPerLine(body) + } + return heredocBody{text: body, stripIndent: stripIndent}, true +} + +// stripLeadingTabsPerLine removes leading TAB characters (not spaces) +// from each newline-delimited line of body. Mirrors Bash's `<<-DELIM` +// semantics so ParseApplyPatch can match line prefixes like +// `*** Begin Patch` even when Codex emits the patch under the +// strip-tabs heredoc operator. +func stripLeadingTabsPerLine(body string) string { + if !strings.ContainsRune(body, '\t') { + return body + } + lines := strings.Split(body, "\n") + for i, line := range lines { + lines[i] = strings.TrimLeft(line, "\t") + } + return strings.Join(lines, "\n") +} diff --git a/codex/apply_patch_test.go b/codex/apply_patch_test.go new file mode 100644 index 0000000..a910763 --- /dev/null +++ b/codex/apply_patch_test.go @@ -0,0 +1,632 @@ +package codex + +import ( + "reflect" + "testing" +) + +func TestParseApplyPatch_AddFile(t *testing.T) { + patch := "*** Begin Patch\n" + + "*** Add File: secrets/api_key.txt\n" + + "+sk-deadbeef\n" + + "+second line\n" + + "*** End Patch\n" + + got := ParseApplyPatch(patch) + want := []PatchFile{{ + Operation: "add", + FilePath: "secrets/api_key.txt", + Edits: []PatchEdit{{ + OldString: "", + NewString: "sk-deadbeef\nsecond line", + }}, + }} + if !reflect.DeepEqual(got, want) { + t.Errorf("ParseApplyPatch(add) =\n %+v\nwant\n %+v", got, want) + } +} + +func TestParseApplyPatch_DeleteFile(t *testing.T) { + patch := "*** Begin Patch\n" + + "*** Delete File: old/secrets.env\n" + + "*** End Patch\n" + + got := ParseApplyPatch(patch) + if len(got) != 1 { + t.Fatalf("got %d files, want 1", len(got)) + } + if got[0].Operation != "delete" { + t.Errorf("Operation = %q, want %q", got[0].Operation, "delete") + } + if got[0].FilePath != "old/secrets.env" { + t.Errorf("FilePath = %q, want %q", got[0].FilePath, "old/secrets.env") + } + if len(got[0].Edits) != 0 { + t.Errorf("Edits = %+v, want empty", got[0].Edits) + } +} + +func TestParseApplyPatch_UpdateFile(t *testing.T) { + patch := "*** Begin Patch\n" + + "*** Update File: src/auth.go\n" + + "@@ func login\n" + + " func login() {\n" + + "- token := \"hardcoded\"\n" + + "+ token := os.Getenv(\"TOKEN\")\n" + + " return token\n" + + " }\n" + + "*** End Patch\n" + + got := ParseApplyPatch(patch) + if len(got) != 1 { + t.Fatalf("got %d files, want 1", len(got)) + } + if got[0].FilePath != "src/auth.go" { + t.Errorf("FilePath = %q, want %q", got[0].FilePath, "src/auth.go") + } + if got[0].Operation != "update" { + t.Errorf("Operation = %q, want %q", got[0].Operation, "update") + } + if len(got[0].Edits) != 1 { + t.Fatalf("Edits = %+v, want 1 edit", got[0].Edits) + } + wantEdit := PatchEdit{ + OldString: " token := \"hardcoded\"", + NewString: " token := os.Getenv(\"TOKEN\")", + } + if got[0].Edits[0] != wantEdit { + t.Errorf("Edits[0] = %+v, want %+v", got[0].Edits[0], wantEdit) + } +} + +func TestParseApplyPatch_UpdateFile_MultipleHunks(t *testing.T) { + patch := "*** Begin Patch\n" + + "*** Update File: a.go\n" + + "@@\n" + + "-foo\n" + + "+bar\n" + + "@@\n" + + "-baz\n" + + "+qux\n" + + "*** End Patch\n" + + got := ParseApplyPatch(patch) + if len(got) != 1 { + t.Fatalf("got %d files, want 1", len(got)) + } + if len(got[0].Edits) != 2 { + t.Fatalf("Edits = %+v, want 2 edits", got[0].Edits) + } + if got[0].Edits[0] != (PatchEdit{OldString: "foo", NewString: "bar"}) { + t.Errorf("Edits[0] = %+v", got[0].Edits[0]) + } + if got[0].Edits[1] != (PatchEdit{OldString: "baz", NewString: "qux"}) { + t.Errorf("Edits[1] = %+v", got[0].Edits[1]) + } +} + +func TestParseApplyPatch_MultiFile(t *testing.T) { + patch := "*** Begin Patch\n" + + "*** Add File: new.txt\n" + + "+hello\n" + + "*** Update File: existing.go\n" + + "@@\n" + + "-old\n" + + "+new\n" + + "*** Delete File: stale.txt\n" + + "*** End Patch\n" + + got := ParseApplyPatch(patch) + if len(got) != 3 { + t.Fatalf("got %d files, want 3", len(got)) + } + if got[0].FilePath != "new.txt" || got[0].Operation != "add" { + t.Errorf("files[0] = %+v", got[0]) + } + if got[1].FilePath != "existing.go" || got[1].Operation != "update" { + t.Errorf("files[1] = %+v", got[1]) + } + if got[2].FilePath != "stale.txt" || got[2].Operation != "delete" { + t.Errorf("files[2] = %+v", got[2]) + } +} + +func TestParseApplyPatch_MoveTo(t *testing.T) { + patch := "*** Begin Patch\n" + + "*** Update File: old/path.go\n" + + "*** Move to: new/path.go\n" + + "@@\n" + + "-foo\n" + + "+bar\n" + + "*** End Patch\n" + + got := ParseApplyPatch(patch) + if len(got) != 1 { + t.Fatalf("got %d files, want 1", len(got)) + } + if got[0].NewFilePath != "new/path.go" { + t.Errorf("NewFilePath = %q, want %q", got[0].NewFilePath, "new/path.go") + } + if got[0].FilePath != "old/path.go" { + t.Errorf("FilePath = %q, want %q", got[0].FilePath, "old/path.go") + } +} + +func TestParseApplyPatch_LeadingWrapper(t *testing.T) { + patch := "apply_patch <<'PATCH'\n" + + "*** Begin Patch\n" + + "*** Add File: foo.txt\n" + + "+hi\n" + + "*** End Patch\n" + + "PATCH\n" + + got := ParseApplyPatch(patch) + if len(got) != 1 { + t.Fatalf("got %d files, want 1", len(got)) + } + if got[0].FilePath != "foo.txt" { + t.Errorf("FilePath = %q, want %q", got[0].FilePath, "foo.txt") + } +} + +func TestParseApplyPatch_EmptyAndMalformed(t *testing.T) { + if files := ParseApplyPatch(""); files != nil { + t.Errorf("empty input should yield nil, got %+v", files) + } + if files := ParseApplyPatch("not a patch"); files != nil { + t.Errorf("non-patch input should yield nil, got %+v", files) + } +} + +// ParseApplyPatchFromBash tests +// --------------------------------------------------------------------------- +// These cover the path used by the unified codex-post-tool-use bridge when +// tool_name is "Bash" rather than "apply_patch". The patches below are +// real-shape examples copied from Codex hook payloads observed in the +// dashboard (see hook_events.data for tool_name="Bash" rows). + +func TestParseApplyPatchFromBash_HeredocInvocation(t *testing.T) { + cmd := "apply_patch <<'PATCH'\n" + + "*** Begin Patch\n" + + "*** Add File: foo.txt\n" + + "+hello\n" + + "*** End Patch\n" + + "PATCH" + + files, ok := ParseApplyPatchFromBash(cmd) + if !ok { + t.Fatal("ParseApplyPatchFromBash(heredoc) returned ok=false, want true") + } + if len(files) != 1 || files[0].FilePath != "foo.txt" { + t.Errorf("files = %+v, want one file with FilePath=foo.txt", files) + } +} + +func TestParseApplyPatchFromBash_AbsolutePathBinary(t *testing.T) { + // Codex sometimes invokes a per-session apply_patch shim by absolute + // path. This was observed in the dashboard for session + // 019dc16e-dd6a-70c3-9e09-2dedd6bab556. Detection must still fire. + cmd := "/Users/me/.codex/tmp/arg0/codex-arg0IuQk4E/apply_patch <<'PATCH'\n" + + "*** Begin Patch\n" + + "*** Update File: app/routes.py\n" + + "@@\n" + + "-old\n" + + "+new\n" + + "*** End Patch\n" + + "PATCH" + + files, ok := ParseApplyPatchFromBash(cmd) + if !ok { + t.Fatal("ParseApplyPatchFromBash(absolute path) returned ok=false, want true") + } + if len(files) != 1 || files[0].FilePath != "app/routes.py" { + t.Errorf("files = %+v, want one file with FilePath=app/routes.py", files) + } + if len(files[0].Edits) != 1 || files[0].Edits[0].OldString != "old" || files[0].Edits[0].NewString != "new" { + t.Errorf("edits = %+v, want [{old,new}]", files[0].Edits) + } +} + +func TestParseApplyPatchFromBash_MultiFile(t *testing.T) { + cmd := "apply_patch <<'PATCH'\n" + + "*** Begin Patch\n" + + "*** Add File: a.txt\n" + + "+a\n" + + "*** Delete File: b.txt\n" + + "*** End Patch\n" + + "PATCH" + + files, ok := ParseApplyPatchFromBash(cmd) + if !ok { + t.Fatal("ParseApplyPatchFromBash returned ok=false, want true") + } + if len(files) != 2 { + t.Fatalf("len(files) = %d, want 2", len(files)) + } + if files[0].Operation != "add" || files[0].FilePath != "a.txt" { + t.Errorf("files[0] = %+v, want add a.txt", files[0]) + } + if files[1].Operation != "delete" || files[1].FilePath != "b.txt" { + t.Errorf("files[1] = %+v, want delete b.txt", files[1]) + } +} + +func TestParseApplyPatchFromBash_PlainBashCommandSkipped(t *testing.T) { + // A garden-variety Bash command that has nothing to do with editing + // files. Detection must return ok=false so the unified handler can + // short-circuit without parsing. + if _, ok := ParseApplyPatchFromBash("ls -la /tmp"); ok { + t.Error("ls -la /tmp wrongly detected as apply_patch") + } + if _, ok := ParseApplyPatchFromBash("git status"); ok { + t.Error("git status wrongly detected as apply_patch") + } + if _, ok := ParseApplyPatchFromBash(""); ok { + t.Error("empty command wrongly detected as apply_patch") + } +} + +func TestParseApplyPatchFromBash_PatchTextInsideUnrelatedHeredocSkipped(t *testing.T) { + // A Bash heredoc that writes a documentation file containing the + // literal "*** Begin Patch" marker — but with NO apply_patch + // invocation before the marker. This must not be classified as a + // file edit; otherwise every docs PR about apply_patch would trigger + // spurious afterFileEdit telemetry. + cmd := "cat > docs/apply_patch_format.md <<'EOF'\n" + + "This documents the apply_patch format. Example envelope:\n" + + "\n" + + "*** Begin Patch\n" + + "*** Add File: example.txt\n" + + "+hi\n" + + "*** End Patch\n" + + "EOF" + + if _, ok := ParseApplyPatchFromBash(cmd); ok { + t.Error("docs heredoc wrongly detected as apply_patch invocation") + } +} + +func TestParseApplyPatchFromBash_ApplyPatchMentionAfterEnvelopeSkipped(t *testing.T) { + // Edge case: "apply_patch" appears in the file content (inside the + // patch body) but not before the envelope. Detection requires the + // token to be BEFORE *** Begin Patch. + cmd := "cat > note.txt <<'EOF'\n" + + "*** Begin Patch (literal text describing apply_patch syntax)\n" + + "EOF" + + if _, ok := ParseApplyPatchFromBash(cmd); ok { + t.Error("apply_patch mention after envelope wrongly detected") + } +} + +func TestParseApplyPatchFromBash_HeredocVariants(t *testing.T) { + // Heredoc operator variants: < FILE` invocations rather than the +// first-class `apply_patch` tool or any `Write`/`Edit` alias. Without +// this parser the corridor `codex-post-tool-use` handler short-circuits +// on every greenfield write — no afterFileEdit fires, no security scan +// runs, and the dashboard shows zero SecurityScanResult rows for any +// Codex session that only creates new files (the exact symptom that +// motivated this helper; see the regression tests below). +// +// Implementation note: detection runs the command through a real Bash +// parser (mvdan.cc/sh/v3/syntax) and walks the AST looking for +// CallExpr statements whose primary command is `cat` or `tee` paired +// with a heredoc redirect (`<<` or `<<-`, quoted or unquoted) on the +// same Stmt. This replaced an earlier regex-based implementation that +// reported only the first match in a command — a multi-heredoc shape +// like +// +// cat <<'EOF' > allowed.txt +// ok +// EOF +// cat <<'EOF' > .env +// TOKEN=secret +// EOF +// +// dispatched only the allowed.txt write to OnAfterFileEdit, silently +// bypassing path-deny rules and secret scanners for the .env write. A +// real parser also closes a handful of adjacent gaps the regex never +// covered: redirects in reverse order (`cat > FILE < "name with spaces.txt"`), writes nested inside compound +// commands (`cd … && cat …`, `if … then cat … fi`, pipelines), +// commands that mix `cat` and `tee` heredoc writes, multi-target tee +// (`tee a.txt b.txt < .env 2> log` correctly +// identifies .env as the write target rather than letting the later 2> +// entry overwrite our pick). Every recognised write becomes a +// PatchFile in the order it appears in the command. +// +// We deliberately fail closed in three places. (1) If the command is +// not valid Bash (parser error) this returns (nil, false) — silently +// treating malformed input as a no-op write would let an attacker +// construct a heredoc the parser rejects but Codex's actual shell +// accepts. (2) If a cat statement has a heredoc but no extractable +// target path (e.g. `cat < $(some_cmd)`), that single write is +// dropped from the result but the rest of the command's writes are +// still reported, and ok stays true as long as at least one well-formed +// write was found. (3) If a tee statement has a heredoc but ANY of its +// target paths can't be cleanly extracted, the entire statement is +// dropped — partial coverage of a multi-target tee is the exact +// confused-deputy bypass we're trying to prevent, so we'd rather hand +// the raw command to the unified bridge's fallback than report a +// subset of writes. Callers that need a stricter posture should layer +// their own checks; the unified bridge then surfaces the raw command +// through the existing fallback path. +func ParseBashRedirectWrite(command string) ([]PatchFile, bool) { + // Cheap pre-check: every heredoc-style write contains `<<`. This + // keeps the parser allocation off the hot path for plain Bash + // commands (`pwd`, `ls`, `git diff`, …) which dominate real Codex + // traffic. + if !strings.Contains(command, "<<") { + return nil, false + } + + file, err := syntax.NewParser().Parse(strings.NewReader(command), "") + if err != nil { + return nil, false + } + + var files []PatchFile + syntax.Walk(file, func(node syntax.Node) bool { + stmt, ok := node.(*syntax.Stmt) + if !ok { + return true + } + if patches, ok := bashRedirectWriteFromStmt(command, stmt); ok { + files = append(files, patches...) + } + return true + }) + if len(files) == 0 { + return nil, false + } + return files, true +} + +// bashRedirectWriteFromStmt extracts one PatchFile per write target from +// stmt if its primary command is a recognised heredoc-style file write +// (cat or tee). Most statements yield a single PatchFile, but `tee +// FILE1 FILE2 …` produces one PatchFile per target so every write goes +// through the per-file policy pipeline. Any other shape returns +// ok=false so syntax.Walk can quietly skip it. +func bashRedirectWriteFromStmt(command string, stmt *syntax.Stmt) ([]PatchFile, bool) { + call, ok := stmt.Cmd.(*syntax.CallExpr) + if !ok || len(call.Args) == 0 { + return nil, false + } + switch call.Args[0].Lit() { + case "cat": + return catRedirectWrite(command, stmt) + case "tee": + return teeRedirectWrite(command, stmt, call) + } + return nil, false +} + +// catRedirectWrite handles `cat < FILE` and `cat > FILE <`/`>>` target as separate +// entries on stmt.Redirs; we pick the last `>`/`>>` on fd 1 (Bash +// semantics — a later redirect on the same fd shadows an earlier one) +// and require exactly one heredoc on the statement. Two heredocs on a +// single cat is invalid Bash and never emitted by Codex; we bail +// rather than guess which one feeds the file. +// +// Crucially we filter out redirects on file descriptors other than +// stdout (fd 1). mvdan.cc/sh's parser represents `2> stderr.log` as +// {Op: RdrOut, N: lit("2"), …} — the same RedirOperator as `> file`, +// distinguished only by the fd field. Without this filter, a command +// like `cat < .env 2> allowed.log` would have `out` overwritten +// by the (later) stderr entry and report `allowed.log` to +// OnAfterFileEdit — leaving the actual .env write completely +// unmonitored. `&>` (RdrAll) and `&>>` (AppAll) combine stdout+stderr +// to one file; that file IS a real write target, so we treat them +// like stdout-bound RdrOut/AppOut. +func catRedirectWrite(command string, stmt *syntax.Stmt) ([]PatchFile, bool) { + var hdoc, out *syntax.Redirect + for _, r := range stmt.Redirs { + switch r.Op { + case syntax.Hdoc, syntax.DashHdoc: + if hdoc != nil { + return nil, false + } + hdoc = r + case syntax.RdrOut, syntax.AppOut: + if !redirectsStdout(r) { + continue + } + out = r + case syntax.RdrAll, syntax.AppAll: + out = r + } + } + if hdoc == nil || out == nil { + return nil, false + } + path := wordText(command, out.Word) + if path == "" { + return nil, false + } + body, ok := heredocBodyContent(command, hdoc) + if !ok { + return nil, false + } + return []PatchFile{{ + Operation: "add", + FilePath: path, + Edits: []PatchEdit{{ + OldString: "", + NewString: body, + }}, + }}, true +} + +// teeRedirectWrite handles `tee [-a] FILE… [< /dev/null] <` → +// N.Value == "2"); a nil/empty N means the default, which for `>` and +// `>>` is fd 1. Anything other than nil/empty/"1" is some other fd +// (stderr, a custom log fd, …) and must NOT be treated as the +// command's primary write target. +func redirectsStdout(r *syntax.Redirect) bool { + if r.N == nil { + return true + } + switch r.N.Value { + case "", "1": + return true + } + return false +} + +// wordText returns a usable string representation of a syntax.Word — +// preferring the unquoted literal value when all parts reduce to plain +// text, otherwise falling back to the verbatim source slice. The +// fallback matters for paths like `$HOME/.env` where downstream policy +// still needs to see the surface text even though Bash would expand it. +func wordText(command string, w *syntax.Word) string { + if w == nil { + return "" + } + if lit, ok := wordToLiteral(w); ok { + return lit + } + start := int(w.Pos().Offset()) + end := int(w.End().Offset()) + if start < 0 || end > len(command) || start >= end { + return "" + } + return command[start:end] +} + +// wordToLiteral returns w's effective literal text if every part is a +// plain literal or quoted literal (no expansions). The second return +// is false when the word requires shell evaluation; callers fall back +// to the verbatim source slice via wordText. +func wordToLiteral(w *syntax.Word) (string, bool) { + var b strings.Builder + for _, part := range w.Parts { + switch p := part.(type) { + case *syntax.Lit: + b.WriteString(p.Value) + case *syntax.SglQuoted: + b.WriteString(p.Value) + case *syntax.DblQuoted: + for _, inner := range p.Parts { + lit, ok := inner.(*syntax.Lit) + if !ok { + return "", false + } + b.WriteString(lit.Value) + } + default: + return "", false + } + } + return b.String(), true +} + +// heredocBodyContent returns the on-disk content a heredoc redirect +// would have produced — the body bytes with the trailing delimiter +// line removed and POSIX `<<-` tab-stripping applied. mvdan.cc/sh's +// Hdoc Word spans `body…\nDELIM`, so we strip from the last newline +// onward to drop the terminator. An entirely empty heredoc +// (`cat <= 0 { + return text[:idx], true + } + return "", true +} diff --git a/codex/bash_write_test.go b/codex/bash_write_test.go new file mode 100644 index 0000000..d3b0f59 --- /dev/null +++ b/codex/bash_write_test.go @@ -0,0 +1,550 @@ +package codex + +import ( + "reflect" + "testing" +) + +func TestParseBashRedirectWrite_CatHeredocSingleQuotedDelim(t *testing.T) { + // Captured verbatim from a Codex 0.130.0 session that produced no + // afterFileEdit before we added this parser — the canonical + // greenfield-write shape. + cmd := "cat <<'EOF' > greet.txt\nhello world\nEOF" + + got, ok := ParseBashRedirectWrite(cmd) + if !ok { + t.Fatalf("ParseBashRedirectWrite(...) ok = false; want true") + } + want := []PatchFile{{ + Operation: "add", + FilePath: "greet.txt", + Edits: []PatchEdit{{ + OldString: "", + NewString: "hello world", + }}, + }} + if !reflect.DeepEqual(got, want) { + t.Errorf("ParseBashRedirectWrite =\n %+v\nwant\n %+v", got, want) + } +} + +func TestParseBashRedirectWrite_CatHeredocDoubleQuotedDelim(t *testing.T) { + cmd := "cat <<\"PYEOF\" > app.py\nprint('hi')\nPYEOF" + + got, ok := ParseBashRedirectWrite(cmd) + if !ok { + t.Fatalf("ParseBashRedirectWrite(...) ok = false; want true") + } + if got[0].FilePath != "app.py" { + t.Errorf("FilePath = %q, want %q", got[0].FilePath, "app.py") + } + if got[0].Edits[0].NewString != "print('hi')" { + t.Errorf("NewString = %q, want %q", got[0].Edits[0].NewString, "print('hi')") + } +} + +func TestParseBashRedirectWrite_CatHeredocUnquotedDelim(t *testing.T) { + cmd := "cat < notes.md\nfirst line\nsecond line\nEOF" + + got, ok := ParseBashRedirectWrite(cmd) + if !ok { + t.Fatalf("ParseBashRedirectWrite(...) ok = false; want true") + } + if got[0].Edits[0].NewString != "first line\nsecond line" { + t.Errorf("NewString = %q", got[0].Edits[0].NewString) + } +} + +func TestParseBashRedirectWrite_CatHeredocAppendRedirect(t *testing.T) { + cmd := "cat <<'EOF' >> log.txt\nappended line\nEOF" + + got, ok := ParseBashRedirectWrite(cmd) + if !ok { + t.Fatalf("ParseBashRedirectWrite(...) ok = false; want true") + } + if got[0].FilePath != "log.txt" { + t.Errorf("FilePath = %q, want %q", got[0].FilePath, "log.txt") + } +} + +func TestParseBashRedirectWrite_CdAndCatHeredoc(t *testing.T) { + // Codex frequently prefixes the cat invocation with a `cd` so the + // relative path resolves correctly. The regex anchors at `;`, `&`, + // `|`, or start-of-line, so `&&` separators must still match. + cmd := "cd /tmp/probe && cat <<'EOF' > out.txt\nbody\nEOF" + + got, ok := ParseBashRedirectWrite(cmd) + if !ok { + t.Fatalf("ParseBashRedirectWrite(...) ok = false; want true") + } + if got[0].FilePath != "out.txt" { + t.Errorf("FilePath = %q, want %q", got[0].FilePath, "out.txt") + } +} + +func TestParseBashRedirectWrite_CatHeredocTabIndent(t *testing.T) { + // `<<-` strips leading TABs (not spaces) from each body line and + // from the delimiter line. Verify both behaviours. + cmd := "cat <<-EOF > indented.txt\n\tline one\n\tline two\n\tEOF" + + got, ok := ParseBashRedirectWrite(cmd) + if !ok { + t.Fatalf("ParseBashRedirectWrite(...) ok = false; want true") + } + if got[0].Edits[0].NewString != "line one\nline two" { + t.Errorf("NewString = %q (tabs not stripped)", got[0].Edits[0].NewString) + } +} + +func TestParseBashRedirectWrite_TeeHeredoc(t *testing.T) { + cmd := "tee out.txt <<'EOF'\nfrom tee\nEOF" + + got, ok := ParseBashRedirectWrite(cmd) + if !ok { + t.Fatalf("ParseBashRedirectWrite(...) ok = false; want true") + } + if got[0].FilePath != "out.txt" { + t.Errorf("FilePath = %q, want %q", got[0].FilePath, "out.txt") + } + if got[0].Edits[0].NewString != "from tee" { + t.Errorf("NewString = %q", got[0].Edits[0].NewString) + } +} + +func TestParseBashRedirectWrite_TeeAppend(t *testing.T) { + cmd := "tee -a log.txt < greet.txt", // simple redirect (no heredoc) — out of scope + "printf 'hi' > greet.txt", // not handled here + "apply_patch <<'PATCH'\n*** Begin Patch\n*** Add File: a\n+x\n*** End Patch\nPATCH", + } + for _, cmd := range tests { + if _, ok := ParseBashRedirectWrite(cmd); ok { + t.Errorf("ParseBashRedirectWrite(%q) ok = true; want false", cmd) + } + } +} + +func TestParseBashRedirectWrite_HeredocMissingTerminator(t *testing.T) { + // Without the closing EOF line we can't trust the body — bail. + cmd := "cat <<'EOF' > greet.txt\nhello world\n" + + if _, ok := ParseBashRedirectWrite(cmd); ok { + t.Errorf("expected ok=false for heredoc missing terminator") + } +} + +func TestParseBashRedirectWrite_DelimAppearingInsideBodyIsNotTerminator(t *testing.T) { + // The delimiter must appear on a line by itself — `EOFISH` should + // not terminate an `EOF` heredoc. + cmd := "cat <<'EOF' > out.txt\nEOFISH is not a terminator\nstill in body\nEOF" + + got, ok := ParseBashRedirectWrite(cmd) + if !ok { + t.Fatalf("ParseBashRedirectWrite(...) ok = false; want true") + } + want := "EOFISH is not a terminator\nstill in body" + if got[0].Edits[0].NewString != want { + t.Errorf("NewString = %q, want %q", got[0].Edits[0].NewString, want) + } +} + +func TestParseBashRedirectWrite_PathTraversalIsPreservedVerbatim(t *testing.T) { + // We never normalize the path — if Codex emits something + // suspicious like `../../etc/passwd`, downstream policy needs to + // see it as-is so it can reject it. Verify we don't accidentally + // rewrite or strip such paths. + cmd := "cat <<'EOF' > ../../etc/passwd\nroot:x:0:0\nEOF" + + got, ok := ParseBashRedirectWrite(cmd) + if !ok { + t.Fatalf("ParseBashRedirectWrite(...) ok = false; want true") + } + if got[0].FilePath != "../../etc/passwd" { + t.Errorf("FilePath = %q, want preserved verbatim", got[0].FilePath) + } +} + +func TestParseBashRedirectWrite_BodyWithEmptyLines(t *testing.T) { + cmd := "cat <<'EOF' > out.txt\n\nline two\n\nEOF" + + got, ok := ParseBashRedirectWrite(cmd) + if !ok { + t.Fatalf("ParseBashRedirectWrite(...) ok = false; want true") + } + want := "\nline two\n" + if got[0].Edits[0].NewString != want { + t.Errorf("NewString = %q, want %q", got[0].Edits[0].NewString, want) + } +} + +// Regression: the original regex-based parser used FindStringSubmatchIndex +// inside matchWriteRedirect, which only returns the first match. A +// Codex Bash command chaining two heredoc writes would dispatch only +// the first to OnAfterFileEdit, silently bypassing path-deny rules, +// secret scanners, and audit logging for every subsequent write. See +// the security report on the cursor/add-codex-support branch for the +// attack-path narrative; the literal payload below is the canonical +// shape from that report. +func TestParseBashRedirectWrite_MultipleCatHeredocs_AllReported(t *testing.T) { + cmd := "cat <<'EOF' > allowed.txt\nok\nEOF\ncat <<'EOF' > .env\nTOKEN=secret\nEOF" + + got, ok := ParseBashRedirectWrite(cmd) + if !ok { + t.Fatalf("ParseBashRedirectWrite(...) ok = false; want true") + } + want := []PatchFile{ + {Operation: "add", FilePath: "allowed.txt", Edits: []PatchEdit{{OldString: "", NewString: "ok"}}}, + {Operation: "add", FilePath: ".env", Edits: []PatchEdit{{OldString: "", NewString: "TOKEN=secret"}}}, + } + if !reflect.DeepEqual(got, want) { + t.Errorf("ParseBashRedirectWrite =\n %+v\nwant\n %+v", got, want) + } +} + +// Regression: multi-heredoc commands prefixed with the `cd … && cat …` +// shape Codex often emits — Stmts nested inside BinaryCmds must still +// be visited by the AST walk and reported alongside the top-level +// writes that follow them. +func TestParseBashRedirectWrite_MultipleCatHeredocs_WithCdPrefix(t *testing.T) { + cmd := "cd /tmp && cat <<'EOF' > a.txt\nA\nEOF\ncat <<'EOF' > b.txt\nB\nEOF" + + got, ok := ParseBashRedirectWrite(cmd) + if !ok { + t.Fatalf("ParseBashRedirectWrite(...) ok = false; want true") + } + if len(got) != 2 { + t.Fatalf("expected 2 PatchFiles, got %d: %+v", len(got), got) + } + if got[0].FilePath != "a.txt" || got[1].FilePath != "b.txt" { + t.Errorf("FilePaths = [%q, %q], want [a.txt, b.txt]", got[0].FilePath, got[1].FilePath) + } +} + +// Regression: a command that mixes cat and tee heredoc writes must +// report both. The original ParseBashRedirectWrite returned on the +// first matchWriteRedirect hit, so the catRedirectHeredocRE branch +// short-circuited the teeRedirectHeredocRE branch even when the +// command contained a tee write the cat regex didn't match. +func TestParseBashRedirectWrite_CatAndTeeMixed_BothReported(t *testing.T) { + cmd := "cat <<'EOF' > from_cat.txt\nA\nEOF\ntee from_tee.txt <<'EOF'\nB\nEOF" + + got, ok := ParseBashRedirectWrite(cmd) + if !ok { + t.Fatalf("ParseBashRedirectWrite(...) ok = false; want true") + } + if len(got) != 2 { + t.Fatalf("expected 2 PatchFiles, got %d: %+v", len(got), got) + } + if got[0].FilePath != "from_cat.txt" || got[1].FilePath != "from_tee.txt" { + t.Errorf("FilePaths = [%q, %q], want [from_cat.txt, from_tee.txt]", got[0].FilePath, got[1].FilePath) + } +} + +// Regression: redirects can legally appear before the heredoc in Bash +// (`cat > FILE <` after the +// heredoc declarator and missed this shape entirely; the AST walk +// finds both redirects regardless of order. +func TestParseBashRedirectWrite_CatReverseRedirectOrder(t *testing.T) { + cmd := "cat > greet.txt <<'EOF'\nhello\nEOF" + + got, ok := ParseBashRedirectWrite(cmd) + if !ok { + t.Fatalf("ParseBashRedirectWrite(...) ok = false; want true") + } + if got[0].FilePath != "greet.txt" { + t.Errorf("FilePath = %q, want greet.txt", got[0].FilePath) + } + if got[0].Edits[0].NewString != "hello" { + t.Errorf("NewString = %q, want hello", got[0].Edits[0].NewString) + } +} + +// Regression: paths that need shell quoting (spaces, special chars) +// were broken by the regex's `[^\s;&|]+` path class. The AST parser +// gives us a syntax.Word we can unquote correctly. +func TestParseBashRedirectWrite_QuotedPathWithSpaces(t *testing.T) { + cmd := "cat <<'EOF' > \"name with spaces.txt\"\nbody\nEOF" + + got, ok := ParseBashRedirectWrite(cmd) + if !ok { + t.Fatalf("ParseBashRedirectWrite(...) ok = false; want true") + } + if got[0].FilePath != "name with spaces.txt" { + t.Errorf("FilePath = %q, want %q", got[0].FilePath, "name with spaces.txt") + } +} + +// Regression: writes nested inside an `if`/`fi` block (and other +// compound shapes) must still be visited by the walker. The regex +// implementation happened to match these because it ignored block +// structure entirely; the AST implementation has to actively recurse +// into compound commands. This locks that recursion in. +func TestParseBashRedirectWrite_HeredocInsideIfBlock(t *testing.T) { + cmd := "if true; then cat <<'EOF' > out.txt\nbody\nEOF\nfi" + + got, ok := ParseBashRedirectWrite(cmd) + if !ok { + t.Fatalf("ParseBashRedirectWrite(...) ok = false; want true") + } + if len(got) != 1 || got[0].FilePath != "out.txt" { + t.Fatalf("got %+v, want one PatchFile with FilePath=out.txt", got) + } +} + +// Regression: a quoted target path (single or double quotes) must be +// reported to OnAfterFileEdit with the surrounding quotes stripped — +// i.e. the value Bash would actually write to, not the surface token. +// +// The original regex implementation captured the path via +// `[^\s;&|]+`, which happily included surrounding quote characters, +// so policies running `filepath.Rel(repoRoot, ctx.FilePath)` followed +// by `strings.HasPrefix(rel, "..")` would see a string starting with +// `'` or `"` and the containment check would silently pass while the +// dangerous write proceeded. The AST-based parser unquotes +// SglQuoted/DblQuoted Word parts via wordToLiteral; this test pins +// that behavior so a future refactor can't regress it. +func TestParseBashRedirectWrite_CatSingleQuotedPathEscapesContainment(t *testing.T) { + cmd := "cat <<'EOF' > '../../.ssh/authorized_keys'\nattacker-controlled\nEOF" + + got, ok := ParseBashRedirectWrite(cmd) + if !ok { + t.Fatalf("ParseBashRedirectWrite(...) ok = false; want true") + } + if got[0].FilePath != "../../.ssh/authorized_keys" { + t.Errorf("FilePath = %q, want unquoted %q (quoted-path containment bypass)", + got[0].FilePath, "../../.ssh/authorized_keys") + } +} + +func TestParseBashRedirectWrite_CatDoubleQuotedPathEscapesContainment(t *testing.T) { + cmd := "cat <<'EOF' > \"../../.ssh/authorized_keys\"\nattacker-controlled\nEOF" + + got, ok := ParseBashRedirectWrite(cmd) + if !ok { + t.Fatalf("ParseBashRedirectWrite(...) ok = false; want true") + } + if got[0].FilePath != "../../.ssh/authorized_keys" { + t.Errorf("FilePath = %q, want unquoted %q", + got[0].FilePath, "../../.ssh/authorized_keys") + } +} + +func TestParseBashRedirectWrite_TeeQuotedPathEscapesContainment(t *testing.T) { + cmd := "tee '../../.ssh/authorized_keys' <<'EOF'\nattacker-controlled\nEOF" + + got, ok := ParseBashRedirectWrite(cmd) + if !ok { + t.Fatalf("ParseBashRedirectWrite(...) ok = false; want true") + } + if got[0].FilePath != "../../.ssh/authorized_keys" { + t.Errorf("FilePath = %q, want unquoted %q", got[0].FilePath, "../../.ssh/authorized_keys") + } +} + +// Regression: an invalid Bash command (unclosed heredoc, dangling +// quote, etc.) must fail closed rather than fall back to a partial +// parse. The unified bridge surfaces the raw command through its +// fallback path when both detectors return ok=false, so the +// underlying policy still sees the event — but only after the +// detector has explicitly declined to invent a PatchFile out of +// malformed input. +func TestParseBashRedirectWrite_InvalidBash_FailsClosed(t *testing.T) { + cmd := "cat <<'EOF' > greet.txt\nhello\n" // missing terminator + + if files, ok := ParseBashRedirectWrite(cmd); ok { + t.Errorf("expected ok=false on invalid Bash; got %+v", files) + } +} + +// Regression: when one heredoc in a multi-write command is well-formed +// and a later token makes the command unparseable, we should not +// silently return the well-formed write — the entire command is +// suspect and the unified bridge's fallback path is more appropriate. +func TestParseBashRedirectWrite_PartiallyMalformed_FailsClosed(t *testing.T) { + // Second heredoc never terminates; parser will reject the whole + // command. The first write must not slip through. + cmd := "cat <<'EOF' > a.txt\nA\nEOF\ncat <<'EOF' > b.txt\nB\n" + + if files, ok := ParseBashRedirectWrite(cmd); ok { + t.Errorf("expected ok=false when later heredoc lacks terminator; got %+v", files) + } +} + +// Regression: tee writes its stdin to EVERY file operand, not just +// the first. The original teeRedirectWrite extracted only the first +// non-option positional arg and broke out of the loop, so a command +// like `tee allowed.txt ../../.ssh/authorized_keys < stderr.log` as +// {Op: RdrOut, N: lit("2"), …} — the same RedirOperator as a stdout +// `> file` redirect, distinguished only by the fd field N. The +// original catRedirectWrite ignored N and just picked the last +// RdrOut/AppOut entry, so a command like +// `cat < .env 2> allowed.log` would have `out` overwritten by +// the (later) stderr entry and report `allowed.log` as the write +// target. Path-deny rules on `.env` and secret scanners would never +// see the actual write — a complete bypass with no expansion trick. +func TestParseBashRedirectWrite_CatStderrShadowsStdoutTarget(t *testing.T) { + cmd := "cat <<'EOF' > .env 2> allowed.log\nTOKEN=secret\nEOF" + + got, ok := ParseBashRedirectWrite(cmd) + if !ok { + t.Fatalf("ParseBashRedirectWrite(...) ok = false; want true") + } + if len(got) != 1 { + t.Fatalf("expected 1 PatchFile, got %d: %+v", len(got), got) + } + if got[0].FilePath != ".env" { + t.Errorf("FilePath = %q, want .env (stderr 2> redirect must not shadow stdout target)", + got[0].FilePath) + } + if got[0].Edits[0].NewString != "TOKEN=secret" { + t.Errorf("NewString = %q, want %q", got[0].Edits[0].NewString, "TOKEN=secret") + } +} + +// Regression: explicit `1>` is identical to bare `>` — both target +// stdout. Verify the fd filter accepts `1` as a synonym for "no fd". +func TestParseBashRedirectWrite_CatExplicitStdoutFd(t *testing.T) { + cmd := "cat <<'EOF' 1> out.txt\nbody\nEOF" + + got, ok := ParseBashRedirectWrite(cmd) + if !ok { + t.Fatalf("ParseBashRedirectWrite(...) ok = false; want true") + } + if got[0].FilePath != "out.txt" { + t.Errorf("FilePath = %q, want out.txt", got[0].FilePath) + } +} + +// Regression: a cat statement whose ONLY `>` redirect is on a +// non-stdout fd (e.g. `cat < log.txt` — stdin from heredoc, +// stderr to log.txt, stdout discarded) has no actual write target we +// can attribute to the heredoc body. The heredoc body just goes to +// stdout (which here is the default tty/pipe), so we must NOT report +// log.txt as if the heredoc landed there. The detector returns +// ok=false and the unified bridge's fallback handles the raw command. +func TestParseBashRedirectWrite_CatStderrOnlyRedirect_NotReported(t *testing.T) { + cmd := "cat <<'EOF' 2> log.txt\nbody\nEOF" + + if files, ok := ParseBashRedirectWrite(cmd); ok { + t.Errorf("expected ok=false when only redirect is non-stdout; got %+v", files) + } +} + +// Regression: `&>` (RdrAll) redirects both stdout and stderr to a +// single file. That file IS the heredoc's write target, so it must +// be reported. Without explicit handling of RdrAll/AppAll, a command +// like `cat < .env` would silently bypass detection. +func TestParseBashRedirectWrite_CatRdrAll(t *testing.T) { + cmd := "cat <<'EOF' &> combined.log\nbody\nEOF" + + got, ok := ParseBashRedirectWrite(cmd) + if !ok { + t.Fatalf("ParseBashRedirectWrite(...) ok = false; want true") + } + if got[0].FilePath != "combined.log" { + t.Errorf("FilePath = %q, want combined.log", got[0].FilePath) + } +} + +// Regression: a tee statement with multiple targets where ONE target +// is unparseable (empty word offsets, etc.) must fail closed for the +// whole statement — partial coverage of a multi-target tee is the +// exact bypass we're guarding against. We trigger the empty-path +// codepath via `tee "" foo.txt <. + +## Configuration + +Codex hooks are enabled by default (the `hooks` feature flag is stable). If +your organization disabled hooks, set `[features].hooks = true` in +`~/.codex/config.toml` to re-enable. The older `codex_hooks` key still works +as a deprecated alias. + +Hook commands live in `~/.codex/hooks.json` (or an inline `[hooks]` table in +`~/.codex/config.toml`). The minimum useful layout: + +```json +{ + "hooks": { + "PreToolUse": [{ "matcher": "Bash|apply_patch|mcp__.*", "hooks": [{ "type": "command", "command": "/path/to/my-hooks codex-pre-tool-use" }] }], + "PostToolUse": [{ "matcher": "Bash|apply_patch|mcp__.*", "hooks": [{ "type": "command", "command": "/path/to/my-hooks codex-post-tool-use" }] }], + "UserPromptSubmit": [{ "hooks": [{ "type": "command", "command": "/path/to/my-hooks codex-user-prompt-submit" }] }], + "Stop": [{ "hooks": [{ "type": "command", "command": "/path/to/my-hooks codex-stop", "timeout": 30 }] }] + } +} +``` + +`hookshot install --codex --binary /path/to/my-hooks` will generate this for +you. + +> **Why `mcp__.*` is in the matcher.** Codex passes MCP tool names to +> PreToolUse / PostToolUse using the `mcp__server__tool` convention. +> Omitting `mcp__.*` would silently bypass any `OnBeforeExecution` policy +> meant to enforce MCP allowlists. +> +> **Why `Edit|Write` is not in the matcher.** Codex emits `Edit` and `Write` +> as *matcher aliases* for `apply_patch`. The canonical `tool_name` Codex +> sends to the hook is always `apply_patch`, so a matcher of `apply_patch` +> alone covers every file-edit call. + +## Events + +| Event | Types | Codex notes | +|---|---|---| +| SessionStart | `codex.SessionStartInput` / `Output` | `source` is `startup`, `resume`, or `clear`. | +| PreToolUse | `codex.PreToolUseInput` / `Output` | `tool_name` is `Bash`, `apply_patch`, or `mcp__server__tool`. See **Ask is not enforced** below. | +| PermissionRequest | `codex.PermissionRequestInput` / `Output` | Codex-specific. Fires only when Codex is about to surface an approval prompt; see below. | +| PostToolUse | `codex.PostToolUseInput` / `Output` | Codex 0.130.0+ routes most file edits through `Bash` rather than `apply_patch`/`Write`/`Edit`. See **Bash bridge** and **apply_patch on the unified API** below. | +| UserPromptSubmit | `codex.UserPromptSubmitInput` / `Output` | Same shape as Claude. | +| Stop | `codex.StopInput` / `Output` | Same shape as Claude; Codex expects JSON on stdout (not plain text). | + +Codex also sends a `model` field (active model slug) on every hook event and +a `turn_id` field on turn-scoped events (`PreToolUse`, `PermissionRequest`, +`PostToolUse`, `UserPromptSubmit`, `Stop`). These aren't on the shared +`BaseInput` struct — read them with `hookshot.ReadRawInput` if you need +them. Stop also carries `last_assistant_message`. + +Codex enforces `continue: false` on `SessionStart`, `UserPromptSubmit`, +`PostToolUse`, and `Stop`. For `PreToolUse` and `PermissionRequest`, Codex +rejects `continue`, `stopReason`, and `suppressOutput` with an +`unsupported ` error and discards the whole hook output — these +fields fail **closed** and must be omitted. The upstream +[Codex hooks doc](https://developers.openai.com/codex/hooks) currently +describes these as fail-open; the runtime behavior is fail-closed. + +## PreToolUse: which output fields actually work + +Codex honors `permissionDecision: "deny"` (or the older `decision: "block"` +shape) on Bash and `apply_patch`. Codex also honors +`hookSpecificOutput.additionalContext`, which is injected as developer +context without blocking the call (the upstream +[Codex hooks doc](https://developers.openai.com/codex/hooks#pretooluse) +shows this case as a first-class example). + +`permissionDecision: "ask"` is parsed by Codex but currently not enforced +at the platform level — emitting it would silently allow the tool to run. +To prevent "require confirmation" policies from turning into free passes, +both the high-level and platform-level APIs fail closed: + +- `hookshot.OnBeforeExecution` returning `AskExecution(...)` is rewritten + to `Deny` on Codex. +- `codex.Ask(reason)` is a fail-closed shim that returns `Deny(reason)` — + it does not emit `"ask"`. + +If you want to react when Codex is actually about to prompt the user, +register a separate handler for the **PermissionRequest** event below — +that event's enforcement is supported. + +**`updatedInput`, `continue: false`, `stopReason`, and `suppressOutput` +fail closed** — Codex rejects the whole hook output with +`PreToolUse hook returned unsupported ` rather than ignoring the +field, so these must be omitted. Concretely, this means: + +- `codex.Allow(reason)`, `codex.Deny(reason)`, and `codex.PassThrough()` + are all safe on Codex. +- `codex.AllowSilent()` is **not safe**: it sets `suppressOutput: true`, + which Codex rejects. Use `codex.PassThrough()` for an empty + no-side-effects allow. +- `codex.AllowWithInput(reason, input)` is a **fail-closed shim**: there + is no Codex-supported way to mutate `tool_input` from a PreToolUse + hook (the CLI rejects `updatedInput`), and silently allowing the call + with the original, unsanitized input would defeat the purpose of + rewriting it. The helper therefore returns `Deny(reason + " (input + rewriting not supported on Codex; fail-closed)")`. If you need to + inject model-visible context on Codex, use `codex.AllowWithContext` + (or `additionalContext` directly) instead. +- To attach model-visible context to an allow, return + `codex.AllowWithContext(reason, context)` (or build the output by hand + with `hookSpecificOutput.additionalContext`). + +The hookshot unified bridge already strips `suppressOutput` from Codex +`OnBeforeExecution(AllowExecution())` outputs, but if you call the +platform-level helpers directly you need to pick safe ones yourself. + +## PermissionRequest (Codex-only) + +Fires when Codex is about to ask for approval (shell escalation, managed- +network approval). Doesn't run for commands that don't need approval. The +`tool_input` may include a `description` field with a human-readable reason. + +If multiple matching hooks return decisions, any `deny` wins. Otherwise an +`allow` lets the request proceed without surfacing the approval prompt. If +no matching hook decides, Codex uses the normal approval flow. `updatedInput`, +`updatedPermissions`, and `interrupt` are reserved for future behavior and +fail closed today. + +Helpers: `codex.AllowPermission()` and `codex.DenyPermission(message string)`. + +## apply_patch on the unified API + +`hookshot.OnAfterFileEdit` parses Codex `apply_patch` events by unpacking +the unified-diff envelope in `tool_input.command` and invoking your handler +**once per file** mentioned in the patch. Each invocation receives a fully +populated `FileEditContext`: + +- `FilePath` is the path declared in the `*** Add File:`, + `*** Update File:`, or `*** Delete File:` section. +- `NewFilePath` is the destination path for rename operations + (`*** Move to:`); empty otherwise. +- `Edits` is `[{OldString: "", NewString: }]` for Add, one + `FileEdit` per hunk for Update, and empty for Delete. + +For renames (`*** Update File: ` followed by `*** Move to: `) the +handler is invoked **twice** — once with `FilePath` set to the source and +once with `FilePath` set to the destination — and `NewFilePath` is populated +on both. This means a FilePath-only allowlist that permits the benign source +still receives a separate call for the destination so it can deny moves to +sensitive locations like `../../.ssh/authorized_keys`. Policies that want +to react specifically to renames should check +`ctx.NewFilePath != "" && ctx.NewFilePath != ctx.FilePath`. + +If any per-file invocation returns `FileEditBlock`, the unified bridge +concatenates the reasons and emits a single `PostToolBlock`. + +The same parser is also exported as +`codex.ParseApplyPatch(rawCommand string) []codex.PatchFile` for callers +that want to parse a patch envelope themselves — for example from the raw +`codex.PostToolUseInput.ToolInput`. + +## Bash bridge: file edits routed through Bash + +Codex `0.130.0`+ routes most file operations through plain `Bash` +invocations rather than the native `apply_patch`, `Write`, or `Edit` +tools. The unified `hookshot.OnAfterFileEdit` bridge recognizes this +and dispatches the same per-file pipeline as the native shapes, so +policy handlers see one `FileEditContext` per file regardless of how +Codex chose to encode the edit. + +The four shapes the bridge recognizes today: + +| `tool_name` | `tool_input.command` shape | Parser | +|----------------|-----------------------------------------------------------------------|---------------------------------| +| `Write` / `Edit` | (native fields — uncommon on 0.130.0+) | inline | +| `apply_patch` | unified-diff envelope, no Bash wrapper | `codex.ParseApplyPatch` | +| `Bash` | `apply_patch <<'PATCH' … *** End Patch … PATCH` | `codex.ParseApplyPatchFromBash` | +| `Bash` | `cat <<'EOF' > FILE … EOF` or `tee FILE <<'EOF' … EOF` | `codex.ParseBashRedirectWrite` | + +This is why the `PostToolUse` matcher in `~/.codex/hooks.json` must +include `Bash` (not just `apply_patch`): + +```jsonc +"PostToolUse": [{ "matcher": "Bash|apply_patch|mcp__.*", /* … */ }] +``` + +`hookshot install --codex` writes this matcher by default. If you +configured hooks before adding the `Bash` token, you'll see zero +`afterFileEdit` events for any Codex session that edits or creates +files — that's the symptom that motivated this bridge. + +### `apply_patch` via Bash heredoc + +Edits to existing files almost always arrive as a Bash heredoc that +pipes a unified-diff envelope into `apply_patch`. Two shape variants +show up in practice: + +- **Built-in name**: `apply_patch <<'PATCH' … *** End Patch … PATCH` +- **Per-session shim**: `/Users/me/.codex/tmp/arg0/codex-arg0…/apply_patch <<'PATCH' …` + +`codex.ParseApplyPatchFromBash` accepts both. Detection requires the +literal token `apply_patch` followed by a heredoc operator (`<<` or +`<<-`) somewhere before the `*** Begin Patch` marker, which keeps +plain Bash commands that merely *mention* `apply_patch` (filenames, +docs, log lines) from spuriously firing the file-edit handler. + +Once detected, the envelope is fed to `codex.ParseApplyPatch` and +the per-file dispatch behaves exactly like the native `apply_patch` +case described above — including the rename double-invocation for +`*** Move to:` paths. + +### Greenfield writes via `cat` / `tee` heredoc + +New-file creation typically arrives as `cat <<'EOF' > FILE … EOF` +(or, less commonly, `tee FILE <<'EOF' … EOF`). There's no prior +content to diff against, so `codex.ParseBashRedirectWrite` +synthesizes a single `PatchEdit{OldString: "", NewString: }` +and a `PatchFile{Operation: "add", FilePath: , Edits: …}`, +then dispatches it through the same per-file pipeline. Variants +currently recognized: + +- quoted (`<<'EOF'`, `<<"EOF"`) and unquoted (`<` (overwrite) and `>>` (append) redirections, +- `cd … && cat <<'EOF' > FILE … EOF` cwd-prefixed forms. + +File paths are surfaced verbatim — `../`, `~/`, and absolute paths +pass through unchanged so downstream policies can apply their own +canonicalization rules (the bridge can't safely guess the agent's +cwd, so it doesn't try). + +If a Codex Bash command matches neither parser, the +`codex-post-tool-use` handler returns `PostToolOK()` without invoking +`OnAfterFileEdit`. That matches the platform semantics of every +other backend: non-edit Bash commands belong to `OnBeforeExecution`, +not `OnAfterFileEdit`. + +### Calling the parsers from your own handler + +Every parser used by the bridge is exported from the `codex` package +so handlers that want full control can short-circuit the unified +dispatch: + +```go +func ParseApplyPatch(rawCommand string) []PatchFile +func ParseApplyPatchFromBash(bashCommand string) (files []PatchFile, ok bool) +func ParseBashRedirectWrite(bashCommand string) (files []PatchFile, ok bool) + +type PatchFile struct { + Operation string // "add", "update", or "delete" + FilePath string + NewFilePath string // set for "*** Move to:" renames + Edits []PatchEdit +} + +type PatchEdit struct { + OldString string + NewString string +} +``` + +Wiring them up manually: + +```go +hookshot.Register("codex-post-tool-use", func() { + hookshot.Run(func(input codex.PostToolUseInput) codex.PostToolUseOutput { + if input.ToolName != "Bash" { + return codex.PostToolOK() + } + var bash struct{ Command string `json:"command"` } + json.Unmarshal(input.ToolInput, &bash) + + if files, ok := codex.ParseApplyPatchFromBash(bash.Command); ok { + return scanPatches(files) + } + if files, ok := codex.ParseBashRedirectWrite(bash.Command); ok { + return scanPatches(files) + } + return codex.PostToolOK() + }) +}) +``` + +### Known limitations + +The Bash bridge is deliberately conservative — it short-circuits on +ambiguous input rather than guessing. Known gaps today: + +- **`echo … > FILE` and `printf … > FILE` writes are not + recognized.** Codex prefers heredocs for any multi-line content, + so these are rare in practice. If you observe them in your + sessions please open an issue with the captured + `tool_input.command`. +- **Only the first `cat`/`tee` heredoc redirect in a single Bash + command is currently surfaced.** A command that performs several + heredoc writes in sequence (e.g. + `cat < a.txt … EOF; cat < b.txt … EOF`) will fire + `OnAfterFileEdit` only for the first file. As defence in depth + against this gap, also register an `OnBeforeExecution` policy + that grep/regex-scans `ctx.Command` for sensitive paths in Bash + commands — that handler runs *before* any heredoc writes execute + and isn't subject to the first-match restriction. +- **The bridge does not normalize file paths.** `../`, `~/`, and + symbolic links are passed through verbatim. Handlers that want to + enforce a containment policy should apply `filepath.Clean` and a + cwd-escape check themselves (the bridge can't safely assume the + agent's working directory matches the hook process's). + +## PostToolUse semantics + +`decision: "block"` doesn't undo the completed tool call. Codex records the +feedback, replaces the tool result with it, and continues the model from +the hook-provided message. To stop normal processing of the original tool +result, also return `continue: false`. `updatedMCPToolOutput` and +`suppressOutput` are parsed but not supported today. + +## Stop semantics + +`decision: "block"` doesn't reject the turn. Instead it tells Codex to +continue and creates a new continuation prompt that acts as a new user +prompt, using `reason` as that prompt text. If any matching Stop hook +returns `continue: false`, that takes precedence over continuation +decisions from other matching Stop hooks. + +## Example + +```go +hookshot.Register("codex-pre-tool-use", func() { + hookshot.Run(func(input codex.PreToolUseInput) codex.PreToolUseOutput { + if input.ToolName == "Bash" { + var ti struct{ Command string `json:"command"` } + json.Unmarshal(input.ToolInput, &ti) + if strings.Contains(ti.Command, "rm -rf /") { + return codex.Deny("Destructive command blocked by hook.") + } + } + return codex.PassThrough() + }) +}) +``` + +You can also use exit code `2` with the reason written to stderr instead of +returning the JSON output — that's what `RunE` does for you when the +handler returns an error. diff --git a/docs/reference-unified.md b/docs/reference-unified.md index 28b1397..ba73d6b 100644 --- a/docs/reference-unified.md +++ b/docs/reference-unified.md @@ -1,6 +1,6 @@ # Unified API Reference -The unified API provides cross-platform handlers that work on Claude Code, Cursor, Windsurf Cascade, and Factory Droid. Write once, run on all platforms. +The unified API provides cross-platform handlers that work on Claude Code, Cursor, Windsurf Cascade, Factory Droid, and OpenAI Codex. Write once, run on all platforms. ## Platform Constants @@ -12,6 +12,7 @@ const ( PlatformCursor Platform = "cursor" PlatformDroid Platform = "droid" PlatformCascade Platform = "cascade" + PlatformCodex Platform = "codex" ) ``` @@ -19,17 +20,17 @@ const ( Handles stop events when the agent is about to finish. -**Registers:** `claude-stop`, `cursor-stop`, `droid-stop`, `cascade-post-cascade-response` +**Registers:** `claude-stop`, `cursor-stop`, `droid-stop`, `cascade-post-cascade-response`, `codex-stop` ### StopContext ```go type StopContext struct { Platform Platform - SessionID string // Claude/Droid: session_id, Cursor: conversation_id, Cascade: trajectory_id - Cwd string // Working directory (Claude/Droid only, empty for Cursor/Cascade) + SessionID string // Claude/Droid/Codex: session_id, Cursor: conversation_id, Cascade: trajectory_id + Cwd string // Working directory (Claude/Droid/Codex only, empty for Cursor/Cascade) - // Claude/Droid-specific + // Claude/Droid/Codex-specific StopHookActive bool // True if already continuing from a previous stop hook // Cursor-specific @@ -42,7 +43,7 @@ type StopContext struct { ```go // ShouldSkip returns true if the stop hook should be skipped to prevent loops. -// Claude/Droid: checks StopHookActive +// Claude/Droid/Codex: checks StopHookActive // Cursor: checks LoopCount >= 3 // Cascade: always returns false (no loop prevention mechanism) func (c StopContext) ShouldSkip() bool @@ -86,7 +87,11 @@ hookshot.OnStop(func(ctx hookshot.StopContext) hookshot.StopDecision { Handles pre-execution events for shell commands and MCP tools. -**Registers:** `claude-pre-tool-use`, `cursor-before-shell`, `cursor-before-mcp`, `droid-pre-tool-use`, `cascade-pre-run-command`, `cascade-pre-mcp-tool-use` +**Registers:** `claude-pre-tool-use`, `cursor-before-shell`, `cursor-before-mcp`, `droid-pre-tool-use`, `cascade-pre-run-command`, `cascade-pre-mcp-tool-use`, `codex-pre-tool-use` + +For Codex, `apply_patch` is classified as `ExecutionTool` (not `ExecutionShell` or `ExecutionMCP`); use `ctx.ToolName == "apply_patch"` to detect it. The patch text is exposed via `ctx.Command` so policies can inspect it without re-parsing `ToolInput`. + +Codex does not currently enforce `permissionDecision: "ask"`. To avoid a silent fail-open, the unified bridge rewrites `AskExecution(...)` decisions to a `Deny` on Codex; on every other platform `Ask` still surfaces an approval prompt as before. ### ExecutionType @@ -107,9 +112,9 @@ type ExecutionContext struct { Platform Platform Type ExecutionType - // For shell execution (Cursor beforeShellExecution, Claude Code Bash tool) + // For shell execution (Cursor beforeShellExecution, Claude Code/Codex Bash tool) // Also used for local MCP servers on Cursor (command-based MCP servers) - // NOTE: Only populated for Cursor and Cascade, not Claude Code or Droid + // NOTE: For Claude Code, Droid, and Codex, Command is parsed from tool_input.command for Bash. Command string Cwd string // Working directory @@ -119,6 +124,8 @@ type ExecutionContext struct { ServerURL string // MCP server URL (Cursor/Cascade only, for URL-based servers) // Raw access + // For Codex, the raw input is shared with Claude Code (RawClaudeCode) because + // Codex uses the same JSON wire format. RawClaudeCode *claude.PreToolUseInput RawCursor any // *cursor.BeforeShellExecutionInput or *cursor.BeforeMCPExecutionInput RawDroid *droid.PreToolUseInput @@ -178,7 +185,15 @@ hookshot.OnBeforeExecution(func(ctx hookshot.ExecutionContext) hookshot.Executio Handles post-file-edit events. -**Registers:** `claude-after-file-edit`, `cursor-after-file-edit`, `droid-after-file-edit`, `cascade-post-write-code` +**Registers:** `claude-after-file-edit`, `cursor-after-file-edit`, `droid-after-file-edit`, `cascade-post-write-code`, `codex-post-tool-use` + +For Codex, the underlying PostToolUse handler must match `Bash` in addition to `apply_patch`: Codex 0.130.0+ routes greenfield writes through `cat <<'EOF' > FILE` and edits through `apply_patch <<'PATCH'` heredocs — both shapes ride a `tool_name="Bash"` PostToolUse, parsed by `codex.ParseBashRedirectWrite` and `codex.ParseApplyPatchFromBash` respectively. Configure the hook with `matcher: "Bash|apply_patch|mcp__.*"` in `~/.codex/hooks.json`. + +For Codex `apply_patch`, the unified bridge parses the unified-diff envelope in `tool_input.command` and invokes your handler **once per file** in the patch, with `FilePath` set to the file declared in the `*** Add/Update/Delete File:` header and `Edits` populated from each hunk. If any of those invocations returns `FileEditBlock`, the reasons are concatenated and emitted as a single `PostToolBlock`. + +For renames (`*** Update File: ` + `*** Move to: `) the handler is invoked **twice** — once with `FilePath` set to the source and once with `FilePath` set to the destination — and `NewFilePath` is populated on both invocations. This means a FilePath-only allowlist that permits the benign source still receives a separate call for the destination so it can deny something like `../../.ssh/authorized_keys`. Handlers that want to detect the rename relationship should check `ctx.NewFilePath != "" && ctx.NewFilePath != ctx.FilePath`. + +The same parser is exported as `codex.ParseApplyPatch` for callers that want to parse a patch envelope themselves. ### FileEdit @@ -193,11 +208,12 @@ type FileEdit struct { ```go type FileEditContext struct { - Platform Platform - SessionID string // Claude/Droid: session_id, Cursor: conversation_id, Cascade: trajectory_id - FilePath string - Edits []FileEdit - Cwd string + Platform Platform + SessionID string // Claude/Droid: session_id, Cursor: conversation_id, Cascade: trajectory_id + FilePath string + NewFilePath string // Destination path for rename operations (Codex apply_patch "*** Move to:"); empty otherwise. + Edits []FileEdit + Cwd string // Raw access RawClaudeCode *claude.PostToolUseInput @@ -251,7 +267,7 @@ hookshot.OnAfterFileEdit(func(ctx hookshot.FileEditContext) hookshot.FileEditDec Handles prompt submission events. -**Registers:** `claude-user-prompt-submit`, `cursor-before-submit-prompt`, `droid-user-prompt-submit`, `cascade-pre-user-prompt` +**Registers:** `claude-user-prompt-submit`, `cursor-before-submit-prompt`, `droid-user-prompt-submit`, `cascade-pre-user-prompt`, `codex-user-prompt-submit` ### PromptContext diff --git a/examples/multi-hook/main.go b/examples/multi-hook/main.go index f5f49e1..5d01d0e 100644 --- a/examples/multi-hook/main.go +++ b/examples/multi-hook/main.go @@ -53,6 +53,18 @@ // "UserPromptSubmit": [{ "command": "/path/to/my-hooks droid-user-prompt-submit" }] // } // } +// +// Configure in OpenAI Codex (~/.codex/hooks.json; hooks are enabled by +// default in current Codex releases): +// +// { +// "hooks": { +// "Stop": [{ "hooks": [{ "type": "command", "command": "/path/to/my-hooks codex-stop" }] }], +// "PreToolUse": [{ "matcher": "Bash|apply_patch|mcp__.*", "hooks": [{ "type": "command", "command": "/path/to/my-hooks codex-pre-tool-use" }] }], +// "PostToolUse": [{ "matcher": "Bash|apply_patch|mcp__.*", "hooks": [{ "type": "command", "command": "/path/to/my-hooks codex-post-tool-use" }] }], +// "UserPromptSubmit": [{ "hooks": [{ "type": "command", "command": "/path/to/my-hooks codex-user-prompt-submit" }] }] +// } +// } package main import ( @@ -68,8 +80,8 @@ import ( func main() { // ========================================================================== // UNIFIED HANDLERS - // Write once, works on Claude Code, Cursor, Windsurf Cascade, and - // Factory Droid automatically. + // Write once, works on Claude Code, Cursor, Windsurf Cascade, + // Factory Droid, and OpenAI Codex automatically. // ========================================================================== hookshot.OnStop(handleStop) diff --git a/go.mod b/go.mod index bd2e643..8c2c648 100644 --- a/go.mod +++ b/go.mod @@ -1,3 +1,5 @@ module github.com/CorridorSecurity/hookshot go 1.21 + +require mvdan.cc/sh/v3 v3.8.0 diff --git a/go.sum b/go.sum new file mode 100644 index 0000000..a34a4cb --- /dev/null +++ b/go.sum @@ -0,0 +1,12 @@ +github.com/frankban/quicktest v1.14.6 h1:7Xjx+VpznH+oBnejlPUj8oUpdxnVs4f8XU8WnHkI4W8= +github.com/frankban/quicktest v1.14.6/go.mod h1:4ptaffx2x8+WTWXmUCuVU6aPUX1/Mz7zb5vbUoiM6w0= +github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI= +github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= +github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE= +github.com/kr/pretty v0.3.1/go.mod h1:hoEshYVHaxMs3cyo3Yncou5ZscifuDolrwPKZanG3xk= +github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= +github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE= +github.com/rogpeppe/go-internal v1.12.0 h1:exVL4IDcn6na9z1rAb56Vxr+CgyK3nn3O+epU5NdKM8= +github.com/rogpeppe/go-internal v1.12.0/go.mod h1:E+RYuTGaKKdloAfM02xzb0FW3Paa99yedzYV+kq4uf4= +mvdan.cc/sh/v3 v3.8.0 h1:ZxuJipLZwr/HLbASonmXtcvvC9HXY9d2lXZHnKGjFc8= +mvdan.cc/sh/v3 v3.8.0/go.mod h1:w04623xkgBVo7/IUK89E0g8hBykgEpN0vgOj3RJr6MY= diff --git a/hookshot.go b/hookshot.go index 6c3c3d9..b7d216e 100644 --- a/hookshot.go +++ b/hookshot.go @@ -1,5 +1,5 @@ // Package hookshot provides a framework for building hooks for AI coding agents -// like Cursor and Claude Code. +// like Cursor, Claude Code, Windsurf Cascade, Factory Droid, and OpenAI Codex. // // Hookshot handles the boilerplate of reading JSON from stdin and writing JSON to stdout, // letting you focus on your hook logic. All hooks must use the multi-hook pattern with diff --git a/unified.go b/unified.go index c8a5ed0..6a8c5dd 100644 --- a/unified.go +++ b/unified.go @@ -7,6 +7,7 @@ import ( "github.com/CorridorSecurity/hookshot/cascade" "github.com/CorridorSecurity/hookshot/claude" + "github.com/CorridorSecurity/hookshot/codex" "github.com/CorridorSecurity/hookshot/cursor" "github.com/CorridorSecurity/hookshot/droid" "github.com/CorridorSecurity/hookshot/internal" @@ -20,6 +21,7 @@ const ( PlatformCursor Platform = "cursor" PlatformDroid Platform = "droid" PlatformCascade Platform = "cascade" + PlatformCodex Platform = "codex" ) // ============================================================================= @@ -41,10 +43,11 @@ type StopContext struct { } // ShouldSkip returns true if the stop hook should be skipped to prevent loops. -// For Claude Code and Droid, this checks StopHookActive. For Cursor, this checks LoopCount >= 3. -// For Cascade, there is no loop prevention mechanism (returns false). +// For Claude Code, Droid, and Codex, this checks StopHookActive. For Cursor, +// this checks LoopCount >= 3. For Cascade, there is no loop prevention +// mechanism (returns false). func (c StopContext) ShouldSkip() bool { - if c.Platform == PlatformClaude || c.Platform == PlatformDroid { + if c.Platform == PlatformClaude || c.Platform == PlatformDroid || c.Platform == PlatformCodex { return c.StopHookActive } if c.Platform == PlatformCursor { @@ -81,7 +84,7 @@ type StopHandler func(StopContext) StopDecision // OnStop registers a unified handler for stop events on all platforms. // It automatically registers handlers for "claude-stop", "cursor-stop", -// "droid-stop", and "cascade-post-cascade-response". +// "droid-stop", "cascade-post-cascade-response", and "codex-stop". func OnStop(handler StopHandler) { Register("claude-stop", func() { Run(func(input claude.StopInput) claude.StopOutput { @@ -145,6 +148,26 @@ func OnStop(handler StopHandler) { return cascade.PostCascadeResponseOK() }) }) + + // Codex uses the same JSON wire protocol as Claude Code but with stricter + // validation. Use codex.* helpers (not claude.*) so any Codex-specific + // quirks (e.g. rejected suppressOutput / updatedInput) are handled in + // one place — the codex package. + Register("codex-stop", func() { + Run(func(input codex.StopInput) codex.StopOutput { + ctx := StopContext{ + Platform: PlatformCodex, + SessionID: input.SessionID, + Cwd: input.Cwd, + StopHookActive: input.StopHookActive, + } + decision := handler(ctx) + if decision.Continue { + return codex.Continue() + } + return codex.Block(decision.Message) + }) + }) } // ============================================================================= @@ -233,6 +256,7 @@ type ExecutionHandler func(ExecutionContext) ExecutionDecision // - "droid-pre-tool-use" (filters to Bash and mcp__* tools) // - "cascade-pre-run-command" // - "cascade-pre-mcp-tool-use" +// - "codex-pre-tool-use" (filters to Bash, apply_patch, and mcp__* tools) func OnBeforeExecution(handler ExecutionHandler) { // Claude Code PreToolUse (for Bash and MCP tools) Register("claude-pre-tool-use", func() { @@ -431,6 +455,67 @@ func OnBeforeExecution(handler ExecutionHandler) { return cascade.PreMCPToolUseOutput{}, errors.New(decision.Reason) }) }) + + // Codex PreToolUse (same JSON wire protocol as Claude Code, stricter + // validation). Codex tool names include "Bash", "apply_patch", and MCP + // names like "mcp__server__tool". apply_patch is classified as + // ExecutionTool because it represents a file edit rather than a shell + // command or MCP call. For apply_patch the underlying tool_input.command + // is parsed and exposed via ExecutionContext.Command so policies can + // inspect the patch text. Uses codex.* helpers so Codex quirks (no + // suppressOutput, no updatedInput) live in the codex package. + Register("codex-pre-tool-use", func() { + Run(func(input codex.PreToolUseInput) codex.PreToolUseOutput { + var execType ExecutionType + if input.ToolName == "Bash" { + execType = ExecutionShell + } else if len(input.ToolName) > 5 && input.ToolName[:5] == "mcp__" { + execType = ExecutionMCP + } else { + execType = ExecutionTool + } + + var command string + if execType == ExecutionShell || input.ToolName == "apply_patch" { + var cmdInput struct { + Command string `json:"command"` + } + json.Unmarshal(input.ToolInput, &cmdInput) + command = cmdInput.Command + } + + ctx := ExecutionContext{ + Platform: PlatformCodex, + Type: execType, + Command: command, + Cwd: input.Cwd, + ToolName: input.ToolName, + ToolInput: input.ToolInput, + RawClaudeCode: &input, + } + + decision := handler(ctx) + if decision.Allow { + if decision.Reason != "" { + return codex.Allow(decision.Reason) + } + // codex.AllowSilent is a Codex-safe no-op (emits {}) — it + // does NOT set suppressOutput like claude.AllowSilent, + // because Codex rejects that field with "PreToolUse hook + // returned unsupported suppressOutput". + return codex.AllowSilent() + } + // Codex currently parses but does not enforce permissionDecision + // "ask" for PreToolUse, so an Ask decision would silently fail + // open. Until Codex enforces it, fail closed by denying — this + // matches the security posture of the other platforms where + // Ask actually surfaces an approval prompt. + if decision.Ask { + return codex.Deny(decision.Reason) + } + return codex.Deny(decision.Reason) + }) + }) } // ============================================================================= @@ -448,8 +533,19 @@ type FileEditContext struct { Platform Platform SessionID string // Claude Code: session_id, Cursor: conversation_id, Cascade: trajectory_id FilePath string - Edits []FileEdit - Cwd string + // NewFilePath is the destination path when the edit also renames the + // file (Codex apply_patch "*** Move to:" today; future platforms may + // surface their own rename semantics). It is empty for in-place edits. + // + // For Codex moves, the unified bridge invokes OnAfterFileEdit twice — + // once with FilePath set to the source and once with FilePath set to + // the destination — and populates NewFilePath on both invocations so + // path-based policies can never be bypassed by inspecting only + // FilePath. Handlers that want to detect a rename should check + // `ctx.NewFilePath != "" && ctx.NewFilePath != ctx.FilePath`. + NewFilePath string + Edits []FileEdit + Cwd string // Raw input for advanced use cases RawClaudeCode *claude.PostToolUseInput @@ -494,6 +590,7 @@ type FileEditHandler func(FileEditContext) FileEditDecision // - "cursor-after-file-edit" // - "droid-after-file-edit" (PostToolUse for Write/Edit) // - "cascade-post-write-code" +// - "codex-post-tool-use" (PostToolUse for Write/Edit/apply_patch) func OnAfterFileEdit(handler FileEditHandler) { // Claude Code PostToolUse (for Write/Edit) Register("claude-after-file-edit", func() { @@ -630,6 +727,196 @@ func OnAfterFileEdit(handler FileEditHandler) { return cascade.PostWriteCodeOK() }) }) + + // Codex PostToolUse (same JSON protocol as Claude Code). + // + // Codex emits file edits in three shapes: + // 1. Write / Edit — Claude-style tool_input with file_path + content + // or old_string/new_string. Native first-class tools. + // 2. apply_patch — tool_input.command holds a unified-diff envelope + // that may touch multiple files in a single call. + // 3. Bash — tool_input.command holds one of two shapes: + // a. Edits: `apply_patch <<'PATCH' … PATCH` + // (sometimes with an absolute path + // to a per-session apply_patch shim) + // b. Writes: `cat <<'EOF' > FILE … EOF` or + // `tee FILE <<'EOF' … EOF` + // Codex routes virtually all file operations this + // way: edits via (a), greenfield writes via (b). + // Without handling BOTH cases the + // SecurityScanResults dashboard stayed empty for + // every Codex session — no afterFileEdit + // telemetry was ever emitted for "Create a file" + // prompts or for in-place edits. + // + // For (2) and (3a) we run the patch through codex.ParseApplyPatch and + // invoke the user's handler exactly once per file in the envelope. For + // (3b) codex.ParseBashRedirectWrite synthesizes a single PatchFile so + // the same dispatchPatch reducer drives every shape. Per-file + // decisions reduce as: a Block from any file wins, otherwise context + // strings are concatenated. The parsers are also exported as + // codex.ParseApplyPatch / codex.ParseApplyPatchFromBash / + // codex.ParseBashRedirectWrite for callers that want raw access. + // + // Configure the hook with matcher "Bash|apply_patch|mcp__.*" in + // hooks.json — "Edit" and "Write" matcher aliases exist but are + // redundant with "apply_patch", and "Bash" is required to catch the + // heredoc shape. + Register("codex-post-tool-use", func() { + Run(func(input codex.PostToolUseInput) codex.PostToolUseOutput { + // dispatchPatch invokes handler once per file in the patch + // envelope, then reduces per-file decisions to one + // PostToolUseOutput. fallbackCommand is used as the + // NewString of a single synthetic FileEdit when the patch + // could not be parsed (len(files)==0), so policies still + // see a Codex PostToolUse event rather than nothing. + dispatchPatch := func(files []codex.PatchFile, fallbackCommand string) codex.PostToolUseOutput { + if len(files) == 0 { + ctx := FileEditContext{ + Platform: PlatformCodex, + SessionID: input.SessionID, + Cwd: input.Cwd, + Edits: []FileEdit{{OldString: "", NewString: fallbackCommand}}, + RawClaudeCode: &input, + } + decision := handler(ctx) + if decision.Block { + return codex.PostToolBlock(decision.Reason) + } + if decision.Context != "" { + return codex.PostToolContext(decision.Context) + } + return codex.PostToolOK() + } + + var ( + blockReasons []string + contexts []string + ) + invoke := func(filePath string, f codex.PatchFile) { + edits := make([]FileEdit, 0, len(f.Edits)) + for _, e := range f.Edits { + edits = append(edits, FileEdit{OldString: e.OldString, NewString: e.NewString}) + } + ctx := FileEditContext{ + Platform: PlatformCodex, + SessionID: input.SessionID, + FilePath: filePath, + NewFilePath: f.NewFilePath, + Edits: edits, + Cwd: input.Cwd, + RawClaudeCode: &input, + } + decision := handler(ctx) + if decision.Block { + blockReasons = append(blockReasons, decision.Reason) + } else if decision.Context != "" { + contexts = append(contexts, decision.Context) + } + } + for _, f := range files { + // Always invoke for the declared source path. + invoke(f.FilePath, f) + // For renames, invoke again with the destination so + // policies that only inspect ctx.FilePath cannot be + // bypassed by a "*** Move to:" pointing at a + // sensitive path (e.g. "../../.ssh/authorized_keys"). + // NewFilePath is populated on both invocations so + // policies that want to detect the rename + // relationship can. + if f.NewFilePath != "" && f.NewFilePath != f.FilePath { + invoke(f.NewFilePath, f) + } + } + if len(blockReasons) > 0 { + return codex.PostToolBlock(strings.Join(blockReasons, "\n")) + } + if len(contexts) > 0 { + return codex.PostToolContext(strings.Join(contexts, "\n")) + } + return codex.PostToolOK() + } + + switch input.ToolName { + case "Write", "Edit": + var toolInput struct { + FilePath string `json:"file_path"` + Content string `json:"content"` + OldString string `json:"old_string"` + NewString string `json:"new_string"` + } + json.Unmarshal(input.ToolInput, &toolInput) + + var edits []FileEdit + if input.ToolName == "Edit" { + edits = []FileEdit{{OldString: toolInput.OldString, NewString: toolInput.NewString}} + } else { + edits = []FileEdit{{OldString: "", NewString: toolInput.Content}} + } + + ctx := FileEditContext{ + Platform: PlatformCodex, + SessionID: input.SessionID, + FilePath: toolInput.FilePath, + Edits: edits, + Cwd: input.Cwd, + RawClaudeCode: &input, + } + + decision := handler(ctx) + if decision.Block { + return codex.PostToolBlock(decision.Reason) + } + if decision.Context != "" { + return codex.PostToolContext(decision.Context) + } + return codex.PostToolOK() + + case "apply_patch": + var applyInput struct { + Command string `json:"command"` + } + json.Unmarshal(input.ToolInput, &applyInput) + return dispatchPatch(codex.ParseApplyPatch(applyInput.Command), applyInput.Command) + + case "Bash": + // Codex routes file operations through Bash in two + // distinct shapes that look superficially similar but + // require different parsers: + // + // 1. Edits to existing files: + // apply_patch <<'PATCH' … *** End Patch … PATCH + // parsed by codex.ParseApplyPatchFromBash. + // + // 2. Greenfield writes (and overwrite-style writes): + // cat <<'EOF' > newfile.txt … EOF + // tee newfile.txt <<'EOF' … EOF + // parsed by codex.ParseBashRedirectWrite. + // + // Both produce []PatchFile suitable for the shared + // dispatchPatch reducer. We try apply_patch first + // because it's the higher-fidelity shape (per-hunk + // old/new) when both detectors would match, then fall + // back to the heredoc-write detector. Non-edit Bash + // commands short-circuit at the second `if !ok` check + // without paying for either full parse pass. + var bashInput struct { + Command string `json:"command"` + } + json.Unmarshal(input.ToolInput, &bashInput) + if files, ok := codex.ParseApplyPatchFromBash(bashInput.Command); ok { + return dispatchPatch(files, bashInput.Command) + } + if files, ok := codex.ParseBashRedirectWrite(bashInput.Command); ok { + return dispatchPatch(files, bashInput.Command) + } + return codex.PostToolOK() + + default: + return codex.PostToolOK() + } + }) + }) } // ============================================================================= @@ -686,6 +973,7 @@ type PromptHandler func(PromptContext) PromptDecision // - "cursor-before-submit-prompt" // - "droid-user-prompt-submit" // - "cascade-pre-user-prompt" +// - "codex-user-prompt-submit" func OnPromptSubmit(handler PromptHandler) { // Claude Code UserPromptSubmit Register("claude-user-prompt-submit", func() { @@ -694,6 +982,7 @@ func OnPromptSubmit(handler PromptHandler) { Platform: PlatformClaude, SessionID: input.SessionID, Prompt: input.Prompt, + Cwd: input.Cwd, RawClaudeCode: &input, } @@ -734,6 +1023,7 @@ func OnPromptSubmit(handler PromptHandler) { Platform: PlatformDroid, SessionID: input.SessionID, Prompt: input.Prompt, + Cwd: input.Cwd, RawDroid: &input, } @@ -767,6 +1057,29 @@ func OnPromptSubmit(handler PromptHandler) { return cascade.AllowPrompt(), nil }) }) + + // Codex UserPromptSubmit (same JSON wire protocol as Claude Code, + // stricter validation — use codex.* helpers). + Register("codex-user-prompt-submit", func() { + Run(func(input codex.UserPromptSubmitInput) codex.UserPromptSubmitOutput { + ctx := PromptContext{ + Platform: PlatformCodex, + SessionID: input.SessionID, + Prompt: input.Prompt, + Cwd: input.Cwd, + RawClaudeCode: &input, + } + + decision := handler(ctx) + if !decision.Allow { + return codex.BlockPrompt(decision.Reason) + } + if decision.Context != "" { + return codex.AddContext(decision.Context) + } + return codex.AllowPrompt() + }) + }) } // ============================================================================= diff --git a/unified_test.go b/unified_test.go index 9e7f505..b3afe29 100644 --- a/unified_test.go +++ b/unified_test.go @@ -262,6 +262,25 @@ func TestPlatformConstants(t *testing.T) { if PlatformCursor != "cursor" { t.Errorf("PlatformCursor = %q, want %q", PlatformCursor, "cursor") } + if PlatformDroid != "droid" { + t.Errorf("PlatformDroid = %q, want %q", PlatformDroid, "droid") + } + if PlatformCascade != "cascade" { + t.Errorf("PlatformCascade = %q, want %q", PlatformCascade, "cascade") + } + if PlatformCodex != "codex" { + t.Errorf("PlatformCodex = %q, want %q", PlatformCodex, "codex") + } +} + +func TestStopContext_ShouldSkip_Codex(t *testing.T) { + // Codex uses StopHookActive like Claude Code and Droid. + if !(StopContext{Platform: PlatformCodex, StopHookActive: true}).ShouldSkip() { + t.Error("Codex StopHookActive=true should skip") + } + if (StopContext{Platform: PlatformCodex, StopHookActive: false}).ShouldSkip() { + t.Error("Codex StopHookActive=false should not skip") + } } // ============================================================================= @@ -307,6 +326,7 @@ func TestOnBeforeExecution_RegistersAllHandlers(t *testing.T) { "droid-pre-tool-use", "cascade-pre-run-command", "cascade-pre-mcp-tool-use", + "codex-pre-tool-use", } for _, name := range expectedHandlers { @@ -329,6 +349,7 @@ func TestOnPromptSubmit_RegistersAllHandlers(t *testing.T) { "cursor-before-submit-prompt", "droid-user-prompt-submit", "cascade-pre-user-prompt", + "codex-user-prompt-submit", } for _, name := range expectedHandlers { @@ -351,6 +372,7 @@ func TestOnStop_RegistersAllHandlers(t *testing.T) { "cursor-stop", "droid-stop", "cascade-post-cascade-response", + "codex-stop", } for _, name := range expectedHandlers { @@ -373,6 +395,7 @@ func TestOnAfterFileEdit_RegistersAllHandlers(t *testing.T) { "cursor-after-file-edit", "droid-after-file-edit", "cascade-post-write-code", + "codex-post-tool-use", } for _, name := range expectedHandlers { @@ -776,6 +799,575 @@ func TestOnPromptSubmit_CursorPopulatesCwd(t *testing.T) { } } +// ============================================================================= +// Codex-specific behavior tests +// ============================================================================= + +func TestCodexPreToolUse_AskFailsClosed(t *testing.T) { + // Codex currently parses but does not enforce permissionDecision "ask", + // so the unified Codex pre-tool-use bridge must convert an Ask decision + // into a Deny so policies still fail closed. + ClearHandlers() + defer ClearHandlers() + + OnBeforeExecution(func(ctx ExecutionContext) ExecutionDecision { + return AskExecution("please confirm") + }) + + // Capture stdout to assert on the emitted JSON. + stdinR, stdinW, _ := os.Pipe() + stdinW.Write([]byte(`{"session_id":"s","tool_name":"Bash","tool_input":{"command":"rm -rf /"},"cwd":"/tmp"}`)) + stdinW.Close() + stdoutR, stdoutW, _ := os.Pipe() + origStdin, origStdout := os.Stdin, os.Stdout + os.Stdin, os.Stdout = stdinR, stdoutW + defer func() { + stdoutW.Close() + os.Stdin, os.Stdout = origStdin, origStdout + }() + + handlers["codex-pre-tool-use"]() + stdoutW.Close() + + var buf bytes.Buffer + buf.ReadFrom(stdoutR) + got := buf.String() + if !strings.Contains(got, `"permissionDecision":"deny"`) { + t.Errorf("Codex Ask decision should emit deny, got %q", got) + } + if strings.Contains(got, `"permissionDecision":"ask"`) { + t.Errorf("Codex Ask decision should NOT emit ask, got %q", got) + } +} + +func TestCodexPreToolUse_AllowDoesNotEmitSuppressOutput(t *testing.T) { + // Codex rejects suppressOutput with "PreToolUse hook returned unsupported + // suppressOutput", so the Codex pre-tool-use bridge must NOT use + // claude.AllowSilent (which sets suppressOutput: true) when translating + // an Allow decision with no reason. PassThrough (empty {}) is the right + // shape — Codex treats an empty output as success. + ClearHandlers() + defer ClearHandlers() + + OnBeforeExecution(func(ctx ExecutionContext) ExecutionDecision { + return AllowExecution() + }) + + stdinR, stdinW, _ := os.Pipe() + stdinW.Write([]byte(`{"session_id":"s","tool_name":"Bash","tool_input":{"command":"echo hi"},"cwd":"/tmp"}`)) + stdinW.Close() + stdoutR, stdoutW, _ := os.Pipe() + origStdin, origStdout := os.Stdin, os.Stdout + os.Stdin, os.Stdout = stdinR, stdoutW + defer func() { + stdoutW.Close() + os.Stdin, os.Stdout = origStdin, origStdout + }() + + handlers["codex-pre-tool-use"]() + stdoutW.Close() + + var buf bytes.Buffer + buf.ReadFrom(stdoutR) + got := buf.String() + if strings.Contains(got, "suppressOutput") { + t.Errorf("Codex Allow decision must not emit suppressOutput, got %q", got) + } +} + +func TestCodexPreToolUse_ApplyPatchPopulatesCommand(t *testing.T) { + // For apply_patch, the unified handler should pull tool_input.command + // out as ExecutionContext.Command so policies can inspect the patch. + ClearHandlers() + defer ClearHandlers() + + var captured ExecutionContext + OnBeforeExecution(func(ctx ExecutionContext) ExecutionDecision { + captured = ctx + return AllowExecution() + }) + + stdin := `{"session_id":"s","tool_name":"apply_patch","tool_input":{"command":"*** Begin Patch\n*** Add File: foo.txt\n+hi\n*** End Patch"},"cwd":"/tmp"}` + runHandlerWithStdin(t, "codex-pre-tool-use", stdin) + + if captured.Platform != PlatformCodex { + t.Errorf("Platform = %q, want %q", captured.Platform, PlatformCodex) + } + if captured.Type != ExecutionTool { + t.Errorf("Type = %q, want %q (apply_patch is a tool, not shell/mcp)", captured.Type, ExecutionTool) + } + if captured.ToolName != "apply_patch" { + t.Errorf("ToolName = %q, want %q", captured.ToolName, "apply_patch") + } + if !strings.Contains(captured.Command, "*** Add File: foo.txt") { + t.Errorf("Command should contain the patch text, got %q", captured.Command) + } +} + +func TestCodexPreToolUse_MCPClassifiedAsMCP(t *testing.T) { + // Verifies that MCP tools reaching the codex-pre-tool-use handler are + // classified as ExecutionMCP. (Reaching the handler at all also + // requires the installer to include mcp__.* in the matcher; see + // TestCodexInstaller_MatcherIncludesMCP for that side.) + ClearHandlers() + defer ClearHandlers() + + var captured ExecutionContext + OnBeforeExecution(func(ctx ExecutionContext) ExecutionDecision { + captured = ctx + return AllowExecution() + }) + + stdin := `{"session_id":"s","tool_name":"mcp__net__fetch","tool_input":{"url":"http://example.com"},"cwd":"/tmp"}` + runHandlerWithStdin(t, "codex-pre-tool-use", stdin) + + if !captured.IsMCP() { + t.Errorf("Type = %q, want %q for MCP tool", captured.Type, ExecutionMCP) + } + if captured.ToolName != "mcp__net__fetch" { + t.Errorf("ToolName = %q, want %q", captured.ToolName, "mcp__net__fetch") + } +} + +func TestCodexPostToolUse_ApplyPatchParsesFilesAndEdits(t *testing.T) { + ClearHandlers() + defer ClearHandlers() + + var got []FileEditContext + OnAfterFileEdit(func(ctx FileEditContext) FileEditDecision { + got = append(got, ctx) + return FileEditOK() + }) + + // One add, one update, one delete in a single apply_patch call. + patch := "*** Begin Patch\\n" + + "*** Add File: secrets/api_key.txt\\n" + + "+sk-deadbeef\\n" + + "*** Update File: src/auth.go\\n" + + "@@\\n" + + "-old\\n" + + "+new\\n" + + "*** Delete File: stale.env\\n" + + "*** End Patch" + stdin := `{"session_id":"s","tool_name":"apply_patch","tool_input":{"command":"` + patch + `"},"cwd":"/repo"}` + runHandlerWithStdin(t, "codex-post-tool-use", stdin) + + if len(got) != 3 { + t.Fatalf("handler invocations = %d, want 3 (one per file in patch); got=%+v", len(got), got) + } + if got[0].FilePath != "secrets/api_key.txt" { + t.Errorf("got[0].FilePath = %q, want %q", got[0].FilePath, "secrets/api_key.txt") + } + if len(got[0].Edits) != 1 || got[0].Edits[0].NewString != "sk-deadbeef" { + t.Errorf("got[0].Edits = %+v, want [{OldString:\"\", NewString:\"sk-deadbeef\"}]", got[0].Edits) + } + if got[1].FilePath != "src/auth.go" { + t.Errorf("got[1].FilePath = %q, want %q", got[1].FilePath, "src/auth.go") + } + wantEdit := FileEdit{OldString: "old", NewString: "new"} + if len(got[1].Edits) != 1 || got[1].Edits[0] != wantEdit { + t.Errorf("got[1].Edits = %+v, want [%+v]", got[1].Edits, wantEdit) + } + // Delete sections carry no per-edit content; we just verify the file + // path made it through so path-based policies can still react. + if got[2].FilePath != "stale.env" { + t.Errorf("got[2].FilePath = %q, want %q", got[2].FilePath, "stale.env") + } + if len(got[2].Edits) != 0 { + t.Errorf("got[2].Edits = %+v, want empty for delete", got[2].Edits) + } + for _, c := range got { + if c.Platform != PlatformCodex { + t.Errorf("Platform = %q, want %q", c.Platform, PlatformCodex) + } + if c.Cwd != "/repo" { + t.Errorf("Cwd = %q, want %q", c.Cwd, "/repo") + } + } +} + +func TestCodexPostToolUse_ApplyPatchMoveToInvokesHandlerForDestination(t *testing.T) { + // An apply_patch with a "*** Move to:" directive must surface the + // destination path to the handler. Without this, a patch like + // "*** Update File: app/config.go\n*** Move to: ../../.ssh/authorized_keys" + // would slip past a path-based policy that only inspects + // ctx.FilePath. The bridge invokes the handler twice for a rename so + // existing FilePath-only policies catch the destination. + ClearHandlers() + defer ClearHandlers() + + var seen []FileEditContext + OnAfterFileEdit(func(ctx FileEditContext) FileEditDecision { + seen = append(seen, ctx) + return FileEditOK() + }) + + patch := "*** Begin Patch\\n" + + "*** Update File: app/config.go\\n" + + "*** Move to: ../../.ssh/authorized_keys\\n" + + "@@\\n" + + "-old\\n" + + "+attacker-controlled\\n" + + "*** End Patch" + stdin := `{"session_id":"s","tool_name":"apply_patch","tool_input":{"command":"` + patch + `"},"cwd":"/repo"}` + runHandlerWithStdin(t, "codex-post-tool-use", stdin) + + if len(seen) != 2 { + t.Fatalf("handler invocations = %d, want 2 (source + destination); got %+v", len(seen), seen) + } + // Both invocations must expose the destination via NewFilePath so a + // policy can detect that a rename is happening. + for _, c := range seen { + if c.NewFilePath != "../../.ssh/authorized_keys" { + t.Errorf("NewFilePath = %q, want %q", c.NewFilePath, "../../.ssh/authorized_keys") + } + } + // First call: source path. Second call: destination path. A + // FilePath-only allowlist that permits "app/config.go" must still see + // "../../.ssh/authorized_keys" so it can deny it. + if seen[0].FilePath != "app/config.go" { + t.Errorf("seen[0].FilePath = %q, want %q", seen[0].FilePath, "app/config.go") + } + if seen[1].FilePath != "../../.ssh/authorized_keys" { + t.Errorf("seen[1].FilePath = %q, want %q", seen[1].FilePath, "../../.ssh/authorized_keys") + } +} + +func TestCodexPostToolUse_ApplyPatchMoveToBlockOnDestinationPropagates(t *testing.T) { + // A FilePath-only policy that blocks ".ssh/authorized_keys" must + // surface as a PostToolBlock even when the source path was benign. + ClearHandlers() + defer ClearHandlers() + + OnAfterFileEdit(func(ctx FileEditContext) FileEditDecision { + if strings.Contains(ctx.FilePath, ".ssh/authorized_keys") { + return FileEditBlock("destination is a sensitive path") + } + return FileEditOK() + }) + + patch := "*** Begin Patch\\n" + + "*** Update File: app/config.go\\n" + + "*** Move to: ../../.ssh/authorized_keys\\n" + + "@@\\n" + + "-old\\n" + + "+attacker-controlled\\n" + + "*** End Patch" + stdin := `{"session_id":"s","tool_name":"apply_patch","tool_input":{"command":"` + patch + `"},"cwd":"/repo"}` + + stdinR, stdinW, _ := os.Pipe() + stdinW.Write([]byte(stdin)) + stdinW.Close() + stdoutR, stdoutW, _ := os.Pipe() + origStdin, origStdout := os.Stdin, os.Stdout + os.Stdin, os.Stdout = stdinR, stdoutW + defer func() { + stdoutW.Close() + os.Stdin, os.Stdout = origStdin, origStdout + }() + + handlers["codex-post-tool-use"]() + stdoutW.Close() + + var buf bytes.Buffer + buf.ReadFrom(stdoutR) + out := buf.String() + if !strings.Contains(out, `"decision":"block"`) { + t.Errorf("expected PostToolBlock JSON output, got %q", out) + } + if !strings.Contains(out, "destination is a sensitive path") { + t.Errorf("expected block reason to be present, got %q", out) + } +} + +func TestCodexPostToolUse_ApplyPatchBlockPropagates(t *testing.T) { + // If any file in a multi-file apply_patch is blocked, the unified + // handler must emit a PostToolBlock so Codex sees feedback rather than + // silently passing through. + ClearHandlers() + defer ClearHandlers() + + OnAfterFileEdit(func(ctx FileEditContext) FileEditDecision { + if strings.Contains(ctx.FilePath, "secrets") { + return FileEditBlock("contains secrets") + } + return FileEditOK() + }) + + patch := "*** Begin Patch\\n" + + "*** Add File: secrets/key.txt\\n" + + "+sk-deadbeef\\n" + + "*** Update File: src/auth.go\\n" + + "@@\\n" + + "-x\\n" + + "+y\\n" + + "*** End Patch" + stdin := `{"session_id":"s","tool_name":"apply_patch","tool_input":{"command":"` + patch + `"},"cwd":"/repo"}` + + stdinR, stdinW, _ := os.Pipe() + stdinW.Write([]byte(stdin)) + stdinW.Close() + stdoutR, stdoutW, _ := os.Pipe() + origStdin, origStdout := os.Stdin, os.Stdout + os.Stdin, os.Stdout = stdinR, stdoutW + defer func() { + stdoutW.Close() + os.Stdin, os.Stdout = origStdin, origStdout + }() + + handlers["codex-post-tool-use"]() + stdoutW.Close() + + var buf bytes.Buffer + buf.ReadFrom(stdoutR) + out := buf.String() + if !strings.Contains(out, `"decision":"block"`) { + t.Errorf("expected PostToolBlock JSON output, got %q", out) + } + if !strings.Contains(out, "contains secrets") { + t.Errorf("expected block reason to be present, got %q", out) + } +} + +// --------------------------------------------------------------------------- +// Codex Bash → apply_patch heredoc bridging +// --------------------------------------------------------------------------- +// These tests cover the case where Codex routes a file edit through a Bash +// `apply_patch <<'PATCH' … PATCH` invocation instead of a first-class +// tool_name="apply_patch" call. The unified bridge must detect that shape +// and dispatch through OnAfterFileEdit identically to the native path — +// otherwise SecurityScanResults never get populated for Codex sessions and +// the analytics dashboard shows zero stop-hook security checks. + +func TestCodexPostToolUse_BashApplyPatchHeredocDispatches(t *testing.T) { + ClearHandlers() + defer ClearHandlers() + + var got []FileEditContext + OnAfterFileEdit(func(ctx FileEditContext) FileEditDecision { + got = append(got, ctx) + return FileEditOK() + }) + + // Multi-file patch wrapped in a real-shape Codex Bash heredoc. The \\n + // sequences become literal newlines after JSON decoding so the parser + // sees the same line breaks Codex would emit. + cmd := "apply_patch <<'PATCH'\\n" + + "*** Begin Patch\\n" + + "*** Add File: secrets/api_key.txt\\n" + + "+sk-deadbeef\\n" + + "*** Update File: src/auth.go\\n" + + "@@\\n" + + "-old\\n" + + "+new\\n" + + "*** End Patch\\n" + + "PATCH" + stdin := `{"session_id":"s","tool_name":"Bash","tool_input":{"command":"` + cmd + `"},"cwd":"/repo"}` + runHandlerWithStdin(t, "codex-post-tool-use", stdin) + + if len(got) != 2 { + t.Fatalf("handler invocations = %d, want 2 (one per file in patch); got=%+v", len(got), got) + } + if got[0].FilePath != "secrets/api_key.txt" { + t.Errorf("got[0].FilePath = %q, want %q", got[0].FilePath, "secrets/api_key.txt") + } + if got[1].FilePath != "src/auth.go" { + t.Errorf("got[1].FilePath = %q, want %q", got[1].FilePath, "src/auth.go") + } + for _, c := range got { + if c.Platform != PlatformCodex { + t.Errorf("Platform = %q, want %q", c.Platform, PlatformCodex) + } + if c.Cwd != "/repo" { + t.Errorf("Cwd = %q, want %q", c.Cwd, "/repo") + } + if c.SessionID != "s" { + t.Errorf("SessionID = %q, want %q", c.SessionID, "s") + } + } + wantEdit := FileEdit{OldString: "old", NewString: "new"} + if len(got[1].Edits) != 1 || got[1].Edits[0] != wantEdit { + t.Errorf("got[1].Edits = %+v, want [%+v]", got[1].Edits, wantEdit) + } +} + +func TestCodexPostToolUse_BashApplyPatchAbsolutePathBinaryDispatches(t *testing.T) { + // Codex sometimes invokes a per-session apply_patch shim by absolute + // path (observed in hook_events.data for real sessions). The bridge + // must still recognize this as an apply_patch invocation. + ClearHandlers() + defer ClearHandlers() + + var got []FileEditContext + OnAfterFileEdit(func(ctx FileEditContext) FileEditDecision { + got = append(got, ctx) + return FileEditOK() + }) + + cmd := "/Users/me/.codex/tmp/arg0/codex-arg0IuQk4E/apply_patch <<'PATCH'\\n" + + "*** Begin Patch\\n" + + "*** Add File: foo.txt\\n" + + "+hi\\n" + + "*** End Patch\\n" + + "PATCH" + stdin := `{"session_id":"s","tool_name":"Bash","tool_input":{"command":"` + cmd + `"},"cwd":"/repo"}` + runHandlerWithStdin(t, "codex-post-tool-use", stdin) + + if len(got) != 1 || got[0].FilePath != "foo.txt" { + t.Fatalf("got = %+v, want one invocation with FilePath=foo.txt", got) + } +} + +func TestCodexPostToolUse_BashNonApplyPatchCommandSkipped(t *testing.T) { + // A normal Bash command must not trigger the file-edit handler. + // Otherwise every `ls`, `git status`, `cat`, etc. emitted by Codex + // would be misclassified as a file edit, flooding the security + // scanner with garbage events. + ClearHandlers() + defer ClearHandlers() + + invoked := 0 + OnAfterFileEdit(func(FileEditContext) FileEditDecision { + invoked++ + return FileEditOK() + }) + + stdin := `{"session_id":"s","tool_name":"Bash","tool_input":{"command":"ls -la /tmp"},"cwd":"/repo"}` + runHandlerWithStdin(t, "codex-post-tool-use", stdin) + + if invoked != 0 { + t.Errorf("handler invoked %d times for plain Bash command, want 0", invoked) + } +} + +func TestCodexPostToolUse_BashCatHeredocWriteDispatches(t *testing.T) { + // Codex 0.130.0+ routes greenfield writes through a plain Bash + // `cat <<'EOF' > FILE … EOF` heredoc rather than apply_patch or any + // Write/Edit tool alias. Without this dispatch path, no afterFileEdit + // fires for "Create a file …" prompts and the security scanner + // never sees the new file — the exact regression that motivated + // ParseBashRedirectWrite. This test pins the wiring end-to-end. + ClearHandlers() + defer ClearHandlers() + + var got []FileEditContext + OnAfterFileEdit(func(ctx FileEditContext) FileEditDecision { + got = append(got, ctx) + return FileEditOK() + }) + + cmd := "cat <<'EOF' > greet.txt\\nhello world\\nEOF" + stdin := `{"session_id":"s","tool_name":"Bash","tool_input":{"command":"` + cmd + `"},"cwd":"/repo"}` + runHandlerWithStdin(t, "codex-post-tool-use", stdin) + + if len(got) != 1 { + t.Fatalf("handler invocations = %d, want 1; got=%+v", len(got), got) + } + if got[0].FilePath != "greet.txt" { + t.Errorf("FilePath = %q, want %q", got[0].FilePath, "greet.txt") + } + if got[0].Platform != PlatformCodex { + t.Errorf("Platform = %q, want %q", got[0].Platform, PlatformCodex) + } + if got[0].Cwd != "/repo" { + t.Errorf("Cwd = %q, want %q", got[0].Cwd, "/repo") + } + wantEdit := FileEdit{OldString: "", NewString: "hello world"} + if len(got[0].Edits) != 1 || got[0].Edits[0] != wantEdit { + t.Errorf("Edits = %+v, want [%+v]", got[0].Edits, wantEdit) + } +} + +func TestCodexPostToolUse_BashCatHeredocBlockPropagates(t *testing.T) { + // Blocking decisions on cat-heredoc writes must surface as + // PostToolBlock JSON, identical to the apply_patch path. This is + // what lets policies like "no .env files" actually stop Codex + // from creating sensitive files via greenfield writes. + ClearHandlers() + defer ClearHandlers() + + OnAfterFileEdit(func(ctx FileEditContext) FileEditDecision { + if strings.HasSuffix(ctx.FilePath, ".env") { + return FileEditBlock("no env files") + } + return FileEditOK() + }) + + cmd := "cat <<'EOF' > .env\\nSECRET=abc\\nEOF" + stdin := `{"session_id":"s","tool_name":"Bash","tool_input":{"command":"` + cmd + `"},"cwd":"/repo"}` + + stdinR, stdinW, _ := os.Pipe() + stdinW.Write([]byte(stdin)) + stdinW.Close() + stdoutR, stdoutW, _ := os.Pipe() + origStdin, origStdout := os.Stdin, os.Stdout + os.Stdin, os.Stdout = stdinR, stdoutW + defer func() { + stdoutW.Close() + os.Stdin, os.Stdout = origStdin, origStdout + }() + + handlers["codex-post-tool-use"]() + stdoutW.Close() + + var buf bytes.Buffer + buf.ReadFrom(stdoutR) + out := buf.String() + if !strings.Contains(out, `"decision":"block"`) { + t.Errorf("expected PostToolBlock JSON output, got %q", out) + } + if !strings.Contains(out, "no env files") { + t.Errorf("expected block reason in output, got %q", out) + } +} + +func TestCodexPostToolUse_BashApplyPatchBlockPropagates(t *testing.T) { + // A blocking decision from the user's handler on a Bash-heredoc + // apply_patch must emit PostToolBlock so Codex surfaces the feedback, + // identically to the native apply_patch path. + ClearHandlers() + defer ClearHandlers() + + OnAfterFileEdit(func(ctx FileEditContext) FileEditDecision { + if strings.Contains(ctx.FilePath, "secrets") { + return FileEditBlock("contains secrets") + } + return FileEditOK() + }) + + cmd := "apply_patch <<'PATCH'\\n" + + "*** Begin Patch\\n" + + "*** Add File: secrets/key.txt\\n" + + "+sk-deadbeef\\n" + + "*** End Patch\\n" + + "PATCH" + stdin := `{"session_id":"s","tool_name":"Bash","tool_input":{"command":"` + cmd + `"},"cwd":"/repo"}` + + stdinR, stdinW, _ := os.Pipe() + stdinW.Write([]byte(stdin)) + stdinW.Close() + stdoutR, stdoutW, _ := os.Pipe() + origStdin, origStdout := os.Stdin, os.Stdout + os.Stdin, os.Stdout = stdinR, stdoutW + defer func() { + stdoutW.Close() + os.Stdin, os.Stdout = origStdin, origStdout + }() + + handlers["codex-post-tool-use"]() + stdoutW.Close() + + var buf bytes.Buffer + buf.ReadFrom(stdoutR) + out := buf.String() + if !strings.Contains(out, `"decision":"block"`) { + t.Errorf("expected PostToolBlock JSON output, got %q", out) + } + if !strings.Contains(out, "contains secrets") { + t.Errorf("expected block reason to be present, got %q", out) + } +} + func TestOnStop_CursorEmptyWorkspaceRootsLeavesCwdEmpty(t *testing.T) { ClearHandlers() defer ClearHandlers()