Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 44 additions & 0 deletions .github/scripts/claude-lane-bot-association.test.cjs
Original file line number Diff line number Diff line change
@@ -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);
});
}
15 changes: 15 additions & 0 deletions .github/scripts/claude-security-review-fail-closed.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -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", () => {
Expand Down
15 changes: 15 additions & 0 deletions .github/workflows/claude-review.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down
20 changes: 16 additions & 4 deletions .github/workflows/claude-security-review.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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:
Expand Down