auth: let over-quota users delete by granting the non-push subset

The quota gate ran on any scope containing "push" and denied the entire
token request, so "quota exceeded ... Delete images to free space" named
a remedy the gate itself blocked: docker and crane both request
pull,push,delete for a manifest delete, and manifest DELETE is
bearer-only, so there was no path left to free space.

When the request also asks for delete, drop push from the repository
entries and issue the reduced token instead of denying. A plain
pull,push is still denied so the quota message reaches the client that
needs to see it; granting a pushless token there would turn a clear
error into an opaque 401 on the first blob upload.

The narrowing happens in place on the access slice the handler hands to
the issuer, so document that on token.Authorizer along with the ordering
the gate goroutine depends on.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

pkg/appview/authgate/push_authorizer.go

@@ -118,7 +118,53 @@ func (a *Authorizer) Authorize(ctx context.Context, did, authMethod string, acce
 		}
 	}
 
-	return a.checkQuota(ctx, did, holdDID)
+	if err := a.checkQuota(ctx, did, holdDID); err != nil {
+		// Over quota. Denying outright would also deny the delete our own
+		// error text tells the user to perform, and OCI clients bundle the
+		// two (docker and crane both request pull,push,delete for a delete),
+		// so an over-quota user has no way to free space. Grant the non-push
+		// subset when delete was asked for. A plain push is still denied, so
+		// the quota message still reaches the client that needs to see it.
+		if dropPushForDelete(access) {
+			slog.Info("push gate: over quota, granting delete without push",
+				"did", did, "hold_did", holdDID, "reason", err)
+			return nil
+		}
+		return err
+	}
+	return nil
+}
+
+// dropPushForDelete removes the push action from every repository entry when
+// the request also asks for delete, reporting whether it did.
+//
+// The narrowing is in place because the caller passes this same slice to the
+// JWT issuer, so the reduced action set is what the token actually grants.
+// See the Authorizer contract in pkg/auth/token.
+func dropPushForDelete(access []auth.AccessEntry) bool {
+	requestsDelete := false
+	for _, entry := range access {
+		if entry.Type == "repository" && entry.Name != "*" && slices.Contains(entry.Actions, "delete") {
+			requestsDelete = true
+			break
+		}
+	}
+	if !requestsDelete {
+		return false
+	}
+
+	for i := range access {
+		if access[i].Type != "repository" {
+			continue
+		}
+		// Clone first: DeleteFunc compacts in place, so filtering the stored
+		// slice directly would scramble any other holder of that same array.
+		access[i].Actions = slices.DeleteFunc(
+			slices.Clone(access[i].Actions),
+			func(action string) bool { return action == "push" },
+		)
+	}
+	return true
 }
 
 // isCaptain returns true if userDID owns holdDID per the local Jetstream-

pkg/appview/authgate/push_authorizer_test.go

@@ -2,6 +2,7 @@ package authgate
 
 import (
 	"context"
+	"slices"
 	"strings"
 	"testing"
 
@@ -444,6 +445,102 @@ func TestAuthorize_NonCaptainPushOverQuotaDenied(t *testing.T) {
 	}
 }
 
+// An over-quota user has to be able to delete: the denial message tells them
+// to, and docker/crane ask for pull,push,delete on a delete. The gate grants
+// the non-push subset instead of failing the whole request.
+func TestAuthorize_OverQuotaGrantsDeleteWithoutPush(t *testing.T) {
+	atproto.SetTestMode(true)
+	t.Cleanup(func() { atproto.SetTestMode(false) })
+
+	srv := quotaServer(t, 200, `{"totalSize":1000,"limit":1000}`)
+	d := newTestDB(t)
+	seedUser(t, d, "did:plc:bob", "bob.test", srv.holdDID)
+	seedCaptain(t, d, srv.holdDID, "did:plc:alice")
+	seedCrewMember(t, d, srv.holdDID, "did:plc:bob", `["blob:write"]`)
+
+	a := New(d, fakeHoldAuthorizer{}, nil, "", WithHTTPClient(srv.httpClient()))
+	access := []auth.AccessEntry{
+		{Type: "repository", Name: "bob/x", Actions: []string{"pull", "push", "delete"}},
+	}
+	if err := a.Authorize(context.Background(), "did:plc:bob", "", access); err != nil {
+		t.Fatalf("Authorize(over quota, delete requested) = %v, want nil", err)
+	}
+	if got := access[0].Actions; !slices.Equal(got, []string{"pull", "delete"}) {
+		t.Errorf("granted actions = %v, want [pull delete]", got)
+	}
+}
+
+// Delete-only never carries push, so it must survive untouched.
+func TestAuthorize_OverQuotaAllowsDeleteOnlyScope(t *testing.T) {
+	atproto.SetTestMode(true)
+	t.Cleanup(func() { atproto.SetTestMode(false) })
+
+	srv := quotaServer(t, 200, `{"totalSize":1000,"limit":1000}`)
+	d := newTestDB(t)
+	seedUser(t, d, "did:plc:bob", "bob.test", srv.holdDID)
+	seedCaptain(t, d, srv.holdDID, "did:plc:alice")
+
+	a := New(d, fakeHoldAuthorizer{}, nil, "", WithHTTPClient(srv.httpClient()))
+	access := []auth.AccessEntry{
+		{Type: "repository", Name: "bob/x", Actions: []string{"delete"}},
+	}
+	if err := a.Authorize(context.Background(), "did:plc:bob", "", access); err != nil {
+		t.Fatalf("Authorize(delete only) = %v, want nil", err)
+	}
+	if got := access[0].Actions; !slices.Equal(got, []string{"delete"}) {
+		t.Errorf("granted actions = %v, want [delete]", got)
+	}
+	if srv.hits != 0 {
+		t.Errorf("quota endpoint hit %d times for delete-only scope, want 0", srv.hits)
+	}
+}
+
+// A plain push must keep failing loudly, otherwise the client never sees the
+// quota message and just gets an opaque 401 on the first blob upload.
+func TestAuthorize_OverQuotaStillDeniesPlainPush(t *testing.T) {
+	atproto.SetTestMode(true)
+	t.Cleanup(func() { atproto.SetTestMode(false) })
+
+	srv := quotaServer(t, 200, `{"totalSize":1000,"limit":1000}`)
+	d := newTestDB(t)
+	seedUser(t, d, "did:plc:bob", "bob.test", srv.holdDID)
+	seedCaptain(t, d, srv.holdDID, "did:plc:alice")
+	seedCrewMember(t, d, srv.holdDID, "did:plc:bob", `["blob:write"]`)
+
+	a := New(d, fakeHoldAuthorizer{}, nil, "", WithHTTPClient(srv.httpClient()))
+	access := pushAccess("bob/x")
+	err := a.Authorize(context.Background(), "did:plc:bob", "", access)
+	if err == nil || !strings.Contains(err.Error(), "quota exceeded") {
+		t.Fatalf("expected 'quota exceeded', got %v", err)
+	}
+	if got := access[0].Actions; !slices.Equal(got, []string{"pull", "push"}) {
+		t.Errorf("denied request should leave actions untouched, got %v", got)
+	}
+}
+
+// Under quota, a delete request keeps its push action.
+func TestAuthorize_UnderQuotaKeepsPushAlongsideDelete(t *testing.T) {
+	atproto.SetTestMode(true)
+	t.Cleanup(func() { atproto.SetTestMode(false) })
+
+	srv := quotaServer(t, 200, `{"totalSize":1,"limit":1000}`)
+	d := newTestDB(t)
+	seedUser(t, d, "did:plc:bob", "bob.test", srv.holdDID)
+	seedCaptain(t, d, srv.holdDID, "did:plc:alice")
+	seedCrewMember(t, d, srv.holdDID, "did:plc:bob", `["blob:write"]`)
+
+	a := New(d, fakeHoldAuthorizer{}, nil, "", WithHTTPClient(srv.httpClient()))
+	access := []auth.AccessEntry{
+		{Type: "repository", Name: "bob/x", Actions: []string{"pull", "push", "delete"}},
+	}
+	if err := a.Authorize(context.Background(), "did:plc:bob", "", access); err != nil {
+		t.Fatalf("Authorize(under quota) = %v, want nil", err)
+	}
+	if got := access[0].Actions; !slices.Equal(got, []string{"pull", "push", "delete"}) {
+		t.Errorf("granted actions = %v, want all three preserved", got)
+	}
+}
+
 func TestAuthorize_PullOnlySkipsMembershipAndQuota(t *testing.T) {
 	atproto.SetTestMode(true)
 	t.Cleanup(func() { atproto.SetTestMode(false) })

pkg/auth/token/handler.go

@@ -45,6 +45,13 @@ type Authorizer interface {
 	// allowed, or an error describing the denial reason. The error message
 	// surfaces to the OCI client inside the distribution error JSON body.
 	//
+	// Implementations may narrow `access` in place (dropping actions from an
+	// entry) to grant a subset rather than deny the whole request, and the
+	// issued JWT carries whatever survives. They must not widen it: every
+	// entry has already cleared ValidateAccess. The handler reads `access`
+	// only after draining the gate, so the narrowing is ordered before the
+	// JWT is signed — keep it that way if this call site moves.
+	//
 	// authMethod is one of AuthMethodOAuth or AuthMethodAppPassword and lets
 	// the implementation pick the right service-token fetcher when it needs
 	// to talk to the hold (e.g. for crew reconciliation).