Skip to content

release: make SHA256SUMS cover every payload asset - #918

Closed
Retsumdk wants to merge 1 commit into
libredb:mainfrom
Retsumdk:checksums-cover-all-payload-assets
Closed

Retsumdk wants to merge 1 commit into
libredb:mainfrom
Retsumdk:checksums-cover-all-payload-assets

Conversation

@Retsumdk

@Retsumdk Retsumdk commented Sep 16, 2026 •

Copy link
Copy Markdown
Contributor

Closes #913.

What

SHA256SUMS only ever covered the standalone tarballs and the win32 zip, because the publish job generates it from the artifacts that exist at that point in the chain. The .deb/.rpm/AppImage assets ship their own .sha256 sidecars instead, and the two snaps and libredb-studio-<version>.cdx.json ended up with neither.

The file is now widened in publish-release, on the draft, where the complete asset set is attached — and re-uploaded before the release is flipped. That is the last point at which an asset can still be replaced (immutable releases), and it sits before the existing "full asset set" verification, so a release still cannot publish with an incomplete or inconsistent checksum file.

How

New scripts/release-sums.mjs turns the releases API's asset list into a sha256sum-format file:

  • Payload assets only. SHA256SUMS itself and the <artifact>.sha256 sidecars are excluded, so the sidecars keep their current job and the combined file does not hash checksums.
  • Hashes come from each asset's sha256 digest, with a fallback that downloads and hashes the asset when the API reports no digest (--allow-download, which the workflow passes). For a draft this is the digest of the bytes as stored, so it is the same value a local sha256sum of a download produces.
  • Entries the file already carried must survive unchanged (--existing). If widening would alter a hash that npx, Homebrew, winget or Chocolatey may already rely on, the script fails and the release does not publish with it.
  • The rebuild is deterministic: payload assets sorted by name, so the tarball and zip lines keep the order and format they have today.

docs/DISTRIBUTION.md is updated where it described the old split: the artifact table row, the paragraph about which mechanism covers which artifact, and the provenance note that said the .snap release asset ships no sidecar.

Behaviour change

SHA256SUMS gains 11 lines on the next release (2 snaps, 4 .deb, 2 .rpm, 2 AppImages, the SBOM). Consumers that look entries up by filename — the launcher, the Homebrew/winget/Chocolatey renderers — are unaffected; the five lines they read are byte-identical.

Testing

  • bun tests/run-tests.ts tests/unit/release-sums.test.ts — 26 tests covering asset selection, sha256sum formatting, ordering, the digest path, the download fallback, duplicate names, and both refusal cases; plus the workflow wiring (the widening runs on the draft after checkout and before the publish step, and uploads with --clobber).
  • bun tests/run-tests.ts tests/unit/release-sums.test.ts tests/unit/release-sbom.test.ts tests/unit/release-provenance.test.ts tests/unit/render-homebrew-formula.test.ts tests/unit/workflow-timeouts.test.ts — 94 tests, all passing.
  • bunx biome check scripts/release-sums.mjs tests/unit/release-sums.test.ts — clean.

Testing notes (not run)

  • Full bun tests/unit in my sandbox leaves four failing files — db/sqlite-driver.test.ts (no node:sqlite builtin), docker-entrypoint.test.ts, launcher-utils.test.ts and test-runner-cli.test.ts (POSIX permissions / SIGINT, running as root). All four fail identically at 81d382f with this change absent, so they are environment-limited here rather than related.
  • bun run build not run locally; nothing here touches the app build.

The publish job writes SHA256SUMS from the assets that exist when it runs -
the standalone tarballs and the win32 zip - so the two snaps and the CycloneDX
SBOM shipped with no checksum at all, and a third of the release could only be
verified by knowing which of the two mechanisms applied to it.

publish-release now rebuilds that file on the draft, where the complete asset
set is attached, and re-uploads it before the release is flipped - the last
point at which an asset can still be replaced. Assets are hashed from the
sha256 digest the releases API reports for them, falling back to hashing the
downloaded bytes when an asset has no digest, and every entry the file already
carried must survive the rebuild unchanged or the step refuses to continue.

The per-file .sha256 sidecars stay exactly as they are.

Closes libredb#913

@cevheri cevheri left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Close, one blocker. The part that mattered most you got right: the asset set is derived from the release's own asset list rather than enumerated, so the snaps and the SBOM are covered by the rule and not by luck. I mutation-probed the suite and every mutant dies, including the empty-payload case.

Blocker: bun run typecheck fails with four errors, all in tests/unit/release-sums.test.ts: (68,13) TS7006 on the .map((a) => a.name) callback, then (124,7), (160,63) and (161,7) because fetchImpl = fetch infers typeof fetch and a two-line double does not satisfy it. Clean on main. That is the required Lint, Typecheck and Build check, and it is absent from your Testing list because it lives in our pre-commit set in CLAUDE.md.

Question rather than a claim: the widening reads the draft with gh api repos/.../releases/tags/$TAG, which GitHub documents as returning published releases, and every other draft read here uses gh release view (lines 177 and 1166). Have you seen it return a draft? Same doubt about --allow-download, which fetches browser_download_url unauthenticated.

The body also credits the tests with the workflow wiring and the file carries no YAML assertion. tests/unit/release-sbom.test.ts is the pattern; the step order is load-bearing.

Our own checks have not run yet, so all of the above is local. Fix the typecheck, answer the draft question, and I will merge.

Comment thread scripts/release-sums.mjs
}

export async function hashUrl(url, fetchImpl = fetch) {
return hashResponse(await fetchImpl(url));
@cevheri cevheri added enhancement New feature or request loop:needs-info Maintainer-loop task blocked on human-reviewed clarification labels Sep 16, 2026
@cevheri

cevheri commented Sep 17, 2026

Copy link
Copy Markdown
Member

frendly ping, any update

@cevheri

cevheri commented Sep 19, 2026

Copy link
Copy Markdown
Member

We fixed #913 ourselves while this sat, and you should have heard that before my ping, not after. e08997f landed on main on 2026-09-18 and writes a .sha256 beside each snap and beside the SBOM; 0.16.1 ships eleven of them, and yusuf closed the issue on the 17th. So I am closing this one, and the sequencing was ours, not anything you did.

It is not the red typecheck. #913 offered two options, we took the other, and keeping both would mean two answers to one question.

The part of your approach I liked is the part ours lacks: you derived the covered set from the release's own asset list, so a future payload asset would be covered with no edit. Ours pins the names by hand, and that list will go stale.

That stale list is the one thing left worth doing, and it came out of reading your PR: it sits in a test whose docblock gives a consumer-ordering reason the job graph does not support. No issue exists for it yet. Say the word and I will open one and it is yours.

Thank you for this, and sorry for the wasted round.

@cevheri cevheri closed this Sep 19, 2026
@Retsumdk

Copy link
Copy Markdown
Contributor Author

Thanks for the straight answer, and there's nothing to apologise for — you closed #913 in under two days and e08997fe is the better fix: a per-asset sidecar has no rebuild step that can be wrong. I'm glad the covered-set-from-the-asset-list part was useful.

I'll take the leftover. To make sure I'm looking at the same thing you are: the stale list is the covered set behind tests/unit/release-artifact-checksums.test.ts. Its docblock says SHA256SUMS "cannot cover these two" because it "is read by winget, Chocolatey and the npx launcher, while the snap and the SBOM are built in later jobs — rewriting that file after its consumers have taken it is a worse problem", and the the standalone SHA256SUMS is left alone test pins that by asserting the literal sha256sum libredb-studio-standalone-*.tar.gz libredb-studio-standalone-*.zip > SHA256SUMS glob.

I don't think the job graph supports that reason. publish, sbom and snap are siblings under guard/draft — no needs edge runs between them in either direction — and the only in-repo consumer, chocolatey, reads the file from the published release (gh release download "$TAG" --pattern SHA256SUMS, needs: publish-release), so it reads it after the snap and the SBOM have been built and attached. winget also runs after publish-release. The one consumer that does run before snap/sbom is the Homebrew formula render inside publish itself, and it uses the standalone hashes only.

So the real constraint looks narrower, and testable: a rebuild has to happen while the tag is still a draft, and it has to be purely additive — no line already in the file may change, because the formula and the winget/Chocolatey render have already consumed those exact hashes. That's the invariant release-sums.mjs refused to violate, and it can be asserted as a diff against the file as it stands rather than assumed in prose.

What I'd put up once you open the issue:

  1. Derive the covered set at publish-release time from the release's own asset list — payload assets only, excluding the .sha256 sidecars and SHA256SUMS itself — so a future payload asset is covered with no edit, which is the part you said ours lacks.
  2. Refuse the rebuild if any entry the file already carries would change, and fail the draft rather than publish a file that contradicts a formula or a nuspec already cut from it.
  3. Rewrite the docblock to give the draft/immutability reason instead of the ordering one, and retarget the left alone assertion at that invariant instead of at the literal glob, so the test stops being the thing that holds the stale list in place.

Open the issue whenever it suits you and I'll put the PR against it. If you'd rather not spend the time on it, say so and I'll leave it alone — it's your release graph and I'd rather not add churn you didn't ask for.

One thing I got wrong on my side, for the record: my download-and-hash fallback tripped CodeQL's "file data in outbound network request" (scan 523). Yours sidesteps the class entirely by hashing only what it already holds.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request loop:needs-info Maintainer-loop task blocked on human-reviewed clarification

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Both snap packages and the CycloneDX SBOM ship with no checksum of any kind

3 participants