unkin/artifactapi
2
0
Fork 0
Code Issues 5 Pull Requests 2 Actions Packages Projects Releases 1 Wiki Activity

fix: GC has no grace period (TOCTOU with dedup uploads) #86

Merged
benvin merged 2 commits from benvin/gc-grace-period into master 2026-07-02 22:43:18 +10:00
Conversation 1 Commits 2 Files Changed 2
+15 -4
unkinben commented 2026-07-02 00:30:12 +10:00
Owner
Copy Link
Copy Source

Fixes #71

Why

FindOrphanedBlobs returned any blob not currently referenced. Because CAS dedups (the blob row can exist before its artifact/local_files row is written), a concurrent upload reusing an existing hash could have its S3 object deleted mid-flight by the GC.

Changes

  • FindOrphanedBlobs now takes a minAge and only returns blobs with created_at < now()-minAge.
  • The collector passes a 1h blobGracePeriod.

Validation

  • go test ./internal/gc/... and make e2e pass.
Fixes #71 ## Why `FindOrphanedBlobs` returned any blob not currently referenced. Because CAS dedups (the blob row can exist before its artifact/local_files row is written), a concurrent upload reusing an existing hash could have its S3 object deleted mid-flight by the GC. ## Changes - `FindOrphanedBlobs` now takes a `minAge` and only returns blobs with `created_at < now()-minAge`. - The collector passes a 1h `blobGracePeriod`. ## Validation - `go test ./internal/gc/...` and `make e2e` pass.
unkinben added 1 commit 2026-07-02 22:30:47 +10:00
fix: add a grace period before GC deletes orphaned blobs
ci/woodpecker/pr/pre-commit Pipeline failed
Details
ci/woodpecker/pr/test Pipeline failed
Details
ci/woodpecker/pr/build Pipeline failed
Details
c47daca1f1 
FindOrphanedBlobs returned any unreferenced blob, so a concurrent dedup
upload (which inserts the blob row before its artifact/local_files row)
could have its S3 object deleted mid-flight. Restrict collection to blobs
older than a 1h grace period.

Refs #71
unkinben force-pushed benvin/gc-grace-period from 8fc1635d11 to c47daca1f1 2026-07-02 22:30:47 +10:00 Compare
unkinben commented 2026-07-02 22:34:35 +10:00
Author
Owner
Copy Link
Copy Source

Rebased onto latest master (resolved a conflict in artifacts.go against the batched access-log writer). Note: master currently does not compile — see #96, which must merge first; this branch builds cleanly on top of it (verified: build + make e2e).

Rebased onto latest master (resolved a conflict in artifacts.go against the batched access-log writer). Note: master currently does not compile — see #96, which must merge first; this branch builds cleanly on top of it (verified: build + make e2e).
benvin added 1 commit 2026-07-02 22:36:30 +10:00
Merge branch 'master' into benvin/gc-grace-period
ci/woodpecker/pr/pre-commit Pipeline was successful
Details
ci/woodpecker/pr/build Pipeline was successful
Details
ci/woodpecker/pr/test Pipeline was successful
Details
bf690dda54
benvin merged commit e7c9387bcc into master 2026-07-02 22:43:18 +10:00
benvin deleted branch benvin/gc-grace-period 2026-07-02 22:43:18 +10:00
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
No items
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
benvin (BenVincent) unkinben (Ben Vincent)
No Assignees
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: unkin/artifactapi#86
Delete Branch "benvin/gc-grace-period"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?