diff --git a/.github/workflows/pr-security-review.yml b/.github/workflows/pr-security-review.yml index 6d2184f84..daf9fc176 100644 --- a/.github/workflows/pr-security-review.yml +++ b/.github/workflows/pr-security-review.yml @@ -2,7 +2,7 @@ name: Claude Security Review on: pull_request_target: - types: [opened, reopened, synchronize] + types: [opened, reopened, synchronize, labeled] pull_request_review: types: [submitted] workflow_dispatch: @@ -30,11 +30,13 @@ concurrency: jobs: authorize: runs-on: ubuntu-latest - # On pull_request_review events, only proceed when the review state is 'approved'. - # Comment/changes-requested reviews are ignored to avoid running the workflow on every review. + # Filter event subtypes early: + # - pull_request_review: only the 'approved' state authorizes (comment/changes-requested are ignored). + # - pull_request_target labeled: only the 'safe-to-review' label triggers (other label changes are ignored). if: | - github.event_name != 'pull_request_review' || - github.event.review.state == 'approved' + (github.event_name != 'pull_request_review' || github.event.review.state == 'approved') && + (github.event_name != 'pull_request_target' || github.event.action != 'labeled' || + github.event.label.name == 'safe-to-review') outputs: authorized: ${{ steps.auth.outputs.authorized || steps.dispatch-auth.outputs.authorized }} steps: @@ -44,14 +46,24 @@ jobs: uses: actions/github-script@v9 with: script: | - // pull_request_target: gate on the PR author (maintainer-authored PRs auto-run on open/reopen). - // pull_request_review: gate on the reviewer who approved (so a maintainer approving a community PR - // authorizes the security review). + // pull_request_target opened/reopened/synchronize: gate on the PR author. + // pull_request_target labeled (safe-to-review): gate on the labeler (sender). + // pull_request_review (approved): gate on the reviewer who approved. const isApproval = context.eventName === 'pull_request_review'; - const user = isApproval - ? context.payload.review.user.login - : context.payload.pull_request.user.login; - const reason = isApproval ? `approver ${user}` : `PR author ${user}`; + const isLabel = + context.eventName === 'pull_request_target' && context.payload.action === 'labeled'; + let user; + let reason; + if (isApproval) { + user = context.payload.review.user.login; + reason = `approver ${user}`; + } else if (isLabel) { + user = context.payload.sender.login; + reason = `labeler ${user}`; + } else { + user = context.payload.pull_request.user.login; + reason = `PR author ${user}`; + } try { await github.rest.teams.getMembershipForUserInOrg({ org: context.repo.owner, @@ -94,12 +106,23 @@ jobs: env: AWS_REGION: us-west-2 steps: + # Generate the GitHub App token first so every subsequent github-script step can + # use it. The default GITHUB_TOKEN is read-only on pull_request_target / + # pull_request_review events from forks, which makes label/comment writes 403. + - name: Generate GitHub App token + id: app-token + uses: actions/create-github-app-token@v1 + with: + app-id: ${{ vars.APP_ID }} + private-key: ${{ secrets.APP_PRIVATE_KEY }} + - name: Resolve PR number id: pr uses: actions/github-script@v9 env: PR_NUMBER_INPUT: ${{ inputs.pr_number }} with: + github-token: ${{ steps.app-token.outputs.token }} script: | const num = context.eventName === 'workflow_dispatch' @@ -119,6 +142,7 @@ jobs: env: PR_NUMBER: ${{ steps.pr.outputs.number }} with: + github-token: ${{ steps.app-token.outputs.token }} script: | const prNumber = parseInt(process.env.PR_NUMBER, 10); try { @@ -181,13 +205,6 @@ jobs: echo "prompt_file=$OUT" >> "$GITHUB_OUTPUT" echo "Prompt size: $(wc -c < "$OUT") bytes" - - name: Generate GitHub App token - id: app-token - uses: actions/create-github-app-token@v1 - with: - app-id: ${{ vars.APP_ID }} - private-key: ${{ secrets.APP_PRIVATE_KEY }} - - name: Configure AWS credentials (OIDC) uses: aws-actions/configure-aws-credentials@v6 with: @@ -264,6 +281,7 @@ jobs: env: PR_NUMBER: ${{ steps.pr.outputs.number }} with: + github-token: ${{ steps.app-token.outputs.token }} script: | const prNumber = parseInt(process.env.PR_NUMBER, 10); try { diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 2c58358e4..296863c11 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -51,13 +51,17 @@ wanted' issues is a great place to start. ## Maintainer notes: Claude security review on community PRs The `Claude Security Review` workflow runs automatically on maintainer-authored PRs (opened/reopened) and on community -PRs as soon as a maintainer submits an **approving review**. PRs from non-collaborators are otherwise skipped — the -approval is the gate, so a maintainer must manually review the diff before the automated reviewer runs. +PRs once a maintainer either submits an **approving review** _or_ applies the **`safe-to-review`** label. PRs from +non-collaborators are otherwise skipped — that explicit signal is the gate, so a maintainer must manually review the +diff before the automated reviewer runs. -To re-run the review on a later commit, submit another approving review (resolves to a fresh workflow run), or trigger -the `Claude Security Review` workflow manually from the Actions tab with the PR number. Note that manual dispatch can -verify the analysis and prompt plumbing but cannot post inline comments — the action's inline-comment MCP server only -attaches on PR-context events (`pull_request_target`, `pull_request_review`). +The label flow is convenient when you want to kick the security review without committing to an approval: drop the label +on the PR and the workflow fires once. Removing and re-applying the label re-triggers the review. + +To re-run the review on a later commit, submit another approving review, re-apply the label, or trigger the +`Claude Security Review` workflow manually from the Actions tab with the PR number. Note that manual dispatch can verify +the analysis and prompt plumbing but cannot post inline comments — the action's inline-comment MCP server only attaches +on PR-context events (`pull_request_target`, `pull_request_review`). ## Code of Conduct