From 289a4d902edd0a38abb0f80b192d3fd89048c960 Mon Sep 17 00:00:00 2001 From: Yijie Xu Date: Thu, 13 Aug 2026 06:07:04 +0800 Subject: [PATCH 1/5] feat(hooks): add direct Codex command rewrite --- README.md | 2 +- docs/contributing/TECHNICAL.md | 4 +- .../guide/getting-started/supported-agents.md | 10 +- hooks/README.md | 41 ++- hooks/codex/README.md | 12 +- hooks/codex/rtk-awareness.md | 4 +- src/hooks/README.md | 6 +- src/hooks/constants.rs | 2 + src/hooks/hook_cmd.rs | 211 +++++++++++ src/hooks/init.rs | 334 +++++++++++++++++- src/hooks/mod.rs | 44 ++- src/main.rs | 19 +- 12 files changed, 648 insertions(+), 41 deletions(-) diff --git a/README.md b/README.md index 420a237c2e..bfee29a3cc 100644 --- a/README.md +++ b/README.md @@ -390,7 +390,7 @@ RTK supports 16 AI coding tools. Each integration rewrites shell commands to `rt | **GitHub Copilot CLI** | `rtk init -g --copilot` | PreToolUse deny-with-suggestion (CLI limitation) | | **Cursor** | `rtk init -g --agent cursor` | preToolUse hook (hooks.json) | | **Gemini CLI** | `rtk init -g --gemini` | BeforeTool hook | -| **Codex** | `rtk init -g --codex` | AGENTS.md + RTK.md instructions | +| **Codex** | `rtk init -g --codex` | PreToolUse hook (`updatedInput`) + AGENTS.md | | **Windsurf** | `rtk init -g --agent windsurf` | .windsurfrules (project-scoped) | | **Cline / Roo Code** | `rtk init --agent cline` | .clinerules (project-scoped) | | **OpenCode** | `rtk init -g --opencode` | Plugin TS (tool.execute.before) | diff --git a/docs/contributing/TECHNICAL.md b/docs/contributing/TECHNICAL.md index 54f15422ae..9dd830a1cb 100644 --- a/docs/contributing/TECHNICAL.md +++ b/docs/contributing/TECHNICAL.md @@ -318,7 +318,7 @@ Start here, then drill down into each README for file-level details. | [`cursor/`](../hooks/cursor/README.md) | Cursor IDE | Shell hook, empty JSON response requirement | | [`cline/`](../hooks/cline/README.md) | Cline / Roo Code | Rules file (prompt-level, no programmatic hook) | | [`windsurf/`](../hooks/windsurf/README.md) | Windsurf / Cascade | Rules file (workspace-scoped) | -| [`codex/`](../hooks/codex/README.md) | OpenAI Codex CLI | Awareness document, AGENTS.md integration | +| [`codex/`](../hooks/codex/README.md) | OpenAI Codex CLI | Native `PreToolUse` processor, hooks.json registration, AGENTS.md awareness | | [`opencode/`](../hooks/opencode/README.md) | OpenCode | TypeScript plugin, zx library, in-place mutation | --- @@ -336,7 +336,7 @@ RTK supports the following LLM agents through hook integrations: | Gemini CLI | Rust binary | `rtk hook gemini` reads JSON | Yes (`hookSpecificOutput`) | | Cline/Roo Code | Rules file | Prompt-level guidance | N/A (prompt) | | Windsurf | Rules file | Prompt-level guidance | N/A (prompt) | -| Codex CLI | Awareness doc | AGENTS.md integration | N/A (prompt) | +| Codex CLI | Rust binary | `rtk hook codex` reads JSON | Yes (`updatedInput`) | | OpenCode | TS plugin | `tool.execute.before` event | Yes (in-place mutation) | > **Details**: [`hooks/README.md`](../hooks/README.md) has the full JSON schemas for each agent. [`src/hooks/README.md`](../src/hooks/README.md) covers installation, integrity verification, and the rewrite command. diff --git a/docs/guide/getting-started/supported-agents.md b/docs/guide/getting-started/supported-agents.md index 6af4995e51..addcb496e9 100644 --- a/docs/guide/getting-started/supported-agents.md +++ b/docs/guide/getting-started/supported-agents.md @@ -40,7 +40,7 @@ Agent runs "cargo test" | Factory Droid | Shell hook (`PreToolUse`, matcher `Execute`) | Yes | | Cline / Roo Code | Rules file (prompt-level) | N/A | | Windsurf | Rules file (prompt-level) | N/A | -| Codex CLI | AGENTS.md instructions | N/A | +| Codex CLI | Rust binary (`PreToolUse`) | Yes | | Kilo Code | Rules file (prompt-level) | N/A | | Google Antigravity | Rules file (prompt-level) | N/A | | Mistral Vibe | Rust binary (`pre_tool`) | Yes | @@ -177,10 +177,12 @@ rtk init --global --agent windsurf # creates .windsurfrules in current projec ### Codex CLI ```bash -rtk init --codex # project-scoped (AGENTS.md) -rtk init --global --codex # user-global (~/.codex/AGENTS.md) +rtk init --codex # project-scoped (.codex/hooks.json + AGENTS.md) +rtk init --global --codex # user-global ($CODEX_HOME or ~/.codex/) ``` +Restart Codex after installation. Project-scoped hooks must be trusted when Codex prompts. The native `rtk hook codex` processor rewrites supported `Bash` commands through `PreToolUse.updatedInput`; Codex then applies its normal approval and sandbox checks to the rewritten command. + ### Kilo Code ```bash @@ -228,7 +230,7 @@ Strips only RTK's `[[hooks]]` block and the `~/.vibe/prompts/rtk.md` file. Any o | **Plugin** | TypeScript, JavaScript, or Python in agent's plugin system | Transparent, in-place mutation when the agent allows it | | **Rules file** | Prompt-level instructions | Guidance only — agent is told to prefer `rtk ` | -Rules file integrations (Cline, Windsurf, Codex, Kilo Code, Antigravity) rely on the model following instructions. Full hook integrations (Claude Code, Cursor, Gemini) are guaranteed — the command is rewritten before the agent sees it. Plugin integrations (OpenCode, Pi) use in-place mutation via the agent's TypeScript extension API. +Rules file integrations (Cline, Windsurf, Kilo Code, Antigravity) rely on the model following instructions. Full hook integrations (Claude Code, Cursor, Gemini, Codex) rewrite the command before execution. Plugin integrations (OpenCode, Pi) use in-place mutation via the agent's TypeScript extension API. ## Windows support diff --git a/hooks/README.md b/hooks/README.md index 400f844bcd..b24e7574f3 100644 --- a/hooks/README.md +++ b/hooks/README.md @@ -2,7 +2,7 @@ ## Scope -**Deployed hook artifacts** — the actual files installed on user machines by `rtk init`. These are shell scripts, TypeScript plugins, and rules files that run outside the Rust binary. They are **thin delegates**: parse agent-specific JSON, call `rtk rewrite` as a subprocess, format agent-specific response. Zero filtering logic lives here. +**Deployed hook artifacts** — the files and agent-specific configuration installed on user machines by `rtk init`. External scripts and plugins are thin delegates to `rtk rewrite`; native processors such as Codex call the same Rust rewrite registry directly. Zero filtering logic is duplicated in this directory. Owns: per-agent hook scripts and configuration files for 10 supported agents (Claude Code, Copilot, Cursor, Cline, Windsurf, Codex, OpenCode, Hermes, Pi, Mistral Vibe). @@ -20,14 +20,14 @@ LLM agent integrations that intercept CLI commands and route them through RTK fo Agent runs command (e.g., "cargo test --nocapture") -> Hook intercepts (PreToolUse / plugin event) -> Reads JSON input, extracts command string - -> Calls `rtk rewrite "cargo test --nocapture"` + -> Uses the shared RTK rewrite registry -> Registry matches pattern, returns "rtk cargo test --nocapture" -> Hook sends response in agent-specific JSON format -> Agent executes "rtk cargo test --nocapture" instead -> Filtered output reaches LLM (up to 90% fewer bash output bytes) ``` -All rewrite logic lives in the Rust binary (`src/discover/registry.rs`). Hook scripts are **thin delegates** that handle agent-specific JSON formats and call `rtk rewrite` for the actual decision. This ensures a single source of truth for all 70+ rewrite patterns. +All rewrite logic lives in the Rust binary (`src/discover/registry.rs`). External hook scripts call `rtk rewrite`; native Rust hook processors call the registry directly. This keeps one source of truth for all 70+ rewrite patterns. ## Directory Structure @@ -38,7 +38,7 @@ Each agent subdirectory has its own README with hook-specific details: - **[`cursor/`](cursor/README.md)** — Shell hook, Cursor JSON format, empty `{}` response requirement - **[`cline/`](cline/README.md)** — Rules file (prompt-level), `.clinerules` project-local installation - **[`windsurf/`](windsurf/README.md)** — Rules file (prompt-level), `.windsurfrules` workspace-scoped -- **[`codex/`](codex/README.md)** — Awareness document, `AGENTS.md` integration, `$CODEX_HOME` or `~/.codex/` location +- **[`codex/`](codex/README.md)** — Rust binary hook (`rtk hook codex`), `PreToolUse.updatedInput`, `.codex/hooks.json` / `$CODEX_HOME/hooks.json`, plus `AGENTS.md` awareness - **[`opencode/`](opencode/README.md)** — TypeScript plugin, `zx` library, `tool.execute.before` event, in-place mutation - **[`pi/`](pi/README.md)** — TypeScript extension, `tool_call` event, local `isBashToolCallEvent` guard, in-place mutation, `~/.pi/agent/extensions/` - **[`hermes/`](hermes/README.md)** — Python plugin, `pre_tool_call` hook, in-place terminal command mutation @@ -55,7 +55,7 @@ Each agent subdirectory has its own README with hook-specific details: | Gemini CLI | Rust binary (`rtk hook gemini`) | Transparent rewrite | Yes (`hookSpecificOutput`) | | Cline / Roo Code | Custom instructions (rules file) | Prompt-level guidance | N/A | | Windsurf | Custom instructions (rules file) | Prompt-level guidance | N/A | -| Codex CLI | AGENTS.md / instructions | Prompt-level guidance | N/A | +| Codex CLI | Rust binary (`rtk hook codex`) | Transparent rewrite | Yes (`updatedInput`) | | OpenCode | TypeScript plugin (`tool.execute.before`) | In-place mutation | Yes | | Pi | TypeScript extension (`tool_call` event) | In-place mutation | Yes | | Hermes | Python plugin (`pre_tool_call`) | In-place mutation | Yes | @@ -102,6 +102,33 @@ Each agent subdirectory has its own README with hook-specific details: Returns `{}` when no rewrite (Cursor requires JSON for all paths). +### Codex CLI (Rust Binary) + +**Input** (stdin): + +```json +{ + "hook_event_name": "PreToolUse", + "tool_name": "Bash", + "tool_input": { "command": "git status" } +} +``` + +**Output** (stdout, when rewritten): + +```json +{ + "hookSpecificOutput": { + "hookEventName": "PreToolUse", + "permissionDecision": "allow", + "permissionDecisionReason": "RTK auto-rewrite", + "updatedInput": { "command": "rtk git status" } + } +} +``` + +The `allow` value is required by Codex to accept `updatedInput`; Codex still runs its native approval and sandbox checks after applying the replacement. No rewrite produces no stdout. + ### Copilot CLI (Rust Binary) **Input** (stdin, camelCase, `toolArgs` is JSON-stringified): @@ -269,9 +296,9 @@ New integrations must follow the [Exit Code Contract](#exit-code-contract) and [ | Tier | Mechanism | Maintenance | Examples | |------|-----------|-------------|----------| -| **Full hook** | Shell script or Rust binary, intercepts commands via agent's hook API | High — must track agent API changes | Claude Code, Cursor, Copilot, Gemini | +| **Full hook** | Shell script or Rust binary, intercepts commands via agent's hook API | High — must track agent API changes | Claude Code, Cursor, Copilot, Gemini, Codex | | **Plugin** | TypeScript/JS/Python plugin in agent's plugin system | Medium — agent manages loading | OpenCode, Hermes, Pi | -| **Rules file** | Prompt-level instructions the agent reads | Low — no code to break | Cline, Windsurf, Codex | +| **Rules file** | Prompt-level instructions the agent reads | Low — no code to break | Cline, Windsurf | ### Eligibility diff --git a/hooks/codex/README.md b/hooks/codex/README.md index 50030e958a..def3760ed6 100644 --- a/hooks/codex/README.md +++ b/hooks/codex/README.md @@ -4,6 +4,12 @@ ## Specifics -- Prompt-level guidance via awareness document -- no programmatic hook -- `rtk-awareness.md` is injected into `AGENTS.md` with an `@RTK.md` reference -- Installed to `$CODEX_HOME` when set, otherwise `~/.codex/`, by `rtk init --codex` +- Native Rust `PreToolUse` processor: `rtk hook codex` +- Transparently rewrites `tool_input.command` with Codex's `updatedInput` response +- Registers a `Bash` matcher in `.codex/hooks.json` (project) or `$CODEX_HOME/hooks.json` (global) +- Keeps `rtk-awareness.md` in `AGENTS.md` through an `@RTK.md` reference for RTK meta-command guidance +- Installed by `rtk init --codex` (project) or `rtk init -g --codex` (global) + +Codex requires `permissionDecision: "allow"` in the hook response for `updatedInput` to take effect. Codex applies the replacement before its normal command approval and sandbox checks, so those native checks still run on the rewritten command. + +No match, malformed JSON, unsupported commands, heredocs, substitutions, and file redirections fail open: the hook exits successfully without stdout and Codex executes the original command. diff --git a/hooks/codex/rtk-awareness.md b/hooks/codex/rtk-awareness.md index 7ae285e1a8..9ee01342cf 100644 --- a/hooks/codex/rtk-awareness.md +++ b/hooks/codex/rtk-awareness.md @@ -4,7 +4,9 @@ ## Rule -Always prefix shell commands with `rtk`. +Run shell commands normally. The Codex `PreToolUse` hook transparently routes supported commands through `rtk`; commands without an RTK filter run unchanged. + +You may invoke `rtk` explicitly for manual use, but do not add a second prefix to an already-prefixed command. Examples: diff --git a/src/hooks/README.md b/src/hooks/README.md index 586105c8ae..2e49c1aadc 100644 --- a/src/hooks/README.md +++ b/src/hooks/README.md @@ -28,7 +28,7 @@ LLM agent integration layer that installs, validates, and executes command-rewri | Claude-MD (legacy) | `rtk init --claude-md` | 134-line RTK block | CLAUDE.md | | Windsurf | `rtk init -g --agent windsurf` | `.windsurfrules` | -- | | Cline | `rtk init --agent cline` | `.clinerules` | -- | -| Codex | `rtk init --codex` | RTK.md in `$CODEX_HOME` or `~/.codex` | AGENTS.md | +| Codex | `rtk init --codex` | RTK.md + `.codex/hooks.json` (local) or `$CODEX_HOME/hooks.json` (global) | AGENTS.md + `PreToolUse` hook | | Cursor | `rtk init -g --agent cursor` | Cursor hook | hooks.json | | Pi | `rtk init --agent pi` | `.pi/extensions/rtk.ts` | -- | | Hermes | `rtk init --agent hermes` | Python plugin in `~/.hermes/plugins/rtk-rewrite/` | `config.yaml` `plugins.enabled` | @@ -89,14 +89,14 @@ Rules are loaded from all Claude Code `settings.json` files (project + global, i | Cursor (rtk hook cursor) | Ready | `permission: "ask",` — users will be prompted when Cursor enforces the permission; in the meantime, allow | | Gemini CLI (rtk hook gemini) | No (allow/deny only) | allow (limitation — no ask mode in Gemini) | | Copilot CLI (rtk hook copilot) | No updatedInput | deny-with-suggestion (unchanged) | -| Codex | ask parsed but no-op | allow (limitation — fails open) | +| Codex (`rtk hook codex`) | Native approval runs after rewrite | Emit required protocol `allow` with `updatedInput`; Codex then evaluates the rewritten command normally | | Mistral Vibe (rtk hook vibe) | No native ask surface | passthrough — Vibe's own approval prompt fires on the rewritten command | ### Implementation - `permissions.rs` — loads deny/ask/allow rules, evaluates precedence, returns `PermissionVerdict` - `rewrite_cmd.rs` — maps verdict to exit code (consumed by shell hook) -- `hook_cmd.rs` — maps verdict to JSON `permissionDecision` field (Copilot/Gemini) +- `hook_cmd.rs` — maps decisions to each agent's JSON protocol, including Codex `updatedInput` ## Exit Code Contract diff --git a/src/hooks/constants.rs b/src/hooks/constants.rs index 700e9798f4..52baf78783 100644 --- a/src/hooks/constants.rs +++ b/src/hooks/constants.rs @@ -10,6 +10,8 @@ pub const BEFORE_TOOL_KEY: &str = "BeforeTool"; /// Native Rust hook command for Claude Code (replaces rtk-rewrite.sh). pub const CLAUDE_HOOK_COMMAND: &str = "rtk hook claude"; +/// Native Rust hook command for Codex CLI. +pub const CODEX_HOOK_COMMAND: &str = "rtk hook codex"; /// Native Rust hook command for Cursor (replaces rtk-rewrite.sh). pub const CURSOR_HOOK_COMMAND: &str = "rtk hook cursor"; /// Native Rust hook command for Factory Droid. diff --git a/src/hooks/hook_cmd.rs b/src/hooks/hook_cmd.rs index 3cd58f018c..ebd80890df 100644 --- a/src/hooks/hook_cmd.rs +++ b/src/hooks/hook_cmd.rs @@ -670,6 +670,115 @@ fn run_claude_inner(input: &str) -> Option { } } +// ── Codex CLI native hook ───────────────────────────────────── + +fn process_codex_payload(v: &Value) -> PayloadAction { + if v.get("hook_event_name").and_then(Value::as_str) != Some(PRE_TOOL_USE_KEY) + || !matches!( + v.get("tool_name").and_then(Value::as_str), + Some("Bash" | "bash") + ) + { + return PayloadAction::Ignore; + } + + let cmd = match v + .pointer("/tool_input/command") + .and_then(Value::as_str) + .filter(|cmd| !cmd.is_empty()) + { + Some(cmd) => cmd, + None => return PayloadAction::Ignore, + }; + + if crate::discover::lexer::contains_unattestable_construct(cmd) { + return PayloadAction::Skip { + reason: "skip:defer", + cmd: cmd.to_string(), + }; + } + + let rewritten = match get_rewritten(cmd) { + Some(rewritten) => rewritten, + None => { + return PayloadAction::Skip { + reason: "skip:no_rewrite", + cmd: cmd.to_string(), + } + } + }; + + // Codex requires `permissionDecision: allow` alongside `updatedInput`. + // Its runtime applies the replacement before the command handler performs + // native approval and sandbox checks, so this protocol-level allow does + // not replace Codex's execution policy. + let mut updated_input = v.get("tool_input").cloned().unwrap_or_else(|| json!({})); + if let Some(obj) = updated_input.as_object_mut() { + obj.insert("command".into(), Value::String(rewritten.clone())); + } + + PayloadAction::Rewrite { + cmd: cmd.to_string(), + rewritten, + output: json!({ + "hookSpecificOutput": { + "hookEventName": PRE_TOOL_USE_KEY, + "permissionDecision": "allow", + "permissionDecisionReason": "RTK auto-rewrite", + "updatedInput": updated_input + } + }), + } +} + +/// Run the Codex CLI PreToolUse hook natively. +pub fn run_codex() -> Result<()> { + let input = match read_stdin_limited() { + Ok(input) => input, + Err(e) => { + let _ = writeln!(io::stderr(), "[rtk hook] Failed to read JSON input: {e}"); + return Ok(()); + } + }; + let input = strip_leading_bom(&input).trim(); + if input.is_empty() { + return Ok(()); + } + + let v: Value = match serde_json::from_str(input) { + Ok(v) => v, + Err(e) => { + let _ = writeln!(io::stderr(), "[rtk hook] Failed to parse JSON input: {e}"); + return Ok(()); + } + }; + + match process_codex_payload(&v) { + PayloadAction::Rewrite { + cmd, + rewritten, + output, + } => { + audit_log("rewrite", &cmd, &rewritten); + let _ = writeln!(io::stdout(), "{output}"); + } + PayloadAction::Skip { reason, cmd } => audit_log(reason, &cmd, ""), + PayloadAction::Ignore => {} + } + + Ok(()) +} + +#[cfg(test)] +fn run_codex_inner(input: &str) -> Option { + let input = strip_leading_bom(input).trim(); + let v: Value = serde_json::from_str(input).ok()?; + match process_codex_payload(&v) { + PayloadAction::Rewrite { output, .. } => Some(output.to_string()), + _ => None, + } +} + // ── Cursor native hook ───────────────────────────────────────── /// Cursor on Windows ships hook payloads with one or more leading @@ -1527,6 +1636,108 @@ mod tests { assert!(run_claude_inner(&input).is_none()); } + // --- Codex handler --- + + fn codex_input(cmd: &str) -> String { + json!({ + "session_id": "session-1", + "turn_id": "turn-1", + "hook_event_name": PRE_TOOL_USE_KEY, + "tool_name": "Bash", + "tool_use_id": "tool-1", + "tool_input": { "command": cmd } + }) + .to_string() + } + + #[test] + fn test_codex_rewrite_uses_required_allow_shape() { + let result = run_codex_inner(&codex_input("git status")).unwrap(); + let v: Value = serde_json::from_str(&result).unwrap(); + let hook = &v["hookSpecificOutput"]; + + assert_eq!(hook["hookEventName"], PRE_TOOL_USE_KEY); + assert_eq!(hook["permissionDecision"], "allow"); + assert_eq!(hook["permissionDecisionReason"], "RTK auto-rewrite"); + assert_eq!(hook["updatedInput"]["command"], "rtk git status"); + } + + #[test] + fn test_codex_rewrites_commands_that_still_need_native_approval() { + let result = run_codex_inner(&codex_input("cargo test")).unwrap(); + let v: Value = serde_json::from_str(&result).unwrap(); + + assert_eq!( + v.pointer("/hookSpecificOutput/updatedInput/command"), + Some(&json!("rtk cargo test")) + ); + } + + #[test] + fn test_codex_rewrite_preserves_tool_input_fields() { + let input = json!({ + "hook_event_name": PRE_TOOL_USE_KEY, + "tool_name": "Bash", + "tool_input": { + "command": "git status --short", + "timeout": 30_000, + "description": "Inspect the working tree" + } + }) + .to_string(); + let result = run_codex_inner(&input).unwrap(); + let v: Value = serde_json::from_str(&result).unwrap(); + let updated = &v["hookSpecificOutput"]["updatedInput"]; + + assert_eq!(updated["command"], "rtk git status --short"); + assert_eq!(updated["timeout"], 30_000); + assert_eq!(updated["description"], "Inspect the working tree"); + } + + #[test] + fn test_codex_ignores_other_events_and_tools() { + let other_event = json!({ + "hook_event_name": "PostToolUse", + "tool_name": "Bash", + "tool_input": { "command": "git status" } + }) + .to_string(); + let other_tool = json!({ + "hook_event_name": PRE_TOOL_USE_KEY, + "tool_name": "apply_patch", + "tool_input": { "command": "git status" } + }) + .to_string(); + + assert!(run_codex_inner(&other_event).is_none()); + assert!(run_codex_inner(&other_tool).is_none()); + } + + #[test] + fn test_codex_passthrough_for_unsupported_or_unattestable_commands() { + assert!(run_codex_inner(&codex_input("rtk git status")).is_none()); + assert!(run_codex_inner(&codex_input("htop")).is_none()); + assert!(run_codex_inner(&codex_input("git status > /tmp/status")).is_none()); + assert!(run_codex_inner(&codex_input("git status $(touch /tmp/x)")).is_none()); + } + + #[test] + fn test_codex_rewrites_compound_command_directly() { + let result = run_codex_inner(&codex_input("git add . && cargo test")).unwrap(); + let v: Value = serde_json::from_str(&result).unwrap(); + + assert_eq!( + v.pointer("/hookSpecificOutput/updatedInput/command"), + Some(&json!("rtk git add . && rtk cargo test")) + ); + } + + #[test] + fn test_codex_malformed_json_and_empty_command_pass_through() { + assert!(run_codex_inner("not valid json {{{").is_none()); + assert!(run_codex_inner(&codex_input("")).is_none()); + } + // --- Cursor handler --- fn cursor_input(cmd: &str) -> String { diff --git a/src/hooks/init.rs b/src/hooks/init.rs index 104811fd60..aa79fb17a0 100644 --- a/src/hooks/init.rs +++ b/src/hooks/init.rs @@ -13,16 +13,17 @@ use crate::hooks::constants::{ }; use super::constants::{ - BEFORE_TOOL_KEY, CLAUDE_DIR, CLAUDE_HOOK_COMMAND, CODEX_DIR, CURSOR_HOOK_COMMAND, DROID_DIR, - DROID_EXECUTE_MATCHER, DROID_HOME_ENV, DROID_HOOKS_FILE, DROID_HOOKS_SUBDIR, - DROID_HOOK_COMMAND, DROID_SETTINGS_FILE, GEMINI_HOOK_FILE, HERMES_DIR, HERMES_PLUGINS_SUBDIR, - HERMES_PLUGIN_INIT_FILE, HERMES_PLUGIN_MANIFEST_FILE, HERMES_PLUGIN_NAME, HOOKS_JSON, - HOOKS_SUBDIR, PI_CODING_AGENT_DIR_ENV, PI_DIR, PI_EXTENSIONS_SUBDIR, PI_LOCAL_DIR, - PI_PLUGIN_FILE, PRE_TOOL_USE_KEY, REWRITE_HOOK_FILE, SETTINGS_JSON, VIBE_BASH_MATCH, VIBE_DIR, - VIBE_HOOKS_FILE, VIBE_HOOK_COMMAND, VIBE_HOOK_NAME, VIBE_PROMPTS_SUBDIR, VIBE_PROMPT_FILE, + BEFORE_TOOL_KEY, CLAUDE_DIR, CLAUDE_HOOK_COMMAND, CODEX_DIR, CODEX_HOOK_COMMAND, + CURSOR_HOOK_COMMAND, DROID_DIR, DROID_EXECUTE_MATCHER, DROID_HOME_ENV, DROID_HOOKS_FILE, + DROID_HOOKS_SUBDIR, DROID_HOOK_COMMAND, DROID_SETTINGS_FILE, GEMINI_HOOK_FILE, HERMES_DIR, + HERMES_PLUGINS_SUBDIR, HERMES_PLUGIN_INIT_FILE, HERMES_PLUGIN_MANIFEST_FILE, + HERMES_PLUGIN_NAME, HOOKS_JSON, HOOKS_SUBDIR, PI_CODING_AGENT_DIR_ENV, PI_DIR, + PI_EXTENSIONS_SUBDIR, PI_LOCAL_DIR, PI_PLUGIN_FILE, PRE_TOOL_USE_KEY, REWRITE_HOOK_FILE, + SETTINGS_JSON, VIBE_BASH_MATCH, VIBE_DIR, VIBE_HOOKS_FILE, VIBE_HOOK_COMMAND, VIBE_HOOK_NAME, + VIBE_PROMPTS_SUBDIR, VIBE_PROMPT_FILE, }; use super::integrity; -use super::is_claude_hook_command; +use super::{is_claude_hook_command, is_codex_hook_command}; // Embedded OpenCode plugin (auto-rewrite) const OPENCODE_PLUGIN: &str = include_str!("../../hooks/opencode/rtk.ts"); @@ -901,6 +902,11 @@ fn uninstall_codex_at(codex_dir: &Path, ctx: InitContext) -> Result> let mut removed = Vec::new(); let absolute_rtk_md_ref = codex_rtk_md_ref(codex_dir); + let hooks_json_path = codex_dir.join(HOOKS_JSON); + if remove_codex_hook_from_file(&hooks_json_path, ctx)? { + removed.push(format!("hooks.json: removed {} entry", CODEX_HOOK_COMMAND)); + } + let rtk_md_path = codex_dir.join(RTK_MD); if rtk_md_path.exists() { if dry_run { @@ -2371,19 +2377,28 @@ fn normalized_yaml_scalar(value: &str) -> Option { } fn run_codex_mode(global: bool, ctx: InitContext) -> Result<()> { - let (agents_md_path, rtk_md_path) = if global { + let (agents_md_path, rtk_md_path, hooks_json_path) = if global { let codex_dir = resolve_codex_dir()?; - (codex_dir.join(AGENTS_MD), codex_dir.join(RTK_MD)) + ( + codex_dir.join(AGENTS_MD), + codex_dir.join(RTK_MD), + codex_dir.join(HOOKS_JSON), + ) } else { - (PathBuf::from(AGENTS_MD), PathBuf::from(RTK_MD)) + ( + PathBuf::from(AGENTS_MD), + PathBuf::from(RTK_MD), + PathBuf::from(CODEX_DIR).join(HOOKS_JSON), + ) }; - run_codex_mode_with_paths(agents_md_path, rtk_md_path, global, ctx) + run_codex_mode_with_paths(agents_md_path, rtk_md_path, hooks_json_path, global, ctx) } fn run_codex_mode_with_paths( agents_md_path: PathBuf, rtk_md_path: PathBuf, + hooks_json_path: PathBuf, global: bool, ctx: InitContext, ) -> Result<()> { @@ -2414,10 +2429,20 @@ fn run_codex_mode_with_paths( write_if_changed(&rtk_md_path, RTK_SLIM_CODEX, RTK_MD, ctx)?; let added_ref = patch_agents_md(&agents_md_path, &rtk_md_ref, ctx)?; + let hook_added = patch_codex_hooks_json(&hooks_json_path, ctx)?; if !dry_run { println!("\nRTK configured for Codex CLI.\n"); println!(" RTK.md: {}", rtk_md_path.display()); + println!( + " Hook: {} ({})", + hooks_json_path.display(), + if hook_added { + "registered" + } else { + "already present" + } + ); if added_ref { println!(" AGENTS.md: {} reference added", rtk_md_ref); } else { @@ -2434,11 +2459,152 @@ fn run_codex_mode_with_paths( agents_md_path.display() ); } + println!( + "\n Restart Codex. For a project hook, approve it when Codex asks you to trust it." + ); } Ok(()) } +fn codex_hook_already_present(root: &serde_json::Value) -> bool { + root.pointer("/hooks/PreToolUse") + .and_then(serde_json::Value::as_array) + .into_iter() + .flatten() + .filter_map(|entry| entry.get("hooks")?.as_array()) + .flatten() + .filter_map(|hook| hook.get("command")?.as_str()) + .any(is_codex_hook_command) +} + +fn patch_codex_hooks_json(path: &Path, ctx: InitContext) -> Result { + let InitContext { verbose, dry_run } = ctx; + let mut root = if path.exists() { + let content = fs::read_to_string(path) + .with_context(|| format!("Failed to read Codex hooks: {}", path.display()))?; + if content.trim().is_empty() { + serde_json::json!({}) + } else { + serde_json::from_str(&content) + .with_context(|| format!("Failed to parse Codex hooks: {}", path.display()))? + } + } else { + serde_json::json!({}) + }; + + if codex_hook_already_present(&root) { + return Ok(false); + } + + insert_hook_entry(&mut root, CODEX_HOOK_COMMAND)?; + let serialized = + serde_json::to_string_pretty(&root).context("Failed to serialize Codex hooks.json")?; + + if dry_run { + println!("[dry-run] would patch Codex hooks: {}", path.display()); + if verbose > 0 { + println!("[dry-run] content:\n{}", serialized); + } + return Ok(true); + } + + if let Some(parent) = path.parent() { + fs::create_dir_all(parent).with_context(|| { + format!( + "Failed to create Codex config directory: {}", + parent.display() + ) + })?; + } + if path.exists() { + fs::copy(path, path.with_extension("json.bak")) + .with_context(|| format!("Failed to backup Codex hooks: {}", path.display()))?; + } + atomic_write(path, &serialized)?; + if verbose > 0 { + eprintln!("Patched Codex hooks: {}", path.display()); + } + + Ok(true) +} + +fn remove_codex_hook_from_json(root: &mut serde_json::Value) -> bool { + let Some(pre_tool_use) = root + .pointer_mut("/hooks/PreToolUse") + .and_then(serde_json::Value::as_array_mut) + else { + return false; + }; + + let mut removed = false; + for entry in pre_tool_use.iter_mut() { + let Some(hooks) = entry + .get_mut("hooks") + .and_then(serde_json::Value::as_array_mut) + else { + continue; + }; + let before = hooks.len(); + hooks.retain(|hook| { + !hook + .get("command") + .and_then(serde_json::Value::as_str) + .is_some_and(is_codex_hook_command) + }); + removed |= hooks.len() != before; + } + pre_tool_use.retain(|entry| { + entry + .get("hooks") + .and_then(serde_json::Value::as_array) + .is_none_or(|hooks| !hooks.is_empty()) + }); + + removed +} + +fn remove_codex_hook_from_file(path: &Path, ctx: InitContext) -> Result { + let InitContext { verbose, dry_run } = ctx; + if !path.exists() { + return Ok(false); + } + + let content = fs::read_to_string(path) + .with_context(|| format!("Failed to read Codex hooks: {}", path.display()))?; + if content.trim().is_empty() { + return Ok(false); + } + + let mut root: serde_json::Value = serde_json::from_str(&content) + .with_context(|| format!("Failed to parse Codex hooks: {}", path.display()))?; + if !remove_codex_hook_from_json(&mut root) { + return Ok(false); + } + + let serialized = + serde_json::to_string_pretty(&root).context("Failed to serialize Codex hooks.json")?; + if dry_run { + println!( + "[dry-run] would remove RTK hook entry from {}", + path.display() + ); + if verbose > 0 { + println!("[dry-run] content:\n{}", serialized); + } + return Ok(true); + } + + fs::copy(path, path.with_extension("json.bak")) + .with_context(|| format!("Failed to backup Codex hooks: {}", path.display()))?; + atomic_write(path, &serialized)?; + if verbose > 0 { + eprintln!("Removed Codex RTK hook: {}", path.display()); + } + + Ok(true) +} + // --- upsert_rtk_block: idempotent RTK block management --- #[derive(Debug, Clone, Copy, PartialEq)] @@ -4100,8 +4266,8 @@ fn show_claude_config() -> Result<()> { println!(" rtk init -g --uninstall # Remove all RTK artifacts"); println!(" rtk init -g --claude-md # Legacy: full injection into ~/.claude/CLAUDE.md"); println!(" rtk init -g --hook-only # Hook only, no RTK.md"); - println!(" rtk init --codex # Configure local AGENTS.md + RTK.md"); - println!(" rtk init -g --codex # Configure $CODEX_HOME/AGENTS.md + $CODEX_HOME/RTK.md (or ~/.codex/)"); + println!(" rtk init --codex # Configure local AGENTS.md + RTK.md + hooks.json"); + println!(" rtk init -g --codex # Configure global AGENTS.md + RTK.md + hooks.json"); println!(" rtk init -g --opencode # OpenCode plugin only"); println!(" rtk init -g --agent cursor # Install Cursor Agent hooks"); @@ -4112,9 +4278,11 @@ fn show_codex_config() -> Result<()> { let codex_dir = resolve_codex_dir()?; let global_agents_md = codex_dir.join(AGENTS_MD); let global_rtk_md = codex_dir.join(RTK_MD); + let global_hooks_json = codex_dir.join(HOOKS_JSON); let global_rtk_md_ref = codex_rtk_md_ref(&codex_dir); let local_agents_md = PathBuf::from(AGENTS_MD); let local_rtk_md = PathBuf::from(RTK_MD); + let local_hooks_json = PathBuf::from(CODEX_DIR).join(HOOKS_JSON); println!("rtk Configuration (Codex CLI):\n"); @@ -4124,6 +4292,19 @@ fn show_codex_config() -> Result<()> { println!("[--] Global RTK.md: not found"); } + if global_hooks_json.exists() { + let content = fs::read_to_string(&global_hooks_json)?; + match serde_json::from_str::(&content) { + Ok(root) if codex_hook_already_present(&root) => { + println!("[ok] Global hook: {}", global_hooks_json.display()); + } + Ok(_) => println!("[--] Global hooks.json exists but RTK hook is not configured"), + Err(_) => println!("[!!] Global hooks.json is invalid JSON"), + } + } else { + println!("[--] Global hook: not found"); + } + if global_agents_md.exists() { let content = fs::read_to_string(&global_agents_md)?; if has_rtk_reference(&content, &[RTK_MD_REF, global_rtk_md_ref.as_str()]) { @@ -4143,6 +4324,19 @@ fn show_codex_config() -> Result<()> { println!("[--] Local RTK.md: not found"); } + if local_hooks_json.exists() { + let content = fs::read_to_string(&local_hooks_json)?; + match serde_json::from_str::(&content) { + Ok(root) if codex_hook_already_present(&root) => { + println!("[ok] Local hook: {}", local_hooks_json.display()); + } + Ok(_) => println!("[--] Local hooks.json exists but RTK hook is not configured"), + Err(_) => println!("[!!] Local hooks.json is invalid JSON"), + } + } else { + println!("[--] Local hook: not found"); + } + if local_agents_md.exists() { let content = fs::read_to_string(&local_agents_md)?; if has_rtk_reference(&content, &[RTK_MD_REF]) { @@ -4157,8 +4351,8 @@ fn show_codex_config() -> Result<()> { } println!("\nUsage:"); - println!(" rtk init --codex # Configure local AGENTS.md + RTK.md"); - println!(" rtk init -g --codex # Configure $CODEX_HOME/AGENTS.md + $CODEX_HOME/RTK.md (or ~/.codex/)"); + println!(" rtk init --codex # Configure local AGENTS.md + RTK.md + hooks.json"); + println!(" rtk init -g --codex # Configure global AGENTS.md + RTK.md + hooks.json"); println!(" rtk init -g --codex --uninstall # Remove global Codex RTK artifacts"); Ok(()) @@ -5941,10 +6135,12 @@ mod tests { let temp = TempDir::new().unwrap(); let agents_md = temp.path().join("AGENTS.md"); let rtk_md = temp.path().join("RTK.md"); + let hooks_json = temp.path().join(HOOKS_JSON); run_codex_mode_with_paths( agents_md.clone(), rtk_md.clone(), + hooks_json.clone(), true, InitContext::default(), ) @@ -5956,6 +6152,45 @@ mod tests { fs::read_to_string(&agents_md).unwrap(), format!("{}\n", codex_rtk_md_ref(temp.path())) ); + let hooks: serde_json::Value = + serde_json::from_str(&fs::read_to_string(&hooks_json).unwrap()).unwrap(); + assert!(codex_hook_already_present(&hooks)); + } + + #[test] + fn test_patch_codex_hooks_is_idempotent_and_preserves_existing_hooks() { + let temp = TempDir::new().unwrap(); + let hooks_json = temp.path().join(HOOKS_JSON); + fs::write( + &hooks_json, + serde_json::to_string_pretty(&serde_json::json!({ + "hooks": { + "PreToolUse": [{ + "matcher": "Bash", + "hooks": [{ "type": "command", "command": "echo existing" }] + }], + "Stop": [{ + "hooks": [{ "type": "command", "command": "echo stop" }] + }] + } + })) + .unwrap(), + ) + .unwrap(); + + assert!(patch_codex_hooks_json(&hooks_json, InitContext::default()).unwrap()); + assert!(!patch_codex_hooks_json(&hooks_json, InitContext::default()).unwrap()); + + let root: serde_json::Value = + serde_json::from_str(&fs::read_to_string(&hooks_json).unwrap()).unwrap(); + assert!(codex_hook_already_present(&root)); + assert_eq!(root["hooks"]["PreToolUse"].as_array().unwrap().len(), 2); + assert_eq!( + root["hooks"]["PreToolUse"][0]["hooks"][0]["command"], + "echo existing" + ); + assert_eq!(root["hooks"]["Stop"][0]["hooks"][0]["command"], "echo stop"); + assert!(hooks_json.with_extension("json.bak").exists()); } #[test] @@ -6372,6 +6607,66 @@ mod tests { assert!(content.contains("# Team rules")); } + #[test] + fn test_remove_codex_hook_preserves_other_hooks_in_same_entry() { + let mut root = serde_json::json!({ + "hooks": { + "PreToolUse": [{ + "matcher": "Bash", + "hooks": [ + { "type": "command", "command": "echo user hook" }, + { "type": "command", "command": CODEX_HOOK_COMMAND } + ] + }], + "Stop": [{ + "hooks": [{ "type": "command", "command": "echo stop" }] + }] + } + }); + + assert!(remove_codex_hook_from_json(&mut root)); + assert!(!codex_hook_already_present(&root)); + assert_eq!( + root["hooks"]["PreToolUse"][0]["hooks"][0]["command"], + "echo user hook" + ); + assert_eq!(root["hooks"]["Stop"][0]["hooks"][0]["command"], "echo stop"); + } + + #[test] + fn test_uninstall_codex_at_removes_hook_and_preserves_other_hooks() { + let temp = TempDir::new().unwrap(); + let hooks_json = temp.path().join(HOOKS_JSON); + fs::write( + &hooks_json, + serde_json::to_string_pretty(&serde_json::json!({ + "hooks": { + "PreToolUse": [{ + "matcher": "Bash", + "hooks": [ + { "type": "command", "command": "echo user hook" }, + { "type": "command", "command": CODEX_HOOK_COMMAND } + ] + }] + } + })) + .unwrap(), + ) + .unwrap(); + + let removed = uninstall_codex_at(temp.path(), InitContext::default()).unwrap(); + + assert_eq!(removed.len(), 1); + let root: serde_json::Value = + serde_json::from_str(&fs::read_to_string(&hooks_json).unwrap()).unwrap(); + assert!(!codex_hook_already_present(&root)); + assert_eq!( + root["hooks"]["PreToolUse"][0]["hooks"][0]["command"], + "echo user hook" + ); + assert!(hooks_json.with_extension("json.bak").exists()); + } + #[test] fn test_write_if_changed_dry_run_does_not_create_file() { let temp = TempDir::new().unwrap(); @@ -6429,10 +6724,12 @@ mod tests { let temp = TempDir::new().unwrap(); let agents_md = temp.path().join("AGENTS.md"); let rtk_md = temp.path().join("RTK.md"); + let hooks_json = temp.path().join(HOOKS_JSON); run_codex_mode_with_paths( agents_md.clone(), rtk_md.clone(), + hooks_json.clone(), true, InitContext { dry_run: true, @@ -6451,6 +6748,11 @@ mod tests { "dry-run must not create AGENTS.md: {}", agents_md.display() ); + assert!( + !hooks_json.exists(), + "dry-run must not create hooks.json: {}", + hooks_json.display() + ); } #[test] diff --git a/src/hooks/mod.rs b/src/hooks/mod.rs index fe7f4c3f1d..006bcd6784 100644 --- a/src/hooks/mod.rs +++ b/src/hooks/mod.rs @@ -12,15 +12,38 @@ pub mod rewrite_cmd; pub mod trust; pub mod verify_cmd; -pub fn is_claude_hook_command(command: &str) -> bool { +fn is_rtk_hook_command(command: &str, agent: &str) -> bool { let parts = crate::discover::lexer::shell_split(command); - let [binary, hook, claude] = parts.as_slice() else { + let [_parsed_binary, hook, target] = parts.as_slice() else { return false; }; + // shell_split treats backslashes as escapes, so use the raw first token + // for basename detection to preserve quoted Windows paths. + let command = command.trim_start(); + let binary = match command.as_bytes().first() { + Some(b'"') => command[1..] + .find('"') + .map(|end| &command[1..end + 1]) + .unwrap_or(""), + Some(b'\'') => command[1..] + .find('\'') + .map(|end| &command[1..end + 1]) + .unwrap_or(""), + Some(_) => command.split_whitespace().next().unwrap_or(""), + None => "", + }; let binary_name = binary.rsplit(['/', '\\']).next().unwrap_or(binary); - binary_name == "rtk" && hook == "hook" && claude == "claude" + matches!(binary_name, "rtk" | "rtk.exe") && hook == "hook" && target == agent +} + +pub fn is_claude_hook_command(command: &str) -> bool { + is_rtk_hook_command(command, "claude") +} + +pub fn is_codex_hook_command(command: &str) -> bool { + is_rtk_hook_command(command, "codex") } #[cfg(test)] @@ -42,4 +65,19 @@ mod tests { assert!(!is_claude_hook_command("/opt/homebrew/bin/rtk hook cursor")); assert!(!is_claude_hook_command("echo rtk hook claude")); } + + #[test] + fn codex_hook_command_matches_bare_absolute_and_windows_rtk() { + assert!(is_codex_hook_command("rtk hook codex")); + assert!(is_codex_hook_command("/opt/homebrew/bin/rtk hook codex")); + assert!(is_codex_hook_command( + "\"C:\\Program Files\\rtk.exe\" hook codex" + )); + } + + #[test] + fn codex_hook_command_rejects_other_commands() { + assert!(!is_codex_hook_command("rtk hook claude")); + assert!(!is_codex_hook_command("echo rtk hook codex")); + } } diff --git a/src/main.rs b/src/main.rs index 30ba275b38..ac2670dcfc 100644 --- a/src/main.rs +++ b/src/main.rs @@ -387,7 +387,7 @@ enum Commands { #[arg(long)] uninstall: bool, - /// Target Codex CLI (uses AGENTS.md + RTK.md, no Claude hook patching) + /// Target Codex CLI (uses PreToolUse hook + AGENTS.md + RTK.md) #[arg(long)] codex: bool, @@ -860,6 +860,8 @@ enum Commands { enum HookCommands { /// Process Claude Code PreToolUse hook (reads JSON from stdin) Claude, + /// Process Codex CLI PreToolUse hook (reads JSON from stdin) + Codex, /// Process Cursor Agent hook (reads JSON from stdin) Cursor, /// Process Gemini CLI BeforeTool hook (reads JSON from stdin) @@ -2437,6 +2439,10 @@ fn run_cli() -> Result { hooks::hook_cmd::run_claude()?; 0 } + HookCommands::Codex => { + hooks::hook_cmd::run_codex()?; + 0 + } HookCommands::Cursor => { hooks::hook_cmd::run_cursor()?; 0 @@ -3226,6 +3232,17 @@ mod tests { )); } + #[test] + fn test_hook_codex_parses() { + let cli = Cli::try_parse_from(["rtk", "hook", "codex"]).unwrap(); + assert!(matches!( + cli.command, + Commands::Hook { + command: HookCommands::Codex + } + )); + } + #[test] fn test_hook_check_parses() { let cli = Cli::try_parse_from(["rtk", "hook", "check", "git", "status"]).unwrap(); From cdddd4748f5cfba406f1f9cfa424bc0fa18dfa9c Mon Sep 17 00:00:00 2001 From: Yijie Xu Date: Mon, 17 Aug 2026 11:40:43 +0800 Subject: [PATCH 2/5] fix(hooks): address Codex integration review feedback --- .../guide/getting-started/supported-agents.md | 2 + hooks/README.md | 3 +- hooks/codex/README.md | 5 +- src/hooks/hook_cmd.rs | 74 +++++++++++++++-- src/hooks/init.rs | 82 ++++++++++++++----- src/hooks/mod.rs | 52 ++++++++---- 6 files changed, 169 insertions(+), 49 deletions(-) diff --git a/docs/guide/getting-started/supported-agents.md b/docs/guide/getting-started/supported-agents.md index addcb496e9..77910c071d 100644 --- a/docs/guide/getting-started/supported-agents.md +++ b/docs/guide/getting-started/supported-agents.md @@ -179,6 +179,8 @@ rtk init --global --agent windsurf # creates .windsurfrules in current projec ```bash rtk init --codex # project-scoped (.codex/hooks.json + AGENTS.md) rtk init --global --codex # user-global ($CODEX_HOME or ~/.codex/) +rtk init --codex --uninstall # remove project-scoped integration +rtk init --global --codex --uninstall # remove user-global integration ``` Restart Codex after installation. Project-scoped hooks must be trusted when Codex prompts. The native `rtk hook codex` processor rewrites supported `Bash` commands through `PreToolUse.updatedInput`; Codex then applies its normal approval and sandbox checks to the rewritten command. diff --git a/hooks/README.md b/hooks/README.md index b24e7574f3..f9e3e95344 100644 --- a/hooks/README.md +++ b/hooks/README.md @@ -110,6 +110,7 @@ Returns `{}` when no rewrite (Cursor requires JSON for all paths). { "hook_event_name": "PreToolUse", "tool_name": "Bash", + "permission_mode": "default", "tool_input": { "command": "git status" } } ``` @@ -127,7 +128,7 @@ Returns `{}` when no rewrite (Cursor requires JSON for all paths). } ``` -The `allow` value is required by Codex to accept `updatedInput`; Codex still runs its native approval and sandbox checks after applying the replacement. No rewrite produces no stdout. +The `allow` value is required by Codex to accept `updatedInput`; Codex still runs its native approval and sandbox checks after applying the replacement. Those checks classify the rewritten command, and Codex does not currently unwrap the `rtk` binary for its safe/dangerous-command heuristics. Missing or unknown permission modes and other no-rewrite cases produce no stdout. ### Copilot CLI (Rust Binary) diff --git a/hooks/codex/README.md b/hooks/codex/README.md index def3760ed6..ade15bf07b 100644 --- a/hooks/codex/README.md +++ b/hooks/codex/README.md @@ -9,7 +9,8 @@ - Registers a `Bash` matcher in `.codex/hooks.json` (project) or `$CODEX_HOME/hooks.json` (global) - Keeps `rtk-awareness.md` in `AGENTS.md` through an `@RTK.md` reference for RTK meta-command guidance - Installed by `rtk init --codex` (project) or `rtk init -g --codex` (global) +- Uninstalled by adding `--uninstall` to the corresponding project or global command -Codex requires `permissionDecision: "allow"` in the hook response for `updatedInput` to take effect. Codex applies the replacement before its normal command approval and sandbox checks, so those native checks still run on the rewritten command. +Codex requires `permissionDecision: "allow"` in the hook response for `updatedInput` to take effect. Codex applies the replacement before its normal command approval and sandbox checks, so those native checks still run on the rewritten command. Codex's command safety classifier does not currently unwrap the `rtk` binary, so classification is based on the rewritten command. This can add prompts for known-safe commands or obscure signals for wrapped mutating commands such as `git push`. -No match, malformed JSON, unsupported commands, heredocs, substitutions, and file redirections fail open: the hook exits successfully without stdout and Codex executes the original command. +RTK rewrites the documented Codex permission modes. Missing or unknown permission modes, no match, malformed JSON, unsupported commands, heredocs, substitutions, and file redirections fail open: the hook exits successfully without stdout and Codex executes the original command. diff --git a/src/hooks/hook_cmd.rs b/src/hooks/hook_cmd.rs index ebd80890df..5bb5dae4d3 100644 --- a/src/hooks/hook_cmd.rs +++ b/src/hooks/hook_cmd.rs @@ -672,6 +672,13 @@ fn run_claude_inner(input: &str) -> Option { // ── Codex CLI native hook ───────────────────────────────────── +fn is_supported_codex_permission_mode(v: &Value) -> bool { + matches!( + v.get("permission_mode").and_then(Value::as_str), + Some("default" | "acceptEdits" | "plan" | "dontAsk" | "bypassPermissions") + ) +} + fn process_codex_payload(v: &Value) -> PayloadAction { if v.get("hook_event_name").and_then(Value::as_str) != Some(PRE_TOOL_USE_KEY) || !matches!( @@ -691,6 +698,15 @@ fn process_codex_payload(v: &Value) -> PayloadAction { None => return PayloadAction::Ignore, }; + // Codex deliberately includes this field in every hook event. Fail open + // for missing or future modes instead of assuming their approval semantics. + if !is_supported_codex_permission_mode(v) { + return PayloadAction::Skip { + reason: "skip:unsupported_permission_mode", + cmd: cmd.to_string(), + }; + } + if crate::discover::lexer::contains_unattestable_construct(cmd) { return PayloadAction::Skip { reason: "skip:defer", @@ -709,9 +725,13 @@ fn process_codex_payload(v: &Value) -> PayloadAction { }; // Codex requires `permissionDecision: allow` alongside `updatedInput`. - // Its runtime applies the replacement before the command handler performs - // native approval and sandbox checks, so this protocol-level allow does - // not replace Codex's execution policy. + // Its runtime applies the replacement before native approval and sandbox + // checks, so this is a protocol-level allow rather than RTK approving the + // command. Those checks inspect the rewritten argv, however, and Codex's + // safe/dangerous-command classifiers do not currently unwrap `rtk`. This + // can add prompts for known-safe commands and obscure classifier signals + // for wrapped mutating commands such as git push. Keep the passthrough gates + // above conservative and revisit this boundary whenever coverage expands. let mut updated_input = v.get("tool_input").cloned().unwrap_or_else(|| json!({})); if let Some(obj) = updated_input.as_object_mut() { obj.insert("command".into(), Value::String(rewritten.clone())); @@ -1638,16 +1658,23 @@ mod tests { // --- Codex handler --- - fn codex_input(cmd: &str) -> String { - json!({ + fn codex_input_with_permission_mode(cmd: &str, permission_mode: Option<&str>) -> String { + let mut input = json!({ "session_id": "session-1", "turn_id": "turn-1", "hook_event_name": PRE_TOOL_USE_KEY, "tool_name": "Bash", "tool_use_id": "tool-1", "tool_input": { "command": cmd } - }) - .to_string() + }); + if let Some(permission_mode) = permission_mode { + input["permission_mode"] = json!(permission_mode); + } + input.to_string() + } + + fn codex_input(cmd: &str) -> String { + codex_input_with_permission_mode(cmd, Some("default")) } #[test] @@ -1662,6 +1689,36 @@ mod tests { assert_eq!(hook["updatedInput"]["command"], "rtk git status"); } + #[test] + fn test_codex_rewrites_all_documented_permission_modes() { + for permission_mode in [ + "default", + "acceptEdits", + "plan", + "dontAsk", + "bypassPermissions", + ] { + assert!( + run_codex_inner(&codex_input_with_permission_mode( + "git status", + Some(permission_mode) + )) + .is_some(), + "documented permission mode should rewrite: {permission_mode}" + ); + } + } + + #[test] + fn test_codex_unknown_or_missing_permission_mode_passes_through() { + assert!(run_codex_inner(&codex_input_with_permission_mode("git status", None)).is_none()); + assert!(run_codex_inner(&codex_input_with_permission_mode( + "git status", + Some("futureMode") + )) + .is_none()); + } + #[test] fn test_codex_rewrites_commands_that_still_need_native_approval() { let result = run_codex_inner(&codex_input("cargo test")).unwrap(); @@ -1678,6 +1735,7 @@ mod tests { let input = json!({ "hook_event_name": PRE_TOOL_USE_KEY, "tool_name": "Bash", + "permission_mode": "default", "tool_input": { "command": "git status --short", "timeout": 30_000, @@ -1717,6 +1775,8 @@ mod tests { fn test_codex_passthrough_for_unsupported_or_unattestable_commands() { assert!(run_codex_inner(&codex_input("rtk git status")).is_none()); assert!(run_codex_inner(&codex_input("htop")).is_none()); + assert!(run_codex_inner(&codex_input("rm -rf /tmp/rtk-safety-test")).is_none()); + assert!(run_codex_inner(&codex_input("sudo rm -rf /tmp/rtk-safety-test")).is_none()); assert!(run_codex_inner(&codex_input("git status > /tmp/status")).is_none()); assert!(run_codex_inner(&codex_input("git status $(touch /tmp/x)")).is_none()); } diff --git a/src/hooks/init.rs b/src/hooks/init.rs index aa79fb17a0..f333c25644 100644 --- a/src/hooks/init.rs +++ b/src/hooks/init.rs @@ -871,14 +871,18 @@ pub fn uninstall( fn uninstall_codex(global: bool, ctx: InitContext) -> Result<()> { let InitContext { dry_run, .. } = ctx; - if !global { - anyhow::bail!( - "Uninstall only works with --global flag. For local projects, manually remove RTK from AGENTS.md" - ); - } - - let codex_dir = resolve_codex_dir()?; - let removed = uninstall_codex_at(&codex_dir, ctx)?; + let removed = if global { + let codex_dir = resolve_codex_dir()?; + uninstall_codex_at(&codex_dir, ctx)? + } else { + uninstall_codex_with_paths( + Path::new(AGENTS_MD), + Path::new(RTK_MD), + &Path::new(CODEX_DIR).join(HOOKS_JSON), + &[RTK_MD_REF], + ctx, + )? + }; if removed.is_empty() { println!("RTK was not installed for Codex CLI (nothing to remove)"); @@ -898,21 +902,35 @@ fn uninstall_codex(global: bool, ctx: InitContext) -> Result<()> { } fn uninstall_codex_at(codex_dir: &Path, ctx: InitContext) -> Result> { + let absolute_rtk_md_ref = codex_rtk_md_ref(codex_dir); + uninstall_codex_with_paths( + &codex_dir.join(AGENTS_MD), + &codex_dir.join(RTK_MD), + &codex_dir.join(HOOKS_JSON), + &[RTK_MD_REF, absolute_rtk_md_ref.as_str()], + ctx, + ) +} + +fn uninstall_codex_with_paths( + agents_md_path: &Path, + rtk_md_path: &Path, + hooks_json_path: &Path, + rtk_md_refs: &[&str], + ctx: InitContext, +) -> Result> { let InitContext { verbose, dry_run } = ctx; let mut removed = Vec::new(); - let absolute_rtk_md_ref = codex_rtk_md_ref(codex_dir); - let hooks_json_path = codex_dir.join(HOOKS_JSON); - if remove_codex_hook_from_file(&hooks_json_path, ctx)? { + if remove_codex_hook_from_file(hooks_json_path, ctx)? { removed.push(format!("hooks.json: removed {} entry", CODEX_HOOK_COMMAND)); } - let rtk_md_path = codex_dir.join(RTK_MD); if rtk_md_path.exists() { if dry_run { println!("[dry-run] would remove RTK.md: {}", rtk_md_path.display()); } else { - fs::remove_file(&rtk_md_path) + fs::remove_file(rtk_md_path) .with_context(|| format!("Failed to remove RTK.md: {}", rtk_md_path.display()))?; if verbose > 0 { eprintln!("Removed RTK.md: {}", rtk_md_path.display()); @@ -921,9 +939,8 @@ fn uninstall_codex_at(codex_dir: &Path, ctx: InitContext) -> Result> removed.push(format!("RTK.md: {}", rtk_md_path.display())); } - let agents_md_path = codex_dir.join(AGENTS_MD); if agents_md_path.exists() { - let content = fs::read_to_string(&agents_md_path) + let content = fs::read_to_string(agents_md_path) .with_context(|| format!("Failed to read AGENTS.md: {}", agents_md_path.display()))?; let mut working_content = content.clone(); @@ -939,17 +956,13 @@ fn uninstall_codex_at(codex_dir: &Path, ctx: InitContext) -> Result> } if agents_changed { - atomic_write(&agents_md_path, &working_content).with_context(|| { + atomic_write(agents_md_path, &working_content).with_context(|| { format!("Failed to write AGENTS.md: {}", agents_md_path.display()) })?; } } - if remove_rtk_reference_from_agents( - &agents_md_path, - &[RTK_MD_REF, absolute_rtk_md_ref.as_str()], - ctx, - )? { + if remove_rtk_reference_from_agents(agents_md_path, rtk_md_refs, ctx)? { removed.push("AGENTS.md: removed @RTK.md reference".to_string()); } @@ -4353,6 +4366,7 @@ fn show_codex_config() -> Result<()> { println!("\nUsage:"); println!(" rtk init --codex # Configure local AGENTS.md + RTK.md + hooks.json"); println!(" rtk init -g --codex # Configure global AGENTS.md + RTK.md + hooks.json"); + println!(" rtk init --codex --uninstall # Remove local Codex RTK artifacts"); println!(" rtk init -g --codex --uninstall # Remove global Codex RTK artifacts"); Ok(()) @@ -6588,6 +6602,32 @@ mod tests { assert!(content.contains("# Team rules")); } + #[test] + fn test_local_codex_install_can_be_uninstalled() { + let _cwd_guard = CWD_LOCK.lock().unwrap_or_else(|e| e.into_inner()); + let temp = TempDir::new().unwrap(); + let original_cwd = std::env::current_dir().unwrap(); + std::env::set_current_dir(temp.path()).unwrap(); + + fs::write(AGENTS_MD, "# Team rules\n").unwrap(); + run_codex_mode(false, InitContext::default()).unwrap(); + let uninstall_result = uninstall_codex(false, InitContext::default()); + + std::env::set_current_dir(original_cwd).unwrap(); + uninstall_result.unwrap(); + + assert!(!temp.path().join(RTK_MD).exists()); + assert!(!codex_hook_already_present( + &serde_json::from_str( + &fs::read_to_string(temp.path().join(CODEX_DIR).join(HOOKS_JSON)).unwrap() + ) + .unwrap() + )); + let agents = fs::read_to_string(temp.path().join(AGENTS_MD)).unwrap(); + assert_eq!(agents.trim_end(), "# Team rules"); + assert!(!agents.contains(RTK_MD_REF)); + } + #[test] fn test_uninstall_codex_at_removes_absolute_reference() { let temp = TempDir::new().unwrap(); diff --git a/src/hooks/mod.rs b/src/hooks/mod.rs index 006bcd6784..d699bae085 100644 --- a/src/hooks/mod.rs +++ b/src/hooks/mod.rs @@ -12,30 +12,42 @@ pub mod rewrite_cmd; pub mod trust; pub mod verify_cmd; +fn is_rtk_binary(binary: &str) -> bool { + let binary_name = binary.rsplit(['/', '\\']).next().unwrap_or(binary); + matches!(binary_name, "rtk" | "rtk.exe") +} + +fn raw_first_token(command: &str) -> Option<&str> { + let command = command.trim_start(); + let quote = match command.as_bytes().first() { + Some(b'"') => Some('"'), + Some(b'\'') => Some('\''), + Some(_) => return command.split_whitespace().next(), + None => return None, + }?; + + let quoted = &command[1..]; + let end = quoted.find(quote)?; + let suffix = "ed[end + quote.len_utf8()..]; + if suffix.chars().next().is_some_and(|ch| !ch.is_whitespace()) { + return None; + } + Some("ed[..end]) +} + fn is_rtk_hook_command(command: &str, agent: &str) -> bool { let parts = crate::discover::lexer::shell_split(command); - let [_parsed_binary, hook, target] = parts.as_slice() else { + let [parsed_binary, hook, target] = parts.as_slice() else { return false; }; - // shell_split treats backslashes as escapes, so use the raw first token - // for basename detection to preserve quoted Windows paths. - let command = command.trim_start(); - let binary = match command.as_bytes().first() { - Some(b'"') => command[1..] - .find('"') - .map(|end| &command[1..end + 1]) - .unwrap_or(""), - Some(b'\'') => command[1..] - .find('\'') - .map(|end| &command[1..end + 1]) - .unwrap_or(""), - Some(_) => command.split_whitespace().next().unwrap_or(""), - None => "", - }; - let binary_name = binary.rsplit(['/', '\\']).next().unwrap_or(binary); + // Prefer the shell-parsed token so POSIX escaped spaces are resolved. + // Fall back to the raw token because shell_split treats Windows path + // backslashes as escapes. + let has_rtk_binary = + is_rtk_binary(parsed_binary) || raw_first_token(command).is_some_and(is_rtk_binary); - matches!(binary_name, "rtk" | "rtk.exe") && hook == "hook" && target == agent + has_rtk_binary && hook == "hook" && target == agent } pub fn is_claude_hook_command(command: &str) -> bool { @@ -57,6 +69,9 @@ mod tests { assert!(is_claude_hook_command( "\"/opt/homebrew/bin/rtk\" hook claude" )); + assert!(is_claude_hook_command( + "/Users/jane/My\\ Apps/rtk hook claude" + )); } #[test] @@ -79,5 +94,6 @@ mod tests { fn codex_hook_command_rejects_other_commands() { assert!(!is_codex_hook_command("rtk hook claude")); assert!(!is_codex_hook_command("echo rtk hook codex")); + assert!(!is_codex_hook_command("\"rtk\"evil hook codex")); } } From 53fc1b61471c1e287072356c6a46d23a7338cb45 Mon Sep 17 00:00:00 2001 From: Yijie Xu Date: Thu, 20 Aug 2026 12:04:03 +0800 Subject: [PATCH 3/5] refactor(hooks): centralize hook JSON file I/O --- src/hooks/hook_cmd.rs | 2 + src/hooks/init.rs | 315 ++++++++++++++++++++++++++---------------- 2 files changed, 196 insertions(+), 121 deletions(-) diff --git a/src/hooks/hook_cmd.rs b/src/hooks/hook_cmd.rs index 5bb5dae4d3..bcd5568e62 100644 --- a/src/hooks/hook_cmd.rs +++ b/src/hooks/hook_cmd.rs @@ -1673,6 +1673,8 @@ mod tests { input.to_string() } + // Normal Codex payloads include `default`; missing permission_mode is + // covered deliberately by the dedicated fail-open test below. fn codex_input(cmd: &str) -> String { codex_input_with_permission_mode(cmd, Some("default")) } diff --git a/src/hooks/init.rs b/src/hooks/init.rs index f333c25644..9cf2c6505e 100644 --- a/src/hooks/init.rs +++ b/src/hooks/init.rs @@ -434,6 +434,45 @@ fn atomic_write(path: &Path, content: &str) -> Result<()> { Ok(()) } +/// Read a JSON file with path-aware errors. Missing files return `None` and +/// empty files are treated as an empty JSON object. +fn read_json_file(path: &Path) -> Result> { + if !path.exists() { + return Ok(None); + } + + let content = + fs::read_to_string(path).with_context(|| format!("Failed to read {}", path.display()))?; + if content.trim().is_empty() { + return Ok(Some(serde_json::json!({}))); + } + + serde_json::from_str(&content) + .map(Some) + .with_context(|| format!("Failed to parse {} as JSON", path.display())) +} + +/// Back up an existing JSON file before replacing it atomically. +fn backup_and_atomic_write(path: &Path, content: &str) -> Result> { + let backup_path = if path.exists() { + let backup_path = path.with_extension("json.bak"); + fs::copy(path, &backup_path).with_context(|| { + format!( + "Failed to backup {} to {}", + path.display(), + backup_path.display() + ) + })?; + Some(backup_path) + } else { + None + }; + + atomic_write(path, content) + .with_context(|| format!("Failed to update JSON file: {}", path.display()))?; + Ok(backup_path) +} + /// Prompt user for consent to patch settings.json /// Prints to stderr (stdout may be piped), reads from stdin /// Default is No (capital N) @@ -930,6 +969,7 @@ fn uninstall_codex_with_paths( if dry_run { println!("[dry-run] would remove RTK.md: {}", rtk_md_path.display()); } else { + // nosemgrep: filesystem-deletion fs::remove_file(rtk_md_path) .with_context(|| format!("Failed to remove RTK.md: {}", rtk_md_path.display()))?; if verbose > 0 { @@ -2493,18 +2533,7 @@ fn codex_hook_already_present(root: &serde_json::Value) -> bool { fn patch_codex_hooks_json(path: &Path, ctx: InitContext) -> Result { let InitContext { verbose, dry_run } = ctx; - let mut root = if path.exists() { - let content = fs::read_to_string(path) - .with_context(|| format!("Failed to read Codex hooks: {}", path.display()))?; - if content.trim().is_empty() { - serde_json::json!({}) - } else { - serde_json::from_str(&content) - .with_context(|| format!("Failed to parse Codex hooks: {}", path.display()))? - } - } else { - serde_json::json!({}) - }; + let mut root = read_json_file(path)?.unwrap_or_else(|| serde_json::json!({})); if codex_hook_already_present(&root) { return Ok(false); @@ -2530,11 +2559,7 @@ fn patch_codex_hooks_json(path: &Path, ctx: InitContext) -> Result { ) })?; } - if path.exists() { - fs::copy(path, path.with_extension("json.bak")) - .with_context(|| format!("Failed to backup Codex hooks: {}", path.display()))?; - } - atomic_write(path, &serialized)?; + backup_and_atomic_write(path, &serialized)?; if verbose > 0 { eprintln!("Patched Codex hooks: {}", path.display()); } @@ -2579,18 +2604,9 @@ fn remove_codex_hook_from_json(root: &mut serde_json::Value) -> bool { fn remove_codex_hook_from_file(path: &Path, ctx: InitContext) -> Result { let InitContext { verbose, dry_run } = ctx; - if !path.exists() { - return Ok(false); - } - - let content = fs::read_to_string(path) - .with_context(|| format!("Failed to read Codex hooks: {}", path.display()))?; - if content.trim().is_empty() { + let Some(mut root) = read_json_file(path)? else { return Ok(false); - } - - let mut root: serde_json::Value = serde_json::from_str(&content) - .with_context(|| format!("Failed to parse Codex hooks: {}", path.display()))?; + }; if !remove_codex_hook_from_json(&mut root) { return Ok(false); } @@ -2608,9 +2624,7 @@ fn remove_codex_hook_from_file(path: &Path, ctx: InitContext) -> Result { return Ok(true); } - fs::copy(path, path.with_extension("json.bak")) - .with_context(|| format!("Failed to backup Codex hooks: {}", path.display()))?; - atomic_write(path, &serialized)?; + backup_and_atomic_write(path, &serialized)?; if verbose > 0 { eprintln!("Removed Codex RTK hook: {}", path.display()); } @@ -3129,22 +3143,6 @@ fn droid_hook_file_candidates(droid_dir: &Path) -> [DroidHookFile; 3] { ] } -/// Read a Droid config file as JSON. `Ok(None)` when the file doesn't exist; -/// an empty file parses as `{}`. -fn read_droid_json(path: &Path) -> Result> { - if !path.exists() { - return Ok(None); - } - let content = - fs::read_to_string(path).with_context(|| format!("Failed to read {}", path.display()))?; - if content.trim().is_empty() { - return Ok(Some(serde_json::json!({}))); - } - serde_json::from_str(&content) - .map(Some) - .with_context(|| format!("Failed to parse {} as JSON", path.display())) -} - /// The JSON object holding hook events for the given layout, if present. fn droid_events(root: &serde_json::Value, layout: DroidLayout) -> &serde_json::Value { match layout { @@ -3187,7 +3185,7 @@ fn resolve_droid_install_target(droid_dir: &Path) -> Result { }; if let Some(path) = &live_hooks_json { - if let Some(json) = read_droid_json(path)? { + if let Some(json) = read_json_file(path)? { if droid_has_pre_tool_use(&json, DroidLayout::Root) { return Ok(DroidHookFile { path: path.clone(), @@ -3197,7 +3195,7 @@ fn resolve_droid_install_target(droid_dir: &Path) -> Result { } } - if let Some(json) = read_droid_json(&settings)? { + if let Some(json) = read_json_file(&settings)? { if droid_has_pre_tool_use(&json, DroidLayout::Nested) { return Ok(DroidHookFile { path: settings, @@ -3281,7 +3279,7 @@ fn run_droid_mode_at(droid_dir: &Path, global: bool, ctx: InitContext) -> Result fn patch_droid_hook_file(file: &DroidHookFile, ctx: InitContext) -> Result { let InitContext { verbose, dry_run } = ctx; let path = &file.path; - let mut root = read_droid_json(path)?.unwrap_or_else(|| serde_json::json!({})); + let mut root = read_json_file(path)?.unwrap_or_else(|| serde_json::json!({})); if droid_hook_already_present(&root, file.layout) { if verbose > 0 { @@ -3303,16 +3301,11 @@ fn patch_droid_hook_file(file: &DroidHookFile, ctx: InitContext) -> Result return Ok(true); } - if path.exists() { - let backup_path = path.with_extension("json.bak"); - fs::copy(path, &backup_path) - .with_context(|| format!("Failed to backup to {}", backup_path.display()))?; + if let Some(backup_path) = backup_and_atomic_write(path, &serialized)? { if verbose > 0 { eprintln!("Backup: {}", backup_path.display()); } } - - atomic_write(path, &serialized)?; Ok(true) } @@ -3440,7 +3433,7 @@ fn remove_droid_hook_from_file(file: &DroidHookFile, ctx: InitContext) -> Result let InitContext { verbose, dry_run } = ctx; let path = &file.path; - let mut root = match read_droid_json(path)? { + let mut root = match read_json_file(path)? { Some(v) => v, None => return Ok(false), }; @@ -3455,12 +3448,9 @@ fn remove_droid_hook_from_file(file: &DroidHookFile, ctx: InitContext) -> Result path.display() ); } else { - let backup_path = path.with_extension("json.bak"); - fs::copy(path, &backup_path).ok(); - let serialized = serde_json::to_string_pretty(&root).context("Failed to serialize Droid hook file")?; - atomic_write(path, &serialized)?; + backup_and_atomic_write(path, &serialized)?; if verbose > 0 { eprintln!("Removed RTK hook from {}", path.display()); @@ -3798,18 +3788,7 @@ fn install_cursor_hooks(ctx: InitContext) -> Result<()> { /// Returns true if the file was modified. fn patch_cursor_hooks_json(path: &Path, ctx: InitContext) -> Result { let InitContext { verbose, dry_run } = ctx; - let mut root = if path.exists() { - let content = fs::read_to_string(path) - .with_context(|| format!("Failed to read {}", path.display()))?; - if content.trim().is_empty() { - serde_json::json!({ "version": 1 }) - } else { - serde_json::from_str(&content) - .with_context(|| format!("Failed to parse {} as JSON", path.display()))? - } - } else { - serde_json::json!({ "version": 1 }) - }; + let mut root = read_json_file(path)?.unwrap_or_else(|| serde_json::json!({ "version": 1 })); // Check idempotency if cursor_hook_already_present(&root) { @@ -3835,19 +3814,12 @@ fn patch_cursor_hooks_json(path: &Path, ctx: InitContext) -> Result { return Ok(true); } - // Backup if exists - if path.exists() { - let backup_path = path.with_extension("json.bak"); - fs::copy(path, &backup_path) - .with_context(|| format!("Failed to backup to {}", backup_path.display()))?; + if let Some(backup_path) = backup_and_atomic_write(path, &serialized)? { if verbose > 0 { eprintln!("Backup: {}", backup_path.display()); } } - // Atomic write - atomic_write(path, &serialized)?; - Ok(true) } @@ -3906,18 +3878,9 @@ fn insert_cursor_hook_entry(root: &mut serde_json::Value) -> Result<()> { /// Preserves any existing `rtk hook cursor` entries (new format). fn remove_legacy_cursor_hooks_json_entries(path: &Path, ctx: InitContext) -> Result<()> { let InitContext { verbose, dry_run } = ctx; - if !path.exists() { + let Some(mut root) = read_json_file(path)? else { return Ok(()); - } - - let content = - fs::read_to_string(path).with_context(|| format!("Failed to read {}", path.display()))?; - if content.trim().is_empty() { - return Ok(()); - } - - let mut root: serde_json::Value = serde_json::from_str(&content) - .with_context(|| format!("Failed to parse {}", path.display()))?; + }; if !remove_legacy_cursor_hook_entries_from_json(&mut root) { return Ok(()); @@ -3933,7 +3896,7 @@ fn remove_legacy_cursor_hooks_json_entries(path: &Path, ctx: InitContext) -> Res let serialized = serde_json::to_string_pretty(&root).context("Failed to serialize hooks.json")?; - atomic_write(path, &serialized)?; + backup_and_atomic_write(path, &serialized)?; if verbose > 0 { eprintln!(" [ok] Removed legacy rtk-rewrite.sh entry from Cursor hooks.json"); @@ -3990,33 +3953,23 @@ fn remove_cursor_hooks(ctx: InitContext) -> Result> { // 2. Remove RTK entry from hooks.json let hooks_json_path = cursor_dir.join(HOOKS_JSON); - if hooks_json_path.exists() { - let content = fs::read_to_string(&hooks_json_path) - .with_context(|| format!("Failed to read {}", hooks_json_path.display()))?; - - if !content.trim().is_empty() { - if let Ok(mut root) = serde_json::from_str::(&content) { - if remove_cursor_hook_from_json(&mut root) { - if dry_run { - println!( - "[dry-run] would remove RTK entry from Cursor hooks.json: {}", - hooks_json_path.display() - ); - } else { - let backup_path = hooks_json_path.with_extension("json.bak"); - fs::copy(&hooks_json_path, &backup_path).ok(); - - let serialized = serde_json::to_string_pretty(&root) - .context("Failed to serialize hooks.json")?; - atomic_write(&hooks_json_path, &serialized)?; + if let Some(mut root) = read_json_file(&hooks_json_path)? { + if remove_cursor_hook_from_json(&mut root) { + if dry_run { + println!( + "[dry-run] would remove RTK entry from Cursor hooks.json: {}", + hooks_json_path.display() + ); + } else { + let serialized = serde_json::to_string_pretty(&root) + .context("Failed to serialize hooks.json")?; + backup_and_atomic_write(&hooks_json_path, &serialized)?; - if verbose > 0 { - eprintln!("Removed RTK hook from Cursor hooks.json"); - } - } - removed.push("Cursor hooks.json: removed RTK entry".to_string()); + if verbose > 0 { + eprintln!("Removed RTK hook from Cursor hooks.json"); } } + removed.push("Cursor hooks.json: removed RTK entry".to_string()); } } @@ -4306,7 +4259,12 @@ fn show_codex_config() -> Result<()> { } if global_hooks_json.exists() { - let content = fs::read_to_string(&global_hooks_json)?; + let content = fs::read_to_string(&global_hooks_json).with_context(|| { + format!( + "Failed to read global Codex hooks: {}", + global_hooks_json.display() + ) + })?; match serde_json::from_str::(&content) { Ok(root) if codex_hook_already_present(&root) => { println!("[ok] Global hook: {}", global_hooks_json.display()); @@ -4319,7 +4277,12 @@ fn show_codex_config() -> Result<()> { } if global_agents_md.exists() { - let content = fs::read_to_string(&global_agents_md)?; + let content = fs::read_to_string(&global_agents_md).with_context(|| { + format!( + "Failed to read global Codex instructions: {}", + global_agents_md.display() + ) + })?; if has_rtk_reference(&content, &[RTK_MD_REF, global_rtk_md_ref.as_str()]) { println!("[ok] Global AGENTS.md: RTK.md reference"); } else if content.contains(RTK_BLOCK_START) { @@ -4338,7 +4301,12 @@ fn show_codex_config() -> Result<()> { } if local_hooks_json.exists() { - let content = fs::read_to_string(&local_hooks_json)?; + let content = fs::read_to_string(&local_hooks_json).with_context(|| { + format!( + "Failed to read local Codex hooks: {}", + local_hooks_json.display() + ) + })?; match serde_json::from_str::(&content) { Ok(root) if codex_hook_already_present(&root) => { println!("[ok] Local hook: {}", local_hooks_json.display()); @@ -4351,7 +4319,12 @@ fn show_codex_config() -> Result<()> { } if local_agents_md.exists() { - let content = fs::read_to_string(&local_agents_md)?; + let content = fs::read_to_string(&local_agents_md).with_context(|| { + format!( + "Failed to read local Codex instructions: {}", + local_agents_md.display() + ) + })?; if has_rtk_reference(&content, &[RTK_MD_REF]) { println!("[ok] Local AGENTS.md: @RTK.md reference"); } else if content.contains(RTK_BLOCK_START) { @@ -6438,6 +6411,33 @@ mod tests { ); } + #[test] + fn test_remove_droid_hook_propagates_backup_failure_and_preserves_file() { + let temp = TempDir::new().unwrap(); + let path = temp.path().join(DROID_HOOKS_FILE); + let original = serde_json::to_string_pretty(&serde_json::json!({ + "PreToolUse": [{ + "matcher": DROID_EXECUTE_MATCHER, + "hooks": [{ "type": "command", "command": DROID_HOOK_COMMAND }] + }] + })) + .unwrap(); + fs::write(&path, &original).unwrap(); + fs::create_dir(path.with_extension("json.bak")).unwrap(); + + let err = remove_droid_hook_from_file( + &DroidHookFile { + path: path.clone(), + layout: DroidLayout::Root, + }, + InitContext::default(), + ) + .unwrap_err(); + + assert!(format!("{err:#}").contains("backup")); + assert_eq!(fs::read_to_string(path).unwrap(), original); + } + #[test] fn test_droid_target_defaults_to_hooks_json() { // Fresh setup: the canonical hooks.json is created (Droid's own @@ -7017,6 +7017,56 @@ mod tests { assert_eq!(written, content); } + #[test] + fn test_read_json_file_handles_missing_and_empty_files() { + let temp = TempDir::new().unwrap(); + let missing = temp.path().join("missing.json"); + let empty = temp.path().join("empty.json"); + fs::write(&empty, " \n").unwrap(); + + assert!(read_json_file(&missing).unwrap().is_none()); + assert_eq!(read_json_file(&empty).unwrap(), Some(serde_json::json!({}))); + } + + #[test] + fn test_read_json_file_errors_include_the_path() { + let temp = TempDir::new().unwrap(); + let invalid = temp.path().join("invalid.json"); + fs::write(&invalid, "{").unwrap(); + + let parse_error = read_json_file(&invalid).unwrap_err(); + assert!(format!("{parse_error:#}").contains(&invalid.display().to_string())); + + let read_error = read_json_file(temp.path()).unwrap_err(); + assert!(format!("{read_error:#}").contains(&temp.path().display().to_string())); + } + + #[test] + fn test_backup_and_atomic_write_preserves_previous_content() { + let temp = TempDir::new().unwrap(); + let path = temp.path().join("hooks.json"); + fs::write(&path, "old").unwrap(); + + let backup = backup_and_atomic_write(&path, "new").unwrap().unwrap(); + + assert_eq!(backup, path.with_extension("json.bak")); + assert_eq!(fs::read_to_string(path).unwrap(), "new"); + assert_eq!(fs::read_to_string(backup).unwrap(), "old"); + } + + #[test] + fn test_backup_and_atomic_write_failure_preserves_original() { + let temp = TempDir::new().unwrap(); + let path = temp.path().join("hooks.json"); + fs::write(&path, "old").unwrap(); + fs::create_dir(path.with_extension("json.bak")).unwrap(); + + let err = backup_and_atomic_write(&path, "new").unwrap_err(); + + assert!(format!("{err:#}").contains("backup")); + assert_eq!(fs::read_to_string(path).unwrap(), "old"); + } + #[cfg(unix)] #[test] fn test_atomic_write_preserves_symlink() { @@ -7306,6 +7356,29 @@ mod tests { assert!(json_content["hooks"]["afterFileEdit"].is_array()); } + #[test] + fn test_patch_cursor_hook_propagates_backup_failure_and_preserves_file() { + let temp = TempDir::new().unwrap(); + let path = temp.path().join(HOOKS_JSON); + let original = serde_json::to_string_pretty(&serde_json::json!({ + "version": 1, + "hooks": { + "preToolUse": [{ + "command": "./hooks/other.sh", + "matcher": "Shell" + }] + } + })) + .unwrap(); + fs::write(&path, &original).unwrap(); + fs::create_dir(path.with_extension("json.bak")).unwrap(); + + let err = patch_cursor_hooks_json(&path, InitContext::default()).unwrap_err(); + + assert!(format!("{err:#}").contains("backup")); + assert_eq!(fs::read_to_string(path).unwrap(), original); + } + #[test] fn test_remove_cursor_hook_from_json() { let mut json_content = serde_json::json!({ From e4b74fd35b3cec3e2fad5c4ba6797b7f1d300784 Mon Sep 17 00:00:00 2001 From: Yijie Xu Date: Fri, 28 Aug 2026 09:04:31 +0800 Subject: [PATCH 4/5] refactor(hooks): share PreToolUse rewrite payload builder --- src/hooks/hook_cmd.rs | 101 +++++++++++++++++++++--------------------- 1 file changed, 51 insertions(+), 50 deletions(-) diff --git a/src/hooks/hook_cmd.rs b/src/hooks/hook_cmd.rs index bcd5568e62..8756314045 100644 --- a/src/hooks/hook_cmd.rs +++ b/src/hooks/hook_cmd.rs @@ -571,6 +571,28 @@ enum PayloadAction { Ignore, } +fn pre_tool_use_rewrite_output( + v: &Value, + rewritten: &str, + permission_decision: Option<&str>, +) -> Value { + let mut updated_input = v.get("tool_input").cloned().unwrap_or_else(|| json!({})); + if let Some(obj) = updated_input.as_object_mut() { + obj.insert("command".into(), Value::String(rewritten.to_string())); + } + + let mut hook_output = json!({ + "hookEventName": PRE_TOOL_USE_KEY, + "permissionDecisionReason": "RTK auto-rewrite", + "updatedInput": updated_input + }); + if let Some(decision) = permission_decision { + hook_output["permissionDecision"] = Value::String(decision.to_string()); + } + + json!({ "hookSpecificOutput": hook_output }) +} + fn process_claude_payload(v: &Value) -> PayloadAction { let cmd = match v .pointer("/tool_input/command") @@ -598,31 +620,10 @@ fn process_claude_payload(v: &Value) -> PayloadAction { HookDecision::AskRewrite(r) => (r, false), }; - let updated_input = { - let mut ti = v.get("tool_input").cloned().unwrap_or_else(|| json!({})); - if let Some(obj) = ti.as_object_mut() { - obj.insert("command".into(), Value::String(rewritten.clone())); - } - ti - }; - - let mut hook_output = json!({ - "hookEventName": PRE_TOOL_USE_KEY, - "permissionDecisionReason": "RTK auto-rewrite", - "updatedInput": updated_input - }); - - if allow { - hook_output - .as_object_mut() - .unwrap() - .insert("permissionDecision".into(), json!("allow")); - } - PayloadAction::Rewrite { cmd: cmd.to_string(), + output: pre_tool_use_rewrite_output(v, &rewritten, allow.then_some("allow")), rewritten, - output: json!({ "hookSpecificOutput": hook_output }), } } @@ -732,22 +733,10 @@ fn process_codex_payload(v: &Value) -> PayloadAction { // can add prompts for known-safe commands and obscure classifier signals // for wrapped mutating commands such as git push. Keep the passthrough gates // above conservative and revisit this boundary whenever coverage expands. - let mut updated_input = v.get("tool_input").cloned().unwrap_or_else(|| json!({})); - if let Some(obj) = updated_input.as_object_mut() { - obj.insert("command".into(), Value::String(rewritten.clone())); - } - PayloadAction::Rewrite { cmd: cmd.to_string(), + output: pre_tool_use_rewrite_output(v, &rewritten, Some("allow")), rewritten, - output: json!({ - "hookSpecificOutput": { - "hookEventName": PRE_TOOL_USE_KEY, - "permissionDecision": "allow", - "permissionDecisionReason": "RTK auto-rewrite", - "updatedInput": updated_input - } - }), } } @@ -962,21 +951,7 @@ fn droid_response_from_decision(v: &Value, cmd: &str, decision: HookDecision) -> audit_log("rewrite", cmd, &rewritten); - let updated_input = { - let mut ti = v.get("tool_input").cloned().unwrap_or_else(|| json!({})); - if let Some(obj) = ti.as_object_mut() { - obj.insert("command".into(), Value::String(rewritten)); - } - ti - }; - - Some(json!({ - "hookSpecificOutput": { - "hookEventName": PRE_TOOL_USE_KEY, - "permissionDecisionReason": "RTK auto-rewrite", - "updatedInput": updated_input - } - })) + Some(pre_tool_use_rewrite_output(v, &rewritten, None)) } /// Run the Factory Droid PreToolUse hook natively. @@ -1509,6 +1484,32 @@ mod tests { ); } + #[test] + fn test_pre_tool_use_rewrite_output_merges_input_and_optional_decision() { + let input = json!({ + "tool_input": { + "command": "git status", + "timeout": 30_000, + "description": "Inspect the working tree" + } + }); + + let ask = pre_tool_use_rewrite_output(&input, "rtk git status", None); + let ask_hook = &ask["hookSpecificOutput"]; + assert_eq!(ask_hook["hookEventName"], PRE_TOOL_USE_KEY); + assert_eq!(ask_hook["permissionDecisionReason"], "RTK auto-rewrite"); + assert!(ask_hook.get("permissionDecision").is_none()); + assert_eq!(ask_hook["updatedInput"]["command"], "rtk git status"); + assert_eq!(ask_hook["updatedInput"]["timeout"], 30_000); + assert_eq!( + ask_hook["updatedInput"]["description"], + "Inspect the working tree" + ); + + let allow = pre_tool_use_rewrite_output(&input, "rtk git status", Some("allow")); + assert_eq!(allow["hookSpecificOutput"]["permissionDecision"], "allow"); + } + // --- Claude handler --- fn claude_input(cmd: &str) -> String { From e260715df923abf0523854fbf0bb5eadd766329c Mon Sep 17 00:00:00 2001 From: Yijie Xu Date: Fri, 28 Aug 2026 11:50:26 +0800 Subject: [PATCH 5/5] fix(hooks): preserve best-effort multi-file cleanup --- src/hooks/init.rs | 96 +++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 92 insertions(+), 4 deletions(-) diff --git a/src/hooks/init.rs b/src/hooks/init.rs index 9cf2c6505e..95a14dcac5 100644 --- a/src/hooks/init.rs +++ b/src/hooks/init.rs @@ -3419,11 +3419,24 @@ pub fn uninstall_droid(global: bool, ctx: InitContext) -> Result<()> { fn uninstall_droid_at(droid_dir: &Path, ctx: InitContext) -> Result> { let mut removed = Vec::new(); + let mut errors = Vec::new(); for candidate in droid_hook_file_candidates(droid_dir) { - if remove_droid_hook_from_file(&candidate, ctx)? { - removed.push(format!("Droid hook file: {}", candidate.path.display())); + match remove_droid_hook_from_file(&candidate, ctx) { + Ok(true) => { + removed.push(format!("Droid hook file: {}", candidate.path.display())); + } + Ok(false) => {} + Err(error) => errors.push(format!("{}: {error:#}", candidate.path.display())), } } + + if !errors.is_empty() { + return Err(anyhow::anyhow!( + "Failed to uninstall RTK from one or more Droid hook files:\n - {}", + errors.join("\n - ") + )); + } + Ok(removed) } @@ -3930,8 +3943,12 @@ fn remove_legacy_cursor_hook_entries_from_json(root: &mut serde_json::Value) -> /// Remove Cursor RTK artifacts: hook script + hooks.json entry fn remove_cursor_hooks(ctx: InitContext) -> Result> { - let InitContext { verbose, dry_run } = ctx; let cursor_dir = resolve_cursor_dir()?; + remove_cursor_hooks_at(&cursor_dir, ctx) +} + +fn remove_cursor_hooks_at(cursor_dir: &Path, ctx: InitContext) -> Result> { + let InitContext { verbose, dry_run } = ctx; let mut removed = Vec::new(); // 1. Remove hook script @@ -3953,7 +3970,17 @@ fn remove_cursor_hooks(ctx: InitContext) -> Result> { // 2. Remove RTK entry from hooks.json let hooks_json_path = cursor_dir.join(HOOKS_JSON); - if let Some(mut root) = read_json_file(&hooks_json_path)? { + let root = match read_json_file(&hooks_json_path) { + Ok(root) => root, + Err(error) if error.downcast_ref::().is_some() => { + eprintln!( + "rtk: warning: leaving malformed Cursor hooks.json unchanged during uninstall: {error:#}" + ); + None + } + Err(error) => return Err(error), + }; + if let Some(mut root) = root { if remove_cursor_hook_from_json(&mut root) { if dry_run { println!( @@ -6438,6 +6465,35 @@ mod tests { assert_eq!(fs::read_to_string(path).unwrap(), original); } + #[test] + fn test_uninstall_droid_continues_after_candidate_failure() { + let temp = TempDir::new().unwrap(); + let droid_dir = temp.path().join(DROID_DIR); + let root_path = droid_dir.join(DROID_HOOKS_FILE); + let legacy_path = droid_dir.join(DROID_HOOKS_SUBDIR).join(DROID_HOOKS_FILE); + fs::create_dir_all(legacy_path.parent().unwrap()).unwrap(); + + let hook_json = serde_json::to_string_pretty(&serde_json::json!({ + "PreToolUse": [{ + "matcher": DROID_EXECUTE_MATCHER, + "hooks": [{ "type": "command", "command": DROID_HOOK_COMMAND }] + }] + })) + .unwrap(); + fs::write(&root_path, &hook_json).unwrap(); + fs::write(&legacy_path, &hook_json).unwrap(); + fs::create_dir(root_path.with_extension("json.bak")).unwrap(); + + let err = uninstall_droid_at(&droid_dir, InitContext::default()).unwrap_err(); + + assert!(format!("{err:#}").contains(&root_path.display().to_string())); + assert_eq!(fs::read_to_string(&root_path).unwrap(), hook_json); + let legacy: serde_json::Value = + serde_json::from_str(&fs::read_to_string(&legacy_path).unwrap()).unwrap(); + assert!(!droid_hook_already_present(&legacy, DroidLayout::Root)); + assert!(legacy_path.with_extension("json.bak").exists()); + } + #[test] fn test_droid_target_defaults_to_hooks_json() { // Fresh setup: the canonical hooks.json is created (Droid's own @@ -7379,6 +7435,38 @@ mod tests { assert_eq!(fs::read_to_string(path).unwrap(), original); } + #[test] + fn test_remove_cursor_hooks_keeps_malformed_json_best_effort() { + let temp = TempDir::new().unwrap(); + let cursor_dir = temp.path().join(CURSOR_DIR); + let hook_path = cursor_dir.join(HOOKS_SUBDIR).join(REWRITE_HOOK_FILE); + let hooks_json = cursor_dir.join(HOOKS_JSON); + fs::create_dir_all(hook_path.parent().unwrap()).unwrap(); + fs::write(&hook_path, "legacy hook").unwrap(); + fs::write(&hooks_json, "{").unwrap(); + + let removed = remove_cursor_hooks_at(&cursor_dir, InitContext::default()).unwrap(); + + assert!(!hook_path.exists()); + assert_eq!(fs::read_to_string(hooks_json).unwrap(), "{"); + assert_eq!( + removed, + vec![format!("Cursor hook: {}", hook_path.display())] + ); + } + + #[test] + fn test_remove_cursor_hooks_still_propagates_read_errors() { + let temp = TempDir::new().unwrap(); + let cursor_dir = temp.path().join(CURSOR_DIR); + let hooks_json = cursor_dir.join(HOOKS_JSON); + fs::create_dir_all(&hooks_json).unwrap(); + + let err = remove_cursor_hooks_at(&cursor_dir, InitContext::default()).unwrap_err(); + + assert!(format!("{err:#}").contains(&hooks_json.display().to_string())); + } + #[test] fn test_remove_cursor_hook_from_json() { let mut json_content = serde_json::json!({