fix(ci): tolerate merged PR base advancement
Bind coverage repair to the stable PR head and merge identity plus protected-main containment without treating the live base SHA projection as permanent identity. Add semantic regression coverage for base advancement and update the governance documentation.
This commit is contained in:
@@ -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}`,
|
||||
);
|
||||
|
||||
+5
-2
@@ -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
|
||||
|
||||
+6
-3
@@ -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
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
@@ -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 {
|
||||
|
||||
Reference in New Issue
Block a user