Repository navigation
Conversation
jeefy
added
the
full-build
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Needs approval from an approver in each of these files:Approvers can indicate their approval by writing |
![hivecommons-hive[bot]](https://avatars.githubusercontent.com/in/3568013?s=60&v=4){kind=small}
There was a problem hiding this comment.
HUMAN DECISION NEEDED — @jeefy merge only after the full-build boot-test (A/B sysupdate) is green; it is the only check that exercises the gpg verification path this prune leaves in place.
Reviewed at abbd646. The removals are mechanically safe: files/prune/prune.sh:17-19 fails the build on any pattern that matches nothing, and image-check (.github/workflows/build.yml:603-644) catches any unit/udev/library reference to a pruned path. No script under files/ or any page under docs/ invokes a pruned tool (only gpgv in files/os/update-check/usr/libexec/bluefin-diskless-update-check:93 and files/initrd-ignition/usr/libexec/bluefin-ignition-credentials:90, both kept).
correctness
- The one load-bearing claim neither tool can see is that
systemd-pull'sgpgexec still works with 17 gnupg binaries and 7 libexec helpers gone (files/prune/usr.list:7-36). The PR body's bwrap test covers it, but the end-to-end proof is the A/B update inboot-test(.github/workflows/build.yml:364-401), which is running now under thefull-buildlabel. Gate the merge on it. - Test adequacy (low): the PR establishes that
gpg-agentandkeyboxdare load-bearing (files/prune/usr.list:7-9,docs/skills/ddi-installer-build.md:250-252), buttests/unit/test_check_image.py:185still protects onlyusr/bin/gpg,usr/bin/gpgv,usr/lib/locale,usr/lib/systemd. Addusr/bin/gpg-agentandusr/libexec/keyboxdto the protected tuple so a later prune cannot silently remove them.
style
usr/bin/lv*,usr/bin/vg*,usr/bin/pv*(files/prune/usr.list:50-52) are the only prefix globs in the lists; every other block, including the 17 ICU tools infiles/prune/common.list:8-27, names each binary. A future unrelatedpv/lv*/vg*binary would be pruned silently, andimage-checkcannot flag interactive tools. List the LVM commands explicitly for consistency.
No findings from: security, intent-alignment, docs-currency.
Confidence: 3/5 (needs attention) — 1 medium finding; 1 perspective requires a human decision
— hive: agent=reviewer backend=copilot model=claude-fable-5.1 copilot=1.0.88
abbd646 to
d6f61d4
Compare
There was a problem hiding this comment.
This change shrinks the server's read-only system image by deleting command-line tools nothing on a running node uses (GnuPG extras, LVM, cracklib, the OpenSSL CLI and similar). I read the diff plus files/prune/prune.sh, .github/scripts/check-image.py, the FSDK lvm2/cracklib/gnupg element definitions at the pinned ref, and scanned the v26.10.1182 usr.raw to check the body's claims. The pruning itself looks safe; the one thing worth fixing is how the lists are organised.
style
files/prune/initrd.list:6-19is now a strict subset offiles/prune/usr.list(all 11 entries:sqlite3,gpgsm,gpg-wks-*,gpgtar,gpg-card,gpg-mail-tube,gpgsplit,gpgparsemail,libexec/gpg-wks-client,libexec/scdaemon).files/prune/common.list:1anddocs/skills/ddi-installer-build.md:230-232definecommon.listas the list applied to both trees, so these belong there; keeping the same entries in two per-tree lists is the driftcommon.listexists to prevent. (Ifinitrd.listends up empty,tests/unit/test_check_image.py:174-181asserts each list is non-empty, so either keep one initrd-only entry or adjust the parametrize.)files/prune/usr.list:50-52(usr/bin/lv*,usr/bin/vg*,usr/bin/pv*) are the only open-ended globs in the list;prune.sh:16-23only errors when a pattern matches nothing, so any future FSDK binary starting withpv/lv/vgis deleted silently. Everything else in the list names binaries explicitly — consider the same here, or a comment that the globs are intentional.
correctness (info, not blocking)
- Verified against v26.10.1182
usr.raw: nopw_dict.{pwd,pwi,hwm}exists (body's cracklib claim holds), and nolvm2-monitor/dm-event/lvm2-lvmpolldunits ship (the only name hits are text inside thelvmdumpscript), so droppingusr/bin/lvm/dmeventdleaves no danglingExec*=. The end-to-endsystemd-pull → gpgproof the body defers to CI is covered byboot-test(.github/workflows/build.yml:542-552); thefull-buildlabel is set, so that job should be green before merge.
No findings from: security, intent-alignment, docs-currency.
Confidence: 5/5 (safe)
— hive: agent=reviewer backend=copilot model=claude-fable-5.1 copilot=1.0.88
… /usr /usr is shared by the diskless, disk and installer boots, so every entry is something none of them runs. Groups added to files/prune/usr.list: - GnuPG beyond verification: gpgsm, gpgtar, gpg-card, wks, dirmngr, gpgconf, gpg-connect-agent, scdaemon and the agent helpers, plus the libgpg-error/libgcrypt developer tools and Lisp bindings. gpg, gpgv, gpg-agent and keyboxd stay: systemd-pull (behind systemd-sysupdate) execs gpg, and bluefin-diskless-update-check runs gpgv. Checked by running gpg exactly as systemd-pull does, and gpgv, against the release SHA256SUMS(.gpg) in the pruned tree: good signature accepted, tampered manifest refused. - cracklib tools and its uncompiled word list; no pw_dict is built into the image. libcrack stays for libpwquality. - LVM2: lvm and its lv*/vg*/pv* links, dmeventd, fsadm, blkdeactivate, liblvm2cmd, libdevmapper-event and the dmeventd plugins, and 11-dm-lvm.rules (acts only on LVM- devices) and 69-dm-lvm.rules (runs lvm pvscan). libdevmapper, dmsetup and 10-dm, 13-dm-disk and 95-dm-notify.rules stay for dm-verity /usr and cryptsetup; the initrd is untouched. - openssl CLI (libssl/libcrypto stay). - cal, hexdump, look, mesg, more, rev, ul: nothing on a node runs them; less is the pager. - sqlite3 CLI (libsqlite3 stays), build-time generators (compile_et, mk_cmds, asn1*, pcre2test, bashbug), Python scripts with no Python in the image (psl-make-dafsa, routel, xfs_protofile), libdevmapper's header and .pc, and zsh/fish completions with no zsh or fish. just check-image against v26.10.1182 passes: 190 paths removed, 0 added, no new or stale KNOWN_BROKEN waivers. usr.raw: 196075520 -> 186998784 bytes (-8.7 MiB, -4.6%) OS DDI: 480256512 -> 471179776 bytes disk UKI: 81622960 -> 81627648 bytes (initrd unchanged) Also note in ddi-installer-build.md that the check cannot see programs a binary execs itself, and why systemd features are not disabled. Assisted-by: Claude Fable 5.1 via GitHub Copilot Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…protect gpg-agent Review follow-ups: the eleven entries initrd.list and usr.list both carried move to common.list, which exists for exactly that; initrd.list is left empty with a note, and the list test no longer requires each list to be non-empty. The lv*/vg*/pv* globs are spelled out as the 48 lvm2 names FSDK ships, so a future binary starting with those letters cannot be swept up silently. The prune-list test now also protects gpg-agent and keyboxd, which systemd-pull's gpg needs. Assisted-by: Claude Fable 5.1 via GitHub Copilot Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
b2248ad to
0e26a8b
Compare
There was a problem hiding this comment.
HUMAN DECISION NEEDED — @jeefy asks for the full-build label so CI's boot-test runs an A/B systemd-sysupdate before merge; that is the only end-to-end proof of the pruned tree's gpg exec path, which check-image cannot see (.github/scripts/check-image.py:10-12 only follows Exec*= and udev RUN/PROGRAM/IMPORT). A maintainer must decide whether to gate merge on that run.
correctness
- (open question, low)
files/prune/usr.list:33deletesusr/share/cracklibwholesale.elements/bluefin-server/os-filesystem.bst:20composes withintegrate: True, and FSDK'scomponents/cracklib.bstintegration command runscreate-cracklib-dict -o /usr/share/cracklib/pw_dict …into that very directory, so a compiledpw_dict.{pwd,pwi,hwm}can exist there beforeprune.shruns (elements/oci/bluefin-server-usr.bst:90). The PR body says the image never shipped one; please confirmpw_dict.*was not among the 190 removed paths in thecheck-imagediff — if it was,pam_pwquality's defaultdictcheckwould start failing every password change with "error loading dictionary".
style
files/prune/usr.list:118—# SQLite CLI (gpg, keyboxd and liblastlog2 keep libsqlite3).is now an orphan comment: theusr/bin/sqlite3entry moved tofiles/prune/common.list:43. Delete it (or move the note there).
No findings from: security, intent-alignment, docs-currency.
Confidence: 3/5 (needs attention) — 1 medium finding; 1 perspective requires a human decision
— hive: agent=reviewer backend=copilot model=claude-fable-5.1 copilot=1.0.88
This branch has not been deployed
1 participant
First of two "how minimal can the runtime image be" PRs. This one extends
files/prune/usr.list(the #419 mechanism) with tools no unit, udev rule, script or documented operator workflow uses. −9.1 MB usr.raw (−4.6%), proven by a full export andcheck-imageagainst v26.10.1182: 190 paths removed, 0 added, no staleKNOWN_BROKENwaiver.A finding worth knowing: the installer's
/usris aCopyBlocks=of the sameusr.raw, so there is no "installer-only" tier — pruning must be safe for diskless, disk and stick alike.systemd-sysinstallis C and needs none of this.Pruned
share/gnupg): gpgsm, dirmngr*, scdaemon, gpg-wks-*, gpgtar, gpgsplit, kbxutil, gpgconf, gpg-connect-agent, … plus libgpg-error/libgcrypt dev tools and their Lisp bindings. Keptgpg,gpg-agent,keyboxd,gpgv:systemd-pullexecsgpg(thengpg2) to verifySHA256SUMS.gpgon every sysupdate —check-imagecannot see that (nothing in a unit names it). Verified by runninggpgwithsystemd-pull's exact arguments andgpgvasbluefin-diskless-update-checkdoes, inside a bwrap root built from the pruned/usr: good signature accepted, tampered manifest refused.share/cracklib: the image never shipped a compiledpw_dict, sopam_pwquality's behaviour is unchanged;libcrackstays (libpwquality links it).lvm+ 48 links,dmeventd,fsadm,blkdeactivate,liblvm2cmd,libdevmapper-event*, the device-mapper plugin dir,11-dm-lvm.rules,69-dm-lvm.rules. Keptlibdevmapper,dmsetup(+dmstatslinks) and10-dm/13-dm-disk/95-dm-notify.rules— dm-verity/usrand cryptsetup depend ondmsetup udevflags/udevcomplete.11-dm-lvm.rulesexits unlessDM_UUIDstarts withLVM-. The initrd is untouched.cal hexdump look mesg more rev ul; sqlite3 CLI (the initrd already prunes it); build-time generators (compile_et mk_cmds asn1* pcre2test bashbug); Python scripts with no Python (psl-make-dafsa routel xfs_protofile);share/zsh,share/fish(neither shell ships);usr/include,lib/*/pkgconfigleftovers.Not done, documented
docs/skills/ddi-installer-build.mdrecords why disabling systemd features (homed, userdb, portabled, oomd, localed, timedated, backlight, rfkill, hibernate, xdg-autostart) was not attempted: it needs a patch on FSDK's systemd element, which takes systemd and every dependent off FSDK's artifact cache for every build, for a few MB. Not until measured.Sizes (v26.10.1182 → this)
Verification
just validate;just export-image+just check-imagevs v26.10.1182 (190/0); gpg/gpgv signature checks in a pruned root;veritysetup,cryptsetup,dmsetup,systemd-sysupdate,passwdstart from the pruned tree; pytest 1325 passed; docs-checks. Not run: a QEMU A/B update (just dogfood-install … next) — that is the end-to-end proof of thegpgpath; please labelfull-buildso CI's boot-test runs it before merge.