Harden tag mode tool permissions against prompt injection#1002
Harden tag mode tool permissions against prompt injection#1002km-anthropic wants to merge 1 commit intomainfrom
Conversation
Two defenses for tag mode where an attacker with repo write access could craft a prompt injection payload in an issue/PR to gain RCE on the Actions runner: 1. git-push wrapper (H1 #3556799) The Bash(git\ push:*) rule permitted arbitrary flags and remotes, including combinations that execute shell commands locally. Replaced with scripts/git-push.sh which allowlists exactly 'origin <ref>' with no flags, validates the ref via check-ref-format. Same pattern as scripts/gh.sh. 2. acceptEdits instead of blanket Write/Edit (Asana 1213310082312048) Edit/MultiEdit/Write in allowedTools granted write access to the whole runner filesystem (~/.bashrc etc). Removed from allowedTools and set --permission-mode acceptEdits, which auto-accepts edits inside cwd ($GITHUB_WORKSPACE) and denies outside. Headless SDK has no prompt handler so 'ask' becomes deny. Also: - Noted that create-prompt/index.ts exports ALLOWED_TOOLS env var that nothing reads. The live path is modes/tag/index.ts. Mirrored the fix in both so the file the H1 report likely points to stays in sync. - Updated prompt text (3 callsites) to reference the wrapper. - Updated tests (4 prompt-content asserts, 7 tool-list asserts).
| @@ -137,7 +138,7 @@ export async function prepareTagMode({ | |||
| tagModeTools.push( | |||
| "Bash(git add:*)", | |||
| "Bash(git commit:*)", | |||
There was a problem hiding this comment.
Potential remaining RCE vector via git hooks: The git push:* wildcard is now properly locked down, but Bash(git commit:*) remains a wildcard. If acceptEdits allows writes to .git/hooks/ (which is inside $GITHUB_WORKSPACE), a prompt-injected Claude could write a malicious .git/hooks/pre-commit script and then trigger it via git commit.
Worth verifying whether acceptEdits mode blocks writes to .git/hooks/ or treats .git/ as part of the writable workspace. If it doesn't block it, consider either:
- Adding a
git commitwrapper similar togit-push.shthat runsgit commit --no-verify(to skip hooks), or - Ensuring
.git/hooks/is cleaned/protected before execution
| import { extractUserRequest } from "../utils/extract-user-request"; | ||
| export type { CommonFields, PreparedContext } from "./types"; | ||
|
|
||
| const GIT_PUSH_WRAPPER = `${process.env.GITHUB_ACTION_PATH}/scripts/git-push.sh`; |
There was a problem hiding this comment.
Module-level env var capture: This top-level const reads process.env.GITHUB_ACTION_PATH at import time, not at call time. If the module is imported before the env var is set, this bakes in "undefined/scripts/git-push.sh".
In production (GitHub Actions runtime), this is fine since the runner sets GITHUB_ACTION_PATH before execution. But in tests, beforeAll sets the env var after the module is already imported, so GIT_PUSH_WRAPPER captures undefined. Tests currently pass because they check for "scripts/git-push.sh" as a substring rather than the full path, which masks the issue.
Contrast with src/modes/tag/index.ts which correctly reads it inside the function body. Consider moving this inside buildAllowedToolsString (or making it a getter) to match that pattern.
|
|
||
| // Set allowed tools | ||
| // NOTE: these env var exports are dead — nothing reads ALLOWED_TOOLS / DISALLOWED_TOOLS. | ||
| // The live path is modes/tag/index.ts which builds --allowedTools into claudeArgs directly. |
There was a problem hiding this comment.
Consider removing this dead code: This block still calls core.exportVariable("ALLOWED_TOOLS", ...), which writes real env vars into the runner environment. Even though nothing reads them today, they could confuse future code or downstream workflow steps. The tool lists here also already diverge from the live path in modes/tag/index.ts (e.g., CI tools are conditional here but unconditional there).
A comment referencing the H1 report near the live code in tag/index.ts (which the wrapper script already does) would provide the same traceability without maintaining a shadow copy of the security-critical allowlist.
| // Add required tools for tag mode. | ||
| // acceptEdits: file edits auto-allowed inside cwd ($GITHUB_WORKSPACE), denied outside. | ||
| // Headless SDK has no prompt handler, so anything that falls through to "ask" is denied. | ||
| claudeArgs += ` --permission-mode acceptEdits --allowedTools "${tagModeTools.join(",")}"`; |
There was a problem hiding this comment.
userClaudeArgs appended after --permission-mode could override it: If Claude Code CLI uses "last wins" semantics for --permission-mode, a repo owner setting claude_args: "--permission-mode full" would silently defeat this hardening. Low severity since claude_args is workflow-author-controlled (not attacker-controlled via prompt injection), but the intent of this PR is to enforce acceptEdits as a security boundary.
Consider either appending the hardened flags after user args (so they take precedence), or stripping --permission-mode from userClaudeArgs before appending.
|
Review Summary - Solid security hardening PR. See inline comments for specific findings. Key areas: (1) potential remaining RCE vector via git commit hooks if acceptEdits allows .git/hooks writes, (2) GIT_PUSH_WRAPPER module-level env var capture issue in create-prompt/index.ts, (3) test coverage gaps for git-push.sh wrapper and prepareTagMode live code path, (4) dead code in create-prompt/index.ts still exports real env vars. |
Addresses two bug bounty reports about tag mode's default tool permissions. Both require an attacker to already have write access to the target repo and to craft a prompt-injection payload in an issue/PR body, but once that bar is cleared they yield RCE on the Actions runner and exfil of
GITHUB_TOKEN/ANTHROPIC_API_KEY.1.
git pushwrapper — H1 #3556799The
Bash(git push:*)wildcard permitted arbitrary flags and remotes. Replaced withscripts/git-push.shwhich allowlists exactlyorigin <ref>(no flags, ref validated viagit check-ref-format). Same pattern asscripts/gh.sh(#996).The prompt already instructs Claude to run exactly
git push origin HEADorgit push origin <branch>, so this is transparent to normal operation.2.
acceptEditsinstead of blanket Write/Edit — AsanaEdit/MultiEdit/WriteinallowedToolsgranted unrestricted filesystem write across the entire runner. Removed from the allowlist and set--permission-mode acceptEdits, which auto-accepts edits inside the working directory ($GITHUB_WORKSPACE) and denies outside. The headless SDK has no prompt handler, so anything that falls through to "ask" is denied.Dead code note
src/create-prompt/index.tsexports anALLOWED_TOOLSenv var that nothing reads — the live path issrc/modes/tag/index.tswhich builds--allowedToolsintoclaudeArgsdirectly. Mirrored the fix in both so the file the H1 report points to stays in sync, and left a comment.Testing
origin HEADand valid branch namescc @dworken