From 35f947f7e9129755d04522be9f61c91a594a4aea Mon Sep 17 00:00:00 2001 From: Alan Shaw Date: Mon, 19 Oct 2026 18:10:33 +0100 Subject: [PATCH] feat(s3/bucket): allow tagging, attribute and bucket configuration reads in policies Hilt is giving each S3 operation the gateway serves its own permission instead of letting unrecognized subresources fall through to the bare bucket or object operation. A principal could reach these through s3:ListBucket, s3:GetObject and the plain object writes, so a policy has to be able to grant them by name: object tagging, GetObjectAttributes, and reads of the bucket's ACL, CORS, location, object lock, tagging and versioning configuration. The configuration writes stay out, since they change the bucket for every principal. Co-Authored-By: Claude Opus 5.5 --- commands/s3/bucket/policy_json.go | 22 ++++++++++++++++++++-- commands/s3/bucket/policy_test.go | 6 +++++- 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/commands/s3/bucket/policy_json.go b/commands/s3/bucket/policy_json.go index ad6267f..7c06180 100644 --- a/commands/s3/bucket/policy_json.go +++ b/commands/s3/bucket/policy_json.go @@ -45,15 +45,26 @@ const MaxSetSize = 4096 // carry, sorted for [PolicyAction]'s binary search. The rest of the S3 // permission vocabulary stays out on purpose: a key holding s3:CreateBucket // or s3:DeleteBucket acts outside the policy that granted it, a key holding a -// policy operation could rewrite the policy that granted it, and every -// principal holds s3:ListAllMyBuckets. +// policy operation could rewrite the policy that granted it, a key that +// rewrites the bucket's configuration (versioning, default object lock, +// tags, encryption) changes it for every principal, and every principal +// holds s3:ListAllMyBuckets. Reading that configuration is a policy action. var policyActions = sorted([]string{ "s3:AbortMultipartUpload", "s3:DeleteObject", + "s3:DeleteObjectTagging", "s3:DeleteObjectVersion", + "s3:GetBucketAcl", + "s3:GetBucketCORS", + "s3:GetBucketLocation", + "s3:GetBucketObjectLockConfiguration", + "s3:GetBucketTagging", + "s3:GetBucketVersioning", "s3:GetObject", + "s3:GetObjectAttributes", "s3:GetObjectLegalHold", "s3:GetObjectRetention", + "s3:GetObjectTagging", "s3:GetObjectVersion", "s3:ListBucket", "s3:ListBucketMultipartUploads", @@ -62,6 +73,7 @@ var policyActions = sorted([]string{ "s3:PutObject", "s3:PutObjectLegalHold", "s3:PutObjectRetention", + "s3:PutObjectTagging", }) func sorted(s []string) []string { @@ -73,6 +85,12 @@ func sorted(s []string) []string { // its objects; every other policy action is an object action. The split is // versitygw's, which evaluates the policy against these ARNs. var bucketActions = []string{ + "s3:GetBucketAcl", + "s3:GetBucketCORS", + "s3:GetBucketLocation", + "s3:GetBucketObjectLockConfiguration", + "s3:GetBucketTagging", + "s3:GetBucketVersioning", "s3:ListBucket", "s3:ListBucketMultipartUploads", "s3:ListBucketVersions", diff --git a/commands/s3/bucket/policy_test.go b/commands/s3/bucket/policy_test.go index 8a71a61..ee1d7af 100644 --- a/commands/s3/bucket/policy_test.go +++ b/commands/s3/bucket/policy_test.go @@ -214,6 +214,9 @@ func TestBucketPolicyUnmarshalJSON(t *testing.T) { ), `statement 1: Resource "arn:aws:s3:::videos" names bucket "videos"`}, {"an object action on the bucket ARN", policyJSON(`{"Effect": "Allow", "Principal": "*", "Action": ["s3:GetObject"], "Resource": "arn:aws:s3:::photos"}`), `Action "s3:GetObject" needs Resource "arn:aws:s3:::photos/*"`}, {"a bucket action on the objects ARN", policyJSON(`{"Effect": "Allow", "Principal": "*", "Action": ["s3:ListBucket"], "Resource": "arn:aws:s3:::photos/*"}`), `Action "s3:ListBucket" needs Resource "arn:aws:s3:::photos"`}, + {"a bucket configuration read on the objects ARN", policyJSON(`{"Effect": "Allow", "Principal": "*", "Action": ["s3:GetBucketVersioning"], "Resource": "arn:aws:s3:::photos/*"}`), `Action "s3:GetBucketVersioning" needs Resource "arn:aws:s3:::photos"`}, + {"an object tagging action on the bucket ARN", policyJSON(`{"Effect": "Allow", "Principal": "*", "Action": ["s3:PutObjectTagging"], "Resource": "arn:aws:s3:::photos"}`), `Action "s3:PutObjectTagging" needs Resource "arn:aws:s3:::photos/*"`}, + {"a bucket configuration write", policyJSON(`{"Effect": "Allow", "Principal": "*", "Action": ["s3:PutBucketVersioning"], "Resource": "arn:aws:s3:::photos"}`), `s3:PutBucketVersioning`}, {"the action wildcard on the objects ARN alone", policyJSON(`{"Effect": "Allow", "Principal": "*", "Action": "s3:*", "Resource": "arn:aws:s3:::photos/*"}`), `Action "s3:*" needs Resource "arn:aws:s3:::photos"`}, {"the action wildcard on the bucket ARN alone", policyJSON(`{"Effect": "Allow", "Principal": "*", "Action": "s3:*", "Resource": "arn:aws:s3:::photos"}`), `Action "s3:*" needs Resource "arn:aws:s3:::photos/*"`}, } @@ -233,7 +236,8 @@ func TestPolicyActions(t *testing.T) { for _, a := range actions { require.True(t, bucket.PolicyAction(a), a) } - for _, a := range []string{bucket.PolicyWildcard, "*", "s3:CreateBucket", "s3:DeleteBucket", "s3:ListAllMyBuckets", "s3:PutBucketPolicy"} { + for _, a := range []string{bucket.PolicyWildcard, "*", "s3:CreateBucket", "s3:DeleteBucket", "s3:ListAllMyBuckets", "s3:PutBucketPolicy", + "s3:PutBucketVersioning", "s3:PutBucketObjectLockConfiguration", "s3:PutBucketTagging"} { require.False(t, bucket.PolicyAction(a), a) } }