backup: refuse dot-segment scratch paths in HandleBlob (PR #718, round 9)

Codex P1 round 11 (commit 9a63e32):

`HandleBlob` composed scratch paths with `filepath.Join(s.scratchRoot,
EncodeSegment([]byte(bucket)), EncodeSegment([]byte(object)))`.
EncodeSegment uses the RFC3986 unreserved set
(ALPHA/DIGIT/-/./_) — `/` is percent-encoded but `.` is preserved,
so the literal segment `..` survives unchanged. A crafted
`bucket=".."` and/or `object=".."` would resolve to
`<scratchRoot>/../...`, letting `writeFileAtomic` land outside the
decoder's controlled directory before `safeJoinUnderRoot` runs at
output time.

Add `scratchDirForBlob` which rejects `.` / `..` / "" bucket and
object literals at the encoder boundary so the spill-to-disk
step inherits the same containment invariant the final output
path enforces. Apply the same guard to `flushOrphanObject` which
shared the failure mode under `--include-orphans`.

(Multi-segment dot keys like `a/../b` continue to be caught at
Finalize via `safeJoinUnderRoot` because EncodeSegment keeps the
whole key in one filename segment that splits cleanly there.)

Tests:
  - TestS3_HandleBlobRejectsScratchEscape: 5 sub-cases covering
    bucket/object/both variants of `.`/`..` literals.
  - TestS3_DotSegmentObjectKeyRejected updated to allow either
    HandleBlob or Finalize to surface ErrS3MalformedKey, since
    sole-dot keys are now caught earlier.

Author: bootjp

internal/backup/s3.go

@@ -411,14 +411,25 @@ func (s *S3Encoder) HandleObjectManifest(key, value []byte) error {
 
 // HandleBlob spills a !s3|blob| record to a per-chunk scratch file
 // and registers it under the (bucket, object, gen, uploadID, partNo,
-// chunkNo, partVersion) routing key.
+// chunkNo, partVersion) routing key. EncodeSegment percent-encodes
+// `/` so a multi-segment object key like `../../tmp/pwn` collapses
+// into one filename, but a literal `..` (or `.`) survives unchanged
+// because both `.` chars are RFC3986-unreserved. Without explicit
+// validation, a crafted bucket+object pair like `bucket="..",
+// object=".."` would resolve to filepath.Join(scratchRoot, "..",
+// "..") = the parent of scratchRoot, letting writeFileAtomic
+// land outside the decoder's controlled directory before
+// safeJoinUnderRoot ever runs at output time. Codex P1 round 11.
 func (s *S3Encoder) HandleBlob(key, value []byte) error {
 	bucket, gen, object, uploadID, partNo, chunkNo, partVersion, ok := s3keys.ParseBlobKey(key)
 	if !ok {
 		return errors.Wrapf(ErrS3MalformedKey, "blob key: %q", key)
 	}
 	st := s.objectState(bucket, gen, object)
-	dir := filepath.Join(s.scratchRoot, EncodeSegment([]byte(bucket)), EncodeSegment([]byte(object)))
+	dir, err := scratchDirForBlob(s.scratchRoot, bucket, object)
+	if err != nil {
+		return err
+	}
 	if !st.scratchDirCreated {
 		if err := os.MkdirAll(dir, 0o755); err != nil { //nolint:mnd // 0755 == standard dir mode
 			return errors.WithStack(err)
@@ -434,6 +445,27 @@ func (s *S3Encoder) HandleBlob(key, value []byte) error {
 	return nil
 }
 
+// scratchDirForBlob builds the per-(bucket,object) scratch path and
+// validates it stays under scratchRoot. A bucket or object name of
+// `.` / `..` would let `filepath.Join` resolve out of scratchRoot
+// before anything else gets a chance to refuse the key. Reject the
+// dot-component case at the encoder boundary so the spill-to-disk
+// step inherits the same containment invariant the final output
+// path enforces via safeJoinUnderRoot.
+func scratchDirForBlob(scratchRoot, bucket, object string) (string, error) {
+	for _, seg := range [...]string{bucket, object} {
+		switch seg {
+		case ".", "..":
+			return "", errors.Wrapf(ErrS3MalformedKey,
+				"bucket or object key %q is a dot segment (would escape scratch root)", seg)
+		case "":
+			return "", errors.Wrapf(ErrS3MalformedKey,
+				"bucket or object key is empty (cannot construct scratch path)")
+		}
+	}
+	return filepath.Join(scratchRoot, EncodeSegment([]byte(bucket)), EncodeSegment([]byte(object))), nil
+}
+
 // HandleIncompleteUpload routes !s3|upload|meta|/!s3|upload|part|
 // records to <bucket>/_incomplete_uploads/records.jsonl when the
 // include flag is on; otherwise drops them.
@@ -682,6 +714,10 @@ func (s *S3Encoder) flushOrphanObject(b *s3BucketState, bucketDir string, obj *s
 	if len(obj.chunkPaths) == 0 {
 		return nil
 	}
+	if obj.object == "." || obj.object == ".." || obj.object == "" {
+		return errors.Wrapf(ErrS3MalformedKey,
+			"orphan object key %q is a dot segment (would escape orphan dir)", obj.object)
+	}
 	dir := filepath.Join(bucketDir, "_orphans", EncodeSegment([]byte(obj.object)))
 	if err := os.MkdirAll(dir, 0o755); err != nil { //nolint:mnd // 0755 == standard dir mode
 		return errors.WithStack(err)

internal/backup/s3_test.go

@@ -584,14 +584,81 @@ func TestS3_StalePartVersionExcludedFromAssembledBody(t *testing.T) {
 	}
 }
 
+// TestS3_HandleBlobRejectsScratchEscape is the regression for Codex
+// P1 round 11: HandleBlob composed scratch paths with EncodeSegment,
+// which preserves `.` and `..` (RFC3986 unreserved). A bucket or
+// object literal of `..` would resolve to `<scratchRoot>/../...`,
+// letting writeFileAtomic land outside the decoder's scratch tree
+// before safeJoinUnderRoot ever ran at Finalize. The encoder now
+// refuses dot-component bucket/object names at HandleBlob.
+func TestS3_HandleBlobRejectsScratchEscape(t *testing.T) {
+	t.Parallel()
+	cases := []struct {
+		name           string
+		bucket, object string
+	}{
+		{"object_dotdot", "b", ".."},
+		{"object_dot", "b", "."},
+		{"bucket_dotdot", "..", "x"},
+		{"bucket_dot", ".", "x"},
+		{"both_dotdot", "..", ".."},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+			enc, _ := newS3Encoder(t)
+			err := enc.HandleBlob(
+				s3keys.BlobKey(tc.bucket, 1, tc.object, "u-1", 1, 0),
+				[]byte("payload"),
+			)
+			if !errors.Is(err, ErrS3MalformedKey) {
+				t.Fatalf("err=%v want ErrS3MalformedKey for bucket=%q object=%q", err, tc.bucket, tc.object)
+			}
+		})
+	}
+}
+
 func TestS3_DotSegmentObjectKeyRejected(t *testing.T) {
 	t.Parallel()
 	cases := []string{"a/../b", "a/./b", "..", "."}
 	for _, key := range cases {
 		t.Run(key, func(t *testing.T) {
+			t.Parallel()
 			enc, _ := newS3Encoder(t)
-			emitObject(t, enc, "b", 1, key, []byte("x"), "")
-			err := enc.Finalize()
+			// Refusal must happen at OR BEFORE Finalize. The
+			// scratch-path guard (Codex P1 round 11) catches sole-
+			// dot keys at HandleBlob time; multi-segment dot keys
+			// like "a/../b" pass through to Finalize where
+			// safeJoinUnderRoot rejects them. Either point is
+			// acceptable as long as ErrS3MalformedKey surfaces.
+			err := enc.HandleBucketMeta(
+				s3keys.BucketMetaKey("b"),
+				encodeS3BucketMetaValue(t, map[string]any{"bucket_name": "b", "generation": 1}),
+			)
+			if err != nil {
+				t.Fatalf("HandleBucketMeta: %v", err)
+			}
+			err = enc.HandleObjectManifest(
+				s3keys.ObjectManifestKey("b", 1, key),
+				encodeS3ManifestValue(t, map[string]any{
+					"upload_id": "u-1", "size_bytes": int64(1),
+					"parts": []map[string]any{{"part_no": 1, "size_bytes": int64(1), "chunk_count": 1}},
+				}),
+			)
+			if err != nil {
+				if errors.Is(err, ErrS3MalformedKey) {
+					return
+				}
+				t.Fatalf("HandleObjectManifest: %v", err)
+			}
+			err = enc.HandleBlob(s3keys.BlobKey("b", 1, key, "u-1", 1, 0), []byte("x"))
+			if err != nil {
+				if errors.Is(err, ErrS3MalformedKey) {
+					return
+				}
+				t.Fatalf("HandleBlob: %v", err)
+			}
+			err = enc.Finalize()
 			if !errors.Is(err, ErrS3MalformedKey) {
 				t.Fatalf("err=%v want ErrS3MalformedKey for key %q", err, key)
 			}