Fix six pivot-path blockers + add real-kernel integration test#14
Merged
Conversation
A senior-engineer review of the pivot pipeline surfaced six issues that
could brick or expose a machine you can't physically reach. Fix all six.
1. Signal hardening (internal/cli/pivot.go): the transient-scope relocation
only fixes cgroup reaping, not the controlling TTY, so a dropped SSH
session delivered a fatal SIGHUP mid-pivot — exactly the documented
rescue flow. Ignore SIGHUP/SIGPIPE once committed to proceed, and
SIGINT/SIGTERM at the start of the destructive window (both modes).
2. Tar extraction containment (internal/oci/extract.go): extraction runs as
root into what becomes the live root, but the only guard was a string
check on the entry name. A layer could plant a symlink and write/delete
through it onto the host, or hardlink a host secret into the rootfs.
Route every path through a symlink-clamping secureJoin, create files with
O_EXCL after unlinking planted symlinks, and clamp hardlink sources.
Also restore setuid/setgid/sticky bits (were masked to 0o777, breaking
sudo/ping/mount). Adds escape + setuid tests.
3. Mount ordering (internal/pivot/{prepare,pivot}.go): the new root was
self-bound AFTER the essentials were mounted, and a non-recursive
self-bind stacks a mount that shadows child mounts — so the pivoted
system came up with no /proc, /sys, or /dev. Self-bind first, then mount
essentials; drop the second self-bind in PivotRoot. Adds a root-guarded
regression test that also reproduces the old shadowing behaviour.
4. Secret file permissions (internal/postpivot/{configwrite,run}.go): the
SSH password and Tailscale auth key were written world-readable (0644)
and never removed. Write 0600 and unlink after loading.
5. Nix module (nix/module.nix): --tailscale.authkey=$(cat file) was passed
literally (ExecStart is not a shell), and --ssh.* flags leaked into the
build subcommand (which errors "unknown flag"). Read the key at runtime
via a wrapper script; scope SSH flags to pivot only.
6. README install: pointed at a nonexistent raw-binary asset; use the real
xmorph-<arch>-linux.tar.gz archive.
Validated on a real Linux kernel (Apple container, kernel 6.18.15): full
suite passes as root with -race, the mount-ordering fix is confirmed
before/after, and the extraction escape tests pass as root.
…o CI The pivot path (pivot_root, mount ordering, root tar extraction) can only be exercised against a real kernel with CAP_SYS_ADMIN, which the nix build sandbox and `go test` on darwin can't provide — so it had no automated coverage. Add it: - internal/pivot/prepare_linux_test.go grows a full end-to-end pivot_root test (TestPivotRootEndToEnd) that runs the pivot in a child process so the swap can't disturb the test driver, asserting /proc, /sys, /dev survive. Joins the existing mount-visibility + buggy-order-reproduction tests. - flake.nix: xmorphIntegrationTests compiles the pivot/oci test binaries; the nixos-pivot NixOS VM test runs them as root on a live kernel. - .github/workflows/ci.yml: new `integration` job runs the VM test on every push/PR (enables /dev/kvm on the runner). - docs/testing.md documents the three test tiers and keeps Apple `container` as a fast real-kernel dev loop on macOS; linked from the README. Validated end to end under Apple `container` (kernel 6.18.15): the compiled test binaries pass as root, mirroring what the VM test runs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A senior-engineer review of the pivot pipeline surfaced six issues that could brick or expose a machine you can't physically reach. This fixes all six and adds the project's first automated real-kernel coverage of the pivot path.
Blockers fixed
internal/cli/pivot.go) — the transient-scope relocation only fixes cgroup reaping, not the controlling TTY, so a dropped SSH session delivered a fatal SIGHUP mid-pivot (the documented rescue flow). IgnoreSIGHUP/SIGPIPEonce committed to proceed, andSIGINT/SIGTERMat the start of the destructive window (covers--systemd-modetoo). Local Ctrl-C still aborts a build.internal/oci/extract.go) — extraction runs as root into what becomes the live root, guarded only by a string check on the entry name. A layer could plant a symlink and write/delete through it onto the host, or hardlink a host secret into the rootfs. Added a symlink-clampingsecureJoin,O_EXCLcreation after unlinking planted symlinks, and hardlink-source clamping. Also restored setuid/setgid/sticky bits (were masked to0o777, breakingsudo/ping/mount).internal/pivot/{prepare,pivot}.go) — the new root was self-bound after the essentials were mounted; a non-recursive self-bind stacks a mount that shadows child mounts, so the pivoted system came up with no/proc,/sys, or/dev. Self-bind first, then mount essentials; dropped the redundant second self-bind inPivotRoot.internal/postpivot/{configwrite,run}.go) — the SSH password and Tailscale auth key were written0644and never removed. Now0600, unlinked after load.nix/module.nix) —--tailscale.authkey=$(cat file)was passed literally (ExecStart is not a shell), and--ssh.*leaked into thebuildsubcommand (which errors "unknown flag"). Read the key at runtime via a wrapper; scope SSH flags topivot.xmorph-<arch>-linux.tar.gz.New tests + CI
internal/pivot/prepare_linux_test.go: mount-visibility test, a reproduction of the old shadowing bug, and a full end-to-endpivot_roottest (run in a child process so the swap can't disturb the test driver). Root-guarded — skips in unprivileged CI.flake.nix:xmorphIntegrationTestscompiles the pivot/oci test binaries; newnixos-pivotNixOS VM test runs them as root on a live kernel..github/workflows/ci.yml: newintegrationjob runs the VM test on every push/PR (enables/dev/kvm).docs/testing.md: documents the three test tiers; keeps Applecontaineras a fast real-kernel dev loop on macOS.Validation
Full suite passes with
-raceas root on a real Linux kernel (6.18.15, via Applecontainer). The mount-ordering fix is confirmed before/after: the reproduction test shows the old order shadows/proc, and the fixedPrepare/pivot_rootkeep it visible. The tar-extraction escape tests pass as root (where an escape would actually bite).