fix(s3tables): validate policy conditions and fail safe on unusable ones (#12389)

* s3tables: validate policy conditions and fail safe on unusable ones * s3vectors: reject a condition operator with no keys The shared evaluator now treats a keyless operator as unusable, so a stored keyless Allow would never grant while the write succeeded. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * s3vectors: reject an Allow whose Condition is null A null Condition decodes as no Condition, so the Allow granted unconditionally. A null Condition on a Deny only widens the veto and stays accepted, as in the S3 Tables validator. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * s3tables: treat a NaN numeric condition value as unreadable strconv.ParseFloat accepts "NaN", but no comparison with it holds, so a Deny on NumericLessThan never applied and an Allow on NumericNotEquals matched every request. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * s3tables: reject condition keys the evaluator never supplies A condition on a key such as aws:SourceIp reads no request value, so a Deny on it never applied and an Allow never granted. Accept only the keys getConditionContextValues resolves, as the vector validator does. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * s3vectors: require Statement and Effect spelled as written The evaluator decodes field names regardless of case, so an "effect" or "statement" key slipped past the null Condition check and the other raw-key checks while the stored Allow still granted. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * s3tables: treat an infinite numeric condition value as unreadable Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * s3tables: a stored Allow with a null Condition never grants The typed decode reads "Condition": null as no Condition, so an Allow stored before validation refused it still granted unconditionally, for table and vector policies alike. Record the literal null while decoding a Statement and skip such an Allow during evaluation. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * s3tables: validate a null Condition from the decoded Statement validatePolicyDocument re-parsed the document to find a literal null Condition; the Statement decode now records it, so the validator reads the same flag the evaluator does and nullConditionStatements goes away. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * s3tables: keep Condition objects merged across key spellings Decoding Condition through a json.RawMessage kept only the last of "Condition" and "condition", so an empty object under one spelling erased the other's restriction. Decode the statement as before and probe for the literal null separately. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * s3tables: refuse a Condition split across key spellings Condition objects under differently cased keys decode into one map, and an operator they share keeps only the last spelling's keys, so a restriction under the other spelling was dropped. Validation now refuses a statement whose Condition is under more than one key, and one stored before that never grants on an Allow and vetoes on a Deny. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * s3tables: refuse a Condition key repeated under the same spelling PolicyDocument re-marshaled each statement through interface{}, and Statement.UnmarshalJSON counted keys from a map, so both kept only the last of a repeated Condition key. "Condition":null,"Condition":{} or two Condition objects sharing an operator passed validation with a restriction dropped. Statements now decode from their original bytes and the keys are counted while walking the object. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * s3vectors: refuse a repeated Condition key in a vector bucket policy The validator read Condition from a map, which keeps only the last of a repeated key, so it accepted a statement the evaluator never grants on. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * s3tables: keep the re-marshaled statement decode, count Condition keys apart Decoding statements from their original bytes let document order pick between spellings of every field, where the re-marshal sorted the keys first, so a stored "effect":"Deny","Effect":"Allow" became an Allow. Statements decode as before again; only the Condition keys are counted from the original bytes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * s3tables: test that validation refuses a split Condition on a Deny Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * s3vectors: test that validation refuses a repeated Condition on a Deny Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Chris Lu <chrislusf@users.noreply.github.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
2026-11-05 03:27:42 +01:00 · 2026-11-04 08:13:05 -08:00
1 parent d727c6970f
commit 15ff5c113a
@@ -85,6 +85,10 @@ func TestS3TablesIntegration(t *testing.T) {
 		testTablePolicy(t, client)
 	})

+	t.Run("PolicyConditionValidation", func(t *testing.T) {
+		testPolicyConditionValidation(t, client)
+	})
+
 	t.Run("Tagging", func(t *testing.T) {
 		testTagging(t, client)
 	})
@@ -425,6 +429,46 @@ func testTablePolicy(t *testing.T, client *S3TablesClient) {
 	t.Logf("Verified policy deletion")
 }

+// A null condition value is refused with InvalidRequest instead of being
+// stored and later panicking the evaluator, and the bucket stays reachable.
+// A well-formed conditional policy is still stored as written.
+func testPolicyConditionValidation(t *testing.T, client *S3TablesClient) {
+	bucketName := "test-policy-cond-" + randomString(8)
+	createBucketResp, err := client.CreateTableBucket(bucketName, nil)
+	require.NoError(t, err, "Failed to create table bucket")
+	defer client.DeleteTableBucket(createBucketResp.ARN)
+	bucketARN := createBucketResp.ARN
+
+	nullCondition := `{"Version":"2012-10-17","Statement":[{"Effect":"Deny","Principal":"*","Action":"s3tables:*","Resource":"*","Condition":{"StringEquals":{"aws:ResourceTag/env":null}}}]}`
+	err = client.PutTableBucketPolicy(bucketARN, nullCondition)
+	require.Error(t, err, "a null condition value must be rejected")
+	assert.Contains(t, err.Error(), s3tables.ErrCodeInvalidRequest)
+
+	_, err = client.GetTableBucket(bucketARN)
+	require.NoError(t, err, "the bucket must stay reachable after the rejected policy")
+	_, err = client.GetTableBucketPolicy(bucketARN)
+	assert.Error(t, err, "the rejected policy must not be stored")
+
+	validCondition := `{"Version":"2012-10-17","Statement":[{"Effect":"Deny","Principal":"*","Action":"s3tables:DeleteTableBucket","Resource":"*","Condition":{"StringEquals":{"aws:ResourceTag/env":"prod"}}}]}`
+	require.NoError(t, client.PutTableBucketPolicy(bucketARN, validCondition), "a valid conditional policy must be accepted")
+	getPolicyResp, err := client.GetTableBucketPolicy(bucketARN)
+	require.NoError(t, err)
+	assert.Equal(t, validCondition, getPolicyResp.ResourcePolicy)
+
+	// A null Condition reads as none, so this Allow would grant everyone.
+	allowNull := `{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Principal":"*","Action":"s3tables:*","Resource":"*","Condition":null}]}`
+	err = client.PutTableBucketPolicy(bucketARN, allowNull)
+	require.Error(t, err, "an Allow with a null Condition must be rejected")
+	assert.Contains(t, err.Error(), s3tables.ErrCodeInvalidRequest)
+	getPolicyResp, err = client.GetTableBucketPolicy(bucketARN)
+	require.NoError(t, err)
+	assert.Equal(t, validCondition, getPolicyResp.ResourcePolicy, "the stored policy must be unchanged")
+
+	_, err = client.GetTableBucket(bucketARN)
+	require.NoError(t, err, "the bucket must stay reachable under a valid conditional policy")
+	require.NoError(t, client.DeleteTableBucketPolicy(bucketARN))
+}
+
 func testTagging(t *testing.T, client *S3TablesClient) {
 	bucketName := "test-tag-bucket-" + randomString(8)

@@ -130,6 +130,9 @@ var normalizedValueCache = NewNormalizedValueCache(1000)

 // getCachedNormalizedValues returns cached normalized values or caches new ones
 func getCachedNormalizedValues(value interface{}) []string {
+	if value == nil {
+		return nil // no expected values; never reach reflect.TypeOf(nil)
+	}
 	// Create a string key for caching - more efficient than fmt.Sprintf
 	typeStr := reflect.TypeOf(value).String()
 	cacheKey := typeStr + ":" + fmt.Sprint(value)
@@ -16,3 +16,35 @@ func TestEvaluateConditionsFailsClosedOnUnsupportedOperator(t *testing.T) {
 		t.Fatalf("EvaluateConditions returned true for unsupported operator; expected false (fail closed)")
 	}
 }
+
+// A policy parsed into interface{} can hand an evaluator a null condition
+// value; no operator, set-qualified or not, may panic on it.
+func TestConditionEvaluatorsTolerateNilValue(t *testing.T) {
+	operators := []string{
+		"StringEquals", "StringNotEquals", "StringLike", "StringNotLike",
+		"StringEqualsIgnoreCase", "StringNotEqualsIgnoreCase",
+		"NumericEquals", "NumericNotEquals", "NumericLessThan", "NumericLessThanEquals",
+		"NumericGreaterThan", "NumericGreaterThanEquals",
+		"DateEquals", "DateNotEquals", "DateLessThan", "DateLessThanEquals",
+		"DateGreaterThan", "DateGreaterThanEquals",
+		"Bool", "IpAddress", "NotIpAddress", "ArnEquals", "ArnLike", "Null",
+	}
+	for _, base := range operators {
+		for _, operator := range []string{base, "ForAnyValue:" + base, "ForAllValues:" + base} {
+			evaluator, err := GetConditionEvaluator(operator)
+			if err != nil {
+				t.Fatalf("%s: %v", operator, err)
+			}
+			for _, contextValues := range [][]string{nil, {"x"}} {
+				func() {
+					defer func() {
+						if r := recover(); r != nil {
+							t.Errorf("%s with a nil value and context %v panicked: %v", operator, contextValues, r)
+						}
+					}()
+					evaluator.Evaluate(nil, contextValues)
+				}()
+			}
+		}
+	}
+}
@@ -0,0 +1,149 @@
+package s3tables
+
+import (
+	"testing"
+
+	"github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants"
+	"github.com/seaweedfs/seaweedfs/weed/s3api/s3tables/s3tablestest"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+const nullConditionDeny = `{"Statement":[{"Effect":"Deny","Principal":"*","Action":"s3tables:*","Resource":"*","Condition":{"StringEquals":{"aws:ResourceTag/env":null}}}]}`
+
+// startConditionManager seeds startPolicyDenyManager's bucket, namespace and
+// table with no table policy and bucketPolicy, if any, stored on the bucket
+// the way a release without write-time condition checks could have left it.
+func startConditionManager(t *testing.T, bucketPolicy string) (*s3tablestest.MemFiler, *Manager) {
+	t.Helper()
+	fs, m := startPolicyDenyManager(t)
+	bucket := fs.Get(TablesPath, policyDenyBucket)
+	require.NotNil(t, bucket)
+	delete(bucket.Extended, ExtendedKeyPolicy)
+	if bucketPolicy != "" {
+		bucket.Extended[ExtendedKeyPolicy] = []byte(bucketPolicy)
+	}
+	table := fs.Get(GetNamespacePath(policyDenyBucket, "ns"), "t")
+	require.NotNil(t, table)
+	delete(table.Extended, ExtendedKeyPolicy)
+	return fs, m
+}
+
+func requireS3TablesError(t *testing.T, err error, code string) *S3TablesError {
+	t.Helper()
+	var s3err *S3TablesError
+	require.ErrorAs(t, err, &s3err)
+	assert.Equal(t, code, s3err.Type, s3err.Message)
+	return s3err
+}
+
+// A condition the evaluator cannot compare is refused at write time, on the
+// bucket and on the table, and nothing is stored.
+func TestPutPolicyRejectsUnusableCondition(t *testing.T) {
+	fs, m := startConditionManager(t, "")
+	bucketARN, ns, name := deniedTableReq(t)
+
+	err := runPolicyOp(t, m, fs, "PutTableBucketPolicy", &PutTableBucketPolicyRequest{
+		TableBucketARN: bucketARN, ResourcePolicy: nullConditionDeny,
+	}, DefaultAccountID)
+	s3err := requireS3TablesError(t, err, ErrCodeInvalidRequest)
+	assert.Contains(t, s3err.Message, `"aws:ResourceTag/env"`)
+	_, stored := fs.Get(TablesPath, policyDenyBucket).Extended[ExtendedKeyPolicy]
+	assert.False(t, stored, "the bucket policy must not be written")
+
+	err = runPolicyOp(t, m, fs, "PutTablePolicy", &PutTablePolicyRequest{
+		TableBucketARN: bucketARN, Namespace: []string{ns}, Name: name, ResourcePolicy: nullConditionDeny,
+	}, DefaultAccountID)
+	s3err = requireS3TablesError(t, err, ErrCodeInvalidRequest)
+	assert.Contains(t, s3err.Message, `"aws:ResourceTag/env"`)
+	_, stored = fs.Get(GetNamespacePath(policyDenyBucket, ns), name).Extended[ExtendedKeyPolicy]
+	assert.False(t, stored, "the table policy must not be written")
+
+	// A string the numeric evaluator would skip is refused like a null one.
+	unreadable := `{"Statement":[{"Effect":"Deny","Principal":"*","Action":"s3tables:*","Resource":"*","Condition":{"NumericLessThan":{"aws:RequestTag/priority":"low"}}}]}`
+	err = runPolicyOp(t, m, fs, "PutTableBucketPolicy", &PutTableBucketPolicyRequest{
+		TableBucketARN: bucketARN, ResourcePolicy: unreadable,
+	}, DefaultAccountID)
+	s3err = requireS3TablesError(t, err, ErrCodeInvalidRequest)
+	assert.Contains(t, s3err.Message, "NumericLessThan")
+	_, stored = fs.Get(TablesPath, policyDenyBucket).Extended[ExtendedKeyPolicy]
+	assert.False(t, stored, "the bucket policy must not be written")
+
+	valid := `{"Statement":[{"Effect":"Deny","Principal":"*","Action":"s3tables:*","Resource":"*","Condition":{"StringEquals":{"aws:ResourceTag/env":"prod"}}}]}`
+	require.NoError(t, runPolicyOp(t, m, fs, "PutTableBucketPolicy", &PutTableBucketPolicyRequest{
+		TableBucketARN: bucketARN, ResourcePolicy: valid,
+	}, DefaultAccountID))
+	assert.Equal(t, valid, string(fs.Get(TablesPath, policyDenyBucket).Extended[ExtendedKeyPolicy]))
+}
+
+// The typed decode reads a null Condition as none, so an Allow carrying one is
+// refused on the bucket and on the table, and nothing is stored.
+func TestPutPolicyRejectsNullAllowCondition(t *testing.T) {
+	fs, m := startConditionManager(t, "")
+	bucketARN, ns, name := deniedTableReq(t)
+	allowNull := `{"Statement":[{"Effect":"Allow","Principal":"*","Action":"s3tables:*","Resource":"*","Condition":null}]}`
+
+	requireS3TablesError(t, runPolicyOp(t, m, fs, "PutTableBucketPolicy", &PutTableBucketPolicyRequest{
+		TableBucketARN: bucketARN, ResourcePolicy: allowNull,
+	}, DefaultAccountID), ErrCodeInvalidRequest)
+	_, stored := fs.Get(TablesPath, policyDenyBucket).Extended[ExtendedKeyPolicy]
+	assert.False(t, stored, "the bucket policy must not be written")
+
+	requireS3TablesError(t, runPolicyOp(t, m, fs, "PutTablePolicy", &PutTablePolicyRequest{
+		TableBucketARN: bucketARN, Namespace: []string{ns}, Name: name, ResourcePolicy: allowNull,
+	}, DefaultAccountID), ErrCodeInvalidRequest)
+	_, stored = fs.Get(GetNamespacePath(policyDenyBucket, ns), name).Extended[ExtendedKeyPolicy]
+	assert.False(t, stored, "the table policy must not be written")
+}
+
+// A keyless operator stored before write-time checks made its Allow grant
+// everyone; it is now unusable, so a non-owner is turned away. GetTableBucket
+// hides a bucket the caller may not read, so the refusal is NoSuchBucket,
+// while GetTableBucketPolicy answers AccessDenied.
+func TestStoredKeylessAllowDoesNotGrant(t *testing.T) {
+	keylessAllow := `{"Statement":[{"Effect":"Allow","Principal":"*","Action":"s3tables:*","Resource":"*","Condition":{"StringEquals":{}}}]}`
+	fs, m := startConditionManager(t, keylessAllow)
+	bucketARN := policyDenyBucketARN(t)
+
+	requireS3TablesError(t, runPolicyOp(t, m, fs, "GetTableBucket", &GetTableBucketRequest{
+		TableBucketARN: bucketARN,
+	}, "reader"), ErrCodeNoSuchBucket)
+	requireS3TablesError(t, runPolicyOp(t, m, fs, "GetTableBucketPolicy", &GetTableBucketPolicyRequest{
+		TableBucketARN: bucketARN,
+	}, "reader"), ErrCodeAccessDenied)
+	require.NoError(t, runPolicyOp(t, m, fs, "GetTableBucket", &GetTableBucketRequest{
+		TableBucketARN: bucketARN,
+	}, DefaultAccountID), "the owner keeps access")
+}
+
+// A bucket policy stored before conditions were validated must not cut the
+// connection: the owner gets an authorization error, the admin can still
+// remove the policy, and the owner is let back in once it is gone.
+func TestStoredUnusableConditionPolicyIsRecoverable(t *testing.T) {
+	fs, m := startConditionManager(t, nullConditionDeny)
+	bucketARN, ns, name := deniedTableReq(t)
+
+	requireS3TablesError(t, runPolicyOp(t, m, fs, "GetTableBucket", &GetTableBucketRequest{
+		TableBucketARN: bucketARN,
+	}, DefaultAccountID), ErrCodeNoSuchBucket)
+	require.Error(t, runPolicyOp(t, m, fs, "GetTable", &GetTableRequest{
+		TableBucketARN: bucketARN, Namespace: []string{ns}, Name: name,
+	}, DefaultAccountID), "the broken bucket Deny must veto table reads")
+	requireS3TablesError(t, runPolicyOp(t, m, fs, "DeleteTableBucketPolicy", &DeleteTableBucketPolicyRequest{
+		TableBucketARN: bucketARN,
+	}, DefaultAccountID), ErrCodeAccessDenied)
+	assert.Equal(t, nullConditionDeny, string(fs.Get(TablesPath, policyDenyBucket).Extended[ExtendedKeyPolicy]))
+
+	require.NoError(t, runPolicyOp(t, m, fs, "DeleteTableBucketPolicy", &DeleteTableBucketPolicyRequest{
+		TableBucketARN: bucketARN,
+	}, s3_constants.AccountAdminId), "the admin must be able to remove the broken policy")
+	_, stored := fs.Get(TablesPath, policyDenyBucket).Extended[ExtendedKeyPolicy]
+	assert.False(t, stored, "the broken policy must be gone")
+
+	require.NoError(t, runPolicyOp(t, m, fs, "GetTableBucket", &GetTableBucketRequest{
+		TableBucketARN: bucketARN,
+	}, DefaultAccountID))
+	require.NoError(t, runPolicyOp(t, m, fs, "GetTable", &GetTableRequest{
+		TableBucketARN: bucketARN, Namespace: []string{ns}, Name: name,
+	}, DefaultAccountID))
+}
@@ -1,9 +1,14 @@
 package s3tables

 import (
+	"bytes"
 	"encoding/json"
 	"fmt"
+	"math"
+	"net"
+	"strconv"
 	"strings"
+	"time"

 	"github.com/seaweedfs/seaweedfs/weed/glog"
 	"github.com/seaweedfs/seaweedfs/weed/iam/sts"
@@ -26,7 +31,7 @@ type PolicyDocument struct {
 func (pd *PolicyDocument) UnmarshalJSON(data []byte) error {
 	type Alias PolicyDocument
 	aux := &struct {
-		Statement interface{} `json:"Statement"`
+		Statement json.RawMessage `json:"Statement"`
 		*Alias
 	}{
 		Alias: (*Alias)(pd),
@@ -35,9 +40,16 @@ func (pd *PolicyDocument) UnmarshalJSON(data []byte) error {
 	if err := json.Unmarshal(data, &aux); err != nil {
 		return err
 	}
+	var statement interface{}
+	if len(aux.Statement) > 0 {
+		if err := json.Unmarshal(aux.Statement, &statement); err != nil {
+			return err
+		}
+	}

 	// Handle Statement as either a single object or array
-	switch s := aux.Statement.(type) {
+	raws := []json.RawMessage{aux.Statement}
+	switch s := statement.(type) {
 	case map[string]interface{}:
 		// Single statement object - unmarshal to one Statement
 		stmtData, err := json.Marshal(s)
@@ -58,13 +70,23 @@ func (pd *PolicyDocument) UnmarshalJSON(data []byte) error {
 		if err := json.Unmarshal(stmtData, &pd.Statement); err != nil {
 			return fmt.Errorf("failed to unmarshal statement array: %w", err)
 		}
+		if err := json.Unmarshal(aux.Statement, &raws); err != nil {
+			return err
+		}
 	case nil:
 		// No statements
 		pd.Statement = []Statement{}
 	default:
-		return fmt.Errorf("Statement must be an object or array, got %T", aux.Statement)
+		return fmt.Errorf("Statement must be an object or array, got %T", statement)
 	}

+	// The re-marshal above keeps only the last of a repeated key, so the
+	// Condition keys are counted from the original bytes.
+	for i := range pd.Statement {
+		if i < len(raws) {
+			pd.Statement[i].nullCondition, pd.Statement[i].splitCondition = conditionKeys(raws[i])
+		}
+	}
 	return nil
 }

@@ -74,6 +96,44 @@ type Statement struct {
 	Action    interface{}                       `json:"Action"`    // Can be string or []string
 	Resource  interface{}                       `json:"Resource"`  // Can be string or []string
 	Condition map[string]map[string]interface{} `json:"Condition,omitempty"`
+	// nullCondition records a literal "Condition": null, which decodes the
+	// same as no Condition.
+	nullCondition bool
+	// splitCondition records more than one Condition key, in any spelling.
+	// Those objects decode into one map in which an operator they share keeps
+	// only the last one's keys.
+	splitCondition bool
+}
+
+// conditionKeys walks a statement object's keys, which a map would collapse,
+// and reports a null Condition and a Condition under more than one key.
+func conditionKeys(data []byte) (null, split bool) {
+	dec := json.NewDecoder(bytes.NewReader(data))
+	if tok, err := dec.Token(); err != nil || tok != json.Delim('{') {
+		return false, false
+	}
+	conditions := 0
+	for dec.More() {
+		key, err := dec.Token()
+		if err != nil {
+			break
+		}
+		var raw json.RawMessage
+		if err := dec.Decode(&raw); err != nil {
+			break
+		}
+		if name, _ := key.(string); strings.EqualFold(name, "Condition") {
+			conditions++
+			null = null || string(raw) == "null"
+		}
+	}
+	return null, conditions > 1
+}
+
+// SplitCondition reports a statement with more than one Condition key, which
+// the evaluator never grants on and always vetoes with.
+func (s *Statement) SplitCondition() bool {
+	return s.splitCondition
 }

 type PolicyContext struct {
@@ -142,15 +202,42 @@ func CheckTableOrBucketPermission(operation, principal, resourceOwner, resourceP
 }

 // validatePolicyDocument rejects a policy the evaluator could not honor: one
-// that does not parse, or whose actions and resources name the vector service.
-// Either stored here would be a dead rule the writer believes is enforced —
-// vector requests authorize their s3tables action on a s3tables ARN.
+// that does not parse, whose actions and resources name the vector service,
+// whose conditions the evaluator cannot compare, or whose Allow carries a null
+// Condition. Any of them stored here would be a rule the writer believes is
+// enforced — vector requests authorize their s3tables action on a s3tables
+// ARN, an unusable condition is skipped on an Allow and vetoes everything on a
+// Deny, and a null Condition reads as none, so the Allow grants unconditionally.
 func validatePolicyDocument(raw string) error {
 	var doc PolicyDocument
 	if err := json.Unmarshal([]byte(raw), &doc); err != nil {
 		return fmt.Errorf("invalid policy document: %w", err)
 	}
 	for _, stmt := range doc.Statement {
+		// A null Condition on a Deny only widens the veto, so it stays allowed.
+		if stmt.Effect == "Allow" && stmt.nullCondition {
+			return fmt.Errorf("invalid policy document: an Allow statement's Condition must not be null")
+		}
+		if stmt.splitCondition {
+			return fmt.Errorf("invalid policy document: a statement's Condition must be under one key, spelled Condition")
+		}
+		for operator, keys := range stmt.Condition {
+			if _, err := policy_engine.GetConditionEvaluator(operator); err != nil {
+				return fmt.Errorf("invalid policy document: condition operator %q is not supported", operator)
+			}
+			// An operator with no keys holds for every request.
+			if len(keys) == 0 {
+				return fmt.Errorf("invalid policy document: condition %s needs at least one key", operator)
+			}
+			for key, value := range keys {
+				if !tableConditionKey(key) {
+					return fmt.Errorf("invalid policy document: condition key %q is not supported in a table policy", key)
+				}
+				if !ConditionValue(operator, value) {
+					return fmt.Errorf("invalid policy document: condition %s on %q needs values %s can read (a number for Numeric*, an RFC 3339 time for Date*, true or false for Bool and Null, an IP or CIDR for IpAddress)", operator, key, operator)
+				}
+			}
+		}
 		for _, action := range statementActions(stmt.Action) {
 			if strings.HasPrefix(strings.ToLower(action), "s3vectors:") {
 				return fmt.Errorf("action %q is not enforceable in a resource policy", action)
@@ -165,6 +252,73 @@ func validatePolicyDocument(raw string) error {
 	return nil
 }

+// ConditionValue reports whether operator's evaluator can read v: a JSON
+// type it can compare, and a value it parses rather than skips. A null
+// value would panic it, an empty list never matches, and an unreadable
+// value is skipped — dropping a Deny or matching a negated Allow.
+func ConditionValue(operator string, v interface{}) bool {
+	list, ok := v.([]interface{})
+	if !ok {
+		list = []interface{}{v}
+	}
+	base, qualified := strings.CutPrefix(operator, "ForAllValues:")
+	if !qualified {
+		base, _ = strings.CutPrefix(operator, "ForAnyValue:")
+	}
+	for _, item := range list {
+		switch item.(type) {
+		case string, float64, bool:
+		default:
+			return false
+		}
+		if !conditionValueReadable(base, item) {
+			return false
+		}
+	}
+	return len(list) > 0
+}
+
+// conditionValueReadable reports whether the evaluator for base parses item
+// the way it reads it. An operator without a case here is refused, so one
+// added to the policy engine cannot be accepted before it is checked.
+func conditionValueReadable(base string, item interface{}) bool {
+	switch base {
+	case "StringEquals", "StringNotEquals", "StringLike", "StringNotLike",
+		"StringEqualsIgnoreCase", "StringNotEqualsIgnoreCase", "ArnEquals", "ArnLike":
+		return true
+	case "NumericEquals", "NumericNotEquals", "NumericLessThan", "NumericLessThanEquals",
+		"NumericGreaterThan", "NumericGreaterThanEquals":
+		switch v := item.(type) {
+		case float64:
+			return true
+		case string:
+			f, err := strconv.ParseFloat(v, 64)
+			return err == nil && !math.IsNaN(f) && !math.IsInf(f, 0)
+		}
+	case "DateEquals", "DateNotEquals", "DateLessThan", "DateLessThanEquals",
+		"DateGreaterThan", "DateGreaterThanEquals":
+		if v, ok := item.(string); ok {
+			_, err := time.Parse(time.RFC3339, v)
+			return err == nil
+		}
+	case "Bool", "Null":
+		switch v := item.(type) {
+		case bool:
+			return true
+		case string:
+			return strings.EqualFold(v, "true") || strings.EqualFold(v, "false")
+		}
+	case "IpAddress", "NotIpAddress":
+		if v, ok := item.(string); ok {
+			if _, _, err := net.ParseCIDR(v); err == nil {
+				return true
+			}
+			return net.ParseIP(v) != nil
+		}
+	}
+	return false
+}
+
 // arnService returns the service field of an ARN in any partition, so a
 // resource naming s3vectors is caught for aws-cn, aws-us-gov, and friends —
 // not just the commercial partition.
@@ -210,7 +364,9 @@ func adminCaller(operation string, ctx *PolicyContext) bool {

 // matchesDenyStatement reports whether the JSON policy carries a statement
 // matching this request with Effect Deny. Malformed policies return false;
-// the caller's normal evaluation still denies them.
+// the caller's normal evaluation still denies them. A Deny whose condition
+// cannot be evaluated applies: it was written to protect the resource, and
+// dropping it would widen access.
 func matchesDenyStatement(operation, principal, resourcePolicy, resourceARN string, ctx *PolicyContext) bool {
 	if resourcePolicy == "" {
 		return false
@@ -227,11 +383,17 @@ func matchesDenyStatement(operation, principal, resourcePolicy, resourceARN stri
 		if stmt.Effect != "Deny" ||
 			!matchesDenyCaller(stmt.Principal, principal, ctx) ||
 			!matchesAction(stmt.Action, fullAction) ||
-			(resourceARN != "" && !matchesResource(stmt.Resource, resourceARN)) ||
-			!matchesConditions(stmt.Condition, ctx) {
+			(resourceARN != "" && !matchesResource(stmt.Resource, resourceARN)) {
 			continue
 		}
-		return true
+		matched, usable := matchesConditions(stmt.Condition, ctx)
+		if !usable || stmt.splitCondition {
+			glog.Warningf("S3Tables: policy on %q has a Deny condition that cannot be evaluated; denying %s for %s until the policy is replaced", resourceARN, fullAction, principal)
+			return true
+		}
+		if matched {
+			return true
+		}
 	}
 	return false
 }
@@ -294,7 +456,12 @@ func checkPermission(operation, principal, owner, resourcePolicy, resourceARN st
 			continue
 		}

-		if !matchesConditions(stmt.Condition, ctx) {
+		// A condition that cannot be evaluated never grants, nor does a null
+		// or split one stored before validatePolicyDocument refused it.
+		if stmt.Effect == "Allow" && (stmt.nullCondition || stmt.splitCondition) {
+			continue
+		}
+		if matched, usable := matchesConditions(stmt.Condition, ctx); !usable || !matched {
 			continue
 		}

@@ -527,34 +694,67 @@ func matchesActionPattern(pattern, action string) bool {
 	return wildcard.MatchesWildcard(pattern, action)
 }

-func matchesConditions(conditions map[string]map[string]interface{}, ctx *PolicyContext) bool {
+// matchesConditions reports whether every condition holds for ctx, and
+// whether the block can be evaluated at all: one unsupported operator, keyless
+// operator or uncomparable value leaves it unusable, and the caller decides
+// what that means for the statement's Effect. Every operator is checked
+// before answering, so the verdict does not depend on map iteration order.
+func matchesConditions(conditions map[string]map[string]interface{}, ctx *PolicyContext) (matched, usable bool) {
 	if len(conditions) == 0 {
-		return true
-	}
-	if ctx == nil {
-		return false
+		return true, true
 	}
+	matched = true
 	for operator, conditionValues := range conditions {
-		if !matchesConditionOperator(operator, conditionValues, ctx) {
-			return false
+		ok, valid := matchesConditionOperator(operator, conditionValues, ctx)
+		if !valid {
+			return false, false
 		}
+		matched = matched && ok
 	}
-	return true
+	return matched, true
 }

-func matchesConditionOperator(operator string, conditionValues map[string]interface{}, ctx *PolicyContext) bool {
+func matchesConditionOperator(operator string, conditionValues map[string]interface{}, ctx *PolicyContext) (matched, usable bool) {
 	evaluator, err := policy_engine.GetConditionEvaluator(operator)
 	if err != nil {
-		return false
+		return false, false
+	}
+	// With no keys the loop below would hold for every request; a stored
+	// keyless operator is unusable, so it never lets an Allow grant.
+	if len(conditionValues) == 0 {
+		return false, false
+	}
+	for _, value := range conditionValues {
+		if !ConditionValue(operator, value) {
+			return false, false
+		}
+	}
+	if ctx == nil {
+		return false, true
 	}

 	for key, value := range conditionValues {
 		contextVals := getConditionContextValues(key, ctx)
 		if !evaluator.Evaluate(value, contextVals) {
-			return false
+			return false, true
 		}
 	}
-	return true
+	return true, true
+}
+
+// tableConditionKey reports whether getConditionContextValues supplies key.
+func tableConditionKey(key string) bool {
+	switch key {
+	case "s3tables:namespace", "s3tables:tableName", "s3tables:tableBucketName",
+		"s3tables:SSEAlgorithm", "s3tables:KMSKeyArn", "s3tables:StorageClass", "aws:TagKeys":
+		return true
+	}
+	for _, prefix := range []string{"aws:RequestTag/", "aws:ResourceTag/", "s3tables:TableBucketTag/", "s3vectors:VectorBucketTag/"} {
+		if tagKey, ok := strings.CutPrefix(key, prefix); ok {
+			return tagKey != ""
+		}
+	}
+	return false
 }

 func getConditionContextValues(key string, ctx *PolicyContext) []string {
@@ -2,9 +2,17 @@ package s3tables

 import (
 	"encoding/json"
+	"fmt"
+	"go/ast"
+	"go/parser"
+	"go/token"
+	"path/filepath"
+	"strconv"
+	"strings"
 	"testing"

 	"github.com/seaweedfs/seaweedfs/weed/iam/sts"
+	"github.com/seaweedfs/seaweedfs/weed/s3api/policy_engine"
 )

 func TestMatchesActionPattern(t *testing.T) {
@@ -127,6 +135,9 @@ func TestEvaluatePolicyWithConditions(t *testing.T) {
 	}
 	policyBytes, _ := json.Marshal(policy)
 	policyStr := string(policyBytes)
+	if err := validatePolicyDocument(policyStr); err != nil {
+		t.Fatalf("conditional policy rejected by the validator: %v", err)
+	}

 	tests := []struct {
 		name     string
@@ -492,6 +503,391 @@ func TestValidatePolicyDocument(t *testing.T) {
 	if err := validatePolicyDocument(deadMixed); err == nil {
 		t.Fatal("resource list containing s3vectors should be rejected")
 	}
+
+	withCondition := func(condition string) string {
+		return `{"Version":"2012-10-17","Statement":[{"Effect":"Deny","Principal":"*","Action":"s3tables:GetTable","Resource":"*","Condition":` + condition + `}]}`
+	}
+	for _, tc := range []struct {
+		condition string
+		mentions  []string // the error must name these; empty means accepted
+	}{
+		{`{"StringEquals":{"aws:ResourceTag/env":"prod"}}`, nil},
+		{`{"StringEquals":{"aws:ResourceTag/env":["a","b"]}}`, nil},
+		{`{"NumericLessThan":{"aws:RequestTag/priority":10}}`, nil},
+		{`{"Bool":{"aws:ResourceTag/is_public":true}}`, nil},
+		{`{"ForAnyValue:StringEquals":{"aws:TagKeys":["team","env"]}}`, nil},
+		{`{"Null":{"aws:ResourceTag/owner":"true"}}`, nil},
+		{`{"StringEquals":{"s3tables:namespace":"default"}}`, nil},
+		{`{"StringEquals":{"aws:ResourceTag/env":null}}`, []string{"StringEquals", `"aws:ResourceTag/env"`}},
+		{`{"StringEquals":{"aws:ResourceTag/env":[null]}}`, []string{"StringEquals", `"aws:ResourceTag/env"`}},
+		{`{"StringNotEquals":{"aws:ResourceTag/env":[]}}`, []string{"StringNotEquals", `"aws:ResourceTag/env"`}},
+		{`{"StringEquals":{"aws:ResourceTag/env":{}}}`, []string{"StringEquals", `"aws:ResourceTag/env"`}},
+		{`{"StringEquals":{"aws:ResourceTag/env":{"a":1}}}`, []string{"StringEquals", `"aws:ResourceTag/env"`}},
+		{`{"StringEquals":{"aws:ResourceTag/env":[["a"]]}}`, []string{"StringEquals", `"aws:ResourceTag/env"`}},
+		{`{"StringEquals":null}`, []string{"StringEquals", "at least one key"}},
+		{`{"StringEquals":{}}`, []string{"StringEquals", "at least one key"}},
+		{`{"StringEqualsBogus":{"aws:ResourceTag/env":"prod"}}`, []string{`"StringEqualsBogus"`, "not supported"}},
+		{`{"NumericLessThan":{"aws:RequestTag/priority":"10"}}`, nil},
+		{`{"DateLessThan":{"aws:ResourceTag/created":"2026-01-01T00:00:00Z"}}`, nil},
+		{`{"Bool":{"aws:ResourceTag/is_public":"TRUE"}}`, nil},
+		{`{"IpAddress":{"aws:ResourceTag/net":"10.0.0.0/8"}}`, nil},
+		{`{"StringEquals":{"aws:ResourceTag/env":10}}`, nil},
+		{`{"NumericLessThan":{"aws:RequestTag/priority":"low"}}`, []string{"NumericLessThan", `"aws:RequestTag/priority"`}},
+		{`{"NumericLessThan":{"aws:RequestTag/priority":true}}`, []string{"NumericLessThan", `"aws:RequestTag/priority"`}},
+		{`{"NumericLessThan":{"aws:RequestTag/priority":["low","10"]}}`, []string{"NumericLessThan", `"aws:RequestTag/priority"`}},
+		{`{"ForAnyValue:NumericLessThan":{"aws:RequestTag/priority":"low"}}`, []string{"ForAnyValue:NumericLessThan", `"aws:RequestTag/priority"`}},
+		{`{"DateLessThan":{"aws:ResourceTag/created":"2026-01-01"}}`, []string{"DateLessThan", `"aws:ResourceTag/created"`}},
+		{`{"Bool":{"aws:ResourceTag/is_public":"yes"}}`, []string{"Bool", `"aws:ResourceTag/is_public"`}},
+		{`{"Null":{"aws:ResourceTag/owner":"maybe"}}`, []string{"Null", `"aws:ResourceTag/owner"`}},
+		{`{"IpAddress":{"aws:ResourceTag/net":"not-an-ip"}}`, []string{"IpAddress", `"aws:ResourceTag/net"`}},
+		{`{"StringEquals":{"s3tables:tableName":"t","s3tables:tableBucketName":"b","s3tables:StorageClass":"STANDARD"}}`, nil},
+		{`{"StringEquals":{"s3tables:SSEAlgorithm":"AES256","s3tables:KMSKeyArn":"arn:k","s3tables:TableBucketTag/env":"prod"}}`, nil},
+		{`{"IpAddress":{"aws:SourceIp":"10.0.0.0/8"}}`, []string{`"aws:SourceIp"`, "not supported"}},
+		{`{"StringEquals":{"aws:resourcetag/env":"prod"}}`, []string{`"aws:resourcetag/env"`, "not supported"}},
+		{`{"StringEquals":{"aws:ResourceTag/":"prod"}}`, []string{`"aws:ResourceTag/"`, "not supported"}},
+		{`{"Null":{"aws:PrincipalArn":"true"}}`, []string{`"aws:PrincipalArn"`, "not supported"}},
+	} {
+		err := validatePolicyDocument(withCondition(tc.condition))
+		if len(tc.mentions) == 0 {
+			if err != nil {
+				t.Errorf("%s rejected: %v", tc.condition, err)
+			}
+			continue
+		}
+		if err == nil {
+			t.Errorf("%s accepted", tc.condition)
+			continue
+		}
+		for _, want := range tc.mentions {
+			if !strings.Contains(err.Error(), want) {
+				t.Errorf("%s: error %q does not mention %s", tc.condition, err, want)
+			}
+		}
+	}
+
+	// The typed decode reads a null Condition as none, so an Allow carrying
+	// one would grant unconditionally; a Deny carrying one only vetoes more.
+	const (
+		allowNull      = `{"Effect":"Allow","Principal":"*","Action":"s3tables:GetTable","Resource":"*","Condition":null}`
+		allowNullLower = `{"Effect":"Allow","Principal":"*","Action":"s3tables:GetTable","Resource":"*","condition":null}`
+		allowValid     = `{"Effect":"Allow","Principal":"*","Action":"s3tables:GetTable","Resource":"*","Condition":{"StringEquals":{"aws:ResourceTag/env":"prod"}}}`
+		allowNone      = `{"Effect":"Allow","Principal":"*","Action":"s3tables:GetTable","Resource":"*"}`
+		denyNull       = `{"Effect":"Deny","Principal":"*","Action":"s3tables:GetTable","Resource":"*","Condition":null}`
+		denyNone       = `{"Effect":"Deny","Principal":"*","Action":"s3tables:GetTable","Resource":"*"}`
+		// Duplicate keys reach the evaluator in sorted order, not document
+		// order: the lower-case null wins here, the object wins in the mirror,
+		// and both are refused whichever spelling is null.
+		allowNullWins   = `{"Effect":"Allow","Principal":"*","Action":"s3tables:GetTable","Resource":"*","condition":null,"Condition":{"StringEquals":{"aws:ResourceTag/env":"prod"}}}`
+		allowObjectWins = `{"Effect":"Allow","Principal":"*","Action":"s3tables:GetTable","Resource":"*","Condition":null,"condition":{"StringEquals":{"aws:ResourceTag/env":"prod"}}}`
+	)
+	for _, tc := range []struct {
+		statement string
+		rejected  bool
+	}{
+		{`[` + allowNull + `]`, true},
+		{`[` + allowNullLower + `]`, true},
+		{allowNull, true},
+		{`[` + denyNone + `,` + allowNull + `]`, true},
+		{`[` + allowNullWins + `]`, true},
+		{`[` + denyNull + `]`, false},
+		{`[` + denyNull + `,` + allowValid + `]`, false},
+		{`[` + allowNone + `]`, false},
+		{`[` + allowObjectWins + `]`, true},
+	} {
+		raw := `{"Version":"2012-10-17","Statement":` + tc.statement + `}`
+		err := validatePolicyDocument(raw)
+		switch {
+		case tc.rejected && err == nil:
+			t.Errorf("%s accepted", tc.statement)
+		case tc.rejected && !strings.Contains(err.Error(), "Condition must not be null"):
+			t.Errorf("%s: unexpected error %v", tc.statement, err)
+		case !tc.rejected && err != nil:
+			t.Errorf("%s rejected: %v", tc.statement, err)
+		}
+	}
+}
+
+func TestConditionValue(t *testing.T) {
+	for _, tc := range []struct {
+		operator string
+		raw      string
+		want     bool
+	}{
+		{"StringEquals", `"prod"`, true},
+		{"StringEquals", `["a","b"]`, true},
+		{"StringEquals", `10`, true},
+		{"StringEquals", `true`, true},
+		{"StringEquals", `null`, false},
+		{"StringEquals", `[null]`, false},
+		{"StringEquals", `["a",null]`, false},
+		{"StringEquals", `[]`, false},
+		{"StringEquals", `{}`, false},
+		{"StringEquals", `{"a":1}`, false},
+		{"StringEquals", `[["a"]]`, false},
+
+		{"NumericLessThan", `10`, true},
+		{"NumericLessThan", `"10"`, true},
+		{"NumericLessThan", `"low"`, false},
+		{"NumericLessThan", `true`, false},
+		{"NumericLessThan", `["low","10"]`, false},
+		{"NumericLessThan", `"NaN"`, false},
+		{"NumericNotEquals", `"nan"`, false},
+		{"NumericNotEquals", `"Inf"`, false},
+		{"NumericGreaterThan", `"-Infinity"`, false},
+		{"ForAnyValue:NumericLessThan", `"low"`, false},
+		{"ForAllValues:NumericLessThan", `"10"`, true},
+
+		{"DateLessThan", `"2026-01-01T00:00:00Z"`, true},
+		{"DateLessThan", `"2026-01-01"`, false},
+
+		{"Bool", `true`, true},
+		{"Bool", `"TRUE"`, true},
+		{"Bool", `"yes"`, false},
+		{"Null", `"false"`, true},
+		{"Null", `"maybe"`, false},
+
+		{"IpAddress", `"10.0.0.0/8"`, true},
+		{"IpAddress", `"10.0.0.1"`, true},
+		{"IpAddress", `"not-an-ip"`, false},
+
+		{"StringEqualsBogus", `"prod"`, false},
+	} {
+		var v interface{}
+		if err := json.Unmarshal([]byte(tc.raw), &v); err != nil {
+			t.Fatalf("%s: %v", tc.raw, err)
+		}
+		if got := ConditionValue(tc.operator, v); got != tc.want {
+			t.Errorf("ConditionValue(%s, %s) = %v, want %v", tc.operator, tc.raw, got, tc.want)
+		}
+	}
+}
+
+// Every operator the policy engine evaluates must have its own case in
+// ConditionValue: one that falls through to the default refuses every value,
+// so a newly added operator turns this test red instead of being accepted
+// unchecked or refused unnoticed.
+func TestConditionValueCoversEveryOperator(t *testing.T) {
+	file, err := parser.ParseFile(token.NewFileSet(), filepath.Join("..", "policy_engine", "conditions.go"), nil, 0)
+	if err != nil {
+		t.Fatal(err)
+	}
+	var operators []string
+	ast.Inspect(file, func(n ast.Node) bool {
+		fn, ok := n.(*ast.FuncDecl)
+		if !ok || fn.Name.Name != "baseConditionEvaluator" {
+			return true
+		}
+		ast.Inspect(fn.Body, func(n ast.Node) bool {
+			if clause, ok := n.(*ast.CaseClause); ok {
+				for _, expr := range clause.List {
+					if lit, ok := expr.(*ast.BasicLit); ok && lit.Kind == token.STRING {
+						operator, _ := strconv.Unquote(lit.Value)
+						operators = append(operators, operator)
+					}
+				}
+			}
+			return true
+		})
+		return false
+	})
+	if len(operators) < 20 {
+		t.Fatalf("found only %d operators in baseConditionEvaluator", len(operators)) // positive control
+	}
+	samples := []interface{}{"x", "10", "2026-01-01T00:00:00Z", "true", "10.0.0.0/8"}
+	for _, operator := range operators {
+		if _, err := policy_engine.GetConditionEvaluator(operator); err != nil {
+			t.Fatalf("%s: %v", operator, err)
+		}
+		readable := false
+		for _, sample := range samples {
+			readable = readable || ConditionValue(operator, sample)
+		}
+		if !readable {
+			t.Errorf("ConditionValue has no case for %s", operator)
+		}
+	}
+}
+
+// A value the operator cannot read is skipped by its evaluator, so before
+// write-time checks such a Deny never matched and the owner kept access. A
+// stored one is now unusable: the Deny applies, and the admin can still
+// remove the policy.
+func TestUnreadableConditionDenyApplies(t *testing.T) {
+	arn := "arn:aws:s3tables:us-east-1:acct:bucket/b"
+	ctx := &PolicyContext{
+		RequestTags:  map[string]string{"priority": "5"},
+		ResourceTags: map[string]string{"is_public": "true", "created": "2025-06-01T00:00:00Z"},
+	}
+	admin := &PolicyContext{IdentityActions: []string{"Admin"}, RequestTags: ctx.RequestTags, ResourceTags: ctx.ResourceTags}
+	for _, condition := range []string{
+		`{"NumericLessThan":{"aws:RequestTag/priority":"low"}}`,
+		`{"DateLessThan":{"aws:ResourceTag/created":"2026-01-01"}}`,
+		`{"Bool":{"aws:ResourceTag/is_public":"yes"}}`,
+		`{"NumericLessThan":{"aws:RequestTag/priority":"NaN"}}`,
+	} {
+		var conditions map[string]map[string]interface{}
+		if err := json.Unmarshal([]byte(condition), &conditions); err != nil {
+			t.Fatalf("%s: %v", condition, err)
+		}
+		if _, usable := matchesConditions(conditions, ctx); usable {
+			t.Errorf("%s reported usable", condition)
+		}
+		deny := `{"Statement":[{"Effect":"Deny","Principal":"*","Action":"s3tables:*","Resource":"*","Condition":` + condition + `}]}`
+		if checkPermission("s3tables:GetTableBucket", "owner", "owner", deny, arn, ctx) {
+			t.Errorf("%s: the Deny must apply to the owner", condition)
+		}
+		if !CheckPermissionWithContext("s3tables:DeleteTableBucketPolicy", "admin", "owner", deny, arn, admin) {
+			t.Errorf("%s: the admin must keep the bypass", condition)
+		}
+	}
+}
+
+// A policy stored before conditions were validated may still carry a null
+// value. Evaluating it must not panic: the Deny it sits on applies to the
+// owner too, while the admin keeps the bypass needed to remove the policy.
+func TestNullConditionDoesNotPanic(t *testing.T) {
+	arn := "arn:aws:s3tables:us-east-1:acct:bucket/b"
+	denyNull := `{"Statement":[{"Effect":"Deny","Principal":"*","Action":"s3tables:*","Resource":"*","Condition":{"StringEquals":{"aws:ResourceTag/env":null}}}]}`
+	ctx := &PolicyContext{ResourceTags: map[string]string{"env": "prod"}}
+
+	if checkPermission("s3tables:GetTableBucket", "owner", "owner", denyNull, arn, ctx) {
+		t.Error("a Deny whose condition cannot be evaluated must still deny the owner")
+	}
+	if CheckTableOrBucketPermission("s3tables:GetTable", "owner", "owner", "", arn+"/table/ns/t", "owner", denyNull, arn, ctx) {
+		t.Error("a broken bucket Deny must veto the table request too")
+	}
+	admin := &PolicyContext{IdentityActions: []string{"Admin"}, ResourceTags: ctx.ResourceTags}
+	if !CheckPermissionWithContext("s3tables:DeleteTableBucketPolicy", "admin", "owner", denyNull, arn, admin) {
+		t.Error("the admin must keep the bypass to remove a broken policy")
+	}
+}
+
+// An Allow whose condition cannot be evaluated grants nothing. A [null] value
+// used to normalize to "<nil>", so StringNotEquals matched every request.
+func TestUnusableConditionAllowNeverGrants(t *testing.T) {
+	ctx := &PolicyContext{ResourceTags: map[string]string{"env": "prod"}}
+	for _, condition := range []string{
+		`{"StringNotEquals":{"aws:ResourceTag/env":[null]}}`,
+		`{"StringNotEquals":{"aws:ResourceTag/env":[]}}`,
+		`{"StringNotEquals":{"aws:ResourceTag/env":{"a":1}}}`,
+		`{"StringEquals":{"aws:ResourceTag/env":null}}`,
+		`{"StringEqualsBogus":{"aws:ResourceTag/env":"prod"}}`,
+		`{"NumericNotEquals":{"aws:ResourceTag/env":"low"}}`,
+		`{"NotIpAddress":{"aws:ResourceTag/env":"not-an-ip"}}`,
+		`{"NumericNotEquals":{"aws:ResourceTag/env":"NaN"}}`,
+		`{"StringEquals":{}}`,
+		`{"StringEquals":null}`,
+	} {
+		allow := `{"Statement":[{"Effect":"Allow","Principal":"*","Action":"s3tables:GetTable","Resource":"*","Condition":` + condition + `}]}`
+		if CheckPermissionWithContext("s3tables:GetTable", "reader", "owner", allow, "", ctx) {
+			t.Errorf("%s granted a non-owner", condition)
+		}
+	}
+	// A keyless operator is unusable whether or not there is a context.
+	if _, usable := matchesConditions(map[string]map[string]interface{}{"StringEquals": {}}, nil); usable {
+		t.Error("a keyless operator without a context reported usable")
+	}
+}
+
+// A stored Allow whose Condition is null never grants; a Deny with one still
+// applies.
+func TestStoredNullConditionAllowNeverGrants(t *testing.T) {
+	ctx := &PolicyContext{}
+	for _, tc := range []struct{ action, policy string }{
+		{"s3tables:GetTable", `{"Statement":[{"Effect":"Allow","Principal":"*","Action":"s3tables:GetTable","Resource":"*","Condition":null}]}`},
+		{"s3tables:GetTable", `{"Statement":{"Effect":"Allow","Principal":"*","Action":"s3tables:GetTable","Resource":"*","Condition":null}}`},
+		{"s3tables:GetTable", `{"Statement":[{"Effect":"Allow","Principal":"*","Action":"s3tables:GetTable","Resource":"*","condition":null}]}`},
+		{"s3tables:GetTable", `{"Statement":[{"Effect":"Allow","Principal":"*","Action":"s3tables:GetTable","Resource":"*","Condition":{"StringEquals":{"aws:ResourceTag/env":"prod"}},"condition":null}]}`},
+		{"s3vectors:GetVectorBucket", `{"Statement":[{"Effect":"Allow","Principal":"*","Action":"s3vectors:GetVectorBucket","Resource":"*","Condition":null}]}`},
+	} {
+		if CheckPermissionWithContext(tc.action, "reader", "owner", tc.policy, "", ctx) {
+			t.Errorf("%s granted a non-owner", tc.policy)
+		}
+	}
+	allow := `{"Statement":[{"Effect":"Allow","Principal":"*","Action":"s3tables:GetTable","Resource":"*"}]}`
+	if !CheckPermissionWithContext("s3tables:GetTable", "reader", "owner", allow, "", ctx) {
+		t.Error("an Allow without a Condition must still grant")
+	}
+	deny := `{"Statement":[{"Effect":"Deny","Principal":"*","Action":"s3tables:GetTable","Resource":"*","Condition":null}]}`
+	if CheckPermissionWithContext("s3tables:GetTable", "owner", "owner", deny, "", ctx) {
+		t.Error("a Deny with a null Condition must apply")
+	}
+}
+
+// Condition objects under two keys, repeated or respelled, decode into one map
+// in which an operator they share keeps only the last one's keys. Such a
+// statement is refused, and one stored before that never grants and always
+// vetoes.
+func TestConditionSplitAcrossSpellings(t *testing.T) {
+	for _, split := range []string{
+		`"Condition":{"StringEquals":{"aws:ResourceTag/env":"prod"}},"condition":{"StringEquals":{"aws:ResourceTag/team":"x"}}`,
+		`"Condition":{"StringEquals":{"aws:ResourceTag/env":"prod"}},"condition":{}`,
+		`"Condition":{"StringEquals":{"aws:ResourceTag/env":"prod"}},"Condition":{"StringEquals":{"aws:ResourceTag/team":"x"}}`,
+		`"Condition":null,"Condition":{}`,
+	} {
+		for _, wrap := range []string{`{"Statement":[%s]}`, `{"Statement":%s}`} {
+			allow := fmt.Sprintf(wrap, `{"Effect":"Allow","Principal":"*","Action":"s3tables:GetTable","Resource":"*",`+split+`}`)
+			if err := validatePolicyDocument(allow); err == nil {
+				t.Errorf("%s: validatePolicyDocument accepted it", allow)
+			}
+			if CheckPermissionWithContext("s3tables:GetTable", "reader", "owner", allow, "", &PolicyContext{ResourceTags: map[string]string{"env": "dev", "team": "x"}}) {
+				t.Errorf("%s: the Allow granted", allow)
+			}
+			deny := fmt.Sprintf(wrap, `{"Effect":"Deny","Principal":"*","Action":"s3tables:GetTable","Resource":"*",`+split+`}`)
+			if err := validatePolicyDocument(deny); err == nil {
+				t.Errorf("%s: validatePolicyDocument accepted it", deny)
+			}
+			if CheckPermissionWithContext("s3tables:GetTable", "owner", "owner", deny, "", &PolicyContext{ResourceTags: map[string]string{"env": "dev", "team": "x"}}) {
+				t.Errorf("%s: the Deny was skipped", deny)
+			}
+		}
+	}
+	single := `{"Statement":[{"Effect":"Allow","Principal":"*","Action":"s3tables:GetTable","Resource":"*","condition":{"StringEquals":{"aws:ResourceTag/env":"prod"}}}]}`
+	if err := validatePolicyDocument(single); err != nil {
+		t.Errorf("one lowercase spelling: %v", err)
+	}
+	if !CheckPermissionWithContext("s3tables:GetTable", "reader", "owner", single, "", &PolicyContext{ResourceTags: map[string]string{"env": "prod"}}) {
+		t.Error("one lowercase spelling no longer grants")
+	}
+}
+
+// Only the Condition keys are read from the original bytes. Every other field
+// keeps the spelling the re-marshaled decode always picked, so a stored Deny
+// written twice stays a Deny.
+func TestStatementFieldSpellingsDecodeAsBefore(t *testing.T) {
+	stmt := `{"effect":"Deny","Effect":"Allow","Principal":"*","Action":"s3tables:*","Resource":"*"}`
+	for _, policy := range []string{`{"Statement":[` + stmt + `]}`, `{"Statement":` + stmt + `}`} {
+		if CheckPermissionWithContext("s3tables:GetTable", "owner", "owner", policy, "", nil) {
+			t.Errorf("%s: the Deny no longer applies", policy)
+		}
+	}
+}
+
+// A Deny whose operator the evaluator does not know applies instead of being
+// skipped: the writer meant it to protect the resource.
+func TestUnsupportedOperatorDenyApplies(t *testing.T) {
+	deny := `{"Statement":[{"Effect":"Deny","Principal":"*","Action":"s3tables:GetTable","Resource":"*","Condition":{"StringEqualsBogus":{"aws:ResourceTag/env":"prod"}}}]}`
+	ctx := &PolicyContext{ResourceTags: map[string]string{"env": "dev"}}
+	if CheckPermissionWithContext("s3tables:GetTable", "owner", "owner", deny, "", ctx) {
+		t.Error("a Deny with an unsupported operator must apply")
+	}
+}
+
+// One unusable entry makes the whole block unusable whichever entry the map
+// yields first, so a non-matching sibling cannot hide it.
+func TestUnusableConditionIgnoresMapOrder(t *testing.T) {
+	ctx := &PolicyContext{ResourceTags: map[string]string{"env": "prod"}}
+	for _, condition := range []string{
+		`{"StringEquals":{"aws:ResourceTag/env":"dev","aws:ResourceTag/team":null}}`,
+		`{"StringEquals":{"aws:ResourceTag/env":"dev"},"StringNotEquals":{"aws:ResourceTag/team":[null]}}`,
+	} {
+		deny := `{"Statement":[{"Effect":"Deny","Principal":"*","Action":"s3tables:GetTable","Resource":"*","Condition":` + condition + `}]}`
+		for i := 0; i < 32; i++ {
+			if CheckPermissionWithContext("s3tables:GetTable", "owner", "owner", deny, "", ctx) {
+				t.Fatalf("%s: the Deny was skipped on attempt %d", condition, i)
+			}
+		}
+	}
 }

 // An ARN-scoped statement reaches a caller recorded under the ARN's trailing
@@ -500,6 +500,11 @@ func validateVectorPolicy(raw string) error {
 	if err := json.Unmarshal([]byte(raw), &doc); err != nil {
 		return fmt.Errorf("invalid policy document: %w", err)
 	}
+	for key := range doc {
+		if key != "Statement" && strings.EqualFold(key, "Statement") {
+			return fmt.Errorf("invalid policy document: %s must be spelled Statement", key)
+		}
+	}
 	for _, stmt := range policyStatements(doc) {
 		for key := range stmt {
 			for _, field := range []string{"NotAction", "NotResource", "NotPrincipal"} {
@@ -507,22 +512,33 @@ func validateVectorPolicy(raw string) error {
 					return fmt.Errorf("invalid policy document: %s is not supported", field)
 				}
 			}
-			if key != "Condition" && strings.EqualFold(key, "Condition") {
-				return fmt.Errorf("invalid policy document: %s must be spelled Condition", key)
+			for _, field := range []string{"Condition", "Effect"} {
+				if key != field && strings.EqualFold(key, field) {
+					return fmt.Errorf("invalid policy document: %s must be spelled %s", key, field)
+				}
 			}
 		}
+		if condition, ok := stmt["Condition"]; ok && condition == nil && stmt["Effect"] == "Allow" {
+			return fmt.Errorf("invalid policy document: an Allow statement's Condition must not be null")
+		}
 	}
 	for _, stmt := range typed.Statement {
+		if stmt.SplitCondition() {
+			return fmt.Errorf("invalid policy document: a statement's Condition must be under one key")
+		}
 		for operator, keys := range stmt.Condition {
 			if _, err := policy_engine.GetConditionEvaluator(operator); err != nil {
 				return fmt.Errorf("invalid policy document: condition operator %q is not supported", operator)
 			}
+			if len(keys) == 0 {
+				return fmt.Errorf("invalid policy document: condition %s needs at least one key", operator)
+			}
 			for key, value := range keys {
 				if !vectorConditionKey(key) {
 					return fmt.Errorf("invalid policy document: condition key %q is not supported in a vector bucket policy", key)
 				}
-				if !conditionValue(value) {
-					return fmt.Errorf("invalid policy document: condition %s on %q needs a string, number or boolean, or a non-empty list of them", operator, key)
+				if !s3tables.ConditionValue(operator, value) {
+					return fmt.Errorf("invalid policy document: condition %s on %q needs values %s can read (a number for Numeric*, an RFC 3339 time for Date*, true or false for Bool and Null, an IP or CIDR for IpAddress)", operator, key, operator)
 				}
 			}
 		}
@@ -540,23 +556,6 @@ func validateVectorPolicy(raw string) error {
 	return nil
 }

-// conditionValue reports whether the evaluator can compare v: a null value
-// would panic it, and an empty list never matches.
-func conditionValue(v interface{}) bool {
-	list, ok := v.([]interface{})
-	if !ok {
-		list = []interface{}{v}
-	}
-	for _, item := range list {
-		switch item.(type) {
-		case string, float64, bool:
-		default:
-			return false
-		}
-	}
-	return len(list) > 0
-}
-
 // vectorConditionKey reports whether the vector evaluation supplies key.
 func vectorConditionKey(key string) bool {
 	if key == "aws:TagKeys" {
@@ -282,6 +282,9 @@ func TestValidateVectorPolicyConditions(t *testing.T) {
 		`{"StringEquals":{"aws:ResourceTag/env":[null]}}`,
 		`{"StringEquals":{"aws:ResourceTag/env":{"a":"b"}}}`,
 		`{"StringEquals":{"aws:ResourceTag/env":["prod",["dev"]]}}`,
+		`{"NumericLessThan":{"aws:RequestTag/priority":"low"}}`,
+		`{"StringEquals":{}}`,
+		`null`,
 	} {
 		if err := validateVectorPolicy(withCondition(bad)); err == nil {
 			t.Errorf("%s accepted", bad)
@@ -291,6 +294,29 @@ func TestValidateVectorPolicyConditions(t *testing.T) {
 	if err := validateVectorPolicy(lower); err == nil {
 		t.Error("a lower-case condition field was accepted")
 	}
+	for _, misspelled := range []string{
+		`{"Version":"2012-10-17","Statement":[{"effect":"Allow","Principal":"*","Action":"s3vectors:GetIndex","Resource":"*","Condition":null}]}`,
+		`{"Version":"2012-10-17","Statement":[{"Effect":"Deny","effect":"Allow","Principal":"*","Action":"s3vectors:GetIndex","Resource":"*","Condition":null}]}`,
+		`{"Version":"2012-10-17","statement":[{"Effect":"Allow","Principal":"*","Action":"s3vectors:GetIndex","Resource":"*","Condition":null}]}`,
+	} {
+		if err := validateVectorPolicy(misspelled); err == nil {
+			t.Errorf("%s accepted", misspelled)
+		}
+	}
+	for _, repeated := range []string{
+		`{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Principal":"*","Action":"s3vectors:GetIndex","Resource":"*","Condition":null,"Condition":{}}]}`,
+		`{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Principal":"*","Action":"s3vectors:GetIndex","Resource":"*","Condition":{"StringEquals":{"aws:ResourceTag/env":"prod"}},"Condition":{"StringEquals":{"aws:ResourceTag/team":"x"}}}]}`,
+		`{"Version":"2012-10-17","Statement":[{"Effect":"Deny","Principal":"*","Action":"s3vectors:GetIndex","Resource":"*","Condition":{"StringEquals":{"aws:ResourceTag/env":"prod"}},"Condition":{"StringEquals":{"aws:ResourceTag/team":"x"}}}]}`,
+		`{"Version":"2012-10-17","Statement":[{"Effect":"Deny","Principal":"*","Action":"s3vectors:GetIndex","Resource":"*","Condition":null,"Condition":{}}]}`,
+	} {
+		if err := validateVectorPolicy(repeated); err == nil {
+			t.Errorf("%s accepted", repeated)
+		}
+	}
+	denyNull := `{"Version":"2012-10-17","Statement":[{"Effect":"Deny","Principal":"*","Action":"s3vectors:GetIndex","Resource":"*","Condition":null}]}`
+	if err := validateVectorPolicy(denyNull); err != nil {
+		t.Errorf("a Deny with a null Condition was rejected: %v", err)
+	}
 }

 func TestPutVectorBucketPolicyWithTagConditions(t *testing.T) {