From e8ee76d1fbfe37d5ad81bf81a100d9ce8befdabf Mon Sep 17 00:00:00 2001 From: Codex Date: Thu, 5 Nov 2026 22:19:55 +0800 Subject: [PATCH 1/2] ci: Propose automated approval for reviewed high-risk paths --- .github/workflows/governance-shadow-tests.yml | 2 +- .github/workflows/risk-gate.yml | 289 +++++++++++++++--- AGENTS.md | 7 +- PROPOSAL.md | 174 +++++++++++ README.md | 1 + README.zh-CN.md | 1 + scripts/check_style.py | 2 +- scripts/test-risk-gate.cjs | 235 ++++++++++++++ scripts/test_style.py | 12 + 9 files changed, 673 insertions(+), 50 deletions(-) create mode 100644 PROPOSAL.md create mode 100644 scripts/test-risk-gate.cjs diff --git a/.github/workflows/governance-shadow-tests.yml b/.github/workflows/governance-shadow-tests.yml index 2057b1a..2dc23c7 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 + - run: node --test scripts/test-governance-shadow.cjs scripts/test-risk-gate.cjs diff --git a/.github/workflows/risk-gate.yml b/.github/workflows/risk-gate.yml index 1b2fdce..a1a4c82 100644 --- a/.github/workflows/risk-gate.yml +++ b/.github/workflows/risk-gate.yml @@ -3,6 +3,16 @@ name: PR risk gate on: pull_request_target: types: [opened, reopened, synchronize, edited, labeled, unlabeled, ready_for_review] + issue_comment: + types: [created, edited, deleted] + 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: @@ -10,19 +20,59 @@ on: required: true type: string -# No checkout, PR code execution, secret access, label writes or merge permission. +# Trusted workflow only: no candidate code, label writes or merge permission. permissions: contents: read pull-requests: read issues: read + checks: read statuses: write -concurrency: - group: risk-gate-${{ github.event.pull_request.number || inputs.pr_number }} - cancel-in-progress: true - jobs: + 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')) + runs-on: ubuntu-latest + timeout-minutes: 5 + permissions: + contents: read + pull-requests: 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); + 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 + if: 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 runs-on: ubuntu-latest timeout-minutes: 5 @@ -30,18 +80,47 @@ jobs: - name: Publish risk-gate and PR template checks on the PR head uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8 env: - PR_NUMBER: ${{ inputs.pr_number }} + PR_NUMBER: ${{ matrix.pr }} with: script: | - // Executed only by the trusted base-branch workflow. Never load PR code. - const number = context.payload.pull_request?.number || Number(process.env.PR_NUMBER); - if (!Number.isSafeInteger(number) || number < 1) throw new Error('Invalid PR number'); + // Only trusted base/default-branch workflow code executes; never load PR code. 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}`; - let sha; - const status = (name, state, description) => github.rest.repos.createCommitStatus({ - ...repo, sha, context: name, state, description, target_url: target, - }); + const number = Number(process.env.PR_NUMBER); + // Explicit owner-only globs: match normalized lowercase snake_case paths. + // * never crosses /; ** does. No docs/tests exemption applies to this list. + const ownerOnlyGlobs = [ + '**/*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 ownerOnly = path => ownerOnlyGlobs.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/workflows/') || /(^|\/)codeowners$/.test(p)) return true; @@ -61,39 +140,157 @@ jobs: && present(/what changed|summary|problem|changes|变更|改动/i) && present(/how verified|verif|validat|test(?:ing| plan)?|验证|测试/i); } - try { - const { data: pr } = await github.rest.pulls.get({ ...repo, pull_number: number }); - sha = pr.head.sha; - await status('risk-gate', 'pending', 'Evaluating changed paths and owner approval'); - await status('pr-template', 'pending', 'Checking AI, change and verification fields'); - 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'); - const risky = files.some(file => [file.filename, file.previous_filename].filter(Boolean).some(highRisk)); - let approved = false; - if (pr.labels.some(label => label.name === 'owner-approved')) { - const events = await github.paginate(github.rest.issues.listEvents, { ...repo, issue_number: number, per_page: 100 }); - const latest = events.filter(event => ['labeled', 'unlabeled'].includes(event.event) - && event.label?.name === 'owner-approved').sort((a, b) => b.id - a.id)[0]; - // A shared GitHub identity cannot distinguish the owner from their AI agents. - approved = latest?.event === 'labeled' && latest.actor?.login === 'Pigbibi'; - } - 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'); + + 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; + } + 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}); + } } - const allowed = !risky || approved; - await status('risk-gate', allowed ? 'success' : 'failure', !risky ? 'Low-risk paths: no owner approval needed' - : approved ? 'High-risk paths: owner-approved label verified' : 'High-risk paths: needs-owner-approval; owner must add owner-approved'); - const templateOK = completeTemplate(pr.body); - await status('pr-template', templateOK ? 'success' : 'failure', templateOK - ? 'AI, change and verification fields are filled in' : 'Fill in AI assistance, What changed and How verified sections'); - if (!allowed || !templateOK) core.setFailed('Required PR governance checks did not pass'); - } catch (error) { - if (sha) { - await status('risk-gate', 'error', 'Evaluation failed; inspect the trusted workflow run'); - await status('pr-template', 'error', 'Evaluation failed; inspect the trusted workflow run'); + 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}; + } + function checksGreen(evidence, sha, templateOK) { + return evidence.required.every(required => { + // These two contexts are produced here. 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'].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].sha === sha && statuses[0].state === 'success')); + }); + } + { + 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(); } + }; + 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(); + 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 status('risk-gate', 'pending', 'Evaluating current review, paths 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.status === 'renamed' && !file.previous_filename))) fail(); + return files; + }; + const files = await readFiles(); + const paths = files.flatMap(file => [file.filename, file.previous_filename].filter(Boolean)); + const protectedPath = paths.some(ownerOnly); + const risky = protectedPath || paths.some(highRisk); + let approved = false; + let before, after; + if (protectedPath) { + const readEvents = async () => github.paginate(github.rest.issues.listEvents, + {...repo, issue_number: number, per_page: 100}); + before = await readEvents(); + if (pr.labels.some(label => label.name === 'owner-approved')) { + const events = before; + const latest = events.filter(event => ['labeled', 'unlabeled'].includes(event.event) + && event.label?.name === 'owner-approved').sort((a, b) => b.id - a.id)[0]; + // A shared GitHub identity cannot distinguish the owner from their AI agents. + approved = latest?.event === 'labeled' && latest.actor?.login === 'Pigbibi'; + } + after = await readEvents(); + } else if (risky) { + 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'].includes(entry.context)), + }); + before = await readEvidence(); + approved = templateOK && cleanReview(before.comments, policy, sha) && checksGreen(before, sha, templateOK); + 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(); + const allowed = !risky || approved; + await status('risk-gate', allowed ? 'success' : 'failure', !risky ? 'Low-risk paths: no owner approval needed' + : protectedPath ? (approved ? 'Owner-only paths: existing owner label verified' : 'Owner-only paths: owner approval required') + : approved ? 'Exact-head clean review and required checks verified' : 'Automatic approval blocked: review or required checks incomplete'); + if (!allowed || !templateOK) core.setFailed('Required PR governance checks did not pass'); + } catch { + // Never expose API errors, comment bodies, paths, identities or tokens. + if (hash(sha)) { + try { + await status('risk-gate', 'error', 'Evidence unavailable or changed; approval blocked'); + await status('pr-template', 'error', 'Evaluation incomplete; approval blocked'); + } catch { /* Status transport failure must not expose API diagnostics. */ } + } + core.setFailed('Governance evidence unavailable; approval blocked'); } - throw error; } diff --git a/AGENTS.md b/AGENTS.md index 231776a..d06bab8 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -39,7 +39,8 @@ Organization documentation rules are checked by `python scripts/check_style.py`, - Developer documentation is English by default; Chinese siblings use `.zh-CN.md` and exist only where a reader needs them. - New document names are lowercase with hyphens. `README.md`, `AGENTS.md`, - `CLAUDE.md` and other tool-defined names keep their spelling. + `CLAUDE.md` and other tool-defined names keep their spelling. The root + `PROPOSAL.md` keeps the filename explicitly requested for this governance decision. - Temporary internal task reports, retrospectives, handoffs and progress snapshots stay outside Git. Sanitized research baselines, design decisions, financial and accounting contracts, provenance/PIT/license evidence, and @@ -89,7 +90,9 @@ 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` and `needs-owner-approval` for protected paths. Classification labels describe risk; the path-based `risk-gate` determines whether approval is required. -- AI-only development uses required automated checks, template validation and independent AI review when available, with zero required human or code-owner approvals. High-risk paths require the owner's `owner-approved` label; agents must never add or remove it. +- AI-only development uses required automated checks, template validation and independent AI review when available, with zero required human or code-owner approvals. Ordinary high-risk paths need no manual label only when the trusted Codex App's latest `codex-review-state:v1` marker is completed for the exact current head with `has_unresolved_findings=false`, and every required check is green. Missing, stale, malformed, unreadable or changing evidence blocks automatic approval. +- The owner-only globs in [PROPOSAL.md](PROPOSAL.md#owner-only-paths) retain the existing owner-applied `owner-approved` requirement. They cover secrets, IAM, deployments, trading/execution and broker credentials, plus Kelly, stop-loss and this gate's trust configuration. Renames check both paths; docs and tests cannot exempt these globs. Agents must never add or remove approval labels. +- Automated path approval satisfies only `risk-gate`; it grants no merge, deployment, trading or configuration authority. The reusable governance shadow observer remains report-only. This proposal affects this repository only until separately adopted elsewhere. - 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. ## Scope and authorization diff --git a/PROPOSAL.md b/PROPOSAL.md new file mode 100644 index 0000000..19ba252 --- /dev/null +++ b/PROPOSAL.md @@ -0,0 +1,174 @@ +# Automated approval for ordinary high-risk paths + +Status: proposal for owner decision. Opening this Draft PR does not adopt it. + +## Problem and rationale + +The current path gate requires an owner-applied label for every high-risk +change, including ordinary strategy, risk and runtime code. It ignores an +already completed, clean Codex review and passing required checks. The proposed +policy removes that repeated manual step while keeping owner control over +sensitive operations and the gate's trust configuration. + +## Decision rule + +Low-risk paths retain their existing behavior. For other paths, inspect both +current and previous filenames before any docs or tests exemption. + +An ordinary high-risk change passes `risk-gate` only when all of these hold: + +1. The newest marker from a trusted Codex App is exactly one well-formed + `codex-review-state:v1` comment marker. The App IDs come from the existing + policy at the immutable trusted base revision, never the PR's candidate tree. +2. Its `code_review.commit_sha` equals the current PR head, its status is + `completed`, and `has_unresolved_findings` is the boolean `false`. + `has_any_findings` must also be a boolean; resolved findings are permitted. + A newer running, malformed or unresolved marker supersedes an older clean one. +3. All required contexts from classic branch protection and applicable rulesets + are discovered and successful for the same head. Missing protection evidence, + missing results, skipped/neutral checks, wrong App bindings and API errors + block automatic approval. Legacy statuses cannot prove an App-bound identity + and therefore cannot satisfy such a requirement. +4. No changed or previous path matches the owner-only list below. An old owner + label cannot substitute for missing automated evidence on ordinary high-risk + paths. + +`risk-gate` is the output of this decision, so it cannot be its own prerequisite. +The locally validated `pr-template` result is published before the required-check +read. These are the only two self-produced contexts; their expected App binding +is checked. Every other required job stays mandatory. Native branch protection +and the required-check list stay unchanged. + +## Owner-only paths + +These are the exact implemented globs. Paths are converted from camelCase to +snake_case and lowercased before matching. `*` stays within one path segment; +`**/` matches zero or more directory segments. Broad credential, trading and +infrastructure families intentionally keep ambiguous files under owner control. +There is no docs, tests, examples or fixtures exemption for this list. + +```text +**/*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 +``` + +The list includes the gate itself, its trust policy and CODEOWNERS because a PR +must not grant itself authority by modifying those files. Broker, order and +execution paths stay owner-only even when a filename does not say live. Kelly +and stop-loss paths stay protected; this proposal enables no trading feature. +Other workflow changes still require the automated high-risk rule. + +Owner-only paths retain the existing verified owner-label event predicate. +This proposal neither strengthens that legacy human-identity proof nor allows +an agent to add/remove a label. A label is not operational authorization to +execute a deployment or trade. + +## Execution and evidence freshness + +The privileged workflow executes trusted workflow code only. It adds read-only +check access; status publication remains its only write capability. It never +checks out or executes a PR, reads secrets, writes labels, merges or deploys. +The existing shadow observer and its report-only policy are unchanged. + +PR/comment/check/status events refresh evidence. The existing native CI workflows +also trigger reevaluation through `workflow_run`, because GitHub Actions check +events do not reliably trigger other workflows. The explicit workflow-name list +must be maintained if required native workflows are renamed or added. A manual +workflow dispatch remains an existing recovery entry, with no new schedule. + +A read-only discovery job resolves every event to PR numbers. All event types +then share one concurrency group per PR without cancellation. Replaced pending +events evaluate the same PR; unrelated PRs never replace each other in the queue. +Each evaluation reconstructs evidence after restart; no persisted approval is +reused. It rereads PR head/base/body/labels, changed paths, owner events or +review/check/protection evidence before publishing. A mismatch or read failure +blocks the decision. API exception text and user bodies are never logged. + +The final status is a snapshot, not a transaction with GitHub's mutable evidence. +Events refresh it and native branch protection still enforces checks at merge. +No local test proves hosted token permissions, event delivery or runtime adoption. +If protection APIs are unreadable to the installed token, the gate blocks; +this Draft PR requests no settings or permission changes outside workflow scope. + +## Scope and adoption + +Only this organization's metadata repository changes. Other repositories and +callers pinned to older revisions do not adopt this proposal automatically. +The workflow diff, local rules, tests and this decision document form one review +unit. The root filename `PROPOSAL.md` is explicitly requested for this decision; +the naming checker permits only this root path, while nested uppercase proposal +filenames remain invalid. No security or identifier rule is relaxed. + +This PR stays Draft for the owner's decision and the established review bot. +The current base-branch risk gate still governs this PR. Opening it does not +satisfy its own approval gate and does not authorize merge, undraft, labels, +repository/organization settings, deployment or live activation. + +## Validation + +`node --test scripts/test-risk-gate.cjs` executes the actual embedded workflow +script with fake native API data. It checks the positive decision, invalid or +stale review markers, required checks and App identities, protected paths, +renames, event routing, read failures, redaction, concurrent evaluations and +snapshot drift. The existing CI test job runs these tests alongside the shadow +engine tests. Run all available Python checker/scanner tests and the phase-2 docs +checker before publication. The PR description records the actual results. + +## Rollback + +Through a separately authorized reviewed change, restore the previous +`risk-gate.yml` and the corresponding AGENTS approval wording, remove the added +risk tests from the CI command and revert this proposal's document/name-only +changes. Keep every existing native required check, the owner-only controls and +branch protection enabled. Reevaluate affected open PRs under the restored gate +using its existing workflow-dispatch input; do not reuse successful statuses +from the proposed policy as proof of legacy owner approval. No deployment, +settings edit, history rewrite or label change is part of this Draft PR. diff --git a/README.md b/README.md index 703b654..33263c9 100644 --- a/README.md +++ b/README.md @@ -16,6 +16,7 @@ Organization-wide files for QuantStrategyLab repositories. | [Secret scanning](docs/secret-scanning.md) | Publication guard, PR check and weekly all-branch scan templates. | | `profile/README.md` | Text shown on the organization's GitHub profile page. | | `PULL_REQUEST_TEMPLATE.md` | Default pull request template for repositories without their own. | +| [Automated risk approval proposal](PROPOSAL.md) | Approval conditions, owner-only paths and rollback. | | `AGENTS.md` | AI contribution and maintenance rules for this repository. | ## Contributing diff --git a/README.zh-CN.md b/README.zh-CN.md index e632add..6bbfc3a 100644 --- a/README.zh-CN.md +++ b/README.zh-CN.md @@ -16,6 +16,7 @@ QuantStrategyLab 各仓库共用的组织级文件。 | [密钥扫描](docs/secret-scanning.md) | 发布前扫描、PR 检查和每周全分支扫描模板。 | | `profile/README.md` | 组织 GitHub 主页上显示的介绍。 | | `PULL_REQUEST_TEMPLATE.md` | 没有自带模板的仓库默认使用的 pull request 模板。 | +| [风险门禁自动批准提案](PROPOSAL.md) | 自动批准条件、保留 owner 审批的路径和回退方式。 | | `AGENTS.md` | 本仓库的 AI 参与与维护规则。 | ## 参与贡献 diff --git a/scripts/check_style.py b/scripts/check_style.py index b4d63d0..0d2eadb 100755 --- a/scripts/check_style.py +++ b/scripts/check_style.py @@ -351,7 +351,7 @@ def check_naming(root: Path, report: Report) -> None: f"{rel}: English is the base name (section 4); .en.md is not a suffix", ) - if core not in CONVENTIONAL_UPPER and core not in GITHUB_TEMPLATE_NAMES: + if rel != "PROPOSAL.md" and core not in CONVENTIONAL_UPPER and core not in GITHUB_TEMPLATE_NAMES: if core != core.lower(): report.add("naming", f"{rel}: file name must be lowercase (section 7)") if "_" in core: diff --git a/scripts/test-risk-gate.cjs b/scripts/test-risk-gate.cjs new file mode 100644 index 0000000..1f1e67e --- /dev/null +++ b/scripts/test-risk-gate.cjs @@ -0,0 +1,235 @@ +const {test} = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); +const workflow = fs.readFileSync(process.env.RISK_WORKFLOW || path.join(__dirname, '../.github/workflows/risk-gate.yml'), 'utf8'); +const script = workflow.split(' script: |\n').at(-1).split('\n').map(x => x.slice(12)).join('\n'); +// Substitute only the legacy identity literal so fixtures contain no real owner. +const testScript = script.replace(/(latest\.actor\?\.login === ')[^']+(')/, '$1-owner$2'); +assert.notEqual(testScript, script, 'Legacy owner predicate must remain present'); +const execute = new (Object.getPrototypeOf(async function(){}).constructor)('github', 'context', 'core', 'process', testScript); +const discoveryScript = workflow.split(' script: |\n')[1].split('\n\n evaluate:')[0].split('\n').map(x => x.slice(12)).join('\n'); +const discover = new (Object.getPrototypeOf(async function(){}).constructor)('github', 'context', 'core', 'process', discoveryScript); +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: [], + 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'}], + statuses: [], 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: []}; + 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'}, + issues: {listComments: 'comments', listEvents: 'events'}, + checks: {listForRef: 'checks'}, repos: {listCommitStatusesForRef: 'statuses', + getBranchRules: 'rules', listPullRequestsAssociatedWithCommit: 'associated', + getContent: async () => ({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 => 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, + payload: f.payload || {pull_request: {number: 1}}}; + const core = {setFailed: text => out.failures.push(text), setOutput: (name, value) => {out[name] = value;}}; + try { + if (f.eventOnly) await discover(github, context, core, {env: {PR_NUMBER: ''}}); + const numbers = f.eventOnly ? JSON.parse(out.prs || '[]') : [1]; + for (const number of numbers) await execute(github, context, core, {env: {PR_NUMBER: String(number)}}); + } 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(`Owner-only path cannot use automatic approval: ${filename}`, async () => { + const f = fixture(); f.files[0].filename = filename; + assert.notEqual((await run(f)).risk, 'success'); + }); +} +test('Renaming an owner-only file to ordinary code keeps owner approval', async () => { + const f = fixture(); f.files[0].status = 'renamed'; f.files[0].previous_filename = 'src/secrets.js'; + 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', sha: H}];} + assert.notEqual((await run(f)).risk, 'success'); + }); +} +test('Required status contexts also need an exact-head success', async () => { + const f = fixture(); f.checks = []; f.required[2].app = null; + f.statuses = [{id: 1, context: 'Unit tests', sha: H, state: 'success'}]; + assert.equal((await run(f)).risk, 'success'); +}); +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', sha: H}); + assert.notEqual((await run(f)).risk, 'success'); + }); +} +test('Low-risk prose keeps existing behavior', 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('Owner 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 two 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-only paths retain the existing owner-label event predicate', 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'}}]; + 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'); + }); +} diff --git a/scripts/test_style.py b/scripts/test_style.py index 675b791..b9ab02d 100644 --- a/scripts/test_style.py +++ b/scripts/test_style.py @@ -99,6 +99,18 @@ def test_en_suffix_is_flagged(self): check_style.check_naming(self.root, report) self.assertTrue(any(".en.md is not a suffix" in m for m in self.messages(report, "naming"))) + def test_root_proposal_has_its_owner_requested_name(self): + self.write("PROPOSAL.md", "# Proposed governance change\n") + report = self.report() + check_style.check_naming(self.root, report) + self.assertEqual(self.messages(report, "naming"), []) + + def test_nested_proposal_still_requires_lowercase(self): + self.write("docs/PROPOSAL.md", "# x\n") + report = self.report() + check_style.check_naming(self.root, report) + self.assertTrue(any("lowercase" in m for m in self.messages(report, "naming"))) + def test_uppercase_convention_is_allowed(self): self.write("SECURITY.md", "# x\n") report = self.report() From e40b3e29a05002a0c8f71e60e36fb5d8195ec4c8 Mon Sep 17 00:00:00 2001 From: Codex Date: Thu, 5 Nov 2026 23:20:12 +0800 Subject: [PATCH 2/2] fix: Bind legacy statuses to the immutable request ref --- .github/workflows/risk-gate.yml | 3 ++- scripts/test-risk-gate.cjs | 36 +++++++++++++++++++++++++++------ 2 files changed, 32 insertions(+), 7 deletions(-) diff --git a/.github/workflows/risk-gate.yml b/.github/workflows/risk-gate.yml index a1a4c82..c5436be 100644 --- a/.github/workflows/risk-gate.yml +++ b/.github/workflows/risk-gate.yml @@ -203,7 +203,7 @@ jobs: 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].sha === sha && statuses[0].state === 'success')); + && (!statuses.length || statuses[0].state === 'success'); }); } { @@ -266,6 +266,7 @@ jobs: ...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}), + // The immutable request ref binds legacy statuses; responses have no sha field. statuses: (await github.paginate(github.rest.repos.listCommitStatusesForRef, {...repo, ref: sha, per_page: 100})) .filter(entry => !['risk-gate', 'pr-template'].includes(entry.context)), }); diff --git a/scripts/test-risk-gate.cjs b/scripts/test-risk-gate.cjs index 1f1e67e..0d6c09c 100644 --- a/scripts/test-risk-gate.cjs +++ b/scripts/test-risk-gate.cjs @@ -28,7 +28,7 @@ function marker(changes = {}) { status: 'completed', has_any_findings: false, has_unresolved_findings: false, ...changes}})} -->`; } async function run(f = fixture()) { - const out = {statuses: [], errors: [], reads: {}, failures: []}; + const out = {statuses: [], errors: [], reads: {}, failures: [], statusRefs: []}; const clone = x => structuredClone(x); const read = key => { out.reads[key] = (out.reads[key] || 0) + 1; @@ -41,7 +41,10 @@ async function run(f = fixture()) { getBranchRules: 'rules', listPullRequestsAssociatedWithCommit: 'associated', getContent: async () => ({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 => read(method), + 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, @@ -101,14 +104,35 @@ for (const mutation of ['failure', 'skipped', 'neutral', 'pending', 'missing', ' 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', sha: H}];} + 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('Required status contexts also need an exact-head success', async () => { +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 = [{id: 1, context: 'Unit tests', sha: H, state: 'success'}]; + f.statuses = [{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 = [{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[0].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 = [{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}]}}]; @@ -138,7 +162,7 @@ for (const key of ['pr', 'comments', 'checks', 'rules', 'required', 'files', 'st 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', sha: H}); + if (key === 'statuses') f.changed.statuses.push({id: 2, context: 'Unit tests', state: 'pending'}); assert.notEqual((await run(f)).risk, 'success'); }); }