Skip to content

Commit 9029add

Browse files
committed
backup: reject truncated DynamoDB keys (PR #716, round 3)
Two Codex P2 strict-validation follow-ups. #117 -- empty table-meta segment. HandleTableMeta accepted `!ddb|meta|table|` (no encoded segment) because base64.RawURLEncoding.DecodeString("") returns empty bytes without error, so the schema would route under the empty table name. Now rejected with ErrDDBMalformedKey before the JSON decode. TestDDB_RejectsTableMetaKeyWithEmptySegment. #303 -- truncated item key. parseDDBItemKey accepted `!ddb|item|<table>|7|` (gen separator present, no primary-key payload). The gen-end check was "genEnd > 0" which a trailing `|` satisfies. Added a follow-up check that genEnd+1 != len(afterTable) so a payload-less key surfaces as ErrDDBMalformedKey rather than emit under value-side attributes only. TestDDB_RejectsItemKeyWithEmptyPrimaryKeyPayload.
1 parent 305b14f commit 9029add

2 files changed

Lines changed: 41 additions & 0 deletions

File tree

internal/backup/dynamodb.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,13 @@ func (d *DDBEncoder) HandleTableMeta(key, value []byte) error {
113113
if err != nil {
114114
return errors.Wrap(ErrDDBMalformedKey, err.Error())
115115
}
116+
if encoded == "" {
117+
// base64.RawURLEncoding.DecodeString("") succeeds with an
118+
// empty slice, so without this guard a truncated key like
119+
// `!ddb|meta|table|` would be routed under the empty table
120+
// name. That hides corruption (Codex P2 #117).
121+
return errors.Wrapf(ErrDDBMalformedKey, "empty table segment: %q", key)
122+
}
116123
rawName, err := base64.RawURLEncoding.DecodeString(encoded)
117124
if err != nil {
118125
return errors.Wrap(ErrDDBMalformedKey, err.Error())
@@ -305,6 +312,16 @@ func parseDDBItemKey(key []byte) (string, uint64, error) {
305312
if err != nil {
306313
return "", 0, errors.Wrap(ErrDDBMalformedKey, err.Error())
307314
}
315+
// Item keys must carry a primary-key payload after the gen
316+
// separator (the encoded hash + range bytes). A bare
317+
// `!ddb|item|<table>|<gen>|` cannot identify any item; treating
318+
// such a key as valid would let a truncated record slip past
319+
// malformed-key detection and emit under value-side attributes
320+
// only, masking snapshot corruption (Codex P2 #303).
321+
if genEnd+1 == len(afterTable) {
322+
return "", 0, errors.Wrapf(ErrDDBMalformedKey,
323+
"item key missing primary-key payload: %q", key)
324+
}
308325
return enc, gen, nil
309326
}
310327

internal/backup/dynamodb_test.go

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -490,6 +490,30 @@ func TestDDB_ParseItemKeyExtractsGeneration(t *testing.T) {
490490
}
491491
}
492492

493+
func TestDDB_RejectsTableMetaKeyWithEmptySegment(t *testing.T) {
494+
t.Parallel()
495+
enc, _ := newDDBEncoder(t)
496+
// `!ddb|meta|table|` (no encoded segment) -- base64url-decodes to
497+
// an empty name and would otherwise route the schema under "".
498+
// Codex P2 #117.
499+
err := enc.HandleTableMeta([]byte(DDBTableMetaPrefix), []byte("ignored"))
500+
if !errors.Is(err, ErrDDBMalformedKey) {
501+
t.Fatalf("err=%v", err)
502+
}
503+
}
504+
505+
func TestDDB_RejectsItemKeyWithEmptyPrimaryKeyPayload(t *testing.T) {
506+
t.Parallel()
507+
// `!ddb|item|<table>|7|` -- gen separator present but no
508+
// primary-key payload. Codex P2 #303.
509+
key := []byte(DDBItemPrefix)
510+
key = append(key, []byte("dA")...) // base64url("t")
511+
key = append(key, []byte("|7|")...)
512+
if _, _, err := parseDDBItemKey(key); !errors.Is(err, ErrDDBMalformedKey) {
513+
t.Fatalf("err=%v want ErrDDBMalformedKey for truncated item key", err)
514+
}
515+
}
516+
493517
func TestDDB_RejectsKeyWithMissingTableSegment(t *testing.T) {
494518
t.Parallel()
495519
enc, _ := newDDBEncoder(t)

0 commit comments

Comments
 (0)