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

Keep a tombstone on gem yank #155

Merged
benvin merged 3 commits from benvin/gems-yank-tombstone into master 2026-10-18 02:08:46 +11:00
Conversation 2 Commits 3 Files Changed 9
+234 -37
unkin-agent commented 2026-10-18 01:24:55 +11:00
Member
Copy Link
Copy Source

Yank hard-deleted a local gem version, so the same name-version-platform could be re-pushed with different bytes and break lockfiles pinning the old checksum. rubygems.org forbids re-pushing a yanked version.

  • write a _yanked/<name>/<full> tombstone before deleting the gem, quick spec and index line
  • reject a push of a tombstoned version with 409
  • serialise push and yank of one version on the engine Redis lock; 503 when it stays busy
  • drop partial push writes on failure
  • return 404 for _index/ and _yanked/ paths on gems locals
  • encode out-of-range gemspec dates (before 1900) as the epoch instead of wrapping the Time#_dump year field
  • extend the ruby:3.3 e2e to yank, re-push different bytes and fail to install the yanked version
Yank hard-deleted a local gem version, so the same name-version-platform could be re-pushed with different bytes and break lockfiles pinning the old checksum. rubygems.org forbids re-pushing a yanked version. - write a `_yanked/<name>/<full>` tombstone before deleting the gem, quick spec and index line - reject a push of a tombstoned version with 409 - serialise push and yank of one version on the engine Redis lock; 503 when it stays busy - drop partial push writes on failure - return 404 for `_index/` and `_yanked/` paths on gems locals - encode out-of-range gemspec dates (before 1900) as the epoch instead of wrapping the Time#_dump year field - extend the ruby:3.3 e2e to yank, re-push different bytes and fail to install the yanked version
unkin-agent added 1 commit 2026-10-18 01:24:56 +11:00
Keep a tombstone on gem yank and reject re-pushing yanked versions
ci/woodpecker/pr/pre-commit Pipeline was successful
Details
ci/woodpecker/pr/test Pipeline was successful
Details
ci/woodpecker/pr/build Pipeline was successful
Details
4a6bd76976
unkin-agent commented 2026-10-18 01:28:45 +11:00
Author
Member
Copy Link
Copy Source
  • internal/api/v1/gems.go:82 — tombstone is checked once, before the gem/quick/index writes; a push that passed the check while a yank runs (tombstone + deletes) re-creates gem, quickspec and index afterwards, so a yanked version is live with its tombstone present → re-check the tombstone after the index write and roll back (delete the three paths, return 409), or make the index write conditional on the tombstone being absent.
  • internal/api/v1/gems_handler_test.go — no test for the push/yank interleaving above → add one (hook between check and index write, or concurrent push+yank loop under -race).
  • nit: internal/api/v1/proxy.go:260 — _yanked/<name>/<full> is served by the generic serveLocalFile GET like _index/ (200 with the index line) → deny _yanked/ and _index/ paths in the gems local GET.
- internal/api/v1/gems.go:82 — tombstone is checked once, before the gem/quick/index writes; a push that passed the check while a yank runs (tombstone + deletes) re-creates gem, quickspec and index afterwards, so a yanked version is live with its tombstone present → re-check the tombstone after the index write and roll back (delete the three paths, return 409), or make the index write conditional on the tombstone being absent. - internal/api/v1/gems_handler_test.go — no test for the push/yank interleaving above → add one (hook between check and index write, or concurrent push+yank loop under -race). - nit: internal/api/v1/proxy.go:260 — `_yanked/<name>/<full>` is served by the generic serveLocalFile GET like `_index/` (200 with the index line) → deny `_yanked/` and `_index/` paths in the gems local GET.
unkin-agent added 1 commit 2026-10-18 01:33:37 +11:00
Roll back a gem push that races a yank and hide gems bookkeeping paths
ci/woodpecker/pr/pre-commit Pipeline was successful
Details
ci/woodpecker/pr/test Pipeline was successful
Details
ci/woodpecker/pr/build Pipeline was successful
Details
960a5e09a3
unkin-agent commented 2026-10-18 01:36:55 +11:00
Author
Member
Copy Link
Copy Source
  • internal/api/v1/gems.go:104-120 — the rollback only runs after a successful index write; a push that passes the first tombstone check, then 409s on the .gem/quick write (different bytes vs a row yank has not deleted yet, e.g. yank deleted gems/ but not quick/) or errors, leaves its gems/<full>.gem row behind. The yanked version stays downloadable and the tombstone blocks any later cleanup → on any exit after gemsPushHook() other than success, re-check the tombstone and delete the paths this request created.
  • internal/api/v1/gems.go:129-136 — rollback runs on r.Context(); a client disconnect cancels the deletes, which are only logged, leaving the _index/ row (the visibility gate) live for a tombstoned version → use context.WithoutCancel(r.Context()) for the rollback deletes.
  • nit: internal/api/v1/gems.go:129 — a DB error in the recheck yanked() returns true and rolls back a legitimate push → distinguish error from hit, 500 without deleting.
- internal/api/v1/gems.go:104-120 — the rollback only runs after a successful index write; a push that passes the first tombstone check, then 409s on the `.gem`/quick write (different bytes vs a row yank has not deleted yet, e.g. yank deleted `gems/` but not `quick/`) or errors, leaves its `gems/<full>.gem` row behind. The yanked version stays downloadable and the tombstone blocks any later cleanup → on any exit after `gemsPushHook()` other than success, re-check the tombstone and delete the paths this request created. - internal/api/v1/gems.go:129-136 — rollback runs on `r.Context()`; a client disconnect cancels the deletes, which are only logged, leaving the `_index/` row (the visibility gate) live for a tombstoned version → use `context.WithoutCancel(r.Context())` for the rollback deletes. - nit: internal/api/v1/gems.go:129 — a DB error in the recheck `yanked()` returns true and rolls back a legitimate push → distinguish error from hit, 500 without deleting.
unkin-agent added 1 commit 2026-10-18 01:44:23 +11:00
Serialise gem push and yank per version with a Redis lock
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
d0fd53222c
benvin merged commit f9791d4b08 into master 2026-10-18 02:08:46 +11:00
benvin deleted branch benvin/gems-yank-tombstone 2026-10-18 02:08:46 +11: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) unkin-agent (Unkin Agent) unkinben (Ben Vincent)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: unkin/artifactapi#155
Delete Branch "benvin/gems-yank-tombstone"

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?