Commit 43607fa
authored
ci(security-review): drop pull_request_review trigger (broken on fork PRs) (#1310)
* ci(security-review): drop pull_request_review trigger - secrets not injected on fork PRs
GitHub does not inject repo/org secrets (vars.APP_ID, secrets.APP_PRIVATE_KEY,
the Bedrock OIDC role) into workflows triggered by pull_request_review when the
PR head is a fork, regardless of the reviewer's access level. Approving a fork
PR therefore fired the workflow but the App-token step crashed with 'Input
required and not supplied: app-id' because vars.APP_ID resolved to empty.
The label-on-pull_request_target path does inject secrets and is the only
trigger that works end-to-end for fork PRs. Drop the pull_request_review
trigger entirely; same-repo maintainer PRs auto-run on opened/synchronize as
before, community PRs require the safe-to-review label.
* ci(security-review): use bundled /security-review skill; honest summary on cancel
Three fixes rolled in:
1. Use the SDK's bundled /security-review slash command instead of inlining
the prompt. The Claude Code SDK that ships with anthropics/claude-code-action
exposes /security-review as a registered slash command (visible in the
'slash_commands' list its system init prints). It drives its own
git-diff-based context, sub-task fan-out, and false-positive filtering.
We were re-implementing all of that in a 23 KB inline prompt, plus a
fragile build step that crashed on fork PRs because
.github/prompts/security-review.md doesn't exist on the head ref.
Drops .github/prompts/security-review.md, the Build/Read prompt steps,
and the python templating. Workflow is ~300 lines lighter.
2. Concurrency: cancel-in-progress: false. Cancelling a run mid-analysis
leaves the buffer file empty but lets the if: always() summary step
fire, posting a misleading 'no high-confidence findings' comment.
Letting both runs complete is the safer default.
3. Summary step branches on steps.review.conclusion. Only post a real
findings count when the review step actually completed; on
failure/cancellation/skip, post a transparent status that says the run
didn't complete and a later run will replace it.1 parent fc67534 commit 43607fa
3 files changed
Lines changed: 73 additions & 383 deletions
This file was deleted.
0 commit comments