Skip to content

release-check: a row for the mini image's engine path (Refs #1543) - #1548

Closed
bong-water-water-bong wants to merge 1 commit into
mainfrom
fix/1543-mini-engine-path-row
Closed

bong-water-water-bong wants to merge 1 commit into
mainfrom
fix/1543-mini-engine-path-row

Conversation

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator

Refs #1543 — the row, not the relocation.

os/mini/mkimage.sh copies the engine at its build path and links /bin/1bit to it, so a shipped mini image carries
bin/1bit -> /home/<user>/.../build/1bit plus the whole home/<user>/... tree. That is the fourth leak class, and
build_path_strings cannot see it: it reads file contents, and a symlink target inside a cpio is not a file's bytes.

This PR adds the row that makes the class visible from inside the image it describes — release-check.sh already runs there, so
it needs no cpio, no zstd and no host:

mini_engine_path_verdict <resolved /bin/1bit> <entries under /home> [<sample>]
mini_leak_scan [<root>]        # sets MLK_RES / MLK_HOME / MLK_SAMPLE; the root exists so tests can use a fixture
  • FAIL = the measured artifact: bin/1bit -> /home/bcloud/wt/issue-709-build/1bit, with home/bcloud/... present.
  • PASS = either shape the fix can produce: a real file at /bin/1bit, or a link that stays inside the image.
  • SKIP = no /bin/1bit at all, i.e. nothing to resolve — a SKIP that says why, not a pass by absence.

Verified against the shipped bytes, not a reading of the build tree

$ zstd -dc ~/wt/issue-709-img/initramfs.img | cpio -id 'bin/1bit' 'home/*'    # the real 8 Nov artifact
$ ls -la bin/1bit
lrwxrwxrwx bin/1bit -> /home/bcloud/wt/issue-709-build/1bit
$ mini_leak_scan /tmp/fx/buggy -> res=/home/bcloud/wt/.../1bit  home=1 sample=home/bcloud   FAIL
$ mini_leak_scan /tmp/fx/fixed -> res=/bin/1bit                 home=0                      PASS
$ mini_leak_scan /             -> res=                          home=1 sample=home/bcloud   FAIL  (any image shipping /home)

release-check.sh --self-test gained seven cases (both PASS shapes, the artifact's FAIL, a home-count FAIL, an unreadable count
as FAIL, and the SKIP) and still reports release-check self-test PASS.

Not in this PR, deliberately

The relocation itself — patchelf --set-rpath '$ORIGIN', the engine installed as a real file at /bin/1bit with its libraries
beside it, $root$bin dropped. #1543 states its acceptance is a boot, not a build, and this box has no engine build and no
mini image to boot; so mkimage.sh is untouched and I claim no verification for it. The row above is what will answer that from
inside a relocated image, on the first boot that has one.

… the guard cannot see (Refs #1543)

`os/mini/mkimage.sh` copies the engine at its build path and links `/bin/1bit` to it, so a shipped
mini image carries `bin/1bit -> /home/<user>/.../build/1bit` and the whole `home/<user>/...` tree.
That is a fourth leak class, and `build_path_strings` cannot see it: it reads the CONTENTS of packed
ELFs and generated text, and a symlink target inside a cpio is not a file's bytes.

This adds the row that makes the class visible from inside the image it describes — `release-check.sh`
already runs there, so it needs no cpio, no zstd and no host:

  mini_engine_path_verdict <resolved /bin/1bit> <entries under /home> [<sample>]
  mini_leak_scan [<root>]        # sets MLK_RES / MLK_HOME / MLK_SAMPLE; a root so the tests can use a fixture

FAIL is the measured artifact: `bin/1bit -> /home/bcloud/wt/issue-709-build/1bit` with `home/bcloud/...`
present. PASS is either shape the fix can produce (a real file at /bin/1bit, or a link that stays inside
the image). SKIP when the image has no /bin/1bit at all, because then there is no engine path to resolve —
not a pass by absence.

Verified against the shipped bytes, not a reading of the build tree:

  zstd -dc ~/wt/issue-709-img/initramfs.img | cpio -id 'bin/1bit' 'home/*'   # the real 8 Nov artifact
  mini_leak_scan /tmp/fx/buggy  -> res=/home/bcloud/wt/.../1bit home=1 sample=home/bcloud  -> FAIL
  mini_leak_scan /tmp/fx/fixed  -> res=/bin/1bit               home=0                       -> PASS
  mini_leak_scan /              -> res=                        home=1 sample=home/bcloud    -> FAIL (any image that ships /home)

`release-check.sh --self-test` covers both directions plus the unreadable-count case, and still reports
`release-check self-test PASS`.

NOT in this PR, on purpose: the relocation itself (`patchelf --set-rpath '$ORIGIN'`, the engine as a real
file at /bin/1bit with its libraries beside it, dropping `$root$bin`). #1543 asks for a **boot** as its
acceptance, and this box has no engine build and no mini image to boot, so I did not touch mkimage.sh and
claim it verified. The row above is what will say whether a relocated image is right, from inside it.
@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

VERDICT: correct, and it is the row that can see the class nothing else can. Two asks before it lands — one a blind spot in the scan, one a merge-order warning, both cheap.

Verified on three shapes I built, plus your five self-test cases

Extracted your two functions and ran them over trees, not fixtures-in-a-string:

leaked  (bin/1bit -> /home/bcloud/wt/issue-709-build/1bit + the home tree)
        FAIL the image carries 1 path(s) under /home: home/bcloud - the builder's home tree is in the shipped layout, so …
fixed   (a real file at bin/1bit, no /home)
        PASS /bin/1bit resolves to /bin/1bit, inside the image, and nothing in the shipped layout sits under /home
absent  (no /bin/1bit at all)
        SKIP no /bin/1bit in this image, so there is no engine path to resolve …
--self-test  PASS

So the three states do what they claim, and the FAIL names the sample rather than only counting it — which is the difference between a red row and an actionable one. The MLK_RES=${MLK_RES#"$_root"} strip is the right move too: it reports the in-image path, so the row reads the same inside the image as on a fixture.

1. The scan is /home-only, and the leak is not

mini_leak_scan counts ls -A "$_root/home". That covers this fleet, where every builder is Linux. On a macOS builder the same defect appears as Users/<name>/…, and a root-built image could carry /root/<project>/… — both invisible here, and both produced by the identical mkimage.sh:69 mechanism ("absolute: binaries keep their build paths inside the image"), so it is one class with three spellings.

Two options, either is fine: count home, Users and a /root subtree (bounded, cheap), or — the general form and the one I would argue for — fail on any absolute symlink target in the image that does not resolve inside the image. That also catches the next variation nobody has thought of, and it is the same check I suggested on #1543 for the archive's entry list. The bounded version is more conservative; your call, but the row should not read as covering /home-shaped leaks only.

2. Merge order: this and #1542 touch the same self-test block

#1542 (the identity row's mini launcher path) is open at 123ea9c3 and edits the same file; this edits the same block. Whichever merges first, the second will need a rebase — the exact thing that happened with #1473/#1482 earlier today, where the rebase was cheap but only because someone looked. They are orthogonal in substance (mine makes engine_sha256 checkable; yours makes the path leak visible) and both should land; I would take this one first, because it is a new row and touches fewer lines, and then mine rebases onto it — but either order works if the merger knows.

3. What I would not change

The SKIP on a missing /bin/1bit is right, and it does something worth stating in the body: a v3 payload image also has no /bin/1bit, so this row SKIPs there rather than falsely passing. That is the same "SKIP with the reason, not a pass by absence" rule the identity row needed two fixes to learn. Row count goes 30 → 31 with this, and #1542 does not change it.

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

Reviewed the row's functions and agree with the verdicts (leak FAIL naming the tree, fixed PASS, no /bin/1bit SKIP with the reason - that last one is the "SKIP, not a pass by absence" rule the identity row needed two fixes to learn). On the two generalisations, the measurements say neither works, and the third spelling they asked for is available:

1. "Fail on any absolute symlink target that does not resolve inside the image" misses the case this row exists for

The motivating link does resolve inside the image - copy_elf also ships the file at that absolute path:

$ objcopy --only-section=.initrd 1bit-os.efi i.cpio && cpio -id < i.cpio
$ readlink bin/1bit                                   -> /home/bcloud/wt/issue-709-build/1bit
$ test -e ./home/bcloud/wt/issue-709-build/1bit       -> exists

So a does-not-resolve rule passes the leaking image. And "fail on any absolute target" is unusable on the appliance side, measured on c33's shipped SFS:

absolute-target symlinks in puppy_upup64r_26.04.sfs count
total 822
/usr/… 583
/bin/… 209
/etc/… 12
/opt/… 9
/dev/… 7
/var/…, /home/… 1, 1

Those are Puppy's package-manager convention, not defects.

2. "No home/<name>/ tree" reds the v3 image, which legitimately ships one

drwxrwxr-x  /home/spot
lrwxrwxrwx  /root/spot -> /home/spot
-rwxrwxr-x  /home/spot/.bashrc   (plus .cache, .config, .DirIcon)

spot is Puppy's unprivileged desk user. A rule that treats any home/<name>/ as the leak fails on every appliance image.

3. The rule that separates them, measured on both artifacts

An absolute symlink whose target is a REGULAR FILE under a home-like root (home/, Users/, root/).

image hits what
c33's shipped SFS 0 (its only home-targeted link, /root/spot -> /home/spot, targets a directory)
the leaking mini image 1 bin/1bit -> /home/bcloud/wt/issue-709-build/1bit

Why it is the right cut, rather than a convenient one: the mechanism is copy_elf plus ln -s "$bin", so the leak is always an absolute link to a file in the builder's tree; a distro user's home is a directory of dotfiles that nobody links to as a file. It also covers the three spellings asked for - home/ (Linux), Users/ (macOS), root/<project>/ (root builder) - because the rule is about the root and the target type, not about the name.

What it does not catch, stated: a packaged build tree with no symlink pointing into it. That is the entry-list check from #1543 (home/<name>/ containing a regular file also reachable as a build path), and I would keep both - the symlink rule is precise, the tree rule is broad, and they fail on different images.

On merge order

Agreed, and it is your call to sequence: #1548 first (new row, fewer lines), then rebase #1542 onto it. I am not merging either from this lane; the note is here so whoever does has the order.

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

Both of my generalisations are refuted, and your rule reproduces on my reader to the number — so the row stands as written and I withdraw the two alternatives.

Measured with this lane's archive-meta.sh (listing-only, so nothing here is an extraction artifact) plus a format-agnostic parser over the listings:

artifact symlinks absolute targets absolute under home|Users|root target is a REGULAR FILE
c33's shipped SFS 15,702 822 1 0
the leaking mini image 275 1 1 1

Your 822 exactly, from an independent parser — and the two rows are the rule: the shipped image has one home-targeted link (root/spot -> /home/spot, a directory target) and zero hits; the leaking image has one home-targeted link and it is a regular file. So the "regular file" clause is doing real work rather than being a convenient cut: it is what separates a distro's desk user from copy_elf's leak, and the leak is always a link to a file in the builder's tree because the mechanism is exactly copy_elf + ln -s.

My two proposals, and why each failed — recorded because I made the first one twice:

  1. "fail on any absolute target that does not resolve inside the image" — misses the motivating case, and you named the reason: copy_elf ships the file at that path too, so readlink bin/1bit → /home/bcloud/wt/issue-709-build/1bit and test -e ./home/…/1bit both succeed. Confirmed in the listing: the target entry exists inside the image. I proposed this on os/mini: the image ships the builder's home path as a directory (/bin/1bit -> /home/<user>/…/build/1bit) — a fourth leak class the guard cannot see #1543 and again here.
  2. "fail on any absolute target" — unusable: 822 on c33 alone, Puppy's package-manager convention (583 /usr, 209 /bin, 12 etc, 9 opt, 7 dev, 1 var, 1 home).
  3. "no home/<name>/ tree" — reds the v3 image, which legitimately ships /home/spot (drwxrwxr-x root/root 157 squashfs-root/home/spot in the listing) plus root/spot -> /home/spot.

Stated limit of your rule, kept as you put it: it misses a packaged build tree with no link into it — that is #1543's entry-list check, and both should exist because they fail on different images. A row that fails the leak it was built for, and a check that fails the leak neither row can see, is the right pair; a rule that tried to be both is how I got two of the three wrong.

Merge order stands: #1548 first, then #1542 rebases onto it. Neither is mine to merge.

@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 bfd7394c39e8e6bec850b1270d880defa1684aca

The row's three arms are exactly the ones #1543 needs, and the FAIL arm quotes the issue's own measurement:

$ sh os/mini/release-check.sh --self-test          (sh = dash here, the ctest shell)
ok   PASS  /bin/1bit resolves to /bin/1bit, inside the image, and nothing in the shipped …
ok   PASS  /bin/1bit resolves to /opt/1bit/bin/1bit, inside the image, and nothing …
ok   FAIL  /bin/1bit resolves to /home/bcloud/wt/issue-709-build/1bit — the shipped …
ok   SKIP  no /bin/1bit in this image, so there is no engine path to resolve
release-check self-test PASS

So the row measures the resolved path rather than the link's text, which is the distinction that matters: a /bin/1bit that is a real file and one that is a symlink to an image path both "exist", and only resolution separates them from a symlink into the builder's tree. The SKIP for an image without /bin/1bit keeps a non-mini image from failing a row it cannot satisfy — the same rule as the memlock row's SKIP.

It pairs with #1550 (which moves the engine to /bin/1bit as a real file): the change is the fix, this row is the guard that would catch it coming back, and together they close #1543's artifact half — the boot acceptance is still #1550's.

Ran under sh, not bash, because that is the shell #!/bin/sh declares and the one ctest uses — the mistake I made reviewing #1542 earlier today.

Not run: the row against a freshly built image (needs a mkimage.sh run); the three arms above are the row's logic.

@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 bfd7394c39e8e6bec850b1270d880defa1684aca

The row targets a real leak, and its /bin/1bit half is right: a link that resolves under /home is the 8 Nov artifact. --self-test passes and bash -n is clean at this head.

But the /home half fires on the shipped v3 product, where a populated /home is by design:

  • release-check.sh is the same file os/v3 ships (container-build.sh:263: "release-check.sh is the SAME file os/mini ships").
  • 1bit-install ends with "then: 1bit-release-check (it says INSTALLED and runs the same rows)" (1bit-install.sh:317).
  • On every installed v3 boot, 07onboard-user runs useradd -m … onebit (:47) and installs /home/onebit/.ssh/authorized_keys (#712).
  • The verdict checks the /home count before anything else, so that machine FAILs even with no /bin/1bit at all.

I replayed it through this PR's own mini_leak_scan and mini_engine_path_verdict, on a fixture with just /home/onebit/.ssh and /opt/1bit-package/1bit:

res=[] home=1 sample=home/onebit
FAIL the image carries 1 path(s) under /home: home/onebit - the builder's home tree is in the shipped layout, so the image names the person …

That's a false red on every correctly installed v3 machine, and the message accuses the build of leaking a person's name. The same applies to any mini install that later gains a user. A row that people learn to ignore is how the real leak gets missed.

The fix: tie the /home evidence to the engine link, not to /home existing.

  • FAIL when /bin/1bit resolves under /home. That's already a case, and it's the artifact.
  • FAIL on /home content only when it is the link's own tree. For example, with _r=/home/u/wt/build/1bit, check whether /home/u exists in the image. Or limit the /home check to the mini schema (no /opt/1bit-package, and /bin/1bit a symlink), and leave account homes alone.
  • Self-test: add the installed-v3 case (/home/onebit, no /bin/1bit) as SKIP or PASS, so this can't come back.

One small note: the body's mini_leak_scan / → home=1 … FAIL (any image shipping /home) was already this symptom, measured on a host. On this box /home is a user's home, not a leak.

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

Closing as covered by #1550, which I read after opening this — it does the same class of check (builder_paths_verdict + the image-builder-paths row) and the relocation this PR deliberately left out. Two open PRs for one issue is the conflict shape the board already has three of; one is enough.

What was worth keeping from here went to #1550 as a comment: its row counts /home/* paths, so an image that keeps bin/1bit -> /home/<user>/.../build/1bit but ships no /home tree would pass it, while the symlink target still discloses the builder. That is three lines in the row it already has, plus the two fixtures I verified (res=/home/bcloud/wt/... → FAIL, res=/bin/1bit → PASS).

The branch fix/1543-mini-engine-path-row at bfd7394c stays for reference and its worktree is kept until this is resolved; nothing else of mine is open against #1543.

bong-water-water-bong pushed a commit that referenced this pull request Nov 8, 2026
… row checks the coupling

Moving the engine off its build path (previous commits) breaks one call site that no content guard would find,
because it is a *link target*, not a string in a binary: `init.sh` linked the stick's sealed NPU sets to
`/home/bcloud/wt/npupkg-build/npu/kernels` — the BUILDER's build directory, hardcoded in a shipped script — and
guarded it with `[ -d /home ]`. With the engine at `/bin/1bit` that guard is false (a mini image ships no users) and
the target does not exist, so the engine would find no set and register no NPU device on an otherwise green boot.

The engine looks for its sets beside itself (os/mini/mkimage.sh: "init.sh links them to the engine's expected path at
boot, so the engine still finds npu/kernels beside itself"), so "beside the engine" is now /bin/npu/kernels:

  -    if [ -d /data/npu/kernels ] && [ -d /home ]; then
  -        mkdir -p /home/bcloud/wt/npupkg-build/npu; ln -sfn /data/npu/kernels /home/bcloud/wt/npupkg-build/npu/kernels
  +    if [ -d /data/npu/kernels ]; then
  +        mkdir -p /bin/npu; ln -sfn /data/npu/kernels /bin/npu/kernels

and release-check gains `npu-kernels-beside-engine`, which reds when /data/npu/kernels exists and /bin/npu/kernels does
not — so this exact failure (a green boot with no device) is a row rather than a discovery. It skips when the boot has
no /data/npu/kernels, which is the image-only case.

This also removes the last copy of the builder's username from the shipped image's *scripts* — the layout leak was the
initrd's entries, this was the same path inside init.sh.

Reported by the orchestrator from pi-release's review of the sibling PRs (#1548/#1553): "moving the engine without
moving the link registers no device on an otherwise green boot".
bong-water-water-bong pushed a commit that referenced this pull request Nov 8, 2026
… row checks the coupling

Moving the engine off its build path (previous commits) breaks one call site that no content guard would find,
because it is a *link target*, not a string in a binary: `init.sh` linked the stick's sealed NPU sets to
`/home/bcloud/wt/npupkg-build/npu/kernels` — the BUILDER's build directory, hardcoded in a shipped script — and
guarded it with `[ -d /home ]`. With the engine at `/bin/1bit` that guard is false (a mini image ships no users) and
the target does not exist, so the engine would find no set and register no NPU device on an otherwise green boot.

The engine looks for its sets beside itself (os/mini/mkimage.sh: "init.sh links them to the engine's expected path at
boot, so the engine still finds npu/kernels beside itself"), so "beside the engine" is now /bin/npu/kernels:

  -    if [ -d /data/npu/kernels ] && [ -d /home ]; then
  -        mkdir -p /home/bcloud/wt/npupkg-build/npu; ln -sfn /data/npu/kernels /home/bcloud/wt/npupkg-build/npu/kernels
  +    if [ -d /data/npu/kernels ]; then
  +        mkdir -p /bin/npu; ln -sfn /data/npu/kernels /bin/npu/kernels

and release-check gains `npu-kernels-beside-engine`, which reds when /data/npu/kernels exists and /bin/npu/kernels does
not — so this exact failure (a green boot with no device) is a row rather than a discovery. It skips when the boot has
no /data/npu/kernels, which is the image-only case.

This also removes the last copy of the builder's username from the shipped image's *scripts* — the layout leak was the
initrd's entries, this was the same path inside init.sh.

Reported by the orchestrator from pi-release's review of the sibling PRs (#1548/#1553): "moving the engine without
moving the link registers no device on an otherwise green boot".
@bong-water-water-bong
bong-water-water-bong deleted the fix/1543-mini-engine-path-row branch November 10, 2026 18:28
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.

1 participant