diff --git a/.github/LADON-ADOPTION.md b/.github/LADON-ADOPTION.md new file mode 100644 index 000000000..429522a95 --- /dev/null +++ b/.github/LADON-ADOPTION.md @@ -0,0 +1,213 @@ +# Ladon findings without bot approval: adoption hold + +**Branch protection is not fixed by this PR.** Keep this PR Draft with +no auto-merge until exact-head CI, an independent audit, and actual human +maintainer review. Agent audits and requested reviewers are not human approval. +The existing review-workflow-modification gate remains unchanged: this mixed +workflow/test/doc PR receives only the trusted base workflow's COMMENT if run +while ready; it cannot review or approve its own adoption. Draft runs skip Ladon. + +## Full intended inventory + +The human-only rollout includes **five** Ladon consumers: actions itself, +adcp, adcp-client, adcp-go and adcp-client-python. There are four separate +external adoption PRs; actions' own workflow adopts the immutable pin and +explicit false as part of human-reviewed #29. No approving consumer is accepted +or excluded by this rollout. A new consumer requires an explicit inventory +update and human review. + +The read-only organization audit in #29 (`node scripts/audit-ladon-consumers.cjs`) +reconciles organization code search with all visible active repositories' exact +default-branch workflow blobs. It fails on a sixth consumer, a missing expected +consumer, wrong/floating/local pin, nonliteral or missing false input, incomplete +search/API data or head movement. Run it after external adoption with +`--actions-head EXACT_REVIEWED_ACTIONS_PR_HEAD` to inspect #29's proposed +self-review workflow alongside the four live external consumers before any +publication. This is explicitly a candidate report, not live enforcement. +Without that option it checks all five live default branches and remains failed +until actions' own change lands. Repeat the live audit afterward. Do not weaken +the audit to make rollout CI green. A human administrator must also +confirm full organization visibility; neither code search nor this read-only +report configures protection. + +## Immutable dependency contract + +| Dependency | Commit | +| ------------------------------------------------------- | ------------------------------------------ | +| Orchestrator / reviewed regression suite | `d6e930d4a2a01ee0d476fd8347eaf258f85d889c` | +| Nested setup and arbiter (including executable bundles) | `d2422f6b24f6c0b7535c1155e9175d388be44174` | +| Nested reviewer | `a64a17ba369122d6b3401f614a31df7b8607f043` | + +The single current Ladon invocation pins the orchestrator and explicitly sets +`auto-approve: 'false'`. Its nested Ladon actions are immutable at the revisions +above. These are the reviewed revisions in +[actions #29](https://github.com/adcontextprotocol/actions/pull/29), even if that +Draft PR later receives documentation-only commits. No floating Ladon tag is +used. Preserve these commits when landing the actions series: retain the existing +`conductor/enforce-human-only-ladon-gate` branch until an authorized maintainer +has established and verified a durable retention reference containing all three +pins. Do not delete that branch after a squash merge without this validation. +This rollout does not create or move a tag. + +The read-only `Ladon approval policy` CI job checks out the exact consumer head, +parses Ladon-named `uses` sites in tracked workflows/local actions, and rejects missing, +enabled, malformed, boolean or expression-valued approval inputs. It verifies +nested runtime bytes against their pins and runs the reviewed upstream tests, +type checks and builds. Upstream tests cover clean COMMENT, findings, failing +critical/high requests for changes, failing human escalations, stale dismissed +approvals, direct stale-reapproval shell execution, bots/authors and zero APPROVE +API calls in disabled mode. It inspects calls before any cleanup, so there is no +transient approving write to race auto-merge in this pinned mode. Arbitrarily named +wrappers and shell/API review writers require the separate administrator audit; +the static inventory does not prove their absence. Existing supporting GitHub +actions in the privileged workflow retain their pre-existing version tags; the +immutable contract here covers Ladon code, not every third-party dependency. +The administrator supply-chain audit must review those dependencies separately. Reviewer prompt inputs fetched +from external repositories at runtime are not made immutable by these code pins. + +The consumer's **configuration tests** reject an omitted input. The revised +review/setup/arbiter manifests default to `'false'`; omission cannot enable +approval. Explicit `'true'` is an opt-in compatibility path, forbidden by this +rollout's inventory policy. +Explicit invalid runtime values fail before posting. The base workflow's human +modification gate protects changes to this invocation and is extended to the new +policy workflow, policy tests and adoption document; the new unprivileged CI +is a regression check, **not** the trusted human-approval status described below. +Blocking findings and escalations still fail; this adoption does not turn Ladon +into an unconditional successful check. + +## Three incidents, two distinct bypasses (2026-09-14 UTC) + +1. [adcp-client #2911](https://github.com/adcontextprotocol/adcp-client/pull/2911) + had only `aao-secretariat[bot]` APPROVED at exact head + `a6834d67870253674178d099447af36dd6640b11`. Requested `bmilekic` never reviewed. + REST review `5200764177` was submitted at `17:23:07Z`; author-enabled + auto-merge completed at `17:23:15Z` as + `8ba12c2ace85a88533ce1d56efd35badea51a97c`. No package release was recorded. +2. [adcp #7521](https://github.com/adcontextprotocol/adcp/pull/7521) + had **zero APPROVED reviews**. Its only review, `5200346889`, was + `aao-secretariat[bot]` COMMENTED at `16:38:41Z` on exact head + `e680c97e05e69cf3d7b6bf60aed07c42657c420c`, explicitly escalating changes to + `static/schemas/source/compliance/comply-test-controller-{request,response}.json` + for human/CODEOWNERS approval with `REVIEW_REQUIRED`. Base was + `36a86d6c4ae5c42e5e2e783d95bdb97b5cb6f614`. Author `bokelley` nevertheless + merged it himself at `18:02:02Z` as + `1467e46117e8d329f4116e548e9290be75d8e3b3`; final `auto_merge` was null. + This demonstrates author self-merge despite an explicit human escalation; + disabling bot APPROVE addresses only the first incident's approval path. + It does not prove which effective bypass permission enabled the second. + +3. [adcp-client #2913](https://github.com/adcontextprotocol/adcp-client/pull/2913) + had only bot approval at exact head + `bfacfeb66178d161254830faef48f36a5a6a629e`: Secretariat review `5201034616` + APPROVED at `17:51:07Z`; `bmilekic` remained requested without reviewing. + After the coordinator disabled auto-merge and put the PR in Draft, author + `bokelley` re-enabled auto-merge at `17:57:24Z` and it merged at `17:59:28Z` + as `55829081dae2673f73615af2a4f6f029cddff813` (base + `8ba12c2ace85a88533ce1d56efd35badea51a97c`). Bot review `5201137108` + APPROVED the same head at `18:01:40Z`, **after merge**; it cannot be human + evidence or a cause of that merge. Old approval-capable runs can still write + approvals, and Draft/auto-off comments are advisory against an author/admin. + +Initial inspection found inherited organization ruleset `8519441` and repository +rulesets `15545291` (adcp) / `15545837` (client). Approval-count/CODEOWNERS rules +and stale/last-push rules were visible, with role/admin/integration bypasses. +Visible required checks did not include `code_review` or a separate human gate. +Classic protection returned 403, so this was **not a complete effective audit**. +An escalation comment or an optional failed check does not enforce a merge hold. + +## Coordinated dependency order and quarantine + +1. Maintain the operational human merge hold. Inventory active bot approvals and + all older runs (including reruns, queued jobs, other review entrypoints and + App/machine users). Authorized maintainers must cancel/drain them and dismiss + obsolete approvals under the hold. COMMENT never revokes an old approval; + this pin cannot stop an in-flight older action. Do not race cleanup against + auto-merge. No such cancellation/dismissal is performed by this PR. +2. Obtain exact-head CI and independent audit, then actual human review of + **all four** separate external consumer PRs. Land all four external immutable pins through the audited + human procedure. External consumers may land in any order; all four precede actions + promotion. Do not loosen the workflow-modification gate for testing. +3. Complete the administrator audit/controlled validation below before lifting + the hold or merging actions #29. Merging actions/main invokes automatic tag + publication. **Do not merge #29 or move `ladon/review/v1` until all four + human-reviewed external consumer pins have landed and the effective human gate is + verified.** Keep consumer SHA pins after promotion. +4. Keep [Version Packages #2912](https://github.com/adcontextprotocol/adcp-client/pull/2912) + quarantined Draft/auto-off. Its generated head advanced to + `1be6b25e8083601f8bf08f1b701435181f33215f` on base + `55829081dae2673f73615af2a4f6f029cddff813` after #2913 merged externally. + Coordinator evidence reports npm dist-tags still at rc.36 / latest 13.0.4, + with no new package published. Maintain a **global release/merge hold** + until all four external consumer pins land and an authorized human configures and + validates the actually required trusted human-approval gate. This work does + not undo #2913's merge, authorize a release, or change either PR. + +Main snapshots are evidence at a point in time, not moving claims. The consumer +bases were refreshed to adcp `1467e46117e8d329f4116e548e9290be75d8e3b3` and +client `55829081dae2673f73615af2a4f6f029cddff813`. PR bodies record their exact +base/head/tree and CI/audit evidence. Other open PRs, including adcp #7450/#7462, +retain their older-base evidence until individually refreshed; this adoption +makes no claim to have recomputed their merge trees. + +## Required administrator action (not executed here) + +An explicitly authorized repository/organization administrator must: + +1. Export the **effective** main rules from repository Settings → Rules → + Rulesets, inherited organization rules, classic branch protection, and all + bypass lists. Record rule IDs, branch patterns, enforcement, expected check + producer App IDs, CODEOWNERS coverage and current permissions. Determine + exactly how #7521's author was permitted to merge. Inspect admin/custom-role, + write/maintain-role, integration, merge-queue and direct-push bypasses. +2. Install a separately reviewed, trusted producer for required status + `Human review / exact head`. Run trusted base or independent App code only; + never PR-head code with write credentials. Require its specific producer + App identity, not just a spoofable context name. Require the policy's Ladon + blocking result as well, with explicit handling of workflow-modification + human holds and high-risk exceptions; optional failures do not block merge. + Also require `Ladon approval policy` as a configuration regression check and + verify that deleting its workflow leaves a missing required result and blocks + merging. This PR-head check can be edited by a PR author and must never serve + as the trusted human-approval producer or replace independent human review. +3. The human producer must paginate reviews, resolve the latest effective + non-dismissed review per person, and count only APPROVED at the **live exact + head** from currently authorized collaborators/required CODEOWNERS. Exclude + the PR author, all Apps/bots, and known automation accounts typed as User. + Check current collaborator permissions, required teams and CODEOWNERS at the + trusted base; fail closed on unknown identities, missing authority, partial + data or API failure. Requested reviewers alone never count. +4. Recompute on push/synchronize, ready-for-review, review submission/dismissal, + base/policy/CODEOWNERS and membership/permission changes; handle merge queues + explicitly. Serialize per PR, re-read head and review/authority state before + publishing, reject stale workers, and invalidate success when any input + changes. Combine with native stale-review dismissal and last-push approval. + Status webhooks are asynchronous: document and test the dismissal/merge + race; if atomic enforcement is not established, retain the operational hold + and use an authorized, trusted merge procedure that revalidates immediately. +5. Make the requirement **non-bypassable by ordinary authors**, including the + author role implicated by #7521. Remove broad role/App bypasses or constrain + them to an independently controlled, logged emergency procedure unavailable + to ordinary authors. Prevent direct-push and alternative merge entrypoints + from evading the same requirement. Settings changes require explicit human + authority; neither this PR nor green CI grants that authority. +6. Record controlled validation under both ordinary author and maintainer roles. + Begin on a non-production branch with matching effective protections and no + deployment/release hooks; any actual merge attempts need separate explicit + administrator authorization. Prove rejection for no reviews, bot-only + approval, COMMENT-only escalation (#7521), requested-but-absent review, + self/old-head/dismissed approval, lost collaborator/CODEOWNER authority, + failed/skipped/missing human status, API failure, stale worker, fork, + workflow-modifying PR, rebase and merge queue. Exercise concurrent push, + dismissal and auto-merge; an obsolete success must not permit merge. A + valid current-head authorized **non-author human** approval may satisfy only + the human gate; unresolved blocking Ladon findings must remain blocking. +7. Verify the actual main branch's effective rules and producer identity after + the controlled tests. Record settings exports, exact PR/head/status/reviewer + evidence and permission-denied merge results, including denial of the + #7521 author scenario. A sandbox-only test is not proof of main enforcement. + Lift the hold only with a named human administrator's recorded acceptance. + +No ruleset, branch setting, tag, merge, deployment or release mutation is part of +these Draft adoption PRs. Do not report branch protection fixed until the +consumer deployments and administrator enforcement/validation are complete. diff --git a/.github/ladon-policy.test.cjs b/.github/ladon-policy.test.cjs new file mode 100644 index 000000000..c50613d8b --- /dev/null +++ b/.github/ladon-policy.test.cjs @@ -0,0 +1,321 @@ +const assert = require("node:assert/strict"); +const { execFileSync, spawnSync } = require("node:child_process"); +const { readFileSync, mkdtempSync, rmSync } = require("node:fs"); +const { createRequire } = require("node:module"); +const { tmpdir } = require("node:os"); +const { join, resolve } = require("node:path"); +const { test } = require("node:test"); + +const REVIEW = "d6e930d4a2a01ee0d476fd8347eaf258f85d889c"; +const IMPLEMENTATION = "d2422f6b24f6c0b7535c1155e9175d388be44174"; +const REVIEWER = "a64a17ba369122d6b3401f614a31df7b8607f043"; +const actions = resolve( + process.env.LADON_ACTIONS_PATH || ".ladon-reviewed-actions", +); +// Use the reviewed action's locked YAML parser; no consumer dependency changes. +const { parse } = createRequire(join(actions, "package.json"))("yaml"); +const read = (file) => readFileSync(file, "utf8"); +const git = (...args) => execFileSync("git", args, { encoding: "utf8" }).trim(); +const workflow = parse(read(".github/workflows/ai-review.yml")); +const steps = workflow.jobs.code_review.steps; +const gate = steps.find((step) => step.id === "workflow-mod"); +const notice = steps.find( + (step) => step.name === "Comment and skip when PR modifies review workflow", +); +const invocation = steps.find((step) => step.name === "Run Ladon"); + +function validateInvocation(step) { + assert.equal(step.uses, `adcontextprotocol/actions/ladon/review@${REVIEW}`); + // Require an explicit policy even though the audited runtime defaults false. + // This rejects a partial migration or later regression in any invocation. + assert.equal(step.with?.["auto-approve"], "false"); + assert.equal(step.if, "steps.workflow-mod.outputs.modified != 'true'"); + assert.equal(step["continue-on-error"], undefined); +} + +function collectInvocations(value, location, result = []) { + if (!value || typeof value !== "object") return result; + if (typeof value.uses === "string" && /ladon/i.test(value.uses)) { + result.push({ location, step: value }); + } + for (const [key, child] of Object.entries(value)) { + collectInvocations(child, `${location}.${key}`, result); + } + return result; +} + +function runGate(paths, failedApi = false) { + const directory = mkdtempSync(join(tmpdir(), "ladon-gate-")); + const output = join(directory, "output"); + try { + const result = spawnSync( + "bash", + [ + "-c", + `gh() { ${failedApi ? "return 1;" : 'printf "%s\\n" "$TEST_PATHS";'} }; ${gate.run}`, + ], + { + encoding: "utf8", + env: { + ...process.env, + TEST_PATHS: paths.join("\n"), + PR_NUMBER: "1", + REPO: "adcontextprotocol/test", + GITHUB_OUTPUT: output, + }, + }, + ); + return { ...result, output: result.status === 0 ? read(output) : "" }; + } finally { + rmSync(directory, { recursive: true, force: true }); + } +} + +test("all tracked workflow and local-action Ladon sites are inventoried, pinned, and disabled", () => { + const files = git( + "ls-files", + "-z", + "--", + ".github/workflows", + ":(glob)**/action.yml", + ":(glob)**/action.yaml", + ) + .split("\0") + .filter( + (file) => + /^\.github\/workflows\/[^/]+\.ya?ml$/.test(file) || + /(^|\/)action\.ya?ml$/.test(file), + ); + const sites = files.flatMap((file) => + collectInvocations(parse(read(file)), file), + ); + assert.equal( + sites.length, + 1, + "new invocation sites require an explicit human-reviewed policy update", + ); + assert.match( + sites[0].location, + /^\.github\/workflows\/ai-review\.yml\.jobs\.code_review\.steps\./, + ); + sites.forEach(({ step }) => validateInvocation(step)); +}); + +test("consumer contract rejects missing, malformed, enabled, boolean, and expression inputs", () => { + for (const value of [ + undefined, + null, + "", + "true", + true, + false, + "False", + "off", + " false", + "${{ inputs.auto-approve }}", + ]) { + const changed = structuredClone(invocation); + if (value === undefined) delete changed.with["auto-approve"]; + else changed.with["auto-approve"] = value; + assert.throws(() => validateInvocation(changed), String(value)); + } + for (const pin of ["ladon/review/v1", "main", IMPLEMENTATION]) { + assert.throws(() => + validateInvocation({ + ...invocation, + uses: `adcontextprotocol/actions/ladon/review@${pin}`, + }), + ); + } +}); + +test("trusted base checkout, draft guard, modification gate, and COMMENT-only human notice remain", async () => { + assert.ok(workflow.on.pull_request_target); + assert.equal(workflow.on.pull_request, undefined); + assert.deepEqual(workflow.on.pull_request_target["paths-ignore"], [ + ".github/workflows/ai-review.yml", + "LADON.md", + "scripts/retire-superseded-ladon-reviews.cjs", + ]); + assert.match( + workflow.jobs.code_review.if, + /github\.event\.pull_request\.draft == false/, + ); + assert.equal(workflow.jobs.code_review["continue-on-error"], undefined); + assert.equal(steps[0].with.ref, "${{ github.event.pull_request.base.sha }}"); + assert.ok(steps.indexOf(gate) < steps.indexOf(invocation)); + assert.equal(notice.if, "steps.workflow-mod.outputs.modified == 'true'"); + const calls = []; + const AsyncFunction = Object.getPrototypeOf(async function () {}).constructor; + await new AsyncFunction("github", "context", "process", notice.with.script)( + { rest: { pulls: { createReview: async (review) => calls.push(review) } } }, + { + repo: { owner: "adcontextprotocol", repo: "test" }, + issue: { number: 1 }, + }, + { env: { MODIFIED_FILES: ".github/workflows/ai-review.yml" } }, + ); + assert.equal(calls.length, 1); + assert.equal(calls[0].event, "COMMENT"); + assert.match(calls[0].body, /human reviewer/i); +}); + +for (const files of [ + [".github/workflows/ai-review.yml"], + ["LADON.md"], + [".github/workflows/ladon-policy.yml"], + [".github/workflows/ladon-policy.yml", "src/example.ts"], + [".github/ladon-policy.test.cjs"], + [".github/ladon-policy.test.cjs", "src/example.ts"], + [".github/LADON-ADOPTION.md"], + [".github/LADON-ADOPTION.md", "src/example.ts"], + + [ + ".github/workflows/ai-review.yml", + ".github/ladon-policy.test.cjs", + "src/example.ts", + ], + ["LADON.md", "src/example.ts"], +]) { + test(`existing shell gate holds pure/mixed review-system changes: ${files.join(", ")}`, () => { + const result = runGate(files); + assert.equal(result.status, 0, result.stderr); + assert.match(result.output, /^modified=true$/m); + }); +} + +test("file API failure cannot proceed past the workflow-modification gate", () => { + assert.notEqual(runGate([], true).status, 0); +}); + +test("ordinary source review proceeds only through the disabled invocation", () => { + const result = runGate(["src/example.ts"]); + assert.equal(result.status, 0, result.stderr); + assert.match(result.output, /^modified=false$/m); + validateInvocation(invocation); +}); + +test("reviewed checkout and every nested Ladon implementation are immutable and byte-identical", () => { + assert.equal(git("-C", actions, "rev-parse", "HEAD"), REVIEW); + const orchestrator = parse(read(join(actions, "ladon/review/action.yml"))); + for (const name of ["review", "setup", "arbiter"]) { + const manifest = parse(read(join(actions, `ladon/${name}/action.yml`))); + assert.equal(manifest.inputs["auto-approve"].default, "false"); + } + const nested = collectInvocations(orchestrator, "review"); + assert.deepEqual( + nested.map(({ step }) => step.uses), + [ + `adcontextprotocol/actions/ladon/setup@${IMPLEMENTATION}`, + `adcontextprotocol/actions/ladon/reviewer@${REVIEWER}`, + `adcontextprotocol/actions/ladon/arbiter@${IMPLEMENTATION}`, + ], + ); + for (const name of ["setup", "arbiter", "reviewer"]) { + const pin = name === "reviewer" ? REVIEWER : IMPLEMENTATION; + // Compare the full action directory, including executable dist bundles. + // Include reviewer rules, schema and every other prompt/runtime asset. + const paths = + name === "reviewer" + ? ["ladon/reviewer"] + : [ + `ladon/${name}/action.yml`, + `ladon/${name}/src`, + `ladon/${name}/dist`, + ]; + git("-C", actions, "diff", "--exit-code", pin, REVIEW, "--", ...paths); + } +}); + +test("read-only policy CI runs exact-head parsing and upstream approval/race regressions without secrets", () => { + const ci = parse(read(".github/workflows/ladon-policy.yml")); + assert.ok(Object.hasOwn(ci.on, "pull_request")); + assert.equal(ci.on.pull_request_target, undefined); + assert.deepEqual(ci.permissions, { contents: "read" }); + assert.doesNotMatch(read(".github/workflows/ladon-policy.yml"), /secrets\./); + const checkouts = ci.jobs.policy.steps.filter((step) => + step.uses?.startsWith("actions/checkout@"), + ); + assert.equal(checkouts.length, 2); + assert.equal( + checkouts[0].with.ref, + "${{ github.event.pull_request.head.sha || github.sha }}", + ); + assert.equal(checkouts[0].with.path, "consumer"); + assert.equal(checkouts[1].with.path, ".ladon-reviewed-actions"); + assert.equal( + ci.jobs.policy.env.LADON_ACTIONS_PATH, + "${{ github.workspace }}/.ladon-reviewed-actions", + ); + assert.equal( + ci.jobs.policy.steps.find((step) => step.run?.startsWith("node --test"))[ + "working-directory" + ], + "consumer", + ); + assert.equal( + ci.jobs.policy.steps.find((step) => + step.uses?.startsWith("actions/setup-node@"), + ).with["node-version-file"], + ".ladon-reviewed-actions/.nvmrc", + ); + assert.equal(checkouts[1].with.ref, REVIEW); + assert.equal(checkouts[1].with.repository, "adcontextprotocol/actions"); + checkouts.forEach((step) => + assert.equal(step.with["persist-credentials"], false), + ); + const run = ci.jobs.policy.steps.map((step) => step.run || "").join("\n"); + assert.match(run, /node --test \.github\/ladon-policy\.test\.cjs/); + assert.match(run, /npm test -- --force/); + assert.equal(ci.jobs.policy.if, undefined); + assert.equal(ci.jobs.policy["continue-on-error"], undefined); + ci.jobs.policy.steps.forEach((step) => { + assert.equal(step.if, undefined); + assert.equal(step["continue-on-error"], undefined); + }); +}); + +test("human hold and two distinct protection bypasses remain visible", () => { + assert.match(read(".github/workflows/ai-review.yml"), /HUMAN MERGE HOLD/); + const adoption = read(".github/LADON-ADOPTION.md"); + for (const marker of [ + "#2911", + "#7521", + "exact-head", + "PR author", + "CODEOWNERS", + "ordinary authors", + "older runs", + "not fixed", + ]) { + assert.ok( + adoption.includes(marker), + `missing hold/audit evidence: ${marker}`, + ); + } +}); + +test("local, reusable and direct-subaction Ladon sites cannot evade inventory", () => { + for (const uses of [ + "./ladon/review", + "./.github/workflows/ladon-review.yml", + "adcontextprotocol/actions/ladon/setup@" + IMPLEMENTATION, + "adcontextprotocol/actions/ladon/arbiter@" + IMPLEMENTATION, + ]) { + const step = { uses, with: { "auto-approve": "true" } }; + const sites = collectInvocations( + { jobs: { added: { steps: [step] } } }, + "new-workflow.yml", + ); + assert.equal(sites.length, 1, `uninventoried invocation: ${uses}`); + assert.throws(() => validateInvocation(sites[0].step)); + } +}); + +test("legacy approval-based retirement cannot run after the new arbiter", () => { + assert.equal(steps.at(-1).name, "Run Ladon"); + assert.doesNotMatch( + read(".github/workflows/ai-review.yml"), + /retireSupersededLadonReviews/, + ); +}); diff --git a/.github/workflows/ai-review.yml b/.github/workflows/ai-review.yml index 783794427..c750c1fea 100644 --- a/.github/workflows/ai-review.yml +++ b/.github/workflows/ai-review.yml @@ -1,15 +1,20 @@ name: AI PR Review (Ladon) # Ladon reviews every non-draft, non-dependabot PR. The engine lives in -# adcontextprotocol/actions (ladon/), consumed here by floating major tag. This +# adcontextprotocol/actions (ladon/), pinned to a reviewed commit. This # workflow handles the pull_request_target security posture (trusted base-SHA # checkout only; the PR head is never fetched, checked out, or executed - setup # reads head content via the GitHub API), the review-workflow-modification gate, -# and invoking the composite. Reviews post as the AAO Secretariat App. +# and invoking the composite. Reviews post as the AAO Secretariat App as +# automated findings; human approval must be enforced separately. # # pull_request_target (not pull_request) is required on a public repo so that PRs # from forks can access the App token and Anthropic key. +# HUMAN MERGE HOLD: this pin prevents bot approval, not author self-merge. +# Keep older runs/approvals quarantined until the effective human gate is audited. +# See .github/LADON-ADOPTION.md before lifting the hold or promoting actions tags. + on: pull_request_target: types: [opened, labeled, ready_for_review, synchronize] @@ -61,7 +66,7 @@ jobs: MODIFIED="" while IFS= read -r f; do case "$f" in - .github/workflows/ai-review.yml|LADON.md|scripts/retire-superseded-ladon-reviews.cjs) + .github/workflows/ai-review.yml|LADON.md|.github/workflows/ladon-policy.yml|.github/ladon-policy.test.cjs|.github/LADON-ADOPTION.md|scripts/retire-superseded-ladon-reviews.cjs) MODIFIED="${MODIFIED}${f}"$'\n' ;; esac done <<< "$CHANGED" @@ -95,8 +100,9 @@ jobs: if: steps.workflow-mod.outputs.modified != 'true' # Pin the shared reviewer because this step receives the Secretariat # private key and Anthropic API key. Update from ladon/review/v1 deliberately. - uses: adcontextprotocol/actions/ladon/review@48c0dfef96594f74f9606d68597c0f7062a5a337 # includes reviewer/v1.0.7 + uses: adcontextprotocol/actions/ladon/review@d6e930d4a2a01ee0d476fd8347eaf258f85d889c with: + auto-approve: 'false' anthropic-api-key: ${{ secrets.ANTHROPIC_API_KEY }} app-id: ${{ secrets.SECRETARIAT_APP_ID }} app-private-key: ${{ secrets.SECRETARIAT_APP_PRIVATE_KEY }} @@ -105,28 +111,3 @@ jobs: skip-bot-authors: dependabot[bot],renovate[bot],github-actions[bot],aao-ipr-bot[bot] # Optional; defaults to the review action's pinned model. model: claude-opus-4-8 - - # GitHub keeps an earlier CHANGES_REQUESTED review blocking even after - # the same App approves a corrected head. Retire only Ladon's own older - # change requests, and only when its latest review approves this head. - - name: Retire superseded Ladon change requests - if: steps.workflow-mod.outputs.modified != 'true' - uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8 - env: - LADON_BOT_LOGIN: ${{ steps.app-token.outputs.app-slug }}[bot] - PR_HEAD_SHA: ${{ github.event.pull_request.head.sha }} - with: - github-token: ${{ steps.app-token.outputs.token }} - script: | - const { retireSupersededLadonReviews } = require( - `${process.env.GITHUB_WORKSPACE}/scripts/retire-superseded-ladon-reviews.cjs` - ); - const dismissed = await retireSupersededLadonReviews({ - github, - owner: context.repo.owner, - repo: context.repo.repo, - pullNumber: context.issue.number, - botLogin: process.env.LADON_BOT_LOGIN, - headSha: process.env.PR_HEAD_SHA, - }); - core.info(`Dismissed ${dismissed.length} superseded Ladon review(s).`); diff --git a/.github/workflows/ladon-policy.yml b/.github/workflows/ladon-policy.yml new file mode 100644 index 000000000..c434edc93 --- /dev/null +++ b/.github/workflows/ladon-policy.yml @@ -0,0 +1,56 @@ +name: Ladon approval policy + +# Read-only PR-head tests. This is a configuration/regression check, not the +# separate trusted exact-head human-approval check required before lifting hold. +on: + pull_request: + push: + branches: [main] + +permissions: + contents: read + +jobs: + policy: + name: Ladon approval policy + runs-on: ubuntu-latest + timeout-minutes: 20 + env: + LADON_ACTIONS_PATH: ${{ github.workspace }}/.ladon-reviewed-actions + steps: + - name: Check out exact consumer head without write credentials + uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + with: + ref: ${{ github.event.pull_request.head.sha || github.sha }} + path: consumer + persist-credentials: false + + - name: Check out reviewed Ladon implementation and pin history + uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + with: + repository: adcontextprotocol/actions + ref: d6e930d4a2a01ee0d476fd8347eaf258f85d889c + path: .ladon-reviewed-actions + fetch-depth: 0 + persist-credentials: false + + - name: Set up Node.js + uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4.4.0 + with: + node-version-file: .ladon-reviewed-actions/.nvmrc + + - name: Install locked review test dependencies without lifecycle scripts + working-directory: .ladon-reviewed-actions + run: npm ci --ignore-scripts + + - name: Verify every invocation, immutable nested pins, and human hold + working-directory: consumer + run: node --test .github/ladon-policy.test.cjs + + - name: Run reviewed approval-boundary regressions and static checks + working-directory: .ladon-reviewed-actions + run: | + npm test -- --force + npm run type-check + npm run build + git diff --exit-code -- ladon