Skip to content

Commit efcb6c5

Browse files
committed
backup: M4-2b reject encoded reserved-subtree targets (codex P2 round 5 #928)
walkObjectSubdir skips top-level '_orphans/' and '_incomplete_uploads/' when they contain no sidecars (the dump's own payload). A KEYMAP entry whose Encoded names a file under those subtrees (e.g. '_orphans/foo.bin' or '_incomplete_uploads/records.jsonl') passes the IsRegular check in verifyKeymapTargetsExist - the dump-control payload genuinely exists - but the walk never consumes it, so Encode finishes while silently dropping the intended object. The companion gate for the decoded original key was already in validateKeymapReservedRoot; this commit mirrors the same restriction on the encoded side via validateKeymapEncodedPathWalkerSkipped. Function structure: validateKeymapEncodedPath now delegates the walker-skip checks (sidecar suffix, top-level control files, reserved-subtree roots) to a helper to keep cyclop under 10. Caller audit: validateKeymapEncodedPath is invoked once from validateKeymapRecord. No call-site changes. Regression: TestValidateKeymapEncodedPath_ReservedSubtree with 6 sub-cases - top-level + nested _orphans / _incomplete_uploads reject; _orphansFoo (longer name, NOT reserved) and prefix/_orphans (nested, not top-level) accept.
1 parent 24e75d3 commit efcb6c5

2 files changed

Lines changed: 69 additions & 2 deletions

File tree

internal/backup/encode_s3_collision.go

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -219,8 +219,17 @@ func validateKeymapEncodedPath(encoded string) error {
219219
"encoded path %q contains invalid segment %q", encoded, seg)
220220
}
221221
}
222-
last := segs[len(segs)-1]
223-
if strings.HasSuffix(last, S3MetaSuffixReserved) {
222+
return validateKeymapEncodedPathWalkerSkipped(encoded, segs)
223+
}
224+
225+
// validateKeymapEncodedPathWalkerSkipped rejects encoded paths the
226+
// object walk would silently skip — sidecar suffixes at any depth,
227+
// top-level "_bucket.json" / "KEYMAP.jsonl" control files, and any
228+
// path rooted in a reserved dump subtree (_orphans /
229+
// _incomplete_uploads). Split out of validateKeymapEncodedPath so
230+
// the parent stays under cyclop. Codex P2 #928 round 4 + round 5.
231+
func validateKeymapEncodedPathWalkerSkipped(encoded string, segs []string) error {
232+
if strings.HasSuffix(segs[len(segs)-1], S3MetaSuffixReserved) {
224233
return errors.Wrapf(ErrInvalidKeymapRecord,
225234
"encoded path %q ends in reserved sidecar suffix %q (walker would skip)",
226235
encoded, S3MetaSuffixReserved)
@@ -233,6 +242,19 @@ func validateKeymapEncodedPath(encoded string) error {
233242
return errors.Wrapf(ErrInvalidKeymapRecord,
234243
"encoded path %q names a top-level control file the walker skips", encoded)
235244
}
245+
// Reserved-subtree first-segment guard. walkObjectSubdir skips
246+
// top-level "_orphans/" and "_incomplete_uploads/" when they
247+
// hold no sidecars (the dump's own payload). A KEYMAP entry
248+
// whose Encoded is under those subtrees passes the IsRegular
249+
// check on the existing dump-control payload (e.g.
250+
// "_orphans/foo.bin" or "_incomplete_uploads/records.jsonl") but
251+
// the walk never consumes it. The companion gate for the
252+
// decoded original key lives in validateKeymapReservedRoot;
253+
// this mirror catches the encoded side. Codex P2 #928 round 5.
254+
if _, reserved := keymapReservedRoots[segs[0]]; reserved {
255+
return errors.Wrapf(ErrInvalidKeymapRecord,
256+
"encoded path %q targets reserved dump subtree %q (walker would skip)", encoded, segs[0])
257+
}
236258
return nil
237259
}
238260

internal/backup/encode_s3_collision_test.go

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -281,6 +281,51 @@ func TestValidateKeymapEncodedPath_Backslash(t *testing.T) {
281281
}
282282
}
283283

284+
// TestValidateKeymapEncodedPath_ReservedSubtree pins codex P2 #928
285+
// round 5. walkObjectSubdir skips top-level "_orphans/" and
286+
// "_incomplete_uploads/" when they hold no sidecars (the dump's
287+
// own payload). A KEYMAP entry pointing into those subtrees passes
288+
// the IsRegular existence check but the walk never consumes it -
289+
// the rename is silently dropped.
290+
func TestValidateKeymapEncodedPath_ReservedSubtree(t *testing.T) {
291+
t.Parallel()
292+
cases := []struct {
293+
name string
294+
encoded string
295+
wantErr bool
296+
}{
297+
{"_orphans top-level payload", "_orphans/foo.bin", true},
298+
{"_orphans nested", "_orphans/gen-1/x.bin", true},
299+
{"_incomplete_uploads top-level", "_incomplete_uploads/records.jsonl", true},
300+
{"_incomplete_uploads nested", "_incomplete_uploads/sub/x", true},
301+
// _orphansFoo / nested _orphans are NOT reserved — only the
302+
// exact top-level segment matches.
303+
{"_orphansFoo (not reserved)", "_orphansFoo/x" + S3LeafDataSuffix, false},
304+
{"nested _orphans (not top-level)", "prefix/_orphans/x" + S3LeafDataSuffix, false},
305+
}
306+
for _, c := range cases {
307+
t.Run(c.name, func(t *testing.T) {
308+
t.Parallel()
309+
rec := KeymapRecord{
310+
Encoded: c.encoded,
311+
OriginalB64: base64.RawURLEncoding.EncodeToString([]byte("original/key")),
312+
Kind: KindSHAFallback,
313+
}
314+
err := validateKeymapRecord(rec)
315+
if c.wantErr {
316+
if !errors.Is(err, ErrInvalidKeymapRecord) {
317+
t.Fatalf("err = %v, want ErrInvalidKeymapRecord", err)
318+
}
319+
if !strings.Contains(err.Error(), "reserved dump subtree") {
320+
t.Fatalf("err = %v, want reserved-dump-subtree message", err)
321+
}
322+
} else if err != nil {
323+
t.Fatalf("unexpected err = %v", err)
324+
}
325+
})
326+
}
327+
}
328+
284329
// TestValidateKeymapEncodedPath_WalkerSkipped pins codex P2 #928
285330
// round 4 (walker-skipped reserved files): a corrupt KEYMAP entry
286331
// whose Encoded names a top-level "_bucket.json" / "KEYMAP.jsonl"

0 commit comments

Comments
 (0)