Skip to content

Harden tag mode tool permissions against prompt injection #1002

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
km-anthropic merged 1 commit into from
Mar 12, 2026
Merged

Harden tag mode tool permissions against prompt injection #1002

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions scripts/git-push.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
#!/usr/bin/env bash
set -euo pipefail

# Wrapper around `git push` that only allows `origin <ref>` with no flags.
# Defends against --receive-pack / --exec RCE and arbitrary-remote exfiltration
# (H1 #3556799). `git push:*` in allowedTools permits `git push --receive-pack='sh -c ...' ext::sh`
# which runs arbitrary shell on the Actions runner. This wrapper closes that.
#
# Usage:
# git-push.sh origin HEAD
# git-push.sh origin claude/issue-123-20260304

if [[ $# -ne 2 ]]; then
echo "Error: exactly two arguments required: origin <ref>" >&2
exit 1
fi

for arg in "$@"; do
if [[ "$arg" == -* ]]; then
echo "Error: flags are not allowed (got: $arg)" >&2
exit 1
fi
done

if [[ "$1" != "origin" ]]; then
echo "Error: remote must be 'origin' (got: $1)" >&2
exit 1
fi

REF="$2"
if [[ "$REF" != "HEAD" ]] && ! git check-ref-format --branch "$REF" >/dev/null 2>&1; then
echo "Error: invalid ref: $REF" >&2
exit 1
fi

exec git push origin "$REF"
28 changes: 13 additions & 15 deletions src/create-prompt/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,19 +23,15 @@ import { GITHUB_SERVER_URL } from "../github/api/config";
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`;

@claude claude Bot Mar 4, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


/** Filename for the user request file, read by the SDK runner */
const USER_REQUEST_FILENAME = "claude-user-request.txt";

// Tag mode defaults - these tools are needed for tag mode to function
const BASE_ALLOWED_TOOLS = [
"Edit",
"MultiEdit",
"Glob",
"Grep",
"LS",
"Read",
"Write",
];
// Tag mode defaults - these tools are needed for tag mode to function.
// Edit/MultiEdit/Write are intentionally omitted: acceptEdits permission mode
// auto-allows file edits inside $GITHUB_WORKSPACE and denies writes outside it.
const BASE_ALLOWED_TOOLS = ["Glob", "Grep", "LS", "Read"];

export function buildAllowedToolsString(
customAllowedTools?: string[],
Expand All @@ -59,7 +55,7 @@ export function buildAllowedToolsString(
baseTools.push(
"Bash(git add:*)",
"Bash(git commit:*)",
"Bash(git push:*)",
`Bash(${GIT_PUSH_WRAPPER}:*)`,
"Bash(git status:*)",
"Bash(git diff:*)",
"Bash(git log:*)",
Expand Down Expand Up @@ -434,7 +430,7 @@ function getCommitInstructions(
Bash(git commit -m "<message>\\n\\n${coAuthorLine}")`
: ""
}
- Push to the remote: Bash(git push origin HEAD)`;
- Push to the remote: Bash(${GIT_PUSH_WRAPPER} origin HEAD)`;
} else {
const branchName = eventData.claudeBranch || eventData.baseBranch;
return `
Expand All @@ -448,7 +444,7 @@ function getCommitInstructions(
Bash(git commit -m "<message>\\n\\n${coAuthorLine}")`
: ""
}
- Push to the remote: Bash(git push origin ${branchName})`;
- Push to the remote: Bash(${GIT_PUSH_WRAPPER} origin ${branchName})`;
}
}
}
Expand Down Expand Up @@ -823,7 +819,7 @@ ${
: `- Use git commands via the Bash tool for version control (remember that you have access to these git commands):
- Stage files: Bash(git add <files>)
- Commit changes: Bash(git commit -m "<message>")
- Push to remote: Bash(git push origin <branch>) (NEVER force push)
- Push to remote: Bash(${GIT_PUSH_WRAPPER} origin <branch>)
- Delete files: Bash(git rm <files>) followed by commit and push
- Check status: Bash(git status)
- View diff: Bash(git diff)${eventData.isPR && eventData.baseBranch ? `\n - IMPORTANT: For PR diffs, use: Bash(git diff origin/${eventData.baseBranch}...HEAD)` : ""}`
Expand Down Expand Up @@ -977,7 +973,9 @@ export async function createPrompt(
console.log("========================");
}

// 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.
Comment on lines 975 to +977

@claude claude Bot Mar 4, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

// Kept only so the H1 report's pointed-to file stays in sync with the live fix.
const hasActionsReadPermission = false;

const allAllowedTools = buildAllowedToolsString(
Expand Down
19 changes: 11 additions & 8 deletions src/modes/tag/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,16 +114,17 @@ export async function prepareTagMode({
tool.startsWith("mcp__github_"),
);

// Build claude_args for tag mode with required tools
// Tag mode REQUIRES these tools to function properly
const gitPushWrapper = `${process.env.GITHUB_ACTION_PATH}/scripts/git-push.sh`;

// Build claude_args for tag mode with required tools.
// Edit/MultiEdit/Write are intentionally omitted: acceptEdits permission mode (set below)
// auto-allows file edits inside $GITHUB_WORKSPACE and denies writes outside (e.g. ~/.bashrc).
// Listing them here would grant blanket write access to the whole runner (Asana 1213310082312048).
const tagModeTools = [
"Edit",
"MultiEdit",
"Glob",
"Grep",
"LS",
"Read",
"Write",
"mcp__github_comment__update_claude_comment",
"mcp__github_ci__get_ci_status",
"mcp__github_ci__get_workflow_run_details",
Expand All @@ -137,7 +138,7 @@ export async function prepareTagMode({
tagModeTools.push(
"Bash(git add:*)",
"Bash(git commit:*)",

@claude claude Bot Mar 4, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 commit wrapper similar to git-push.sh that runs git commit --no-verify (to skip hooks), or
  • Ensuring .git/hooks/ is cleaned/protected before execution

"Bash(git push:*)",
`Bash(${gitPushWrapper}:*)`,
"Bash(git status:*)",
"Bash(git diff:*)",
"Bash(git log:*)",
Expand Down Expand Up @@ -171,8 +172,10 @@ export async function prepareTagMode({
const escapedOurConfig = ourMcpConfig.replace(/'/g, "'\\''");
claudeArgs = `--mcp-config '${escapedOurConfig}'`;

// Add required tools for tag mode
claudeArgs += ` --allowedTools "${tagModeTools.join(",")}"`;
// 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(",")}"`;

@claude claude Bot Mar 4, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


// Append user's claude_args (which may have more --mcp-config flags)
if (userClaudeArgs) {
Expand Down
41 changes: 23 additions & 18 deletions test/create-prompt.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#!/usr/bin/env bun

import { describe, test, expect } from "bun:test";
import { describe, test, expect, beforeAll } from "bun:test";
import {
generatePrompt,
getEventTypeAndContext,
Expand All @@ -9,6 +9,10 @@ import {
} from "../src/create-prompt";
import type { PreparedContext } from "../src/create-prompt";

beforeAll(() => {
process.env.GITHUB_ACTION_PATH = "/test/action/path";
});

describe("generatePrompt", () => {
const mockGitHubData = {
contextData: {
Expand Down Expand Up @@ -505,7 +509,7 @@ describe("generatePrompt", () => {
const prompt = await generatePrompt(envVars, mockGitHubData, false, "tag");

// Should contain PR-specific instructions (git commands when not using signing)
expect(prompt).toContain("git push");
expect(prompt).toContain("scripts/git-push.sh origin");
expect(prompt).toContain(
"Always push to the existing branch when triggered on a PR",
);
Expand Down Expand Up @@ -643,7 +647,7 @@ describe("generatePrompt", () => {
const prompt = await generatePrompt(envVars, mockGitHubData, false, "tag");

// Should contain open PR instructions (git commands when not using signing)
expect(prompt).toContain("git push");
expect(prompt).toContain("scripts/git-push.sh origin");
expect(prompt).toContain(
"Always push to the existing branch when triggered on a PR",
);
Expand Down Expand Up @@ -757,7 +761,7 @@ describe("generatePrompt", () => {
expect(prompt).toContain("Use git commands via the Bash tool");
expect(prompt).toContain("git add");
expect(prompt).toContain("git commit");
expect(prompt).toContain("git push");
expect(prompt).toContain("scripts/git-push.sh origin");

// Should use the minimal comment tool
expect(prompt).toContain("mcp__github_comment__update_claude_comment");
Expand Down Expand Up @@ -886,17 +890,18 @@ describe("buildAllowedToolsString", () => {
const result = buildAllowedToolsString();

// The base tools should be in the result
expect(result).toContain("Edit");
// Edit/MultiEdit/Write are NOT in allowedTools — acceptEdits permission mode handles them
expect(result).not.toContain("Edit");
expect(result).not.toContain("Write");
expect(result).toContain("Glob");
expect(result).toContain("Grep");
expect(result).toContain("LS");
expect(result).toContain("Read");
expect(result).toContain("Write");

// Default is no commit signing, so should have specific Bash git commands
expect(result).toContain("Bash(git add:*)");
expect(result).toContain("Bash(git commit:*)");
expect(result).toContain("Bash(git push:*)");
expect(result).toContain("scripts/git-push.sh:*)");
expect(result).toContain("mcp__github_comment__update_claude_comment");

// Should not have commit signing tools
Expand All @@ -908,12 +913,12 @@ describe("buildAllowedToolsString", () => {
const result = buildAllowedToolsString([], false, false);

// The base tools should be in the result
expect(result).toContain("Edit");
expect(result).not.toContain("Edit");
expect(result).toContain("Glob");
expect(result).toContain("Grep");
expect(result).toContain("LS");
expect(result).toContain("Read");
expect(result).toContain("Write");
expect(result).not.toContain("Write");

// Should have specific Bash git commands for non-signing mode
expect(result).toContain("Bash(git add:*)");
Expand All @@ -930,7 +935,7 @@ describe("buildAllowedToolsString", () => {
const result = buildAllowedToolsString(customTools);

// Base tools should be present
expect(result).toContain("Edit");
expect(result).toContain("Read");
expect(result).toContain("Glob");

// Custom tools should be appended
Expand All @@ -950,7 +955,7 @@ describe("buildAllowedToolsString", () => {
const result = buildAllowedToolsString([], true);

// Base tools should be present
expect(result).toContain("Edit");
expect(result).toContain("Read");
expect(result).toContain("Glob");

// GitHub Actions tools should be included
Expand All @@ -964,7 +969,7 @@ describe("buildAllowedToolsString", () => {
const result = buildAllowedToolsString(customTools, true);

// Base tools should be present
expect(result).toContain("Edit");
expect(result).toContain("Read");

// Custom tools should be included
expect(result).toContain("Tool1");
Expand All @@ -980,12 +985,12 @@ describe("buildAllowedToolsString", () => {
const result = buildAllowedToolsString([], false, true);

// Base tools should be present
expect(result).toContain("Edit");
expect(result).not.toContain("Edit");
expect(result).toContain("Glob");
expect(result).toContain("Grep");
expect(result).toContain("LS");
expect(result).toContain("Read");
expect(result).toContain("Write");
expect(result).not.toContain("Write");

// Commit signing tools should be included
expect(result).toContain("mcp__github_file_ops__commit_files");
Expand All @@ -1001,17 +1006,17 @@ describe("buildAllowedToolsString", () => {
const result = buildAllowedToolsString([], false, false);

// Base tools should be present
expect(result).toContain("Edit");
expect(result).not.toContain("Edit");
expect(result).toContain("Glob");
expect(result).toContain("Grep");
expect(result).toContain("LS");
expect(result).toContain("Read");
expect(result).toContain("Write");
expect(result).not.toContain("Write");

// Specific Bash git commands should be included
expect(result).toContain("Bash(git add:*)");
expect(result).toContain("Bash(git commit:*)");
expect(result).toContain("Bash(git push:*)");
expect(result).toContain("scripts/git-push.sh:*)");
expect(result).toContain("Bash(git status:*)");
expect(result).toContain("Bash(git diff:*)");
expect(result).toContain("Bash(git log:*)");
Expand All @@ -1030,7 +1035,7 @@ describe("buildAllowedToolsString", () => {
const result = buildAllowedToolsString(customTools, true, false);

// Base tools should be present
expect(result).toContain("Edit");
expect(result).toContain("Read");
expect(result).toContain("Bash(git add:*)");

// Custom tools should be included
Expand Down
2 changes: 1 addition & 1 deletion test/pull-request-target.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ describe("pull_request_target event support", () => {
const prompt = generatePrompt(envVars, mockGitHubData, false, "tag");

// Should include git commands for non-commit-signing mode
expect(prompt).toContain("git push");
expect(prompt).toContain("scripts/git-push.sh origin");
expect(prompt).toContain(
"Always push to the existing branch when triggered on a PR",
);
Expand Down
Loading