From 7b5e3e2d1f097d6ec3bcf51847a26b6c387d6cf1 Mon Sep 17 00:00:00 2001 From: chichuan Date: Thu, 20 Aug 2026 16:29:41 +0800 Subject: [PATCH 1/5] ci: harden coverage baseline governance --- .github/actionlint.yaml | 10 + .github/workflows/ci.yml | 342 +++++++- .../workflows/coverage-baseline-promotion.yml | 322 ++++++++ .../workflows/coverage-baseline-repair.yml | 529 ++++++++++++ .github/workflows/release.yml | 130 +++ .github/workflows/reviewer-router.yml | 756 +++++++++++++++++- docs/automation.md | 159 +++- docs/ci-pr-gates.md | 134 +++- .../coverage_workflow_contract_test.go | 506 +++++++++++- test/scripts/package_script_test.go | 81 +- test/scripts/reviewer_router_workflow_test.go | 469 ++++++++++- 11 files changed, 3357 insertions(+), 81 deletions(-) create mode 100644 .github/workflows/coverage-baseline-promotion.yml create mode 100644 .github/workflows/coverage-baseline-repair.yml diff --git a/.github/actionlint.yaml b/.github/actionlint.yaml index eea35124..b8b67108 100644 --- a/.github/actionlint.yaml +++ b/.github/actionlint.yaml @@ -4,3 +4,13 @@ paths: # GitHub Actions added concurrency.queue in 2026. actionlint v1.7.12's # bundled workflow schema has not caught up with the platform syntax. - 'unexpected key "queue" for "concurrency" section' + .github/workflows/coverage-baseline-promotion.yml: + ignore: + # Serialize every acknowledgement for one Formula target without + # allowing Actions' default single-pending replacement to orphan a run. + - 'unexpected key "queue" for "concurrency" section' + .github/workflows/coverage-baseline-repair.yml: + ignore: + # Keep the closed-event dispatcher and its exact-SHA producer queued for + # the same target instead of replacing either half of the repair chain. + - 'unexpected key "queue" for "concurrency" section' diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 08fb7c86..04e93660 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -5,12 +5,16 @@ on: branches: - main pull_request: + types: [opened, synchronize, reopened, ready_for_review, edited, auto_merge_enabled, auto_merge_disabled] permissions: contents: read concurrency: - group: ci-${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }} + # Only duplicate runs for the exact PR base/head revision cancel each other. + # A later revision must not kill an earlier cold-cache producer, and every + # protected-main SHA keeps an independent producer run. + group: ci-${{ github.workflow }}-${{ github.event_name == 'pull_request' && format('pr-{0}-{1}-{2}', github.event.pull_request.number, github.event.pull_request.base.sha, github.event.pull_request.head.sha) || format('push-{0}', github.sha) }} cancel-in-progress: true jobs: @@ -827,7 +831,9 @@ jobs: if: ${{ always() && needs.lint.result == 'success' }} runs-on: ubuntu-latest timeout-minutes: 5 - permissions: {} + permissions: + contents: read + pull-requests: read steps: - name: Verify test shards env: @@ -906,6 +912,174 @@ jobs: done test "$failed" -eq 0 + # Null and non-built-in merge identities emit either the protected-main + # push or the trusted pull_request_target closed repair. The built-in + # Actions identity is the exceptional unsafe path, so its own token must + # prove that main-merge-writers never lets it update main. + - name: Verify auto-merge identity + if: github.event_name == 'pull_request' && github.event.pull_request.draft == false + uses: actions/github-script@f28e40c7f34bde8b3046d885e986cb6290c5673b # v7.1.0 + env: + REVIEWER_ROUTER_APP_SLUG: ${{ vars.REVIEWER_ROUTER_APP_SLUG }} + with: + script: | + const owner = context.repo.owner; + const repo = context.repo.repo; + const pullNumber = context.payload.pull_request.number; + const eventHeadSha = context.payload.pull_request.head.sha; + const eventBaseSha = context.payload.pull_request.base.sha; + const appSlug = process.env.REVIEWER_ROUTER_APP_SLUG?.trim(); + if ( + !appSlug || + appSlug !== appSlug.toLowerCase() || + appSlug === 'github-actions' + ) { + core.setFailed( + 'Reviewer Router App slug repository variable is missing or unsafe.', + ); + return; + } + const expectedAppOwner = `${appSlug}[bot]`; + const writerRulesetName = 'main-merge-writers'; + const skipWorkflowPattern = + /\[(?:skip ci|ci skip|no ci|skip actions|actions skip)\]|\bskip-checks\s*:\s*true\b/i; + const {data: repository} = await github.rest.repos.get({owner, repo}); + if ( + repository.merge_commit_title !== 'MERGE_MESSAGE' || + !['PR_TITLE', 'BLANK'].includes(repository.merge_commit_message) + ) { + core.setFailed( + 'Repository merge-message defaults changed from their reviewed values.', + ); + return; + } + + const appliedRules = await github.paginate( + 'GET /repos/{owner}/{repo}/rules/branches/{branch}', + {owner, repo, branch: 'main', per_page: 100}, + ); + const repositorySource = `${owner}/${repo}`.toLowerCase(); + const applicableRulesetIDs = [ + ...new Set( + appliedRules + .filter(rule => + rule.ruleset_source_type === 'Repository' && + rule.ruleset_source?.toLowerCase() === repositorySource && + Number.isSafeInteger(Number(rule.ruleset_id)) && + Number(rule.ruleset_id) > 0, + ) + .map(rule => Number(rule.ruleset_id)), + ), + ]; + const activeMainRulesets = []; + for (const rulesetID of applicableRulesetIDs) { + const {data: ruleset} = await github.request( + 'GET /repos/{owner}/{repo}/rulesets/{ruleset_id}', + {owner, repo, ruleset_id: rulesetID}, + ); + if ( + ruleset.enforcement !== 'active' || + ruleset.target !== 'branch' || + ruleset.source_type !== 'Repository' || + ruleset.source?.toLowerCase() !== repositorySource + ) { + core.setFailed( + `Applicable repository ruleset ${ruleset.name || rulesetID} is not an active branch ruleset owned by this repository.`, + ); + return; + } + activeMainRulesets.push(ruleset); + } + + const writerRulesets = activeMainRulesets.filter( + ruleset => ruleset.name === writerRulesetName, + ); + if (writerRulesets.length !== 1) { + core.setFailed( + `Expected exactly one active ${writerRulesetName} ruleset on main; found ${writerRulesets.length}.`, + ); + return; + } + const writerRuleset = writerRulesets[0]; + const writerIncludes = writerRuleset.conditions?.ref_name?.include || []; + const writerExcludes = writerRuleset.conditions?.ref_name?.exclude || []; + if ( + writerIncludes.length !== 1 || + writerIncludes[0] !== 'refs/heads/main' || + writerExcludes.length !== 0 || + writerRuleset.rules?.length !== 1 || + writerRuleset.rules[0].type !== 'update' || + writerRuleset.rules[0].parameters?.update_allows_fetch_and_merge !== false || + writerRuleset.current_user_can_bypass !== 'never' + ) { + core.setFailed( + `${writerRulesetName} must target only refs/heads/main, contain only the strict update rule, and deny this built-in Actions identity any bypass.`, + ); + return; + } + + const maxAttempts = 6; + for (let attempt = 1; attempt <= maxAttempts; attempt += 1) { + const {data: currentPull} = await github.rest.pulls.get({ + owner, + repo, + pull_number: pullNumber, + }); + if ( + currentPull.head.sha !== eventHeadSha || + currentPull.base.sha !== eventBaseSha || + currentPull.state !== 'open' || + currentPull.draft || + currentPull.base.ref !== 'main' + ) { + core.setFailed( + `PR #${pullNumber} state or revision changed before the Test aggregate verified auto-merge identity.`, + ); + return; + } + const mergeTexts = [ + currentPull.title, + currentPull.auto_merge?.commit_title, + currentPull.auto_merge?.commit_message, + ].filter(value => typeof value === 'string'); + if (mergeTexts.some(value => skipWorkflowPattern.test(value))) { + core.setFailed( + `PR #${pullNumber} merge metadata contains a GitHub workflow-skip directive.`, + ); + return; + } + if (!currentPull.auto_merge) { + core.info( + `PR #${pullNumber} has no auto-merge request; protected-main push or closed-event repair remains authoritative.`, + ); + return; + } + const enabledBy = currentPull.auto_merge.enabled_by?.login?.toLowerCase(); + const safeCommitHeadline = `Merge pull request #${pullNumber}`; + const safeCommitBody = + `Merged by the dedicated Reviewer Router GitHub App for PR #${pullNumber}.`; + if ( + enabledBy === expectedAppOwner && + currentPull.auto_merge.commit_title === safeCommitHeadline && + currentPull.auto_merge.commit_message === safeCommitBody + ) { + core.info( + `PR #${pullNumber} auto-merge is owned by the reviewed ${expectedAppOwner} identity with fixed metadata.`, + ); + return; + } + if (attempt < maxAttempts) { + core.info( + `PR #${pullNumber} auto-merge owner or metadata is not the reviewed App value; waiting for Reviewer Router takeover (${attempt}/${maxAttempts}).`, + ); + await new Promise(resolve => setTimeout(resolve, 5000)); + continue; + } + core.setFailed( + `PR #${pullNumber} auto-merge must be null or owned by ${expectedAppOwner} with the reviewed fixed metadata.`, + ); + } + test-darwin: name: Test (macOS auth/keychain) needs: lint @@ -1236,7 +1410,7 @@ jobs: needs: lint if: ${{ needs.lint.outputs.changelog_only != 'true' && needs.lint.outputs.docs_only != 'true' }} runs-on: ubuntu-latest - timeout-minutes: 20 + timeout-minutes: 30 steps: - name: Check out repository uses: actions/checkout@v4 @@ -1371,6 +1545,143 @@ jobs: path: coverage-base.txt retention-days: 1 + # Documentation and release-seal pushes do not change executable coverage, + # but their new main SHA is still a future PR merge base. Promote only an + # exact predecessor cache after independently proving the whole push changed + # metadata paths; fall back to a full authoritative profile on a cold chain. + coverage-main-metadata: + name: Coverage (main metadata cache) + needs: lint + if: ${{ github.event_name == 'push' && (needs.lint.outputs.changelog_only == 'true' || needs.lint.outputs.docs_only == 'true') }} + runs-on: ubuntu-latest + timeout-minutes: 30 + steps: + - name: Check out exact metadata-only main revision + uses: actions/checkout@v4 + with: + fetch-depth: 0 + ref: ${{ github.sha }} + + - name: Set up Go + id: setup-go-metadata + uses: actions/setup-go@v5 + with: + go-version-file: go.mod + + - name: Verify metadata-only main successor + shell: bash + env: + PUSH_BEFORE_SHA: ${{ github.event.before }} + PUSH_AFTER_SHA: ${{ github.event.after }} + run: | + set -euo pipefail + full_commit='^[0-9a-f]{40}$' + [[ "$PUSH_BEFORE_SHA" =~ $full_commit ]] + [[ "$PUSH_AFTER_SHA" =~ $full_commit ]] + test "$PUSH_BEFORE_SHA" != 0000000000000000000000000000000000000000 + test "$PUSH_AFTER_SHA" = "$GITHUB_SHA" + test "$(git rev-parse HEAD)" = "$GITHUB_SHA" + git rev-parse --verify "${PUSH_BEFORE_SHA}^{commit}" >/dev/null + git merge-base --is-ancestor "$PUSH_BEFORE_SHA" "$PUSH_AFTER_SHA" + + changed_count=0 + while IFS= read -r -d '' path; do + changed_count=$((changed_count + 1)) + case "$path" in + CHANGELOG.md|README.md|README_zh.md|CONTRIBUTING.md|SECURITY.md|CODE_OF_CONDUCT.md|LICENSE|NOTICE|.github/PULL_REQUEST_TEMPLATE.md|.github/ISSUE_TEMPLATE/*|docs/*) + ;; + .changes/*) + if [[ "$path" =~ ^\.changes/[a-z0-9][a-z0-9._-]*\.md$ ]] || + [[ "$path" =~ ^\.changes/released/[0-9]+\.[0-9]+\.[0-9]+(-beta\.[1-9][0-9]*)?/[a-z0-9][a-z0-9._-]*\.md$ ]]; then + continue + fi + echo "Refusing coverage-cache promotion for unreviewed change-fragment path: $path" >&2 + exit 1 + ;; + *) + echo "Refusing coverage-cache promotion for executable path: $path" >&2 + exit 1 + ;; + esac + done < <(git diff --name-only --no-renames -z "$PUSH_BEFORE_SHA" "$PUSH_AFTER_SHA") + test "$changed_count" -gt 0 + echo "COVERAGE_SOURCE_REF=$PUSH_BEFORE_SHA" >> "$GITHUB_ENV" + + - name: Restore existing current-SHA coverage profile + id: metadata-current-cache + uses: actions/cache/restore@v4 + with: + path: coverage-cache.txt + key: dws-coverage-full-v2-${{ github.sha }}-go${{ steps.setup-go-metadata.outputs.go-version }} + + - name: Validate existing current-SHA coverage profile + if: steps.metadata-current-cache.outputs.cache-hit == 'true' + run: | + set -eu + test -s coverage-cache.txt + test "$(head -n 1 coverage-cache.txt)" = "mode: atomic" + + - name: Restore exact predecessor coverage profile + id: metadata-source-cache + if: steps.metadata-current-cache.outputs.cache-hit != 'true' + uses: actions/cache/restore@v4 + with: + path: coverage-cache.txt + key: dws-coverage-full-v2-${{ env.COVERAGE_SOURCE_REF }}-go${{ steps.setup-go-metadata.outputs.go-version }} + + - name: Validate promoted predecessor coverage profile + if: steps.metadata-current-cache.outputs.cache-hit != 'true' && steps.metadata-source-cache.outputs.cache-hit == 'true' + run: | + set -eu + test -s coverage-cache.txt + test "$(head -n 1 coverage-cache.txt)" = "mode: atomic" + + - name: Install archive tooling for cold metadata baseline + if: steps.metadata-current-cache.outputs.cache-hit != 'true' && steps.metadata-source-cache.outputs.cache-hit != 'true' + run: | + if command -v zip >/dev/null && command -v unzip >/dev/null; then + echo "zip and unzip are already available" + else + sudo apt-get update + sudo apt-get install -y zip unzip + fi + + - name: Recompute cold metadata baseline + if: steps.metadata-current-cache.outputs.cache-hit != 'true' && steps.metadata-source-cache.outputs.cache-hit != 'true' + env: + DWS_PACKAGE_VERSION: 0.0.0-test + run: | + set -euo pipefail + go test -count=1 -p 1 \ + -coverprofile=coverage-cache.txt \ + -covermode=atomic \ + ./ ./cmd/... ./internal/... ./skills/... + test -s coverage-cache.txt + test "$(head -n 1 coverage-cache.txt)" = "mode: atomic" + + - name: Save metadata main SHA coverage profile + if: steps.metadata-current-cache.outputs.cache-hit != 'true' + uses: actions/cache/save@v4 + with: + path: coverage-cache.txt + key: dws-coverage-full-v2-${{ github.sha }}-go${{ steps.setup-go-metadata.outputs.go-version }} + + # actions/cache/save reports upload failures as warnings. Convert an + # absent exact target key into a hard producer failure. + - name: Verify metadata main SHA coverage cache exists + id: metadata-target-cache-verification + uses: actions/cache/restore@v4 + with: + path: coverage-cache.txt + key: dws-coverage-full-v2-${{ github.sha }}-go${{ steps.setup-go-metadata.outputs.go-version }} + lookup-only: true + fail-on-cache-miss: true + + - name: Require exact metadata main SHA coverage cache + env: + EXACT_CACHE_HIT: ${{ steps.metadata-target-cache-verification.outputs.cache-hit }} + run: test "$EXACT_CACHE_HIT" = true + coverage: name: Coverage needs: @@ -1379,6 +1690,7 @@ jobs: - coverage-current-full - coverage-supporting - coverage-baseline + - coverage-main-metadata - coverage-darwin - coverage-windows if: ${{ always() && needs.lint.result == 'success' }} @@ -1395,6 +1707,7 @@ jobs: CURRENT_FULL_RESULT: ${{ needs.coverage-current-full.result }} SUPPORTING_RESULT: ${{ needs.coverage-supporting.result }} BASELINE_RESULT: ${{ needs.coverage-baseline.result }} + MAIN_METADATA_RESULT: ${{ needs.coverage-main-metadata.result }} DARWIN_RESULT: ${{ needs.coverage-darwin.result }} WINDOWS_RESULT: ${{ needs.coverage-windows.result }} run: | @@ -1403,10 +1716,14 @@ jobs: current_full_expected=skipped supporting_expected=skipped baseline_expected=success + main_metadata_expected=skipped native_expected=skipped if [ "$CHANGELOG_ONLY" = true ] || [ "$DOCS_ONLY" = true ]; then current_expected=skipped baseline_expected=skipped + if [ "$GITHUB_EVENT_NAME" = push ]; then + main_metadata_expected=success + fi elif [ "$FULL_SUITE" = true ]; then current_expected=skipped current_full_expected=success @@ -1422,7 +1739,8 @@ jobs: "current:$CURRENT_RESULT:$current_expected" \ "current shards:$CURRENT_FULL_RESULT:$current_full_expected" \ "supporting:$SUPPORTING_RESULT:$supporting_expected" \ - "baseline:$BASELINE_RESULT:$baseline_expected" + "baseline:$BASELINE_RESULT:$baseline_expected" \ + "main metadata cache:$MAIN_METADATA_RESULT:$main_metadata_expected" do name="${profile%%:*}" remainder="${profile#*:}" @@ -1562,6 +1880,22 @@ jobs: path: coverage-cache.txt key: dws-coverage-full-v2-${{ github.sha }}-go${{ steps.setup-go.outputs.go-version }} + - name: Verify push coverage cache exists + id: push-cache-verification + if: github.event_name == 'push' && needs.lint.outputs.changelog_only != 'true' && needs.lint.outputs.docs_only != 'true' && needs.lint.outputs.full_suite == 'true' + uses: actions/cache/restore@v4 + with: + path: coverage-cache.txt + key: dws-coverage-full-v2-${{ github.sha }}-go${{ steps.setup-go.outputs.go-version }} + lookup-only: true + fail-on-cache-miss: true + + - name: Require exact push coverage cache + if: github.event_name == 'push' && needs.lint.outputs.changelog_only != 'true' && needs.lint.outputs.docs_only != 'true' && needs.lint.outputs.full_suite == 'true' + env: + EXACT_CACHE_HIT: ${{ steps.push-cache-verification.outputs.cache-hit }} + run: test "$EXACT_CACHE_HIT" = true + - name: Generate coverage report if: needs.lint.outputs.changelog_only != 'true' && needs.lint.outputs.docs_only != 'true' run: | diff --git a/.github/workflows/coverage-baseline-promotion.yml b/.github/workflows/coverage-baseline-promotion.yml new file mode 100644 index 00000000..6cff727e --- /dev/null +++ b/.github/workflows/coverage-baseline-promotion.yml @@ -0,0 +1,322 @@ +name: Coverage Baseline Promotion + +run-name: Promote coverage baseline for ${{ github.event.client_payload.target_sha }} + +on: + repository_dispatch: + types: [coverage-baseline-promote] + +# repository_dispatch loads this workflow from the protected default branch. +# The requested target is treated as untrusted input until the validation step +# proves it is an exact Formula-only successor already contained in main. +permissions: + checks: write + contents: read + +concurrency: + group: coverage-baseline-promotion-${{ github.event.client_payload.target_sha }} + cancel-in-progress: false + queue: max + +jobs: + promote: + if: github.repository == 'DingTalk-Real-AI/dingtalk-workspace-cli' + runs-on: ubuntu-latest + timeout-minutes: 30 + steps: + - name: Validate Formula-only main target + id: validate-target + uses: actions/github-script@f28e40c7f34bde8b3046d885e986cb6290c5673b # v7.1.0 + with: + script: | + const owner = context.repo.owner; + const repo = context.repo.repo; + const targetSha = context.payload.client_payload?.target_sha; + const sourceRunId = context.payload.client_payload?.source_run_id; + const checkRunId = Number(context.payload.client_payload?.check_run_id); + if (!/^[0-9a-f]{40}$/.test(targetSha || '')) { + throw new Error('coverage-baseline-promote requires one full target_sha'); + } + if (!/^[1-9][0-9]*$/.test(sourceRunId || '')) { + throw new Error('coverage-baseline-promote requires one source_run_id'); + } + if (!Number.isSafeInteger(checkRunId) || checkRunId <= 0) { + throw new Error('coverage-baseline-promote requires one safe check_run_id'); + } + + // Bind the finalizer before any target or cache validation. A + // later failure must complete the release-created acknowledgement + // instead of leaving Release to poll a permanently queued check. + const promotionExternalId = `release-${sourceRunId}-${targetSha}`; + const {data: promotionCheck} = await github.rest.checks.get({ + owner, + repo, + check_run_id: checkRunId, + }); + if ( + promotionCheck.id !== checkRunId || + promotionCheck.head_sha !== targetSha || + promotionCheck.name !== 'Coverage Baseline Cache' || + promotionCheck.external_id !== promotionExternalId || + promotionCheck.app?.slug !== 'github-actions' || + promotionCheck.status !== 'queued' || + promotionCheck.conclusion !== null + ) { + throw new Error('coverage baseline acknowledgement has an invalid identity'); + } + core.setOutput('target_sha', targetSha); + core.setOutput('check_run_id', String(checkRunId)); + core.setOutput('check_external_id', promotionExternalId); + + const {data: targetCommit} = await github.rest.repos.getCommit({ + owner, + repo, + ref: targetSha, + per_page: 100, + }); + const files = targetCommit.files || []; + const message = targetCommit.commit.message; + const formulaPath = files[0]?.filename; + const stableFormula = + formulaPath === 'Formula/dingtalk-workspace-cli.rb' && + /^chore: update formula for v[0-9]+\.[0-9]+\.[0-9]+ \[skip ci\]$/.test(message); + const betaFormula = + formulaPath === 'Formula/dingtalk-workspace-cli-beta.rb' && + /^chore: update beta formula for v[0-9]+\.[0-9]+\.[0-9]+-beta\.[1-9][0-9]* \[skip ci\]$/.test(message); + if ( + targetCommit.sha !== targetSha || + targetCommit.parents.length !== 1 || + targetCommit.author?.login !== 'github-actions[bot]' || + targetCommit.committer?.login !== 'github-actions[bot]' || + files.length !== 1 || + !['added', 'modified'].includes(files[0].status) || + (!stableFormula && !betaFormula) + ) { + throw new Error( + `${targetSha} is not an exact release-produced Formula-only commit`, + ); + } + + const parentSha = targetCommit.parents[0].sha; + const requiredContexts = [ + 'Lint', + 'Test', + 'Coverage', + 'Policy', + 'Edition', + 'Interface Integrity', + 'AI Behavior', + 'CLI Smoke', + 'Mock MCP', + ]; + async function requireSuccessfulAdmission(ref, label) { + for (let attempt = 1; attempt <= 6; attempt += 1) { + const runs = await github.paginate(github.rest.checks.listForRef, { + owner, + repo, + ref, + filter: 'latest', + per_page: 100, + }); + const latestByName = new Map(); + for (const run of runs) { + if ( + run.head_sha !== ref || + run.app?.slug !== 'github-actions' || + !requiredContexts.includes(run.name) + ) { + continue; + } + const current = latestByName.get(run.name); + if (!current || run.id > current.id) { + latestByName.set(run.name, run); + } + } + const invalid = requiredContexts.filter((name) => { + const run = latestByName.get(name); + return !run || run.conclusion !== 'success'; + }); + if (invalid.length === 0) { + return; + } + if (attempt < 6) { + await new Promise(resolve => setTimeout(resolve, 5000)); + continue; + } + throw new Error( + `${label} ${ref} lacks successful Code Admission contexts: ${invalid.join(', ')}`, + ); + } + } + await requireSuccessfulAdmission(parentSha, 'Formula parent'); + await requireSuccessfulAdmission(targetSha, 'Formula target'); + + const {data: branch} = await github.rest.repos.getBranch({ + owner, + repo, + branch: context.payload.repository.default_branch, + }); + const {data: containment} = + await github.rest.repos.compareCommitsWithBasehead({ + owner, + repo, + basehead: `${targetSha}...${branch.commit.sha}`, + }); + if (!['ahead', 'identical'].includes(containment.status)) { + throw new Error(`${targetSha} is not contained in the protected default branch`); + } + + core.setOutput('parent_sha', parentSha); + core.setOutput('formula_path', formulaPath); + + - name: Mark Formula cache promotion in progress + uses: actions/github-script@f28e40c7f34bde8b3046d885e986cb6290c5673b # v7.1.0 + with: + script: | + await github.rest.checks.update({ + ...context.repo, + check_run_id: Number('${{ steps.validate-target.outputs.check_run_id }}'), + status: 'in_progress', + started_at: new Date().toISOString(), + output: { + title: 'Producing exact-SHA coverage baseline', + summary: 'The trusted default-branch workflow is validating or producing the main-scoped cache.', + }, + }); + + - name: Check out validated Formula-only target + uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + with: + fetch-depth: 0 + persist-credentials: false + ref: ${{ steps.validate-target.outputs.target_sha }} + + - name: Verify checked-out Formula-only identity + shell: bash + env: + TARGET_SHA: ${{ steps.validate-target.outputs.target_sha }} + PARENT_SHA: ${{ steps.validate-target.outputs.parent_sha }} + FORMULA_PATH: ${{ steps.validate-target.outputs.formula_path }} + run: | + set -euo pipefail + test "$(git rev-parse HEAD)" = "$TARGET_SHA" + test "$(git rev-parse HEAD^)" = "$PARENT_SHA" + test "$(git diff --name-only --no-renames "$PARENT_SHA" "$TARGET_SHA")" = "$FORMULA_PATH" + + - name: Set up Go + id: setup-go + uses: actions/setup-go@40f1582b2485089dde7abd97c1529aa768e1baff # v5 + with: + go-version-file: go.mod + + - name: Restore existing target coverage profile + id: target-cache + uses: actions/cache/restore@0057852bfaa89a56745cba8c7296529d2fc39830 # v4 + with: + path: coverage-cache.txt + key: dws-coverage-full-v2-${{ steps.validate-target.outputs.target_sha }}-go${{ steps.setup-go.outputs.go-version }} + + - name: Validate existing target coverage profile + if: steps.target-cache.outputs.cache-hit == 'true' + run: | + set -eu + test -s coverage-cache.txt + test "$(head -n 1 coverage-cache.txt)" = "mode: atomic" + + - name: Restore exact Formula parent coverage profile + id: parent-cache + if: steps.target-cache.outputs.cache-hit != 'true' + uses: actions/cache/restore@0057852bfaa89a56745cba8c7296529d2fc39830 # v4 + with: + path: coverage-cache.txt + key: dws-coverage-full-v2-${{ steps.validate-target.outputs.parent_sha }}-go${{ steps.setup-go.outputs.go-version }} + + - name: Validate promoted Formula parent profile + if: steps.target-cache.outputs.cache-hit != 'true' && steps.parent-cache.outputs.cache-hit == 'true' + run: | + set -eu + test -s coverage-cache.txt + test "$(head -n 1 coverage-cache.txt)" = "mode: atomic" + + - name: Install archive tooling for cold Formula baseline + if: steps.target-cache.outputs.cache-hit != 'true' && steps.parent-cache.outputs.cache-hit != 'true' + run: | + if command -v zip >/dev/null && command -v unzip >/dev/null; then + echo "zip and unzip are already available" + else + sudo apt-get update + sudo apt-get install -y zip unzip + fi + + - name: Recompute cold Formula baseline + if: steps.target-cache.outputs.cache-hit != 'true' && steps.parent-cache.outputs.cache-hit != 'true' + env: + DWS_PACKAGE_VERSION: 0.0.0-test + run: | + set -euo pipefail + go test -count=1 -p 1 \ + -coverprofile=coverage-cache.txt \ + -covermode=atomic \ + ./ ./cmd/... ./internal/... ./skills/... + test -s coverage-cache.txt + test "$(head -n 1 coverage-cache.txt)" = "mode: atomic" + + - name: Save Formula main SHA coverage profile + if: steps.target-cache.outputs.cache-hit != 'true' + uses: actions/cache/save@0057852bfaa89a56745cba8c7296529d2fc39830 # v4 + with: + path: coverage-cache.txt + key: dws-coverage-full-v2-${{ steps.validate-target.outputs.target_sha }}-go${{ steps.setup-go.outputs.go-version }} + + - name: Verify Formula main SHA coverage cache exists + id: formula-target-cache-verification + uses: actions/cache/restore@0057852bfaa89a56745cba8c7296529d2fc39830 # v4 + with: + path: coverage-cache.txt + key: dws-coverage-full-v2-${{ steps.validate-target.outputs.target_sha }}-go${{ steps.setup-go.outputs.go-version }} + lookup-only: true + fail-on-cache-miss: true + + - name: Require exact Formula main SHA coverage cache + env: + EXACT_CACHE_HIT: ${{ steps.formula-target-cache-verification.outputs.cache-hit }} + run: test "$EXACT_CACHE_HIT" = true + + - name: Complete Formula cache promotion acknowledgement + if: ${{ always() && steps.validate-target.outputs.check_run_id != '' }} + uses: actions/github-script@f28e40c7f34bde8b3046d885e986cb6290c5673b # v7.1.0 + env: + PROMOTION_JOB_STATUS: ${{ job.status }} + with: + script: | + const checkRunId = Number('${{ steps.validate-target.outputs.check_run_id }}'); + const targetSha = '${{ steps.validate-target.outputs.target_sha }}'; + const expectedExternalId = '${{ steps.validate-target.outputs.check_external_id }}'; + const {data: currentCheck} = await github.rest.checks.get({ + ...context.repo, + check_run_id: checkRunId, + }); + if ( + currentCheck.head_sha !== targetSha || + currentCheck.name !== 'Coverage Baseline Cache' || + currentCheck.external_id !== expectedExternalId || + currentCheck.app?.slug !== 'github-actions' + ) { + throw new Error('refusing to update a changed promotion acknowledgement'); + } + const succeeded = process.env.PROMOTION_JOB_STATUS === 'success'; + await github.rest.checks.update({ + ...context.repo, + check_run_id: checkRunId, + status: 'completed', + conclusion: succeeded ? 'success' : 'failure', + completed_at: new Date().toISOString(), + output: { + title: succeeded + ? 'Exact-SHA coverage baseline is available' + : 'Exact-SHA coverage baseline promotion failed', + summary: succeeded + ? `Verified the main-scoped exact cache for ${targetSha}.` + : `Promotion failed for ${targetSha}; rerun the failed Release job after correcting the producer.`, + }, + }); diff --git a/.github/workflows/coverage-baseline-repair.yml b/.github/workflows/coverage-baseline-repair.yml new file mode 100644 index 00000000..23190ae5 --- /dev/null +++ b/.github/workflows/coverage-baseline-repair.yml @@ -0,0 +1,529 @@ +name: Coverage Baseline Repair + +run-name: Repair coverage baseline from ${{ github.event_name }} + +on: + pull_request_target: + branches: [main] + types: [closed] + workflow_run: + workflows: [CI] + types: [completed] + branches: [main] + repository_dispatch: + types: [coverage-baseline-repair] + schedule: + - cron: "23 * * * *" + workflow_dispatch: + +# pull_request_target and workflow_run are allowed to inspect only GitHub API +# data and dispatch the trusted producer. GitHub deliberately makes both +# triggers read-only for the default-branch cache, so all checkout and cache +# writes live in repository_dispatch, schedule, or main-only workflow_dispatch. +permissions: + contents: read + +concurrency: + group: coverage-baseline-repair-${{ github.event_name == 'pull_request_target' && github.event.pull_request.merge_commit_sha || github.event_name == 'workflow_run' && github.event.workflow_run.head_sha || github.event_name == 'repository_dispatch' && github.event.client_payload.merge_commit_sha || github.sha }} + cancel-in-progress: false + # Retain every pending repair for one target. actionlint v1.7.12's bundled + # schema predates GitHub's concurrency.queue support. + queue: max + +jobs: + dispatch-merged-pr: + if: ${{ github.event_name == 'pull_request_target' && github.event.pull_request.merged == true && github.repository == 'DingTalk-Real-AI/dingtalk-workspace-cli' }} + runs-on: ubuntu-latest + timeout-minutes: 5 + permissions: + actions: read + contents: write + pull-requests: read + steps: + # Never check out or execute pull-request content in this privileged + # base-owned event. Re-read the merged PR, bind every immutable identity, + # prove the result is in main, and send only those values to the producer. + - name: Dispatch trusted merged-PR repair + uses: actions/github-script@f28e40c7f34bde8b3046d885e986cb6290c5673b # v7.1.0 + with: + script: | + const owner = context.repo.owner; + const repo = context.repo.repo; + const eventPull = context.payload.pull_request; + const fullCommit = /^[0-9a-f]{40}$/; + const pullNumber = Number(eventPull?.number); + const headSha = eventPull?.head?.sha; + const baseSha = eventPull?.base?.sha; + const mergeCommitSha = eventPull?.merge_commit_sha; + if ( + context.payload.repository?.full_name !== + 'DingTalk-Real-AI/dingtalk-workspace-cli' || + context.payload.repository?.default_branch !== 'main' || + !Number.isSafeInteger(pullNumber) || + pullNumber <= 0 || + !fullCommit.test(headSha || '') || + !fullCommit.test(baseSha || '') || + !fullCommit.test(mergeCommitSha || '') + ) { + throw new Error('closed PR event has an invalid repository or revision identity'); + } + + 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 + ) { + throw new Error(`PR #${pullNumber} no longer matches the merged-main event`); + } + + async function requireMainContainment(targetSha) { + let lastState = 'not checked'; + for (let attempt = 1; attempt <= 6; attempt += 1) { + try { + const {data: branch} = await github.rest.repos.getBranch({ + owner, + repo, + branch: 'main', + }); + const {data: comparison} = + await github.rest.repos.compareCommitsWithBasehead({ + owner, + repo, + basehead: `${targetSha}...${branch.commit.sha}`, + }); + lastState = comparison.status; + if (['ahead', 'identical'].includes(comparison.status)) { + return; + } + } catch (error) { + lastState = error.message; + } + if (attempt < 6) { + await new Promise(resolve => setTimeout(resolve, 5000)); + } + } + throw new Error( + `${targetSha} is not contained in protected main after retries: ${lastState}`, + ); + } + await requireMainContainment(mergeCommitSha); + + // Normal App or human merges emit a protected-main push run whose + // CI producer owns this exact key. Give Actions event delivery a + // short visibility window and avoid a duplicate full-suite repair. + // A workflow-skip directive or suppressed built-in-token event has + // no such run, so only that missing-event path reaches dispatch. + const {data: ciWorkflow} = await github.rest.actions.getWorkflow({ + owner, + repo, + workflow_id: '.github/workflows/ci.yml', + }); + if ( + ciWorkflow.name !== 'CI' || + ciWorkflow.path !== '.github/workflows/ci.yml' || + ciWorkflow.state !== 'active' + ) { + throw new Error('protected CI workflow identity is not active or exact'); + } + for (let attempt = 1; attempt <= 12; attempt += 1) { + const {data: workflowRuns} = + await github.rest.actions.listWorkflowRunsForRepo({ + owner, + repo, + branch: 'main', + event: 'push', + per_page: 100, + }); + const exactPushRun = workflowRuns.workflow_runs.find(run => + run.name === 'CI' && + run.workflow_id === ciWorkflow.id && + run.path === ciWorkflow.path && + run.event === 'push' && + run.head_sha === mergeCommitSha && + run.head_branch === 'main' && + ['queued', 'in_progress', 'completed'].includes(run.status), + ); + if (exactPushRun) { + core.info( + `CI push run ${exactPushRun.id} already owns the exact-SHA producer for ${mergeCommitSha}; repair dispatch is unnecessary.`, + ); + return; + } + if (attempt < 12) { + await new Promise(resolve => setTimeout(resolve, 5000)); + } + } + + // repository_dispatch is one of GitHub's explicit GITHUB_TOKEN + // recursion exceptions and receives default-branch cache-write scope. + await github.rest.repos.createDispatchEvent({ + owner, + repo, + event_type: 'coverage-baseline-repair', + client_payload: { + source: 'merged_pr', + pull_number: String(pullNumber), + head_sha: headSha, + base_sha: baseSha, + merge_commit_sha: mergeCommitSha, + source_run_id: String(context.runId), + }, + }); + core.info( + `Dispatched exact-SHA coverage repair for merged PR #${pullNumber} at ${mergeCommitSha}.`, + ); + + dispatch-failed-ci: + if: >- + ${{ + github.event_name == 'workflow_run' && + github.repository == 'DingTalk-Real-AI/dingtalk-workspace-cli' && + github.event.workflow_run.name == 'CI' && + github.event.workflow_run.event == 'push' && + github.event.workflow_run.head_branch == 'main' && + github.event.workflow_run.status == 'completed' && + github.event.workflow_run.conclusion != 'success' + }} + runs-on: ubuntu-latest + timeout-minutes: 5 + permissions: + actions: read + contents: write + steps: + # workflow_run cannot write the default-branch cache. Re-read the exact + # completed CI run from Actions, bind it to the protected CI workflow and + # main revision, then use the repository_dispatch recursion exception. + - name: Dispatch trusted failed-CI repair + uses: actions/github-script@f28e40c7f34bde8b3046d885e986cb6290c5673b # v7.1.0 + with: + script: | + const owner = context.repo.owner; + const repo = context.repo.repo; + const upstream = 'DingTalk-Real-AI/dingtalk-workspace-cli'; + const eventRun = context.payload.workflow_run; + const fullCommit = /^[0-9a-f]{40}$/; + const runID = Number(eventRun?.id); + const runAttempt = Number(eventRun?.run_attempt); + const headSha = eventRun?.head_sha; + const conclusion = eventRun?.conclusion; + if ( + context.payload.repository?.full_name !== upstream || + context.payload.repository?.default_branch !== 'main' || + !Number.isSafeInteger(runID) || + runID <= 0 || + !Number.isSafeInteger(runAttempt) || + runAttempt <= 0 || + eventRun?.name !== 'CI' || + eventRun?.event !== 'push' || + eventRun?.head_branch !== 'main' || + eventRun?.status !== 'completed' || + typeof conclusion !== 'string' || + conclusion.length === 0 || + conclusion === 'success' || + !fullCommit.test(headSha || '') + ) { + throw new Error('workflow_run event is not one completed non-success main CI push'); + } + + const {data: ciWorkflow} = await github.rest.actions.getWorkflow({ + owner, + repo, + workflow_id: '.github/workflows/ci.yml', + }); + const {data: currentRun} = await github.rest.actions.getWorkflowRun({ + owner, + repo, + run_id: runID, + }); + if ( + ciWorkflow.name !== 'CI' || + ciWorkflow.path !== '.github/workflows/ci.yml' || + eventRun.workflow_id !== ciWorkflow.id || + currentRun.id !== runID || + currentRun.workflow_id !== ciWorkflow.id || + currentRun.name !== 'CI' || + currentRun.event !== 'push' || + currentRun.head_branch !== 'main' || + currentRun.head_sha !== headSha || + currentRun.run_attempt !== runAttempt || + currentRun.status !== 'completed' || + currentRun.conclusion !== conclusion || + currentRun.conclusion === 'success' || + currentRun.repository?.full_name !== upstream || + currentRun.head_repository?.full_name !== upstream + ) { + throw new Error(`CI workflow run ${runID} no longer matches the completed event`); + } + + await github.rest.repos.createDispatchEvent({ + owner, + repo, + event_type: 'coverage-baseline-repair', + client_payload: { + source: 'failed_ci', + workflow_run_id: String(runID), + workflow_run_attempt: String(runAttempt), + workflow_conclusion: conclusion, + merge_commit_sha: headSha, + source_run_id: String(context.runId), + }, + }); + core.info( + `Dispatched exact-SHA coverage repair for ${conclusion} CI run ${runID} at ${headSha}.`, + ); + + repair: + if: ${{ github.event_name == 'repository_dispatch' || github.event_name == 'schedule' || github.event_name == 'workflow_dispatch' }} + runs-on: ubuntu-latest + timeout-minutes: 35 + permissions: + actions: read + contents: read + pull-requests: read + steps: + - name: Resolve trusted main repair target + id: resolve-target + uses: actions/github-script@f28e40c7f34bde8b3046d885e986cb6290c5673b # v7.1.0 + with: + script: | + const owner = context.repo.owner; + const repo = context.repo.repo; + const fullCommit = /^[0-9a-f]{40}$/; + if ( + context.payload.repository?.full_name !== + 'DingTalk-Real-AI/dingtalk-workspace-cli' || + context.payload.repository?.default_branch !== 'main' + ) { + throw new Error('coverage repair is restricted to the protected upstream repository'); + } + + async function requireMainContainment(targetSha) { + let lastState = 'not checked'; + for (let attempt = 1; attempt <= 6; attempt += 1) { + try { + const {data: branch} = await github.rest.repos.getBranch({ + owner, + repo, + branch: 'main', + }); + const {data: comparison} = + await github.rest.repos.compareCommitsWithBasehead({ + owner, + repo, + basehead: `${targetSha}...${branch.commit.sha}`, + }); + lastState = comparison.status; + if (['ahead', 'identical'].includes(comparison.status)) { + return branch.commit.sha; + } + } catch (error) { + lastState = error.message; + } + if (attempt < 6) { + await new Promise(resolve => setTimeout(resolve, 5000)); + } + } + throw new Error( + `${targetSha} is not contained in protected main after retries: ${lastState}`, + ); + } + + let targetSha; + if (context.eventName === 'repository_dispatch') { + const payload = context.payload.client_payload || {}; + const sourceRunIDText = String(payload.source_run_id || ''); + if (!/^[1-9][0-9]*$/.test(sourceRunIDText)) { + throw new Error('coverage-baseline-repair payload has an invalid source run'); + } + if (payload.source === 'merged_pr') { + 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'); + } + 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 !== targetSha + ) { + throw new Error( + `repair payload no longer matches merged PR #${pullNumber}`, + ); + } + } else if (payload.source === 'failed_ci') { + const rawWorkflowRunID = String(payload.workflow_run_id || ''); + const workflowRunID = Number(rawWorkflowRunID); + const rawWorkflowRunAttempt = String(payload.workflow_run_attempt || ''); + const workflowRunAttempt = Number(rawWorkflowRunAttempt); + const workflowConclusion = payload.workflow_conclusion; + targetSha = payload.merge_commit_sha; + if ( + !/^[1-9][0-9]*$/.test(rawWorkflowRunID) || + !Number.isSafeInteger(workflowRunID) || + !/^[1-9][0-9]*$/.test(rawWorkflowRunAttempt) || + !Number.isSafeInteger(workflowRunAttempt) || + typeof workflowConclusion !== 'string' || + workflowConclusion.length === 0 || + workflowConclusion === 'success' || + !fullCommit.test(targetSha || '') + ) { + throw new Error('coverage-baseline-repair payload has an invalid CI identity'); + } + const {data: ciWorkflow} = await github.rest.actions.getWorkflow({ + owner, + repo, + workflow_id: '.github/workflows/ci.yml', + }); + const {data: currentRun} = await github.rest.actions.getWorkflowRun({ + owner, + repo, + run_id: workflowRunID, + }); + if ( + ciWorkflow.name !== 'CI' || + ciWorkflow.path !== '.github/workflows/ci.yml' || + currentRun.id !== workflowRunID || + currentRun.workflow_id !== ciWorkflow.id || + currentRun.name !== 'CI' || + currentRun.event !== 'push' || + currentRun.head_branch !== 'main' || + currentRun.head_sha !== targetSha || + currentRun.run_attempt !== workflowRunAttempt || + currentRun.status !== 'completed' || + currentRun.conclusion !== workflowConclusion || + currentRun.conclusion === 'success' || + currentRun.repository?.full_name !== + 'DingTalk-Real-AI/dingtalk-workspace-cli' || + currentRun.head_repository?.full_name !== + 'DingTalk-Real-AI/dingtalk-workspace-cli' + ) { + throw new Error( + `repair payload no longer matches failed CI run ${workflowRunID}`, + ); + } + } else { + throw new Error('coverage-baseline-repair payload has an unknown source'); + } + await requireMainContainment(targetSha); + } else { + if (context.ref !== 'refs/heads/main') { + throw new Error('scheduled and manual repair must run from refs/heads/main'); + } + // github.sha is the default-branch tip that keyed this workflow's + // concurrency group. Keep the producer bound to that exact + // event-time target even if main advances while this run queues. + targetSha = context.sha; + if (!fullCommit.test(targetSha || '')) { + throw new Error('protected main did not resolve to one full commit SHA'); + } + await requireMainContainment(targetSha); + } + core.setOutput('target_sha', targetSha); + core.info(`Resolved protected-main coverage repair target ${targetSha}.`); + + - name: Check out exact protected-main target + uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + with: + fetch-depth: 0 + persist-credentials: false + ref: ${{ steps.resolve-target.outputs.target_sha }} + + - name: Verify checked-out repair target + env: + TARGET_SHA: ${{ steps.resolve-target.outputs.target_sha }} + run: test "$(git rev-parse HEAD)" = "$TARGET_SHA" + + - name: Set up Go + id: setup-go + uses: actions/setup-go@40f1582b2485089dde7abd97c1529aa768e1baff # v5 + with: + go-version-file: go.mod + + - name: Restore exact target coverage profile + id: target-cache + uses: actions/cache/restore@0057852bfaa89a56745cba8c7296529d2fc39830 # v4 + with: + path: coverage-cache.txt + key: dws-coverage-full-v2-${{ steps.resolve-target.outputs.target_sha }}-go${{ steps.setup-go.outputs.go-version }} + + - name: Validate existing exact target profile + if: steps.target-cache.outputs.cache-hit == 'true' + run: | + set -eu + test -s coverage-cache.txt + test "$(head -n 1 coverage-cache.txt)" = "mode: atomic" + + - name: Install archive tooling for cold repair + if: steps.target-cache.outputs.cache-hit != 'true' + run: | + if command -v zip >/dev/null && command -v unzip >/dev/null; then + echo "zip and unzip are already available" + else + sudo apt-get update + sudo apt-get install -y zip unzip + fi + + - name: Recompute complete target coverage profile + if: steps.target-cache.outputs.cache-hit != 'true' + env: + DWS_PACKAGE_VERSION: 0.0.0-test + run: | + set -euo pipefail + go test -count=1 -p 1 \ + -coverprofile=coverage-cache.txt \ + -covermode=atomic \ + ./ ./cmd/... ./internal/... ./skills/... + test -s coverage-cache.txt + test "$(head -n 1 coverage-cache.txt)" = "mode: atomic" + + - name: Save exact protected-main coverage profile + if: steps.target-cache.outputs.cache-hit != 'true' + uses: actions/cache/save@0057852bfaa89a56745cba8c7296529d2fc39830 # v4 + with: + path: coverage-cache.txt + key: dws-coverage-full-v2-${{ steps.resolve-target.outputs.target_sha }}-go${{ steps.setup-go.outputs.go-version }} + + # Cache uploads are fail-open warnings. A lookup-only restore plus the + # explicit cache-hit assertion makes an absent or partial key fail hard. + - name: Verify exact protected-main coverage cache exists + id: target-cache-verification + uses: actions/cache/restore@0057852bfaa89a56745cba8c7296529d2fc39830 # v4 + with: + path: coverage-cache.txt + key: dws-coverage-full-v2-${{ steps.resolve-target.outputs.target_sha }}-go${{ steps.setup-go.outputs.go-version }} + lookup-only: true + fail-on-cache-miss: true + + - name: Require exact protected-main coverage cache + env: + EXACT_CACHE_HIT: ${{ steps.target-cache-verification.outputs.cache-hit }} + run: test "$EXACT_CACHE_HIT" = true diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 774495a5..89f38b3a 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -1112,6 +1112,9 @@ jobs: needs: [release-contract, release-validation, release, verify-darwin-signatures] runs-on: ubuntu-latest timeout-minutes: 30 + outputs: + coverage_baseline_required: ${{ steps.seal-formula.outputs.coverage_baseline_required }} + coverage_baseline_commit: ${{ steps.seal-formula.outputs.coverage_baseline_commit }} permissions: checks: write contents: write @@ -1495,6 +1498,7 @@ jobs: DWS_GIT_EMAIL: 41898282+github-actions[bot]@users.noreply.github.com - name: Seal Formula-only Code Admission contexts + id: seal-formula if: ${{ github.repository_owner == 'DingTalk-Real-AI' }} uses: actions/github-script@v7 env: @@ -1515,6 +1519,8 @@ jobs: const sourcePath = channel === "stable" ? "dist/homebrew/dingtalk-workspace-cli.rb" : "dist/homebrew/dingtalk-workspace-cli-beta.rb"; + core.setOutput("coverage_baseline_required", "false"); + core.setOutput("coverage_baseline_commit", ""); const expectedMessage = channel === "stable" ? `chore: update formula for ${version} [skip ci]` : `chore: update beta formula for ${version} [skip ci]`; @@ -1650,6 +1656,11 @@ jobs: }, }); } + core.setOutput("coverage_baseline_required", "true"); + core.setOutput("coverage_baseline_commit", commit); + core.info( + `Formula-only Code Admission is sealed for ${commit}; the independent confirmation job will dispatch its exact-SHA cache producer.`, + ); - name: Reverify exact immutable npm package run: ./scripts/release/verify-package-managers.sh --npm-only --expected-version "$RELEASE_VERSION" @@ -2177,6 +2188,117 @@ jobs: env: NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }} + coverage-baseline-confirmation: + name: Confirm Formula coverage baseline + # Once Formula sealing has exposed a target SHA, later publication + # verification failures must not orphan its exact-main cache producer. + if: ${{ !cancelled() && (needs.publish-release.result == 'success' || needs.publish-release.outputs.coverage_baseline_required == 'true') }} + needs: publish-release + runs-on: ubuntu-latest + timeout-minutes: 35 + permissions: + checks: write + contents: write + steps: + - name: Require exact Formula cache acknowledgement + uses: actions/github-script@f28e40c7f34bde8b3046d885e986cb6290c5673b # v7.1.0 + env: + BASELINE_REQUIRED: ${{ needs.publish-release.outputs.coverage_baseline_required }} + FORMULA_COMMIT: ${{ needs.publish-release.outputs.coverage_baseline_commit }} + with: + script: | + const rawRequired = process.env.BASELINE_REQUIRED; + if (!['true', 'false'].includes(rawRequired)) { + throw new Error(`Formula baseline requirement is invalid: ${rawRequired || 'empty'}`); + } + const required = rawRequired === 'true'; + const targetSha = process.env.FORMULA_COMMIT; + if (!required) { + if (targetSha) { + throw new Error('Formula baseline outputs are inconsistent for a no-op publication'); + } + core.info('Formula was already current; no new exact-SHA cache acknowledgement is required.'); + return; + } + if (!/^[0-9a-f]{40}$/.test(targetSha)) { + throw new Error('Formula baseline target output is malformed'); + } + const expectedExternalId = `release-${context.runId}-${targetSha}`; + let promotionCheck; + try { + const created = await github.rest.checks.create({ + ...context.repo, + name: 'Coverage Baseline Cache', + head_sha: targetSha, + status: 'queued', + external_id: expectedExternalId, + output: { + title: 'Waiting for exact-SHA baseline promotion', + summary: + 'The independent release governance job is waiting for the default-branch cache producer.', + }, + }); + promotionCheck = created.data; + await github.rest.repos.createDispatchEvent({ + ...context.repo, + event_type: 'coverage-baseline-promote', + client_payload: { + target_sha: targetSha, + source_run_id: String(context.runId), + check_run_id: String(promotionCheck.id), + }, + }); + } catch (error) { + if (promotionCheck) { + try { + await github.rest.checks.update({ + ...context.repo, + check_run_id: promotionCheck.id, + status: 'completed', + conclusion: 'failure', + completed_at: new Date().toISOString(), + output: { + title: 'Coverage baseline dispatch failed', + summary: `Release could not dispatch the exact-SHA producer: ${error.message}`, + }, + }); + } catch (cleanupError) { + core.error( + `Could not close failed cache acknowledgement ${promotionCheck.id}: ${cleanupError.message}`, + ); + } + } + throw error; + } + const checkRunId = promotionCheck.id; + for (let attempt = 1; attempt <= 180; attempt += 1) { + const {data: currentCheck} = await github.rest.checks.get({ + ...context.repo, + check_run_id: checkRunId, + }); + if ( + currentCheck.head_sha !== targetSha || + currentCheck.name !== 'Coverage Baseline Cache' || + currentCheck.external_id !== expectedExternalId || + currentCheck.app?.slug !== 'github-actions' + ) { + throw new Error('Formula baseline promotion acknowledgement changed identity'); + } + if (currentCheck.status === 'completed') { + if (currentCheck.conclusion !== 'success') { + throw new Error( + `Formula baseline promotion failed with ${currentCheck.conclusion || 'unknown'}`, + ); + } + core.info(`Formula baseline promotion completed for ${targetSha}.`); + return; + } + if (attempt < 180) { + await new Promise(resolve => setTimeout(resolve, 10000)); + } + } + throw new Error(`Formula baseline promotion timed out for ${targetSha}`); + release-delivery-gate: name: Release delivery gate if: ${{ !cancelled() }} @@ -2189,6 +2311,7 @@ jobs: - verify-darwin-signatures - publish-release - publish-channels + - coverage-baseline-confirmation - mirror-gitee-release - repair-npm - repair-channel @@ -2211,6 +2334,7 @@ jobs: DARWIN_SIGNATURE_RESULT: ${{ needs.verify-darwin-signatures.result }} PUBLISH_RELEASE_RESULT: ${{ needs.publish-release.result }} PUBLISH_CHANNELS_RESULT: ${{ needs.publish-channels.result }} + COVERAGE_BASELINE_CONFIRMATION_RESULT: ${{ needs.coverage-baseline-confirmation.result }} MIRROR_GITEE_RESULT: ${{ needs.mirror-gitee-release.result }} REPAIR_NPM_RESULT: ${{ needs.repair-npm.result }} REPAIR_CHANNEL_RESULT: ${{ needs.repair-channel.result }} @@ -2234,6 +2358,7 @@ jobs: require_result verify-darwin-signatures "$DARWIN_SIGNATURE_RESULT" success require_result publish-release "$PUBLISH_RELEASE_RESULT" success require_result publish-channels "$PUBLISH_CHANNELS_RESULT" success + require_result coverage-baseline-confirmation "$COVERAGE_BASELINE_CONFIRMATION_RESULT" success if test "$GITEE_FALLBACK_ENABLED" = true; then require_result mirror-gitee-release "$MIRROR_GITEE_RESULT" success else @@ -2273,6 +2398,7 @@ jobs: require_result verify-darwin-signatures "$DARWIN_SIGNATURE_RESULT" skipped require_result publish-release "$PUBLISH_RELEASE_RESULT" skipped require_result publish-channels "$PUBLISH_CHANNELS_RESULT" skipped + require_result coverage-baseline-confirmation "$COVERAGE_BASELINE_CONFIRMATION_RESULT" skipped require_result mirror-gitee-release "$MIRROR_GITEE_RESULT" skipped require_result repair-npm "$REPAIR_NPM_RESULT" skipped require_result repair-channel "$REPAIR_CHANNEL_RESULT" skipped @@ -2294,6 +2420,7 @@ jobs: require_result verify-darwin-signatures "$DARWIN_SIGNATURE_RESULT" skipped require_result publish-release "$PUBLISH_RELEASE_RESULT" skipped require_result publish-channels "$PUBLISH_CHANNELS_RESULT" skipped + require_result coverage-baseline-confirmation "$COVERAGE_BASELINE_CONFIRMATION_RESULT" skipped require_result mirror-gitee-release "$MIRROR_GITEE_RESULT" skipped require_result repair-npm "$REPAIR_NPM_RESULT" skipped require_result repair-channel "$REPAIR_CHANNEL_RESULT" skipped @@ -2309,6 +2436,7 @@ jobs: require_result verify-darwin-signatures "$DARWIN_SIGNATURE_RESULT" skipped require_result publish-release "$PUBLISH_RELEASE_RESULT" skipped require_result publish-channels "$PUBLISH_CHANNELS_RESULT" skipped + require_result coverage-baseline-confirmation "$COVERAGE_BASELINE_CONFIRMATION_RESULT" skipped require_result mirror-gitee-release "$MIRROR_GITEE_RESULT" skipped require_result repair-channel "$REPAIR_CHANNEL_RESULT" skipped require_cloud_jobs_skipped @@ -2323,6 +2451,7 @@ jobs: require_result verify-darwin-signatures "$DARWIN_SIGNATURE_RESULT" skipped require_result publish-release "$PUBLISH_RELEASE_RESULT" skipped require_result publish-channels "$PUBLISH_CHANNELS_RESULT" skipped + require_result coverage-baseline-confirmation "$COVERAGE_BASELINE_CONFIRMATION_RESULT" skipped require_result mirror-gitee-release "$MIRROR_GITEE_RESULT" skipped require_result repair-npm "$REPAIR_NPM_RESULT" skipped require_cloud_jobs_skipped @@ -2337,6 +2466,7 @@ jobs: require_result verify-darwin-signatures "$DARWIN_SIGNATURE_RESULT" skipped require_result publish-release "$PUBLISH_RELEASE_RESULT" skipped require_result publish-channels "$PUBLISH_CHANNELS_RESULT" skipped + require_result coverage-baseline-confirmation "$COVERAGE_BASELINE_CONFIRMATION_RESULT" skipped require_result mirror-gitee-release "$MIRROR_GITEE_RESULT" skipped require_result repair-npm "$REPAIR_NPM_RESULT" skipped require_cloud_jobs_skipped diff --git a/.github/workflows/reviewer-router.yml b/.github/workflows/reviewer-router.yml index 105ac713..7e33568a 100644 --- a/.github/workflows/reviewer-router.yml +++ b/.github/workflows/reviewer-router.yml @@ -3,21 +3,24 @@ name: Reviewer routing on: pull_request_target: branches: [main] - types: [opened, synchronize, reopened, ready_for_review] + types: [opened, synchronize, reopened, ready_for_review, edited, auto_merge_enabled] + push: + branches: [main] + workflow_dispatch: # pull_request_target deliberately runs only this workflow from the protected # base branch. Never check out or execute pull-request code here. permissions: - contents: write + contents: read pull-requests: write concurrency: - group: reviewer-router-${{ github.event.pull_request.number }} - cancel-in-progress: true + group: reviewer-router-${{ github.event_name == 'pull_request_target' && format('pr-{0}', github.event.pull_request.number) || 'reconcile-main' }} + cancel-in-progress: ${{ github.event_name == 'pull_request_target' }} jobs: route: - if: github.event.pull_request.draft == false + if: github.event_name == 'pull_request_target' && github.event.pull_request.draft == false runs-on: ubuntu-latest timeout-minutes: 5 steps: @@ -26,7 +29,7 @@ jobs: with: ref: ${{ github.event.pull_request.base.sha }} persist-credentials: false - - name: Route review and enable auto-merge + - name: Route review uses: actions/github-script@f28e40c7f34bde8b3046d885e986cb6290c5673b # v7.1.0 with: script: | @@ -34,6 +37,7 @@ jobs: const repo = context.repo.repo; const pullNumber = context.payload.pull_request.number; const eventHeadSha = context.payload.pull_request.head.sha; + const eventBaseSha = context.payload.pull_request.base.sha; const { REVIEWER_POOL, requestReviewersWithFallback, @@ -50,6 +54,7 @@ jobs: }); if ( currentPull.head.sha !== eventHeadSha || + currentPull.base.sha !== eventBaseSha || currentPull.state !== 'open' || currentPull.draft || currentPull.base.ref !== 'main' @@ -256,41 +261,6 @@ jobs: } } - async function enableAutoMerge() { - try { - const currentPull = await getReadyEventPull('auto-merge enable'); - if (!currentPull) { - return; - } - if (currentPull.auto_merge) { - core.info(`Auto-merge is already enabled for PR #${pullNumber}.`); - return; - } - await github.graphql( - `mutation EnableAutoMerge($pullRequestId: ID!) { - enablePullRequestAutoMerge( - input: { - pullRequestId: $pullRequestId - mergeMethod: MERGE - } - ) { - pullRequest { - autoMergeRequest { - enabledAt - } - } - } - }`, - {pullRequestId: currentPull.node_id}, - ); - core.info(`Enabled native auto-merge for PR #${pullNumber}.`); - } catch (error) { - core.warning( - `Could not enable auto-merge for PR #${pullNumber}; checks and review can continue normally (${error.message}).`, - ); - } - } - try { await routeReview(); } catch (error) { @@ -298,4 +268,706 @@ jobs: `Reviewer routing hit an unexpected error for PR #${pullNumber}; review can still proceed manually (${error.message}).`, ); } - await enableAutoMerge(); + manage-auto-merge: + if: github.event_name == 'pull_request_target' && github.event.pull_request.draft == false + runs-on: ubuntu-latest + timeout-minutes: 5 + # Contents write is isolated to the trusted cleanup job because GitHub + # treats auto-merge management as merge authority. Only the App token is + # ever used to enable auto-merge. + permissions: + contents: write + pull-requests: write + steps: + # Auto-merge must use a dedicated identity. GitHub suppresses most + # workflow events created by the built-in GITHUB_TOKEN, including the + # main push that produces post-merge admission and baseline caches. + # Clear the known-unsafe owner and any workflow-skipping merge request + # before touching App credentials. Missing credentials then fail closed + # with manual merge intact. + - name: Clear unsafe auto-merge request + uses: actions/github-script@f28e40c7f34bde8b3046d885e986cb6290c5673b # v7.1.0 + with: + script: | + const owner = context.repo.owner; + const repo = context.repo.repo; + const pullNumber = context.payload.pull_request.number; + const eventHeadSha = context.payload.pull_request.head.sha; + const eventBaseSha = context.payload.pull_request.base.sha; + const unsafeOwner = 'github-actions[bot]'; + const skipWorkflowPattern = + /\[(?:skip ci|ci skip|no ci|skip actions|actions skip)\]|\bskip-checks\s*:\s*true\b/i; + + const {data: currentPull} = await github.rest.pulls.get({ + owner, + repo, + pull_number: pullNumber, + }); + if ( + currentPull.head.sha !== eventHeadSha || + currentPull.base.sha !== eventBaseSha || + currentPull.state !== 'open' || + currentPull.draft || + currentPull.base.ref !== 'main' + ) { + core.info( + `PR #${pullNumber} state or revision no longer matches this ready-main event; unsafe-owner cleanup stopped.`, + ); + return; + } + + const enabledBy = currentPull.auto_merge?.enabled_by?.login?.toLowerCase(); + const mergeTexts = [ + currentPull.title, + currentPull.auto_merge?.commit_title, + currentPull.auto_merge?.commit_message, + ].filter(value => typeof value === 'string'); + const skipRequested = mergeTexts.some(value => + skipWorkflowPattern.test(value), + ); + if (!currentPull.auto_merge || (enabledBy !== unsafeOwner && !skipRequested)) { + core.info( + currentPull.auto_merge + ? `PR #${pullNumber} auto-merge is owned by ${enabledBy || 'an unknown actor'}; preserving it.` + : `PR #${pullNumber} has no auto-merge request to clear.`, + ); + return; + } + + await github.graphql( + `mutation DisableAutoMerge($pullRequestId: ID!) { + disablePullRequestAutoMerge( + input: {pullRequestId: $pullRequestId} + ) { + pullRequest { + id + } + } + }`, + {pullRequestId: currentPull.node_id}, + ); + const {data: cleanedPull} = await github.rest.pulls.get({ + owner, + repo, + pull_number: pullNumber, + }); + const cleanedOwner = cleanedPull.auto_merge?.enabled_by?.login?.toLowerCase(); + if ( + cleanedPull.head.sha === eventHeadSha && + cleanedPull.base.sha === eventBaseSha && + cleanedPull.state === 'open' && + !cleanedPull.draft && + cleanedPull.base.ref === 'main' && + cleanedPull.auto_merge + ) { + throw new Error( + `PR #${pullNumber} still has auto-merge owned by ${cleanedOwner || 'an unknown actor'} after unsafe-request cleanup.`, + ); + } + core.info( + skipRequested + ? `Cleared workflow-skipping auto-merge metadata from PR #${pullNumber}.` + : `Cleared built-in GITHUB_TOKEN auto-merge ownership from PR #${pullNumber}.`, + ); + - name: Mint Reviewer Router GitHub App token + id: reviewer-router-token + uses: actions/create-github-app-token@bcd2ba49218906704ab6c1aa796996da409d3eb1 # v3.2.0 + with: + client-id: ${{ vars.REVIEWER_ROUTER_APP_CLIENT_ID }} + private-key: ${{ secrets.REVIEWER_ROUTER_APP_PRIVATE_KEY }} + owner: ${{ github.repository_owner }} + repositories: ${{ github.event.repository.name }} + permission-contents: write + permission-pull-requests: write + - name: Enable auto-merge with dedicated App + uses: actions/github-script@f28e40c7f34bde8b3046d885e986cb6290c5673b # v7.1.0 + env: + MINTED_REVIEWER_ROUTER_APP_SLUG: ${{ steps.reviewer-router-token.outputs.app-slug }} + REVIEWER_ROUTER_APP_SLUG: ${{ vars.REVIEWER_ROUTER_APP_SLUG }} + with: + github-token: ${{ steps.reviewer-router-token.outputs.token }} + script: | + const owner = context.repo.owner; + const repo = context.repo.repo; + const pullNumber = context.payload.pull_request.number; + const eventHeadSha = context.payload.pull_request.head.sha; + const eventBaseSha = context.payload.pull_request.base.sha; + const mintedAppSlug = process.env.MINTED_REVIEWER_ROUTER_APP_SLUG?.trim(); + const appSlug = process.env.REVIEWER_ROUTER_APP_SLUG?.trim(); + if ( + !appSlug || + appSlug !== appSlug.toLowerCase() || + appSlug === 'github-actions' || + mintedAppSlug !== appSlug + ) { + throw new Error( + 'Minted Reviewer Router App slug does not exactly match the reviewed repository variable.', + ); + } + const expectedAppOwner = `${appSlug}[bot]`; + const repositorySource = `${owner}/${repo}`.toLowerCase(); + const appliedRules = await github.paginate( + 'GET /repos/{owner}/{repo}/rules/branches/{branch}', + {owner, repo, branch: 'main', per_page: 100}, + ); + const applicableRulesetIDs = [ + ...new Set( + appliedRules + .map(rule => Number(rule.ruleset_id)) + .filter(rulesetID => + Number.isSafeInteger(rulesetID) && rulesetID > 0, + ), + ), + ]; + const activeMainRulesets = []; + for (const rulesetID of applicableRulesetIDs) { + const {data: ruleset} = await github.request( + 'GET /repos/{owner}/{repo}/rulesets/{ruleset_id}', + {owner, repo, ruleset_id: rulesetID}, + ); + if ( + ruleset.enforcement !== 'active' || + ruleset.target !== 'branch' + ) { + throw new Error( + `Applicable ruleset ${ruleset.name || rulesetID} is not an active branch ruleset.`, + ); + } + activeMainRulesets.push(ruleset); + } + const writerRulesets = activeMainRulesets.filter( + ruleset => ruleset.name === 'main-merge-writers', + ); + if (writerRulesets.length !== 1) { + throw new Error( + 'Expected exactly one active main-merge-writers ruleset on main.', + ); + } + const writerRuleset = writerRulesets[0]; + const writerIncludes = writerRuleset.conditions?.ref_name?.include || []; + const writerExcludes = writerRuleset.conditions?.ref_name?.exclude || []; + if ( + writerRuleset.source_type !== 'Repository' || + writerRuleset.source?.toLowerCase() !== repositorySource || + writerIncludes.length !== 1 || + writerIncludes[0] !== 'refs/heads/main' || + writerExcludes.length !== 0 || + writerRuleset.rules?.length !== 1 || + writerRuleset.rules[0].type !== 'update' || + writerRuleset.rules[0].parameters?.update_allows_fetch_and_merge !== false || + writerRuleset.current_user_can_bypass !== 'pull_requests_only' + ) { + throw new Error( + 'Reviewer Router App requires the exact repository-owned main writer rule with pull-request-only bypass.', + ); + } + for (const ruleset of activeMainRulesets) { + if (ruleset.id === writerRuleset.id) { + continue; + } + if (ruleset.current_user_can_bypass !== 'never') { + throw new Error( + `Reviewer Router App must never bypass main ruleset ${ruleset.name || ruleset.id}.`, + ); + } + } + const skipWorkflowPattern = + /\[(?:skip ci|ci skip|no ci|skip actions|actions skip)\]|\bskip-checks\s*:\s*true\b/i; + const safeCommitHeadline = `Merge pull request #${pullNumber}`; + const safeCommitBody = + `Merged by the dedicated Reviewer Router GitHub App for PR #${pullNumber}.`; + + async function disableAutoMerge(pull, phase) { + await github.graphql( + `mutation DisableAutoMerge($pullRequestId: ID!) { + disablePullRequestAutoMerge( + input: {pullRequestId: $pullRequestId} + ) { + pullRequest { id } + } + }`, + {pullRequestId: pull.node_id}, + ); + const {data: revertedPull} = await github.rest.pulls.get({ + owner, + repo, + pull_number: pullNumber, + }); + if (revertedPull.auto_merge) { + throw new Error( + `PR #${pullNumber} retained auto-merge owned by ${revertedPull.auto_merge.enabled_by?.login || 'an unknown actor'} after ${phase}.`, + ); + } + return revertedPull; + } + + async function getReadyEventPull(phase) { + const {data: currentPull} = await github.rest.pulls.get({ + owner, + repo, + pull_number: pullNumber, + }); + if ( + currentPull.head.sha !== eventHeadSha || + currentPull.base.sha !== eventBaseSha || + currentPull.state !== 'open' || + currentPull.draft || + currentPull.base.ref !== 'main' + ) { + core.info( + `PR #${pullNumber} state or revision no longer matches this ready-main event during ${phase}; auto-merge stopped.`, + ); + return null; + } + return currentPull; + } + + let currentPull = await getReadyEventPull('auto-merge enable'); + if (!currentPull) { + return; + } + const mergeTexts = [ + currentPull.title, + currentPull.auto_merge?.commit_title, + currentPull.auto_merge?.commit_message, + ].filter(value => typeof value === 'string'); + if (mergeTexts.some(value => skipWorkflowPattern.test(value))) { + if (currentPull.auto_merge) { + await disableAutoMerge(currentPull, 'workflow-skip cleanup'); + } + throw new Error( + `PR #${pullNumber} merge metadata contains a GitHub workflow-skip directive.`, + ); + } + if (currentPull.auto_merge) { + const existingOwner = currentPull.auto_merge.enabled_by?.login?.toLowerCase(); + if ( + existingOwner === expectedAppOwner && + currentPull.auto_merge.commit_title === safeCommitHeadline && + currentPull.auto_merge.commit_message === safeCommitBody + ) { + core.info( + `PR #${pullNumber} already has the reviewed ${expectedAppOwner} auto-merge request; preserving it.`, + ); + return; + } + await disableAutoMerge( + currentPull, + existingOwner === expectedAppOwner + ? 'unsafe dedicated-App metadata cleanup' + : 'non-dedicated owner cleanup', + ); + currentPull = await getReadyEventPull('post-owner cleanup'); + if (!currentPull) { + return; + } + if (currentPull.auto_merge) { + throw new Error( + `PR #${pullNumber} acquired another auto-merge owner before dedicated-App enable.`, + ); + } + } + await github.graphql( + `mutation EnableAutoMerge( + $pullRequestId: ID! + $expectedHeadOid: GitObjectID! + $commitHeadline: String! + $commitBody: String! + ) { + enablePullRequestAutoMerge( + input: { + pullRequestId: $pullRequestId + mergeMethod: MERGE + expectedHeadOid: $expectedHeadOid + commitHeadline: $commitHeadline + commitBody: $commitBody + } + ) { + pullRequest { + autoMergeRequest { + enabledAt + } + } + } + }`, + { + pullRequestId: currentPull.node_id, + expectedHeadOid: eventHeadSha, + commitHeadline: safeCommitHeadline, + commitBody: safeCommitBody, + }, + ); + const {data: enabledPull} = await github.rest.pulls.get({ + owner, + repo, + pull_number: pullNumber, + }); + const enabledBy = enabledPull?.auto_merge?.enabled_by?.login?.toLowerCase(); + const exactRevision = + enabledPull.head.sha === eventHeadSha && + enabledPull.base.sha === eventBaseSha && + enabledPull.state === 'open' && + !enabledPull.draft && + enabledPull.base.ref === 'main'; + if (!exactRevision && enabledBy === expectedAppOwner) { + await disableAutoMerge(enabledPull, 'stale-enable cleanup'); + } + if (!exactRevision) { + throw new Error( + `PR #${pullNumber} state or revision changed during dedicated-App enable.`, + ); + } + if (enabledBy !== expectedAppOwner) { + if (enabledPull.auto_merge) { + await disableAutoMerge(enabledPull, 'unexpected owner cleanup'); + } + throw new Error( + `PR #${pullNumber} auto-merge owner ${enabledBy || 'unknown'} does not match ${expectedAppOwner}.`, + ); + } + if ( + enabledPull.auto_merge?.commit_title !== safeCommitHeadline || + enabledPull.auto_merge?.commit_message !== safeCommitBody + ) { + await disableAutoMerge(enabledPull, 'unsafe-merge-message cleanup'); + throw new Error( + `PR #${pullNumber} dedicated-App merge metadata was not preserved exactly.`, + ); + } + core.info( + `Enabled native auto-merge for PR #${pullNumber} with ${enabledBy}.`, + ); + + # Existing ready PRs do not emit a fresh routing event when this workflow is + # deployed. The protected-main push that deploys it migrates every non-App + # request, repairs App requests with unreviewed metadata, and clears + # workflow-skipping merge metadata; workflow_dispatch provides an idempotent + # recovery path. Null and exact safe App requests remain untouched. + reconcile: + if: (github.event_name == 'push' || github.event_name == 'workflow_dispatch') && github.ref == 'refs/heads/main' && github.repository == 'DingTalk-Real-AI/dingtalk-workspace-cli' + runs-on: ubuntu-latest + timeout-minutes: 5 + permissions: {} + steps: + - name: Mint Reviewer Router GitHub App token + id: reviewer-router-reconcile-token + uses: actions/create-github-app-token@bcd2ba49218906704ab6c1aa796996da409d3eb1 # v3.2.0 + with: + client-id: ${{ vars.REVIEWER_ROUTER_APP_CLIENT_ID }} + private-key: ${{ secrets.REVIEWER_ROUTER_APP_PRIVATE_KEY }} + owner: ${{ github.repository_owner }} + repositories: ${{ github.event.repository.name }} + permission-contents: write + permission-pull-requests: write + - name: Reconcile unsafe auto-merge requests + uses: actions/github-script@f28e40c7f34bde8b3046d885e986cb6290c5673b # v7.1.0 + env: + MINTED_REVIEWER_ROUTER_APP_SLUG: ${{ steps.reviewer-router-reconcile-token.outputs.app-slug }} + REVIEWER_ROUTER_APP_SLUG: ${{ vars.REVIEWER_ROUTER_APP_SLUG }} + with: + github-token: ${{ steps.reviewer-router-reconcile-token.outputs.token }} + script: | + const owner = context.repo.owner; + const repo = context.repo.repo; + const mintedAppSlug = process.env.MINTED_REVIEWER_ROUTER_APP_SLUG?.trim(); + const appSlug = process.env.REVIEWER_ROUTER_APP_SLUG?.trim(); + if ( + !appSlug || + appSlug !== appSlug.toLowerCase() || + appSlug === 'github-actions' || + mintedAppSlug !== appSlug + ) { + throw new Error( + 'Minted Reviewer Router App slug does not exactly match the reviewed repository variable.', + ); + } + const expectedAppOwner = `${appSlug}[bot]`; + const repositorySource = `${owner}/${repo}`.toLowerCase(); + const appliedRules = await github.paginate( + 'GET /repos/{owner}/{repo}/rules/branches/{branch}', + {owner, repo, branch: 'main', per_page: 100}, + ); + const applicableRulesetIDs = [ + ...new Set( + appliedRules + .map(rule => Number(rule.ruleset_id)) + .filter(rulesetID => + Number.isSafeInteger(rulesetID) && rulesetID > 0, + ), + ), + ]; + const activeMainRulesets = []; + for (const rulesetID of applicableRulesetIDs) { + const {data: ruleset} = await github.request( + 'GET /repos/{owner}/{repo}/rulesets/{ruleset_id}', + {owner, repo, ruleset_id: rulesetID}, + ); + if ( + ruleset.enforcement !== 'active' || + ruleset.target !== 'branch' + ) { + throw new Error( + `Applicable ruleset ${ruleset.name || rulesetID} is not an active branch ruleset.`, + ); + } + activeMainRulesets.push(ruleset); + } + const writerRulesets = activeMainRulesets.filter( + ruleset => ruleset.name === 'main-merge-writers', + ); + if (writerRulesets.length !== 1) { + throw new Error( + 'Expected exactly one active main-merge-writers ruleset on main.', + ); + } + const writerRuleset = writerRulesets[0]; + const writerIncludes = writerRuleset.conditions?.ref_name?.include || []; + const writerExcludes = writerRuleset.conditions?.ref_name?.exclude || []; + if ( + writerRuleset.source_type !== 'Repository' || + writerRuleset.source?.toLowerCase() !== repositorySource || + writerIncludes.length !== 1 || + writerIncludes[0] !== 'refs/heads/main' || + writerExcludes.length !== 0 || + writerRuleset.rules?.length !== 1 || + writerRuleset.rules[0].type !== 'update' || + writerRuleset.rules[0].parameters?.update_allows_fetch_and_merge !== false || + writerRuleset.current_user_can_bypass !== 'pull_requests_only' + ) { + throw new Error( + 'Reviewer Router App requires the exact repository-owned main writer rule with pull-request-only bypass.', + ); + } + for (const ruleset of activeMainRulesets) { + if (ruleset.id === writerRuleset.id) { + continue; + } + if (ruleset.current_user_can_bypass !== 'never') { + throw new Error( + `Reviewer Router App must never bypass main ruleset ${ruleset.name || ruleset.id}.`, + ); + } + } + const skipWorkflowPattern = + /\[(?:skip ci|ci skip|no ci|skip actions|actions skip)\]|\bskip-checks\s*:\s*true\b/i; + const openPulls = await github.paginate(github.rest.pulls.list, { + owner, + repo, + state: 'open', + base: 'main', + per_page: 100, + }); + let migrated = 0; + let skipped = 0; + const failures = []; + + async function disableAutoMerge(pullNumber, pullRequestId, phase) { + await github.graphql( + `mutation DisableAutoMerge($pullRequestId: ID!) { + disablePullRequestAutoMerge( + input: {pullRequestId: $pullRequestId} + ) { + pullRequest { id } + } + }`, + {pullRequestId}, + ); + const {data: revertedPull} = await github.rest.pulls.get({ + owner, + repo, + pull_number: pullNumber, + }); + if (revertedPull.auto_merge) { + throw new Error( + `auto-merge owned by ${revertedPull.auto_merge.enabled_by?.login || 'an unknown actor'} survived ${phase}`, + ); + } + return revertedPull; + } + + for (const candidate of openPulls) { + const safeCommitHeadline = `Merge pull request #${candidate.number}`; + const safeCommitBody = + `Merged by the dedicated Reviewer Router GitHub App for PR #${candidate.number}.`; + const candidateOwner = + candidate.auto_merge?.enabled_by?.login?.toLowerCase(); + const candidateMergeTexts = [ + candidate.title, + candidate.auto_merge?.commit_title, + candidate.auto_merge?.commit_message, + ].filter(value => typeof value === 'string'); + const candidateSkipRequested = candidateMergeTexts.some(value => + skipWorkflowPattern.test(value), + ); + const candidateHasSafeAppRequest = + candidateOwner === expectedAppOwner && + candidate.auto_merge?.commit_title === safeCommitHeadline && + candidate.auto_merge?.commit_message === safeCommitBody && + !candidateSkipRequested; + if ( + candidate.draft || + candidate.base.ref !== 'main' || + !candidate.auto_merge || + candidateHasSafeAppRequest + ) { + skipped += 1; + continue; + } + let stage = 'preflight'; + try { + const expectedHeadSha = candidate.head.sha; + const expectedBaseSha = candidate.base.sha; + const {data: currentPull} = await github.rest.pulls.get({ + owner, + repo, + pull_number: candidate.number, + }); + const mergeTexts = [ + currentPull.title, + currentPull.auto_merge?.commit_title, + currentPull.auto_merge?.commit_message, + ].filter(value => typeof value === 'string'); + const skipRequested = mergeTexts.some(value => + skipWorkflowPattern.test(value), + ); + const currentOwner = + currentPull.auto_merge?.enabled_by?.login?.toLowerCase(); + const currentHasSafeAppRequest = + currentOwner === expectedAppOwner && + currentPull.auto_merge?.commit_title === safeCommitHeadline && + currentPull.auto_merge?.commit_message === safeCommitBody && + !skipRequested; + if ( + currentPull.head.sha !== expectedHeadSha || + currentPull.base.sha !== expectedBaseSha || + currentPull.state !== 'open' || + currentPull.draft || + currentPull.base.ref !== 'main' || + !currentPull.auto_merge || + currentHasSafeAppRequest + ) { + skipped += 1; + core.info(`PR #${candidate.number} changed before reconciliation; skipped.`); + continue; + } + + stage = skipRequested + ? 'disable workflow-skipping auto-merge request' + : currentOwner === expectedAppOwner + ? 'repair dedicated-App merge metadata' + : 'replace non-dedicated auto-merge owner'; + const disabledPull = await disableAutoMerge( + candidate.number, + currentPull.node_id, + stage, + ); + stage = 'post-disable validation'; + if ( + disabledPull.head.sha !== expectedHeadSha || + disabledPull.base.sha !== expectedBaseSha || + disabledPull.state !== 'open' || + disabledPull.draft || + disabledPull.base.ref !== 'main' + ) { + throw new Error( + 'revision changed after the unsafe owner was disabled; left manual-merge only', + ); + } + if (disabledPull.auto_merge) { + throw new Error('another auto-merge owner appeared after disable'); + } + if (skipRequested) { + throw new Error( + 'workflow-skip directive found in merge metadata; left manual-merge only', + ); + } + + stage = 'dedicated-App enable'; + await github.graphql( + `mutation EnableAutoMerge( + $pullRequestId: ID! + $expectedHeadOid: GitObjectID! + $commitHeadline: String! + $commitBody: String! + ) { + enablePullRequestAutoMerge( + input: { + pullRequestId: $pullRequestId + mergeMethod: MERGE + expectedHeadOid: $expectedHeadOid + commitHeadline: $commitHeadline + commitBody: $commitBody + } + ) { + pullRequest { id } + } + }`, + { + pullRequestId: disabledPull.node_id, + expectedHeadOid: expectedHeadSha, + commitHeadline: safeCommitHeadline, + commitBody: safeCommitBody, + }, + ); + stage = 'post-enable validation'; + const {data: enabledPull} = await github.rest.pulls.get({ + owner, + repo, + pull_number: candidate.number, + }); + const enabledBy = enabledPull.auto_merge?.enabled_by?.login?.toLowerCase(); + const revisionChanged = + enabledPull.head.sha !== expectedHeadSha || + enabledPull.base.sha !== expectedBaseSha || + enabledPull.state !== 'open' || + enabledPull.draft || + enabledPull.base.ref !== 'main'; + if (revisionChanged && enabledBy === expectedAppOwner) { + await disableAutoMerge( + candidate.number, + enabledPull.node_id, + 'stale-enable cleanup', + ); + } + if (revisionChanged) { + throw new Error('revision changed during dedicated-App enable'); + } + if (enabledBy !== expectedAppOwner) { + if (enabledPull.auto_merge) { + await disableAutoMerge( + candidate.number, + enabledPull.node_id, + 'unexpected owner cleanup', + ); + } + throw new Error( + `auto-merge owner ${enabledBy || 'unknown'} does not match ${expectedAppOwner}`, + ); + } + if ( + enabledPull.auto_merge?.commit_title !== safeCommitHeadline || + enabledPull.auto_merge?.commit_message !== safeCommitBody + ) { + await disableAutoMerge( + candidate.number, + enabledPull.node_id, + 'unsafe merge-message cleanup', + ); + throw new Error('dedicated-App merge metadata was not preserved exactly'); + } + migrated += 1; + core.info(`Migrated PR #${candidate.number} auto-merge to ${enabledBy}.`); + } catch (error) { + const failure = `PR #${candidate.number} failed during ${stage}: ${error.message}`; + failures.push(failure); + core.error(failure); + } + } + core.info( + `Reconciliation summary: migrated=${migrated}, skipped=${skipped}, failed=${failures.length}.`, + ); + if (failures.length > 0) { + core.setFailed( + `Legacy auto-merge reconciliation had ${failures.length} failure(s): ${failures.join('; ')}`, + ); + } diff --git a/docs/automation.md b/docs/automation.md index b2d5de21..666d2a07 100644 --- a/docs/automation.md +++ b/docs/automation.md @@ -62,9 +62,114 @@ make lint git diff --check ``` +## Reviewer Router GitHub App + +Reviewer requests and native auto-merge intentionally use different +identities. The base-owned `pull_request_target` workflow may use its built-in +`GITHUB_TOKEN` to request reviewers, but it must mint a dedicated GitHub App +installation token before enabling auto-merge. GitHub suppresses most workflow +events created by the built-in token; using it for auto-merge prevents the +merge commit's `push` workflows from running and leaves the exact-SHA Coverage +baseline without a trusted main-scoped producer. + +Configure the dedicated App before merging a workflow revision that requires +it: + +- install it only on `DingTalk-Real-AI/dingtalk-workspace-cli`; +- grant only `Contents: read and write` and `Pull requests: read and write`; +- set repository variable `REVIEWER_ROUTER_APP_CLIENT_ID` to its client ID; +- set `REVIEWER_ROUTER_APP_SLUG` to its exact lowercase slug; +- set repository secret `REVIEWER_ROUTER_APP_PRIVATE_KEY` to its private key; +- create one active repository branch ruleset named `main-merge-writers`, + targeting only `refs/heads/main`, with exactly one `Restrict updates` rule + (`update_allows_fetch_and_merge: false`); +- give that ruleset exactly two bypass actors: the Reviewer Router App as an + `Integration` in `pull_request` mode, and `PeterGuy326` (ID `47820304`) in + `always` mode for Formula publication and break-glass recovery; +- never give the App bypass on `main-protection`, `main-quality`, or any other + ruleset, and never reuse `HOMEBREW_PR_TOKEN`, + `RELEASE_GOVERNANCE_TOKEN`, or a personal token for Reviewer Router. + +The workflow limits each minted token to the current repository, requests the +two permissions explicitly, and lets the token action revoke it at job end. +It also requires the minted App slug to equal the reviewed repository variable; +there is no `GITHUB_TOKEN` fallback. Before reading App credentials, the +base-owned workflow revalidates the event's exact base/head and uses its +built-in token only to disable an existing request owned by +`github-actions[bot]` or one whose title or merge metadata requests that GitHub +skip workflows. A mint or permission failure therefore leaves that PR +manual-merge only. The built-in token's `Contents: write` permission is +isolated to this trusted cleanup job and is never used to enable auto-merge; +review routing keeps `Contents: read`. Existing requests owned by a human or +another non-built-in identity are replaced with the exact dedicated-App +request after token minting. Only an already App-owned request with the fixed +headline/body is preserved. The required `Test` context reads the live +repository settings and applied rulesets, verifies the exact writer-rule +shape, and requires its own built-in Actions identity to report +`current_user_can_bypass: never`. Before enabling or reconciling auto-merge, +the minted App independently requires `pull_requests_only` on that writer rule +and `never` on every other active main ruleset. These identity-relative checks +remain available to low-privilege tokens; GitHub deliberately hides the full +`bypass_actors` list from callers without ruleset-write access. Operators must +therefore inspect that list during rollout and keep it at the exact two actors +above. The required `Test` context then briefly waits for the concurrent +router takeover and accepts only a null request or the configured App owner +with exact fixed metadata. A null request is safe for this failure mode because +the built-in Actions identity cannot pass the writer rule; other permitted +identities emit either a protected-main push or the trusted closed-PR repair. +Draft PRs skip this identity check; the explicit `ready_for_review` trigger +reruns admission when they become merge-eligible, +while `edited` and `auto_merge_enabled` rerun both workflows when the PR title +or merge request changes. A human `auto_merge_disabled` event reruns CI without +silently re-enabling the request, leaving it available only to the designated +break-glass identity. The required `Test` context rejects GitHub workflow-skip +directives in the PR title or an existing auto-merge request and verifies the +repository's reviewed `MERGE_MESSAGE` title plus `PR_TITLE` or `BLANK` body +defaults. The dedicated App binds the mutation to the exact head OID and +supplies a fixed safe headline and body, so GitHub cannot copy an unsafe PR +title into its merge commit. +After enabling, the workflow requires the owner to equal the token action's +exact `[bot]` output. If the event base/head changes during the +mutation window, it removes only that App-owned request and fails the run. +The break-glass publisher must preserve a safe final commit message; +`[skip ci]`, `[ci skip]`, `[no ci]`, `[skip actions]`, +`[actions skip]`, and a `skip-checks: true` trailer are forbidden outside the +release-controlled Formula-only path below. + +GitHub may suppress `pull_request_target` entirely for security-sensitive head +branch names, including names that look like commit SHAs. Such a PR receives +neither App takeover nor the closed-event repair. Rename the head branch for +the normal path; if break-glass merge is unavoidable, preserve a safe final +message so the protected-main push CI remains the authoritative producer. + +After installing the App, the protected-main push that deploys this workflow +runs reconciliation automatically. The job enumerates open, ready `main` PRs +with any non-App owner, unsafe App metadata, or workflow-skip metadata. It +revalidates each base/head, converges a safe request to the exact dedicated-App +owner and fixed message, and leaves a workflow-skipping request disabled for +manual correction. It never enables auto-merge where the request was already +null. A mid-migration failure leaves the affected PR disabled for a fresh +routing event or break-glass merge. One PR failure is recorded +without preventing later legacy owners from being attempted; the batch ends +red with a per-PR summary. Manually dispatch `Reviewer routing` from `main` +until the failed count is zero. + +A PR that introduces or rotates this identity still runs the old base-owned +router. Install/configure the App and activate the exact writer ruleset first; +this blocks its legacy `github-actions[bot]` request from writing `main`. After +the governance PR's final push, disable that old request, confirm the live +settings/ruleset contract and all required checks are green for the exact head, +then have only `PeterGuy326` merge that head with the repository-generated safe +merge message. Verify the resulting merge SHA has a `CI` run with `event=push`, +a successful `Coverage` context, and an exact-SHA baseline cache under +`refs/heads/main`. Confirm automatic reconciliation reports zero failures and +zero non-App owners. Finally use a normal canary PR to verify that the dedicated +App is both `enabledBy` and `mergedBy`, and that the same post-merge chain +repeats before declaring the rollout complete. + ## Homebrew Formula Delivery -Official releases use the Release workflow's built-in `GITHUB_TOKEN` to update +Official releases use the designated `HOMEBREW_PR_TOKEN` identity to update exactly one tracked Formula after the immutable GitHub assets and their checksums have passed verification. The publisher validates the rendered Ruby, commits only the configured Formula path, never force-pushes `main`, and retries @@ -72,11 +177,57 @@ from a fresh clone up to three times when `main` advances concurrently. Normal stable and beta releases do not create a Formula PR or run a permission canary. The workflow uses the existing repository-scoped `HOMEBREW_PR_TOKEN` release identity because GitHub does not allow its built-in -Actions App to bypass this repository's rulesets. That identity is the sole -user bypass actor on the two default-branch rulesets. The workflow creates the +Actions App to bypass this repository's rulesets. Its owner is the designated +always-bypass actor for controlled Formula publication and break-glass recovery, +including on `main-merge-writers`. The workflow creates the nine Code Admission checks for the Formula-only commit only after proving its sole parent already has all nine successful checks and the committed Formula -exactly matches this release's verified bytes. +exactly matches this release's verified bytes. Formula commits retain +`[skip ci]`, so the sealing step exposes only the reviewed commit identity to +an independent confirmation job. That job creates the +`Coverage Baseline Cache` acknowledgement and emits the reviewed +`coverage-baseline-promote` repository dispatch. The default-branch +`Coverage Baseline Promotion` workflow independently verifies the exact +single-parent Formula commit, both parent and target admission contexts, and +default-branch containment before checking out the target. It restores only +the exact parent profile, recomputes the complete profile if that cache is +absent, and saves the Formula SHA under the `main` cache scope. Because the +cache save action treats upload errors as warnings, a second lookup must report +`cache-hit=true` for the exact target key before the producer succeeds. The +promotion completes the unique acknowledgement, and the confirmation job +waits for that exact check-run ID. npm and mirror publication depend only on +the immutable release job, so a transient +cache-service failure cannot strand an otherwise valid release between +channels; the final release-delivery gate still fails until the exact cache is +confirmed. Once Formula sealing exposes the target SHA, the confirmation job +also runs when a later immutable-package recheck fails, so a post-push failure +cannot orphan the producer. Rerun the failed promotion/confirmation path after +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 +`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 +trigger is forbidden from writing the default-branch cache directly. Before +dispatching, it gives Actions event delivery one minute to expose a run from +the exact protected `.github/workflows/ci.yml` workflow and exits if that normal producer already +owns the SHA, avoiding a duplicate full-suite run. A successful CI producer +must hard-verify its exact cache key. If that run instead completes with any +non-success conclusion, a separate base-owned `workflow_run` dispatcher binds +the exact workflow ID/path, run ID/attempt, conclusion, repository, branch, and +head SHA before requesting repair. `workflow_run` also has read-only +default-branch cache access, so both dispatchers use the reviewed +`repository_dispatch` exception. The dispatched default-branch producer +revalidates the corresponding merged-PR or failed-CI identity before checkout, +restores only the exact target key, recomputes the complete profile on a miss, +and verifies `cache-hit=true` after saving. An hourly schedule refreshes the +event-time `main` SHA after direct break-glass pushes or cache eviction; +`workflow_dispatch` provides the same current-main repair on demand. The +dedicated App identity remains mandatory because events created by the built-in +`GITHUB_TOKEN` can suppress both the main push and the closed-PR event. Keep `HOMEBREW_PR_TOKEN` repository-scoped with `Contents: write` and `Pull requests: write` (the latter remains necessary for withdrawal rollback), diff --git a/docs/ci-pr-gates.md b/docs/ci-pr-gates.md index 8016833d..c8387c6c 100644 --- a/docs/ci-pr-gates.md +++ b/docs/ci-pr-gates.md @@ -147,13 +147,57 @@ maintainer pool. A current-head approval or change request is preserved; after a new push, stale activity does not suppress a fresh request, and an outstanding change requester is preferred for continuity. -The branch ruleset keeps one human approval and all nine strict required -contexts, and requires someone other than the latest pusher to approve after -the most recent head update. Repository auto-merge is enabled for ready PRs, -so a PR merges after that approval and the current revision's nine checks are -green. If `main` advances, strict checks rerun before merge. The reviewer -router is orchestration, not a quality context, and must not be added to the -ruleset. +The branch rulesets keep one human approval and all nine strict required +contexts, require someone other than the latest pusher to approve after the +most recent head update, and restrict `main` updates to the dedicated Reviewer +Router App in pull-request mode plus the designated Formula publisher in +always/break-glass mode. Repository auto-merge is enabled for ready PRs, so the +App-owned request merges after that approval and the current revision's nine +checks are green. If `main` advances, strict checks rerun before merge. The +reviewer routing job uses the built-in `GITHUB_TOKEN` to request reviewers with +`Contents: read` and `Pull requests: write`. A separate base-owned cleanup job +isolates the merge-authority permissions (`Contents: write` and `Pull +requests: write`), revalidates the exact event base/head, and uses the built-in +token only to disable an existing request owned by `github-actions[bot]` or one +whose title or merge metadata requests that GitHub skip workflows; it never +enables auto-merge. The job then mints a current-repository installation token +for the dedicated Reviewer Router GitHub App, proves its emitted slug matches +the reviewed `REVIEWER_ROUTER_APP_SLUG`, replaces every non-App request, and +enables native auto-merge with fixed metadata. This +identity boundary is required because GitHub suppresses +most workflow events created by the built-in token; using it for auto-merge would +silently skip the merge commit's protected-main CI and baseline-cache +producer. Token minting or takeover fails closed without falling back to +`GITHUB_TOKEN`: the unsafe request is cleared before credentials are read, and +the required `Test` context live-verifies the exact `main-merge-writers` update +rule and requires its own built-in token to report +`current_user_can_bypass: never`. The minted App separately requires +`pull_requests_only` on that writer rule and `never` on every other active main +ruleset before it can enable or reconcile auto-merge. GitHub hides the complete +`bypass_actors` list from low-privilege callers, so the rollout audit must still +keep the writer list at exactly the Reviewer App plus `PeterGuy326` (ID +`47820304`). The required check finally accepts a null or exact App-owned +request after a short takeover grace period. Null is safe from the suppressed +event path because the built-in Actions identity cannot update `main`; other +permitted identities produce either a main push or the trusted closed-PR +repair. Drafts skip the identity step, while `ready_for_review`, `edited`, +`auto_merge_enabled`, and `auto_merge_disabled` explicitly start fresh admission +for readiness, title, and merge-request changes. Router does not react to +`auto_merge_disabled`, so a +human can deliberately leave the PR manual-only for break-glass handling. +Reviewer routing remains available. The protected-main push that deploys the +workflow automatically migrates every open, ready non-App request and repairs +unsafe App metadata; it disables workflow-skipping requests for correction. A +manual `workflow_dispatch` from `main` is the idempotent retry path. +Reconciliation never enables an originally null request. The reviewer router +is orchestration, not a quality context, and +must not be added to the ruleset. + +GitHub may omit `pull_request_target` for security-sensitive head branch names, +including names that look like commit SHAs. Those PRs cannot use Router App +takeover or the closed-event repair: rename the branch for the supported path, +or use the designated break-glass identity with a safe final message so main +push CI remains the exact-SHA producer. ## Running focused gates locally @@ -213,7 +257,72 @@ the same dedicated cache profile path because GitHub includes that path in the cache version; the runtime-facing candidate and baseline filenames remain separate. Near-miss reuse is forbidden — the caches carry no prefix restore keys, because a neighbouring commit's profile would compare the candidate -against the wrong baseline. Supporting and (when +against the wrong baseline. CI concurrency is keyed by PR number plus exact +event base/head SHA. Duplicate runs for that exact revision may cancel each +other, but a later revision cannot kill an earlier cold-cache producer. Main +runs use the pushed SHA, so a newer main push cannot cancel a predecessor's +producer. + +Every supported main advancement path has an exact-SHA producer. The required +`Test` context rejects GitHub workflow-skip directives in PR and auto-merge +metadata, reruns when that metadata is enabled, disabled, or edited, and +verifies the live App/writer-ruleset identity contract. Reviewer Router +additionally binds auto-merge to the exact head OID and writes a fixed safe +merge headline/body. The sole break-glass publisher must retain a safe final +message; the release-controlled Formula-only path +is the sole supported use of `[skip ci]`. A full source push +saves the assembled profile after the aggregate gate passes. A trusted +documentation or release-seal push independently verifies that the complete +`before...after` diff contains only the reviewed metadata allowlist, restores +only the exact `before` cache, recomputes the full profile if the chain is +cold, and makes that helper a dependency of the required `Coverage` context. +Release-generated Formula commits intentionally retain `[skip ci]`; after +their nine synthetic contexts are sealed, an independent release-governance +job creates an acknowledgement and emits a `coverage-baseline-promote` +repository dispatch. The default-branch promotion +workflow revalidates the exact single-parent Formula identity, successful +parent and target contexts, and main containment before it promotes the exact +parent cache or performs the same full fallback. Every target-main producer +follows its save with a lookup-only restore and requires +`cache-hit=true` for the exact key; this turns the cache action's otherwise +warning-only upload failure or prefix match into a hard failure. Formula +promotion additionally updates one release-created `Coverage Baseline Cache` +check. A separate confirmation job waits for that exact check-run ID while npm +and mirrors remain dependent only on the immutable publication job; cache +failure therefore makes the final delivery gate red without creating a +partially published release. Once Formula sealing exposes its SHA, a later +publication verification failure cannot suppress that confirmation job. + +A separate base-owned `pull_request_target: closed` safety net covers the final +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 +`.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 +binds the exact CI workflow ID/path, run ID/attempt, conclusion, upstream +repository, `main` branch, and head SHA. That trigger is also cache-read-only, +so either trusted dispatcher uses `repository_dispatch`; its producer +revalidates the merged-PR or failed-CI identity, checks out the contained SHA, +and produces/verifies the exact full cache. +An hourly schedule and a main-only manual dispatch repair the event-time main +SHA after a direct break-glass push or cache eviction. The dispatch exception +is intentional: unlike an ordinary event created by `GITHUB_TOKEN`, GitHub +allows `repository_dispatch` to start another workflow. A legacy built-in-token +merge can suppress the closed event too, which is why the required `Test` +identity gate and dedicated Reviewer Router App are still mandatory. + +A cold miss can still occur during a producer race or after cache eviction, +but it remains fail-safe: the PR recomputes the authoritative baseline with a +30-minute job budget and saves a PR-scoped copy for same-PR reruns. It is no +longer possible for a supported main-advance path to omit its producer +silently. That PR-scoped fallback save remains a best-effort acceleration and +does not replace the normal push, metadata, Formula, and merged-PR repair +producers. Supporting and (when platform-selected) native profiles are generated before the aggregate `Coverage` context evaluates them. The aggregate and native gates require 100% coverage for changed executable Go @@ -265,4 +374,11 @@ unproducible required context. The branch ruleset also requires one approval after the latest push. Enable repository auto-merge and automatic head-branch deletion; keep the base-owned -reviewer router outside the required-context list. +reviewer router outside the required-context list. Install its dedicated +GitHub App only on this repository with `Contents: read and write` and `Pull +requests: read and write`; do not grant Actions, Workflows, or Administration. +Give it pull-request-only bypass on `main-merge-writers` and no bypass on any +other ruleset. Store the App client ID and lowercase slug in repository +variables `REVIEWER_ROUTER_APP_CLIENT_ID` and `REVIEWER_ROUTER_APP_SLUG`, and +its private key in repository secret `REVIEWER_ROUTER_APP_PRIVATE_KEY`. Do not +reuse release, Homebrew, or personal tokens for this boundary. diff --git a/test/scripts/coverage_workflow_contract_test.go b/test/scripts/coverage_workflow_contract_test.go index 5ffb1298..047569ea 100644 --- a/test/scripts/coverage_workflow_contract_test.go +++ b/test/scripts/coverage_workflow_contract_test.go @@ -135,15 +135,32 @@ func TestCoverageWorkflowShardsAndBaselineCache(t *testing.T) { t.Fatalf("ReadFile(ci.yml) error = %v", err) } admission := string(data) + for _, want := range []string{ + "group: ci-${{ github.workflow }}-${{ github.event_name == 'pull_request' && format('pr-{0}-{1}-{2}', github.event.pull_request.number, github.event.pull_request.base.sha, github.event.pull_request.head.sha) || format('push-{0}', github.sha) }}", + "cancel-in-progress: true", + } { + if !strings.Contains(admission, want) { + t.Errorf("CI workflow missing exact-revision producer contract %q", want) + } + } + for _, forbidden := range []string{ + "github.event.pull_request.number || github.ref", + "github.event.pull_request.number || github.sha", + } { + if strings.Contains(admission, forbidden) { + t.Errorf("CI producers must not share the coarse concurrency identity %q", forbidden) + } + } currentStart := strings.Index(admission, "\n coverage-current:\n") fullStart := strings.Index(admission, "\n coverage-current-full:\n") supportingStart := strings.Index(admission, "\n coverage-supporting:\n") baselineStart := strings.Index(admission, "\n coverage-baseline:\n") + metadataStart := strings.Index(admission, "\n coverage-main-metadata:\n") gateStart := strings.Index(admission, "\n coverage:\n") policyStart := strings.Index(admission, "\n policy:\n") if currentStart < 0 || fullStart <= currentStart || supportingStart <= fullStart || - baselineStart <= supportingStart || gateStart <= baselineStart || policyStart <= gateStart { + baselineStart <= supportingStart || metadataStart <= baselineStart || gateStart <= metadataStart || policyStart <= gateStart { t.Fatal("CI workflow missing ordered coverage job boundaries") } @@ -175,7 +192,10 @@ func TestCoverageWorkflowShardsAndBaselineCache(t *testing.T) { } } - baselineJob := admission[baselineStart:gateStart] + baselineJob := admission[baselineStart:metadataStart] + if !strings.Contains(baselineJob, "timeout-minutes: 30") { + t.Error("coverage-baseline must retain enough headroom for an authoritative cold-cache fallback") + } cachePath := "coverage-cache.txt" baselineKey := "dws-coverage-full-v2-${{ env.COVERAGE_BASE_REF }}-go${{ steps.setup-go.outputs.go-version }}" for _, want := range []string{ @@ -201,8 +221,50 @@ func TestCoverageWorkflowShardsAndBaselineCache(t *testing.T) { t.Error("coverage baseline cache must stay exact-key; prefix restore-keys can resurrect a wrong-commit baseline") } + metadataJob := admission[metadataStart:gateStart] + for _, want := range []string{ + "github.event_name == 'push'", + "needs.lint.outputs.changelog_only == 'true' || needs.lint.outputs.docs_only == 'true'", + "timeout-minutes: 30", + "fetch-depth: 0", + "PUSH_BEFORE_SHA: ${{ github.event.before }}", + "PUSH_AFTER_SHA: ${{ github.event.after }}", + "git merge-base --is-ancestor \"$PUSH_BEFORE_SHA\" \"$PUSH_AFTER_SHA\"", + "git diff --name-only --no-renames -z", + `^\.changes/[a-z0-9][a-z0-9._-]*\.md$`, + `^\.changes/released/[0-9]+\.[0-9]+\.[0-9]+(-beta\.[1-9][0-9]*)?/[a-z0-9][a-z0-9._-]*\.md$`, + "Refusing coverage-cache promotion for unreviewed change-fragment path", + "Refusing coverage-cache promotion for executable path", + "COVERAGE_SOURCE_REF=$PUSH_BEFORE_SHA", + "id: metadata-current-cache", + "id: metadata-source-cache", + "key: dws-coverage-full-v2-${{ env.COVERAGE_SOURCE_REF }}-go${{ steps.setup-go-metadata.outputs.go-version }}", + "go test -count=1 -p 1", + "./ ./cmd/... ./internal/... ./skills/...", + "key: dws-coverage-full-v2-${{ github.sha }}-go${{ steps.setup-go-metadata.outputs.go-version }}", + "id: metadata-target-cache-verification", + "lookup-only: true", + "fail-on-cache-miss: true", + "EXACT_CACHE_HIT: ${{ steps.metadata-target-cache-verification.outputs.cache-hit }}", + `run: test "$EXACT_CACHE_HIT" = true`, + } { + if !strings.Contains(metadataJob, want) { + t.Errorf("metadata-only main cache producer missing contract %q", want) + } + } + if strings.Contains(metadataJob, "restore-keys") { + t.Error("metadata-only main cache promotion must use exact source and target keys") + } + if strings.Count(metadataJob, "path: "+cachePath) != 4 { + t.Error("metadata-only producer must restore target/source, save, and verify through the shared cache version path") + } + gateJob := admission[gateStart:policyStart] for _, want := range []string{ + "- coverage-main-metadata", + "MAIN_METADATA_RESULT: ${{ needs.coverage-main-metadata.result }}", + "main_metadata_expected=success", + `"main metadata cache:$MAIN_METADATA_RESULT:$main_metadata_expected"`, "pattern: coverage-current-*", "merge-multiple: true", "for shard in app cli generators helpers remaining; do", @@ -212,6 +274,12 @@ func TestCoverageWorkflowShardsAndBaselineCache(t *testing.T) { "cp coverage.txt coverage-cache.txt", "path: " + cachePath, "key: dws-coverage-full-v2-${{ github.sha }}-go${{ steps.setup-go.outputs.go-version }}", + "name: Verify push coverage cache exists", + "id: push-cache-verification", + "lookup-only: true", + "fail-on-cache-miss: true", + "EXACT_CACHE_HIT: ${{ steps.push-cache-verification.outputs.cache-hit }}", + `run: test "$EXACT_CACHE_HIT" = true`, `"current shards:$CURRENT_FULL_RESULT:$current_full_expected"`, } { if !strings.Contains(gateJob, want) { @@ -219,7 +287,437 @@ func TestCoverageWorkflowShardsAndBaselineCache(t *testing.T) { } } - if strings.Count(gateJob, "path: "+cachePath) != 1 { - t.Error("green main push must save the candidate profile through the same cache path/version as baseline restore") + if strings.Count(gateJob, "path: "+cachePath) != 2 { + t.Error("green main push must save and verify the candidate profile through the same cache path/version as baseline restore") + } +} + +func TestFormulaCoverageBaselinePromotionContract(t *testing.T) { + root, err := filepath.Abs(filepath.Join("..", "..")) + if err != nil { + t.Fatalf("Abs(repo root) error = %v", err) + } + promotionData, err := os.ReadFile(filepath.Join(root, ".github", "workflows", "coverage-baseline-promotion.yml")) + if err != nil { + t.Fatalf("ReadFile(coverage-baseline-promotion.yml) error = %v", err) + } + promotion := string(promotionData) + for _, want := range []string{ + "repository_dispatch:", + "types: [coverage-baseline-promote]", + "checks: write", + "contents: read", + "group: coverage-baseline-promotion-${{ github.event.client_payload.target_sha }}", + "cancel-in-progress: false", + "queue: max", + "github.repository == 'DingTalk-Real-AI/dingtalk-workspace-cli'", + "timeout-minutes: 30", + "context.payload.client_payload?.target_sha", + "context.payload.client_payload?.source_run_id", + "context.payload.client_payload?.check_run_id", + "targetCommit.parents.length !== 1", + "targetCommit.author?.login !== 'github-actions[bot]'", + "targetCommit.committer?.login !== 'github-actions[bot]'", + "files.length !== 1", + "Formula/dingtalk-workspace-cli.rb", + "Formula/dingtalk-workspace-cli-beta.rb", + "[skip ci", + "await requireSuccessfulAdmission(parentSha, 'Formula parent')", + "await requireSuccessfulAdmission(targetSha, 'Formula target')", + "for (let attempt = 1; attempt <= 6; attempt += 1)", + "setTimeout(resolve, 5000)", + "run.app?.slug !== 'github-actions'", + "run.conclusion !== 'success'", + "basehead: `${targetSha}...${branch.commit.sha}`", + "['ahead', 'identical'].includes(containment.status)", + "promotionCheck.name !== 'Coverage Baseline Cache'", + "promotionCheck.external_id !== promotionExternalId", + "promotionCheck.status !== 'queued'", + "core.setOutput('check_run_id', String(checkRunId))", + "name: Mark Formula cache promotion in progress", + "status: 'in_progress'", + "persist-credentials: false", + "ref: ${{ steps.validate-target.outputs.target_sha }}", + "test \"$(git rev-parse HEAD^)\" = \"$PARENT_SHA\"", + "id: target-cache", + "id: parent-cache", + "key: dws-coverage-full-v2-${{ steps.validate-target.outputs.parent_sha }}-go${{ steps.setup-go.outputs.go-version }}", + "go test -count=1 -p 1", + "./ ./cmd/... ./internal/... ./skills/...", + "key: dws-coverage-full-v2-${{ steps.validate-target.outputs.target_sha }}-go${{ steps.setup-go.outputs.go-version }}", + "lookup-only: true", + "fail-on-cache-miss: true", + "id: formula-target-cache-verification", + "EXACT_CACHE_HIT: ${{ steps.formula-target-cache-verification.outputs.cache-hit }}", + `run: test "$EXACT_CACHE_HIT" = true`, + "name: Complete Formula cache promotion acknowledgement", + "PROMOTION_JOB_STATUS: ${{ job.status }}", + "conclusion: succeeded ? 'success' : 'failure'", + } { + if !strings.Contains(promotion, want) { + t.Errorf("Formula baseline promotion missing contract %q", want) + } + } + for _, want := range []string{ + "actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683", + "actions/setup-go@40f1582b2485089dde7abd97c1529aa768e1baff", + "actions/cache/restore@0057852bfaa89a56745cba8c7296529d2fc39830", + "actions/cache/save@0057852bfaa89a56745cba8c7296529d2fc39830", + } { + if !strings.Contains(promotion, want) { + t.Errorf("Formula baseline promotion must pin trusted action %q", want) + } + } + for _, forbidden := range []string{ + "pull_request:", + "pull_request_target:", + "push:", + "workflow_dispatch:", + "restore-keys", + "HOMEBREW_PR_TOKEN", + "RELEASE_GOVERNANCE_TOKEN", + "contents: write", + } { + if strings.Contains(promotion, forbidden) { + t.Errorf("Formula baseline promotion must not contain %q", forbidden) + } + } + validateIndex := strings.Index(promotion, "name: Validate Formula-only main target") + checkoutIndex := strings.Index(promotion, "name: Check out validated Formula-only target") + if validateIndex < 0 || checkoutIndex <= validateIndex { + t.Error("Formula target must be fully validated before it is checked out or executed") + } + ackReadIndex := strings.Index(promotion, "const {data: promotionCheck} = await github.rest.checks.get") + ackOutputIndex := strings.Index(promotion, "core.setOutput('check_run_id', String(checkRunId))") + targetReadIndex := strings.Index(promotion, "const {data: targetCommit} = await github.rest.repos.getCommit") + if ackReadIndex < 0 || ackOutputIndex <= ackReadIndex || targetReadIndex <= ackOutputIndex { + t.Error("Formula acknowledgement must be identity-checked and bound to the finalizer before target validation") + } + if strings.Count(promotion, "path: coverage-cache.txt") != 4 { + t.Error("Formula promotion must restore target/source, save, and verify through the shared cache version path") + } + + releaseData, err := os.ReadFile(filepath.Join(root, ".github", "workflows", "release.yml")) + if err != nil { + t.Fatalf("ReadFile(release.yml) error = %v", err) + } + release := string(releaseData) + publishStart := strings.Index(release, " publish-release:\n") + sealStart := strings.Index(release, " - name: Seal Formula-only Code Admission contexts\n") + if publishStart < 0 || sealStart <= publishStart { + t.Fatal("release workflow missing Formula-only seal start") + } + if !strings.Contains(release[publishStart:sealStart], "timeout-minutes: 30") { + t.Error("publish-release must not be extended by cache acknowledgement polling") + } + sealEnd := strings.Index(release[sealStart:], "\n - name: Reverify exact immutable npm package\n") + if sealEnd < 0 { + t.Fatal("release workflow missing Formula-only seal boundaries") + } + seal := release[sealStart : sealStart+sealEnd] + for _, want := range []string{ + `core.setOutput("coverage_baseline_required", "true")`, + `core.setOutput("coverage_baseline_commit", commit)`, + } { + if !strings.Contains(seal, want) { + t.Errorf("Formula seal missing cache-promotion dispatch contract %q", want) + } + } + for _, forbidden := range []string{ + "for (let attempt = 1; attempt <= 180; attempt += 1)", + "github.rest.checks.get", + "promotionComplete", + "Coverage Baseline Cache", + "github.rest.repos.createDispatchEvent", + } { + if strings.Contains(seal, forbidden) { + t.Errorf("irreversible publish-release must not wait on cache through %q", forbidden) + } + } + confirmationStart := strings.Index(release, " coverage-baseline-confirmation:\n") + deliveryGateStart := strings.Index(release, " release-delivery-gate:\n") + if confirmationStart < 0 || deliveryGateStart <= confirmationStart { + t.Fatal("release workflow missing independent Formula baseline confirmation job") + } + confirmation := release[confirmationStart:deliveryGateStart] + for _, want := range []string{ + "if (!['true', 'false'].includes(rawRequired))", + "github.rest.checks.create", + "name: 'Coverage Baseline Cache'", + "status: 'queued'", + "external_id: expectedExternalId", + "github.rest.repos.createDispatchEvent", + "event_type: 'coverage-baseline-promote'", + "check_run_id: String(promotionCheck.id)", + "conclusion: 'failure'", + "for (let attempt = 1; attempt <= 180; attempt += 1)", + "github.rest.checks.get", + "currentCheck.name !== 'Coverage Baseline Cache'", + "currentCheck.conclusion !== 'success'", + "setTimeout(resolve, 10000)", + "Formula baseline promotion timed out", + } { + if !strings.Contains(confirmation, want) { + t.Errorf("independent Formula baseline confirmation missing %q", want) + } + } +} + +func TestCoverageBaselineRepairContract(t *testing.T) { + root, err := filepath.Abs(filepath.Join("..", "..")) + if err != nil { + t.Fatalf("Abs(repo root) error = %v", err) + } + data, err := os.ReadFile(filepath.Join(root, ".github", "workflows", "coverage-baseline-repair.yml")) + if err != nil { + t.Fatalf("ReadFile(coverage-baseline-repair.yml) error = %v", err) + } + workflow := string(data) + + for _, want := range []string{ + "pull_request_target:", + "branches: [main]", + "types: [closed]", + "workflow_run:", + "workflows: [CI]", + "types: [completed]", + "repository_dispatch:", + "types: [coverage-baseline-repair]", + "schedule:", + `cron: "23 * * * *"`, + "workflow_dispatch:", + "group: coverage-baseline-repair-${{ github.event_name == 'pull_request_target' && github.event.pull_request.merge_commit_sha || github.event_name == 'workflow_run' && github.event.workflow_run.head_sha || github.event_name == 'repository_dispatch' && github.event.client_payload.merge_commit_sha || github.sha }}", + "cancel-in-progress: false", + "queue: max", + } { + if !strings.Contains(workflow, want) { + t.Errorf("coverage baseline repair missing workflow contract %q", want) + } + } + + dispatchStart := strings.Index(workflow, "\n dispatch-merged-pr:\n") + failedDispatchStart := strings.Index(workflow, "\n dispatch-failed-ci:\n") + repairStart := strings.Index(workflow, "\n repair:\n") + if dispatchStart < 0 || failedDispatchStart <= dispatchStart || repairStart <= failedDispatchStart { + t.Fatal("coverage baseline repair is missing ordered merged-PR, failed-CI, and producer jobs") + } + dispatcher := workflow[dispatchStart:failedDispatchStart] + for _, want := range []string{ + "github.event_name == 'pull_request_target'", + "github.event.pull_request.merged == true", + "github.repository == 'DingTalk-Real-AI/dingtalk-workspace-cli'", + "actions: read", + "contents: write", + "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", + "for (let attempt = 1; attempt <= 6; attempt += 1)", + "setTimeout(resolve, 5000)", + "basehead: `${targetSha}...${branch.commit.sha}`", + "['ahead', 'identical'].includes(comparison.status)", + "for (let attempt = 1; attempt <= 12; attempt += 1)", + "github.rest.actions.getWorkflow", + "workflow_id: '.github/workflows/ci.yml'", + "ciWorkflow.name !== 'CI'", + "ciWorkflow.path !== '.github/workflows/ci.yml'", + "ciWorkflow.state !== 'active'", + "github.rest.actions.listWorkflowRunsForRepo", + "branch: 'main'", + "event: 'push'", + "run.name === 'CI'", + "run.workflow_id === ciWorkflow.id", + "run.path === ciWorkflow.path", + "run.event === 'push'", + "run.head_sha === mergeCommitSha", + "run.head_branch === 'main'", + "['queued', 'in_progress', 'completed'].includes(run.status)", + "setTimeout(resolve, 5000)", + "repair dispatch is unnecessary", + "github.rest.repos.createDispatchEvent", + "event_type: 'coverage-baseline-repair'", + "source: 'merged_pr'", + "pull_number: String(pullNumber)", + "head_sha: headSha", + "base_sha: baseSha", + "merge_commit_sha: mergeCommitSha", + } { + if !strings.Contains(dispatcher, want) { + t.Errorf("merged-PR repair dispatcher missing %q", want) + } + } + ciWorkflowLookup := strings.Index(dispatcher, "github.rest.actions.getWorkflow") + pushRunLookup := strings.Index(dispatcher, "github.rest.actions.listWorkflowRunsForRepo") + repairDispatch := strings.Index(dispatcher, "github.rest.repos.createDispatchEvent") + 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") + } + for _, forbidden := range []string{ + "actions/checkout@", + "actions/cache/", + "actions/setup-go@", + "go test ", + "github.event.pull_request.head.ref", + "secrets.", + } { + if strings.Contains(dispatcher, forbidden) { + t.Errorf("privileged pull_request_target dispatcher must not contain %q", forbidden) + } + } + + failedDispatcher := workflow[failedDispatchStart:repairStart] + for _, want := range []string{ + "github.event_name == 'workflow_run'", + "github.repository == 'DingTalk-Real-AI/dingtalk-workspace-cli'", + "github.event.workflow_run.name == 'CI'", + "github.event.workflow_run.event == 'push'", + "github.event.workflow_run.head_branch == 'main'", + "github.event.workflow_run.status == 'completed'", + "github.event.workflow_run.conclusion != 'success'", + "actions: read", + "contents: write", + "actions/github-script@f28e40c7f34bde8b3046d885e986cb6290c5673b", + "context.payload.repository?.default_branch !== 'main'", + "eventRun?.name !== 'CI'", + "eventRun?.event !== 'push'", + "eventRun?.head_branch !== 'main'", + "eventRun?.status !== 'completed'", + "conclusion === 'success'", + "github.rest.actions.getWorkflow", + "workflow_id: '.github/workflows/ci.yml'", + "github.rest.actions.getWorkflowRun", + "eventRun.workflow_id !== ciWorkflow.id", + "currentRun.workflow_id !== ciWorkflow.id", + "currentRun.name !== 'CI'", + "currentRun.event !== 'push'", + "currentRun.head_branch !== 'main'", + "currentRun.head_sha !== headSha", + "currentRun.run_attempt !== runAttempt", + "currentRun.status !== 'completed'", + "currentRun.conclusion !== conclusion", + "currentRun.repository?.full_name !== upstream", + "currentRun.head_repository?.full_name !== upstream", + "github.rest.repos.createDispatchEvent", + "event_type: 'coverage-baseline-repair'", + "source: 'failed_ci'", + "workflow_run_id: String(runID)", + "workflow_run_attempt: String(runAttempt)", + "workflow_conclusion: conclusion", + "merge_commit_sha: headSha", + } { + if !strings.Contains(failedDispatcher, want) { + t.Errorf("failed-CI repair dispatcher missing %q", want) + } + } + failedRunRead := strings.Index(failedDispatcher, "github.rest.actions.getWorkflowRun") + failedRepairDispatch := strings.Index(failedDispatcher, "github.rest.repos.createDispatchEvent") + if failedRunRead < 0 || failedRepairDispatch <= failedRunRead { + t.Error("workflow_run dispatcher must API-bind the exact failed CI run before repository dispatch") + } + for _, forbidden := range []string{ + "actions/checkout@", + "actions/cache/", + "actions/setup-go@", + "go test ", + "secrets.", + } { + if strings.Contains(failedDispatcher, forbidden) { + t.Errorf("privileged workflow_run dispatcher must not contain %q", forbidden) + } + } + + producer := workflow[repairStart:] + for _, want := range []string{ + "github.event_name == 'repository_dispatch' || github.event_name == 'schedule' || github.event_name == 'workflow_dispatch'", + "timeout-minutes: 35", + "actions: read", + "contents: read", + "pull-requests: read", + "id: resolve-target", + "context.eventName === 'repository_dispatch'", + "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", + "payload.source === 'failed_ci'", + "payload.workflow_run_id", + "payload.workflow_run_attempt", + "payload.workflow_conclusion", + "github.rest.actions.getWorkflow", + "workflow_id: '.github/workflows/ci.yml'", + "github.rest.actions.getWorkflowRun", + "ciWorkflow.name !== 'CI'", + "ciWorkflow.path !== '.github/workflows/ci.yml'", + "currentRun.id !== workflowRunID", + "currentRun.workflow_id !== ciWorkflow.id", + "currentRun.name !== 'CI'", + "currentRun.event !== 'push'", + "currentRun.head_branch !== 'main'", + "currentRun.head_sha !== targetSha", + "currentRun.run_attempt !== workflowRunAttempt", + "currentRun.status !== 'completed'", + "currentRun.conclusion !== workflowConclusion", + "coverage-baseline-repair payload has an unknown source", + "context.ref !== 'refs/heads/main'", + "targetSha = context.sha", + "for (let attempt = 1; attempt <= 6; attempt += 1)", + "basehead: `${targetSha}...${branch.commit.sha}`", + "core.setOutput('target_sha', targetSha)", + "actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683", + "fetch-depth: 0", + "persist-credentials: false", + "ref: ${{ steps.resolve-target.outputs.target_sha }}", + `run: test "$(git rev-parse HEAD)" = "$TARGET_SHA"`, + "actions/setup-go@40f1582b2485089dde7abd97c1529aa768e1baff", + "id: target-cache", + "actions/cache/restore@0057852bfaa89a56745cba8c7296529d2fc39830", + "key: dws-coverage-full-v2-${{ steps.resolve-target.outputs.target_sha }}-go${{ steps.setup-go.outputs.go-version }}", + "go test -count=1 -p 1", + "./ ./cmd/... ./internal/... ./skills/...", + "actions/cache/save@0057852bfaa89a56745cba8c7296529d2fc39830", + "id: target-cache-verification", + "lookup-only: true", + "fail-on-cache-miss: true", + "EXACT_CACHE_HIT: ${{ steps.target-cache-verification.outputs.cache-hit }}", + `run: test "$EXACT_CACHE_HIT" = true`, + } { + if !strings.Contains(producer, want) { + t.Errorf("trusted coverage repair producer missing %q", want) + } + } + for _, forbidden := range []string{ + "restore-keys", + "github.event.pull_request.head.ref", + "HOMEBREW_PR_TOKEN", + "RELEASE_GOVERNANCE_TOKEN", + "REVIEWER_ROUTER_APP_PRIVATE_KEY", + "contents: write", + } { + if strings.Contains(producer, forbidden) { + t.Errorf("trusted coverage repair producer must not contain %q", forbidden) + } + } + if strings.Count(producer, "path: coverage-cache.txt") != 3 { + t.Error("coverage repair must restore, save, and independently verify one exact cache path/version") + } + 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") + } + 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 { + t.Error("repository_dispatch input must be fully bound to its protected-main source before checkout") } } diff --git a/test/scripts/package_script_test.go b/test/scripts/package_script_test.go index 73798dcd..96d3de5c 100644 --- a/test/scripts/package_script_test.go +++ b/test/scripts/package_script_test.go @@ -2082,6 +2082,7 @@ func TestReleaseWorkflowDeliveryGateFailsClosed(t *testing.T) { "- verify-darwin-signatures", "- publish-release", "- publish-channels", + "- coverage-baseline-confirmation", "- mirror-gitee-release", "- repair-npm", "- repair-channel", @@ -2091,6 +2092,7 @@ func TestReleaseWorkflowDeliveryGateFailsClosed(t *testing.T) { `RELEASE_VALIDATION_RESULT: ${{ needs.release-validation.result }}`, `RELEASE_PLAN_RESULT: ${{ needs.release-plan.result }}`, `SEAL_RELEASE_RESULT: ${{ needs.seal-release.result }}`, + `COVERAGE_BASELINE_CONFIRMATION_RESULT: ${{ needs.coverage-baseline-confirmation.result }}`, "require_publication", `require_result release-contract "$RELEASE_CONTRACT_RESULT" success`, `require_result release-validation "$RELEASE_VALIDATION_RESULT" success`, @@ -2098,6 +2100,8 @@ func TestReleaseWorkflowDeliveryGateFailsClosed(t *testing.T) { `require_result verify-darwin-signatures "$DARWIN_SIGNATURE_RESULT" success`, `require_result publish-release "$PUBLISH_RELEASE_RESULT" success`, `require_result publish-channels "$PUBLISH_CHANNELS_RESULT" success`, + `require_result coverage-baseline-confirmation "$COVERAGE_BASELINE_CONFIRMATION_RESULT" success`, + `require_result coverage-baseline-confirmation "$COVERAGE_BASELINE_CONFIRMATION_RESULT" skipped`, "workflow_dispatch:recover_release", "workflow_dispatch:create_release", "workflow_dispatch:plan_release", @@ -2438,8 +2442,12 @@ func TestReleaseWorkflowSealsOnlyVerifiedFormulaCommitContexts(t *testing.T) { } for _, required := range []string{ + "timeout-minutes: 30", + `coverage_baseline_required: ${{ steps.seal-formula.outputs.coverage_baseline_required }}`, + `coverage_baseline_commit: ${{ steps.seal-formula.outputs.coverage_baseline_commit }}`, "id: homebrew-stable", "id: homebrew-beta", + "id: seal-formula", `FORMULA_CHANGED: ${{ steps.homebrew-stable.outputs.formula_changed || steps.homebrew-beta.outputs.formula_changed }}`, `FORMULA_COMMIT: ${{ steps.homebrew-stable.outputs.published_commit || steps.homebrew-beta.outputs.published_commit }}`, } { @@ -2481,6 +2489,8 @@ func TestReleaseWorkflowSealsOnlyVerifiedFormulaCommitContexts(t *testing.T) { `head_sha: commit`, `status: "completed"`, `conclusion: "success"`, + `core.setOutput("coverage_baseline_required", "true")`, + `core.setOutput("coverage_baseline_commit", commit)`, } { if !strings.Contains(seal, required) { t.Errorf("Formula-only Code Admission sealing is missing %q", required) @@ -2492,12 +2502,17 @@ func TestReleaseWorkflowSealsOnlyVerifiedFormulaCommitContexts(t *testing.T) { } } if strings.Count(seal, "github.rest.checks.create") != 1 { - t.Error("Formula-only checks must be created only by the single verified context loop") + t.Error("Formula sealing must write only the reviewed Code Admission contexts") } for _, forbidden := range []string{ "head_sha: context.sha", "head_sha: parent", "head_sha: branch.data.commit.sha", + "github.rest.checks.get", + "promotionComplete", + "setTimeout(resolve, 10000)", + "Coverage Baseline Cache", + "github.rest.repos.createDispatchEvent", } { if strings.Contains(seal, forbidden) { t.Errorf("Formula-only Code Admission must not mark an unverified head green: found %q", forbidden) @@ -2505,6 +2520,9 @@ func TestReleaseWorkflowSealsOnlyVerifiedFormulaCommitContexts(t *testing.T) { } createCheck := strings.Index(seal, "github.rest.checks.create") + if createCheck == -1 { + t.Error("Formula sealing must create the verified Code Admission contexts") + } for name, marker := range map[string]string{ "single-parent Formula-only identity": "const exactFormulaCommit", "successful parent contexts": "invalidParentContexts.length > 0", @@ -2518,6 +2536,67 @@ func TestReleaseWorkflowSealsOnlyVerifiedFormulaCommitContexts(t *testing.T) { } } +func TestReleaseWorkflowConfirmsFormulaCacheWithoutBlockingChannels(t *testing.T) { + t.Parallel() + workflow := readReleaseWorkflow(t) + publishJob := releaseWorkflowSection(t, workflow, " publish-release:\n", "\n publish-channels:\n") + channelsJob := releaseWorkflowSection(t, workflow, " publish-channels:\n", "\n mirror-gitee-release:\n") + confirmation := releaseWorkflowSection( + t, + workflow, + " coverage-baseline-confirmation:\n", + "\n release-delivery-gate:\n", + ) + + for _, required := range []string{ + `if: ${{ !cancelled() && (needs.publish-release.result == 'success' || needs.publish-release.outputs.coverage_baseline_required == 'true') }}`, + "needs: publish-release", + "timeout-minutes: 35", + "checks: write", + "contents: write", + `BASELINE_REQUIRED: ${{ needs.publish-release.outputs.coverage_baseline_required }}`, + `FORMULA_COMMIT: ${{ needs.publish-release.outputs.coverage_baseline_commit }}`, + "if (!['true', 'false'].includes(rawRequired))", + "Formula baseline requirement is invalid", + "github.rest.checks.create", + "name: 'Coverage Baseline Cache'", + "status: 'queued'", + "external_id: expectedExternalId", + "github.rest.repos.createDispatchEvent", + "event_type: 'coverage-baseline-promote'", + "source_run_id: String(context.runId)", + "check_run_id: String(promotionCheck.id)", + "github.rest.checks.update", + "conclusion: 'failure'", + "github.rest.checks.get", + "currentCheck.head_sha !== targetSha", + "currentCheck.external_id !== expectedExternalId", + "currentCheck.app?.slug !== 'github-actions'", + "for (let attempt = 1; attempt <= 180; attempt += 1)", + "currentCheck.conclusion !== 'success'", + "await new Promise(resolve => setTimeout(resolve, 10000))", + "Formula baseline promotion timed out", + } { + if !strings.Contains(confirmation, required) { + t.Errorf("Formula baseline confirmation missing %q", required) + } + } + if strings.Contains(channelsJob, "coverage-baseline-confirmation") || + strings.Contains(channelsJob, "needs.coverage-baseline-confirmation") { + t.Error("cache acknowledgement must not block npm or mirror publication") + } + for _, forbidden := range []string{ + "Coverage Baseline Cache", + "github.rest.repos.createDispatchEvent", + "github.rest.checks.get", + "promotionComplete", + } { + if strings.Contains(publishJob, forbidden) { + t.Errorf("irreversible publication job must not own cache lifecycle marker %q", forbidden) + } + } +} + func TestReleaseWorkflowWaitsForNPMDistTagPropagation(t *testing.T) { workflow := readReleaseWorkflow(t) script := releaseWorkflowRunScript( diff --git a/test/scripts/reviewer_router_workflow_test.go b/test/scripts/reviewer_router_workflow_test.go index 61fe102a..4c9e5419 100644 --- a/test/scripts/reviewer_router_workflow_test.go +++ b/test/scripts/reviewer_router_workflow_test.go @@ -16,6 +16,7 @@ import ( type reviewerRouterWorkflow struct { On map[string]reviewerRouterTrigger `yaml:"on"` Permissions map[string]string `yaml:"permissions"` + Concurrency map[string]string `yaml:"concurrency"` Jobs map[string]reviewerRouterJob `yaml:"jobs"` } @@ -25,12 +26,16 @@ type reviewerRouterTrigger struct { } type reviewerRouterJob struct { - If string `yaml:"if"` - Steps []reviewerRouterStep `yaml:"steps"` + If string `yaml:"if"` + Permissions map[string]string `yaml:"permissions"` + Steps []reviewerRouterStep `yaml:"steps"` } type reviewerRouterStep struct { + ID string `yaml:"id"` + Name string `yaml:"name"` Uses string `yaml:"uses"` + Env map[string]string `yaml:"env"` With map[string]string `yaml:"with"` } @@ -68,8 +73,8 @@ func TestReviewerRouterWorkflowContract(t *testing.T) { } } - if len(workflow.On) != 1 { - t.Fatalf("workflow triggers = %v, want pull_request_target only", workflow.On) + if len(workflow.On) != 3 { + t.Fatalf("workflow triggers = %v, want pull_request_target plus protected-main reconciliation", workflow.On) } trigger, ok := workflow.On["pull_request_target"] if !ok { @@ -78,47 +83,108 @@ func TestReviewerRouterWorkflowContract(t *testing.T) { if wantBranches := []string{"main"}; !reflect.DeepEqual(trigger.Branches, wantBranches) { t.Fatalf("pull_request_target branches = %v, want %v", trigger.Branches, wantBranches) } - wantTypes := []string{"opened", "synchronize", "reopened", "ready_for_review"} + wantTypes := []string{"opened", "synchronize", "reopened", "ready_for_review", "edited", "auto_merge_enabled"} if !reflect.DeepEqual(trigger.Types, wantTypes) { t.Fatalf("pull_request_target types = %v, want %v", trigger.Types, wantTypes) } + if _, ok := workflow.On["workflow_dispatch"]; !ok { + t.Fatalf("workflow triggers = %v, want workflow_dispatch reconciliation", workflow.On) + } + push, ok := workflow.On["push"] + if !ok || !reflect.DeepEqual(push.Branches, []string{"main"}) { + t.Fatalf("push trigger = %v, want protected main only", push) + } wantPermissions := map[string]string{ - "contents": "write", + "contents": "read", "pull-requests": "write", } if !reflect.DeepEqual(workflow.Permissions, wantPermissions) { t.Fatalf("workflow permissions = %v, want exactly %v", workflow.Permissions, wantPermissions) } + wantConcurrency := map[string]string{ + "group": "reviewer-router-${{ github.event_name == 'pull_request_target' && format('pr-{0}', github.event.pull_request.number) || 'reconcile-main' }}", + "cancel-in-progress": "${{ github.event_name == 'pull_request_target' }}", + } + if !reflect.DeepEqual(workflow.Concurrency, wantConcurrency) { + t.Fatalf("workflow concurrency = %v, want serialized reconciliation and cancellable PR events %v", workflow.Concurrency, wantConcurrency) + } - if len(workflow.Jobs) != 1 { - t.Fatalf("workflow jobs = %v, want one isolated routing job", workflow.Jobs) + if len(workflow.Jobs) != 3 { + t.Fatalf("workflow jobs = %v, want event routing plus controlled reconciliation", workflow.Jobs) } job, ok := workflow.Jobs["route"] if !ok { t.Fatalf("workflow jobs = %v, want route", workflow.Jobs) } - if job.If != "github.event.pull_request.draft == false" { + if job.If != "github.event_name == 'pull_request_target' && github.event.pull_request.draft == false" { t.Fatalf("route.if = %q, want non-draft guard", job.If) } const checkoutSHA = "actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683" const githubScriptSHA = "actions/github-script@f28e40c7f34bde8b3046d885e986cb6290c5673b" - if len(job.Steps) != 2 || job.Steps[0].Uses != checkoutSHA || job.Steps[0].With["ref"] != "${{ github.event.pull_request.base.sha }}" || job.Steps[0].With["persist-credentials"] != "false" || job.Steps[1].Uses != githubScriptSHA { - t.Fatalf("route steps = %#v, want trusted base checkout followed by pinned github-script", job.Steps) + const appTokenSHA = "actions/create-github-app-token@bcd2ba49218906704ab6c1aa796996da409d3eb1" + if len(job.Steps) != 2 || + job.Steps[0].Uses != checkoutSHA || + job.Steps[0].With["ref"] != "${{ github.event.pull_request.base.sha }}" || + job.Steps[0].With["persist-credentials"] != "false" || + job.Steps[1].Uses != githubScriptSHA { + t.Fatalf("route steps = %#v, want trusted base checkout followed by routing only", job.Steps) } - script := job.Steps[1].With["script"] + autoMergeJob, ok := workflow.Jobs["manage-auto-merge"] + if !ok { + t.Fatalf("workflow jobs = %v, want manage-auto-merge", workflow.Jobs) + } + if autoMergeJob.If != "github.event_name == 'pull_request_target' && github.event.pull_request.draft == false" { + t.Fatalf("manage-auto-merge.if = %q, want exact ready event guard", autoMergeJob.If) + } + wantAutoPermissions := map[string]string{ + "contents": "write", + "pull-requests": "write", + } + if !reflect.DeepEqual(autoMergeJob.Permissions, wantAutoPermissions) { + t.Fatalf("manage-auto-merge permissions = %v, want exactly %v", autoMergeJob.Permissions, wantAutoPermissions) + } + if len(autoMergeJob.Steps) != 3 || + autoMergeJob.Steps[0].Uses != githubScriptSHA || + autoMergeJob.Steps[1].ID != "reviewer-router-token" || + autoMergeJob.Steps[1].Uses != appTokenSHA || + autoMergeJob.Steps[2].Uses != githubScriptSHA { + t.Fatalf("manage-auto-merge steps = %#v, want unsafe-owner cleanup before dedicated App enable", autoMergeJob.Steps) + } + + appToken := autoMergeJob.Steps[1] + if len(appToken.With) != 6 { + t.Fatalf("reviewer-router token inputs = %v, want exactly the reviewed repository scope and two permissions", appToken.With) + } + for key, want := range map[string]string{ + "client-id": "${{ vars.REVIEWER_ROUTER_APP_CLIENT_ID }}", + "private-key": "${{ secrets.REVIEWER_ROUTER_APP_PRIVATE_KEY }}", + "owner": "${{ github.repository_owner }}", + "repositories": "${{ github.event.repository.name }}", + "permission-contents": "write", + "permission-pull-requests": "write", + } { + if got := appToken.With[key]; got != want { + t.Errorf("reviewer-router token input %q = %q, want %q", key, got, want) + } + } + if _, ok := appToken.With["skip-token-revoke"]; ok { + t.Error("reviewer-router App token must be revoked automatically after the job") + } + + routingScript := job.Steps[1].With["script"] for _, want := range []string{ "REVIEWER_POOL", "resolveReviewRouting", "github.rest.pulls.get", "github.rest.pulls.listFiles", "currentPull.head.sha !== eventHeadSha", + "currentPull.base.sha !== eventBaseSha", "currentPull.state !== 'open'", "currentPull.draft", "currentPull.base.ref !== 'main'", "getReadyEventPull('review request')", - "getReadyEventPull('auto-merge enable')", "context.payload.action === 'synchronize'", "context.payload.sender?.login?.toLowerCase()", "reviewer.toLowerCase() !== author", @@ -142,22 +208,391 @@ func TestReviewerRouterWorkflowContract(t *testing.T) { "github.rest.pulls.requestReviewers", "trying the next candidate", "Reviewer routing hit an unexpected error", - "enablePullRequestAutoMerge", - "mergeMethod: MERGE", "core.warning", } { - if !strings.Contains(script, want) { + if !strings.Contains(routingScript, want) { t.Errorf("reviewer router script is missing contract marker %q", want) } } + for _, forbidden := range []string{ + "enablePullRequestAutoMerge", + "disablePullRequestAutoMerge", + "mergeMethod: MERGE", + } { + if strings.Contains(routingScript, forbidden) { + t.Errorf("built-in routing token must not control auto-merge through %q", forbidden) + } + } + + cleanupStep := autoMergeJob.Steps[0] + if _, ok := cleanupStep.With["github-token"]; ok { + t.Error("unsafe-owner cleanup must use only the job-scoped built-in token") + } + cleanupScript := cleanupStep.With["script"] + for _, want := range []string{ + "currentPull.head.sha !== eventHeadSha", + "currentPull.base.sha !== eventBaseSha", + "currentPull.state !== 'open'", + "currentPull.draft", + "currentPull.base.ref !== 'main'", + "const unsafeOwner = 'github-actions[bot]'", + "const skipWorkflowPattern =", + "currentPull.title", + "currentPull.auto_merge?.commit_title", + "currentPull.auto_merge?.commit_message", + "const skipRequested =", + "enabledBy !== unsafeOwner", + "disablePullRequestAutoMerge", + "cleanedPull.auto_merge", + "Cleared workflow-skipping auto-merge metadata", + "throw new Error", + } { + if !strings.Contains(cleanupScript, want) { + t.Errorf("unsafe-owner cleanup is missing contract marker %q", want) + } + } + if strings.Contains(cleanupScript, "enablePullRequestAutoMerge") { + t.Error("built-in cleanup token must never enable auto-merge") + } + if got := strings.Count(cleanupScript, "const skipWorkflowPattern ="); got != 1 { + t.Errorf("unsafe-request cleanup skip-workflow pattern declarations = %d, want exactly 1", got) + } + + autoMergeStep := autoMergeJob.Steps[2] + if got, want := autoMergeStep.With["github-token"], "${{ steps.reviewer-router-token.outputs.token }}"; got != want { + t.Fatalf("auto-merge github-token = %q, want %q", got, want) + } + if got, want := autoMergeStep.Env["MINTED_REVIEWER_ROUTER_APP_SLUG"], "${{ steps.reviewer-router-token.outputs.app-slug }}"; got != want { + t.Fatalf("minted auto-merge App slug = %q, want %q", got, want) + } + if got, want := autoMergeStep.Env["REVIEWER_ROUTER_APP_SLUG"], "${{ vars.REVIEWER_ROUTER_APP_SLUG }}"; got != want { + t.Fatalf("configured auto-merge App slug = %q, want %q", got, want) + } + autoMergeScript := autoMergeStep.With["script"] + for _, want := range []string{ + "github.rest.pulls.get", + "currentPull.head.sha !== eventHeadSha", + "currentPull.base.sha !== eventBaseSha", + "currentPull.state !== 'open'", + "currentPull.draft", + "currentPull.base.ref !== 'main'", + "getReadyEventPull('auto-merge enable')", + "process.env.MINTED_REVIEWER_ROUTER_APP_SLUG?.trim()", + "process.env.REVIEWER_ROUTER_APP_SLUG?.trim()", + "appSlug === 'github-actions'", + "mintedAppSlug !== appSlug", + "const expectedAppOwner = `${appSlug}[bot]`", + "GET /repos/{owner}/{repo}/rules/branches/{branch}", + ".map(rule => Number(rule.ruleset_id))", + "Number.isSafeInteger(rulesetID) && rulesetID > 0", + "GET /repos/{owner}/{repo}/rulesets/{ruleset_id}", + "ruleset.enforcement !== 'active'", + "ruleset.target !== 'branch'", + "ruleset.name === 'main-merge-writers'", + "writerRuleset.source_type !== 'Repository'", + "writerRuleset.source?.toLowerCase() !== repositorySource", + "writerIncludes[0] !== 'refs/heads/main'", + "writerExcludes.length !== 0", + "writerRuleset.rules?.length !== 1", + "writerRuleset.rules[0].type !== 'update'", + "update_allows_fetch_and_merge !== false", + "current_user_can_bypass !== 'pull_requests_only'", + "ruleset.current_user_can_bypass !== 'never'", + "Reviewer Router App must never bypass main ruleset", + "const skipWorkflowPattern =", + "const safeCommitHeadline = `Merge pull request #${pullNumber}`", + "const safeCommitBody =", + "workflow-skip cleanup", + "already has the reviewed ${expectedAppOwner} auto-merge request", + "preserving it", + "non-dedicated owner cleanup", + "unsafe dedicated-App metadata cleanup", + "getReadyEventPull('post-owner cleanup')", + "enablePullRequestAutoMerge", + "mergeMethod: MERGE", + "expectedHeadOid: $expectedHeadOid", + "commitHeadline: $commitHeadline", + "commitBody: $commitBody", + "expectedHeadOid: eventHeadSha", + "const exactRevision =", + "enabledBy === expectedAppOwner", + "disablePullRequestAutoMerge", + "unsafe-merge-message cleanup", + "enabledPull.auto_merge?.commit_title !== safeCommitHeadline", + "enabledPull.auto_merge?.commit_message !== safeCommitBody", + "if (revertedPull.auto_merge)", + "enabledBy !== expectedAppOwner", + } { + if !strings.Contains(autoMergeScript, want) { + t.Errorf("dedicated auto-merge script is missing contract marker %q", want) + } + } + if got := strings.Count(autoMergeScript, "const skipWorkflowPattern ="); got != 1 { + t.Errorf("dedicated auto-merge script skip-workflow pattern declarations = %d, want exactly 1", got) + } + for _, want := range []string{ + "if (!exactRevision && enabledBy === expectedAppOwner)", + "disableAutoMerge(enabledPull, 'stale-enable cleanup')", + "existingOwner === expectedAppOwner &&", + "disableAutoMerge(currentPull, 'workflow-skip cleanup')", + "currentPull.auto_merge.commit_title === safeCommitHeadline", + "currentPull.auto_merge.commit_message === safeCommitBody", + "disableAutoMerge(enabledPull, 'unexpected owner cleanup')", + } { + if !strings.Contains(autoMergeScript, want) { + t.Errorf("dedicated App stale-state cleanup is missing %q", want) + } + } + for _, forbidden := range []string{"core.warning", "catch ("} { + if strings.Contains(autoMergeScript, forbidden) { + t.Errorf("dedicated auto-merge must fail closed instead of handling errors through %q", forbidden) + } + } + for _, forbidden := range []string{ + "rule.ruleset_source_type", + "rule.ruleset_source?.toLowerCase()", + "ruleset.source_type !== 'Repository'", + "ruleset.source?.toLowerCase() !== repositorySource", + } { + if strings.Contains(autoMergeScript, forbidden) { + t.Errorf("dedicated App must inspect every applicable main ruleset, not prefilter through %q", forbidden) + } + } + + reconcile, ok := workflow.Jobs["reconcile"] + if !ok { + t.Fatalf("workflow jobs = %v, want reconcile", workflow.Jobs) + } + if reconcile.If != "(github.event_name == 'push' || github.event_name == 'workflow_dispatch') && github.ref == 'refs/heads/main' && github.repository == 'DingTalk-Real-AI/dingtalk-workspace-cli'" { + t.Fatalf("reconcile.if = %q, want exact protected-main manual guard", reconcile.If) + } + if len(reconcile.Steps) != 2 || + reconcile.Steps[0].ID != "reviewer-router-reconcile-token" || + reconcile.Steps[0].Uses != appTokenSHA || + reconcile.Steps[1].Uses != githubScriptSHA { + t.Fatalf("reconcile steps = %#v, want scoped App token followed by pinned migration script", reconcile.Steps) + } + if len(reconcile.Permissions) != 0 { + t.Fatalf("reconcile built-in token permissions = %v, want none", reconcile.Permissions) + } + if len(reconcile.Steps[0].With) != 6 { + t.Fatalf("reconcile token inputs = %v, want exactly the reviewed repository scope and two permissions", reconcile.Steps[0].With) + } + for key, want := range map[string]string{ + "client-id": "${{ vars.REVIEWER_ROUTER_APP_CLIENT_ID }}", + "private-key": "${{ secrets.REVIEWER_ROUTER_APP_PRIVATE_KEY }}", + "owner": "${{ github.repository_owner }}", + "repositories": "${{ github.event.repository.name }}", + "permission-contents": "write", + "permission-pull-requests": "write", + } { + if got := reconcile.Steps[0].With[key]; got != want { + t.Errorf("reconcile token input %q = %q, want %q", key, got, want) + } + } + if _, ok := reconcile.Steps[0].With["skip-token-revoke"]; ok { + t.Error("reconcile App token must be revoked automatically after the job") + } + if got, want := reconcile.Steps[1].With["github-token"], "${{ steps.reviewer-router-reconcile-token.outputs.token }}"; got != want { + t.Fatalf("reconcile github-token = %q, want %q", got, want) + } + if got, want := reconcile.Steps[1].Env["MINTED_REVIEWER_ROUTER_APP_SLUG"], "${{ steps.reviewer-router-reconcile-token.outputs.app-slug }}"; got != want { + t.Fatalf("minted reconcile App slug = %q, want %q", got, want) + } + if got, want := reconcile.Steps[1].Env["REVIEWER_ROUTER_APP_SLUG"], "${{ vars.REVIEWER_ROUTER_APP_SLUG }}"; got != want { + t.Fatalf("configured reconcile App slug = %q, want %q", got, want) + } + reconcileScript := reconcile.Steps[1].With["script"] + for _, want := range []string{ + "github.paginate(github.rest.pulls.list", + "state: 'open'", + "base: 'main'", + "candidate.draft", + "const candidateOwner =", + "const candidateSkipRequested =", + "const candidateHasSafeAppRequest =", + "candidateOwner === expectedAppOwner", + "!candidate.auto_merge", + "candidateHasSafeAppRequest", + "currentPull.head.sha !== expectedHeadSha", + "currentPull.base.sha !== expectedBaseSha", + "currentPull.state !== 'open'", + "currentPull.draft", + "currentPull.base.ref !== 'main'", + "const currentHasSafeAppRequest =", + "currentOwner === expectedAppOwner", + "currentHasSafeAppRequest", + "async function disableAutoMerge(pullNumber, pullRequestId, phase)", + "disablePullRequestAutoMerge", + "if (revertedPull.auto_merge)", + "return revertedPull", + "replace non-dedicated auto-merge owner", + "repair dedicated-App merge metadata", + "enablePullRequestAutoMerge", + "mergeMethod: MERGE", + "const expectedAppOwner = `${appSlug}[bot]`", + "GET /repos/{owner}/{repo}/rules/branches/{branch}", + ".map(rule => Number(rule.ruleset_id))", + "Number.isSafeInteger(rulesetID) && rulesetID > 0", + "GET /repos/{owner}/{repo}/rulesets/{ruleset_id}", + "ruleset.enforcement !== 'active'", + "ruleset.target !== 'branch'", + "ruleset.name === 'main-merge-writers'", + "writerRuleset.source_type !== 'Repository'", + "writerRuleset.source?.toLowerCase() !== repositorySource", + "writerIncludes[0] !== 'refs/heads/main'", + "writerExcludes.length !== 0", + "writerRuleset.rules?.length !== 1", + "writerRuleset.rules[0].type !== 'update'", + "update_allows_fetch_and_merge !== false", + "current_user_can_bypass !== 'pull_requests_only'", + "ruleset.current_user_can_bypass !== 'never'", + "Reviewer Router App must never bypass main ruleset", + "const skipWorkflowPattern =", + "const skipRequested =", + "workflow-skip directive found in merge metadata; left manual-merge only", + "const safeCommitHeadline = `Merge pull request #${candidate.number}`", + "const safeCommitBody =", + "expectedHeadOid: $expectedHeadOid", + "commitHeadline: $commitHeadline", + "commitBody: $commitBody", + "expectedHeadOid: expectedHeadSha", + "enabledPull.auto_merge?.commit_title !== safeCommitHeadline", + "enabledPull.auto_merge?.commit_message !== safeCommitBody", + "const failures = []", + "catch (error)", + "failures.push(failure)", + "core.setFailed", + "revisionChanged && enabledBy === expectedAppOwner", + "enabledBy !== expectedAppOwner", + "unexpected owner cleanup", + "unsafe merge-message cleanup", + } { + if !strings.Contains(reconcileScript, want) { + t.Errorf("reconciliation script is missing contract marker %q", want) + } + } + if disableIndex, enableIndex := strings.Index(reconcileScript, "disablePullRequestAutoMerge"), strings.Index(reconcileScript, "enablePullRequestAutoMerge"); disableIndex < 0 || enableIndex <= disableIndex { + t.Error("legacy reconciliation must disable the exact unsafe owner before App-token enable") + } + if got := strings.Count(reconcileScript, "const skipWorkflowPattern ="); got != 1 { + t.Errorf("reconciliation script skip-workflow pattern declarations = %d, want exactly 1", got) + } + if strings.Contains(reconcileScript, "const unsafeOwner") { + t.Error("reconciliation must converge every non-App owner, not only the legacy built-in owner") + } + for _, forbidden := range []string{ + "rule.ruleset_source_type", + "rule.ruleset_source?.toLowerCase()", + "ruleset.source_type !== 'Repository'", + "ruleset.source?.toLowerCase() !== repositorySource", + } { + if strings.Contains(reconcileScript, forbidden) { + t.Errorf("reconciliation App must inspect every applicable main ruleset, not prefilter through %q", forbidden) + } + } for _, forbidden := range []string{ "['APPROVED', 'CHANGES_REQUESTED', 'COMMENTED']", "PeterGuy326", - "core.setFailed", + "secrets.HOMEBREW_PR_TOKEN", + "secrets.RELEASE_GOVERNANCE_TOKEN", + "secrets.GITHUB_TOKEN", } { if strings.Contains(string(data), forbidden) { t.Errorf("reviewer router must not contain %q", forbidden) } } } + +func TestCodeAdmissionEnforcesReviewerRouterWriterBoundary(t *testing.T) { + t.Parallel() + + root, err := filepath.Abs(filepath.Join("..", "..")) + if err != nil { + t.Fatalf("Abs(repo root) error = %v", err) + } + data, err := os.ReadFile(filepath.Join(root, ".github", "workflows", "ci.yml")) + if err != nil { + t.Fatalf("ReadFile(ci.yml) error = %v", err) + } + workflow := string(data) + if !strings.Contains(workflow, "pull_request:\n types: [opened, synchronize, reopened, ready_for_review, edited, auto_merge_enabled, auto_merge_disabled]") { + t.Error("CI must rerun admission when a draft becomes ready or merge metadata changes") + } + start := strings.Index(workflow, "\n test:\n") + end := strings.Index(workflow, "\n test-darwin:\n") + if start < 0 || end <= start { + t.Fatal("CI workflow missing Test aggregate boundaries") + } + testJob := workflow[start:end] + for _, want := range []string{ + "contents: read", + "pull-requests: read", + "name: Verify auto-merge identity", + "if: github.event_name == 'pull_request' && github.event.pull_request.draft == false", + "REVIEWER_ROUTER_APP_SLUG: ${{ vars.REVIEWER_ROUTER_APP_SLUG }}", + "process.env.REVIEWER_ROUTER_APP_SLUG?.trim()", + "const expectedAppOwner = `${appSlug}[bot]`", + "github.rest.pulls.get", + "github.rest.repos.get", + "repository.merge_commit_title !== 'MERGE_MESSAGE'", + "!['PR_TITLE', 'BLANK'].includes(repository.merge_commit_message)", + "const writerRulesetName = 'main-merge-writers'", + "GET /repos/{owner}/{repo}/rules/branches/{branch}", + "rule.ruleset_source_type === 'Repository'", + "rule.ruleset_source?.toLowerCase() === repositorySource", + "GET /repos/{owner}/{repo}/rulesets/{ruleset_id}", + "ruleset.enforcement !== 'active'", + "ruleset.target !== 'branch'", + "ruleset.source_type !== 'Repository'", + "ruleset.source?.toLowerCase() !== repositorySource", + "writerIncludes[0] !== 'refs/heads/main'", + "writerExcludes.length !== 0", + "writerRuleset.rules?.length !== 1", + "writerRuleset.rules[0].type !== 'update'", + "update_allows_fetch_and_merge !== false", + "writerRuleset.current_user_can_bypass !== 'never'", + "deny this built-in Actions identity any bypass", + "const skipWorkflowPattern =", + "currentPull.title", + "currentPull.auto_merge?.commit_title", + "currentPull.auto_merge?.commit_message", + "merge metadata contains a GitHub workflow-skip directive", + "currentPull.head.sha !== eventHeadSha", + "currentPull.base.sha !== eventBaseSha", + "currentPull.state !== 'open'", + "currentPull.draft", + "currentPull.base.ref !== 'main'", + "currentPull.auto_merge", + "enabled_by?.login", + "enabledBy === expectedAppOwner", + "currentPull.auto_merge.commit_title === safeCommitHeadline", + "currentPull.auto_merge.commit_message === safeCommitBody", + "must be null or owned by ${expectedAppOwner}", + "const maxAttempts = 6", + "setTimeout(resolve, 5000)", + "core.setFailed", + } { + if !strings.Contains(testJob, want) { + t.Errorf("Test aggregate missing auto-merge identity contract %q", want) + } + } + if !strings.Contains(testJob, "Null and non-built-in merge identities emit either the protected-main") || + !strings.Contains(testJob, "main-merge-writers never lets it update main") { + t.Error("auto-merge identity comment must name the push/repair paths and built-in writer self-check") + } + for _, forbidden := range []string{ + "REVIEWER_ROUTER_APP_PRIVATE_KEY", + "HOMEBREW_PR_TOKEN", + "RELEASE_GOVERNANCE_TOKEN", + "can emit protected-main workflow events", + "pull-requests: write", + "contents: write", + "actions: write", + } { + if strings.Contains(testJob, forbidden) { + t.Errorf("read-only admission identity check must not consume %q", forbidden) + } + } +} From e3124ccea11e6267d67febd6956733c49dda1bed Mon Sep 17 00:00:00 2001 From: chichuan Date: Tue, 25 Aug 2026 11:11:21 +0800 Subject: [PATCH 2/5] fix(ci): accept strict ruleset read projection --- .github/workflows/ci.yml | 17 ++- .github/workflows/reviewer-router.yml | 34 +++++- docs/automation.md | 5 +- docs/ci-pr-gates.md | 5 +- ...reviewer_router_ruleset_projection_test.go | 104 ++++++++++++++++++ test/scripts/reviewer_router_workflow_test.go | 24 +++- 6 files changed, 175 insertions(+), 14 deletions(-) create mode 100644 test/scripts/reviewer_router_ruleset_projection_test.go diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 54a0383d..86ea48b4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1013,13 +1013,26 @@ jobs: const writerRuleset = writerRulesets[0]; const writerIncludes = writerRuleset.conditions?.ref_name?.include || []; const writerExcludes = writerRuleset.conditions?.ref_name?.exclude || []; + // GitHub's read projection omits parameters when this update-rule + // exception is disabled. Accept only that omission or exact false; + // reject a present null/non-boolean field instead of weakening the gate. + function isStrictUpdateRule(rule) { + if (rule?.type !== 'update') { + return false; + } + const updateAllowsFetchAndMerge = + rule.parameters?.update_allows_fetch_and_merge; + return ( + updateAllowsFetchAndMerge === undefined || + updateAllowsFetchAndMerge === false + ); + } if ( writerIncludes.length !== 1 || writerIncludes[0] !== 'refs/heads/main' || writerExcludes.length !== 0 || writerRuleset.rules?.length !== 1 || - writerRuleset.rules[0].type !== 'update' || - writerRuleset.rules[0].parameters?.update_allows_fetch_and_merge !== false || + !isStrictUpdateRule(writerRuleset.rules[0]) || writerRuleset.current_user_can_bypass !== 'never' ) { core.setFailed( diff --git a/.github/workflows/reviewer-router.yml b/.github/workflows/reviewer-router.yml index 7e33568a..68df6c71 100644 --- a/.github/workflows/reviewer-router.yml +++ b/.github/workflows/reviewer-router.yml @@ -446,6 +446,20 @@ jobs: const writerRuleset = writerRulesets[0]; const writerIncludes = writerRuleset.conditions?.ref_name?.include || []; const writerExcludes = writerRuleset.conditions?.ref_name?.exclude || []; + // GitHub's read projection omits parameters when this update-rule + // exception is disabled. Accept only that omission or exact false; + // reject a present null/non-boolean field instead of weakening the gate. + function isStrictUpdateRule(rule) { + if (rule?.type !== 'update') { + return false; + } + const updateAllowsFetchAndMerge = + rule.parameters?.update_allows_fetch_and_merge; + return ( + updateAllowsFetchAndMerge === undefined || + updateAllowsFetchAndMerge === false + ); + } if ( writerRuleset.source_type !== 'Repository' || writerRuleset.source?.toLowerCase() !== repositorySource || @@ -453,8 +467,7 @@ jobs: writerIncludes[0] !== 'refs/heads/main' || writerExcludes.length !== 0 || writerRuleset.rules?.length !== 1 || - writerRuleset.rules[0].type !== 'update' || - writerRuleset.rules[0].parameters?.update_allows_fetch_and_merge !== false || + !isStrictUpdateRule(writerRuleset.rules[0]) || writerRuleset.current_user_can_bypass !== 'pull_requests_only' ) { throw new Error( @@ -723,6 +736,20 @@ jobs: const writerRuleset = writerRulesets[0]; const writerIncludes = writerRuleset.conditions?.ref_name?.include || []; const writerExcludes = writerRuleset.conditions?.ref_name?.exclude || []; + // GitHub's read projection omits parameters when this update-rule + // exception is disabled. Accept only that omission or exact false; + // reject a present null/non-boolean field instead of weakening the gate. + function isStrictUpdateRule(rule) { + if (rule?.type !== 'update') { + return false; + } + const updateAllowsFetchAndMerge = + rule.parameters?.update_allows_fetch_and_merge; + return ( + updateAllowsFetchAndMerge === undefined || + updateAllowsFetchAndMerge === false + ); + } if ( writerRuleset.source_type !== 'Repository' || writerRuleset.source?.toLowerCase() !== repositorySource || @@ -730,8 +757,7 @@ jobs: writerIncludes[0] !== 'refs/heads/main' || writerExcludes.length !== 0 || writerRuleset.rules?.length !== 1 || - writerRuleset.rules[0].type !== 'update' || - writerRuleset.rules[0].parameters?.update_allows_fetch_and_merge !== false || + !isStrictUpdateRule(writerRuleset.rules[0]) || writerRuleset.current_user_can_bypass !== 'pull_requests_only' ) { throw new Error( diff --git a/docs/automation.md b/docs/automation.md index 666d2a07..e10868f8 100644 --- a/docs/automation.md +++ b/docs/automation.md @@ -82,7 +82,10 @@ it: - set repository secret `REVIEWER_ROUTER_APP_PRIVATE_KEY` to its private key; - create one active repository branch ruleset named `main-merge-writers`, targeting only `refs/heads/main`, with exactly one `Restrict updates` rule - (`update_allows_fetch_and_merge: false`); + (`update_allows_fetch_and_merge: false`). GitHub may project that strict + value through the read APIs as `{type: "update"}` with `parameters` omitted; + consumers accept only that omission or an explicit boolean `false`, and + reject every present non-false value; - give that ruleset exactly two bypass actors: the Reviewer Router App as an `Integration` in `pull_request` mode, and `PeterGuy326` (ID `47820304`) in `always` mode for Formula publication and break-glass recovery; diff --git a/docs/ci-pr-gates.md b/docs/ci-pr-gates.md index c8387c6c..3a998255 100644 --- a/docs/ci-pr-gates.md +++ b/docs/ci-pr-gates.md @@ -170,7 +170,10 @@ silently skip the merge commit's protected-main CI and baseline-cache producer. Token minting or takeover fails closed without falling back to `GITHUB_TOKEN`: the unsafe request is cleared before credentials are read, and the required `Test` context live-verifies the exact `main-merge-writers` update -rule and requires its own built-in token to report +rule. GitHub's read APIs may omit `parameters` for the strict +`update_allows_fetch_and_merge: false` value, so the gate accepts only that +omission or an explicit boolean `false` and rejects every present non-false +value. It also requires its own built-in token to report `current_user_can_bypass: never`. The minted App separately requires `pull_requests_only` on that writer rule and `never` on every other active main ruleset before it can enable or reconcile auto-merge. GitHub hides the complete diff --git a/test/scripts/reviewer_router_ruleset_projection_test.go b/test/scripts/reviewer_router_ruleset_projection_test.go new file mode 100644 index 00000000..84aad8b1 --- /dev/null +++ b/test/scripts/reviewer_router_ruleset_projection_test.go @@ -0,0 +1,104 @@ +// 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 strictUpdateRuleHelper = `function isStrictUpdateRule(rule) { + if (rule?.type !== 'update') { + return false; + } + const updateAllowsFetchAndMerge = + rule.parameters?.update_allows_fetch_and_merge; + return ( + updateAllowsFetchAndMerge === undefined || + updateAllowsFetchAndMerge === false + ); +}` + +func TestReviewerRouterAcceptsGitHubStrictUpdateReadProjection(t *testing.T) { + t.Parallel() + + node, err := exec.LookPath("node") + if err != nil { + t.Skip("node is required to verify the ruleset projection helper") + } + root, err := filepath.Abs(filepath.Join("..", "..")) + if err != nil { + t.Fatalf("resolve repository root: %v", err) + } + + var scripts []string + for _, relativePath := range []string{ + filepath.Join(".github", "workflows", "ci.yml"), + filepath.Join(".github", "workflows", "reviewer-router.yml"), + } { + path := filepath.Join(root, relativePath) + data, readErr := os.ReadFile(path) + if readErr != nil { + t.Fatalf("read %s: %v", path, readErr) + } + var workflow any + if unmarshalErr := yaml.Unmarshal(data, &workflow); unmarshalErr != nil { + t.Fatalf("parse %s: %v", path, unmarshalErr) + } + collectGitHubScripts(workflow, &scripts) + } + + checked := 0 + for _, script := range scripts { + if !strings.Contains(script, "const writerRuleset = writerRulesets[0];") { + continue + } + checked++ + if got := strings.Count(script, strictUpdateRuleHelper); got != 1 { + t.Errorf("writer-ruleset script contains strict update helper %d times, want 1", got) + } + if strings.Contains( + script, + "parameters?.update_allows_fetch_and_merge !== false", + ) { + t.Error("writer-ruleset script rejects GitHub's omitted strict-false read projection") + } + if !strings.Contains(script, "!isStrictUpdateRule(writerRuleset.rules[0])") { + t.Error("writer-ruleset script does not enforce the strict update helper") + } + } + if checked != 3 { + t.Fatalf("writer-ruleset scripts checked = %d, want App enable, reconcile, and CI self-check", checked) + } + + verification := strictUpdateRuleHelper + ` +const cases = [ + ['omitted parameters', {type: 'update'}, true], + ['null parameters', {type: 'update', parameters: null}, true], + ['empty parameters', {type: 'update', parameters: {}}, true], + ['explicit false', {type: 'update', parameters: {update_allows_fetch_and_merge: false}}, true], + ['explicit true', {type: 'update', parameters: {update_allows_fetch_and_merge: true}}, false], + ['null field', {type: 'update', parameters: {update_allows_fetch_and_merge: null}}, false], + ['string false', {type: 'update', parameters: {update_allows_fetch_and_merge: 'false'}}, false], + ['numeric false', {type: 'update', parameters: {update_allows_fetch_and_merge: 0}}, false], + ['wrong rule type', {type: 'creation'}, false], + ['null rule', null, false], +]; +for (const [name, rule, want] of cases) { + const got = isStrictUpdateRule(rule); + 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("strict update projection verification failed: %v\n%s", runErr, output) + } +} diff --git a/test/scripts/reviewer_router_workflow_test.go b/test/scripts/reviewer_router_workflow_test.go index 4c9e5419..ba972fa2 100644 --- a/test/scripts/reviewer_router_workflow_test.go +++ b/test/scripts/reviewer_router_workflow_test.go @@ -294,8 +294,12 @@ func TestReviewerRouterWorkflowContract(t *testing.T) { "writerIncludes[0] !== 'refs/heads/main'", "writerExcludes.length !== 0", "writerRuleset.rules?.length !== 1", - "writerRuleset.rules[0].type !== 'update'", - "update_allows_fetch_and_merge !== false", + "function isStrictUpdateRule(rule)", + "rule?.type !== 'update'", + "rule.parameters?.update_allows_fetch_and_merge", + "updateAllowsFetchAndMerge === undefined", + "updateAllowsFetchAndMerge === false", + "!isStrictUpdateRule(writerRuleset.rules[0])", "current_user_can_bypass !== 'pull_requests_only'", "ruleset.current_user_can_bypass !== 'never'", "Reviewer Router App must never bypass main ruleset", @@ -443,8 +447,12 @@ func TestReviewerRouterWorkflowContract(t *testing.T) { "writerIncludes[0] !== 'refs/heads/main'", "writerExcludes.length !== 0", "writerRuleset.rules?.length !== 1", - "writerRuleset.rules[0].type !== 'update'", - "update_allows_fetch_and_merge !== false", + "function isStrictUpdateRule(rule)", + "rule?.type !== 'update'", + "rule.parameters?.update_allows_fetch_and_merge", + "updateAllowsFetchAndMerge === undefined", + "updateAllowsFetchAndMerge === false", + "!isStrictUpdateRule(writerRuleset.rules[0])", "current_user_can_bypass !== 'pull_requests_only'", "ruleset.current_user_can_bypass !== 'never'", "Reviewer Router App must never bypass main ruleset", @@ -550,8 +558,12 @@ func TestCodeAdmissionEnforcesReviewerRouterWriterBoundary(t *testing.T) { "writerIncludes[0] !== 'refs/heads/main'", "writerExcludes.length !== 0", "writerRuleset.rules?.length !== 1", - "writerRuleset.rules[0].type !== 'update'", - "update_allows_fetch_and_merge !== false", + "function isStrictUpdateRule(rule)", + "rule?.type !== 'update'", + "rule.parameters?.update_allows_fetch_and_merge", + "updateAllowsFetchAndMerge === undefined", + "updateAllowsFetchAndMerge === false", + "!isStrictUpdateRule(writerRuleset.rules[0])", "writerRuleset.current_user_can_bypass !== 'never'", "deny this built-in Actions identity any bypass", "const skipWorkflowPattern =", From aeb1b2ced20f0af1e604daee31636355d72fed10 Mon Sep 17 00:00:00 2001 From: chichuan Date: Tue, 25 Aug 2026 11:39:33 +0800 Subject: [PATCH 3/5] fix(ci): cross-check strict ruleset via GraphQL --- .github/workflows/ci.yml | 77 ++++++++- .github/workflows/reviewer-router.yml | 152 ++++++++++++++++-- docs/automation.md | 6 +- docs/ci-pr-gates.md | 7 +- ...reviewer_router_ruleset_projection_test.go | 98 ++++++++++- test/scripts/reviewer_router_workflow_test.go | 54 +++++-- 6 files changed, 353 insertions(+), 41 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 86ea48b4..e8c8c124 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1013,24 +1013,56 @@ jobs: const writerRuleset = writerRulesets[0]; const writerIncludes = writerRuleset.conditions?.ref_name?.include || []; const writerExcludes = writerRuleset.conditions?.ref_name?.exclude || []; - // GitHub's read projection omits parameters when this update-rule - // exception is disabled. Accept only that omission or exact false; - // reject a present null/non-boolean field instead of weakening the gate. + // GitHub's read projection omits the entire parameters property + // when this exception is disabled. Accept only that exact omission + // or a one-field object containing exact false. function isStrictUpdateRule(rule) { if (rule?.type !== 'update') { return false; } - const updateAllowsFetchAndMerge = - rule.parameters?.update_allows_fetch_and_merge; + if (!Object.prototype.hasOwnProperty.call(rule, 'parameters')) { + return true; + } + const parameters = rule.parameters; + if ( + parameters === null || + typeof parameters !== 'object' || + Array.isArray(parameters) + ) { + return false; + } + const parameterKeys = Object.keys(parameters); return ( - updateAllowsFetchAndMerge === undefined || - updateAllowsFetchAndMerge === false + parameterKeys.length === 1 && + parameterKeys[0] === 'update_allows_fetch_and_merge' && + parameters.update_allows_fetch_and_merge === false + ); + } + function isStrictGraphQLUpdateRule(restRuleset, graphRuleset) { + const restRulesetID = Number(restRuleset?.id); + const graphRulesetID = Number(graphRuleset?.databaseId); + const graphRules = graphRuleset?.rules; + const graphRule = graphRules?.nodes?.[0]; + return ( + Number.isSafeInteger(restRulesetID) && + restRulesetID > 0 && + graphRulesetID === restRulesetID && + graphRuleset.name === restRuleset.name && + graphRuleset.enforcement === 'ACTIVE' && + graphRuleset.target === 'BRANCH' && + graphRules?.totalCount === 1 && + graphRules.nodes?.length === 1 && + graphRule?.type === 'UPDATE' && + graphRule.parameters?.__typename === 'UpdateParameters' && + graphRule.parameters.updateAllowsFetchAndMerge === false ); } if ( writerIncludes.length !== 1 || writerIncludes[0] !== 'refs/heads/main' || writerExcludes.length !== 0 || + typeof writerRuleset.node_id !== 'string' || + !writerRuleset.node_id || writerRuleset.rules?.length !== 1 || !isStrictUpdateRule(writerRuleset.rules[0]) || writerRuleset.current_user_can_bypass !== 'never' @@ -1040,6 +1072,37 @@ jobs: ); return; } + const {node: graphWriterRuleset} = await github.graphql( + `query ReviewerRouterWriterRule($rulesetID: ID!) { + node(id: $rulesetID) { + ... on RepositoryRuleset { + databaseId + name + enforcement + target + rules(first: 2) { + totalCount + nodes { + type + parameters { + __typename + ... on UpdateParameters { + updateAllowsFetchAndMerge + } + } + } + } + } + } + }`, + {rulesetID: writerRuleset.node_id}, + ); + if (!isStrictGraphQLUpdateRule(writerRuleset, graphWriterRuleset)) { + core.setFailed( + `${writerRulesetName} must expose one strict UPDATE rule with updateAllowsFetchAndMerge=false through GraphQL.`, + ); + return; + } const maxAttempts = 6; for (let attempt = 1; attempt <= maxAttempts; attempt += 1) { diff --git a/.github/workflows/reviewer-router.yml b/.github/workflows/reviewer-router.yml index 68df6c71..b4066afd 100644 --- a/.github/workflows/reviewer-router.yml +++ b/.github/workflows/reviewer-router.yml @@ -446,18 +446,48 @@ jobs: const writerRuleset = writerRulesets[0]; const writerIncludes = writerRuleset.conditions?.ref_name?.include || []; const writerExcludes = writerRuleset.conditions?.ref_name?.exclude || []; - // GitHub's read projection omits parameters when this update-rule - // exception is disabled. Accept only that omission or exact false; - // reject a present null/non-boolean field instead of weakening the gate. + // GitHub's read projection omits the entire parameters property + // when this exception is disabled. Accept only that exact omission + // or a one-field object containing exact false. function isStrictUpdateRule(rule) { if (rule?.type !== 'update') { return false; } - const updateAllowsFetchAndMerge = - rule.parameters?.update_allows_fetch_and_merge; + if (!Object.prototype.hasOwnProperty.call(rule, 'parameters')) { + return true; + } + const parameters = rule.parameters; + if ( + parameters === null || + typeof parameters !== 'object' || + Array.isArray(parameters) + ) { + return false; + } + const parameterKeys = Object.keys(parameters); return ( - updateAllowsFetchAndMerge === undefined || - updateAllowsFetchAndMerge === false + parameterKeys.length === 1 && + parameterKeys[0] === 'update_allows_fetch_and_merge' && + parameters.update_allows_fetch_and_merge === false + ); + } + function isStrictGraphQLUpdateRule(restRuleset, graphRuleset) { + const restRulesetID = Number(restRuleset?.id); + const graphRulesetID = Number(graphRuleset?.databaseId); + const graphRules = graphRuleset?.rules; + const graphRule = graphRules?.nodes?.[0]; + return ( + Number.isSafeInteger(restRulesetID) && + restRulesetID > 0 && + graphRulesetID === restRulesetID && + graphRuleset.name === restRuleset.name && + graphRuleset.enforcement === 'ACTIVE' && + graphRuleset.target === 'BRANCH' && + graphRules?.totalCount === 1 && + graphRules.nodes?.length === 1 && + graphRule?.type === 'UPDATE' && + graphRule.parameters?.__typename === 'UpdateParameters' && + graphRule.parameters.updateAllowsFetchAndMerge === false ); } if ( @@ -466,6 +496,8 @@ jobs: writerIncludes.length !== 1 || writerIncludes[0] !== 'refs/heads/main' || writerExcludes.length !== 0 || + typeof writerRuleset.node_id !== 'string' || + !writerRuleset.node_id || writerRuleset.rules?.length !== 1 || !isStrictUpdateRule(writerRuleset.rules[0]) || writerRuleset.current_user_can_bypass !== 'pull_requests_only' @@ -474,6 +506,36 @@ jobs: 'Reviewer Router App requires the exact repository-owned main writer rule with pull-request-only bypass.', ); } + const {node: graphWriterRuleset} = await github.graphql( + `query ReviewerRouterWriterRule($rulesetID: ID!) { + node(id: $rulesetID) { + ... on RepositoryRuleset { + databaseId + name + enforcement + target + rules(first: 2) { + totalCount + nodes { + type + parameters { + __typename + ... on UpdateParameters { + updateAllowsFetchAndMerge + } + } + } + } + } + } + }`, + {rulesetID: writerRuleset.node_id}, + ); + if (!isStrictGraphQLUpdateRule(writerRuleset, graphWriterRuleset)) { + throw new Error( + 'Reviewer Router App requires GraphQL to expose one strict UPDATE rule with updateAllowsFetchAndMerge=false.', + ); + } for (const ruleset of activeMainRulesets) { if (ruleset.id === writerRuleset.id) { continue; @@ -736,18 +798,48 @@ jobs: const writerRuleset = writerRulesets[0]; const writerIncludes = writerRuleset.conditions?.ref_name?.include || []; const writerExcludes = writerRuleset.conditions?.ref_name?.exclude || []; - // GitHub's read projection omits parameters when this update-rule - // exception is disabled. Accept only that omission or exact false; - // reject a present null/non-boolean field instead of weakening the gate. + // GitHub's read projection omits the entire parameters property + // when this exception is disabled. Accept only that exact omission + // or a one-field object containing exact false. function isStrictUpdateRule(rule) { if (rule?.type !== 'update') { return false; } - const updateAllowsFetchAndMerge = - rule.parameters?.update_allows_fetch_and_merge; + if (!Object.prototype.hasOwnProperty.call(rule, 'parameters')) { + return true; + } + const parameters = rule.parameters; + if ( + parameters === null || + typeof parameters !== 'object' || + Array.isArray(parameters) + ) { + return false; + } + const parameterKeys = Object.keys(parameters); return ( - updateAllowsFetchAndMerge === undefined || - updateAllowsFetchAndMerge === false + parameterKeys.length === 1 && + parameterKeys[0] === 'update_allows_fetch_and_merge' && + parameters.update_allows_fetch_and_merge === false + ); + } + function isStrictGraphQLUpdateRule(restRuleset, graphRuleset) { + const restRulesetID = Number(restRuleset?.id); + const graphRulesetID = Number(graphRuleset?.databaseId); + const graphRules = graphRuleset?.rules; + const graphRule = graphRules?.nodes?.[0]; + return ( + Number.isSafeInteger(restRulesetID) && + restRulesetID > 0 && + graphRulesetID === restRulesetID && + graphRuleset.name === restRuleset.name && + graphRuleset.enforcement === 'ACTIVE' && + graphRuleset.target === 'BRANCH' && + graphRules?.totalCount === 1 && + graphRules.nodes?.length === 1 && + graphRule?.type === 'UPDATE' && + graphRule.parameters?.__typename === 'UpdateParameters' && + graphRule.parameters.updateAllowsFetchAndMerge === false ); } if ( @@ -756,6 +848,8 @@ jobs: writerIncludes.length !== 1 || writerIncludes[0] !== 'refs/heads/main' || writerExcludes.length !== 0 || + typeof writerRuleset.node_id !== 'string' || + !writerRuleset.node_id || writerRuleset.rules?.length !== 1 || !isStrictUpdateRule(writerRuleset.rules[0]) || writerRuleset.current_user_can_bypass !== 'pull_requests_only' @@ -764,6 +858,36 @@ jobs: 'Reviewer Router App requires the exact repository-owned main writer rule with pull-request-only bypass.', ); } + const {node: graphWriterRuleset} = await github.graphql( + `query ReviewerRouterWriterRule($rulesetID: ID!) { + node(id: $rulesetID) { + ... on RepositoryRuleset { + databaseId + name + enforcement + target + rules(first: 2) { + totalCount + nodes { + type + parameters { + __typename + ... on UpdateParameters { + updateAllowsFetchAndMerge + } + } + } + } + } + } + }`, + {rulesetID: writerRuleset.node_id}, + ); + if (!isStrictGraphQLUpdateRule(writerRuleset, graphWriterRuleset)) { + throw new Error( + 'Reviewer Router App requires GraphQL to expose one strict UPDATE rule with updateAllowsFetchAndMerge=false.', + ); + } for (const ruleset of activeMainRulesets) { if (ruleset.id === writerRuleset.id) { continue; diff --git a/docs/automation.md b/docs/automation.md index e10868f8..a3636bf1 100644 --- a/docs/automation.md +++ b/docs/automation.md @@ -84,8 +84,10 @@ it: targeting only `refs/heads/main`, with exactly one `Restrict updates` rule (`update_allows_fetch_and_merge: false`). GitHub may project that strict value through the read APIs as `{type: "update"}` with `parameters` omitted; - consumers accept only that omission or an explicit boolean `false`, and - reject every present non-false value; + consumers accept only that exact omission or a one-field `parameters` object + containing explicit boolean `false`, and reject every other present shape or + value. They then bind the same ruleset node through GraphQL and require its + non-null `updateAllowsFetchAndMerge` value to be exactly `false`; - give that ruleset exactly two bypass actors: the Reviewer Router App as an `Integration` in `pull_request` mode, and `PeterGuy326` (ID `47820304`) in `always` mode for Formula publication and break-glass recovery; diff --git a/docs/ci-pr-gates.md b/docs/ci-pr-gates.md index 3a998255..6254e564 100644 --- a/docs/ci-pr-gates.md +++ b/docs/ci-pr-gates.md @@ -172,8 +172,11 @@ producer. Token minting or takeover fails closed without falling back to the required `Test` context live-verifies the exact `main-merge-writers` update rule. GitHub's read APIs may omit `parameters` for the strict `update_allows_fetch_and_merge: false` value, so the gate accepts only that -omission or an explicit boolean `false` and rejects every present non-false -value. It also requires its own built-in token to report +exact omission or a one-field `parameters` object containing explicit boolean +`false`; every other present shape or value fails closed. The gate then binds +the same ruleset node through GraphQL and requires its non-null +`updateAllowsFetchAndMerge` value to be exactly `false`. It also requires its +own built-in token to report `current_user_can_bypass: never`. The minted App separately requires `pull_requests_only` on that writer rule and `never` on every other active main ruleset before it can enable or reconcile auto-merge. GitHub hides the complete diff --git a/test/scripts/reviewer_router_ruleset_projection_test.go b/test/scripts/reviewer_router_ruleset_projection_test.go index 84aad8b1..2b9457af 100644 --- a/test/scripts/reviewer_router_ruleset_projection_test.go +++ b/test/scripts/reviewer_router_ruleset_projection_test.go @@ -17,11 +17,42 @@ const strictUpdateRuleHelper = `function isStrictUpdateRule(rule) { if (rule?.type !== 'update') { return false; } - const updateAllowsFetchAndMerge = - rule.parameters?.update_allows_fetch_and_merge; + if (!Object.prototype.hasOwnProperty.call(rule, 'parameters')) { + return true; + } + const parameters = rule.parameters; + if ( + parameters === null || + typeof parameters !== 'object' || + Array.isArray(parameters) + ) { + return false; + } + const parameterKeys = Object.keys(parameters); return ( - updateAllowsFetchAndMerge === undefined || - updateAllowsFetchAndMerge === false + parameterKeys.length === 1 && + parameterKeys[0] === 'update_allows_fetch_and_merge' && + parameters.update_allows_fetch_and_merge === false + ); +}` + +const strictGraphQLUpdateRuleHelper = `function isStrictGraphQLUpdateRule(restRuleset, graphRuleset) { + const restRulesetID = Number(restRuleset?.id); + const graphRulesetID = Number(graphRuleset?.databaseId); + const graphRules = graphRuleset?.rules; + const graphRule = graphRules?.nodes?.[0]; + return ( + Number.isSafeInteger(restRulesetID) && + restRulesetID > 0 && + graphRulesetID === restRulesetID && + graphRuleset.name === restRuleset.name && + graphRuleset.enforcement === 'ACTIVE' && + graphRuleset.target === 'BRANCH' && + graphRules?.totalCount === 1 && + graphRules.nodes?.length === 1 && + graphRule?.type === 'UPDATE' && + graphRule.parameters?.__typename === 'UpdateParameters' && + graphRule.parameters.updateAllowsFetchAndMerge === false ); }` @@ -63,6 +94,9 @@ func TestReviewerRouterAcceptsGitHubStrictUpdateReadProjection(t *testing.T) { if got := strings.Count(script, strictUpdateRuleHelper); got != 1 { t.Errorf("writer-ruleset script contains strict update helper %d times, want 1", got) } + if got := strings.Count(script, strictGraphQLUpdateRuleHelper); got != 1 { + t.Errorf("writer-ruleset script contains GraphQL strict update helper %d times, want 1", got) + } if strings.Contains( script, "parameters?.update_allows_fetch_and_merge !== false", @@ -72,21 +106,36 @@ func TestReviewerRouterAcceptsGitHubStrictUpdateReadProjection(t *testing.T) { if !strings.Contains(script, "!isStrictUpdateRule(writerRuleset.rules[0])") { t.Error("writer-ruleset script does not enforce the strict update helper") } + for _, marker := range []string{ + "query ReviewerRouterWriterRule($rulesetID: ID!)", + "rules(first: 2)", + "updateAllowsFetchAndMerge", + "{rulesetID: writerRuleset.node_id}", + "!isStrictGraphQLUpdateRule(writerRuleset, graphWriterRuleset)", + } { + if !strings.Contains(script, marker) { + t.Errorf("writer-ruleset script does not enforce GraphQL marker %q", marker) + } + } } if checked != 3 { t.Fatalf("writer-ruleset scripts checked = %d, want App enable, reconcile, and CI self-check", checked) } - verification := strictUpdateRuleHelper + ` + verification := strictUpdateRuleHelper + "\n" + strictGraphQLUpdateRuleHelper + ` const cases = [ ['omitted parameters', {type: 'update'}, true], - ['null parameters', {type: 'update', parameters: null}, true], - ['empty parameters', {type: 'update', parameters: {}}, true], ['explicit false', {type: 'update', parameters: {update_allows_fetch_and_merge: false}}, true], + ['null parameters', {type: 'update', parameters: null}, false], + ['empty parameters', {type: 'update', parameters: {}}, false], + ['boolean parameters', {type: 'update', parameters: false}, false], + ['string parameters', {type: 'update', parameters: 'malformed'}, false], + ['array parameters', {type: 'update', parameters: []}, false], ['explicit true', {type: 'update', parameters: {update_allows_fetch_and_merge: true}}, false], ['null field', {type: 'update', parameters: {update_allows_fetch_and_merge: null}}, false], ['string false', {type: 'update', parameters: {update_allows_fetch_and_merge: 'false'}}, false], ['numeric false', {type: 'update', parameters: {update_allows_fetch_and_merge: 0}}, false], + ['extra parameter', {type: 'update', parameters: {update_allows_fetch_and_merge: false, future_exception: false}}, false], ['wrong rule type', {type: 'creation'}, false], ['null rule', null, false], ]; @@ -96,6 +145,41 @@ for (const [name, rule, want] of cases) { throw new Error(name + ': got ' + got + ', want ' + want); } } + +const restRuleset = {id: 21363804, name: 'main-merge-writers'}; +const graphRule = { + type: 'UPDATE', + parameters: { + __typename: 'UpdateParameters', + updateAllowsFetchAndMerge: false, + }, +}; +const graphRuleset = { + databaseId: 21363804, + name: 'main-merge-writers', + enforcement: 'ACTIVE', + target: 'BRANCH', + rules: {totalCount: 1, nodes: [graphRule]}, +}; +const graphCases = [ + ['valid GraphQL projection', restRuleset, graphRuleset, true], + ['invalid REST id', {id: 0, name: 'main-merge-writers'}, graphRuleset, false], + ['mismatched GraphQL id', restRuleset, {...graphRuleset, databaseId: 7}, false], + ['mismatched name', restRuleset, {...graphRuleset, name: 'other'}, false], + ['inactive ruleset', restRuleset, {...graphRuleset, enforcement: 'EVALUATE'}, false], + ['wrong target', restRuleset, {...graphRuleset, target: 'TAG'}, false], + ['extra rule', restRuleset, {...graphRuleset, rules: {totalCount: 2, nodes: [graphRule, graphRule]}}, false], + ['wrong rule type', restRuleset, {...graphRuleset, rules: {totalCount: 1, nodes: [{...graphRule, type: 'CREATION'}]}}, false], + ['wrong parameter type', restRuleset, {...graphRuleset, rules: {totalCount: 1, nodes: [{...graphRule, parameters: {...graphRule.parameters, __typename: 'PullRequestParameters'}}]}}, false], + ['fetch and merge enabled', restRuleset, {...graphRuleset, rules: {totalCount: 1, nodes: [{...graphRule, parameters: {...graphRule.parameters, updateAllowsFetchAndMerge: true}}]}}, false], + ['missing GraphQL node', restRuleset, null, false], +]; +for (const [name, rest, graph, want] of graphCases) { + const got = isStrictGraphQLUpdateRule(rest, graph); + if (got !== want) { + throw new Error(name + ': got ' + got + ', want ' + want); + } +} ` command := exec.Command(node, "-e", verification) if output, runErr := command.CombinedOutput(); runErr != nil { diff --git a/test/scripts/reviewer_router_workflow_test.go b/test/scripts/reviewer_router_workflow_test.go index ba972fa2..0d947707 100644 --- a/test/scripts/reviewer_router_workflow_test.go +++ b/test/scripts/reviewer_router_workflow_test.go @@ -296,10 +296,22 @@ func TestReviewerRouterWorkflowContract(t *testing.T) { "writerRuleset.rules?.length !== 1", "function isStrictUpdateRule(rule)", "rule?.type !== 'update'", - "rule.parameters?.update_allows_fetch_and_merge", - "updateAllowsFetchAndMerge === undefined", - "updateAllowsFetchAndMerge === false", + "!Object.prototype.hasOwnProperty.call(rule, 'parameters')", + "parameters === null", + "typeof parameters !== 'object'", + "Array.isArray(parameters)", + "const parameterKeys = Object.keys(parameters)", + "parameterKeys.length === 1", + "parameterKeys[0] === 'update_allows_fetch_and_merge'", + "parameters.update_allows_fetch_and_merge === false", "!isStrictUpdateRule(writerRuleset.rules[0])", + "typeof writerRuleset.node_id !== 'string'", + "function isStrictGraphQLUpdateRule(restRuleset, graphRuleset)", + "graphRule.parameters.updateAllowsFetchAndMerge === false", + "query ReviewerRouterWriterRule($rulesetID: ID!)", + "... on UpdateParameters", + "{rulesetID: writerRuleset.node_id}", + "!isStrictGraphQLUpdateRule(writerRuleset, graphWriterRuleset)", "current_user_can_bypass !== 'pull_requests_only'", "ruleset.current_user_can_bypass !== 'never'", "Reviewer Router App must never bypass main ruleset", @@ -449,10 +461,22 @@ func TestReviewerRouterWorkflowContract(t *testing.T) { "writerRuleset.rules?.length !== 1", "function isStrictUpdateRule(rule)", "rule?.type !== 'update'", - "rule.parameters?.update_allows_fetch_and_merge", - "updateAllowsFetchAndMerge === undefined", - "updateAllowsFetchAndMerge === false", + "!Object.prototype.hasOwnProperty.call(rule, 'parameters')", + "parameters === null", + "typeof parameters !== 'object'", + "Array.isArray(parameters)", + "const parameterKeys = Object.keys(parameters)", + "parameterKeys.length === 1", + "parameterKeys[0] === 'update_allows_fetch_and_merge'", + "parameters.update_allows_fetch_and_merge === false", "!isStrictUpdateRule(writerRuleset.rules[0])", + "typeof writerRuleset.node_id !== 'string'", + "function isStrictGraphQLUpdateRule(restRuleset, graphRuleset)", + "graphRule.parameters.updateAllowsFetchAndMerge === false", + "query ReviewerRouterWriterRule($rulesetID: ID!)", + "... on UpdateParameters", + "{rulesetID: writerRuleset.node_id}", + "!isStrictGraphQLUpdateRule(writerRuleset, graphWriterRuleset)", "current_user_can_bypass !== 'pull_requests_only'", "ruleset.current_user_can_bypass !== 'never'", "Reviewer Router App must never bypass main ruleset", @@ -560,10 +584,22 @@ func TestCodeAdmissionEnforcesReviewerRouterWriterBoundary(t *testing.T) { "writerRuleset.rules?.length !== 1", "function isStrictUpdateRule(rule)", "rule?.type !== 'update'", - "rule.parameters?.update_allows_fetch_and_merge", - "updateAllowsFetchAndMerge === undefined", - "updateAllowsFetchAndMerge === false", + "!Object.prototype.hasOwnProperty.call(rule, 'parameters')", + "parameters === null", + "typeof parameters !== 'object'", + "Array.isArray(parameters)", + "const parameterKeys = Object.keys(parameters)", + "parameterKeys.length === 1", + "parameterKeys[0] === 'update_allows_fetch_and_merge'", + "parameters.update_allows_fetch_and_merge === false", "!isStrictUpdateRule(writerRuleset.rules[0])", + "typeof writerRuleset.node_id !== 'string'", + "function isStrictGraphQLUpdateRule(restRuleset, graphRuleset)", + "graphRule.parameters.updateAllowsFetchAndMerge === false", + "query ReviewerRouterWriterRule($rulesetID: ID!)", + "... on UpdateParameters", + "{rulesetID: writerRuleset.node_id}", + "!isStrictGraphQLUpdateRule(writerRuleset, graphWriterRuleset)", "writerRuleset.current_user_can_bypass !== 'never'", "deny this built-in Actions identity any bypass", "const skipWorkflowPattern =", From c629e1e3eb59b4db83bb837e0f5ce9a27f315de9 Mon Sep 17 00:00:00 2001 From: chichuan Date: Tue, 25 Aug 2026 12:34:32 +0800 Subject: [PATCH 4/5] fix(ci): handle read-only merge defaults projection GitHub omits merge-related repository settings from tokens without Contents write. Accept only the exact dual omission in read-only admission, and require the dedicated App to observe the reviewed values before any auto-merge mutation. --- .github/workflows/ci.yml | 43 ++++- .github/workflows/reviewer-router.yml | 74 ++++++++ docs/automation.md | 7 +- docs/ci-pr-gates.md | 7 +- ...r_router_merge_defaults_projection_test.go | 173 ++++++++++++++++++ test/scripts/reviewer_router_workflow_test.go | 6 +- 6 files changed, 301 insertions(+), 9 deletions(-) create mode 100644 test/scripts/reviewer_router_merge_defaults_projection_test.go diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e8c8c124..31c4a2ee 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -954,15 +954,48 @@ jobs: const skipWorkflowPattern = /\[(?:skip ci|ci skip|no ci|skip actions|actions skip)\]|\bskip-checks\s*:\s*true\b/i; const {data: repository} = await github.rest.repos.get({owner, repo}); - if ( - repository.merge_commit_title !== 'MERGE_MESSAGE' || - !['PR_TITLE', 'BLANK'].includes(repository.merge_commit_message) - ) { + function classifyMergeDefaults(repository) { + if ( + repository === null || + typeof repository !== 'object' || + Array.isArray(repository) + ) { + return 'invalid'; + } + const hasTitle = Object.prototype.hasOwnProperty.call( + repository, + 'merge_commit_title', + ); + const hasMessage = Object.prototype.hasOwnProperty.call( + repository, + 'merge_commit_message', + ); + if (!hasTitle && !hasMessage) { + return 'omitted'; + } + if (!hasTitle || !hasMessage) { + return 'invalid'; + } + if ( + repository.merge_commit_title === 'MERGE_MESSAGE' && + ['PR_TITLE', 'BLANK'].includes(repository.merge_commit_message) + ) { + return 'reviewed'; + } + return 'invalid'; + } + const mergeDefaultsProjection = classifyMergeDefaults(repository); + if (mergeDefaultsProjection === 'invalid') { core.setFailed( - 'Repository merge-message defaults changed from their reviewed values.', + 'Repository merge-message defaults are malformed or changed from their reviewed values.', ); return; } + if (mergeDefaultsProjection === 'omitted') { + core.info( + 'Read-only CI cannot observe repository merge-message defaults; exact validation is delegated to the dedicated App.', + ); + } const appliedRules = await github.paginate( 'GET /repos/{owner}/{repo}/rules/branches/{branch}', diff --git a/.github/workflows/reviewer-router.yml b/.github/workflows/reviewer-router.yml index b4066afd..0b7988f5 100644 --- a/.github/workflows/reviewer-router.yml +++ b/.github/workflows/reviewer-router.yml @@ -405,6 +405,43 @@ jobs: ); } const expectedAppOwner = `${appSlug}[bot]`; + const {data: repository} = await github.rest.repos.get({owner, repo}); + function classifyMergeDefaults(repository) { + if ( + repository === null || + typeof repository !== 'object' || + Array.isArray(repository) + ) { + return 'invalid'; + } + const hasTitle = Object.prototype.hasOwnProperty.call( + repository, + 'merge_commit_title', + ); + const hasMessage = Object.prototype.hasOwnProperty.call( + repository, + 'merge_commit_message', + ); + if (!hasTitle && !hasMessage) { + return 'omitted'; + } + if (!hasTitle || !hasMessage) { + return 'invalid'; + } + if ( + repository.merge_commit_title === 'MERGE_MESSAGE' && + ['PR_TITLE', 'BLANK'].includes(repository.merge_commit_message) + ) { + return 'reviewed'; + } + return 'invalid'; + } + const mergeDefaultsProjection = classifyMergeDefaults(repository); + if (mergeDefaultsProjection !== 'reviewed') { + throw new Error( + 'Dedicated Reviewer Router App cannot verify the reviewed repository merge-message defaults.', + ); + } const repositorySource = `${owner}/${repo}`.toLowerCase(); const appliedRules = await github.paginate( 'GET /repos/{owner}/{repo}/rules/branches/{branch}', @@ -757,6 +794,43 @@ jobs: ); } const expectedAppOwner = `${appSlug}[bot]`; + const {data: repository} = await github.rest.repos.get({owner, repo}); + function classifyMergeDefaults(repository) { + if ( + repository === null || + typeof repository !== 'object' || + Array.isArray(repository) + ) { + return 'invalid'; + } + const hasTitle = Object.prototype.hasOwnProperty.call( + repository, + 'merge_commit_title', + ); + const hasMessage = Object.prototype.hasOwnProperty.call( + repository, + 'merge_commit_message', + ); + if (!hasTitle && !hasMessage) { + return 'omitted'; + } + if (!hasTitle || !hasMessage) { + return 'invalid'; + } + if ( + repository.merge_commit_title === 'MERGE_MESSAGE' && + ['PR_TITLE', 'BLANK'].includes(repository.merge_commit_message) + ) { + return 'reviewed'; + } + return 'invalid'; + } + const mergeDefaultsProjection = classifyMergeDefaults(repository); + if (mergeDefaultsProjection !== 'reviewed') { + throw new Error( + 'Dedicated Reviewer Router App cannot verify the reviewed repository merge-message defaults.', + ); + } const repositorySource = `${owner}/${repo}`.toLowerCase(); const appliedRules = await github.paginate( 'GET /repos/{owner}/{repo}/rules/branches/{branch}', diff --git a/docs/automation.md b/docs/automation.md index a3636bf1..e2efc038 100644 --- a/docs/automation.md +++ b/docs/automation.md @@ -130,7 +130,12 @@ silently re-enabling the request, leaving it available only to the designated break-glass identity. The required `Test` context rejects GitHub workflow-skip directives in the PR title or an existing auto-merge request and verifies the repository's reviewed `MERGE_MESSAGE` title plus `PR_TITLE` or `BLANK` body -defaults. The dedicated App binds the mutation to the exact head OID and +defaults. GitHub does not expose those merge-related settings to the read-only +admission token: the classifier accepts only both exact reviewed values or the +complete omission of both properties, and rejects partial omission, `null`, or +any other value. Before any enable or reconcile mutation, the dedicated App's +current-repository token (which has `Contents: write`) must observe both exact +reviewed values. The dedicated App binds the mutation to the exact head OID and supplies a fixed safe headline and body, so GitHub cannot copy an unsafe PR title into its merge commit. After enabling, the workflow requires the owner to equal the token action's diff --git a/docs/ci-pr-gates.md b/docs/ci-pr-gates.md index 6254e564..1b64ea74 100644 --- a/docs/ci-pr-gates.md +++ b/docs/ci-pr-gates.md @@ -179,7 +179,12 @@ the same ruleset node through GraphQL and requires its non-null own built-in token to report `current_user_can_bypass: never`. The minted App separately requires `pull_requests_only` on that writer rule and `never` on every other active main -ruleset before it can enable or reconcile auto-merge. GitHub hides the complete +ruleset before it can enable or reconcile auto-merge. The read-only `Test` +token may receive a repository projection with both merge-default properties +omitted; it accepts only that complete omission or exact `MERGE_MESSAGE` plus +`PR_TITLE`/`BLANK`, while partial or malformed projections fail closed. The +minted App's `Contents: write` token must observe the exact reviewed defaults +before either mutation path proceeds. GitHub hides the complete `bypass_actors` list from low-privilege callers, so the rollout audit must still keep the writer list at exactly the Reviewer App plus `PeterGuy326` (ID `47820304`). The required check finally accepts a null or exact App-owned diff --git a/test/scripts/reviewer_router_merge_defaults_projection_test.go b/test/scripts/reviewer_router_merge_defaults_projection_test.go new file mode 100644 index 00000000..1f201b3b --- /dev/null +++ b/test/scripts/reviewer_router_merge_defaults_projection_test.go @@ -0,0 +1,173 @@ +// 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 mergeDefaultsProjectionHelper = `function classifyMergeDefaults(repository) { + if ( + repository === null || + typeof repository !== 'object' || + Array.isArray(repository) + ) { + return 'invalid'; + } + const hasTitle = Object.prototype.hasOwnProperty.call( + repository, + 'merge_commit_title', + ); + const hasMessage = Object.prototype.hasOwnProperty.call( + repository, + 'merge_commit_message', + ); + if (!hasTitle && !hasMessage) { + return 'omitted'; + } + if (!hasTitle || !hasMessage) { + return 'invalid'; + } + if ( + repository.merge_commit_title === 'MERGE_MESSAGE' && + ['PR_TITLE', 'BLANK'].includes(repository.merge_commit_message) + ) { + return 'reviewed'; + } + return 'invalid'; +}` + +const privilegedMergeDefaultsGuard = `if (mergeDefaultsProjection !== 'reviewed') { + throw new Error( + 'Dedicated Reviewer Router App cannot verify the reviewed repository merge-message defaults.', + ); +}` + +func TestReviewerRouterMergeDefaultsProjection(t *testing.T) { + t.Parallel() + + node, err := exec.LookPath("node") + if err != nil { + t.Skip("node is required to verify merge-default projection semantics") + } + root, err := filepath.Abs(filepath.Join("..", "..")) + if err != nil { + t.Fatalf("resolve repository root: %v", err) + } + + var scripts []string + for _, relativePath := range []string{ + filepath.Join(".github", "workflows", "ci.yml"), + filepath.Join(".github", "workflows", "reviewer-router.yml"), + } { + path := filepath.Join(root, relativePath) + data, readErr := os.ReadFile(path) + if readErr != nil { + t.Fatalf("read %s: %v", path, readErr) + } + var workflow any + if unmarshalErr := yaml.Unmarshal(data, &workflow); unmarshalErr != nil { + t.Fatalf("parse %s: %v", path, unmarshalErr) + } + collectGitHubScripts(workflow, &scripts) + } + + checked := 0 + readOnlyChecks := 0 + privilegedChecks := 0 + for _, script := range scripts { + if strings.Count(script, mergeDefaultsProjectionHelper) != 1 { + continue + } + checked++ + for _, marker := range []string{ + "github.rest.repos.get", + "const mergeDefaultsProjection = classifyMergeDefaults(repository);", + } { + if !strings.Contains(script, marker) { + t.Errorf("merge-default script does not enforce marker %q", marker) + } + } + if strings.Contains(script, "MINTED_REVIEWER_ROUTER_APP_SLUG") { + privilegedChecks++ + for _, marker := range []string{ + privilegedMergeDefaultsGuard, + } { + if !strings.Contains(script, marker) { + t.Errorf("privileged merge-default script missing marker %q", marker) + } + } + guardIndex := strings.Index(script, privilegedMergeDefaultsGuard) + guardEndIndex := guardIndex + len(privilegedMergeDefaultsGuard) + firstMutationIndex := len(script) + for _, mutation := range []string{ + "disablePullRequestAutoMerge", + "enablePullRequestAutoMerge", + } { + if index := strings.Index(script, mutation); index >= 0 && index < firstMutationIndex { + firstMutationIndex = index + } + } + if guardIndex < 0 || firstMutationIndex == len(script) || guardEndIndex >= firstMutationIndex { + t.Error("privileged merge-default validation must precede every auto-merge mutation") + } + continue + } + readOnlyChecks++ + for _, marker := range []string{ + "mergeDefaultsProjection === 'invalid'", + "mergeDefaultsProjection === 'omitted'", + "Read-only CI cannot observe repository merge-message defaults; exact validation is delegated to the dedicated App.", + } { + if !strings.Contains(script, marker) { + t.Errorf("read-only merge-default script missing marker %q", marker) + } + } + if strings.Contains(script, "mergeDefaultsProjection !== 'reviewed'") { + t.Error("read-only CI must distinguish exact dual omission from malformed projections") + } + } + if checked != 3 || readOnlyChecks != 1 || privilegedChecks != 2 { + t.Fatalf( + "merge-default scripts checked=%d read-only=%d privileged=%d, want 3/1/2", + checked, + readOnlyChecks, + privilegedChecks, + ) + } + + verification := mergeDefaultsProjectionHelper + ` +const cases = [ + ['reviewed PR title', {merge_commit_title: 'MERGE_MESSAGE', merge_commit_message: 'PR_TITLE'}, 'reviewed'], + ['reviewed blank body', {merge_commit_title: 'MERGE_MESSAGE', merge_commit_message: 'BLANK'}, 'reviewed'], + ['dual omission', {}, 'omitted'], + ['null repository', null, 'invalid'], + ['array repository', [], 'invalid'], + ['title only', {merge_commit_title: 'MERGE_MESSAGE'}, 'invalid'], + ['message only', {merge_commit_message: 'PR_TITLE'}, 'invalid'], + ['own undefined title', {merge_commit_title: undefined, merge_commit_message: 'PR_TITLE'}, 'invalid'], + ['own undefined message', {merge_commit_title: 'MERGE_MESSAGE', merge_commit_message: undefined}, 'invalid'], + ['null title', {merge_commit_title: null, merge_commit_message: 'PR_TITLE'}, 'invalid'], + ['null message', {merge_commit_title: 'MERGE_MESSAGE', merge_commit_message: null}, 'invalid'], + ['unsafe title', {merge_commit_title: 'PR_TITLE', merge_commit_message: 'PR_TITLE'}, 'invalid'], + ['unsafe body', {merge_commit_title: 'MERGE_MESSAGE', merge_commit_message: 'PR_BODY'}, 'invalid'], +]; +for (const [name, repository, want] of cases) { + const got = classifyMergeDefaults(repository); + 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("merge-default projection verification failed: %v\n%s", runErr, output) + } +} diff --git a/test/scripts/reviewer_router_workflow_test.go b/test/scripts/reviewer_router_workflow_test.go index 0d947707..e33345e5 100644 --- a/test/scripts/reviewer_router_workflow_test.go +++ b/test/scripts/reviewer_router_workflow_test.go @@ -568,8 +568,10 @@ func TestCodeAdmissionEnforcesReviewerRouterWriterBoundary(t *testing.T) { "const expectedAppOwner = `${appSlug}[bot]`", "github.rest.pulls.get", "github.rest.repos.get", - "repository.merge_commit_title !== 'MERGE_MESSAGE'", - "!['PR_TITLE', 'BLANK'].includes(repository.merge_commit_message)", + "function classifyMergeDefaults(repository)", + "mergeDefaultsProjection === 'invalid'", + "mergeDefaultsProjection === 'omitted'", + "exact validation is delegated to the dedicated App", "const writerRulesetName = 'main-merge-writers'", "GET /repos/{owner}/{repo}/rules/branches/{branch}", "rule.ruleset_source_type === 'Repository'", From b45eba0f5c00ff51b036ddc3d1819a486fbc024e Mon Sep 17 00:00:00 2001 From: chichuan Date: Tue, 25 Aug 2026 14:06:09 +0800 Subject: [PATCH 5/5] 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. --- .../workflows/coverage-baseline-repair.yml | 81 +++++++--- docs/automation.md | 7 +- docs/ci-pr-gates.md | 9 +- .../coverage_baseline_repair_identity_test.go | 145 ++++++++++++++++++ .../coverage_workflow_contract_test.go | 47 ++++-- 5 files changed, 245 insertions(+), 44 deletions(-) create mode 100644 test/scripts/coverage_baseline_repair_identity_test.go 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 {