[codex] disable tools for Claude text generation#4169
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
ApprovabilityVerdict: Needs human review This PR changes the security/permission model for Claude CLI invocation, replacing You can customize Macroscope's approvability policy. Learn more. |
Fixes #3594.
What Changed
--dangerously-skip-permissionsfrom Claude CLI text-generation subprocesses--toolsargument and rejects the permission-bypass flagWhy
Claude-backed title and branch generation receives the first user message. When that message is a skill invocation such as
/call-script, the backgroundclaude -pprocess could resolve and execute the skill with full tool access while the actual conversation executed it separately.Title, branch, commit, and PR metadata generation only need structured model output. They all share the same spawn path, so removing executable capabilities there makes the entire background text-generation boundary side-effect free without changing normal Claude conversation behavior.
UI Changes
None. This is a server-side subprocess capability restriction.
Verification
vp check(passes with 10 existing warnings outside this diff)vp run typecheckClaudeTextGeneration.test.ts: 5 passedvp test --maxWorkers=4: 608 files and 4,823 tests passed; 2 files and 7 tests skipped by the existing suitegit diff --checkChecklist
Note
Medium Risk
Narrows subprocess capabilities on a security-sensitive boundary (background Claude CLI) but is a focused spawn-arg change with regression tests and no UI impact.
Overview
Locks down background Claude
claude -psubprocesses used for structured metadata (titles, branches, commits, PR text) so they cannot run skills or tools.The spawn args drop
--dangerously-skip-permissionsand add an explicit empty--toolsvalue plus--disable-slash-commands, so a first user message like/call-scriptcannot be executed again with full capabilities while the main conversation runs it separately.Tests extend the fake Claude harness to assert per-argument argv (including the empty
--toolstoken) and add a thread-title regression that requires those flags and forbids the permission-bypass flag.Reviewed by Cursor Bugbot for commit 42a6417. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Disable tools for Claude CLI structured text generation calls
Replaces
--dangerously-skip-permissionswith--tools ''and--disable-slash-commandswhen spawning the Claude CLI for structured JSON generation (e.g. thread title generation), ensuring background generation calls do not inherit executable skills or tools.Test coverage is extended in ClaudeTextGeneration.test.ts to assert the new flags are present and
--dangerously-skip-permissionsis absent from the spawned argv.Macroscope summarized 42a6417.