s3: embedded IAM inline policy honors prefix-scoped resources (#10192)

* s3: embedded IAM inline policy honors prefix-scoped resources

getActions stripped the trailing wildcard from a resource like
arn:aws:s3:::bucket/prefix/*, producing a non-wildcard action
(Write:bucket/prefix) that CanDo only ever matched at bucket level, so
PutObject under the prefix was denied. Preserve the object path with its
wildcard (Write:bucket/prefix/*) to match objects under the prefix,
matching the standalone iamapi behavior.

* s3: prune bucket-confined wildcard actions on bucket delete

actionScopedToBucket treated any wildcard as multi-bucket, so a
prefix-scoped action like Write:bucket/prefix/* survived deletion of its
own bucket and could re-grant access if the bucket was recreated. Scope
the wildcard check to the bucket segment only: a wildcard in the object
path stays scoped to its bucket, while one in the bucket segment does
not.
This commit is contained in:
Chris Lu
2026-07-02 09:14:54 -07:00
committed by GitHub
parent cc4043c9d2
commit c46526822b
4 changed files with 75 additions and 13 deletions
+7 -4
View File
@@ -2059,18 +2059,21 @@ func (iam *IdentityAccessManagement) PruneBucketFromConfiguration(ctx context.Co
}
// actionScopedToBucket reports whether a configured action string like
// "Read:bucket" or "Write:bucket/prefix" is scoped exclusively to the given
// bucket. Wildcard resources are never considered scoped to a single bucket.
// "Read:bucket", "Write:bucket/prefix" or "Write:bucket/prefix/*" is scoped
// exclusively to the given bucket. A wildcard in the bucket segment (e.g. "*"
// or "buck*/x") may cover other buckets and is never single-bucket scoped; a
// wildcard confined to the object path stays scoped to this bucket.
func actionScopedToBucket(action, bucket string) bool {
idx := strings.Index(action, ":")
if idx < 0 {
return false
}
resource := action[idx+1:]
if strings.ContainsAny(resource, "*?") {
bucketSeg, _, _ := strings.Cut(resource, "/")
if strings.ContainsAny(bucketSeg, "*?") {
return false
}
return resource == bucket || strings.HasPrefix(resource, bucket+"/")
return bucketSeg == bucket
}
// LoadS3ApiConfigurationFromCredentialManager loads configuration using the credential manager
+26
View File
@@ -0,0 +1,26 @@
package s3api
import "testing"
func TestActionScopedToBucket(t *testing.T) {
cases := []struct {
action string
bucket string
want bool
}{
{"Read:bucket", "bucket", true},
{"Write:bucket/prefix", "bucket", true},
{"Write:bucket/prefix/*", "bucket", true},
{"Write:bucket/*", "bucket", true},
{"Read:other/prefix/*", "bucket", false},
{"Write:*", "bucket", false},
{"Write:buck*/x", "bucket", false},
{"Write:bucketother", "bucket", false},
{"Admin", "bucket", false},
}
for _, c := range cases {
if got := actionScopedToBucket(c.action, c.bucket); got != c.want {
t.Errorf("actionScopedToBucket(%q, %q) = %v, want %v", c.action, c.bucket, got, c.want)
}
}
}
+5 -9
View File
@@ -989,15 +989,11 @@ func (e *EmbeddedIamApi) getActions(policy *policy_engine.PolicyDocument) ([]str
// Bucket-level or bucket/* - use just bucket name
actions = append(actions, fmt.Sprintf("%s:%s", statementAction, bucket))
} else {
// Path-specific: bucket/path/* -> Action:bucket/path
// Remove trailing /* if present for cleaner action format
objectPath = strings.TrimSuffix(objectPath, "/*")
objectPath = strings.TrimSuffix(objectPath, "*")
if objectPath == "" {
actions = append(actions, fmt.Sprintf("%s:%s", statementAction, bucket))
} else {
actions = append(actions, fmt.Sprintf("%s:%s/%s", statementAction, bucket, objectPath))
}
// Path-specific: preserve the object path, including any
// trailing wildcard, so CanDo can match objects under the
// prefix. Stripping the wildcard yielded a non-wildcard
// action that only matched at bucket level.
actions = append(actions, fmt.Sprintf("%s:%s/%s", statementAction, bucket, objectPath))
}
}
}
+37
View File
@@ -2095,6 +2095,43 @@ func TestEmbeddedIamGetActionsFromPolicy(t *testing.T) {
assert.Contains(t, actions, "Write:mybucket")
}
// TestEmbeddedIamGetActionsPrefixScopedResource verifies that a prefix-scoped
// resource keeps its trailing wildcard so CanDo authorizes objects under the
// prefix. Stripping the wildcard produced a non-wildcard action that only
// matched at bucket level, denying every PutObject under the prefix.
func TestEmbeddedIamGetActionsPrefixScopedResource(t *testing.T) {
api := NewEmbeddedIamApiForTest()
policyDoc := `{
"Version": "2012-10-17",
"Statement": [{
"Effect": "Allow",
"Action": ["s3:Get*", "s3:Put*", "s3:DeleteObject"],
"Resource": ["arn:aws:s3:::mcp-upload/mcp/TMS/api_keys/example/*"]
}]
}`
policy, err := api.GetPolicyDocument(&policyDoc)
require.NoError(t, err)
actions, err := api.getActions(&policy)
require.NoError(t, err)
assert.Contains(t, actions, "Read:mcp-upload/mcp/TMS/api_keys/example/*")
assert.Contains(t, actions, "Write:mcp-upload/mcp/TMS/api_keys/example/*")
identity := &Identity{Name: "prefix-user"}
for _, a := range actions {
identity.Actions = append(identity.Actions, Action(a))
}
assert.True(t, identity.CanDo(ACTION_WRITE, "mcp-upload", "mcp/TMS/api_keys/example/logo.png"),
"PutObject under the allowed prefix should be authorized")
assert.True(t, identity.CanDo(ACTION_READ, "mcp-upload", "mcp/TMS/api_keys/example/logo.png"),
"GetObject under the allowed prefix should be authorized")
assert.False(t, identity.CanDo(ACTION_WRITE, "mcp-upload", "other/logo.png"),
"PutObject outside the allowed prefix should be denied")
}
// TestEmbeddedIamPutUserPolicyAllResourceWildcard reproduces issue #9209:
// an AWS-style policy using "Action":"s3:*" with a bare "Resource":"*" is
// valid AWS IAM syntax (meaning "any resource") but was rejected with