From 4fbed4cd9e5302f8ea4286913f9bcb3c0ca6193f Mon Sep 17 00:00:00 2001 From: Teodor Calin Date: Thu, 24 Sep 2026 12:15:54 +0300 Subject: [PATCH 1/6] pilotctl: uninstall stops what the app left running `appstore uninstall` deleted the app dir and left everything the app had started that detached from it: a smolvm microVM (`smolvm-bin _boot-vm`, own session, ~311 MiB RSS), a daemonized redis/postgres/mysql server, an in-flight exec child orphaned by the supervisor's pid-only SIGKILL, an app instance orphaned by a daemon that died hard. Reparented to init/launchd, they ran on with nothing left to manage them. Every one of them runs a binary that lives in the app dir (the app itself or a native tool it staged under $APP), so uninstall now finds them by executable path, not by process tree, group or session, which they are free to leave: SIGTERM, 5 s grace, SIGKILL. It covers the app dir and its kept backups (a process started before an upgrade runs from the retired dir), runs once before the delete and once after (a supervisor that had not yet seen the manifest go may respawn the app in between), and reports what it stopped in the output, --json (`stopped_processes`) and the audit record. - macOS reads the exec path from kern.procargs2 (kept after the file is deleted); Linux reads /proc//exe (" (deleted)" dropped) and, for a binary run under binfmt emulation (Rosetta for Linux, qemu-user) where the kernel reports the translator, an executable mapping in /proc//maps. - Each pid is re-checked right before it is signalled, so a reused pid is never touched. pid 1, pilotctl and its parent are skipped. io.pilot.smolmachines 1.2.0 (the real published darwin/arm64 bundle, VM started through smolmachines.exec) against origin/main pilotctl: VM pid=58593 STILL RUNNING after uninstall ... 319712 KB smolvm-bin LEAK: VM pid=58593 still running with nothing left to manage it with this change: stopped 2 process(es) still running from the app's files: smolmachines-app (pid 58665), smolvm-bin (pid 58844) Linux arm64 and amd64 (Rosetta): the orphaned `smolvm-bin serve start` child is LEFT on main and stopped here. Co-Authored-By: Claude Opus 5.5 (1M context) --- cmd/pilotctl/appstore.go | 47 ++++- cmd/pilotctl/appstore_procs.go | 181 ++++++++++++++++ cmd/pilotctl/appstore_procs_darwin.go | 51 +++++ cmd/pilotctl/appstore_procs_linux.go | 66 ++++++ cmd/pilotctl/appstore_procs_other.go | 17 ++ cmd/pilotctl/appstore_procs_test.go | 292 ++++++++++++++++++++++++++ 6 files changed, 652 insertions(+), 2 deletions(-) create mode 100644 cmd/pilotctl/appstore_procs.go create mode 100644 cmd/pilotctl/appstore_procs_darwin.go create mode 100644 cmd/pilotctl/appstore_procs_linux.go create mode 100644 cmd/pilotctl/appstore_procs_other.go create mode 100644 cmd/pilotctl/appstore_procs_test.go diff --git a/cmd/pilotctl/appstore.go b/cmd/pilotctl/appstore.go index 13067a44..6f680ff6 100644 --- a/cmd/pilotctl/appstore.go +++ b/cmd/pilotctl/appstore.go @@ -30,6 +30,7 @@ import ( "os" "path/filepath" "runtime" + "slices" "sort" "strconv" "strings" @@ -830,6 +831,19 @@ func cmdAppStoreUninstall(args []string) { "check install root permissions", "remove manifest %s: %v", mfPath, err) } + // Stop everything still running from the app's files: the app itself + // and whatever it started that outlives it (a smolvm microVM, a + // daemonized database server, an instance orphaned by a daemon that + // died hard). After the delete nothing could reach them any more. The + // backups of earlier installs are included: a process started before an + // upgrade keeps running from the retired dir. See appstore_procs.go. + backups := listAppBackups(root, appID) + procRoots := appDirRoots(dir) + for _, b := range backups { + procRoots = append(procRoots, appDirRoots(b)...) + } + stopped, _ := stopProcessesRunningFrom(procRoots, appDirStopGrace) + // Retry RemoveAll a few times to ride out the supervisor's // in-flight audit writes; the rescan loop cancels the goroutine // within ~RescanInterval (default 30s in prod, but the audit @@ -855,22 +869,39 @@ func cmdAppStoreUninstall(args []string) { "remove %s: %v", dir, rmErr) } + // A supervisor that had not yet seen the manifest go may have respawned + // the app between the stop above and the delete (its restart backoff + // starts at 1s). That instance runs a now-deleted binary; stop it too. + respawned, _ := stopProcessesRunningFrom(procRoots, appDirStopGrace) + for _, p := range respawned { + if !slices.ContainsFunc(stopped, func(q appDirProcess) bool { return q.PID == p.PID }) { + stopped = append(stopped, p) + } + } + stillRunning := processesRunningFrom(procRoots) + // Forensic trail at the install-root level (survives the deletion // of the app dir). Pairs with the install-time event we wrote // into supervisor.log earlier — gives "this app existed between // install T0 and uninstall T1" reconstructable post-hoc. + reason := fmt.Sprintf("actor=%s removed=%s", currentActor(), dir) + if len(stopped) > 0 { + reason += " stopped=" + describeAppDirProcesses(stopped) + } + if len(stillRunning) > 0 { + reason += " still_running=" + describeAppDirProcesses(stillRunning) + } writePilotctlAudit(root, pilotctlAuditEvent{ Event: "uninstalled", AppID: appID, SHA256: snapSHA, AppVer: snapVer, - Reason: fmt.Sprintf("actor=%s removed=%s", currentActor(), dir), + Reason: reason, }) // Replaced installs are kept as backups (appstore_state.go), wherever // retireAppDir had to put them, and may hold the app's keys. Uninstall // leaves them, so it says where every one of them is. - backups := listAppBackups(root, appID) if jsonOutput { out := map[string]any{ "id": appID, @@ -880,10 +911,22 @@ func cmdAppStoreUninstall(args []string) { if len(backups) > 0 { out["backups"] = backups } + if len(stopped) > 0 { + out["stopped_processes"] = stopped + } + if len(stillRunning) > 0 { + out["still_running"] = stillRunning + } _ = json.NewEncoder(os.Stdout).Encode(out) return } fmt.Printf("removed %s\n", dir) + if len(stopped) > 0 { + fmt.Printf("stopped %d process(es) still running from the app's files: %s\n", len(stopped), describeAppDirProcesses(stopped)) + } + if len(stillRunning) > 0 { + fmt.Fprintf(os.Stderr, "warn: still running from the removed app's files after SIGKILL: %s; stop them by hand\n", describeAppDirProcesses(stillRunning)) + } fmt.Println("note: the daemon's supervisor will cancel its per-app goroutine on its next rescan") fmt.Println(" (≤30s); no daemon restart needed") if len(backups) > 0 { diff --git a/cmd/pilotctl/appstore_procs.go b/cmd/pilotctl/appstore_procs.go new file mode 100644 index 00000000..7b16d9bb --- /dev/null +++ b/cmd/pilotctl/appstore_procs.go @@ -0,0 +1,181 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later + +package main + +import ( + "os" + "path/filepath" + "sort" + "strconv" + "strings" + "syscall" + "time" +) + +// Stopping what an app leaves running when it is uninstalled. +// +// The supervisor stops an app by signalling the app's own process (and, with +// newer app-store releases, its process group). Anything the app started that +// detached into its own session or group survives that: a smolvm microVM +// (`smolvm-bin _boot-vm`, measured at ~311 MiB RSS), a `redis-server +// --daemonize`, a postmaster, a mysqld. They are reparented to init/launchd. +// So is an app instance orphaned by a daemon that died hard (macOS has no +// parent-death signal). Once the app's directory is deleted nothing can +// manage them any more: no app answers for them, `reapStale` matches only the +// app binary + socket argv, and they keep running until the machine reboots. +// +// Every one of those processes runs a binary that lives inside the app's +// directory (the app binary itself, or a native tool the app staged under +// $APP), so that is how uninstall finds them: by the path of the executable, +// not by process tree, group or session, which the processes are free to +// leave. + +// appDirStopGrace is how long processes running from an app dir get to exit +// on SIGTERM before they are SIGKILLed. smolvm stops a VM in ~60 ms and a +// postmaster's fast paths finish well inside this. +const appDirStopGrace = 5 * time.Second + +// appDirProcess is a running process whose executable lives under an app dir. +type appDirProcess struct { + PID int `json:"pid"` + Exe string `json:"exe"` +} + +// appDirRoots returns the forms of dir a kernel may report an executable +// under: the cleaned absolute path and, when it differs, the path with +// symlinks resolved (/tmp → /private/tmp on macOS; /proc//exe is always +// resolved). Call it while dir still exists. +func appDirRoots(dir string) []string { + var roots []string + add := func(p string) { + if p == "" || p == string(filepath.Separator) { + return // never treat the filesystem root as an app dir + } + for _, r := range roots { + if r == p { + return + } + } + roots = append(roots, p) + } + abs, err := filepath.Abs(dir) + if err != nil { + abs = filepath.Clean(dir) + } + add(abs) + if real, err := filepath.EvalSymlinks(abs); err == nil { + add(real) + } + return roots +} + +// pathUnder reports whether p is strictly inside one of roots. +func pathUnder(p string, roots []string) bool { + if !filepath.IsAbs(p) { + return false + } + p = filepath.Clean(p) + for _, r := range roots { + if strings.HasPrefix(p, r+string(filepath.Separator)) { + return true + } + } + return false +} + +// processesRunningFrom returns every process whose executable lives under +// roots. It skips pid 1, this process and its parent (an app that runs +// `pilotctl appstore uninstall` on itself is stopped by the supervisor, not +// by the pilotctl it is waiting on). +func processesRunningFrom(roots []string) []appDirProcess { + if len(roots) == 0 { + return nil + } + self, parent := os.Getpid(), os.Getppid() + var out []appDirProcess + for _, pid := range listProcessIDs() { + if pid <= 1 || pid == self || pid == parent { + continue + } + exe, ok := runningFrom(pid, roots) + if !ok { + continue + } + out = append(out, appDirProcess{PID: pid, Exe: exe}) + } + sort.Slice(out, func(i, j int) bool { return out[i].PID < out[j].PID }) + return out +} + +// runningFrom reports whether pid runs code from under roots, and which file: +// its executable, or else (Linux) an executable mapping of a file there. The +// mapping catches a binary run under binfmt emulation (Rosetta for Linux, +// qemu-user), where the kernel reports the translator as the executable. +func runningFrom(pid int, roots []string) (string, bool) { + if exe, err := processExecutable(pid); err == nil && pathUnder(exe, roots) { + return exe, true + } + return processCodeUnder(pid, roots) +} + +// stillRunningFrom reports whether pid is alive and still runs code from +// under roots. A pid that exited, turned into a zombie (no executable any +// more) or was reused by an unrelated program is not. +func stillRunningFrom(pid int, roots []string) bool { + _, ok := runningFrom(pid, roots) + return ok +} + +// stopProcessesRunningFrom SIGTERMs every process running from roots, gives +// them grace to exit, then SIGKILLs whatever is left. Each pid is re-checked +// against roots immediately before it is signalled, so a pid that exited and +// was reused in between is never touched. It returns the processes it found +// and those that were still running afterwards (signal refused, or stuck in +// the kernel). +func stopProcessesRunningFrom(roots []string, grace time.Duration) (found, left []appDirProcess) { + found = processesRunningFrom(roots) + if len(found) == 0 { + return nil, nil + } + signal := func(p appDirProcess, sig syscall.Signal) { + if !stillRunningFrom(p.PID, roots) { + return + } + if proc, err := os.FindProcess(p.PID); err == nil { + _ = proc.Signal(sig) + } + } + for _, p := range found { + signal(p, syscall.SIGTERM) + } + alive := func() []appDirProcess { + var out []appDirProcess + for _, p := range found { + if stillRunningFrom(p.PID, roots) { + out = append(out, p) + } + } + return out + } + deadline := time.Now().Add(grace) + for len(alive()) > 0 && time.Now().Before(deadline) { + time.Sleep(50 * time.Millisecond) + } + for _, p := range alive() { + signal(p, syscall.SIGKILL) + } + killDeadline := time.Now().Add(2 * time.Second) + for len(alive()) > 0 && time.Now().Before(killDeadline) { + time.Sleep(50 * time.Millisecond) + } + return found, alive() +} + +// describeAppDirProcesses renders "name (pid N), …" for messages. +func describeAppDirProcesses(ps []appDirProcess) string { + parts := make([]string, 0, len(ps)) + for _, p := range ps { + parts = append(parts, filepath.Base(p.Exe)+" (pid "+strconv.Itoa(p.PID)+")") + } + return strings.Join(parts, ", ") +} diff --git a/cmd/pilotctl/appstore_procs_darwin.go b/cmd/pilotctl/appstore_procs_darwin.go new file mode 100644 index 00000000..10bd63b1 --- /dev/null +++ b/cmd/pilotctl/appstore_procs_darwin.go @@ -0,0 +1,51 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later + +//go:build darwin + +package main + +import ( + "bytes" + "errors" + + "golang.org/x/sys/unix" +) + +// listProcessIDs returns every pid in the process table. +func listProcessIDs() []int { + procs, err := unix.SysctlKinfoProcSlice("kern.proc.all") + if err != nil { + return nil + } + pids := make([]int, 0, len(procs)) + for _, p := range procs { + pids = append(pids, int(p.Proc.P_pid)) + } + return pids +} + +// processExecutable returns the path pid was exec'd from, read from +// kern.procargs2: a 32-bit argc, then the exec path, NUL-terminated. The +// kernel copies the path at exec time, so it is still there after the file +// was deleted or its dir removed. Fails for another user's process and for a +// zombie (no address space left to read). +func processExecutable(pid int) (string, error) { + buf, err := unix.SysctlRaw("kern.procargs2", pid) + if err != nil { + return "", err + } + if len(buf) < 5 { + return "", errors.New("short kern.procargs2") + } + rest := buf[4:] // skip argc + if i := bytes.IndexByte(rest, 0); i >= 0 { + rest = rest[:i] + } + if len(rest) == 0 { + return "", errors.New("empty exec path") + } + return string(rest), nil +} + +// processCodeUnder: the executable path above is authoritative here. +func processCodeUnder(int, []string) (string, bool) { return "", false } diff --git a/cmd/pilotctl/appstore_procs_linux.go b/cmd/pilotctl/appstore_procs_linux.go new file mode 100644 index 00000000..e8ca7724 --- /dev/null +++ b/cmd/pilotctl/appstore_procs_linux.go @@ -0,0 +1,66 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later + +//go:build linux + +package main + +import ( + "bufio" + "os" + "strconv" + "strings" +) + +// listProcessIDs returns every pid in /proc. +func listProcessIDs() []int { + entries, err := os.ReadDir("/proc") + if err != nil { + return nil + } + pids := make([]int, 0, len(entries)) + for _, e := range entries { + if pid, err := strconv.Atoi(e.Name()); err == nil { + pids = append(pids, pid) + } + } + return pids +} + +// processExecutable returns the resolved path of pid's executable. The kernel +// appends " (deleted)" once the file is unlinked; that suffix is dropped so a +// process whose app dir was already removed is still recognised. Fails for +// another user's process and for a zombie. +func processExecutable(pid int) (string, error) { + exe, err := os.Readlink("/proc/" + strconv.Itoa(pid) + "/exe") + if err != nil { + return "", err + } + return strings.TrimSuffix(exe, " (deleted)"), nil +} + +// processCodeUnder returns the first file under roots that pid has mapped +// executable (an "x" in /proc//maps). Under binfmt emulation the kernel's +// executable is the translator (/run/rosetta/rosetta, qemu-*), but the guest +// binary is still mapped r-x at its load address. Unreadable for another +// user's process. +func processCodeUnder(pid int, roots []string) (string, bool) { + f, err := os.Open("/proc/" + strconv.Itoa(pid) + "/maps") // #nosec G304 -- /proc//maps for an integer pid + if err != nil { + return "", false + } + defer f.Close() + sc := bufio.NewScanner(f) + sc.Buffer(make([]byte, 0, 64*1024), 1024*1024) + for sc.Scan() { + // address perms offset dev inode pathname + fields := strings.Fields(sc.Text()) + if len(fields) < 6 || !strings.Contains(fields[1], "x") { + continue + } + p := strings.TrimSuffix(strings.Join(fields[5:], " "), " (deleted)") + if pathUnder(p, roots) { + return p, true + } + } + return "", false +} diff --git a/cmd/pilotctl/appstore_procs_other.go b/cmd/pilotctl/appstore_procs_other.go new file mode 100644 index 00000000..9e37f710 --- /dev/null +++ b/cmd/pilotctl/appstore_procs_other.go @@ -0,0 +1,17 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later + +//go:build !linux && !darwin + +package main + +import "errors" + +// No process table access here: uninstall finds nothing to stop. +func listProcessIDs() []int { return nil } + +func processExecutable(int) (string, error) { + return "", errors.New("process executables are not readable on this platform") +} + +// processCodeUnder: the executable path above is authoritative here. +func processCodeUnder(int, []string) (string, bool) { return "", false } diff --git a/cmd/pilotctl/appstore_procs_test.go b/cmd/pilotctl/appstore_procs_test.go new file mode 100644 index 00000000..0db4f6f0 --- /dev/null +++ b/cmd/pilotctl/appstore_procs_test.go @@ -0,0 +1,292 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later + +//go:build linux || darwin + +package main + +import ( + "encoding/json" + "io" + "os" + "os/exec" + "os/signal" + "path/filepath" + "strings" + "syscall" + "testing" + "time" +) + +// A copy of this test binary started with PILOTCTL_TEST_APPDIR_PROC set plays +// a process an app left running (a VM, a daemonized server): it optionally +// ignores SIGTERM, reports that it is ready, and sleeps. init runs before +// TestMain, so the copy never runs any test. +func init() { + mode := os.Getenv("PILOTCTL_TEST_APPDIR_PROC") + if mode == "" { + return + } + if mode == "ignore-term" { + signal.Ignore(syscall.SIGTERM) + } + if ready := os.Getenv("PILOTCTL_TEST_APPDIR_READY"); ready != "" { + _ = os.WriteFile(ready, nil, 0o600) + } + time.Sleep(time.Hour) + os.Exit(0) +} + +// placeTestBinary puts this test binary at dst (hard link, else a copy). +func placeTestBinary(t *testing.T, dst string) { + t.Helper() + self, err := os.Executable() + if err != nil { + t.Fatal(err) + } + if err := os.MkdirAll(filepath.Dir(dst), 0o755); err != nil { + t.Fatal(err) + } + if os.Link(self, dst) == nil { + return + } + in, err := os.Open(self) + if err != nil { + t.Fatal(err) + } + defer in.Close() + out, err := os.OpenFile(dst, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0o755) + if err != nil { + t.Fatal(err) + } + if _, err := io.Copy(out, in); err != nil { + t.Fatal(err) + } + if err := out.Close(); err != nil { + t.Fatal(err) + } +} + +// startDetached runs bin as a sleeper in its own session, the way smolvm +// starts a VM and redis-server --daemonize detaches, and waits until it is +// up. The process is reaped in the background so a stopped one never lingers +// as a zombie of the test. +func startDetached(t *testing.T, bin, mode string) (pid int, exited <-chan struct{}) { + t.Helper() + ready := filepath.Join(t.TempDir(), "ready") + cmd := exec.Command(bin) + cmd.Env = append(os.Environ(), "PILOTCTL_TEST_APPDIR_PROC="+mode, "PILOTCTL_TEST_APPDIR_READY="+ready) + cmd.SysProcAttr = &syscall.SysProcAttr{Setsid: true} + if err := cmd.Start(); err != nil { + t.Fatal(err) + } + done := make(chan struct{}) + go func() { _ = cmd.Wait(); close(done) }() + t.Cleanup(func() { + _ = cmd.Process.Kill() + <-done + }) + deadline := time.Now().Add(20 * time.Second) + for { + if _, err := os.Stat(ready); err == nil { + break + } + if time.Now().After(deadline) { + t.Fatalf("helper %s never became ready", bin) + } + time.Sleep(20 * time.Millisecond) + } + return cmd.Process.Pid, done +} + +func requireProcessTable(t *testing.T) { + t.Helper() + if _, err := processExecutable(os.Getpid()); err != nil { + t.Skipf("process executables are not readable here (%v); a sandbox may block it", err) + } +} + +func waitExited(t *testing.T, what string, done <-chan struct{}) { + t.Helper() + select { + case <-done: + case <-time.After(10 * time.Second): + t.Fatalf("%s is still running", what) + } +} + +func pidsOf(ps []appDirProcess) map[int]bool { + out := map[int]bool{} + for _, p := range ps { + out[p.PID] = true + } + return out +} + +func TestPathUnder(t *testing.T) { + roots := []string{"/srv/apps/io.x", "/private/srv/apps/io.x"} + for p, want := range map[string]bool{ + "/srv/apps/io.x/bin/app": true, + "/private/srv/apps/io.x/smolvm-bin": true, + "/srv/apps/io.x": false, // the dir itself is not a binary in it + "/srv/apps/io.xy/bin/app": false, // sibling app with a common prefix + "/srv/apps/io.x/../io.y/bin/app": false, + "bin/app": false, // relative exec paths are never matched + "": false, + } { + if got := pathUnder(p, roots); got != want { + t.Errorf("pathUnder(%q) = %v, want %v", p, got, want) + } + } +} + +func TestAppDirRootsResolvesSymlinks(t *testing.T) { + base := t.TempDir() + real := filepath.Join(base, "real") + if err := os.MkdirAll(real, 0o755); err != nil { + t.Fatal(err) + } + link := filepath.Join(base, "link") + if err := os.Symlink(real, link); err != nil { + t.Fatal(err) + } + roots := appDirRoots(link) + resolved, _ := filepath.EvalSymlinks(real) + if !pathUnder(filepath.Join(link, "bin", "x"), roots) || !pathUnder(filepath.Join(resolved, "bin", "x"), roots) { + t.Fatalf("roots %v must cover both the link and its target", roots) + } + if got := appDirRoots("/"); len(got) != 0 { + t.Fatalf("the filesystem root must never be an app dir: %v", got) + } +} + +// A detached process running from the app dir is stopped; one running the +// same program from elsewhere is left alone. +func TestStopProcessesRunningFromStopsOnlyTheAppsOwn(t *testing.T) { + requireProcessTable(t) + appDir := filepath.Join(t.TempDir(), "io.test.vm") + inside := filepath.Join(appDir, "tool-1.0-os-arch", "tool-bin") + placeTestBinary(t, inside) + outside := filepath.Join(t.TempDir(), "elsewhere", "tool-bin") + placeTestBinary(t, outside) + + inPID, inDone := startDetached(t, inside, "sleep") + outPID, outDone := startDetached(t, outside, "sleep") + + roots := appDirRoots(appDir) + found, left := stopProcessesRunningFrom(roots, 5*time.Second) + if !pidsOf(found)[inPID] { + t.Fatalf("found %+v, want pid %d (running %s)", found, inPID, inside) + } + if pidsOf(found)[outPID] { + t.Fatalf("pid %d runs from outside the app dir and must not be touched: %+v", outPID, found) + } + if len(left) != 0 { + t.Fatalf("left running: %+v", left) + } + waitExited(t, "the app dir's process", inDone) + select { + case <-outDone: + t.Fatal("the process running from outside the app dir was stopped") + default: + } +} + +// A process that ignores SIGTERM is SIGKILLed once the grace runs out. +func TestStopProcessesRunningFromKillsAfterGrace(t *testing.T) { + requireProcessTable(t) + appDir := filepath.Join(t.TempDir(), "io.test.stubborn") + bin := filepath.Join(appDir, "bin", "server") + placeTestBinary(t, bin) + pid, done := startDetached(t, bin, "ignore-term") + + start := time.Now() + found, left := stopProcessesRunningFrom(appDirRoots(appDir), 300*time.Millisecond) + if !pidsOf(found)[pid] || len(left) != 0 { + t.Fatalf("found=%+v left=%+v, want pid %d stopped", found, left, pid) + } + waitExited(t, "the SIGTERM-ignoring process", done) + if took := time.Since(start); took < 300*time.Millisecond { + t.Fatalf("stopped in %s: SIGTERM should have been ignored until the grace ran out", took) + } +} + +// The app dir may already be gone (a supervisor respawned the app between +// the stop and the delete): the process still counts as the app's. +func TestStopProcessesRunningFromDeletedDir(t *testing.T) { + requireProcessTable(t) + appDir := filepath.Join(t.TempDir(), "io.test.gone") + bin := filepath.Join(appDir, "bin", "app") + placeTestBinary(t, bin) + pid, done := startDetached(t, bin, "sleep") + + roots := appDirRoots(appDir) + if err := os.RemoveAll(appDir); err != nil { + t.Fatal(err) + } + found, left := stopProcessesRunningFrom(roots, 5*time.Second) + if !pidsOf(found)[pid] || len(left) != 0 { + t.Fatalf("found=%+v left=%+v, want pid %d stopped", found, left, pid) + } + waitExited(t, "the process of the deleted app dir", done) +} + +// `appstore uninstall` stops what the app left running — from the app dir +// and from a backup of an earlier install — before deleting the dir, and +// reports it. +func TestCmdAppStoreUninstallStopsLeftoverProcesses(t *testing.T) { + requireProcessTable(t) + base := t.TempDir() + root := filepath.Join(base, "apps") + t.Setenv("PILOT_APPSTORE_ROOT", root) + t.Setenv("PILOT_APPSTORE_BACKUP_ROOT", "") + appID := "io.test.leftover" + appDir := filepath.Join(root, appID) + if err := os.MkdirAll(appDir, 0o755); err != nil { + t.Fatal(err) + } + mf := `{"id":"` + appID + `","app_version":"1.0.0","manifest_version":1,` + + `"binary":{"path":"bin/app","sha256":"` + hex64 + `"},"exposes":["leftover.help"]}` + if err := os.WriteFile(filepath.Join(appDir, "manifest.json"), []byte(mf), 0o644); err != nil { + t.Fatal(err) + } + vm := filepath.Join(appDir, "tool-1.0", "tool-bin") + placeTestBinary(t, vm) + vmPID, vmDone := startDetached(t, vm, "sleep") + + backup := filepath.Join(base, "app-backups", appID, "20260101T000000Z-upgrade") + old := filepath.Join(backup, "tool-0.9", "tool-bin") + placeTestBinary(t, old) + oldPID, oldDone := startDetached(t, old, "sleep") + + prev := jsonOutput + defer func() { jsonOutput = prev }() + jsonOutput = true + out := captureStdout(t, func() { cmdAppStoreUninstall([]string{appID, "--yes"}) }) + var resp struct { + Stopped []appDirProcess `json:"stopped_processes"` + StillRunning []appDirProcess `json:"still_running"` + Backups []string `json:"backups"` + } + if err := json.Unmarshal([]byte(out), &resp); err != nil { + t.Fatalf("parse: %v\n%s", err, out) + } + got := pidsOf(resp.Stopped) + if !got[vmPID] || !got[oldPID] { + t.Fatalf("stopped_processes = %+v, want pids %d and %d", resp.Stopped, vmPID, oldPID) + } + if len(resp.StillRunning) != 0 { + t.Fatalf("still_running = %+v", resp.StillRunning) + } + waitExited(t, "the process running from the app dir", vmDone) + waitExited(t, "the process running from the backup", oldDone) + if _, err := os.Stat(appDir); !os.IsNotExist(err) { + t.Fatalf("app dir should be gone: %v", err) + } + if _, err := os.Stat(backup); err != nil { + t.Fatalf("the backup must be kept: %v", err) + } + audit, _ := os.ReadFile(filepath.Join(root, pilotctlAuditFileName)) + if !strings.Contains(string(audit), "stopped=") { + t.Fatalf("audit record does not name the stopped processes: %s", audit) + } +} From 5e570877f978d349829b643328a28e87ed3e3dc8 Mon Sep 17 00:00:00 2001 From: Teodor Calin Date: Thu, 24 Sep 2026 12:16:13 +0300 Subject: [PATCH 2/6] pilotctl: renamed apps point to their new id MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit catalogue/README.md ("Renaming an app") promised that a tombstone entry (`renamed_to` + `hidden`, no bundle) is hidden from the listing and that install/view/call of the old id warn and route to the new one. No released pilotctl implements it: catalogueEntry had neither field (checked on main, v1.13.9, v1.13.10-rc.1, v1.12.0, v1.11.0). With the io.pilot.smolmachines tombstone (renamed to io.pilot.smol) on main today: catalogue lists "Smol Machines (renamed → io.pilot.smol)" install "catalogue entry io.pilot.smolmachines has placeholder sha256 — the release pipeline hasn't filled this in yet" outdated "all installed apps are up to date" (with 1.2.0 installed) upgrade "already up to date (or not a catalogue app)" call "is the daemon running ...?" so installed copies never learn about the rename and keep running an adapter that leaves its VMs behind. Now: - `catalogue` (text and --json) omits tombstones. - `install ` warns and installs `renamed_to` (one hop; a missing or chained target is an error). `install --version X` refuses and names the new id: a pin names a release of one app, and following it would make the managed-fleet reconcile, which checks for the old id afterwards, reinstall the new app every cycle. - `outdated` reports an installed old id as `renamed` (AVAILABLE = new id). `upgrade --all`, which the hourly updater runs, skips it: the new id has its own publisher key and method names. `upgrade ` exits 1 with the install-then-uninstall steps. - `view ` warns; `call` of an old id that is not installed says it was renamed (the catalogue is only fetched when the app dir is absent). Verified in docker linux/arm64 against the committed signed catalogue: install io.pilot.smolmachines fetched io.pilot.smol-1.2.0-linux-arm64.tar.gz, "sha256 OK (62cd9b71…)", installed io.pilot.smol v1.2.0. On darwin/amd64 (Rosetta) it fails with "io.pilot.smol has no bundle for this platform (darwin/amd64); published platforms: darwin/arm64, linux/amd64, linux/arm64" instead of the placeholder-sha error. Co-Authored-By: Claude Opus 5.5 (1M context) --- cmd/pilotctl/appstore.go | 13 +++ cmd/pilotctl/appstore_catalogue.go | 85 ++++++++++++--- cmd/pilotctl/appstore_tombstone_test.go | 139 ++++++++++++++++++++++++ cmd/pilotctl/appstore_update.go | 45 +++++++- cmd/pilotctl/appstore_view.go | 3 + 5 files changed, 268 insertions(+), 17 deletions(-) create mode 100644 cmd/pilotctl/appstore_tombstone_test.go diff --git a/cmd/pilotctl/appstore.go b/cmd/pilotctl/appstore.go index 6f680ff6..5ba9ff6c 100644 --- a/cmd/pilotctl/appstore.go +++ b/cmd/pilotctl/appstore.go @@ -2648,6 +2648,19 @@ func cmdAppStoreCall(args []string) { sockPath := filepath.Join(appStoreRoot(), appID, "app.sock") if _, err := os.Stat(sockPath); err != nil { + // Not installed here at all: if the id is a rename tombstone, say so + // instead of blaming the daemon. No silent retarget: the method + // namespace changed too. (An installed app whose daemon is down skips + // the catalogue fetch.) + if _, derr := os.Stat(filepath.Dir(sockPath)); errors.Is(derr, os.ErrNotExist) { + if c, lerr := loadCatalogue(); lerr == nil { + if e := c.findEntry(appID); e != nil && e.RenamedTo != "" { + fatalHint("invalid_argument", + fmt.Sprintf("install it with `pilotctl appstore install %s` and call its methods (`pilotctl appstore view %s`)", e.RenamedTo, e.RenamedTo), + "app %q was renamed to %q", appID, e.RenamedTo) + } + } + } fatalHint("io_error", "is the daemon running and has it supervised this app yet?", "socket %s not present: %v", sockPath, err) diff --git a/cmd/pilotctl/appstore_catalogue.go b/cmd/pilotctl/appstore_catalogue.go index 9e68ccf9..271c6c50 100644 --- a/cmd/pilotctl/appstore_catalogue.go +++ b/cmd/pilotctl/appstore_catalogue.go @@ -118,6 +118,50 @@ type catalogueEntry struct { // manifest. MetadataURL string `json:"metadata_url,omitempty"` MetadataSHA string `json:"metadata_sha256,omitempty"` + + // --- rename tombstone (catalogue/README.md "Renaming an app") --- + // RenamedTo marks a tombstone: the app now lives under this id. The entry + // is kept only so installed copies keep their publisher pin (the daemon + // fail-closes an installed app with no pin). It is never listed and never + // installed itself; install/view/call of the old id point at RenamedTo. + // One hop only: a RenamedTo that names another tombstone is rejected. + RenamedTo string `json:"renamed_to,omitempty"` + // Hidden omits the entry from `catalogue`; it stays resolvable by id. + Hidden bool `json:"hidden,omitempty"` +} + +// listed reports whether the entry belongs in the `catalogue` listing. +func (e catalogueEntry) listed() bool { return !e.Hidden && e.RenamedTo == "" } + +// findEntry returns the entry with this id, or nil. +func (c *catalogue) findEntry(id string) *catalogueEntry { + for i := range c.Apps { + if c.Apps[i].ID == id { + return &c.Apps[i] + } + } + return nil +} + +// resolveRenamed maps id to the entry to install. A plain id returns its own +// entry (nil if absent). A tombstone warns on stderr and returns the canonical +// entry; a missing canonical entry, or one that is itself a tombstone, is an +// error rather than a silent fallback. +func resolveRenamed(c *catalogue, id string) (*catalogueEntry, error) { + e := c.findEntry(id) + if e == nil || e.RenamedTo == "" { + return e, nil + } + to := c.findEntry(e.RenamedTo) + if to == nil { + return nil, fmt.Errorf("app %q was renamed to %q, but %q is not in the catalogue", id, e.RenamedTo, e.RenamedTo) + } + if to.RenamedTo != "" { + return nil, fmt.Errorf("app %q was renamed to %q, which is itself a tombstone (catalogue bug; renames are one hop)", id, e.RenamedTo) + } + fmt.Fprintf(os.Stderr, "warn: app %q was renamed to %q; installing %q instead. Its methods have new names (`pilotctl appstore view %s`).\n", id, to.ID, to.ID, to.ID) + fmt.Fprintf(os.Stderr, " The old id gets no further updates; if it is installed here, remove it once %s works: pilotctl appstore uninstall %s --yes\n", to.ID, id) + return to, nil } // bundleVariant is one platform's downloadable tarball + its pinned sha256. @@ -307,15 +351,21 @@ func cmdAppStoreCatalogue(_ []string) { "check $PILOT_APPSTORE_CATALOG_URL (currently: "+catalogueURL()+")", "%v", err) } + visible := make([]catalogueEntry, 0, len(c.Apps)) + for _, e := range c.Apps { + if e.listed() { + visible = append(visible, e) + } + } if jsonOutput { - _ = json.NewEncoder(os.Stdout).Encode(c.Apps) + _ = json.NewEncoder(os.Stdout).Encode(visible) return } - if len(c.Apps) == 0 { + if len(visible) == 0 { fmt.Println("catalogue is empty") return } - for _, e := range c.Apps { + for _, e := range visible { headline := e.DisplayName if headline == "" { headline = e.Description @@ -363,14 +413,21 @@ func resolveInstallTargetVersion(target, wantVersion string) (string, installSou } c, err := loadCatalogue() if err == nil { - for _, e := range c.Apps { - if target != e.ID { - continue - } + e := c.findEntry(target) + if e != nil && e.RenamedTo != "" { + // A pin names a release of one app. The old id has no releases, + // and following the rename would install a different app id (with + // its own publisher key) than the one the caller checks for + // afterwards — the managed-fleet reconcile would reinstall it + // every cycle. Fail closed and name the new id instead. + return "", installSourceCatalogue, fmt.Errorf("%w: %s was renamed to %s and has no releases of its own; pin %s instead", + ErrCatalogueVersionUnavailable, target, e.RenamedTo, e.RenamedTo) + } + if e != nil { if e.Version != wantVersion { return "", installSourceCatalogue, fmt.Errorf("%w: %s offers %q, not %q", ErrCatalogueVersionUnavailable, e.ID, e.Version, wantVersion) } - dir, fetchErr := fetchAndUnpackBundle(e) + dir, fetchErr := fetchAndUnpackBundle(*e) return dir, installSourceCatalogue, fetchErr } } @@ -388,11 +445,13 @@ func resolveInstallTarget(target string) (string, installSource, error) { // the user knows their URL or env override might be the issue. fmt.Fprintf(os.Stderr, "warn: catalogue lookup failed (%v); proceeding with local-path interpretation\n", err) } else { - for _, e := range c.Apps { - if target == e.ID { - dir, err := fetchAndUnpackBundle(e) - return dir, installSourceCatalogue, err - } + e, rerr := resolveRenamed(c, target) + if rerr != nil { + return "", installSourceCatalogue, rerr + } + if e != nil { + dir, err := fetchAndUnpackBundle(*e) + return dir, installSourceCatalogue, err } } info, err := os.Stat(target) diff --git a/cmd/pilotctl/appstore_tombstone_test.go b/cmd/pilotctl/appstore_tombstone_test.go new file mode 100644 index 00000000..bb53a7cf --- /dev/null +++ b/cmd/pilotctl/appstore_tombstone_test.go @@ -0,0 +1,139 @@ +package main + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +func TestResolveRenamed(t *testing.T) { + c := &catalogue{Apps: []catalogueEntry{ + {ID: "io.pilot.smolmachines", RenamedTo: "io.pilot.smol", Hidden: true, Publisher: "ed25519:old"}, + {ID: "io.pilot.smol", Version: "1.2.0", BundleURL: "https://x/smol.tgz", BundleSHA: hex64, Publisher: "ed25519:new"}, + {ID: "io.pilot.gone", RenamedTo: "io.pilot.missing", Hidden: true}, + {ID: "io.pilot.chain", RenamedTo: "io.pilot.smolmachines", Hidden: true}, + }} + + e, err := resolveRenamed(c, "io.pilot.smolmachines") + if err != nil || e == nil || e.ID != "io.pilot.smol" { + t.Fatalf("tombstone: got (%+v, %v), want io.pilot.smol", e, err) + } + if e, err := resolveRenamed(c, "io.pilot.smol"); err != nil || e == nil || e.ID != "io.pilot.smol" { + t.Fatalf("plain id: got (%+v, %v)", e, err) + } + if e, err := resolveRenamed(c, "io.pilot.unknown"); err != nil || e != nil { + t.Fatalf("unknown id: got (%+v, %v), want (nil, nil)", e, err) + } + if _, err := resolveRenamed(c, "io.pilot.gone"); err == nil || !strings.Contains(err.Error(), "not in the catalogue") { + t.Fatalf("missing canonical: err = %v", err) + } + if _, err := resolveRenamed(c, "io.pilot.chain"); err == nil || !strings.Contains(err.Error(), "one hop") { + t.Fatalf("chained tombstone: err = %v", err) + } +} + +// The committed catalogue carries the io.pilot.smolmachines tombstone. It must +// not be listed, and an installed copy must be reported as renamed (never as up +// to date, and never as a plain version upgrade). +func TestRepoCatalogueTombstone(t *testing.T) { + p := repoCataloguePath(t) + t.Setenv("PILOT_APPSTORE_CATALOG_URL", "file://"+p) + c, err := loadCatalogue() + if err != nil { + t.Fatal(err) + } + e := c.findEntry("io.pilot.smolmachines") + if e == nil { + t.Skip("tombstone no longer in the catalogue") + } + if e.RenamedTo != "io.pilot.smol" || !e.Hidden { + t.Fatalf("tombstone fields not decoded: %+v", *e) + } + if e.listed() { + t.Error("tombstone must not be listed") + } + + root := t.TempDir() + t.Setenv("PILOT_APPSTORE_ROOT", root) + d := filepath.Join(root, "io.pilot.smolmachines") + if err := os.MkdirAll(d, 0o755); err != nil { + t.Fatal(err) + } + mf := `{"id":"io.pilot.smolmachines","app_version":"1.2.0","manifest_version":1,` + + `"binary":{"path":"bin/x","sha256":"` + hex64 + `"},"exposes":["smolmachines.help"],` + + `"store":{"publisher":"ed25519:AAA"}}` + if err := os.WriteFile(filepath.Join(d, "manifest.json"), []byte(mf), 0o644); err != nil { + t.Fatal(err) + } + out, err := findOutdated() + if err != nil { + t.Fatal(err) + } + if len(out) != 1 || out[0].Reason != "renamed" || out[0].RenamedTo != "io.pilot.smol" { + t.Fatalf("findOutdated = %+v, want one renamed → io.pilot.smol", out) + } +} + +// The CLI surface for a renamed id, against the committed signed catalogue: +// hidden from the listing, a version pin fails closed naming the new id, +// `upgrade --all` (the hourly updater) never crosses the rename, `upgrade +// ` explains how to move, and `call` of an old id that is not installed +// says it was renamed instead of blaming the daemon. +func TestRenamedAppCLI(t *testing.T) { + cat := repoCataloguePath(t) + root := t.TempDir() + env := map[string]string{ + "PILOT_APPSTORE_CATALOG_URL": "file://" + cat, + "PILOT_APPSTORE_ROOT": root, + "PILOT_SOCKET": filepath.Join(t.TempDir(), "no-daemon.sock"), + "PILOT_TELEMETRY_URL": "http://127.0.0.1:9/", + } + c, err := func() (*catalogue, error) { + t.Setenv("PILOT_APPSTORE_CATALOG_URL", "file://"+cat) + return loadCatalogue() + }() + if err != nil { + t.Fatal(err) + } + if e := c.findEntry("io.pilot.smolmachines"); e == nil || e.RenamedTo == "" { + t.Skip("the committed catalogue no longer carries the io.pilot.smolmachines tombstone") + } + + out, _, code := runCLI(t, []string{"--json", "appstore", "catalogue"}, env) + if code != 0 || strings.Contains(out, `"io.pilot.smolmachines"`) || !strings.Contains(out, `"io.pilot.smol"`) { + t.Fatalf("catalogue --json (exit %d) must list io.pilot.smol and not the tombstone:\n%s", code, out) + } + + _, errOut, code := runCLI(t, []string{"appstore", "install", "io.pilot.smolmachines", "--version", "1.2.0"}, env) + if code == 0 || !strings.Contains(errOut, "renamed to io.pilot.smol") || !strings.Contains(errOut, "pin io.pilot.smol") { + t.Fatalf("install --version of the old id (exit %d) must fail naming the new id:\n%s", code, errOut) + } + + _, errOut, code = runCLI(t, []string{"appstore", "call", "io.pilot.smolmachines", "smolmachines.help"}, env) + if code == 0 || !strings.Contains(errOut, `renamed to "io.pilot.smol"`) { + t.Fatalf("call of the old id (exit %d) must say it was renamed:\n%s", code, errOut) + } + + d := filepath.Join(root, "io.pilot.smolmachines") + if err := os.MkdirAll(d, 0o755); err != nil { + t.Fatal(err) + } + mf := `{"id":"io.pilot.smolmachines","app_version":"1.2.0","manifest_version":1,` + + `"binary":{"path":"bin/x","sha256":"` + hex64 + `"},"exposes":["smolmachines.help"],` + + `"store":{"publisher":"ed25519:AAA"}}` + if err := os.WriteFile(filepath.Join(d, "manifest.json"), []byte(mf), 0o644); err != nil { + t.Fatal(err) + } + out, _, code = runCLI(t, []string{"appstore", "upgrade", "--all"}, env) + if code != 0 || !strings.Contains(out, "skip io.pilot.smolmachines: renamed to io.pilot.smol") || strings.Contains(out, "upgrading") { + t.Fatalf("upgrade --all (exit %d) must skip the renamed app and upgrade nothing:\n%s", code, out) + } + _, errOut, code = runCLI(t, []string{"appstore", "upgrade", "io.pilot.smolmachines"}, env) + if code == 0 || !strings.Contains(errOut, "pilotctl appstore install io.pilot.smol") { + t.Fatalf("upgrade of the old id (exit %d) must point at installing the new id:\n%s", code, errOut) + } + if _, err := os.Stat(filepath.Join(root, "io.pilot.smol")); !os.IsNotExist(err) { + t.Fatalf("nothing may be installed under the new id by upgrade: %v", err) + } +} diff --git a/cmd/pilotctl/appstore_update.go b/cmd/pilotctl/appstore_update.go index 600fb4e2..db97bc9d 100644 --- a/cmd/pilotctl/appstore_update.go +++ b/cmd/pilotctl/appstore_update.go @@ -98,6 +98,10 @@ type outdatedApp struct { // "rebuilt" (a same-version republish — the catalogue bundle changed under a // version we already have). Both upgrade the same way. Reason string `json:"reason,omitempty"` + // RenamedTo is set when the installed id is a rename tombstone. Such an app + // is reported but never auto-upgraded: the new id has its own publisher key + // and method namespace, so moving to it is the operator's call. + RenamedTo string `json:"renamed_to,omitempty"` } // findOutdated cross-references installed apps against the signed catalogue and @@ -123,6 +127,10 @@ func findOutdated() ([]outdatedApp, error) { if !ok { continue } + if e.RenamedTo != "" { + out = append(out, outdatedApp{ID: a.ID, Installed: a.AppVersion, Available: e.RenamedTo, Reason: "renamed", RenamedTo: e.RenamedTo}) + continue + } // resolveBundle picks THIS host's platform bundle, matching what install // recorded; an error/empty sha just disables republish detection for the app. catBundleSHA := "" @@ -174,14 +182,23 @@ func cmdAppStoreOutdated(_ []string) { return } fmt.Printf("%-32s %-12s %-12s %s\n", "APP", "INSTALLED", "AVAILABLE", "WHY") + upgradable := 0 for _, o := range out { + if o.RenamedTo == "" { + upgradable++ + } why := o.Reason - if why == "rebuilt" { + switch why { + case "rebuilt": why = "rebuilt (same version, new bundle)" + case "renamed": + why = fmt.Sprintf("renamed: install %s, then uninstall %s", o.RenamedTo, o.ID) } fmt.Printf("%-32s %-12s %-12s %s\n", o.ID, o.Installed, o.Available, why) } - fmt.Printf("\nupgrade with: pilotctl appstore upgrade (or --all)\n") + if upgradable > 0 { + fmt.Printf("\nupgrade with: pilotctl appstore upgrade (or --all)\n") + } } // cmdAppStoreUpgrade upgrades one app (or --all outdated apps) to the catalogue's @@ -219,9 +236,24 @@ func cmdAppStoreUpgrade(args []string) { var targets []outdatedApp if all { - targets = outdated + skipped := 0 + for _, o := range outdated { + if o.RenamedTo != "" { + // Never migrate across a rename on its own: the hourly updater + // runs --all, and the new id has its own publisher key and + // method names, so switching is the operator's call. + fmt.Printf("skip %s: renamed to %s (install it with `pilotctl appstore install %s`, then `pilotctl appstore uninstall %s --yes`)\n", o.ID, o.RenamedTo, o.RenamedTo, o.ID) + skipped++ + continue + } + targets = append(targets, o) + } if len(targets) == 0 { - fmt.Println("all installed apps are up to date") + if skipped > 0 { + fmt.Println("nothing else to upgrade") + } else { + fmt.Println("all installed apps are up to date") + } return } } else { @@ -231,6 +263,11 @@ func cmdAppStoreUpgrade(args []string) { fmt.Printf("%s is already up to date (or not a catalogue app)\n", id) return } + if o.RenamedTo != "" { + fatalHint("invalid_argument", + fmt.Sprintf("install the new id: pilotctl appstore install %s (then: pilotctl appstore uninstall %s --yes)", o.RenamedTo, o.ID), + "%s was renamed to %s; upgrade does not switch app ids", o.ID, o.RenamedTo) + } targets = []outdatedApp{o} } diff --git a/cmd/pilotctl/appstore_view.go b/cmd/pilotctl/appstore_view.go index afd96b74..4a1a3f41 100644 --- a/cmd/pilotctl/appstore_view.go +++ b/cmd/pilotctl/appstore_view.go @@ -163,6 +163,9 @@ func cmdAppStoreView(args []string) { break } } + if entry != nil && entry.RenamedTo != "" { + fmt.Fprintf(os.Stderr, "warn: app %q was renamed to %q; see `pilotctl appstore view %s`\n", appID, entry.RenamedTo, entry.RenamedTo) + } if entry != nil { if m, err := loadAppMetadata(*entry); err != nil { fmt.Fprintf(os.Stderr, "warn: could not load detail metadata: %v\n", err) From 1f54661f9cbf18b09f8e199e793ab96ed8c185cb Mon Sep 17 00:00:00 2001 From: Teodor Calin Date: Thu, 24 Sep 2026 12:16:26 +0300 Subject: [PATCH 3/6] pilotctl: refuse a cli bundle with no native tool for this platform A "cli" app's adapter is portable Go, so the binary platform check passes, but at its first start it stages the fronted tool from install.json and exits 1 when this host's os/arch is not listed. The supervisor restarts it until the crash-loop limit suspends it: the install "succeeds" and the app never runs. io.pilot.smolmachines 1.2.0 was published for darwin/amd64 like that (upstream smolvm has no macOS x86_64 build): install assets: stage: no asset for darwin/amd64; available: darwin/arm64, linux/amd64, linux/arm64 install now refuses such a bundle with platform_mismatch before anything is staged, naming the platforms the tool ships for. An install.json that does not parse or lists no assets is left to the app, as before. Co-Authored-By: Claude Opus 5.5 (1M context) --- cmd/pilotctl/appstore.go | 6 ++ cmd/pilotctl/appstore_assets_platform_test.go | 59 +++++++++++++++++++ cmd/pilotctl/appstore_platform.go | 52 ++++++++++++++++ 3 files changed, 117 insertions(+) create mode 100644 cmd/pilotctl/appstore_assets_platform_test.go diff --git a/cmd/pilotctl/appstore.go b/cmd/pilotctl/appstore.go index 5ba9ff6c..175fac20 100644 --- a/cmd/pilotctl/appstore.go +++ b/cmd/pilotctl/appstore.go @@ -1302,6 +1302,12 @@ func cmdAppStoreInstall(args []string) { m.ID, m.AppVersion, m.Binary.Path, runtime.GOOS, runtime.GOARCH, err) } + if err := checkBundleAssetsPlatform(bundleDir, runtime.GOOS, runtime.GOARCH); err != nil { + fatalHint("platform_mismatch", + fmt.Sprintf("nothing was installed and any existing install of %s is untouched. %s/%s is not supported by this app; it would exit at every start", m.ID, runtime.GOOS, runtime.GOARCH), + "refusing to install %s v%s: %v", m.ID, m.AppVersion, err) + } + root := appStoreRoot() finalDir := filepath.Join(root, m.ID) stagingDir := finalDir + appStagingSuffix diff --git a/cmd/pilotctl/appstore_assets_platform_test.go b/cmd/pilotctl/appstore_assets_platform_test.go new file mode 100644 index 00000000..e523e740 --- /dev/null +++ b/cmd/pilotctl/appstore_assets_platform_test.go @@ -0,0 +1,59 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later + +package main + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +// The install.json of io.pilot.smolmachines 1.2.0 (every platform's bundle +// carried the same one): smolvm for darwin/arm64, linux/amd64 and linux/arm64, +// nothing for darwin/amd64 — yet a darwin/amd64 bundle was published. +const smolmachinesInstallJSON = `{"schema":1,"app":"io.pilot.smolmachines","version":"1.2.0","command":"smolvm","assets":[ + {"name":"smolvm","role":"binary","os":"darwin","arch":"arm64","exec_path":"smolvm-1.2.0-darwin-arm64/smolvm"}, + {"name":"smolvm","role":"binary","os":"linux","arch":"amd64","exec_path":"smolvm-1.2.0-linux-x86_64/smolvm"}, + {"name":"smolvm","role":"binary","os":"linux","arch":"arm64","exec_path":"smolvm-1.2.0-linux-arm64/smolvm"}]}` + +func bundleWithInstallJSON(t *testing.T, body string) string { + t.Helper() + dir := t.TempDir() + if body != "" { + if err := os.WriteFile(filepath.Join(dir, "install.json"), []byte(body), 0o644); err != nil { + t.Fatal(err) + } + } + return dir +} + +func TestCheckBundleAssetsPlatform(t *testing.T) { + smol := bundleWithInstallJSON(t, smolmachinesInstallJSON) + for _, plat := range [][2]string{{"darwin", "arm64"}, {"linux", "amd64"}, {"linux", "arm64"}} { + if err := checkBundleAssetsPlatform(smol, plat[0], plat[1]); err != nil { + t.Errorf("%s/%s has a smolvm asset: %v", plat[0], plat[1], err) + } + } + err := checkBundleAssetsPlatform(smol, "darwin", "amd64") + if err == nil { + t.Fatal("darwin/amd64 has no smolvm asset; the install must be refused") + } + for _, want := range []string{"smolvm", "darwin/arm64, linux/amd64, linux/arm64", "darwin/amd64"} { + if !strings.Contains(err.Error(), want) { + t.Errorf("error %q does not mention %q", err, want) + } + } + + // Left to the app, as before: no install.json (not a cli app), an + // install.json without assets, or one that does not parse. + for name, body := range map[string]string{ + "no install.json": "", + "no assets": `{"schema":1,"command":"x","assets":[]}`, + "unparseable": `{"assets":`, + } { + if err := checkBundleAssetsPlatform(bundleWithInstallJSON(t, body), "darwin", "amd64"); err != nil { + t.Errorf("%s: %v", name, err) + } + } +} diff --git a/cmd/pilotctl/appstore_platform.go b/cmd/pilotctl/appstore_platform.go index 86221faf..b38f33f9 100644 --- a/cmd/pilotctl/appstore_platform.go +++ b/cmd/pilotctl/appstore_platform.go @@ -5,7 +5,12 @@ package main import ( "debug/macho" "debug/pe" + "encoding/json" + "errors" "fmt" + "io/fs" + "os" + "path/filepath" "runtime" "sort" "strings" @@ -92,3 +97,50 @@ func peMachineArch(machine uint16) string { return fmt.Sprintf("machine %#x", machine) } } + +// checkBundleAssetsPlatform refuses a bundle whose install.json lists native +// tools but none for goos/goarch. A "cli" app's adapter is portable Go, so the +// binary check above passes, but at its first start the adapter stages the +// fronted tool from install.json and exits 1 when this host is not listed +// ("install assets: stage: no asset for darwin/amd64; available: ..."). The +// supervisor restarts it until the crash-loop limit suspends it, so the +// install "succeeds" and the app never runs. io.pilot.smolmachines 1.2.0 was +// published for darwin/amd64 like that (upstream smolvm has no macOS x86_64 +// build). An install.json that does not parse, or lists no assets, is left to +// the app, as before. +func checkBundleAssetsPlatform(bundleDir, goos, goarch string) error { + raw, err := os.ReadFile(filepath.Join(bundleDir, "install.json")) // #nosec G304 -- a file of the bundle being installed + if errors.Is(err, fs.ErrNotExist) { + return nil + } + if err != nil { + return fmt.Errorf("read install.json: %w", err) + } + var spec struct { + Command string `json:"command"` + Assets []struct { + OS string `json:"os"` + Arch string `json:"arch"` + } `json:"assets"` + } + if json.Unmarshal(raw, &spec) != nil || len(spec.Assets) == 0 { + return nil + } + seen := map[string]bool{} + var avail []string + for _, a := range spec.Assets { + if a.OS == goos && a.Arch == goarch { + return nil + } + if p := a.OS + "/" + a.Arch; !seen[p] { + seen[p] = true + avail = append(avail, p) + } + } + sort.Strings(avail) + tool := spec.Command + if tool == "" { + tool = "its native tool" + } + return fmt.Errorf("the bundle ships %s only for %s, not for %s/%s", tool, strings.Join(avail, ", "), goos, goarch) +} From 3864533d504c3545bd8cb0c43fb697aa232ade3f Mon Sep 17 00:00:00 2001 From: Teodor Calin Date: Thu, 24 Sep 2026 12:16:41 +0300 Subject: [PATCH 4/6] catalogue lint: tombstone shape; a native tool for every published platform Two classes of broken entry passed the lint: - An entry with no bundle was skipped as "a tombstone" whatever it held. Now it must be a rename tombstone as catalogue/README.md describes: renamed_to naming an installable entry (not itself a tombstone, one hop), hidden, the old publisher key kept (installed copies are pinned to it), no version (older pilotctl compares it against installed copies) and no metadata_url. Tombstones are checked on every run, touched or not, since a PR can remove or retire the target without editing the tombstone. - A cli bundle published for a platform its install.json ships no native tool for. Against the historical io.pilot.smolmachines 1.2.0 entry (catalogue 386a77dd) the lint now reports: darwin/amd64: install.json ships smolvm only for darwin/arm64, linux/amd64, linux/arm64, so the app exits at every start on darwin/amd64 ... Drop darwin/amd64 from `bundles`, or add its asset The committed catalogue passes both checks: offline in the unit test, and online with every entry treated as new (the only findings are the four already known: cosift, generallegal and wallet legacy native bundles, and the slipstream linux/amd64 pin). Co-Authored-By: Claude Opus 5.5 (1M context) --- catalogue/lint/main.go | 169 ++++++++++++++++++++++++++++++------ catalogue/lint/main_test.go | 73 +++++++++++++++- 2 files changed, 214 insertions(+), 28 deletions(-) diff --git a/catalogue/lint/main.go b/catalogue/lint/main.go index 3c9d7473..36dbb04c 100644 --- a/catalogue/lint/main.go +++ b/catalogue/lint/main.go @@ -19,7 +19,14 @@ // whose id and app_version match the entry and whose binary matches its // pinned sha256, and its binary can run on the platform it is published // for. A legacy single-bundle entry (no `bundles` map) is installed by every -// platform, so it must not ship a native binary at all. +// platform, so it must not ship a native binary at all. A "cli" bundle's +// install.json must carry a native tool for every platform the bundle is +// published for; the adapter exits at every start where it has none. +// +// 3. Rename tombstones (not overridable), checked on every run: an entry with +// no bundle must be a tombstone (renamed_to + hidden + the old publisher +// key, no version, no metadata_url) whose renamed_to names an installable +// entry. // // Usage: // @@ -67,12 +74,15 @@ type catalogue struct { } type entry struct { - ID string `json:"id"` - Version string `json:"version"` - BundleURL string `json:"bundle_url"` - BundleSHA string `json:"bundle_sha256"` - Bundles map[string]variant `json:"bundles,omitempty"` - RenamedTo string `json:"renamed_to,omitempty"` + ID string `json:"id"` + Version string `json:"version"` + BundleURL string `json:"bundle_url"` + BundleSHA string `json:"bundle_sha256"` + Bundles map[string]variant `json:"bundles,omitempty"` + RenamedTo string `json:"renamed_to,omitempty"` + Hidden bool `json:"hidden,omitempty"` + Publisher string `json:"publisher,omitempty"` + MetadataURL string `json:"metadata_url,omitempty"` } type variant struct { @@ -125,9 +135,18 @@ type bundleInfo struct { manifest *manifest binary binaryFormat binarySHA string + assets assetPlatforms fetchError error } +// assetPlatforms describes a "cli" bundle's install.json: the native tool the +// adapter stages at first start and the os/arch it is published for. nil when +// the bundle has no install.json (or it lists no assets). +type assetPlatforms struct { + Command string + Platforms []string +} + func main() { base := flag.String("base", "", "catalogue.json at the PR base (omit or empty file: every entry is new)") head := flag.String("head", "catalogue/catalogue.json", "catalogue.json at the PR head") @@ -201,19 +220,26 @@ func (l *linter) lint(base, head *catalogue) []finding { for _, e := range base.Apps { baseByID[e.ID] = e } + headByID := map[string]entry{} + for _, e := range head.Apps { + headByID[e.ID] = e + } seen := map[string]bool{} for _, e := range head.Apps { if seen[e.ID] { out = append(out, finding{AppID: e.ID, Title: "duplicate catalogue id", Msg: fmt.Sprintf("%s appears more than once in the catalogue", e.ID)}) } seen[e.ID] = true + // Checked even when untouched: a rename target can change or go + // away in a PR that never edits the tombstone itself. + if tombstone(e) || e.RenamedTo != "" { + out = append(out, checkTombstone(e, headByID)...) + continue // not installable; no bundle to check + } old, existed := baseByID[e.ID] if existed && reflect.DeepEqual(old, e) { continue // untouched entry } - if tombstone(e) { - continue // not installable; nothing to check - } out = append(out, l.checkBundles(e)...) if existed { if reason := updateTrigger(old, e); reason != "" { @@ -227,6 +253,47 @@ func (l *linter) lint(base, head *catalogue) []finding { func tombstone(e entry) bool { return e.BundleURL == "" && len(e.Bundles) == 0 } +// checkTombstone enforces catalogue/README.md "Renaming an app". An entry with +// no bundle is only meaningful as a rename tombstone: it keeps the old id's +// publisher pin (the daemon stops an installed app whose id has no pin) and +// points pilotctl at the new id. Anything else there is a broken entry that +// pilotctl reports as a "placeholder sha256" at install time. +func checkTombstone(e entry, headByID map[string]entry) []finding { + var out []finding + fail := func(format string, args ...any) { + out = append(out, finding{AppID: e.ID, Title: "bad rename tombstone", Msg: e.ID + ": " + fmt.Sprintf(format, args...)}) + } + if e.RenamedTo == "" { + fail("the entry has no bundle_url and no bundles, so nothing can install it; publish a bundle, or make it a rename tombstone (renamed_to + hidden, see catalogue/README.md)") + return out + } + if !tombstone(e) { + fail("renamed_to is set but the entry still publishes a bundle; a tombstone must drop bundle_url and bundles") + } + if e.Version != "" { + fail("a tombstone has no version (it has no release); older pilotctl compares it against installed copies and tries to upgrade them to it") + } + if e.MetadataURL != "" { + fail("a tombstone has no metadata_url; delete apps/%s/ and the pin", e.ID) + } + if !e.Hidden { + fail("a tombstone must set \"hidden\": true") + } + if e.Publisher == "" { + fail("a tombstone must keep the old publisher key: installed copies are pinned to it, and the daemon stops an installed app whose id has no pin") + } + to, ok := headByID[e.RenamedTo] + switch { + case e.RenamedTo == e.ID: + fail("renamed_to names the entry itself") + case !ok: + fail("renamed_to %q is not in the catalogue", e.RenamedTo) + case to.RenamedTo != "" || tombstone(to): + fail("renamed_to %q is itself a tombstone; renames are one hop", e.RenamedTo) + } + return out +} + // resolved mirrors pilotctl's resolveBundle for one platform. func resolved(e entry, plat string) variant { if len(e.Bundles) == 0 { @@ -387,6 +454,19 @@ func (l *linter) checkBundles(e entry) []finding { if info.binarySHA != m.Binary.SHA256 { fail("bundle does not match entry", "%s: binary %s has sha256 %s but the manifest pins %s; pilotctl refuses to install it", where, m.Binary.Path, info.binarySHA, m.Binary.SHA256) } + if a := info.assets; len(a.Platforms) > 0 { + plats := []string{t.plat} + if t.plat == "" { + plats = knownPlatforms // a legacy bundle is installed everywhere + } + for _, p := range plats { + if !contains(a.Platforms, p) { + fail("no native tool for a published platform", + "%s: install.json ships %s only for %s, so the app exits at every start on %s (\"install assets: stage: no asset for %s\") and the supervisor suspends it. Drop %s from `bundles`, or add its asset", + where, a.Command, strings.Join(a.Platforms, ", "), p, p, p) + } + } + } bf := info.binary switch { case !bf.Native: @@ -410,70 +490,107 @@ func (l *linter) inspect(v variant) *bundleInfo { return info } info := &bundleInfo{} - info.manifest, info.binary, info.binarySHA, info.fetchError = l.readBundle(v) + info.manifest, info.binary, info.binarySHA, info.assets, info.fetchError = l.readBundle(v) l.cache[key] = info return info } -func (l *linter) readBundle(v variant) (*manifest, binaryFormat, string, error) { +func (l *linter) readBundle(v variant) (*manifest, binaryFormat, string, assetPlatforms, error) { var none binaryFormat body, err := l.fetch(v.BundleURL) if err != nil { - return nil, none, "", fmt.Errorf("fetch %s: %w", v.BundleURL, err) + return nil, none, "", assetPlatforms{}, fmt.Errorf("fetch %s: %w", v.BundleURL, err) } defer body.Close() tmp, err := os.MkdirTemp("", "catalogue-lint-*") if err != nil { - return nil, none, "", err + return nil, none, "", assetPlatforms{}, err } defer os.RemoveAll(tmp) tarPath := filepath.Join(tmp, "bundle.tar.gz") f, err := os.Create(tarPath) // #nosec G304 -- fixed name in our own temp dir if err != nil { - return nil, none, "", err + return nil, none, "", assetPlatforms{}, err } h := sha256.New() n, err := io.Copy(io.MultiWriter(f, h), io.LimitReader(body, maxBundleBytes+1)) _ = f.Close() if err != nil { - return nil, none, "", fmt.Errorf("download %s: %w", v.BundleURL, err) + return nil, none, "", assetPlatforms{}, fmt.Errorf("download %s: %w", v.BundleURL, err) } if n > maxBundleBytes { - return nil, none, "", fmt.Errorf("%s is larger than pilotctl's %d-byte download cap", v.BundleURL, maxBundleBytes) + return nil, none, "", assetPlatforms{}, fmt.Errorf("%s is larger than pilotctl's %d-byte download cap", v.BundleURL, maxBundleBytes) } if got := hex.EncodeToString(h.Sum(nil)); got != v.BundleSHA { - return nil, none, "", fmt.Errorf("%s has sha256 %s, the catalogue pins %s", v.BundleURL, got, v.BundleSHA) + return nil, none, "", assetPlatforms{}, fmt.Errorf("%s has sha256 %s, the catalogue pins %s", v.BundleURL, got, v.BundleSHA) } files, err := extract(tarPath, tmp) if err != nil { - return nil, none, "", fmt.Errorf("unpack %s: %w", v.BundleURL, err) + return nil, none, "", assetPlatforms{}, fmt.Errorf("unpack %s: %w", v.BundleURL, err) } mfPath, ok := files["manifest.json"] if !ok { - return nil, none, "", errors.New("bundle has no top-level manifest.json") + return nil, none, "", assetPlatforms{}, errors.New("bundle has no top-level manifest.json") } raw, err := os.ReadFile(mfPath) // #nosec G304 -- a file we extracted into our temp dir if err != nil { - return nil, none, "", err + return nil, none, "", assetPlatforms{}, err } var m manifest if err := json.Unmarshal(raw, &m); err != nil { - return nil, none, "", fmt.Errorf("parse manifest.json: %w", err) + return nil, none, "", assetPlatforms{}, fmt.Errorf("parse manifest.json: %w", err) } binPath, ok := files[path.Clean(m.Binary.Path)] if !ok { - return nil, none, "", fmt.Errorf("manifest binary %q is not in the bundle", m.Binary.Path) + return nil, none, "", assetPlatforms{}, fmt.Errorf("manifest binary %q is not in the bundle", m.Binary.Path) } bin, err := os.ReadFile(binPath) // #nosec G304 -- a file we extracted into our temp dir if err != nil { - return nil, none, "", err + return nil, none, "", assetPlatforms{}, err } sum := sha256.Sum256(bin) bf, err := detectBinary(binPath) if err != nil { - return nil, none, "", err + return nil, none, "", assetPlatforms{}, err + } + var assets assetPlatforms + if p, ok := files["install.json"]; ok { + if assets, err = readAssetPlatforms(p); err != nil { + return nil, none, "", assetPlatforms{}, err + } + } + return &m, bf, hex.EncodeToString(sum[:]), assets, nil +} + +// readAssetPlatforms reads a bundle's install.json. It mirrors the adapter's +// StageAssets (app-template internal/scaffold/templates/stage.go.tmpl), which +// picks the assets whose os and arch equal the host's. +func readAssetPlatforms(p string) (assetPlatforms, error) { + raw, err := os.ReadFile(p) // #nosec G304 -- a file we extracted into our temp dir + if err != nil { + return assetPlatforms{}, err + } + var spec struct { + Command string `json:"command"` + Assets []struct { + OS string `json:"os"` + Arch string `json:"arch"` + } `json:"assets"` + } + if err := json.Unmarshal(raw, &spec); err != nil { + return assetPlatforms{}, fmt.Errorf("parse install.json: %w", err) + } + out := assetPlatforms{Command: spec.Command} + for _, a := range spec.Assets { + if p := a.OS + "/" + a.Arch; !contains(out.Platforms, p) { + out.Platforms = append(out.Platforms, p) + } + } + sort.Strings(out.Platforms) + if out.Command == "" { + out.Command = "its native tool" } - return &m, bf, hex.EncodeToString(sum[:]), nil + return out, nil } // extract writes each regular file of the gzipped tar at tarPath into dir under diff --git a/catalogue/lint/main_test.go b/catalogue/lint/main_test.go index 0ff8f491..90e222eb 100644 --- a/catalogue/lint/main_test.go +++ b/catalogue/lint/main_test.go @@ -78,6 +78,13 @@ func newBundleServer(t *testing.T) *bundleServer { // returning its catalogue variant. binSHAOverride, when set, is what the // manifest pins instead of the binary's real sha. func (b *bundleServer) publish(id, version string, caps []string, bin []byte, binSHAOverride string) variant { + b.t.Helper() + return b.publishWith(id, version, caps, bin, binSHAOverride, nil) +} + +// publishWith is publish with extra top-level files in the bundle (a cli +// app's install.json). +func (b *bundleServer) publishWith(id, version string, caps []string, bin []byte, binSHAOverride string, extra map[string][]byte) variant { b.t.Helper() sum := sha256.Sum256(bin) binSHA := hex.EncodeToString(sum[:]) @@ -96,10 +103,15 @@ func (b *bundleServer) publish(id, version string, caps []string, bin []byte, bi var buf bytes.Buffer gz := gzip.NewWriter(&buf) tw := tar.NewWriter(gz) - for _, f := range []struct { + type file struct { name string body []byte - }{{"./manifest.json", mf}, {"bin/app", bin}} { + } + files := []file{{"./manifest.json", mf}, {"bin/app", bin}} + for name, body := range extra { + files = append(files, file{name, body}) + } + for _, f := range files { if err := tw.WriteHeader(&tar.Header{Name: f.name, Size: int64(len(f.body)), Typeflag: tar.TypeReg, Mode: 0o755}); err != nil { b.t.Fatal(err) } @@ -357,3 +369,60 @@ func TestLoadCatalogueOptionalBase(t *testing.T) { t.Fatalf("empty base file: %v %v", c, err) } } + +// A bundle-less entry must be a well-formed rename tombstone, checked even +// when the PR does not touch it. +func TestTombstonesMustBeWellFormed(t *testing.T) { + s := newBundleServer(t) + target := legacy("io.test.new", "1.0.0", s.publish("io.test.new", "1.0.0", nil, scriptBin, "")) + good := entry{ID: "io.test.old", RenamedTo: "io.test.new", Hidden: true, Publisher: "ed25519:old"} + if got := testLinter(policy{}, false).lint(&catalogue{}, &catalogue{Apps: []entry{good, target}}); len(got) != 0 { + t.Fatalf("valid tombstone: %+v", got) + } + for name, tc := range map[string]struct { + apps []entry + want string + }{ + "no bundle, no rename": {[]entry{{ID: "io.test.old", Publisher: "ed25519:old"}}, "nothing can install it"}, + "target missing": {[]entry{good}, `renamed_to "io.test.new" is not in the catalogue`}, + "chained": {[]entry{good, {ID: "io.test.new", RenamedTo: "io.test.newer", Hidden: true, Publisher: "ed25519:x"}}, "renames are one hop"}, + "self": {[]entry{{ID: "io.test.old", RenamedTo: "io.test.old", Hidden: true, Publisher: "ed25519:old"}}, "names the entry itself"}, + "version": {[]entry{{ID: "io.test.old", RenamedTo: "io.test.new", Hidden: true, Publisher: "ed25519:old", Version: "1.2.0"}, target}, "has no version"}, + "metadata": {[]entry{{ID: "io.test.old", RenamedTo: "io.test.new", Hidden: true, Publisher: "ed25519:old", MetadataURL: "https://x/m.json"}, target}, "no metadata_url"}, + "not hidden": {[]entry{{ID: "io.test.old", RenamedTo: "io.test.new", Publisher: "ed25519:old"}, target}, `"hidden": true`}, + "no publisher": {[]entry{{ID: "io.test.old", RenamedTo: "io.test.new", Hidden: true}, target}, "keep the old publisher key"}, + "still has a bundle": {[]entry{{ID: "io.test.old", RenamedTo: "io.test.new", Hidden: true, Publisher: "ed25519:old", BundleURL: target.BundleURL, BundleSHA: target.BundleSHA}, target}, "still publishes a bundle"}, + } { + head := &catalogue{Apps: tc.apps} + // base == head: the tombstone itself is untouched and still checked. + if got := testLinter(policy{}, false).lint(head, head); !hasFinding(got, false, "bad rename tombstone", tc.want) { + t.Errorf("%s: want a finding containing %q, got %+v", name, tc.want, got) + } + } +} + +// A cli bundle published for a platform its install.json has no native tool +// for exits at every start there. io.pilot.smolmachines 1.2.0 shipped a +// darwin/amd64 bundle like that (upstream smolvm has no macOS x86_64 build). +func TestCLIBundleNeedsANativeToolForEachPublishedPlatform(t *testing.T) { + s := newBundleServer(t) + const id = "io.test.vm" + installJSON := []byte(`{"schema":1,"app":"io.test.vm","version":"1.0.0","command":"smolvm","assets":[ + {"os":"darwin","arch":"arm64"},{"os":"linux","arch":"amd64"},{"os":"linux","arch":"arm64"}]}`) + extra := map[string][]byte{"install.json": installJSON} + e := entry{ID: id, Version: "1.0.0", Bundles: map[string]variant{ + "darwin/arm64": s.publishWith(id, "1.0.0", nil, machArm64, "", extra), + "darwin/amd64": s.publishWith(id, "1.0.0", nil, machHeader(macho.CpuAmd64), "", extra), + "linux/amd64": s.publishWith(id, "1.0.0", nil, elfAmd64, "", extra), + "linux/arm64": s.publishWith(id, "1.0.0", nil, elfArm64, "", extra), + }} + e.BundleURL, e.BundleSHA = e.Bundles["linux/amd64"].BundleURL, e.Bundles["linux/amd64"].BundleSHA + got := testLinter(policy{}, false).lint(&catalogue{}, &catalogue{Apps: []entry{e}}) + if len(errorsOf(got)) != 1 || !hasFinding(got, false, "no native tool", "darwin/amd64: install.json ships smolvm only for darwin/arm64, linux/amd64, linux/arm64") { + t.Fatalf("got %+v", got) + } + delete(e.Bundles, "darwin/amd64") + if got := testLinter(policy{}, false).lint(&catalogue{}, &catalogue{Apps: []entry{e}}); len(got) != 0 { + t.Fatalf("every published platform has its tool: %+v", got) + } +} From 9d6163c59a4e3ae1ff09aa5b5a4186aa12e82062 Mon Sep 17 00:00:00 2001 From: Teodor Calin Date: Thu, 24 Sep 2026 12:16:52 +0300 Subject: [PATCH 5/6] catalogue/README: renames, uninstall cleanup, native-tool install check Document what pilotctl now does with a rename tombstone (listing, install, --version, outdated, upgrade, view, call), how to move a node to the new id, that the lint checks tombstones, and to keep the old id's R2 prefix while the new id's install.json still downloads from it (io.pilot.smol 1.2.0 stages smolvm from io.pilot.smolmachines/1.2.0/). Add uninstall's process stop and install's native-tool platform check to the install/upgrade notes. Co-Authored-By: Claude Opus 5.5 (1M context) --- catalogue/README.md | 43 +++++++++++++++++++++++++++++++++++++------ 1 file changed, 37 insertions(+), 6 deletions(-) diff --git a/catalogue/README.md b/catalogue/README.md index c2a7baf2..19a56203 100644 --- a/catalogue/README.md +++ b/catalogue/README.md @@ -80,12 +80,33 @@ and `publisher` (so existing installs keep their pin and keep running), set `"renamed_to": ""`, set `"hidden": true`, drop `bundle_url` / `bundles` / `metadata_url` (the tombstone is not installable), and delete the old `apps//` detail dir. The full new entry lives under the new id, and the -catalogue is re-signed. A bundles-aware `pilotctl` then omits the old id from the -listing and, on `install`/`view`/`call`, prints a deprecation warning and routes -to `renamed_to`. `hidden` alone (without `renamed_to`) just omits an entry from -the listing while keeping it resolvable. Older clients ignore both fields. One -hop only — a `renamed_to` that points at another tombstone is a bug and is not -chased. +catalogue is re-signed. A `pilotctl` that knows the fields (this repo's, from +the release after v1.13.10) then: + +- omits the old id from `catalogue` (text and `--json`); +- on `install `, warns and installs `renamed_to` instead. With + `--version` (a pin, as the managed-fleet reconcile passes) it refuses, naming + the new id: a pin names a release of one app, and the old id has none; +- reports an installed old id in `outdated` as `renamed` (AVAILABLE is the new + id). `upgrade --all` skips it, since the hourly updater runs it and the new id + has its own publisher key and method names; `upgrade ` exits 1 with the + install-then-uninstall steps; +- on `view ` warns, and on `call` of an old id that is not installed says + it was renamed. + +Moving a node over is `pilotctl appstore install ` then `pilotctl appstore +uninstall --yes`; uninstall also stops anything still running from the +old app's files (see "What install and upgrade do with app state" below). +Older clients ignore both fields: they list the tombstone and fail its install +with "placeholder sha256". `hidden` alone (without `renamed_to`) just omits an +entry from the listing while keeping it resolvable. One hop only — a +`renamed_to` that points at another tombstone is a bug and is not chased. +`catalogue/lint` checks every tombstone's shape on every PR. + +Keep the tombstone while any install of the old id may exist, and keep any +native-tool assets the new id's `install.json` still downloads from the old +id's R2 prefix (io.pilot.smol 1.2.0 stages smolvm from +`io.pilot.smolmachines/1.2.0/`). ## Detail schema (`apps//metadata.json`) @@ -310,6 +331,16 @@ git show origin/main:catalogue/catalogue.json > /tmp/base.json app empty. It warns loudly and still keeps the backup. - `upgrade --all` goes on to the next app when one fails, and exits 1 at the end naming the apps that were not upgraded. +- `uninstall` first stops every process whose executable lives in the app's + dir or in one of its backups (SIGTERM, then SIGKILL after 5 s), then deletes + the dir, then stops anything the supervisor respawned in between. That is + the app itself, an instance orphaned by a daemon that died hard, and what the + app started that detached from it: a smolvm microVM, a daemonized + redis/postgres/mysql server. Nothing could manage those once the dir is + gone. The output (and `--json` `stopped_processes`) names them. +- `install` refuses a bundle whose `install.json` lists native tools but none + for this host's os/arch: the adapter would exit at every start and the + supervisor would suspend it. ## Catalogue signing key From bb34e0db0918d560e9be36e268b59f63e566b265 Mon Sep 17 00:00:00 2001 From: Teodor Calin Date: Thu, 24 Sep 2026 12:40:09 +0300 Subject: [PATCH 6/6] pilotctl: annotate the two new gosec G703 findings Code scanning flagged the stat of / in `appstore call` (only reached when app.sock is missing, and only stat'ed) and the read of the bundle's install.json in the install platform check. Neither writes, and both use the same paths the surrounding code already touches. Co-Authored-By: Claude Opus 5.5 (1M context) --- cmd/pilotctl/appstore.go | 2 +- cmd/pilotctl/appstore_platform.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/pilotctl/appstore.go b/cmd/pilotctl/appstore.go index 175fac20..19ad3f61 100644 --- a/cmd/pilotctl/appstore.go +++ b/cmd/pilotctl/appstore.go @@ -2658,7 +2658,7 @@ func cmdAppStoreCall(args []string) { // instead of blaming the daemon. No silent retarget: the method // namespace changed too. (An installed app whose daemon is down skips // the catalogue fetch.) - if _, derr := os.Stat(filepath.Dir(sockPath)); errors.Is(derr, os.ErrNotExist) { + if _, derr := os.Stat(filepath.Dir(sockPath)); errors.Is(derr, os.ErrNotExist) { // #nosec G703 -- only a stat of the dir whose app.sock was stat'ed just above; nothing is read or written if c, lerr := loadCatalogue(); lerr == nil { if e := c.findEntry(appID); e != nil && e.RenamedTo != "" { fatalHint("invalid_argument", diff --git a/cmd/pilotctl/appstore_platform.go b/cmd/pilotctl/appstore_platform.go index b38f33f9..21210f54 100644 --- a/cmd/pilotctl/appstore_platform.go +++ b/cmd/pilotctl/appstore_platform.go @@ -109,7 +109,7 @@ func peMachineArch(machine uint16) string { // build). An install.json that does not parse, or lists no assets, is left to // the app, as before. func checkBundleAssetsPlatform(bundleDir, goos, goarch string) error { - raw, err := os.ReadFile(filepath.Join(bundleDir, "install.json")) // #nosec G304 -- a file of the bundle being installed + raw, err := os.ReadFile(filepath.Join(bundleDir, "install.json")) // #nosec G304 G703 -- install.json of the bundle being installed (the dir pilotctl unpacked, or the --local dir the operator named); only read and parsed if errors.Is(err, fs.ErrNotExist) { return nil }