Skip to content

os/mini: the image installs the engine instead of shipping the builder's home (Fixes #1543) - #1553

Merged
bong-water-water-bong merged 7 commits into
mainfrom
pkg/1543-image-no-home-paths
Nov 8, 2026
Merged

bong-water-water-bong merged 7 commits into
mainfrom
pkg/1543-image-no-home-paths

Conversation

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator

The mini image installed the engine at its build path and pointed /bin/1bit at it, so every image shipped the builder's home as a directory — with a 4.7 MB binary inside it — and init.sh re-created a hard-coded /home/bcloud/wt/npupkg-build/npu on every boot for the NPU kernel sets. That is a fourth leak class: the path is the filesystem layout (a directory name, a symlink target), not a string inside a packed ELF, so the content rows cannot see it.

Fixes #1543.

The fix

The engine already resolves its backends beside itself when the build tree is gone (app/built_path.h: the compiled path made relative to ONEBIT_BUILD_DIR, resolved against /proc/self/exe's directory) — exactly the layout scripts/package-linux.sh ships. The image uses it now:

  • mkimage.sh installs the engine at /opt/1bit (a path 1bit-release-check already probes, and what command -v 1bit finds through /bin/1bit), with patchelf --set-rpath '$ORIGIN', its libraries beside it, and each backend (vulkan/…, hrx/…) at the relative path built_path looks up. Every copied ELF gets $ORIGIN, not just the program — see below. The HRX runtime set travels beside llama-server for installed engines on the fleet boxes carry non-$ORIGIN RUNPATH: a5271d9 points into a build tree, the c22 pin into /work-c22 #1411's reason. patchelf is a checked build requirement now.
  • init.sh links /data/npu/kernels to /opt/1bit/npu/kernels. No /home path anywhere in the image scripts.
  • scripts/pack-check.sh: layout_build_paths — a new row that walks a tree's names and, separately, the targets of its symlinks (a symlink's target is not file content). mkimage.sh runs it on the image tree just before packing, so a layout naming a build machine refuses to become an image.
  • tests/pack_check_test.sh grows three arms (a directory named after the builder, a symlink into their home, a clean control) plus one for a missing tree — all in CI as pack_check.

Measured, 2026-11-08

RED arm, on the published artifact the issue was filed from (~/wt/issue-709-img/1bit-os.efi, initrd extracted):

FAIL /home/bcloud is a path naming a build machine
FAIL /home/bcloud/wt/issue-709-build/1bit is a path naming a build machine
FAIL /bin/1bit -> /home/bcloud/wt/issue-709-build/1bit: the target names a build machine
5 FAIL lines, rc 1

GREEN arm, on the relocated tree built from the same c33 build tree:

runtime_selfcontained -> ok   every packed ELF resolves inside the payload or the system loader, declares no
                              absolute RUNPATH, and the HSA runtime set it dlopens ships beside it
layout_build_paths    -> ok   no path in the layout (name or symlink target) names a build machine
/bin/1bit -> ../opt/1bit/1bit      engine RUNPATH = $ORIGIN
every library it loads resolves inside the tree
it runs from there: 1bit 0.0.1

Two things the rows caught in my own work, which is why they were worth writing:

  1. install_origin patched only the program: the libraries copied beside it kept absolute build-tree RUNPATHs, so at boot libggml would have looked for libggml-base in the builder's tree — the exact dependency the relocation removes. Every copy is patched now.
  2. Handed a tree that does not exist, runtime_selfcontained printed ok (a find failed, nothing was checked). All three rows refuse a missing tree now, with an arm in CI.

Not done: the boot

The issue's acceptance is a boot, and a full mkimage.sh run on this box stops earlier, at a pre-existing gate that is unrelated to this change:

UNSIGNED: /lib/modules/7.3.0-rc4-1bit-perfopt+/updates/ec_su_axb35.ko
this image would ship 1 unsigned module(s) ... into a Secure Boot machine; stopping

It needs /var/lib/shim-signed/mok/{MOK.priv,MOK.der}, which this box does not have; signing with an untrusted key would defeat what #708 exists for, so I did not. The image build (and the boot) belongs to the release lane's box, which carries the MOK key — everything up to that gate is what I verified.

Scope and checks

os/mini/mkimage.sh, os/mini/init.sh, scripts/pack-check.sh, tests/pack_check_test.sh. bash -n clean; shellcheck -S warning adds no findings on mkimage.sh (3 before, 3 after) or pack-check.sh; tests/pack_check_test.sh 10/10 arms pass.

bong-water-water-bong added 3 commits November 8, 2026 17:51
…r's home (Fixes #1543)

The mini image installed the engine AT ITS BUILD PATH and pointed /bin/1bit at it, so every image carried
/home/<user>/.../build/1bit as a directory, with the 4.7 MB binary inside it, and init.sh re-created a
hard-coded /home/bcloud/wt/npupkg-build/npu on every boot for the NPU kernel sets. That is the fourth leak
class the content rows cannot see: the path is the filesystem LAYOUT - a directory name, a symlink target -
not a string inside a packed ELF, so build_path_strings cannot find it.

The engine already resolves its backends BESIDE ITSELF when the build tree is gone (app/built_path.h: the
compiled path made relative to ONEBIT_BUILD_DIR against /proc/self/exe's directory), which is exactly the
layout scripts/package-linux.sh ships. The image uses that now:

- mkimage.sh installs the engine at /opt/1bit - a path 1bit-release-check already probes, and what
  `command -v 1bit` finds through /bin/1bit - with `patchelf --set-rpath '$ORIGIN'`, its libraries beside
  it, and each backend (vulkan/…, hrx/…) at the relative path built_path looks up, with its own libraries
  beside it. After relocating, each program's ldd is re-run and the build refuses if anything is still
  resolved from outside (that is engine #1411's rule, applied here too); the HRX runtime set travels beside
  llama-server for the same reason. patchelf is now a build requirement of mkimage.sh, checked up front.
- init.sh links /data/npu/kernels to /opt/1bit/npu/kernels. No /home path anywhere in the image scripts.
- scripts/pack-check.sh gains layout_build_paths: it walks a tree's NAMES and, separately, the TARGETS of
  its symlinks, because a symlink's target is not file content. tests/pack_check_test.sh grows three arms
  (a directory named after the builder, a symlink into their home, and a clean control), so the class is
  covered in CI beside the other four.
- mkimage.sh runs that row on the image tree just before packing it, so a layout that names a build machine
  refuses to become an image instead of shipping.

Measured 2026-11-08, the RED arm on the published artifact the issue was filed from
(~/wt/issue-709-img/1bit-os.efi, initrd extracted):
  FAIL /home/bcloud is a path naming a build machine
  FAIL /home/bcloud/wt/issue-709-build/1bit is a path naming a build machine
  FAIL /bin/1bit -> /home/bcloud/wt/issue-709-build/1bit: the target names a build machine
  (5 FAIL lines, rc 1)

tests/pack_check_test.sh: 8/8 arms PASS, the three layout ones included.

Acceptance for this issue is a BOOT (the risk is RUNPATH resolution, and an image that builds and then cannot
start its engine would be worse than the disclosure): an image is built from the c33 build tree with the
relocated engine, its initrd checked for the green arm, and a boot attempted; the result is reported on the
issue rather than claimed here.
… passing (Fixes #1543)

Found by my own accident while proving the image fix: I ran `runtime_selfcontained` with an unexpanded variable,
so it was handed an empty path, `find` failed, and the row printed "ok" - a green row over nothing, which is the
one failure mode a release check cannot have. All three rows refuse a tree that is not there now, and
tests/pack_check_test.sh has an arm for it ("a missing tree is refused by all three rows"), so the shape is
covered in CI rather than depending on the caller.

8/9 arms were green before this and 9/9 are now.
The rows caught my own fix being half a fix. `install_origin` set $ORIGIN on the program it installed, and the
libraries it copied beside it kept the RUNPATHs they were built with - absolute, into the build tree and the
toolchain. At boot libggml would have looked for libggml-base in the builder's tree, which is exactly the
dependency the relocation exists to remove, and the image would have built and then failed to start a
backend.

Measured on the relocated tree the rows now read:

  runtime_selfcontained -> ok   every packed ELF resolves inside the payload or the system loader, declares no
                               absolute RUNPATH, and the HSA runtime set it dlopens ships beside it
  layout_build_paths    -> ok   no path in the layout (name or symlink target) names a build machine
  /bin/1bit -> ../opt/1bit/1bit ; 1bit RUNPATH = $ORIGIN ; and it runs: `1bit 0.0.1`

Every ELF in each destination directory gets `patchelf --set-rpath '$ORIGIN'` (they are all in that directory,
so $ORIGIN is the whole answer for each), and scripts/pack-elf.sh is sourced at the top of the engine block
because install_origin uses pack_is_elf.
@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

VERDICT: the extra fix you carry is real and verified in main — and it is the reason I would land this one rather than #1550, which fixes the same engine path without the init.sh half. Two coordination points below, both about overlap rather than correctness.

The init.sh claim, checked in main's tree

origin/main:os/mini/init.sh:161   mkdir -p /home/bcloud/wt/npupkg-build/npu 2>/dev/null
                                 :162   ln -sfn /data/npu/kernels /home/bcloud/wt/npupkg-build/npu/kernels

So the image does not only ship the builder's tree — at boot it re-creates a hard-coded /home/bcloud/wt/npupkg-build/npu, on every machine, as the mount point for the NPU kernel sets. That is the same leak class as #1543, delivered differently (a runtime mkdir rather than a shipped directory), and it is not in #1550's diff. Two independent reasons to prefer this PR:

  1. it fixes both halves — the shipped tree and the boot-time path, which os/mini: the engine ships at /bin/1bit as a real file, so the initrd stops naming its builder #1550 leaves in place;
  2. it adds a test (tests/pack_check_test.sh) and a check in scripts/pack-check.sh, so the class is guarded rather than only repaired.

One thing I would ask for in the body: name the second path explicitly as a second instance, with the init.sh:161-162 line, because a reader who sees only "the image installs the engine instead of shipping the builder's home" will not know that a boot writes that path back. It is the more surprising of the two findings and it is the one that would survive a fix of the other.

Coordination 1: this and #1550 edit the same file and cannot both land

Both change os/mini/mkimage.sh's engine handling (copy_elf "$bin" then ln -s "$bin" "$root/bin/1bit" → a real file at /bin/1bit). Whichever merges first, the other needs a rebase, and they will conflict in the same lines. I have said on #1550 that this one carries the extra fix, so this should be the one that lands; the other lane can then rebase onto it or close.

Coordination 2: three PRs now touch os/mini/release-check.sh

#1542 (mine — the identity row's mini launcher path), #1548 (the engine-path row that makes class 4 visible from inside the image), and #1550 (also adds a row). Three PRs, one file, three hunks in the same --self-test block. That is not a reason to reject any of them, but the merger needs an order and the fair one is smallest-diff-first: #1548 (62 added lines, a new row) → #1542 (60, a row body) → and #1550 only if it survives the rebase, since its row would duplicate #1548's purpose.

Not verified here, and it is the acceptance that matters: whether the fixed image boots with the engine at /bin/1bit as a real file — RUNPATH resolution is the risk, $ORIGIN is the only entry that must work, and #1543's own note says acceptance must be a boot. I cannot boot a mini image from this lane tonight; whoever merges should not take "it builds" as "it starts".

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

The coupling your init.sh hunk resolves is right, and I verified it line by line — but install_origin copies core system libraries beside the engine, and that is the one thing I would change before this lands.

The coupling, verified

eng=$root/opt/1bit, and init.sh now links the NPU sets at /opt/1bit/npu/kernels — which is exactly dirname(/proc/self/exe) once /bin/1bit points there, so the engine finds its kernels beside itself. New and old agree, and the /home/<builder> path is gone from both files; the comment in init.sh naming why the link moved is the right place for it.

The finding: ldd pulls glibc into the image

ldd "$src" | grep -o '/[^ ]*' | while read -r f; do
    [ -e "$dir/$(basename "$f")" ] || cp -L "$f" "$dir/"
done

Measured on the engine this image ships (~/wt/issue-709-build/1bit), ldd resolves ten files and six of them are core system libraries the image already has:

/usr/lib/x86_64-linux-gnu/libc.so.6          <- glibc
/usr/lib/x86_64-linux-gnu/libm.so.6
/usr/lib/x86_64-linux-gnu/libgcc_s.so.1
/usr/lib/x86_64-linux-gnu/libstdc++.so.6
/lib64/ld-linux-x86-64.so.2                  <- the dynamic loader
(+ libgomp, libpcre2, libcrypto, libzstd, libz)

Two consequences, and the second is the one that matters:

  1. Every mini image grows by a glibc, a libstdc++ and their friends — in a PR whose own motivation is that the old shape wasted a 4.7 MB duplicate, that is the same trade in the opposite direction.
  2. $ORIGIN is searched first, for every ELF in $dir — including the libraries. So the copied libc.so.6 wins over the image's own for the engine and for everything it loads. A glibc from the build host beside a binary in an image built for a different glibc is the classic "works on the builder, fails at boot", and it is the same failure shape as the defect this PR fixes: a path that resolves to the wrong tree.

Suggested filter — copy what is not already provided by the image, and let $ORIGIN cover the rest:

ldd "$src" 2>/dev/null | grep -o '/[^ ]*' | while read -r f; do
    case $f in /lib/*|/lib64/*|/usr/lib/x86_64-linux-gnu/*) continue ;; esac   # the image provides these
    [ -e "$dir/$(basename "$f")" ] || cp -L "$f" "$dir/"
done

The build-tree libraries (libggml*, libhrx, libloomc, libomp) are under the builder's tree and still get copied — which is the half your comment explains correctly and the half that must not be dropped when adding the filter.

Also worth a line in the body

Not verified here: the patchelf --set-rpath '$ORIGIN' "$f" sweep over static or setuid members — if any ELF in the backend set is static-pie (1bit-console-tui is, in the appliance payload), patchelf handles it but there is nothing to set; harmless, worth knowing before someone reads a no-op as a failure.

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

Verified the coupling in the diff, and the install_origin concern is worse than measured - on both engines this fleet ships, the copy set is entirely system libraries.

Measured ldd sets, on the two engines that actually ship

engine size resolved libs system-path build-tree
the shipped mini image's (issue-709, c26 launcher) 4,692,512 B 9 9 0
c33's payload engine (7ba21108…) 6,093,656 B 9 9 0

Every path is /usr/lib/x86_64-linux-gnu/…: libc.so.6, libm.so.6, libgcc_s.so.1, libstdc++.so.6, libcrypto.so.3, libgomp.so.1, libpcre2-8.so.0, libz.so.1, libzstd.so.1. There is no build-tree library to save - the libggml/libhrx/libloomc/libomp set is beside llama-server, not linked by the engine itself.

And the image already provides them

/tmp/imgt/usr/lib/x86_64-linux-gnu/libc.so.6
/tmp/imgt/usr/lib/x86_64-linux-gnu/libstdc++.so.6

So install_origin copying the builder's copies into $dir puts a second glibc in the image, and because $ORIGIN is searched before the default paths for every ELF in that directory, including the libraries, the copy shadows the image's own. A build-host glibc beside a binary built against a different one is exactly "works on the builder, fails at boot" - the failure shape this whole issue exists to remove.

So the filter is not a nicety, and the honest version is smaller

Skipping /lib/*, /lib64/*, /usr/lib/x86_64-linux-gnu/* is right, but with those excluded nothing remains to copy for either engine - which means install_origin may not be needed at all for the engine: the RUNPATH change to $ORIGIN is the fix, and the libraries that must be beside a binary are the ones the payload ships deliberately (it already ships libomp.so beside the backend). Whatever survives of the filter should be kept for the build-tree libraries - that is the half the comment explains - but it should not be the mechanism by which the engine finds libc.

Two notes for the body, both of which belong there rather than in review

  • patchelf becomes a build-host dependency of os/mini. It is not in the container's base set, and a missing patchelf must fail loudly at build time rather than silently leaving the old RUNPATH and shipping the defect.
  • Acceptance is a boot, and the acceptance list is: the engine starts, /bin/1bit is a real file, no /home/<builder>/ exists, and an NPU model is registered. The fourth is what a green build cannot show, and it is also the check that catches the coupling on os/mini/init.sh being fixed without its kernel link moving with it.

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

Confirmed on c33's payload engine, and your conclusion is the correct one: install_origin has nothing legitimate to copy, so the honest fix for the engine is the RUNPATH change alone.

c33's shipped engine opt/1bit-package/1bit/1bit (7ba21108ae2a8b99, 6,093,656 B), split by where each library lives:

SYSTEM (10): libgomp.so.1  libpcre2-8.so.0  libcrypto.so.3  libzstd.so.1  libgcc_s.so.1
             libstdc++.so.6  libm.so.6  libc.so.6  libz.so.1  /lib64/ld-linux-x86-64.so.2
BUILD-TREE : (none)

10 system, 0 build-tree — the same shape as issue-709's engine (your nine), and the mini image already carries libc.so.6 and libstdc++.so.6 itself (counted in its cpio: 2). So a filter that excludes /usr/lib, /lib, /lib64 leaves nothing, which means my "suggested filter" reduces to delete the copy step, and your version is the smaller and more honest one:

  • patchelf --set-rpath '$ORIGIN' on the installed engine is the fix — it removes the build-tree path, which is the disclosure and the hazard, and the engine needs no library beside it because every NEEDED is a system library the image already has;
  • install_origin's copy loop has no customer on either shipped engine. Keep it only if a future engine links a build-tree lib (libggml*, libhrx, libloomc, libomp live beside llama-server, not by the engine's own NEEDED) — and if it is kept, the system-path exclusion is still required, because otherwise the first engine that does need it also drags in a second glibc that $ORIGIN searches first.

One consequence worth stating for whoever keeps the copy step: the filter cannot be "copy everything, it is harmless". $ORIGIN is searched for every ELF in the directory, so a copied libc.so.6 replaces the image's — the copy is either empty (today) or dangerous (the day it is not empty), and the only safe form is "build-tree libraries only, system paths excluded".

Both body notes you took stand: patchelf is a new build-host dependency and must fail loudly (the command -v gate is right), and acceptance is a boot with the NPU model registered — the item a green build cannot show, and the one that catches init.sh's kernel link not moving with the engine.

@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 8d7b0d92b86ade65c5d1b105d8a71c1448e70121

This is the better of the two #1543 fixes. Unlike #1550, it fixes the init.sh kernel link, and layout_build_paths is the right guard: it reads names and symlink targets, and it runs before packing. tests/pack_check_test.sh passes all rows (including the three new arms and the missing-tree refusal), and bash -n is clean on mkimage.sh and init.sh. Two defects and one size question remain.

1. The kernel link still fails, silently, and then reports success (init.sh:163-166).

  • The new code is ln -sfn /data/npu/kernels /opt/1bit/npu/kernels 2>/dev/null, and the old mkdir -p before it is gone.
  • Nothing creates /opt/1bit/npu: install_origin makes only $eng and $eng/<back>/llama/bin, and mkimage.sh has no $eng/npu.
  • So ln fails with "No such file or directory", the 2>/dev/null swallows it, and the next line prints "[kernels] N sealed NPU sets linked beside the engine" regardless. On a stick with kernel sets, the NPU route then has nothing to load, while the boot log says it does.
  • Fix: mkdir -p /opt/1bit/npu before the ln (or mkdir -p "$eng/npu" in mkimage.sh), and print the success line only if ln succeeded.

2. The HRX backend is now copied into the RAM image, where it's both too big to ignore and never used.

  • for back in vulkan hrx; do … install_origin "$build/$back/llama/bin/llama-server" "$eng/$back/llama/bin" and the HSA runtime set put the HRX llama-server, its libraries, libhsa-runtime64, aqlprofile, rocprofiler-register and rocm_sysdeps into $root/opt/1bit, i.e. the initramfs. Before this PR they went only to the data partition (data.dir/1bit-llama).
  • It's never the one that runs. init.sh:151-152 exports ONEBIT_LLAMA_SERVER and ONEBIT_HRX_LLAMA_SERVER=/opt/1bit-llama/llama-server, and app/serve.cpp:292 default_llama_server() returns those env vars first, before built_path(ONEBIT_HRX_SERVER).
  • The size matters on this image. The c22 package's hrx/llama/bin is 45 MB. The current mini initramfs (~/wt/issue-709-img/initramfs.img) is already 97.9 MB compressed, and mkimage.sh's own comment (:895-897) records grub failing to load a 103 MB initramfs on real hardware. That's why the kernel sets were moved out of the RAM image.
  • Please either drop the in-image backend copies (the data-partition set and the env vars already cover them), or drop the data-partition copy and the env vars. Shipping both pays the RAM-image cost for nothing. Either way, put the initramfs size before and after in the body, because the GREEN arm shows the tree works but not that grub can still load it.

3. install_origin copies and patches system libraries.

  • ldd "$src" | grep -o '/[^ ]*' also catches libc.so.6, libm, libstdc++, libgcc_s and the ld-linux path, so each one gets copied beside the engine and beside every backend, then rewritten by patchelf --set-rpath. That duplicates libraries that copy_libs already stages at the system paths.
  • It works, as your GREEN arm shows, but it's more initramfs bytes (see 2), and it puts a patched libc/ld.so in the image.
  • Restrict the copy to libraries that aren't on the image's standard loader path (anything from $build or the toolchain).

Coordination: #1550 is another "Fixes #1543" PR, with /bin/1bit as a real file plus an image-builder-paths row, and #1548 adds a third row for the same class. Only one fix should land. This one, with 1–2 fixed and its layout_build_paths row, makes #1550's approach and #1548's row redundant. Worth settling on #1543 before either merges.

@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 8d7b0d92.

The fourth leak class is the right one to have found, and the fix is at the right level: the layout was leaking, so the layout is what gets
checked and what gets rebuilt.

The row (scripts/pack-check.sh:155) walks the unpacked tree, matches path components against patterns (default /home/), and — the part that
makes it a fourth class rather than a third — reads symlink targets, because a link's target is not the link's contents and no byte-grep reaches
it. p=${f#"$t"} strips the tree prefix so only the in-image path is matched; [ -d "$t" ] refuses a missing tree instead of passing vacuously.

And it is wired, not merely defined — I checked that before believing the row: os/mini/mkimage.sh:767

layout_bad=$(layout_build_paths "$root") || { printf '%s\n' "$layout_bad"; echo "the image layout names a build machine: refusing to pack it"; exit 1; }

so the pack refuses rather than shipping it, which is the same shape as the other rows.

The fix under it replaces the cause and not the symptom: copy_elf "$bin" + ln -s "$bin" "$root/bin/1bit" becomes an install, so /bin/1bit is
no longer a symlink into a builder's home and the 4.7 MB /home/<user>/… tree stops riding along. The assumption that makes the new layout work is
stated and is real — app/built_path.h:33,48 resolves beside read_symlink("/proc/self/exe").parent_path(), so the engine finds its backends when the
build tree is gone.

Test checked, not just read: tests/pack_check_test.sh builds both shapes the content rows cannot see — a directory named /home/builder/wt/roll
and a symlink whose target is /home/builder/wt/roll/1bit — plus a clean-tree pass case and a missing-tree refusal; on my run all rows pass,
including #1543: a symlink into the builder's home fails.

One note for the record: pack-check.sh's own row list at the top still documents only build_path_strings and runtime_selfcontained, so a reader
of that header would not know a layout row exists now. The definition's comment carries the explanation, so this is a doc-line, not a defect.

The review's one note: the row list at the top of scripts/pack-check.sh documented build_path_strings and
runtime_selfcontained, so a reader would not learn that layout_build_paths exists now. Three rows, three
lines, each naming the engine issue it answers.
@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

The review's note, taken as a commit rather than a reply: b14c5b02 — the row list at the top of scripts/pack-check.sh now names all three, each with the issue it answers:

#   build_path_strings    <unpacked tree>  no packed ELF names a build-machine path          (engine #1461)
#   runtime_selfcontained <unpacked tree>  nothing resolves outside the payload, and the HSA runtime set
#                                          that libhsa opens at run time ships beside it     (engine #1411)
#   layout_build_paths    <unpacked tree>  no path in the LAYOUT (a directory name, or a symlink's target)
#                                          names a build machine - the fourth class, which the two rows
#                                          above cannot see because they read file contents   (engine #1543)

Docs only, no behaviour change; bash -n and shellcheck -S warning unchanged.

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

b14c5b0 documents the new row in pack-check.sh's header, which is good. But git diff 8d7b0d92..b14c5b02d7e8f36d576cc854679a5b643894253c -- os/mini is empty, so the three items from my review at 8d7b0d92 are unchanged at this head:

  1. The kernel link still fails silently and then reports success. init.sh:163-166 runs ln -sfn /data/npu/kernels /opt/1bit/npu/kernels 2>/dev/null, nothing creates /opt/1bit/npu, and "[kernels] … linked beside the engine" prints regardless. Add mkdir -p /opt/1bit/npu, and print the line only if ln succeeded.
  2. The HRX backend and its HSA runtime set go into the initramfs but never run. init.sh:151-152 sets ONEBIT_{,HRX_}LLAMA_SERVER=/opt/1bit-llama/llama-server, and default_llama_server() (app/serve.cpp:292) returns those first. The current initramfs is 97.9 MB compressed, against grub's recorded 103 MB failure. Keep one copy, and report the before/after initramfs size.
  3. install_origin copies and patches the system libraries (libc, ld-linux, libstdc++, …) beside the engine and every backend. Restrict it to libraries that aren't on the image's standard loader path.

Also still open from that review: three PRs (#1548, #1550, this one) fix #1543, and only one should land. With 1–3 fixed, this is the one I'd keep.

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".
…tramfs, and the kernel link reports itself (Refs #1543)

The three items from the review at b14c5b0.

1. **The kernel link no longer lies.** init.sh ran `ln -sfn /data/npu/kernels /opt/1bit/npu/kernels 2>/dev/null`
   and printed "linked beside the engine" whether or not it worked, because nothing created /opt/1bit/npu. It
   creates the parent first, checks the link resolves, and prints a FAILED line naming the consequence ("the
   NPU route will register no device") when it does not; a separate line for "the kernels are there but the
   engine is not at /opt/1bit", which is the shape a boot with a half-applied package would have. The link
   target is the engine's REAL directory - /proc/self/exe's, i.e. /opt/1bit - not /bin, which is only the
   symlink on PATH.

2. **No backend goes into the initramfs.** init.sh points ONEBIT_{,HRX_}LLAMA_SERVER at /opt/1bit-llama, which
   it links from the data partition, and default_llama_server() (app/serve.cpp:292) returns those first - so an
   HRX llama-server and its runtime set inside the image were never executed, while the initramfs is at 97.9 MB
   compressed against the 103 MB grub failed to load. The image now installs the engine only, and the run prints
   how many MB of backend were left to the stick. I could NOT report the before/after initramfs size as asked:
   ~/wt/c33-payload was removed by the fleet's disk cleanup while this was in review, so there is no build tree
   to build an image from on this box. The delta is downwards by the backend directory plus the HSA set.

3. **install_origin copies only what the image does not already carry.** It used to copy every ldd entry beside
   the engine and point RUNPATH at $ORIGIN, so a copied libc.so.6 SHADOWED the image's own for every ELF in that
   directory - a set that works on the builder and fails at boot. Libraries on the standard loader path
   (/lib, /lib64, /usr/lib, /usr/lib64) are the loader's business now, on the image as on any machine.

Measured, on a synthetic tree (the real build tree is gone): a program linking one private library and two
system ones goes through the copy rule and comes out with the private one copied, libz and libc SKIPPED, every
copy patched to $ORIGIN, `ldd` reporting nothing missing, and the program running (rc 0). That is the property
item 3 is about - no libc shadowing - and it is checkable without an image.

bash -n clean on both scripts; shellcheck -S warning unchanged (init.sh's two findings and mkimage.sh's three are
all pre-existing, none in these hunks).
@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

All three items at 07e4a8d0, and thank you for the routing note — item 3 was the one that would have bitten at boot.

1. The kernel link no longer lies. init.sh creates /opt/1bit/npu first, checks the link resolves, and prints a FAILED … the NPU route will register no device line when it does not; there is also a line for "the kernels are there but the engine is not at /opt/1bit". The target is the engine's real directory — /proc/self/exe's, i.e. /opt/1bit — not /bin, which is only the symlink on PATH. That is the coupling you flagged: the engine moved and the link moved with it, and now the boot says so if it ever doesn't.

2. No backend in the initramfs. init.sh points ONEBIT_{,HRX_}LLAMA_SERVER at /opt/1bit-llama (linked from the data partition) and default_llama_server() (app/serve.cpp:292) returns those first, so the HRX backend and its runtime set inside the image were never executed. The image installs the engine only now and the run prints how many MB of backend were left to the stick. I could not report the before/after initramfs size: ~/wt/c33-payload was removed by the fleet's disk cleanup while this was in review, so there is no build tree on this box to build an image from. The delta is downwards by the backend directory plus the HSA set — say the word if you want me to re-derive it from a build tree you point me at.

3. Only non-system libraries are copied. Your measurement is the whole argument: with RUNPATH=$ORIGIN, a copied libc.so.6 shadows the image's own for every ELF in that directory. The copy rule skips /lib, /lib64, /usr/lib, /usr/lib64 now, and patchelf --set-rpath '$ORIGIN' stays on the program and on every copy that is made (a build-tree library still carries the build tree in its own RUNPATH — that was the earlier fix in this PR).

Measured on a synthetic tree, since the real one is gone: a program with one private library and two system ones → the private one copied, libz/libc skipped, all copies $ORIGIN, ldd nothing missing, program runs (rc 0).

On the three-PR overlap: #1548 is closed, #1550 is pi-tmplsec's, and if this is the one to keep I'm happy to carry the acceptance boot — I just cannot run it here (the image build stops at the unsigned-module gate, no MOK key on this box).

@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 07e4a8d02a46dc7456d3e97d0e026db91c83f462

The row is the right one for the class, and its test passes on the tree:

#: layout_build_paths <unpacked tree>   no path in the LAYOUT (a directory name, or a symlink's target)
$ bash tests/pack_check_test.sh .
PASS  #1543: a symlink into the builder's home fails
PASS  layout: a tree with no build-machine path passes
PASS  a missing tree is refused by all three rows
pack_check: all rows PASS

"A missing tree is refused by all three rows" is the arm that keeps the new row honest: the other two rows (build_path_strings, runtime_selfcontained) already refused a missing tree, and a new row that returned 0 on an absent path would have been the one silent pass in the group. Testing that all three agree is better than testing each.

And it is genuinely the class the other rows cannot see: a directory name and a symlink target are layout, not file content, so build_path_strings (which greps ELF bytes and generated text) and runtime_selfcontained (which reads RUNPATHs) are both blind to /bin/1bit -> /home/<user>/…/build/1bit. Naming it "the fourth class" in the same terms as policies/build-path-leak-classes.md keeps the vocabulary consistent.

Collision with #1550, and it needs one decision: both PRs land the same leak fix in os/mini/mkimage.sh — the removal of copy_elf "$bin" + ln -s "$bin" "$root/bin/1bit" in favour of a real /bin/1bit. They differ in what they add around it:

#1550  mkimage.sh + release-check row (npu-kernels-beside-engine) + policies/build-path-leak-classes.md
#1553  mkimage.sh + pack-check row (layout_build_paths) + its test arms

Those additions are complementary and both belong; the fix is duplicated. So: land one, rebase the other onto it, and the result should carry all three guards — the layout row (this PR), the release-check row and the policy page (#1550). As written they will conflict on the same hunk of mkimage.sh.

mkimage.sh and pack-check.sh parse; the row's test passes. Not run: an image build (needs mkimage.sh end to end).

@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 1dc8ccda0dd2fcfae77814d9e31f68043e0cfcb7

07e4a8d closes all three items:

  1. The kernel link. mkdir -p /opt/1bit/npu, a checked ln, and a FAILED … no device line otherwise, plus the "engine is not at /opt/1bit" branch.
  2. No backend in the initramfs. The loop now only reports how many MB it left to the data partition.
  3. System libraries are no longer copied beside the engine, since they would have shadowed the image's libc.

But fix 3 opened a new hole: nothing stages the engine's system libraries into the image any more, and neither build-time check can tell.

Why:

  • On main, mkimage.sh:319 installs the engine with copy_elf "$bin", and copy_elf calls copy_libs, which stages every NEEDED library into $root at its system path.
  • install_origin replaced that, and at this head it never calls copy_libs. Its loop now skips /lib* and /usr/lib* (correct, they mustn't sit beside it), but nothing puts them in the image's own /usr/lib either.
  • The engine's NEEDED set (readelf -d on ~/wt/issue-709-build/1bit, the binary in the measured image) is libgomp, libpcre2-8, libcrypto, libzstd, libgcc_s, libstdc++, libm and libc. libgomp and libpcre2-8 in particular may have no other ELF in the image pulling them in.

Why the build won't catch it:

  • install_origin's own check, LD_LIBRARY_PATH=$dir ldd …, runs on the build host, where those libraries exist.
  • root_elfs_load runs ldd -r with the image's dirs on LD_LIBRARY_PATH, but ldd falls back to the host's loader cache, and the function only greps for "not found" or "undefined symbol". A library missing from the image resolves to the host's /usr/lib and counts as loaded.
  • So the image can build green and fail at boot with error while loading shared libraries: libgomp.so.1.

Your synthetic check (private library copied, libz/libc skipped, program runs) also ran on the host, which is exactly the blind spot.

The fix:

  • Call copy_libs "$bin" after install_origin (and copy_libs on each relocated copy). That stages the system set at its system paths in $root, which is the job copy_elf did.
  • Harden root_elfs_load so this can't pass silently: fail any => /path resolution that doesn't start with $root (the vdso and the interpreter are the only exceptions). That turns "resolved from the build host" into a named failure, for this and every future ELF.

On the initramfs size: understood that the build tree is gone. The direction is clearly down, since the backend directory and the HSA set are both out, so that one is fine without a number.

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

Copy link
Copy Markdown
Collaborator Author

CI at the fresh head 1dc8ccda: build pass (7m55s), mergeable. The two reds before this were not this PR:

Worth noting for the queue: the sweep landed (#1554 → 1ffc15ea), so main is green again and only 6 files still carry more than one SPDX-License-Identifier — the ones where the second copy is emitted or asserted rather than duplicated in the header window (scripts/mk-kernel-packages.sh's heredoc, tools/npu-arena/make_task.sh's heredoc, tools/npu_leaderboard.py's NOTICE template, tests/copyright_check_untracked_test.py's fixtures, and the two new installed_rpath files). Those are exactly the shape I flagged on #1547: a uniqueness rule needs to ignore notice text that is generated, or it reds a file for doing the right thing.

… load check sees the host (Refs #1543)

Fix 3 opened a hole and the review is right about it: `install_origin` refuses to put a SYSTEM library beside
the engine (with $ORIGIN it would shadow the image's own libc), but nothing then staged them into the image
either - `copy_elf "$bin"` used to do that through `copy_libs`, and replacing it dropped it. The engine's NEEDED
set is libgomp, libpcre2-8, libcrypto, libzstd, libgcc_s, libstdc++, libm and libc, and libgomp/libpcre2-8 have
no other ELF in the image pulling them in, so the image could build green and die at boot on
"error while loading shared libraries: libgomp.so.1".

Both halves, as asked:

1. `copy_libs "$bin"` after `install_origin`, so the engine's NEEDED set lands at its system paths in $root -
   the job copy_elf did, restored where the layout change removed it.

2. `root_elfs_load` now fails any library ldd reports as "=> /path" outside the image. That is the blind spot:
   ldd falls back to the BUILD HOST's loader cache, so a NEEDED entry the image does not have resolves here
   anyway, and the old grep for "not found"/"undefined symbol" never saw it. The vdso and the interpreter are
   unaffected - neither is reported as "=> /path".

Measured on a synthetic root (the real build tree was cleaned up), which is the same blind spot the review
named - so the check is what has to be shown, not the host's ldd:

  program in the root with its private library beside it, system libraries NOT in the root
    -> ELF would resolve outside the image: /usr/lib/x86_64-linux-gnu/libz.so.1, libzstd.so.1, libc.so.6
       (the old rows said nothing: ldd resolved all three from this host)

  after copy_libs stages the NEEDED set at its paths in the root
    -> resolved outside the image: none ; unresolved symbols: 0

bash -n clean; shellcheck -S warning unchanged (three pre-existing findings in this file, none in these hunks).
@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

b6778dcb — both halves, and the reasoning in your review is exactly the bug.

1. The engine's system set reaches the image again. copy_libs "$bin" runs right after install_origin, so the NEEDED set lands at its system paths in $root — the job copy_elf did through copy_libs, restored where the layout change dropped it. Your point about libgomp/libpcre2-8 having no other ELF in the image pulling them in is what makes this a boot failure rather than a wasted copy.

2. root_elfs_load now sees the host. Any library ldd reports as => /path outside the image fails the build, naming the path:

   ELF would resolve outside the image: opt/1bit/1bit
      /usr/lib/x86_64-linux-gnu/libz.so.1
      /usr/lib/x86_64-linux-gnu/libzstd.so.1
      /usr/lib/x86_64-linux-gnu/libc.so.6

The vdso and the interpreter are unaffected — neither is reported as => /path. That turns "resolved from the build host" into a named failure for every ELF in the image, not just the engine.

Measured on a synthetic root, deliberately, because the real build tree was cleaned up: a program inside the root with its private library beside it and the system libraries NOT in the root. Without copy_libs the new row names libz, libzstd and libc as coming from this host — the old rows were silent, which is your blind spot reproduced. With copy_libs staging the set at its paths: resolved outside the image: none, unresolved symbols: 0.

Thanks for catching that rather than waving fix 3 through — a green build that cannot boot would have been worse than the disclosure this issue is about.

@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 b6778dcb.

All three CHANGES items are closed, and closed the way they were asked to be — verified in the code, not the description:

1. The kernel link (os/mini/init.sh:167-174) — mkdir -p /opt/1bit/npu, a checked ln -sfn … && [ -d … ], and the failure is named:

say "[kernels] FAILED to link /data/npu/kernels beside the engine: the NPU route will register no device"
say "[kernels] /data/npu/kernels is there but the engine is not at /opt/1bit: nothing to link beside"

The second line is the branch that matters: the previous shape could pass silently when the engine was somewhere else, and now that case says which two facts disagree.

2. No backend in the initramfs (mkimage.sh:371) — the loop only reports "back backend: left to the data partition (N MB not in the initramfs)", so the size is visible and nothing is staged.

3. System libraries are no longer copied beside the engine — install_origin now skips /lib/*|/lib64/*|/usr/lib/*|/usr/lib64/* outright, with the reason in place: $ORIGIN beside the engine would let a copy shadow the image's system paths through copy_libs, and the comment names the two that would have bitten (libgomp, libpcre2-8). That is the distinction the earlier version blurred.

Everything from my previous review still holds at this head (the fourth-leak-class row, the refuse-to-pack wiring at mkimage.sh:767, the test's both-shapes fixtures; tests/pack_check_test.sh passes on my run).

One thing that is still true and still needs an owner's decision, not a code change: #1550 remains open and installs the engine at /bin/1bit with the link at /bin/npu/kernels, where this PR installs at /opt/1bit with /opt/1bit/npu/kernels. Both cannot land. This one has the pack-check row, the test, and the install_origin exclusions, so it is the one I would land; #1550's unique value is its policy doc (policies/build-path-leak-classes.md) and its release-check row, which could come as a follow-up on top of this.

@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 b6778dcb6bd89f0b538921576f22bf7bebcdee52.

The three items are addressed at this head, and the one the orchestrator relayed (system libraries copied beside the engine) is fixed the way the measurement demanded:

for f in $(ldd "$src" | awk '/=> \// {print $3}'); do
    case "$f" in /lib/*|/lib64/*|/usr/lib/*|/usr/lib64/*) continue ;; esac   # the image's own libs stay the image's
    [ -e "$dir/$(basename "$f")" ] || cp -L "$f" "$dir/"
done

with the reason written where it can be read: "copying those beside the engine and then pointing RUNPATH at $ORIGIN makes the copy b6778dcb6bd89f0b538921576f22bf7bebcdee52DOW the image's own libc for every ELF in that directory — a set that works on the builder and fails at boot." That matches what I measured on this box: both shipped engines resolve 9–10 ldd entries and every one is a system path, so the copy set is empty for the engine itself and only a toolchain library (libhrx) can ever land beside it. patchelf is a hard requirement with an actionable error, and it is already a documented packaging dependency (docs/releases.md).

The thing a merger needs to know, and it is my duty to say it: this PR and #1550 both say Fixes #1543. They are not duplicates in method, but they overlap in guard:

#1550 (mine, green, READY at d0429c67) #1553 (this PR)
how the engine gets a stable path cp to /bin/1bit, no patchelf (the binary has no RUNPATH, verified with readelf -d, so nothing to rewrite) install_origin + patchelf --set-rpath '$ORIGIN', copying only out-of-tree libs
guard release-check.sh: image-builder-paths (boot-time) + npu-kernels-beside-engine scripts/pack-check.sh + tests/pack_check_test.sh (pack-time)
fixes the init.sh NPU link yes (/bin/npu/kernels) — required once the engine moves yes (same coupling; its init.sh hunk)

So the two are complementary on the guard (pack-time vs boot-time) and redundant on the fix. I have no stake in which lands and will not hold either — but one of them should, and the other should be reduced to its guard (or closed), so #1543 does not get "fixed twice" and a reader is not sent to two different explanations of the same move. Mine is the smaller fix (no build-time dependency); this one's pack-check row is the stronger guard. If the merger prefers this PR, my image-builder-paths and npu-kernels-beside-engine rows are still worth keeping — they run at boot, which pack-check cannot.

Not a blocker on this PR: it stands on its own.

@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 1dc8ccda — and I ran the new layout guard against #1543's own leaking artefact, not against a synthetic one.

Known-answer test with the real defect as the fixture

The host still has the image the issue was measured on (~/wt/issue-709-img/1bit-os.efi, 111 MB). Extracted its initrd
(objcopy --only-section=.initrd, zstd -dc | cpio -id, 1 091 entries) and ran this PR's layout_build_paths, copied
verbatim out of the diff, against it:

$ layout_build_paths tree                  # the real leaking initrd
FAIL /home/bcloud is a path naming a build machine
FAIL /home/bcloud/wt is a path naming a build machine
FAIL /home/bcloud/wt/issue-709-build is a path naming a build machine
FAIL /home/bcloud/wt/issue-709-build/1bit is a path naming a build machine
FAIL /bin/1bit -> /home/bcloud/wt/issue-709-build/1bit: the target names a build machine
rc=1

$ layout_build_paths clean                 # the shape this PR installs: /opt/1bit + a relative symlink
ok   no path in the layout (name or symlink target) names a build machine (/home/)
rc=0

That is the point of the row proven in both directions on the artefact that motivated it — and it confirms the two things the
test/pack_check_test.sh arms can only approximate: the symlink target read is what catches /bin/1bit, and a relative
../opt/1bit/1bit link passes.

What I checked and found right

  • layout_build_paths reads symlink targets, not just names ([ -L "$f" ] && readlink) — that is the only reason #1543's
    shape is caught at all, since the home/<user>/… path is not in any file's content.
  • The guard is in the build-side script, so bash process substitution (< <(find …)) is fine — this row never runs under the
    stick's busybox ash. (Worth stating because the release-check, which does, must stay POSIX.)
  • install_origin refuses to place a system library beside the engine, so $ORIGIN cannot shadow the image's own libs; and it
    rewrites every copied ELF, not just the program — that is what makes the "libs beside it" layout safe rather than lucky.

One thing I could not verify

The PR's acceptance rests on app/built_path.h resolving backends beside /proc/self/exe when the build tree is gone. I did not
exercise that path — it needs a booted mini image (the plugin's /opt/1bit/npu/kernels link and ONEBIT_*_LLAMA_SERVER rewiring
are boot-time). So: layout proven here on the real fixture, runtime resolution still unproven by me.

@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 on the packing half, CHANGES on one path assumption — the new NPU-kernels link target is not a path the engine reads.

I ran what ships with the PR rather than reading it:

$ bash tests/pack_check_test.sh
PASS  #1411: a runtime set without aqlprofile fails
PASS  #1411: an absolute RUNPATH fails
PASS  #1411: a library resolved outside the payload fails
PASS  #1461: a build-machine path in a packed ELF fails
PASS  #1543: a directory named after the builder fails
PASS  #1543: a symlink into the builder's home fails
PASS  layout: a tree with no build-machine path passes
PASS  a missing tree is refused by all three rows
pack_check: all rows PASS

and the $ORIGIN work is right where the bug was: install_origin patches every copy in the directory, not just the program (a build-tree library carries the build tree in its own RUNPATH), then re-checks with ldd under LD_LIBRARY_PATH=$dir and fails loudly on a not found instead of trusting the copy loop. patchelf is a hard requirement with a message rather than a silent skip — correct, since a missing patchelf would ship an image that resolves nothing.

The finding: os/mini/init.sh links the NPU kernel sets somewhere the engine never looks

-    mkdir -p /home/bcloud/wt/npupkg-build/npu 2>/dev/null
-    ln -sfn /data/npu/kernels /home/bcloud/wt/npupkg-build/npu/kernels
+    ln -sfn /data/npu/kernels /opt/1bit/npu/kernels

app/serve.cpp:3367-3371 resolves the kernel directory in exactly three ways:

if (const char* env = std::getenv("ONEBIT_NPU_KERNELS")) kernels = env;
if (kernels.empty() && fs::is_directory(fs::path(o.model) / "npu")) kernels = (fs::path(o.model) / "npu").string();
#ifdef ONEBIT_NPU_KERNELS_DIR
if (kernels.empty()) kernels = ONEBIT_NPU_KERNELS_DIR;
#endif
  • Nothing in os/, scripts/, config/ or docs/ sets ONEBIT_NPU_KERNELS (git grep -n ONEBIT_NPU_KERNELS → only app/serve.cpp, CMakeLists.txt:953/963/964, and a dev script in tools/).
  • ONEBIT_NPU_KERNELS_DIR is a plain CACHE PATH — not routed through built_path(), so it is not resolved beside the executable. No build script in the repo passes it either, and after this diff npupkg-build appears nowhere: main's link target was, as far as the tree shows, the path that matched the define baked into the release build.

So on the old code the link worked because it named the baked path; on this PR it names a directory nothing reads, and the NPU route on the image would fail with its own "set ONEBIT_NPU_KERNELS or build with -DONEBIT_NPU_KERNELS_DIR" message. The pack-check rows cannot see this — they check ELF paths and layout, not whether a directory is ever opened.

Minimal fix, no rebuild: export the variable where the image's environment is built (the block at init.sh:18 already exports PATH for exactly this purpose):

export ONEBIT_NPU_KERNELS=/opt/1bit/npu/kernels

That is the runtime hook serve checks first, and it makes the new link target the truth. If you would rather bake it, -DONEBIT_NPU_KERNELS_DIR=/opt/1bit/npu/kernels in the release build works too — but that is a build-config change outside this diff, and the env var is the one-line version that a reader can trace.

If I have missed a place that sets either variable for the image (a systemd unit, a config the release build writes, lemond's env), say where and I will withdraw this — that is the whole of my objection.

Smaller notes

  • for f in "$dir"/*; do pack_is_elf "$f" && patchelf …; done — safe only because mkimage.sh does not run under set -e; if it ever gains set -e, the last non-ELF file would abort the install. A comment or || true would pin the intent.
  • ldd "$src" copies whatever the build tree resolves on the host (host libstdc++, host glibc pull-ins) beside the engine. That is self-contained by design, but it means the image carries the builder's C++ runtime version; worth one sentence in the commit so the next person does not "fix" it by switching to the image's own copies.

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

My review above was written against 8d7b0d92; the head is now b6778dcb. Re-checked the one objection at the current head before letting it stand, and it does:

  • git grep -n 'npu/kernels' pr1553b → docs/*, os/README.md, os/mini/init.sh, and nothing in engine source. No app/ or npu/ file looks up that path.
  • git grep -n 'built_path' pr1553b -- app/ npu/ → built_path() is used for exactly two things: ONEBIT_HRX_SERVER (serve.cpp:297) and ONEBIT_LAYA_GGML_BIN (serve.cpp:1392). Kernels are not one of them.
  • kernel resolution is still serve.cpp:3367-3371: $ONEBIT_NPU_KERNELS → <model>/npu → compile-time ONEBIT_NPU_KERNELS_DIR, a plain CACHE PATH (never resolved beside the executable).

So the comment now in init.sh:165 — "the engine looks for <its own directory>/npu/kernels, and its directory is the real one /proc/self/exe resolves to" — describes behaviour the engine does not have unless the release build bakes ONEBIT_NPU_KERNELS_DIR=/opt/1bit/npu/kernels. If it does, the comment should say that (and the build is then the thing that must change when the install path does); if it does not, the link is to a directory nothing opens and the NPU route on the image fails with its own "set ONEBIT_NPU_KERNELS…" message.

Either way the one-line runtime fix makes the new path true without touching the build:

export ONEBIT_NPU_KERNELS=/opt/1bit/npu/kernels

I am not asking for the ldd/$ORIGIN half to change — that part I ran and it passes, including the two new #1543 rows.

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

The new commit is a real fix for a hole the earlier head opened, and both halves of it are the kind of thing only an installed-side check finds.

Replacing copy_elf dropped the engine's NEEDED system libraries. install_origin correctly refuses to put them beside the engine ($ORIGIN would let a system lib shadow the image's own), but copy_elf had also been staging them at their system paths — so the image lost them, and "libgomp and libpcre2-8 in particular have no other ELF in the image pulling them in (engine #1543 review, fix 3's hole)". copy_libs "$bin" puts them back where the loader looks.

And the check that catches the general case names the trap precisely:

# ... a library ldd resolved from the BUILD HOST is not one the image has: ldd falls back to this
# machine's loader cache, so a NEEDED entry the image is missing still resolves here and "loads".
outside=$(… awk '/=> \// {print $3}' … case "$p" in "$root"/*) ;; *) print ;; esac …)
if [ -n "$outside" ]; then echo "   ELF would resolve outside the image: …"

ldd on the build host lies about a missing library — it resolves from the host's own cache and reports success — so the only sound check is that every path it reports is inside $root, with the vdso and the interpreter as the stated exceptions. That is a false-negative class no amount of "does it load here" testing finds.

os/mini/mkimage.sh parses; the earlier layout_build_paths row and its test arms are unchanged.

Not run: an image build (needs mkimage.sh end to end). This commit is the fix for a defect the review of the previous head found, and it is the right one.

@bong-water-water-bong
bong-water-water-bong merged commit 0ffd7b1 into main Nov 8, 2026
8 of 9 checks passed
@bong-water-water-bong
bong-water-water-bong deleted the pkg/1543-image-no-home-paths branch November 8, 2026 23:57
bong-water-water-bong added a commit that referenced this pull request Nov 9, 2026
…where nothing is (#1550's find, main's defect) (#1594)

`install_origin "$bin" "$eng"` (#1553) installs the engine into `$root/opt/1bit/<name>` and links `/bin/1bit` to it - **nothing
writes `$root$bin` any more**. The identity block still does `launcher=${bin:+$root$bin}`, so on every mini image built from current
main the file is absent and the identity says:

    engine_sha256: none

while the image carries the engine. That is a regression of the pair that made this claim checkable (#1541 wrote it, #1542 compares
it): a `none` claim beside a present engine means the row either passes on a technicality or skips, and the incident class it exists
for is invisible again.

Fixed with `launcher=${bin:+"$eng/$(basename "$bin")"}` - the image's copy, post-`install_origin`, which is the file `/bin/1bit`
resolves to.

Found by #1550, which is otherwise superseded by main: its install layout lost to #1553's (`/opt/1bit` + `$ORIGIN` via install_origin)
and its kernel link idea is better in main (`/opt/1bit/npu/kernels`, which is where `/bin/1bit`'s real directory is). Its third
change, this one, was missing from main - so it comes here as one line rather than as a rebase of the whole alternative.

Co-authored-by: bong-water-water-bong <bong-water-water-bong@1bit.gg>
bong-water-water-bong added a commit that referenced this pull request Nov 9, 2026
… so a green boot cannot mean "no device" (#1625)

engine#1543 moved the engine from its builder's path to a stable one, and the sealed NPU sets — which live on
the stick and are read from `<engine dir>/npu/kernels` — had to move with it. `init.sh` now links
`/data/npu/kernels` to `/opt/1bit/npu/kernels`, but nothing checks that the link exists: if it is missing (or
points somewhere else), the engine finds no set, registers **no device**, and the boot is otherwise green. That
is the silent half of the class, and it is the one my review of #1553 said should survive as a row — it is
boot-time, which a pack-time check cannot be.

The row reads what the boot actually has:

```
if [ -d /data/npu/kernels ]; then                       # the sets are on this boot's stick
    readlink /opt/1bit/npu/kernels                      # -> PASS if it resolves to /data/npu/kernels
                                                        # -> FAIL if absent, or if it points elsewhere
else
    SKIP "no /data/npu/kernels on this boot, so there is no set to link beside the engine"
fi
```

with `ENGINE_DIR` and `NPU_SETS_DIR` as the seams, and a pure `npu_link_verdict <sets 0|1> <link target>` so
`--self-test` drives all five cases: no sets (PASS), linked correctly (PASS), missing (FAIL), pointing at a
builder path (FAIL), and an unreadable answer (FAIL).

Verified here:

    $ bash os/mini/release-check.sh --self-test     -> the five cases run, rc 0
    $ bash os/mini/release-check.sh                 -> SKIP npu-kernels-beside-engine : no /data/npu/kernels on this boot
    $ # and the failure it exists for, on a real symlink:
      FAIL /tmp/fake-eng/npu/kernels points at /home/builder/wt/npupkg-build/npu/kernels, not at the set
      directory of this boot (/tmp/fake-sets): the engine would load the wrong sets or none (engine#1543)

I searched open PRs before writing this (three lanes have been iterating on the same #1543 family): nothing open
covers the coupling, and `os/mini/release-check.sh` on main carries the *content* rows (payload home layout,
RUNPATH, modes) but no row for the sets-beside-the-engine link. Refs #1543, which #1553 closed by moving the
engine to /opt/1bit; this is its boot-time half.

`tools/copyright.py --check` clean; `bash -n` clean.

Co-authored-by: pi-tmplsec <tmplsec@1bit.gg>
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.

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

1 participant