diff --git a/internal/api/v1/gems.go b/internal/api/v1/gems.go index 3ce02b6..5d81de8 100644 --- a/internal/api/v1/gems.go +++ b/internal/api/v1/gems.go @@ -28,7 +28,10 @@ import ( // _index// compact index /info line; gates visibility // _yanked// 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+")") diff --git a/internal/api/v1/gems_handler_test.go b/internal/api/v1/gems_handler_test.go index fcbd6d6..1d56e9b 100644 --- a/internal/api/v1/gems_handler_test.go +++ b/internal/api/v1/gems_handler_test.go @@ -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) + } + } +} diff --git a/internal/api/v1/proxy.go b/internal/api/v1/proxy.go index 16ebc55..4c2f11f 100644 --- a/internal/api/v1/proxy.go +++ b/internal/api/v1/proxy.go @@ -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)