Skip to content

fix(claudecode): align acceptEdits Bash auto-approval with execution#16183

Open
Jiangrong-W wants to merge 1 commit into
CherryHQ:mainfrom
Jiangrong-W:harness-fix/cherrystudio-approval-acceptedits-bash-first-token-bypass
Open

fix(claudecode): align acceptEdits Bash auto-approval with execution#16183
Jiangrong-W wants to merge 1 commit into
CherryHQ:mainfrom
Jiangrong-W:harness-fix/cherrystudio-approval-acceptedits-bash-first-token-bypass

Conversation

@Jiangrong-W

Copy link
Copy Markdown

🚨 Branch strategy — read before opening this PR

The v2 refactor has merged into main, so main is the default branch for active development (v1 and v2 code currently coexist there — expect large, breaking changes).

  • Active development (features, refactors, optimizations, fixes for the current codebase) → target main (the default base).
  • v1 maintenance (hotfixes and subsequent v1 releases) → branch from and target v1, not main.

A v1 fix does not auto-carry to main: if the same bug exists on main, open a separate forward-port PR targeting main. Before touching subsystems being replaced, read docs/references/data/ and watch for @deprecated markers — they flag code being deleted.

What this PR does

Before this PR:

In acceptEdits permission mode, the Claude Code Bash auto-approval gate decided whether to skip the command-approval prompt by inspecting only the first whitespace-delimited token of the command, while the runtime then executed the full, unmodified command string. Because the approval decision and the execution point disagreed on what "the command" is, a Bash invocation that merely starts with an allowlisted edit verb (mkdir / touch / mv / cp) could carry a second shell action after a separator (;, &&, ||, |, &, redirection, command substitution) and have the whole string run without ever prompting the user. This affects only the non-default acceptEdits mode; the default mode already prompts for Bash.

Affected sites (decision point vs. real execution, relative to current main):

  • src/shared/ai/claudecode/toolRules.ts:104matchesAcceptEditsBashInvocation normalizes the command to its first whitespace-delimited token before checking the allowlist:
    const command = commandFromInput(invocation.input).split(/\s+/, 1)[0]
    return ACCEPT_EDITS_BASH_COMMANDS.has(command)   // mkdir / touch / mv / cp
  • src/main/ai/runtime/claudeCode/settingsBuilder.ts:623canUseTool allows the call with the original input, so the entire payload runs unchanged:
    if (access?.approval === 'auto') {
      return { behavior: 'allow', updatedInput: input }   // full command string, unchanged
    }
  • src/main/ai/tools/adapters/claudeCode/agentTools.ts:174 — the live tool-policy snapshot resolves the runtime tool name together with the model-supplied tool input, feeding that input into the gate above.

After this PR:

Auto-approval only fires for a single simple command whose verb is allowlisted. Before checking the verb, matchesAcceptEditsBashInvocation rejects any command containing shell chaining / redirection / command-substitution / backgrounding operators and instead falls through to the normal approval prompt:

const SHELL_COMPOUND_METACHARACTERS = /[;&|\n\r`$(){}<>]/
// ...
if (SHELL_COMPOUND_METACHARACTERS.test(command)) return false
const verb = command.split(/\s+/, 1)[0]
return ACCEPT_EDITS_BASH_COMMANDS.has(verb)

This covers ;, &&, ||, |, &, $(), backticks, grouping (){}, redirections </>, and embedded newlines — every way a second action can ride behind the allowlisted verb. The approval decision point and the real execution point now agree on what will run.

Behavior:

  • Plain edit commands such as mkdir tmp, cp a b, mv old new keep auto-approving — no change to the intended acceptEdits workflow.
  • A command that combines an allowlisted verb with any further shell action now prompts the user, exactly like any other non-allowlisted Bash command.
  • Only the non-default acceptEdits mode is affected; default mode already prompts for Bash.

Fixes #

Why we need it and why it was done in this way

The auto-approval is a lexical decision made on the first token, but the executor keeps the entire payload, so the approval can describe strictly less than what actually runs. Realigning the gate with execution removes that mismatch and prevents an unintended second shell action from running unprompted in acceptEdits mode.

The following tradeoffs were made:

  • The check is intentionally conservative: any compound / redirected / substituted command now prompts rather than auto-approving. This may add a prompt for a small number of legitimate compound edit commands, but the fallthrough is the existing safe default ("ask the user"), so nothing is blocked outright.

The following alternatives were considered:

  • Parsing the command and approving each sub-command against the allowlist. Rejected as larger, harder to get right, and unnecessary for the intended single-edit-command workflow; a single decision function with a metacharacter guard keeps the change minimal and self-contained.
  • The metacharacter check is a plain character-class literal (no nested quantifiers), evaluated once per tool call on a short command string, so it adds no meaningful cost and no ReDoS surface.

Links to places where the discussion took place:

Breaking changes

None. The function keeps the same signature, adds no dependencies, and preserves auto-approval for plain single edit commands. The only observable change is that a command mixing an allowlisted verb with an additional shell action now prompts in acceptEdits mode instead of auto-approving.

Special notes for your reviewer

  • Scope is one decision function in src/shared/ai/claudecode/toolRules.ts; the new module-local literal SHELL_COMPOUND_METACHARACTERS is the only added symbol.
  • Regression coverage was added to src/shared/ai/claudecode/__tests__/toolRules.test.ts. The new case does not auto-approve acceptEdits Bash compound commands hidden behind an allowed first token fails on the pre-change code (expected 'auto' to be 'prompt') and passes with this change; the existing applies invocation-level acceptEdits Bash defaults case still passes, confirming single-command auto-approval is preserved.
npx vitest run --project shared src/shared/ai/claudecode/__tests__/toolRules.test.ts
 Test Files  1 passed (1)
      Tests  6 passed (6)

oxlint and eslint report no issues on the changed files.

  • As a first-time contributor, the gated workflows will need a maintainer /ok-to-test (or equivalent approval) before CI runs in full.

Checklist

This checklist is not enforcing, but it's a reminder of items that could be relevant to every PR.
Approvers are expected to review this list.

  • Branch: This PR targets the correct branch — main for active development, v1 for v1 maintenance fixes
  • PR: The PR description is expressive enough and will help future contributors
  • Code: Write code that humans can understand and Keep it simple
  • Refactor: You have left the code cleaner than you found it (Boy Scout Rule)
  • Upgrade: Impact of this change on upgrade flows was considered and addressed if required
  • Documentation: A user-guide update was considered and is present (link) or not required. Check this only when the PR introduces or changes a user-facing feature or behavior.
  • Self-review: I have reviewed my own code (e.g., via /gh-pr-review, gh pr diff, or GitHub UI) before requesting review from others

Release note

In acceptEdits permission mode, the Claude Code Bash auto-approval now only applies to a single simple allowlisted command (mkdir/touch/mv/cp). A command that appends a second shell action after an allowed verb now prompts for approval instead of running unprompted.

In acceptEdits permission mode, Bash invocations were auto-approved when
the first whitespace-delimited token was an allowed edit verb
(mkdir/touch/mv/cp). The approval decision inspected only that lexical
prefix, but canUseTool executes the full, unchanged command string.

A compound command such as `cp ./a ./b; <second action>` therefore
skipped the approval prompt on the `cp` prefix while still running the
appended action — an approval/execution mismatch (approval_lexical_bypass).

Realign the approval decision point with the real execution point:
matchesAcceptEditsBashInvocation now refuses to auto-approve any command
containing shell chaining/redirection/substitution operators
(`;` `&&` `||` `|` `&` `$()` backticks `(){}` `<>` newlines). Only a
single simple command whose verb is allowlisted may skip the prompt;
anything else falls through to the normal approval prompt (the safe
default). Plain `mkdir tmp` / `cp a b` continue to auto-approve.

Adds regression tests covering the compound-command bypass vectors.

Signed-off-by: christop <825583681@qq.com>
@Jiangrong-W Jiangrong-W requested a review from a team June 18, 2026 05:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant