From cf2fe789af95529cae9094be833103bd299fd001 Mon Sep 17 00:00:00 2001 From: Nathan Vale Date: Fri, 14 Aug 2026 08:47:33 +1000 Subject: [PATCH 1/7] feat(release): add pre-tag recovery for stranded proven candidates A release cancelled after proof but before tag creation left the candidate proven, merged, and untagged. Release Please then aborted every later run with "There are untagged, merged release PRs outstanding", and `repair` could not help because it requires an existing immutable tag. Recovery depended on rerunning the original run before its artifacts expired. Root cause: the workflow had no operation for the merged-and-proven-but- untagged state, and maintenance treated that state as silence. Add `operation=resume`, which publishes one exact candidate SHA: - Recover the publication-candidate record the original run persisted before proof; never mint a fresh admission. - Revalidate PR identity, base branch, merge binding, two-parent topology, projection digest, and manifest version against that record. - Require the target tag to be absent; a tagged candidate belongs to repair. - Repeat packaging, four-platform compatibility, and candidate proof, then enter the same protected `release` environment. - Fail closed when the record is missing or expired. Reconstructing it would admit a candidate no run ever proved. Make maintenance fail loudly with the exact resume command, including the candidate SHA, when it detects a merged release PR whose tag never existed. Also verify hosted-canary credential lifecycle rather than secret names alone: a private key whose registered public half was deleted kept every name-based check green while SSH admission failed. Readiness now requires a registered public half and surfaces a rotation action. Closes #17 --- .github/workflows/release.yml | 182 ++++++++++++++++++- README.md | 2 +- docs/adr/0003-reviewed-versioned-releases.md | 2 +- docs/release-repair.md | 35 +++- docs/releasing.md | 2 +- scripts/readme-release-pin.test.ts | 12 +- scripts/release-validate.test.ts | 166 ++++++++++++++++- scripts/release-validate.ts | 58 ++++++ scripts/repository-readiness.test.ts | 31 +++- scripts/repository-readiness.ts | 73 ++++++-- 10 files changed, 524 insertions(+), 39 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 11f89ad..e49252a 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -7,13 +7,18 @@ on: workflow_dispatch: inputs: operation: - description: Maintain the release PR or repair an admitted immutable release + description: Maintain the release PR, resume a stranded untagged candidate, or repair an admitted immutable release required: true default: maintenance type: choice options: - maintenance + - resume - repair + candidate_sha: + description: Merged, proven, untagged candidate SHA to resume through protected publication + required: false + type: string release_tag: description: Existing vX.Y.Z tag whose incomplete publication needs repair required: false @@ -61,6 +66,7 @@ jobs: PUSH_BEFORE_SHA: ${{ github.event.before }} PUSH_FORCED: ${{ github.event.forced }} REPAIR_TAG: ${{ inputs.release_tag }} + RESUME_SHA: ${{ inputs.candidate_sha }} OPERATION: ${{ inputs.operation }} shell: bash run: | @@ -84,6 +90,129 @@ jobs: } >> "$GITHUB_OUTPUT" exit 0 fi + if [[ "$OPERATION" == "resume" ]]; then + if [[ ! "$RESUME_SHA" =~ ^[0-9a-f]{40}$ ]]; then + echo "Manual resume requires candidate_sha as a full 40-character commit SHA." >&2 + exit 1 + fi + workflow_policy_sha=$(git rev-parse HEAD) + # Resume replays the admission its original run already persisted. A + # missing record means no run ever proved this candidate, so resume + # must not mint a fresh admission from GitHub state. + artifact_id=$(gh api "repos/${GITHUB_REPOSITORY}/actions/artifacts?name=publication-candidate-${RESUME_SHA}" \ + --jq '.artifacts | map(select(.expired == false)) | sort_by(.created_at) | reverse | .[0].id // empty') + if [[ -z "$artifact_id" ]]; then + echo "Resume requires the persisted publication candidate for ${RESUME_SHA}; it is absent or expired." >&2 + echo "Recover by rerunning the original release run while its candidate artifact is retained." >&2 + exit 1 + fi + gh api "repos/${GITHUB_REPOSITORY}/actions/artifacts/${artifact_id}/zip" > resume-candidate.zip + unzip -p resume-candidate.zip candidate.json > persisted-candidate.json + cp persisted-candidate.json candidate.json + + version=$(jq -er '.version | select(type == "string" and length > 0)' persisted-candidate.json) + release_tag="v${version}" + if gh api "repos/${GITHUB_REPOSITORY}/git/ref/tags/${release_tag}" >/dev/null 2>&1; then + echo "Candidate tag ${release_tag} already exists; use manual incomplete-publication repair." >&2 + exit 1 + fi + + pr_number=$(jq -er '.pullRequest | select(type == "number" and . > 0 and . == floor)' persisted-candidate.json) + admitted_automation_identity=$(jq -er '.automationIdentity | select(type == "string" and length > 0)' persisted-candidate.json) + if [[ "$admitted_automation_identity" != "$EXPECTED_RELEASE_PLEASE_LOGIN" ]]; then + echo "Persisted resume identity does not match the configured Release Please identity." >&2 + exit 1 + fi + release_pr=$(gh api "repos/${GITHUB_REPOSITORY}/pulls/${pr_number}") + base_branch=$(jq -r .base.ref <<< "$release_pr") + merged_pr_base_sha=$(jq -r .base.sha <<< "$release_pr") + reviewed_pr_head_sha=$(jq -r .head.sha <<< "$release_pr") + automation_identity=$(jq -r .user.login <<< "$release_pr") + merge_commit_sha=$(jq -r .merge_commit_sha <<< "$release_pr") + merged_at=$(jq -r '.merged_at // empty' <<< "$release_pr") + if [[ "$automation_identity" != "$EXPECTED_RELEASE_PLEASE_LOGIN" ]]; then + echo "Fresh resume PR identity does not match the configured Release Please identity." >&2 + exit 1 + fi + if [[ -z "$merged_at" || "$base_branch" != "$BASE_BRANCH" || "$merge_commit_sha" != "$RESUME_SHA" ]]; then + echo "Resume provenance does not match a merged Release Please candidate." >&2 + exit 1 + fi + candidate_commit=$(gh api "repos/${GITHUB_REPOSITORY}/commits/${RESUME_SHA}") + candidate_parent_shas=$(jq -c '[.parents[].sha]' <<< "$candidate_commit") + trusted_base_sha=$(git rev-parse "${RESUME_SHA}^1") + projection=$(gh api --paginate "repos/${GITHUB_REPOSITORY}/pulls/${pr_number}/files" \ + | jq -sc 'add | map({filename,status,sha}) | sort_by(.filename)') + printf '%s\n' "$projection" > resume-projection.json + # The candidate's first parent owns its historical projection policy. + git checkout --detach "$trusted_base_sha" + projection_result=$(bun run scripts/release-projection.ts \ + --base "$trusted_base_sha" \ + --head "$RESUME_SHA" \ + --projection resume-projection.json \ + --json) + projection_digest=$(jq -r .projectionDigest <<< "$projection_result") + manifest_version=$(gh api "repos/${GITHUB_REPOSITORY}/contents/plugin.config.json?ref=${RESUME_SHA}" --jq .content \ + | tr -d '\n' | base64 --decode | jq -r .version) + jq -nS \ + --argjson number "$pr_number" \ + --arg baseBranch "$base_branch" \ + --arg automationIdentity "$automation_identity" \ + --arg mergeCommit "$merge_commit_sha" \ + --arg projectionDigest "$projection_digest" \ + --argjson candidateParentShas "$candidate_parent_shas" \ + --arg trustedBaseSha "$trusted_base_sha" \ + --arg mergedPrBaseSha "$merged_pr_base_sha" \ + --arg reviewedPrHeadSha "$reviewed_pr_head_sha" \ + --argjson changedFiles "$(jq '[.[].filename]' resume-projection.json)" \ + --argjson changedFileStatuses "$(jq '[.[].status]' resume-projection.json)" \ + '{number:$number,baseBranch:$baseBranch,automationIdentity:$automationIdentity,mergeCommit:$mergeCommit,changedFiles:$changedFiles,changedFileStatuses:$changedFileStatuses,projectionDigest:$projectionDigest,candidateParentShas:$candidateParentShas,trustedBaseSha:$trustedBaseSha,mergedPrBaseSha:$mergedPrBaseSha,reviewedPrHeadSha:$reviewedPrHeadSha}' \ + > trusted-resume-candidate.json + # The workflow source owns the current identity and topology contract. + git checkout --detach "$workflow_policy_sha" + RESUME_REPOSITORY="$GITHUB_REPOSITORY" \ + RESUME_BASE_BRANCH="$BASE_BRANCH" \ + RESUME_AUTOMATION_LOGIN="$EXPECTED_RELEASE_PLEASE_LOGIN" \ + RESUME_CANDIDATE_SHA="$RESUME_SHA" \ + RESUME_VERSION="$manifest_version" \ + bun -e ' + import { readFileSync } from "node:fs"; + import { validateResumeCandidateBinding } from "./scripts/release-validate.ts"; + const { + candidateParentShas, + trustedBaseSha, + mergedPrBaseSha, + reviewedPrHeadSha, + ...trustedCandidate + } = JSON.parse(readFileSync("trusted-resume-candidate.json", "utf8")); + validateResumeCandidateBinding({ + candidate: JSON.parse(readFileSync("persisted-candidate.json", "utf8")), + trustedCandidate, + candidateParentShas, + trustedBaseSha, + mergedPrBaseSha, + reviewedPrHeadSha, + repository: process.env.RESUME_REPOSITORY ?? "", + expectedBaseBranch: process.env.RESUME_BASE_BRANCH ?? "", + expectedAutomationIdentities: [process.env.RESUME_AUTOMATION_LOGIN ?? ""], + candidateSha: process.env.RESUME_CANDIDATE_SHA ?? "", + manifestVersion: process.env.RESUME_VERSION ?? "", + tagExists: false, + }); + ' + { + echo "mode=resume" + echo "candidate_sha=${RESUME_SHA}" + echo "release_tag=${release_tag}" + echo "version=${version}" + echo "release_target_sha=" + echo "trusted_base_sha=${trusted_base_sha}" + echo "merged_pr_base_sha=${merged_pr_base_sha}" + echo "reviewed_pr_head_sha=${reviewed_pr_head_sha}" + echo "record_reused=true" + } >> "$GITHUB_OUTPUT" + exit 0 + fi if [[ "$OPERATION" != "repair" ]]; then echo "Unsupported manual operation: ${OPERATION}." >&2 exit 1 @@ -411,6 +540,38 @@ jobs: echo "Release Please token identity ${actual_login} does not match configured identity ${EXPECTED_RELEASE_PLEASE_LOGIN}." >&2 exit 1 fi + # Release Please aborts silently on an untagged merged release PR. Detect that + # state first and name the exact recovery command, so a stranded candidate + # cannot quietly stop every later release. + - name: Detect a merged release candidate stranded before its tag + env: + GH_TOKEN: ${{ github.token }} + run: | + set -euo pipefail + stranded=$(gh api --paginate \ + "repos/${GITHUB_REPOSITORY}/issues?state=closed&labels=autorelease:%20pending&per_page=100" \ + --jq '[.[] | select(.pull_request != null) | .number] | .[]' || true) + for pr_number in $stranded; do + release_pr=$(gh api "repos/${GITHUB_REPOSITORY}/pulls/${pr_number}") + merged_at=$(jq -r '.merged_at // empty' <<< "$release_pr") + merge_commit_sha=$(jq -r '.merge_commit_sha // empty' <<< "$release_pr") + [[ -n "$merged_at" && "$merge_commit_sha" =~ ^[0-9a-f]{40}$ ]] || continue + version=$(gh api "repos/${GITHUB_REPOSITORY}/contents/plugin.config.json?ref=${merge_commit_sha}" --jq .content \ + | tr -d '\n' | base64 --decode | jq -r .version) + if gh api "repos/${GITHUB_REPOSITORY}/git/ref/tags/v${version}" >/dev/null 2>&1; then + continue + fi + echo "Release PR #${pr_number} merged at ${merge_commit_sha} but v${version} was never tagged." >&2 + echo "Release Please will abort until this candidate is published or its lineage is reconciled." >&2 + echo "Resume the proven candidate through the protected release environment:" >&2 + echo "" >&2 + echo " gh workflow run Release \\" >&2 + echo " --repo ${GITHUB_REPOSITORY} \\" >&2 + echo " --ref main \\" >&2 + echo " -f operation=resume \\" >&2 + echo " -f candidate_sha=${merge_commit_sha}" >&2 + exit 1 + done # skip-github-release is set in release-please config. The action may maintain PRs but cannot tag or create a GitHub Release. - name: Maintain release pull request uses: googleapis/release-please-action@45996ed1f6d02564a971a2fa1b5860e934307cf7 # v5.0.0 @@ -422,7 +583,10 @@ jobs: candidate: name: Build release candidate once - if: needs.resolve.outputs.mode == 'publish' || needs.resolve.outputs.mode == 'repair' + if: >- + needs.resolve.outputs.mode == 'publish' || + needs.resolve.outputs.mode == 'resume' || + needs.resolve.outputs.mode == 'repair' needs: resolve permissions: actions: read @@ -451,7 +615,7 @@ jobs: compatibility: name: Release compatibility (${{ matrix.target }}) - if: needs.resolve.outputs.mode == 'publish' + if: needs.resolve.outputs.mode == 'publish' || needs.resolve.outputs.mode == 'resume' needs: - resolve - candidate @@ -502,7 +666,9 @@ jobs: always() && needs.resolve.result == 'success' && needs.candidate.result == 'success' && - (needs.resolve.outputs.mode == 'publish' || needs.resolve.outputs.mode == 'repair') && + (needs.resolve.outputs.mode == 'publish' || + needs.resolve.outputs.mode == 'resume' || + needs.resolve.outputs.mode == 'repair') && (needs.compatibility.result == 'success' || (needs.resolve.outputs.mode == 'repair' && needs.compatibility.result == 'skipped')) needs: @@ -582,7 +748,9 @@ jobs: always() && needs.resolve.result == 'success' && needs.package.result == 'success' && - (needs.resolve.outputs.mode == 'publish' || needs.resolve.outputs.mode == 'repair') + (needs.resolve.outputs.mode == 'publish' || + needs.resolve.outputs.mode == 'resume' || + needs.resolve.outputs.mode == 'repair') needs: - resolve - package @@ -850,7 +1018,7 @@ jobs: echo "Immutable remote tag does not target the proven candidate SHA." >&2 exit 1 fi - if [[ -z "$remote_tag_sha" && "$MODE" == "publish" ]]; then + if [[ -z "$remote_tag_sha" && ( "$MODE" == "publish" || "$MODE" == "resume" ) ]]; then if [[ -n "$local_tag_sha" && "$local_tag_sha" != "$CANDIDATE_SHA" ]]; then echo "Local release tag does not target the proven candidate SHA." >&2 exit 1 @@ -1027,7 +1195,7 @@ jobs: "$PACKAGE_RESULT" == "skipped" && "$RELEASE_RESULT" == "skipped" ]] && converged=true ;; - publish) + publish|resume) [[ "$MAINTAIN_RESULT" == "skipped" && "$CANDIDATE_RESULT" == "success" && "$COMPATIBILITY_RESULT" == "success" && diff --git a/README.md b/README.md index a571d96..c040019 100644 --- a/README.md +++ b/README.md @@ -22,7 +22,7 @@ Contributors need Git and [Bun](https://bun.sh/docs/installation). Plugin consum | Configure GitHub release automation | Read [Configure release automation](docs/release-setup.md). | | Qualify fresh native capabilities | Read [Qualify fresh native capabilities](docs/native-capability-qualification.md). | | Publish a release | Read [Publish a release](docs/publishing.md). | -| Maintain or repair release state | Read [Maintain or repair release state](docs/release-repair.md). | +| Maintain, resume, or repair release state | Read [Maintain, resume, or repair release state](docs/release-repair.md). | | Qualify publishing-system changes with public/private canaries | Read [Qualify public and private canaries](docs/canary-qualification.md). | | Change the distribution architecture or its language | Read [`CONTEXT.md`](CONTEXT.md), then the relevant [ADR](docs/adr/). | diff --git a/docs/adr/0003-reviewed-versioned-releases.md b/docs/adr/0003-reviewed-versioned-releases.md index 789f472..c6a7f93 100644 --- a/docs/adr/0003-reviewed-versioned-releases.md +++ b/docs/adr/0003-reviewed-versioned-releases.md @@ -24,7 +24,7 @@ Packaging emits a deterministic `tar.gz` and `*.checksums.json`. The JSON binds ## Repair contract -Manual dispatch accepts `operation=maintenance` or `operation=repair`. Maintenance is the default and only updates the standing release PR. Repair requires `operation=repair` plus `release_tag` naming an existing `vX.Y.Z` tag. Repair begins from the immutable tag, rederives candidate topology and fresh PR authorship from GitHub, and checks identity against repository configuration rather than trusting the tag-carried record. It repeats the complete proof and validates any existing GitHub Release target. It compares each asset before writing: matching assets remain untouched, missing assets are added, and mismatches fail closed. A mismatched asset may be replaced only when `replace_mismatched_assets=true` is approved through the protected `release` environment. Repair never moves the tag and never represents a new release. +Manual dispatch accepts `operation=maintenance`, `operation=resume`, or `operation=repair`. Maintenance is the default and only updates the standing release PR; it fails closed and names the exact resume command when it detects a merged release PR whose tag was never created, so a stranded candidate cannot silently stop later releases. Resume requires `operation=resume` plus `candidate_sha` naming a merged, proven, untagged candidate. Resume recovers the publication-candidate record persisted before proof, revalidates PR identity, base branch, merge binding, topology, projection, and version against that record, requires the target tag to be absent, and repeats the complete proof before entering the protected `release` environment. It never mints a fresh admission, never reconstructs an expired record, and never creates an ad hoc tag. Repair requires `operation=repair` plus `release_tag` naming an existing `vX.Y.Z` tag. Repair begins from the immutable tag, rederives candidate topology and fresh PR authorship from GitHub, and checks identity against repository configuration rather than trusting the tag-carried record. It repeats the complete proof and validates any existing GitHub Release target. It compares each asset before writing: matching assets remain untouched, missing assets are added, and mismatches fail closed. A mismatched asset may be replaced only when `replace_mismatched_assets=true` is approved through the protected `release` environment. Repair never moves the tag and never represents a new release. ## Human-owned safeguards diff --git a/docs/release-repair.md b/docs/release-repair.md index e56e7af..03626f1 100644 --- a/docs/release-repair.md +++ b/docs/release-repair.md @@ -1,8 +1,10 @@ -# Maintain or repair release state +# Maintain, resume, or repair release state -Use this runbook to update the standing release PR or repair an incomplete publication at an existing immutable tag. +Use this runbook to update the standing release PR, resume a proven candidate stranded before its tag, or repair an incomplete publication at an existing immutable tag. -Manual dispatch accepts two operation values. `maintenance` is the default; it only updates the standing release PR and never publishes. `repair` requires `release_tag` set to the exact existing `vX.Y.Z` tag. This repairs an incomplete publication; it does not create a new release. +Manual dispatch accepts three operation values. `maintenance` is the default; it only updates the standing release PR and never publishes. `resume` requires `candidate_sha` and publishes a merged, proven candidate whose tag was never created. `repair` requires `release_tag` set to the exact existing `vX.Y.Z` tag; it repairs an incomplete publication and does not create a new release. + +Choose by tag state: no tag yet means `resume`; an existing tag means `repair`. ## Maintain the release PR @@ -15,6 +17,33 @@ gh workflow run Release \ Maintenance completes when the standing release PR reflects the current releasable commits and its generated version projection. +## Resume a candidate stranded before its tag + +A release PR can merge and pass full proof, then lose its publication job to a cancellation before the immutable tag exists. The candidate is proven but untagged, so Release Please refuses to open the next release PR and reports `There are untagged, merged release PRs outstanding - aborting`. Repair cannot help, because repair requires an existing tag. + +Maintenance detects this state and fails with the exact command, including the candidate SHA. Run it: + +```sh +gh workflow run Release \ + --repo OWNER/REPOSITORY \ + --ref main \ + -f operation=resume \ + -f candidate_sha=SHA +``` + +Resume never mints a fresh admission. It recovers the publication-candidate record the original run persisted before proof, revalidates Release Please PR identity, base branch, merge-commit binding, commit topology, projection digest, and manifest version against that record, and requires the target tag to be absent. It then repeats packaging, four-platform compatibility, and candidate proof before entering the same protected `release` environment as a normal publication. + +Resume fails closed and creates nothing when: + +- The persisted candidate record is missing or its 90-day retention expired. +- The target tag already exists — use repair instead. +- The record is rebound to another candidate, version, or pull request. +- The pull request is not a merged Release Please PR on the release base branch. + +An expired record cannot be reconstructed, because rebuilding it would admit a candidate no run ever proved. Recover by rerunning the original release run while its candidate artifact is still retained. + +Resume completes when the immutable tag targets the admitted candidate, the GitHub Release and assets match the proven package, the attestation exists, and the release PR carries `autorelease: tagged` so the next maintenance run creates the following release PR. + ## Repair an incomplete publication Start with compare-before-write repair and mismatched replacement disabled: diff --git a/docs/releasing.md b/docs/releasing.md index e2f7859..5b678b5 100644 --- a/docs/releasing.md +++ b/docs/releasing.md @@ -8,7 +8,7 @@ Use the runbook that matches the current job: | Configure GitHub release automation | [Configure release automation](release-setup.md) | | Record candidate-bound evidence from fresh clients | [Qualify fresh native capabilities](native-capability-qualification.md) | | Publish an admitted candidate | [Publish a release](publishing.md) | -| Maintain the release PR or repair an incomplete publication | [Maintain or repair release state](release-repair.md) | +| Maintain the release PR, resume a stranded candidate, or repair an incomplete publication | [Maintain, resume, or repair release state](release-repair.md) | | Qualify publishing-system changes in public and private repositories | [Qualify public and private canaries](canary-qualification.md) | The root [README](../README.md) owns plugin creation and extension. The [installation guide](installing.md) owns production installation, upgrade, replacement, and rollback. diff --git a/scripts/readme-release-pin.test.ts b/scripts/readme-release-pin.test.ts index ea2edfc..f902829 100644 --- a/scripts/readme-release-pin.test.ts +++ b/scripts/readme-release-pin.test.ts @@ -25,7 +25,7 @@ test("README routes each branch directly to its owner", async () => { "[Configure release automation](docs/release-setup.md)", "[Qualify fresh native capabilities](docs/native-capability-qualification.md)", "[Publish a release](docs/publishing.md)", - "[Maintain or repair release state](docs/release-repair.md)", + "[Maintain, resume, or repair release state](docs/release-repair.md)", "[Qualify public and private canaries](docs/canary-qualification.md)", ]) { expect(readme).toContain(pointer) @@ -44,7 +44,7 @@ test("maintainer index preserves one pointer per operator branch", async () => { "[Configure release automation](release-setup.md)", "[Qualify fresh native capabilities](native-capability-qualification.md)", "[Publish a release](publishing.md)", - "[Maintain or repair release state](release-repair.md)", + "[Maintain, resume, or repair release state](release-repair.md)", "[Qualify public and private canaries](canary-qualification.md)", ]) { expect(index).toContain(pointer) @@ -163,8 +163,14 @@ test("release repair preserves exact operations and a complete terminal bound", expect(repair).toContain("it only updates the standing release PR and never publishes") expect(repair).toContain("`repair` requires `release_tag`") expect(repair).toContain("exact existing `vX.Y.Z` tag") - expect(repair).toContain("This repairs an incomplete publication; it does not create a new release.") + expect(repair).toContain("it repairs an incomplete publication and does not create a new release") + expect(repair).toContain("`resume` requires `candidate_sha`") + expect(repair).toContain("no tag yet means `resume`; an existing tag means `repair`") + expect(repair).toContain("Resume never mints a fresh admission") + expect(repair).toContain("requires the target tag to be absent") + expect(repair).toContain("Resume completes when the immutable tag targets the admitted candidate") expect(repair.match(/-f operation=maintenance/g)).toHaveLength(1) + expect(repair.match(/-f operation=resume/g)).toHaveLength(1) expect(repair.match(/-f operation=repair/g)).toHaveLength(2) expect(repair).toContain("Repair completes when the immutable tag still targets the admitted candidate") expect(repair).toContain("the GitHub Release targets that commit") diff --git a/scripts/release-validate.test.ts b/scripts/release-validate.test.ts index 94f8e8c..1b56305 100644 --- a/scripts/release-validate.test.ts +++ b/scripts/release-validate.test.ts @@ -19,6 +19,7 @@ import { validateCapabilitySidecars, validateRepairCandidateBinding, validateRepairBinding, + validateResumeCandidateBinding, } from "./release-validate" const root = resolve(import.meta.dir, "..") @@ -335,6 +336,10 @@ test("release workflow is pinned and publishes proven assets after validation", workflow.indexOf(" - name: Resolve unique candidate or immutable repair tag\n"), workflow.indexOf("\n\n associated_prs="), ) + // The repair branch owns its own ordering; resume resolves before it. + const repairOnlyStep = repairValidationStep.slice( + repairValidationStep.indexOf('if [[ "$OPERATION" != "repair" ]]'), + ) const publishResolutionStep = workflow.slice( workflow.indexOf(" associated_prs="), workflow.indexOf(" - name: Persist publication candidate before proof\n"), @@ -376,15 +381,15 @@ test("release workflow is pinned and publishes proven assets after validation", expect(workflow).toContain("admitted_automation_identity") expect(workflow).not.toContain('--expected-automation-login "$admitted_automation_identity"') expect(workflow).toContain("scripts/release-projection.ts") - const historicalPolicyCheckout = repairValidationStep.indexOf( + const historicalPolicyCheckout = repairOnlyStep.indexOf( 'git checkout --detach "$trusted_base_sha"', ) - const historicalPolicyExecution = repairValidationStep.indexOf("bun run scripts/release-projection.ts") - const currentAdmissionCheckout = repairValidationStep.indexOf( + const historicalPolicyExecution = repairOnlyStep.indexOf("bun run scripts/release-projection.ts") + const currentAdmissionCheckout = repairOnlyStep.indexOf( 'git checkout --detach "$workflow_policy_sha"', ) - const currentAdmissionExecution = repairValidationStep.indexOf("validateRepairCandidateBinding") - const provenanceGuard = repairValidationStep.indexOf('if [[ -z "$merged_at"') + const currentAdmissionExecution = repairOnlyStep.indexOf("validateRepairCandidateBinding") + const provenanceGuard = repairOnlyStep.indexOf('if [[ -z "$merged_at"') const dispatchBaseGuard = repairValidationStep.indexOf( 'if [[ "$GITHUB_REF" != "refs/heads/${BASE_BRANCH}" ]]', ) @@ -407,7 +412,7 @@ test("release workflow is pinned and publishes proven assets after validation", expect(workflow).toContain("PUSH_BEFORE_SHA: ${{ github.event.before }}") expect(workflow).toContain("PUSH_FORCED: ${{ github.event.forced }}") expect(workflow).toContain('if [[ "$PUSH_FORCED" != "false" ]]') - expect(workflow.match(/candidate_parent_shas=/g)).toHaveLength(3) + expect(workflow.match(/candidate_parent_shas=/g)).toHaveLength(4) expect(publishResolutionStep).toContain('trusted_base_sha="$PUSH_BEFORE_SHA"') expect(publishResolutionStep).toContain('merged_pr_base_sha=$(jq -r .base.sha <<< "$release_pr")') expect(publishResolutionStep).toContain('reviewed_pr_head_sha=$(jq -r .head.sha <<< "$release_pr")') @@ -478,13 +483,13 @@ test("release workflow is pinned and publishes proven assets after validation", expect(candidateJob).toContain(" permissions:\n actions: read\n contents: read\n") expect(candidateJob).toContain("persist-credentials: false") expect(normalizePredicate(parsedWorkflow.jobs.compatibility.if)).toBe( - "needs.resolve.outputs.mode == 'publish'", + "needs.resolve.outputs.mode == 'publish' || needs.resolve.outputs.mode == 'resume'", ) expect(normalizePredicate(parsedWorkflow.jobs.package.if)).toBe( - "always() && needs.resolve.result == 'success' && needs.candidate.result == 'success' && (needs.resolve.outputs.mode == 'publish' || needs.resolve.outputs.mode == 'repair') && (needs.compatibility.result == 'success' || (needs.resolve.outputs.mode == 'repair' && needs.compatibility.result == 'skipped'))", + "always() && needs.resolve.result == 'success' && needs.candidate.result == 'success' && (needs.resolve.outputs.mode == 'publish' || needs.resolve.outputs.mode == 'resume' || needs.resolve.outputs.mode == 'repair') && (needs.compatibility.result == 'success' || (needs.resolve.outputs.mode == 'repair' && needs.compatibility.result == 'skipped'))", ) expect(normalizePredicate(parsedWorkflow.jobs.release.if)).toBe( - "always() && needs.resolve.result == 'success' && needs.package.result == 'success' && (needs.resolve.outputs.mode == 'publish' || needs.resolve.outputs.mode == 'repair')", + "always() && needs.resolve.result == 'success' && needs.package.result == 'success' && (needs.resolve.outputs.mode == 'publish' || needs.resolve.outputs.mode == 'resume' || needs.resolve.outputs.mode == 'repair')", ) expect(workflow.match(/--dir "\$RUNNER_TEMP\/platform-candidate"/g)).toHaveLength(2) expect(maintainJob).toContain("persist-credentials: false") @@ -506,7 +511,8 @@ test("release workflow is pinned and publishes proven assets after validation", expect(repairValidationStep).toContain('gh api "repos/${GITHUB_REPOSITORY}/pulls/${pr_number}"') expect(repairValidationStep).toContain("trusted-repair-candidate.json") expect(repairValidationStep).toContain("validateRepairCandidateBinding") - expect(repairValidationStep).not.toContain("actions/artifacts") + // Repair recovers its admission from the annotated tag, never from artifact retention. + expect(repairOnlyStep).not.toContain("actions/artifacts") expect(persistedCandidateStep).toContain('if [[ "$MODE" == "repair" ]]') expect(persistedCandidateStep).toContain("git for-each-ref --format='%(contents)'") expect(finalReleaseJob).toContain(" needs:\n - resolve\n - package\n") @@ -533,6 +539,80 @@ test("release workflow is pinned and publishes proven assets after validation", expect(workflow).not.toContain("endswith($repository)") }) +test("release workflow resumes a proven merged candidate stranded before its tag", () => { + const workflow = readFileSync(join(root, ".github", "workflows", "release.yml"), "utf8") + const parsedWorkflow = Bun.YAML.parse(workflow) as { + on: { workflow_dispatch: { inputs: { operation: { options: string[] } } } } + jobs: Record + } + const normalizePredicate = (predicate: string | undefined) => predicate?.replace(/\s+/g, " ") + const resolveStep = workflow.slice( + workflow.indexOf(" - name: Resolve unique candidate or immutable repair tag\n"), + workflow.indexOf(" - name: Persist publication candidate before proof\n"), + ) + const resumeBranch = resolveStep.slice( + resolveStep.indexOf('if [[ "$OPERATION" == "resume" ]]'), + resolveStep.indexOf('if [[ "$OPERATION" != "repair" ]]'), + ) + + expect(parsedWorkflow.on.workflow_dispatch.inputs.operation.options).toEqual([ + "maintenance", + "resume", + "repair", + ]) + expect(resolveStep).toContain('if [[ "$OPERATION" == "resume" ]]') + expect(resumeBranch).toContain("mode=resume") + + // Resume must reuse the original admission, never mint a fresh one. + expect(resumeBranch).toContain("publication-candidate-${RESUME_SHA}") + expect(resumeBranch).toContain("Resume requires the persisted publication candidate") + expect(resumeBranch).not.toContain("admitPublicationCandidate") + expect(resumeBranch).toContain("validateResumeCandidateBinding") + + // The target tag must still be absent; a tagged candidate belongs to repair. + expect(resumeBranch).toContain("already exists; use manual incomplete-publication repair") + + // Resume reaches publication through the same proof and protected environment. + expect(normalizePredicate(parsedWorkflow.jobs.candidate.if)).toContain( + "needs.resolve.outputs.mode == 'resume'", + ) + expect(normalizePredicate(parsedWorkflow.jobs.compatibility.if)).toContain( + "needs.resolve.outputs.mode == 'resume'", + ) + expect(normalizePredicate(parsedWorkflow.jobs.package.if)).toContain( + "needs.resolve.outputs.mode == 'resume'", + ) + expect(normalizePredicate(parsedWorkflow.jobs.release.if)).toContain( + "needs.resolve.outputs.mode == 'resume'", + ) + + // Resume must never create an ad hoc tag outside the admitted record. + expect(workflow).not.toContain('git tag -a "$RELEASE_TAG" "$RESUME_SHA"') +}) + +test("release maintenance fails loudly with the exact resume command when a candidate is stranded", () => { + const workflow = readFileSync(join(root, ".github", "workflows", "release.yml"), "utf8") + const maintainJob = workflow.slice( + workflow.indexOf("\n maintain:\n"), + workflow.indexOf("\n candidate:\n"), + ) + const strandedStep = maintainJob.slice( + maintainJob.indexOf(" - name: Detect a merged release candidate stranded before its tag\n"), + ) + const releasePleaseStep = maintainJob.indexOf("googleapis/release-please-action") + + expect(strandedStep).not.toBe("") + // Detection must precede Release Please, whose own abort message is silent. + expect(maintainJob.indexOf("Detect a merged release candidate stranded before its tag")).toBeLessThan( + releasePleaseStep, + ) + expect(strandedStep).toContain("autorelease:%20pending") + expect(strandedStep).toContain("git/ref/tags/") + expect(strandedStep).toContain("-f operation=resume") + expect(strandedStep).toContain("-f candidate_sha=") + expect(strandedStep).toContain("exit 1") +}) + test("release workflow fails closed when the requested terminal operation is skipped", () => { const workflow = Bun.YAML.parse( readFileSync(join(root, ".github", "workflows", "release.yml"), "utf8"), @@ -1376,3 +1456,69 @@ test("manual repair CLI fails closed without a persisted publication candidate", expect(result.exitCode).toBe(1) expect(result.stderr.toString()).toContain("publication candidate record is required") }) + +function resumeInput(overrides: Record = {}) { + return { + candidate: admitPublicationCandidate(admissionInput()), + repository: "myagentdojo/agent-plugin-template", + expectedBaseBranch: "main", + expectedAutomationIdentities: ["github-actions[bot]"], + trustedCandidate: releasePullRequest(), + ...candidateTopology(), + candidateSha: "a".repeat(40), + manifestVersion: "0.1.0", + tagExists: false, + ...overrides, + } +} + +test("resume binds a stranded candidate to its original untagged admission", () => { + expect(validateResumeCandidateBinding(resumeInput())).toEqual({ + tag: "v0.1.0", + commit: "a".repeat(40), + version: "0.1.0", + }) +}) + +test("resume refuses a candidate whose immutable tag already exists", () => { + expect(() => validateResumeCandidateBinding(resumeInput({ tagExists: true }))).toThrow( + "must be absent", + ) +}) + +test("resume refuses a persisted record rebound to another candidate", () => { + expect(() => + validateResumeCandidateBinding( + resumeInput({ + trustedCandidate: releasePullRequest({ mergeCommit: "e".repeat(40) }), + candidateSha: "e".repeat(40), + }), + ), + ).toThrow("rebound to another release identity") +}) + +test("resume refuses a candidate that is not a Release Please pull request", () => { + expect(() => + validateResumeCandidateBinding( + resumeInput({ trustedCandidate: releasePullRequest({ automationIdentity: "attacker" }) }), + ), + ).toThrow("unexpected automation identity") +}) + +test("resume refuses a candidate whose manifest version moved after admission", () => { + expect(() => validateResumeCandidateBinding(resumeInput({ manifestVersion: "0.2.0" }))).toThrow( + "rebound to another release identity", + ) +}) + +test("resume refuses a malformed persisted record", () => { + expect(() => validateResumeCandidateBinding(resumeInput({ candidate: { pullRequest: 42 } }))).toThrow( + "publication candidate record shape is invalid", + ) +}) + +test("resume refuses a candidate whose merge commit is not the resumed SHA", () => { + expect(() => validateResumeCandidateBinding(resumeInput({ candidateSha: "f".repeat(40) }))).toThrow( + "merge commit does not equal", + ) +}) diff --git a/scripts/release-validate.ts b/scripts/release-validate.ts index 536a007..15f9e20 100644 --- a/scripts/release-validate.ts +++ b/scripts/release-validate.ts @@ -468,6 +468,58 @@ export function validateRepairBinding( return { tag: input.tag, commit: input.checkoutSha, version: input.manifestVersion } } +/** GitHub-derived facts required to resume a merged candidate stranded before its tag. */ +export interface ResumeCandidateBindingInput extends CandidateTopology { + /** Untrusted publication record recovered from the persisted pre-proof artifact. */ + candidate: unknown + /** Current GitHub owner/repository identity. */ + repository: string + /** Trusted release base branch policy. */ + expectedBaseBranch: string + /** Trusted automation logins allowed to own the release pull request. */ + expectedAutomationIdentities: string[] + /** Pull-request and projection facts re-derived from GitHub for this resume. */ + trustedCandidate: ReleasePullRequestCandidate + /** Merged candidate commit being resumed. */ + candidateSha: string + /** Version read from the candidate manifest. */ + manifestVersion: string + /** Whether the candidate version tag already exists remotely. */ + tagExists: boolean +} + +/** + * Bind a pre-tag resume to the admission its original run already persisted. + * + * Resume never mints a new admission. It replays the original record against + * freshly derived GitHub facts, so a candidate that was never admitted, was + * rebound, or has since been tagged cannot reach protected publication. + * + * @param input - Persisted record plus GitHub-derived publication facts + * @returns The resumed tag, commit, and version + * @throws {Error} When the candidate is missing, rebound, mismatched, or already tagged + */ +export function validateResumeCandidateBinding( + input: ResumeCandidateBindingInput, +): { tag: string; commit: string; version: string } { + const candidate = parsePublicationCandidateRecord(input.candidate) + admitCandidate({ + repository: input.repository, + expectedBaseBranch: input.expectedBaseBranch, + expectedAutomationIdentities: input.expectedAutomationIdentities, + githubSha: input.candidateSha, + candidateParentShas: input.candidateParentShas, + trustedBaseSha: input.trustedBaseSha, + mergedPrBaseSha: input.mergedPrBaseSha, + reviewedPrHeadSha: input.reviewedPrHeadSha, + manifestVersion: input.manifestVersion, + tagExists: input.tagExists, + candidates: [input.trustedCandidate], + priorRecord: candidate, + }, true) + return { tag: candidate.tag, commit: candidate.mergeCommit, version: candidate.version } +} + /** Bind a manual repair to its original immutable publication admission record. */ export function validateRepairCandidateBinding( input: RepairCandidateBindingInput, @@ -621,6 +673,12 @@ function validateRepository(repositoryRoot: string) { "reviewed_pr_head_sha", "trusted_base_sha", "admitPublicationCandidate", + "validateResumeCandidateBinding", + 'if [[ "$OPERATION" == "resume" ]]', + "publication-candidate-${RESUME_SHA}", + "Resume requires the persisted publication candidate", + "Detect a merged release candidate stranded before its tag", + "-f operation=resume", "scripts/release-projection.ts", "bun run prove:all", "git diff --exit-code -- plugin/", diff --git a/scripts/repository-readiness.test.ts b/scripts/repository-readiness.test.ts index 1d8b793..cdf9e79 100644 --- a/scripts/repository-readiness.test.ts +++ b/scripts/repository-readiness.test.ts @@ -84,6 +84,7 @@ test("reports ready when every publication safeguard is proven", () => { classifyHostedCanaryConfiguration( { name: "hosted-canary-qualification" }, { secrets: REQUIRED_HOSTED_CANARY_SECRETS.map((name) => ({ name })) }, + { registeredKeyFingerprints: ["SHA256:abcdef"] }, ), classifyRequiredStatusChecks({ strict: true, contexts: REQUIRED_STATUS_CHECKS }), classifyWorkflowAdminPermissions([ @@ -328,7 +329,11 @@ describe("hosted-canary qualification configuration", () => { const secrets = { secrets: REQUIRED_HOSTED_CANARY_SECRETS.map((name) => ({ name })) } test("accepts the environment with all required secret names", () => { - expect(classifyHostedCanaryConfiguration(environment, secrets)).toMatchObject({ + expect( + classifyHostedCanaryConfiguration(environment, secrets, { + registeredKeyFingerprints: ["SHA256:abcdef"], + }), + ).toMatchObject({ name: "hosted-canary-configuration", status: "ready", repair: "", @@ -345,6 +350,30 @@ describe("hosted-canary qualification configuration", () => { expect(JSON.stringify(check)).not.toContain("must-not-leak") }) + test("reports a stranded private key whose registered public half was deleted", () => { + const check = classifyHostedCanaryConfiguration(environment, secrets, { + registeredKeyFingerprints: [], + }) + + expect(check).toMatchObject({ status: "missing" }) + expect(check.detail).toContain("public half") + expect(check.repair).toContain("rotate") + }) + + test("accepts a secret whose registered public half is still present", () => { + expect( + classifyHostedCanaryConfiguration(environment, secrets, { + registeredKeyFingerprints: ["SHA256:abcdef"], + }), + ).toMatchObject({ status: "ready", repair: "" }) + }) + + test("fails closed when the registered public half cannot be read", () => { + expect(classifyHostedCanaryConfiguration(environment, secrets, undefined)).toMatchObject({ + status: "unavailable", + }) + }) + test.each([ ["environment metadata", undefined, secrets], ["wrong environment", { name: "release" }, secrets], diff --git a/scripts/repository-readiness.ts b/scripts/repository-readiness.ts index 4802314..b3b0d0f 100644 --- a/scripts/repository-readiness.ts +++ b/scripts/repository-readiness.ts @@ -23,6 +23,8 @@ const releaseEnvironmentRepair = "Settings > Environments > release > Deployment protection rules: enable Required reviewers and add at least one reviewer" const hostedCanaryRepair = "Settings > Environments > hosted-canary-qualification: create the environment and add secrets CANARY_GH_TOKEN, CANARY_SSH_KNOWN_HOSTS, and CANARY_SSH_PRIVATE_KEY" +const hostedCanaryKeyRotationRepair = + "rotate the canary key: generate a dedicated purpose key, register its public half first, then update the CANARY_SSH_PRIVATE_KEY and CANARY_SSH_KNOWN_HOSTS pair" const HOSTED_CANARY_ENVIRONMENT = "hosted-canary-qualification" export const REQUIRED_HOSTED_CANARY_SECRETS = [ @@ -613,10 +615,23 @@ export function classifyReleaseEnvironmentProtection(environment: unknown): Read ) } -/** Verify hosted-canary qualification authority from environment and secret names only. */ +/** Registered public half of the canary admission credential. */ +export interface CanaryKeyRegistration { + /** Fingerprints GitHub still lists for the canary key purpose. */ + registeredKeyFingerprints: string[] +} + +/** + * Verify hosted-canary qualification authority, including credential lifecycle. + * + * A configured secret name proves only that a value exists. When the registered + * public half has been deleted, SSH admission fails while every name-based check + * stays green, so the private key is verified to still have a counterpart. + */ export function classifyHostedCanaryConfiguration( environment: unknown, secretsResponse: unknown, + keyRegistration?: CanaryKeyRegistration, ): ReadinessCheck { if (!isRecord(environment) || environment.name !== HOSTED_CANARY_ENVIRONMENT) { return { @@ -652,16 +667,36 @@ export function classifyHostedCanaryConfiguration( ), ) const absent = REQUIRED_HOSTED_CANARY_SECRETS.filter((name) => !configured.has(name)) - return absent.length === 0 - ? ready( - "hosted-canary-configuration", - `${HOSTED_CANARY_ENVIRONMENT} and all required secret names are configured`, - ) - : missing( - "hosted-canary-configuration", - `Hosted-canary qualification is missing environment secret${absent.length === 1 ? "" : "s"}: ${absent.join(", ")}`, - hostedCanaryRepair, - ) + if (absent.length > 0) { + return missing( + "hosted-canary-configuration", + `Hosted-canary qualification is missing environment secret${absent.length === 1 ? "" : "s"}: ${absent.join(", ")}`, + hostedCanaryRepair, + ) + } + if ( + !isRecord(keyRegistration) || + !Array.isArray(keyRegistration.registeredKeyFingerprints) || + keyRegistration.registeredKeyFingerprints.some((fingerprint) => typeof fingerprint !== "string") + ) { + return { + name: "hosted-canary-configuration", + status: "unavailable", + detail: "GitHub returned unreadable canary key registration; credential lifecycle is unproven", + repair: hostedCanaryKeyRotationRepair, + } + } + if (keyRegistration.registeredKeyFingerprints.length === 0) { + return missing( + "hosted-canary-configuration", + `${HOSTED_CANARY_ENVIRONMENT} retains CANARY_SSH_PRIVATE_KEY with no registered public half; SSH admission will fail`, + hostedCanaryKeyRotationRepair, + ) + } + return ready( + "hosted-canary-configuration", + `${HOSTED_CANARY_ENVIRONMENT}, all required secret names, and a registered public half are configured`, + ) } /** @@ -1020,7 +1055,21 @@ function checkHostedCanaryConfiguration(repository: string): ReadinessCheck { if (!secrets.ok) { return apiFailure("hosted-canary-configuration", secrets, hostedCanaryRepair) } - return classifyHostedCanaryConfiguration(environment.data, secrets.data) + // A secret name proves a value exists, not that its public half survives. + const deployKeys = readApi(`repos/${repository}/keys`) + if (!deployKeys.ok) { + return apiFailure("hosted-canary-configuration", deployKeys, hostedCanaryKeyRotationRepair) + } + const registeredKeyFingerprints = Array.isArray(deployKeys.data) + ? deployKeys.data.flatMap((entry) => + isRecord(entry) && typeof entry.key === "string" ? [entry.key] : [], + ) + : undefined + return classifyHostedCanaryConfiguration( + environment.data, + secrets.data, + registeredKeyFingerprints ? { registeredKeyFingerprints } : undefined, + ) } function localWorkflows(): ReadinessCheck { From 5a44456a6b478d7eee6e0f5277883454120d0486 Mon Sep 17 00:00:00 2001 From: Nathan Vale Date: Fri, 14 Aug 2026 09:18:37 +1000 Subject: [PATCH 2/7] Address PR review feedback (#43) Bind canary readiness to the exact registered deploy key. A non-empty deploy-key list only proved that some key existed, so an unrelated key could report ready after the canary key was deleted -- the exact failure this check exists to catch. Readiness now matches the deploy-key title against a new CANARY_SSH_KEY_TITLE variable, paginates the deploy-key list, and fails closed on a truncated or malformed response. Bound by title rather than by deriving the public half from CANARY_SSH_PRIVATE_KEY: readiness must never read secret values (docs/release-setup.md), and derivation would breach that boundary. Replace two assertions that could not fail: - The ad hoc tag guard matched a string the workflow never contains, since the tag step uses $CANDIDATE_SHA. It now asserts the resume branch runs no git tag/push and that exactly one tag -a call exists. - The stranded-step guard compared slice(-1) against "", which returns the last character, never "". It now asserts the index directly. --- docs/release-setup.md | 2 +- scripts/release-validate.test.ts | 12 +++-- scripts/repository-readiness.test.ts | 55 +++++++++++++++++---- scripts/repository-readiness.ts | 72 ++++++++++++++++++++++------ 4 files changed, 114 insertions(+), 27 deletions(-) diff --git a/docs/release-setup.md b/docs/release-setup.md index 602b322..b6544b1 100644 --- a/docs/release-setup.md +++ b/docs/release-setup.md @@ -7,7 +7,7 @@ Use this runbook once per repository before enabling Release Please maintenance 3. Protect `main` in two places. In **Settings → Branches → Branch protection rules**, require `Conventional Commit title`, `Release impact`, `Hosted public and private Git canaries`, all four `Compatibility` checks, and `Deterministic package`; readiness reads that classic branch-protection endpoint. In an active, no-bypass `main` ruleset, enable **Require a pull request before merging** and **Block force pushes**. 4. Open **Settings → Rules → Rulesets**. Create an active tag ruleset for `v*` that restricts tag deletion and updates with no bypass actors. 5. Open **Settings → Environments**. Create `release` and configure required reviewers for publication and same-tag asset replacement. -6. Create the public and private canary repositories named by `plugin.config.json`, then create the `hosted-canary-qualification` environment. Add `CANARY_GH_TOKEN`: a fine-grained token for the exact configured `canary.actor`, scoped only to both canary repositories, with Contents read/write, Actions read, and metadata read. Add `CANARY_SSH_PRIVATE_KEY` and `CANARY_SSH_KNOWN_HOSTS` for the same canary identity. Qualification uses the token-backed GitHub API identity and SSH Git identity, and limits writes to create-only immutable candidate refs. Keeping Git transport on SSH allows candidates containing workflow files without broadening the API token. +6. Create the public and private canary repositories named by `plugin.config.json`, then create the `hosted-canary-qualification` environment. Add `CANARY_GH_TOKEN`: a fine-grained token for the exact configured `canary.actor`, scoped only to both canary repositories, with Contents read/write, Actions read, and metadata read. Add `CANARY_SSH_PRIVATE_KEY` and `CANARY_SSH_KNOWN_HOSTS` for the same canary identity. Register that key's public half as a repository deploy key, and set the repository variable `CANARY_SSH_KEY_TITLE` to that deploy key's exact title so readiness can prove the registered public half still exists without reading the private key. Qualification uses the token-backed GitHub API identity and SSH Git identity, and limits writes to create-only immutable candidate refs. Keeping Git transport on SSH allows candidates containing workflow files without broadening the API token. Keep secret values out of repository files, logs, and readiness output. `bun run readiness` proves repository configuration and required environment, variable, and secret names; it never reads secret values or proves real credential access. Hosted qualification binds the token-backed GitHub API identity and SSH Git identity and proves their real access. 7. Create a fine-grained token with only repository contents, pull requests, and issues write access. Store it as the GitHub Actions repository secret `RELEASE_PLEASE_TOKEN`. Record its expiry and rotate the stored token before expiry or revocation. diff --git a/scripts/release-validate.test.ts b/scripts/release-validate.test.ts index 1b56305..718a38f 100644 --- a/scripts/release-validate.test.ts +++ b/scripts/release-validate.test.ts @@ -587,7 +587,10 @@ test("release workflow resumes a proven merged candidate stranded before its tag ) // Resume must never create an ad hoc tag outside the admitted record. - expect(workflow).not.toContain('git tag -a "$RELEASE_TAG" "$RESUME_SHA"') + expect(resumeBranch).not.toContain("git tag") + expect(resumeBranch).not.toContain("git push") + expect(workflow.match(/tag -a "\$RELEASE_TAG"/g)).toHaveLength(1) + expect(workflow).toContain('tag -a "$RELEASE_TAG" "$CANDIDATE_SHA" -F persisted-candidate.json') }) test("release maintenance fails loudly with the exact resume command when a candidate is stranded", () => { @@ -596,12 +599,13 @@ test("release maintenance fails loudly with the exact resume command when a cand workflow.indexOf("\n maintain:\n"), workflow.indexOf("\n candidate:\n"), ) - const strandedStep = maintainJob.slice( - maintainJob.indexOf(" - name: Detect a merged release candidate stranded before its tag\n"), + const strandedStepStart = maintainJob.indexOf( + " - name: Detect a merged release candidate stranded before its tag\n", ) + expect(strandedStepStart).toBeGreaterThan(-1) + const strandedStep = maintainJob.slice(strandedStepStart) const releasePleaseStep = maintainJob.indexOf("googleapis/release-please-action") - expect(strandedStep).not.toBe("") // Detection must precede Release Please, whose own abort message is silent. expect(maintainJob.indexOf("Detect a merged release candidate stranded before its tag")).toBeLessThan( releasePleaseStep, diff --git a/scripts/repository-readiness.test.ts b/scripts/repository-readiness.test.ts index cdf9e79..4f9ee3a 100644 --- a/scripts/repository-readiness.test.ts +++ b/scripts/repository-readiness.test.ts @@ -84,7 +84,11 @@ test("reports ready when every publication safeguard is proven", () => { classifyHostedCanaryConfiguration( { name: "hosted-canary-qualification" }, { secrets: REQUIRED_HOSTED_CANARY_SECRETS.map((name) => ({ name })) }, - { registeredKeyFingerprints: ["SHA256:abcdef"] }, + { + registeredKeyTitles: ["hosted-canary-qualification"], + expectedKeyTitle: "hosted-canary-qualification", + registrationComplete: true, + }, ), classifyRequiredStatusChecks({ strict: true, contexts: REQUIRED_STATUS_CHECKS }), classifyWorkflowAdminPermissions([ @@ -327,12 +331,15 @@ describe("release environment required reviewers", () => { describe("hosted-canary qualification configuration", () => { const environment = { name: "hosted-canary-qualification" } const secrets = { secrets: REQUIRED_HOSTED_CANARY_SECRETS.map((name) => ({ name })) } + const canaryKey = { + registeredKeyTitles: ["hosted-canary-qualification"], + expectedKeyTitle: "hosted-canary-qualification", + registrationComplete: true, + } test("accepts the environment with all required secret names", () => { expect( - classifyHostedCanaryConfiguration(environment, secrets, { - registeredKeyFingerprints: ["SHA256:abcdef"], - }), + classifyHostedCanaryConfiguration(environment, secrets, canaryKey), ).toMatchObject({ name: "hosted-canary-configuration", status: "ready", @@ -352,22 +359,54 @@ describe("hosted-canary qualification configuration", () => { test("reports a stranded private key whose registered public half was deleted", () => { const check = classifyHostedCanaryConfiguration(environment, secrets, { - registeredKeyFingerprints: [], + ...canaryKey, + registeredKeyTitles: [], }) expect(check).toMatchObject({ status: "missing" }) - expect(check.detail).toContain("public half") + expect(check.detail).toContain("hosted-canary-qualification") expect(check.repair).toContain("rotate") }) - test("accepts a secret whose registered public half is still present", () => { + test("rejects an unrelated deploy key standing in for the canary key", () => { + const check = classifyHostedCanaryConfiguration(environment, secrets, { + ...canaryKey, + registeredKeyTitles: ["some-other-service", "backup-deploy-key"], + }) + + expect(check).toMatchObject({ status: "missing" }) + expect(check.detail).toContain("hosted-canary-qualification") + }) + + test("accepts only the deploy key titled by CANARY_SSH_KEY_TITLE", () => { expect( classifyHostedCanaryConfiguration(environment, secrets, { - registeredKeyFingerprints: ["SHA256:abcdef"], + ...canaryKey, + registeredKeyTitles: ["some-other-service", "hosted-canary-qualification"], }), ).toMatchObject({ status: "ready", repair: "" }) }) + test("reports a missing CANARY_SSH_KEY_TITLE binding variable", () => { + const check = classifyHostedCanaryConfiguration(environment, secrets, { + ...canaryKey, + expectedKeyTitle: "", + }) + + expect(check).toMatchObject({ status: "missing" }) + expect(check.detail).toContain("CANARY_SSH_KEY_TITLE") + }) + + test("fails closed when the deploy-key list was truncated", () => { + expect( + classifyHostedCanaryConfiguration(environment, secrets, { + ...canaryKey, + registeredKeyTitles: [], + registrationComplete: false, + }), + ).toMatchObject({ status: "unavailable" }) + }) + test("fails closed when the registered public half cannot be read", () => { expect(classifyHostedCanaryConfiguration(environment, secrets, undefined)).toMatchObject({ status: "unavailable", diff --git a/scripts/repository-readiness.ts b/scripts/repository-readiness.ts index b3b0d0f..c939db0 100644 --- a/scripts/repository-readiness.ts +++ b/scripts/repository-readiness.ts @@ -24,7 +24,9 @@ const releaseEnvironmentRepair = const hostedCanaryRepair = "Settings > Environments > hosted-canary-qualification: create the environment and add secrets CANARY_GH_TOKEN, CANARY_SSH_KNOWN_HOSTS, and CANARY_SSH_PRIVATE_KEY" const hostedCanaryKeyRotationRepair = - "rotate the canary key: generate a dedicated purpose key, register its public half first, then update the CANARY_SSH_PRIVATE_KEY and CANARY_SSH_KNOWN_HOSTS pair" + "rotate the canary key: generate a dedicated purpose key, register its public half first under the CANARY_SSH_KEY_TITLE title, then update the CANARY_SSH_PRIVATE_KEY and CANARY_SSH_KNOWN_HOSTS pair" +const hostedCanaryKeyTitleRepair = + "Settings > Secrets and variables > Actions: add variable CANARY_SSH_KEY_TITLE naming the deploy key that carries the canary public half" const HOSTED_CANARY_ENVIRONMENT = "hosted-canary-qualification" export const REQUIRED_HOSTED_CANARY_SECRETS = [ @@ -617,8 +619,12 @@ export function classifyReleaseEnvironmentProtection(environment: unknown): Read /** Registered public half of the canary admission credential. */ export interface CanaryKeyRegistration { - /** Fingerprints GitHub still lists for the canary key purpose. */ - registeredKeyFingerprints: string[] + /** Deploy-key titles GitHub still lists, across every page. */ + registeredKeyTitles: string[] + /** Title recorded by CANARY_SSH_KEY_TITLE that identifies the canary key. */ + expectedKeyTitle: string + /** Whether every deploy-key page was read; a truncated list cannot prove absence. */ + registrationComplete: boolean } /** @@ -676,8 +682,10 @@ export function classifyHostedCanaryConfiguration( } if ( !isRecord(keyRegistration) || - !Array.isArray(keyRegistration.registeredKeyFingerprints) || - keyRegistration.registeredKeyFingerprints.some((fingerprint) => typeof fingerprint !== "string") + !Array.isArray(keyRegistration.registeredKeyTitles) || + keyRegistration.registeredKeyTitles.some((title) => typeof title !== "string") || + typeof keyRegistration.registrationComplete !== "boolean" || + typeof keyRegistration.expectedKeyTitle !== "string" ) { return { name: "hosted-canary-configuration", @@ -686,16 +694,34 @@ export function classifyHostedCanaryConfiguration( repair: hostedCanaryKeyRotationRepair, } } - if (keyRegistration.registeredKeyFingerprints.length === 0) { + // A truncated key list cannot prove the canary key is absent. + if (!keyRegistration.registrationComplete) { + return { + name: "hosted-canary-configuration", + status: "unavailable", + detail: "GitHub returned an incomplete deploy-key list; credential lifecycle is unproven", + repair: hostedCanaryKeyRotationRepair, + } + } + if (keyRegistration.expectedKeyTitle.length === 0) { return missing( "hosted-canary-configuration", - `${HOSTED_CANARY_ENVIRONMENT} retains CANARY_SSH_PRIVATE_KEY with no registered public half; SSH admission will fail`, + "Hosted-canary qualification is missing variable CANARY_SSH_KEY_TITLE; the registered public half cannot be bound to CANARY_SSH_PRIVATE_KEY", + hostedCanaryKeyTitleRepair, + ) + } + // Any deploy key proves only that some key exists. Bind the exact canary key, + // or a rotation that deletes it stays green while SSH admission fails. + if (!keyRegistration.registeredKeyTitles.includes(keyRegistration.expectedKeyTitle)) { + return missing( + "hosted-canary-configuration", + `${HOSTED_CANARY_ENVIRONMENT} retains CANARY_SSH_PRIVATE_KEY, but no registered deploy key is titled ${keyRegistration.expectedKeyTitle}; SSH admission will fail`, hostedCanaryKeyRotationRepair, ) } return ready( "hosted-canary-configuration", - `${HOSTED_CANARY_ENVIRONMENT}, all required secret names, and a registered public half are configured`, + `${HOSTED_CANARY_ENVIRONMENT}, all required secret names, and the registered ${keyRegistration.expectedKeyTitle} public half are configured`, ) } @@ -1056,19 +1082,37 @@ function checkHostedCanaryConfiguration(repository: string): ReadinessCheck { return apiFailure("hosted-canary-configuration", secrets, hostedCanaryRepair) } // A secret name proves a value exists, not that its public half survives. - const deployKeys = readApi(`repos/${repository}/keys`) + // Paginate: the canary key can sit past the first page of deploy keys. + const deployKeys = readApi(`repos/${repository}/keys?per_page=100`, true) if (!deployKeys.ok) { return apiFailure("hosted-canary-configuration", deployKeys, hostedCanaryKeyRotationRepair) } - const registeredKeyFingerprints = Array.isArray(deployKeys.data) - ? deployKeys.data.flatMap((entry) => - isRecord(entry) && typeof entry.key === "string" ? [entry.key] : [], - ) + const variables = readApi(`repos/${repository}/actions/variables?per_page=100`, true) + if (!variables.ok) { + return apiFailure("hosted-canary-configuration", variables, hostedCanaryKeyTitleRepair) + } + const keyPages = Array.isArray(deployKeys.data) ? deployKeys.data : undefined + // --slurp yields one array per page; a malformed entry must fail closed, never be dropped. + const registeredKeyTitles = keyPages + ?.flatMap((page) => (Array.isArray(page) ? page : [undefined])) + .map((entry) => (isRecord(entry) && typeof entry.title === "string" ? entry.title : undefined)) + const registrationComplete = + registeredKeyTitles !== undefined && registeredKeyTitles.every((title) => title !== undefined) + const expectedKeyTitle = Array.isArray(variables.data) + ? variables.data + .flatMap((page) => (Array.isArray(page?.variables) ? page.variables : [])) + .find((entry) => isRecord(entry) && entry.name === "CANARY_SSH_KEY_TITLE")?.value : undefined return classifyHostedCanaryConfiguration( environment.data, secrets.data, - registeredKeyFingerprints ? { registeredKeyFingerprints } : undefined, + registeredKeyTitles + ? { + registeredKeyTitles: registeredKeyTitles.filter((title) => title !== undefined), + expectedKeyTitle: typeof expectedKeyTitle === "string" ? expectedKeyTitle : "", + registrationComplete, + } + : undefined, ) } From bef052b6db2b5b6abf0653360dddf02e4ce7d296 Mon Sep 17 00:00:00 2001 From: Nathan Vale Date: Fri, 14 Aug 2026 09:28:55 +1000 Subject: [PATCH 3/7] Address Codex review feedback (#43) Read the canary credential from the account that owns it. The canary SSH key belongs to plugin.config.json's canary.actor and authenticates against the separate canary repositories, but the check read this template repo's deploy keys -- a different object that never carries it. Against the live repository the actor has 3 registered keys and this repo has 0 deploy keys, so the check reported missing on a correctly configured setup while an unrelated deploy key could still have reported ready. Readiness now reads /users/{canary.actor}/keys and matches the public half against a CANARY_SSH_PUBLIC_KEY variable, comparing key type and material so a differing comment or spacing does not cause a false mismatch. A public key is not a secret, so readiness still never reads secret values. A regression test pins the endpoint, since the defect was the endpoint itself. Fail closed when stranded-candidate discovery fails. `|| true` turned an outage or authorization change into an empty candidate list, letting maintenance proceed and stop silently on an untagged merged PR -- exactly the failure this detection exists to prevent. The API failure now propagates, and the step assertion is bounded so it cannot pass on an unrelated later step. --- .github/workflows/release.yml | 4 +- docs/release-setup.md | 2 +- scripts/release-validate.test.ts | 11 +++- scripts/repository-readiness.test.ts | 52 +++++++++++----- scripts/repository-readiness.ts | 93 +++++++++++++++++----------- 5 files changed, 108 insertions(+), 54 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index e49252a..9b6edcc 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -548,9 +548,11 @@ jobs: GH_TOKEN: ${{ github.token }} run: | set -euo pipefail + # Let this API failure propagate. Swallowing it would read as + # "nothing stranded" and let maintenance stop silently again. stranded=$(gh api --paginate \ "repos/${GITHUB_REPOSITORY}/issues?state=closed&labels=autorelease:%20pending&per_page=100" \ - --jq '[.[] | select(.pull_request != null) | .number] | .[]' || true) + --jq '[.[] | select(.pull_request != null) | .number] | .[]') for pr_number in $stranded; do release_pr=$(gh api "repos/${GITHUB_REPOSITORY}/pulls/${pr_number}") merged_at=$(jq -r '.merged_at // empty' <<< "$release_pr") diff --git a/docs/release-setup.md b/docs/release-setup.md index b6544b1..aae1bec 100644 --- a/docs/release-setup.md +++ b/docs/release-setup.md @@ -7,7 +7,7 @@ Use this runbook once per repository before enabling Release Please maintenance 3. Protect `main` in two places. In **Settings → Branches → Branch protection rules**, require `Conventional Commit title`, `Release impact`, `Hosted public and private Git canaries`, all four `Compatibility` checks, and `Deterministic package`; readiness reads that classic branch-protection endpoint. In an active, no-bypass `main` ruleset, enable **Require a pull request before merging** and **Block force pushes**. 4. Open **Settings → Rules → Rulesets**. Create an active tag ruleset for `v*` that restricts tag deletion and updates with no bypass actors. 5. Open **Settings → Environments**. Create `release` and configure required reviewers for publication and same-tag asset replacement. -6. Create the public and private canary repositories named by `plugin.config.json`, then create the `hosted-canary-qualification` environment. Add `CANARY_GH_TOKEN`: a fine-grained token for the exact configured `canary.actor`, scoped only to both canary repositories, with Contents read/write, Actions read, and metadata read. Add `CANARY_SSH_PRIVATE_KEY` and `CANARY_SSH_KNOWN_HOSTS` for the same canary identity. Register that key's public half as a repository deploy key, and set the repository variable `CANARY_SSH_KEY_TITLE` to that deploy key's exact title so readiness can prove the registered public half still exists without reading the private key. Qualification uses the token-backed GitHub API identity and SSH Git identity, and limits writes to create-only immutable candidate refs. Keeping Git transport on SSH allows candidates containing workflow files without broadening the API token. +6. Create the public and private canary repositories named by `plugin.config.json`, then create the `hosted-canary-qualification` environment. Add `CANARY_GH_TOKEN`: a fine-grained token for the exact configured `canary.actor`, scoped only to both canary repositories, with Contents read/write, Actions read, and metadata read. Add `CANARY_SSH_PRIVATE_KEY` and `CANARY_SSH_KNOWN_HOSTS` for the same canary identity. Register that key's public half on the `canary.actor` account, and set the repository variable `CANARY_SSH_PUBLIC_KEY` to that same public half so readiness can prove the registered key still exists without reading the private key. A public key is not a secret; readiness compares it against the actor's registered keys, never against this repository's deploy keys, which carry a different credential. Qualification uses the token-backed GitHub API identity and SSH Git identity, and limits writes to create-only immutable candidate refs. Keeping Git transport on SSH allows candidates containing workflow files without broadening the API token. Keep secret values out of repository files, logs, and readiness output. `bun run readiness` proves repository configuration and required environment, variable, and secret names; it never reads secret values or proves real credential access. Hosted qualification binds the token-backed GitHub API identity and SSH Git identity and proves their real access. 7. Create a fine-grained token with only repository contents, pull requests, and issues write access. Store it as the GitHub Actions repository secret `RELEASE_PLEASE_TOKEN`. Record its expiry and rotate the stored token before expiry or revocation. diff --git a/scripts/release-validate.test.ts b/scripts/release-validate.test.ts index 718a38f..81069a6 100644 --- a/scripts/release-validate.test.ts +++ b/scripts/release-validate.test.ts @@ -603,8 +603,13 @@ test("release maintenance fails loudly with the exact resume command when a cand " - name: Detect a merged release candidate stranded before its tag\n", ) expect(strandedStepStart).toBeGreaterThan(-1) - const strandedStep = maintainJob.slice(strandedStepStart) const releasePleaseStep = maintainJob.indexOf("googleapis/release-please-action") + // Bound the step so assertions cannot pass on unrelated later steps. + const strandedStep = maintainJob.slice( + strandedStepStart, + maintainJob.indexOf(" # skip-github-release is set", strandedStepStart), + ) + expect(strandedStep).not.toBe("") // Detection must precede Release Please, whose own abort message is silent. expect(maintainJob.indexOf("Detect a merged release candidate stranded before its tag")).toBeLessThan( @@ -615,6 +620,10 @@ test("release maintenance fails loudly with the exact resume command when a cand expect(strandedStep).toContain("-f operation=resume") expect(strandedStep).toContain("-f candidate_sha=") expect(strandedStep).toContain("exit 1") + // A swallowed API failure would read as "nothing stranded" and let + // maintenance stop silently again, defeating the detection entirely. + expect(strandedStep).toContain("set -euo pipefail") + expect(strandedStep).not.toContain("|| true") }) test("release workflow fails closed when the requested terminal operation is skipped", () => { diff --git a/scripts/repository-readiness.test.ts b/scripts/repository-readiness.test.ts index 4f9ee3a..258846c 100644 --- a/scripts/repository-readiness.test.ts +++ b/scripts/repository-readiness.test.ts @@ -85,8 +85,9 @@ test("reports ready when every publication safeguard is proven", () => { { name: "hosted-canary-qualification" }, { secrets: REQUIRED_HOSTED_CANARY_SECRETS.map((name) => ({ name })) }, { - registeredKeyTitles: ["hosted-canary-qualification"], - expectedKeyTitle: "hosted-canary-qualification", + registeredPublicKeys: ["ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAICanary"], + expectedPublicKey: "ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAICanary", + actor: "myagentdojo", registrationComplete: true, }, ), @@ -331,9 +332,12 @@ describe("release environment required reviewers", () => { describe("hosted-canary qualification configuration", () => { const environment = { name: "hosted-canary-qualification" } const secrets = { secrets: REQUIRED_HOSTED_CANARY_SECRETS.map((name) => ({ name })) } + const canaryPublicKey = "ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAICanary" + const otherPublicKey = "ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIUnrelated" const canaryKey = { - registeredKeyTitles: ["hosted-canary-qualification"], - expectedKeyTitle: "hosted-canary-qualification", + registeredPublicKeys: [canaryPublicKey], + expectedPublicKey: canaryPublicKey, + actor: "myagentdojo", registrationComplete: true, } @@ -360,53 +364,69 @@ describe("hosted-canary qualification configuration", () => { test("reports a stranded private key whose registered public half was deleted", () => { const check = classifyHostedCanaryConfiguration(environment, secrets, { ...canaryKey, - registeredKeyTitles: [], + registeredPublicKeys: [], }) expect(check).toMatchObject({ status: "missing" }) - expect(check.detail).toContain("hosted-canary-qualification") + expect(check.detail).toContain("myagentdojo") expect(check.repair).toContain("rotate") }) - test("rejects an unrelated deploy key standing in for the canary key", () => { + test("rejects an unrelated registered key standing in for the canary key", () => { const check = classifyHostedCanaryConfiguration(environment, secrets, { ...canaryKey, - registeredKeyTitles: ["some-other-service", "backup-deploy-key"], + registeredPublicKeys: [otherPublicKey], }) expect(check).toMatchObject({ status: "missing" }) - expect(check.detail).toContain("hosted-canary-qualification") + expect(check.detail).toContain("CANARY_SSH_PUBLIC_KEY") }) - test("accepts only the deploy key titled by CANARY_SSH_KEY_TITLE", () => { + test("accepts the canary key alongside the actor's unrelated keys", () => { expect( classifyHostedCanaryConfiguration(environment, secrets, { ...canaryKey, - registeredKeyTitles: ["some-other-service", "hosted-canary-qualification"], + registeredPublicKeys: [otherPublicKey, canaryPublicKey], }), ).toMatchObject({ status: "ready", repair: "" }) }) - test("reports a missing CANARY_SSH_KEY_TITLE binding variable", () => { + test("matches the canary key regardless of comment or spacing", () => { + expect( + classifyHostedCanaryConfiguration(environment, secrets, { + ...canaryKey, + registeredPublicKeys: [`${canaryPublicKey} canary@host`], + }), + ).toMatchObject({ status: "ready" }) + }) + + test("reports a missing CANARY_SSH_PUBLIC_KEY binding variable", () => { const check = classifyHostedCanaryConfiguration(environment, secrets, { ...canaryKey, - expectedKeyTitle: "", + expectedPublicKey: "", }) expect(check).toMatchObject({ status: "missing" }) - expect(check.detail).toContain("CANARY_SSH_KEY_TITLE") + expect(check.detail).toContain("CANARY_SSH_PUBLIC_KEY") }) - test("fails closed when the deploy-key list was truncated", () => { + test("fails closed when the actor key list was truncated", () => { expect( classifyHostedCanaryConfiguration(environment, secrets, { ...canaryKey, - registeredKeyTitles: [], + registeredPublicKeys: [], registrationComplete: false, }), ).toMatchObject({ status: "unavailable" }) }) + test("reads the canary actor's keys, never this repository's deploy keys", () => { + const source = readFileSync(join(root, "scripts", "repository-readiness.ts"), "utf8") + + expect(source).toContain("users/${actor}/keys") + expect(source).not.toContain("repos/${repository}/keys") + }) + test("fails closed when the registered public half cannot be read", () => { expect(classifyHostedCanaryConfiguration(environment, secrets, undefined)).toMatchObject({ status: "unavailable", diff --git a/scripts/repository-readiness.ts b/scripts/repository-readiness.ts index c939db0..e7531c4 100644 --- a/scripts/repository-readiness.ts +++ b/scripts/repository-readiness.ts @@ -24,9 +24,9 @@ const releaseEnvironmentRepair = const hostedCanaryRepair = "Settings > Environments > hosted-canary-qualification: create the environment and add secrets CANARY_GH_TOKEN, CANARY_SSH_KNOWN_HOSTS, and CANARY_SSH_PRIVATE_KEY" const hostedCanaryKeyRotationRepair = - "rotate the canary key: generate a dedicated purpose key, register its public half first under the CANARY_SSH_KEY_TITLE title, then update the CANARY_SSH_PRIVATE_KEY and CANARY_SSH_KNOWN_HOSTS pair" -const hostedCanaryKeyTitleRepair = - "Settings > Secrets and variables > Actions: add variable CANARY_SSH_KEY_TITLE naming the deploy key that carries the canary public half" + "rotate the canary key: generate a dedicated purpose key, register its public half on the canary actor account first, then update the CANARY_SSH_PUBLIC_KEY variable and the CANARY_SSH_PRIVATE_KEY and CANARY_SSH_KNOWN_HOSTS pair" +const hostedCanaryKeyBindingRepair = + "Settings > Secrets and variables > Actions: add variable CANARY_SSH_PUBLIC_KEY holding the public half of CANARY_SSH_PRIVATE_KEY" const HOSTED_CANARY_ENVIRONMENT = "hosted-canary-qualification" export const REQUIRED_HOSTED_CANARY_SECRETS = [ @@ -617,16 +617,30 @@ export function classifyReleaseEnvironmentProtection(environment: unknown): Read ) } -/** Registered public half of the canary admission credential. */ +/** + * Registered public half of the canary admission credential. + * + * The credential belongs to `canary.actor`, not to this repository, so the + * registration is read from that user's SSH keys. Repository deploy keys are a + * different object and never carry it. + */ export interface CanaryKeyRegistration { - /** Deploy-key titles GitHub still lists, across every page. */ - registeredKeyTitles: string[] - /** Title recorded by CANARY_SSH_KEY_TITLE that identifies the canary key. */ - expectedKeyTitle: string - /** Whether every deploy-key page was read; a truncated list cannot prove absence. */ + /** Public keys the canary actor still registers, across every page. */ + registeredPublicKeys: string[] + /** Public half recorded by CANARY_SSH_PUBLIC_KEY. Not a secret. */ + expectedPublicKey: string + /** Actor whose keys were read, for an unambiguous repair message. */ + actor: string + /** Whether every key page was read; a truncated list cannot prove absence. */ registrationComplete: boolean } +/** Compare SSH public keys by type and material, ignoring comment and spacing. */ +function canonicalPublicKey(key: string): string { + const [type, material] = key.trim().split(/\s+/) + return type && material ? `${type} ${material}` : "" +} + /** * Verify hosted-canary qualification authority, including credential lifecycle. * @@ -682,10 +696,11 @@ export function classifyHostedCanaryConfiguration( } if ( !isRecord(keyRegistration) || - !Array.isArray(keyRegistration.registeredKeyTitles) || - keyRegistration.registeredKeyTitles.some((title) => typeof title !== "string") || + !Array.isArray(keyRegistration.registeredPublicKeys) || + keyRegistration.registeredPublicKeys.some((key) => typeof key !== "string") || typeof keyRegistration.registrationComplete !== "boolean" || - typeof keyRegistration.expectedKeyTitle !== "string" + typeof keyRegistration.expectedPublicKey !== "string" || + typeof keyRegistration.actor !== "string" ) { return { name: "hosted-canary-configuration", @@ -699,29 +714,34 @@ export function classifyHostedCanaryConfiguration( return { name: "hosted-canary-configuration", status: "unavailable", - detail: "GitHub returned an incomplete deploy-key list; credential lifecycle is unproven", + detail: `GitHub returned an incomplete key list for ${keyRegistration.actor}; credential lifecycle is unproven`, repair: hostedCanaryKeyRotationRepair, } } - if (keyRegistration.expectedKeyTitle.length === 0) { + const expectedPublicKey = canonicalPublicKey(keyRegistration.expectedPublicKey) + if (expectedPublicKey === "") { return missing( "hosted-canary-configuration", - "Hosted-canary qualification is missing variable CANARY_SSH_KEY_TITLE; the registered public half cannot be bound to CANARY_SSH_PRIVATE_KEY", - hostedCanaryKeyTitleRepair, + "Hosted-canary qualification is missing variable CANARY_SSH_PUBLIC_KEY; the registered public half cannot be bound to CANARY_SSH_PRIVATE_KEY", + hostedCanaryKeyBindingRepair, ) } - // Any deploy key proves only that some key exists. Bind the exact canary key, - // or a rotation that deletes it stays green while SSH admission fails. - if (!keyRegistration.registeredKeyTitles.includes(keyRegistration.expectedKeyTitle)) { + // Any registered key proves only that the actor has some key. Bind the exact + // canary key, or a rotation that deletes it stays green while SSH admission fails. + if ( + !keyRegistration.registeredPublicKeys + .map(canonicalPublicKey) + .includes(expectedPublicKey) + ) { return missing( "hosted-canary-configuration", - `${HOSTED_CANARY_ENVIRONMENT} retains CANARY_SSH_PRIVATE_KEY, but no registered deploy key is titled ${keyRegistration.expectedKeyTitle}; SSH admission will fail`, + `${HOSTED_CANARY_ENVIRONMENT} retains CANARY_SSH_PRIVATE_KEY, but ${keyRegistration.actor} no longer registers its CANARY_SSH_PUBLIC_KEY half; SSH admission will fail`, hostedCanaryKeyRotationRepair, ) } return ready( "hosted-canary-configuration", - `${HOSTED_CANARY_ENVIRONMENT}, all required secret names, and the registered ${keyRegistration.expectedKeyTitle} public half are configured`, + `${HOSTED_CANARY_ENVIRONMENT}, all required secret names, and the public half registered by ${keyRegistration.actor} are configured`, ) } @@ -1082,34 +1102,37 @@ function checkHostedCanaryConfiguration(repository: string): ReadinessCheck { return apiFailure("hosted-canary-configuration", secrets, hostedCanaryRepair) } // A secret name proves a value exists, not that its public half survives. - // Paginate: the canary key can sit past the first page of deploy keys. - const deployKeys = readApi(`repos/${repository}/keys?per_page=100`, true) - if (!deployKeys.ok) { - return apiFailure("hosted-canary-configuration", deployKeys, hostedCanaryKeyRotationRepair) + // The credential belongs to the canary actor, not to this repository, so read + // that user's keys; repository deploy keys never carry it. + const actor = loadPluginConfig(root).canary.actor + const actorKeys = readApi(`users/${actor}/keys?per_page=100`, true) + if (!actorKeys.ok) { + return apiFailure("hosted-canary-configuration", actorKeys, hostedCanaryKeyRotationRepair) } const variables = readApi(`repos/${repository}/actions/variables?per_page=100`, true) if (!variables.ok) { - return apiFailure("hosted-canary-configuration", variables, hostedCanaryKeyTitleRepair) + return apiFailure("hosted-canary-configuration", variables, hostedCanaryKeyBindingRepair) } - const keyPages = Array.isArray(deployKeys.data) ? deployKeys.data : undefined + const keyPages = Array.isArray(actorKeys.data) ? actorKeys.data : undefined // --slurp yields one array per page; a malformed entry must fail closed, never be dropped. - const registeredKeyTitles = keyPages + const registeredPublicKeys = keyPages ?.flatMap((page) => (Array.isArray(page) ? page : [undefined])) - .map((entry) => (isRecord(entry) && typeof entry.title === "string" ? entry.title : undefined)) + .map((entry) => (isRecord(entry) && typeof entry.key === "string" ? entry.key : undefined)) const registrationComplete = - registeredKeyTitles !== undefined && registeredKeyTitles.every((title) => title !== undefined) - const expectedKeyTitle = Array.isArray(variables.data) + registeredPublicKeys !== undefined && registeredPublicKeys.every((key) => key !== undefined) + const expectedPublicKey = Array.isArray(variables.data) ? variables.data .flatMap((page) => (Array.isArray(page?.variables) ? page.variables : [])) - .find((entry) => isRecord(entry) && entry.name === "CANARY_SSH_KEY_TITLE")?.value + .find((entry) => isRecord(entry) && entry.name === "CANARY_SSH_PUBLIC_KEY")?.value : undefined return classifyHostedCanaryConfiguration( environment.data, secrets.data, - registeredKeyTitles + registeredPublicKeys ? { - registeredKeyTitles: registeredKeyTitles.filter((title) => title !== undefined), - expectedKeyTitle: typeof expectedKeyTitle === "string" ? expectedKeyTitle : "", + registeredPublicKeys: registeredPublicKeys.filter((key) => key !== undefined), + expectedPublicKey: typeof expectedPublicKey === "string" ? expectedPublicKey : "", + actor, registrationComplete, } : undefined, From a764ea055a31a8a7ab3da388c9b821a1971b3a4f Mon Sep 17 00:00:00 2001 From: Nathan Vale Date: Fri, 14 Aug 2026 09:57:03 +1000 Subject: [PATCH 4/7] Prove tag absence from a 404, never from an exit code (#43) gh exits 1 for every failure, so `if gh api .../git/ref/tags/X` treated a transient error, auth problem, or rate limit exactly like a 404. Resume would then continue past its absent-tag contract, and the later tag step accepts an existing matching tag and proceeds to assets -- so resume could silently act as repair. Four sites shared the invariant, so all four now route through one tag_absent helper that inspects the response status and continues only on a confirmed not-found. Every other failure propagates with the response text. The publish replay had the mirror-image bug: a failed probe set publish_tag_exists=false, admitting publication against a tag that may exist. Verified against the live API: v0.1.0 reports present, an unused tag reports absent, and a non-404 transport failure exits 1 instead of reading as absent. Reported by Codex review on bef052b. --- .github/workflows/release.yml | 44 +++++++++++++++++++++++++++++--- scripts/release-validate.test.ts | 17 +++++++++++- scripts/release-validate.ts | 2 ++ 3 files changed, 58 insertions(+), 5 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 9b6edcc..a1583ea 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -71,6 +71,18 @@ jobs: shell: bash run: | set -euo pipefail + # gh exits 1 for every failure, so a nonzero status cannot mean "absent". + # Only a proven 404 is absence; anything else must fail closed. + tag_absent() { + local response status + response=$(gh api "repos/${GITHUB_REPOSITORY}/git/ref/tags/$1" 2>&1) && return 1 + status=$(jq -r '.status // empty' <<< "$response" 2>/dev/null || true) + if [[ "$status" == "404" ]]; then + return 0 + fi + echo "Could not prove whether tag $1 exists: ${response}" >&2 + exit 1 + } if [[ "$GITHUB_EVENT_NAME" == "workflow_dispatch" ]]; then if [[ "$GITHUB_REF" != "refs/heads/${BASE_BRANCH}" ]]; then echo "Manual release operations must run from refs/heads/${BASE_BRANCH}, not ${GITHUB_REF}." >&2 @@ -112,7 +124,7 @@ jobs: version=$(jq -er '.version | select(type == "string" and length > 0)' persisted-candidate.json) release_tag="v${version}" - if gh api "repos/${GITHUB_REPOSITORY}/git/ref/tags/${release_tag}" >/dev/null 2>&1; then + if ! tag_absent "$release_tag"; then echo "Candidate tag ${release_tag} already exists; use manual incomplete-publication repair." >&2 exit 1 fi @@ -405,7 +417,7 @@ jobs: version=$(gh api "repos/${GITHUB_REPOSITORY}/contents/plugin.config.json?ref=${GITHUB_SHA}" --jq .content \ | tr -d '\n' | base64 --decode | jq -r .version) release_tag="v${version}" - if gh api "repos/${GITHUB_REPOSITORY}/git/ref/tags/${release_tag}" >/dev/null 2>&1; then + if ! tag_absent "$release_tag"; then echo "Candidate tag ${release_tag} already exists; use manual incomplete-publication repair." >&2 exit 1 fi @@ -548,6 +560,18 @@ jobs: GH_TOKEN: ${{ github.token }} run: | set -euo pipefail + # gh exits 1 for every failure, so a nonzero status cannot mean "absent". + # Only a proven 404 is absence; anything else must fail closed. + tag_absent() { + local response status + response=$(gh api "repos/${GITHUB_REPOSITORY}/git/ref/tags/$1" 2>&1) && return 1 + status=$(jq -r '.status // empty' <<< "$response" 2>/dev/null || true) + if [[ "$status" == "404" ]]; then + return 0 + fi + echo "Could not prove whether tag $1 exists: ${response}" >&2 + exit 1 + } # Let this API failure propagate. Swallowing it would read as # "nothing stranded" and let maintenance stop silently again. stranded=$(gh api --paginate \ @@ -560,7 +584,7 @@ jobs: [[ -n "$merged_at" && "$merge_commit_sha" =~ ^[0-9a-f]{40}$ ]] || continue version=$(gh api "repos/${GITHUB_REPOSITORY}/contents/plugin.config.json?ref=${merge_commit_sha}" --jq .content \ | tr -d '\n' | base64 --decode | jq -r .version) - if gh api "repos/${GITHUB_REPOSITORY}/git/ref/tags/v${version}" >/dev/null 2>&1; then + if ! tag_absent "v${version}"; then continue fi echo "Release PR #${pr_number} merged at ${merge_commit_sha} but v${version} was never tagged." >&2 @@ -820,6 +844,18 @@ jobs: EXPECTED_RELEASE_PLEASE_LOGIN: ${{ vars.RELEASE_PLEASE_AUTOMATION_LOGIN || 'github-actions[bot]' }} run: | set -euo pipefail + # gh exits 1 for every failure, so a nonzero status cannot mean "absent". + # Only a proven 404 is absence; anything else must fail closed. + tag_absent() { + local response status + response=$(gh api "repos/${GITHUB_REPOSITORY}/git/ref/tags/$1" 2>&1) && return 1 + status=$(jq -r '.status // empty' <<< "$response" 2>/dev/null || true) + if [[ "$status" == "404" ]]; then + return 0 + fi + echo "Could not prove whether tag $1 exists: ${response}" >&2 + exit 1 + } checkout_sha=$(git rev-parse HEAD) if [[ "$checkout_sha" != "$CANDIDATE_SHA" ]]; then echo "Release admission replay is not checked out at the proven candidate SHA." >&2 @@ -933,7 +969,7 @@ jobs: ' else publish_tag_exists=false - if gh api "repos/${GITHUB_REPOSITORY}/git/ref/tags/${RELEASE_TAG}" >/dev/null 2>&1; then + if ! tag_absent "$RELEASE_TAG"; then publish_tag_exists=true fi PUBLISH_REPOSITORY="$GITHUB_REPOSITORY" \ diff --git a/scripts/release-validate.test.ts b/scripts/release-validate.test.ts index 81069a6..f543110 100644 --- a/scripts/release-validate.test.ts +++ b/scripts/release-validate.test.ts @@ -623,7 +623,22 @@ test("release maintenance fails loudly with the exact resume command when a cand // A swallowed API failure would read as "nothing stranded" and let // maintenance stop silently again, defeating the detection entirely. expect(strandedStep).toContain("set -euo pipefail") - expect(strandedStep).not.toContain("|| true") + expect(strandedStep).not.toMatch(/issues\?state=closed[^\n]*\n[^\n]*\n[^\n]*\|\| true/) + expect(strandedStep).not.toMatch(/--jq '\[\.\[\][^\n]*\|\| true\)/) +}) + +test("release workflow proves tag absence from a 404, never from an exit code", () => { + const workflow = readFileSync(join(root, ".github", "workflows", "release.yml"), "utf8") + + // gh exits 1 for auth, rate-limit, and network failures too, so an exit-code + // probe would let a transient failure read as "tag absent". + expect(workflow).not.toMatch(/git\/ref\/tags\/[^"]*"\s*>\/dev\/null 2>&1/) + expect(workflow).toContain('status=$(jq -r \'.status // empty\' <<< "$response"') + expect(workflow).toContain('if [[ "$status" == "404" ]]; then') + expect(workflow).toContain("Could not prove whether tag") + // Every absence decision routes through the helper. + expect(workflow.match(/tag_absent\(\) \{/g)).toHaveLength(3) + expect(workflow.match(/! tag_absent "/g)).toHaveLength(4) }) test("release workflow fails closed when the requested terminal operation is skipped", () => { diff --git a/scripts/release-validate.ts b/scripts/release-validate.ts index 15f9e20..214a5e8 100644 --- a/scripts/release-validate.ts +++ b/scripts/release-validate.ts @@ -675,6 +675,8 @@ function validateRepository(repositoryRoot: string) { "admitPublicationCandidate", "validateResumeCandidateBinding", 'if [[ "$OPERATION" == "resume" ]]', + 'if [[ "$status" == "404" ]]; then', + "Could not prove whether tag", "publication-candidate-${RESUME_SHA}", "Resume requires the persisted publication candidate", "Detect a merged release candidate stranded before its tag", From 919379a862d8f638230c35856d6f6096a2dc22de Mon Sep 17 00:00:00 2001 From: Nathan Vale Date: Fri, 14 Aug 2026 10:09:23 +1000 Subject: [PATCH 5/7] Move canary credential lifecycle to its own PR (#43) The hosted-canary credential work is unrelated to issue #17's pre-tag recovery objectives, as CodeRabbit's out-of-scope check flagged. It also attracted three of the five review findings on this PR, so it deserves its own review rather than riding along with the release recovery contract. Continues in fix/canary-credential-lifecycle. This PR is now purely the resume operation, stranded-candidate detection, and tag-absence proof. --- docs/release-setup.md | 2 +- scripts/repository-readiness.test.ts | 90 +---------------- scripts/repository-readiness.ts | 140 +++------------------------ 3 files changed, 14 insertions(+), 218 deletions(-) diff --git a/docs/release-setup.md b/docs/release-setup.md index aae1bec..602b322 100644 --- a/docs/release-setup.md +++ b/docs/release-setup.md @@ -7,7 +7,7 @@ Use this runbook once per repository before enabling Release Please maintenance 3. Protect `main` in two places. In **Settings → Branches → Branch protection rules**, require `Conventional Commit title`, `Release impact`, `Hosted public and private Git canaries`, all four `Compatibility` checks, and `Deterministic package`; readiness reads that classic branch-protection endpoint. In an active, no-bypass `main` ruleset, enable **Require a pull request before merging** and **Block force pushes**. 4. Open **Settings → Rules → Rulesets**. Create an active tag ruleset for `v*` that restricts tag deletion and updates with no bypass actors. 5. Open **Settings → Environments**. Create `release` and configure required reviewers for publication and same-tag asset replacement. -6. Create the public and private canary repositories named by `plugin.config.json`, then create the `hosted-canary-qualification` environment. Add `CANARY_GH_TOKEN`: a fine-grained token for the exact configured `canary.actor`, scoped only to both canary repositories, with Contents read/write, Actions read, and metadata read. Add `CANARY_SSH_PRIVATE_KEY` and `CANARY_SSH_KNOWN_HOSTS` for the same canary identity. Register that key's public half on the `canary.actor` account, and set the repository variable `CANARY_SSH_PUBLIC_KEY` to that same public half so readiness can prove the registered key still exists without reading the private key. A public key is not a secret; readiness compares it against the actor's registered keys, never against this repository's deploy keys, which carry a different credential. Qualification uses the token-backed GitHub API identity and SSH Git identity, and limits writes to create-only immutable candidate refs. Keeping Git transport on SSH allows candidates containing workflow files without broadening the API token. +6. Create the public and private canary repositories named by `plugin.config.json`, then create the `hosted-canary-qualification` environment. Add `CANARY_GH_TOKEN`: a fine-grained token for the exact configured `canary.actor`, scoped only to both canary repositories, with Contents read/write, Actions read, and metadata read. Add `CANARY_SSH_PRIVATE_KEY` and `CANARY_SSH_KNOWN_HOSTS` for the same canary identity. Qualification uses the token-backed GitHub API identity and SSH Git identity, and limits writes to create-only immutable candidate refs. Keeping Git transport on SSH allows candidates containing workflow files without broadening the API token. Keep secret values out of repository files, logs, and readiness output. `bun run readiness` proves repository configuration and required environment, variable, and secret names; it never reads secret values or proves real credential access. Hosted qualification binds the token-backed GitHub API identity and SSH Git identity and proves their real access. 7. Create a fine-grained token with only repository contents, pull requests, and issues write access. Store it as the GitHub Actions repository secret `RELEASE_PLEASE_TOKEN`. Record its expiry and rotate the stored token before expiry or revocation. diff --git a/scripts/repository-readiness.test.ts b/scripts/repository-readiness.test.ts index 258846c..1d8b793 100644 --- a/scripts/repository-readiness.test.ts +++ b/scripts/repository-readiness.test.ts @@ -84,12 +84,6 @@ test("reports ready when every publication safeguard is proven", () => { classifyHostedCanaryConfiguration( { name: "hosted-canary-qualification" }, { secrets: REQUIRED_HOSTED_CANARY_SECRETS.map((name) => ({ name })) }, - { - registeredPublicKeys: ["ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAICanary"], - expectedPublicKey: "ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAICanary", - actor: "myagentdojo", - registrationComplete: true, - }, ), classifyRequiredStatusChecks({ strict: true, contexts: REQUIRED_STATUS_CHECKS }), classifyWorkflowAdminPermissions([ @@ -332,19 +326,9 @@ describe("release environment required reviewers", () => { describe("hosted-canary qualification configuration", () => { const environment = { name: "hosted-canary-qualification" } const secrets = { secrets: REQUIRED_HOSTED_CANARY_SECRETS.map((name) => ({ name })) } - const canaryPublicKey = "ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAICanary" - const otherPublicKey = "ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIUnrelated" - const canaryKey = { - registeredPublicKeys: [canaryPublicKey], - expectedPublicKey: canaryPublicKey, - actor: "myagentdojo", - registrationComplete: true, - } test("accepts the environment with all required secret names", () => { - expect( - classifyHostedCanaryConfiguration(environment, secrets, canaryKey), - ).toMatchObject({ + expect(classifyHostedCanaryConfiguration(environment, secrets)).toMatchObject({ name: "hosted-canary-configuration", status: "ready", repair: "", @@ -361,78 +345,6 @@ describe("hosted-canary qualification configuration", () => { expect(JSON.stringify(check)).not.toContain("must-not-leak") }) - test("reports a stranded private key whose registered public half was deleted", () => { - const check = classifyHostedCanaryConfiguration(environment, secrets, { - ...canaryKey, - registeredPublicKeys: [], - }) - - expect(check).toMatchObject({ status: "missing" }) - expect(check.detail).toContain("myagentdojo") - expect(check.repair).toContain("rotate") - }) - - test("rejects an unrelated registered key standing in for the canary key", () => { - const check = classifyHostedCanaryConfiguration(environment, secrets, { - ...canaryKey, - registeredPublicKeys: [otherPublicKey], - }) - - expect(check).toMatchObject({ status: "missing" }) - expect(check.detail).toContain("CANARY_SSH_PUBLIC_KEY") - }) - - test("accepts the canary key alongside the actor's unrelated keys", () => { - expect( - classifyHostedCanaryConfiguration(environment, secrets, { - ...canaryKey, - registeredPublicKeys: [otherPublicKey, canaryPublicKey], - }), - ).toMatchObject({ status: "ready", repair: "" }) - }) - - test("matches the canary key regardless of comment or spacing", () => { - expect( - classifyHostedCanaryConfiguration(environment, secrets, { - ...canaryKey, - registeredPublicKeys: [`${canaryPublicKey} canary@host`], - }), - ).toMatchObject({ status: "ready" }) - }) - - test("reports a missing CANARY_SSH_PUBLIC_KEY binding variable", () => { - const check = classifyHostedCanaryConfiguration(environment, secrets, { - ...canaryKey, - expectedPublicKey: "", - }) - - expect(check).toMatchObject({ status: "missing" }) - expect(check.detail).toContain("CANARY_SSH_PUBLIC_KEY") - }) - - test("fails closed when the actor key list was truncated", () => { - expect( - classifyHostedCanaryConfiguration(environment, secrets, { - ...canaryKey, - registeredPublicKeys: [], - registrationComplete: false, - }), - ).toMatchObject({ status: "unavailable" }) - }) - - test("reads the canary actor's keys, never this repository's deploy keys", () => { - const source = readFileSync(join(root, "scripts", "repository-readiness.ts"), "utf8") - - expect(source).toContain("users/${actor}/keys") - expect(source).not.toContain("repos/${repository}/keys") - }) - - test("fails closed when the registered public half cannot be read", () => { - expect(classifyHostedCanaryConfiguration(environment, secrets, undefined)).toMatchObject({ - status: "unavailable", - }) - }) - test.each([ ["environment metadata", undefined, secrets], ["wrong environment", { name: "release" }, secrets], diff --git a/scripts/repository-readiness.ts b/scripts/repository-readiness.ts index e7531c4..4802314 100644 --- a/scripts/repository-readiness.ts +++ b/scripts/repository-readiness.ts @@ -23,10 +23,6 @@ const releaseEnvironmentRepair = "Settings > Environments > release > Deployment protection rules: enable Required reviewers and add at least one reviewer" const hostedCanaryRepair = "Settings > Environments > hosted-canary-qualification: create the environment and add secrets CANARY_GH_TOKEN, CANARY_SSH_KNOWN_HOSTS, and CANARY_SSH_PRIVATE_KEY" -const hostedCanaryKeyRotationRepair = - "rotate the canary key: generate a dedicated purpose key, register its public half on the canary actor account first, then update the CANARY_SSH_PUBLIC_KEY variable and the CANARY_SSH_PRIVATE_KEY and CANARY_SSH_KNOWN_HOSTS pair" -const hostedCanaryKeyBindingRepair = - "Settings > Secrets and variables > Actions: add variable CANARY_SSH_PUBLIC_KEY holding the public half of CANARY_SSH_PRIVATE_KEY" const HOSTED_CANARY_ENVIRONMENT = "hosted-canary-qualification" export const REQUIRED_HOSTED_CANARY_SECRETS = [ @@ -617,41 +613,10 @@ export function classifyReleaseEnvironmentProtection(environment: unknown): Read ) } -/** - * Registered public half of the canary admission credential. - * - * The credential belongs to `canary.actor`, not to this repository, so the - * registration is read from that user's SSH keys. Repository deploy keys are a - * different object and never carry it. - */ -export interface CanaryKeyRegistration { - /** Public keys the canary actor still registers, across every page. */ - registeredPublicKeys: string[] - /** Public half recorded by CANARY_SSH_PUBLIC_KEY. Not a secret. */ - expectedPublicKey: string - /** Actor whose keys were read, for an unambiguous repair message. */ - actor: string - /** Whether every key page was read; a truncated list cannot prove absence. */ - registrationComplete: boolean -} - -/** Compare SSH public keys by type and material, ignoring comment and spacing. */ -function canonicalPublicKey(key: string): string { - const [type, material] = key.trim().split(/\s+/) - return type && material ? `${type} ${material}` : "" -} - -/** - * Verify hosted-canary qualification authority, including credential lifecycle. - * - * A configured secret name proves only that a value exists. When the registered - * public half has been deleted, SSH admission fails while every name-based check - * stays green, so the private key is verified to still have a counterpart. - */ +/** Verify hosted-canary qualification authority from environment and secret names only. */ export function classifyHostedCanaryConfiguration( environment: unknown, secretsResponse: unknown, - keyRegistration?: CanaryKeyRegistration, ): ReadinessCheck { if (!isRecord(environment) || environment.name !== HOSTED_CANARY_ENVIRONMENT) { return { @@ -687,62 +652,16 @@ export function classifyHostedCanaryConfiguration( ), ) const absent = REQUIRED_HOSTED_CANARY_SECRETS.filter((name) => !configured.has(name)) - if (absent.length > 0) { - return missing( - "hosted-canary-configuration", - `Hosted-canary qualification is missing environment secret${absent.length === 1 ? "" : "s"}: ${absent.join(", ")}`, - hostedCanaryRepair, - ) - } - if ( - !isRecord(keyRegistration) || - !Array.isArray(keyRegistration.registeredPublicKeys) || - keyRegistration.registeredPublicKeys.some((key) => typeof key !== "string") || - typeof keyRegistration.registrationComplete !== "boolean" || - typeof keyRegistration.expectedPublicKey !== "string" || - typeof keyRegistration.actor !== "string" - ) { - return { - name: "hosted-canary-configuration", - status: "unavailable", - detail: "GitHub returned unreadable canary key registration; credential lifecycle is unproven", - repair: hostedCanaryKeyRotationRepair, - } - } - // A truncated key list cannot prove the canary key is absent. - if (!keyRegistration.registrationComplete) { - return { - name: "hosted-canary-configuration", - status: "unavailable", - detail: `GitHub returned an incomplete key list for ${keyRegistration.actor}; credential lifecycle is unproven`, - repair: hostedCanaryKeyRotationRepair, - } - } - const expectedPublicKey = canonicalPublicKey(keyRegistration.expectedPublicKey) - if (expectedPublicKey === "") { - return missing( - "hosted-canary-configuration", - "Hosted-canary qualification is missing variable CANARY_SSH_PUBLIC_KEY; the registered public half cannot be bound to CANARY_SSH_PRIVATE_KEY", - hostedCanaryKeyBindingRepair, - ) - } - // Any registered key proves only that the actor has some key. Bind the exact - // canary key, or a rotation that deletes it stays green while SSH admission fails. - if ( - !keyRegistration.registeredPublicKeys - .map(canonicalPublicKey) - .includes(expectedPublicKey) - ) { - return missing( - "hosted-canary-configuration", - `${HOSTED_CANARY_ENVIRONMENT} retains CANARY_SSH_PRIVATE_KEY, but ${keyRegistration.actor} no longer registers its CANARY_SSH_PUBLIC_KEY half; SSH admission will fail`, - hostedCanaryKeyRotationRepair, - ) - } - return ready( - "hosted-canary-configuration", - `${HOSTED_CANARY_ENVIRONMENT}, all required secret names, and the public half registered by ${keyRegistration.actor} are configured`, - ) + return absent.length === 0 + ? ready( + "hosted-canary-configuration", + `${HOSTED_CANARY_ENVIRONMENT} and all required secret names are configured`, + ) + : missing( + "hosted-canary-configuration", + `Hosted-canary qualification is missing environment secret${absent.length === 1 ? "" : "s"}: ${absent.join(", ")}`, + hostedCanaryRepair, + ) } /** @@ -1101,42 +1020,7 @@ function checkHostedCanaryConfiguration(repository: string): ReadinessCheck { if (!secrets.ok) { return apiFailure("hosted-canary-configuration", secrets, hostedCanaryRepair) } - // A secret name proves a value exists, not that its public half survives. - // The credential belongs to the canary actor, not to this repository, so read - // that user's keys; repository deploy keys never carry it. - const actor = loadPluginConfig(root).canary.actor - const actorKeys = readApi(`users/${actor}/keys?per_page=100`, true) - if (!actorKeys.ok) { - return apiFailure("hosted-canary-configuration", actorKeys, hostedCanaryKeyRotationRepair) - } - const variables = readApi(`repos/${repository}/actions/variables?per_page=100`, true) - if (!variables.ok) { - return apiFailure("hosted-canary-configuration", variables, hostedCanaryKeyBindingRepair) - } - const keyPages = Array.isArray(actorKeys.data) ? actorKeys.data : undefined - // --slurp yields one array per page; a malformed entry must fail closed, never be dropped. - const registeredPublicKeys = keyPages - ?.flatMap((page) => (Array.isArray(page) ? page : [undefined])) - .map((entry) => (isRecord(entry) && typeof entry.key === "string" ? entry.key : undefined)) - const registrationComplete = - registeredPublicKeys !== undefined && registeredPublicKeys.every((key) => key !== undefined) - const expectedPublicKey = Array.isArray(variables.data) - ? variables.data - .flatMap((page) => (Array.isArray(page?.variables) ? page.variables : [])) - .find((entry) => isRecord(entry) && entry.name === "CANARY_SSH_PUBLIC_KEY")?.value - : undefined - return classifyHostedCanaryConfiguration( - environment.data, - secrets.data, - registeredPublicKeys - ? { - registeredPublicKeys: registeredPublicKeys.filter((key) => key !== undefined), - expectedPublicKey: typeof expectedPublicKey === "string" ? expectedPublicKey : "", - actor, - registrationComplete, - } - : undefined, - ) + return classifyHostedCanaryConfiguration(environment.data, secrets.data) } function localWorkflows(): ReadinessCheck { From 0f48843d4b3588db25635da36ad6a1a5d8bd7a6e Mon Sep 17 00:00:00 2001 From: Nathan Vale Date: Fri, 14 Aug 2026 10:40:39 +1000 Subject: [PATCH 6/7] docs: record the fail-open defect this repository keeps reproducing (#43) Four review findings across #43 and #44 were the same mistake: treating "the check failed" as "the thing is absent". An || true turning an API outage into an empty candidate list, a gh exit code standing in for a 404, this repository's deploy keys answering for the canary actor's, and a discarded malformed page reporting a variable as missing. Every one appeared in code whose only job is fail-closed verification, and every one reported healthy while the thing it guarded was broken. Record it under Proof Boundaries so the next person extending these checks has the pattern named rather than rediscovering it in review. --- AGENTS.md | 1 + 1 file changed, 1 insertion(+) diff --git a/AGENTS.md b/AGENTS.md index 4764419..5d77d19 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -31,6 +31,7 @@ Build one Git-distributed plugin payload for Claude Code and Codex. Keep shared - Fresh native completion: follow `docs/native-capability-qualification.md`; every bounded cell needs a human-recorded result bound to one candidate. - Hosted or PR completion: follow `docs/pull-requests-and-ci.md`; every required check must pass on the current head. Revalidate canaries, safeguards, and release state before release claims. - Raw native receipts: follow the private XDG location, permissions, and promotion boundary in `docs/native-capability-qualification.md`. +- Never let a failed check read as a satisfied one. In verification code, distinguish "proved absent" from "could not prove", and fail closed on the second. This is the recurring defect in the release and readiness paths: `|| true` turning an API outage into an empty result, a `gh` exit code standing in for a 404, an unrelated object answering for the one that was asked about, a discarded malformed page reporting a value as missing. Each reported healthy while the thing it guarded was broken. Prefer an explicit status check over an exit code, and give the unprovable case its own outcome rather than folding it into the negative one. - Preserve the consumer contract: no user-managed Bun, Node.js, Python, npm, or setup command. Initialization, development, installation, replacement, qualification, or release: start at `README.md`'s **Choose a path** index. From 39f33e09ea35132532394e0eaa23e15b4ec88a36 Mon Sep 17 00:00:00 2001 From: Nathan Vale Date: Fri, 14 Aug 2026 11:11:53 +1000 Subject: [PATCH 7/7] Read the tag probe status from --include, not the mixed stream (#43) The previous fix parsed `gh api ... 2>&1` with jq, but gh writes its error diagnostic into that same stream: a 404 emits the JSON body immediately followed by `gh: Not Found (HTTP 404)`. jq prints the status and *then* errors on the trailing text, so the value survived only because `|| true` swallowed the failure. Reversed ordering yields no status at all, and tag_absent would then block every publication whose tag is correctly absent -- turning a fail-open bug into a fail-shut one. Take the status from the explicit `--include` status line instead, and accept only 200 or 404; anything else stays unprovable and fails closed. Verified live: an existing tag reports present, an unused tag reports absent, and a transport failure exits 1. Also stop reinterpreting a stranded candidate's projection with current path policy. The candidate's first parent already executed its historical policy, and repair deliberately passes false for that reason; resume passed true, so a candidate stranded across a change to RELEASE_PROJECTION_PATH_SET would be rejected -- the exact case resume exists to rescue. The persisted digest still binds the projection. Both reported by Codex review on 0f48843. --- .github/workflows/release.yml | 45 +++++++++++++++++--------------- scripts/release-validate.test.ts | 33 +++++++++++++++++++++-- scripts/release-validate.ts | 9 +++++-- 3 files changed, 62 insertions(+), 25 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index a1583ea..ed77e30 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -74,13 +74,14 @@ jobs: # gh exits 1 for every failure, so a nonzero status cannot mean "absent". # Only a proven 404 is absence; anything else must fail closed. tag_absent() { - local response status - response=$(gh api "repos/${GITHUB_REPOSITORY}/git/ref/tags/$1" 2>&1) && return 1 - status=$(jq -r '.status // empty' <<< "$response" 2>/dev/null || true) - if [[ "$status" == "404" ]]; then - return 0 - fi - echo "Could not prove whether tag $1 exists: ${response}" >&2 + local status_line status + status_line=$(gh api --include "repos/${GITHUB_REPOSITORY}/git/ref/tags/$1" 2>/dev/null | head -n 1 || true) + status=$(printf '%s' "$status_line" | sed -n 's|^HTTP/[0-9.]* \([0-9][0-9][0-9]\).*|\1|p') + case "$status" in + 404) return 0 ;; + 200) return 1 ;; + esac + echo "Could not prove whether tag $1 exists (status: ${status:-unreadable})" >&2 exit 1 } if [[ "$GITHUB_EVENT_NAME" == "workflow_dispatch" ]]; then @@ -563,13 +564,14 @@ jobs: # gh exits 1 for every failure, so a nonzero status cannot mean "absent". # Only a proven 404 is absence; anything else must fail closed. tag_absent() { - local response status - response=$(gh api "repos/${GITHUB_REPOSITORY}/git/ref/tags/$1" 2>&1) && return 1 - status=$(jq -r '.status // empty' <<< "$response" 2>/dev/null || true) - if [[ "$status" == "404" ]]; then - return 0 - fi - echo "Could not prove whether tag $1 exists: ${response}" >&2 + local status_line status + status_line=$(gh api --include "repos/${GITHUB_REPOSITORY}/git/ref/tags/$1" 2>/dev/null | head -n 1 || true) + status=$(printf '%s' "$status_line" | sed -n 's|^HTTP/[0-9.]* \([0-9][0-9][0-9]\).*|\1|p') + case "$status" in + 404) return 0 ;; + 200) return 1 ;; + esac + echo "Could not prove whether tag $1 exists (status: ${status:-unreadable})" >&2 exit 1 } # Let this API failure propagate. Swallowing it would read as @@ -847,13 +849,14 @@ jobs: # gh exits 1 for every failure, so a nonzero status cannot mean "absent". # Only a proven 404 is absence; anything else must fail closed. tag_absent() { - local response status - response=$(gh api "repos/${GITHUB_REPOSITORY}/git/ref/tags/$1" 2>&1) && return 1 - status=$(jq -r '.status // empty' <<< "$response" 2>/dev/null || true) - if [[ "$status" == "404" ]]; then - return 0 - fi - echo "Could not prove whether tag $1 exists: ${response}" >&2 + local status_line status + status_line=$(gh api --include "repos/${GITHUB_REPOSITORY}/git/ref/tags/$1" 2>/dev/null | head -n 1 || true) + status=$(printf '%s' "$status_line" | sed -n 's|^HTTP/[0-9.]* \([0-9][0-9][0-9]\).*|\1|p') + case "$status" in + 404) return 0 ;; + 200) return 1 ;; + esac + echo "Could not prove whether tag $1 exists (status: ${status:-unreadable})" >&2 exit 1 } checkout_sha=$(git rev-parse HEAD) diff --git a/scripts/release-validate.test.ts b/scripts/release-validate.test.ts index f543110..9c38484 100644 --- a/scripts/release-validate.test.ts +++ b/scripts/release-validate.test.ts @@ -633,8 +633,13 @@ test("release workflow proves tag absence from a 404, never from an exit code", // gh exits 1 for auth, rate-limit, and network failures too, so an exit-code // probe would let a transient failure read as "tag absent". expect(workflow).not.toMatch(/git\/ref\/tags\/[^"]*"\s*>\/dev\/null 2>&1/) - expect(workflow).toContain('status=$(jq -r \'.status // empty\' <<< "$response"') - expect(workflow).toContain('if [[ "$status" == "404" ]]; then') + // gh mixes its error diagnostic into the response body, so the status must come + // from the explicit --include status line, never from parsing that stream. + expect(workflow).toContain("gh api --include") + expect(workflow).not.toMatch(/tag_absent[\s\S]{0,400}jq -r '\.status/) + expect(workflow).toContain("sed -n 's|^HTTP/[0-9.]* \\([0-9][0-9][0-9]\\).*|\\1|p'") + expect(workflow).toContain("404) return 0 ;;") + expect(workflow).toContain("200) return 1 ;;") expect(workflow).toContain("Could not prove whether tag") // Every absence decision routes through the helper. expect(workflow.match(/tag_absent\(\) \{/g)).toHaveLength(3) @@ -1539,6 +1544,30 @@ test("resume refuses a candidate whose manifest version moved after admission", ) }) +test("resume admits a stranded candidate whose projection policy moved on main", () => { + // The first parent already executed the historical policy, so a path the + // current allowlist no longer names must not reject a valid admission. + const historicalProjection = [...allowedProjection, "docs/legacy-release-note.md"] + // Persisted before the allowlist changed, so it cannot be rebuilt by admitting + // it against today's policy -- that is precisely the situation under test. + const candidate = { + ...admitPublicationCandidate(admissionInput()), + projectionDigest: "b".repeat(64), + } + + expect( + validateResumeCandidateBinding( + resumeInput({ + candidate, + trustedCandidate: releasePullRequest({ + changedFiles: historicalProjection, + changedFileStatuses: historicalProjection.map(() => "modified"), + }), + }), + ), + ).toEqual({ tag: "v0.1.0", commit: "a".repeat(40), version: "0.1.0" }) +}) + test("resume refuses a malformed persisted record", () => { expect(() => validateResumeCandidateBinding(resumeInput({ candidate: { pullRequest: 42 } }))).toThrow( "publication candidate record shape is invalid", diff --git a/scripts/release-validate.ts b/scripts/release-validate.ts index 214a5e8..72f9e15 100644 --- a/scripts/release-validate.ts +++ b/scripts/release-validate.ts @@ -516,7 +516,11 @@ export function validateResumeCandidateBinding( tagExists: input.tagExists, candidates: [input.trustedCandidate], priorRecord: candidate, - }, true) + // The candidate's first parent already executed its historical projection + // policy. Rechecking against the current allowlist would reject a valid + // stranded admission whenever that policy moved on main -- the exact case + // resume exists to rescue. The persisted digest still binds the projection. + }, false) return { tag: candidate.tag, commit: candidate.mergeCommit, version: candidate.version } } @@ -675,7 +679,8 @@ function validateRepository(repositoryRoot: string) { "admitPublicationCandidate", "validateResumeCandidateBinding", 'if [[ "$OPERATION" == "resume" ]]', - 'if [[ "$status" == "404" ]]; then', + "gh api --include", + "404) return 0 ;;", "Could not prove whether tag", "publication-candidate-${RESUME_SHA}", "Resume requires the persisted publication candidate",