Repository navigation
Conversation
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
left a comment
There was a problem hiding this comment.
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.
| } | ||
|
|
||
| export async function hashUrl(url, fetchImpl = fetch) { | ||
| return hashResponse(await fetchImpl(url)); |
|
frendly ping, any update |
|
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 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. |
|
Thanks for the straight answer, and there's nothing to apologise for — you closed #913 in under two days and 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 I don't think the job graph supports that reason. 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 What I'd put up once you open the issue:
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. |
Closes #913.
What
SHA256SUMSonly ever covered the standalone tarballs and the win32 zip, because thepublishjob generates it from the artifacts that exist at that point in the chain. The.deb/.rpm/AppImage assets ship their own.sha256sidecars instead, and the two snaps andlibredb-studio-<version>.cdx.jsonended 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.mjsturns the releases API's asset list into asha256sum-format file:SHA256SUMSitself and the<artifact>.sha256sidecars are excluded, so the sidecars keep their current job and the combined file does not hash checksums.sha256digest, 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 localsha256sumof a download produces.--existing). If widening would alter a hash thatnpx, Homebrew, winget or Chocolatey may already rely on, the script fails and the release does not publish with it.docs/DISTRIBUTION.mdis 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.snaprelease asset ships no sidecar.Behaviour change
SHA256SUMSgains 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,sha256sumformatting, 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)
bun tests/unitin my sandbox leaves four failing files —db/sqlite-driver.test.ts(nonode:sqlitebuiltin),docker-entrypoint.test.ts,launcher-utils.test.tsandtest-runner-cli.test.ts(POSIX permissions / SIGINT, running as root). All four fail identically at81d382fwith this change absent, so they are environment-limited here rather than related.bun run buildnot run locally; nothing here touches the app build.