Skip to content

Commit dd97766

Browse files
authored
quarantine: dedupe by payload SHA256 (cap unbounded growth from client retries)
[squashed; see PR body for full description] 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: terafin <terafin@users.noreply.github.com>
1 parent 7919079 commit dd97766

2 files changed

Lines changed: 106 additions & 0 deletions

File tree

backend/cmd/server/save_validation.go

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,13 @@ type quarantineRecord struct {
6767
TrustLevel string `json:"trustLevel,omitempty"`
6868
UploadedAt time.Time `json:"uploadedAt"`
6969
UploadSource string `json:"uploadSource,omitempty"`
70+
// Last time this same payload was re-attempted by a client; nil on the
71+
// first observation. Updated by quarantineRejectedUpload when an entry
72+
// with the same SHA256 already exists.
73+
LastSeenAt *time.Time `json:"lastSeenAt,omitempty"`
74+
// Total observations of this payload, including the first. Always >= 1
75+
// for any persisted record.
76+
RetryCount int `json:"retryCount,omitempty"`
7077
}
7178

7279
type validationStatus struct {
@@ -322,6 +329,33 @@ func (a *app) quarantineRejectedUpload(filename, sourcePath string, payload []by
322329
now := time.Now().UTC()
323330
sum := sha256.Sum256(payload)
324331
shaHex := hex.EncodeToString(sum[:])
332+
333+
// Dedupe: if a quarantine entry with the same payload SHA256 already
334+
// exists, update its LastSeenAt + RetryCount instead of writing a fresh
335+
// directory. Without this, a misbehaving client that re-attempts a
336+
// rejected save every sync cycle creates one new quarantine entry per
337+
// retry, growing on-disk state without bound. listQuarantineRecords()
338+
// is O(N) but N stays small post-fix; if it ever doesn't, indexing by
339+
// SHA256 would be the next step.
340+
if existingDir, existingRecord, ok := a.findQuarantineRecordBySHA(shaHex); ok {
341+
count := existingRecord.RetryCount
342+
if count < 1 {
343+
count = 1
344+
}
345+
count++
346+
seen := now
347+
existingRecord.LastSeenAt = &seen
348+
existingRecord.RetryCount = count
349+
// Refresh the rejection reason in case the validator's reasoning
350+
// changed (e.g. the heuristic was tuned), so operators see the
351+
// latest classification when triaging.
352+
if reason := strings.TrimSpace(item.Reason); reason != "" {
353+
existingRecord.Reason = reason
354+
}
355+
_ = a.writeQuarantineRecord(existingDir, existingRecord)
356+
return
357+
}
358+
325359
id := fmt.Sprintf("%s-%s", now.Format("20060102T150405.000000000Z"), shaHex[:12])
326360
dir, err := safeJoinUnderRoot(store.root, quarantineDirName, canonicalSegment(id, "rejected"))
327361
if err != nil {
@@ -355,6 +389,7 @@ func (a *app) quarantineRejectedUpload(filename, sourcePath string, payload []by
355389
TrustLevel: firstNonEmpty(strings.TrimSpace(item.TrustLevel), validationTrustLevel(item.ParserLevel)),
356390
UploadedAt: now,
357391
UploadSource: strings.TrimSpace(uploadSource),
392+
RetryCount: 1,
358393
}
359394
if err := writeFileAtomic(filepath.Join(dir, payloadFile), payload, 0o644); err != nil {
360395
return
@@ -366,6 +401,45 @@ func (a *app) quarantineRejectedUpload(filename, sourcePath string, payload []by
366401
_ = writeFileAtomic(filepath.Join(dir, "metadata.json"), data, 0o644)
367402
}
368403

404+
// findQuarantineRecordBySHA scans the on-disk quarantine for an entry whose
405+
// payload SHA256 matches the given hex digest. Returns the directory path,
406+
// the parsed record, and whether a match was found.
407+
func (a *app) findQuarantineRecordBySHA(shaHex string) (string, quarantineRecord, bool) {
408+
store := a.currentSaveStore()
409+
if store == nil || strings.TrimSpace(shaHex) == "" {
410+
return "", quarantineRecord{}, false
411+
}
412+
root, err := safeJoinUnderRoot(store.root, quarantineDirName)
413+
if err != nil {
414+
return "", quarantineRecord{}, false
415+
}
416+
entries, err := os.ReadDir(root)
417+
if err != nil {
418+
return "", quarantineRecord{}, false
419+
}
420+
for _, entry := range entries {
421+
if !entry.IsDir() {
422+
continue
423+
}
424+
dir := filepath.Join(root, entry.Name())
425+
data, err := os.ReadFile(filepath.Join(dir, "metadata.json"))
426+
if err != nil {
427+
continue
428+
}
429+
var record quarantineRecord
430+
if err := json.Unmarshal(data, &record); err != nil {
431+
continue
432+
}
433+
if strings.EqualFold(record.SHA256, shaHex) {
434+
if strings.TrimSpace(record.ID) == "" {
435+
record.ID = entry.Name()
436+
}
437+
return dir, record, true
438+
}
439+
}
440+
return "", quarantineRecord{}, false
441+
}
442+
369443
func (a *app) listQuarantineRecords() ([]quarantineRecord, error) {
370444
store := a.currentSaveStore()
371445
if store == nil {

backend/cmd/server/save_validation_test.go

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,3 +126,35 @@ func TestValidationQuarantineDeleteRemovesOnlyQuarantineItem(t *testing.T) {
126126
t.Fatalf("expected quarantine delete to remove item: %s", prettyJSON(afterValidation))
127127
}
128128
}
129+
130+
// Re-uploading the same payload (same SHA256) must not create a new quarantine
131+
// entry — instead the existing record's RetryCount + LastSeenAt are bumped.
132+
// Without this, a misbehaving sync client (we observed sgm-steamdeck-helper
133+
// retrying a rejected save every 30s cycle) makes quarantine grow unbounded
134+
// on disk.
135+
func TestQuarantineDedupesByPayloadSHA256(t *testing.T) {
136+
h := newContractHarness(t)
137+
138+
body := []byte("not a save file")
139+
for i := 0; i < 5; i++ {
140+
upload := h.multipart("/saves", nil, "file", "notes.txt", body)
141+
assertStatus(t, upload, http.StatusUnprocessableEntity)
142+
}
143+
144+
status := h.request(http.MethodGet, "/api/validation", nil)
145+
assertStatus(t, status, http.StatusOK)
146+
statusBody := decodeJSONMap(t, status.Body)
147+
validation := mustObject(t, statusBody["validation"], "validation")
148+
if got := mustNumber(t, validation["quarantineCount"], "validation.quarantineCount"); got != 1 {
149+
t.Fatalf("expected exactly 1 quarantine entry after 5 dup uploads, got %v: %s", got, prettyJSON(validation))
150+
}
151+
quarantine := mustArray(t, validation["quarantine"], "validation.quarantine")
152+
first := mustObject(t, quarantine[0], "quarantine[0]")
153+
retryCount := mustNumber(t, first["retryCount"], "quarantine[0].retryCount")
154+
if retryCount != 5 {
155+
t.Fatalf("expected retryCount=5 after 5 uploads, got %v: %s", retryCount, prettyJSON(first))
156+
}
157+
if first["lastSeenAt"] == nil {
158+
t.Fatalf("expected lastSeenAt to be set after a retry: %s", prettyJSON(first))
159+
}
160+
}

0 commit comments

Comments
 (0)