Skip to content

fix(recipe): validate customMount fstypes and reject encryption on manual layouts - #58

Merged
hanthor merged 1 commit into
devfrom
fix/validate-manual-layouts
Jul 26, 2026
Merged

fix(recipe): validate customMount fstypes and reject encryption on manual layouts#58
hanthor merged 1 commit into
devfrom
fix/validate-manual-layouts

Conversation

@hanthor

@hanthor hanthor commented Jul 25, 2026

Copy link
Copy Markdown
Member

Two ways a manual (customMounts) recipe could pass Validate() and then do the wrong thing later. Both hit in practice by tuna-os/bootc-installer-asahi.

1. Unsupported fstypes were only caught mid-install

Validate() checks r.Filesystem on the auto-partition path but checked nothing on the manual path, so an unsupported customMount fstype survived validation and only failed once disk.ApplyCustomLayout() reached formatPartition().

Callers treat validation as the gate before committing to a disk. A caller passing "vfat" for an ESP — the obvious spelling, and not one we accept; formatPartition knows "fat32" — got exactly that: validate passed, install died mid-flight after partitioning. Confirmed empirically in CI against the shipped binary before this change.

Now rejected up front, naming the offending value and listing what's accepted — including "unformatted"/"" for mounting a pre-populated partition without reformatting. That case is load-bearing: an existing ESP holds the bootloader and, on Apple Silicon, non-redistributable vendor firmware extracted on-device. Reformatting it is unrecoverable.

2. Encryption was silently ignored on manual layouts

luksFormat/luksOpen run only in the auto-partition branch, and TPM enrolment needs an activeRootPart that manual mode leaves empty. An encrypted manual recipe produced an install that completed unencrypted while the caller reported success — a security-boundary failure, not a missing feature.

Fails closed, same shape as the existing ZFS+LUKS rejection just below it.

Blast radius

wootc is the other consumer and uses the auto-partition path ("disk": <dev>), zero customMounts usage. Neither check can fire for it.

Six tests. Same change proposed to projectbluefin/fisherman as #12.

🤖 Generated with Claude Code

…nual layouts

Two ways a manual (customMounts) recipe could pass Validate() and then do the
wrong thing later. Both were hit in practice by tuna-os/bootc-installer-asahi.

## Unsupported fstypes were only caught mid-install

Validate() checked r.Filesystem on the auto-partition path but nothing on the
manual path, so an unsupported customMount fstype survived validation and only
failed once disk.ApplyCustomLayout() reached formatPartition().

That matters because callers treat validation as the gate before committing to
a disk. A caller passing "vfat" for an ESP — the obvious spelling, and not one
we accept; formatPartition knows "fat32" — got exactly that: validate passed,
the install died mid-flight after partitioning had already happened. Confirmed
empirically in CI against the shipped binary before this change.

Now rejected up front, naming the offending value and listing what is
accepted, including "unformatted"/"" for mounting a pre-populated partition
without reformatting it. That case is load-bearing: an existing ESP holds the
bootloader and, on Apple Silicon, non-redistributable vendor firmware
extracted on-device, so reformatting it is unrecoverable.

## Encryption was silently ignored on manual layouts

luksFormat/luksOpen run only in the auto-partition branch, and TPM enrolment
additionally needs an activeRootPart that manual mode leaves empty. So an
encrypted manual recipe produced an install that completed UNENCRYPTED while
the caller reported success — a security-boundary failure, not a missing
feature.

Fail closed, in the same shape as the existing ZFS+LUKS rejection just below.
The error explains the consequence rather than just naming the field, because
"unsupported" reads as a limitation while "would complete unencrypted" reads
as the bug it is.

## Blast radius

wootc is the other consumer of this package and uses the auto-partition path
("disk": <dev>), with zero customMounts usage in the repo. Neither check can
fire for it.

Six tests. Same change is proposed to projectbluefin/fisherman as PR #12.
@hanthor
hanthor force-pushed the fix/validate-manual-layouts branch from 3837cc2 to 98bdb2b Compare July 26, 2026 00:02
@hanthor

hanthor commented Jul 26, 2026

Copy link
Copy Markdown
Member Author

Rebased onto dev now that #59 (the projectbluefin sync) has landed, and re-running CI.

Worth recording why, because the first run here was misleading. This PR originally failed required / dakota, advisory / arch-bootc, advisory / arch-bootc-luks-xfs and advisory / debian-bootc — while #59, which does not contain this change, passed all twelve checks.

The dakota failure was:

fisherman: fatal: remounting root partition after retagging:
mount /dev/loop0p2 /mnt/fisherman-target: exit status 32

That is the auto-partition path. This PR only touches Validate()'s customMounts branch, and nothing outside the library uses customMounts at all — so it could not have caused it. What it actually was: dev was red on the very bugs the sync repairs, specifically a883428 (mount the root with an explicit -t — the initramfs cannot auto-detect) with b8d7905 explaining why we only ever saw a bare errno.

So the failures were pre-existing breakage in this fork, not a regression here. Rebasing rather than merging into a red base, so that if anything fails now it is genuinely attributable to this change.

@hanthor
hanthor merged commit 0cdf755 into dev Jul 26, 2026
12 checks passed
hanthor added a commit to tuna-os/bootc-installer-asahi that referenced this pull request Jul 26, 2026
… useradd claim (#40)

## Drop the projectbluefin workaround

The bootstrap image pinned projectbluefin/fisherman because tuna-os/fisherman
lacked a883428 (mount the root with an explicit -t), without which an xfs root
gets attempted as ext4 and the install dies. That was a workaround for a fork
divergence, not a preference.

tuna-os/fisherman#59 synced all 14 commits across — and that sync was
load-bearing rather than housekeeping: tuna-os/fisherman's own CI was RED on
exactly those bugs (`mount /dev/loop0p2 ...: exit status 32`, the bare-errno
symptom b8d7905 exists to explain) and went 12/12 green with the sync applied.

Our fork now additionally carries two things projectbluefin does not: TPM2
enrolment on first boot, and the customMounts validation from
tuna-os/fisherman#58 — which rejects unsupported fstypes and refuses
encryption on manual layouts, both defects this project actually hit.

Switching the pin measurably upgrades the guarantee. Verified by building the
newly-pinned revision and running the disk selftest against it:

  before (projectbluefin d12b6cb2): "note: this fisherman ACCEPTS fstype=vfat
                                     at validate time"
  after  (tuna-os 0cdf755f):        "ok: fisherman rejects the vfat defect at
                                     validate time"

So the recipe defect that shipped is now caught by the real consumer, before
any disk is touched, rather than only by our own assertions.

## Correct a claim I repeated in three documents

"chroot <target> useradd instead of useradd --root" is SUPERSEDED, and it
appeared in the contract doc, the agent README and the testing checklist.

Reading the four useradd commits in order: 5025d4d moved to chroot because
--root drags in the host's PAM/SELinux stack; e2a6499 then REVERSED that for
composefs-native (dakota exit 127) back to --root; f94a716 and d12b6cb
refined it further. Classic ostree and composefs-native need different
handling and fisherman detects which at runtime.

So the correct statement is not "chroot is right" — it is "these two layouts
differ". Any phrasing of the form "use chroot, not --root", including earlier
revisions of these documents, quotes one step of a sequence as the
conclusion. Marked as superseded rather than deleted, since the earlier
claim is referenced from issue #6's thread.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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