diff --git a/.github/workflows/coverage-baseline-repair.yml b/.github/workflows/coverage-baseline-repair.yml index 23190ae5..2d1b30f7 100644 --- a/.github/workflows/coverage-baseline-repair.yml +++ b/.github/workflows/coverage-baseline-repair.yml @@ -53,7 +53,7 @@ jobs: const fullCommit = /^[0-9a-f]{40}$/; const pullNumber = Number(eventPull?.number); const headSha = eventPull?.head?.sha; - const baseSha = eventPull?.base?.sha; + const baseRef = eventPull?.base?.ref; const mergeCommitSha = eventPull?.merge_commit_sha; if ( context.payload.repository?.full_name !== @@ -62,27 +62,44 @@ jobs: !Number.isSafeInteger(pullNumber) || pullNumber <= 0 || !fullCommit.test(headSha || '') || - !fullCommit.test(baseSha || '') || + baseRef !== 'main' || !fullCommit.test(mergeCommitSha || '') ) { throw new Error('closed PR event has an invalid repository or revision identity'); } + // REST base.sha follows the live base branch and can move after + // merge. Bind the closed event's stable PR head snapshot and merge + // facts, then authorize the target through main containment. + function isStableMergedPRIdentity( + currentPull, + pullNumber, + headSha, + mergeCommitSha, + ) { + return ( + currentPull?.number === pullNumber && + currentPull.state === 'closed' && + currentPull.merged === true && + typeof currentPull.merged_at === 'string' && + currentPull.merged_at.length > 0 && + currentPull.base?.ref === 'main' && + currentPull.head?.sha === headSha && + currentPull.merge_commit_sha === mergeCommitSha + ); + } + const {data: currentPull} = await github.rest.pulls.get({ owner, repo, pull_number: pullNumber, }); - if ( - currentPull.number !== pullNumber || - currentPull.state !== 'closed' || - currentPull.merged !== true || - !currentPull.merged_at || - currentPull.base.ref !== 'main' || - currentPull.base.sha !== baseSha || - currentPull.head.sha !== headSha || - currentPull.merge_commit_sha !== mergeCommitSha - ) { + if (!isStableMergedPRIdentity( + currentPull, + pullNumber, + headSha, + mergeCommitSha, + )) { throw new Error(`PR #${pullNumber} no longer matches the merged-main event`); } @@ -174,7 +191,6 @@ jobs: source: 'merged_pr', pull_number: String(pullNumber), head_sha: headSha, - base_sha: baseSha, merge_commit_sha: mergeCommitSha, source_run_id: String(context.runId), }, @@ -338,6 +354,27 @@ jobs: ); } + // The dispatcher froze the stable PR head snapshot in this payload. + // Do not re-read mutable base.sha; bind the head and stable merge + // facts, then prove protected-main containment below. + function isStableMergedPRIdentity( + currentPull, + pullNumber, + headSha, + mergeCommitSha, + ) { + return ( + currentPull?.number === pullNumber && + currentPull.state === 'closed' && + currentPull.merged === true && + typeof currentPull.merged_at === 'string' && + currentPull.merged_at.length > 0 && + currentPull.base?.ref === 'main' && + currentPull.head?.sha === headSha && + currentPull.merge_commit_sha === mergeCommitSha + ); + } + let targetSha; if (context.eventName === 'repository_dispatch') { const payload = context.payload.client_payload || {}; @@ -349,13 +386,11 @@ jobs: const rawPullNumber = String(payload.pull_number || ''); const pullNumber = Number(rawPullNumber); const headSha = payload.head_sha; - const baseSha = payload.base_sha; targetSha = payload.merge_commit_sha; if ( !/^[1-9][0-9]*$/.test(rawPullNumber) || !Number.isSafeInteger(pullNumber) || !fullCommit.test(headSha || '') || - !fullCommit.test(baseSha || '') || !fullCommit.test(targetSha || '') ) { throw new Error('coverage-baseline-repair payload has an invalid PR identity'); @@ -365,16 +400,12 @@ jobs: repo, pull_number: pullNumber, }); - if ( - currentPull.number !== pullNumber || - currentPull.state !== 'closed' || - currentPull.merged !== true || - !currentPull.merged_at || - currentPull.base.ref !== 'main' || - currentPull.base.sha !== baseSha || - currentPull.head.sha !== headSha || - currentPull.merge_commit_sha !== targetSha - ) { + if (!isStableMergedPRIdentity( + currentPull, + pullNumber, + headSha, + targetSha, + )) { throw new Error( `repair payload no longer matches merged PR #${pullNumber}`, ); diff --git a/docs/automation.md b/docs/automation.md index e2efc038..87f0c911 100644 --- a/docs/automation.md +++ b/docs/automation.md @@ -216,8 +216,11 @@ repairing the producer. Never add a prefix `restore-keys` fallback to this path. `Coverage Baseline Repair` is the independent safety net for every merged PR. Its base-owned `pull_request_target: closed` job never checks out or executes PR -content: it binds the closed event to the current merged PR, verifies the exact -head/base/merge SHA and `main` containment, then emits only a +content: it binds the closed event's PR number and stable head SHA to the +current merged-PR facts (`merged_at`, `base.ref`, and `merge_commit_sha`) and +proves that merge commit is contained in `main`. It deliberately does not +compare REST `base.sha`, because that field follows the live base branch and +can move after the merge. Only then does it emit a `coverage-baseline-repair` repository dispatch. Workflow-skip directives alone do not suppress `pull_request_target`, subject to GitHub's separate security-sensitive branch-name restriction described above. The low-trust diff --git a/docs/ci-pr-gates.md b/docs/ci-pr-gates.md index 1b64ea74..00d043a7 100644 --- a/docs/ci-pr-gates.md +++ b/docs/ci-pr-gates.md @@ -309,9 +309,12 @@ merged SHA even if a human or integration changes the merge message after PR checks finish. Skip directives alone do not suppress `pull_request_target`, subject to GitHub's separate security-sensitive branch-name restriction above. That job executes no PR code and only dispatches after binding the exact -closed/merged PR head, base, merge SHA, and `main` containment. Because GitHub -makes default-branch caches read-only to `pull_request_target`, the dispatcher first -waits up to one minute for a run from the exact protected +closed-event PR number and stable head SHA to merged-PR facts +(`merged_at`, `base.ref`, and `merge_commit_sha`) and proving `main` +containment. It does not compare the later REST `base.sha`, which follows the +live base branch after merge. Because GitHub makes default-branch caches +read-only to `pull_request_target`, the dispatcher first waits up to one minute +for a run from the exact protected `.github/workflows/ci.yml` workflow and exits when that normal producer exists. A successful main CI hard-verifies the exact key itself. A completed non-success run starts a separate base-owned `workflow_run` dispatcher, which diff --git a/test/scripts/coverage_baseline_repair_identity_test.go b/test/scripts/coverage_baseline_repair_identity_test.go new file mode 100644 index 00000000..698fed25 --- /dev/null +++ b/test/scripts/coverage_baseline_repair_identity_test.go @@ -0,0 +1,145 @@ +// Copyright 2026 Alibaba Group +// Licensed under the Apache License, Version 2.0 (the "License"); + +package scripts_test + +import ( + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + + "gopkg.in/yaml.v3" +) + +const stableMergedPRIdentityHelper = `function isStableMergedPRIdentity( + currentPull, + pullNumber, + headSha, + mergeCommitSha, +) { + return ( + currentPull?.number === pullNumber && + currentPull.state === 'closed' && + currentPull.merged === true && + typeof currentPull.merged_at === 'string' && + currentPull.merged_at.length > 0 && + currentPull.base?.ref === 'main' && + currentPull.head?.sha === headSha && + currentPull.merge_commit_sha === mergeCommitSha + ); +}` + +func TestCoverageBaselineRepairUsesStableMergedPRIdentity(t *testing.T) { + t.Parallel() + + node, err := exec.LookPath("node") + if err != nil { + t.Skip("node is required to verify merged-PR repair identity semantics") + } + root, err := filepath.Abs(filepath.Join("..", "..")) + if err != nil { + t.Fatalf("resolve repository root: %v", err) + } + workflowPath := filepath.Join( + root, + ".github", + "workflows", + "coverage-baseline-repair.yml", + ) + data, err := os.ReadFile(workflowPath) + if err != nil { + t.Fatalf("read %s: %v", workflowPath, err) + } + workflow := string(data) + + var parsedWorkflow any + if err := yaml.Unmarshal(data, &parsedWorkflow); err != nil { + t.Fatalf("parse %s: %v", workflowPath, err) + } + var scripts []string + collectGitHubScripts(parsedWorkflow, &scripts) + helperCount := 0 + checkCount := 0 + for _, script := range scripts { + helperCount += strings.Count(script, stableMergedPRIdentityHelper) + checkCount += strings.Count(script, "!isStableMergedPRIdentity(") + } + if helperCount != 2 { + t.Fatalf("stable merged-PR identity helper occurrences = %d, want dispatcher and producer", helperCount) + } + if checkCount != 2 { + t.Fatalf("stable merged-PR identity checks = %d, want dispatcher and producer", checkCount) + } + for _, forbidden := range []string{ + "currentPull.base.sha", + "const baseSha =", + "base_sha", + } { + if strings.Contains(workflow, forbidden) { + t.Errorf("merged-PR repair must not bind mutable Ref SHA via %q", forbidden) + } + } + for _, want := range []string{ + "const baseRef = eventPull?.base?.ref;", + "baseRef !== 'main'", + "const headSha = eventPull?.head?.sha;", + "const headSha = payload.head_sha;", + "head_sha: headSha", + "currentPull.head?.sha === headSha", + "currentPull.merge_commit_sha === mergeCommitSha", + "await requireMainContainment(mergeCommitSha)", + "await requireMainContainment(targetSha)", + } { + if !strings.Contains(workflow, want) { + t.Errorf("merged-PR repair missing stable identity guard %q", want) + } + } + + verification := stableMergedPRIdentityHelper + ` +const pullNumber = 1077; +const headSha = '1'.repeat(40); +const mergeCommitSha = '2'.repeat(40); +const validPull = { + number: pullNumber, + state: 'closed', + merged: true, + merged_at: '2026-08-25T04:55:30Z', + base: {ref: 'main', sha: '3'.repeat(40)}, + head: {sha: headSha}, + merge_commit_sha: mergeCommitSha, +}; +const cases = [ + ['base ref advanced after event', {...validPull, base: {...validPull.base, sha: '4'.repeat(40)}}, true], + ['base sha omitted by projection', {...validPull, base: {ref: 'main'}}, true], + ['wrong pull number', {...validPull, number: pullNumber + 1}, false], + ['open pull request', {...validPull, state: 'open'}, false], + ['not merged', {...validPull, merged: false}, false], + ['missing merged at', {...validPull, merged_at: null}, false], + ['empty merged at', {...validPull, merged_at: ''}, false], + ['non-string merged at', {...validPull, merged_at: 1}, false], + ['wrong base ref', {...validPull, base: {...validPull.base, ref: 'release'}}, false], + ['missing base ref', {...validPull, base: {}}, false], + ['missing head', {...validPull, head: null}, false], + ['wrong head sha', {...validPull, head: {sha: '5'.repeat(40)}}, false], + ['wrong merge sha', {...validPull, merge_commit_sha: '6'.repeat(40)}, false], + ['null pull', null, false], +]; +for (const [name, currentPull, want] of cases) { + const got = isStableMergedPRIdentity( + currentPull, + pullNumber, + headSha, + mergeCommitSha, + ); + if (got !== want) { + throw new Error(name + ': got ' + got + ', want ' + want); + } +} +` + command := exec.Command(node, "-e", verification) + if output, runErr := command.CombinedOutput(); runErr != nil { + t.Fatalf("stable merged-PR identity verification failed: %v\n%s", runErr, output) + } +} diff --git a/test/scripts/coverage_workflow_contract_test.go b/test/scripts/coverage_workflow_contract_test.go index 047569ea..facc8b8d 100644 --- a/test/scripts/coverage_workflow_contract_test.go +++ b/test/scripts/coverage_workflow_contract_test.go @@ -511,12 +511,15 @@ func TestCoverageBaselineRepairContract(t *testing.T) { "pull-requests: read", "actions/github-script@f28e40c7f34bde8b3046d885e986cb6290c5673b", "context.payload.repository?.default_branch !== 'main'", - "currentPull.state !== 'closed'", - "currentPull.merged !== true", - "currentPull.base.ref !== 'main'", - "currentPull.base.sha !== baseSha", - "currentPull.head.sha !== headSha", - "currentPull.merge_commit_sha !== mergeCommitSha", + "currentPull.state === 'closed'", + "currentPull.merged === true", + "typeof currentPull.merged_at === 'string'", + "const baseRef = eventPull?.base?.ref;", + "baseRef !== 'main'", + "currentPull.base?.ref === 'main'", + "isStableMergedPRIdentity(", + "currentPull.head?.sha === headSha", + "currentPull.merge_commit_sha === mergeCommitSha", "for (let attempt = 1; attempt <= 6; attempt += 1)", "setTimeout(resolve, 5000)", "basehead: `${targetSha}...${branch.commit.sha}`", @@ -544,7 +547,6 @@ func TestCoverageBaselineRepairContract(t *testing.T) { "source: 'merged_pr'", "pull_number: String(pullNumber)", "head_sha: headSha", - "base_sha: baseSha", "merge_commit_sha: mergeCommitSha", } { if !strings.Contains(dispatcher, want) { @@ -557,12 +559,19 @@ func TestCoverageBaselineRepairContract(t *testing.T) { if ciWorkflowLookup < 0 || pushRunLookup <= ciWorkflowLookup || repairDispatch <= pushRunLookup { t.Error("merged-PR dispatcher must bind the fixed CI workflow before exhausting exact push-run lookup and dispatch") } + stableIdentityCheck := strings.Index(dispatcher, "if (!isStableMergedPRIdentity(") + mainContainmentCheck := strings.Index(dispatcher, "await requireMainContainment(mergeCommitSha)") + if stableIdentityCheck < 0 || mainContainmentCheck <= stableIdentityCheck || repairDispatch <= mainContainmentCheck { + t.Error("merged-PR dispatcher must prove stable PR identity and main containment before dispatch") + } for _, forbidden := range []string{ "actions/checkout@", "actions/cache/", "actions/setup-go@", "go test ", "github.event.pull_request.head.ref", + "currentPull.base.sha", + "base_sha", "secrets.", } { if strings.Contains(dispatcher, forbidden) { @@ -643,14 +652,14 @@ func TestCoverageBaselineRepairContract(t *testing.T) { "payload.source === 'merged_pr'", "payload.pull_number", "payload.head_sha", - "payload.base_sha", "payload.merge_commit_sha", - "currentPull.state !== 'closed'", - "currentPull.merged !== true", - "currentPull.base.ref !== 'main'", - "currentPull.base.sha !== baseSha", - "currentPull.head.sha !== headSha", - "currentPull.merge_commit_sha !== targetSha", + "currentPull.state === 'closed'", + "currentPull.merged === true", + "typeof currentPull.merged_at === 'string'", + "currentPull.base?.ref === 'main'", + "isStableMergedPRIdentity(", + "currentPull.head?.sha === headSha", + "currentPull.merge_commit_sha === mergeCommitSha", "payload.source === 'failed_ci'", "payload.workflow_run_id", "payload.workflow_run_attempt", @@ -700,6 +709,8 @@ func TestCoverageBaselineRepairContract(t *testing.T) { for _, forbidden := range []string{ "restore-keys", "github.event.pull_request.head.ref", + "currentPull.base.sha", + "base_sha", "HOMEBREW_PR_TOKEN", "RELEASE_GOVERNANCE_TOKEN", "REVIEWER_ROUTER_APP_PRIVATE_KEY", @@ -715,6 +726,14 @@ func TestCoverageBaselineRepairContract(t *testing.T) { if strings.Count(producer, "key: dws-coverage-full-v2-${{ steps.resolve-target.outputs.target_sha }}-go${{ steps.setup-go.outputs.go-version }}") != 3 { t.Error("coverage repair restore, save, and verification must share one exact target key") } + stablePayloadIdentityCheck := strings.Index(producer, "if (!isStableMergedPRIdentity(") + mainPayloadContainmentCheck := strings.Index(producer, "await requireMainContainment(targetSha)") + resolvedTargetOutput := strings.Index(producer, "core.setOutput('target_sha', targetSha)") + if stablePayloadIdentityCheck < 0 || + mainPayloadContainmentCheck <= stablePayloadIdentityCheck || + resolvedTargetOutput <= mainPayloadContainmentCheck { + t.Error("merged-PR producer must prove stable payload identity and main containment before resolving checkout target") + } validateIndex := strings.Index(producer, "name: Resolve trusted main repair target") checkoutIndex := strings.Index(producer, "name: Check out exact protected-main target") if validateIndex < 0 || checkoutIndex <= validateIndex {