diff --git a/CHANGELOG.md b/CHANGELOG.md index 8468314a..b0835dca 100644 --- a/CHANGELOG.md +++ b/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//`), + 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 diff --git a/cmd/pilotctl/appstore.go b/cmd/pilotctl/appstore.go index 5531648b..e03afd27 100644 --- a/cmd/pilotctl/appstore.go +++ b/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 --yes remove an installed app from the install root + pilotctl appstore uninstall --yes stop and remove an installed app; its state (keys, data) is kept in the app backups pilotctl appstore verify sha256-check a pre-install bundle against its manifest pilotctl appstore catalogue list apps available for one-command install pilotctl appstore install [--version ] [--force [--reset-state]] [--no-wait | --wait ] @@ -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)) } diff --git a/cmd/pilotctl/appstore_state.go b/cmd/pilotctl/appstore_state.go index f36cf81b..6e9e4dd2 100644 --- a/cmd/pilotctl/appstore_state.go +++ b/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 .previous-, the name backup listings and +// the wallet's key restore look for — whether it was .previous (an +// upgrade) or itself (an uninstall). +func parkedDirName(dir, appID, stamp string) string { + return filepath.Join(filepath.Dir(dir), appID+".previous-"+stamp) +} + // moveIntoBackupLocation moves previousDir to //. 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 } diff --git a/cmd/pilotctl/main.go b/cmd/pilotctl/main.go index bbc60188..24e6b4ac 100644 --- a/cmd/pilotctl/main.go +++ b/cmd/pilotctl/main.go @@ -2635,7 +2635,7 @@ func contextCatalog() map[string]interface{} { "audit": map[string]interface{}{"args": []string{"", "[--tail ]", "[--event ]", "[--since ]"}, "description": "Show the supervisor lifecycle log (spawn/exit/suspend/verify-fail)"}, "actions": map[string]interface{}{"args": []string{"[--tail ]", "[--event ]"}, "description": "Show the pilotctl-side install/uninstall action log (survives app removal)"}, "restart": map[string]interface{}{"args": []string{""}, "description": "Clear crash-loop suspension and respawn the app"}, - "uninstall": map[string]interface{}{"args": []string{"", "--yes"}, "description": "Remove an installed app from the install root"}, + "uninstall": map[string]interface{}{"args": []string{"", "--yes"}, "description": "Stop and remove an installed app; its state (keys, data) is kept in the app backups"}, "verify": map[string]interface{}{"args": []string{""}, "description": "sha256-check a pre-install bundle against its manifest"}, "gen-key": map[string]interface{}{"args": []string{""}, "description": "Generate a fresh ed25519 publisher keypair (publisher tooling)"}, "sign": map[string]interface{}{"args": []string{"--key ", ""}, "description": "Sign (or re-sign) a manifest's store.signature (publisher tooling)"}, diff --git a/cmd/pilotctl/zz_uninstall_keeps_state_test.go b/cmd/pilotctl/zz_uninstall_keeps_state_test.go new file mode 100644 index 00000000..7d7f0d24 --- /dev/null +++ b/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-", 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) + } +}