From 1b993a86a66c41c04066f7a56fe0130ec9019bf4 Mon Sep 17 00:00:00 2001 From: dpiet-oai <326640584+dpiet-oai@users.noreply.github.com> Date: Fri, 23 Oct 2026 01:40:34 +0000 Subject: [PATCH] chore(api): refresh custom-code enforcement tooling Castiron-Internal-PR: https://github.com/openai/openai-java-internal/pull/349 Castiron-Source-SHA: 8f04b7124b1bf9fae2e7cfb5bf089c5a61be0365 Castiron-Public-Base-SHA: 65bd9e403f8e7f576bf7de904ba7237b6142368f --- .castiron.stats.yml | 8 +- .../castiron-custom-code-comment.yml | 388 +++++++- .github/workflows/castiron-custom-code.yml | 6 +- scripts/castiron/CUSTOM_CODE.md | 73 +- scripts/castiron/custom_code_budget.py | 135 ++- scripts/castiron/custom_code_policy.py | 86 +- scripts/castiron/custom_code_report.py | 273 +++++- scripts/castiron/custom_code_test_support.py | 1 + .../castiron/fixtures/github_publisher.cjs | 90 +- scripts/castiron/generated-files.json | 32 +- scripts/castiron/test_custom_code_budget.py | 146 +++ scripts/castiron/test_custom_code_github.py | 229 ++++- scripts/castiron/test_custom_code_policy.py | 342 ++++++- .../castiron/test_custom_code_publication.py | 885 +++++++++++++++++- scripts/castiron/test_custom_code_report.py | 489 ++++++++++ 15 files changed, 3003 insertions(+), 180 deletions(-) mode change 100644 => 100755 scripts/castiron/custom_code_budget.py mode change 100644 => 100755 scripts/castiron/custom_code_policy.py mode change 100644 => 100755 scripts/castiron/custom_code_report.py diff --git a/.castiron.stats.yml b/.castiron.stats.yml index e7df48682..aa113a34c 100644 --- a/.castiron.stats.yml +++ b/.castiron.stats.yml @@ -1,8 +1,8 @@ schema_version: 1 -generation_id: ed22133c-9fa7-43fe-8968-e883cf5c1247 +generation_id: f664572a-7dc0-499b-9a19-46357300068f openapi_spec_hash: 28504532afa0c3f477f005e641ec7d6d openapi_transformed_spec_hash: 13422b63a47a5d25533cf55573de7632 config_hash: 4446d9cc8e7a00d29ce72282aa5616f9 -codegen_sha: 60fe0b2314d2370d16ef526cebfd5aa058962d68 -codegen_hash: e20a23b060bd9cb27f5b9e51b6bff14be4f845d2585d0ef54dfb6693747439b5 -public_codegen_sha: 63178f1c3de0fdf686ebb74b4233a6dc7b8b6eaf +codegen_sha: 46b4c0fbaf91c9d178f4b29d2fed8a03794b5662 +codegen_hash: 753430c43af080e5995298094195c43f14ec9db4b146a14c7e33bf2c2d12df8b +public_codegen_sha: 89db964dcda3fa55c381ce03108f5d46b61279eb diff --git a/.github/workflows/castiron-custom-code-comment.yml b/.github/workflows/castiron-custom-code-comment.yml index 183679450..ace9ba11a 100644 --- a/.github/workflows/castiron-custom-code-comment.yml +++ b/.github/workflows/castiron-custom-code-comment.yml @@ -14,9 +14,314 @@ concurrency: queue: max # Preserve newer pending evaluations if older runs arrive out of order. jobs: + review-change: + name: Invalidate reviewed policy statuses + if: github.event.workflow_run.event == 'pull_request_review' + runs-on: ubuntu-latest + timeout-minutes: 5 + concurrency: + group: castiron-budget-writes-${{ github.repository_id }} + cancel-in-progress: false + queue: max + permissions: + actions: write + contents: read + pull-requests: read + statuses: write + steps: + # Trusted main definition: no candidate checkout, artifacts, or executable inputs. + - name: Re-evaluate current PR and queue after review changes + uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 + with: + script: | + // Trusted main orchestration only. Signal fields never choose status targets. + const repo = `${context.repo.owner}/${context.repo.repo}`; + const signal = context.payload.workflow_run; + const {data: run} = await github.rest.actions.getWorkflowRun({...context.repo, run_id: signal.id}); + const {data: workflow} = await github.rest.actions.getWorkflow({ + ...context.repo, workflow_id: 'castiron-custom-code.yml', + }); + if (!Number.isSafeInteger(run.workflow_id) || run.workflow_id !== workflow.id || + workflow.path !== '.github/workflows/castiron-custom-code.yml') return; + if (run.repository.full_name !== repo || run.event !== 'pull_request_review' || + run.status !== 'completed' || run.id !== signal.id || run.run_attempt !== signal.run_attempt || + run.head_sha !== signal.head_sha || run.path.split('@', 1)[0] !== '.github/workflows/castiron-custom-code.yml') { + throw new Error('Unexpected or superseded review signal'); + } + async function mergeBase(base, head) { + const {data: comparison} = await github.rest.repos.compareCommitsWithBasehead({ + ...context.repo, basehead: `${base}...${head}`, per_page: 1, + }); + const sha = comparison.merge_base_commit?.sha; + if (!/^[0-9a-f]{40}$/.test(sha || '')) throw new Error('Invalid policy merge base'); + return sha; + } + async function readPolicy(ref) { + const {data: file} = await github.rest.repos.getContent({ + ...context.repo, path: '.castiron-ratchet.json', ref, + }); + if (file.type !== 'file' || file.encoding !== 'base64' || !Number.isSafeInteger(file.size) || + file.size > 4096 || typeof file.content !== 'string') throw new Error('Invalid policy file'); + const content = Buffer.from(file.content, 'base64'); + if (content.length !== file.size) throw new Error('Invalid policy content'); + const policy = JSON.parse(content.toString('utf8'), (key, value, source) => + key === 'max_custom_patch_lines' && typeof value === 'number' ? BigInt(source.source) : value); + if (!policy || typeof policy !== 'object' || typeof policy.max_custom_patch_lines !== 'bigint' || + policy.max_custom_patch_lines < 0n) throw new Error('Invalid policy budget'); + return policy; + } + async function pinRequiresHuman(pr, headPolicy, mainSha) { + const targetPolicy = await readPolicy(pr.base.sha); + const pinSha = pin => pin?.repository === repo && /^[0-9a-f]{40}$/.test(pin.main_sha || '') + ? pin.main_sha : null; + const proposedChecker = pinSha(headPolicy.next_policy); + const previousChecker = targetPolicy.next_policy ? pinSha(targetPolicy.next_policy) : mainSha; + if (!proposedChecker || !/^[0-9a-f]{40}$/.test(previousChecker || '') || + !/^[0-9a-f]{40}$/.test(mainSha || '')) return true; + const [previous, proposed, previousBase, proposedBase] = await Promise.all([ + targetPolicy.next_policy ? readPolicy(previousChecker) : targetPolicy, + readPolicy(proposedChecker), mergeBase(previousChecker, proposedChecker), + mergeBase(proposedChecker, mainSha), + ]); + return previousBase !== previousChecker || proposedBase !== proposedChecker || + proposed.max_custom_patch_lines > previous.max_custom_patch_lines || + proposed.schema_version !== previous.schema_version || + proposed.generated_files !== previous.generated_files || proposed.ratchet !== previous.ratchet; + } + const dirtyTargets = new Set(); + // Retry discovery within this job timeout while targets are changing. + while (true) { + const candidates = []; + let pulls = run.pull_requests?.length ? run.pull_requests : await github.paginate( + github.rest.repos.listPullRequestsAssociatedWithCommit, {...context.repo, commit_sha: run.head_sha}); + const hasAssociation = pulls.length > 0; + if (!pulls.length) pulls = await github.paginate(github.rest.pulls.list, { + ...context.repo, state: 'open', head: `${run.head_repository.owner.login}:${run.head_branch}`, + }); + for (const item of pulls) { + const {data: pr} = await github.rest.pulls.get({...context.repo, pull_number: item.number}); + if (pr.state !== 'open' || pr.base.ref !== 'next' || pr.base.repo.full_name !== repo || + (item.head?.sha && pr.head.sha !== item.head.sha) || !/^[0-9a-f]{40}$/.test(pr.head.sha)) continue; + if (run.head_sha !== pr.head.sha) { + const {data: commit} = await github.rest.repos.getCommit({...context.repo, ref: run.head_sha}); + if (commit.parents.length !== 2 || commit.parents[1].sha !== pr.head.sha || + !/^[0-9a-f]{40}$/.test(commit.parents[0].sha) || + await mergeBase(commit.parents[0].sha, pr.base.sha) !== commit.parents[0].sha) continue; + // Without a commit association, the live merge ref must identify this PR. + if (!hasAssociation) { + const {data: merge} = await github.rest.git.getRef({...context.repo, ref: `pull/${pr.number}/merge`}); + if (merge.object.sha !== run.head_sha || commit.parents[0].sha !== pr.base.sha) continue; + } + } + candidates.push(pr); + } + if (!candidates.length) return; // Main, closed, and superseded PR reviews have no next target. + // Commit associations may include other PRs sharing the same head. + const matching = candidates.length === 1 ? candidates : candidates.filter(pr => + Number.isSafeInteger(run.head_repository?.id) && run.head_repository.id > 0 && + pr.head.repo?.id === run.head_repository.id && typeof run.head_branch === 'string' && + run.head_branch.length > 0 && pr.head.ref === run.head_branch); + if (matching.length !== 1) throw new Error('Review signal identifies ambiguous next PRs'); + const pr = matching[0]; + const files = await github.paginate(github.rest.pulls.listFiles, {...context.repo, pull_number: pr.number}); + if (!files.some(file => file.filename === '.castiron-ratchet.json')) return; + const baseline = await mergeBase(pr.base.sha, pr.head.sha); + const [basePolicy, headPolicy] = await Promise.all([readPolicy(baseline), readPolicy(pr.head.sha)]); + const pinChanged = !require('node:util').isDeepStrictEqual(basePolicy.next_policy ?? null, headPolicy.next_policy ?? null); + let requiresHuman = headPolicy.max_custom_patch_lines > basePolicy.max_custom_patch_lines || + (headPolicy.ratchet === 'go' && basePolicy.ratchet !== 'go'); + let mainSha = null; + if (!requiresHuman && pinChanged) { + const {data: main} = await github.rest.git.getRef({...context.repo, ref: 'heads/main'}); + mainSha = main.object.sha; + requiresHuman = await pinRequiresHuman(pr, headPolicy, mainSha); + } + if (!requiresHuman) { + const [{data: current}, {data: target}, main] = await Promise.all([ + github.rest.pulls.get({...context.repo, pull_number: pr.number}), + github.rest.git.getRef({...context.repo, ref: 'heads/next'}), + mainSha && github.rest.git.getRef({...context.repo, ref: 'heads/main'}), + ]); + if (current.head.sha !== pr.head.sha || current.base.sha !== pr.base.sha || + target.object.sha !== pr.base.sha || (main && main.data.object.sha !== mainSha)) continue; + return; + } + // Read authoritative reviews, including edits: submittedAt alone misses + // removal/restoration of an attestation. Never trust candidate event artifacts. + let reviewCursor = null; + const latest = new Map(); + do { + const result = await github.graphql(`query($owner:String!,$name:String!,$number:Int!,$cursor:String) { + repository(owner:$owner,name:$name) { pullRequest(number:$number) { + reviews(first:100,after:$cursor) { nodes { fullDatabaseId state body updatedAt + author { login __typename } authorAssociation commit { oid } } + pageInfo { hasNextPage endCursor } } + } } + }`, {...context.repo, name: context.repo.repo, number: pr.number, cursor: reviewCursor}); + const page = result.repository.pullRequest.reviews; + for (const review of page.nodes) { + const value = review.fullDatabaseId; + if (!(typeof value === 'string' || Number.isSafeInteger(value)) || + !/^[1-9][0-9]*$/.test(String(value))) throw new Error('Invalid review identity'); + const identity = String(value); + if (review.state === 'PENDING' || !review.author) continue; + const previous = latest.get(review.author.login); + if (!previous || BigInt(identity) > BigInt(previous.identity)) latest.set(review.author.login, {...review, identity}); + } + if (!page.pageInfo.hasNextPage) break; + if (!page.pageInfo.endCursor || page.pageInfo.endCursor === reviewCursor) throw new Error('Invalid review cursor'); + reviewCursor = page.pageInfo.endCursor; + } while (true); + // Same v2 whole-body contract as the selector/publisher. This can only + // request reevaluation; the normal publisher still decides authorization. + const approvals = [...latest.values()].filter(review => + review.state === 'APPROVED' && review.commit?.oid === pr.head.sha && + review.author.__typename === 'User' && !review.author.login.endsWith('[bot]') && + review.author.login !== pr.user.login && + ['OWNER', 'MEMBER', 'COLLABORATOR'].includes(review.authorAssociation) && + typeof review.body === 'string' && + review.body.trim() === 'I personally reviewed and approve this policy change as a human: ' + pr.head.sha); + const authorized = approvals.length > 0; + if (authorized && (run.actor?.type !== 'User' || + !approvals.some(review => review.author.login === run.actor.login))) return; + for (const review of approvals) { + if (!Number.isFinite(Date.parse(review.updatedAt))) throw new Error('Missing review update time'); + } + const fingerprint = require('node:crypto').createHash('sha256').update(JSON.stringify( + [pr.head.sha, approvals.map(review => [review.identity, review.updatedAt]).sort((a, b) => a[0].localeCompare(b[0]))] + )).digest('hex'); + const marker = `[review ${authorized ? 'approve' : 'revoke'} ${fingerprint}]`; + async function queue() { + let cursor = null; + const entries = []; + do { + const result = await github.graphql(`query($owner:String!,$name:String!,$cursor:String) { + repository(owner:$owner,name:$name) { mergeQueue(branch:"next") { + entries(first:100,after:$cursor) { nodes { headCommit { oid } pullRequest { number headRefOid } } + pageInfo { hasNextPage endCursor } } + } } + }`, {...context.repo, name: context.repo.repo, cursor}); + const page = result.repository.mergeQueue?.entries; + if (!page) return []; + entries.push(...page.nodes); + if (!page.pageInfo.hasNextPage) return entries; + if (!page.pageInfo.endCursor || page.pageInfo.endCursor === cursor) throw new Error('Invalid queue cursor'); + cursor = page.pageInfo.endCursor; + } while (true); + } + const entries = await queue(); + const index = entries.findIndex(entry => entry.pullRequest?.number === pr.number && entry.pullRequest.headRefOid === pr.head.sha); + // Later queue entries can include the affected constituent. Re-evaluate all of them. + const targets = new Map([[pr.head.sha, 'pull_request']]); + for (const entry of index < 0 ? [] : entries.slice(index)) { + if (entry.headCommit === null) continue; // Queue preparation has not produced statuses yet. + if (!/^[0-9a-f]{40}$/.test(entry.headCommit.oid)) throw new Error('Invalid queue SHA'); + targets.set(entry.headCommit.oid, 'merge_group'); + } + // Revoke the authenticated snapshot before slower reads. Pending on an obsolete + // affected SHA cannot authorize a merge, and a later race must not retain success. + const url = `${context.serverUrl}/${repo}/actions/runs/${context.runId}`; + const activeTargets = new Map(); + const priorWriters = new Map(); + async function priorWriter(status) { + if (!/\[review (?:approve|revoke) [0-9a-f]{64}\]$/.test(status.description || '')) return null; + const identity = status.target_url?.slice(`${context.serverUrl}/${repo}/actions/runs/`.length); + if (!/^[1-9][0-9]*$/.test(identity || '') || !Number.isSafeInteger(Number(identity))) return null; + if (!priorWriters.has(identity)) { + const {data: definition} = await github.rest.actions.getWorkflow({ + ...context.repo, workflow_id: 'castiron-custom-code-comment.yml', + }); + let writer; + try { + ({data: writer} = await github.rest.actions.getWorkflowRun({...context.repo, run_id: Number(identity)})); + } catch (error) { + if (error.status !== 404) throw error; + priorWriters.set(identity, null); // Deleted runs cannot authenticate a marker. + return null; + } + const authenticated = definition.path === '.github/workflows/castiron-custom-code-comment.yml' && + writer.workflow_id === definition.id && writer.repository.full_name === repo && + writer.event === 'workflow_run' && writer.id === Number(identity); + priorWriters.set(identity, authenticated ? { + completed: writer.status === 'completed' && writer.conclusion === 'success', + } : null); + } + return priorWriters.get(identity); + } + const names = ['Castiron next / budget-only change', 'Castiron next / custom-code budget']; + for (const [sha, event] of targets) { + const statuses = await github.paginate(github.rest.repos.listCommitStatusesForRef, {...context.repo, ref: sha}); + const trusted = statuses.filter(status => status.creator?.login === 'github-actions[bot]' && + status.target_url?.startsWith(`${context.serverUrl}/${repo}/actions/runs/`)); + const current = new Map(); + for (const status of trusted) if (!current.has(status.context)) current.set(status.context, status); + // Only current pending markers coalesce. A later publisher failure + // supersedes recovery even if a restored review has the same timestamp. + const changed = []; + for (const name of names) { + const status = current.get(name); + // A failed/cancelled writer may have stopped after pending but + // before dispatch. Re-stamp and retry; only successful writers coalesce. + const writer = status?.state === 'pending' ? await priorWriter(status) : null; + const handled = status?.state === 'success' || + (status?.state === 'pending' && status.description?.endsWith(marker) && writer?.completed); + if (authorized ? !handled : + status?.state === 'success' || (writer && !writer.completed)) changed.push(name); + } + if (authorized && names.every(name => current.get(name)?.state === 'success')) continue; + for (const name of changed) { + await github.rest.repos.createCommitStatus({...context.repo, sha, context: name, state: 'pending', + description: `Review changed; reevaluation pending. ${marker}`, target_url: url}); + dirtyTargets.add(sha); + } + if (dirtyTargets.has(sha)) activeTargets.set(sha, event); + } + if (!activeTargets.size) return; + const reruns = []; + for (const [sha, event] of activeTargets) { + const runs = await github.paginate(github.rest.actions.listWorkflowRuns, { + ...context.repo, workflow_id: 'castiron-custom-code.yml', head_sha: sha, event, per_page: 100, + }); + const matches = []; + for (const item of runs) { + if (item.repository.full_name !== repo || item.event !== event || item.head_sha !== sha || + item.path.split('@', 1)[0] !== '.github/workflows/castiron-custom-code.yml') continue; + if (event === 'pull_request') { + let associated = item.pull_requests?.length ? item.pull_requests : await github.paginate( + github.rest.repos.listPullRequestsAssociatedWithCommit, {...context.repo, commit_sha: sha}); + if (!associated.length) associated = await github.paginate(github.rest.pulls.list, { + ...context.repo, state: 'open', head: `${item.head_repository.owner.login}:${item.head_branch}`, + }); + const live = []; + for (const reference of associated) { + const {data: pull} = await github.rest.pulls.get({...context.repo, pull_number: reference.number}); + if (pull.state === 'open' && pull.head.sha === sha && pull.base.repo.full_name === repo && + ['main', 'next'].includes(pull.base.ref)) live.push(pull); + } + // Match the canonical reporter's main preference for shared-head discovery. + const main = live.filter(pull => pull.base.ref === 'main'); + const selected = main.length ? main : live; + if (selected.length !== 1 || selected[0].number !== pr.number) continue; + } + matches.push(item); + } + matches.sort((a, b) => b.id - a.id); + if (matches[0]?.status === 'completed') reruns.push(matches[0].id); + } + // Retry against the new snapshot; already-observed affected statuses stay pending. + const {data: current} = await github.rest.pulls.get({...context.repo, pull_number: pr.number}); + const {data: base} = await github.rest.git.getRef({...context.repo, ref: 'heads/next'}); + if (current.state !== 'open' || current.head.sha !== pr.head.sha || current.base.ref !== 'next' || + current.base.repo.full_name !== repo || current.base.sha !== pr.base.sha || base.object.sha !== pr.base.sha || + JSON.stringify(await queue()) !== JSON.stringify(entries)) continue; + // Only normal authenticated reporter runs may restore success. Never infer approval here. + for (const run_id of reruns) await github.rest.actions.reRunWorkflow({...context.repo, run_id}); + return; + } + compute: name: Compute trusted custom-code report - if: contains(fromJSON('["pull_request", "merge_group"]'), github.event.workflow_run.event) && github.event.workflow_run.path == '.github/workflows/castiron-custom-code.yml' + if: contains(fromJSON('["pull_request", "merge_group"]'), github.event.workflow_run.event) && (github.event.workflow_run.path == '.github/workflows/castiron-custom-code.yml' || startsWith(github.event.workflow_run.path, '.github/workflows/castiron-custom-code.yml@')) runs-on: ubuntu-latest timeout-minutes: 10 permissions: @@ -153,6 +458,10 @@ jobs: retention-days: 7 budget-status: + concurrency: + group: castiron-budget-writes-${{ github.repository_id }} + cancel-in-progress: false + queue: max name: Publish custom-code budget checks needs: compute if: always() && !cancelled() && needs.compute.result != 'skipped' @@ -185,6 +494,11 @@ jobs: script: | const event = context.payload.workflow_run; const {data: run} = await github.rest.actions.getWorkflowRun({...context.repo, run_id: event.id}); + const {data: workflow} = await github.rest.actions.getWorkflow({ + ...context.repo, workflow_id: 'castiron-custom-code.yml', + }); + if (!Number.isSafeInteger(run.workflow_id) || run.workflow_id !== workflow.id || + workflow.path !== '.github/workflows/castiron-custom-code.yml') return; if (run.event !== event.event || run.head_sha !== event.head_sha || run.run_attempt !== event.run_attempt || run.status !== 'completed' || run.path.split('@', 1)[0] !== '.github/workflows/castiron-custom-code.yml' || run.repository.full_name !== `${context.repo.owner}/${context.repo.repo}`) return; @@ -219,13 +533,10 @@ jobs: if (run.event !== 'merge_group' || !match) return; branch = match[1]; } - // Next policy changes alter its own protected base, rebuilding its queue. - // Its approved main policy version is independent of later main changes. - if (branch === 'next') { - fresh = fresh && process.env.SELECTED_BRANCH === branch && - process.env.SELECTED_BASE_SHA === base && process.env.SELECTED_HEAD_SHA === head && - process.env.SELECTED_POLICY_SHA === policy; - } + // Bind every target to the trusted selector, including next-to-main retargeting. + fresh = fresh && process.env.SELECTED_BRANCH === branch && + process.env.SELECTED_BASE_SHA === base && process.env.SELECTED_HEAD_SHA === head && + process.env.SELECTED_POLICY_SHA === policy; fresh = fresh && evaluatedBranch === branch; const url = `${context.serverUrl}/${context.repo.owner}/${context.repo.repo}/actions/runs/${context.runId}`; // Concurrency serializes writes, but does not order runs or retries. @@ -282,8 +593,9 @@ jobs: try { const proofs = JSON.parse(process.env.POLICY_REVIEWS); const rules = JSON.parse(process.env.HUMAN_REVIEW_RULES); - if (!Array.isArray(proofs) || rules.version !== 1 || !Array.isArray(rules.associations) || - typeof rules.negative_pattern !== 'string') throw new Error('Missing trusted review contract'); + if (!Array.isArray(proofs) || rules.version !== 2 || !Array.isArray(rules.associations) || + typeof rules.negative_pattern !== 'string' || typeof rules.attestation_prefix !== 'string' || + !rules.attestation_prefix) throw new Error('Missing trusted review contract'); const negative = new RegExp(rules.negative_pattern, 'i'); const seen = new Set(); for (const proof of proofs) { @@ -295,17 +607,36 @@ jobs: const {data: pr} = await github.rest.pulls.get({...context.repo, pull_number: proof.number}); if (pr.state !== 'open' || pr.head.sha !== proof.head_sha || pr.base.ref !== 'next' || pr.base.repo.full_name !== `${context.repo.owner}/${context.repo.repo}`) throw new Error('Policy PR changed'); - const reviews = await github.paginate(github.rest.pulls.listReviews, {...context.repo, pull_number: proof.number, per_page: 100}); + let reviewCursor = null; const latest = new Map(); - for (const review of reviews) { - if (review.state === 'PENDING') continue; - const login = review.user.login; - if (!latest.has(login) || review.id > latest.get(login).id) latest.set(login, review); - } + do { + const result = await github.graphql(`query($owner:String!,$name:String!,$number:Int!,$cursor:String) { + repository(owner:$owner,name:$name) { pullRequest(number:$number) { + reviews(first:100,after:$cursor) { nodes { fullDatabaseId state body + author { login __typename } authorAssociation commit { oid } } + pageInfo { hasNextPage endCursor } } + } } + }`, {...context.repo, name: context.repo.repo, number: proof.number, cursor: reviewCursor}); + const page = result.repository.pullRequest.reviews; + for (const review of page.nodes) { + const value = review.fullDatabaseId; + if (!(typeof value === 'string' || Number.isSafeInteger(value)) || + !/^[1-9][0-9]*$/.test(String(value))) throw new Error('Invalid review identity'); + const identity = String(value); + if (review.state === 'PENDING' || !review.author) continue; + const previous = latest.get(review.author.login); + if (!previous || BigInt(identity) > BigInt(previous.identity)) latest.set(review.author.login, {...review, identity}); + } + if (!page.pageInfo.hasNextPage) break; + if (!page.pageInfo.endCursor || page.pageInfo.endCursor === reviewCursor) throw new Error('Invalid review cursor'); + reviewCursor = page.pageInfo.endCursor; + } while (true); const approved = [...latest.values()].some(review => review.state === 'APPROVED' && - review.commit_id === proof.head_sha && review.user.type === 'User' && - rules.associations.includes(review.author_association) && !review.user.login.endsWith('[bot]') && - review.user.login !== pr.user.login && !negative.test(review.body || '')); + review.commit?.oid === proof.head_sha && review.author.__typename === 'User' && + rules.associations.includes(review.authorAssociation) && !review.author.login.endsWith('[bot]') && + review.author.login !== pr.user.login && typeof review.body === 'string' && + review.body.trim() === rules.attestation_prefix + proof.head_sha && + !negative.test(review.body)); if (!approved) throw new Error('Human policy approval is no longer valid'); } } catch { fresh = false; } @@ -371,14 +702,27 @@ jobs: script: | const marker = ''; const run = context.payload.workflow_run; - if (run.event !== 'pull_request' || run.path !== '.github/workflows/castiron-custom-code.yml') return; + const {data: workflow} = await github.rest.actions.getWorkflow({ + ...context.repo, workflow_id: 'castiron-custom-code.yml', + }); + if (!Number.isSafeInteger(run.workflow_id) || run.workflow_id !== workflow.id || + workflow.path !== '.github/workflows/castiron-custom-code.yml') return; + if (run.event !== 'pull_request' || run.path.split('@', 1)[0] !== '.github/workflows/castiron-custom-code.yml') return; let pulls = run.pull_requests?.length ? run.pull_requests : await github.paginate(github.rest.repos.listPullRequestsAssociatedWithCommit, {...context.repo, commit_sha: run.head_sha}); if (!pulls.length) pulls = await github.paginate(github.rest.pulls.list, { ...context.repo, state: 'open', head: `${run.head_repository.owner.login}:${run.head_branch}`, }); + const current = []; for (const pull of pulls) { - const {data: current} = await github.rest.pulls.get({...context.repo, pull_number: pull.number}); - if (current.state !== 'open' || current.head.sha !== run.head_sha) continue; + const {data: pr} = await github.rest.pulls.get({...context.repo, pull_number: pull.number}); + if (pr.state === 'open' && pr.head.sha === run.head_sha && + ['main', 'next'].includes(pr.base.ref) && pr.base.repo.full_name === `${context.repo.owner}/${context.repo.repo}`) current.push(pr); + } + // Match trusted compute's supported-target and main-preference selection. + const main = current.filter(pr => pr.base.ref === 'main'); + const selected = main.length ? main : current; + if (selected.length !== 1) return; + for (const pull of selected) { const comments = await github.paginate(github.rest.issues.listComments, {...context.repo, issue_number: pull.number}); const previous = comments.find(c => c.user?.type === 'Bot' && c.user?.login === 'github-actions[bot]' && c.body?.startsWith(marker)); const prior = previous?.body?.match(//); diff --git a/.github/workflows/castiron-custom-code.yml b/.github/workflows/castiron-custom-code.yml index bde6e47d7..e553b6b01 100644 --- a/.github/workflows/castiron-custom-code.yml +++ b/.github/workflows/castiron-custom-code.yml @@ -4,6 +4,8 @@ name: Castiron custom code on: pull_request: types: [opened, synchronize, reopened, ready_for_review, edited] + pull_request_review: + types: [submitted, edited, dismissed] # Notify the existing trusted handler for queue candidates. merge_group: types: [checks_requested] @@ -16,11 +18,11 @@ concurrency: cancel-in-progress: false env: - REPORTER_SHA256: 6b7cfda1f85426daaa2e4f6186e2d1b20ac638dadaa54be8142ecec89fefd880 + REPORTER_SHA256: a52a1aabf4047f4c20e96d1d788c25a4aa78c1987b4fa73a13d22c92ef213a3f jobs: queue-signal: - if: github.event_name == 'merge_group' + if: github.event_name == 'merge_group' || github.event_name == 'pull_request_review' runs-on: ubuntu-latest timeout-minutes: 1 permissions: {} diff --git a/scripts/castiron/CUSTOM_CODE.md b/scripts/castiron/CUSTOM_CODE.md index 5ee61c8bf..5c8a53ae3 100644 --- a/scripts/castiron/CUSTOM_CODE.md +++ b/scripts/castiron/CUSTOM_CODE.md @@ -9,6 +9,24 @@ and human review, and [AGENTS.md](../../AGENTS.md#custom-code-budget) for agent ## What is counted +Repositories may separately activate `"ratchet": "go"` in their policy after +this checker reaches trusted main. Activation must be a human-reviewed, +policy-only PR; SDK promotion does not carry policy edits. Keep the existing +`max_custom_patch_lines` ceiling. + +The Go ratchet rejects net growth in generated-owned custom patch additions +plus deletions **and physical lines of handwritten Go** compared with the +captured live target. It includes helpers, tests, comments, blank lines, nested +modules, and SDK-owned CI Go retained in private checkpoints. Non-text and +nonregular handwritten Go fail validation. Both revisions use their own verified +generated snapshots, read as Git objects without executing candidate code. + +Stale PRs compare with the live target, not their historical merge base. Merge +queues check the complete combined candidate. Every landed reduction becomes +the next baseline automatically. An activated ratchet cannot be disabled; +raising the independent patch-only ceiling does not restore retired headroom. +Existing policy isolation, human review, and snapshot ownership rules still apply. + The budget is **added lines plus deleted lines in the entire current custom patch** against the candidate's verified pure-generated snapshot. It is not the PR's diff against main, and additions and deletions never cancel. Replacing 500 generated @@ -188,14 +206,34 @@ PR targeting next; preserve the existing budget/schema fields: The SHA must belong to that same repository's authenticated main first-parent history and support this pin protocol. Selection reads the protected target -base, never the candidate. Updates require an exact-head human approving review -from a repository owner, member, or collaborator, in addition to ordinary -CODEOWNERS and required reviews. Bot reviews, review-body markers, stale reviews, -and dismissed approvals do not authorize activation. +base, never the candidate. Pin-only updates and initial pin adoption use ordinary +CODEOWNERS and required reviews when the pin does not roll back to an older main +checker, the effective pinned budget stays the same or decreases, and its schema, +strictness and ratchet mode stay unchanged. Initial adoption compares checker age +with authenticated bootstrap main; later updates compare with the current protected +target pin and effective policy, even for stale PRs. A +checker rollback, an effective pinned-budget increase or mode transition, a local numeric budget increase, or Go +ratchet activation still requires an exact-head human approving review from a +repository owner, member, or collaborator. Bot reviews, stale reviews and dismissed +approvals do not authorize those changes. + +For those human-required changes, the approving reviewer must explicitly attest that +they personally performed the human review. The entire review body must be the +following sentence, substituting the exact full current head SHA: + +```text +I personally reviewed and approve this policy change as a human: <40-character head SHA> +``` + +Blank reviews, ordinary code-review text, quoted attestations, and attestations for +an earlier head do not qualify. Agents must never submit this attestation, even +through a human account. This is an explicit reviewer attestation, not a claim that +GitHub account type or team membership proves who operated the account. Ordinary +SDK code reviews continue to accept automation under their normal repository gates. For initial adoption only, a base with no pin may accept an exact policy-only pin-initialization PR under authenticated current-main checker/policy, with the -same human review requirement. The proposed pin is validated but never executed +same effective-budget and mode review requirements. The proposed pin is validated but never executed for its own initialization. A malformed base pin cannot enter this path; other changes without a base pin fail closed. Ordinary pin updates execute under the previous approved version and cannot include SDK/tooling changes. @@ -215,3 +253,28 @@ immediately before publication. Dismissed, replaced, stale, automated or missing approvals fail closed even if ordinary automated approvals keep the queue live. The publisher consumes the selector's versioned review rules, with cross-runtime predicate fixtures; candidate artifacts never supply authorization or outcomes. + +### Review changes after publication + +The unprivileged reporter workflow signals submitted, edited and dismissed reviews. +For a current next PR changing the policy file, the trusted main handler marks its +required budget statuses and affected live queue statuses pending, then requests +normal reporter reruns. Only the ordinary authenticated evaluator may restore +success; the review signal never selects executable code, approval rules or an +arbitrary status target. Fork runs with empty PR lists use authenticated commit +associations and open-PR lookup, retaining live head/merge binding. The handler +marks each authenticated snapshot pending before slow run pagination, then retries +discovery if the PR or queue changes. Pending statuses on superseded affected +SHAs cannot authorize a merge; observed successes stay revoked even if a later +lookup fails. Reporter reruns must resolve to this same PR under the canonical +main-preference rule, not merely share its head SHA. Missing or ambiguous reporter +runs remain pending. Status writers share a serialization group. + +Retain native required approving reviews and CODEOWNERS at merge time. Review-event +delivery and reevaluation are asynchronous; this mechanism is not an atomic human +approval check at merge. In particular, automated reviews using human accounts do +not become human policy authorization. Do not weaken native gates or treat this +handler as their replacement. A failed invalidation or rerun must be investigated; +missing reporter runs remain pending until the normal trusted run is requested. +Install and validate the review signal on the protected target before activating +next policy checks, including actual PR and queue revocation after dismissal. diff --git a/scripts/castiron/custom_code_budget.py b/scripts/castiron/custom_code_budget.py old mode 100644 new mode 100755 index e6ab6091e..e1dd34e07 --- a/scripts/castiron/custom_code_budget.py +++ b/scripts/castiron/custom_code_budget.py @@ -50,6 +50,10 @@ def strict_policy(repo: Path, revision: str) -> bool: return cast(int, read_policy(repo, revision)["schema_version"]) == 2 +def go_ratchet(repo: Path, revision: str) -> bool: + return cast(bool, read_policy(repo, revision).get("ratchet") == "go") + + MANIFEST = b"scripts/castiron/generated-files.json" # Accept the relocated tooling only from the verified checkpoint. Deploy this # reader before changing the producer so trusted-main checks can validate moves. @@ -266,6 +270,8 @@ def check_isolation(repo: Path, base: str, head: str) -> int | None: return None if strict_policy(repo, base) and not strict_policy(repo, head): raise ValueError("strict generated-file ownership cannot be downgraded") + if go_ratchet(repo, base) and not go_ratchet(repo, head): + raise ValueError("Go customization ratchet cannot be disabled") if paths != {POLICY.encode()}: raise ValueError(ISOLATION_MESSAGE) return proposed @@ -290,6 +296,40 @@ def outcome(state: str, description: str) -> dict[str, str]: return {"state": state, "description": description} +def go_customization(repo: Path, revision: str, measured: dict[str, Any]) -> dict[str, Any]: + """Count patches plus physical handwritten Go, using only verified Git objects.""" + added, removed = count_custom_lines(measured) + # Use the reporter's ownership view. Private snapshots also retain SDK-owned + # CI files excluded from that view; those Go files are still handwritten. + generated = report.tree_entries(repo, measured["after"]["commit"]) + handwritten = { + path: entry + for path, entry in report.tree_entries(repo, revision, include_workflows=True).items() + if path.endswith(b".go") and path not in generated + } + for path, entry in handwritten.items(): + if entry.mode not in {b"100644", b"100755"}: + raise ValueError(f"nonregular handwritten Go: {path!r}") + blobs = report.read_blobs(repo, handwritten) + files: list[dict[str, Any]] = [] + for path, entry in sorted(handwritten.items()): + content = blobs[entry.oid] + content.decode("utf-8") + if b"\0" in content: + raise ValueError(f"non-text handwritten Go: {path!r}") + lines = content.count(b"\n") + int(bool(content) and not content.endswith(b"\n")) + files.append({"path": path.decode("utf-8"), "lines": lines}) + handwritten_lines = sum(file["lines"] for file in files) + return { + "revision": revision, + "generated": measured["after"], + "patch_lines": added + removed, + "handwritten_lines": handwritten_lines, + "handwritten_files": files, + "total": added + removed + handwritten_lines, + } + + def evaluate( repo: Path, base: str, @@ -362,11 +402,38 @@ def evaluate( result["checked_limit"] = checked_limit state = "success" if total <= checked_limit else "failure" description = f"+{added} / -{removed} = {total} custom lines; limit {checked_limit}." + if any(go_ratchet(repo, revision) for revision in (policy, base, head)): + # The report's historical merge base is for presentation only. Measure + # the captured live target against its own hash-verified snapshot. + baseline, _ = report.build_report( + repo, + base, + base, + public=public, + fetch=fetch, + require_head_hash=True, + target_branch=target_branch, + ) + before = go_customization(repo, base, baseline) + after = go_customization(repo, head, measured) + delta = after["total"] - before["total"] + result["ratchet"] = {"kind": "go", "before": before, "after": after, "delta": delta} + description += ( + f" Go customization {before['total']} -> {after['total']} (delta {delta:+d})." + ) + if delta > 0: + state = "failure" + if go_ratchet(repo, base) and not go_ratchet(repo, head): + state = "failure" + description += " Go customization ratchet cannot be disabled." # Main's strict policy applies even if the candidate tries to remove it. # A proposed activation must itself satisfy the stricter contract. if strict_policy(repo, policy) or strict_policy(repo, base) or strict_policy(repo, head): violations = verify_generated( - repo, measured["after"]["commit"], head, previous=measured["before"]["commit"] + repo, + measured["after"]["commit"], + head, + previous=(measured["before"] or measured["adoption_baseline"])["commit"], ) result["strict_violations"] = violations if violations: @@ -512,24 +579,23 @@ def github_evaluate( f"{branch} must require a merge queue before activating the budget gate" ) page += 1 - if branch == "next": - # This receipt is created locally by the trusted-main selector before the - # pinned checkout. It must never come from candidate workflow artifacts. - if ( - selection is None - or type(selection.get("schema_version")) is not int - or any( - selection.get(key) != value - for key, value in { - "schema_version": 1, - "target_branch": branch, - "base_sha": base, - "head_sha": head, - "policy_sha": main, - }.items() - ) - ): - raise ValueError("trusted next policy selection does not match evaluation") + # This receipt is created locally by the trusted-main selector before the + # pinned checkout. It must never come from candidate workflow artifacts. + if ( + selection is None + or type(selection.get("schema_version")) is not int + or any( + selection.get(key) != value + for key, value in { + "schema_version": 1, + "target_branch": branch, + "base_sha": base, + "head_sha": head, + "policy_sha": main, + }.items() + ) + ): + raise ValueError("trusted policy selection does not match evaluation") measurement = None if trusted_report_dir is not None: # Only the preceding main-branch step may supply this directory. Never @@ -556,19 +622,6 @@ def github_evaluate( report.git( repo, "fetch", "--quiet", "--no-tags", "origin", *dict.fromkeys((main, base, head)) ) - if branch == "next": - assert selection is not None - base_pin = read_policy(repo, base).get("next_policy") - if base_pin is None: - if selection.get("bootstrap") is not True or changed_paths(repo, base, head) != { - POLICY.encode() - }: - raise ValueError("unreviewed next policy initialization") - elif selection.get("bootstrap") is not False or base_pin != { - "repository": repository, - "main_sha": main, - }: - raise ValueError("trusted next base pin changed; rerun policy selection") pull_heads: list[str] | None = None if run["event"] == "merge_group": if report.git(repo, "merge-base", base, head).decode().strip() != base: @@ -585,6 +638,22 @@ def github_evaluate( pull_heads.append(candidate) if not pull_heads: raise ValueError("cannot verify merge-group PR membership; retry the current group") + if branch == "next": + assert selection is not None + base_pin = read_policy(repo, base).get("next_policy") + if base_pin is None: + # evaluate() checks each verified queue constituent for policy isolation. + if ( + selection.get("bootstrap") is not True + or "next_policy" not in read_policy(repo, head) + or (pull_heads is None and changed_paths(repo, base, head) != {POLICY.encode()}) + ): + raise ValueError("unreviewed next policy initialization") + elif selection.get("bootstrap") is not False or base_pin != { + "repository": repository, + "main_sha": main, + }: + raise ValueError("trusted next base pin changed; rerun policy selection") result, patch = evaluate( repo, base, @@ -602,7 +671,7 @@ def github_evaluate( def main() -> int: if sys.argv[1:2] == ["select-policy"]: - return policy.main(sys.argv[1:]) + return cast(int, policy.main(sys.argv[1:])) parser = argparse.ArgumentParser(description=__doc__) commands = parser.add_subparsers(dest="command", required=True) generated = commands.add_parser("verify-generated") diff --git a/scripts/castiron/custom_code_policy.py b/scripts/castiron/custom_code_policy.py old mode 100644 new mode 100755 index 2317ad2af..65a63d5e7 --- a/scripts/castiron/custom_code_policy.py +++ b/scripts/castiron/custom_code_policy.py @@ -59,6 +59,10 @@ def read_policy(repo: Path, revision: str) -> dict[str, Any]: if type(version) is not int or version not in (1, 2): raise ValueError("unsupported budget schema_version") keys = {"schema_version", "max_custom_patch_lines"} + if "ratchet" in value: + keys.add("ratchet") + if value["ratchet"] != "go": + raise ValueError("ratchet must be go") if version == 2: keys.add("generated_files") if "next_policy" in value: @@ -99,7 +103,8 @@ def next_policy_sha(repo: Path, repository: str, base: str, authenticated_main: if fetched_main != authenticated_main: raise ValueError("authenticated main changed during policy selection; retry") pin = read_policy(repo, base).get("next_policy") - if not isinstance(pin, dict): + # read_policy validates the shape of every present next_policy pin. + if pin is None: raise ValueError("trusted next base has no reviewed policy pin") if pin["repository"] != repository: raise ValueError("next policy pin belongs to another repository") @@ -125,17 +130,19 @@ def next_policy_sha(repo: Path, repository: str, base: str, authenticated_main: ) if not capable: raise ValueError("pinned checker does not support reviewed next policies") - return policy + return cast(str, policy) class HumanReviewRules(TypedDict): version: int associations: list[str] negative_pattern: str + attestation_prefix: str HUMAN_REVIEW_RULES: HumanReviewRules = { - "version": 1, + "version": 2, + "attestation_prefix": "I personally reviewed and approve this policy change as a human: ", "associations": ["OWNER", "MEMBER", "COLLABORATOR"], "negative_pattern": ( r"automated[\s_:-]*(?:source[\s_:-]*)?review|source[- ]only review|" @@ -149,7 +156,7 @@ def has_human_approval(reviews: list[dict[str, Any]], author: str, head: str) -> """Versioned review predicate; publisher consumes these same trusted rules.""" latest: dict[str, dict[str, Any]] = {} for review in reviews: - if review["state"] != "PENDING": + if review["state"] != "PENDING" and review.get("user") is not None: login = review["user"]["login"] if login not in latest or review["id"] > latest[login]["id"]: latest[login] = review @@ -160,6 +167,7 @@ def has_human_approval(reviews: list[dict[str, Any]], author: str, head: str) -> and review.get("author_association") in HUMAN_REVIEW_RULES["associations"] and not review["user"]["login"].endswith("[bot]") and review["user"]["login"] != author + and (review.get("body") or "").strip() == HUMAN_REVIEW_RULES["attestation_prefix"] + head and not re.search( HUMAN_REVIEW_RULES["negative_pattern"], review.get("body") or "", re.IGNORECASE ) @@ -227,7 +235,9 @@ def select_policy( head_policy = read_policy(repo, head) bootstrap = "next_policy" not in base_policy if bootstrap: - if "next_policy" not in head_policy or changed_paths(repo, base, head) != {POLICY.encode()}: + if "next_policy" not in head_policy or ( + pull_heads is None and changed_paths(repo, base, head) != {POLICY.encode()} + ): raise ValueError("missing next pin allows only reviewed policy-only initialization") policy = authenticated_main else: @@ -246,7 +256,8 @@ def select_policy( raise ValueError("next evaluation has no verified constituent PR") for candidate in candidates: candidate_policy = read_policy(repo, candidate) - candidate_base_policy = read_policy(repo, merge_base(repo, base, candidate)) + candidate_base = merge_base(repo, base, candidate) + candidate_base_policy = read_policy(repo, candidate_base) pin_changed = candidate_policy.get("next_policy") != candidate_base_policy.get( "next_policy" ) @@ -254,9 +265,46 @@ def select_policy( candidate_policy["max_custom_patch_lines"] > candidate_base_policy["max_custom_patch_lines"] ) - if pin_changed or budget_increased: - if changed_paths(repo, base, candidate) != {POLICY.encode()}: - raise ValueError("each next policy activation or increase must be policy-only") + ratchet_activated = ( + candidate_policy.get("ratchet") == "go" and candidate_base_policy.get("ratchet") != "go" + ) + if (pin_changed or budget_increased or ratchet_activated) and changed_paths( + repo, base, candidate + ) != {POLICY.encode()}: + raise ValueError("each next policy activation or increase must be policy-only") + pin_requires_human = False + if pin_changed: + proposed = read_policy( + repo, next_policy_sha(repo, repository, candidate, authenticated_main) + ) + # Authored changes come from the merge base, but their safety must be + # measured against the protected target that will actually be replaced. + previous = read_policy(repo, policy) if "next_policy" in base_policy else base_policy + # Bootstrap executes authenticated main; initial adoption must not silently + # replace it with older checker code, even when policy fields match. + previous_checker = ( + base_policy["next_policy"]["main_sha"] + if "next_policy" in base_policy + else authenticated_main + ) + rolled_back = bool( + report.git( + repo, + "rev-list", + "--first-parent", + f"{candidate_policy['next_policy']['main_sha']}..{previous_checker}", + ).strip() + ) + # Compare effective policies, not the candidate's unchanged local ceiling. + # Schema/strictness or ratchet transitions also retain human authorization. + pin_requires_human = ( + rolled_back + or proposed["max_custom_patch_lines"] > previous["max_custom_patch_lines"] + or proposed["schema_version"] != previous["schema_version"] + or proposed.get("generated_files") != previous.get("generated_files") + or proposed.get("ratchet") != previous.get("ratchet") + ) + if budget_increased or ratchet_activated or pin_requires_human: number = require_human_policy_review(repository, candidate) policy_reviews.append({"number": number, "head_sha": candidate}) return { @@ -305,6 +353,13 @@ def select_github_policy( or run["path"].split("@", 1)[0] != source_workflow ): raise ValueError("unexpected or superseded policy source run") + workflow = report.api("GET", f"{root}/actions/workflows/{source_workflow.rsplit('/', 1)[1]}") + if ( + type(run.get("workflow_id")) is not int + or run["workflow_id"] != workflow["id"] + or workflow["path"] != source_workflow + ): + raise ValueError("policy source run is not the canonical workflow") head = report.require_sha(run["head_sha"]) if run["event"] == "pull_request": current = report.current_pull(repository, run) @@ -368,10 +423,11 @@ def select_github_policy( def merge_base(repo: Path, base: str, head: str) -> str: """Distinguish actual PR policy edits from later changes on the target.""" - return ( + return cast( + str, report.git(repo, "merge-base", report.require_sha(base), report.require_sha(head)) .decode() - .strip() + .strip(), ) @@ -459,10 +515,10 @@ def main(argv: list[str] | None = None) -> int: args.out.write_text(json.dumps(selected, sort_keys=True) + "\n") if "GITHUB_OUTPUT" in os.environ: with Path(os.environ["GITHUB_OUTPUT"]).open("a") as output: - for key, value in selected.items(): - output.write( - f"{key}={json.dumps(value) if isinstance(value, (bool, list, dict)) else value}\n" - ) + output.writelines( + f"{key}={json.dumps(value) if isinstance(value, (bool, list, dict)) else value}\n" + for key, value in selected.items() + ) return 0 diff --git a/scripts/castiron/custom_code_report.py b/scripts/castiron/custom_code_report.py old mode 100644 new mode 100755 index f9e22afb4..88c897bbf --- a/scripts/castiron/custom_code_report.py +++ b/scripts/castiron/custom_code_report.py @@ -48,6 +48,7 @@ "newly_customized": "Newly customized", "removed": "Customization removed", "existing_changed": "Existing customization changed", + "first_adoption": "Current customization; legacy baseline unavailable", "baseline_changed": "Generated baseline changed", "unchanged": "Existing customization unchanged", "no_longer_generated": "No longer generated", @@ -201,6 +202,14 @@ def public_metadata(repo: Path, commit: str) -> dict[str, str]: if not fields["Branch"].startswith("codegen/"): raise ReportError("invalid public codegen branch") git(repo, "check-ref-format", f"refs/heads/{fields['Branch']}") + for key in ("Adoption-Base", "Adoption-Source"): + values = re.findall(rf"^Castiron-Public-Codegen-{key}: (.+)$", message, re.MULTILINE) + if values: + if len(values) != 1: + raise ReportError("duplicate adoption metadata") + fields[key] = require_sha(values[0]) + if ("Adoption-Base" in fields) != ("Adoption-Source" in fields): + raise ReportError("incomplete adoption metadata") return fields @@ -392,6 +401,7 @@ def create_public_snapshot( content_hash: str, branch: str, parent: str | None, + adoption: tuple[str, str] | None = None, ) -> str: target = ( "next" @@ -406,6 +416,11 @@ def create_public_snapshot( f"Castiron-Public-Codegen-Branch: {branch}\n" f"Castiron-Public-Codegen-Target: {target}\n" ) + if adoption is not None: + if target != "next": + raise ReportError("adoption snapshot must target next") + message += f"Castiron-Public-Codegen-Adoption-Base: {require_sha(adoption[0])}\n" + message += f"Castiron-Public-Codegen-Adoption-Source: {require_sha(adoption[1])}\n" parents = ["-p", require_sha(parent)] if parent is not None else [] snapshot = ( git( @@ -429,6 +444,124 @@ def create_public_snapshot( return snapshot +def first_adoption_source(repo: Path, base: str, *, target_branch: str, fetch: bool) -> str: + """Authority is the protected target base policy, never a candidate receipt.""" + base = require_sha(base) + if target_branch != "next": + raise ReportError("first adoption is next-only") + if git(repo, "ls-tree", base, "--", ".castiron.stats.yml"): + raise ReportError("existing/corrupt Castiron stats require ordinary recovery") + if git(repo, "log", "--full-history", "--format=%H", base, "--", ".castiron.stats.yml").strip(): + raise ReportError("deleted Castiron stats cannot authorize first adoption") + if not git(repo, "ls-tree", base, "--", ".stats.yml").startswith(b"100644 blob "): + raise ReportError("first adoption requires a legacy SDK target") + if not git(repo, "ls-tree", base, "--", ".castiron-ratchet.json").startswith(b"100644 blob "): + raise ReportError("first adoption requires an approved target policy") + + def unique(pairs: list[tuple[str, Any]]) -> dict[str, Any]: + result: dict[str, Any] = {} + for key, value in pairs: + if key in result: + raise ReportError("duplicate policy field") + result[key] = value + return result + + policy = json.loads( + git(repo, "show", f"{base}:.castiron-ratchet.json"), object_pairs_hook=unique + ) + pin = policy.get("next_policy") + if not isinstance(pin, dict) or set(cast(dict[str, Any], pin)) != {"repository", "main_sha"}: + raise ReportError("first adoption requires the protected next-policy pin") + pin = cast(dict[str, Any], pin) + repository = pin["repository"] + if repository not in {"openai/openai-java", "openai/openai-cli"}: + raise ReportError("first adoption is only supported for legacy Java/CLI") + remote = git(repo, "remote", "get-url", "origin").decode().strip() + allowed = { + f"https://github.com/{repository}.git", + f"https://github.com/{repository}", + f"git@github.com:{repository}.git", + f"https://github.com/{repository}-internal.git", + f"https://github.com/{repository}-internal", + f"git@github.com:{repository}-internal.git", + } + if remote not in allowed: + raise ReportError("adoption policy repository does not match SDK origin") + source = require_sha(pin["main_sha"]) + main_ref = "refs/castiron/adoption-main" if fetch else "refs/remotes/origin/main" + if fetch: + git( + repo, + "fetch", + "--quiet", + "--no-tags", + f"https://github.com/{repository}.git", + f"+refs/heads/main:{main_ref}", + ) + if source not in git(repo, "rev-list", "--first-parent", main_ref).decode().splitlines(): + raise ReportError("adoption source is not on authenticated main history") + source_policy = json.loads( + git(repo, "show", f"{source}:.castiron-ratchet.json"), object_pairs_hook=unique + ) + if {key: value for key, value in policy.items() if key != "next_policy"} != source_policy: + raise ReportError("first adoption must preserve approved policy limits") + if not read_stats(repo, source).get("public_codegen_sha"): + raise ReportError( + "approved main must have a genuine public checkpoint before first adoption" + ) + return source + + +def adoption_head( + repo: Path, + base: str, + source: str, + head: str, + after: dict[str, Any], + *, + public: bool, +) -> None: + git(repo, "merge-base", "--is-ancestor", base, head) + if not public: + # Private initialization retains the authenticated source parent. Normal + # promotion deliberately publishes a single-parent target-branch commit. + git(repo, "merge-base", "--is-ancestor", source, head) + if git(repo, "show", f"{base}:.castiron-ratchet.json") != git( + repo, "show", f"{head}:.castiron-ratchet.json" + ): + raise ReportError("first adoption must preserve protected target policy") + original, current = read_stats(repo, source), read_stats(repo, head) + if original["generation_id"] == current["generation_id"]: + raise ReportError("first adoption requires a new generation") + for key in ("config_hash", "openapi_spec_hash"): + if not original.get(key) or current.get(key) != original[key]: + raise ReportError("first adoption must preserve authenticated main inputs") + if public: + # Public authorization comes from the protected pin and verified public + # snapshot ancestry, never from a candidate-supplied adoption receipt. + git(repo, "merge-base", "--is-ancestor", original["public_codegen_sha"], after["commit"]) + metadata = public_metadata(repo, after["commit"]) + if ( + metadata.get("Target") != "next" + or metadata.get("Adoption-Base") != base + or metadata.get("Adoption-Source") != source + ): + raise ReportError("snapshot does not bind the authorized first adoption") + else: + message = git(repo, "show", "-s", "--format=%B", current["codegen_sha"]).decode() + bindings = re.findall(r"^Generation metadata: ([A-Za-z0-9+/=]+)$", message, re.MULTILINE) + if len(bindings) != 1: + raise ReportError("invalid adoption generation binding") + envelope = json.loads(base64.b64decode(bindings[0], validate=True)) + record = envelope.get("record", {}) + if envelope.get("release_target") != "next" or record.get("public_baseline_commit") != base: + raise ReportError("new checkpoint must bind the old target base") + pin = json.loads(git(repo, "show", f"{base}:.castiron-ratchet.json"))["next_policy"] + sdk = pin["repository"].split("/")[1] + if record.get("target") != sdk or record.get("language") != sdk.removeprefix("openai-"): + raise ReportError("adoption generation SDK mismatch") + + def prepare_public_snapshot( source: Path, source_base: str, @@ -438,15 +571,25 @@ def prepare_public_snapshot( branch: str, ) -> dict[str, Any]: """Prepare public-only generated history; never push anything.""" - read_stats(source, source_base) after = read_stats(source, source_head) - public = read_stats(destination, public_base) target_branch = ( "next" if branch == "castiron/next" or branch.startswith("castiron/next/") else "main" ) + adoption_source = None + if not git(destination, "ls-tree", public_base, "--", ".castiron.stats.yml"): + adoption_source = first_adoption_source( + destination, public_base, target_branch=target_branch, fetch=True + ) + else: + read_stats(source, source_base) + public = read_stats(destination, adoption_source or public_base) baseline = resolve_checkpoint( source, after, fetch=True, require_hash=True, target_branch=target_branch ) + if adoption_source: + # Public source commit is already fetched into the private promotion clone. + adoption_head(source, public_base, adoption_source, source_head, baseline, public=False) + codegen_branch = f"codegen/{branch}" ref = f"refs/heads/{codegen_branch}" git(destination, "check-ref-format", ref) @@ -477,7 +620,11 @@ def prepare_public_snapshot( base_snapshot = public.get("public_codegen_sha") if base_snapshot: base_snapshot = resolve_public_baseline( - destination, public_base, fetch=True, require_hash=False, target_branch=target_branch + destination, + adoption_source or public_base, + fetch=True, + require_hash=False, + target_branch="main" if adoption_source else target_branch, )["commit"] elif previous: try: @@ -515,6 +662,13 @@ def prepare_public_snapshot( if ( not upgrade_legacy_parent and metadata["Generation-ID"] == after["generation_id"] + and ( + not adoption_source + or ( + metadata.get("Adoption-Base") == public_base + and metadata.get("Adoption-Source") == adoption_source + ) + ) and metadata["Hash"] == baseline["hash"] and git(destination, "rev-parse", f"{previous}^{{tree}}").decode().strip() == tree and base_snapshot in ancestors @@ -522,7 +676,13 @@ def prepare_public_snapshot( snapshot = previous else: snapshot = create_public_snapshot( - destination, tree, after["generation_id"], baseline["hash"], codegen_branch, parent + destination, + tree, + after["generation_id"], + baseline["hash"], + codegen_branch, + parent, + (public_base, adoption_source) if adoption_source else None, ) elif ( public_metadata(destination, base_snapshot)["Generation-ID"] == after["generation_id"] @@ -531,7 +691,13 @@ def prepare_public_snapshot( snapshot = base_snapshot else: snapshot = create_public_snapshot( - destination, tree, after["generation_id"], baseline["hash"], codegen_branch, parent + destination, + tree, + after["generation_id"], + baseline["hash"], + codegen_branch, + parent, + (public_base, adoption_source) if adoption_source else None, ) stats_path = destination / ".castiron.stats.yml" current = git(source, "show", f"{require_sha(source_head)}:.castiron.stats.yml").decode("utf-8") @@ -678,26 +844,53 @@ def build_report( ) -> tuple[dict[str, Any], bytes]: base, head = require_sha(base), require_sha(head) comparison_base = git(repo, "merge-base", base, head).decode().strip() + adoption_source = None + if not git(repo, "ls-tree", comparison_base, "--", ".castiron.stats.yml"): + if comparison_base != base: + raise ReportError( + "first-adoption target advanced; regenerate against the protected base" + ) + adoption_source = first_adoption_source( + repo, comparison_base, target_branch=target_branch, fetch=fetch + ) if public: after = resolve_public_baseline( - repo, head, fetch=fetch, require_hash=require_head_hash, target_branch=target_branch + repo, + head, + fetch=fetch, + require_hash=require_head_hash, + target_branch=target_branch, ) before = resolve_public_baseline( repo, - comparison_base, + adoption_source or comparison_base, fetch=fetch, require_hash=False, lineage=after["commit"], - target_branch=target_branch, + target_branch="main" if adoption_source else target_branch, ) else: before = resolve_baseline( - repo, comparison_base, fetch=fetch, require_hash=False, target_branch=target_branch + repo, + adoption_source or comparison_base, + fetch=fetch, + require_hash=False, + target_branch="main" if adoption_source else target_branch, ) after = resolve_baseline( - repo, head, fetch=fetch, require_hash=require_head_hash, target_branch=target_branch + repo, + head, + fetch=fetch, + require_hash=require_head_hash, + target_branch=target_branch, ) - g0, i0 = tree_entries(repo, before["commit"]), tree_entries(repo, comparison_base) + if adoption_source: + adoption_head(repo, comparison_base, adoption_source, head, after, public=public) + g0, i0 = ( + ({}, {}) + if adoption_source + else (tree_entries(repo, before["commit"]), tree_entries(repo, comparison_base)) + ) g1, i1 = tree_entries(repo, after["commit"]), tree_entries(repo, head) stats = numstats(repo, after["commit"], head) files: list[dict[str, Any]] = [] @@ -712,8 +905,12 @@ def build_report( files.append( { "path": path.decode("utf-8"), - "category": category, - "custom_before": custom_before, + "category": ( + ("first_adoption" if custom_after else "unchanged") + if adoption_source + else category + ), + "custom_before": None if adoption_source else custom_before, "custom_after": custom_after, "added": added, "removed": removed, @@ -745,11 +942,14 @@ def build_report( "target_base_sha": base, "base_sha": comparison_base, "head_sha": head, - "before": before, + "before": None if adoption_source else before, + "adoption_baseline": before if adoption_source else None, + "adoption_source": adoption_source, + "before_status": "legacy_generation_unavailable" if adoption_source else "verified", "after": after, "files": files, "counts": { - "before": sum(f["custom_before"] for f in files), + "before": None if adoption_source else sum(f["custom_before"] for f in files), "after": sum(f["custom_after"] for f in files), **dict(collections.Counter(f["category"] for f in files)), }, @@ -794,7 +994,15 @@ def render_report( and file["category"] != "no_longer_generated" for file in files ) - if newly_customized == 0: + if report.get("before_status") == "legacy_generation_unavailable": + lines.extend( + [ + "Legacy before-generation unavailable; no historical customization claim.", + "", + f"Current mixed files: {after}. Approved main reference: `{require_sha(report['adoption_source'])}`.", + ] + ) + elif newly_customized == 0: remaining = ( f"{after} mixed file{'s' if after != 1 else ''} remain{'s' if after == 1 else ''}" ) @@ -822,8 +1030,16 @@ def render_report( ), ] ) - lines.extend(["", f"Compared `{base[:12]}` → `{head[:12]}`. Generated baselines verified."]) - if not report["before"]["hash_recorded"] or not report["after"]["hash_recorded"]: + verification = ( + "Current generation and approved main reference verified." + if report["before"] is None + else "Generated baselines verified." + ) + lines.extend(["", f"Compared `{base[:12]}` → `{head[:12]}`. {verification}"]) + if ( + not (report["before"] or report["adoption_baseline"])["hash_recorded"] + or not report["after"]["hash_recorded"] + ): lines.extend( [ "", @@ -1175,6 +1391,10 @@ def main() -> int: trusted.add_argument("--run-attempt", type=int, required=True) trusted.add_argument("--out", type=Path, required=True) trusted.add_argument("--base", required=True, help="immutable main SHA of the trusted checkout") + selecting = commands.add_parser("promotion-base") + selecting.add_argument("--repo", type=Path, required=True) + selecting.add_argument("--base", required=True) + selecting.add_argument("--branch", required=True) preparing = commands.add_parser("prepare-public") preparing.add_argument("--source-repo", type=Path, required=True) preparing.add_argument("--source-base", required=True) @@ -1194,6 +1414,16 @@ def main() -> int: try: if args.command == "hash": sys.stdout.write(hash_codegen_commit(args.repo, args.commit) + "\n") + elif args.command == "promotion-base": + base = require_sha(args.base) + if not git(args.repo, "ls-tree", base, "--", ".castiron.stats.yml"): + target = ( + "next" + if args.branch == "castiron/next" or args.branch.startswith("castiron/next/") + else "main" + ) + base = first_adoption_source(args.repo, base, target_branch=target, fetch=True) + sys.stdout.write(base + "\n") elif args.command == "prepare-public": sys.stdout.write( json.dumps( @@ -1210,7 +1440,12 @@ def main() -> int: ) elif args.command == "trusted-report": trusted_report( - args.repo, args.repository, args.run_id, args.run_attempt, args.out, base=args.base + args.repo, + args.repository, + args.run_id, + args.run_attempt, + args.out, + base=args.base, ) elif args.command == "comment": if args.report.stat().st_size > 5_000_000: diff --git a/scripts/castiron/custom_code_test_support.py b/scripts/castiron/custom_code_test_support.py index 7c9404b02..bd7f1885d 100644 --- a/scripts/castiron/custom_code_test_support.py +++ b/scripts/castiron/custom_code_test_support.py @@ -61,6 +61,7 @@ def baseline(self) -> tuple[str, str]: def source_run(head: str, event: str = "pull_request") -> dict[str, Any]: return { "id": 123, + "workflow_id": 456, "event": event, "head_sha": head, "head_branch": "gh-readonly-queue/main/pr-7-example" if event == "merge_group" else "sdk", diff --git a/scripts/castiron/fixtures/github_publisher.cjs b/scripts/castiron/fixtures/github_publisher.cjs index a959fa880..a2d0e4722 100644 --- a/scripts/castiron/fixtures/github_publisher.cjs +++ b/scripts/castiron/fixtures/github_publisher.cjs @@ -5,23 +5,71 @@ const data = JSON.parse(fs.readFileSync(0, 'utf8')); const writes = []; const github = { rest: { - actions: { getWorkflowRun: async () => ({ data: data.run }) }, + actions: { + getWorkflowRun: async ({ run_id }) => { + if (data.run_errors?.[run_id]) + throw Object.assign(new Error('Workflow run lookup failed'), { status: data.run_errors[run_id] }); + return { data: data.runs_by_id?.[run_id] || data.run }; + }, + getWorkflow: async ({ workflow_id }) => ({ + data: { + id: workflow_id === 'castiron-custom-code-comment.yml' ? 789 : 456, + path: `.github/workflows/${workflow_id}`, + }, + }), + listWorkflowRuns: 'workflow-runs', + reRunWorkflow: async (value) => { + if (data.fail_rerun) throw new Error('Simulated dispatch failure'); + writes.push({ operation: 'rerun', ...value }); + }, + }, pulls: { get: async ({ pull_number }) => ({ data: data.current_sequence?.shift() || data.pulls_by_number?.[pull_number] || data.current, }), list: 'pulls', listReviews: 'reviews', + listFiles: 'files', }, git: { getRef: async ({ ref }) => ({ data: { - object: { sha: ref === 'heads/main' && data.main_sha ? data.main_sha : data.current.base.sha }, + object: { + sha: ref.startsWith('pull/') + ? data.merge_sha + : ref === 'heads/main' && (data.main_sha_sequence?.length || data.main_sha) + ? data.main_sha_sequence?.shift() || data.main_sha + : data.current.base.sha, + }, }, }), }, repos: { - createCommitStatus: async (value) => writes.push(value), + getCommit: async () => ({ data: data.merge_commit }), + compareCommitsWithBasehead: async ({ basehead }) => ({ + data: data.comparisons?.[basehead] || { merge_base_commit: { sha: basehead.split('...')[0] } }, + }), + getContent: async ({ path, ref }) => { + if (path !== '.castiron-ratchet.json' || !data.policies_by_sha?.[ref]) + throw new Error('Unexpected policy lookup'); + const content = Buffer.from(JSON.stringify(data.policies_by_sha[ref])); + return { + data: { + type: 'file', + encoding: 'base64', + size: content.length, + content: content.toString('base64'), + }, + }; + }, + createCommitStatus: async (value) => { + writes.push(value); + data.statuses_by_sha ||= {}; + data.statuses_by_sha[value.sha] = [ + { ...value, creator: { login: 'github-actions[bot]' } }, + ...(data.statuses_by_sha[value.sha] || data.previous_statuses || []), + ]; + }, listCommitStatusesForRef: 'statuses', listPullRequestsAssociatedWithCommit: 'associations', }, @@ -31,15 +79,31 @@ const github = { updateComment: async (value) => writes.push({ operation: 'update', ...value }), }, }, - graphql: async (_query, { branch, cursor }) => { - if (branch !== data.current.base.ref) throw new Error('Wrong queue target'); + graphql: async (_query, { branch, cursor, number }) => { + if (_query.includes('reviews(first:100')) { + const page = data.review_pages?.shift(); + if (page && page.expected_cursor !== cursor) throw new Error('Wrong review cursor'); + return { + repository: { + pullRequest: { + reviews: page || { + nodes: data.review_nodes_by_number?.[number] || data.review_nodes || [], + pageInfo: { hasNextPage: false, endCursor: null }, + }, + }, + }, + }; + } + if (data.forbid_queue) throw new Error('Unexpected queue discovery'); + if ((branch || 'next') !== data.current.base.ref) throw new Error('Wrong queue target'); const page = data.queue_pages?.shift(); if (page && page.expected_cursor !== cursor) throw new Error('Wrong queue cursor'); return { repository: { mergeQueue: { entries: page || { - nodes: data.queue_absent ? [] : [{ headCommit: { oid: data.run.head_sha } }], + nodes: + data.queue_nodes || (data.queue_absent ? [] : [{ headCommit: { oid: data.run.head_sha } }]), pageInfo: { hasNextPage: false, endCursor: null }, }, }, @@ -47,10 +111,12 @@ const github = { }; }, paginate: async (method, params) => { + if (method === 'files') return data.files || [{ filename: '.castiron-ratchet.json' }]; + if (method === 'workflow-runs') return data.runs_by_sha?.[params.head_sha] || []; if (method === 'reviews') return data.reviews_by_number?.[params.pull_number] || data.reviews || []; - if (method === 'statuses') return data.previous_statuses || []; + if (method === 'statuses') return data.statuses_by_sha?.[params.ref] || data.previous_statuses || []; if (method === 'comments') return data.comments || []; - if (method === 'associations') return []; + if (method === 'associations') return data.associated_pulls || []; if (method === 'pulls' && params.head === 'contributor:sdk' && params.state === 'open') { return data.fallback_pulls || []; } @@ -58,9 +124,15 @@ const github = { }, }; const AsyncFunction = Object.getPrototypeOf(async function () {}).constructor; -new AsyncFunction('github', 'context', 'process', data.script)(github, data.context, { env: data.env || {} }) +new AsyncFunction('github', 'context', 'process', 'require', data.script)( + github, + data.context, + { env: data.env || {} }, + require, +) .then(() => process.stdout.write(JSON.stringify(writes))) .catch((error) => { + if (data.capture_writes) fs.writeFileSync(data.capture_writes, JSON.stringify(writes)); console.error(error); process.exitCode = 1; }); diff --git a/scripts/castiron/generated-files.json b/scripts/castiron/generated-files.json index 09f3e8d43..d3d3cfe4a 100644 --- a/scripts/castiron/generated-files.json +++ b/scripts/castiron/generated-files.json @@ -3,11 +3,11 @@ "files": { ".github/workflows/castiron-custom-code-comment.yml": { "mode": "100644", - "sha256": "87fda8ee677c655a5e8138491e71358f3631700ba87bb1f420aa0736c889381a" + "sha256": "afebb2d21c7b322905e67452004da087d5bd3f60fc3c0cc11d9bccad9900103d" }, ".github/workflows/castiron-custom-code.yml": { "mode": "100644", - "sha256": "402da2fd16cc575eef7f4bf6bb1cff25272ee350452cb776f17c1589c036a3a1" + "sha256": "2760a19c1e05a2a01d73a679dac9d59ba7cfee2faa2fe426182f740ae1603202" }, "openai-java-core/src/main/kotlin/com/openai/client/OpenAIClient.kt": { "mode": "100644", @@ -15911,31 +15911,31 @@ }, "scripts/castiron/CUSTOM_CODE.md": { "mode": "100644", - "sha256": "6578db0d17cf297ea01b862f772a716bef56a81192498f423463505950707701" + "sha256": "c0315b363849bcebb2819d07ed84615db47f03132ad3f59f1fcdfcf9ffad2ece" }, "scripts/castiron/README.md": { "mode": "100644", "sha256": "4aff82cb988d01babde79902ae20de842fdd4b2b447d5f4cb3c5ce0a36d6fab8" }, "scripts/castiron/custom_code_budget.py": { - "mode": "100644", - "sha256": "447adc841dfdca7ad75956d277778b6f79d77efa17e8b5f08a4a79664060da77" + "mode": "100755", + "sha256": "3c96300546afb5e57fbfb596d4e1bde90821928e44904c4aeb91e2d7e4e27ff5" }, "scripts/castiron/custom_code_policy.py": { - "mode": "100644", - "sha256": "4b9c374b838bb544e1325823f195bca0f99989745ad09add43ad3b702016e492" + "mode": "100755", + "sha256": "b1425b2c9c5fcf47897c266776fbd1ce28f98df1cfd8af492335589a707f840a" }, "scripts/castiron/custom_code_report.py": { - "mode": "100644", - "sha256": "6b7cfda1f85426daaa2e4f6186e2d1b20ac638dadaa54be8142ecec89fefd880" + "mode": "100755", + "sha256": "a52a1aabf4047f4c20e96d1d788c25a4aa78c1987b4fa73a13d22c92ef213a3f" }, "scripts/castiron/custom_code_test_support.py": { "mode": "100644", - "sha256": "27a1c5ee377fa601f896191e3914cb74e4bf65ab814819a408e884332b713bde" + "sha256": "15b2c1243f1cbec53b73707e5806a8566d40f78763fe85c4ab7dc48bc1405a6c" }, "scripts/castiron/fixtures/github_publisher.cjs": { "mode": "100644", - "sha256": "3c39971b4371f2dd547f0dfa692e2980f1e7a14844799fe1834f7d8d1ab0a60b" + "sha256": "7cd554d124da0ae9eb7fac3c4bbb1c8129f0d6868e442698105fe4945f6ee814" }, "scripts/castiron/ruff.toml": { "mode": "100644", @@ -15943,23 +15943,23 @@ }, "scripts/castiron/test_custom_code_budget.py": { "mode": "100644", - "sha256": "71527f2e594989bb3f7f94a723322714bf9bd2669aa861719f29f462b925a4b6" + "sha256": "feaf35f7f30deecb89d95b31b8a325033180c64650e1d965cbdee486ab11f16d" }, "scripts/castiron/test_custom_code_github.py": { "mode": "100644", - "sha256": "203acb9491117d6fb7e3c7fd3379055aa42d7e4884be4dcbef364c9f13ea4c7c" + "sha256": "f0f3fa6810ef73b0656d722e65125003a5685b61324dda6c93d1d0dbf619df37" }, "scripts/castiron/test_custom_code_policy.py": { "mode": "100644", - "sha256": "14cc036ca21d6cdc271cce93027e7b90636d780d9550bf119f70a2316fab782d" + "sha256": "5de73d64a2f89a4b7bd805f59f0c6a6d29452866ef050c1191e04a8c48e6db25" }, "scripts/castiron/test_custom_code_publication.py": { "mode": "100644", - "sha256": "8c55aaf375ca85cdf6921598205f3a67dee008f471404015c48bbcf4c8104dfc" + "sha256": "4e9077ffc214685e49d4156163eaf904e650bc9ed82ecbdb03a19fc22bd4fbbf" }, "scripts/castiron/test_custom_code_report.py": { "mode": "100644", - "sha256": "f8f6b883c545963b4f82d8b078734298294607a9371e70e01fb0c3a37304be61" + "sha256": "f4d90fd38b17aac59096488391b65ef9da5493d2049d4c038d5926a960257ca7" } } } diff --git a/scripts/castiron/test_custom_code_budget.py b/scripts/castiron/test_custom_code_budget.py index 602ade2c8..b35e8fad4 100644 --- a/scripts/castiron/test_custom_code_budget.py +++ b/scripts/castiron/test_custom_code_budget.py @@ -16,6 +16,152 @@ from custom_code_test_support import GitTestCase +class GoRatchetTests(GitTestCase): + def prepare(self, *, inherited_ci: bool = False) -> str: + if inherited_ci: + self.write(".github/actions/check/main.go", "package main\n") + self.baseline() + self.policy() + return self.commit("activate Go ratchet") + + def policy(self, *, active: bool = True, limit: int = 2000) -> None: + value: dict[str, Any] = {"schema_version": 1, "max_custom_patch_lines": limit} + if active: + value["ratchet"] = "go" + self.write(budget.POLICY, json.dumps(value) + "\n") + + def evaluate(self, base: str, head: str, **kwargs: Any) -> dict[str, Any]: + return budget.evaluate(self.repo, base, head, public=False, **kwargs)[0] + + def test_handwritten_helpers_tests_comments_and_nested_modules_count(self) -> None: + base = self.prepare() + for path in ( + "helper.go", + "helper_test.go", + "nested/module/helper.go", + ".github/workflows/check.go", + ): + with self.subTest(path=path): + self.git("checkout", "--detach", "-q", base) + self.write(path, "// comment\n\npackage example\n// final line") + result = self.evaluate(base, self.commit()) + self.assertEqual(result["checks"]["budget"]["state"], "failure") + self.assertEqual(result["ratchet"]["delta"], 4) + self.assertEqual( + result["ratchet"]["after"]["handwritten_files"], [{"path": path, "lines": 4}] + ) + + def test_inherited_private_checkpoint_ci_is_handwritten(self) -> None: + base = self.prepare(inherited_ci=True) + self.write(".github/actions/check/main.go", "package main\n// added\n") + result = self.evaluate(base, self.commit()) + self.assertEqual(result["total"], 0) + self.assertEqual(result["ratchet"]["before"]["handwritten_lines"], 1) + self.assertEqual(result["ratchet"]["delta"], 1) + self.assertEqual( + result["ratchet"]["after"]["handwritten_files"], + [{"path": ".github/actions/check/main.go", "lines": 2}], + ) + self.assertEqual(result["checks"]["budget"]["state"], "failure") + + def test_patch_reduction_can_pay_for_handwritten_lines(self) -> None: + self.prepare() + self.write("generated.py", "generated\ncustom\ncustom\n") + base = self.commit() + self.write("generated.py", "generated\n") + self.write("helper.go", "package example\n") + result = self.evaluate(base, self.commit()) + self.assertEqual(result["ratchet"]["delta"], -1) + self.assertEqual(result["checks"]["budget"]["state"], "success") + + def test_stale_branch_cannot_reclaim_landed_reduction(self) -> None: + self.prepare() + self.write("helper.go", "package example\n// old allowance\n") + old = self.commit() + self.git("checkout", "-q", "-b", "stale") + head = self.commit() + self.git("checkout", "-q", "main") + self.write("helper.go", "package example\n") + base = self.commit() + self.assertEqual(self.git("merge-base", base, head), old) + result = self.evaluate(base, head) + self.assertEqual(result["ratchet"]["before"]["revision"], base) + self.assertEqual(result["ratchet"]["delta"], 1) + self.assertEqual(result["checks"]["budget"]["state"], "failure") + + def test_complete_merge_queue_candidate_is_counted(self) -> None: + base = self.prepare() + constituent = self.commit("empty constituent") + self.write("helper.go", "package example\n") + result = self.evaluate(base, self.commit("combined queue"), pull_heads=[constituent]) + self.assertEqual(result["checks"]["isolation"]["state"], "success") + self.assertEqual(result["checks"]["budget"]["state"], "failure") + + def test_ratchet_cannot_be_disabled(self) -> None: + base = self.prepare() + self.policy(active=False) + result = self.evaluate(base, self.commit()) + for name in ("budget", "isolation"): + self.assertEqual(result["checks"][name]["state"], "failure") + self.assertIn("cannot be disabled", result["checks"][name]["description"]) + + def test_old_ceiling_increase_cannot_restore_ratchet_headroom(self) -> None: + self.prepare() + self.policy(limit=3000) + base = self.commit("separately reviewed ceiling increase") + self.write("helper.go", "package example\n") + result = self.evaluate(base, self.commit()) + self.assertEqual(result["limit"], 3000) + self.assertEqual(result["checks"]["budget"]["state"], "failure") + + def test_fixed_patch_ceiling_remains_independent(self) -> None: + self.prepare() + self.policy(limit=1) + self.write("helper.go", "package example\n// two\n// three\n") + base = self.commit() + (self.repo / "helper.go").unlink() + self.write("generated.py", "generated\ncustom\ncustom\n") + result = self.evaluate(base, self.commit()) + self.assertEqual(result["ratchet"]["delta"], -1) + self.assertEqual(result["checks"]["budget"]["state"], "failure") + + def test_nontext_and_symlink_go_fail_closed(self) -> None: + base = self.prepare() + for content in (b"package example\0", b"\xff", None): + with self.subTest(content=content): + self.git("checkout", "--detach", "-q", base) + path = self.repo / "helper.go" + if content is None: + path.symlink_to("generated.py") + else: + path.write_bytes(content) + result = self.evaluate(base, self.commit()) + self.assertEqual(result["checks"]["budget"]["state"], "failure") + + def test_live_target_snapshot_hash_is_verified(self) -> None: + self.prepare() + self.git("checkout", "-q", "-b", "candidate") + head = self.commit() + self.git("checkout", "-q", "main") + stats = self.repo / ".castiron.stats.yml" + stats.write_text(stats.read_text().replace("codegen_hash: ", "codegen_hash: bad")) + result = self.evaluate(self.commit(), head) + self.assertEqual(result["checks"]["budget"]["state"], "failure") + + def test_activation_must_be_policy_only(self) -> None: + self.prepare() + self.policy(active=False) + base = self.commit() + self.policy() + head = self.commit() + result = self.evaluate(base, head) + self.assertTrue(all(check["state"] == "success" for check in result["checks"].values())) + self.write("helper.go", "package example\n") + result = self.evaluate(base, self.commit()) + self.assertEqual(result["checks"]["isolation"]["state"], "failure") + self.assertEqual(result["checks"]["budget"]["state"], "failure") + + class BudgetTests(GitTestCase): def setUp(self) -> None: # pyright: ignore[reportImplicitOverride] super().setUp() diff --git a/scripts/castiron/test_custom_code_github.py b/scripts/castiron/test_custom_code_github.py index d8cc48bb9..1d970a0ff 100644 --- a/scripts/castiron/test_custom_code_github.py +++ b/scripts/castiron/test_custom_code_github.py @@ -185,6 +185,13 @@ def local_git(repo: Path, *args: str, input_bytes: bytes | None = None) -> bytes {"repository": {"full_name": REPOSITORY}, "workflow_run": api.run}, base, out, + selection={ + "schema_version": 1, + "target_branch": "main", + "base_sha": base, + "head_sha": revision, + "policy_sha": base, + }, ) self.assertEqual((result["base_sha"], result["head_sha"]), (base, revision)) report.publish_comment( @@ -347,7 +354,19 @@ def local_fetch(repo: Path, *args: str, input_bytes: bytes | None = None) -> byt ), ): repo = Path(temp) / "queue.git" - result, _ = budget.github_evaluate(repo, "openai/example", event, base) + result, _ = budget.github_evaluate( + repo, + "openai/example", + event, + base, + selection={ + "schema_version": 1, + "target_branch": "main", + "base_sha": base, + "head_sha": queue_head, + "policy_sha": base, + }, + ) self.assertEqual(result["head_sha"], queue_head) self.assertEqual(result["checks"]["budget"]["state"], "success", result["checks"]) self.assertEqual(result["total"], 1) @@ -355,6 +374,76 @@ def local_fetch(repo: Path, *args: str, input_bytes: bytes | None = None) -> byt [c["state"] for c in result["checks"].values()], ["success", "success"] ) + def test_next_queue_bootstrap_isolated_per_verified_constituent(self) -> None: + generated, _ = self.baseline() + self.git("branch", "codegen/test", generated) + policy = {"schema_version": 1, "max_custom_patch_lines": 10} + self.write(budget.POLICY, json.dumps(policy)) + base = self.commit("unpinned next budget") + self.git("checkout", "-q", "-b", "initialize", base) + self.write( + budget.POLICY, + json.dumps({**policy, "next_policy": {"repository": REPOSITORY, "main_sha": base}}), + ) + pin_head = self.commit("policy-only initialization") + self.git("checkout", "-q", "-b", "sdk", base) + self.write("generated.py", "generated\ncustom\n") + sdk_head = self.commit("unrelated SDK PR") + self.git("checkout", "-q", "-b", "queue", base) + self.git("merge", "--squash", "initialize") + pin_queue = self.commit("synthetic policy queue commit") + self.git("merge", "--squash", "sdk") + queue_head = self.commit("synthetic combined queue commit") + run = source_run(queue_head, "merge_group") + run["head_branch"] = "gh-readonly-queue/next/pr-7-example" + event = {"repository": {"full_name": REPOSITORY}, "workflow_run": run} + original_git = report.git + + def local_fetch(repo: Path, *args: str, input_bytes: bytes | None = None) -> bytes: + if args[0] == "fetch": + args = tuple(str(self.repo) if arg == "origin" else arg for arg in args) + return original_git(repo, *args, input_bytes=input_bytes) + + for entries, state in ( + ([(pin_head, pin_queue), (sdk_head, queue_head)], "success"), + ([(queue_head, queue_head)], "failure"), + ): + with ( + self.subTest(entries=entries), + tempfile.TemporaryDirectory() as temp, + mock.patch.object( + budget.report, + "api", + side_effect=[ + {"default_branch": "main", "private": True}, + run, + {"object": {"sha": base}}, + {"object": {"sha": base}}, + [{"type": "merge_queue"}], + ], + ), + mock.patch.object(budget, "queued_entries", return_value=entries), + mock.patch.object(budget.report, "git", side_effect=local_fetch), + ): + result, _ = budget.github_evaluate( + Path(temp) / "queue.git", + REPOSITORY, + event, + base, + selection={ + "schema_version": 1, + "target_branch": "next", + "base_sha": base, + "head_sha": queue_head, + "policy_sha": base, + "bootstrap": True, + }, + ) + self.assertEqual(result["checks"]["isolation"]["state"], state) + self.assertEqual(result["checks"]["budget"]["state"], "success") + self.assertEqual(result["limit"], 10) + self.assertEqual(result["total"], 1) + def test_next_queue_uses_reviewed_main_policy_after_main_advances(self) -> None: generated, _ = self.baseline() self.git("branch", "codegen/test", generated) @@ -441,6 +530,96 @@ def local_fetch(repo: Path, *args: str, input_bytes: bytes | None = None) -> byt [c["state"] for c in result["checks"].values()], ["success", "success"] ) + def test_next_queue_bootstrap_with_separate_sdk_pr(self) -> None: + generated, _ = self.baseline() + self.git("branch", "codegen/test", generated) + self.write(budget.POLICY, json.dumps({"schema_version": 1, "max_custom_patch_lines": 10})) + approved_policy = self.commit("budget") + self.write( + budget.POLICY, + json.dumps( + { + "schema_version": 1, + "max_custom_patch_lines": 10, + "next_policy": {"repository": "openai/example", "main_sha": approved_policy}, + } + ), + ) + pin_head = self.commit("isolated initialization PR") + base = approved_policy + self.git("checkout", "-q", "-b", "sdk", base) + self.write("generated.py", "generated\ncustom\n") + pr_head = self.commit() + self.git("checkout", "-q", "-b", "queue", base) + self.git("merge", "--squash", "sdk") + self.commit("SDK queue entry") + self.git("merge", "--squash", pin_head) + queue_head = self.commit("synthetic queue commit") + self.assertNotEqual(self.git("merge-base", pr_head, queue_head), pr_head) + policy = approved_policy + run = source_run(queue_head, "merge_group") + run["head_branch"] = "gh-readonly-queue/next/pr-7-example" + event = { + "repository": {"full_name": "openai/example"}, + "workflow_run": run, + } + original_git = report.git + + def local_fetch(repo: Path, *args: str, input_bytes: bytes | None = None) -> bytes: + if args[0] == "fetch": + args = tuple(str(self.repo) if arg == "origin" else arg for arg in args) + return original_git(repo, *args, input_bytes=input_bytes) + + with ( + tempfile.TemporaryDirectory() as temp, + mock.patch.object( + budget.report, + "api", + side_effect=[ + {"default_branch": "main", "private": True}, + run, + {"object": {"sha": policy}}, + {"object": {"sha": base}}, + [{"type": "merge_queue"}], + ], + ), + mock.patch.object( + budget, + "queued_entries", + return_value=[(pin_head, queue_head), (pr_head, queue_head)], + ), + mock.patch.object( + budget.report, + "git", + side_effect=local_fetch, + ), + ): + repo = Path(temp) / "queue.git" + result, _ = budget.github_evaluate( + repo, + "openai/example", + event, + approved_policy, + selection={ + "target_branch": "next", + "base_sha": base, + "head_sha": queue_head, + "policy_sha": approved_policy, + "bootstrap": True, + "schema_version": 1, + }, + ) + self.assertEqual(result["head_sha"], queue_head) + self.assertEqual(result["checks"]["budget"]["state"], "success", result["checks"]) + self.assertEqual(result["limit"], 10) + self.assertEqual(result["base_sha"], base) + self.assertEqual(result["policy_sha"], approved_policy) + self.assertEqual(result["target_branch"], "next") + self.assertEqual(result["total"], 1) + self.assertEqual( + [c["state"] for c in result["checks"].values()], ["success", "success"] + ) + class GitHubBudgetTests(unittest.TestCase): def setUp(self) -> None: # pyright: ignore[reportImplicitOverride] @@ -459,7 +638,46 @@ def evaluate(self, *, trusted_report: Path | None = None) -> tuple[dict[str, Any "repository": {"full_name": REPOSITORY}, "workflow_run": source_run(self.head, self.api.run["event"]), } - return budget.github_evaluate(self.repo, REPOSITORY, event, self.base, trusted_report) + return budget.github_evaluate( + self.repo, + REPOSITORY, + event, + self.base, + trusted_report, + selection={ + "schema_version": 1, + "target_branch": "main", + "base_sha": self.base, + "head_sha": self.head, + "policy_sha": self.base, + }, + ) + + def test_main_rejects_missing_or_retargeted_selection(self) -> None: + for branch in (None, "next"): + selection = ( + None + if branch is None + else { + "schema_version": 1, + "target_branch": branch, + "base_sha": self.base, + "head_sha": self.head, + "policy_sha": self.base, + } + ) + with ( + self.subTest(branch=branch), + self.assertRaisesRegex(ValueError, "policy selection"), + ): + budget.github_evaluate( + self.repo, + REPOSITORY, + {"repository": {"full_name": REPOSITORY}, "workflow_run": self.api.run}, + self.base, + selection=selection, + ) + self.assertFalse(self.repo.exists()) def test_merge_queue_rule_is_required_before_passing(self) -> None: self.api.run = source_run(self.head, "merge_group") @@ -516,7 +734,12 @@ def test_reuses_only_matching_main_job_report(self) -> None: mock.patch.object(budget, "evaluate", return_value=({}, b"")) as evaluate, ): if stale: - with self.assertRaisesRegex(ValueError, "trusted report does not match"): + with self.assertRaisesRegex( + ValueError, + "trusted policy selection" + if stale == "base" + else "trusted report does not match", + ): self.evaluate(trusted_report=self.root) evaluate.assert_not_called() else: diff --git a/scripts/castiron/test_custom_code_policy.py b/scripts/castiron/test_custom_code_policy.py index a4af69177..57aae389c 100644 --- a/scripts/castiron/test_custom_code_policy.py +++ b/scripts/castiron/test_custom_code_policy.py @@ -8,9 +8,10 @@ import sys import tempfile import unittest +import unittest.mock as mock # noqa: PLR0402 - keep unittest imports consistent +from collections.abc import Callable from pathlib import Path -from unittest import mock -from typing import Any, Callable +from typing import Any import custom_code_budget as budget import custom_code_policy as selector @@ -76,6 +77,7 @@ def approved(self, **kwargs: Any) -> dict[str, Any]: "commit_id": self.head, "user": {"type": "User", "login": "reviewer"}, "author_association": "MEMBER", + "body": selector.HUMAN_REVIEW_RULES["attestation_prefix"] + self.head, **kwargs, } @@ -124,6 +126,10 @@ def test_absent_pin_ordinary_change_fails(self) -> None: with self.assertRaisesRegex(ValueError, "initialization"): self.select(base=self.main, head=self.head) + def test_absent_pin_preserves_value_error(self) -> None: + with self.assertRaisesRegex(ValueError, "trusted next base has no reviewed policy pin"): + selector.next_policy_sha(self.repo, REPO, self.main, self.main) + def test_malformed_base_pin_cannot_bootstrap(self) -> None: self.git("checkout", "-q", "--detach", self.main) self.write_policy({**self.policy, "next_policy": {}}) @@ -131,22 +137,36 @@ def test_malformed_base_pin_cannot_bootstrap(self) -> None: with self.assertRaisesRegex(ValueError, "exactly"): self.select(base=base, head=self.base) - def test_bootstrap_requires_exact_policy_only_and_human(self) -> None: + def test_bootstrap_requires_exact_policy_only_with_native_review(self) -> None: self.head = self.base - with mock.patch.object( - selector.report, "api", side_effect=self.review_api([self.approved()]) - ): + with mock.patch.object(selector, "require_human_policy_review") as review: result = self.select(base=self.main) self.assertTrue(result["bootstrap"]) - self.assertEqual(result["policy_reviews"], [{"number": 3, "head_sha": self.head}]) + self.assertEqual(result["policy_reviews"], []) self.assertEqual(result["human_review_rules"], selector.HUMAN_REVIEW_RULES) self.assertEqual(result["policy_sha"], self.main) - with mock.patch.object(selector.report, "api", side_effect=self.review_api([])): - with self.assertRaisesRegex(ValueError, "human"): - self.select(base=self.main) + review.assert_not_called() with self.assertRaisesRegex(ValueError, "policy-only"): self.select(base=self.main, head=self.git("rev-parse", "next")) + def test_queue_bootstrap_isolated_per_constituent(self) -> None: + self.git("checkout", "-q", "--detach", self.main) + (self.repo / "unrelated.txt").write_text("independent SDK change") + sdk_head = self.commit("unrelated PR on unpinned next") + self.git("merge", "--no-ff", "-qm", "queue initialization and SDK", self.base) + group = self.git("rev-parse", "HEAD") + with mock.patch.object(selector, "require_human_policy_review", return_value=3) as review: + result = self.select(base=self.main, head=group, pull_heads=[self.base, sdk_head]) + self.assertTrue(result["bootstrap"]) + self.assertEqual(result["policy_sha"], self.main) + self.assertEqual(result["policy_reviews"], []) + review.assert_not_called() + for heads in (None, [group]): + with self.subTest(heads=heads), self.assertRaisesRegex(ValueError, "policy-only"): + self.select(base=self.main, head=group, pull_heads=heads) + with self.assertRaisesRegex(ValueError, "verified constituent"): + self.select(base=self.main, head=group, pull_heads=[]) + def test_queue_pin_update_isolated_per_constituent(self) -> None: self.git("checkout", "-q", "--detach", self.main) (self.repo / "main-new.txt").write_text("later main") @@ -160,8 +180,8 @@ def test_queue_pin_update_isolated_per_constituent(self) -> None: with mock.patch.object(selector, "require_human_policy_review", return_value=3) as review: result = self.select(head=group, main=later, pull_heads=[pin_head, self.head]) self.assertEqual(result["policy_sha"], self.main) - self.assertEqual(result["policy_reviews"], [{"number": 3, "head_sha": pin_head}]) - review.assert_called_once_with(REPO, pin_head) + self.assertEqual(result["policy_reviews"], []) + review.assert_not_called() # A combined SDK/pin change in one PR is still forbidden. with self.assertRaisesRegex(ValueError, "policy-only"): self.select(head=group, main=later) @@ -195,20 +215,15 @@ def test_queue_stale_sdk_constituent_inherits_protected_pin(self) -> None: self.assertEqual(result["policy_reviews"], []) review.assert_not_called() - def test_stale_actual_pin_edit_still_requires_isolation_and_human(self) -> None: + def test_stale_actual_pin_edit_still_requires_isolation(self) -> None: later, base = self.advanced_pin_base() self.git("checkout", "-q", "--detach", self.base) self.write_policy({**self.policy, "next_policy": {"repository": REPO, "main_sha": later}}) edited = self.commit("PR independently edits pin") with mock.patch.object(selector, "require_human_policy_review", return_value=3) as review: result = self.select(base=base, head=edited, main=later) - self.assertEqual(result["policy_reviews"], [{"number": 3, "head_sha": edited}]) - review.assert_called_once_with(REPO, edited) - with mock.patch.object( - selector, "require_human_policy_review", side_effect=ValueError("human required") - ): - with self.assertRaisesRegex(ValueError, "human"): - self.select(base=base, head=edited, main=later) + self.assertEqual(result["policy_reviews"], []) + review.assert_not_called() (self.repo / "mixed-sdk.txt").write_text("SDK change") mixed = self.commit("mix policy and SDK") with self.assertRaisesRegex(ValueError, "policy-only"): @@ -242,11 +257,15 @@ def test_actual_budget_increase_is_reviewed_even_if_target_increased_further(sel self.write_policy({**self.policy, "next_policy": self.pin, "max_custom_patch_lines": 5000}) edited = self.commit("PR actually raises budget") for heads in (None, [edited]): - with mock.patch.object( - selector, "require_human_policy_review", side_effect=ValueError("human required") + with ( + mock.patch.object( + selector, + "require_human_policy_review", + side_effect=ValueError("human required"), + ), + self.assertRaisesRegex(ValueError, "human"), ): - with self.assertRaisesRegex(ValueError, "human"): - self.select(base=base, head=edited, pull_heads=heads) + self.select(base=base, head=edited, pull_heads=heads) (self.repo / "mixed-sdk.txt").write_text("SDK change") mixed = self.commit("mix SDK and actual budget increase") with self.assertRaisesRegex(ValueError, "policy-only"): @@ -289,9 +308,20 @@ def test_selector_outputs_transport_review_receipt_as_json(self) -> None: self.assertEqual(json.loads(out.read_text())["schema_version"], 1) def test_forged_bot_stale_dismissed_untrusted_reviews_rejected(self) -> None: - self.head = self.base + self.git("checkout", "-q", "--detach", self.base) + self.write_policy( + { + **self.policy, + "next_policy": self.pin, + "max_custom_patch_lines": self.policy["max_custom_patch_lines"] + 1, + } + ) + self.head = self.commit("bootstrap pin with budget increase") cases = [ [], + [self.approved(body="")], + [self.approved(body="Looks good to me")], + [self.approved(body=selector.HUMAN_REVIEW_RULES["attestation_prefix"] + "f" * 40)], [self.approved(state="COMMENTED", body="APPROVED by a human")], [self.approved(user={"type": "Bot", "login": "reviewer[bot]"})], [self.approved(commit_id="a" * 40)], @@ -318,9 +348,45 @@ def test_forged_bot_stale_dismissed_untrusted_reviews_rejected(self) -> None: with ( self.subTest(reviews=reviews), mock.patch.object(selector.report, "api", side_effect=self.review_api(reviews)), + self.assertRaisesRegex(ValueError, "human"), + ): + self.select(base=self.main) + + def test_bootstrap_budget_increase_keeps_human_proof_for_pr_and_queue(self) -> None: + self.git("checkout", "-q", "--detach", self.base) + self.write_policy( + { + **self.policy, + "next_policy": self.pin, + "max_custom_patch_lines": self.policy["max_custom_patch_lines"] + 1, + } + ) + self.head = self.commit("bootstrap pin with budget increase") + for heads in (None, [self.head]): + with ( + mock.patch.object(selector.report, "api", side_effect=self.review_api([])), + self.assertRaisesRegex(ValueError, "human"), ): - with self.assertRaisesRegex(ValueError, "human"): - self.select(base=self.main) + self.select(base=self.main, pull_heads=heads) + with mock.patch.object( + selector.report, "api", side_effect=self.review_api([self.approved()]) + ): + result = self.select(base=self.main, pull_heads=heads) + self.assertEqual(result["policy_reviews"], [{"number": 3, "head_sha": self.head}]) + + def test_authorless_review_does_not_block_valid_approval(self) -> None: + self.git("checkout", "-q", "--detach", self.base) + policy = selector.read_policy(self.repo, self.base) + self.write_policy( + {**policy, "max_custom_patch_lines": policy["max_custom_patch_lines"] + 1} + ) + self.head = self.commit("increase budget") + reviews = [self.approved(id=9, user=None), self.approved()] + with mock.patch.object(selector.report, "api", side_effect=self.review_api(reviews)): + self.assertEqual( + self.select()["policy_reviews"], + [{"number": 3, "head_sha": self.head}], + ) def test_budget_increase_without_pin_change_requires_human(self) -> None: self.git("checkout", "-q", "--detach", self.base) @@ -330,9 +396,11 @@ def test_budget_increase_without_pin_change_requires_human(self) -> None: ) self.head = self.commit("increase budget without changing pin") for reviews in ([], [self.approved(body="Automated source review.")]): - with mock.patch.object(selector.report, "api", side_effect=self.review_api(reviews)): - with self.assertRaisesRegex(ValueError, "human"): - self.select() + with ( + mock.patch.object(selector.report, "api", side_effect=self.review_api(reviews)), + self.assertRaisesRegex(ValueError, "human"), + ): + self.select() with mock.patch.object( selector.report, "api", side_effect=self.review_api([self.approved()]) ): @@ -343,6 +411,25 @@ def test_changed_authenticated_main_ref_rejected(self) -> None: with self.assertRaisesRegex(ValueError, "changed"): self.select() + def test_go_ratchet_activation_requires_human_for_pr_and_queue(self) -> None: + self.git("checkout", "-q", "--detach", self.base) + self.write_policy({**selector.read_policy(self.repo, self.base), "ratchet": "go"}) + self.head = self.commit("activate Go ratchet without changing pin or ceiling") + for pull_heads in (None, [self.head]): + for reviews in ([], [self.approved(body="Automated source review.")]): + with ( + self.subTest(pull_heads=pull_heads, reviews=reviews), + mock.patch.object(selector.report, "api", side_effect=self.review_api(reviews)), + self.assertRaisesRegex(ValueError, "human"), + ): + self.select(pull_heads=pull_heads) + with mock.patch.object( + selector.report, "api", side_effect=self.review_api([self.approved()]) + ): + selected = self.select(pull_heads=pull_heads) + self.assertEqual(selected["policy_sha"], self.main) + self.assertEqual(selected["policy_reviews"], [{"number": 3, "head_sha": self.head}]) + def test_wrong_origin_rejected(self) -> None: self.git("remote", "set-url", "origin", "https://github.com/attacker/example.git") with self.assertRaisesRegex(ValueError, "origin"): @@ -360,9 +447,164 @@ def test_policy_update_uses_old_base_checker_and_requires_human(self) -> None: selector.report, "api", side_effect=self.review_api([self.approved()]) ): self.assertEqual(self.select(main=newer)["policy_sha"], self.main) - with mock.patch.object(selector.report, "api", side_effect=self.review_api([])): - with self.assertRaisesRegex(ValueError, "human"): - self.select(main=newer) + with ( + mock.patch.object(selector.report, "api", side_effect=self.review_api([])), + self.assertRaisesRegex(ValueError, "human"), + ): + self.select(main=newer) + + def test_pin_effective_policy_review_for_pr_and_queue(self) -> None: + cases = ( + ({**self.policy, "max_custom_patch_lines": 4000}, False), + ({**self.policy, "max_custom_patch_lines": 3999}, False), + ({**self.policy, "max_custom_patch_lines": 5000}, True), + ({**self.policy, "ratchet": "go"}, True), + ({"schema_version": 2, "max_custom_patch_lines": 0, "generated_files": "strict"}, True), + ) + for proposed, requires_human in cases: + self.git("checkout", "-q", "--detach", self.main) + self.write_policy(proposed) + newer = self.commit("adopt proposed effective policy") + self.git("update-ref", "refs/remotes/origin/main", newer) + for bootstrap in (False, True): + base = self.main if bootstrap else self.base + self.git("checkout", "-q", "--detach", base) + self.write_policy( + {**self.policy, "next_policy": {"repository": REPO, "main_sha": newer}} + ) + self.head = self.commit("request pin without changing local budget") + for heads in (None, [self.head]): + with self.subTest(proposed=proposed, bootstrap=bootstrap, heads=heads): + with mock.patch.object( + selector, + "require_human_policy_review", + side_effect=ValueError("human review missing"), + ) as review: + if requires_human: + with self.assertRaisesRegex(ValueError, "human"): + self.select(base=base, main=newer, pull_heads=heads) + review.assert_called_once_with(REPO, self.head) + else: + result = self.select(base=base, main=newer, pull_heads=heads) + review.assert_not_called() + self.assertEqual(result["policy_reviews"], []) + if requires_human: + with mock.patch.object( + selector.report, + "api", + side_effect=self.review_api([self.approved()]), + ): + result = self.select(base=base, main=newer, pull_heads=heads) + self.assertEqual( + result["policy_reviews"], [{"number": 3, "head_sha": self.head}] + ) + + def test_rollback_pin_keeps_human_proof_for_pr_and_queue(self) -> None: + self.git("checkout", "-q", "--detach", self.main) + (self.repo / "checker-fix.txt").write_text("reviewed checker fix") + newer = self.commit("newer main checker with unchanged policy") + self.git("update-ref", "refs/remotes/origin/main", newer) + self.git("checkout", "-q", "--detach", self.base) + self.write_policy({**self.policy, "next_policy": {"repository": REPO, "main_sha": newer}}) + advanced_base = self.commit("adopt newer checker") + self.write_policy({**self.policy, "next_policy": self.pin}) + self.head = self.commit("roll back checker without changing budget") + for heads in (None, [self.head]): + with ( + mock.patch.object(selector.report, "api", side_effect=self.review_api([])), + self.assertRaisesRegex(ValueError, "human"), + ): + self.select(base=advanced_base, main=newer, pull_heads=heads) + with mock.patch.object( + selector.report, "api", side_effect=self.review_api([self.approved()]) + ): + result = self.select(base=advanced_base, main=newer, pull_heads=heads) + self.assertEqual(result["policy_sha"], newer) + self.assertEqual(result["policy_reviews"], [{"number": 3, "head_sha": self.head}]) + + def test_stale_pin_compares_current_protected_policy_for_pr_and_queue(self) -> None: + for protected_policy in ( + self.policy, + {**self.policy, "max_custom_patch_lines": 3999}, + {"schema_version": 2, "max_custom_patch_lines": 0, "generated_files": "strict"}, + ): + self.git("checkout", "-q", "--detach", self.main) + (self.repo / "checker-fix.txt").write_text("intermediate checker") + intermediate = self.commit("intermediate main checker") + self.write_policy(protected_policy) + (self.repo / "checker-fix.txt").write_text("protected checker") + current = self.commit("current main checker") + (self.repo / "checker-fix.txt").write_text("forward checker") + forward = self.commit("forward main checker") + self.git("update-ref", "refs/remotes/origin/main", forward) + self.git("checkout", "-q", "--detach", self.base) + self.write_policy( + {**self.policy, "next_policy": {"repository": REPO, "main_sha": current}} + ) + protected_base = self.commit("advance protected next pin") + for proposed in (intermediate, current, forward): + self.git("checkout", "-q", "--detach", self.base) + self.write_policy( + {**self.policy, "next_policy": {"repository": REPO, "main_sha": proposed}} + ) + self.head = self.commit("stale policy-only PR") + for heads in (None, [self.head]): + with self.subTest(policy=protected_policy, proposed=proposed, heads=heads): + with mock.patch.object( + selector.report, "api", side_effect=self.review_api([]) + ): + if proposed == intermediate: + with self.assertRaisesRegex(ValueError, "human"): + self.select(base=protected_base, main=forward, pull_heads=heads) + else: + result = self.select( + base=protected_base, main=forward, pull_heads=heads + ) + self.assertEqual(result["policy_reviews"], []) + with mock.patch.object( + selector.report, "api", side_effect=self.review_api([self.approved()]) + ): + result = self.select( + base=protected_base, main=forward, pull_heads=heads + ) + self.assertEqual(result["policy_sha"], current) + self.assertEqual( + result["policy_reviews"], + [{"number": 3, "head_sha": self.head}] + if proposed == intermediate + else [], + ) + + def test_initial_older_pin_keeps_human_proof_for_pr_and_queue(self) -> None: + self.git("checkout", "-q", "--detach", self.main) + (self.repo / "checker-fix.txt").write_text("reviewed bootstrap checker fix") + newer = self.commit("newer main checker with unchanged policy") + self.git("update-ref", "refs/remotes/origin/main", newer) + for pin_sha in (self.main, newer): + self.git("checkout", "-q", "--detach", self.main) + self.write_policy( + {**self.policy, "next_policy": {"repository": REPO, "main_sha": pin_sha}} + ) + self.head = self.commit("initial checker pin") + for heads in (None, [self.head]): + with mock.patch.object(selector.report, "api", side_effect=self.review_api([])): + if pin_sha == self.main: + with self.assertRaisesRegex(ValueError, "human"): + self.select(base=self.main, main=newer, pull_heads=heads) + else: + result = self.select(base=self.main, main=newer, pull_heads=heads) + self.assertEqual(result["policy_reviews"], []) + self.assertEqual(result["policy_sha"], newer) + with mock.patch.object( + selector.report, "api", side_effect=self.review_api([self.approved()]) + ): + result = self.select(base=self.main, main=newer, pull_heads=heads) + self.assertEqual(result["policy_sha"], newer) + self.assertTrue(result["bootstrap"]) + self.assertEqual( + result["policy_reviews"], + [{"number": 3, "head_sha": self.head}] if pin_sha == self.main else [], + ) def test_incapable_policy_is_not_executed(self) -> None: self.git("checkout", "-q", "--detach", self.main) @@ -394,15 +636,16 @@ def setUp(self) -> None: # pyright: ignore[reportImplicitOverride] self.temp = tempfile.TemporaryDirectory() self.addCleanup(self.temp.cleanup) self.repo = Path(self.temp.name) / "objects" - self.workflow_run: dict[str, Any] = dict( - id=42, - repository={"full_name": REPO}, - head_sha="a" * 40, - status="completed", - event="pull_request", - run_attempt=3, - path=CANONICAL_POLICY_WORKFLOW + "@refs/pull/12/merge", - ) + self.workflow_run: dict[str, Any] = { + "id": 42, + "workflow_id": 456, + "repository": {"full_name": REPO}, + "head_sha": "a" * 40, + "status": "completed", + "event": "pull_request", + "run_attempt": 3, + "path": CANONICAL_POLICY_WORKFLOW + "@refs/pull/12/merge", + } self.event: dict[str, Any] = { "repository": {"full_name": REPO}, "workflow_run": copy.deepcopy(self.workflow_run), @@ -438,6 +681,14 @@ def api(self, method: str, path: str, *args: Any) -> Any: return { POLICY_SOURCE_ROOT: {"default_branch": "main"}, POLICY_SOURCE_ROOT + "/actions/runs/42": self.workflow_run, + POLICY_SOURCE_ROOT + "/actions/workflows/castiron-custom-code.yml": { + "id": 456, + "path": CANONICAL_POLICY_WORKFLOW, + }, + POLICY_SOURCE_ROOT + "/actions/workflows/ci.yml": { + "id": 456, + "path": CI_POLICY_WORKFLOW, + }, POLICY_SOURCE_ROOT + "/git/ref/heads/main": {"object": {"sha": "c" * 40}}, POLICY_SOURCE_ROOT + "/git/ref/heads/next": {"object": {"sha": "b" * 40}}, }[path] @@ -448,6 +699,13 @@ def call(self, **kwargs: Any) -> dict[str, Any]: def use_ci(self) -> None: self.workflow_run["path"] = CI_POLICY_WORKFLOW + "@refs/pull/12/merge" + def test_lookalike_workflow_cannot_select_policy(self) -> None: + self.workflow_run["path"] = CANONICAL_POLICY_WORKFLOW + "@evil.yml" + self.workflow_run["workflow_id"] = 789 + with self.assertRaisesRegex(ValueError, "canonical workflow"): + self.call() + self.select.assert_not_called() + def test_default_canonical_unchanged(self) -> None: self.branch = "main" self.assertEqual(self.call(), {"selected": True}) diff --git a/scripts/castiron/test_custom_code_publication.py b/scripts/castiron/test_custom_code_publication.py index c0e2e877c..190a9db69 100644 --- a/scripts/castiron/test_custom_code_publication.py +++ b/scripts/castiron/test_custom_code_publication.py @@ -7,6 +7,7 @@ import json import shutil import subprocess +import tempfile import unittest from pathlib import Path from typing import Any, cast @@ -29,9 +30,13 @@ def run_publisher(step: str, payload: dict[str, Any]) -> list[dict[str, Any]]: if line and not line.startswith(" "): break lines.append(line[12:]) + return run_workflow_script("\n".join(lines), payload) + + +def run_workflow_script(script: str, payload: dict[str, Any]) -> list[dict[str, Any]]: output = subprocess.run( ["node", str(Path(__file__).with_name("fixtures") / "github_publisher.cjs")], - input=json.dumps({**payload, "script": "\n".join(lines)}), + input=json.dumps({**payload, "script": script}), text=True, capture_output=True, check=True, @@ -40,6 +45,63 @@ def run_publisher(step: str, payload: dict[str, Any]) -> list[dict[str, Any]]: class WorkflowTests(unittest.TestCase): + @unittest.skipUnless(shutil.which("node"), "GitHub Actions JavaScript runtime") + def test_workflow_path_filters_accept_refs_without_accepting_other_files(self) -> None: + compute = WORKFLOW.read_text().split("\n compute:\n", 1)[1] + condition = next( + line.removeprefix(" if: ") + for line in compute.splitlines() + if line.startswith(" if: ") + ) + expected_path = ".github/workflows/castiron-custom-code.yml" + cases = [ + (expected_path, True), + (expected_path + "@main", True), + (expected_path + "@refs/heads/next", True), + (expected_path + "@refs/pull/42/merge", True), + (".github/workflows/other.yml@main", False), + (expected_path + ".bak@main", False), + (expected_path + "/other@main", False), + ("other/" + expected_path + "@main", False), + ("", False), + ] + head = "a" * 40 + pull = { + "number": 3, + "state": "open", + "head": {"sha": head}, + "base": {"ref": "next", "repo": {"full_name": "openai/example"}}, + } + for path, accepted in cases: + for event in ("pull_request", "merge_group", "push"): + with self.subTest(path=path, event=event): + run = {**source_run(head), "id": 20, "path": path, "event": event} + payload = { + "context": { + "payload": {"workflow_run": run}, + "runId": 20, + "repo": {"owner": "openai", "repo": "example"}, + "serverUrl": "https://github.com", + }, + "current": pull, + } + # Evaluate the emitted job condition with its Actions builtins. + script = ( + "const fromJSON = JSON.parse;\n" + "const contains = (items, value) => items.includes(value);\n" + "const startsWith = (value, prefix) => value.startsWith(prefix);\n" + "github.event = context.payload;\n" + f"if ({condition}) await github.rest.issues.createComment({{}});" + ) + self.assertEqual( + bool(run_workflow_script(script, payload)), + accepted and event in ("pull_request", "merge_group"), + ) + self.assertEqual( + bool(run_publisher("Publish a trusted failure status", payload)), + accepted and event == "pull_request", + ) + def test_candidate_tests_cannot_modify_credentialed_reporter_runner(self) -> None: producer = (WORKFLOW.parent / "castiron-custom-code.yml").read_text() tests, reporter = producer.split("\n report:\n", 1) @@ -57,7 +119,12 @@ def test_candidate_tests_cannot_modify_credentialed_reporter_runner(self) -> Non def test_trusted_failure_publisher_updates_one_current_comment(self) -> None: head = "a" * 40 run = {**source_run(head), "id": 20} - current = {"state": "open", "head": {"sha": head}} + current = { + "number": 3, + "state": "open", + "head": {"sha": head}, + "base": {"ref": "main", "repo": {"full_name": "openai/example"}}, + } previous = { "id": 42, "user": {"type": "Bot", "login": "github-actions[bot]"}, @@ -98,6 +165,50 @@ def test_trusted_failure_publisher_updates_one_current_comment(self) -> None: self.assertIn("Report unavailable", writes[0]["body"]) self.assertIn("castiron:run:v1:20:1", writes[0]["body"]) + @unittest.skipUnless(shutil.which("node"), "GitHub Actions JavaScript runtime") + def test_failure_comments_require_one_supported_target(self) -> None: + head = "a" * 40 + + def pull(number: int, branch: str = "main", repo: str = "openai/example") -> dict[str, Any]: + return { + "number": number, + "state": "open", + "head": {"sha": head}, + "base": {"ref": branch, "repo": {"full_name": repo}}, + } + + cases: list[tuple[str, list[dict[str, Any]], list[int]]] = [ + ("retargeted", [pull(3, "release")], []), + ("other repository", [pull(3, repo="other/example")], []), + ("ambiguous main", [pull(3), pull(4)], []), + ("ambiguous next", [pull(3, "next"), pull(4, "next")], []), + ("prefer main", [pull(3, "next"), pull(4)], [4]), + ("one next", [pull(3, "next")], [3]), + ("ignore unsupported", [pull(3, "release"), pull(4)], [4]), + ] + for name, pulls, expected in cases: + for fallback in (False, True): + with self.subTest(name=name, fallback=fallback): + run = { + **source_run(head), + "id": 20, + "pull_requests": [] if fallback else pulls, + } + writes = run_publisher( + "Publish a trusted failure status", + { + "context": { + "payload": {"workflow_run": run}, + "runId": 20, + "repo": {"owner": "openai", "repo": "example"}, + "serverUrl": "https://github.com", + }, + "pulls_by_number": {str(pr["number"]): pr for pr in pulls}, + "fallback_pulls": pulls, + }, + ) + self.assertEqual([write["issue_number"] for write in writes], expected) + def test_workflow_reports_all_branches_without_write_credentials(self) -> None: workflows = Path(__file__).resolve().parents[2] / ".github/workflows" producer = (workflows / "castiron-custom-code.yml").read_text() @@ -345,8 +456,11 @@ def publish( same_head_next_pr: bool = False, policy_reviews: list[dict[str, Any]] | None = None, reviews: list[dict[str, Any]] | None = None, + review_nodes: list[dict[str, Any]] | None = None, + review_pages: list[dict[str, Any]] | None = None, policy_pull: dict[str, Any] | None = None, missing_review_contract: bool = False, + review_contract: dict[str, Any] | None = None, ) -> list[dict[str, Any]]: base, head = "a" * 40, "b" * 40 run = source_run(head, event_name) @@ -354,6 +468,23 @@ def publish( run["head_branch"] = f"gh-readonly-queue/{branch}/pr-7-example" payload: dict[str, Any] = { "reviews": reviews or [], + "review_nodes": review_nodes + if review_nodes is not None + else [ + { + "fullDatabaseId": str(review["id"]), + "state": review["state"], + "body": review.get("body"), + "commit": {"oid": review.get("commit_id")}, + "author": { + "login": review["user"]["login"], + "__typename": review["user"]["type"], + }, + "authorAssociation": review.get("author_association"), + } + for review in reviews or [] + ], + "review_pages": review_pages, "main_sha": "d" * 40 if policy_changed else (policy_sha or ("c" * 40 if branch == "main" and base_changed else base)), @@ -386,7 +517,9 @@ def publish( "POLICY_REVIEWS": json.dumps(policy_reviews or []), "HUMAN_REVIEW_RULES": "" if missing_review_contract - else json.dumps(policy.HUMAN_REVIEW_RULES), + else json.dumps( + policy.HUMAN_REVIEW_RULES if review_contract is None else review_contract + ), "SELECTED_BRANCH": branch, "SELECTED_BASE_SHA": base, "SELECTED_HEAD_SHA": head, @@ -418,7 +551,7 @@ def publish( ] return run_publisher("Publish exact-head statuses after checking freshness", payload) - def test_live_human_review_predicate_matches_selector(self): + def test_live_human_review_predicate_matches_selector(self) -> None: head = "b" * 40 approved = { "id": 10, @@ -426,7 +559,7 @@ def test_live_human_review_predicate_matches_selector(self): "commit_id": head, "user": {"login": "human", "type": "User"}, "author_association": "MEMBER", - "body": "", + "body": policy.HUMAN_REVIEW_RULES["attestation_prefix"] + head, } cases = [ [approved], @@ -440,6 +573,10 @@ def test_live_human_review_predicate_matches_selector(self): [{**approved, "author_association": "NONE"}], ] for body in [ + "", + "Looks good to me", + policy.HUMAN_REVIEW_RULES["attestation_prefix"] + "f" * 40, + "> " + policy.HUMAN_REVIEW_RULES["attestation_prefix"] + head, "", "Automated source review.", "Source-only review; I did not execute candidate code or workflows.", @@ -457,7 +594,81 @@ def test_live_human_review_predicate_matches_selector(self): ) self.assertEqual([w["state"] for w in writes], [expected, expected]) - def test_queue_policy_approval_dismissal_after_selection_fails(self): + def test_final_publisher_preserves_review_identity_precision(self) -> None: + head = "b" * 40 + approval = { + "id": 9007199254740992, + "state": "APPROVED", + "commit_id": head, + "user": {"login": "human", "type": "User"}, + "author_association": "MEMBER", + "body": policy.HUMAN_REVIEW_RULES["attestation_prefix"] + head, + } + replacement = {**approval, "id": 9007199254740993, "state": "COMMENTED"} + for reviews in ([approval, replacement], [replacement, approval]): + with self.subTest(reviews=reviews): + writes = self.publish( + branch="next", + policy_reviews=[{"number": 3, "head_sha": head}], + reviews=reviews, + ) + self.assertEqual([w["state"] for w in writes], ["failure"] * 2) + writes = self.publish( + branch="next", + policy_reviews=[{"number": 3, "head_sha": head}], + reviews=[{**replacement, "state": "APPROVED"}, {**approval, "state": "DISMISSED"}], + ) + self.assertEqual([w["state"] for w in writes], ["success"] * 2) + + def test_final_publisher_review_pages_and_malformed_identity(self) -> None: + head = "b" * 40 + approval = { + "fullDatabaseId": "9007199254740992", + "state": "APPROVED", + "commit": {"oid": head}, + "author": {"login": "human", "__typename": "User"}, + "authorAssociation": "MEMBER", + "body": policy.HUMAN_REVIEW_RULES["attestation_prefix"] + head, + } + + def publish(**kwargs: Any) -> list[str]: + return [ + w["state"] + for w in self.publish( + branch="next", + policy_reviews=[{"number": 3, "head_sha": head}], + **kwargs, + ) + ] + + self.assertEqual(publish(review_nodes=[approval]), ["success"] * 2) + for identity in (None, "", "1.5", "-1", "0", 9007199254740992): + with self.subTest(identity=identity): + self.assertEqual( + publish(review_nodes=[{**approval, "fullDatabaseId": identity}]), + ["failure"] * 2, + ) + pages: list[dict[str, Any]] = [ + { + "expected_cursor": None, + "nodes": [approval], + "pageInfo": {"hasNextPage": True, "endCursor": "page2"}, + }, + { + "expected_cursor": "page2", + "nodes": [{**approval, "fullDatabaseId": "9007199254740993", "state": "DISMISSED"}], + "pageInfo": {"hasNextPage": False, "endCursor": None}, + }, + ] + self.assertEqual(publish(review_pages=pages), ["failure"] * 2) + self.assertEqual( + publish( + review_pages=[{**pages[0], "pageInfo": {"hasNextPage": True, "endCursor": None}}] + ), + ["failure"] * 2, + ) + + def test_queue_policy_approval_dismissal_after_selection_fails(self) -> None: candidate = "e" * 40 pull: dict[str, Any] = { "number": 7, @@ -472,7 +683,7 @@ def test_queue_policy_approval_dismissal_after_selection_fails(self): "commit_id": candidate, "user": {"login": "human", "type": "User"}, "author_association": "MEMBER", - "body": "", + "body": policy.HUMAN_REVIEW_RULES["attestation_prefix"] + candidate, } for state in ("APPROVED", "DISMISSED", "CHANGES_REQUESTED"): with self.subTest(state=state): @@ -506,7 +717,7 @@ def test_queue_policy_approval_dismissal_after_selection_fails(self): ) self.assertEqual([w["state"] for w in writes], ["failure", "failure"]) - def test_queue_policy_constituent_changes_fail_closed(self): + def test_queue_policy_constituent_changes_fail_closed(self) -> None: candidate = "e" * 40 pull: dict[str, Any] = { "number": 7, @@ -521,7 +732,7 @@ def test_queue_policy_constituent_changes_fail_closed(self): "commit_id": candidate, "user": {"login": "human", "type": "User"}, "author_association": "MEMBER", - "body": "", + "body": policy.HUMAN_REVIEW_RULES["attestation_prefix"] + candidate, } for changed in [ {**pull, "state": "closed"}, @@ -538,12 +749,26 @@ def test_queue_policy_constituent_changes_fail_closed(self): ) self.assertEqual([w["state"] for w in writes], ["failure", "failure"]) - def test_missing_policy_review_contract_fails_closed(self): + def test_missing_policy_review_contract_fails_closed(self) -> None: self.assertEqual( [w["state"] for w in self.publish(branch="next", missing_review_contract=True)], ["failure", "failure"], ) + def test_legacy_review_contract_fails_closed_only_on_next(self) -> None: + legacy = { + key: value + for key, value in policy.HUMAN_REVIEW_RULES.items() + if key != "attestation_prefix" + } + legacy["version"] = 1 + for branch, expected in (("next", "failure"), ("main", "success")): + with self.subTest(branch=branch): + self.assertEqual( + [w["state"] for w in self.publish(branch=branch, review_contract=legacy)], + [expected, expected], + ) + def test_statuses_attach_to_candidate_not_main(self) -> None: for event in ("pull_request", "merge_group"): with self.subTest(event=event): @@ -692,6 +917,22 @@ def test_missing_evaluation_publishes_actionable_failures(self) -> None: self.assertIn("inspect the trusted run", result["description"]) self.assertTrue(result["target_url"].endswith("/actions/runs/123")) + def test_main_rejects_mismatched_selector_receipt(self) -> None: + results = self.publish(selected_policy_matches=False) + self.assertTrue(results) + self.assertTrue(all(result["state"] != "success" for result in results)) + + def test_lookalike_workflow_cannot_publish(self) -> None: + self.assertEqual( + self.publish( + run_overrides={ + "path": ".github/workflows/castiron-custom-code.yml@evil.yml", + "workflow_id": 789, + } + ), + [], + ) + def test_superseded_or_wrong_source_run_cannot_publish(self) -> None: for overrides in ( {"run_attempt": 2}, @@ -743,5 +984,629 @@ def test_publication_guard_allows_current_retry_and_ignores_unrelated_statuses(s self.assertTrue(all(len(r["description"]) <= 140 for r in results)) +@unittest.skipUnless( + shutil.which("node"), "Node is needed to execute the review-revocation fixture" +) +class ReviewRevocationTests(unittest.TestCase): + def payload(self) -> dict[str, Any]: + head, base, queue = "a" * 40, "b" * 40, "c" * 40 + pull = { + "number": 3, + "user": {"login": "author"}, + "state": "open", + "head": {"sha": head}, + "base": {"ref": "next", "sha": base, "repo": {"full_name": "openai/example"}}, + } + run = { + **source_run(head), + "id": 20, + "actor": {"login": "outsider", "type": "User"}, + "event": "pull_request_review", + "status": "completed", + "pull_requests": [{"number": 3, "head": {"sha": head}}], + } + + def original(sha: str, event: str, identity: int) -> dict[str, Any]: + return {**run, "head_sha": sha, "id": identity, "event": event} + + return { + "run": run, + "policies_by_sha": { + base: {"schema_version": 1, "max_custom_patch_lines": 0}, + head: {"schema_version": 1, "max_custom_patch_lines": 1}, + }, + "previous_statuses": [ + { + "context": name, + "state": "success", + "creator": {"login": "github-actions[bot]"}, + "target_url": "https://github.com/openai/example/actions/runs/19", + } + for name in ( + "Castiron next / budget-only change", + "Castiron next / custom-code budget", + ) + ], + "current": pull, + "queue_nodes": [ + {"headCommit": {"oid": queue}, "pullRequest": {"number": 3, "headRefOid": head}} + ], + "runs_by_sha": { + head: [original(head, "pull_request", 12)], + queue: [original(queue, "merge_group", 14)], + }, + "context": { + "payload": {"workflow_run": dict(run)}, + "runId": 21, + "repo": {"owner": "openai", "repo": "example"}, + "serverUrl": "https://github.com", + }, + } + + def invoke(self, payload: dict[str, Any]) -> list[dict[str, Any]]: + return run_publisher("Re-evaluate current PR and queue after review changes", payload) + + def test_revokes_pr_and_queue_before_requesting_normal_re_evaluation(self) -> None: + result = self.invoke(self.payload()) + self.assertEqual([item.get("state") for item in result[:4]], ["pending"] * 4) + self.assertEqual({item["sha"] for item in result[:4]}, {"a" * 40, "c" * 40}) + self.assertTrue(all(item["context"].startswith("Castiron next /") for item in result[:4])) + self.assertEqual([item["run_id"] for item in result[4:]], [12, 14]) + self.assertNotIn("success", [item.get("state") for item in result]) + + def approval(self) -> dict[str, Any]: + return { + "fullDatabaseId": "101", + "state": "APPROVED", + "body": policy.HUMAN_REVIEW_RULES["attestation_prefix"] + "a" * 40, + "author": {"login": "reviewer", "__typename": "User"}, + "authorAssociation": "MEMBER", + "commit": {"oid": "a" * 40}, + "updatedAt": "2026-10-22T01:00:00Z", + } + + def recovery(self) -> dict[str, Any]: + payload = self.payload() + payload["review_nodes"] = [self.approval()] + payload["run"]["actor"] = {"login": "reviewer", "type": "User"} + for status in payload["previous_statuses"]: + status["state"] = "failure" + return payload + + def history(self, payload: dict[str, Any], writes: list[dict[str, Any]]) -> None: + payload["statuses_by_sha"] = { + sha: [ + {**item, "creator": {"login": "github-actions[bot]"}} + for item in writes + if item.get("sha") == sha + ] + for sha in ("a" * 40, "c" * 40) + } + payload["runs_by_id"] = { + "21": { + "id": 21, + "workflow_id": 789, + "status": "completed", + "conclusion": "success", + "repository": {"full_name": "openai/example"}, + "event": "workflow_run", + } + } + + def test_unrelated_comment_cannot_invalidate_valid_approval_or_request_recovery(self) -> None: + for state in ("success", "failure", "pending"): + payload = self.recovery() + payload["run"]["actor"]["login"] = "outsider" + payload["review_nodes"].append( + { + **self.approval(), + "fullDatabaseId": "102", + "author": {"login": "outsider", "__typename": "User"}, + "state": "COMMENTED", + "body": "hello", + } + ) + for status in payload["previous_statuses"]: + status["state"] = state + payload["forbid_queue"] = True + self.assertEqual(self.invoke(payload), []) + + def test_ineligible_review_cannot_request_recovery(self) -> None: + for overrides in ( + {"state": "COMMENTED"}, + {"body": ""}, + {"authorAssociation": "CONTRIBUTOR"}, + {"commit": {"oid": "b" * 40}}, + {"author": {"login": "author", "__typename": "User"}}, + {"author": {"login": "reviewer", "__typename": "Bot"}}, + ): + with self.subTest(overrides=overrides): + payload = self.recovery() + payload["review_nodes"][0].update(overrides) + self.assertEqual(self.invoke(payload), []) + + def test_repeated_invalid_signals_do_not_rerun_already_revoked_heads(self) -> None: + payload = self.payload() + writes = self.invoke(payload) + self.history(payload, writes) + self.assertEqual(self.invoke(payload), []) + for statuses in payload["statuses_by_sha"].values(): + for status in statuses: + status["state"] = "failure" + self.assertEqual(self.invoke(payload), []) + + def test_approval_recovery_coalesces_and_edited_restoration_recovers_again(self) -> None: + payload = self.recovery() + writes = self.invoke(payload) + self.assertEqual(len(writes), 6) + self.assertTrue(all(len(item["description"]) <= 140 for item in writes[:4])) + self.history(payload, writes) + self.assertEqual(self.invoke(payload), []) + payload["review_nodes"][0]["updatedAt"] = "2026-10-22T01:02:00Z" + restored = self.invoke(payload) + self.assertEqual(len(restored), 6) + self.assertNotEqual(writes[0]["description"], restored[0]["description"]) + + def test_new_revocation_supersedes_old_recovery_even_with_equal_edit_timestamp(self) -> None: + payload = self.recovery() + approved = self.invoke(payload) + self.history(payload, approved) + for statuses in payload["statuses_by_sha"].values(): + statuses[:0] = [ + {**status, "state": "success", "description": "Successful evaluation"} + for status in statuses + ] + payload["review_nodes"][0]["body"] = "" + revoked = self.invoke(payload) + self.assertEqual(len(revoked), 6) + self.history(payload, revoked) + for sha, statuses in payload["statuses_by_sha"].items(): + statuses.extend( + {**item, "creator": {"login": "github-actions[bot]"}} + for item in approved + if item.get("sha") == sha + ) + payload["review_nodes"][0] = ( + self.approval() + ) # Same ID, body and second-precision update time. + self.assertEqual(len(self.invoke(payload)), 6) + + def test_full_review_id_handles_current_and_beyond_safe_integer_ids(self) -> None: + payload = self.recovery() + payload["review_nodes"][0].update(databaseId=None, fullDatabaseId="5614624241") + self.assertEqual(len(self.invoke(payload)), 6) + payload = self.payload() + payload["review_nodes"] = [ + {**self.approval(), "databaseId": None, "fullDatabaseId": "9007199254740992"}, + { + **self.approval(), + "databaseId": None, + "fullDatabaseId": "9007199254740993", + "state": "COMMENTED", + }, + ] + self.assertEqual(len(self.invoke(payload)), 6) + payload["review_nodes"][0]["fullDatabaseId"] = 9007199254740992 + with self.assertRaises(subprocess.CalledProcessError): + self.invoke(payload) + + def test_publisher_failure_supersedes_recovery_without_observed_revocation(self) -> None: + payload = self.recovery() + approved = self.invoke(payload) + self.history(payload, approved) + for statuses in payload["statuses_by_sha"].values(): + statuses[:0] = [ + {**status, "state": "failure", "description": "Approval missing during publication"} + for status in statuses + ] + # Publisher observed invalid body; review jobs only see its same-second restoration. + # No new review identity/timestamp and no review-handler revocation marker exists. + recovered = self.invoke(payload) + self.assertEqual(len(recovered), 6) + self.history(payload, recovered) + self.assertEqual(self.invoke(payload), []) + + def test_deleted_marker_writer_is_untrusted_but_service_errors_propagate(self) -> None: + payload = self.recovery() + approved = self.invoke(payload) + self.history(payload, approved) + payload["run_errors"] = {"21": 404} + self.assertEqual(len(self.invoke(payload)), 6) + for status in (403, 429, 500): + payload["run_errors"]["21"] = status + with self.subTest(status=status), self.assertRaises(subprocess.CalledProcessError): + self.invoke(payload) + + def test_failed_partial_dispatch_is_retried_before_signals_coalesce(self) -> None: + for authorized in (False, True): + for conclusion in ("failure", "cancelled", "timed_out"): + payload = self.recovery() if authorized else self.payload() + payload["fail_rerun"] = True + # The fixture persists actual pending writes before the simulated API error. + with tempfile.TemporaryDirectory() as directory: + capture = Path(directory) / "pending.json" + payload["capture_writes"] = str(capture) + with self.assertRaises(subprocess.CalledProcessError): + self.invoke(payload) + pending = json.loads(capture.read_text()) + self.assertEqual(len(pending), 4) + self.history(payload, pending) + payload.pop("capture_writes") + payload.pop("fail_rerun") + payload["runs_by_id"]["21"]["conclusion"] = conclusion + payload["context"]["runId"] = 22 + retried = self.invoke(payload) + self.assertEqual([item["run_id"] for item in retried if "run_id" in item], [12, 14]) + self.history(payload, retried) + payload["runs_by_id"]["22"] = {**payload["runs_by_id"]["21"], "id": 22} + self.assertEqual(self.invoke(payload), []) + + def test_untrusted_marker_cannot_suppress_recovery(self) -> None: + for field, value in ( + ("workflow_id", 999), + ("event", "pull_request"), + ("repository", {"full_name": "attacker/example"}), + ): + payload = self.recovery() + self.history(payload, self.invoke(payload)) + payload["runs_by_id"]["21"][field] = value + self.assertEqual(len(self.invoke(payload)), 6) + + def test_dismissal_edit_away_and_later_comment_revoke_even_for_other_signal_actor(self) -> None: + for changed in ( + {"state": "DISMISSED"}, + {"body": ""}, + {"state": "COMMENTED", "fullDatabaseId": "102"}, + ): + payload = self.payload() + payload["review_nodes"] = [{**self.approval(), **changed}] + self.assertEqual(len(self.invoke(payload)), 6) + payload = self.payload() + payload["review_pages"] = [ + { + "expected_cursor": None, + "nodes": [{**self.approval(), "fullDatabaseId": "102", "state": "COMMENTED"}], + "pageInfo": {"hasNextPage": True, "endCursor": "more"}, + }, + { + "expected_cursor": "more", + "nodes": [self.approval()], + "pageInfo": {"hasNextPage": False}, + }, + ] + self.assertEqual(len(self.invoke(payload)), 6) + + def test_missing_authoritative_edit_time_rejects_recovery(self) -> None: + payload = self.recovery() + payload["review_nodes"][0]["updatedAt"] = None + with self.assertRaises(subprocess.CalledProcessError): + self.invoke(payload) + + def test_review_signal_requires_canonical_workflow_identity_before_writes(self) -> None: + for identity in (None, "456", 999): + for path in ( + ".github/workflows/castiron-custom-code.yml", + ".github/workflows/castiron-custom-code.yml@evil.yml", + ): + with self.subTest(identity=identity, path=path): + payload = self.payload() + payload["run"].update({"workflow_id": identity, "path": path}) + payload["context"]["payload"]["workflow_run"] = dict(payload["run"]) + self.assertEqual(self.invoke(payload), []) + + def test_rejects_forged_or_superseded_review_signals(self) -> None: + for field, value in [ + ("path", ".github/workflows/other.yml"), + ("run_attempt", 9), + ("head_sha", "d" * 40), + ("event", "pull_request"), + ]: + with self.subTest(field=field): + payload = self.payload() + payload["run"][field] = value + with self.assertRaises(subprocess.CalledProcessError): + self.invoke(payload) + payload = self.payload() + payload["run"]["repository"] = {"full_name": "attacker/example"} + with self.assertRaises(subprocess.CalledProcessError): + self.invoke(payload) + + def test_head_change_keeps_observed_statuses_revoked_without_stale_reruns(self) -> None: + payload = self.payload() + changed = {**payload["current"], "head": {"sha": "d" * 40}} + payload["current_sequence"] = [payload["current"], changed] + payload["current"] = changed + result = self.invoke(payload) + self.assertEqual(len(result), 4) + self.assertTrue(all(item["state"] == "pending" for item in result)) + self.assertNotIn("d" * 40, {item["sha"] for item in result}) + + def test_queue_change_retries_and_invalidates_new_snapshot(self) -> None: + payload = self.payload() + old = list(payload["queue_nodes"]) + payload["queue_nodes"].append( + { + "headCommit": {"oid": "d" * 40}, + "pullRequest": {"number": 4, "headRefOid": "e" * 40}, + } + ) + payload["runs_by_sha"]["d" * 40] = [ + { + **payload["runs_by_sha"]["c" * 40][0], + "head_sha": "d" * 40, + "id": 16, + } + ] + payload["queue_pages"] = [ + { + "expected_cursor": None, + "nodes": old, + "pageInfo": {"hasNextPage": False}, + } + ] + result = self.invoke(payload) + pending = [item for item in result if item.get("state") == "pending"] + self.assertEqual({item["sha"] for item in pending}, {"a" * 40, "c" * 40, "d" * 40}) + self.assertEqual([item["run_id"] for item in result if "run_id" in item], [12, 14, 16]) + self.assertTrue(all(item.get("state") == "pending" for item in result[:-3])) + + def test_empty_review_pr_list_uses_authenticated_fork_discovery(self) -> None: + for associated in ([], [{"number": 3}]): + payload = self.payload() + payload["run"].update( + { + "pull_requests": [], + "head_repository": {"owner": {"login": "contributor"}}, + "head_branch": "sdk", + } + ) + payload["associated_pulls"] = associated + payload["fallback_pulls"] = [{"number": 3}] + self.assertEqual(len(self.invoke(payload)), 6) + payload["current"]["head"]["sha"] = "d" * 40 + payload["merge_sha"] = "e" * 40 + payload["merge_commit"] = {"parents": []} + self.assertEqual(self.invoke(payload), []) + + def test_shared_head_revokes_the_run_source_pr(self) -> None: + payload = self.payload() + payload["run"]["pull_requests"] = [] + payload["run"]["head_repository"]["id"] = 101 + payload["current"]["head"].update(ref="sdk", repo={"id": 101}) + payload["associated_pulls"] = [{"number": number} for number in (3, 4, 5)] + payload["pulls_by_number"] = { + str(number): { + **payload["current"], + "number": number, + "head": {**payload["current"]["head"], **change}, + } + for number, change in ((4, {"ref": "other"}), (5, {"repo": {"id": 102}})) + } + result = self.invoke(payload) + self.assertEqual([item.get("state") for item in result[:4]], ["pending"] * 4) + self.assertEqual({item["sha"] for item in result[:4]}, {"a" * 40, "c" * 40}) + + def test_branch_only_merge_signal_requires_current_merge_and_base(self) -> None: + payload = self.payload() + payload["run"]["pull_requests"] = [] + payload["run"]["head_sha"] = payload["context"]["payload"]["workflow_run"]["head_sha"] = ( + "e" * 40 + ) + payload["fallback_pulls"] = [{"number": 3}] + payload["merge_commit"] = {"parents": [{"sha": "b" * 40}, {"sha": "a" * 40}]} + payload["merge_sha"] = "f" * 40 + self.assertEqual(self.invoke(payload), []) + + payload["merge_sha"] = "e" * 40 + self.assertEqual(len(self.invoke(payload)), 6) + payload["current"]["base"]["sha"] = "d" * 40 + payload["comparisons"] = { + f"{'d' * 40}...{'a' * 40}": {"merge_base_commit": {"sha": "b" * 40}} + } + self.assertEqual(self.invoke(payload), []) + + def test_merge_sha_signal_survives_base_advance_and_rejects_wrong_parents(self) -> None: + payload = self.payload() + payload["run"]["head_sha"] = payload["context"]["payload"]["workflow_run"]["head_sha"] = ( + "e" * 40 + ) + payload["current"]["base"]["sha"] = "d" * 40 + payload["comparisons"] = { + f"{'d' * 40}...{'a' * 40}": {"merge_base_commit": {"sha": "b" * 40}} + } + payload["merge_commit"] = {"parents": [{"sha": "b" * 40}, {"sha": "a" * 40}]} + self.assertEqual(len(self.invoke(payload)), 6) + payload["comparisons"][f"{'b' * 40}...{'d' * 40}"] = { + "merge_base_commit": {"sha": "f" * 40} + } + self.assertEqual(self.invoke(payload), []) + payload["merge_commit"]["parents"].reverse() + self.assertEqual(self.invoke(payload), []) + + def test_policy_changes_without_human_attestation_do_not_invalidate_statuses(self) -> None: + cases = ( + ( + {"schema_version": 1, "max_custom_patch_lines": 2}, + {"schema_version": 1, "max_custom_patch_lines": 1}, + ), + ( + {"schema_version": 1, "max_custom_patch_lines": 0}, + {"schema_version": 2, "max_custom_patch_lines": 0, "generated_files": "strict"}, + ), + ) + for base_policy, head_policy in cases: + with self.subTest(base_policy=base_policy, head_policy=head_policy): + payload = self.payload() + payload["policies_by_sha"] = {"b" * 40: base_policy, "a" * 40: head_policy} + self.assertEqual(self.invoke(payload), []) + + def test_go_ratchet_activation_invalidates_statuses(self) -> None: + payload = self.payload() + payload["policies_by_sha"]["a" * 40] = { + **payload["policies_by_sha"]["b" * 40], + "ratchet": "go", + } + self.assertEqual(len(self.invoke(payload)), 6) + + def test_policy_pin_reviews_follow_effective_protected_target(self) -> None: + historical_checker, target_checker, proposed_checker = "e" * 40, "f" * 40, "0" * 40 + target = "d" * 40 + + def pinned(checker: str) -> dict[str, Any]: + return { + "schema_version": 1, + "max_custom_patch_lines": 0, + "next_policy": {"repository": "openai/example", "main_sha": checker}, + } + + payload = self.payload() + payload["current"]["base"]["sha"] = target + payload["main_sha"] = proposed_checker + payload["comparisons"] = { + f"{target}...{'a' * 40}": {"merge_base_commit": {"sha": "b" * 40}} + } + payload["policies_by_sha"] = { + "a" * 40: pinned(proposed_checker), + "b" * 40: pinned(historical_checker), + target: pinned(target_checker), + historical_checker: {"schema_version": 1, "max_custom_patch_lines": 10}, + target_checker: {"schema_version": 1, "max_custom_patch_lines": 5}, + proposed_checker: {"schema_version": 1, "max_custom_patch_lines": 5}, + } + self.assertEqual(self.invoke(payload), []) + payload["policies_by_sha"][proposed_checker]["max_custom_patch_lines"] = 7 + self.assertEqual(len(self.invoke(payload)), 6) + payload["policies_by_sha"][proposed_checker]["max_custom_patch_lines"] = 5 + payload["comparisons"][f"{target_checker}...{proposed_checker}"] = { + "merge_base_commit": {"sha": proposed_checker} + } + self.assertEqual(len(self.invoke(payload)), 6) + + def test_initial_pin_at_current_main_keeps_statuses(self) -> None: + payload = self.payload() + checker = "f" * 40 + payload["main_sha"] = checker + payload["policies_by_sha"][checker] = payload["policies_by_sha"]["b" * 40] + payload["policies_by_sha"]["a" * 40] = { + **payload["policies_by_sha"]["b" * 40], + "next_policy": {"repository": "openai/example", "main_sha": checker}, + } + self.assertEqual(self.invoke(payload), []) + advanced_main = "e" * 40 + payload["main_sha_sequence"] = [checker, advanced_main] + payload["main_sha"] = advanced_main + payload["comparisons"] = { + f"{advanced_main}...{checker}": {"merge_base_commit": {"sha": checker}} + } + self.assertEqual(len(self.invoke(payload)), 6) + + def test_revoked_go_ratchet_activation_invalidates_pr_and_queue_statuses(self) -> None: + for changed in ({"state": "DISMISSED"}, {"body": ""}): + with self.subTest(changed=changed): + payload = self.payload() + payload["policies_by_sha"]["a" * 40] = { + **payload["policies_by_sha"]["b" * 40], + "ratchet": "go", + } + payload["review_nodes"] = [{**self.approval(), **changed}] + writes = self.invoke(payload) + self.assertCountEqual( + [(item["sha"], item["state"]) for item in writes if "state" in item], + [("a" * 40, "pending")] * 2 + [("c" * 40, "pending")] * 2, + ) + self.assertCountEqual( + [item["run_id"] for item in writes if "run_id" in item], [12, 14] + ) + + def test_closed_or_superseded_review_is_a_no_op(self) -> None: + for changed in ({"state": "closed"}, {"head": {"sha": "f" * 40}}): + payload = self.payload() + payload["current"].update(changed) + self.assertEqual(self.invoke(payload), []) + + def test_null_queue_association_does_not_block_revocation(self) -> None: + payload = self.payload() + payload["queue_nodes"].insert(0, {"headCommit": {"oid": "f" * 40}, "pullRequest": None}) + result = self.invoke(payload) + self.assertEqual(len(result), 6) + self.assertNotIn("f" * 40, {item.get("sha") for item in result}) + payload["queue_nodes"] = payload["queue_nodes"][:1] + self.assertEqual(len(self.invoke(payload)), 3) + + def test_rerun_requires_the_reviewed_pr_not_just_shared_head(self) -> None: + payload = self.payload() + correct = payload["runs_by_sha"]["a" * 40][0] + main = { + **payload["current"], + "number": 4, + "base": {**payload["current"]["base"], "ref": "main"}, + } + payload["pulls_by_number"] = {"4": main} + other: dict[str, Any] = { + **correct, + "id": 30, + "pull_requests": [{"number": 4, "head": {"sha": "a" * 40}}], + } + payload["runs_by_sha"]["a" * 40].append(other) + result = self.invoke(payload) + self.assertEqual([item["run_id"] for item in result if "run_id" in item], [12, 14]) + # Empty fork associations may rerun only if canonical discovery selects this next PR. + fork: dict[str, Any] = { + **correct, + "pull_requests": [], + "head_repository": {"owner": {"login": "contributor"}}, + "head_branch": "sdk", + } + payload["fallback_pulls"] = [{"number": 3}] + payload["runs_by_sha"]["a" * 40] = [fork, other] + result = self.invoke(payload) + self.assertEqual([item["run_id"] for item in result if "run_id" in item], [12, 14]) + association_cases: tuple[list[dict[str, Any]], ...] = ( + [], + [*correct["pull_requests"], *other["pull_requests"]], + ) + for associations in association_cases: + payload["associated_pulls"] = [{"number": 3}, {"number": 4}] + payload["runs_by_sha"]["a" * 40] = [{**fork, "pull_requests": associations}, other] + result = self.invoke(payload) + self.assertEqual([item["run_id"] for item in result if "run_id" in item], [14]) + self.assertTrue(all(item["state"] == "pending" for item in result[:4])) + + def test_unprepared_queue_entry_still_revokes_pr_statuses(self) -> None: + payload = self.payload() + payload["queue_nodes"][0]["headCommit"] = None + result = self.invoke(payload) + self.assertEqual([item.get("state") for item in result], ["pending", "pending", None]) + self.assertEqual(result[-1]["run_id"], 12) + + def test_unrelated_review_does_not_restart_reporter(self) -> None: + payload = self.payload() + payload["files"] = [{"filename": "src/client.ts"}] + self.assertEqual(self.invoke(payload), []) + + def test_no_success_or_foreign_run_can_be_published(self) -> None: + payload = self.payload() + payload["runs_by_sha"]["a" * 40][0]["repository"] = {"full_name": "other/repo"} + result = self.invoke(payload) + self.assertEqual( + [item["run_id"] for item in result if item.get("operation") == "rerun"], [14] + ) + self.assertTrue(all(item.get("state") != "success" for item in result)) + + def test_signal_is_unprivileged_and_writers_serialize(self) -> None: + producer = (WORKFLOW.parent / "castiron-custom-code.yml").read_text() + self.assertIn("pull_request_review:\n types: [submitted, edited, dismissed]", producer) + signal = producer.split(" queue-signal:\n", 1)[1].split(" candidate-tests:\n", 1)[0] + self.assertIn("permissions: {}", signal) + self.assertNotIn("checkout@", signal) + handler = WORKFLOW.read_text() + revoke = handler.split(" review-change:\n", 1)[1].split(" compute:\n", 1)[0] + self.assertNotIn("checkout@", revoke) + self.assertNotIn("download-artifact", revoke) + self.assertEqual( + handler.count("group: castiron-budget-writes-${{ github.repository_id }}"), 2 + ) + + if __name__ == "__main__": unittest.main() diff --git a/scripts/castiron/test_custom_code_report.py b/scripts/castiron/test_custom_code_report.py index c523765a1..6c42233e2 100644 --- a/scripts/castiron/test_custom_code_report.py +++ b/scripts/castiron/test_custom_code_report.py @@ -3,11 +3,14 @@ import base64 import hashlib +import io import json import os import struct import subprocess import unittest +from pathlib import Path +from typing import Any from unittest import mock import custom_code_report as report @@ -1080,5 +1083,491 @@ def test_public_snapshot_has_no_private_history_and_reports_without_private_remo self.assertEqual(merged_report["counts"]["newly_customized"], 1) +ADOPTION_GENERATION = "550e8400-e29b-41d4-a716-446655440001" + + +class FirstAdoptionTests(GitTestCase): + def setUp(self) -> None: # pyright: ignore[reportImplicitOverride] + super().setUp() + self.repository = "openai/openai-java" + self.write(".stats.yml", "configured_endpoints: 278\n") + self.root = self.commit("legacy root") + self.git("remote", "add", "origin", "https://github.com/openai/openai-java.git") + self.policy: dict[str, Any] = {"schema_version": 1, "max_custom_patch_lines": 3000} + self.write(".castiron-ratchet.json", json.dumps(self.policy) + "\n") + self.g0 = self.generated("source\n", GENERATION, False) + self.public0 = report.create_public_snapshot( + self.repo, + self.git("rev-parse", f"{self.g0}^{{tree}}"), + GENERATION, + report.hash_codegen_commit(self.repo, self.g0), + "codegen/main", + None, + ) + self.git("update-ref", "refs/remotes/origin/codegen/main", self.public0) + self.write("generated.py", "source\n") + self.stats(self.g0, self.public0, GENERATION) + self.main = self.commit("approved main") + self.git("update-ref", "refs/remotes/origin/main", self.main) + self.git("checkout", "--detach", self.root) + self.pin: dict[str, Any] = { + **self.policy, + "next_policy": {"repository": self.repository, "main_sha": self.main}, + } + self.write(".castiron-ratchet.json", json.dumps(self.pin) + "\n") + self.base = self.commit("separate approved next policy") + self.git("update-ref", "refs/remotes/origin/next", self.base) + + def generated(self, text: str, generation: str, next_target: bool) -> str: + blob = ( + report.git(self.repo, "hash-object", "-w", "--stdin", input_bytes=text.encode()) + .decode() + .strip() + ) + tree = ( + report.git( + self.repo, + "mktree", + input_bytes=f"100644 blob {blob}\tgenerated.py\n".encode(), + ) + .decode() + .strip() + ) + record: dict[str, Any] = { + "generation_id": generation, + "source_branch": "test", + "target": self.repository.split("/")[1], + "language": self.repository.split("-")[-1], + } + if next_target: + record["public_baseline_commit"] = self.base + record = { + "schema_version": "castiron-checkpoint/v2", + "release_target": "next", + "record": record, + } + encoded = base64.b64encode(json.dumps(record).encode()).decode() + commit = ( + report.git( + self.repo, + "commit-tree", + tree, + input_bytes=f"codegen\n\nGeneration metadata: {encoded}\n".encode(), + ) + .decode() + .strip() + ) + namespace = "castiron/next/" if next_target else "" + self.git("update-ref", f"refs/remotes/origin/codegen/{namespace}test", commit) + return commit + + def stats(self, private: str, public: str, generation: str) -> None: + self.write( + ".castiron.stats.yml", + f"schema_version: 1\ngeneration_id: {generation}\ncodegen_sha: {private}\npublic_codegen_sha: {public}\ncodegen_hash: {report.hash_codegen_commit(self.repo, private)}\nconfig_hash: source-config\nopenapi_spec_hash: source-spec\n", + ) + + def seed(self) -> str: + # Synthetic two-parent private seed; production worker initialization is + # covered separately in the SDK worker candidate-flow suite. + self.git("read-tree", "--reset", "-u", self.main) + self.write(".castiron-ratchet.json", json.dumps(self.pin) + "\n") + self.git("add", "-A") + return self.git( + "commit-tree", + self.git("write-tree"), + "-p", + self.base, + "-p", + self.main, + "-m", + "approved main adoption fixture", + ) + + def head(self, *, public_receipt: bool = True, customize: bool = False) -> str: + seed = self.seed() + self.git("checkout", "--detach", seed) + self.g1 = self.generated("next generated\n", ADOPTION_GENERATION, True) + snap = report.create_public_snapshot( + self.repo, + self.git("rev-parse", f"{self.g1}^{{tree}}"), + ADOPTION_GENERATION, + report.hash_codegen_commit(self.repo, self.g1), + "codegen/castiron/next/new", + self.public0, + (self.base, self.main) if public_receipt else None, + ) + self.git("update-ref", "refs/remotes/origin/codegen/castiron/next/new", snap) + self.write( + "generated.py", + "next generated\n" + ("# customization\n" if customize else ""), + ) + self.stats(self.g1, snap, ADOPTION_GENERATION) + return self.commit("next candidate") + + def test_java_public_and_private_report_have_no_fake_before_generation(self) -> None: + head = self.head(customize=True) + for public in (False, True): + result, patch = report.build_report( + self.repo, + self.base, + head, + public=public, + target_branch="next", + require_head_hash=True, + ) + self.assertIsNone(result["before"]) + self.assertEqual(result["adoption_baseline"]["generation_id"], GENERATION) + self.assertEqual(result["files"][0]["added"], "1") + self.assertIn(b"# customization", patch) + self.assertIn("Legacy before-generation unavailable", report.render_report(result)) + + def test_cli_uses_same_protected_pin_path(self) -> None: + self.repository = "openai/openai-cli" + self.git("remote", "set-url", "origin", "https://github.com/openai/openai-cli.git") + self.pin["next_policy"]["repository"] = self.repository + self.write(".castiron-ratchet.json", json.dumps(self.pin) + "\n") + self.base = self.commit("CLI approved policy fixture") + head = self.head() + result, _ = report.build_report( + self.repo, + self.base, + head, + public=False, + target_branch="next", + require_head_hash=True, + ) + self.assertIsNone(result["before"]) + + def test_promotion_base_cli_recognizes_exact_next_branch(self) -> None: + original = report.git + + def offline_git(repo: Path, *args: str, input_bytes: bytes | None = None) -> bytes: + if args[0] == "fetch": + source, destination = args[-1].lstrip("+").split(":", 1) + return original( + repo, + "update-ref", + destination, + source.replace("refs/heads/", "refs/remotes/origin/"), + ) + return original(repo, *args, input_bytes=input_bytes) + + for base, branch, expected in ( + (self.base, "castiron/next", self.main), + (self.base, "castiron/next/child", self.main), + (self.base, "castiron/nextish", None), + (self.base, "castiron/next-door", None), + (self.base, "castiron/promotions/main", None), + (self.main, "castiron/promotions/main", self.main), + ): + with self.subTest(base=base, branch=branch): + output, errors = io.StringIO(), io.StringIO() + with ( + mock.patch.object(report, "git", side_effect=offline_git), + mock.patch.object( + report.sys, + "argv", + [ + "custom_code_report.py", + "promotion-base", + "--repo", + str(self.repo), + "--base", + base, + "--branch", + branch, + ], + ), + mock.patch.object(report.sys, "stdout", output), + mock.patch.object(report.sys, "stderr", errors), + ): + result = report.main() + if expected is None: + self.assertEqual(result, 1) + self.assertEqual(output.getvalue(), "") + self.assertIn("first adoption is next-only", errors.getvalue()) + else: + self.assertEqual(result, 0, errors.getvalue()) + self.assertEqual(output.getvalue(), expected + "\n") + + def test_promotion_prepares_new_snapshot_from_real_main_snapshot(self) -> None: + head = self.head() + original = report.git + + def offline_git(repo: Path, *args: str, input_bytes: bytes | None = None) -> bytes: + if args[0] == "fetch": + for arg in args: + if arg.startswith("+refs/") and ":" in arg: + source, destination = arg.lstrip("+").split(":", 1) + original( + repo, + "update-ref", + destination, + source.replace("refs/heads/", "refs/remotes/origin/"), + ) + return b"" + if args[0] == "ls-remote": + return b"" + return original(repo, *args, input_bytes=input_bytes) + + with mock.patch.object(report, "git", side_effect=offline_git): + prepared = report.prepare_public_snapshot( + self.repo, self.base, head, self.repo, self.base, "castiron/next/fresh" + ) + metadata = report.public_metadata(self.repo, prepared["commit"]) + self.assertEqual(metadata["Target"], "next") + self.assertEqual(metadata["Adoption-Base"], self.base) + self.assertEqual(metadata["Adoption-Source"], self.main) + self.assertEqual(prepared["base_commit"], self.public0) + self.assertEqual(self.git("rev-parse", prepared["commit"] + "^"), self.public0) + + def test_missing_receipt_rejected(self) -> None: + head = self.head(public_receipt=False) + with self.assertRaisesRegex(report.ReportError, "authorized first adoption"): + report.build_report(self.repo, self.base, head, public=True, target_branch="next") + + def test_wrong_sdk_record_rejected(self) -> None: + head = self.head() + self.repository = "openai/openai-cli" + wrong = self.generated("next generated\n", ADOPTION_GENERATION, True) + self.stats( + wrong, report.read_stats(self.repo, head)["public_codegen_sha"], ADOPTION_GENERATION + ) + head = self.commit("wrong SDK binding") + with self.assertRaisesRegex(report.ReportError, "SDK mismatch"): + report.build_report(self.repo, self.base, head, public=False, target_branch="next") + + def test_config_change_rejected(self) -> None: + self.head() + path = self.repo / ".castiron.stats.yml" + path.write_text(path.read_text().replace("source-config", "wrong-config")) + head = self.commit() + with self.assertRaisesRegex(report.ReportError, "authenticated main inputs"): + report.build_report(self.repo, self.base, head, public=False, target_branch="next") + + def test_existing_corrupt_deleted_stats_rejected(self) -> None: + self.write(".castiron.stats.yml", "corrupt: true\n") + corrupt = self.commit() + with self.assertRaisesRegex(report.ReportError, "existing/corrupt"): + report.first_adoption_source(self.repo, corrupt, target_branch="next", fetch=False) + self.git("rm", ".castiron.stats.yml") + deleted = self.commit() + with self.assertRaisesRegex(report.ReportError, "deleted"): + report.first_adoption_source(self.repo, deleted, target_branch="next", fetch=False) + + def test_candidate_cannot_select_source_or_remove_policy(self) -> None: + self.head() + self.write(".castiron-ratchet.json", json.dumps(self.policy) + "\n") + head = self.commit() + with self.assertRaisesRegex(report.ReportError, "protected target policy"): + report.build_report(self.repo, self.base, head, public=True, target_branch="next") + + def test_current_count_and_strict_zero_still_enforced(self) -> None: + import custom_code_budget as budget + + head = self.head(customize=True) + measurement = report.build_report( + self.repo, self.base, head, public=True, target_branch="next" + ) + evaluated, _ = budget.evaluate( + self.repo, + self.base, + head, + public=True, + policy_sha=self.main, + target_branch="next", + measurement=measurement, + ) + self.assertEqual(evaluated["total"], 1) + self.assertEqual(evaluated["checks"]["budget"]["state"], "success", evaluated) + self.write( + ".castiron-ratchet.json", + '{"schema_version":2,"max_custom_patch_lines":0,"generated_files":"strict"}\n', + ) + strict = self.commit() + evaluated, _ = budget.evaluate( + self.repo, + self.base, + head, + public=True, + policy_sha=strict, + target_branch="next", + measurement=measurement, + ) + self.assertEqual(evaluated["limit"], 0) + self.assertEqual(evaluated["checks"]["budget"]["state"], "failure") + self.assertIn("manifest", evaluated["checks"]["budget"]["description"]) + + def test_unapproved_source_pin_rejected(self) -> None: + self.pin["next_policy"]["main_sha"] = self.base + self.write(".castiron-ratchet.json", json.dumps(self.pin) + "\n") + invalid = self.commit() + with self.assertRaisesRegex(report.ReportError, "authenticated main history"): + report.first_adoption_source(self.repo, invalid, target_branch="next", fetch=False) + + def test_advanced_target_requires_fresh_adoption(self) -> None: + head = self.head() + self.git("checkout", "--detach", self.base) + self.write("legacy-note.txt", "another approved target change\n") + advanced = self.commit() + with self.assertRaisesRegex(report.ReportError, "target advanced"): + report.build_report(self.repo, advanced, head, public=True, target_branch="next") + + def test_main_target_rejected(self) -> None: + with self.assertRaisesRegex(report.ReportError, "next-only"): + report.first_adoption_source(self.repo, self.base, target_branch="main", fetch=False) + + def test_normal_public_main_report_unchanged(self) -> None: + self.git("checkout", "--detach", self.main) + self.write("generated.py", "source\n# customization\n") + head = self.commit() + result, _ = report.build_report( + self.repo, self.main, head, public=True, target_branch="main" + ) + self.assertIsNotNone(result["before"]) + self.assertEqual(result["before_status"], "verified") + + def promoted_public_head(self) -> tuple[Path, str, dict[str, Any]]: + # Exercise the separate public checkout and the actual single-parent + # commit-tree shape in sdk/promote.rs, not the private merge seed. + import subprocess + + destination = self.repo / "public-checkout" + head = self.head(customize=True) + subprocess.run( + ["git", "clone", "-q", "--no-hardlinks", str(self.repo), str(destination)], check=True + ) + + def public_git(*args: str) -> str: + return report.git(destination, *args).decode().strip() + + public_git("config", "user.name", "Castiron test") + public_git("config", "user.email", "castiron@example.test") + public_git("remote", "set-url", "origin", "https://github.com/" + self.repository + ".git") + for ref, commit in [ + ("main", self.main), + ("next", self.base), + ("codegen/main", self.public0), + ]: + public_git("update-ref", "refs/remotes/origin/" + ref, commit) + public_git("checkout", "--detach", self.base) + # Normal promotion applies the reviewed private patch to this index. + public_git("read-tree", "--reset", "-u", head) + original = report.git + + def offline_git(repo: Path, *args: str, input_bytes: bytes | None = None) -> bytes: + if args[0] == "fetch": + for arg in args: + if arg.startswith("+refs/") and ":" in arg: + source, target = arg.lstrip("+").split(":", 1) + original( + repo, + "update-ref", + target, + source.replace("refs/heads/", "refs/remotes/origin/"), + ) + return b"" + if args[0] == "ls-remote": + return b"" + return original(repo, *args, input_bytes=input_bytes) + + with mock.patch.object(report, "git", side_effect=offline_git): + prepared = report.prepare_public_snapshot( + self.repo, self.base, head, destination, self.base, "castiron/next/fresh" + ) + public_git("update-ref", "refs/remotes/origin/" + prepared["branch"], prepared["commit"]) + promoted = public_git( + "commit-tree", + public_git("write-tree"), + "-p", + self.base, + "-m", + "normal public promotion", + ) + self.assertEqual( + public_git("rev-list", "--parents", "-n", "1", promoted), promoted + " " + self.base + ) + with self.assertRaises(report.ReportError): + public_git("merge-base", "--is-ancestor", self.main, promoted) + # Private authorization still retains both parents. + self.git("merge-base", "--is-ancestor", self.main, head) + self.git("merge-base", "--is-ancestor", self.base, head) + return destination, promoted, prepared + + def test_normal_single_parent_public_promotion_reports_first_adoption(self) -> None: + destination, promoted, _ = self.promoted_public_head() + result, patch = report.build_report( + destination, + self.base, + promoted, + public=True, + target_branch="next", + require_head_hash=True, + ) + self.assertIsNone(result["before"]) + self.assertEqual(result["adoption_baseline"]["generation_id"], GENERATION) + self.assertIn(b"# customization", patch) + + def test_public_adoption_rejects_disconnected_snapshot_with_valid_receipt(self) -> None: + destination, promoted, prepared = self.promoted_public_head() + # The right receipt/hash alone does not authenticate approved-main lineage. + detached = report.create_public_snapshot( + destination, + report.git(destination, "rev-parse", prepared["commit"] + "^{tree}").decode().strip(), + ADOPTION_GENERATION, + prepared["hash"], + prepared["branch"], + None, + (self.base, self.main), + ) + report.git(destination, "update-ref", "refs/remotes/origin/" + prepared["branch"], detached) + report.git(destination, "checkout", "--detach", promoted) + stats = destination / ".castiron.stats.yml" + stats.write_text(stats.read_text().replace(prepared["commit"], detached)) + report.git(destination, "add", ".castiron.stats.yml") + invalid = ( + report.git( + destination, + "commit-tree", + report.git(destination, "write-tree").decode().strip(), + "-p", + self.base, + "-m", + "disconnected snapshot", + ) + .decode() + .strip() + ) + with self.assertRaises(report.ReportError): + report.build_report( + destination, + self.base, + invalid, + public=True, + target_branch="next", + require_head_hash=True, + ) + + def test_cli_normal_single_parent_public_promotion_reports_first_adoption(self) -> None: + self.repository = "openai/openai-cli" + self.git("remote", "set-url", "origin", "https://github.com/openai/openai-cli.git") + self.pin["next_policy"]["repository"] = self.repository + self.write(".castiron-ratchet.json", json.dumps(self.pin) + "\n") + self.base = self.commit("CLI approved next policy") + self.git("update-ref", "refs/remotes/origin/next", self.base) + destination, promoted, _ = self.promoted_public_head() + result, _ = report.build_report( + destination, + self.base, + promoted, + public=True, + target_branch="next", + require_head_hash=True, + ) + self.assertIsNone(result["before"]) + self.assertEqual(result["adoption_baseline"]["generation_id"], GENERATION) + + if __name__ == "__main__": unittest.main()