backup: PR810 r3 — codex r3 P1 (Windows reserved names via filepath.IsLocal)

Codex r3 P1: validateChecksumRelPath rejected `..`-traversal and
absolute paths but accepted Windows reserved device names like
CON / NUL / COM1 / CONOUT$. On Windows, os.Open of those names
yields the host's console / device rather than a regular file
under the dump root, so verification can succeed/fail based on
host device state instead of dump contents.

Fix: replace the explicit absolute/traversal checks with
filepath.IsLocal (Go 1.20+), which delegates path-locality to
the stdlib. On Windows IsLocal additionally rejects the
reserved device names; on Unix the names look like ordinary
files (and would be hashed if they exist under root, the
honest dump shape).

Empty / `.` are still explicitly rejected because IsLocal
considers `.` local (it stays within the subtree, but it
names the root directory, which is not a hashable regular
file). The post-Clean recheck below IsLocal is belt-and-braces
against a future stdlib change.

Tests:
- TestValidateChecksumRelPath_RejectsWindowsDeviceNames pins
  the rejection. The reserved-name check is Windows-only by
  filepath.IsLocal's contract, so the test skips on non-Windows;
  the lexical fix itself runs on every platform.
- TestValidateChecksumRelPath_AcceptsHonestPaths cross-checks
  the IsLocal switchover does not regress the five legitimate
  Phase 0a dump-tree shapes (Redis blob, S3 nested object,
  DynamoDB schema, SQS messages.jsonl).

Caller audit (CLAUDE.md "semantic-change → grep all callers"):
- validateChecksumRelPath only called from
  verifyOneChecksumLine → VerifyChecksums; the latter has no
  production callers (test-only). The semantic change is
  purely rejection-side tightening on Windows; honest CHECKSUMS
  unaffected.

Self-review:
1. Data loss — none (read-side hardening).
2. Concurrency — none.
3. Performance — IsLocal is O(path-len); a single pass.
4. Data consistency — Windows device-name escape closed.
5. Test coverage — two new tests; existing five-shape
   traversal-path table test still passes.

Author: bootjp

internal/backup/checksums.go

@@ -259,39 +259,46 @@ func refuseSymlinkComponents(root, rel string) error {
 var ErrChecksumsSymlinkEscape = errors.New("backup: CHECKSUMS path traverses symlink")
 
 // validateChecksumRelPath rejects CHECKSUMS rows whose path field
-// could escape the dump root. The acceptance rules:
+// could escape the dump root or otherwise resolve outside the
+// regular-file subtree the verifier expects. The acceptance rule
+// is `filepath.IsLocal` — Go-stdlib's lexical "stays within the
+// subtree rooted at the evaluation directory" check, which
+// covers:
 //
-//   - absolute paths are rejected;
-//   - paths containing a `..` segment (before or after Clean) are
-//     rejected;
-//   - the cleaned form must not start with `../` (covers
-//     filepath.Clean turning `a/../../b` into `../b`);
-//   - empty / `.` paths are rejected so a malformed entry that
-//     would otherwise read the root directory itself surfaces.
+//   - absolute paths (any platform);
+//   - traversal via `..` segments (before or after Clean);
+//   - on Windows, reserved device names like `CON`, `NUL`,
+//     `COM1`-`COM9`, `LPT1`-`LPT9`, `PRN`, `AUX`, `CONIN$`,
+//     `CONOUT$`, etc. — opened via `os.Open` these would yield
+//     the host's console / device, not a dump file, and produce
+//     hash mismatches keyed off host state. Codex r3 P1 on
+//     PR #810.
 //
-// Returns the cleaned form so callers join it without redoing the
-// canonicalisation. Mirrors the same guard the S3 encoder applies
-// to bucket/object path segments before scratch-dir layout.
+// Empty and "." are rejected explicitly because IsLocal returns
+// true for "." (it does stay within the subtree, but it names
+// the root directory itself, which is not a file we can hash);
+// honest CHECKSUMS lines name regular files, not the root dir.
+//
+// Returns the Cleaned form so callers join it without redoing
+// the canonicalisation. Mirrors the same guard the S3 encoder
+// applies to bucket/object path segments before scratch-dir
+// layout.
 func validateChecksumRelPath(rel string) (string, error) {
 	if rel == "" || rel == "." {
 		return "", errors.Wrapf(ErrChecksumsMalformedLine, "empty path field")
 	}
-	if filepath.IsAbs(rel) {
-		return "", errors.Wrapf(ErrChecksumsPathTraversal, "absolute path: %q", rel)
+	if !filepath.IsLocal(rel) {
+		return "", errors.Wrapf(ErrChecksumsPathTraversal,
+			"not a local path (absolute / traversal / reserved name): %q", rel)
 	}
 	cleaned := filepath.Clean(rel)
-	// Defence in depth: Clean turns `a/b/../../../etc/passwd` into
-	// `../etc/passwd`. A leading `..` segment in the cleaned form
-	// means the relative path escaped its parent, regardless of
-	// what tricks were used to write it.
+	// IsLocal already rejects absolute paths and `..` traversal
+	// before/after Clean; the post-Clean recheck below is
+	// belt-and-braces against a future stdlib change that would
+	// loosen IsLocal but keep the cleaned-shape invariants.
 	if cleaned == ".." || strings.HasPrefix(cleaned, ".."+string(filepath.Separator)) {
-		return "", errors.Wrapf(ErrChecksumsPathTraversal, "escapes root: %q", rel)
+		return "", errors.Wrapf(ErrChecksumsPathTraversal, "escapes root after Clean: %q", rel)
 	}
-	// `Clean` strips trailing slashes but leaves the leading
-	// separator on absolute paths intact; we already filtered
-	// those, so a remaining absolute-shaped Clean result is from
-	// inputs like `/foo` on platforms where filepath.IsAbs
-	// disagrees with filepath.Clean. Belt-and-braces.
 	if filepath.IsAbs(cleaned) {
 		return "", errors.Wrapf(ErrChecksumsPathTraversal, "absolute after Clean: %q", rel)
 	}

internal/backup/checksums_test.go

@@ -244,6 +244,70 @@ func TestVerifyChecksums_RejectsIntermediateSymlink(t *testing.T) {
 	}
 }
 
+// TestValidateChecksumRelPath_RejectsWindowsDeviceNames is the
+// regression for codex r3 P1 on PR #810: even after the textual
+// `..`-traversal guard, names like CON / NUL / COM1 / CONOUT$
+// would open the host's console / device on Windows via
+// `os.Open(filepath.Join(root, "CON"))`. The verifier delegates
+// locality to filepath.IsLocal which refuses these names
+// lexically — but only on Windows (filepath.IsLocal is
+// platform-aware; on Unix these names are valid filenames).
+//
+// The test runs the unit-level validateChecksumRelPath check
+// rather than the full VerifyChecksums flow because the lstat
+// step downstream of the IsLocal guard would fail with a
+// generic ENOENT on Unix where the names are not pre-created,
+// masking the rejection class we want to pin.
+func TestValidateChecksumRelPath_RejectsWindowsDeviceNames(t *testing.T) {
+	if runtime.GOOS != "windows" {
+		// filepath.IsLocal only rejects reserved device names on
+		// Windows; on Unix these strings are ordinary filenames.
+		// The lexical fix lands on every platform, but the
+		// rejection is observable only where it matters.
+		t.Skip("Windows-only: filepath.IsLocal reserved-name check is platform-specific")
+	}
+	t.Parallel()
+	names := []string{"CON", "NUL", "PRN", "AUX", "COM1", "LPT1", "CONOUT$"}
+	for _, name := range names {
+		t.Run(name, func(t *testing.T) {
+			t.Parallel()
+			_, err := validateChecksumRelPath(name)
+			if err == nil {
+				t.Fatalf("expected ErrChecksumsPathTraversal for reserved name %q, got nil", name)
+			}
+			if !errors.Is(err, ErrChecksumsPathTraversal) {
+				t.Fatalf("err = %v, want ErrChecksumsPathTraversal", err)
+			}
+		})
+	}
+}
+
+// TestValidateChecksumRelPath_AcceptsHonestPaths cross-checks the
+// IsLocal switchover did not regress legitimate relative paths
+// (the Phase 0a dump tree).
+func TestValidateChecksumRelPath_AcceptsHonestPaths(t *testing.T) {
+	t.Parallel()
+	cases := []string{
+		"a.txt",
+		"redis/db_0/strings/foo.bin",
+		"s3/photos/2026/04/29/img.jpg",
+		"dynamodb/orders/_schema.json",
+		"sqs/queue-1/messages.jsonl",
+	}
+	for _, p := range cases {
+		t.Run(p, func(t *testing.T) {
+			t.Parallel()
+			got, err := validateChecksumRelPath(p)
+			if err != nil {
+				t.Fatalf("validateChecksumRelPath(%q) unexpected err: %v", p, err)
+			}
+			if got != filepath.Clean(p) {
+				t.Fatalf("validateChecksumRelPath(%q) = %q, want %q", p, got, filepath.Clean(p))
+			}
+		})
+	}
+}
+
 // 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