From f956510b1afc643e768961656c10f7039534d553 Mon Sep 17 00:00:00 2001 From: kashyap murali Date: Thu, 12 Mar 2026 13:35:17 -0700 Subject: [PATCH] Harden tag mode tool permissions against prompt injection (#1002) 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 ' 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). --- scripts/git-push.sh | 36 ++++++++++++++++++++++++++++ src/create-prompt/index.ts | 28 ++++++++++------------ src/modes/tag/index.ts | 19 ++++++++------- test/create-prompt.test.ts | 41 ++++++++++++++++++-------------- test/pull-request-target.test.ts | 2 +- 5 files changed, 84 insertions(+), 42 deletions(-) create mode 100755 scripts/git-push.sh diff --git a/scripts/git-push.sh b/scripts/git-push.sh new file mode 100755 index 0000000..a8afcb1 --- /dev/null +++ b/scripts/git-push.sh @@ -0,0 +1,36 @@ +#!/usr/bin/env bash +set -euo pipefail + +# Wrapper around `git push` that only allows `origin ` 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 " >&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" diff --git a/src/create-prompt/index.ts b/src/create-prompt/index.ts index 4624c19..c21d831 100644 --- a/src/create-prompt/index.ts +++ b/src/create-prompt/index.ts @@ -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`; + /** 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[], @@ -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:*)", @@ -434,7 +430,7 @@ function getCommitInstructions( Bash(git commit -m "\\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 ` @@ -448,7 +444,7 @@ function getCommitInstructions( Bash(git commit -m "\\n\\n${coAuthorLine}")` : "" } - - Push to the remote: Bash(git push origin ${branchName})`; + - Push to the remote: Bash(${GIT_PUSH_WRAPPER} origin ${branchName})`; } } } @@ -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 ) - Commit changes: Bash(git commit -m "") - - Push to remote: Bash(git push origin ) (NEVER force push) + - Push to remote: Bash(${GIT_PUSH_WRAPPER} origin ) - Delete files: Bash(git rm ) 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)` : ""}` @@ -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. + // Kept only so the H1 report's pointed-to file stays in sync with the live fix. const hasActionsReadPermission = false; const allAllowedTools = buildAllowedToolsString( diff --git a/src/modes/tag/index.ts b/src/modes/tag/index.ts index 0adf746..14af9af 100644 --- a/src/modes/tag/index.ts +++ b/src/modes/tag/index.ts @@ -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", @@ -137,7 +138,7 @@ export async function prepareTagMode({ tagModeTools.push( "Bash(git add:*)", "Bash(git commit:*)", - "Bash(git push:*)", + `Bash(${gitPushWrapper}:*)`, "Bash(git status:*)", "Bash(git diff:*)", "Bash(git log:*)", @@ -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(",")}"`; // Append user's claude_args (which may have more --mcp-config flags) if (userClaudeArgs) { diff --git a/test/create-prompt.test.ts b/test/create-prompt.test.ts index 0a5ea82..3bf8b6c 100644 --- a/test/create-prompt.test.ts +++ b/test/create-prompt.test.ts @@ -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, @@ -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: { @@ -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", ); @@ -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", ); @@ -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"); @@ -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 @@ -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:*)"); @@ -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 @@ -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 @@ -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"); @@ -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"); @@ -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:*)"); @@ -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 diff --git a/test/pull-request-target.test.ts b/test/pull-request-target.test.ts index 59c747f..6d9cac0 100644 --- a/test/pull-request-target.test.ts +++ b/test/pull-request-target.test.ts @@ -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", );