backup: PR810 r2 — codex r2 P1 (symlink escape on VerifyChecksums)

Codex r2 P1: after the r1 textual `..`-traversal guard,
VerifyChecksums still followed symlinks when sha256-ing the
joined path. A CHECKSUMS line `<hex>  safe/file` could direct
the verifier to hash a host file outside the dump root via a
planted symlink at `safe/file` (or any intermediate parent).

Fix: refuseSymlinkComponents walks the relative path segment by
segment from root, os.Lstat()s each component, and returns
ErrChecksumsSymlinkEscape the moment any component is a
symlink. Catches BOTH terminal symlinks AND intermediate ones
(a parent directory along the way being symlinked). The
per-component walk is stricter than syscall O_NOFOLLOW (which
only covers the terminal case) and avoids the
filepath.EvalSymlinks resolution we are trying to refuse.

Phase 0a dumps contain only regular files by design, so
refusing every symlink found in the path is the right policy
for the only honest dump shape.

Tests:
- TestVerifyChecksums_RejectsTerminalSymlink — plant a symlink
  at safe/escape pointing at a host file, add a CHECKSUMS line
  for it; assert ErrChecksumsSymlinkEscape.
- TestVerifyChecksums_RejectsIntermediateSymlink — plant a
  symlink at safe/ (a directory link to an outside dir),
  reference safe/target.txt in CHECKSUMS; same assertion.
- Both tests skip on Windows (os.Symlink requires
  SeCreateSymbolicLinkPrivilege there; the lstat-walk fix
  itself runs on every platform).

Caller audit (CLAUDE.md "semantic-change → grep all callers"):
- refuseSymlinkComponents is only called from
  verifyOneChecksumLine, which is only called from
  VerifyChecksums. VerifyChecksums has no production callers
  (test-only); CLI uses WriteChecksums.

Self-review:
1. Data loss — none (read-side hardening).
2. Concurrency — none.
3. Performance — one extra Lstat per path component during
   verify; verify is offline-only and rare.
4. Data consistency — symlink-escape closed.
5. Test coverage — two regression tests (terminal + intermediate
   symlink); existing five-shape traversal-path table test
   still passes.

Author: bootjp

internal/backup/checksums.go

@@ -187,8 +187,9 @@ func VerifyChecksums(root string) error {
 // verifyOneChecksumLine handles a single CHECKSUMS line: parse,
 // path-validate, recompute, compare. Extracted from VerifyChecksums
 // so the parent stays under the project's cyclop ceiling and the
-// per-line failure modes (malformed shape, traversal, missing file,
-// mismatch) each surface as a distinct typed error.
+// per-line failure modes (malformed shape, traversal, symlink
+// escape, missing file, mismatch) each surface as a distinct
+// typed error.
 func verifyOneChecksumLine(root, line string, lineNum int) error {
 	if line == "" {
 		return nil
@@ -201,7 +202,11 @@ func verifyOneChecksumLine(root, line string, lineNum int) error {
 	if err != nil {
 		return errors.Wrapf(err, "line %d", lineNum)
 	}
-	got, err := sha256File(filepath.Join(root, cleaned))
+	joined := filepath.Join(root, cleaned)
+	if err := refuseSymlinkComponents(root, cleaned); err != nil {
+		return errors.Wrapf(err, "line %d", lineNum)
+	}
+	got, err := sha256File(joined)
 	if err != nil {
 		return errors.Wrapf(err, "verifying %s", cleaned)
 	}
@@ -211,6 +216,48 @@ func verifyOneChecksumLine(root, line string, lineNum int) error {
 	return nil
 }
 
+// refuseSymlinkComponents walks rel one path segment at a time
+// starting at root and rejects the first component that is a
+// symlink. This closes the codex r2 P1 symlink-escape attack on
+// PR #810: after the textual `..`-traversal guard runs,
+// `os.Open(filepath.Join(root, "safe/file"))` would still follow a
+// symlink at `safe/file` (or at any of its parent components) to
+// a file outside root and hash whatever it found. A Phase 0a dump
+// produced by the encoder writes only regular files, so refusing
+// every symlink found in the path keeps the verifier honest
+// regardless of which component an adversary planted the link in.
+//
+// The per-component walk catches BOTH terminal symlinks (where
+// `cleaned` itself names a symlink) AND intermediate ones (where
+// a parent directory along the way is a symlink). syscall
+// O_NOFOLLOW would only cover the terminal case, and
+// filepath.EvalSymlinks runs the resolution we are trying to
+// avoid in the first place.
+func refuseSymlinkComponents(root, rel string) error {
+	cur := root
+	for _, seg := range strings.Split(rel, string(filepath.Separator)) {
+		if seg == "" {
+			continue
+		}
+		cur = filepath.Join(cur, seg)
+		info, err := os.Lstat(cur)
+		if err != nil {
+			return errors.WithStack(err)
+		}
+		if info.Mode()&os.ModeSymlink != 0 {
+			return errors.Wrapf(ErrChecksumsSymlinkEscape,
+				"symlink in path: %s", cur)
+		}
+	}
+	return nil
+}
+
+// ErrChecksumsSymlinkEscape is returned by VerifyChecksums when a
+// CHECKSUMS line's resolved path traverses a symlink. Typed so a
+// caller branching on errors.Is can distinguish symlink-tampering
+// from textual `..`-traversal (ErrChecksumsPathTraversal).
+var ErrChecksumsSymlinkEscape = errors.New("backup: CHECKSUMS path traverses symlink")
+
 // validateChecksumRelPath rejects CHECKSUMS rows whose path field
 // could escape the dump root. The acceptance rules:
 //

internal/backup/checksums_test.go

@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+	"runtime"
 	"strings"
 	"testing"
 
@@ -158,6 +159,91 @@ func TestVerifyChecksums_RejectsTraversalPath(t *testing.T) {
 	}
 }
 
+// TestVerifyChecksums_RejectsTerminalSymlink is the regression for
+// codex r2 P1 on PR #810: after the textual `..` guard, a
+// CHECKSUMS line `<hex>  safe/file` whose `safe/file` is a symlink
+// pointing outside the dump root would have been silently
+// followed by os.Open, hashing the host-side target. The
+// per-component lstat walk in refuseSymlinkComponents catches
+// both terminal and intermediate symlinks.
+func TestVerifyChecksums_RejectsTerminalSymlink(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		// Windows symlink creation requires
+		// SeCreateSymbolicLinkPrivilege which is rarely available
+		// to CI users. The lstat-walk fix runs on every platform;
+		// the test only exercises Linux/macOS where os.Symlink is
+		// unprivileged.
+		t.Skip("symlink test requires unprivileged os.Symlink (skipped on Windows)")
+	}
+	t.Parallel()
+	root := t.TempDir()
+	outsideDir := t.TempDir()
+	outsideFile := filepath.Join(outsideDir, "secret")
+	mustWrite(t, outsideFile, []byte("host-only"))
+	// Build a CHECKSUMS that lists `safe/file` with a digest that
+	// would match the outside file's contents — proving the
+	// verifier did NOT follow the symlink.
+	mustWrite(t, filepath.Join(root, "safe", "honest"), []byte("dump-only"))
+	if err := WriteChecksums(root); err != nil {
+		t.Fatalf("WriteChecksums: %v", err)
+	}
+	// Now plant a symlink at safe/escape -> outsideFile and add an
+	// entry for it to CHECKSUMS. WriteChecksums followed the link
+	// during its own walk too, but VerifyChecksums must reject.
+	link := filepath.Join(root, "safe", "escape")
+	if err := os.Symlink(outsideFile, link); err != nil {
+		t.Fatalf("symlink: %v", err)
+	}
+	// Append a CHECKSUMS entry for the symlinked file.
+	checksumsPath := filepath.Join(root, CHECKSUMSFilename)
+	body, err := os.ReadFile(checksumsPath) //nolint:gosec // test path
+	if err != nil {
+		t.Fatalf("read CHECKSUMS: %v", err)
+	}
+	// Use the digest of the outside file's actual content so the
+	// shape-parse + path-traversal checks succeed; the symlink
+	// guard is the first thing that must reject.
+	body = append(body, []byte("e3d1ac1b09a9e88c0c12e9b1d31d6a92e2ec43cf2bda7bcd58e3a3b2c50e2dd2  safe/escape\n")...)
+	if err := os.WriteFile(checksumsPath, body, 0o600); err != nil {
+		t.Fatalf("write CHECKSUMS: %v", err)
+	}
+	err = VerifyChecksums(root)
+	if err == nil {
+		t.Fatalf("expected ErrChecksumsSymlinkEscape, got nil")
+	}
+	if !errors.Is(err, ErrChecksumsSymlinkEscape) {
+		t.Fatalf("err = %v, want ErrChecksumsSymlinkEscape", err)
+	}
+}
+
+// TestVerifyChecksums_RejectsIntermediateSymlink pins that a
+// symlinked PARENT component is also caught — without the
+// per-component walk, refusing only the terminal symlink would
+// still leave the verifier vulnerable to a dump with a top-level
+// symlinked directory.
+func TestVerifyChecksums_RejectsIntermediateSymlink(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		t.Skip("symlink test requires unprivileged os.Symlink")
+	}
+	t.Parallel()
+	root := t.TempDir()
+	outsideDir := t.TempDir()
+	mustWrite(t, filepath.Join(outsideDir, "target.txt"), []byte("host-side"))
+	// safe/ is a symlink to a directory outside the dump root.
+	if err := os.Symlink(outsideDir, filepath.Join(root, "safe")); err != nil {
+		t.Fatalf("symlink: %v", err)
+	}
+	body := "0000000000000000000000000000000000000000000000000000000000000000  safe/target.txt\n"
+	mustWrite(t, filepath.Join(root, CHECKSUMSFilename), []byte(body))
+	err := VerifyChecksums(root)
+	if err == nil {
+		t.Fatalf("expected ErrChecksumsSymlinkEscape, got nil")
+	}
+	if !errors.Is(err, ErrChecksumsSymlinkEscape) {
+		t.Fatalf("err = %v, want ErrChecksumsSymlinkEscape", err)
+	}
+}
+
 // TestVerifyChecksums_StreamsLargeFile pins the bufio.Scanner path
 // — the previous implementation slurped the entire CHECKSUMS file
 // via os.ReadFile, which OOMs on large dump trees (gemini