fix(release): bind the published wheel to the tagged source and make publication recoverable - #359
Conversation
…publication recoverable The tag workflow established three bindings — tag to pyproject version, qualification payload to wheel bytes, and tag to the wheel's own METADATA version — but nothing tied the shipped bytes back to any source tree. It tested the checkout and published a wheel downloaded from a repository variable, so any wheel declaring Name: agents-shipgate and the right Version satisfied every check. It also ran verification and immutable publication in one job, generated an SBOM describing the CI machine, ran the suite serially including timing-sensitive perf tests, and could only be exercised by pushing a tag — the one action that also publishes. - #342 Rebuild from the tagged checkout and require byte equality with the qualified wheel before publication. Pin the build backend so byte equality is achievable at all: wheels record `Generator: hatchling <version>`. Container-metadata-only differences still fail; the weaker unpacked-content bar sits behind an explicit flag so it cannot be taken silently. - #343 Split read-only verification from publication with a content-addressed handoff whose digest travels via job outputs. Draft the GitHub Release before the PyPI upload, finalize after asset validation, serialize concurrency across the project, and make the upload idempotence-aware so a re-run completes an interrupted transaction but never republishes divergent bytes. - #344 Match CI's -n auto parallelism, exclude perf-marked latency budgets, keep the adapter static-only lint explicit, and size the timeout from a measurement. - #355 Add a workflow_dispatch rehearsal that runs the identical verification path and is structurally incapable of publishing. - #356 Generate the SBOM from an isolated runtime-only install of the wheel, bind it to the wheel digest, and normalize away the file:// build path that leaked runner layout and broke determinism. #341 is deliberately not decided here: the pre-1.0 evidence bar is a human product/security decision. docs/release-evidence-policy-decision.md records both routes, the invariants that stay non-negotiable either way, and what a rehearsal must prove; the enforced policy is unchanged. Refs #342, #343, #344, #355, #356, #341 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ings Replaces the local CPU-time extrapolation with per-phase durations measured on a hosted runner (CI run 31336011667): the correctness suite is 407s and the supporting steps ~40s, so a healthy verification run lands near 10 minutes against the 25-minute budget. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Product decision 2026-08-09: #341 is P2 ("queued; valuable but not blocking") and no longer in the v0.16.0 milestone. Restates the brief accordingly, and is explicit that descoping changed what the milestone tracks rather than what the pipeline enforces: a `v*` tag still fails closed at the qualification step unless a signed artifact satisfies the 100-case bar, so publishing still needs either that artifact or an approved alternative policy. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
pengfei-threemoonslab
left a comment
pengfei-threemoonslab
left a comment
There was a problem hiding this comment.
Engineering/security review — request changes
GitHub does not permit the PR author to submit a formal REQUEST_CHANGES event on their own pull request, so this is posted as a comment review. The engineering verdict is still: do not merge current head bc4d6c78ecc20fc372de3fd9040690b62af3b9a2 until the P1 findings are fixed.
Blocking issues:
- checkout, recorded provenance SHA, and release tag are not bound;
- publication is not gated on a prior candidate-specific rehearsal;
- qualification signer/issuer trust roots moved to repository variables with broader mutation authority;
- an empty output digest disables the protected manifest binding;
- PyPI and existing GitHub Release state are classified too loosely;
- the OIDC-capable publisher executes editable project/dev installation before handoff verification.
Additional inline findings cover the open-world asset set, disconnected CycloneDX root, and wheel filename semantics.
Validation against the exact current tree: all current GitHub workflows are green; 57 targeted release tests and Ruff passed; repeated wheel and SBOM builds were byte-identical; adversarial reproductions confirmed the empty-digest bypass, unlisted-asset acceptance, mixed/yanked PyPI false-idempotence, renamed-wheel acceptance, and disconnected SBOM graph. Agents Shipgate reports review_publishable / human_review_required / review_required for the touched release trust roots.
Please address the P1 findings before merge and add regression tests for each fail-closed boundary.
| contents: read | ||
| uses: ./.github/workflows/release-verify.yml | ||
| with: | ||
| ref: ${{ github.ref }} |
There was a problem hiding this comment.
[P1] This passes a mutable symbolic ref into the reusable verifier. With an explicit ref, the pinned checkout action re-resolves the live tag instead of pairing it with the event SHA, while later provenance records the caller's GITHUB_SHA. A tag move can therefore build commit B while claiming A; deletion is worse because release creation lacks --verify-tag. Pass ${{ github.sha }}, derive and propagate git rev-parse HEAD, verify the peeled remote tag equals it immediately before publication, and use gh release create --verify-tag.
There was a problem hiding this comment.
Fixed in a6eddfd. Agreed on all three legs, and the second one is the part I had wrong: I was treating github.sha and the checked-out tree as interchangeable.
release.ymlnow passesref: ${{ github.sha }}into the reusable verifier instead ofgithub.ref.- The verifier resolves
source_sha="$(git rev-parse HEAD)"after checkout and exports it as a job output; provenance and the candidate manifest are keyed to that value, not toGITHUB_SHA. Measured rather than assumed, so the two cannot drift. - Both
stageandpublishre-peel the tag against the remote (refs/tags/<tag>^{}, falling back to the unpeeled ref) and fail if it no longer equalssource_sha— placed immediately before each irreversible step, and the deletion case is covered because an empty result also fails. gh release create --verify-tag.
Guarded by test_verification_runs_against_an_immutable_commit_not_a_symbolic_ref, test_provenance_is_keyed_to_the_resolved_checkout_sha, and test_tag_is_reconfirmed_before_each_irreversible_step.
|
|
||
| publish: | ||
| name: Publish candidate | ||
| needs: verify |
There was a problem hiding this comment.
[P1] Publication only depends on this verification run; it does not require a prior successful rehearsal for the same source/workflow/candidate. That leaves #343/#355's explicit acceptance criterion unenforced, and the rehearsal workflow itself never performs the required deliberate failure-path exercise. Persist a rehearsal attestation keyed to source SHA, workflow revision, and candidate identity, then require it here before publication.
There was a problem hiding this comment.
Fixed in a6eddfd. You are right that I satisfied #355 in the runbook and left the acceptance criterion resting on operator discipline.
stage now requires, before anything mutates:
- a successful
release-rehearsal.ymlrun withhead_sha == source_sha. Because the workflow and the source live in the same tree, matching onhead_shabinds the workflow revision too; - that run's candidate bundle downloaded and
cmpd against the tagged one. Manifest byte-equality binds candidate identity, so a qualification artifact swapped between the rehearsal and the tag is caught even though the source never changed.
On the missing failure-path exercise: every rehearsal now runs a fault-injection drill (inputs.mode == 'rehearsal') that appends a module to a copy of the qualified wheel and asserts the provenance gate rejects it, failing the rehearsal if the tampered artifact is accepted. That replaces the prose runbook step I had written. Covered by test_publication_requires_a_rehearsal_of_this_exact_candidate and test_every_rehearsal_proves_the_provenance_gate_fails_closed.
|
|
||
| - name: Verify configured qualification signature | ||
| env: | ||
| QUALIFICATION_SIGNER_IDENTITY: ${{ vars.SAFETY_QUALIFICATION_SIGNER_IDENTITY }} |
There was a problem hiding this comment.
[P1] The qualification signer identity and issuer are now repository variables, alongside the artifact locations. A write-level actor can substitute fabricated qualification evidence and simultaneously replace the allowlist that authenticates it; source-wheel equality does not protect the safety claims because the legitimate wheel can be reused. Keep signer/issuer trust roots in reviewed code or separately admin-controlled protected configuration, leaving only content-addressed locations mutable.
There was a problem hiding this comment.
Fixed in a6eddfd. This is the finding I most needed — it is a direct consequence of my moving the variables to repository scope, and I had reasoned that source-wheel equality compensated. It does not: the attack reuses the legitimate wheel and forges only the safety claims about it, so the provenance gate sees nothing wrong.
signer_identity and oidc_issuer now come from .github/release-trust-roots.json — reviewed code — and the workflow no longer references vars.SAFETY_QUALIFICATION_SIGNER_IDENTITY or vars.SAFETY_QUALIFICATION_OIDC_ISSUER at all. Only the content-addressed locations remain variables, which is safe because repointing one fails either the signature check against the committed root or the provenance gate.
Two notes. Both values ship as CHANGE_ME and the release fails closed while either is unset — I did not invent a plausible-looking identity, because a wrong trust root is worse than an absent one. Related: none of the SAFETY_QUALIFICATION_* variables are currently set at either scope, so the promotion flow that would produce this identity does not exist yet.
Covered by test_signer_identity_and_issuer_are_not_mutable_configuration and test_trust_roots_are_committed_and_fail_closed_while_unset.
| if not manifest_path.is_file(): | ||
| raise ConfigError(f"Candidate manifest not found: {manifest_path}") | ||
| actual_sha256 = sha256_file(manifest_path) | ||
| if expected_sha256 and actual_sha256 != expected_sha256: |
There was a problem hiding this comment.
[P1] This truthiness check fails open for expected_sha256 == "". Missing or secret-redacted job outputs become empty strings, and the workflow still invokes --expected-sha256 "", disabling the only trusted-channel binding. Require the CLI argument for publication verification, validate exactly 64 hexadecimal characters, and compare whenever the option was supplied using is not None; add omitted, empty, malformed, and mismatch tests.
There was a problem hiding this comment.
Fixed in a6eddfd. Straightforward fail-open on my part — if expected_sha256 and ... reads "no value supplied" as "no check requested", which is exactly backwards for a security binding.
--expected-sha256 is now required=True on the CLI, and the comparison runs whenever the argument is not None, after validating it against \A[0-9a-f]{64}\Z. An empty or malformed value raises rather than skipping.
Tests: test_a_missing_or_malformed_expected_digest_never_passes (parametrised over empty, non-hex, and too-short), test_the_expected_digest_flag_is_mandatory_on_the_command_line (argparse rejects omission), and the existing mismatch tests for swapped assets and rewritten manifests.
| if not files: | ||
| state = "absent" | ||
| else: | ||
| digests = {str(item.get("digests", {}).get("sha256", "")) for item in files} |
There was a problem hiding this comment.
[P1] Digest membership is not sufficient to classify the release as identical. A matching wheel plus a divergent sdist/second wheel, a renamed file, or a matching-but-yanked record all pass and cause publication to be skipped/finalized. Since this workflow publishes exactly one wheel, require the exact URL set: one unyanked bdist_wheel with the expected filename and SHA-256; treat extras, malformed 200 responses, renamed/yanked files, and missing fields as divergence/error.
There was a problem hiding this comment.
Fixed in a6eddfd. Membership was the wrong predicate — the consequence is that we skip the upload and finalize over a release we never verified.
_classify_published now requires the exact URL set: exactly one record, packagetype == "bdist_wheel", filename equal to the expected basename, digests.sha256 equal, and yanked is not True. Anything else is published_divergent and fatal. Malformed 200 responses are handled at the fetch boundary: a non-dict payload or a urls value that is not a list of objects raises rather than being read as an empty file list.
test_anything_but_the_exact_single_wheel_is_divergent covers all seven cases you named — divergent sdist alongside the wheel, second wheel, renamed file, yanked record, wrong packagetype, missing digests, and different bytes.
| maturity="--prerelease" | ||
| fi | ||
| echo "RELEASE_MATURITY=${maturity}" >> "${GITHUB_ENV}" | ||
| if gh release view "${RELEASE_TAG}" > /dev/null 2>&1; then |
There was a problem hiding this comment.
[P1] Any existing release—including an already-published one—is treated as a recoverable draft and mutated with --clobber before PyPI divergence is checked. A moved-tag rerun can replace public GitHub assets with candidate B and then fail because PyPI holds A, leaving the registries inconsistent. Query isDraft/isImmutable and exact asset digests first: published+identical should no-op, published+divergent must fail untouched, and only drafts may be repaired.
There was a problem hiding this comment.
Fixed in a6eddfd. The ordering was wrong and the consequence you describe — public GitHub assets replaced with candidate B while PyPI still holds A — is worse than the failure it was trying to recover from.
Index classification moved ahead of every release mutation, so a divergent version now fails with both registries untouched. The release handling then branches on isDraft:
- absent → create draft;
- draft → repair with
--clobber; - published → never mutated. It downloads the published assets and re-runs
verify-manifestagainst them with the job-output digest, permitting only the two signature bundles by explicit name, then reports and stops.
Covered by test_index_state_is_classified_before_any_release_is_mutated and test_only_draft_releases_are_ever_mutated.
| - name: Install | ||
| - name: Install publication tooling | ||
| run: | | ||
| python -m pip install -e ".[dev]" |
There was a problem hiding this comment.
[P1] This job already holds id-token: write and contents: write, then installs the editable project plus ranged dev dependencies before verifying the handoff. A compromised build backend/dependency can request the PyPI OIDC token directly. Make the OIDC publisher a minimal job with no editable/dev installation or untrusted checkout execution, using a hash-locked, full-SHA-pinned publication implementation.
There was a problem hiding this comment.
Fixed in a6eddfd. Publication is now split so that write and OIDC authority are never held by the same job:
| job | permissions | installs |
|---|---|---|
stage |
contents: write, actions: read |
project (no OIDC) |
publish |
id-token: write only |
hash-locked toolchain only |
finalize |
contents: write |
project (no OIDC) |
publish has no contents permission, performs no checkout, and installs nothing but constraints/release-publish.txt — sigstore and uv with their full transitive closure pinned and hashed (uv pip compile --universal --generate-hashes), installed with --require-hashes. The lockfile is fetched by immutable commit SHA rather than checked out, so no project code enters the token-bearing job.
To make that possible the publication-side scripts (release_publication.py, release_sbom.py, and the shared _release_support.py) are now standard-library only — no agents_shipgate, no packaging, no pydantic chain. Verified by running them with an empty environment.
Covered by test_write_and_oidc_authority_are_never_held_together, test_the_token_bearing_job_installs_no_project_code, and test_the_publication_toolchain_is_hash_locked.
|
|
||
| base = directory or manifest_path.parent | ||
| errors: list[str] = [] | ||
| for asset in manifest.get("assets", []): |
There was a problem hiding this comment.
[P2] The handoff is open-world: only listed assets are checked, while extra files in the directory are accepted and later uploaded through dist/*. An intact manifest plus an unlisted sdist/executable passes verification. Require the recursive directory set to equal {manifest} ∪ {listed assets}, reject symlinks/non-regular entries, and upload an explicit allowlist; validate the exact remote name+digest set before finalization.
There was a problem hiding this comment.
Fixed in a6eddfd. Agreed — an intact manifest beside an unlisted sdist was enough, because dist/* uploaded whatever was there.
verify_manifest now requires the recursive directory set to equal {manifest} ∪ {listed assets}, and rejects symlinks and non-regular entries before hashing. The only permitted additions come from an explicit --allow allowlist, used solely for the two signature bundles produced after the manifest is sealed — an allowlist rather than an escape hatch.
Uploads use an explicit assets array instead of dist/*, and finalize validates the exact required remote asset set (now including both signature bundles) before undrafting.
Covered by test_handoff_rejects_files_the_manifest_does_not_list and test_release_assets_are_uploaded_from_an_explicit_allowlist.
|
|
||
| _normalise_external_references(document, wheel_path.name) | ||
| metadata = document.setdefault("metadata", {}) | ||
| metadata["component"] = { |
There was a problem hiding this comment.
[P2] cyclonedx-py already emits the installed package component and keys its dependency node by that component's bom-ref. Replacing only metadata.component with this invented ref leaves a duplicate Agents Shipgate component and no dependency graph node for the declared subject. Promote/reuse the original root component/ref, augment it with the wheel hash/property, remove it from components, and assert exactly one dependency node for the metadata subject.
There was a problem hiding this comment.
Fixed in a6eddfd. Correct — inventing a ref left the wheel described twice and the dependency graph pointing at a node that was no longer the declared subject.
_promote_subject_component now finds CycloneDX's own component for the wheel, removes it from components, augments it with the wheel SHA-256 and the filename property, and installs that object — original bom-ref intact — as metadata.component. Verified against a real generated SBOM: subject ref agents-shipgate==0.16.0b7, absent from components, and exactly one dependency node.
verify_release_sbom also enforces both properties now, so a regression is caught rather than merely avoided: duplicate subject → rejected; dependency-node count for the subject ref != 1 → rejected (skipped when the document has no graph at all). Covered by test_sbom_describing_the_subject_twice_is_rejected and test_sbom_subject_must_have_exactly_one_dependency_node.
|
|
||
| built_sha256 = _sha256_file(built_path) | ||
| qualified_sha256 = _sha256_file(qualified_path) | ||
| if built_sha256 == qualified_sha256: |
There was a problem hiding this comment.
[P2] The byte-equality fast path returns before validating wheel filenames. Identical bytes renamed to agents_shipgate-9.9.9-py2-none-any.whl are accepted even though filename version/compatibility tags affect installer selection. Parse both basenames with packaging.utils.parse_wheel_filename before hashing and require identical distribution, version, build, and tag sets—preferably the exact expected basename.
There was a problem hiding this comment.
Fixed in a6eddfd. Good catch — the fast path meant the strongest-looking outcome did the least filename checking.
compare_wheels now calls _assert_compatible_filenames before any hashing: both basenames go through packaging.utils.parse_wheel_filename, and distribution, version, build tag, and the full tag set must all match, with InvalidWheelFilename surfaced as a failure rather than swallowed. Since verify_wheel_provenance runs only in the read-only verification job, using packaging there is fine — the dependency-free constraint applies to the publication-side scripts.
I kept it as an equality check between the two basenames rather than pinning an expected literal, because the qualified filename already has to match SAFETY_QUALIFICATION_WHEEL_FILENAME and the version is bound to the tag upstream; happy to pin the exact expected basename here as well if you would rather have the redundancy.
Covered by test_identical_bytes_under_a_different_filename_are_rejected, parametrised over a changed compatibility tag, version, and distribution name.
Addresses all ten findings on #359. P1 — a moved tag could make the pipeline build one commit while claiming another: verification now runs against the immutable `github.sha`, keys every binding to the SHA it resolved with `git rev-parse HEAD`, re-peels the tag against the remote before each irreversible step, and creates the release with `--verify-tag`. P1 — signer identity and OIDC issuer were mutable variables, so one actor could substitute fabricated qualification evidence and replace the allowlist that authenticates it in a single unreviewed step. They move to `.github/release-trust-roots.json` (reviewed code, fail-closed while unset); only content-addressed locations stay in variables. P1 — the token-bearing job installed the editable project and ranged dev extras before verifying anything, so a compromised dependency could request the PyPI OIDC token. Publication is now split so write and OIDC are never held together: `publish` has `id-token` only, checks out no project code, and installs a hash-locked toolchain with `--require-hashes`. The publication-side scripts are standard-library only to make that possible. P1 — rehearsal was mandatory only in prose. `stage` now requires a successful rehearsal run at the same source SHA whose candidate manifest is byte-identical, and every rehearsal proves the provenance gate fails closed by injecting a tampered wheel and asserting rejection. P1 — `verify-manifest` treated an empty `--expected-sha256` as "no check", so a redacted job output silently disabled the only trusted-channel binding. The flag is now required and must be 64 lowercase hex. P1 — `published_identical` accepted digest membership, so a matching wheel beside a divergent sdist, a renamed file, or a yanked record passed. It now requires exactly one unyanked `bdist_wheel` with the expected filename and digest. P1 — any existing release was clobbered before PyPI divergence was checked. The index is classified first, and only drafts are ever mutated; a published release is verified and left untouched. P2 — the handoff is closed-world (extra files rejected, non-regular entries refused, explicit upload allowlist instead of `dist/*`); the SBOM reuses CycloneDX's own root component and ref instead of inventing one, so the subject is described once and keeps its dependency node; and wheel basenames are parsed and compared before the byte fast path, so identical bytes under a different compatibility tag no longer pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
All ten findings addressed in a6eddfd — CI greenThanks, this was a genuinely sharp review. Three findings were failures of my own reasoning rather than oversights, and I want to name them rather than bury them in a table:
Structural changesPublication is now four jobs, so write and OIDC authority are never held together:
Signer identity and OIDC issuer moved to Two things you should knowI did not invent a signer identity. Related, and worth its own decision: none of the VerificationFull suite green (exit 0), ruff clean, 62 contract tests in One open question, on the P2 wheel-filename check: I compare the two basenames for equality rather than pinning an expected literal, since the qualified filename already has to match Per-finding replies are inline on each thread. |
pengfei-threemoonslab
left a comment
pengfei-threemoonslab
left a comment
There was a problem hiding this comment.
Engineering second-pass review at a6eddfde3ccdeb9e1159916c523ed3a26b1f41a6.
Disposition: changes required before merge. GitHub does not allow the PR author to submit a REQUEST_CHANGES review on their own PR, so this is posted as a formal comment review; the six P1 findings below are merge blockers.
The remediation is substantial and genuinely fixes several prior blockers: immutable source checkout, required manifest digests, closed-world local handoff verification, exact unyanked PyPI matching, committed qualification trust roots, OIDC-job isolation, corrected SBOM subject identity, and wheel-filename validation.
Additional confirmed P2 issues:
- The rehearsal fault drill copies the wheel to invalid
fault-injected.whl, so it fails filename parsing before inspecting the injected member. Preserve the original valid basename in another directory and assert the injected path appears in the mismatch log. - Candidate and signature artifact names are stable across workflow attempts while overwrite is disabled, so “Re-run all jobs” can fail on artifacts retained from the prior attempt.
- Rehearsal can checkout
inputs.ref=B, but publication searches runs by the dispatch run'shead_sha=A; the advertised alternate-ref path cannot satisfy the release precondition. - The payload-equivalent comparator collapses duplicate ZIP members and can accept a malicious duplicate when the opt-in flag is enabled.
- A malformed PyPI HTTP 200 response without
urlsis classified as absent; only an explicit 404 should authorize publication. - Windows SBOM generation selects
Scripts/pythoninstead ofScripts/python.exe.
Validation performed against the exact head: 85 targeted release/action tests passed, Ruff passed, the hash-locked publisher environment resolved, and all current GitHub checks are green. Those checks do not execute the release/rehearsal transaction. Exact-head Agents Shipgate reports review_publishable / human_review_required / review_required, which is expected for release trust-root changes.
| value: ${{ jobs.verify.outputs.manifest_sha256 }} | ||
| artifact_name: | ||
| description: Name of the uploaded candidate bundle. | ||
| value: ${{ jobs.verify.outputs.artifact_name }} |
There was a problem hiding this comment.
[P1] Export source_sha through workflow_call.outputs. The job computes it at line 77, but reusable-workflow callers can consume only outputs declared in this public map. Every needs.verify.outputs.source_sha use in release.yml therefore receives an undefined/empty value, and the stage tag comparison fails on every release. Add source_sha.value: ${{ jobs.verify.outputs.source_sha }} here and a contract test that every caller-consumed output is publicly exported.
There was a problem hiding this comment.
Fixed in 1cad109. You are right, and this one was a live defect rather than a weakness — I added source_sha to the job's outputs map and never to workflow_call.outputs, so every needs.verify.outputs.source_sha would have resolved to the empty string and the stage tag comparison would have failed on the first real release. The silent-empty-string behaviour is exactly why it survived my review.
Exported now, with a description. All output values also re-point at the new artifact job (see the reply on line 282).
On the contract test: test_every_caller_consumed_output_is_publicly_exported regexes needs\.verify\.outputs\.(\w+) out of release.yml and asserts the set is a subset of the declared workflow_call.outputs keys, with a guard that the consumed set is non-empty so it cannot pass vacuously. That covers the class rather than this instance.
| --built source-build/*.whl \ | ||
| --qualified "${QUALIFIED_WHEEL}" \ | ||
| --source-commit "${SOURCE_SHA}" \ | ||
| --report provenance.json |
There was a problem hiding this comment.
[P1] Keep the source-to-wheel binding fresh until handoff sealing. After this equality check, candidate tests, pytest plugins, dev dependencies, and pip-audit execute while QUALIFIED_WHEEL remains writable and exposed through GITHUB_ENV. The later SBOM/manifest steps seal whatever bytes remain, without repeating equality, so post-check mutation can publish a wheel absent from the tagged source while the stale provenance report still claims equality. Perform final qualification/equality verification and handoff assembly on a fresh runner/job that executes no candidate test or dependency code.
There was a problem hiding this comment.
Fixed in 1cad109. Agreed, and the framing is the useful part: the equality check was a point-in-time assertion about a file that stayed writable for the next seven minutes, while the thing being sealed was whatever remained afterwards.
release-verify.yml is now two jobs:
tests— lint, compile, schema check, static-only lint, the suite,pip-audit. Never downloads the qualified wheel;QUALIFIED_WHEELis not in its environment at all.artifact(needs: tests) — build, download, signature, qualification, provenance, SBOM, handoff seal, upload. Runs no pytest and no dependency audit.
So no candidate test code, plugin, or conftest executes on the runner that holds the wheel. The handoff step additionally re-runs verify_wheel_provenance.py on the exact bytes immediately before the copy and manifest, since that is the last point where a substitution would still be invisible downstream.
Covered by test_the_handoff_is_sealed_by_a_job_that_runs_no_candidate_tests (asserts the split, needs, absence of pytest/pip_audit in the sealing job, and absence of QUALIFIED_WHEEL in the suite job) and test_the_binding_is_reasserted_on_the_exact_bytes_being_sealed.
Timeouts are now per job and re-derived: tests 20 min (407s observed), artifact 15 min (~4 min, dominated by the isolated SBOM install).
| --title "${GITHUB_REF_NAME}" \ | ||
| --notes "Agents Shipgate ${GITHUB_REF_NAME}" | ||
| set -euo pipefail | ||
| attached="$(gh release view "${RELEASE_TAG}" --json assets --jq '.assets[].name')" |
There was a problem hiding this comment.
[P1] Finalization must verify remote bytes and the exact asset set, not names. Draft repair clobbers expected names but leaves stale/unlisted assets; during the approval window expected assets can also be replaced. This membership-only check neither rejects extras nor verifies digests/signatures, and it omits safety-qualification.sigstore.json. Immediately before undrafting, assert the release is still a draft, download every remote asset, validate the trusted manifest digest and every listed byte, require exactly the candidate assets plus the two generated signature bundles, and verify those bundles.
There was a problem hiding this comment.
Fixed in 1cad109. Name membership was not evidence, and you are right that the omission of safety-qualification.sigstore.json was itself a symptom — a hand-maintained name list drifts from the manifest.
finalize now, immediately before undrafting:
- asserts the release is still a draft (and again inside the undraft step);
gh release downloads every remote asset;- runs
verify-manifest --directory remote --expected-sha256 "${MANIFEST_SHA256}", which is closed-world — it re-derives every listed byte and rejects anything unlisted, so stale assets left by draft repair and replacements made during the approval window both fail; - permits exactly two additions, by name:
<wheel>.sigstore.jsonandagents-shipgate-sbom.json.sigstore.json; - verifies both bundles with
sigstore verify identityagainsthttps://github.com/<repo>/.github/workflows/release.yml@refs/tags/<tag>.
The required-set question is now answered by the manifest rather than a literal list, so it cannot omit an asset again.
Covered by test_finalisation_verifies_remote_bytes_not_asset_names and test_finalisation_refuses_to_mutate_a_release_that_is_no_longer_a_draft.
| env: | ||
| GH_TOKEN: ${{ github.token }} | ||
| RELEASE_TAG: ${{ needs.verify.outputs.release_tag }} | ||
| run: gh release upload "${RELEASE_TAG}" signatures/*.sigstore.json --clobber |
There was a problem hiding this comment.
[P1] Do not mutate an already-published release on rerun. Stage's published branch says it leaves a verified release untouched, but publish always re-signs and this finalizer always --clobbers the public bundles. Ordinary reruns therefore replace nondeterministic attestations, and a failed clobber can leave a public asset missing. Propagate an explicit release_state; if stage verified an already-published release, skip signing, attachment, and finalization entirely. Only confirmed drafts should be mutable.
There was a problem hiding this comment.
Fixed in 1cad109. Correct — stage's "leaving it untouched" was true of stage and false of the two jobs after it, which is worse than not claiming it.
stage now emits an explicit release_state (absent | draft | published), and both publish and finalize carry if: needs.stage.outputs.release_state != 'published'. A verified, already-published release ends the run after staging: no re-signing, no attachment, no undrafting. Sigstore bundles are not reproducible, so re-signing would have replaced public attestations on every ordinary rerun.
One case your comment made me look at: a published GitHub release with an absent index. That is registries disagreeing, not a recoverable state, so stage now fails explicitly rather than treating it as a completed transaction.
Covered by test_a_completed_transaction_is_left_entirely_alone and test_registry_disagreement_stops_the_release.
| # a re-run after a post-publication failure looks like. `stage` | ||
| # hard-fails if the version exists with anything else, so this can never | ||
| # quietly succeed against a divergent artifact. | ||
| if: needs.stage.outputs.should_publish == 'true' |
There was a problem hiding this comment.
[P1] Reclassify PyPI state inside every publish attempt. This condition consumes stage's cached pre-approval result. If uv publish succeeds and the later signature-artifact upload fails, “Re-run failed jobs” retains should_publish=true, retries the immutable version, and cannot reach recovery/finalization. An external publication during environment approval has the same stale-decision failure. Query the exact index state immediately before each upload attempt: publish only when absent, skip-and-continue when identical, and fail when divergent.
There was a problem hiding this comment.
Fixed in 1cad109. The stale-decision framing is right: should_publish was computed before environment approval and then consumed as though it were still current.
The if: is gone. The upload step now queries the index itself, immediately before acting, and branches on the exact state: absent → uv publish; published_identical → skip and continue to finalisation; anything else → fail. So the post-upload-failure rerun you describe now completes the transaction instead of retrying an immutable version, and an external publication during the approval window is caught at the moment it matters rather than assumed away.
This works in the token-bearing job because release_publication.py and _release_support.py are standard-library only — they are fetched by immutable commit SHA alongside the lockfile, so the job still checks out no project code.
Covered by test_index_state_is_reclassified_inside_the_publish_attempt, which asserts the step has no if: and handles all three outcomes explicitly.
| done | ||
| echo "OK: all required assets are attached to ${RELEASE_TAG}." | ||
|
|
||
| - name: Finalise GitHub release |
There was a problem hiding this comment.
[P1] Rebind the tag immediately before finalization. The last tag check occurs in the earlier publish job. With no tag-targeting ruleset and immutable Releases currently disabled, the tag can move/delete after PyPI receives source A but before GitHub undrafts the release, causing GitHub source archives to resolve to B. Re-peel and compare the tag before any finalizer mutation and again immediately before undrafting, and require protected/immutable v* tags as a repository prerequisite.
There was a problem hiding this comment.
Fixed in 1cad109. Agreed on the window — after this point PyPI holds source A, so a tag moved to B makes GitHub's archives and the release resolve to code the index never served.
finalize re-peels the tag against the remote twice: once before any mutation (before attaching signatures), and again inside the undraft step, immediately before gh release edit --draft=false. Both compare against needs.verify.outputs.source_sha and fail on a moved or deleted tag, since an empty peel also mismatches. The undraft step re-checks isDraft at the same moment.
On the prerequisite: the runbook now states that v* tags must be protected by a ruleset forbidding updates and deletions, and says plainly why — the re-peel checks detect a moved tag, and detection is a weaker control than prevention, with a residual window between GitHub resolving a ref and acting on it. That is a repository setting I cannot apply from here; flagging it alongside the unset SAFETY_QUALIFICATION_* variables as configuration this PR depends on.
Covered by test_the_tag_is_rebound_before_finalisation_and_again_before_undrafting.
…dings The first finding is a defect I introduced in a6eddfd: `source_sha` was declared as a job output but never added to `workflow_call.outputs`, so every `needs.verify.outputs.source_sha` in release.yml resolved to the empty string and the stage tag comparison would have failed on the first real release. Now exported, with a contract test asserting every caller-consumed output is publicly declared — the class of bug, not just this instance. Verification splits into `tests` and `artifact`. Sealing the handoff in the same job that ran pytest meant the qualified wheel stayed writable, with its path in GITHUB_ENV, while candidate tests and plugins executed; a test could replace the bytes after the equality check and before sealing, and the provenance report would still claim equality. The `artifact` job runs no suite and no audit, and re-asserts the binding on the exact bytes it seals. Publication no longer trusts stage's pre-approval snapshot. The index is reclassified inside the upload attempt, so a re-run after a post-upload failure skips and continues instead of retrying an immutable version and never reaching recovery. An already-published release is now left entirely alone: stage records `release_state`, and `publish` and `finalize` do not run when it is `published`. Re-signing would replace public, non-reproducible Sigstore bundles for no benefit. A published release with an absent index is treated as registries disagreeing and stops the run. Finalization verifies bytes rather than names: it re-peels the tag before any mutation and again immediately before undrafting, requires the release to still be a draft, downloads every remote asset and re-derives it against the trusted manifest digest closed-world, and verifies both signature bundles against the release workflow's Sigstore identity. It also fetches its stdlib-only scripts by immutable SHA instead of checking out, so no project code runs in a job that mutates a public release. Runbook documents protected `v*` tags as a repository prerequisite: the re-peel checks detect a moved tag, but detection is weaker than prevention. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…e-milestone-1-00334c # Conflicts: # CHANGELOG.md
Round 2 addressed in 1cad109, merged with main in a322afc — CI greenThe first finding was a live defect I introduced in the previous round, not a weakness: The TOCTOU on the sealed handoffThe framing that mattered: the equality check was a point-in-time assertion about a file that stayed writable for the next seven minutes, and what got sealed was whatever remained. Remaining four
Configuration this PR depends on and I cannot apply
VerificationFull suite green, ruff clean, 71 contract tests. Merged Worth noting for anyone reading the check history: 1cad109 reported no checks at all. That was not a CI failure — the branch had gone |
1 participant
Implements every issue in the v0.16.0 milestone. Closes #342, #343, #344, #355, #356.
#341 was descoped from the milestone by product decision on 2026-08-09 and relabelled P2; this PR still ships its decision brief.
Why one PR
All five rewrite
.github/workflows/release.yml. Split into stacked PRs they conflict on every rebase, and the resulting pipeline is only reviewable as a whole — the job split (#343) is what gives the rehearsal (#355) something to call, and the provenance gate (#342) is what makes the reviewer approval in #343 meaningful.The gap this closes
The pipeline established three bindings: tag ↔
pyproject.tomlversion, qualification payload ↔ wheel bytes, and tag ↔ the wheel's ownMETADATAversion. None tied the shipped bytes back to a source tree. "Lint and test" ran against the checkout; "Publish to PyPI" shipped a wheel downloaded from a repository-variable URL. Any wheel declaringName: agents-shipgateand the rightVersionpassed every check.What changed
#342 — source ↔ wheel binding. Verification rebuilds from the tagged checkout and requires byte equality with the qualified wheel before
uv publish. The published artifact is still the signed, qualified wheel; the rebuilt one is only a comparison reference.Byte equality is only achievable because the build backend is now pinned (
constraints/release-build.txt) — wheels recordGenerator: hatchling <version>in.dist-info/WHEEL, so an unpinnedhatchling>=1.31.0alone changes the archive and would have made this gate fail on legitimate releases. Verified empirically: two builds are byte-identical, andPIP_CONSTRAINTdemonstrably binds (constraining to a conflicting version fails resolution rather than being ignored).Per the PM note on #342, the normalized unpacked-content comparison exists as the approved interim control, but behind an explicit
--allow-payload-equivalentflag — a container-metadata-only difference still fails by default, so the weaker bar can never be taken silently.#343 — separated jobs, recoverable transaction. Read-only
verify(reusable workflow,contents: read, no OIDC) hands off a content-addressed candidate bundle. The manifest digest travels through the job-output channel rather than the artifact store, so swapping an artifact — or rewriting the manifest to agree with the swap — is detected.publishholds the only write/OIDC permissions, drafts the GitHub Release with authoritative assets before the PyPI upload, and finalizes only after asset validation. Concurrency is serialized across the PyPI project, not per tag.The upload is idempotence-aware via
pypi-state:absent→ publish;published_identical→ a re-run completing an interrupted transaction, upload skipped;published_divergent→ always fatal. An unreachable index is never read as permission to upload. All three states verified against the live PyPI index.#356 — wheel-scoped SBOM.
cyclonedx-py environmentover.[dev]inventoried pytest, ruff, twine, Sigstore and CycloneDX itself — a signed attestation about software the user never receives. Now generated from an isolated runtime-only install (19 runtime packages, 0 dev tools) and bound to the wheel digest. It also normalizes away thefile://build-machine path CycloneDX embeds, which leaked runner filesystem layout into a published artifact and made the signed SBOM non-deterministic.#344 — deterministic test workload. CI's
-n auto,perfexcluded (latency stays a merge-time gate), adapter static-only lint kept as its own fail-fast step, coverage floor unchanged at 85.#355 — rehearsal.
workflow_dispatchentry point running the identical path. Structurally incapable of publishing for three independent reasons: no publication job in the file,permissions: contents: readcaps the token so tag/release creation fails, and noid-token: writemeans Trusted Publishing cannot mint a token.Two decisions worth reviewer attention
The
pypireviewer gate moved from the whole run to thepublishjob. Reviewers previously approved before verification ran — approving blind. They now approve with the readiness summary in hand. The consequence is that the six qualification variables must be readable by the unattended verification job, so they need to be defined at repository scope (they arevars, not secrets — URLs and identities). This is a repo-settings action before the next release; seedocs/release-runbook.md.This does weaken who can change those variables relative to environment scoping. Accepted trade, because the control that mattered is now stronger: a tampered wheel URL fails the source-binding gate instead of reaching PyPI. Flagging it explicitly rather than burying it.
Pre-release tags are no longer promoted to Latest. The repo tags betas (
v0.16.0b7); finalizing those with--latestwould advertise a pre-release as current.#341: descoped, brief still shipped
The pre-1.0 evidence bar is a product/security decision reserved for a named human owner, and the issue explicitly forbids an agent choosing a weaker bar because the corpus cannot meet the approved one. It is now P2 and outside the milestone, so the milestone no longer waits on it.
docs/release-evidence-policy-decision.mdstill ships: both routes, the five cross-checked places the bar is defined, the invariants that stay non-negotiable under either route (zero unsafe auto-passes, per-case receipts, holdout, static-only), and what a rehearsal must prove.No code path changed with the descope. The verifier still requires the full 100-case artifact, so a
v*tag fails closed at the qualification step until a conforming signed artifact exists or an alternative policy is approved. Descoping changed what the milestone tracks, not what the pipeline enforces — the brief says so explicitly so nobody reads "not blocking" as "not enforced". In practice a green rehearsal is the check: it cannot pass without a conforming qualification artifact.Verification
-n auto -m "not perf"), ruff clean, schemas in sync.tests/test_release_pipeline.pycovering all five issues.published_divergentfires against the real PyPI 0.8.0 release."python -m build" not in workflow(release security: bind the qualified wheel to the tagged source tree #342 requires the in-job build) and thecyclonedx-py environmentassertion (release: generate and sign an SBOM for the shipped wheel only #356 replaces it).Note: PyPI's latest published version is 0.8.0 while
pyproject.tomlis at0.16.0b7— this pipeline has not published in a long time, consistent with #342's observation that these steps have never run.The
timeout-minutes: 25is derived from measured CPU time (~710s scaled to a 2-core runner) rather than an observed CI rehearsal, which is not obtainable until this merges and a rehearsal runs. The runbook makes re-deriving it from the first real rehearsal an explicit step.🤖 Generated with Claude Code