Roll back a gem push that races a yank and hide gems bookkeeping paths
ci/woodpecker/pr/pre-commit Pipeline was successful
ci/woodpecker/pr/test Pipeline was successful
ci/woodpecker/pr/build Pipeline was successful

This commit is contained in:
2026-10-18 01:33:35 +11:00
parent 4a6bd76976
commit 960a5e09a3
3 changed files with 66 additions and 14 deletions
+31 -11
View File
@@ -28,7 +28,10 @@ import (
// _index/<name>/<full> compact index /info line; gates visibility
// _yanked/<name>/<full> yank tombstone; blocks re-push of the version
var gemsMaxPushBytes int64 = 256 << 20
var (
gemsMaxPushBytes int64 = 256 << 20
gemsPushHook = func() {}
)
const (
gemsIndexPrefix = "_index/"
@@ -79,22 +82,30 @@ func (h *ProxyHandler) handleLocalPost(w http.ResponseWriter, r *http.Request) {
conflict := func() {
gemsText(w, http.StatusConflict, fmt.Sprintf("%s already exists; overwrites are not allowed", full))
}
yanked, err := h.db.GetLocalFile(r.Context(), remote.Name, gemsYankedPrefix+spec.Name+"/"+full)
if err != nil {
gemsText(w, http.StatusInternalServerError, err.Error())
return
}
if yanked != nil {
gemsText(w, http.StatusConflict, fmt.Sprintf("%s was yanked; re-pushing a yanked version is not allowed", full))
index := gemsIndexPrefix + spec.Name + "/" + full
paths := []string{index, "gems/" + full + ".gem", "quick/Marshal.4.8/" + full + ".gemspec.rz"}
yanked := func() bool {
f, err := h.db.GetLocalFile(r.Context(), remote.Name, gemsYankedPrefix+spec.Name+"/"+full)
if err != nil {
gemsText(w, http.StatusInternalServerError, err.Error())
return true
}
if f != nil {
gemsText(w, http.StatusConflict, fmt.Sprintf("%s was yanked; re-pushing a yanked version is not allowed", full))
}
return f != nil
}
if yanked() {
return
}
gemsPushHook()
// The index line gates visibility, so it is written last.
for _, f := range []struct {
path string
data []byte
}{
{"gems/" + full + ".gem", gem},
{"quick/Marshal.4.8/" + full + ".gemspec.rz", quick},
{paths[1], gem},
{paths[2], quick},
} {
ok, err := h.npmStoreImmutable(r.Context(), remote.Name, f.path, f.data, "application/octet-stream")
if err != nil {
@@ -107,7 +118,7 @@ func (h *ProxyHandler) handleLocalPost(w http.ResponseWriter, r *http.Request) {
}
}
line := gems.InfoLine(spec, gem)
if _, err := h.npmStore(r.Context(), remote.Name, gemsIndexPrefix+spec.Name+"/"+full, []byte(line), "text/plain", false); err != nil {
if _, err := h.npmStore(r.Context(), remote.Name, index, []byte(line), "text/plain", false); err != nil {
if errors.Is(err, database.ErrAlreadyExists) {
conflict()
return
@@ -115,6 +126,15 @@ func (h *ProxyHandler) handleLocalPost(w http.ResponseWriter, r *http.Request) {
gemsText(w, http.StatusInternalServerError, err.Error())
return
}
// A yank that raced this push tombstoned the version before or after its deletes ran; undo our writes.
if yanked() {
for _, p := range paths {
if err := h.db.DeleteLocalFile(r.Context(), remote.Name, p); err != nil {
slog.Error("gem push rollback failed", "repo", remote.Name, "path", p, "error", err)
}
}
return
}
slog.Info("local gem pushed", "repo", remote.Name, "gem", full)
gemsText(w, http.StatusOK, "Successfully registered gem: "+spec.Name+" ("+spec.Version+")")
+26
View File
@@ -175,3 +175,29 @@ func TestGemsPushRejects(t *testing.T) {
t.Fatalf("oversized push = %d, want 413", code)
}
}
func TestGemsPushRacingYank(t *testing.T) {
h, srv := npmTestServer(t)
repo := npmRepo(t, h, models.PackageGems)
base := srv.URL + "/api/v1/local/" + repo + "/"
g := testGemFile(t, "race", "1.0.0", "ruby")
if code, body := npmDo(t, http.MethodPost, base+"api/v1/gems", string(g)); code != http.StatusOK {
t.Fatalf("push = %d: %s", code, body)
}
gemsPushHook = func() {
gemsPushHook = func() {}
if code, body := npmDo(t, http.MethodDelete, base+"api/v1/gems/yank?gem_name=race&version=1.0.0", ""); code != http.StatusOK {
t.Errorf("yank = %d: %s", code, body)
}
}
t.Cleanup(func() { gemsPushHook = func() {} })
if code, body := npmDo(t, http.MethodPost, base+"api/v1/gems", string(g)); code != http.StatusConflict || !strings.Contains(body, "yanked") {
t.Fatalf("push racing yank = %d %q, want 409", code, body)
}
for _, p := range []string{"info/race", "gems/race-1.0.0.gem", "quick/Marshal.4.8/race-1.0.0.gemspec.rz", "_index/race/race-1.0.0", "_yanked/race/race-1.0.0"} {
if code, _ := npmDo(t, http.MethodGet, base+p, ""); code != http.StatusNotFound {
t.Errorf("GET %s = %d, want 404", p, code)
}
}
}
+9 -3
View File
@@ -257,9 +257,15 @@ func (h *ProxyHandler) handleLocal(w http.ResponseWriter, r *http.Request) {
return
}
}
if remote.PackageType == models.PackageGems && isGemsIndexPath(path) {
h.serveLocalGemsIndex(w, r, remote, path)
return
if remote.PackageType == models.PackageGems {
if isGemsIndexPath(path) {
h.serveLocalGemsIndex(w, r, remote, path)
return
}
if strings.HasPrefix(path, gemsIndexPrefix) || strings.HasPrefix(path, gemsYankedPrefix) {
http.Error(w, "not found", http.StatusNotFound)
return
}
}
prov, _ := provider.Get(remote.PackageType)