fix: data leaks, alert-channel bypass, and two silent wrong answers

agent/internal/recipes/dockerapp.go

@@ -87,10 +87,10 @@ func (c *DockerAppCheck) Run(ctx context.Context, t *Target) (report.Status, str
 		}
 		hostDir = t.Roots[0].Dir
 	} else {
-		var ok bool
-		hostDir, ok = resolve(t.Roots, restored)
-		if !ok {
-			return report.StatusFail, fmt.Sprintf("mount path %q not found in restored snapshot", c.Mount.Restored)
+		var err error
+		hostDir, err = resolve(t.Roots, restored)
+		if err != nil {
+			return report.StatusFail, fmt.Sprintf("mount path %q %v", c.Mount.Restored, err)
 		}
 	}
 

agent/internal/recipes/files.go

@@ -65,9 +65,9 @@ func (f *FilesCheck) Run(ctx context.Context, t *Target) (report.Status, string)
 
 	for _, req := range f.Require {
 		rel := strings.TrimSuffix(req.Path, "/")
-		abs, ok := resolve(t.Roots, rel)
-		if !ok {
-			issues = append(issues, fmt.Sprintf("required path %q not found in restored snapshot", req.Path))
+		abs, err := resolve(t.Roots, rel)
+		if err != nil {
+			issues = append(issues, fmt.Sprintf("required path %q %v", req.Path, err))
 			continue
 		}
 		if req.MinFiles > 0 {
@@ -124,20 +124,52 @@ func validateRelPath(p string) error {
 	return nil
 }
 
+// errPathMissing means nothing exists at the requested path.
+var errPathMissing = errors.New("not found in restored snapshot")
+
+// errPathEscapes means the path exists but leads out of the restored data. For
+// a backup tool this is a finding in its own right, not a lookup failure: the
+// symlink was backed up and whatever it points at was not.
+var errPathEscapes = errors.New(
+	"is a symlink leading outside the restored snapshot, so its target was never backed up")
+
 // resolve finds rel under one of the restored roots, enforcing containment:
 // even if a malformed path slips past validation, the resolved location must
 // stay inside a root directory (defense in depth against traversal).
-func resolve(roots []Root, rel string) (string, bool) {
+func resolve(roots []Root, rel string) (string, error) {
+	escaped := false
 	for _, root := range roots {
 		abs := filepath.Join(root.Dir, rel)
 		if !withinRoot(root.Dir, abs) {
 			continue
 		}
-		if _, err := os.Stat(abs); err == nil {
-			return abs, true
+		if _, err := os.Stat(abs); err != nil {
+			continue
 		}
+		// os.Stat follows symlinks. Without the check below, a restored
+		// symlink pointing out of the sandbox resolves to the live copy on the
+		// host, so the check verifies the running system and reports the
+		// backup healthy — the exact failure this tool exists to catch.
+		//
+		// Both sides are resolved before comparing because the sandbox itself
+		// routinely sits under a symlinked prefix: /tmp is /private/tmp on
+		// macOS, so comparing a resolved path against an unresolved root would
+		// reject every legitimate path there.
+		realRoot, rootErr := filepath.EvalSymlinks(root.Dir)
+		realAbs, absErr := filepath.EvalSymlinks(abs)
+		if rootErr != nil || absErr != nil {
+			continue
+		}
+		if !withinRoot(realRoot, realAbs) {
+			escaped = true
+			continue
+		}
+		return abs, nil
+	}
+	if escaped {
+		return "", errPathEscapes
 	}
-	return "", false
+	return "", errPathMissing
 }
 
 // withinRoot reports whether abs is inside dir (after cleaning), blocking

agent/internal/recipes/mysql.go

@@ -47,9 +47,9 @@ func (c *MySQLCheck) validate() error {
 
 // Run implements Check.
 func (c *MySQLCheck) Run(ctx context.Context, t *Target) (report.Status, string) {
-	dumpPath, ok := resolve(t.Roots, strings.TrimSuffix(c.Dump, "/"))
-	if !ok {
-		return report.StatusFail, fmt.Sprintf("dump file %q not found in restored snapshot", c.Dump)
+	dumpPath, err := resolve(t.Roots, strings.TrimSuffix(c.Dump, "/"))
+	if err != nil {
+		return report.StatusFail, fmt.Sprintf("dump file %q %v", c.Dump, err)
 	}
 	runner, err := t.Docker(ctx)
 	if err != nil {

agent/internal/recipes/postgres.go

@@ -84,9 +84,9 @@ func (c *PostgresCheck) validate() error {
 
 // Run implements Check.
 func (c *PostgresCheck) Run(ctx context.Context, t *Target) (report.Status, string) {
-	dumpPath, ok := resolve(t.Roots, strings.TrimSuffix(c.Dump, "/"))
-	if !ok {
-		return report.StatusFail, fmt.Sprintf("dump file %q not found in restored snapshot", c.Dump)
+	dumpPath, err := resolve(t.Roots, strings.TrimSuffix(c.Dump, "/"))
+	if err != nil {
+		return report.StatusFail, fmt.Sprintf("dump file %q %v", c.Dump, err)
 	}
 	runner, err := t.Docker(ctx)
 	if err != nil {

agent/internal/recipes/security_test.go

@@ -36,7 +36,7 @@ func TestPathTraversalRejectedEverywhere(t *testing.T) {
 // slips past validation (defense in depth).
 func TestResolveContainment(t *testing.T) {
 	roots := []Root{{Dir: t.TempDir(), SnapPath: "/srv"}}
-	if _, ok := resolve(roots, "../../etc/passwd"); ok {
+	if _, err := resolve(roots, "../../etc/passwd"); err == nil {
 		t.Error("resolve returned a path escaping the root")
 	}
 }

agent/internal/recipes/sqlite.go

@@ -36,9 +36,9 @@ func (c *SQLiteCheck) validate() error {
 // database, integrity errors — is a verification failure: the backup does
 // not contain a working database.
 func (c *SQLiteCheck) Run(ctx context.Context, t *Target) (report.Status, string) {
-	abs, ok := resolve(t.Roots, strings.TrimSuffix(c.Path, "/"))
-	if !ok {
-		return report.StatusFail, fmt.Sprintf("database %q not found in restored snapshot", c.Path)
+	abs, err := resolve(t.Roots, strings.TrimSuffix(c.Path, "/"))
+	if err != nil {
+		return report.StatusFail, fmt.Sprintf("database %q %v", c.Path, err)
 	}
 
 	// immutable=1 guarantees the check never writes (no WAL, no journal

agent/internal/recipes/symlink_escape_test.go

@@ -0,0 +1,89 @@
+package recipes
+
+import (
+	"errors"
+	"os"
+	"path/filepath"
+	"testing"
+)
+
+// restic restores symlinks as symlinks, so a backup of /srv/app where `data`
+// is a link to /var/lib/app restores a link pointing at the live host copy.
+// os.Stat follows it, so before this was fixed the check read the running
+// system and reported the backup healthy — a silent false PASS on exactly the
+// question this tool exists to answer.
+//
+// Note t.TempDir() sits under /var/folders on macOS, and /var is itself a
+// symlink to /private/var, so these cases also cover the sandbox living under
+// a symlinked prefix. A containment check that resolved only one side would
+// reject every legitimate path here.
+func TestResolveSymlinkContainment(t *testing.T) {
+	sandbox := t.TempDir()
+	outside := t.TempDir()
+
+	if err := os.WriteFile(filepath.Join(outside, "live.db"), []byte("LIVE HOST DATA"), 0o600); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.WriteFile(filepath.Join(sandbox, "real.db"), []byte("restored"), 0o600); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.Mkdir(filepath.Join(sandbox, "sub"), 0o755); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.WriteFile(filepath.Join(sandbox, "sub", "inner.db"), []byte("restored"), 0o600); err != nil {
+		t.Fatal(err)
+	}
+	// The dangerous one: points out of the restored tree entirely.
+	if err := os.Symlink(filepath.Join(outside, "live.db"), filepath.Join(sandbox, "escape.db")); err != nil {
+		t.Fatal(err)
+	}
+	// The legitimate one: a relative link the backup itself contained.
+	if err := os.Symlink("sub/inner.db", filepath.Join(sandbox, "inside.db")); err != nil {
+		t.Fatal(err)
+	}
+
+	roots := []Root{{Dir: sandbox, SnapPath: "/srv/app"}}
+
+	tests := []struct {
+		name    string
+		rel     string
+		wantErr error
+	}{
+		{"plain restored file resolves", "real.db", nil},
+		{"symlink within the snapshot resolves", "inside.db", nil},
+		{"symlink out of the snapshot is refused", "escape.db", errPathEscapes},
+		{"absent path is still just missing", "nope.db", errPathMissing},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got, err := resolve(roots, tt.rel)
+			if !errors.Is(err, tt.wantErr) {
+				t.Fatalf("resolve(%q) err = %v, want %v", tt.rel, err, tt.wantErr)
+			}
+			if tt.wantErr == nil && got == "" {
+				t.Fatalf("resolve(%q) returned no path", tt.rel)
+			}
+		})
+	}
+}
+
+// The failure mode that matters is not the error value, it is that a check
+// must never read the host copy. This asserts the data itself never surfaces.
+func TestResolveNeverReachesHostData(t *testing.T) {
+	sandbox := t.TempDir()
+	outside := t.TempDir()
+	secret := filepath.Join(outside, "live.db")
+	if err := os.WriteFile(secret, []byte("LIVE HOST DATA"), 0o600); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.Symlink(secret, filepath.Join(sandbox, "data.db")); err != nil {
+		t.Fatal(err)
+	}
+
+	got, err := resolve([]Root{{Dir: sandbox, SnapPath: "/srv/app"}}, "data.db")
+	if err == nil {
+		content, _ := os.ReadFile(got)
+		t.Fatalf("resolved a path outside the sandbox (%s) reading %q", got, content)
+	}
+}

agent/internal/recipes/transport_redaction_test.go

@@ -0,0 +1,58 @@
+package recipes
+
+import (
+	"strings"
+	"testing"
+
+	"github.com/restorable-dev/restorable/agent/internal/report"
+)
+
+// The redaction bug this guards against was not in either function on its own.
+// tail() joins output with " / " and report's filter was anchored to line
+// starts, so each half was individually reasonable and the seam between them
+// leaked every row value a failed database load echoes. Unit tests on either
+// side passed throughout.
+//
+// This asserts the two agree, using output captured verbatim from psql and
+// mysql, so changing either joiner or pattern in isolation fails here.
+func TestTailOutputSurvivesTransportRedaction(t *testing.T) {
+	tests := []struct {
+		name     string
+		raw      string
+		mustHide []string
+	}{
+		{
+			name: "postgres unique violation echoes the key",
+			raw: "psql:/tmp/dump:5: ERROR:  duplicate key value violates unique constraint \"users_pkey\"\n" +
+				"DETAIL:  Key (email)=(alice@example.com) already exists.\n" +
+				"CONTEXT:  COPY users, line 2",
+			mustHide: []string{"alice@example.com", "COPY users"},
+		},
+		{
+			name: "postgres syntax error echoes the statement",
+			raw: "psql:/tmp/dump:1: ERROR:  syntax error at or near \")\"\n" +
+				"LINE 1: ...email) VALUES ('a@example.com'), ('b@example.com')\n" +
+				"                                                          ^",
+			mustHide: []string{"a@example.com", "b@example.com"},
+		},
+		{
+			name: "mysql detail line",
+			raw: "ERROR 1062 (23000) at line 3: Duplicate entry 'dave@example.com' for key 'users.email'\n" +
+				"DETAIL:  row 3 rejected",
+			mustHide: []string{"row 3 rejected"},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			// Exactly what the database checks do before building a message.
+			joined := tail(tt.raw, 2)
+			got := report.RedactForTransport(joined)
+			for _, secret := range tt.mustHide {
+				if strings.Contains(got, secret) {
+					t.Errorf("row data survived redaction: %q\n  joined: %s\n  sent:   %s", secret, joined, got)
+				}
+			}
+		})
+	}
+}

agent/internal/report/client.go

@@ -14,6 +14,7 @@ import (
 	"path/filepath"
 	"strings"
 	"time"
+	"unicode/utf8"
 )
 
 // ErrNoCredentials means the agent is not registered with a control plane —
@@ -120,6 +121,39 @@ type runPayload struct {
 	Checks            []CheckResult `json:"checks"`
 }
 
+// maxRepoLabelBytes mirrors the control plane's bound on repo_label. Keeping
+// the agent inside it matters more than it looks: the server rejected the
+// whole submission when a label ran long, so the repo row was never created,
+// the run never appeared on the dashboard, and stale detection could never
+// fire for that repo — while the agent still printed PASS and exited 0. Long
+// B2 and S3 URLs reach 200 characters easily.
+//
+// Bounding bytes is enough: the server counts UTF-16 units, which is never
+// more than the UTF-8 byte count.
+const maxRepoLabelBytes = 200
+
+// truncateRepoLabel keeps both ends of an over-long repo string. The head
+// carries the scheme and host, the tail carries the path that tells two repos
+// on the same host apart, so cutting either end alone loses the half that
+// makes the label worth showing. The label is display only; repos are
+// identified by fingerprint, so shortening it costs nothing.
+func truncateRepoLabel(s string) string {
+	if len(s) <= maxRepoLabelBytes {
+		return s
+	}
+	const ellipsis = "…"
+	budget := maxRepoLabelBytes - len(ellipsis)
+	head := budget / 2
+	tailStart := len(s) - (budget - head)
+	for head > 0 && !utf8.RuneStart(s[head]) {
+		head--
+	}
+	for tailStart < len(s) && !utf8.RuneStart(s[tailStart]) {
+		tailStart++
+	}
+	return s[:head] + ellipsis + s[tailStart:]
+}
+
 // SubmitRun reports one run result. Only pass/fail metadata leaves the
 // machine: the repo is reduced to a fingerprint plus its scrubbed label, and
 // every string in the result was scrubbed when the result was built.
@@ -138,11 +172,15 @@ func (c *Client) SubmitRun(ctx context.Context, r *RunResult) error {
 		fingerprint = Fingerprint(r.Repo) // fallback for old restic without a repo ID
 	}
 	payload := runPayload{
-		RepoFingerprint:   fingerprint,
-		RepoLabel:         r.Repo, // already scrubbed at result construction
-		SnapshotID:        r.SnapshotID,
-		Status:            r.Status,
-		Error:             r.Error,
+		RepoFingerprint: fingerprint,
+		RepoLabel:       truncateRepoLabel(r.Repo), // scrubbed at result construction
+		SnapshotID:      r.SnapshotID,
+		Status:          r.Status,
+		// Same treatment as check messages. A failed restore enumerates paths
+		// out of the user's snapshot, so this field needs the DB-detail strip
+		// and the length bound too, not just the credential scrub it already
+		// carries from result construction.
+		Error:             RedactForTransport(r.Error),
 		StartedAt:         r.StartedAt,
 		FinishedAt:        r.FinishedAt,
 		RestoreDurationMS: r.RestoreDurationMS,