feat(rpc): read and write bucket policies over /s3/bucket/policy

AGENTS.md

@@ -5,7 +5,7 @@ keys, and their buckets — plus the UCAN delegations and key material that back
 them. It exposes two APIs and talks to three external services:
 
 - **Tenant REST API** (`pkg/api`, echo) — partner-facing CRUD for tenants,
-  access keys, principals and bucket policies, guarded by a pre-shared partner
+  access keys and principals, guarded by a pre-shared partner
   key. `POST /tenants/{id}/access-keys` creates both key kinds. Without
   `principalId` it is a **service key**: it carries its own `permissions` and
   `buckets`, and the tenant→access-key delegations for them are issued at
@@ -21,7 +21,13 @@ them. It exposes two APIs and talks to three external services:
   revives it with no keys and named in no statement.
 - **Hilt UCAN RPC API** (`pkg/rpc`, ucantone server mounted at `POST /`) — the
   `/s3/*` commands Ingot (the S3 gateway) invokes: `/s3/request/authorize`,
-  `/s3/bucket/{create,delete,info,list}`; and the self-issued admin commands
+  `/s3/bucket/{create,delete,info,list,policy}`; and the self-issued admin
+  commands. `/s3/bucket/policy` carries the S3 GetBucketPolicy,
+  PutBucketPolicy and DeleteBucketPolicy operations (the request's method
+  selects), reached with a service key holding the matching permission; a
+  PUT's body must hash to the signed payload hash, and If-Match /
+  If-None-Match are optional signed headers (absent means unconditional).
+  The admin commands are
   `/admin/provider/{add,list}` and `/admin/provider/nodes/set` (`hilt client admin`).
 - **Sprue** (the Forge upload service) — Hilt calls it to provision/inspect a
   bucket's storage space and to manage routing policies (`pkg/client`): each

go.mod

@@ -11,7 +11,7 @@ require (
 	github.com/aws/smithy-go v1.27.2
 	github.com/docker/docker v28.5.2+incompatible
 	github.com/exaring/otelpgx v0.12.0
-	github.com/fil-forge/libforge v0.0.0-20260928151559-99540866a323
+	github.com/fil-forge/libforge v0.0.0-20261002123347-24293c113b04
 	github.com/fil-forge/smelt v0.0.0-20260914083257-f594cf02655c
 	github.com/fil-forge/swarf v0.0.1-0.20260922133653-0b2e5b2e1c6c
 	github.com/fil-forge/ucantone v0.0.0-20260924160040-c31dec73d9b3

go.sum

@@ -162,8 +162,8 @@ github.com/fatih/color v1.19.0 h1:Zp3PiM21/9Ld6FzSKyL5c/BULoe/ONr9KlbYVOfG8+w=
 github.com/fatih/color v1.19.0/go.mod h1:zNk67I0ZUT1bEGsSGyCZYZNrHuTkJJB+r6Q9VuMi0LE=
 github.com/felixge/httpsnoop v1.1.0 h1:3YtUj32ZZkqZtt3sZZsClsymw/QDuVfpNhoA31zeORc=
 github.com/felixge/httpsnoop v1.1.0/go.mod h1:Zqxgdd+1Rkcz8euOqdr7lqgCRJztwr5hp9vDSi5UZCE=
-github.com/fil-forge/libforge v0.0.0-20260928151559-99540866a323 h1:2hZKA/dxRN7+8Ku9M68irr0LqoHctyx6mpYtUn9rzIs=
-github.com/fil-forge/libforge v0.0.0-20260928151559-99540866a323/go.mod h1:sKml7ScjN2ov8F7jTuBIv823EgyWGTJG6cvA8ronPk4=
+github.com/fil-forge/libforge v0.0.0-20261002123347-24293c113b04 h1:uCEVq6CmYA8k2GONczR9uCpbWvhwCnCq1hbKsuROfYc=
+github.com/fil-forge/libforge v0.0.0-20261002123347-24293c113b04/go.mod h1:sKml7ScjN2ov8F7jTuBIv823EgyWGTJG6cvA8ronPk4=
 github.com/fil-forge/smelt v0.0.0-20260914083257-f594cf02655c h1:AEUEUEs/mDNksJIXdMgNMGJ03SHk6NlKjtDH58d1T04=
 github.com/fil-forge/smelt v0.0.0-20260914083257-f594cf02655c/go.mod h1:OUO2GxgUihmYfywNIfvWTQocg6jqaNiTd04cG5BbdII=
 github.com/fil-forge/swarf v0.0.1-0.20260922133653-0b2e5b2e1c6c h1:uzLK4I8rD2SXBTeb/qOMbnfD/oAr2RoFVP730abzzyo=

pkg/api/policies.go

@@ -48,7 +48,7 @@ func NewListPrincipalPoliciesHandler(logger *zap.Logger, policies *bucketpolicys
 		}
 		items := make([]PrincipalPolicy, len(recs))
 		for i, r := range recs {
-			items[i] = PrincipalPolicy{BucketName: r.BucketName, ETag: r.ETag, Policy: r.Policy}
+			items[i] = PrincipalPolicy{BucketName: r.BucketName, Policy: r.Policy}
 		}
 		return c.JSON(http.StatusOK, PrincipalPolicyList{Items: items})
 	})

pkg/api/policies_test.go

@@ -2,7 +2,9 @@ package api_test
 
 import (
 	"encoding/json"
+	"maps"
 	"net/http"
+	"slices"
 	"testing"
 
 	"github.com/fil-forge/hilt/internal/testutil"
@@ -29,6 +31,7 @@ type policyDeps struct {
 	principals *principalmemory.Store
 	policies   *bucketpolicysvc.Service
 	tenantID   did.DID // "tenant-1"
+	photos     did.DID
 }
 
 // setupPolicies serves every policy route over memory stores, with two tenants
@@ -40,10 +43,11 @@ func setupPolicies(t *testing.T) (*echo.Echo, *policyDeps) {
 		buckets:    bucketmemory.New(),
 		principals: principalmemory.New(),
 		tenantID:   testutil.RandomDID(t),
+		photos:     testutil.RandomDID(t),
 	}
 	require.NoError(t, tenants.Add(t.Context(), deps.tenantID, "tenant-1", testutil.RandomDID(t), tenant.Active))
 	require.NoError(t, tenants.Add(t.Context(), testutil.RandomDID(t), "tenant-2", testutil.RandomDID(t), tenant.Active))
-	require.NoError(t, deps.buckets.Add(t.Context(), testutil.RandomDID(t), deps.tenantID, "photos"))
+	require.NoError(t, deps.buckets.Add(t.Context(), deps.photos, deps.tenantID, "photos"))
 	require.NoError(t, deps.principals.Add(t.Context(), deps.tenantID, "user-1"))
 
 	// No principal holds a key here, so the grant rotator has nothing to
@@ -69,23 +73,30 @@ func allowUser1(actions ...string) bucketpolicy.Statement {
 // as the S3 PutBucketPolicy path does, and returns its ETag.
 func createPolicy(t *testing.T, deps *policyDeps, statements ...bucketpolicy.Statement) string {
 	t.Helper()
-	etag, _, err := deps.policies.Put(t.Context(), "tenant-1", "photos", bucketpolicy.Policy{Statements: statements}, nil)
+	etag, _, err := deps.policies.Write(t.Context(), deps.tenantID, deps.photos, "photos", bucketpolicy.Policy{Statements: statements}, bucketpolicysvc.IfNoneMatch())
 	require.NoError(t, err)
 	return etag
 }
 
 func TestPrincipalPolicyReadHandlers(t *testing.T) {
 	t.Run("lists the policies naming the principal", func(t *testing.T) {
 		e, deps := setupPolicies(t)
-		etag := createPolicy(t, deps, allowUser1("s3:GetObject"))
+		createPolicy(t, deps, allowUser1("s3:GetObject"))
 
 		rec := doRequest(t, e, http.MethodGet, "/tenants/tenant-1/principals/user-1/policies", nil)
 		require.Equal(t, http.StatusOK, rec.Code)
+		var raw struct {
+			Items []map[string]json.RawMessage `json:"items"`
+		}
+		require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &raw))
+		require.Len(t, raw.Items, 1)
+		require.ElementsMatch(t, []string{"bucketName", "policy"}, slices.Collect(maps.Keys(raw.Items[0])))
+
 		var list api.PrincipalPolicyList
 		require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &list))
-		require.Len(t, list.Items, 1)
-		require.Equal(t, "photos", list.Items[0].BucketName)
-		require.Equal(t, etag, list.Items[0].ETag)
+		require.Equal(t, []api.PrincipalPolicy{
+			{BucketName: "photos", Policy: bucketpolicy.Policy{Statements: []bucketpolicy.Statement{allowUser1("s3:GetObject")}}},
+		}, list.Items)
 	})
 
 	t.Run("reports the principal's effective actions per bucket", func(t *testing.T) {

pkg/api/service/bucketpolicy/service.go

@@ -1,5 +1,5 @@
 // Package bucketpolicy provides the business logic for bucket policies: the
-// compare-and-set reads and writes of a bucket's policy document, and the two
+// compare-and-set writes of a bucket's policy document, and the two
 // principal reads computed from it, the policies naming a principal and its
 // effective actions per bucket.
 //
@@ -46,7 +46,8 @@ type Access struct {
 	Actions []string
 }
 
-// Service implements the bucket policy operations shared by the REST handlers.
+// Service implements the bucket policy writes the /s3/bucket/policy handler
+// makes and the principal reads the REST handlers serve.
 type Service struct {
 	logger     *zap.Logger
 	tenants    tenantstore.Store
@@ -75,64 +76,44 @@ func New(
 	}
 }
 
-// Get returns the bucket's policy with the ETag its next write conditions on.
-func (s *Service) Get(ctx context.Context, externalID, bucketName string) (Record, error) {
-	tenantID, err := s.tenant(ctx, externalID)
-	if err != nil {
-		return Record{}, err
-	}
-	b, err := s.bucket(ctx, tenantID, bucketName)
-	if err != nil {
-		return Record{}, err
-	}
-	rec, err := s.policies.Get(ctx, b.ID)
-	if errors.Is(err, store.ErrRecordNotFound) {
-		return Record{}, ErrPolicyNotFound
-	} else if err != nil {
-		return Record{}, fmt.Errorf("reading the policy of bucket %q: %w", bucketName, err)
-	}
-	return Record{BucketName: bucketName, Policy: rec.Policy, ETag: rec.ETag}, nil
-}
-
-// WriteOption adjusts a [Service.Put].
+// WriteOption sets the precondition of a [Service.Write]. Without one the
+// write is unconditional, as a PutBucketPolicy without If-Match or
+// If-None-Match is.
 type WriteOption func(*bucketpolicystore.Input)
 
-// Unconditional makes the write ignore its precondition and replace whatever
-// the bucket holds, as a PutBucketPolicy without If-Match or If-None-Match
-// does.
-func Unconditional() WriteOption {
-	return func(in *bucketpolicystore.Input) { in.Unconditional = true }
+// IfMatch makes the write replace the bucket's policy only if its current ETag
+// is etag.
+func IfMatch(etag string) WriteOption {
+	return func(in *bucketpolicystore.Input) { in.IfMatch, in.Unconditional = &etag, false }
+}
+
+// IfNoneMatch makes the write create the bucket's policy only if the bucket
+// has none (If-None-Match: *).
+func IfNoneMatch() WriteOption {
+	return func(in *bucketpolicystore.Input) { in.IfMatch, in.Unconditional = nil, false }
 }
 
-// Put creates or replaces the bucket's policy. A nil ifMatch is the create
-// (If-None-Match: *) and requires the bucket to have no policy; a non-nil one
-// must equal the current ETag; [Unconditional] waives both. It returns the new
-// ETag and whether the call created the first policy for the bucket.
+// Write creates or replaces the bucket's policy, for a caller that has
+// resolved the tenant and the bucket. It validates doc against the tenant's
+// principals, and checks the precondition [IfMatch] or [IfNoneMatch] sets.
+// bucketName names the bucket in errors. It returns the new ETag and whether
+// the bucket had no policy before.
 //
 // The delegations of the principals the change affects are rotated inside the
 // store's transaction, so a publish failure leaves the old document in place.
-func (s *Service) Put(ctx context.Context, externalID, bucketName string, doc bucketpolicy.Policy, ifMatch *string, opts ...WriteOption) (string, bool, error) {
-	tenantID, err := s.tenant(ctx, externalID)
-	if err != nil {
-		return "", false, err
-	}
-	b, err := s.bucket(ctx, tenantID, bucketName)
-	if err != nil {
-		return "", false, err
-	}
+func (s *Service) Write(ctx context.Context, tenantID, bucketID did.DID, bucketName string, doc bucketpolicy.Policy, opts ...WriteOption) (string, bool, error) {
 	tenantPrincipals, err := s.principals.ListIDsByTenant(ctx, tenantID)
 	if err != nil {
 		return "", false, err
 	}
 	if err := bucketpolicy.Validate(doc, func(p string) bool { return slices.Contains(tenantPrincipals, p) }); err != nil {
 		return "", false, err
 	}
-
 	in := bucketpolicystore.Input{
-		Bucket:  b.ID,
-		Tenant:  tenantID,
-		Policy:  doc,
-		IfMatch: ifMatch,
+		Bucket:        bucketID,
+		Tenant:        tenantID,
+		Policy:        doc,
+		Unconditional: true,
 	}
 	for _, opt := range opts {
 		opt(&in)
@@ -161,35 +142,27 @@ func (s *Service) Put(ctx context.Context, externalID, bucketName string, doc bu
 				return fmt.Errorf("principal %q was removed: %w", p, store.ErrInvalidArgument)
 			}
 		}
-		return s.rotate(ctx, tenantID, b.ID, oldDoc, &doc, principals)
+		return s.rotate(ctx, tenantID, bucketID, oldDoc, &doc, principals)
 	})
 	if err != nil {
 		return "", false, s.writeError(ctx, bucketName, err)
 	}
-	s.logger.Info("wrote bucket policy",
-		zap.Stringer("tenant", tenantID), zap.String("bucket", bucketName), zap.Bool("created", created))
 	return etag, created, nil
 }
 
-// Delete removes the bucket's policy. ifMatch must equal the current ETag.
-// Every principal the policy reached loses its access, so each one's keys lose
-// their delegations over the bucket inside the store's transaction.
-func (s *Service) Delete(ctx context.Context, externalID, bucketName, ifMatch string) error {
-	tenantID, err := s.tenant(ctx, externalID)
-	if err != nil {
-		return err
-	}
-	b, err := s.bucket(ctx, tenantID, bucketName)
-	if err != nil {
-		return err
-	}
-	err = s.policies.Delete(ctx, b.ID, ifMatch, func(ctx context.Context, old bucketpolicystore.Record) error {
-		// Re-list under the bucket lock, as Put does.
+// Remove removes the bucket's policy, for a caller that has resolved the
+// tenant and the bucket. ifMatch must equal the current ETag. Every principal
+// the policy reached loses its access, so each one's keys lose their
+// delegations over the bucket inside the store's transaction. bucketName names
+// the bucket in errors and logs.
+func (s *Service) Remove(ctx context.Context, tenantID, bucketID did.DID, bucketName, ifMatch string) error {
+	err := s.policies.Delete(ctx, bucketID, ifMatch, func(ctx context.Context, old bucketpolicystore.Record) error {
+		// Re-list under the bucket lock, as Write does.
 		principals, err := s.principals.ListIDsByTenant(ctx, tenantID)
 		if err != nil {
 			return err
 		}
-		return s.rotate(ctx, tenantID, b.ID, &old.Policy, nil, principals)
+		return s.rotate(ctx, tenantID, bucketID, &old.Policy, nil, principals)
 	})
 	if errors.Is(err, store.ErrRecordNotFound) {
 		return ErrPolicyNotFound
@@ -319,19 +292,6 @@ func (s *Service) tenant(ctx context.Context, externalID string) (did.DID, error
 	return rec.ID, nil
 }
 
-// bucket resolves a bucket name within the tenant. A bucket another tenant
-// owns is reported as missing, so a policy read tells a caller nothing about
-// another tenant's names.
-func (s *Service) bucket(ctx context.Context, tenantID did.DID, name string) (bucketstore.Record, error) {
-	rec, err := s.buckets.GetByName(ctx, name)
-	if errors.Is(err, store.ErrRecordNotFound) || (err == nil && rec.Tenant != tenantID) {
-		return bucketstore.Record{}, ErrBucketNotFound
-	} else if err != nil {
-		return bucketstore.Record{}, fmt.Errorf("looking up bucket %q: %w", name, err)
-	}
-	return rec, nil
-}
-
 // bucketNames resolves the bucket DIDs the records carry to the names the API
 // addresses them by, in one listing.
 func (s *Service) bucketNames(ctx context.Context, tenantID did.DID, recs []bucketpolicystore.Record) (map[did.DID]string, error) {