diff --git a/components/claude-lanes-hosted/claude-review.yml b/components/claude-lanes-hosted/claude-review.yml index 22109777..ff7df756 100644 --- a/components/claude-lanes-hosted/claude-review.yml +++ b/components/claude-lanes-hosted/claude-review.yml @@ -15,9 +15,10 @@ name: pr-review-hosted # elevate it). # # The reusable skips fork PRs (they receive no secrets and a read-only token) -# and every `[bot]` actor, so neither is reviewed by design. On a public -# repository that means the lane reviews only pull requests from branches of -# this repository: fork and Dependabot pull requests get no secrets. +# and every `[bot]` actor whose sender id is not in `allowed-bot-sender-ids` +# (below), so neither is reviewed by design. On a public repository that means +# the lane reviews only pull requests from branches of this repository: fork +# and Dependabot pull requests get no secrets. # # RUNNER PLACEMENT: hosted shape. This file runs the lane on the GitHub-hosted # `ubuntu-24.04` runner, which the standards `runner-policy` component admits @@ -80,9 +81,14 @@ jobs: # reviewed-head marker comment id-token: write # OIDC, mints the Claude GitHub App token (required # even with an OAuth/API-key credential) - uses: melodic-software/ci-workflows/.github/workflows/pr-review.yml@ab83b01273026ab5c23c6f3b40e946d97a863fa2 # v0.39.3 + uses: melodic-software/ci-workflows/.github/workflows/pr-review.yml@a92020292976c2e9b50f7adc922693a45a6c1226 # v0.40.0 with: runner: ubuntu-24.04 + # Bot senders this lane reviews, by numeric user id: the + # automation-lanes App, so a commit it pushes to a pull request is + # reviewed. Where the repository does not set this variable the value is + # empty and the reusable reviews no bot. + allowed-bot-sender-ids: ${{ vars.AUTOMATION_LANES_APP_SENDER_ID }} # Pass only the one named secret (least privilege) rather than # `secrets: inherit`, which would forward every parent secret to the # called workflow. diff --git a/components/claude-lanes-hosted/claude-security-review.yml b/components/claude-lanes-hosted/claude-security-review.yml index d49727ee..40224d2b 100644 --- a/components/claude-lanes-hosted/claude-security-review.yml +++ b/components/claude-lanes-hosted/claude-security-review.yml @@ -25,10 +25,11 @@ name: pr-review-security-hosted # `pr-review-security / security-review`, never arms. # # The reusable skips fork PRs (they receive no secrets and a read-only token) -# and every `[bot]` actor; a required check does NOT prove a fork PR was -# reviewed (see the reusable's header). On a public repository the lane -# reviews only pull requests from branches of this repository: fork and -# Dependabot pull requests get no secrets. +# and every `[bot]` actor whose sender id is not in `allowed-bot-sender-ids` +# (below); a required check does NOT prove a fork PR was reviewed (see the +# reusable's header). On a public repository the lane reviews only pull +# requests from branches of this repository: fork and Dependabot pull requests +# get no secrets. # # RUNNER PLACEMENT: hosted shape, the sibling of # components/claude-lanes/claude-security-review.yml, differing from it only @@ -81,9 +82,14 @@ jobs: concurrency: group: pr-review-security-${{ github.event.pull_request.number || github.run_id }} cancel-in-progress: false - uses: melodic-software/ci-workflows/.github/workflows/pr-review-security.yml@ab83b01273026ab5c23c6f3b40e946d97a863fa2 # v0.39.3 + uses: melodic-software/ci-workflows/.github/workflows/pr-review-security.yml@a92020292976c2e9b50f7adc922693a45a6c1226 # v0.40.0 with: runner: ubuntu-24.04 + # Bot senders this lane reviews, by numeric user id: the + # automation-lanes App, so a commit it pushes to a pull request is + # reviewed. Where the repository does not set this variable the value is + # empty and the reusable reviews no bot. + allowed-bot-sender-ids: ${{ vars.AUTOMATION_LANES_APP_SENDER_ID }} # Pass only the one named secret (least privilege) rather than # `secrets: inherit`, which would forward every parent secret to the # called workflow. diff --git a/components/claude-lanes/claude-review.yml b/components/claude-lanes/claude-review.yml index 98c977ca..902a978b 100644 --- a/components/claude-lanes/claude-review.yml +++ b/components/claude-lanes/claude-review.yml @@ -14,7 +14,8 @@ name: pr-review # elevate it). # # The reusable skips fork PRs (they receive no secrets and a read-only token) -# and every `[bot]` actor, so neither is reviewed by design. +# and every `[bot]` actor whose sender id is not in `allowed-bot-sender-ids` +# (below), so neither is reviewed by design. # # RUNNER PLACEMENT: this component names the governed managed fleet review # label directly. Every target that manages it is a private repository @@ -75,9 +76,14 @@ jobs: # reviewed-head marker comment id-token: write # OIDC, mints the Claude GitHub App token (required # even with an OAuth/API-key credential) - uses: melodic-software/ci-workflows/.github/workflows/pr-review.yml@ab83b01273026ab5c23c6f3b40e946d97a863fa2 # v0.39.3 + uses: melodic-software/ci-workflows/.github/workflows/pr-review.yml@a92020292976c2e9b50f7adc922693a45a6c1226 # v0.40.0 with: runner: melodic-review-ubuntu-24.04-x64 + # Bot senders this lane reviews, by numeric user id: the + # automation-lanes App, so a commit it pushes to a pull request is + # reviewed. Where the repository does not set this variable the value is + # empty and the reusable reviews no bot. + allowed-bot-sender-ids: ${{ vars.AUTOMATION_LANES_APP_SENDER_ID }} # Pass only the one named secret (least privilege) rather than # `secrets: inherit`, which would forward every parent secret to the # called workflow. diff --git a/components/claude-lanes/claude-security-review.yml b/components/claude-lanes/claude-security-review.yml index b83e1600..6dfd6888 100644 --- a/components/claude-lanes/claude-security-review.yml +++ b/components/claude-lanes/claude-security-review.yml @@ -24,8 +24,9 @@ name: pr-review-security # `pr-review-security / security-review`, never arms. # # The reusable skips fork PRs (they receive no secrets and a read-only token) -# and every `[bot]` actor; a required check does NOT prove a fork PR was -# reviewed (see the reusable's header). +# and every `[bot]` actor whose sender id is not in `allowed-bot-sender-ids` +# (below); a required check does NOT prove a fork PR was reviewed (see the +# reusable's header). # # RUNNER PLACEMENT: this component names the governed managed fleet review # label directly, for the same reason and under the same inventory constraint @@ -77,9 +78,14 @@ jobs: concurrency: group: pr-review-security-${{ github.event.pull_request.number || github.run_id }} cancel-in-progress: false - uses: melodic-software/ci-workflows/.github/workflows/pr-review-security.yml@ab83b01273026ab5c23c6f3b40e946d97a863fa2 # v0.39.3 + uses: melodic-software/ci-workflows/.github/workflows/pr-review-security.yml@a92020292976c2e9b50f7adc922693a45a6c1226 # v0.40.0 with: runner: melodic-review-ubuntu-24.04-x64 + # Bot senders this lane reviews, by numeric user id: the + # automation-lanes App, so a commit it pushes to a pull request is + # reviewed. Where the repository does not set this variable the value is + # empty and the reusable reviews no bot. + allowed-bot-sender-ids: ${{ vars.AUTOMATION_LANES_APP_SENDER_ID }} # Pass only the one named secret (least privilege) rather than # `secrets: inherit`, which would forward every parent secret to the # called workflow. diff --git a/components/runner-policy/README.md b/components/runner-policy/README.md index b12e5fbe..06ae002b 100644 --- a/components/runner-policy/README.md +++ b/components/runner-policy/README.md @@ -1171,6 +1171,16 @@ message. `maintenance-sync-standards` bumps `actions/setup-node` from v7.0.0 to v7.1.0 (`949feb2`) and is declined on shape, because its `sync` job routes through `needs`. `pr-run-checks` keeps its blob, but its `$/markdownlint` action tree takes the same `setup-node` bump. +`pr-review` and `pr-review-security` are also registered at the v0.40.0 tag +`a92020292976c2e9b50f7adc922693a45a6c1226`, each equal to its v0.39.8 entry +plus the input `allowed-bot-sender-ids`, signed off by hand on 2026-10-11. +That input admits a bot sender whose numeric user id the caller lists and hands +only that sender's login to the action's `allowed_bots`; an empty or malformed +list admits no bot. Neither lane changes a secret, a workflow or job +permission, or `runs-on` routing, and no composite action either lane reaches +changes. The lockstep declines both on `inputs`. The claude lane callers pass +`${{ vars.AUTOMATION_LANES_APP_SENDER_ID }}`, which is empty, so no bot is +reviewed, in a repository that does not set the variable. The Zizmor contract at `de50a08b6093d231519ee7a4c9371db76c0a7e1e` uses its reviewed `runner` input and checksum-verified native Linux binary, so enrolled consumers may route that advisory lane onto the managed fleet diff --git a/components/runner-policy/policy.json b/components/runner-policy/policy.json index 32183ea5..751c5ca3 100644 --- a/components/runner-policy/policy.json +++ b/components/runner-policy/policy.json @@ -63,6 +63,19 @@ "id-token": "write" } }, + "melodic-software/ci-workflows/.github/workflows/pr-review.yml@a92020292976c2e9b50f7adc922693a45a6c1226": { + "routing": "runner-input", + "runnerInput": "runner", + "allowedInputs": ["runner", "incremental-review", "allowed-bot-sender-ids"], + "allowedSecrets": { + "CLAUDE_CODE_OAUTH_TOKEN": "${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }}" + }, + "allowedCallerPermissions": { + "contents": "read", + "pull-requests": "write", + "id-token": "write" + } + }, "melodic-software/ci-workflows/.github/workflows/pr-review-security.yml@ab83b01273026ab5c23c6f3b40e946d97a863fa2": { "routing": "runner-input", "runnerInput": "runner", @@ -89,6 +102,19 @@ "id-token": "write" } }, + "melodic-software/ci-workflows/.github/workflows/pr-review-security.yml@a92020292976c2e9b50f7adc922693a45a6c1226": { + "routing": "runner-input", + "runnerInput": "runner", + "allowedInputs": ["runner", "incremental-review", "allowed-bot-sender-ids"], + "allowedSecrets": { + "CLAUDE_CODE_OAUTH_TOKEN": "${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }}" + }, + "allowedCallerPermissions": { + "contents": "read", + "pull-requests": "write", + "id-token": "write" + } + }, "melodic-software/ci-workflows/.github/workflows/intake-label-needs-triage.yml@ab83b01273026ab5c23c6f3b40e946d97a863fa2": { "routing": "runner-input", "runnerInput": "runner", diff --git a/components/runner-policy/runner-policy.test.mjs b/components/runner-policy/runner-policy.test.mjs index 05e389ee..332ff2e9 100644 --- a/components/runner-policy/runner-policy.test.mjs +++ b/components/runner-policy/runner-policy.test.mjs @@ -37,6 +37,7 @@ const REPINE_LANE_SHA_V0_29_0 = "6567856ef6070a4a1c9c2691f610278a7f2de153"; const REPINE_LANE_SHA_V0_29_1 = "0d3e6a6f3851cf678f82fa9a8a17f10faa909ac9"; const REPINE_LANE_SHA_V0_30_1 = "35880dcbb2f174aac90159e276dc7eddf1bc20b9"; const REPINE_LANE_SHA_V0_39_8 = "594987abe8657a40bccc13d301f1f0d5cccb5348"; +const REPINE_LANE_SHA_V0_40_0 = "a92020292976c2e9b50f7adc922693a45a6c1226"; const STANDARDS_SYNC_SHA = "35f2684ac953794b854bac1959df00e74eeca1d9"; const REUSABLE_PATH = "melodic-software/ci-workflows/.github/workflows/osv-scanner.yml"; const REUSABLE_REFERENCE = `${REUSABLE_PATH}@${SHA}`; @@ -9027,6 +9028,28 @@ test("every reusable contract at the v0.39.8 tag copies its v0.39.3 entry forwar ); }); +// v0.40.0 adds one input to the two review lanes, `allowed-bot-sender-ids`, +// and changes no secret, permission or routing field; no other reusable is +// registered at it. +test("the review contracts at the v0.40.0 tag are their v0.39.8 entries plus allowed-bot-sender-ids", () => { + const contracts = BASE_POLICY.approvedReusableWorkflowContracts; + const reusables = ["pr-review", "pr-review-security"]; + for (const reusable of reusables) { + const workflowPath = `melodic-software/ci-workflows/.github/workflows/${reusable}.yml`; + const previous = contracts[`${workflowPath}@${REPINE_LANE_SHA_V0_39_8}`]; + assert.ok(previous, `expected a v0.39.8 contract for ${reusable}`); + assert.deepEqual( + contracts[`${workflowPath}@${REPINE_LANE_SHA_V0_40_0}`], + { ...previous, allowedInputs: [...previous.allowedInputs, "allowed-bot-sender-ids"] }, + `${reusable} at the v0.40.0 tag is not its v0.39.8 entry plus allowed-bot-sender-ids`, + ); + } + assert.equal( + Object.keys(contracts).filter((key) => key.endsWith(`@${REPINE_LANE_SHA_V0_40_0}`)).length, + reusables.length, + ); +}); + // Convergence as a property, never as a hardcoded SHA. The daily // claude-lanes-repin lane rewrites these components on every ci-workflows // release, and a test naming one revision would go red by construction on diff --git a/conventions/review/ai-review-bot-composition.md b/conventions/review/ai-review-bot-composition.md index 627c8da6..722dd9b7 100644 --- a/conventions/review/ai-review-bot-composition.md +++ b/conventions/review/ai-review-bot-composition.md @@ -6,7 +6,7 @@ Which automated AI reviewers a repository runs follows its governance class in ` - **Archived (`Archived: true`)**: a repository frozen read-only (e.g. a superseded snapshot). No PR-time reviewer: a finding against closed history has no one positioned to act on it. - **No CI surface (`RequiresCi: false`, not archived)**: a content, artifact, or single-purpose test repository with no `ci-status`-emitting workflow (for example: an artifact store, or a private acceptance-only canary target). No PR-time reviewer today, because there is no PR-time workflow surface for one to hang off. This is a **deferred**, not a permanent, absence: if such a repo grows a workflow surface, it moves into the governed class below and should adopt that class's default composition. Trigger: the first `.github/workflows/*.yml` lands in the repo. -- **Governed (`RequiresCi: true`, not archived)**: the default and largest class, spanning both public and private repos, and both org-originated and personal-account-transferred repos alike. Two PR-time AI lanes, split by scope rather than doubled up. The **code-review lane**: the org's `pr-review.yml` reusable-workflow caller, reviewing every non-draft PR, with two intentional exclusions built into the reusable lane: fork PRs (no secrets and no automated review, by design) and PRs whose triggering actor is any `[bot]` account, including `melodic-standards-sync[bot]` (sync PRs distribute content already reviewed at its source in `standards`, so a second AI pass adds token cost without marginal catch, while the deterministic gates still run on them). The **security lane**: the org's `pr-review-security.yml` caller, reviewing every non-draft PR head with the same fork and bot exclusions; it has no path gating. The two lanes' scopes are mutually exclusive, a split the root `REVIEW.md` records, so each finding has exactly one owning reviewer; the security lane is scope partitioning, not the same-scope second reviewer whose evidence-gated exception defines the high-traffic class below. +- **Governed (`RequiresCi: true`, not archived)**: the default and largest class, spanning both public and private repos, and both org-originated and personal-account-transferred repos alike. Two PR-time AI lanes, split by scope rather than doubled up. The **code-review lane**: the org's `pr-review.yml` reusable-workflow caller, reviewing every non-draft PR, with two intentional exclusions built into the reusable lane: fork PRs (no secrets and no automated review, by design) and PRs whose triggering actor is any `[bot]` account, including `melodic-standards-sync[bot]` (sync PRs distribute content already reviewed at its source in `standards`, so a second AI pass adds token cost without marginal catch, while the deterministic gates still run on them). The one exception is a bot whose sender id the caller lists in the reusable's `allowed-bot-sender-ids` input (ci-workflows v0.40.0): the synced callers pass the repository variable `AUTOMATION_LANES_APP_SENDER_ID`, so where a repository sets it, commits the automation-lanes App pushes are reviewed; where it is unset, no bot is. The **security lane**: the org's `pr-review-security.yml` caller, reviewing every non-draft PR head with the same fork and bot exclusions and the same bot exception; it has no path gating. The two lanes' scopes are mutually exclusive, a split the root `REVIEW.md` records, so each finding has exactly one owning reviewer; the security lane is scope partitioning, not the same-scope second reviewer whose evidence-gated exception defines the high-traffic class below. - **High-traffic primary development repo**: a repository whose PR volume and change surface justify a second PR-time reviewer, gated alongside the first via a dedicated matrix workflow. A repo in this class may additionally run author-side review agents that inform the author but never post to the PR. That wiring is repo-specific and drifts independently of this class definition, so verify it against the repo's own `.claude/` and `.github/workflows/` rather than assuming it here. Entering this class is an evidence-gated exception, not a default: it follows from a documented per-repo investigation weighing a second bot's marginal catch rate against its added comment volume and PR-cycle-time cost, including whether the two reviewers' findings are deduplicated (a coordinator is a further, separately evidence-gated step, not implied by running two reviewers). A repo does not enter this class by assumption; it earns it by running (and keeping current) the same kind of investigation. ## Why the class, not visibility or origin diff --git a/distribution/README.md b/distribution/README.md index 679296aa..3ba33a6b 100644 --- a/distribution/README.md +++ b/distribution/README.md @@ -324,7 +324,8 @@ What stays consumer-owned: but was never one of them; it is the observer key named above and it stays. The security lane has no path gating from ci-workflows v0.29.0: it reviews every -non-draft same-repository PR whose actor is not a bot, and a target's `.github/claude-security-paths` file is +non-draft same-repository PR whose actor is not a bot (or is a bot whose sender id the +caller lists in `allowed-bot-sender-ids`, from ci-workflows v0.40.0), and a target's `.github/claude-security-paths` file is no longer read. The manifest never managed that file. The two callers deliberately carry different concurrency values (per-PR diff --git a/distribution/sync-manifest.yml b/distribution/sync-manifest.yml index c69de56f..8bea2ccd 100644 --- a/distribution/sync-manifest.yml +++ b/distribution/sync-manifest.yml @@ -99,7 +99,9 @@ components: # its destinations retire. # # The security lane has no path gating from ci-workflows v0.29.0: it reviews - # every non-draft same-repository PR whose actor is not a bot, and a target's `.github/claude-security-paths` + # every non-draft same-repository PR whose actor is not a bot (or is a bot + # whose sender id the caller lists in `allowed-bot-sender-ids`, from + # ci-workflows v0.40.0), and a target's `.github/claude-security-paths` # file is no longer read. The manifest never synced that file, so a target # removes its copy in a repo-local PR if it wants to. #