fix(handler): close purge-vs-finalize race with in-lock site re-check

Author: raisedadead

internal/handler/deploy.go

@@ -13,6 +13,7 @@ import (
 
 	"github.com/freeCodeCamp/artemis/internal/gc"
 	"github.com/freeCodeCamp/artemis/internal/r2"
+	"github.com/freeCodeCamp/artemis/internal/registry"
 	"github.com/go-chi/chi/v5"
 )
 
@@ -232,6 +233,14 @@ func (h *Handlers) DeployFinalize(w http.ResponseWriter, r *http.Request) {
 
 	aliasKey := h.aliasKey(claims.Site, mode)
 	lockErr := h.withSiteLock(r.Context(), h.DeployPrefix.SiteDirname(claims.Site), func() error {
+		if _, err := h.Registry.GetSite(r.Context(), claims.Site); err != nil {
+			if errors.Is(err, registry.ErrNotFound) {
+				writeError(w, http.StatusGone, "site_gone", "site was deleted; deploy cannot be finalized")
+				return errAliasWriteHandled
+			}
+			writeUpstreamError(w, r, http.StatusBadGateway, "registry_read_failed", "registry.get.finalize", err)
+			return errAliasWriteHandled
+		}
 		if err := h.R2.PutAlias(r.Context(), aliasKey, deployID); err != nil {
 			writeUpstreamError(w, r, http.StatusBadGateway, "r2_put_failed", "r2.put.alias.finalize", err)
 			return errAliasWriteHandled

internal/handler/deploy_test.go

@@ -326,6 +326,35 @@ func TestDeployFinalize_VerifyThenAlias(t *testing.T) {
 	assert.Equal(t, deployID, alias)
 }
 
+func TestDeployFinalize_RejectsPurgedSiteUnderLock(t *testing.T) {
+	store := newFakeR2()
+	h, jwt := newTestHandlers(t, &fakeGH{}, standardSites(), store)
+
+	deployID := "20260420-141522-abc1234"
+	prefix := "www/deploys/" + deployID + "/"
+	store.objects[prefix+"index.html"] = []byte("<h1>hi</h1>")
+
+	require.NoError(t, h.Registry.Delete(context.Background(), "www"))
+
+	tok, _, err := jwt.Sign("alice", "www", deployID)
+	require.NoError(t, err)
+
+	body, _ := json.Marshal(DeployFinalizeRequest{Mode: "preview", Files: []string{"index.html"}})
+	w := withChiRoute(http.MethodPost, "/api/deploy/{deployId}/finalize",
+		"/api/deploy/"+deployID+"/finalize",
+		body,
+		map[string]string{"Authorization": "Bearer " + tok},
+		h.RequireDeployJWT(http.HandlerFunc(h.DeployFinalize)).ServeHTTP,
+		context.Background(),
+	)
+
+	require.Equal(t, http.StatusGone, w.Code, w.Body.String())
+	store.mu.Lock()
+	_, aliasWritten := store.aliases["www/preview"]
+	store.mu.Unlock()
+	assert.False(t, aliasWritten, "purged site must not get a resurrected alias even under a stale cache snapshot")
+}
+
 func TestFinalizeMarker(t *testing.T) {
 	store := newFakeR2()
 	h, jwt := newTestHandlers(t, &fakeGH{}, standardSites(), store)

internal/handler/site_delete_test.go

@@ -28,7 +28,7 @@ func callDelete(h *Handlers, slug, login, token string) *httptest.ResponseRecord
 }
 
 func TestSiteDelete_HappyPath(t *testing.T) {
-	h, _ := newTestHandlers(t, staffCallerGH(), standardSites(), newFakeR2())
+	h, _ := newTestHandlers(t, staffCallerGH(), &fakeSites{bySite: map[string][]string{}}, newFakeR2())
 
 	regBody, _ := json.Marshal(SiteRegisterRequest{Slug: "example", Teams: []string{"staff"}})
 	require.Equal(t, http.StatusCreated, callRegister(h, regBody, "alice", "tok").Code)

internal/handler/sites_list_test.go

@@ -21,7 +21,7 @@ func callSitesList(h *Handlers, login, token string) *httptest.ResponseRecorder
 }
 
 func TestSitesList_EmptyRegistry(t *testing.T) {
-	h, _ := newTestHandlers(t, staffCallerGH(), standardSites(), newFakeR2())
+	h, _ := newTestHandlers(t, staffCallerGH(), &fakeSites{bySite: map[string][]string{}}, newFakeR2())
 
 	w := callSitesList(h, "alice", "tok")
 	require.Equal(t, http.StatusOK, w.Code, w.Body.String())
@@ -32,7 +32,7 @@ func TestSitesList_EmptyRegistry(t *testing.T) {
 }
 
 func TestSitesList_PopulatedReturnsRowsSorted(t *testing.T) {
-	h, _ := newTestHandlers(t, staffCallerGH(), standardSites(), newFakeR2())
+	h, _ := newTestHandlers(t, staffCallerGH(), &fakeSites{bySite: map[string][]string{}}, newFakeR2())
 
 	for _, slug := range []string{"charlie", "alpha", "bravo"} {
 		body := []byte(`{"slug":"` + slug + `","teams":["staff"]}`)

internal/handler/test_helpers_test.go

@@ -25,6 +25,7 @@ type fakeRegistry struct {
 	fixedNow time.Time
 	// registerErr forces Register to return this error on the next call.
 	registerErr error
+	getErr      error
 }
 
 func newFakeRegistry() *fakeRegistry {
@@ -91,6 +92,20 @@ func (f *fakeRegistry) Delete(_ context.Context, slug string) error {
 	return nil
 }
 
+func (f *fakeRegistry) GetSite(_ context.Context, slug string) (registry.Site, error) {
+	if f.getErr != nil {
+		return registry.Site{}, f.getErr
+	}
+	site, ok := f.bySite[slug]
+	if !ok {
+		return registry.Site{}, registry.ErrNotFound
+	}
+	dup := make([]string, len(site.Teams))
+	copy(dup, site.Teams)
+	site.Teams = dup
+	return site, nil
+}
+
 func (f *fakeRegistry) Sites(_ context.Context) ([]registry.Site, error) {
 	out := make([]registry.Site, 0, len(f.bySite))
 	for _, s := range f.bySite {
@@ -120,6 +135,9 @@ func (e *erroringRegistry) Delete(_ context.Context, _ string) error {
 func (e *erroringRegistry) Sites(_ context.Context) ([]registry.Site, error) {
 	return nil, e.err
 }
+func (e *erroringRegistry) GetSite(_ context.Context, _ string) (registry.Site, error) {
+	return registry.Site{}, e.err
+}
 
 // fakeGH implements GitHubAuthenticator with deterministic behaviour.
 //
@@ -451,11 +469,15 @@ func trimPrefix(s, p string) string {
 func newTestHandlers(t *testing.T, gh *fakeGH, st *fakeSites, store R2Store) (*Handlers, *fakeJWT) {
 	t.Helper()
 	jwt := newFakeJWT(t)
+	reg := newFakeRegistry()
+	for slug, teams := range st.bySite {
+		_, _ = reg.Register(context.Background(), slug, teams, "test")
+	}
 	h := &Handlers{
 		GH:                 gh,
 		JWT:                jwt,
 		Sites:              st,
-		Registry:           newFakeRegistry(),
+		Registry:           reg,
 		R2:                 store,
 		AliasProductionFmt: "<site>/production",
 		AliasPreviewFmt:    "<site>/preview",

internal/pg/registry.go

@@ -133,6 +133,20 @@ func (s *RegistryStore) Import(ctx context.Context, src SitesSource) (int, error
 	return imported, nil
 }
 
+func (s *RegistryStore) GetSite(ctx context.Context, slug string) (registry.Site, error) {
+	var site registry.Site
+	err := s.pool.QueryRow(ctx,
+		`SELECT slug, teams, created_at, updated_at, created_by FROM sites WHERE slug = $1`,
+		slug).Scan(&site.Slug, &site.Teams, &site.CreatedAt, &site.UpdatedAt, &site.CreatedBy)
+	if errors.Is(err, pgx.ErrNoRows) {
+		return registry.Site{}, registry.ErrNotFound
+	}
+	if err != nil {
+		return registry.Site{}, fmt.Errorf("pg registry get %s: %w", slug, err)
+	}
+	return site, nil
+}
+
 func (s *RegistryStore) Sites(ctx context.Context) ([]registry.Site, error) {
 	rows, err := s.pool.Query(ctx,
 		`SELECT slug, teams, created_at, updated_at, created_by FROM sites ORDER BY slug`)

internal/pg/registry_test.go

@@ -38,6 +38,13 @@ func TestRegistryPG(t *testing.T) {
 	_, err = store.UpdateTeams(ctx, "absent", []string{"x"})
 	assert.ErrorIs(t, err, registry.ErrNotFound)
 
+	got, err := store.GetSite(ctx, "www")
+	require.NoError(t, err)
+	assert.Equal(t, "www", got.Slug)
+	assert.Equal(t, []string{"team-platform"}, got.Teams, "GetSite reflects the latest authoritative row")
+	_, err = store.GetSite(ctx, "absent")
+	assert.ErrorIs(t, err, registry.ErrNotFound)
+
 	_, err = store.Register(ctx, "learn", []string{"team-eng"}, "carol")
 	require.NoError(t, err)
 	sites, err := store.Sites(ctx)
@@ -49,6 +56,9 @@ func TestRegistryPG(t *testing.T) {
 	require.NoError(t, store.Delete(ctx, "www"))
 	assert.ErrorIs(t, store.Delete(ctx, "www"), registry.ErrNotFound, "double delete -> not found")
 
+	_, err = store.GetSite(ctx, "www")
+	assert.ErrorIs(t, err, registry.ErrNotFound, "GetSite after delete -> not found")
+
 	assert.Equal(t, []string{"www", "www", "learn", "www"}, changed,
 		"registry.changed fires on register/update/register/delete for Valkey cache invalidation")
 }

internal/registry/errors_test.go

@@ -50,6 +50,8 @@ func (fakeWriter) UpdateTeams(context.Context, string, []string) (Site, error) {
 
 func (fakeWriter) Delete(context.Context, string) error { return nil }
 
+func (fakeWriter) GetSite(context.Context, string) (Site, error) { return Site{}, nil }
+
 var (
 	_ Snapshot = fakeSnapshot{}
 	_ Reader   = fakeReader{}

internal/registry/types.go

@@ -45,6 +45,8 @@ type Writer interface {
 	// reflects the source-of-truth at call time.
 	Sites(ctx context.Context) ([]Site, error)
 
+	GetSite(ctx context.Context, slug string) (Site, error)
+
 	// Register creates a new site row and publishes a
 	// registry.changed event on success. Returns ErrAlreadyExists
 	// when slug is already registered.