fix: stale repo locks, orphaned sandboxes, leaked username, and a nonsense recovery time

agent/internal/report/redact_test.go

@@ -104,3 +104,25 @@ func TestRedactForTransportBoundsLength(t *testing.T) {
 		t.Errorf("RedactForTransport did not bound length: %d chars", len(got))
 	}
 }
+
+// Docker client errors embed the daemon socket path, which on a desktop
+// install contains the operator's account name. This repo is published under a
+// pseudonym, so a username reaching the control plane is a real problem, not a
+// cosmetic one.
+func TestScrubDockerSocketPath(t *testing.T) {
+	in := `start container from mysql:8: Post "http://%2FUsers%2Fdbelle%2F.docker%2Frun%2Fdocker.sock/v1.48/containers/abc/start": terminated signal received`
+	got := Scrub(in)
+	if strings.Contains(got, "dbelle") {
+		t.Errorf("leaked the account name: %s", got)
+	}
+	for _, keep := range []string{"mysql:8", "terminated signal received"} {
+		if !strings.Contains(got, keep) {
+			t.Errorf("over-redacted, lost %q: %s", keep, got)
+		}
+	}
+	// A plain http URL that is not the docker socket must be untouched.
+	url := "checked https://example.com/v1/thing"
+	if Scrub(url) != url {
+		t.Errorf("rewrote an unrelated URL: %s", Scrub(url))
+	}
+}

agent/internal/report/report.go

@@ -58,12 +58,23 @@ type RunResult struct {
 // permits in rest:/s3: URLs) are fully masked, not partially.
 var credRe = regexp.MustCompile(`([a-zA-Z][a-zA-Z0-9+.-]*://)[^\s]+@`)
 
-// Scrub masks credentials embedded in URLs anywhere in s. Repo strings and
-// error strings must pass through Scrub before entering a RunResult. The
-// regex is whitespace-bounded, so it can only ever over-mask within a token
-// (safe), never leak.
+// dockerSockRe matches the Docker daemon endpoint the client embeds in its
+// errors, e.g.
+//
+//	Post "http://%2FUsers%2Falice%2F.docker%2Frun%2Fdocker.sock/v1.48/…"
+//
+// That path contains the operator's account name, which has no business
+// travelling to the control plane or sitting in a log. Matching on the
+// docker.sock suffix keeps this from touching anything else.
+var dockerSockRe = regexp.MustCompile(`(?i)https?://[^/"\s]*docker\.sock`)
+
+// Scrub masks credentials embedded in URLs anywhere in s, and the local Docker
+// socket path. Repo strings and error strings must pass through Scrub before
+// entering a RunResult. The credential regex is whitespace-bounded, so it can
+// only ever over-mask within a token (safe), never leak.
 func Scrub(s string) string {
-	return credRe.ReplaceAllString(s, "${1}***@")
+	s = credRe.ReplaceAllString(s, "${1}***@")
+	return dockerSockRe.ReplaceAllString(s, "http://docker.sock")
 }
 
 // dbDetailRe matches the start of a message segment that echoes row values out

agent/internal/restic/restic.go

@@ -81,6 +81,14 @@ func (r *Runner) env() []string {
 func (r *Runner) run(ctx context.Context, sub subcommand, out io.Writer, args ...string) error {
 	full := append([]string{string(sub), "--repo", r.repo}, args...)
 	cmd := exec.CommandContext(ctx, r.bin, full...)
+	// Without this, cancellation SIGKILLs restic, which then never releases
+	// the repository lock it took. The user's own `restic forget`/`prune`
+	// afterwards fails with "repository is already locked by PID ...", so a
+	// Ctrl-C here silently breaks their retention job. Interrupt instead, the
+	// way a real Ctrl-C would, and give restic a moment to unlock before the
+	// runtime forces the issue.
+	cmd.Cancel = func() error { return cmd.Process.Signal(os.Interrupt) }
+	cmd.WaitDelay = 10 * time.Second
 	var stderr bytes.Buffer
 	cmd.Stdout = out
 	cmd.Stderr = &stderr

agent/internal/sandbox/reap_test.go

@@ -0,0 +1,87 @@
+package sandbox
+
+import (
+	"os"
+	"path/filepath"
+	"testing"
+	"time"
+)
+
+// Destroy is deferred and survives panics and signals, but nothing survives
+// SIGKILL or an OOM kill. Those leave the user's restored data on disk with no
+// owner, and before this nothing ever reclaimed it.
+func TestReapOrphans(t *testing.T) {
+	base := t.TempDir()
+
+	mk := func(name string, age time.Duration) string {
+		p := filepath.Join(base, name)
+		if err := os.MkdirAll(p, 0o700); err != nil {
+			t.Fatal(err)
+		}
+		// Sandboxes hold restored data, so put something in it.
+		if err := os.WriteFile(filepath.Join(p, "restored.db"), []byte("customer data"), 0o600); err != nil {
+			t.Fatal(err)
+		}
+		when := time.Now().Add(-age)
+		if err := os.Chtimes(p, when, when); err != nil {
+			t.Fatal(err)
+		}
+		return p
+	}
+
+	abandoned := mk("restorable-abandoned", 48*time.Hour)
+	live := mk("restorable-live", time.Minute)          // a concurrent run
+	borderline := mk("restorable-recent", 23*time.Hour) // inside the window
+	foreign := mk("someone-elses-tempdir", 48*time.Hour)
+
+	reapOrphans(base)
+
+	for _, tc := range []struct {
+		path     string
+		wantGone bool
+		why      string
+	}{
+		{abandoned, true, "old sandbox holding restored data must be reclaimed"},
+		{live, false, "a concurrent run's sandbox must never be deleted"},
+		{borderline, false, "inside the age window, still assume an owner"},
+		{foreign, false, "only directories this package creates may be removed"},
+	} {
+		_, err := os.Stat(tc.path)
+		gone := os.IsNotExist(err)
+		if gone != tc.wantGone {
+			t.Errorf("%s: gone=%v, want %v (%s)", filepath.Base(tc.path), gone, tc.wantGone, tc.why)
+		}
+	}
+}
+
+// Reaping is housekeeping. It must never be the reason a run cannot start.
+func TestReapOrphansToleratesUnreadableBase(t *testing.T) {
+	reapOrphans(filepath.Join(t.TempDir(), "does-not-exist"))
+}
+
+// The reaper runs inside New, so a live sandbox created immediately after an
+// abandoned one is reclaimed must still be usable.
+func TestNewStillWorksAfterReaping(t *testing.T) {
+	base := t.TempDir()
+	old := filepath.Join(base, "restorable-old")
+	if err := os.MkdirAll(old, 0o700); err != nil {
+		t.Fatal(err)
+	}
+	when := time.Now().Add(-48 * time.Hour)
+	if err := os.Chtimes(old, when, when); err != nil {
+		t.Fatal(err)
+	}
+
+	sb, err := New(base, 0)
+	if err != nil {
+		t.Fatalf("New after reaping: %v", err)
+	}
+	defer sb.Destroy() //nolint:errcheck // test cleanup
+
+	if _, err := os.Stat(old); !os.IsNotExist(err) {
+		t.Error("abandoned sandbox survived New")
+	}
+	if _, err := os.Stat(sb.Dir()); err != nil {
+		t.Errorf("new sandbox unusable: %v", err)
+	}
+}

agent/internal/sandbox/sandbox.go

@@ -7,6 +7,9 @@ package sandbox
 import (
 	"fmt"
 	"os"
+	"path/filepath"
+	"strings"
+	"time"
 )
 
 // Sandbox is a disposable directory a snapshot is restored into.
@@ -33,13 +36,50 @@ func New(baseDir string, requiredBytes uint64) (*Sandbox, error) {
 			"insufficient disk space in %s: %s free, %s required — aborting before restore",
 			baseDir, humanBytes(free), humanBytes(requiredBytes))
 	}
+	reapOrphans(baseDir)
 	dir, err := os.MkdirTemp(baseDir, "restorable-")
 	if err != nil {
 		return nil, fmt.Errorf("create sandbox dir: %w", err)
 	}
 	return &Sandbox{dir: dir}, nil
 }
 
+// orphanAge is how old a sandbox must be before it is assumed abandoned. A
+// restore is bounded by disk and network, never by days, so this is far above
+// any legitimate run while still well inside "the user would rather not keep
+// their restored data lying around".
+const orphanAge = 24 * time.Hour
+
+// reapOrphans removes sandboxes left behind by runs that died outright.
+//
+// Destroy is deferred and survives panics, failures and signals, but nothing
+// survives SIGKILL, an OOM kill, or the power going out. Those leave a full
+// copy of the user's restored data on disk with no owner, which is both a disk
+// leak and a privacy problem, and no later run ever reclaimed it.
+//
+// Deliberately conservative, because this deletes directories: only entries
+// directly under the configured base dir, only ones matching the prefix this
+// package creates, and only after orphanAge, so a concurrent agent's live
+// sandbox is never a candidate. Failures are ignored; reaping is housekeeping
+// and must never be the reason a verification run cannot start.
+func reapOrphans(baseDir string) {
+	entries, err := os.ReadDir(baseDir)
+	if err != nil {
+		return
+	}
+	cutoff := time.Now().Add(-orphanAge)
+	for _, e := range entries {
+		if !e.IsDir() || !strings.HasPrefix(e.Name(), "restorable-") {
+			continue
+		}
+		info, err := e.Info()
+		if err != nil || info.ModTime().After(cutoff) {
+			continue
+		}
+		_ = os.RemoveAll(filepath.Join(baseDir, e.Name()))
+	}
+}
+
 // Dir returns the sandbox directory path.
 func (s *Sandbox) Dir() string { return s.dir }
 

web/src/lib/api/schemas.test.ts

@@ -73,3 +73,34 @@ describe("registerRequestSchema", () => {
     ).toBe(false);
   });
 });
+
+describe("restore_duration_ms sanity", () => {
+  const base = {
+    repo_fingerprint: "a".repeat(64),
+    repo_label: "/srv/backups/restic",
+    status: "pass" as const,
+    started_at: "2026-09-19T10:00:00.000Z",
+    finished_at: "2026-09-19T10:00:01.000Z", // a one-second run
+    agent_version: "v0.1.2",
+    checks: [],
+  };
+
+  it("rejects a restore longer than the run that contained it", () => {
+    // Previously accepted, and the dashboard rendered "1s · restore 10m" as
+    // the headline verified recovery time.
+    const r = runRequestSchema.safeParse({ ...base, restore_duration_ms: 600_000 });
+    expect(r.success).toBe(false);
+  });
+
+  it("accepts a restore that fits inside the run", () => {
+    expect(runRequestSchema.safeParse({ ...base, restore_duration_ms: 800 }).success).toBe(true);
+  });
+
+  it("allows a second of slack for clock granularity", () => {
+    expect(runRequestSchema.safeParse({ ...base, restore_duration_ms: 1500 }).success).toBe(true);
+  });
+
+  it("still accepts runs that omit it", () => {
+    expect(runRequestSchema.safeParse(base).success).toBe(true);
+  });
+});

web/src/lib/api/schemas.ts

@@ -46,6 +46,18 @@ export const runRequestSchema = z
   .refine(
     (r) => new Date(r.finished_at).getTime() >= new Date(r.started_at).getTime(),
     { message: "finished_at must not be before started_at" },
+  )
+  // The restore is a phase inside the run, so it cannot outlast it. Without
+  // this, a one-second run could claim a ten-minute restore and the dashboard
+  // would render exactly that as "verified recovery time", which is the number
+  // the whole product is sold on. One second of slack covers clock
+  // granularity between the two measurements.
+  .refine(
+    (r) =>
+      r.restore_duration_ms == null ||
+      r.restore_duration_ms <=
+        new Date(r.finished_at).getTime() - new Date(r.started_at).getTime() + 1000,
+    { message: "restore_duration_ms cannot exceed the run's wall-clock duration" },
   );
 
 export type RunRequest = z.infer<typeof runRequestSchema>;