Skip to content

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

Open
hanthor wants to merge 1 commit into
devfrom
fix/validate-manual-layouts
Open

fix(recipe): validate customMount fstypes and reject encryption on manual layouts#12
hanthor wants to merge 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 were 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().

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, install died mid-flight after partitioning had already happened.

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

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

Blast radius — checked before changing shared code

wootc is the other consumer and is unaffected. It uses the auto-partition path ("disk": ${LOOP_DEV} in payload/deployer/deploy.sh), and a code search finds zero customMounts usage in the repo. Neither new check can fire for it.

Tests

Six, covering: unsupported fstype rejected and named; both skip-format sentinels accepted; all five formattable types accepted; both encryption types rejected with the consequence explained; none/"" still accepted.

Not merging this myself since fisherman is shared — happy for you to take it whenever.

🤖 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.

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

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

Six tests covering: unsupported fstype rejected and named, both skip-format
sentinels accepted, all five formattable types accepted, both encryption types
rejected with the consequence explained, and none/"" still accepted.
hanthor added a commit to tuna-os/bootc-installer-asahi that referenced this pull request Jul 25, 2026
Every assertion about the generated recipe was OUR opinion of what fisherman
accepts. That is exactly how the vfat defect survived: the recipe looked right
to us and was never shown to fisherman, which rejects "vfat" (it knows
"fat32") only once it reaches formatPartition — mid-install, after
partitioning has already happened.

test-agent-disk.sh now runs `fisherman validate` against the recipe the agent
actually produced. Non-destructive: validate parses and checks, it does not
mutate.

Two things make this a real check rather than a ritual:

- It also feeds fisherman the exact defect that shipped (fstype=vfat). If that
  were accepted, the positive check above would prove nothing. With
  projectbluefin/fisherman#12 in, it is rejected at validate time and the test
  says so; without #12 it is accepted, and the test says THAT — pointing at
  our own fstype assertions as the only gate meanwhile.
- Absence of a fisherman binary is a loud SKIP naming what is not being
  checked, not a silent pass.

CI builds the exact revision the bootstrap image ships, so the validator under
test is the one that will run on hardware.

Verified locally against a fisherman built with #12 applied: our recipe is
accepted, the vfat variant is rejected. The pre-#12 branch is reasoned from
reading Validate() (it checks no customMount fstypes at all), NOT executed —
the shallow clone I had did not reach the pinned revision, and CI will settle
it on the next run.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
@hanthor

hanthor commented Jul 26, 2026

Copy link
Copy Markdown
Member Author

Landed the same change in tuna-os/fisherman (#58) — 12/12 green there, including all six VM E2E jobs (arch-bootc, arch-bootc-luks-xfs, dakota-luks, dakota-luks-btrfs, debian-bootc, yellowfin-gnome50) and both required (centos-bootc, dakota). So the two new rejections don't disturb any real install flow, which is the thing worth knowing before tightening a validator.

That fork was also 14 commits behind this one, so I synced it (#59) — and the sync turned out to be load-bearing rather than housekeeping. tuna-os/fisherman's dev was red on:

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

which is a883428 (mount the root with an explicit -t) — with b8d7905 explaining why we only ever saw a bare errno. Merging your side turned that CI green.

cc @castrojo, since it sharpens the consolidation question I raised on tuna-os#59: it isn't just two trees drifting, it's one tree shipping install-breaking bugs the other fixed a week earlier, with the CI failures having become background noise. tuna-os/bootc-installer-asahi pins this fork specifically because of a883428 — a workaround for the divergence rather than a preference.

No rush on this PR from my side; it's the same diff and the tests are here whenever it's useful.

bketelsen referenced this pull request in frostyard/fisherman Jul 29, 2026
…nual layouts (tuna-os#58)

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