diff --git a/docs/diagrams.md b/docs/diagrams.md index 985bcfe6..ad8a2f0e 100644 --- a/docs/diagrams.md +++ b/docs/diagrams.md @@ -923,10 +923,13 @@ flowchart TB chain probe alone would let a put-only key read and abort on the bucket, or read a copy's source. - A copy's source and the headers that add a permission to a write (a lock - or tags on the version it creates) are decided from headers. Hilt's - `RequirementsFor` refuses a header the signature does not cover, as hilt - itself does, so the fast path decides nothing from such a request: it - goes to hilt, which refuses it as `UnsignedHeader`. + or tags on the version it creates) are decided from headers, and a + DeleteObjects on its body, which iam attaches for the operations hilt + names (`Operation.ReadsBody`) and no other. Hilt's `RequirementsFor` + refuses a header the signature does not cover or a body it does not bind, + as hilt itself does, so the fast path decides nothing from such a + request: it goes to hilt, which refuses it as `UnsignedHeader` or + `InvalidBody`. Cross-references: [`architecture.md` ยง9](./architecture.md#9-the-system-contract-piri--sprue--indexer). diff --git a/go.mod b/go.mod index e9a26180..d834bf08 100644 --- a/go.mod +++ b/go.mod @@ -12,9 +12,9 @@ require ( github.com/cenkalti/backoff/v5 v5.0.3 github.com/cloud-portable/s3tests/packages/go v0.0.0-20260916120817-ce77c26e7f6d github.com/exaring/otelpgx v0.12.0 - github.com/fil-forge/hilt v0.0.1-0.20261021163409-4fd8d7b920cf + github.com/fil-forge/hilt v0.0.1-0.20261022104055-6d996f93ee9e github.com/fil-forge/indexing-service v1.13.5-0.20260820151722-9eb620409a4e - github.com/fil-forge/libforge v0.0.0-20261020103720-b6aeeaf7c926 + github.com/fil-forge/libforge v0.0.0-20261022083349-59d8bde5f889 github.com/fil-forge/smelt v0.0.0-20261012172342-6eda2d3266f0 github.com/fil-forge/swarf v0.0.1-0.20260922133653-0b2e5b2e1c6c github.com/fil-forge/ucantone v0.0.0-20260924160040-c31dec73d9b3 diff --git a/go.sum b/go.sum index 36819864..d6330da9 100644 --- a/go.sum +++ b/go.sum @@ -183,12 +183,12 @@ github.com/fil-forge/automobile v0.0.1 h1:9xB3yc4l5b9EdRJSJcNwudgBFNHoMPEAdcb7Gf github.com/fil-forge/automobile v0.0.1/go.mod h1:TsO7jlO8ykJZY5tF8j4GsUcu3F02lEzxO7ULoB61hRA= github.com/fil-forge/go-ipni-tools v0.0.0-20260820150418-78ca0c16834d h1:iEXJC5aJgRHq2iM4X6usfow436OQGauLjDuga1Hiq4o= github.com/fil-forge/go-ipni-tools v0.0.0-20260820150418-78ca0c16834d/go.mod h1:fwLjqz34DvfoWcuzyH8Wr7dTJdr3mzYjxZRweysHOH4= -github.com/fil-forge/hilt v0.0.1-0.20261021163409-4fd8d7b920cf h1:jpKCNaF5iiDIxfNW71O6ZBlcF+cdiPJCgeFPwVj10oc= -github.com/fil-forge/hilt v0.0.1-0.20261021163409-4fd8d7b920cf/go.mod h1:E75kjv9Yg0I6VPf0ghWQ8ziYwnY5UM7iSCRm2Hgt8cY= +github.com/fil-forge/hilt v0.0.1-0.20261022104055-6d996f93ee9e h1:EsJ7N8SI/XLMvnpqdVZuj/7/s1PrUVGW4oACb8Lntgg= +github.com/fil-forge/hilt v0.0.1-0.20261022104055-6d996f93ee9e/go.mod h1:O0ULbedtoH4Zixa13p4/3FbWyZUArbpOtWgEgs/IclU= github.com/fil-forge/indexing-service v1.13.5-0.20260820151722-9eb620409a4e h1:+A26/La8EgF1bsGhSGicYV97ckec1SJwNYLWee3Xo8g= github.com/fil-forge/indexing-service v1.13.5-0.20260820151722-9eb620409a4e/go.mod h1:hrry50c9bNciNj9Z63O2QFrzidnTjkYYG2UgH4/rmVo= -github.com/fil-forge/libforge v0.0.0-20261020103720-b6aeeaf7c926 h1:i6Zr11ukpOvJx80T06okJidomSv7DJCj5mSGdOy8ltk= -github.com/fil-forge/libforge v0.0.0-20261020103720-b6aeeaf7c926/go.mod h1:ZtPwUgXm7gqQSmjmIcfMy5AwWpFIR02WMZvbgNOwi/4= +github.com/fil-forge/libforge v0.0.0-20261022083349-59d8bde5f889 h1:lmjXiMsVFHsln4hmSjtuJxsHhLP49WOp48Ot5KPctMQ= +github.com/fil-forge/libforge v0.0.0-20261022083349-59d8bde5f889/go.mod h1:ZtPwUgXm7gqQSmjmIcfMy5AwWpFIR02WMZvbgNOwi/4= github.com/fil-forge/smelt v0.0.0-20261012172342-6eda2d3266f0 h1:q8V6kT/wfDTvnVK5znv0zWKDXrWrIk91xzcYrEgXv9g= github.com/fil-forge/smelt v0.0.0-20261012172342-6eda2d3266f0/go.mod h1:D2ILnC2lBNYav4+y4701sGDClyfsWAfFy19D++6rYP8= github.com/fil-forge/swarf v0.0.1-0.20260922133653-0b2e5b2e1c6c h1:uzLK4I8rD2SXBTeb/qOMbnfD/oAr2RoFVP730abzzyo= diff --git a/iam/service.go b/iam/service.go index d1ff343b..2076c30a 100644 --- a/iam/service.go +++ b/iam/service.go @@ -187,6 +187,25 @@ func (s *Service) GetUserAccountForRequest(ctx fiber.Ctx, accessKeyStr string) ( // so the Hilt round-trip is bounded by the request's lifetime. reqCtx := ctx.RequestCtx() req := fasthttputil.RequestFromHTTPContext(reqCtx) + // The body travels to Hilt only for the operations whose body decides a + // permission (a DeleteObjects' entries may name versions), which Hilt's + // classifier names; every other request goes without one, so a streaming + // write's body is never read here. It is captured up to Hilt's limit and + // no further, since nothing about the request is verified yet, and left + // in place for the handler. Hilt binds it to the signature before reading + // it, and so does the fast path. + if op, err := hiltauth.OperationFor(req); err == nil && op.ReadsBody() { + body, err := fasthttputil.LimitedBody(reqCtx, hiltauth.MaxBodySize) + if errors.Is(err, fasthttputil.ErrBodyTooLarge) { + path = "local" + s.logger.Debug("hilt/iam: request body over the limit", zap.String("access", accessKeyStr), zap.Stringer("operation", op)) + return auth.Account{}, s3err.GetAPIError(s3err.ErrEntityTooLarge) + } + if err != nil { + return auth.Account{}, fmt.Errorf("hilt/iam: %w", err) + } + req.Body = body + } ctx.Locals(reqscope.RequestKey(), req) // A query naming two subresources addresses no operation. S3 refuses it @@ -383,9 +402,10 @@ func mapAuthError(err error) (error, bool) { // Another tenant's bucket is AccessDenied, as S3 answers for another // account's bucket: bucket names are global, so existence is no secret. hiltauth.ForeignBucketErrorName, - // A copy whose source header the signature does not cover cannot be - // authorized; the SDKs always sign it. - hiltauth.UnsignedHeaderErrorName: + // A header the signature does not cover, or a body it does not bind, + // cannot be authorized on; the SDKs sign both. + hiltauth.UnsignedHeaderErrorName, + hiltauth.InvalidBodyErrorName: return s3err.GetAPIError(s3err.ErrAccessDenied), true case hiltauth.TemporarilyUnavailableErrorName: // An authorize that waited out Hilt's lock timeout behind a policy diff --git a/iam/service_internal_test.go b/iam/service_internal_test.go index 0dc6d7a3..be9666df 100644 --- a/iam/service_internal_test.go +++ b/iam/service_internal_test.go @@ -2,6 +2,8 @@ package iam import ( "context" + "crypto/sha256" + "encoding/hex" "errors" "fmt" "net/http" @@ -476,6 +478,105 @@ func signedRequest(t *testing.T, method, host, rawPath, accessKeyID, secret stri return s3.Request{Method: method, Headers: headers, URL: req.URL.RequestURI()} } +// signedBody builds a real SigV4-signed POST of body to path with the payload +// hash signed, as the SDKs sign a DeleteObjects, and attaches the body. +func signedBody(t *testing.T, host, path, accessKeyID, secret, body string) s3.Request { + t.Helper() + sum := sha256.Sum256([]byte(body)) + payloadHash := hex.EncodeToString(sum[:]) + req, err := http.NewRequest(http.MethodPost, "http://"+host+path, strings.NewReader(body)) + require.NoError(t, err) + req.Header.Set("X-Amz-Content-Sha256", payloadHash) + _, err = v4.NewSigner().SignHTTP(context.Background(), + awsv4.Credentials{AccessKeyID: accessKeyID, SecretAccessKey: secret}, + req, payloadHash, "s3", "us-east-1", time.Now(), + []string{"host", "x-amz-content-sha256", "x-amz-date"}, + func(o *v4.SignerOptions) { o.DisableURIPathEscaping = true }) + require.NoError(t, err) + headers := map[string]string{"Host": host} + for k := range req.Header { + headers[k] = req.Header.Get(k) + } + return s3.Request{Method: http.MethodPost, Headers: headers, URL: req.URL.RequestURI(), Body: []byte(body)} +} + +// TestDeleteOnlyKeyIsDeniedOnVersions covers the body the fast path decides +// on: a DeleteObjects entry naming a version needs s3:DeleteObjectVersion, +// which Hilt's classifier reads from the body once the signature binds it. +// A key whose cached set holds s3:DeleteObject alone is refused locally, +// and a tampered body is left to Hilt. +func TestDeleteOnlyKeyIsDeniedOnVersions(t *testing.T) { + accessKey, err := ed25519.GenerateIssuer() + require.NoError(t, err) + spaceIssuer, err := ed25519.GenerateIssuer() + require.NoError(t, err) + agent, err := ed25519.GenerateIssuer() + require.NoError(t, err) + + accessKeyID := accessKey.DID().Identifier() + const secret = "test-secret-access-key" + space := spaceIssuer.DID() + resolver := fixedResolver{name: "bkt", state: ®istry.State{Name: "bkt", Space: space}} + + service := func(t *testing.T, req s3.Request, actions ...string) *Service { + t.Helper() + sr, err := sigv4.Parse(sigv4.Request{Method: req.Method, Headers: req.Headers, URL: req.URL}) + require.NoError(t, err) + key, err := sigv4.DeriveKey(sr, secret) + require.NoError(t, err) + s := localService(agent.DID(), resolver) + s.keys.Put(accessKeyID, time.Hour, s3.VerificationKey{Kind: s3.KeyKindSigV4, Data: key}) + s.proofs.For(accessKey.DID()).PutPermissions(space, time.Hour, actions) + for _, a := range actions { + s.proofs.Deposit(accessKey.DID(), commandChains(t, spaceIssuer, accessKey, agent, s3perm.CommandsFor(a))...) + } + return s + } + const versions = `abv1` + const plain = `a` + + t.Run("a version entry is refused on the cached set", func(t *testing.T) { + req := signedBody(t, "s3.example", "/bkt?delete", accessKeyID, secret, versions) + op, reqs, err := hiltauth.RequirementsFor(req) + require.NoError(t, err) + require.Equal(t, hiltauth.OpDeleteObjects, op) + require.Len(t, reqs, 2) + + s := service(t, req, "s3:DeleteObject") + _, ok, err := s.authorizeLocal(context.Background(), req, accessKeyID, s.proofs.For(accessKey.DID())) + require.False(t, ok) + require.Equal(t, s3err.GetAPIError(s3err.ErrAccessDenied), err) + require.Zero(t, s.authorizer.(*refusingAuthorizer).calls) + }) + + t.Run("plain entries are still authorized", func(t *testing.T) { + req := signedBody(t, "s3.example", "/bkt?delete", accessKeyID, secret, plain) + s := service(t, req, "s3:DeleteObject") + _, ok, err := s.authorizeLocal(context.Background(), req, accessKeyID, s.proofs.For(accessKey.DID())) + require.NoError(t, err) + require.True(t, ok) + }) + + t.Run("the version permission authorizes the batch", func(t *testing.T) { + req := signedBody(t, "s3.example", "/bkt?delete", accessKeyID, secret, versions) + s := service(t, req, "s3:DeleteObject", "s3:DeleteObjectVersion") + _, ok, err := s.authorizeLocal(context.Background(), req, accessKeyID, s.proofs.For(accessKey.DID())) + require.NoError(t, err) + require.True(t, ok) + }) + + t.Run("a body the signature does not bind is left to Hilt", func(t *testing.T) { + req := signedBody(t, "s3.example", "/bkt?delete", accessKeyID, secret, plain) + req.Body = []byte(versions) + _, _, err := hiltauth.RequirementsFor(req) + require.ErrorIs(t, err, hiltauth.ErrInvalidBody) + s := service(t, req, "s3:DeleteObject", "s3:DeleteObjectVersion") + _, ok, err := s.authorizeLocal(context.Background(), req, accessKeyID, s.proofs.For(accessKey.DID())) + require.False(t, ok) + require.NoError(t, err) + }) +} + // TestPutOnlyKeyIsDeniedOnReads pins the defect the action set closes: a key // granted only s3:PutObject holds chains for every command a read, a listing // or a multipart abort needs (PutObject's command set is a superset of each), @@ -681,6 +782,7 @@ func TestMapAuthError(t *testing.T) { hiltauth.BucketNotPermittedErrorName: {"AccessDenied", 403}, hiltauth.ForeignBucketErrorName: {"AccessDenied", 403}, hiltauth.UnsignedHeaderErrorName: {"AccessDenied", 403}, + hiltauth.InvalidBodyErrorName: {"AccessDenied", 403}, hiltauth.TemporarilyUnavailableErrorName: {"ServiceUnavailable", 503}, s3bkt.InvalidPolicyErrorName: {"MalformedPolicy", 400}, } diff --git a/iam/service_test.go b/iam/service_test.go index 6754e5b6..50b90d41 100644 --- a/iam/service_test.go +++ b/iam/service_test.go @@ -1,10 +1,12 @@ package iam_test import ( + "bytes" "context" "errors" "net/http" "net/http/httptest" + "strings" "testing" "time" @@ -24,6 +26,7 @@ import ( "github.com/stretchr/testify/require" hiltclient "github.com/fil-forge/hilt/pkg/client" + hiltauth "github.com/fil-forge/hilt/pkg/rpc/service/auth" "github.com/fil-forge/ingot/iam" "github.com/fil-forge/ingot/internal/reqscope" ) @@ -108,17 +111,73 @@ func signedCopyRequest(t *testing.T, dst, src string) *http.Request { func resolveForRequest(t *testing.T, svc *iam.Service, access string, req *http.Request) (auth.Account, error) { t.Helper() - app := fiber.New() + acct, _, err := resolveStreamed(t, svc, access, req, false) + return acct, err +} + +// resolveStreamed runs the request through the gateway's auth step as +// resolveForRequest does, streaming the request body as the production +// server does when streamed is set, and returns the body the handler then +// reads. +func resolveStreamed(t *testing.T, svc *iam.Service, access string, req *http.Request, streamed bool) (auth.Account, []byte, error) { + t.Helper() + app := fiber.New(fiber.Config{StreamRequestBody: streamed}) var acct auth.Account var authErr error + var seen []byte app.Use(func(c fiber.Ctx) error { acct, authErr = svc.GetUserAccountForRequest(c, access) + seen = bytes.Clone(c.Body()) return nil }) resp, err := app.Test(req) require.NoError(t, err) require.NoError(t, resp.Body.Close()) - return acct, authErr + return acct, seen, authErr +} + +// TestRequestBodyForwarding covers the body hilt decides on, end to end +// through the gateway's auth step with a streamed request: a DeleteObjects +// body reaches the authorizer exactly and is left for the handler, any +// other operation's body is neither forwarded nor consumed, and an +// oversized DeleteObjects body is refused before the authorizer is asked. +func TestRequestBodyForwarding(t *testing.T) { + access, keyDID := newAccessKey(t) + key := s3.VerificationKey{Kind: s3.KeyKindSigV4, Data: []byte("dk")} + const body = `kv` + + t.Run("a DeleteObjects body is forwarded and left for the handler", func(t *testing.T) { + fake := &fakeAuthorizer{res: authorizeOK(t, keyDID, key)} + svc := iam.New(fake, iam.NewKeyProofs(), iam.NewVerificationKeyCache(), iam.NewTenantCache()) + req := httptest.NewRequest(http.MethodPost, "http://example.com/bkt?delete", strings.NewReader(body)) + _, seen, err := resolveStreamed(t, svc, access, req, true) + require.NoError(t, err) + require.Equal(t, 1, fake.calls) + require.Equal(t, []byte(body), fake.got.Body, "the authorizer must see the exact body") + require.Equal(t, []byte(body), seen, "the handler must read the same body") + }) + + t.Run("another operation's body is neither forwarded nor consumed", func(t *testing.T) { + fake := &fakeAuthorizer{res: authorizeOK(t, keyDID, key)} + svc := iam.New(fake, iam.NewKeyProofs(), iam.NewVerificationKeyCache(), iam.NewTenantCache()) + req := httptest.NewRequest(http.MethodPut, "http://example.com/bkt/k", strings.NewReader("object bytes")) + _, seen, err := resolveStreamed(t, svc, access, req, true) + require.NoError(t, err) + require.Nil(t, fake.got.Body) + require.Equal(t, []byte("object bytes"), seen) + }) + + t.Run("an oversized DeleteObjects body is refused before the authorizer is asked", func(t *testing.T) { + fake := &fakeAuthorizer{res: authorizeOK(t, keyDID, key)} + svc := iam.New(fake, iam.NewKeyProofs(), iam.NewVerificationKeyCache(), iam.NewTenantCache()) + huge := strings.Repeat("x", hiltauth.MaxBodySize+1) + req := httptest.NewRequest(http.MethodPost, "http://example.com/bkt?delete", strings.NewReader(huge)) + _, _, err := resolveStreamed(t, svc, access, req, true) + var s3e s3err.APIError + require.ErrorAs(t, err, &s3e) + require.Equal(t, s3err.GetAPIError(s3err.ErrEntityTooLarge).Code, s3e.Code) + require.Zero(t, fake.calls) + }) } func TestGetUserAccountForRequest(t *testing.T) { diff --git a/internal/fasthttputil/request.go b/internal/fasthttputil/request.go index 36d7bd84..67ae5f13 100644 --- a/internal/fasthttputil/request.go +++ b/internal/fasthttputil/request.go @@ -1,6 +1,11 @@ package fasthttputil import ( + "bytes" + "errors" + "fmt" + "io" + "github.com/fil-forge/libforge/commands/s3" "github.com/valyala/fasthttp" ) @@ -35,3 +40,33 @@ func RequestFromHTTPContext(rc *fasthttp.RequestCtx) s3.Request { Headers: headers, } } + +// ErrBodyTooLarge is returned by LimitedBody for a body over its limit. +var ErrBodyTooLarge = errors.New("request body exceeds the limit") + +// LimitedBody captures the request's body, at most limit bytes, as the +// caller's own copy, and leaves it in place for the handler. A streamed body +// (the gateway streams every request body) is read from the stream only up +// to one byte past the limit, so an oversized body costs the limit and no +// more before it is refused; within the limit it is set back on the +// request, so the handler's Body() serves it. An in-memory body over the +// limit is refused too. +func LimitedBody(rc *fasthttp.RequestCtx, limit int) ([]byte, error) { + req := &rc.Request + if req.IsBodyStream() { + buf, err := io.ReadAll(io.LimitReader(req.BodyStream(), int64(limit)+1)) + if err != nil { + return nil, fmt.Errorf("reading request body: %w", err) + } + if len(buf) > limit { + return nil, ErrBodyTooLarge + } + req.SetBody(buf) + return bytes.Clone(buf), nil + } + body := req.Body() + if len(body) > limit { + return nil, ErrBodyTooLarge + } + return bytes.Clone(body), nil +} diff --git a/internal/fasthttputil/request_test.go b/internal/fasthttputil/request_test.go index 1a5732b8..e7b25a3b 100644 --- a/internal/fasthttputil/request_test.go +++ b/internal/fasthttputil/request_test.go @@ -1,6 +1,9 @@ package fasthttputil import ( + "errors" + "io" + "strings" "testing" "github.com/valyala/fasthttp" @@ -41,3 +44,65 @@ func TestRequestFromHTTPContext_SingleHeader(t *testing.T) { t.Errorf("Content-Encoding = %q, want %q", v, "gzip") } } + +// TestLimitedBody covers the capture of a body hilt decides on: a streamed +// body within the limit is returned and left for the handler, one over it is +// refused after reading one byte past the limit, and an in-memory body is +// bounded the same way. +func TestLimitedBody(t *testing.T) { + t.Run("streamed within the limit", func(t *testing.T) { + var rc fasthttp.RequestCtx + rc.Request.Header.SetMethod("POST") + rc.Request.SetBodyStream(strings.NewReader(""), -1) + got, err := LimitedBody(&rc, 16) + if err != nil { + t.Fatal(err) + } + if string(got) != "" { + t.Errorf("body = %q", got) + } + if rc.Request.IsBodyStream() || string(rc.Request.Body()) != "" { + t.Errorf("handler body = %q, stream=%v", rc.Request.Body(), rc.Request.IsBodyStream()) + } + got[0] = 'x' + if string(rc.Request.Body()) != "" { + t.Error("the returned body must be the caller's own copy") + } + }) + t.Run("streamed over the limit", func(t *testing.T) { + var rc fasthttp.RequestCtx + rc.Request.Header.SetMethod("POST") + src := &countingReader{Reader: strings.NewReader(strings.Repeat("x", 100))} + rc.Request.SetBodyStream(src, -1) + _, err := LimitedBody(&rc, 16) + if !errors.Is(err, ErrBodyTooLarge) { + t.Fatalf("err = %v, want ErrBodyTooLarge", err) + } + if src.n > 17 { + t.Errorf("read %d bytes of a refused body, want at most 17", src.n) + } + }) + t.Run("in memory", func(t *testing.T) { + var rc fasthttp.RequestCtx + rc.Request.SetBody([]byte("")) + got, err := LimitedBody(&rc, 16) + if err != nil || string(got) != "" { + t.Fatalf("got %q, %v", got, err) + } + rc.Request.SetBody([]byte(strings.Repeat("x", 17))) + if _, err := LimitedBody(&rc, 16); !errors.Is(err, ErrBodyTooLarge) { + t.Fatalf("err = %v, want ErrBodyTooLarge", err) + } + }) +} + +type countingReader struct { + io.Reader + n int +} + +func (c *countingReader) Read(p []byte) (int, error) { + n, err := c.Reader.Read(p) + c.n += n + return n, err +} diff --git a/itest/forge_route_perms_test.go b/itest/forge_route_perms_test.go index 977080a0..1e01db85 100644 --- a/itest/forge_route_perms_test.go +++ b/itest/forge_route_perms_test.go @@ -20,9 +20,10 @@ import ( // permissions is refused PutBucketVersioning and DeleteObjects on an // existing bucket, since each is authorized on its own permission and // against that bucket's scope; -// - DeleteObjects is authorized on s3:DeleteObject, and an entry naming a -// version is refused, and the version kept, unless the key also holds -// s3:DeleteObjectVersion. +// - DeleteObjects is authorized on s3:DeleteObject, and a batch with an +// entry naming a version is refused as a whole, nothing in it deleted, +// unless the key also holds s3:DeleteObjectVersion: hilt decides the +// request on its body, where S3 would refuse the entry alone. // // Each denied key first makes an allowed request, so the second attempt at a // denial runs against ingot's caches and the local fast path must refuse @@ -91,22 +92,35 @@ func TestForgeRoutePermissions(t *testing.T) { deleter := client("deleter", withoutVersion) v1 := put("kept") put("marked") + // An allowed request first, so the key's action set and chains are + // cached and the second denial below runs against them. + if _, err := deleteObjects(deleter, types.ObjectIdentifier{Key: aws.String("probe")}); err != nil { + t.Fatalf("priming DeleteObjects with s3:DeleteObject: %v", err) + } - out, err := deleteObjects(deleter, - types.ObjectIdentifier{Key: aws.String("kept"), VersionId: aws.String(v1)}, - types.ObjectIdentifier{Key: aws.String("marked")}, - ) - if err != nil { - t.Fatalf("DeleteObjects with s3:DeleteObject: %v", err) + // The mixed batch is refused as a whole, once through hilt and once + // against the caches, with nothing in it deleted. + denied(t, "delete-only key naming a version", func() error { + _, err := deleteObjects(deleter, + types.ObjectIdentifier{Key: aws.String("kept"), VersionId: aws.String(v1)}, + types.ObjectIdentifier{Key: aws.String("marked")}, + ) + return err + }) + if _, err := full.HeadObject(ctx, &s3.HeadObjectInput{Bucket: aws.String(bucket), Key: aws.String("kept"), VersionId: aws.String(v1)}); err != nil { + t.Fatalf("refused version is gone: %v", err) } - if len(out.Errors) != 1 || aws.ToString(out.Errors[0].Key) != "kept" || aws.ToString(out.Errors[0].Code) != "AccessDenied" { - t.Fatalf("errors = %+v, want AccessDenied for \"kept\" alone", out.Errors) + if _, err := full.HeadObject(ctx, &s3.HeadObjectInput{Bucket: aws.String(bucket), Key: aws.String("marked")}); err != nil { + t.Fatalf("\"marked\" was deleted by a refused batch: %v", err) } - if len(out.Deleted) != 1 || aws.ToString(out.Deleted[0].Key) != "marked" || !aws.ToBool(out.Deleted[0].DeleteMarker) { - t.Fatalf("deleted = %+v, want a delete marker for \"marked\"", out.Deleted) + + // The plain batch is authorized on s3:DeleteObject alone. + out, err := deleteObjects(deleter, types.ObjectIdentifier{Key: aws.String("marked")}) + if err != nil { + t.Fatalf("DeleteObjects with s3:DeleteObject: %v", err) } - if _, err := full.HeadObject(ctx, &s3.HeadObjectInput{Bucket: aws.String(bucket), Key: aws.String("kept"), VersionId: aws.String(v1)}); err != nil { - t.Fatalf("refused version is gone: %v", err) + if len(out.Errors) != 0 || len(out.Deleted) != 1 || aws.ToString(out.Deleted[0].Key) != "marked" || !aws.ToBool(out.Deleted[0].DeleteMarker) { + t.Fatalf("result = deleted %+v, errors %+v; want a delete marker for \"marked\"", out.Deleted, out.Errors) } versionDeleter := client("version-deleter", []string{"s3:DeleteObject", "s3:DeleteObjectVersion"})