pilotctl: uninstall keeps the app's state in the backups instead of deleting it

CHANGELOG.md

@@ -394,6 +394,14 @@ Detailed per-release notes are on the
   `appstore upgrade` does not wait.
 
 ### Fixed
+- **`pilotctl appstore uninstall` no longer deletes an app's keys.** It
+  deleted the app's dir, and with it the app's state: the wallet's EVM private
+  key (and the funds at its address), smol's secrets, each metered app's
+  identity. The dir now goes to the app backups (`~/.pilot/app-backups/<id>/`),
+  without the binary and pinned so retention never removes it, and the
+  command says where. The wallet (0.4.0) restores its key from there, or from
+  its own copy in `~/.pilot/keys/io.pilot.wallet`, when it is installed
+  again. If the dir cannot be moved, nothing is deleted.
 - **A write followed at once by a close no longer loses its end under
   loss.** A FIN that overtook a lost segment was acted on at once: the
   receiver closed the stream with the segment missing, and its FIN-ACK made

cmd/pilotctl/appstore.go

@@ -117,7 +117,7 @@ Usage:
                                              event names: supervise-start, supervise-stop,
                                                           spawn, exit, suspend, verify-fail, spawn-fail
                                              duration: Go syntax (e.g. 10m, 1h, 24h)
-  pilotctl appstore uninstall <id> --yes     remove an installed app from the install root
+  pilotctl appstore uninstall <id> --yes     stop and remove an installed app; its state (keys, data) is kept in the app backups
   pilotctl appstore verify <bundle-dir>      sha256-check a pre-install bundle against its manifest
   pilotctl appstore catalogue                list apps available for one-command install
   pilotctl appstore install <app-id> [--version <v>] [--force [--reset-state]] [--no-wait | --wait <dur>]
@@ -750,8 +750,9 @@ func shortHex(h string) string {
 
 // ── uninstall ──────────────────────────────────────────────────────────
 
-// cmdAppStoreUninstall removes an installed app's directory tree from the
-// install root. Destructive — requires --yes to confirm. Does NOT
+// cmdAppStoreUninstall stops an installed app and moves its directory out of
+// the install root into the app backups, keeping its state (keys, data);
+// nothing is deleted. Requires --yes to confirm. Does NOT
 // coordinate with the daemon's supervisor, so until the daemon is
 // restarted the supervisor will keep failing the binary sha256 check on
 // every respawn (visible in the audit log as "verify-fail" lines, capped
@@ -776,7 +777,7 @@ func cmdAppStoreUninstall(args []string) {
 	}
 	if !confirmed {
 		fatalHint("invalid_argument",
-			"uninstall is destructive — pass --yes to confirm",
+			"uninstall stops the app and removes it; its state (keys, data) is kept in the app backups — pass --yes to confirm",
 			"refusing to uninstall %q without --yes", appID)
 	}
 
@@ -816,12 +817,13 @@ func cmdAppStoreUninstall(args []string) {
 	// binary sha256 + app_version into the uninstall audit record.
 	// The per-app supervisor.log is going away with the dir; the
 	// pilotctl-audit log at the install-root level is what survives.
-	var snapSHA, snapVer string
+	var snapSHA, snapVer, snapBin string
 	mfPath := filepath.Join(dir, "manifest.json")
 	if raw, err := os.ReadFile(mfPath); err == nil {
 		if m, perr := manifest.Parse(raw); perr == nil {
 			snapSHA = m.Binary.SHA256
 			snapVer = m.AppVersion
+			snapBin = m.Binary.Path
 		}
 	}
 
@@ -849,30 +851,30 @@ func cmdAppStoreUninstall(args []string) {
 	}
 	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
-	// writes themselves happen at the 30s verify-fail cadence, so a
-	// handful of short retries is usually enough).
-	const (
-		removeRetries = 8
-		retryDelay    = 250 * time.Millisecond
-	)
-	var rmErr error
-	for i := 0; i < removeRetries; i++ {
-		if err := removeAllForce(dir); err == nil { // read-only dirs in $APP (a Go module cache) included
-			rmErr = nil
-			break
-		} else {
-			rmErr = err
-			time.Sleep(retryDelay)
-		}
-	}
-	if rmErr != nil {
+	// The app's dir holds its state: the wallet's EVM private key, smol's
+	// secrets, each metered app's identity. Deleting it deleted those with
+	// no way back. It is moved to the app backups instead, like a replaced
+	// install (without the binary), pinned so retention never removes it;
+	// the wallet restores its key from there when it is installed again.
+	// If it cannot be moved, nothing is deleted: the dir stays, with its
+	// manifest gone so the daemon does not run it.
+	backupDir, bkErr := retireAppDir(dir, appID, appBackupMeta{
+		Kind:        backupKindUninstall,
+		FromVersion: snapVer,
+		Reason:      "uninstalled by " + currentActor(),
+		oldBinary:   snapBin,
+	})
+	if backupDir == "" || filepath.Clean(backupDir) == filepath.Clean(dir) {
 		fatalHint("io_error",
-			"manifest removed but the dir is racing the supervisor; rerun `pilotctl appstore uninstall --yes` after the daemon's next rescan settles (~30s)",
-			"remove %s: %v", dir, rmErr)
+			fmt.Sprintf("nothing was deleted: %s is still there, without its manifest, so the daemon does not run it. Fix the backup location (PILOT_APPSTORE_BACKUP_ROOT or %s) and rerun `pilotctl appstore uninstall %s --yes`", dir, appStoreBackupRoot(), appID),
+			"could not move %s to the app backups: %v", dir, bkErr)
+	}
+	if bkErr != nil {
+		fmt.Fprintf(os.Stderr, "warning: %v\n", bkErr)
 	}
+	// A process still running from the app's files now runs from the
+	// backup (on Linux its executable path follows the move): look there too.
+	procRoots = append(procRoots, appDirRoots(backupDir)...)
 
 	// 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
@@ -889,7 +891,7 @@ func cmdAppStoreUninstall(args []string) {
 	// 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)
+	reason := fmt.Sprintf("actor=%s removed=%s backup=%s", currentActor(), dir, backupDir)
 	if len(stopped) > 0 {
 		reason += " stopped=" + describeAppDirProcesses(stopped)
 	}
@@ -911,6 +913,7 @@ func cmdAppStoreUninstall(args []string) {
 		out := map[string]any{
 			"id":            appID,
 			"removed":       dir,
+			"backup":        backupDir,
 			"daemon_notice": "supervisor's next rescan (≤30s) will cancel the per-app goroutine; manifest already removed",
 		}
 		if len(backups) > 0 {
@@ -926,6 +929,7 @@ func cmdAppStoreUninstall(args []string) {
 		return
 	}
 	fmt.Printf("removed %s\n", dir)
+	fmt.Printf("its state (keys, data) is kept in %s; delete it by hand once you no longer need it (the wallet, from 0.4.0, restores its keys from there when installed again)\n", backupDir)
 	if len(stopped) > 0 {
 		fmt.Printf("stopped %d process(es) still running from the app's files: %s\n", len(stopped), describeAppDirProcesses(stopped))
 	}

cmd/pilotctl/appstore_state.go

@@ -111,6 +111,7 @@ const (
 	backupKindResetState = "reset-state" // --reset-state: the only copy of the dropped state
 	backupKindIncomplete = "incomplete"  // holds state that could not be carried into the new install
 	backupKindRecovered  = "recovered"   // a leftover found by crash recovery; what it holds is unknown
+	backupKindUninstall  = "uninstall"   // removed by uninstall: the only copy of its state (the wallet's key)
 )
 
 // backupKindRotated reports the kinds retention may prune. Anything else,
@@ -880,6 +881,11 @@ type appBackupMeta struct {
 	// backup but could not be carried into the install that replaced it.
 	NotCarried []string `json:"not_carried,omitempty"`
 	CreatedAt  string   `json:"created_at"`
+
+	// oldBinary names the install's binary (relative to the app dir) when
+	// its manifest is already gone, as after uninstall removed it: the
+	// backup keeps state, not the binary.
+	oldBinary string
 }
 
 // replacedInstallBackupKind classifies the backup of a replaced install.
@@ -917,7 +923,7 @@ var unsafeBackupNameChars = regexp.MustCompile(`[^0-9A-Za-z._+-]`)
 // warning (the backup is not where it was configured to go); with an empty
 // path, the dir could not be retired at all.
 func retireAppDir(previousDir, appID string, meta appBackupMeta) (string, error) {
-	oldBinary := ""
+	oldBinary := meta.oldBinary
 	if m, _, err := readInstalledManifest(previousDir); err == nil {
 		oldBinary = m.Binary.Path
 		if meta.FromVersion == "" {
@@ -959,7 +965,7 @@ func retireAppDir(previousDir, appID string, meta appBackupMeta) (string, error)
 		return dst, nil
 	}
 
-	parked := previousDir + "-" + stamp
+	parked := parkedDirName(previousDir, appID, stamp)
 	if err := os.Rename(previousDir, parked); err != nil {
 		parked = previousDir
 	}
@@ -975,6 +981,14 @@ func retireAppDir(previousDir, appID string, meta appBackupMeta) (string, error)
 		appID, strings.Join(failures, "; "), parked)
 }
 
+// parkedDirName is where a dir that could not be moved to any backup location
+// is left: beside it, as <appID>.previous-<stamp>, the name backup listings and
+// the wallet's key restore look for — whether it was <appID>.previous (an
+// upgrade) or <appID> itself (an uninstall).
+func parkedDirName(dir, appID, stamp string) string {
+	return filepath.Join(filepath.Dir(dir), appID+".previous-"+stamp)
+}
+
 // moveIntoBackupLocation moves previousDir to <loc>/<appID>/<name>. Across
 // filesystems it copies (without the old binary), then removes the original;
 // if only that removal fails, the copy is returned with a warning and the
@@ -1009,7 +1023,7 @@ func moveIntoBackupLocation(previousDir, loc, appID, name, oldBinary string) (st
 	// removal that fails halfway can never leave an adoptable dir behind.
 	_ = disableManifest(previousDir)
 	if err := removeAllForce(previousDir); err != nil {
-		rest := previousDir + "-" + time.Now().UTC().Format("20060102T150405.000000000Z")
+		rest := parkedDirName(previousDir, appID, time.Now().UTC().Format("20060102T150405.000000000Z"))
 		if rerr := os.Rename(previousDir, rest); rerr != nil {
 			rest = previousDir
 		}

cmd/pilotctl/main.go

@@ -2635,7 +2635,7 @@ func contextCatalog() map[string]interface{} {
 				"audit":          map[string]interface{}{"args": []string{"<id>", "[--tail <n>]", "[--event <name>]", "[--since <dur>]"}, "description": "Show the supervisor lifecycle log (spawn/exit/suspend/verify-fail)"},
 				"actions":        map[string]interface{}{"args": []string{"[--tail <n>]", "[--event <name>]"}, "description": "Show the pilotctl-side install/uninstall action log (survives app removal)"},
 				"restart":        map[string]interface{}{"args": []string{"<id>"}, "description": "Clear crash-loop suspension and respawn the app"},
-				"uninstall":      map[string]interface{}{"args": []string{"<id>", "--yes"}, "description": "Remove an installed app from the install root"},
+				"uninstall":      map[string]interface{}{"args": []string{"<id>", "--yes"}, "description": "Stop and remove an installed app; its state (keys, data) is kept in the app backups"},
 				"verify":         map[string]interface{}{"args": []string{"<bundle-dir>"}, "description": "sha256-check a pre-install bundle against its manifest"},
 				"gen-key":        map[string]interface{}{"args": []string{"<key-file>"}, "description": "Generate a fresh ed25519 publisher keypair (publisher tooling)"},
 				"sign":           map[string]interface{}{"args": []string{"--key <key-file>", "<manifest>"}, "description": "Sign (or re-sign) a manifest's store.signature (publisher tooling)"},

cmd/pilotctl/zz_uninstall_keeps_state_test.go

@@ -0,0 +1,163 @@
+// SPDX-License-Identifier: AGPL-3.0-or-later
+
+package main
+
+import (
+	"encoding/json"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+)
+
+// Uninstall deleted the app's dir, and with it the app's state: for the
+// wallet, its EVM private key, with the funds at its address out of reach.
+// The dir now goes to the app backups (pinned, without the binary), where the
+// wallet finds its key again when it is installed again; nothing is deleted.
+func TestUninstallKeepsTheAppsStateInABackup(t *testing.T) {
+	home := t.TempDir()
+	root := filepath.Join(home, ".pilot", "apps")
+	t.Setenv("PILOT_APPSTORE_ROOT", root)
+	t.Setenv("PILOT_APPSTORE_BACKUP_ROOT", "")
+	appID := "io.pilot.wallet"
+	appDir := filepath.Join(root, appID)
+	if err := os.MkdirAll(filepath.Join(appDir, "bin"), 0o755); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.WriteFile(filepath.Join(appDir, "manifest.json"), minimalManifestJSON(appID, nil), 0o600); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.WriteFile(filepath.Join(appDir, "bin", "app"), []byte("binary"), 0o755); err != nil {
+		t.Fatal(err)
+	}
+	key := []byte(`{"secp256k1_seed":"the funds"}`)
+	if err := os.WriteFile(filepath.Join(appDir, "identity-evm.json"), key, 0o600); err != nil {
+		t.Fatal(err)
+	}
+
+	prev := jsonOutput
+	defer func() { jsonOutput = prev }()
+	jsonOutput = true
+	out := captureStdout(t, func() { cmdAppStoreUninstall([]string{appID, "--yes"}) })
+	var resp map[string]any
+	if err := json.Unmarshal([]byte(out), &resp); err != nil {
+		t.Fatalf("parse: %v\n%s", err, out)
+	}
+	if _, err := os.Stat(appDir); !os.IsNotExist(err) {
+		t.Fatalf("app dir still in the install root: %v", err)
+	}
+	backup, _ := resp["backup"].(string)
+	wantParent := filepath.Join(home, ".pilot", "app-backups", appID)
+	if !strings.HasPrefix(backup, wantParent+string(filepath.Separator)) {
+		t.Fatalf("backup = %q, want it under %s", backup, wantParent)
+	}
+	got, err := os.ReadFile(filepath.Join(backup, "identity-evm.json"))
+	if err != nil || string(got) != string(key) {
+		t.Fatalf("key in the backup = %q, %v; want it byte for byte", got, err)
+	}
+	if _, err := os.Stat(filepath.Join(backup, "bin", "app")); !os.IsNotExist(err) {
+		t.Errorf("the binary was kept in the backup: %v", err)
+	}
+	var meta appBackupMeta
+	raw, err := os.ReadFile(filepath.Join(backup, appBackupMetaName))
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := json.Unmarshal(raw, &meta); err != nil {
+		t.Fatal(err)
+	}
+	if meta.Kind != backupKindUninstall || !meta.Pinned || meta.FromVersion != "1.0.0" {
+		t.Errorf("backup meta = %+v, want kind %q, pinned, from 1.0.0", meta, backupKindUninstall)
+	}
+}
+
+// When no backup location takes the dir, it is still not deleted: it is kept
+// in the install root (renamed, manifest disabled), or the uninstall stops
+// with the dir in place. Either way the key survives.
+func TestUninstallNeverDeletesTheKeyWhenBackupsFail(t *testing.T) {
+	home := t.TempDir()
+	root := filepath.Join(home, ".pilot", "apps")
+	t.Setenv("PILOT_APPSTORE_ROOT", root)
+	t.Setenv("PILOT_APPSTORE_BACKUP_ROOT", "")
+	appID := "io.pilot.wallet"
+	appDir := filepath.Join(root, appID)
+	if err := os.MkdirAll(appDir, 0o755); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.WriteFile(filepath.Join(appDir, "manifest.json"), minimalManifestJSON(appID, nil), 0o600); err != nil {
+		t.Fatal(err)
+	}
+	key := []byte("the key")
+	if err := os.WriteFile(filepath.Join(appDir, "identity-evm.json"), key, 0o600); err != nil {
+		t.Fatal(err)
+	}
+	prevRename := renameForBackup
+	renameForBackup = func(string, string) error { return os.ErrPermission }
+	t.Cleanup(func() { renameForBackup = prevRename })
+
+	prev := jsonOutput
+	defer func() { jsonOutput = prev }()
+	jsonOutput = true
+	runTrapped(t, func() { cmdAppStoreUninstall([]string{appID, "--yes"}) })
+
+	var found []string
+	_ = filepath.Walk(home, func(p string, fi os.FileInfo, err error) error {
+		if err == nil && fi.Mode().IsRegular() && fi.Name() == "identity-evm.json" {
+			if b, _ := os.ReadFile(p); string(b) == string(key) {
+				found = append(found, p)
+			}
+		}
+		return nil
+	})
+	if len(found) == 0 {
+		t.Fatal("the key is gone after an uninstall whose backup failed")
+	}
+	// Left beside the install, it is named as backup listings and the
+	// wallet's key restore look for it.
+	parked, _ := filepath.Glob(filepath.Join(root, appID+".previous-*", "identity-evm.json"))
+	if len(parked) != 1 {
+		t.Fatalf("the key is at %v, want it in %s.previous-<stamp>", found, appID)
+	}
+}
+
+// When the dir cannot even be renamed in place, uninstall fails and says
+// nothing was deleted; it used to report success with the dir still there.
+func TestUninstallFailsWhenTheDirCannotBeMoved(t *testing.T) {
+	if os.Getuid() == 0 {
+		t.Skip("root renames in read-only dirs")
+	}
+	home := t.TempDir()
+	root := filepath.Join(home, ".pilot", "apps")
+	t.Setenv("PILOT_APPSTORE_ROOT", root)
+	t.Setenv("PILOT_APPSTORE_BACKUP_ROOT", "")
+	appID := "io.pilot.wallet"
+	appDir := filepath.Join(root, appID)
+	if err := os.MkdirAll(appDir, 0o755); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.WriteFile(filepath.Join(appDir, "manifest.json"), minimalManifestJSON(appID, nil), 0o600); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.WriteFile(filepath.Join(appDir, "identity-evm.json"), []byte("the key"), 0o600); err != nil {
+		t.Fatal(err)
+	}
+	// The install lock lives in the root; it exists already on a real node.
+	if err := os.WriteFile(filepath.Join(root, "."+appID+".lock"), nil, 0o600); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.Chmod(root, 0o555); err != nil { // no rename out of, or within, the install root
+		t.Fatal(err)
+	}
+	t.Cleanup(func() { _ = os.Chmod(root, 0o755) })
+
+	prev := jsonOutput
+	defer func() { jsonOutput = prev }()
+	jsonOutput = true
+	_, stderr, f := runTrapped(t, func() { cmdAppStoreUninstall([]string{appID, "--yes"}) })
+	if f == nil || f.Code != "io_error" || !strings.Contains(stderr+f.Message, "nothing was deleted") {
+		t.Fatalf("uninstall = %+v (stderr %q); want it to fail, saying nothing was deleted", f, stderr)
+	}
+	if b, err := os.ReadFile(filepath.Join(appDir, "identity-evm.json")); err != nil || string(b) != "the key" {
+		t.Fatalf("key after the failed uninstall = %q, %v", b, err)
+	}
+}