From f4acdd76eb0d7da5cecd2db6b107114e4d516b17 Mon Sep 17 00:00:00 2001 From: Evan Jarrett Date: Sun, 17 May 2026 15:24:34 -0500 Subject: [PATCH] fix purge manifest when repo owner deletes from UI --- pkg/hold/pds/auth.go | 72 ++++++++++++++++++++++++++++++++++++++++++++ pkg/hold/pds/xrpc.go | 20 +++++++++--- 2 files changed, 87 insertions(+), 5 deletions(-) diff --git a/pkg/hold/pds/auth.go b/pkg/hold/pds/auth.go index 77246bf..2eabad8 100644 --- a/pkg/hold/pds/auth.go +++ b/pkg/hold/pds/auth.go @@ -290,6 +290,78 @@ func ValidateOwnerOrCrewAdmin(r *http.Request, pds *HoldPDS, httpClient HTTPClie return nil, NewAuthError("crew:admin", "user is not a crew member", "crew:admin") } +// ValidateManifestPurger authenticates the caller and verifies they may purge +// the records associated with the given manifest URI. Authorization rules: +// 1. Hold captain (owner) — may purge any manifest's records. +// 2. Crew member with crew:admin permission — may purge any manifest's records. +// 3. Crew member whose DID matches the manifest URI's DID — may purge their +// own manifests (cleanup after delete-tag / delete-untagged). +// +// Rule 3 is what makes /delete-all-untagged work on shared/host holds where +// the calling sailor is crew with only blob:write — they're authorized to +// remove the layer/scan/config records they created. +func ValidateManifestPurger(r *http.Request, pds *HoldPDS, httpClient HTTPClient, manifestURI string) (*ValidatedUser, error) { + authHeader := r.Header.Get("Authorization") + var user *ValidatedUser + var err error + + if strings.HasPrefix(authHeader, "Bearer ") { + user, err = ValidateServiceToken(r, pds.did, httpClient) + if err != nil { + return nil, fmt.Errorf("service token authentication failed: %w", err) + } + } else if strings.HasPrefix(authHeader, "DPoP ") { + user, err = ValidateDPoPRequest(r, httpClient) + if err != nil { + return nil, fmt.Errorf("DPoP authentication failed: %w", err) + } + } else { + return nil, ErrInvalidAuthScheme + } + + _, captain, err := pds.GetCaptainRecord(r.Context()) + if err != nil { + return nil, fmt.Errorf("failed to get captain record: %w", err) + } + if user.DID == captain.Owner { + return user, nil + } + + _, crewRec, crewErr := pds.GetCrewMemberByDID(r.Context(), user.DID) + if crewErr != nil { + return nil, NewAuthError("manifest:purge", + "caller is not the hold owner or a crew member", + "captain", "crew:admin", "manifest-owner+crew") + } + + if slices.Contains(crewRec.Permissions, "crew:admin") { + return user, nil + } + + manifestDID := manifestURIDID(manifestURI) + if manifestDID != "" && user.DID == manifestDID { + return user, nil + } + + return nil, NewAuthError("manifest:purge", + "crew member may only purge their own manifests", + "crew:admin", "manifest-owner") +} + +// manifestURIDID extracts the DID from an at:// URI, or returns "" if the URI +// is malformed. Format: at:////. +func manifestURIDID(uri string) string { + rest, ok := strings.CutPrefix(uri, "at://") + if !ok { + return "" + } + i := strings.Index(rest, "/") + if i <= 0 { + return "" + } + return rest[:i] +} + // ValidateBlobWriteAccess validates that the request has valid authentication // and that the authenticated user is either the hold owner or a crew member with blob:write permission. // Supports two authentication methods: diff --git a/pkg/hold/pds/xrpc.go b/pkg/hold/pds/xrpc.go index cd26adb..ac09478 100644 --- a/pkg/hold/pds/xrpc.go +++ b/pkg/hold/pds/xrpc.go @@ -203,9 +203,13 @@ func (h *XRPCHandler) RegisterHandlers(r chi.Router) { r.Use(h.requireOwnerOrCrewAdmin) r.Post(atproto.RepoDeleteRecord, h.HandleDeleteRecord) r.Post(atproto.RepoUploadBlob, h.HandleUploadBlob) - r.Post(atproto.HoldPurgeManifest, h.HandlePurgeManifest) }) + // Manifest purge: auth happens inside the handler because it needs the + // manifest URI from the request body to allow the manifest's owner to + // purge their own records (in addition to captain / crew:admin). + r.Post(atproto.HoldPurgeManifest, h.HandlePurgeManifest) + // Auth-only endpoints (DPoP auth) r.Group(func(r chi.Router) { r.Use(h.requireAuth) @@ -893,10 +897,11 @@ func (h *XRPCHandler) HandleDeleteRecord(w http.ResponseWriter, r *http.Request) } // HandlePurgeManifest deletes layer, scan, and image-config records associated -// with a single manifest AT-URI. Idempotent. Auth: owner or crew admin -// (enforced by middleware). The manifest record itself lives in the user's PDS -// and is not affected. S3 blobs are not removed; the GC handles those based on -// remaining references and the labeler grace window. +// with a single manifest AT-URI. Idempotent. Auth (handled inline because it +// depends on the request body): captain, crew member with crew:admin, or the +// manifest's owner if they are currently crew. The manifest record itself +// lives in the user's PDS and is not affected. S3 blobs are not removed; the +// GC handles those based on remaining references and the labeler grace window. func (h *XRPCHandler) HandlePurgeManifest(w http.ResponseWriter, r *http.Request) { var input struct { ManifestURI string `json:"manifestUri"` @@ -914,6 +919,11 @@ func (h *XRPCHandler) HandlePurgeManifest(w http.ResponseWriter, r *http.Request return } + if _, err := ValidateManifestPurger(r, h.pds, h.httpClient, input.ManifestURI); err != nil { + http.Error(w, fmt.Sprintf("unauthorized: %v", err), http.StatusForbidden) + return + } + res, err := h.pds.PurgeManifestRecords(r.Context(), input.ManifestURI) if err != nil { http.Error(w, fmt.Sprintf("purge failed: %v", err), http.StatusInternalServerError)