From 1686ac913140eb8a64772d6b04ac367467c5d177 Mon Sep 17 00:00:00 2001 From: Nathan Heskew Date: Tue, 5 May 2026 13:18:58 -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 Mirror of HarperFast/harper#473. Same fix applied to oauth's claude-mention.yml and claude-issue-to-pr.yml. Spike on harper 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 `docs/configuration.md` v1.0.110: `--allowedTools` is ADDITIVE, not exclusive — base GitHub tools are always included; tools not explicitly disallowed still execute. In CI's non-interactive mode (no `canUseTool` callback set by the action), the SDK's `default` permission mode falls through to "execute" rather than prompting. So the comment block claiming "Tool allowlist is a security boundary" was overstated. Real containment lives in token scope, branch protection on protected refs, the CODEOWNERS-driven auth gate, allowed-labels narrowing (issue-to-pr only), runner ephemerality, and the prompt-injection guards. Comments-only. No behavior change. `claude-review.yml` not touched here for the same reason as in harper #473 — it'll be replaced by the caller pattern when HarperFast/ai-review-prompts#8 lands and the oauth migration follows. Fixing it here is unnecessary churn. Co-Authored-By: Claude Opus 4.7 (1M context) --- .github/workflows/claude-issue-to-pr.yml | 41 ++++++++-------- .github/workflows/claude-mention.yml | 59 +++++++++++++++++------- 2 files changed, 65 insertions(+), 35 deletions(-) diff --git a/.github/workflows/claude-issue-to-pr.yml b/.github/workflows/claude-issue-to-pr.yml index 2a548dc..a68ef3b 100644 --- a/.github/workflows/claude-issue-to-pr.yml +++ b/.github/workflows/claude-issue-to-pr.yml @@ -128,24 +128,29 @@ jobs: claude_args: | --model claude-sonnet-4-6 --max-turns 72 - # 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. Whose subprocess reads - # GITHUB_TOKEN from env. - # Partial mitigation (NOT a full boundary): - # * `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. 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. + # `--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:*),Bash(bun install:*),Bash(bun run:*),Bash(bun 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 9fee7d0..8c7754d 100644 --- a/.github/workflows/claude-mention.yml +++ b/.github/workflows/claude-mention.yml @@ -150,24 +150,49 @@ 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/bun access than the review + # path): + # * Token scope: `contents: write`, `pull-requests: write`, + # `issues: write`, `id-token: write`. Repo-scoped only. + # * 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. + # * 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 running them would + # still execute, bounded only by the containment bullets + # above. + # + # 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. + # Future: `ignore-scripts=true` via `.npmrc` and/or + # defer installs to a separate CI job. --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:*),Bash(bun install:*),Bash(bun run:*),Bash(bun test:*)" # In agent mode the action can use the triggering comment as the # prompt, but we inline it explicitly below to guarantee the agent