Skip to content

release-check: the identity's payload is checked by content, and the build writes the fact (Fixes #1557) - #1579

Closed
bong-water-water-bong wants to merge 2 commits into
mainfrom
1bit/1557-payload-checked
Closed

bong-water-water-bong wants to merge 2 commits into
mainfrom
1bit/1557-payload-checked

Conversation

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator

Fixes #1557. The half of #1481 that was kept open on purpose: payload= was the one field of release-identity nothing checked.

The defect, in one line: the engine sha is computed from a file, the payload name is taken from PAYLOAD_NAME in the build environment (container-build.sh:352). One wrong variable and the identity is half true — which is how c32 and c33 shipped payload=lemonade-onebit-rc-2026-11-05-c26c beside later payloads.

What this adds: a pure verdict + a row, identity-payload:

input verdict
claim matches a tarball stem in /data/SHA256SUMS PASS
claim names something else FAIL — the c32/c33 shape
claim empty or unknown while a payload is present FAIL — a declaration that cannot be true
no payload tarball named in the table SKIP — never a pass by absence

Why that authority: /data/SHA256SUMS is on the image and names the payload tarball(s) the image was built with — a fact about the image, not about the build's environment, and it is the same file the sha256sums row verifies right beside it. Deliberately not the payload directory's name: the tree is always /opt/1bit-package, so comparing against that would pass for any claim at all — the vacuous check the issue warns about.

Verification — the issue's own acceptance, fired rather than described. The four cases are in --self-test, which CI already runs (ctest name release_check_row, CMakeLists.txt:241):

ok   PASS payload=lemonade-onebit-rc-2026-11-08-c33 is the payload this image carries
ok   FAIL the identity claims payload=lemonade-onebit-rc-2026-11-05-c26c but the data partition carries …
ok   FAIL the identity names no payload (payload=unknown) while the data partition carries …
ok   SKIP no payload tarball named in /data/SHA256SUMS, so payload= cannot be compared with the bytes

Whole self-test: 123 checks, PASS under both dash and bash (this script runs as #!/bin/sh in the image, so dash is the shell that matters).

Not here, on the issue's own reasoning: option (2), comparing against the release bundle's tarball name, is a bundle-side check and belongs where the bundle is assembled. The release-body rule stands untouched.

One dash trap worth recording: the first version put an apostrophe inside a single-quoted printf, which bash accepts and dash does not — sh -n caught it before the self-test did. The wording changed rather than the escaping.

Fixes #1557)

The engine half of #1481 closed; this is the half that was kept open on purpose. `payload=` was a DECLARED value —
`PAYLOAD_NAME` from the build's environment (container-build.sh:352) — and nothing compared it with the bytes, so
c32 and c33 both shipped `payload=lemonade-onebit-rc-2026-11-05-c26c` beside later payloads. The engine sha is
computed from a file; the name is taken from an environment variable, which is how one wrong variable makes an
identity half true.

New row `identity-payload`, from a pure `payload_verdict() <claimed stem> <stems present>`:

  claim matches a tarball stem in /data/SHA256SUMS   -> PASS
  claim names something else                         -> FAIL  (the c32/c33 shape)
  claim empty or `unknown` with a payload present    -> FAIL  (a declaration that cannot be true)
  no payload tarball named in the table              -> SKIP  (not a pass by absence)

The authority is the data partition's own checksum table — the same file the `sha256sums` row verifies beside it,
so it is a fact about the image rather than about the build environment. Deliberately not the payload directory's
name: the tree is always /opt/1bit-package, so comparing against that would pass for any claim at all, the vacuous
check the issue warns about.

Four self-test cases, which is the acceptance the issue asked for (`cmake --build` runs them via ctest
`release_check_row`): current cut PASS, the c26c-in-a-c33-image FAIL, `unknown` FAIL, absent tarball SKIP.
Whole self-test: 123 checks, PASS under **both** dash and bash (it runs as `#!/bin/sh` in the image).

Option (2) from the issue — comparing against the release bundle's tarball name — is deliberately not here: that
is a bundle-side check and belongs where the bundle is assembled, as the issue says. The release-body rule also
stands untouched: the payload name in a release body comes from the bundle and SHA256SUMS, not from this file.

Fixes #1557.
@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

Fixes #1557. The half of #1481 that was kept open on purpose: payload= was the one field of release-identity nothing checked.

The defect, in one line: the engine sha is computed from a file, the payload name is taken from PAYLOAD_NAME in the build environment (container-build.sh:352). One wrong variable and the identity is half true — which is how c32 and c33 shipped payload=lemonade-onebit-rc-2026-11-05-c26c beside later payloads.

What this adds: a pure verdict + a row, identity-payload:

input verdict
claim matches a tarball stem in /data/SHA256SUMS PASS
claim names something else FAIL — the c32/c33 shape
claim empty or unknown while a payload is present FAIL — a declaration that cannot be true
no payload tarball named in the table SKIP — never a pass by absence

Why that authority: /data/SHA256SUMS is on the image and names the payload tarball(s) the image was built with — a fact about the image, not about the build's environment, and it is the same file the sha256sums row verifies right beside it. Deliberately not the payload directory's name: the tree is always /opt/1bit-package, so comparing against that would pass for any claim at all — the vacuous check the issue warns about.

Verification — the issue's own acceptance, fired rather than described. The four cases are in --self-test, which CI already runs (ctest name release_check_row, CMakeLists.txt:241):

ok   PASS payload=lemonade-onebit-rc-2026-11-08-c33 is the payload this image carries
ok   FAIL the identity claims payload=lemonade-onebit-rc-2026-11-05-c26c but the data partition carries …
ok   FAIL the identity names no payload (payload=unknown) while the data partition carries …
ok   SKIP no payload tarball named in /data/SHA256SUMS, so payload= cannot be compared with the bytes

Whole self-test: 123 checks, PASS under both dash and bash (this script runs as #!/bin/sh in the image, so dash is the shell that matters).

Not here, on the issue's own reasoning: option (2), comparing against the release bundle's tarball name, is a bundle-side check and belongs where the bundle is assembled. The release-body rule stands untouched.

One dash trap worth recording: the first version put an apostrophe inside a single-quoted printf, which bash accepts and dash does not — sh -n caught it before the self-test did. The wording changed rather than the escaping.

@bong-water-water-bong bong-water-water-bong left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

VERDICT: CHANGES at 3b109a4cdfab4a87941b40ffbca0b7fbba6405dd

The verdict function is well shaped: PASS on a match, FAIL on a stale name or on unknown while a payload is present, SKIP rather than PASS when there's nothing to compare. The four --self-test arms pass. But the row's authority isn't established, and the issue itself says it can't be this one.

1. Nothing in the tree writes /data/SHA256SUMS, let alone a payload tarball into it.

  • git grep for /data/SHA256SUMS, $DATA/SHA256SUMS or data.dir/SHA256SUMS finds only readers (release-check.sh:1871, 1bit-proof), and no .tar.zst is copied onto a data partition anywhere in os/v3 or os/mini.
  • The SHA256SUMS files the writers do produce list other things: os/v3/puppy/mkimage.sh:205 writes vmlinuz initrd.gz *.sfs into /1bit/SHA256SUMS. The real release tables on this box list .sfs, 1bit-install, 1bit-os-puppy.img(.xz) (~/wt/c32-artifacts, ~/wt/hf-1bitos). Run through this PR's stem extraction (sed 's|.*[ \t]||' | sed -n 's|\.tar\.zst$||p'), the c32 table yields no stems.
  • So on every image this repo builds, the new row SKIPs. That's honest, but it means payload= still isn't checked anywhere, which is the gap #1557 exists to close. The body's "a fact about the image" needs the writer that puts <payload>.tar.zst into /data/SHA256SUMS, and I can't find one.

2. The issue rules this authority out. #1557: "the release bundle - the tarball name plus its sha from SHA256SUMS … An image cannot see its own tarball, so this is a bundle-side check, not an image-side one." Its other option is the payload directory, with the row saying explicitly which name it compares.

  • Either make this a bundle-side check (in weekly.sh/the release step, where the tarball and its SHA256SUMS are present, comparing the stem against payload= in the built image's identity file),
  • or record the payload's own identity inside the payload at pack time (a stem or sha file in /opt/1bit-package written by the packer), and compare payload= with that on the image.

Either way, cite the writer.

3. Small, if the row stays:

  • the stem parse breaks on sha256sum -b's *name form, and on a subdirectory path (dir/x.tar.zst → dir/x);
  • every .tar.zst in the table counts as a "payload stem", so a claim matching any listed tarball, e.g. a proof bundle, would PASS.

release-check.sh --self-test passes at this head.

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

Duplicate of #1571 (same field, same file, different patch), and both are the weaker half of the pair that already exists.

The payload= field now has four PRs, and they divide cleanly:

PR what it has verdict
#1569 the README authority (1bit/README.txt line 1 → rc-…-cNN), 11 README refs READY — the only one that gates the c26 incident
#1570 the writer half: payload_lemond= from the payload's bytes + a build-time refusal, 3 files complementary, needs the pin-only comparison
#1571 release-check only, no authority (0 README refs, 0 payload_lemond) superseded
#1579 (this) release-check only, no authority superseded

The three values are different things, measured on one artifact (c33's shipped payload):

payload=            lemonade-onebit-rc-2026-11-08-c33     the tarball stem - contains the token, equals nothing else
README line 1       rc-2026-11-08-c33                     the cycle token inside the image     <- #1569
lemond --version    2026.47.0~1.52a2a6b3                  the payload's own bytes (with a builder-dependent ~N)  <- #1570

So a row without either authority can only repeat that payload= is declared — which is what my own #1558 did, and I closed it for that reason rather than landing a second row on one field.

Suggested close: #1579 and #1571 close; #1569 lands (reader/incident), #1570 lands with the pin-only comparison (2026.47.0 + trailing sha, clone count ignored) and a PASS line that names payload_lemond= rather than "the payload on this machine is the one the identity names".

None of this is about the quality of the patch — it is the fourth implementation of one field in 40 minutes, and #1557's own resolution section now says which pair closes it.

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

You are right, and the deeper finding is that the authority does not exist in this repo at all. I went looking for the writer before answering, and the hunt is the answer:

$ git grep -n "tar.zst" -- os/ scripts/          # payload tarballs
os/v3/puppy/build.sh:176   PAYLOAD_NAME="$(basename "$payload")"     # RECEIVES a path, never packs one
os/v3/puppy/mk-huge-kernel.sh:31   huge-<kver>.tar.zst               # the kernel zdrv, not the payload
os/v3/puppy/embed/usr/local/bin/1bit-proof:203   /data/proof-*.tar.zst  # the PROOF bundle — exactly your point 3
$ git grep -n "/data/SHA256SUMS" -- os/ scripts/ # readers only (release-check.sh:1871, 1bit-proof)

So: the payload tarball is packed outside this repo (the release lane hands os/v3/puppy/build.sh --payload <path> and the basename becomes PAYLOAD_NAME), there is no in-repo writer that puts it into any SHA256SUMS, and the only .tar.zst a data partition actually carries is the proof bundle — i.e. my stem parse would have passed a claim naming a proof bundle. Your point 3 was not hypothetical.

Which leaves exactly two owners, and neither is this row:

option where the writer must go who can do it
bundle-side you named first the release step that holds both the payload tarball and the built image — scripts/weekly.sh builds 1bit-os-$tag.img.zst in-repo (line 598), so it could read payload= out of the image's /etc/1bit/release-identity (unsquashfs -cat on the inner sfs) and compare the tarball stem the release lane, in weekly.sh, not a booted-image row
pack-time stamp you named second the payload packer, which is not in this repo: it would write /opt/1bit-package/PAYLOAD-ID = the stem it is about to produce, and then this row compares payload= with that file and needs no SHA256SUMS at all whoever packs lemonade-onebit-rc-*.tar.zst; the image side of this PR would then be five lines

What I am not going to do is keep a row that SKIPs on every image this repo builds and calls that closing #1557, or one whose authority is a file class that also matches proof bundles. So: this PR is parked at 3b109a4c, not ready, and the decision I cannot make is which of those two owners takes the writer — bundle-side in weekly.sh (release lane) or a pack-path stamp (packer owner).

If you would rather it land now, the one variant I can defend in-repo is a content check instead of a name check: container-build.sh already has /payload, so it could record payload_manifest=<sha256 over the payload tree> beside engine_sha256, and the row compares that with the tree present in the image. That closes the lie (an identity describing a different payload than the one the image carries) without any external packer — at the cost of hashing ~1.8 GB at boot unless the manifest is computed over a small subset (the launcher + the model catalogue), which is a design decision I would want you to sanity-check before I build it. Say which and I will take it from there; the technical nits you listed (the *name form, subdirectories, proof bundles) are all inside whichever version survives.

@bong-water-water-bong bong-water-water-bong changed the title release-check: payload= is compared with the payload the image carries (Fixes #1557) release-check: payload= is compared with the payload the image carries — PARKED: the authority needs a writer (Fixes #1557) Nov 8, 2026
…ixes #1557)

The first version of this row anchored on a payload tarball stem in /data/SHA256SUMS, and the review was right
that no such writer exists: the payload tarball is packed outside this repo, nothing writes it into any
SHA256SUMS, and the only .tar.zst a data partition holds is the PROOF bundle - so the stem parse would have
passed a claim naming a proof bundle.

The review's two options both wanted a writer. There is one in-repo, and it was already writing the field:
container-build.sh writes `payload=${PAYLOAD_NAME}` from the /payload tree it was handed. So it now writes a
fact about that same tree:

  payload_manifest=$(find /payload -type f | sed 's|^/payload/||' | LC_ALL=C sort | sha256sum)

and release-check recomputes the SAME expression over the payload the image carries (/opt/1bit-package) and
compares. A different payload has a different path list, which is the lie that survived two cycles - the name
stays in the row's message as the human-readable claim it is. No packer, no tarball, no SHA256SUMS.

  claim matches the payload present     -> PASS
  manifest differs                      -> FAIL  (the c32/c33 shape)
  manifest is not a hex digest          -> FAIL  (malformed, not merely old)
  no manifest (the mini schema)          -> SKIP  (says payload= is still only a declaration)
  payload not at the appliance path      -> SKIP  (never a pass by absence)

Cost: metadata only, and the payload-modes row already walks the same tree - no new order of work at boot.

Five self-test arms; whole self-test 124 checks, PASS under dash and bash (the image runs it as /bin/sh).

Own-fault worth recording: my first edit left the old payload_verdict in place beside the new one, so the
self-test fired the OLD arms and reported two "want PASS, got FAIL" lines - the duplicate was 800 bytes away in
the same file. Removed; the run quoted above is from the file with one verdict.

Fixes #1557.
@bong-water-water-bong bong-water-water-bong changed the title release-check: payload= is compared with the payload the image carries — PARKED: the authority needs a writer (Fixes #1557) release-check: the identity's payload is checked by content, and the build writes the fact (Fixes #1557) Nov 8, 2026
@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

CHANGES answered at c259aeec: the check is now on content, and the writer is in-repo.

You were right that no writer exists for a payload stem in /data/SHA256SUMS — I confirmed it by hunting (build.sh:176 only receives a payload path; readers only for that file; the only .tar.zst on a data partition is the proof bundle, so my stem parse would have passed a claim naming one).

But there is a writer for the field, and it already runs: container-build.sh writes payload=${PAYLOAD_NAME} from the /payload tree it was handed. So it now writes a fact about that same tree, and the row compares it against the payload the image carries. No packer, no tarball, no SHA256SUMS.

container-build.sh   payload_manifest=$(find /payload -type f | sed 's|^/payload/||' | LC_ALL=C sort | sha256sum)
release-check.sh     <same expression over ${PAYLOAD_DIR:-/opt/1bit-package}>

A different payload has a different path list — which is the lie that survived two cycles. The name stays in the row's message as the human-readable claim it is; the manifest is what gets checked.

input verdict
claim matches the payload present PASS
manifest differs FAIL — the c32/c33 shape
manifest is not a hex digest FAIL — malformed, not merely old
no manifest (the mini schema) SKIP — and it says payload= is still only a declaration
payload not at the appliance path SKIP — never a pass by absence

Cost: metadata only, and the payload-modes row already walks the same tree, so this adds no order of work at boot — which was my worry about hashing ~1.8 GB, and the reason the check is the path list rather than the bytes.

Verified: five new self-test arms (the four above plus the malformed case); whole self-test 124 checks, PASS under dash and bash. Both files parse under both shells.

Your point 3 nits are gone with the stem parse (*name form, subdirectories, proof bundles) — there is no tarball parsing left. Your point 2 (cite the writer) is answered: os/v3/puppy/container-build.sh:352-361, and the reader carries the comment pointing back at it.

Own-fault worth the line: my first edit left the old payload_verdict in place beside the new one, so the self-test happily fired the old arms and printed two want PASS, got FAIL lines — the duplicate was 800 bytes away in the same file. Removed; the run above is from the file with exactly one verdict.

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

Closing this as superseded — main already has a better fix for #1557, and I found it by hitting the merge conflict rather than by looking first.

2a41f20b (#1570), merged: container-build.sh records payload_lemond from the payload's own launcher ("$PAYLOAD/lemond-onebit" --version), release-check.sh compares that with what the payload on the machine reports (payload_identity_verdict, four self-test arms), and docs/os-v3.md says explicitly that payload= is a label rather than a check.

That is the better design, for two reasons I would not have reached from my direction:

  • cost: one file read instead of my tree walk over the payload (I had avoided hashing ~1.8 GB by hashing the path list; their answer needs no walk at all);
  • honesty about the field: my version left payload= as the human-readable claim and added a parallel manifest; theirs says out loud that the stem is a label and checks the thing that is checkable. The pattern "two different claims, never mixed" is better than my "name in the message, manifest in the check".

What this PR contributed along the way, and where it stands: the writer-hunt in the CHANGES answer — no payload tarball is written into any SHA256SUMS in this repo, the packer is outside it, and the only .tar.zst on a data partition is the proof bundle — which is why the stem-anchored version could not work, and why the content/self-report direction was the right one. Its reader half and five self-test arms were correct; they are simply redundant now.

So: #1557 is fixed by #1570, not by this. Nothing to merge here, and no branch worth keeping — deleting it. The lesson I am taking, and it is the fleet's as much as mine: I searched for open PRs before opening, and #1570 had already merged; gh pr list --state merged --search 1557 is the search that would have found it, and it is now in my routine.

@bong-water-water-bong bong-water-water-bong left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

VERDICT: READY at c259aeecbc84436a7a0e83a85a1238fdc0a11a60

Comparing a digest of the payload's content against the identity is the strongest form this family has had, and the four arms are the right ones:

payload_verdict()   # <claimed name> <claimed manifest|""> <manifest of the payload present|"">
  no payload at the appliance path       -> SKIP
  no payload_manifest (mini schema)      -> SKIP  "still only a declaration"
  not a hex digest                       -> FAIL  "malformed, not merely old"
  manifest matches                       -> PASS
  manifest differs                       -> FAIL  "the identity describes a different payload … (the c32/c[33 shape])"

"Malformed, not merely old" is the distinction that turns a red into a diagnosis: a short or non-hex manifest is a writer problem, while a well-formed mismatch is a stale identity problem, and the two send people to different places. The SKIP for the mini schema is honest — that schema has no manifest, so payload= there stays a declaration and the row says so rather than manufacturing a pass.

--self-test PASS under sh, 124 ok lines.

One thing to watch as this family grows: there are now three payload rows in flight — payload_name_verdict (#1569 and #1571, which collide with each other), payload_lemond (#1570, a fact read from the payload's --version) and this manifest row. They are complementary rather than alternatives (name = label, lemond = version fact, manifest = content fact), but three rows about one field is where a report starts contradicting itself. Worth a single sentence in each about which question it answers.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

image: payload= in release-identity is still declared, not checked — the engine half is closed, this half is not

1 participant