From 30e2709bf7faf455c7d5726e0bf2890df8f2b489 Mon Sep 17 00:00:00 2001 From: Li SiYi Date: Fri, 6 Nov 2026 02:21:48 +0800 Subject: [PATCH 1/3] ci: unify the risk gate on the canonical implementation The reconciled verifier merged here fed its own check and workflow-run events back into the evidence it rereads, so every evaluation ended in error and both governance repositories were closed to merges. Restore the canonical gate the other 23 repositories run, keep the shadow tests that cover the reusable observer, and drop the gate tests that asserted the removed discovery matrix. --- .github/workflows/governance-shadow-tests.yml | 2 +- .github/workflows/risk-gate.yml | 656 ++++++++++-------- AGENTS.md | 2 +- PROPOSAL.md | 7 + scripts/test-risk-gate.cjs | 372 ---------- 5 files changed, 391 insertions(+), 648 deletions(-) delete mode 100644 scripts/test-risk-gate.cjs diff --git a/.github/workflows/governance-shadow-tests.yml b/.github/workflows/governance-shadow-tests.yml index 2dc23c7..2057b1a 100644 --- a/.github/workflows/governance-shadow-tests.yml +++ b/.github/workflows/governance-shadow-tests.yml @@ -14,4 +14,4 @@ jobs: - uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6 with: persist-credentials: false - - run: node --test scripts/test-governance-shadow.cjs scripts/test-risk-gate.cjs + - run: node --test scripts/test-governance-shadow.cjs diff --git a/.github/workflows/risk-gate.yml b/.github/workflows/risk-gate.yml index 84acd9f..ed83615 100644 --- a/.github/workflows/risk-gate.yml +++ b/.github/workflows/risk-gate.yml @@ -3,20 +3,18 @@ name: PR risk gate on: pull_request_target: types: [opened, reopened, synchronize, edited, labeled, unlabeled, ready_for_review, closed] + # Codex publishes and refreshes its review receipt as a PR comment, which + # raises no pull_request event. Only that bot can re-trigger the evaluation. issue_comment: types: [created, edited, deleted] + # A merge to the base branch can invalidate a review that already passed and + # raises no PR event, so the gate re-sweeps the open pull requests. push: branches: [main, master] + # A merge made with `GITHUB_TOKEN` (the Dependabot auto-merge path) raises no + # push event, so a periodic sweep catches the base moves no event reports. schedule: - cron: '17 */6 * * *' - check_run: - types: [completed, rerequested] - check_suite: - types: [completed] - status: - workflow_run: - workflows: [Documentation check, Engineering governance, Governance shadow tests, Secret scan tests] - types: [completed, in_progress] workflow_dispatch: inputs: pr_number: @@ -24,13 +22,14 @@ on: required: true type: string -# Trusted workflow only: no candidate code, label writes or merge permission. +# No checkout, PR code execution, secret access, label writes or merge permission. +# `actions: write` is only used to dispatch a per-PR evaluation after a base push. permissions: contents: read pull-requests: read issues: read - checks: read statuses: write + actions: write jobs: # The `labeled` payload is the only authenticated record of which head a @@ -53,7 +52,6 @@ jobs: APPROVAL_OWNER: ${{ vars.RISK_GATE_OWNER }} with: script: | - try { const label = context.payload.label?.name; if (!['owner-approved', 'agent-approved'].includes(label)) return; // Only the configured approver's label carries a claim. @@ -96,120 +94,81 @@ jobs: ...context.repo, sha, context: 'clearing-label', state: 'success', description: `${label} for PR ${number} at event ${record.id} on this head`, }); - } catch { core.setFailed('Clearing-label evidence unavailable; approval blocked'); } - - discover: - if: >- - (github.event_name != 'issue_comment' || github.event.issue.pull_request) && - (github.event_name != 'status' || - (github.event.context != 'risk-gate' && github.event.context != 'pr-template' && github.event.context != 'ai-review' && github.event.context != 'clearing-label')) - runs-on: ubuntu-latest - timeout-minutes: 5 - permissions: - contents: read - pull-requests: read - statuses: read - outputs: - prs: ${{ steps.candidates.outputs.prs }} - steps: - - id: candidates - uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8 - env: - PR_NUMBER: ${{ inputs.pr_number }} - with: - script: | - try { - const payload = context.payload; - let numbers = [payload.pull_request?.number || payload.issue?.number || Number(process.env.PR_NUMBER)].filter(Boolean); - const sweep = ['push', 'schedule'].includes(context.eventName) - || (context.eventName === 'pull_request_target' && payload.action === 'closed'); - if (sweep || (context.eventName === 'pull_request_target' && payload.action === 'synchronize')) { - // Base/close recovery and collision release join the same per-PR writer. - const open = await github.paginate(github.rest.pulls.list, - {...context.repo, state: 'open', per_page: 100}); - if (sweep) numbers = open.map(pr => pr.number); - else for (const pr of open) { - if (numbers.includes(pr.number)) continue; - const {data} = await github.rest.repos.getCombinedStatusForRef({...context.repo, ref: pr.head.sha}); - const gate = data.statuses?.find(entry => entry.context === 'risk-gate'); - if (gate?.state === 'error' && /shares this head/.test(gate.description || '')) numbers.push(pr.number); - } - } else if (!numbers.length) { - const sha = payload.check_run?.head_sha || payload.check_suite?.head_sha || payload.workflow_run?.head_sha || payload.sha; - if (typeof sha !== 'string' || !/^[0-9a-f]{40}$/.test(sha)) throw new Error(); - const associated = await github.paginate(github.rest.repos.listPullRequestsAssociatedWithCommit, - {...context.repo, commit_sha: sha, per_page: 100}); - numbers = associated.filter(pr => pr.state === 'open' && pr.head?.sha === sha).map(pr => pr.number); - } - if (numbers.some(number => !Number.isSafeInteger(number) || number < 1)) throw new Error(); - core.setOutput('prs', JSON.stringify([...new Set(numbers)])); - } catch { core.setFailed('Candidate lookup unavailable; approval blocked'); } evaluate: - needs: [discover, record_label] - if: always() && needs.discover.result == 'success' && needs.record_label.result != 'failure' && needs.record_label.result != 'cancelled' && needs.discover.outputs.prs != '[]' && needs.discover.outputs.prs != '' - strategy: - fail-fast: false - matrix: - pr: ${{ fromJSON(needs.discover.outputs.prs) }} - # Comment, PR and check events resolve to the same PR writer across restarts. - concurrency: - group: risk-gate-${{ github.repository }}-${{ matrix.pr }} - cancel-in-progress: false name: Evaluate PR governance + needs: record_label + # A skipped recorder is the normal case for every other event. + if: always() && needs.record_label.result != 'cancelled' + && (github.event_name != 'issue_comment' || (github.event.issue.pull_request && (github.event.comment.user.login == 'chatgpt-codex-connector[bot]' || github.event.comment.user.login == 'chatgpt-codex-connector'))) runs-on: ubuntu-latest timeout-minutes: 5 + # Job-level, so a comment from anyone else skips the job without cancelling + # an evaluation already running for this PR. Label evidence no longer rides + # on this run, so a superseded evaluation costs nothing. + concurrency: + group: risk-gate-${{ github.event.pull_request.number || github.event.issue.number || inputs.pr_number }} + cancel-in-progress: true steps: - - name: Publish risk-gate and PR template checks on the PR head + - name: Publish risk-gate, AI review and PR template checks on the PR head uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8 env: - PR_NUMBER: ${{ matrix.pr }} + PR_NUMBER: ${{ inputs.pr_number }} + # Required setup: set the RISK_GATE_OWNER repository variable to the + # approver's login (for example, ). The approver is + # repository configuration, not a literal, so the owner can change + # without editing the gate. Unset accepts no label and leaves + # high-risk merges blocked. APPROVAL_OWNER: ${{ vars.RISK_GATE_OWNER }} with: script: | - // Only trusted base/default-branch workflow code executes; never load PR code. + // Executed only by the trusted base-branch workflow. Never load PR code. + const eventName = context.eventName; + // A push or a scheduled sweep has no single PR; both re-evaluate + // them all. Closing a PR can end a shared-head collision, so it + // re-evaluates the remaining open PRs instead of the closed one. + const sweep = ['push', 'schedule'].includes(eventName) + || (eventName === 'pull_request_target' && context.payload.action === 'closed'); + const single = !sweep; + const number = context.payload.pull_request?.number + || context.payload.issue?.number + || Number(process.env.PR_NUMBER); + if (single && (!Number.isSafeInteger(number) || number < 1)) throw new Error('Invalid PR number'); const repo = context.repo; - const fail = () => { throw new Error('Governance evidence unavailable'); }; - const hash = value => typeof value === 'string' && /^[0-9a-f]{40}$/.test(value); const target = `${context.serverUrl}/${repo.owner}/${repo.repo}/actions/runs/${context.runId}`; - const number = Number(process.env.PR_NUMBER); + // Definitions that decide what passes cannot be relaxed by a PR. + const ALWAYS_RISK_WORKFLOWS = new Set([ + '.github/workflows/risk-gate.yml', + '.github/workflows/offline-submission-regression.yml', + '.github/workflows/repository-hygiene.yml', + '.github/workflows/engineering-governance.yml', + '.github/workflows/dependabot-policy.yml', + ]); + const WORKFLOW_PRIVILEGE = [ + /^\s*permissions\s*:/, + // Any granted scope, quoted or trailed by a comment, including scopes + // GitHub adds later. An unknown key still fails closed here. + /^\s*["']?[a-z][a-z0-9-]*["']?\s*:\s*["']?write["']?\s*(?:#.*)?$/, + /\bsecrets\b/, + /\b(?:GH_TOKEN|GITHUB_TOKEN|github\.token)\b/, + /\bpull_request_target\b/, + /\bworkflow_run\b/, + // Events an unprivileged user can raise against a write-capable workflow. + /\bissue_comment\b/, + /\bpull_request_review_comment\b/, + /\bdiscussion_comment\b/, + /\brepository_dispatch\b/, + ]; + const ACTION_PIN = /^\s*-?\s*uses\s*:/; + const ACTION_TARGET = /^\s*-?\s*uses\s*:\s*["']?([^@"'\s]+)@/; + // The owner login comes from repository configuration so the gate + // does not hard-code an account. An empty value matches nobody, so + // a missing variable blocks high-risk merges instead of widening. const APPROVAL_OWNER = process.env.APPROVAL_OWNER || ''; const isWorkflow = path => path.startsWith('.github/workflows/'); - // Sensitive-path classification: match normalized lowercase snake_case paths. - // * never crosses /; ** does. No docs/tests exemption applies to this list. - const sensitiveGlobs = [ - '**/*secret*', '**/*secret*/**', '**/*credential*', '**/*credential*/**', - '**/.env*', '**/*.pem', '**/*.key', '**/*.p12', '**/*.pfx', - '**/*iam*', '**/*iam*/**', '**/*service_account*', '**/*service_account*/**', - '**/*serviceaccount*', '**/*serviceaccount*/**', '**/*permission*', '**/*permission*/**', - '**/*role_binding*', '**/*role_binding*/**', - '**/*deploy*', '**/*deploy*/**', '**/*release*', '**/*release*/**', - '**/*.tf', '**/*.tfvars', '**/dockerfile*', '**/docker-compose*', '**/compose.y*ml', - '**/vercel.json', '**/wrangler.*', '**/firebase.json', '**/k8s/**', '**/kubernetes/**', - '**/*trad*', '**/*trad*/**', '**/*execution*', '**/*execution*/**', - '**/*executor*', '**/*executor*/**', '**/*order*', '**/*order*/**', - '**/*broker*', '**/*broker*/**', '**/*kelly*', '**/*kelly*/**', - '**/*stop*loss*', '**/*stop*loss*/**', - '.github/governance/**', '.github/workflows/risk-gate.yml', '**/codeowners', - ]; - const normalize = path => path.replace(/([a-z0-9])([A-Z])/g, '$1_$2').toLowerCase(); - function matchesGlob(path, glob) { - let pattern = '^'; - for (let i = 0; i < glob.length; i++) { - const char = glob[i]; - if (char === '*' && glob[i + 1] === '*') { - i++; - if (glob[i + 1] === '/') { pattern += '(?:.*/)?'; i++; } - else pattern += '.*'; - } else if (char === '*') pattern += '[^/]*'; - else pattern += '\\^$.*+?()[]{}|'.includes(char) ? '\\' + char : char; - } - return new RegExp(pattern + '$').test(normalize(path)); - } - const sensitivePath = path => sensitiveGlobs.some(glob => matchesGlob(path, glob)); function highRisk(path) { const p = path.replace(/([a-z0-9])([A-Z])/g, '$1_$2').toLowerCase(); - if (p.startsWith('.github/governance/') || p.startsWith('.github/scripts/') || /(^|\/)codeowners$/.test(p)) return true; + if (/(^|\/)codeowners$/.test(p)) return true; if (/(^|\/)(docs?|tests?|examples?|fixtures)(\/|$)/.test(p)) return false; if (/(^|\/)\.env(?:[./_-]|$)/.test(p)) return true; // Runtime configuration decides behaviour the diff does not show. @@ -221,10 +180,51 @@ jobs: // workflow file, and a diff alone cannot show who calls it. if (/^\.github\/scripts\//.test(path)) return true; if (/^scripts\/[^/]*(?:check|verify|validate|hygiene|lint)[^/]*\.(?:py|sh|js|cjs|mjs|ts)$/.test(p)) return true; - if (/(^|[\/_.-])(ai_service|aiservice|kelly|stop_loss|trad(?:e|es|ing)|execution|executors?|orders?|brokers?|risk|deploy(?:ment)?|secrets?|credentials?|iam|runtime|strateg(?:y|ies))([\/_.-]|$)/.test(p)) return true; + if (/(^|[\/_.-])(trad(?:e|es|ing)|execution|executors?|orders?|brokers?|risk|deploy(?:ment)?|secrets?|credentials?|iam|runtime|strateg(?:y|ies))([\/_.-]|$)/.test(p)) return true; return /(^|\/)(dockerfile(?:\..*)?|docker-compose[^/]*|compose\.ya?ml|vercel\.json|wrangler\.(?:toml|jsonc?)|firebase\.json)$/.test(p) || /\.(?:tf|tfvars)$/.test(p); } + // A repin keeps the action and changes only its ref. Swapping in a + // different action is a different change and takes the normal tier. + function sameAction(entries) { + const targets = sign => entries.filter(entry => entry.sign === sign) + .map(entry => (ACTION_TARGET.exec(entry.text) || [])[1]); + const removed = targets('-'); + const added = targets('+'); + return removed.length > 0 && removed.length === added.length + && removed.every((target, index) => target === added[index]); + } + async function workflowHighRisk(file, isDependabot, actionFiles) { + // A rename must not slip a protected definition past the check. + if (ALWAYS_RISK_WORKFLOWS.has(file.filename) || ALWAYS_RISK_WORKFLOWS.has(file.previous_filename)) return true; + // Renaming a workflow changes the file GitHub discovers, and moving + // it out of the directory disables the checks it published. A + // `.yml.disabled` suffix is still inside the directory but no + // longer a workflow, so any path change takes the protected tier. + if (file.previous_filename && file.previous_filename !== file.filename) return true; + // Without a readable diff the change cannot be proven benign. + if (typeof file.patch !== 'string') return true; + const changed = file.patch.split('\n') + .filter(line => /^[+-]/.test(line) && !/^(?:\+\+\+|---)/.test(line)) + .map(line => ({ sign: line[0], text: line.slice(1) })); + if (changed.some(entry => WORKFLOW_PRIVILEGE.some(pattern => pattern.test(entry.text)))) return true; + // A verified Dependabot update only repins action versions, so the + // change is nothing but one `uses:` line replaced by another. The + // base-branch dependency policy names the workflows whose action + // updates are eligible; anything else takes the normal path. + const pinned = changed.filter(entry => ACTION_PIN.test(entry.text)); + if (isDependabot && actionFiles?.has(file.filename) + && pinned.length === changed.length && sameAction(pinned)) return false; + // Every remaining edit can change what CI verifies: a removed + // assertion, an added `if: false`, a `continue-on-error`. Even a + // `#` line is not provably inert, because inside a `run:` block it + // is shell text that GitHub still expands expressions in before + // Bash sees it, and a comment after a backslash continuation cuts + // the command short. A workflow that publishes a required status + // is exactly where guessing is most expensive, so this tier needs + // a clearing label. + return true; + } function completeTemplate(body) { const text = (body || '').replace(//g, ''); const sections = [...text.matchAll(/^#{1,6}\s+(.+)\r?\n([\s\S]*?)(?=^#{1,6}\s|$(?![\s\S]))/gm)]; @@ -235,184 +235,292 @@ jobs: && present(/what changed|summary|problem|changes|变更|改动/i) && present(/how verified|verif|validat|test(?:ing| plan)?|验证|测试/i); } - - function cleanReview(comments, policy, sha) { - if (!Array.isArray(policy.codex_app_ids) || !policy.codex_app_ids.length - || !policy.codex_app_ids.every(id => Number.isSafeInteger(id) && id > 0)) return false; - const entries = comments.filter(entry => entry.user?.type === 'Bot' - && policy.codex_app_ids.includes(entry.performed_via_github_app?.id) - && typeof entry.body === 'string' && entry.body.includes('codex-review-state:v1')); - if (!entries.length || entries.some(entry => !Number.isSafeInteger(entry.id) - || !Number.isFinite(Date.parse(entry.updated_at)) || Date.parse(entry.updated_at) > Date.now())) return false; - entries.sort((a, b) => Date.parse(b.updated_at) - Date.parse(a.updated_at) || b.id - a.id); - if ((entries[0].body.match(/codex-review-state:v1/g) || []).length !== 1) return false; - const match = //.exec(entries[0].body); - if (!match) return false; - let review; - try { review = JSON.parse(match[1]).code_review; } catch { return false; } - return review?.commit_sha === sha && review.status === 'completed' - && typeof review.has_any_findings === 'boolean' && review.has_unresolved_findings === false; + // REST and GraphQL disagree on the bot suffix, so accept both forms. + const CODEX_LOGIN = /^chatgpt-codex-connector(?:\[bot\])?$/; + // The receipt is a single JSON object closed just before the HTML + // comment terminator, so the capture must stay greedy. + const REVIEW_RECEIPT = //; + // A receipt names only the head it reviewed, and commit dates are author + // metadata rather than the time a branch moved, so an old review can + // outlive a base push. Git ancestry is the authenticated evidence: the + // reviewed head must contain the current base head, which GitHub + // resolves from the object graph. pr.base.sha lags the live branch, so + // comparing it would miss the same drift. + async function headContainsBase(baseRef, headSha) { + const { data } = await github.rest.repos.getBranch({ ...repo, branch: baseRef }); + const baseHead = data.commit.sha; + const { data: cmp } = await github.rest.repos.compareCommits({ ...repo, base: baseHead, head: headSha }); + return cmp.merge_base_commit?.sha === baseHead; } - async function requiredChecks(number, branch) { - // GraphQL null means no classic rule; absent data/permission errors are unknown. - const data = await github.graphql(`query($owner:String!,$repo:String!,$number:Int!) { - repository(owner:$owner,name:$repo) { pullRequest(number:$number) { baseRef { - branchProtectionRule { requiredStatusCheckContexts requiredStatusChecks { context app { databaseId } } } - } } } - }`, {...repo, number}); - const ref = data.repository?.pullRequest?.baseRef; - if (!ref || !Object.hasOwn(ref, 'branchProtectionRule')) fail(); - const rule = ref.branchProtectionRule; - if (rule !== null && (!Array.isArray(rule.requiredStatusChecks) - || !Array.isArray(rule.requiredStatusCheckContexts))) fail(); - const classic = rule?.requiredStatusChecks.map(check => ({context: check.context, - app: check.app === null ? null : check.app?.databaseId})) || []; - if (rule?.requiredStatusCheckContexts.some(name => !classic.some(check => check.context === name))) fail(); - const rules = await github.paginate(github.rest.repos.getBranchRules, {...repo, branch, per_page: 100}); - const required = [...classic]; - for (const entry of rules) { - if (entry.type !== 'required_status_checks') continue; - if (!Array.isArray(entry.parameters?.required_status_checks)) fail(); - for (const check of entry.parameters.required_status_checks) { - required.push({context: check.context, app: check.integration_id ?? null}); - } - } - if (!required.length || required.some(check => typeof check.context !== 'string' || !check.context - || (check.app !== null && (!Number.isSafeInteger(check.app) || check.app <= 0)))) fail(); - return {required, rules}; + // A Dependabot skip only holds for commits Dependabot itself wrote. + async function dependabotHead(prNumber) { + const commits = await github.paginate(github.rest.pulls.listCommits, { ...repo, pull_number: prNumber, per_page: 100 }); + // The API truncates at 250 entries; an incomplete history cannot + // prove the bot wrote every commit in the head. + if (!commits.length || commits.length >= 250) return false; + // `author.login` is the account GitHub links from the commit email, + // and the author name is free text; a human can set either. A + // verified signature only proves some key signed it, so the rest + // of the shape has to match a commit GitHub itself made for the + // bot: committer `web-flow` from `noreply@github.com`. This is the + // same identity `scripts/dependabot_auto_merge_gate.py` accepts. + return commits.every(commit => commit.author?.login === 'dependabot[bot]' + && commit.committer?.login === 'web-flow' + && commit.commit?.committer?.email === 'noreply@github.com' + && commit.commit?.verification?.verified === true); } - function checksGreen(evidence, sha, templateOK) { - return evidence.required.every(required => { - // These three contexts are produced here; cleanReview validates ai-review. Validate the template locally; risk-gate - // is this decision's output, never an input. Other required jobs are never exempt. - if (['risk-gate', 'pr-template', 'ai-review'].includes(required.context)) { - return (required.app === null || required.app === 15368) && templateOK; - } - const checks = evidence.checks.filter(check => check.name === required.context - && (required.app === null || check.app?.id === required.app)).sort((a, b) => b.id - a.id); - const statuses = evidence.statuses.filter(status => status.context === required.context).sort((a, b) => b.id - a.id); - if (checks.some(check => !Number.isSafeInteger(check.id)) || statuses.some(status => !Number.isSafeInteger(status.id))) return false; - // Legacy statuses do not expose their source App. They cannot satisfy App-bound - // requirements; leave enforcement closed rather than infer identity from a login. - if (required.app !== null && statuses.length) return false; - if (!checks.length && !statuses.length) return false; - return (!checks.length || (checks[0].head_sha === sha && checks[0].status === 'completed' - && checks[0].conclusion === 'success')) - && (!statuses.length || statuses[0].state === 'success'); - }); - } - function clearingLabel(events, statuses, labels, number) { - const onHead = {}; - for (const name of ['owner-approved', 'agent-approved']) { - if (!labels.some(label => label.name === name)) continue; - const latest = events.filter(event => ['labeled', 'unlabeled'].includes(event.event) - && event.label?.name === name).sort((a, b) => b.id - a.id)[0]; - if (!latest || !Number.isSafeInteger(latest.id) || latest.event !== 'labeled' - || !APPROVAL_OWNER || latest.actor?.login !== APPROVAL_OWNER) continue; - onHead[name] = statuses.some(entry => entry.context === 'clearing-label' && entry.state === 'success' - && entry.description === `${name} for PR ${number} at event ${latest.id} on this head`); + // The dependency policy names the workflows whose action updates may + // merge unattended. It is read from the base branch, so a PR cannot + // grant itself the exemption by editing the policy it is judged by. + async function eligibleActionFiles(baseRef) { + try { + const { data } = await github.rest.repos.getContent({ + ...repo, path: '.github/scripts/dependabot-policy.json', ref: baseRef, + }); + if (data.type !== 'file' || typeof data.content !== 'string') return null; + const policy = JSON.parse(Buffer.from(data.content, 'base64').toString('utf8')); + return new Set(Array.isArray(policy.action_files) ? policy.action_files : []); + } catch { + // An unreadable or missing policy grants no exemption. + return null; } - return onHead['owner-approved'] ? 'owner' : onHead['agent-approved'] ? 'agent' : null; - } - async function liveBase(baseRef, sha) { - const {data} = await github.rest.repos.getBranch({...repo, branch: baseRef}); - const base = data.commit?.sha; - if (!hash(base)) fail(); - const {data: cmp} = await github.rest.repos.compareCommits({...repo, base, head: sha}); - return {base, contained: cmp.merge_base_commit?.sha === base}; } - async function noSharedHead(sha) { - const open = await github.paginate(github.rest.pulls.list, {...repo, state: 'open', per_page: 100}); - if (open.some(pr => pr.number !== number && pr.head?.sha === sha)) { - const error = new Error('Shared head'); error.code = 'SHARED_HEAD'; throw error; + // Codex publishes a machine-readable receipt in its rolling summary + // comment. Only a completed review of the current head with no + // unresolved finding counts as a pass; anything else blocks the gate. + async function aiReview(prNumber, headSha, baseContained) { + // A review cannot outlive a base push. Re-requesting a review on the + // same head does not clear this: the merge would still combine that + // head with a base the review never saw, so the branch comes first. + if (!baseContained) { + return { state: 'failure', description: 'Base branch advanced since this head was built; update the branch then comment @codex review' }; + } + const comments = await github.paginate(github.rest.issues.listComments, { ...repo, issue_number: prNumber, per_page: 100 }); + const receipts = comments + .filter(comment => CODEX_LOGIN.test(comment.user?.login || '')) + .map(comment => ({ comment, match: REVIEW_RECEIPT.exec(comment.body || '') })) + .filter(entry => entry.match) + .sort((a, b) => new Date(b.comment.updated_at) - new Date(a.comment.updated_at)); + if (!receipts.length) return { state: 'pending', description: 'No Codex review yet; comment @codex review to start one' }; + let receipt; + try { + receipt = JSON.parse(receipts[0].match[1]); + } catch { + return { state: 'error', description: 'Codex review receipt is not valid JSON' }; + } + const review = receipt?.code_review || {}; + if (review.status !== 'completed') { + return { state: 'pending', description: `Codex review ${review.status || 'not finished'}; comment @codex review to restart it` }; } + const reviewed = typeof review.commit_sha === 'string' ? review.commit_sha : ''; + if (reviewed !== headSha) { + return { state: 'failure', description: `Codex reviewed ${reviewed.slice(0, 7) || 'another commit'}, not this head; comment @codex review` }; + } + if (review.has_unresolved_findings !== false) { + return { state: 'failure', description: 'Codex review still has unresolved findings on this head' }; + } + return { state: 'success', description: 'Codex reviewed this head with no unresolved findings' }; } - { + // Returns true when the gate passes for this PR. Errors are rethrown + // after the statuses are marked, so the caller decides how loud to be. + async function evaluate(number) { let sha; - const status = async (name, state, description) => { - try { - await github.rest.repos.createCommitStatus({ - ...repo, sha, context: name, state, description, target_url: target, - }); - } catch { fail(); } - }; + const status = (name, state, description) => github.rest.repos.createCommitStatus({ + ...repo, sha, context: name, state, description: String(description).slice(0, 140), target_url: target, + }); try { - if (!Number.isSafeInteger(number) || number < 1) fail(); - const args = {...repo, pull_number: number}; - const readPR = async () => (await github.rest.pulls.get(args)).data; - const binding = pr => [pr.state, pr.head?.sha, pr.base?.sha, pr.base?.ref, - pr.base?.repo?.full_name, pr.changed_files, pr.body, pr.labels]; - const pr = await readPR(); - // A closed PR cannot overwrite a live PR's commit statuses. - if (pr.state === 'closed') return; - sha = pr.head?.sha; - if (pr.state !== 'open' || !hash(sha) || !hash(pr.base?.sha) || !pr.base?.ref - || pr.base.repo?.full_name?.toLowerCase() !== `${repo.owner}/${repo.repo}`.toLowerCase()) fail(); - await noSharedHead(sha); - await status('risk-gate', 'pending', 'Evaluating current review, clearing label and required checks'); - const templateOK = completeTemplate(pr.body); - await status('pr-template', templateOK ? 'success' : 'failure', templateOK - ? 'AI, change and verification fields are filled in' : 'Fill in AI, change and verification sections'); - const readFiles = async () => { - const files = await github.paginate(github.rest.pulls.listFiles, {...args, per_page: 100}); - if (!Number.isSafeInteger(pr.changed_files) || pr.changed_files < 1 - || files.length !== pr.changed_files || files.length >= 3000 - || new Set(files.map(file => file.filename)).size !== files.length - || files.some(file => typeof file.filename !== 'string' || !file.filename - || !['added', 'modified', 'removed', 'renamed'].includes(file.status) - || (file.previous_filename !== undefined && (typeof file.previous_filename !== 'string' || !file.previous_filename)) - || (file.status === 'renamed' && !file.previous_filename))) fail(); - return files; + const { data: pr } = await github.rest.pulls.get({ ...repo, pull_number: number }); + // A closed PR keeps no authority over its head: publishing on it + // could overwrite a live PR that now uses the same commit. + if (pr.state !== 'open') return true; + sha = pr.head.sha; + // Commit statuses are keyed by SHA, not by PR. A second open PR on + // the same head would share these contexts, and one verdict could + // overwrite the other, so no status is proof while that is true. + const siblings = await github.paginate(github.rest.pulls.list, { ...repo, state: 'open', per_page: 100 }); + if (siblings.some(item => item.number !== number && item.head?.sha === sha)) { + // The description is the marker the synchronize refresh below + // looks for, so the collision must be named, not folded into + // the generic failure text. + const collision = new Error('Another open pull request shares this head; statuses would collide'); + collision.code = 'SHARED_HEAD'; + throw collision; + } + // Dependabot bodies use their own template; the risk-gate path check still applies below. + // The exemption follows the head commit, not the PR author: a human + // pushing onto a bot branch must not inherit the skip. + const isDependabot = pr.user?.login === 'dependabot[bot]' && await dependabotHead(number); + const actionFiles = isDependabot ? await eligibleActionFiles(pr.base.ref) : null; + await status('risk-gate', 'pending', 'Evaluating changed paths, clearing label and AI review'); + await status('pr-template', 'pending', 'Checking AI, change and verification fields'); + await status('ai-review', 'pending', 'Reading the Codex review receipt'); + const files = await github.paginate(github.rest.pulls.listFiles, { ...repo, pull_number: number, per_page: 100 }); + // GitHub caps this API at 3000 files. Incomplete diffs must not pass as low risk. + if (files.length !== pr.changed_files || files.length >= 3000) throw new Error('Incomplete PR file list'); + let risky = false; + for (const file of files) { + const names = [file.filename, file.previous_filename].filter(Boolean); + risky = names.some(isWorkflow) ? await workflowHighRisk(file, isDependabot, actionFiles) : names.some(highRisk); + if (risky) break; + } + // Two labels, two different claims. The owner and their agents share + // one GitHub login, so the event actor cannot tell them apart; the + // label name is the only honest signal left. owner-approved claims a + // human confirmed it, agent-approved claims an agent did. The status + // description always names which one applied, so an agent-cleared + // high-risk merge is visible instead of passing as a human review. + const APPROVAL_LABELS = ['owner-approved', 'agent-approved']; + // A label lives on the pull request, not on a commit, so a push + // after the label leaves it looking current. The `record-label` job + // writes the head from the `labeled` event to a commit status, and + // this evaluation credits the label only when the current head + // carries that record. Commit dates cannot prove which revision was + // current, and a status is keyed by the SHA it names. + const CLEARING_RECORD = 'clearing-label'; + let approval = null; + if (pr.labels.some(label => APPROVAL_LABELS.includes(label.name))) { + const events = await github.paginate(github.rest.issues.listEvents, { ...repo, issue_number: number, per_page: 100 }); + const liveLabel = name => { + const latest = events.filter(event => ['labeled', 'unlabeled'].includes(event.event) + && event.label?.name === name).sort((a, b) => b.id - a.id)[0]; + return latest?.event === 'labeled' && APPROVAL_OWNER !== '' + && latest.actor?.login === APPROVAL_OWNER ? latest : null; }; - const files = await readFiles(); - const paths = files.flatMap(file => [file.filename, file.previous_filename].filter(Boolean)); - const risky = paths.some(path => isWorkflow(path) || sensitivePath(path) || highRisk(path)); - const {data: content} = await github.rest.repos.getContent({...repo, - path: '.github/governance/policy.json', ref: pr.base.sha}); - if (content.encoding !== 'base64' || typeof content.content !== 'string') fail(); - const policy = JSON.parse(Buffer.from(content.content, 'base64').toString('utf8')); - const readEvidence = async () => ({ - ...await requiredChecks(number, pr.base.ref), - comments: await github.paginate(github.rest.issues.listComments, {...repo, issue_number: number, per_page: 100}), - checks: await github.paginate(github.rest.checks.listForRef, {...repo, ref: sha, filter: 'latest', per_page: 100}), - statuses: (await github.paginate(github.rest.repos.listCommitStatusesForRef, {...repo, ref: sha, per_page: 100})) - .filter(entry => !['risk-gate', 'pr-template', 'ai-review'].includes(entry.context)), - events: await github.paginate(github.rest.issues.listEvents, {...repo, issue_number: number, per_page: 100}), - live: await liveBase(pr.base.ref, sha), - }); - const before = await readEvidence(); - const reviewOK = before.live.contained && cleanReview(before.comments, policy, sha); - const checksOK = checksGreen(before, sha, templateOK); - const approval = clearingLabel(before.events, before.statuses, pr.labels, number); - const after = await readEvidence(); - const currentFiles = await readFiles(); - const current = await readPR(); - if (JSON.stringify(binding(current)) !== JSON.stringify(binding(pr)) - || JSON.stringify(currentFiles) !== JSON.stringify(files) - || JSON.stringify(before) !== JSON.stringify(after)) fail(); - await noSharedHead(sha); - await status('ai-review', reviewOK ? 'success' : 'failure', reviewOK - ? 'Trusted Codex App reviewed this head with no unresolved findings' - : 'Exact-head clean Codex review and live-base ancestry required'); - const allowed = reviewOK && checksOK && templateOK && (!risky || approval !== null); - const note = !risky ? 'Low-risk paths: review and required checks verified' - : approval === 'owner' ? 'High-risk paths: owner-approved label verified' - : approval === 'agent' ? 'High-risk paths: agent-approved, NOT confirmed by the owner' - : 'High-risk paths: needs head-bound agent-approved or owner-approved'; - await status('risk-gate', allowed ? 'success' : 'failure', allowed ? note - : !reviewOK || !checksOK || !templateOK ? 'Approval blocked: review, template or required checks incomplete' : note); - if (!allowed) core.setFailed('Required PR governance checks did not pass'); + // Only a label bound to this head counts, so a stale owner label + // cannot shadow a fresh agent label: each label is checked on its + // own and owner precedence applies after that, never before. + const onHead = {}; + const live = APPROVAL_LABELS.map(name => [name, liveLabel(name)]).filter(([, event]) => event); + if (live.length) { + const statuses = await github.paginate(github.rest.repos.listCommitStatusesForRef, + { ...repo, ref: sha, per_page: 100 }); + for (const [name, event] of live) { + // The record must name the same event that is live now, so a + // revoked approval is not revived by an older record. + if (statuses.some(entry => entry.context === CLEARING_RECORD + && entry.state === 'success' + && entry.description === `${name} for PR ${number} at event ${event.id} on this head`)) { + onHead[name] = true; + } + } + } + // A human confirmation outranks an agent's when both are on this head. + if (onHead['owner-approved']) approval = 'owner'; + else if (onHead['agent-approved']) approval = 'agent'; + } + const { data: current } = await github.rest.pulls.get({ ...repo, pull_number: number }); + if (current.head.sha !== sha || current.body !== pr.body + || JSON.stringify(current.labels.map(x => x.name).sort()) !== JSON.stringify(pr.labels.map(x => x.name).sort())) { + throw new Error('PR changed during evaluation; wait for the latest event'); + } + const review = isDependabot + ? { state: 'success', description: 'Automated dependency update: Codex review not required' } + : await aiReview(number, sha, await headContainsBase(pr.base.ref, sha)); + // The opening snapshot cannot see a pull request that appeared on + // this head while the evaluation ran. Publishing a verdict now + // would overwrite the collision error that the newcomer is about + // to write, leaving two open PRs sharing a passing status, so the + // condition is rechecked immediately before any status is written. + const lateSiblings = await github.paginate(github.rest.pulls.list, { ...repo, state: 'open', per_page: 100 }); + if (lateSiblings.some(item => item.number !== number && item.head?.sha === sha)) { + const collision = new Error('Another open pull request shares this head; statuses would collide'); + collision.code = 'SHARED_HEAD'; + throw collision; + } + await status('ai-review', review.state, review.description); + const tierOK = !risky || approval !== null; + const tierNote = !risky ? 'Low-risk paths: no owner approval needed' + : approval === 'owner' ? 'High-risk paths: owner-approved label verified' + : approval === 'agent' ? 'High-risk paths: cleared by agent-approved, NOT confirmed by the owner' + : 'High-risk paths: needs a clearing label; owner-approved or agent-approved'; + let gateState; + let gateNote; + if (!tierOK) { + gateState = 'failure'; + gateNote = tierNote; + } else if (review.state === 'success') { + gateState = 'success'; + gateNote = `${tierNote}; AI review passed`; + } else if (review.state === 'pending') { + // An unfinished review is neither a pass nor a failure. + gateState = 'pending'; + gateNote = `${tierNote}; waiting for AI review`; + } else { + gateState = 'failure'; + gateNote = `${tierNote}; ${review.description}`; + } + await status('risk-gate', gateState, gateNote); + const templateOK = isDependabot ? true : completeTemplate(pr.body); + await status('pr-template', templateOK ? 'success' : 'failure', isDependabot + ? 'Automated dependency update: PR template not required' + : templateOK ? 'AI, change and verification fields are filled in' : 'Fill in AI assistance, What changed and How verified sections'); + return gateState !== 'failure' && templateOK; } catch (error) { - if (hash(sha)) { + if (sha) { const detail = error?.code === 'SHARED_HEAD' ? 'Another open pull request shares this head; statuses would collide' - : 'Evidence unavailable or changed; approval blocked'; + : 'Evaluation failed; inspect the trusted workflow run'; + await status('risk-gate', 'error', detail); + await status('pr-template', 'error', detail); + await status('ai-review', 'error', detail); + } + throw error; + } + } + // A synchronize moves one pull request off a shared head. The other + // pull request that shared it still carries the collision error and + // would otherwise keep it until its own event or the sweep, so + // re-dispatch only the siblings still showing that verdict. Other + // errors are not collateral: an evaluation that failed for its own + // reasons must not be re-run by an unrelated push. + async function refreshCollidedSiblings() { + if (eventName !== 'pull_request_target' || context.payload.action !== 'synchronize') return; + const open = await github.paginate(github.rest.pulls.list, { ...repo, state: 'open', per_page: 100 }); + for (const item of open) { + if (item.number === number) continue; + const { data: combined } = await github.rest.repos.getCombinedStatusForRef({ ...repo, ref: item.head.sha }); + const gate = (combined.statuses || []).find(entry => entry.context === 'risk-gate'); + if (gate?.state === 'error' && /shares this head/.test(gate.description || '')) { + await github.rest.actions.createWorkflowDispatch({ + ...repo, workflow_id: 'risk-gate.yml', ref: context.payload.repository.default_branch, + inputs: { pr_number: String(item.number) }, + }); + } + } + } + if (single) { + let passed; + try { + passed = await evaluate(number); + } finally { + // A synchronize can release a sibling from a shared head even + // when this evaluation throws, and a stale collision error + // would otherwise keep that sibling blocked until its own + // event or the sweep. Best effort: a refresh failure must not + // replace the evaluation error. try { - await status('risk-gate', 'error', detail); - await status('pr-template', 'error', detail); - await status('ai-review', 'error', detail); - } catch { /* Never expose transport diagnostics. */ } + await refreshCollidedSiblings(); + } catch (refreshError) { + core.warning(`Sibling refresh failed: ${refreshError.message}`); + } } - core.setFailed('Governance evidence unavailable; approval blocked'); + // A dispatched run only refreshes statuses after the base moved, and + // the verdict already lives on the PR's commit status. Failing the run + // here would paint every merge red for open PRs that are legitimately + // not ready, which buries a real evaluation error. Event-driven runs + // keep failing loudly. + if (!passed && eventName !== 'workflow_dispatch') core.setFailed('Required PR governance checks did not pass'); + } else { + // Merging moves the base without raising a PR event. Dispatch a normal + // per-PR evaluation for each open PR instead of evaluating inline, so + // each one joins that PR's concurrency group and cannot race an + // event-driven run into publishing a stale status. + const open = await github.paginate(github.rest.pulls.list, { ...repo, state: 'open', per_page: 100 }); + for (const item of open) { + await github.rest.actions.createWorkflowDispatch({ + ...repo, workflow_id: 'risk-gate.yml', ref: context.payload.repository.default_branch, + inputs: { pr_number: String(item.number) }, + }); } + core.info(`Dispatched ${open.length} pull request evaluations after the base branch moved.`); } diff --git a/AGENTS.md b/AGENTS.md index b914477..9c3386e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -89,7 +89,7 @@ Organization documentation rules are checked by `python scripts/check_style.py`, - Commit and PR titles use professional English, imperative Conventional Commits (`feat`, `fix`, `docs`, `refactor`, `test`, `chore`, `perf`, `ci`) and at most 72 characters. Sentence case is allowed; commitlint ignores merge commits. - PR descriptions explain the problem, changes, verification and risk. AI-authored PRs identify the tool and the model when available. Labels and approval or risk summaries use professional English. - Use `risk:low` for documentation, tests and ordinary checks; use `risk:high` for protected paths. Classification labels describe risk and grant no authority. -- AI-only development uses required automated checks, template validation and the `ai-review` context published from the Codex receipt on the current head; no human or code-owner approval is required, and no change waits on a human label. This includes governance, AI-service, gate and trust-policy source changes. The immutable base policy supplies trusted App IDs; the newest trusted Bot/App marker must be well-formed, completed and exact-head clean, and required checks must be green. Missing or changing evidence blocks clearance. Every PR, including dependency updates, requires AI review; there is no review exemption. High-risk paths require one clearing label, and either label clears `risk-gate`: once the Codex receipt on that head is completed with no unresolved findings and every required context except the label-dependent `risk-gate` is green, an agent may add `agent-approved` itself, record the addition in the ledger, and then wait for `risk-gate` to re-evaluate and pass; the status description then says `NOT confirmed by the owner`. `owner-approved` stays available when the owner chooses to confirm a change personally, and agents must never add or remove it. A review or label that is not on the current head clears nothing. +- AI-only development uses required automated checks, template validation and the `ai-review` context published from the Codex receipt on the current head; no human or code-owner approval is required, and no change waits on a human label. High-risk paths require one clearing label, and either label clears `risk-gate`: once the Codex receipt on that head is completed with no unresolved findings and every required context except the label-dependent `risk-gate` is green, an agent may add `agent-approved` itself, record the addition in the ledger, and then wait for `risk-gate` to re-evaluate and pass; the status description then says `NOT confirmed by the owner`. `owner-approved` stays available when the owner chooses to confirm a change personally, and agents must never add or remove it. A review or label that is not on the current head clears nothing. - Preserve required checks and branch protection; merge only through normal checks. Use `/` PR branches; never push directly to the default branch or force-push a reviewed branch. Commit, push, merge and production actions require authorization; existing authorization remains valid within its scope. ## Operational authorization diff --git a/PROPOSAL.md b/PROPOSAL.md index 2e948f4..ae99280 100644 --- a/PROPOSAL.md +++ b/PROPOSAL.md @@ -3,6 +3,13 @@ Status: owner-authorized reconciliation; effective only after normal protected merge. The current main gate continues to govern this candidate. +Superseded 2026-11-06: this reconciled verifier was merged here and in +QuantAIService and could not converge. Its own check, status and workflow-run +triggers change the check evidence it rereads, so every evaluation ended in +`error` and closed both repositories to merges. Both returned to the canonical +`.github/workflows/risk-gate.yml` that the other 23 repositories run. The text +below is the record of the proposed design, not the deployed gate. + ## Decision rule AI reviews and the established merge bot handle every PR. There is no diff --git a/scripts/test-risk-gate.cjs b/scripts/test-risk-gate.cjs deleted file mode 100644 index e19bf45..0000000 --- a/scripts/test-risk-gate.cjs +++ /dev/null @@ -1,372 +0,0 @@ -const {test} = require('node:test'); -const assert = require('node:assert/strict'); -const fs = require('node:fs'); -const path = require('node:path'); -const workflow = process.env.RISK_REVISION - ? require('node:child_process').execFileSync('git', ['show', `${process.env.RISK_REVISION}:.github/workflows/risk-gate.yml`], {encoding: 'utf8'}) - : fs.readFileSync(process.env.RISK_WORKFLOW || path.join(__dirname, '../.github/workflows/risk-gate.yml'), 'utf8'); -function embedded(job) { - const section = workflow.split(` ${job}:\n`)[1]?.split(/\n [a-z_]+:\n/)[0]; - return section?.split(' script: |\n')[1]?.split('\n').map(x => x.slice(12)).join('\n'); -} -const AsyncFunction = Object.getPrototypeOf(async function(){}).constructor; -const execute = new AsyncFunction('github', 'context', 'core', 'process', embedded('evaluate')); -const discover = new AsyncFunction('github', 'context', 'core', 'process', embedded('discover')); -const record = embedded('record_label') && new AsyncFunction('github', 'context', 'core', 'process', embedded('record_label')); -const H = 'a'.repeat(40), B = 'b'.repeat(40); -const body = '## AI assistance\nCodex, configured model.\n## What changed\nChange ordinary risk logic.\n## How verified\nRun offline regression tests.'; -function fixture() { - return {pr: {number: 1, state: 'open', body, changed_files: 1, labels: [{name: 'agent-approved'}], - head: {sha: H}, base: {sha: B, ref: 'main', repo: {full_name: '/sample'}}}, - files: [{filename: 'src/runtime/model.js', status: 'modified'}], - comments: [{id: 1, user: {type: 'Bot'}, performed_via_github_app: {id: 12345}, - updated_at: new Date(Date.now() - 60000).toISOString(), body: marker()}], - checks: [{id: 1, name: 'Unit tests', head_sha: H, app: {id: 23456}, status: 'completed', conclusion: 'success'}], - events: [{id: 10, event: 'labeled', label: {name: 'agent-approved'}, actor: {login: '-owner'}, created_at: '2026-01-01T00:00:00Z'}], - open: [{number: 1, head: {sha: H}}], branch: {commit: {sha: B}}, compare: {merge_base_commit: {sha: B}}, - statuses: [{id: 10, context: 'clearing-label', state: 'success', description: 'agent-approved for PR 1 at event 10 on this head'}], rules: [], policy: {codex_app_ids: [12345]}, - required: [{context: 'risk-gate', app: null}, - {context: 'pr-template', app: null}, {context: 'Unit tests', app: {databaseId: 23456}}]}; -} -function marker(changes = {}) { - return ``; -} -async function run(f = fixture()) { - const out = {statuses: [], errors: [], reads: {}, failures: [], statusRefs: []}; - const clone = x => structuredClone(x); - const read = key => { - out.reads[key] = (out.reads[key] || 0) + 1; - if (f.apiError === key) throw new Error('private diagnostic '); - return clone(out.reads[key] > 1 && f.changed?.[key] || f[key]); - }; - const github = {rest: {pulls: {get: async () => ({data: read('pr')}), listFiles: 'files', list: 'open'}, - issues: {listComments: 'comments', listEvents: 'events'}, - checks: {listForRef: 'checks'}, repos: {listCommitStatusesForRef: 'statuses', - getBranchRules: 'rules', listPullRequestsAssociatedWithCommit: 'associated', - getBranch: async () => ({data: read('branch')}), compareCommits: async () => ({data: read('compare')}), - getCombinedStatusForRef: async () => ({data: read('combined')}), - getContent: async params => {out.policyRef = params.ref; return ({data: {encoding: 'base64', content: Buffer.from(JSON.stringify(read('policy'))).toString('base64')}});}, - createCommitStatus: async params => {if (f.apiError === 'publish') throw new Error('private diagnostic '); out.statuses.push(params);}}}, - paginate: async (method, params) => { - if (method === 'statuses') out.statusRefs.push(params.ref); - return read(method); - }, - graphql: async () => {const checks = read('required'); return {repository: {pullRequest: {baseRef: { - branchProtectionRule: f.noClassic ? null : {requiredStatusChecks: checks, requiredStatusCheckContexts: checks.map(x => x.context)}}}}};}}; - const context = {repo: {owner: '', repo: 'sample'}, serverUrl: 'https://example.invalid', runId: 1, - eventName: f.eventName || 'pull_request_target', payload: f.payload || {pull_request: {number: 1}}}; - const core = {setFailed: text => out.failures.push(text), setOutput: (name, value) => {out[name] = value;}}; - try { - if (f.recordOnly) { if (!record) throw new Error('Recorder absent'); await record(github, context, core, {env: {APPROVAL_OWNER: '-owner'}}); } - else if (f.eventOnly) await discover(github, context, core, {env: {PR_NUMBER: '', APPROVAL_OWNER: '-owner'}}); - const numbers = f.recordOnly ? [] : f.eventOnly ? JSON.parse(out.prs || '[]') : [1]; - for (const number of numbers) await execute(github, context, core, {env: {PR_NUMBER: String(number), APPROVAL_OWNER: '-owner'}}); - } catch (e) {out.errors.push(e.message);} - out.risk = out.statuses.filter(x => x.context === 'risk-gate').at(-1)?.state; - return out; -} -test('Ordinary high-risk paths pass only with exact-head clean review and required checks', async () => { - assert.equal((await run()).risk, 'success'); -}); -for (const mutation of ['stale', 'running', 'unresolved', 'malformed', 'duplicate', 'untrusted', 'human', 'timestamp', 'missing', 'boolean', 'newer']) { - test(`Reject invalid review: ${mutation}`, async () => { - const f = fixture(), c = f.comments[0]; - if (mutation === 'stale') c.body = marker({commit_sha: B}); - if (mutation === 'running') c.body = marker({status: 'running'}); - if (mutation === 'unresolved') c.body = marker({has_unresolved_findings: true}); - if (mutation === 'malformed') c.body = ''; - if (mutation === 'duplicate') c.body += marker({has_unresolved_findings: true}); - if (mutation === 'untrusted') c.performed_via_github_app.id = 54321; - if (mutation === 'human') c.user.type = 'User'; - if (mutation === 'timestamp') c.updated_at = 'invalid'; - if (mutation === 'missing') f.comments = []; - if (mutation === 'boolean') c.body = marker({has_unresolved_findings: 'false'}); - if (mutation === 'newer') f.comments.push({...c, id: 2, body: marker({status: 'running'})}); - assert.notEqual((await run(f)).risk, 'success'); - }); -} -test('Findings already resolved do not require another owner label', async () => { - const f = fixture(); f.comments[0].body = marker({has_any_findings: true}); - assert.equal((await run(f)).risk, 'success'); -}); -for (const filename of ['docs/secrets.md', 'tests/deploy/test.js', 'src/iam/policy.json', - 'src/broker_credentials.json', '.env.local', 'infra/main.tf', '.github/workflows/deploy.yml', - 'src/liveTrading.js', 'src/stop_loss.js', 'src/kelly.py', 'src/orders/run.js', - '.github/governance/policy.json', '.github/workflows/risk-gate.yml']) { - test(`Sensitive source path accepts head-bound agent clearance: ${filename}`, async () => { - const f = fixture(); f.files[0].filename = filename; - assert.equal((await run(f)).risk, 'success'); - f.pr.labels = []; - assert.notEqual((await run(f)).risk, 'success'); - }); -} -test('Renaming a sensitive file to ordinary code keeps the clearing-label requirement', async () => { - const f = fixture(); f.files[0].status = 'renamed'; f.files[0].previous_filename = 'src/secrets.js'; f.pr.labels = []; - assert.notEqual((await run(f)).risk, 'success'); -}); -for (const mutation of ['failure', 'skipped', 'neutral', 'pending', 'missing', 'wrong-app', 'stale', 'newer-failure', 'status-pending']) { - test(`Reject incomplete required checks: ${mutation}`, async () => { - const f = fixture(), c = f.checks[0]; - if (['failure', 'skipped', 'neutral'].includes(mutation)) c.conclusion = mutation; - if (mutation === 'pending') c.status = 'in_progress'; - if (mutation === 'missing') f.checks = []; - if (mutation === 'wrong-app') c.app.id = 54321; - if (mutation === 'stale') c.head_sha = B; - if (mutation === 'newer-failure') f.checks.push({...c, id: 2, conclusion: 'failure'}); - if (mutation === 'status-pending') {f.required[2].app = null; f.statuses = [{id: 2, context: c.name, state: 'pending'}];} - assert.notEqual((await run(f)).risk, 'success'); - }); -} -test('Legacy status responses without sha are bound to the immutable request ref', async () => { - const f = fixture(); f.checks = []; f.required[2].app = null; - f.statuses.push({id: 1, context: 'Unit tests', state: 'success'}); - const out = await run(f); - assert.equal(out.risk, 'success'); - assert.deepEqual(out.statusRefs, [H, H]); -}); -test('Legacy status approval rejects every non-success response', async () => { - const f = fixture(); f.checks = []; f.required[2].app = null; - f.statuses.push({id: 1, context: 'Unit tests', state: 'success'}); - assert.equal((await run(f)).risk, 'success'); - for (const state of ['failure', 'error', 'pending', undefined, 'unknown']) { - f.statuses.find(x => x.context === 'Unit tests').state = state; - assert.notEqual((await run(f)).risk, 'success'); - } -}); -test('Legacy status success cannot survive a concurrent head change', async () => { - const f = fixture(); f.checks = []; f.required[2].app = null; - f.statuses.push({id: 1, context: 'Unit tests', state: 'success'}); - assert.equal((await run(f)).risk, 'success'); - f.changed = {pr: structuredClone(f.pr)}; - f.changed.pr.head.sha = B; - const out = await run(f); - assert.notEqual(out.risk, 'success'); - assert.deepEqual(out.statusRefs, [H, H]); -}); -test('Ruleset checks are enforced alongside classic protection', async () => { - const f = fixture(); f.rules = [{type: 'required_status_checks', parameters: {required_status_checks: [{context: 'Security', integration_id: 23456}]}}]; - assert.notEqual((await run(f)).risk, 'success'); -}); -test('No discoverable required checks cannot authorize automatic approval', async () => { - const f = fixture(); f.noClassic = true; - assert.notEqual((await run(f)).risk, 'success'); -}); -test('Bad PR template cannot pass through the self-dependency exclusion', async () => { - const f = fixture(); f.pr.body = 'TODO'; assert.notEqual((await run(f)).risk, 'success'); -}); -for (const key of ['comments', 'checks', 'statuses', 'rules', 'required', 'policy', 'files', 'pr']) { - test(`API failure is closed and redacted: ${key}`, async () => { - const f = fixture(); f.apiError = key; - const out = await run(f); - assert.notEqual(out.risk, 'success'); - assert.ok(!JSON.stringify(out).includes('private diagnostic')); - }); -} -for (const key of ['pr', 'comments', 'checks', 'rules', 'required', 'files', 'statuses']) { - test(`Evidence drift between reads fails closed: ${key}`, async () => { - const f = fixture(); f.changed = {[key]: structuredClone(f[key])}; - if (key === 'pr') f.changed.pr.base.sha = 'c'.repeat(40); - if (key === 'comments') f.changed.comments[0].body = marker({has_unresolved_findings: true}); - if (key === 'checks') f.changed.checks[0].conclusion = 'failure'; - if (key === 'rules') f.changed.rules.push({type: 'required_status_checks', parameters: {required_status_checks: [{context: 'New gate'}]}}); - if (key === 'required') f.changed.required.push({context: 'New gate', app: null}); - if (key === 'files') f.changed.files[0].filename = 'src/secrets.js'; - if (key === 'statuses') f.changed.statuses.push({id: 2, context: 'Unit tests', state: 'pending'}); - assert.notEqual((await run(f)).risk, 'success'); - }); -} -test('Low-risk prose passes with clean review and checks', async () => { - const f = fixture(); f.files[0].filename = 'docs/example.md'; assert.equal((await run(f)).risk, 'success'); -}); -test('Concurrent evaluations do not share decisions', async () => { - const f = fixture(); f.comments = []; - const [good, bad] = await Promise.all([run(), run(f)]); - assert.equal(good.risk, 'success'); assert.notEqual(bad.risk, 'success'); -}); -test('Workflow refreshes on review and check events without new write permissions', () => { - for (const event of ['issue_comment:', 'check_run:', 'check_suite:', 'status:']) assert.ok(workflow.includes(event)); - assert.doesNotMatch(workflow, /checkout|secrets:|issues: write|pull-requests: write|actions: write/); -}); -test('A low-risk result cannot survive base drift', async () => { - const f = fixture(); f.files[0].filename = 'docs/example.md'; f.changed = {pr: structuredClone(f.pr)}; - f.changed.pr.base.sha = 'c'.repeat(40); - assert.notEqual((await run(f)).risk, 'success'); -}); -test('Status publication errors never expose API diagnostics', async () => { - const f = fixture(); f.apiError = 'publish'; const out = await run(f); - assert.notEqual(out.risk, 'success'); assert.ok(!JSON.stringify(out).includes('private diagnostic')); - assert.ok(out.failures.length); -}); -test('Sensitive globs escape literal dots', async () => { - const f = fixture(); f.files[0].filename = 'src/notXkey'; - assert.equal((await run(f)).risk, 'success'); -}); -test('Future review timestamps cannot authorize approval', async () => { - const f = fixture(); f.comments[0].updated_at = '2999-01-01T00:00:00Z'; - assert.notEqual((await run(f)).risk, 'success'); -}); -for (const event of ['check_run', 'check_suite', 'status']) { - test(`Check event looks up only open PRs with the current event head: ${event}`, async () => { - const f = fixture(); f.eventOnly = true; f.payload = event === 'status' ? {sha: H} : {[event]: {head_sha: H}}; - f.associated = [{number: 1, state: 'open', head: {sha: H}}, {number: 2, state: 'closed', head: {sha: H}}, - {number: 3, state: 'open', head: {sha: B}}]; - const out = await run(f); assert.equal(out.risk, 'success'); assert.equal(out.reads.pr, 2); - }); -} -test('Review comment event reads the associated PR', async () => { - const f = fixture(); f.eventOnly = true; f.payload = {issue: {number: 1}}; - assert.equal((await run(f)).risk, 'success'); -}); -test('Check event lookup failure is redacted and closed', async () => { - const f = fixture(); f.eventOnly = true; f.payload = {sha: H}; f.apiError = 'associated'; - const out = await run(f); assert.ok(out.failures.length); assert.ok(!JSON.stringify(out).includes('private diagnostic')); -}); -test('The self-produced contexts cannot claim a different App identity', async () => { - const f = fixture(); f.required[0].app = {databaseId: 54321}; - assert.notEqual((await run(f)).risk, 'success'); -}); -test('A ruleset without classic protection can authorize when every check is green', async () => { - const f = fixture(); f.noClassic = true; f.rules = [{type: 'required_status_checks', parameters: { - required_status_checks: [{context: 'Unit tests', integration_id: 23456}]}}]; - assert.equal((await run(f)).risk, 'success'); -}); -test('Owner confirmation requires a matching event and head-bound receipt', async () => { - const f = fixture(); f.files[0].filename = 'src/secrets.js'; f.pr.labels = [{name: 'owner-approved'}]; - f.events = [{id: 1, event: 'labeled', label: {name: 'owner-approved'}, actor: {login: '-owner'}}]; - f.statuses.push({id: 11, context: 'clearing-label', state: 'success', description: 'owner-approved for PR 1 at event 1 on this head'}); - assert.equal((await run(f)).risk, 'success'); - f.events.push({id: 2, event: 'unlabeled', label: {name: 'owner-approved'}, actor: {login: '-owner'}}); - assert.notEqual((await run(f)).risk, 'success'); -}); -test('Owner-label event revocation during evaluation blocks the old approval', async () => { - const f = fixture(); f.files[0].filename = 'src/secrets.js'; f.pr.labels = [{name: 'owner-approved'}]; - f.events = [{id: 1, event: 'labeled', label: {name: 'owner-approved'}, actor: {login: '-owner'}}]; - f.changed = {events: [...f.events, {id: 2, event: 'unlabeled', label: {name: 'owner-approved'}}]}; - assert.notEqual((await run(f)).risk, 'success'); -}); -test('Ordinary high-risk paths cannot use a legacy owner label to bypass review', async () => { - const f = fixture(); f.pr.labels = [{name: 'owner-approved'}]; f.comments = []; - f.events = [{id: 1, event: 'labeled', label: {name: 'owner-approved'}, actor: {login: '-owner'}}]; - assert.notEqual((await run(f)).risk, 'success'); -}); -test('Native workflow events resolve to the same PR writer', async () => { - const f = fixture(); f.eventOnly = true; f.payload = {workflow_run: {head_sha: H}}; - f.associated = [{number: 1, state: 'open', head: {sha: H}}]; - assert.equal((await run(f)).risk, 'success'); - assert.match(workflow, /group: risk-gate-\$\{\{ github.repository \}\}-\$\{\{ matrix.pr \}\}/); - assert.match(workflow, /cancel-in-progress: false/); - assert.match(workflow, /PR_NUMBER: \$\{\{ matrix.pr \}\}/); -}); -for (const mutation of ['missing', 'duplicate', 'rename', 'count']) { - test(`Incomplete file evidence blocks evaluation: ${mutation}`, async () => { - const f = fixture(); - if (mutation === 'missing') f.files = []; - if (mutation === 'duplicate') {f.files.push(f.files[0]); f.pr.changed_files = 2;} - if (mutation === 'rename') f.files[0].status = 'renamed'; - if (mutation === 'count') f.pr.changed_files = 2; - assert.notEqual((await run(f)).risk, 'success'); - }); -} - -// Adverse cases exercise the actual embedded trusted code, with fake APIs only. -test('Newest malformed trusted marker supersedes older clean review', async () => { - const f = fixture(); f.comments.push({...f.comments[0], id: 2, - updated_at: new Date(Date.now() - 1000).toISOString(), body: '/g)]; + // Two markers in one comment cannot both be the review that was run. + if (markers.length !== 1) { + return { state: 'error', description: 'Newest Codex review comment has a missing or duplicated receipt' }; + } let receipt; try { - receipt = JSON.parse(receipts[0].match[1]); + receipt = JSON.parse(markers[0][1]); } catch { return { state: 'error', description: 'Codex review receipt is not valid JSON' }; } From a678467411a3471e712cf752cdd752a927fe2099 Mon Sep 17 00:00:00 2001 From: Li SiYi Date: Fri, 6 Nov 2026 02:33:32 +0800 Subject: [PATCH 3/3] fix(governance): classify release and credential tooling as high risk Release, deployment, service-account, permission and Kubernetes paths, plus pem/key/p12/pfx files, must keep the clearing-label requirement when the classifier is unified. --- .github/workflows/risk-gate.yml | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/.github/workflows/risk-gate.yml b/.github/workflows/risk-gate.yml index 05cff84..d5bff53 100644 --- a/.github/workflows/risk-gate.yml +++ b/.github/workflows/risk-gate.yml @@ -180,9 +180,11 @@ jobs: // workflow file, and a diff alone cannot show who calls it. if (/^\.github\/scripts\//.test(path)) return true; if (/^scripts\/[^/]*(?:check|verify|validate|hygiene|lint)[^/]*\.(?:py|sh|js|cjs|mjs|ts)$/.test(p)) return true; - if (/(^|[\/_.-])(ai_service|aiservice|kelly|stop_loss|trad(?:e|es|ing)|execution|executors?|orders?|brokers?|risk|deploy(?:ment)?|secrets?|credentials?|iam|runtime|strateg(?:y|ies))([\/_.-]|$)/.test(p)) return true; + // Release, deployment, credential and live-trading tooling are named here + // because a diff alone rarely shows what a script changes at run time. + if (/(^|[\/_.-])(ai_service|aiservice|kelly|stop_loss|trad(?:e|es|ing)|execution|executors?|orders?|brokers?|risk|release|deploy(?:ment)?|secrets?|credentials?|iam|service_account|serviceaccount|permission|role_binding|k8s|kubernetes|runtime|strateg(?:y|ies))([\/_.-]|$)/.test(p)) return true; return /(^|\/)(dockerfile(?:\..*)?|docker-compose[^/]*|compose\.ya?ml|vercel\.json|wrangler\.(?:toml|jsonc?)|firebase\.json)$/.test(p) - || /\.(?:tf|tfvars)$/.test(p); + || /\.(?:tf|tfvars|pem|key|p12|pfx)$/.test(p); } // A repin keeps the action and changes only its ref. Swapping in a // different action is a different change and takes the normal tier.