diff --git a/modules/restic/cmd/restic-backups/backups.go b/modules/restic/cmd/restic-backups/backups.go index 89b72b8a..c365010e 100644 --- a/modules/restic/cmd/restic-backups/backups.go +++ b/modules/restic/cmd/restic-backups/backups.go @@ -29,6 +29,7 @@ import ( "path/filepath" "regexp" "slices" + "strconv" "strings" "sync" "time" @@ -432,7 +433,9 @@ type Restored struct { } // Restore puts a module's data from a restore point BESIDE the live data: each directory as -// .restored-. A target that already exists is refused, never overwritten. +// .restored- — or, where a directory on the way belongs to another account, in this module's +// state as restored/// (besideIsUnsafe). A target that already exists is refused, never +// overwritten. func (b *Backups) Restore(ctx context.Context, module, snapshot, path string) (*Restored, error) { b.mu.Lock() defer b.mu.Unlock() @@ -468,6 +471,17 @@ func (b *Backups) Restore(ctx context.Context, module, snapshot, path string) (* r := &Restored{Module: module, From: chosen, Live: "untouched — swapping it in is a person's act"} for _, p := range paths { target := p + ".restored-" + stamp + if why := b.besideIsUnsafe(ctx, target); why != "" { + // Root writes the restore. Beside a path another account controls, that account could put a + // symbolic link where root is about to write and have root write wherever it points (novox/hq ADR + // 0266: the agent's own home is such a place). Kept in this module's state instead, which only + // root and the runtime's account reach. + target = filepath.Join(b.Where.State, "restored", stamp, module) + p + if _, err := b.Run(ctx, "mkdir", "-p", "-m", "0700", filepath.Dir(target)); err != nil { + return nil, fmt.Errorf("%s: %s, and the place in this module's state could not be made: %w", p, why, err) + } + b.Say("%s: %s; restoring into this module's state instead", p, why) + } if b.exists(ctx, target) { return nil, fmt.Errorf("%s already exists; nothing is restored over anything", target) } @@ -480,6 +494,35 @@ func (b *Backups) Restore(ctx context.Context, module, snapshot, path string) (* return r, nil } +// besideIsUnsafe says why root must not restore at target, beside the live data: a directory on the way to it +// is a symbolic link, or belongs to an account other than root and the one this process runs as — one that +// could swap a link in while root writes. Empty when every directory on the way is root's or the runtime's. +func (b *Backups) besideIsUnsafe(ctx context.Context, target string) string { + me := strconv.Itoa(os.Getuid()) + dir := filepath.Dir(filepath.Clean(target)) + var chain []string + for d := dir; ; d = filepath.Dir(d) { + chain = append(chain, d) + if d == "/" || d == "." { + break + } + } + for i := len(chain) - 1; i >= 0; i-- { + out, err := b.Run(ctx, "stat", "-c", "%u %F", chain[i]) + if err != nil { + return chain[i] + " could not be read" + } + owner, kind, _ := strings.Cut(strings.TrimSpace(out), " ") + if kind == "symbolic link" { + return chain[i] + " is a symbolic link" + } + if owner != "0" && owner != me { + return chain[i] + " belongs to another account" + } + } + return "" +} + // restoreOne puts one kept path at target. A directory is restored as the snapshot's subfolder, so // its contents land directly in target; a single file cannot be — restic restores a subfolder, not a // file (a settings file refused with "not a directory" on the first restore of an app, 2026-10-05) — diff --git a/modules/restic/cmd/restic-backups/backups_test.go b/modules/restic/cmd/restic-backups/backups_test.go index 34b75b26..1d9e936f 100644 --- a/modules/restic/cmd/restic-backups/backups_test.go +++ b/modules/restic/cmd/restic-backups/backups_test.go @@ -284,3 +284,38 @@ func TestWhetherADirectoryIsThereIsAskedAsRoot(t *testing.T) { t.Errorf("asked %v", asked) } } + +// Beside a directory another account owns, root does not restore: a link swapped in there would point root's +// write anywhere (novox/hq ADR 0266). The restore goes to the module's state instead. +func TestARestoreBesideAnotherAccountsDirectoryGoesToTheState(t *testing.T) { + state := t.TempDir() + var ran []string + run := func(ctx context.Context, name string, args ...string) (string, error) { + ran = append(ran, name+" "+strings.Join(args, " ")) + if name == "stat" { + if args[len(args)-1] == "/home/agent" { + return "1001 directory\n", nil + } + return "0 directory\n", nil + } + return "", nil + } + b := &Backups{Where: Where{State: state}, Run: run, Now: time.Now, Say: func(string, ...any) {}} + if why := b.besideIsUnsafe(context.Background(), "/home/agent/.claude.restored-x"); !strings.Contains(why, "another account") { + t.Fatalf("an agent's home is unsafe beside: %q", why) + } + if why := b.besideIsUnsafe(context.Background(), "/srv/data.restored-x"); why != "" { + t.Fatalf("root's directories are safe beside: %q", why) + } + run2 := func(ctx context.Context, name string, args ...string) (string, error) { + if name == "stat" && args[len(args)-1] == "/home" { + return "0 symbolic link\n", nil + } + return "0 directory\n", nil + } + b.Run = run2 + if why := b.besideIsUnsafe(context.Background(), "/home/agent/x"); !strings.Contains(why, "symbolic link") { + t.Fatalf("a link on the way is unsafe: %q", why) + } + _ = ran +} diff --git a/modules/restic/cmd/restic-backups/main.go b/modules/restic/cmd/restic-backups/main.go index 1e53e18c..508a69b4 100644 --- a/modules/restic/cmd/restic-backups/main.go +++ b/modules/restic/cmd/restic-backups/main.go @@ -141,7 +141,7 @@ func tools(b *Backups) []stdio.Tool { } return map[string]any{"started": started, "follow": Seat + ".backed-up"}, nil }), - verb("restore", "Restore one module's data from a restore point BESIDE the live data, never over it: each directory as .restored-. Swapping it in is a person's act.", + verb("restore", "Restore one module's data from a restore point BESIDE the live data, never over it: each directory as .restored-, or in this module's state when a directory on the way belongs to another account. Swapping it in is a person's act.", map[string]any{ "module": str("the module"), "snapshot": str("the restore point (the newest when omitted)"),