diff --git a/.github/scripts/claude-lane-bot-association.test.cjs b/.github/scripts/claude-lane-bot-association.test.cjs new file mode 100644 index 00000000..b50221ba --- /dev/null +++ b/.github/scripts/claude-lane-bot-association.test.cjs @@ -0,0 +1,44 @@ +"use strict"; + +// Job-level defence-in-depth (#443): bot actors other than dependabot[bot] +// require the PR author's association to be OWNER, MEMBER, or COLLABORATOR. +// Pins the clause on both review lanes' main attempt jobs (retry is in-job). + +const assert = require("node:assert/strict"); +const fs = require("node:fs"); +const path = require("node:path"); +const test = require("node:test"); + +const repositoryRoot = path.join(__dirname, "..", ".."); +const workflowsDir = path.join(repositoryRoot, ".github", "workflows"); + +const BOT_GUARD = /!endsWith\(github\.actor, '\[bot\]'\)/u; +const DEPENDABOT_EXEMPT = /github\.actor == 'dependabot\[bot\]'/u; +const AUTHOR_ASSOCIATION = + /contains\(format\(',\{0\},', 'OWNER,MEMBER,COLLABORATOR'\), format\(',\{0\},', github\.event\.pull_request\.author_association\)\)/u; + +function reviewJobCondition(fileName, jobName) { + const workflow = fs.readFileSync(path.join(workflowsDir, fileName), "utf8"); + const start = workflow.indexOf(` ${jobName}:`); + assert.notEqual(start, -1, `${fileName} must declare job ${jobName}`); + const slice = workflow.slice(start); + const end = slice.indexOf(" - name: Reject privileged triggers"); + assert.notEqual( + end, + -1, + `${fileName} ${jobName} job must reach the tripwire step`, + ); + return slice.slice(0, end); +} + +for (const { file, job } of [ + { file: "claude-review.yml", job: "review" }, + { file: "claude-security-review.yml", job: "security-review" }, +]) { + test(`${file}: ${job} job gates bot actors on PR author association (#443)`, () => { + const jobCondition = reviewJobCondition(file, job); + assert.match(jobCondition, BOT_GUARD); + assert.match(jobCondition, DEPENDABOT_EXEMPT); + assert.match(jobCondition, AUTHOR_ASSOCIATION); + }); +} diff --git a/.github/scripts/claude-security-review-fail-closed.test.cjs b/.github/scripts/claude-security-review-fail-closed.test.cjs index ffb57069..d19b7590 100644 --- a/.github/scripts/claude-security-review-fail-closed.test.cjs +++ b/.github/scripts/claude-security-review-fail-closed.test.cjs @@ -367,6 +367,21 @@ test("fork, out-of-scope and skip-actor PRs never reach the ruling step", () => /contains\(format\(',\{0\},', inputs\.skip-actors\)/u, "the skip-actors exception is operator-ratified (ADR 0002) and must stay job-level", ); + assert.match( + jobCondition, + /!endsWith\(github\.actor, '\[bot\]'\)/u, + "bot actors must be gated on PR author association (#443)", + ); + assert.match( + jobCondition, + /github\.actor == 'dependabot\[bot\]'/u, + "dependabot[bot] must stay exempt from the author-association gate (#443)", + ); + assert.match( + jobCondition, + /contains\(format\(',\{0\},', 'OWNER,MEMBER,COLLABORATOR'\), format\(',\{0\},', github\.event\.pull_request\.author_association\)\)/u, + "non-dependabot bot actors must require OWNER, MEMBER, or COLLABORATOR author_association (#443)", + ); }); test("the kill-switches gate the review job only, never the changes job", () => { diff --git a/.github/workflows/claude-review.yml b/.github/workflows/claude-review.yml index 33627d13..d305e450 100644 --- a/.github/workflows/claude-review.yml +++ b/.github/workflows/claude-review.yml @@ -86,6 +86,14 @@ name: claude-review # it is ever enabled, re-derive this entry rather than assuming it holds. # Both apps push to SAME-REPO branches, so neither reaches a fork's # privileged context, which the fork rule above already closes. +# Defence in depth (#443): the review job also gates bot actors (any +# `github.actor` ending in `[bot]`) on the PR author's association. +# `dependabot[bot]` is exempt — its PRs may carry CONTRIBUTOR or NONE. +# Any other bot may trigger only when `author_association` is OWNER, +# MEMBER, or COLLABORATOR (e.g. `cursor[bot]` pushing to an operator's +# branch). Human actors are unchanged; the check is scoped to +# `pull_request` like the draft gate because dispatch carries no +# author_association in the event payload. # - `display-report` / `show_full_output` stay off (public-repo log-leak risk). # - The inline-comment tool is now on the hot path (the prompt routes every # line-anchorable finding through it), so untrusted diff content — including @@ -332,12 +340,19 @@ jobs: # Skip configured bot/automation actors (saves runner minutes). The action's # own write-access gate is the security control; this is just an optimization. # Comma-wrap both sides so a substring of one actor can't match another. + # Defence-in-depth (#443): bot actors other than dependabot[bot] require + # the PR author's association to be OWNER, MEMBER, or COLLABORATOR — see + # SECURITY MODEL above. Scoped to pull_request like the draft gate. # Draft PRs are not reviewed either (the cadence is open/ready/reopen); # the draft test is scoped to pull_request so other events still run. if: >- ${{ vars.CLAUDE_LANES_DISABLED != 'true' && vars.CLAUDE_REVIEW_DISABLED != 'true' && !contains(format(',{0},', inputs.skip-actors), format(',{0},', github.actor)) + && (!endsWith(github.actor, '[bot]') + || github.actor == 'dependabot[bot]' + || (github.event_name != 'pull_request' + || contains(format(',{0},', 'OWNER,MEMBER,COLLABORATOR'), format(',{0},', github.event.pull_request.author_association)))) && (github.event_name != 'pull_request' || github.event.pull_request.draft == false) }} permissions: diff --git a/.github/workflows/claude-security-review.yml b/.github/workflows/claude-security-review.yml index 37ae8a3f..2546667f 100644 --- a/.github/workflows/claude-security-review.yml +++ b/.github/workflows/claude-security-review.yml @@ -223,10 +223,15 @@ name: claude-security-review # (https://cursor.com/docs/cloud-agent). Cursor's Bugbot, which by contrast # runs "for all contributors ... regardless of team membership" # (https://cursor.com/docs/bugbot), is DISABLED for this org's account — -# re-derive this entry if that changes. Defence in depth for the residual -# (Cursor states these as product prerequisites, not as a security -# boundary) is tracked separately: gate a bot actor on the PR author's -# association. Note the exposure this does NOT create: cursor[bot] already +# re-derive this entry if that changes. Defence in depth (#443): the +# review job also gates bot actors (any `github.actor` ending in `[bot]`) +# on the PR author's association. `dependabot[bot]` is exempt — its PRs +# may carry CONTRIBUTOR or NONE. Any other bot may trigger only when +# `author_association` is OWNER, MEMBER, or COLLABORATOR (e.g. +# `cursor[bot]` pushing to an operator's branch). Human actors are +# unchanged; the check is scoped to `pull_request` like the fork gate +# because dispatch carries no author_association in the event payload. +# Note the exposure this does NOT create: cursor[bot] already # holds contents:write org-wide, so an actor able to drive it could already # push branches that run every same-repo workflow. # - `display_report` / `show_full_output` stay off (public-repo log-leak risk). @@ -822,6 +827,9 @@ jobs: # Skip configured bot/automation actors (saves runner minutes). The action's # own write-access gate is the security control; this is just an optimization. # Comma-wrap both sides so a substring of one actor can't match another. + # Defence-in-depth (#443): bot actors other than dependabot[bot] require + # the PR author's association to be OWNER, MEMBER, or COLLABORATOR — see + # SECURITY MODEL above. Scoped to pull_request like the fork gate. # # `!cancelled() && needs.changes.outputs.relevant != 'false'` is the # always-report gate: relevant=false (no security-sensitive path changed) @@ -854,6 +862,10 @@ jobs: && vars.CLAUDE_SECURITY_REVIEW_DISABLED != 'true' && needs.changes.outputs.relevant != 'false' && !contains(format(',{0},', inputs.skip-actors), format(',{0},', github.actor)) + && (!endsWith(github.actor, '[bot]') + || github.actor == 'dependabot[bot]' + || (github.event_name != 'pull_request' + || contains(format(',{0},', 'OWNER,MEMBER,COLLABORATOR'), format(',{0},', github.event.pull_request.author_association)))) && (github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository) }} permissions: