From 82f881a9d6ec446bd1b667fe0711dcc6f577eb49 Mon Sep 17 00:00:00 2001 From: Ananth Bhaskararaman Date: Tue, 18 Aug 2026 00:58:03 +0530 Subject: [PATCH 1/3] fix(pivot): serve SSH without an entrypoint, reboot when one exits A remote pivot with --ssh/--tailscale and no --command left the box with a running kernel and no userspace: reachable by ICMP, dead on every port, recoverable only by physically power-cycling it. Observed on an RPi 3A+ that pivoted, registered on the tailnet, and was never reachable again. Three independent defects, each sufficient on its own. The entrypoint defaults to /bin/sh, and a shell driven from a detached pivot has no terminal: stdin is /dev/null, it reads EOF, and it exits 0 before anyone can connect. Supervise then returns and takes the SSH server, started as a goroutine, down with it. Telling users to pass `sleep infinity` works but makes the headline use case depend on a shell idiom and on the image shipping coreutils. Instead, when services are enabled and the operator named neither --entrypoint nor --command, supervise nothing: ServeUntilSignal blocks until TERM/INT, reaping orphans. It needs nothing from the image at all. The image's own default is deliberately ignored here, since minimal images default to a shell and a shell is exactly what must not be supervised in this mode. RebootOnFailure only fired on a non-zero status, so the clean EOF exit above was treated as success and simply returned. But post-pivot there is no init to fall back to -- this supervisor is the only thing holding userspace up, so any exit is fatal and a reboot into the on-disk OS beats being bricked-alive. Renamed to RebootOnExit and fired unconditionally. This alone would have made the incident self-recovering. Minimal images (alpine, busybox, distroless) ship no /etc/resolv.conf, and the host's cannot simply be copied: on systemd-resolved distros it names 127.0.0.53, a stub whose daemon is terminated during pivot preparation, so the result looks configured and resolves nothing. EnsureResolvConf keeps a usable file, else inherits the host's routable nameservers, else falls back to public resolvers -- and replaces a dangling stub symlink with a real file. Tailscale needs DNS to reach the coordination server, so this strands the box precisely when remote access is the only access left. The dry run now states which of the two lifecycles will run, rather than printing "Execute /bin/sh" for a shell that was never going to survive. --- internal/cli/dryrun.go | 6 +- internal/cli/pivot.go | 25 ++++- internal/postpivot/configwrite.go | 11 +- internal/postpivot/configwrite_test.go | 5 +- internal/postpivot/resolvconf.go | 115 ++++++++++++++++++++ internal/postpivot/resolvconf_test.go | 145 +++++++++++++++++++++++++ internal/postpivot/run.go | 16 ++- internal/postpivot/supervise.go | 62 ++++++++++- 8 files changed, 369 insertions(+), 16 deletions(-) create mode 100644 internal/postpivot/resolvconf.go create mode 100644 internal/postpivot/resolvconf_test.go diff --git a/internal/cli/dryrun.go b/internal/cli/dryrun.go index 1a71544..aec7ca6 100644 --- a/internal/cli/dryrun.go +++ b/internal/cli/dryrun.go @@ -103,7 +103,11 @@ func printDryRun(w io.Writer, cfg *config.Config) { step++ fmt.Fprintf(w, " %d. Execute pivot_root\n", step) step++ - fmt.Fprintf(w, " %d. Execute %s\n", step, cfg.Entrypoint) + if serveOnly(cfg) { + fmt.Fprintf(w, " %d. Stay up and serve SSH (no entrypoint; reboots on exit)\n", step) + } else { + fmt.Fprintf(w, " %d. Execute %s (reboots into the on-disk OS when it exits)\n", step, cfg.Entrypoint) + } fmt.Fprintf(w, "\n=== END DRY RUN ===\n") } diff --git a/internal/cli/pivot.go b/internal/cli/pivot.go index 94df135..fe5f74f 100644 --- a/internal/cli/pivot.go +++ b/internal/cli/pivot.go @@ -172,6 +172,11 @@ func runPivot(ctx context.Context, cfg *config.Config, stdout interface { if err := postpivot.CopyBinary(cfg.WorkDir); err != nil { return fmt.Errorf("copy binary: %w", err) } + // Do this while the old root is still mounted — /etc/resolv.conf has to be + // read from it, and after the pivot the resolver it names is gone anyway. + if err := postpivot.EnsureResolvConf(cfg.WorkDir); err != nil { + return fmt.Errorf("prepare resolv.conf: %w", err) + } slog.Info("staged post-pivot config and binary", "work_dir", cfg.WorkDir) // Pre-pivot tailscale auth: validate the authkey against the live @@ -339,7 +344,8 @@ func runPivot(ctx context.Context, cfg *config.Config, stdout interface { func buildPostpivotConfig(cfg *config.Config, entrypoint string, entryArgs []string) *postpivot.Config { pc := &postpivot.Config{ FlushFirewall: !cfg.KeepFirewall, - RebootOnFailure: true, + RebootOnExit: true, + Serve: serveOnly(cfg), WatchdogTimeoutSeconds: int(cfg.WatchdogTimeout / time.Second), KeepOldRoot: cfg.KeepOldRoot, Entrypoint: append([]string{entrypoint}, entryArgs...), @@ -415,6 +421,23 @@ func ensureLogDirWritable(dir string) error { return os.Remove(name) } +// serveOnly reports whether this pivot exists to expose SSH/Tailscale rather +// than to run a program: services are enabled and the operator named neither +// an entrypoint nor a command. +// +// The image's own default is deliberately ignored here. Minimal images +// default to a shell (alpine's Cmd is ["/bin/sh"]), and a shell is precisely +// what must not be supervised in this mode — driven over SSH there is no +// terminal, so it reads EOF on stdin and exits before anyone connects, +// taking the SSH server down with it. An operator who genuinely wants a +// program run says so with --command or --entrypoint, and that still works. +func serveOnly(cfg *config.Config) bool { + if cfg.EntrypointExplicit || len(cfg.Command) > 0 { + return false + } + return cfg.SSHEnabled() || cfg.TailscaleEnabled() +} + // resolveEntrypoint picks the effective entrypoint + args + env from the // CLI config and the merged ImageConfig. Mirrors src/cmd/pivot.zig:194-236. func resolveEntrypoint(cfg *config.Config, ic *oci.ImageConfig) (entrypoint string, args, env []string) { diff --git a/internal/postpivot/configwrite.go b/internal/postpivot/configwrite.go index 00c4943..045d358 100644 --- a/internal/postpivot/configwrite.go +++ b/internal/postpivot/configwrite.go @@ -23,8 +23,15 @@ const BinaryPath = "/usr/local/bin/xmorph" // Config is the JSON schema written to ConfigPath. Mirrors the schema // at the top of src/xenomorph-init.zig. type Config struct { - FlushFirewall bool `json:"flush_firewall"` - RebootOnFailure bool `json:"reboot_on_failure"` + FlushFirewall bool `json:"flush_firewall"` + // RebootOnExit reboots into the on-disk OS when the entrypoint exits, + // whatever its status. See SuperviseOptions.RebootOnExit for why a + // clean exit is treated as fatal too. + RebootOnExit bool `json:"reboot_on_exit"` + // Serve means "there is no entrypoint; stay up and serve SSH until + // signalled". Set when the operator enabled SSH or Tailscale but named + // no command to run, which is the normal shape of a remote rescue pivot. + Serve bool `json:"serve,omitempty"` // WatchdogTimeoutSeconds; 0 disables. WatchdogTimeoutSeconds int `json:"watchdog_timeout_seconds,omitempty"` // KeepOldRoot is the pre-pivot root's mount point (default diff --git a/internal/postpivot/configwrite_test.go b/internal/postpivot/configwrite_test.go index cce0bc5..20e997c 100644 --- a/internal/postpivot/configwrite_test.go +++ b/internal/postpivot/configwrite_test.go @@ -11,7 +11,8 @@ func TestWriteConfigRoundTrip(t *testing.T) { dir := t.TempDir() cfg := &Config{ FlushFirewall: true, - RebootOnFailure: true, + RebootOnExit: true, + Serve: true, WatchdogTimeoutSeconds: 300, KeepOldRoot: "/mnt/oldroot", LogPersistDir: "/mnt/oldroot/var/log/xmorph", @@ -39,7 +40,7 @@ func TestWriteConfigRoundTrip(t *testing.T) { if err := json.Unmarshal(data, &got); err != nil { t.Fatalf("unmarshal: %v", err) } - if !got.FlushFirewall || !got.RebootOnFailure { + if !got.FlushFirewall || !got.RebootOnExit || !got.Serve { t.Error("boolean fields lost") } if got.WatchdogTimeoutSeconds != 300 { diff --git a/internal/postpivot/resolvconf.go b/internal/postpivot/resolvconf.go new file mode 100644 index 0000000..dde4a11 --- /dev/null +++ b/internal/postpivot/resolvconf.go @@ -0,0 +1,115 @@ +package postpivot + +import ( + "bufio" + "fmt" + "log/slog" + "net" + "os" + "path/filepath" + "strings" +) + +// FallbackNameservers are used when the host has no usable resolv.conf. +// Two providers rather than two addresses from one, so a single provider +// outage doesn't leave the pivoted system unable to resolve anything. +var FallbackNameservers = []string{"1.1.1.1", "8.8.8.8", "2606:4700:4700::1111"} + +// EnsureResolvConf gives the new rootfs a working /etc/resolv.conf before +// the pivot. +// +// Two things make this necessary. Minimal OCI images (alpine, busybox, +// distroless) ship no /etc/resolv.conf at all — DNS in a container comes +// from the runtime, and there is no runtime here. And the host's own file +// usually cannot be copied verbatim: on systemd-resolved distros it reads +// "nameserver 127.0.0.53", a stub served by a daemon that gets terminated +// during pivot preparation. Copying it produces a rootfs that looks +// configured and resolves nothing. +// +// So: keep a usable file if the image has one, otherwise inherit the host's +// real nameservers, otherwise fall back to public resolvers. Tailscale needs +// working DNS to reach the coordination server, so getting this wrong +// strands the box exactly when remote access is the only access left. +func EnsureResolvConf(rootfsRoot string) error { + dst := filepath.Join(rootfsRoot, "etc", "resolv.conf") + + if servers := readNameservers(dst); len(servers) > 0 && !allLoopback(servers) { + slog.Debug("rootfs already has a usable resolv.conf", "servers", servers) + return nil + } + + source := "host" + servers := readNameservers("/etc/resolv.conf") + if usable := filterRoutable(servers); len(usable) > 0 { + servers = usable + } else { + // Either the host had nothing, or everything it listed was a local + // stub that dies with the old root. + slog.Warn("host resolv.conf unusable post-pivot; using fallback resolvers", + "host_servers", servers, "fallback", FallbackNameservers) + servers = FallbackNameservers + source = "fallback" + } + + if err := os.MkdirAll(filepath.Dir(dst), 0o755); err != nil { + return fmt.Errorf("mkdir for resolv.conf: %w", err) + } + var b strings.Builder + b.WriteString("# written by xmorph: the pre-pivot resolver does not survive pivot_root\n") + for _, s := range servers { + fmt.Fprintf(&b, "nameserver %s\n", s) + } + // A pre-existing symlink (e.g. ../run/systemd/resolve/stub-resolv.conf) + // would otherwise be followed and write into a directory that will not + // exist after the pivot. + _ = os.Remove(dst) + if err := os.WriteFile(dst, []byte(b.String()), 0o644); err != nil { + return fmt.Errorf("write resolv.conf: %w", err) + } + slog.Info("wrote resolv.conf into new rootfs", "source", source, "servers", servers) + return nil +} + +// readNameservers parses the nameserver lines out of a resolv.conf. +// A missing or unreadable file yields nil rather than an error: every +// caller treats "no servers" and "could not read" the same way. +func readNameservers(path string) []string { + f, err := os.Open(path) + if err != nil { + return nil + } + defer f.Close() + + var out []string + sc := bufio.NewScanner(f) + for sc.Scan() { + line := strings.TrimSpace(sc.Text()) + if line == "" || strings.HasPrefix(line, "#") || strings.HasPrefix(line, ";") { + continue + } + fields := strings.Fields(line) + if len(fields) >= 2 && fields[0] == "nameserver" { + if ip := net.ParseIP(fields[1]); ip != nil { + out = append(out, ip.String()) + } + } + } + return out +} + +// filterRoutable drops loopback addresses. They are the signature of a local +// caching resolver — systemd-resolved on 127.0.0.53, dnsmasq on 127.0.0.1 — +// which is torn down with the old root and cannot answer afterwards. +func filterRoutable(servers []string) []string { + var out []string + for _, s := range servers { + if ip := net.ParseIP(s); ip != nil && !ip.IsLoopback() { + out = append(out, s) + } + } + return out +} + +func allLoopback(servers []string) bool { + return len(filterRoutable(servers)) == 0 +} diff --git a/internal/postpivot/resolvconf_test.go b/internal/postpivot/resolvconf_test.go new file mode 100644 index 0000000..13f168c --- /dev/null +++ b/internal/postpivot/resolvconf_test.go @@ -0,0 +1,145 @@ +package postpivot + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +func TestReadNameservers(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "resolv.conf") + body := `# comment +; also a comment + +nameserver 192.168.1.1 +nameserver 2001:4860:4860::8888 +search lan +nameserver not-an-ip +options edns0 +` + if err := os.WriteFile(path, []byte(body), 0o644); err != nil { + t.Fatal(err) + } + got := readNameservers(path) + want := []string{"192.168.1.1", "2001:4860:4860::8888"} + if len(got) != len(want) { + t.Fatalf("got %v, want %v", got, want) + } + for i := range want { + if got[i] != want[i] { + t.Errorf("index %d: got %q, want %q", i, got[i], want[i]) + } + } +} + +func TestReadNameserversMissingFile(t *testing.T) { + if got := readNameservers(filepath.Join(t.TempDir(), "nope")); got != nil { + t.Errorf("missing file should yield nil, got %v", got) + } +} + +func TestFilterRoutableDropsLocalStubs(t *testing.T) { + // 127.0.0.53 is systemd-resolved's stub and 127.0.0.1 a local dnsmasq. + // Both are served by daemons that do not survive the pivot. + got := filterRoutable([]string{"127.0.0.53", "127.0.0.1", "::1", "10.0.0.1"}) + if len(got) != 1 || got[0] != "10.0.0.1" { + t.Errorf("got %v, want [10.0.0.1]", got) + } + if !allLoopback([]string{"127.0.0.53", "::1"}) { + t.Error("a stub-only resolv.conf must count as unusable") + } +} + +// A rootfs whose resolv.conf points only at a local stub must be rewritten: +// the file looks configured but resolves nothing once the old root is gone. +func TestEnsureResolvConfReplacesStubOnly(t *testing.T) { + root := t.TempDir() + if err := os.MkdirAll(filepath.Join(root, "etc"), 0o755); err != nil { + t.Fatal(err) + } + dst := filepath.Join(root, "etc", "resolv.conf") + if err := os.WriteFile(dst, []byte("nameserver 127.0.0.53\n"), 0o644); err != nil { + t.Fatal(err) + } + if err := EnsureResolvConf(root); err != nil { + t.Fatalf("EnsureResolvConf: %v", err) + } + if servers := readNameservers(dst); allLoopback(servers) { + t.Errorf("stub survived: %v", servers) + } +} + +// A missing resolv.conf (alpine, busybox, distroless) must be created. +func TestEnsureResolvConfCreatesWhenAbsent(t *testing.T) { + root := t.TempDir() + if err := EnsureResolvConf(root); err != nil { + t.Fatalf("EnsureResolvConf: %v", err) + } + dst := filepath.Join(root, "etc", "resolv.conf") + servers := readNameservers(dst) + if len(servers) == 0 { + t.Fatal("no nameservers written") + } + if allLoopback(servers) { + t.Errorf("wrote unusable servers: %v", servers) + } + data, err := os.ReadFile(dst) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(data), "xmorph") { + t.Error("generated file should say what wrote it") + } +} + +// A usable file from the image is authoritative and must be left alone. +func TestEnsureResolvConfKeepsUsable(t *testing.T) { + root := t.TempDir() + if err := os.MkdirAll(filepath.Join(root, "etc"), 0o755); err != nil { + t.Fatal(err) + } + dst := filepath.Join(root, "etc", "resolv.conf") + want := "nameserver 10.9.8.7\n" + if err := os.WriteFile(dst, []byte(want), 0o644); err != nil { + t.Fatal(err) + } + if err := EnsureResolvConf(root); err != nil { + t.Fatalf("EnsureResolvConf: %v", err) + } + got, err := os.ReadFile(dst) + if err != nil { + t.Fatal(err) + } + if string(got) != want { + t.Errorf("rewrote a usable file: got %q, want %q", got, want) + } +} + +// A dangling symlink (../run/systemd/resolve/stub-resolv.conf is the common +// one) must be replaced by a real file rather than followed into a directory +// that will not exist after the pivot. +func TestEnsureResolvConfReplacesDanglingSymlink(t *testing.T) { + root := t.TempDir() + if err := os.MkdirAll(filepath.Join(root, "etc"), 0o755); err != nil { + t.Fatal(err) + } + dst := filepath.Join(root, "etc", "resolv.conf") + if err := os.Symlink("../run/systemd/resolve/stub-resolv.conf", dst); err != nil { + t.Fatal(err) + } + if err := EnsureResolvConf(root); err != nil { + t.Fatalf("EnsureResolvConf: %v", err) + } + fi, err := os.Lstat(dst) + if err != nil { + t.Fatal(err) + } + if fi.Mode()&os.ModeSymlink != 0 { + t.Error("still a symlink") + } + if servers := readNameservers(dst); len(servers) == 0 { + t.Error("no usable nameservers after replacing symlink") + } +} diff --git a/internal/postpivot/run.go b/internal/postpivot/run.go index aff0d79..aadf036 100644 --- a/internal/postpivot/run.go +++ b/internal/postpivot/run.go @@ -103,6 +103,12 @@ func Run(argv []string) int { }() } + // No entrypoint to run: hold the box up and serve SSH. Returning here + // would leave a kernel with no userspace, so this blocks until signalled. + if cfg != nil && cfg.Serve { + return ServeUntilSignal() + } + // Decide what to exec. Config-supplied entrypoint+command beats argv. var supervised []string if cfg != nil && len(cfg.Entrypoint) > 0 { @@ -116,16 +122,16 @@ func Run(argv []string) int { return 1 } - rebootOnFailure := cfg == nil || cfg.RebootOnFailure + rebootOnExit := cfg == nil || cfg.RebootOnExit var oldRoot string if cfg != nil { oldRoot = cfg.KeepOldRoot } code, err := Supervise(SuperviseOptions{ - Argv: supervised, - RebootOnFailure: rebootOnFailure, - OldRootPath: oldRoot, - LogWriter: entrypointLog, + Argv: supervised, + RebootOnExit: rebootOnExit, + OldRootPath: oldRoot, + LogWriter: entrypointLog, }) if err != nil { fmt.Fprintf(os.Stderr, "xmorph --init: %v\n", err) diff --git a/internal/postpivot/supervise.go b/internal/postpivot/supervise.go index cae012d..660879d 100644 --- a/internal/postpivot/supervise.go +++ b/internal/postpivot/supervise.go @@ -4,6 +4,7 @@ import ( "errors" "fmt" "io" + "log/slog" "os" "os/exec" "os/signal" @@ -32,10 +33,22 @@ type SuperviseOptions struct { Argv []string // Env is the environment passed to the entrypoint. Nil = inherit. Env []string - // RebootOnFailure: if true and the entrypoint exits non-zero (or by - // signal), sync the filesystem and trigger LINUX_REBOOT_CMD_RESTART - // so the original OS comes back. Mirrors src/xenomorph-init.zig:336-352. - RebootOnFailure bool + // RebootOnExit: if true, sync the filesystem and trigger + // LINUX_REBOOT_CMD_RESTART when the entrypoint exits — for ANY exit, + // including a clean status 0. + // + // Post-pivot there is no init left to fall back to: the old root's + // systemd was torn down before pivot_root, and this supervisor is the + // only thing keeping userspace alive. When it returns, the box is a + // running kernel with nothing on it — it still answers ICMP (the kernel + // does that), so it looks alive from outside while being unreachable and + // unrecoverable without physical access. + // + // A clean exit is the *likely* case, not the exotic one: the default + // entrypoint is a shell, and a shell whose stdin is /dev/null reads EOF + // and exits 0 immediately. Rebooting instead returns the machine to the + // OS on disk, which is always a better end state than bricked-alive. + RebootOnExit bool // OldRootPath is unmounted before reboot; empty skips. OldRootPath string // LogWriter, if non-nil, tees the child's stdout + stderr. @@ -91,7 +104,9 @@ func Supervise(opts SuperviseOptions) (exitCode int, err error) { signal.Stop(sigCh) reapOrphans() code := exitStatusFrom(cmd, err) - if opts.RebootOnFailure && code != 0 { + if opts.RebootOnExit { + slog.Warn("entrypoint exited; no userspace left, rebooting into the on-disk OS", + "code", code) rebootSystem(opts.OldRootPath) } return code, nil @@ -99,6 +114,43 @@ func Supervise(opts SuperviseOptions) (exitCode int, err error) { } } +// ServeUntilSignal blocks until TERM/INT is received, reaping orphans as +// they appear. It is the entrypoint for a pivot whose purpose is to expose +// SSH and Tailscale rather than to run a program. +// +// This exists because the obvious alternative — supervising `/bin/sh` — is +// wrong for a remotely-driven pivot. There is no terminal on the other end, +// so the shell's stdin is /dev/null (or a closed pipe), it reads EOF, and it +// exits before anyone can connect. Telling users to pass `sleep infinity` +// works but makes the tool's headline use case depend on a shell idiom and on +// coreutils being present in the image. Blocking here needs neither: no +// /bin/sh, no sleep, nothing from the image at all. +// +// SIGCHLD is deliberately not a wake-up condition. As the supervisor we +// inherit every orphan on the box, so children will come and go; that is +// not a reason to tear down the SSH server the operator is relying on. +func ServeUntilSignal() int { + sigCh := make(chan os.Signal, 8) + signal.Notify(sigCh, syscall.SIGTERM, syscall.SIGINT) + defer signal.Stop(sigCh) + + chldCh := make(chan os.Signal, 8) + signal.Notify(chldCh, syscall.SIGCHLD) + defer signal.Stop(chldCh) + + slog.Info("serving; no entrypoint to supervise (send SIGTERM to stop)") + for { + select { + case sig := <-sigCh: + slog.Info("received signal, stopping", "signal", sig) + reapOrphans() + return 0 + case <-chldCh: + reapOrphans() + } + } +} + // reapOrphans waits for any remaining children (non-blocking) so the // kernel doesn't accumulate zombies under us. func reapOrphans() { From 1aca6b2c8ed29e30dffeb430d9bdddb12c6726c5 Mon Sep 17 00:00:00 2001 From: Ananth Bhaskararaman Date: Tue, 18 Aug 2026 01:10:49 +0530 Subject: [PATCH 2/3] fix(pivot): make serve mode explicit, honour the image's command Replaces the implicit serve-mode switch from the previous commit. That version entered serve mode whenever services were enabled and no command was named, reasoning that the image's default shell could not have been intended. That inference is not sound. An image whose Cmd is a real long-running daemon -- exactly what a purpose-built rescue image looks like -- is indistinguishable here from alpine's ["/bin/sh"], so the guess silently skips the very program the operator built the image around. Ignoring a declared Cmd is also just surprising: it makes behaviour depend on an unrelated flag. With RebootOnExit already in place, honouring the image is safe: a shell that exits reboots the box back into the on-disk OS. Predictable beats clever. So serve mode is now the explicit --serve, and nothing is inferred. Safe is not the same as useful, though -- a bare `--image alpine --ssh.enable` would pivot, exit, and reboot in a loop, with the cause buried in a log on a filesystem that just went away. checkEntrypointSurvivesDetach catches that before anything destructive happens, while the old root is intact and aborting is free, and names the three ways forward. Only the no-TTY case is rejected: with a console attached a shell is a legitimate entrypoint, so stdin decides. --serve alongside --entrypoint/--command is rejected outright rather than resolved in either direction, since either choice discards what was asked for. --- internal/cli/pivot.go | 67 ++++++++++++++++++++++++++++------- internal/config/config.go | 3 ++ internal/config/flags.go | 2 ++ internal/config/serve_test.go | 35 ++++++++++++++++++ internal/config/validate.go | 10 ++++++ 5 files changed, 104 insertions(+), 13 deletions(-) create mode 100644 internal/config/serve_test.go diff --git a/internal/cli/pivot.go b/internal/cli/pivot.go index fe5f74f..bcb695d 100644 --- a/internal/cli/pivot.go +++ b/internal/cli/pivot.go @@ -23,6 +23,7 @@ import ( "github.com/ananthb/xmorph/internal/tsnetauth" "github.com/spf13/cobra" "golang.org/x/sys/unix" + "golang.org/x/term" ) // errNotImplemented is retained for the not-yet-wired tsnet path (M6). @@ -163,6 +164,10 @@ func runPivot(ctx context.Context, cfg *config.Config, stdout interface { entrypoint, entryArgs, _ := resolveEntrypoint(cfg, result.Config) slog.Info("entrypoint resolved", "entrypoint", entrypoint, "args", len(entryArgs)) + if err := checkEntrypointSurvivesDetach(cfg, entrypoint); err != nil { + return err + } + // Write the postpivot config (read back by `xmorph --init`) and copy // the running binary into the new rootfs. pivotConfig := buildPostpivotConfig(cfg, entrypoint, entryArgs) @@ -421,21 +426,57 @@ func ensureLogDirWritable(dir string) error { return os.Remove(name) } -// serveOnly reports whether this pivot exists to expose SSH/Tailscale rather -// than to run a program: services are enabled and the operator named neither -// an entrypoint nor a command. +// shellEntrypoints are the interactive shells an image is likely to name as +// its default. Run detached they exit immediately; run from a console they +// are perfectly reasonable. +var shellEntrypoints = map[string]bool{ + "sh": true, "bash": true, "ash": true, "dash": true, "zsh": true, "busybox": true, +} + +// checkEntrypointSurvivesDetach refuses, before anything destructive happens, +// to pivot into a bare shell that cannot survive being detached. // -// The image's own default is deliberately ignored here. Minimal images -// default to a shell (alpine's Cmd is ["/bin/sh"]), and a shell is precisely -// what must not be supervised in this mode — driven over SSH there is no -// terminal, so it reads EOF on stdin and exits before anyone connects, -// taking the SSH server down with it. An operator who genuinely wants a -// program run says so with --command or --entrypoint, and that still works. -func serveOnly(cfg *config.Config) bool { - if cfg.EntrypointExplicit || len(cfg.Command) > 0 { - return false +// A shell with no controlling terminal reads EOF on stdin and exits at once. +// RebootOnExit makes that safe — the box returns to the OS on disk — but safe +// is not useful: the operator wanted a machine they could reach, and instead +// gets a pivot-reboot loop with the cause buried in a log on a filesystem +// that just went away. Both the diagnosis and the fix are known here, while +// the old root is still intact and aborting still costs nothing. +// +// Only the no-TTY case is rejected. With a console attached (--contain, or a +// serial line) a shell is exactly what someone may want, so stdin decides. +func checkEntrypointSurvivesDetach(cfg *config.Config, entrypoint string) error { + if cfg.Serve || cfg.Contain { + return nil } - return cfg.SSHEnabled() || cfg.TailscaleEnabled() + if !shellEntrypoints[filepath.Base(entrypoint)] { + return nil + } + if term.IsTerminal(int(os.Stdin.Fd())) { + return nil + } + return fmt.Errorf( + "entrypoint %q is a shell with no terminal attached: it will read EOF on stdin "+ + "and exit immediately after the pivot, rebooting the box back into the on-disk OS.\n"+ + " --serve stay up serving SSH (what a remote rescue pivot wants)\n"+ + " --command ... run a specific program instead\n"+ + " --entrypoint ... override the image's default explicitly", + entrypoint) +} + +// serveOnly reports whether to hold the box up for SSH instead of running a +// program. This is the operator's explicit --serve and nothing else. +// +// It deliberately does not infer intent from the image. An earlier version +// switched to serve mode whenever services were enabled and no command was +// named, on the theory that the image's default shell could not have been +// meant. That is wrong: an image whose Cmd is a real long-running daemon — +// precisely what a purpose-built rescue image looks like — is +// indistinguishable from alpine's ["/bin/sh"] at this point, so the guess +// silently skips the very program the operator built the image around. +// Honouring the image and rebooting on exit is predictable; guessing is not. +func serveOnly(cfg *config.Config) bool { + return cfg.Serve } // resolveEntrypoint picks the effective entrypoint + args + env from the diff --git a/internal/config/config.go b/internal/config/config.go index e169837..085cbd4 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -70,6 +70,9 @@ type Config struct { Entrypoint string EntrypointExplicit bool Command []string + // Serve holds the box up serving SSH instead of running an entrypoint. + // Mutually exclusive with --entrypoint/--command. + Serve bool KeepOldRoot string diff --git a/internal/config/flags.go b/internal/config/flags.go index 8ff11f8..51f0fe0 100644 --- a/internal/config/flags.go +++ b/internal/config/flags.go @@ -51,6 +51,8 @@ func bindPivotOnly(fs *pflag.FlagSet, cfg *Config) { fs.Var(&appendStringVar{dst: &cfg.Command}, "command", "command/args passed to entrypoint (repeatable)") fs.Var(&appendStringVar{dst: &cfg.Command}, "cmd", "alias for --command") + fs.BoolVar(&cfg.Serve, "serve", false, "run no entrypoint; stay up serving SSH until stopped") + // --keep-old-root with optional value: bare form uses the default, // --keep-old-root=/foo overrides. --no-keep-old-root clears it. fs.StringVar(&cfg.KeepOldRoot, "keep-old-root", DefaultKeepOldRoot, "keep old root mounted at PATH after pivot (default /mnt/oldroot)") diff --git a/internal/config/serve_test.go b/internal/config/serve_test.go new file mode 100644 index 0000000..5e0456b --- /dev/null +++ b/internal/config/serve_test.go @@ -0,0 +1,35 @@ +package config + +import ( + "errors" + "io" + "testing" +) + +// --serve runs nothing, so naming something to run is contradictory and must +// be rejected rather than silently resolved in either direction. +func TestValidateServeWithCommand(t *testing.T) { + for _, tc := range []struct { + name string + mutate func(*Config) + wantErr bool + }{ + {"serve alone", func(c *Config) { c.Serve = true }, false}, + {"serve with command", func(c *Config) { c.Serve = true; c.Command = []string{"sleep", "1"} }, true}, + {"serve with entrypoint", func(c *Config) { c.Serve = true; c.EntrypointExplicit = true }, true}, + {"command without serve", func(c *Config) { c.Command = []string{"sleep", "1"} }, false}, + {"neither", func(*Config) {}, false}, + } { + t.Run(tc.name, func(t *testing.T) { + cfg := New() + tc.mutate(&cfg) + err := cfg.Validate(io.Discard) + if tc.wantErr && !errors.Is(err, ErrServeWithCommand) { + t.Errorf("want ErrServeWithCommand, got %v", err) + } + if !tc.wantErr && errors.Is(err, ErrServeWithCommand) { + t.Errorf("unexpected ErrServeWithCommand: %v", err) + } + }) + } +} diff --git a/internal/config/validate.go b/internal/config/validate.go index b918524..82cd535 100644 --- a/internal/config/validate.go +++ b/internal/config/validate.go @@ -13,6 +13,9 @@ var ErrInvalidTimeout = errors.New("timeout must be greater than zero") // ErrWatchdogTooShort is returned when --watchdog-timeout is set below one second. var ErrWatchdogTooShort = errors.New("watchdog-timeout must be at least 1s") +// ErrServeWithCommand rejects --serve alongside --entrypoint/--command. +var ErrServeWithCommand = errors.New("--serve runs no entrypoint; drop --entrypoint/--command, or drop --serve to run them") + // Validate runs the same set of post-parse checks as src/config.zig:598-626, // minus the containerfile mutual-exclusion rule (containerfile support was // dropped). Warnings go to warnW (typically os.Stderr). @@ -25,6 +28,13 @@ func (c *Config) Validate(warnW io.Writer) error { return ErrWatchdogTooShort } + // --serve runs nothing, so naming something to run contradicts it. Reject + // rather than pick a winner: either choice silently discards what the + // operator asked for, and this is the last cheap moment to say so. + if c.Serve && (c.EntrypointExplicit || len(c.Command) > 0) { + return ErrServeWithCommand + } + if c.TailscaleAuthkey == "" { if c.TailscaleArgs != "" { fmt.Fprintln(warnW, "Warning: --tailscale.args without --tailscale.authkey won't start tailscale") From 6e813ecca0920d615564b247fb7ed58101b8148a Mon Sep 17 00:00:00 2001 From: Ananth Bhaskararaman Date: Tue, 18 Aug 2026 01:18:47 +0530 Subject: [PATCH 3/3] fix(pivot): replace --serve with an `xmorph idle` entrypoint --serve never said serve what. It was also a second lifecycle bolted onto a tool that already has a perfectly good way to say "run this": the entrypoint. xmorph copies its own binary into every pivoted rootfs at /usr/local/bin/xmorph before pivot_root, so "stay alive" can be an ordinary program rather than a mode. `xmorph idle` blocks until signalled, reaping orphans, and is available no matter how minimal the image -- no shell, no coreutils: xmorph pivot --entrypoint /usr/local/bin/xmorph --cmd idle --ssh.enable This removes the Serve config field, the serve-vs-supervise branch in Run, the serveOnly helper, the dry-run fork, and the --serve/--command mutual-exclusion rule, which now cannot arise: naming two entrypoints is already an error. One lifecycle remains -- supervise an entrypoint, reboot when it exits -- and idle is just an entrypoint that does not exit. The pre-flight shell check stays, since a bare shell still cannot survive being detached, and now points at the idle entrypoint instead of a flag. Run recognises [BinaryPath, "idle"] and blocks in the supervisor rather than forking a second copy of a 41 MB Go binary into a tmpfs-backed rootfs on a 415 MB machine. Optimisation only; the semantics are the entrypoint's. --- internal/cli/dryrun.go | 6 +--- internal/cli/idle.go | 38 ++++++++++++++++++++++++++ internal/cli/pivot.go | 25 +++-------------- internal/cli/root.go | 2 +- internal/config/config.go | 3 -- internal/config/flags.go | 2 -- internal/config/serve_test.go | 35 ------------------------ internal/config/validate.go | 10 ------- internal/postpivot/configwrite.go | 4 --- internal/postpivot/configwrite_test.go | 3 +- internal/postpivot/run.go | 17 ++++++++---- 11 files changed, 56 insertions(+), 89 deletions(-) create mode 100644 internal/cli/idle.go delete mode 100644 internal/config/serve_test.go diff --git a/internal/cli/dryrun.go b/internal/cli/dryrun.go index aec7ca6..214634b 100644 --- a/internal/cli/dryrun.go +++ b/internal/cli/dryrun.go @@ -103,11 +103,7 @@ func printDryRun(w io.Writer, cfg *config.Config) { step++ fmt.Fprintf(w, " %d. Execute pivot_root\n", step) step++ - if serveOnly(cfg) { - fmt.Fprintf(w, " %d. Stay up and serve SSH (no entrypoint; reboots on exit)\n", step) - } else { - fmt.Fprintf(w, " %d. Execute %s (reboots into the on-disk OS when it exits)\n", step, cfg.Entrypoint) - } + fmt.Fprintf(w, " %d. Execute %s (reboots into the on-disk OS when it exits)\n", step, cfg.Entrypoint) fmt.Fprintf(w, "\n=== END DRY RUN ===\n") } diff --git a/internal/cli/idle.go b/internal/cli/idle.go new file mode 100644 index 0000000..6867777 --- /dev/null +++ b/internal/cli/idle.go @@ -0,0 +1,38 @@ +package cli + +import ( + "github.com/ananthb/xmorph/internal/postpivot" + "github.com/spf13/cobra" +) + +// newIdleCmd exposes "do nothing, stay alive" as a real program rather than a +// pivot mode. +// +// A remote rescue pivot has no program to run: the point is to reach the box +// over SSH while its disk is free. Something still has to occupy the +// entrypoint, because when the entrypoint exits there is no init left and +// xmorph reboots into the on-disk OS. +// +// `sleep infinity` fills that role on a normal system but needs coreutils, and +// a bare shell exits immediately once detached. xmorph copies its own binary +// into every pivoted rootfs, so this subcommand is always available no matter +// how minimal the image — alpine, busybox, distroless alike: +// +// xmorph pivot --entrypoint /usr/local/bin/xmorph --cmd idle --ssh.enable +func newIdleCmd() *cobra.Command { + return &cobra.Command{ + Use: "idle", + Short: "Block until signalled, keeping a pivoted system up and reachable", + Long: `idle runs nothing and waits for SIGTERM or SIGINT, reaping orphaned +children while it waits. + +It exists to be a pivot's entrypoint when the pivot's purpose is access rather +than execution. xmorph places its own binary at ` + postpivot.BinaryPath + ` in +the new rootfs, so this works in images that ship no shell and no coreutils.`, + Args: cobra.NoArgs, + RunE: func(cmd *cobra.Command, args []string) error { + postpivot.ServeUntilSignal() + return nil + }, + } +} diff --git a/internal/cli/pivot.go b/internal/cli/pivot.go index bcb695d..0241c47 100644 --- a/internal/cli/pivot.go +++ b/internal/cli/pivot.go @@ -350,7 +350,6 @@ func buildPostpivotConfig(cfg *config.Config, entrypoint string, entryArgs []str pc := &postpivot.Config{ FlushFirewall: !cfg.KeepFirewall, RebootOnExit: true, - Serve: serveOnly(cfg), WatchdogTimeoutSeconds: int(cfg.WatchdogTimeout / time.Second), KeepOldRoot: cfg.KeepOldRoot, Entrypoint: append([]string{entrypoint}, entryArgs...), @@ -446,7 +445,7 @@ var shellEntrypoints = map[string]bool{ // Only the no-TTY case is rejected. With a console attached (--contain, or a // serial line) a shell is exactly what someone may want, so stdin decides. func checkEntrypointSurvivesDetach(cfg *config.Config, entrypoint string) error { - if cfg.Serve || cfg.Contain { + if cfg.Contain { return nil } if !shellEntrypoints[filepath.Base(entrypoint)] { @@ -458,25 +457,9 @@ func checkEntrypointSurvivesDetach(cfg *config.Config, entrypoint string) error return fmt.Errorf( "entrypoint %q is a shell with no terminal attached: it will read EOF on stdin "+ "and exit immediately after the pivot, rebooting the box back into the on-disk OS.\n"+ - " --serve stay up serving SSH (what a remote rescue pivot wants)\n"+ - " --command ... run a specific program instead\n"+ - " --entrypoint ... override the image's default explicitly", - entrypoint) -} - -// serveOnly reports whether to hold the box up for SSH instead of running a -// program. This is the operator's explicit --serve and nothing else. -// -// It deliberately does not infer intent from the image. An earlier version -// switched to serve mode whenever services were enabled and no command was -// named, on the theory that the image's default shell could not have been -// meant. That is wrong: an image whose Cmd is a real long-running daemon — -// precisely what a purpose-built rescue image looks like — is -// indistinguishable from alpine's ["/bin/sh"] at this point, so the guess -// silently skips the very program the operator built the image around. -// Honouring the image and rebooting on exit is predictable; guessing is not. -func serveOnly(cfg *config.Config) bool { - return cfg.Serve + " --entrypoint %s --cmd idle stay up and reachable, running nothing\n"+ + " --command ... run a specific program instead", + entrypoint, postpivot.BinaryPath) } // resolveEntrypoint picks the effective entrypoint + args + env from the diff --git a/internal/cli/root.go b/internal/cli/root.go index 0b769df..3e75b73 100644 --- a/internal/cli/root.go +++ b/internal/cli/root.go @@ -92,7 +92,7 @@ supports unattended operation over Tailscale (in-process via tsnet).`, // InitDefaultVersionFlag sees an existing flag and leaves it alone. root.Flags().BoolP("version", "V", false, "print version and exit") - root.AddCommand(newPivotCmd(), newBuildCmd(), newVersionCmd()) + root.AddCommand(newPivotCmd(), newBuildCmd(), newIdleCmd(), newVersionCmd()) return root } diff --git a/internal/config/config.go b/internal/config/config.go index 085cbd4..e169837 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -70,9 +70,6 @@ type Config struct { Entrypoint string EntrypointExplicit bool Command []string - // Serve holds the box up serving SSH instead of running an entrypoint. - // Mutually exclusive with --entrypoint/--command. - Serve bool KeepOldRoot string diff --git a/internal/config/flags.go b/internal/config/flags.go index 51f0fe0..8ff11f8 100644 --- a/internal/config/flags.go +++ b/internal/config/flags.go @@ -51,8 +51,6 @@ func bindPivotOnly(fs *pflag.FlagSet, cfg *Config) { fs.Var(&appendStringVar{dst: &cfg.Command}, "command", "command/args passed to entrypoint (repeatable)") fs.Var(&appendStringVar{dst: &cfg.Command}, "cmd", "alias for --command") - fs.BoolVar(&cfg.Serve, "serve", false, "run no entrypoint; stay up serving SSH until stopped") - // --keep-old-root with optional value: bare form uses the default, // --keep-old-root=/foo overrides. --no-keep-old-root clears it. fs.StringVar(&cfg.KeepOldRoot, "keep-old-root", DefaultKeepOldRoot, "keep old root mounted at PATH after pivot (default /mnt/oldroot)") diff --git a/internal/config/serve_test.go b/internal/config/serve_test.go deleted file mode 100644 index 5e0456b..0000000 --- a/internal/config/serve_test.go +++ /dev/null @@ -1,35 +0,0 @@ -package config - -import ( - "errors" - "io" - "testing" -) - -// --serve runs nothing, so naming something to run is contradictory and must -// be rejected rather than silently resolved in either direction. -func TestValidateServeWithCommand(t *testing.T) { - for _, tc := range []struct { - name string - mutate func(*Config) - wantErr bool - }{ - {"serve alone", func(c *Config) { c.Serve = true }, false}, - {"serve with command", func(c *Config) { c.Serve = true; c.Command = []string{"sleep", "1"} }, true}, - {"serve with entrypoint", func(c *Config) { c.Serve = true; c.EntrypointExplicit = true }, true}, - {"command without serve", func(c *Config) { c.Command = []string{"sleep", "1"} }, false}, - {"neither", func(*Config) {}, false}, - } { - t.Run(tc.name, func(t *testing.T) { - cfg := New() - tc.mutate(&cfg) - err := cfg.Validate(io.Discard) - if tc.wantErr && !errors.Is(err, ErrServeWithCommand) { - t.Errorf("want ErrServeWithCommand, got %v", err) - } - if !tc.wantErr && errors.Is(err, ErrServeWithCommand) { - t.Errorf("unexpected ErrServeWithCommand: %v", err) - } - }) - } -} diff --git a/internal/config/validate.go b/internal/config/validate.go index 82cd535..b918524 100644 --- a/internal/config/validate.go +++ b/internal/config/validate.go @@ -13,9 +13,6 @@ var ErrInvalidTimeout = errors.New("timeout must be greater than zero") // ErrWatchdogTooShort is returned when --watchdog-timeout is set below one second. var ErrWatchdogTooShort = errors.New("watchdog-timeout must be at least 1s") -// ErrServeWithCommand rejects --serve alongside --entrypoint/--command. -var ErrServeWithCommand = errors.New("--serve runs no entrypoint; drop --entrypoint/--command, or drop --serve to run them") - // Validate runs the same set of post-parse checks as src/config.zig:598-626, // minus the containerfile mutual-exclusion rule (containerfile support was // dropped). Warnings go to warnW (typically os.Stderr). @@ -28,13 +25,6 @@ func (c *Config) Validate(warnW io.Writer) error { return ErrWatchdogTooShort } - // --serve runs nothing, so naming something to run contradicts it. Reject - // rather than pick a winner: either choice silently discards what the - // operator asked for, and this is the last cheap moment to say so. - if c.Serve && (c.EntrypointExplicit || len(c.Command) > 0) { - return ErrServeWithCommand - } - if c.TailscaleAuthkey == "" { if c.TailscaleArgs != "" { fmt.Fprintln(warnW, "Warning: --tailscale.args without --tailscale.authkey won't start tailscale") diff --git a/internal/postpivot/configwrite.go b/internal/postpivot/configwrite.go index 045d358..5d6d75d 100644 --- a/internal/postpivot/configwrite.go +++ b/internal/postpivot/configwrite.go @@ -28,10 +28,6 @@ type Config struct { // whatever its status. See SuperviseOptions.RebootOnExit for why a // clean exit is treated as fatal too. RebootOnExit bool `json:"reboot_on_exit"` - // Serve means "there is no entrypoint; stay up and serve SSH until - // signalled". Set when the operator enabled SSH or Tailscale but named - // no command to run, which is the normal shape of a remote rescue pivot. - Serve bool `json:"serve,omitempty"` // WatchdogTimeoutSeconds; 0 disables. WatchdogTimeoutSeconds int `json:"watchdog_timeout_seconds,omitempty"` // KeepOldRoot is the pre-pivot root's mount point (default diff --git a/internal/postpivot/configwrite_test.go b/internal/postpivot/configwrite_test.go index 20e997c..76b4963 100644 --- a/internal/postpivot/configwrite_test.go +++ b/internal/postpivot/configwrite_test.go @@ -12,7 +12,6 @@ func TestWriteConfigRoundTrip(t *testing.T) { cfg := &Config{ FlushFirewall: true, RebootOnExit: true, - Serve: true, WatchdogTimeoutSeconds: 300, KeepOldRoot: "/mnt/oldroot", LogPersistDir: "/mnt/oldroot/var/log/xmorph", @@ -40,7 +39,7 @@ func TestWriteConfigRoundTrip(t *testing.T) { if err := json.Unmarshal(data, &got); err != nil { t.Fatalf("unmarshal: %v", err) } - if !got.FlushFirewall || !got.RebootOnExit || !got.Serve { + if !got.FlushFirewall || !got.RebootOnExit { t.Error("boolean fields lost") } if got.WatchdogTimeoutSeconds != 300 { diff --git a/internal/postpivot/run.go b/internal/postpivot/run.go index aadf036..4d40d13 100644 --- a/internal/postpivot/run.go +++ b/internal/postpivot/run.go @@ -103,12 +103,6 @@ func Run(argv []string) int { }() } - // No entrypoint to run: hold the box up and serve SSH. Returning here - // would leave a kernel with no userspace, so this blocks until signalled. - if cfg != nil && cfg.Serve { - return ServeUntilSignal() - } - // Decide what to exec. Config-supplied entrypoint+command beats argv. var supervised []string if cfg != nil && len(cfg.Entrypoint) > 0 { @@ -122,6 +116,17 @@ func Run(argv []string) int { return 1 } + // `xmorph idle` blocks until signalled, which is exactly what this + // supervisor would do while waiting on it. Recognise our own binary and + // block here rather than forking a second copy — on the small machines + // this tool targets, a redundant Go runtime is real memory in a + // tmpfs-backed rootfs. Purely an optimisation; the semantics are the + // entrypoint's either way. + if len(supervised) == 2 && supervised[0] == BinaryPath && supervised[1] == "idle" { + slog.Info("entrypoint is xmorph idle; blocking in the supervisor") + return ServeUntilSignal() + } + rebootOnExit := cfg == nil || cfg.RebootOnExit var oldRoot string if cfg != nil {