Merge remote-tracking branch 'origin/feat/backup-phase0a-redis-simple' into feat/backup-phase0a-dynamodb

Author: bootjp

internal/backup/open_nofollow_unix.go

@@ -10,14 +10,27 @@ import (
 	cockroachdberr "github.com/cockroachdb/errors"
 )
 
-// openSidecarFile opens path for write with truncate+create semantics
-// and atomically refuses symlinks. The unix build leans on
-// syscall.O_NOFOLLOW so the kernel returns ELOOP if path is a
-// symlink, eliminating the TOCTOU race a separate Lstat-then-Create
-// pattern would have. The Windows build (open_nofollow_windows.go)
-// implements this with the best alternative available there.
+// openSidecarFile opens path for write while refusing both symlink and
+// hard-link clobber attacks.
+//
+//   - O_NOFOLLOW makes the kernel return ELOOP atomically if the path
+//     is a symbolic link — closing the TOCTOU race a separate
+//     Lstat-then-Create pattern would have.
+//   - To also refuse hard links to files outside the dump tree, we
+//     open WITHOUT O_TRUNC, fstat() the descriptor to check the
+//     link count, and only call Truncate(0) if Nlink == 1. An
+//     adversary that pre-created strings_ttl.jsonl as a hard link
+//     to /etc/passwd (or any other writable file outside the dump
+//     tree) would otherwise see the inode truncated on
+//     openSidecarFile despite the symlink guard. Codex P2 round 9.
+//
+// The Windows build (open_nofollow_windows.go) keeps the simpler
+// Lstat-then-OpenFile guard because Windows's
+// SeCreateSymbolicLinkPrivilege already raises the bar for the
+// equivalent attack.
 func openSidecarFile(path string) (*os.File, error) {
-	const flag = os.O_WRONLY | os.O_CREATE | os.O_TRUNC | syscall.O_NOFOLLOW
+	// Note: NO O_TRUNC here — we truncate after the link-count check.
+	const flag = os.O_WRONLY | os.O_CREATE | syscall.O_NOFOLLOW
 	f, err := os.OpenFile(path, flag, 0o600) //nolint:gosec,mnd // path is composed from output-root + fixed file name; 0600 is the standard owner-only mode
 	if err != nil {
 		if errors.Is(err, syscall.ELOOP) {
@@ -26,5 +39,19 @@ func openSidecarFile(path string) (*os.File, error) {
 		}
 		return nil, cockroachdberr.WithStack(err)
 	}
+	info, err := f.Stat()
+	if err != nil {
+		_ = f.Close()
+		return nil, cockroachdberr.WithStack(err)
+	}
+	if sysStat, ok := info.Sys().(*syscall.Stat_t); ok && sysStat.Nlink > 1 {
+		_ = f.Close()
+		return nil, cockroachdberr.WithStack(cockroachdberr.Newf(
+			"backup: refusing to overwrite hard-linked file at %s (nlink=%d)", path, sysStat.Nlink))
+	}
+	if err := f.Truncate(0); err != nil {
+		_ = f.Close()
+		return nil, cockroachdberr.WithStack(err)
+	}
 	return f, nil
 }

internal/backup/redis_string_test.go

@@ -199,6 +199,37 @@ func TestRedisDB_NewFormatStringTTLNotDuplicatedByScanIndex(t *testing.T) {
 	assertTTLSidecar(t, filepath.Join(root, "redis", "db_0", "strings_ttl.jsonl"), "expiring", fixedExpireMs)
 }
 
+// TestRedisDB_OpenJSONLRefusesHardLinkClobber is the regression for
+// Codex P2 round 9: O_NOFOLLOW only blocks symlinks; an adversary
+// who can write the output directory could pre-create
+// strings_ttl.jsonl as a hard link to a file outside the dump tree
+// (e.g. /etc/passwd) and the open's O_TRUNC would clobber that
+// inode. openSidecarFile now opens WITHOUT O_TRUNC, fstat()s the
+// descriptor, refuses if Nlink > 1, and only calls Truncate(0) on
+// the verified-single-link case.
+func TestRedisDB_OpenJSONLRefusesHardLinkClobber(t *testing.T) {
+	t.Parallel()
+	db, root := newRedisDB(t)
+	dir := filepath.Join(root, "redis", "db_0")
+	if err := os.MkdirAll(dir, 0o755); err != nil {
+		t.Fatal(err)
+	}
+	bait := filepath.Join(root, "bait-hardlink")
+	if err := os.WriteFile(bait, []byte("stay-out"), 0o600); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.Link(bait, filepath.Join(dir, redisStringsTTLFile)); err != nil {
+		t.Fatal(err)
+	}
+	err := db.HandleString([]byte("k"), encodeNewStringValue(t, []byte("v"), fixedExpireMs))
+	if err == nil || !strings.Contains(err.Error(), "refusing to overwrite hard-linked file") {
+		t.Fatalf("expected hard-link refusal error from openSidecarFile, got %v", err)
+	}
+	if got, _ := os.ReadFile(bait); string(got) != "stay-out" { //nolint:gosec // test path
+		t.Fatalf("bait file written through hard link: %q", got)
+	}
+}
+
 // TestRedisDB_OpenJSONLRefusesSymlinkOverwrite is the regression for Codex
 // P2 round 5 + P1 round 6: openJSONL must atomically refuse to follow
 // symlinks. The earlier Lstat-then-Create variant left a TOCTOU window