From f944ecd6c91b754d6b42472e76b2c91b6a4f4c6e Mon Sep 17 00:00:00 2001 From: Nathan Heskew Date: Tue, 5 May 2026 13:01:32 -0700 Subject: [PATCH] ci(claude): clarify --allowedTools is convenience, not enforcement MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Spike on PR #452's failed run found that `Bash(grep -rn …)` calls all executed (`permission_denials: []`) despite not matching any listed `Bash(...:*)` pattern. Per claude-code-action's own `docs/configuration.md` (v1.0.110): "The base GitHub tools are always included. Use `--allowedTools` to add additional tools, and `--disallowedTools` to prevent specific tools from being used." `--allowedTools` is ADDITIVE, not exclusive. In CI's non-interactive mode (no `canUseTool` callback), the SDK's `default` permission mode falls through to "execute" rather than prompting. So the comment block in claude-mention.yml claiming "Tool allowlist is a security boundary — every entry is a potential RCE primitive if a prompt injection succeeds" is overstated. The allowlist doesn't gate; the actual containment is elsewhere. The "deliberately absent / tightened" entries (`Bash(npx:*)` absent, `Bash(npm install)` no-arg) are documentation of intent, not enforcement — an injected instruction that ran them would still execute. This commit replaces those comment blocks in both claude-mention.yml and claude-issue-to-pr.yml with honest accounts of where containment actually lives: 1. Token scope (repo-scoped, no cross-repo reach). 2. Branch protection on protected refs. 3. Auth gate (CODEOWNERS-driven team check) — narrow trigger surface. 4. Allowed labels list (issue-to-pr only). 5. Runner ephemerality. 6. Prompt-injection guards in the prompt itself. The notable-absences notes are kept but reframed as prompt-level discipline rather than allowlist enforcement, with explicit notes that real mitigation (e.g., `ignore-scripts=true` in `.npmrc`, deferring installs to a separate CI job) is a future concern. Comments-only. No behavior change. Paired with `HarperFast/ai-review-prompts#8` which fixes the same misleading comment in the reusable `_claude-review.yml`. claude-review.yml in this repo will be replaced by the caller pattern when #8 + the harper migration land — fixing it here is unnecessary churn. Co-Authored-By: Claude Opus 4.7 (1M context) --- .github/workflows/claude-issue-to-pr.yml | 31 +++++++++--- .github/workflows/claude-mention.yml | 62 +++++++++++++++++------- 2 files changed, 68 insertions(+), 25 deletions(-) diff --git a/.github/workflows/claude-issue-to-pr.yml b/.github/workflows/claude-issue-to-pr.yml index 58b7ab272..8ffa32777 100644 --- a/.github/workflows/claude-issue-to-pr.yml +++ b/.github/workflows/claude-issue-to-pr.yml @@ -121,14 +121,29 @@ jobs: claude_args: | --model claude-sonnet-4-6 --max-turns 72 - # Tool allowlist is a security boundary — see the allowlist - # comment in claude-mention.yml for why `Bash(npx:*)` is - # absent and why `Bash(npm install)` (no-arg) is still - # partial mitigation only: the agent has `Write`/`Edit` on - # package.json, so an injected `postinstall` script + bare - # `npm install` is a viable RCE chain. Branch protection and - # the author_association gate are what actually bound blast - # radius here. + # `--allowedTools` is convenience pre-approval, not a + # security boundary — see claude-mention.yml for the full + # rationale. Tools NOT listed below will still execute in + # CI; the entries here document the expected surface. + # + # Real containment for this workflow (issue-to-PR mode + # does AUTHORING work — branch creation, file edits, + # commits, push): + # * Token scope: `contents: write`, `pull-requests: write`, + # `issues: write`, `id-token: write`. Repo-scoped only. + # * Branch protection on protected refs (`main` / + # `release_*` / `v*.x`) — prevents pushes to those + # refs even with broader git access. + # * Auth gate — only trusted labelers (HarperFast team + # members) trigger the workflow at all. + # * Allowed labels list (`claude-fix:typo` / `:docs` / + # `:deps` / `:bug`) — narrow trigger surface. + # * Runner ephemerality. + # + # Notable absences in spirit (`Bash(npx:*)` deliberately + # not listed; `Bash(npm install)` no-arg) are prompt-level + # signals, not enforcement — same caveat as in + # claude-mention.yml. --allowedTools "Read,Write,Edit,Grep,Glob,Bash(gh pr view:*),Bash(gh pr diff:*),Bash(gh pr comment:*),Bash(gh pr create:*),Bash(gh issue view:*),Bash(gh issue comment:*),Bash(git:*),Bash(npm install),Bash(npm ci:*),Bash(npm run:*),Bash(npm test:*)" prompt: | You were invoked because issue #${{ github.event.issue.number }} diff --git a/.github/workflows/claude-mention.yml b/.github/workflows/claude-mention.yml index ec4d4327f..5f3f0e311 100644 --- a/.github/workflows/claude-mention.yml +++ b/.github/workflows/claude-mention.yml @@ -138,24 +138,52 @@ jobs: claude_args: | --model ${{ steps.mention.outputs.model }} --max-turns 48 - # Tool allowlist is a security boundary — every entry is a - # potential RCE primitive if a prompt injection succeeds. - # Deliberately ABSENT: - # * `Bash(npx:*)` — would let an injected instruction run - # arbitrary published packages. - # Deliberately TIGHTENED: + # `--allowedTools` is convenience pre-approval, not a + # security boundary. Per claude-code-action's + # `docs/configuration.md` (v1.0.110): "The base GitHub + # tools are always included. Use `--allowedTools` to add + # additional tools, and `--disallowedTools` to prevent + # specific tools from being used." Tools NOT listed below + # will still execute — in CI's non-interactive mode (no + # `canUseTool` callback), the SDK's `default` permission + # mode falls through to "execute" rather than prompting. + # + # Real containment for this workflow (mention mode does + # AUTHORING work — file edits, commits, pushes — so the + # agent has broader git/npm access than the review path): + # * Token scope: `contents: write`, `pull-requests: write`, + # `issues: write`, `id-token: write`. Repo-scoped only; + # no cross-repo reach. + # * Branch protection on protected refs (`main` / + # `release_*` / `v*.x`) — the agent CAN run + # `git push --force` if it tries; what stops protected- + # ref damage is GitHub's ruleset. + # * Auth gate (CODEOWNERS-driven HarperFast team + # check) — only trusted commenters trigger the + # workflow at all. + # * Runner ephemerality — workspace destroyed at job + # end. + # * Prompt-injection guards in the prompt itself. + # + # The "deliberately absent / tightened" entries below are + # PROMPT-LEVEL discipline, not allowlist enforcement. They + # document our intent for the agent and the expected + # surface; an injected instruction that ran (e.g.) + # `Bash(npx malicious)` would still execute, and the + # damage is bounded by the containment bullets above + # rather than by the allowlist. + # + # Notable absences in spirit (containment is elsewhere): + # * `Bash(npx:*)` — would let an injected instruction + # run arbitrary published packages. Containment: + # runner ephemerality + token scope. # * `Bash(npm install)` (no-arg, not `Bash(npm install:*)`) — - # blocks `npm install @attacker/`. BUT: the agent - # also has `Write`/`Edit` on package.json. A successful - # injection could add a malicious `postinstall` script - # and then invoke bare `npm install` to execute it, with - # GITHUB_TOKEN + the claude[bot] installation token - # reachable from the subprocess. This allowlist ALONE - # does not close that path — branch protection + the - # author_association gate are what actually bound blast - # radius. A future PR may add `ignore-scripts=true` via - # .npmrc and/or drop `Bash(npm install)` entirely, - # deferring installs to a separate CI job. + # "tightening" is illusory: the agent has `Write`/`Edit` + # on package.json and can construct an injected + # `postinstall` script + bare `npm install` chain. + # A future PR may add `ignore-scripts=true` via + # `.npmrc` and/or defer installs to a separate CI job + # for actual enforcement. --allowedTools "Read,Write,Edit,Grep,Glob,mcp__github_inline_comment__create_inline_comment,Bash(gh pr view:*),Bash(gh pr diff:*),Bash(gh pr comment:*),Bash(gh pr checkout:*),Bash(gh pr create:*),Bash(gh issue view:*),Bash(gh issue comment:*),Bash(git:*),Bash(npm install),Bash(npm ci:*),Bash(npm run:*),Bash(npm test:*)" # In agent mode the action can use the triggering comment as the # prompt, but we inline it explicitly below to guarantee the agent