backup: PR810 r4 — codex r4 P2 + gemini r1 mediums

Three remaining reviewer findings on PR #810 (one P2, two
mediums) that the prior rounds did not address. None is a
fail-closed semantic change; together they tighten the
CHECKSUMS on-disk format and the cmd's manifest write path
against operational shapes the design promises to support.

1. (codex r4 P2) WriteChecksums wrote filenames verbatim into a
   newline-delimited file. A path containing `\n`, `\r`, or `\\`
   would corrupt the line-based output and let a single
   adversarial entry inject a second CHECKSUMS row that the
   verifier would accept.

   Fix: formatChecksumLine implements the GNU coreutils
   sha256sum(1) escape convention — leading `\` on the line
   plus `\n` / `\r` / `\\` substitutions. splitChecksumLine
   reverses the convention via unescapeChecksumPath at parse
   time. `sha256sum -c CHECKSUMS` from the dump root parses
   both forms identically, preserving the vendor-independent
   recovery property.

   Tests:
   - TestChecksumLine_EscapeRoundTrip — five paths covering
     `\n`, `\r`, `\\`, multiple escapes mixed, and the
     no-escape passthrough.
   - TestChecksumLine_EscapePrefixMatchesGNUFormat — pins the
     exact `\` + hex + `  ` + escaped-body shape so a future
     change cannot drift from sha256sum compatibility.
   - TestSplitChecksumLine_RejectsDanglingEscape — guards
     against a tampered CHECKSUMS using `\<EOL>` or `\x` for
     unknown `x`.

2. (gemini r1 medium, main.go:281) emitManifest concatenated
   `cfg.outputRoot + "/MANIFEST.json"`. On Windows this
   produces wrong path separators and would refuse a
   Windows-style output root.

   Fix: filepath.Join. Standard cross-platform shape.

3. (gemini r1 medium, main.go:285) emitManifest deferred a
   `_ = out.Close()` that swallowed the Close error. On NFS /
   FUSE filesystems Close is the authoritative durability
   signal (writeback errors surface there, not at Write time),
   so a silently-discarded Close error means the dump-tree
   invariant (MANIFEST.json on disk) can fail without the cmd
   surfacing it.

   Fix: explicit Sync() then Close() with both errors
   propagated; the defer'd close on error paths runs through
   `_ = out.Close()` because we have already surfaced the
   primary failure.

Caller audit:
- formatChecksumLine / unescapeChecksumPath are new helpers
  called only from WriteChecksums / splitChecksumLine. No
  cross-package use.
- splitChecksumLine's exported behavior is rejection-side
  tightening (dangling escape) plus correct round-trip of
  escape-needing paths. No production caller of VerifyChecksums
  exists (test-only).
- emitManifest is internal to cmd/elastickv-snapshot-decode.
  The Close-error surface is purely additive — previously-
  successful runs stay successful.

Self-review:
1. Data loss — none; the new escape path PREVENTS data loss
   (line injection); the Close-error surface PREVENTS silent
   manifest-write loss on NFS.
2. Concurrency — none.
3. Performance — single Builder per escape-needing line; the
   no-escape fast path is unchanged.
4. Data consistency — sha256sum(1) escape round-trip is
   byte-identical (round-trip test pins it).
5. Test coverage — three new tests; existing seven CHECKSUMS
   tests still pass.

Author: bootjp

cmd/elastickv-snapshot-decode/main.go

@@ -22,6 +22,7 @@ import (
 	"io"
 	"log/slog"
 	"os"
+	"path/filepath"
 	"strings"
 	"time"
 
@@ -278,15 +279,35 @@ func emitManifest(cfg *config, res backup.DecodeResult) error {
 	if cfg.bundleJSONL {
 		m.DynamoDBLayout = backup.DynamoDBLayoutJSONL
 	}
-	out, err := os.Create(cfg.outputRoot + "/MANIFEST.json") //nolint:gosec // operator-supplied path
+	// filepath.Join over `+ "/"` so the manifest path uses the
+	// platform separator (gemini r1 medium on PR #810).
+	out, err := os.Create(filepath.Join(cfg.outputRoot, "MANIFEST.json")) //nolint:gosec // operator-supplied path
 	if err != nil {
 		return errors.WithStack(err)
 	}
-	defer func() { _ = out.Close() }()
+	// Surface Close errors instead of swallowing them: on a
+	// filesystem that delays writeback until Close (NFS, some
+	// FUSE mounts), the write succeeds but the buffered-data
+	// flush failure shows up only at Close time. Without this
+	// surface, a manifest that was never durably written would
+	// pass the cmd's error-return contract and the dump-tree
+	// invariant ("MANIFEST.json is on disk") would silently
+	// fail. Gemini r1 medium on PR #810. Sync runs before Close
+	// so the explicit fsync is the authoritative durability
+	// signal; Close's role is just to surface the kernel's
+	// per-fd cleanup result.
 	if err := backup.WriteManifest(out, m); err != nil {
+		_ = out.Close()
 		return errors.WithStack(err)
 	}
-	return errors.WithStack(out.Sync())
+	if err := out.Sync(); err != nil {
+		_ = out.Close()
+		return errors.WithStack(err)
+	}
+	if err := out.Close(); err != nil {
+		return errors.WithStack(err)
+	}
+	return nil
 }
 
 // populateAdapterScopes returns a non-nil pointer for every enabled

internal/backup/checksums.go

@@ -74,13 +74,48 @@ func WriteChecksums(root string) error {
 	}
 	defer func() { _ = out.Close() }()
 	for _, e := range entries {
-		if _, err := fmt.Fprintf(out, "%s  %s\n", e.hex, e.relPath); err != nil {
+		line, prefix := formatChecksumLine(e.hex, e.relPath)
+		if _, err := fmt.Fprintf(out, "%s%s\n", prefix, line); err != nil {
 			return errors.WithStack(err)
 		}
 	}
 	return errors.WithStack(out.Sync())
 }
 
+// formatChecksumLine builds one CHECKSUMS row using GNU coreutils
+// sha256sum(1) escape conventions. When the path contains `\n`,
+// `\r`, or `\\`, sha256sum prefixes the entire line with `\\` and
+// replaces the offending bytes with `\\n` / `\\r` / `\\\\`. Phase 0a
+// emits the same shape so `sha256sum -c CHECKSUMS` from the dump
+// root parses a tampered or path-noisy filename without corrupting
+// the line-based output (codex r4 P2 on PR #810: the previous
+// `%s  %s\n` print would let a filename containing `\n` inject a
+// fake CHECKSUMS row).
+//
+// Returns (line-body, prefix). The prefix is either "" or "\\";
+// callers concatenate `prefix + body + "\n"` exactly like the
+// reference implementation.
+func formatChecksumLine(hex, relPath string) (string, string) {
+	if !strings.ContainsAny(relPath, "\n\r\\") {
+		return hex + "  " + relPath, ""
+	}
+	var sb strings.Builder
+	sb.Grow(len(relPath) + 4) //nolint:mnd // small heuristic to avoid early grow
+	for _, r := range relPath {
+		switch r {
+		case '\n':
+			sb.WriteString(`\n`)
+		case '\r':
+			sb.WriteString(`\r`)
+		case '\\':
+			sb.WriteString(`\\`)
+		default:
+			sb.WriteRune(r)
+		}
+	}
+	return hex + "  " + sb.String(), `\`
+}
+
 // checksumEntry is one row of the CHECKSUMS file.
 type checksumEntry struct {
 	hex     string
@@ -313,10 +348,22 @@ func validateChecksumRelPath(rel string) (string, error) {
 var ErrChecksumsPathTraversal = errors.New("backup: CHECKSUMS path escapes dump root")
 
 // splitChecksumLine parses one sha256sum(1) line of the form
-// `<hex>  <relative-path>`. Returns (hex, path, ok). Lines with the
-// wrong shape are reported as not-ok so the caller can surface a
-// CHECKSUMS-corruption error rather than ingest garbage.
+// `<hex>  <relative-path>` (or its `\\`-prefixed escaped variant
+// when the path contains `\n`/`\r`/`\\`). Returns (hex, path, ok).
+// Lines with the wrong shape are reported as not-ok so the
+// caller can surface a CHECKSUMS-corruption error rather than
+// ingest garbage.
+//
+// Escape convention mirrors GNU coreutils: a leading `\\` signals
+// that the rest of the path has `\n`/`\r`/`\\` written as the
+// two-byte sequences `\\n`, `\\r`, `\\\\`. Codex r4 P2 on PR #810:
+// without this, a filename containing `\n` would split across
+// lines and a verifier would see a bogus second row.
 func splitChecksumLine(line string) (string, string, bool) {
+	escaped := strings.HasPrefix(line, `\`)
+	if escaped {
+		line = line[1:]
+	}
 	// sha256sum's "binary" mode separates digest and filename with
 	// exactly two spaces. Some tools emit `<hex> *<path>` for
 	// binary mode; we accept both.
@@ -332,7 +379,45 @@ func splitChecksumLine(line string) (string, string, bool) {
 	if !strings.HasPrefix(rest, "  ") && !strings.HasPrefix(rest, " *") {
 		return "", "", false
 	}
-	return hexPart, rest[2:], true
+	body := rest[2:]
+	if !escaped {
+		return hexPart, body, true
+	}
+	unescaped, ok := unescapeChecksumPath(body)
+	if !ok {
+		return "", "", false
+	}
+	return hexPart, unescaped, true
+}
+
+// unescapeChecksumPath reverses formatChecksumLine's `\n`/`\r`/`\\`
+// substitutions. Returns (path, ok). An unrecognised `\<byte>` or
+// a dangling trailing `\` is treated as malformed — the line is
+// rejected at the caller via the ok=false return.
+func unescapeChecksumPath(body string) (string, bool) {
+	var sb strings.Builder
+	sb.Grow(len(body))
+	for i := 0; i < len(body); i++ {
+		if body[i] != '\\' {
+			sb.WriteByte(body[i])
+			continue
+		}
+		i++
+		if i >= len(body) {
+			return "", false
+		}
+		switch body[i] {
+		case 'n':
+			sb.WriteByte('\n')
+		case 'r':
+			sb.WriteByte('\r')
+		case '\\':
+			sb.WriteByte('\\')
+		default:
+			return "", false
+		}
+	}
+	return sb.String(), true
 }
 
 func isLowerHex(s string) bool {

internal/backup/checksums_test.go

@@ -308,6 +308,83 @@ func TestValidateChecksumRelPath_AcceptsHonestPaths(t *testing.T) {
 	}
 }
 
+// TestChecksumLine_EscapeRoundTrip is the regression for codex r4
+// P2 on PR #810: a path containing `\n`/`\r`/`\\` would corrupt
+// the line-delimited CHECKSUMS without GNU coreutils-style
+// escaping. WriteChecksums prefixes such lines with `\\` and
+// substitutes `\n`/`\r`/`\\` → `\\n`/`\\r`/`\\\\`; VerifyChecksums
+// reverses the substitution at parse time. Round-trip pins both
+// halves together.
+func TestChecksumLine_EscapeRoundTrip(t *testing.T) {
+	t.Parallel()
+	cases := []struct {
+		name string
+		path string
+	}{
+		{"newline", "weird\nfile"},
+		{"carriage_return", "weird\rfile"},
+		{"backslash", `weird\file`},
+		{"multiple_escapes", "a\n\rb\\c"},
+		{"no_escape_pass_through", "honest/path.txt"},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+			hex := "2cf24dba5fb0a30e26e83b2ac5b9e29e1b161e5c1fa7425e73043362938b9824"
+			body, prefix := formatChecksumLine(hex, tc.path)
+			gotHex, gotPath, ok := splitChecksumLine(prefix + body)
+			if !ok {
+				t.Fatalf("splitChecksumLine(%q) rejected its own output", prefix+body)
+			}
+			if gotHex != hex {
+				t.Fatalf("hex = %q, want %q", gotHex, hex)
+			}
+			if gotPath != tc.path {
+				t.Fatalf("path = %q, want %q", gotPath, tc.path)
+			}
+		})
+	}
+}
+
+// TestChecksumLine_EscapePrefixMatchesGNUFormat pins the exact
+// shape WriteChecksums emits for escape-needing paths:
+//
+//	\<hex>  <body-with-\n-\r-\\\>
+//
+// so `sha256sum -c CHECKSUMS` on the dump root parses the line
+// the same way GNU coreutils does.
+func TestChecksumLine_EscapePrefixMatchesGNUFormat(t *testing.T) {
+	t.Parallel()
+	hex := "2cf24dba5fb0a30e26e83b2ac5b9e29e1b161e5c1fa7425e73043362938b9824"
+	body, prefix := formatChecksumLine(hex, "x\ny")
+	if prefix != `\` {
+		t.Fatalf("escape prefix = %q, want %q", prefix, `\`)
+	}
+	wantBody := hex + `  x\ny`
+	if body != wantBody {
+		t.Fatalf("body = %q, want %q", body, wantBody)
+	}
+}
+
+// TestSplitChecksumLine_RejectsDanglingEscape pins the parse-side
+// guard against malformed escape sequences (`\` at end-of-line, or
+// `\x` for unknown `x`). Catches a tampered CHECKSUMS whose
+// escape-prefixed line is otherwise structurally valid.
+func TestSplitChecksumLine_RejectsDanglingEscape(t *testing.T) {
+	t.Parallel()
+	hex := "2cf24dba5fb0a30e26e83b2ac5b9e29e1b161e5c1fa7425e73043362938b9824"
+	cases := []string{
+		`\` + hex + `  trailing\`,   // dangling \ at end
+		`\` + hex + `  bad\xescape`, // unknown escape \x
+	}
+	for _, c := range cases {
+		_, _, ok := splitChecksumLine(c)
+		if ok {
+			t.Fatalf("splitChecksumLine(%q) accepted malformed escape", c)
+		}
+	}
+}
+
 // 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