diff --git a/.github/workflows/claude-mention.yml b/.github/workflows/claude-mention.yml index 01bef10..c497878 100644 --- a/.github/workflows/claude-mention.yml +++ b/.github/workflows/claude-mention.yml @@ -34,6 +34,11 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v4 + with: + # This path holds live secrets while reviewing UNTRUSTED fork content, so do + # NOT persist the GITHUB_TOKEN into .git/config (default true) — otherwise a + # `cat .git/config` could surface a usable token. + persist-credentials: false - uses: anthropics/claude-code-action@v1 with: @@ -43,10 +48,17 @@ jobs: github_token: ${{ secrets.GITHUB_TOKEN }} # No `prompt`: the @claude comment body is the instruction, so a maintainer # can ask for a full review or a targeted question. - # Fork-safe tool set: read-only git + read utils for static inspection, and - # `gh pr comment` to post the reply. Deliberately NO `npm`/build execution — - # this path may target a fork, so it never runs the PR's code; it reviews - # statically and cites the existing `validate-and-test` CI check for pass/fail. + # SECURITY: this path holds live secrets (CLAUDE_CODE_OAUTH_TOKEN + a + # writable GITHUB_TOKEN) while reviewing UNTRUSTED fork content, so its Bash + # surface is deliberately minimal. It reads the PR via `gh pr diff/view`, + # cites the existing `validate-and-test` CI check for pass/fail (NO npm/build + # execution), and posts via `gh pr comment`. Excluded on purpose: `find` + # (`find -exec` is arbitrary code execution) and general file-read utils + # (`cat`/`grep`/`jq`/`ls`, which could read .git/config or /proc) — a prompt + # injection in the fork content must not be able to reach the tokens. + # NOTE: the OWNER/MEMBER/COLLABORATOR gate above trusts any collaborator, + # including read/triage-only ones — fine for a solo repo; tighten to a + # username allowlist or a repo-permission check if collaborators are added. claude_args: | --max-turns 30 - --allowedTools "Bash(git diff:*),Bash(git log:*),Bash(git show:*),Bash(git ls-tree:*),Bash(git cat-file:*),Bash(git rev-parse:*),Bash(git status:*),Bash(cat:*),Bash(ls:*),Bash(head:*),Bash(tail:*),Bash(grep:*),Bash(wc:*),Bash(find:*),Bash(jq:*),Bash(gh pr view:*),Bash(gh pr diff:*),Bash(gh pr checks:*),Bash(gh pr comment:*)" + --allowedTools "Bash(git diff:*),Bash(git log:*),Bash(git show:*),Bash(gh pr view:*),Bash(gh pr diff:*),Bash(gh pr checks:*),Bash(gh pr comment:*)" diff --git a/.github/workflows/claude-review.yml b/.github/workflows/claude-review.yml index e5b6e9e..9293bb3 100644 --- a/.github/workflows/claude-review.yml +++ b/.github/workflows/claude-review.yml @@ -8,6 +8,12 @@ on: pull_request: types: [opened, synchronize, reopened] +# One in-flight review per PR; a new push cancels the previous run so rapid +# pushes don't stack full Claude reviews and burn quota. +concurrency: + group: claude-review-${{ github.event.pull_request.number }} + cancel-in-progress: true + permissions: contents: read pull-requests: write @@ -46,7 +52,8 @@ jobs: before reviewing. Do all of the following, then post your review as exactly ONE PR - comment using: gh pr comment --repo --body "…". + comment by running: + gh pr comment ${{ github.event.pull_request.number }} --repo ${{ github.repository }} --body "…" Reference specific file:line locations inline in the summary text (you do not have a GitHub inline-comment tool here). You are advisory only: do NOT modify code, approve, or merge. @@ -84,4 +91,4 @@ jobs: locations for any line-specific issues. claude_args: | --max-turns 40 - --allowedTools "Bash(npm ci),Bash(npm run validate),Bash(npm test),Bash(npm run lint),Bash(git diff:*),Bash(git log:*),Bash(git show:*),Bash(git ls-tree:*),Bash(git cat-file:*),Bash(git rev-parse:*),Bash(git status:*),Bash(cat:*),Bash(ls:*),Bash(head:*),Bash(tail:*),Bash(grep:*),Bash(wc:*),Bash(find:*),Bash(jq:*),Bash(gh pr comment:*)" + --allowedTools "Bash(npm ci),Bash(npm run validate),Bash(npm test),Bash(git diff:*),Bash(git log:*),Bash(git show:*),Bash(git ls-tree:*),Bash(git cat-file:*),Bash(git rev-parse:*),Bash(git status:*),Bash(cat:*),Bash(ls:*),Bash(head:*),Bash(tail:*),Bash(grep:*),Bash(wc:*),Bash(jq:*),Bash(gh pr comment:*)" diff --git a/docs/superpowers/specs/2026-07-06-automated-pr-review-design.md b/docs/superpowers/specs/2026-07-06-automated-pr-review-design.md index b8889ee..051eb91 100644 --- a/docs/superpowers/specs/2026-07-06-automated-pr-review-design.md +++ b/docs/superpowers/specs/2026-07-06-automated-pr-review-design.md @@ -230,6 +230,31 @@ ability to trigger downstream workflows) — it would swap `github_token` for - The action sanitizes prompt-injection vectors (hidden HTML/attrs) in PR content; the actor gate is the primary defense on top of that. +### Post-audit hardening (adversarial review of the workflows) + +A multi-agent adversarial audit of the two workflows surfaced that "restricted +allowlist" is only as good as the *specific* commands allowed. Confirmed fixes: + +- **`Bash(find:*)` removed from both files.** `claude-code-action` matches Bash + permissions by command-string prefix, and `find . -exec sh -c '' \;` is + a *single* command (no shell operator to split on) — so `find:*` was a full + arbitrary-code-execution escape. On the secret-bearing mention path (which reviews + untrusted fork content) that was a real token-exfiltration vector, not hygiene. +- **Mention path Bash surface minimized** to `gh pr view/diff/checks/comment` + + read-only `git diff/log/show`. The general file-read utils (`cat`/`grep`/`jq`/`ls`) + were dropped there because prefix rules can't pin them to the repo tree, so they + could read `/proc/self/environ` or `.git/config`. +- **`persist-credentials: false`** on the mention checkout, so the `GITHUB_TOKEN` is + not written into `.git/config` where a read primitive could recover it. +- **Residual risk (documented, accepted):** the mention path still co-locates a + subscription token with an agent reading untrusted content; the real defenses are + the human-gated trigger, the trusted-actor gate, and the minimized (non-exec, + non-arbitrary-read) toolset. A stronger future option is to run the fork static + pass without secrets at all. +- **Nit:** the `OWNER/MEMBER/COLLABORATOR` gate trusts any collaborator tier + (incl. read/triage). Acceptable for a solo repo; tighten to a username allowlist or + an explicit repo-permission check before adding collaborators. + ## Testing / verification plan 1. **Workflow lint:** confirm both YAML files parse (e.g. `actionlint` if available,