s3: support AWS object form for bucket policy Principal, add NotPrincipal (#10125)

* s3: support AWS object form for bucket policy Principal, add NotPrincipal

Bucket policy statements only accepted a bare string or array of strings for
the Principal element, so the AWS-documented object form was rejected:

    "Principal": { "AWS": "arn:aws:iam::123456789012:root" }
    "Principal": { "AWS": ["arn:...", "999999999999"] }

Add a PolicyPrincipal type that parses the bare string, the bare array
(retained for backward compatibility), and the object form keyed by AWS,
Service, Federated or CanonicalUser (each value a string or array). All keyed
values are flattened for principal matching, and the original JSON is preserved
so PutBucketPolicy/GetBucketPolicy returns the exact shape submitted - keeping
infrastructure-as-code tools (Terraform, Ansible) idempotent.

Also add NotPrincipal support (a statement applies to every principal except the
ones named), compiled and evaluated in both policy evaluators, and reject
statements that specify both Principal and NotPrincipal.

* s3: address review - validate principal object form, honor dynamic NotPrincipal

- Reject unsupported Principal object keys (only AWS/Service/Federated/
  CanonicalUser) and empty values, so a form like {"AWS":[]} no longer compiles
  to zero matchers and silently relies on the match-all fallback.
- Detect both Principal and NotPrincipal by field presence, not by flattened
  length, so a present-but-empty field is still rejected.
- Honor dynamic (policy-variable) NotPrincipal/Principal patterns in the
  compiled evaluator; previously a NotPrincipal made only of variables was
  treated as absent and its exclusion bypassed.
- Add regression tests for the object-form validation and dynamic NotPrincipal.

weed/s3api/policy_engine/engine.go

@@ -218,6 +218,17 @@ func (engine *PolicyEngine) evaluateStatement(stmt *CompiledStatement, args *Pol
 		}
 	}
 
+	// Check NotPrincipal (statement applies to everyone EXCEPT these principals)
+	if len(stmt.NotPrincipalPatterns) > 0 || len(stmt.DynamicNotPrincipalPatterns) > 0 {
+		matchedNotPrincipal := engine.matchesPatterns(stmt.NotPrincipalPatterns, args.Principal)
+		if !matchedNotPrincipal {
+			matchedNotPrincipal = engine.matchesDynamicPatterns(stmt.DynamicNotPrincipalPatterns, args.Principal, args)
+		}
+		if matchedNotPrincipal {
+			return false
+		}
+	}
+
 	// Check conditions
 	if len(stmt.Statement.Condition) > 0 {
 		condCtx := args.Conditions

weed/s3api/policy_engine/principal_test.go

@@ -0,0 +1,255 @@
+package policy_engine
+
+import (
+	"encoding/json"
+	"reflect"
+	"slices"
+	"testing"
+)
+
+// TestPolicyPrincipalUnmarshal covers every Principal shape AWS documents, plus
+// the bare string/array forms SeaweedFS accepts for backward compatibility.
+func TestPolicyPrincipalUnmarshal(t *testing.T) {
+	cases := []struct {
+		name string
+		in   string
+		want []string
+	}{
+		{"bare wildcard", `"*"`, []string{"*"}},
+		{"bare arn", `"arn:aws:iam::123456789012:user/alice"`, []string{"arn:aws:iam::123456789012:user/alice"}},
+		{"bare array", `["arn:aws:iam::123:root","arn:aws:iam::456:root"]`, []string{"arn:aws:iam::123:root", "arn:aws:iam::456:root"}},
+		{"AWS single", `{"AWS":"arn:aws:iam::123456789012:root"}`, []string{"arn:aws:iam::123456789012:root"}},
+		{"AWS array", `{"AWS":["arn:aws:iam::123:root","999999999999"]}`, []string{"arn:aws:iam::123:root", "999999999999"}},
+		{"AWS wildcard", `{"AWS":"*"}`, []string{"*"}},
+		{"AWS array wildcard (public read)", `{"AWS":["*"]}`, []string{"*"}},
+		{"service", `{"Service":"s3.amazonaws.com"}`, []string{"s3.amazonaws.com"}},
+		{"canonical user", `{"CanonicalUser":"79a59df900b949e55d96a1e698fbace"}`, []string{"79a59df900b949e55d96a1e698fbace"}},
+		// Object keys are flattened in sorted-key order: AWS < CanonicalUser.
+		{"mixed keys", `{"CanonicalUser":"79a59","AWS":"arn:aws:iam::123:root"}`, []string{"arn:aws:iam::123:root", "79a59"}},
+	}
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+			var p PolicyPrincipal
+			if err := json.Unmarshal([]byte(c.in), &p); err != nil {
+				t.Fatalf("unmarshal %s: %v", c.in, err)
+			}
+			if got := p.Strings(); !slices.Equal(got, c.want) {
+				t.Errorf("Strings() = %v, want %v", got, c.want)
+			}
+		})
+	}
+}
+
+func TestPolicyPrincipalUnmarshalInvalid(t *testing.T) {
+	// Includes object-form abuses: unknown key, empty array, empty string -- all
+	// of which must error rather than compile to zero matchers (which would let
+	// the match-all fallback silently make an Allow statement public).
+	for _, in := range []string{`123`, `{}`, `{"AWS":123}`, `{"Foo":"bar"}`, `{"AWS":[]}`, `{"AWS":""}`} {
+		var p PolicyPrincipal
+		if err := json.Unmarshal([]byte(in), &p); err == nil {
+			t.Errorf("expected error unmarshaling %s, got values %v", in, p.Strings())
+		}
+	}
+}
+
+// TestNotPrincipalDynamicCompiledPath guards the bug where a NotPrincipal made
+// only of policy variables (no static matchers) was treated as absent in the
+// compiled evaluator, bypassing the exclusion. aws:username is pinned via the
+// condition context so the substituted value is independent of the principal.
+func TestNotPrincipalDynamicCompiledPath(t *testing.T) {
+	policyJSON := `{"Version":"2012-10-17","Statement":[
+		{"Effect":"Allow","Principal":"*","Action":"s3:*","Resource":"arn:aws:s3:::b/*"},
+		{"Effect":"Deny","NotPrincipal":{"AWS":"${aws:username}"},"Action":"s3:*","Resource":"arn:aws:s3:::b/*"}
+	]}`
+	doc, err := ParsePolicy(policyJSON)
+	if err != nil {
+		t.Fatalf("ParsePolicy: %v", err)
+	}
+	compiled, err := CompilePolicy(doc)
+	if err != nil {
+		t.Fatalf("CompilePolicy: %v", err)
+	}
+	conds := map[string][]string{"aws:username": {"arn:aws:iam::111:user/alice"}}
+	allowed := func(p string) bool {
+		allow, _ := compiled.EvaluatePolicy(&PolicyEvaluationArgs{
+			Action: "s3:GetObject", Resource: "arn:aws:s3:::b/x", Principal: p, Conditions: conds,
+		})
+		return allow
+	}
+	// alice matches the substituted NotPrincipal -> deny excluded -> allowed.
+	if !allowed("arn:aws:iam::111:user/alice") {
+		t.Errorf("alice should be allowed (matched dynamic NotPrincipal)")
+	}
+	// bob does not match -> deny applies.
+	if allowed("arn:aws:iam::111:user/bob") {
+		t.Errorf("bob should be denied (dynamic NotPrincipal did not match)")
+	}
+}
+
+// TestPolicyPrincipalRoundTrip guards idempotency for IaC tools (Terraform,
+// Ansible): GetBucketPolicy must return the same shape PutBucketPolicy received,
+// otherwise those tools see perpetual drift (cf. NotResource normalization bug).
+func TestPolicyPrincipalRoundTrip(t *testing.T) {
+	inputs := []string{
+		`"*"`,
+		`{"AWS":"arn:aws:iam::123456789012:root"}`,
+		`{"AWS":["arn:aws:iam::123456789012:root","arn:aws:iam::555555555555:root"]}`,
+		`{"AWS":["*"]}`,
+		`["arn:aws:iam::123:root","arn:aws:iam::456:root"]`,
+	}
+	for _, in := range inputs {
+		var p PolicyPrincipal
+		if err := json.Unmarshal([]byte(in), &p); err != nil {
+			t.Fatalf("unmarshal %s: %v", in, err)
+		}
+		out, err := json.Marshal(&p)
+		if err != nil {
+			t.Fatalf("marshal: %v", err)
+		}
+		if !jsonSemanticEqual(t, string(out), in) {
+			t.Errorf("round-trip mismatch: got %s, want %s", out, in)
+		}
+	}
+}
+
+// TestBucketPolicyAWSObjectPrincipalEndToEnd parses, compiles and evaluates a
+// public-read policy written with the AWS object form {"AWS":["*"]} — the
+// AWS-documented shape that previously failed to parse.
+func TestBucketPolicyAWSObjectPrincipalEndToEnd(t *testing.T) {
+	policyJSON := `{
+		"Version": "2012-10-17",
+		"Statement": [{
+			"Sid": "AllowPublicRead",
+			"Effect": "Allow",
+			"Principal": {"AWS": ["*"]},
+			"Action": ["s3:GetObject"],
+			"Resource": ["arn:aws:s3:::my-bucket/*"]
+		}]
+	}`
+	doc, err := ParsePolicy(policyJSON)
+	if err != nil {
+		t.Fatalf("ParsePolicy: %v", err)
+	}
+	compiled, err := CompilePolicy(doc)
+	if err != nil {
+		t.Fatalf("CompilePolicy: %v", err)
+	}
+
+	for _, principal := range []string{"arn:aws:iam::123456789012:user/bob", "*"} {
+		allow, effect := compiled.EvaluatePolicy(&PolicyEvaluationArgs{
+			Action:    "s3:GetObject",
+			Resource:  "arn:aws:s3:::my-bucket/object.txt",
+			Principal: principal,
+		})
+		if !allow || effect != PolicyEffectAllow {
+			t.Errorf("principal %q: allow=%v effect=%v, want allow", principal, allow, effect)
+		}
+	}
+}
+
+// TestBucketPolicySpecificAWSPrincipal verifies a named-principal object form
+// only matches the named principal.
+func TestBucketPolicySpecificAWSPrincipal(t *testing.T) {
+	policyJSON := `{"Version":"2012-10-17","Statement":[{"Effect":"Allow",` +
+		`"Principal":{"AWS":"arn:aws:iam::123456789012:user/alice"},` +
+		`"Action":"s3:*","Resource":"arn:aws:s3:::b/*"}]}`
+	doc, err := ParsePolicy(policyJSON)
+	if err != nil {
+		t.Fatalf("ParsePolicy: %v", err)
+	}
+	compiled, err := CompilePolicy(doc)
+	if err != nil {
+		t.Fatalf("CompilePolicy: %v", err)
+	}
+	args := func(p string) *PolicyEvaluationArgs {
+		return &PolicyEvaluationArgs{Action: "s3:GetObject", Resource: "arn:aws:s3:::b/x", Principal: p}
+	}
+	if allow, _ := compiled.EvaluatePolicy(args("arn:aws:iam::123456789012:user/alice")); !allow {
+		t.Errorf("alice should be allowed")
+	}
+	if allow, _ := compiled.EvaluatePolicy(args("arn:aws:iam::123456789012:user/bob")); allow {
+		t.Errorf("bob should NOT be allowed")
+	}
+}
+
+// TestBucketPolicyNotPrincipalDeny exercises the common "deny everyone except X"
+// pattern through the real bucket-policy evaluator (PolicyEngine). A Deny with
+// NotPrincipal applies to every principal NOT named, so the named principal is
+// spared and everyone else is denied.
+func TestBucketPolicyNotPrincipalDeny(t *testing.T) {
+	engine := NewPolicyEngine()
+	policyJSON := `{
+		"Version": "2012-10-17",
+		"Statement": [
+			{"Sid":"AllowAll","Effect":"Allow","Principal":"*","Action":"s3:*","Resource":"arn:aws:s3:::b/*"},
+			{"Sid":"DenyExceptAlice","Effect":"Deny","NotPrincipal":{"AWS":"arn:aws:iam::123456789012:user/alice"},"Action":"s3:*","Resource":"arn:aws:s3:::b/*"}
+		]
+	}`
+	if err := engine.SetBucketPolicy("b", policyJSON); err != nil {
+		t.Fatalf("SetBucketPolicy: %v", err)
+	}
+	eval := func(p string) PolicyEvaluationResult {
+		return engine.EvaluatePolicy("b", &PolicyEvaluationArgs{Action: "s3:GetObject", Resource: "arn:aws:s3:::b/x", Principal: p})
+	}
+	// alice is excluded from the NotPrincipal deny, so the Allow wins.
+	if got := eval("arn:aws:iam::123456789012:user/alice"); got != PolicyResultAllow {
+		t.Errorf("alice: got %v, want Allow", got)
+	}
+	// bob is not named in NotPrincipal, so the Deny applies.
+	if got := eval("arn:aws:iam::123456789012:user/bob"); got != PolicyResultDeny {
+		t.Errorf("bob: got %v, want Deny", got)
+	}
+}
+
+// TestBucketPolicyNotPrincipalCompiledPath covers the matcher-based evaluator
+// (CompiledPolicy). An Allow-all paired with a Deny/NotPrincipal makes the
+// "excluded vs denied" difference observable (a lone Deny would yield implicit
+// deny for both).
+func TestBucketPolicyNotPrincipalCompiledPath(t *testing.T) {
+	policyJSON := `{"Version":"2012-10-17","Statement":[
+		{"Effect":"Allow","Principal":"*","Action":"s3:*","Resource":"arn:aws:s3:::b/*"},
+		{"Effect":"Deny","NotPrincipal":{"AWS":["arn:aws:iam::123456789012:user/alice"]},"Action":"s3:*","Resource":"arn:aws:s3:::b/*"}
+	]}`
+	doc, err := ParsePolicy(policyJSON)
+	if err != nil {
+		t.Fatalf("ParsePolicy: %v", err)
+	}
+	compiled, err := CompilePolicy(doc)
+	if err != nil {
+		t.Fatalf("CompilePolicy: %v", err)
+	}
+	args := func(p string) *PolicyEvaluationArgs {
+		return &PolicyEvaluationArgs{Action: "s3:GetObject", Resource: "arn:aws:s3:::b/x", Principal: p}
+	}
+	// alice is named in NotPrincipal -> deny excluded -> allowed.
+	if allow, _ := compiled.EvaluatePolicy(args("arn:aws:iam::123456789012:user/alice")); !allow {
+		t.Errorf("alice should be allowed (excluded from NotPrincipal deny)")
+	}
+	// bob is not named -> deny applies.
+	if allow, effect := compiled.EvaluatePolicy(args("arn:aws:iam::123456789012:user/bob")); allow || effect != PolicyEffectDeny {
+		t.Errorf("bob should be denied, got allow=%v effect=%v", allow, effect)
+	}
+}
+
+// TestPolicyBothPrincipalAndNotPrincipalRejected verifies AWS's rule that a
+// single statement may not contain both Principal and NotPrincipal.
+func TestPolicyBothPrincipalAndNotPrincipalRejected(t *testing.T) {
+	policyJSON := `{"Version":"2012-10-17","Statement":[{"Effect":"Deny",` +
+		`"Principal":"*","NotPrincipal":{"AWS":"arn:aws:iam::1:user/a"},` +
+		`"Action":"s3:*","Resource":"arn:aws:s3:::b/*"}]}`
+	if _, err := ParsePolicy(policyJSON); err == nil {
+		t.Errorf("expected error when both Principal and NotPrincipal are set")
+	}
+}
+
+func jsonSemanticEqual(t *testing.T, a, b string) bool {
+	t.Helper()
+	var av, bv interface{}
+	if err := json.Unmarshal([]byte(a), &av); err != nil {
+		t.Fatalf("unmarshal %s: %v", a, err)
+	}
+	if err := json.Unmarshal([]byte(b), &bv); err != nil {
+		t.Fatalf("unmarshal %s: %v", b, err)
+	}
+	return reflect.DeepEqual(av, bv)
+}

weed/s3api/policy_engine/types.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"regexp"
 	"slices"
+	"sort"
 	"strings"
 	"time"
 
@@ -106,6 +107,129 @@ func CloneStringOrStringSlice(value StringOrStringSlice) StringOrStringSlice {
 	return StringOrStringSlice{values: append([]string(nil), value.values...)}
 }
 
+// allowedPrincipalKeys is the set of principal-type keys AWS allows inside the
+// Principal/NotPrincipal object form.
+var allowedPrincipalKeys = map[string]struct{}{
+	"AWS":           {},
+	"Service":       {},
+	"Federated":     {},
+	"CanonicalUser": {},
+}
+
+// PolicyPrincipal represents the Principal element of a policy statement.
+//
+// AWS accepts three shapes (see the IAM "Principal" element reference):
+//
+//	"Principal": "*"                                    // all principals (anonymous)
+//	"Principal": { "AWS": "arn:aws:iam::123:user/x" }   // single, keyed by type
+//	"Principal": { "AWS": ["arn:...", "999999999999"] } // array of values
+//
+// The object may key on "AWS", "Service", "Federated" or "CanonicalUser", and
+// each value is itself a string or an array of strings. For backward
+// compatibility SeaweedFS also accepts a bare string or array of strings.
+//
+// Authorization matches the request principal ARN against the flat set of
+// principal values, so every keyed value is flattened into one list. The
+// original JSON is preserved so GetBucketPolicy returns the exact shape the
+// caller submitted; this keeps PutBucketPolicy/GetBucketPolicy idempotent for
+// infrastructure-as-code tools (Terraform, Ansible) that diff the returned
+// policy against the submitted one.
+type PolicyPrincipal struct {
+	values []string
+	raw    json.RawMessage
+}
+
+// UnmarshalJSON implements json.Unmarshaler for PolicyPrincipal.
+func (p *PolicyPrincipal) UnmarshalJSON(data []byte) error {
+	// Preserve the original encoding for a faithful round-trip on marshal.
+	p.raw = append(json.RawMessage(nil), data...)
+
+	// Bare string, e.g. "*"
+	var str string
+	if err := json.Unmarshal(data, &str); err == nil {
+		p.values = []string{str}
+		return nil
+	}
+
+	// Bare array of strings (SeaweedFS extension; not standard AWS).
+	var strs []string
+	if err := json.Unmarshal(data, &strs); err == nil {
+		p.values = strs
+		return nil
+	}
+
+	// AWS object form: {"AWS": <string|[]string>, "Service": ..., "Federated": ...,
+	// "CanonicalUser": ...}. Each value reuses StringOrStringSlice so it accepts a
+	// single string or an array. All keyed values are flattened for matching.
+	var obj map[string]StringOrStringSlice
+	if err := json.Unmarshal(data, &obj); err == nil && len(obj) > 0 {
+		// Sort keys so the flattened order is deterministic.
+		keys := make([]string, 0, len(obj))
+		for k := range obj {
+			if _, ok := allowedPrincipalKeys[k]; !ok {
+				return fmt.Errorf("unsupported Principal type %q (expected AWS, Service, Federated or CanonicalUser)", k)
+			}
+			keys = append(keys, k)
+		}
+		sort.Strings(keys)
+		values := make([]string, 0, len(obj))
+		for _, k := range keys {
+			v := obj[k]
+			items := v.Strings()
+			if len(items) == 0 {
+				return fmt.Errorf("Principal %q must list at least one value", k)
+			}
+			for _, item := range items {
+				if item == "" {
+					return fmt.Errorf("Principal %q must not contain empty values", k)
+				}
+			}
+			values = append(values, items...)
+		}
+		p.values = values
+		return nil
+	}
+
+	return fmt.Errorf(`Principal must be a string, an array of strings, or an object such as {"AWS": ["arn:..."]}`)
+}
+
+// MarshalJSON implements json.Marshaler for PolicyPrincipal.
+func (p PolicyPrincipal) MarshalJSON() ([]byte, error) {
+	// Echo the original JSON when we parsed it, so the shape is preserved.
+	if len(p.raw) > 0 {
+		return p.raw, nil
+	}
+	// Programmatically-constructed principals marshal as a bare string/array.
+	if len(p.values) == 1 {
+		return json.Marshal(p.values[0])
+	}
+	return json.Marshal(p.values)
+}
+
+// Strings returns the flattened principal values. Nil-safe for pointer receivers.
+func (p *PolicyPrincipal) Strings() []string {
+	if p == nil {
+		return nil
+	}
+	return p.values
+}
+
+// NewPolicyPrincipalPtr builds a *PolicyPrincipal from flat values (no object wrapping).
+func NewPolicyPrincipalPtr(values ...string) *PolicyPrincipal {
+	return &PolicyPrincipal{values: values}
+}
+
+// ClonePolicyPrincipal deep-copies a *PolicyPrincipal (nil-safe).
+func ClonePolicyPrincipal(p *PolicyPrincipal) *PolicyPrincipal {
+	if p == nil {
+		return nil
+	}
+	return &PolicyPrincipal{
+		values: append([]string(nil), p.values...),
+		raw:    append(json.RawMessage(nil), p.raw...),
+	}
+}
+
 // PolicyConditions represents policy conditions with proper typing
 type PolicyConditions map[string]map[string]StringOrStringSlice
 
@@ -148,13 +272,14 @@ func (p *PolicyDocument) UnmarshalJSON(data []byte) error {
 
 // PolicyStatement represents a single policy statement
 type PolicyStatement struct {
-	Sid         string               `json:"Sid,omitempty"`
-	Effect      PolicyEffect         `json:"Effect"`
-	Principal   *StringOrStringSlice `json:"Principal,omitempty"`
-	Action      StringOrStringSlice  `json:"Action"`
-	Resource    *StringOrStringSlice `json:"Resource,omitempty"`
-	NotResource *StringOrStringSlice `json:"NotResource,omitempty"`
-	Condition   PolicyConditions     `json:"Condition,omitempty"`
+	Sid          string               `json:"Sid,omitempty"`
+	Effect       PolicyEffect         `json:"Effect"`
+	Principal    *PolicyPrincipal     `json:"Principal,omitempty"`
+	NotPrincipal *PolicyPrincipal     `json:"NotPrincipal,omitempty"`
+	Action       StringOrStringSlice  `json:"Action"`
+	Resource     *StringOrStringSlice `json:"Resource,omitempty"`
+	NotResource  *StringOrStringSlice `json:"NotResource,omitempty"`
+	Condition    PolicyConditions     `json:"Condition,omitempty"`
 }
 
 // PolicyEffect represents Allow or Deny
@@ -212,6 +337,11 @@ type CompiledStatement struct {
 	DynamicResourcePatterns  []string
 	DynamicPrincipalPatterns []string
 
+	// NotPrincipal patterns (principal should NOT match these)
+	NotPrincipalMatchers        []*wildcard.WildcardMatcher
+	NotPrincipalPatterns        []*regexp.Regexp
+	DynamicNotPrincipalPatterns []string
+
 	// NotResource patterns (resource should NOT match these)
 	NotResourcePatterns        []*regexp.Regexp
 	NotResourceMatchers        []*wildcard.WildcardMatcher
@@ -258,6 +388,11 @@ func validateStatement(stmt *PolicyStatement) error {
 		return fmt.Errorf("statement must specify Resource or NotResource")
 	}
 
+	// AWS does not allow both Principal and NotPrincipal in the same statement.
+	if stmt.Principal != nil && stmt.NotPrincipal != nil {
+		return fmt.Errorf("statement cannot specify both Principal and NotPrincipal")
+	}
+
 	return nil
 }
 
@@ -306,11 +441,12 @@ func compileStatement(stmt *PolicyStatement) (*CompiledStatement, error) {
 		},
 	}
 
-	// Deep clone Principal if present
+	// Deep clone Principal / NotPrincipal if present
 	if stmt.Principal != nil {
-		principalClone := *stmt.Principal
-		principalClone.values = slices.Clone(stmt.Principal.values)
-		compiled.Statement.Principal = &principalClone
+		compiled.Statement.Principal = ClonePolicyPrincipal(stmt.Principal)
+	}
+	if stmt.NotPrincipal != nil {
+		compiled.Statement.NotPrincipal = ClonePolicyPrincipal(stmt.NotPrincipal)
 	}
 
 	// Deep clone Resource/NotResource into the internal statement as well for completeness
@@ -412,6 +548,32 @@ func compileStatement(stmt *PolicyStatement) (*CompiledStatement, error) {
 		}
 	}
 
+	// Compile NotPrincipal patterns and matchers (principal should NOT match these)
+	if stmt.NotPrincipal != nil && len(stmt.NotPrincipal.Strings()) > 0 {
+		for _, notPrincipal := range stmt.NotPrincipal.Strings() {
+			if notPrincipal == "" {
+				continue
+			}
+			// Check for dynamic variables
+			if PolicyVariableRegex.MatchString(notPrincipal) {
+				compiled.DynamicNotPrincipalPatterns = append(compiled.DynamicNotPrincipalPatterns, notPrincipal)
+				continue
+			}
+
+			pattern, err := compilePattern(notPrincipal)
+			if err != nil {
+				return nil, fmt.Errorf("failed to compile NotPrincipal pattern %s: %v", notPrincipal, err)
+			}
+			compiled.NotPrincipalPatterns = append(compiled.NotPrincipalPatterns, pattern)
+
+			matcher, err := wildcard.NewWildcardMatcher(notPrincipal)
+			if err != nil {
+				return nil, fmt.Errorf("failed to create NotPrincipal matcher %s: %v", notPrincipal, err)
+			}
+			compiled.NotPrincipalMatchers = append(compiled.NotPrincipalMatchers, matcher)
+		}
+	}
+
 	// Compile NotResource patterns (resource should NOT match these)
 	if len(notResStrings) > 0 {
 		for _, notResource := range notResStrings {
@@ -546,6 +708,23 @@ func (cs *CompiledStatement) MatchesPrincipal(principal string) bool {
 	return false
 }
 
+// matchesPrincipalSet reports whether the request principal matches any static
+// matcher or dynamic (policy-variable) pattern in the given set.
+func (cs *CompiledStatement) matchesPrincipalSet(args *PolicyEvaluationArgs, matchers []*wildcard.WildcardMatcher, dynamic []string) bool {
+	for _, matcher := range matchers {
+		if matcher.Match(args.Principal) {
+			return true
+		}
+	}
+	for _, pattern := range dynamic {
+		substituted := SubstituteVariables(pattern, args.Conditions, args.Claims)
+		if FastMatchesWildcard(substituted, args.Principal) {
+			return true
+		}
+	}
+	return false
+}
+
 // EvaluateStatement evaluates a compiled statement against the given arguments
 func (cs *CompiledStatement) EvaluateStatement(args *PolicyEvaluationArgs) bool {
 	// Check if action matches
@@ -558,9 +737,18 @@ func (cs *CompiledStatement) EvaluateStatement(args *PolicyEvaluationArgs) bool
 		return false
 	}
 
-	// Check if principal matches
-	if !cs.MatchesPrincipal(args.Principal) {
-		return false
+	// Principal / NotPrincipal (mutually exclusive per AWS): NotPrincipal makes
+	// the statement apply to everyone EXCEPT the named principals; a plain
+	// Principal requires a match. Both static matchers and dynamic
+	// (policy-variable) patterns are honored.
+	if len(cs.NotPrincipalMatchers) > 0 || len(cs.DynamicNotPrincipalPatterns) > 0 {
+		if cs.matchesPrincipalSet(args, cs.NotPrincipalMatchers, cs.DynamicNotPrincipalPatterns) {
+			return false
+		}
+	} else if len(cs.PrincipalMatchers) > 0 || len(cs.DynamicPrincipalPatterns) > 0 {
+		if !cs.matchesPrincipalSet(args, cs.PrincipalMatchers, cs.DynamicPrincipalPatterns) {
+			return false
+		}
 	}
 
 	return true

weed/s3api/s3api_bucket_config.go

@@ -695,12 +695,13 @@ func cloneBucketPolicy(policyDoc *policy_engine.PolicyDocument) *policy_engine.P
 
 func clonePolicyStatement(statement policy_engine.PolicyStatement) policy_engine.PolicyStatement {
 	cloned := policy_engine.PolicyStatement{
-		Sid:         statement.Sid,
-		Effect:      statement.Effect,
-		Action:      cloneStringOrStringSlice(statement.Action),
-		NotResource: cloneStringOrStringSlicePtr(statement.NotResource),
-		Principal:   cloneStringOrStringSlicePtr(statement.Principal),
-		Resource:    cloneStringOrStringSlicePtr(statement.Resource),
+		Sid:          statement.Sid,
+		Effect:       statement.Effect,
+		Action:       cloneStringOrStringSlice(statement.Action),
+		NotResource:  cloneStringOrStringSlicePtr(statement.NotResource),
+		Principal:    policy_engine.ClonePolicyPrincipal(statement.Principal),
+		NotPrincipal: policy_engine.ClonePolicyPrincipal(statement.NotPrincipal),
+		Resource:     cloneStringOrStringSlicePtr(statement.Resource),
 	}
 	if statement.Condition != nil {
 		cloned.Condition = make(policy_engine.PolicyConditions, len(statement.Condition))