fix(workflow): NO-JIRA restore id-token permission and fix concurrency race in claude-review

[belumontoya]

Summary

Fixes two bugs introduced by the SEC-2156 security hardening in #1138 that broke the Claude Code Review workflow.

Bug 1 — id-token: write permission removed (Critical)

claude-code-action@v1 uses GitHub's OIDC provider to authenticate. It exchanges a short-lived JWT (via id-token: write) for API credentials. Without this permission, the action cannot start at all — it retries 3 times and crashes:

Attempt 3 failed: Could not fetch an OIDC token.
Did you remember to add `id-token: write` to your workflow permissions?
##[error] Action failed with error: Could not fetch an OIDC token.

This was the error on PR #1142's review run (run 23315006702).

Fix: Restore id-token: write. This permission is scoped to the workflow run — it allows the action to request a token identifying itself to Anthropic's API. It does not grant write access to repo contents or PRs beyond what pull-requests: write already provides.

Security note for aman-md: id-token: write only enables the runner to mint a short-lived OIDC JWT that identifies the workflow run. It cannot be used to impersonate users, access other repos, or escalate permissions. The GitHub docs confirm this is the intended use for third-party action authentication. Removing it was the right instinct (least privilege), but claude-code-action genuinely requires it.

Bug 2 — Concurrency group race condition (High)

The labeled trigger fires once per label added. All label events on the same PR shared the concurrency group claude-review-<pr_number> with cancel-in-progress: true. When multiple labels are added in quick succession (common workflow), the last label's run cancels all previous ones — including the legitimate claude-review run.

Evidence from PR #1141:

19:13:14  ninarepetto adds "claude-review" label
19:13:17  → Run A starts (condition TRUE) → enters concurrency group
19:13:38  ninarepetto adds "no-visual-test" label  
19:13:42  → Run B enters SAME group → cancels Run A
          → Run B condition: 'no-visual-test' == 'claude-review' → FALSE → skipped

Both runs show as "skipped" in the Actions UI, making this very hard to diagnose.

Fix: Include the label name in the concurrency group key (claude-review-<pr>-<label>). Each label event now gets its own lane. The claude-review run can only be cancelled by another claude-review event on the same PR (e.g., removing and re-adding the label), which is the desired behavior.

What's NOT changed

All security hardening from #1138 is preserved:

• ✅ labeled trigger (requires maintainer to add claude-review label)
• ✅ Base branch checkout (prevents prompt injection via attacker files)
• ✅ Restricted tool allowlist (Read, Glob, Grep + gh pr diff/view only)
• ✅ Prompt injection defense (ignores CLAUDE.md / .claude/rules/)
• ✅ Bot actor exclusion

Known remaining issue (not addressed here)

workflow_dispatch checkout falls back to github.sha (default branch HEAD) instead of the target PR's code. This means manual re-triggers review the wrong code. Flagging for discussion — fixing this without re-introducing the prompt injection vector from #1138 needs a design decision.

Test plan

• Add claude-review label to a test PR → review should run successfully (no OIDC error)
• Add claude-review + another label in quick succession → review should complete (not get cancelled)
• Add a non-claude-review label → workflow should still skip (no change in behavior)
• Verify workflow_dispatch still works for manual triggers

[belumontoya]

aman-md I had to do some changes to the claude run because it was not working, can you review them please?

[Brad Paugh (braddialpad)]

Fine by me, but we should probably wait for aman-md to confirm

[belumontoya]

aman-md Implemented the sandboxed tools you suggested. Here's what was added:

New files — .github/scripts/safe-read.sh, safe-grep.sh, safe-glob.sh

Each script:

• Resolves the requested path with realpath -m (handles symlinks + ../ traversal)
• Validates the resolved path starts with $REPO_DIR ($GITHUB_WORKSPACE)
• Rejects .env / .env.* files and excludes .git/ directories
• Exits with error on any violation

Workflow changes:

• Added REPO_DIR env var set to ${{ github.workspace }}
• Added a "Set up sandboxed tools" step that symlinks the scripts to /usr/local/bin/
• Replaced Read, Glob, Grep in --allowed-tools with Bash(safe-read:*), Bash(safe-grep:*), Bash(safe-glob:*)
• Updated the prompt to instruct Claude to use the safe-* commands

This blocks the /proc/self/environ vector entirely — even if prompt injection succeeds, Claude physically cannot access files outside the repo. Can you review when you get a chance?

[Francis Rupert (francisrupert)]

Where we at with this one since it's been sitting here for 2 weeks? This is obviously a sensitive one, so do we just need aman-md's review and approval? As code owners we can of course approve it, but would only do so after a aman-md's.

[belumontoya]

Francis Rupert (@francisrupert) this is a high security risk, and we should not rush into it. I'll reach out on Monday to see if Aman found another scenario we need to account for or if we are good as is.

[aman-md]

Hey Belu, the changes look good to me.

[Brad Paugh (braddialpad)]

belumontoya I think it will fix the checks if you merge in staging