hulthe/boco
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix: first 3 review findings (unshare reads, unmount cleanup, qemu exit status) #1

Merged
marvin merged 1 commits from fix/review-bug-1-2-3 into master 2026-08-25 22:47:37 +02:00
Conversation 1 Commits 1 Files Changed 1
+89 -56
marvin commented 2026-08-25 22:45:52 +02:00
Collaborator
Copy Link
Copy Source

Fixes the top three findings from the code review:

bug-1 (critical) — mount invisible outside podman unshare

The rootless overlay mount only exists inside podman unshare's user namespace, so every read of the rootfs previously saw an empty directory and slim build always failed with a misleading "missing /vmlinuz" error. Now the vmlinuz check (test -f), the vmlinuz copy (cp), and the find | cpio pipeline all run under podman unshare; cpio's stdout is piped across the namespace boundary and gzipped in the parent. This also eliminates the initrd.tmp round-trip and excludes ./vmlinuz from the archive.

bug-2 (major) — image stayed mounted on build failure

The unmount now runs on all code paths (result is captured, unmount happens unconditionally, then the result propagates). The unmount exit status is checked instead of discarded.

bug-3 (major) — slim run reported every qemu failure as success

timeout's exit 124 is treated as the expected smoke-test timeout; exit 126/127 produce a clear "is qemu installed?" error; any other non-zero status or signal bails.

Verification

  • slim build alpine:3.15 (no kernel): bails with clear error and prints Unmounted image.; namespace overlay-mount count returns to baseline
  • slim build mock-vm (image with fake /vmlinuz + /init): succeeds end-to-end (previously always failed); initrd is valid gzip, contains the full rootfs + init, excludes vmlinuz; extraction round-trip OK
  • slim run mock-vm with invalid kernel: Error: QEMU exited with code 1, slim exits 1
  • qemu hidden from PATH: Error: Failed to start qemu-system-x86_64 (is it installed and in PATH?)
  • cargo fmt --check, cargo clippy -- -D warnings, cargo build, cargo test all pass

Out of scope (separate findings, untouched): registry_dir refactor/path validation (sec-2/bug-7), CI fixes (sec-1/bug-4/bug-9), dead dependencies, CLI flag cleanup.

@hulthe please review — will not merge until approved.

Fixes the top three findings from the code review: ## bug-1 (critical) — mount invisible outside `podman unshare` The rootless overlay mount only exists inside `podman unshare`'s user namespace, so every read of the rootfs previously saw an empty directory and `slim build` always failed with a misleading "missing /vmlinuz" error. Now the vmlinuz check (`test -f`), the vmlinuz copy (`cp`), and the `find | cpio` pipeline all run under `podman unshare`; cpio's stdout is piped across the namespace boundary and gzipped in the parent. This also eliminates the `initrd.tmp` round-trip and excludes `./vmlinuz` from the archive. ## bug-2 (major) — image stayed mounted on build failure The unmount now runs on all code paths (result is captured, unmount happens unconditionally, then the result propagates). The unmount exit status is checked instead of discarded. ## bug-3 (major) — `slim run` reported every qemu failure as success `timeout`'s exit 124 is treated as the expected smoke-test timeout; exit 126/127 produce a clear "is qemu installed?" error; any other non-zero status or signal bails. ## Verification - `slim build alpine:3.15` (no kernel): bails with clear error **and** prints `Unmounted image.`; namespace overlay-mount count returns to baseline - `slim build mock-vm` (image with fake `/vmlinuz` + `/init`): succeeds end-to-end (previously always failed); initrd is valid gzip, contains the full rootfs + `init`, excludes `vmlinuz`; extraction round-trip OK - `slim run mock-vm` with invalid kernel: `Error: QEMU exited with code 1`, slim exits 1 - qemu hidden from PATH: `Error: Failed to start qemu-system-x86_64 (is it installed and in PATH?)` - `cargo fmt --check`, `cargo clippy -- -D warnings`, `cargo build`, `cargo test` all pass Out of scope (separate findings, untouched): registry_dir refactor/path validation (sec-2/bug-7), CI fixes (sec-1/bug-4/bug-9), dead dependencies, CLI flag cleanup. @hulthe please review — will not merge until approved.
marvin added 1 commit 2026-08-25 22:45:53 +02:00
fix: read rootfs inside podman unshare, always unmount, fail on qemu errors
CI / build (pull_request) Successful in 11s
Details
CI / build (push) Successful in 10s
Details
8dd5267bb4 
- build: the rootless overlay mount only exists inside podman unshare's
  user namespace; run the vmlinuz check/copy and the find|cpio pipeline
  there, streaming cpio's stdout to the parent for gzipping (also drops
  the initrd.tmp round-trip and excludes vmlinuz from the archive)
- build: unmount the image on all code paths, not just success
- run: treat timeout's exit 124 as the expected smoke-test timeout and
  error on any other non-zero status (a missing qemu binary no longer
  reports success)

Fixes bug-1, bug-2, bug-3 from the code review.
hulthe approved these changes 2026-08-25 22:47:10 +02:00
marvin merged commit 8dd5267bb4 into master 2026-08-25 22:47:37 +02:00
hulthe deleted branch fix/review-bug-1-2-3 2026-08-25 22:47:50 +02:00
Sign in to join this conversation.
Reviewers
No Reviewers
hulthe
Labels
No items
No labels
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
marvin (Marvin) hulthe (Joakim Hulthe)
No Assignees
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: hulthe/boco#1