diff --git a/.github/LADON-ADOPTION.md b/.github/LADON-ADOPTION.md new file mode 100644 index 000000000..aafe2a4ae --- /dev/null +++ b/.github/LADON-ADOPTION.md @@ -0,0 +1,223 @@ +# 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. The **2026-09-14 historical quarantine snapshot** held + [Version Packages #2912](https://github.com/adcontextprotocol/adcp-client/pull/2912) + Draft/auto-off. Its generated head advanced to + `1be6b25e8083601f8bf08f1b701435181f33215f` on base + `55829081dae2673f73615af2a4f6f029cddff813` after #2913 merged externally. + Coordinator evidence at that time reported npm dist-tags 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. + +The 2026-10-03 refresh supersedes that release snapshot: #2912 has merged, +SDK 14.0.0 is published, and `latest`, `rc`, and `adcp-3.2` currently point to +14.0.0. Release PR #3101 proposes 14.1.0. These observations do not establish +human-only review enforcement or lift this adoption's coordinated policy hold. +The refreshed code rejects policy-file rename evasion, incomplete inventories +(including GitHub's 3,000-file cap), and head changes during inspection. +Its 23 consumer regressions and 330 pinned upstream tests pass. Administrator +enforcement and the five-consumer rollout remain separate prerequisites. + +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..274068fdb --- /dev/null +++ b/.github/ladon-policy.test.cjs @@ -0,0 +1,282 @@ +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, metadata = {}) { + const directory = mkdtempSync(join(tmpdir(), 'ladon-gate-')); + const output = join(directory, 'output'); + try { + const result = spawnSync( + 'bash', + [ + '-c', + `gh() { ${ + failedApi + ? 'return 1;' + : 'local input="$TEST_PR"; for arg in "$@"; do case "$arg" in */files) input="$TEST_FILES" ;; esac; done; while [ "$#" -gt 0 ]; do case "$1" in --jq) shift; printf "%s" "$input" | jq -cr "$1"; return ;; *) shift ;; esac; done; printf "%s" "$input";' + } }; ${gate.run}`, + ], + { + encoding: 'utf8', + env: { + ...process.env, + TEST_FILES: JSON.stringify(paths.map(file => (typeof file === 'string' ? { filename: file } : file))), + TEST_PR: JSON.stringify({ changed_files: paths.length, head: { sha: 'current-head' }, ...metadata }), + EXPECTED_HEAD: 'current-head', + 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']); + 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('renaming protected policy files to an unlisted path still requires human review', () => { + for (const previous_filename of [ + '.github/workflows/ai-review.yml', + 'LADON.md', + '.github/workflows/ladon-policy.yml', + '.github/ladon-policy.test.cjs', + '.github/LADON-ADOPTION.md', + ]) { + const result = runGate([{ filename: 'src/renamed-policy.txt', previous_filename }]); + assert.equal(result.status, 0, result.stderr); + assert.match(result.output, /^modified=true$/m); + assert.ok(result.output.includes(previous_filename)); + } +}); + +test('renaming an ordinary file into a protected policy path requires human review', () => { + const result = runGate([{ filename: '.github/LADON-ADOPTION.md', previous_filename: 'docs/ordinary.md' }]); + assert.equal(result.status, 0, result.stderr); + assert.match(result.output, /^modified=true$/m); +}); + +test('a partial GitHub file inventory cannot proceed to automated review', () => { + const result = runGate(['src/ordinary.ts'], false, { changed_files: 3001 }); + assert.notEqual(result.status, 0); + assert.match(result.stdout, /Incomplete PR file inventory/); +}); + +test('an obsolete PR head cannot proceed to automated review', () => { + const result = runGate(['src/ordinary.ts'], false, { head: { sha: 'replacement-head' } }); + assert.notEqual(result.status, 0); + assert.match(result.stdout, /PR head changed/); +}); + +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)); + } +}); diff --git a/.github/workflows/ai-review.yml b/.github/workflows/ai-review.yml index ae51eaaac..fea8c4aa4 100644 --- a/.github/workflows/ai-review.yml +++ b/.github/workflows/ai-review.yml @@ -2,16 +2,20 @@ name: AI PR Review (Ladon) # Ladon reviews every non-draft, non-dependabot PR. The heavy lifting lives in # the shared action tree at adcontextprotocol/actions (ladon/), consumed here by -# floating major tag. This workflow handles the pull_request_target security +# immutable commit pin. 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 so they count toward the "1 review required" -# branch-protection check. +# the AAO Secretariat App as automated findings. Required human approval +# must be enforced separately; Ladon must never submit an approving review. # # 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] @@ -60,13 +64,30 @@ jobs: GH_TOKEN: ${{ steps.app-token.outputs.token }} PR_NUMBER: ${{ github.event.pull_request.number }} REPO: ${{ github.repository }} + EXPECTED_HEAD: ${{ github.event.pull_request.head.sha }} run: | set -euo pipefail - CHANGED="$(gh api --paginate "repos/${REPO}/pulls/${PR_NUMBER}/files" --jq '.[].filename')" + PR="$(gh api "repos/${REPO}/pulls/${PR_NUMBER}")" + if [ "$(jq -r '.head.sha' <<< "$PR")" != "$EXPECTED_HEAD" ]; then + echo "::error::PR head changed before review-policy inspection." + exit 1 + fi + FILES="$(gh api --paginate --slurp "repos/${REPO}/pulls/${PR_NUMBER}/files" --jq 'flatten')" + # GitHub caps this endpoint at 3,000 files. A partial inventory must + # never allow a protected policy path to bypass human review. + if [ "$(jq 'length' <<< "$FILES")" -ne "$(jq -r '.changed_files' <<< "$PR")" ]; then + echo "::error::Incomplete PR file inventory; human review is required." + exit 1 + fi + if [ "$(gh api "repos/${REPO}/pulls/${PR_NUMBER}" --jq '.head.sha')" != "$EXPECTED_HEAD" ]; then + echo "::error::PR head changed during review-policy inspection." + exit 1 + fi + CHANGED="$(jq -r '.[] | (.filename, (.previous_filename // empty))' <<< "$FILES")" MODIFIED="" while IFS= read -r f; do case "$f" in - .github/workflows/ai-review.yml|LADON.md) + .github/workflows/ai-review.yml|LADON.md|.github/workflows/ladon-policy.yml|.github/ladon-policy.test.cjs|.github/LADON-ADOPTION.md) MODIFIED="${MODIFIED}${f}"$'\n' ;; esac done <<< "$CHANGED" @@ -98,8 +119,9 @@ jobs: - name: Run Ladon if: steps.workflow-mod.outputs.modified != 'true' - uses: adcontextprotocol/actions/ladon/review@ladon/review/v1 + 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 }} 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