policy: keep implied S3 grants of a tables statement on table data

policy/actionset.go

@@ -111,6 +111,17 @@ func (actionSet ActionSet) Match(action Action) bool {
 	return false
 }
 
+// matchesNamed reports whether action matches an action the set names, without
+// the actions those imply.
+func (actionSet ActionSet) matchesNamed(action Action) bool {
+	for r := range actionSet {
+		if r.Match(action) {
+			return true
+		}
+	}
+	return false
+}
+
 // Equals - checks whether given action set is equal to current action set or not.
 func (actionSet ActionSet) Equals(sactionSet ActionSet) bool {
 	// If length of set is not equal to length of given set, the

policy/policy_test.go

@@ -2286,15 +2286,15 @@ func TestS3TablesActionsWithImplicitMatching(t *testing.T) {
 			description:    "S3 Tables action should match S3 Tables resource directly",
 		},
 		{
-			name:       "GetTableData implicit GetObject match with resource conversion",
+			name:       "GetTableData implicit GetObject match on the table resource",
 			policyJSON: policy1JSON,
 			args: Args{
 				Action:     GetObjectAction,
-				BucketName: "my-warehouse",
-				ObjectName: "table-uuid-123",
+				BucketName: "bucket",
+				ObjectName: "my-warehouse/table/table-uuid-123",
 			},
 			expectedResult: true,
-			description:    "GetObject (implicit from GetTableData) should match when resource is converted from S3 to S3Tables format",
+			description:    "GetObject (implicit from GetTableData) should match on the table resource the server presents",
 		},
 		{
 			name:       "GetTableData implicit GetObject with extra path",
@@ -2304,19 +2304,19 @@ func TestS3TablesActionsWithImplicitMatching(t *testing.T) {
 				BucketName: "my-warehouse",
 				ObjectName: "table-uuid-123/data/file.parquet",
 			},
-			expectedResult: true,
-			description:    "GetObject should match even with extra path segments (should be discarded in conversion)",
+			expectedResult: false,
+			description:    "a plain object path is not table data until the server presents it in tables form",
 		},
 		{
 			name:       "GetTableData implicit ListMultipartUploadParts match",
 			policyJSON: policy1JSON,
 			args: Args{
 				Action:     ListMultipartUploadPartsAction,
-				BucketName: "my-warehouse",
-				ObjectName: "table-uuid-123",
+				BucketName: "bucket",
+				ObjectName: "my-warehouse/table/table-uuid-123",
 			},
 			expectedResult: true,
-			description:    "ListMultipartUploadParts (implicit from GetTableData) should match with resource conversion",
+			description:    "ListMultipartUploadParts (implicit from GetTableData) should match on the table resource",
 		},
 		{
 			name:       "GetTableData wrong warehouse - should not match",
@@ -2355,19 +2355,19 @@ func TestS3TablesActionsWithImplicitMatching(t *testing.T) {
 			policyJSON: policy2JSON,
 			args: Args{
 				Action:     PutObjectAction,
-				BucketName: "test-warehouse",
-				ObjectName: "uuid-456",
+				BucketName: "bucket",
+				ObjectName: "test-warehouse/table/uuid-456",
 			},
 			expectedResult: true,
-			description:    "PutObject (implicit from PutTableData) should match with resource conversion",
+			description:    "PutObject (implicit from PutTableData) should match on the table resource",
 		},
 		{
 			name:       "PutTableData implicit AbortMultipartUpload match",
 			policyJSON: policy2JSON,
 			args: Args{
 				Action:     AbortMultipartUploadAction,
-				BucketName: "test-warehouse",
-				ObjectName: "uuid-456/upload",
+				BucketName: "bucket",
+				ObjectName: "test-warehouse/table/uuid-456",
 			},
 			expectedResult: true,
 			description:    "AbortMultipartUpload (implicit from PutTableData) should match",
@@ -2377,8 +2377,8 @@ func TestS3TablesActionsWithImplicitMatching(t *testing.T) {
 			policyJSON: policy3JSON,
 			args: Args{
 				Action:     GetObjectAction,
-				BucketName: "wh",
-				ObjectName: "id",
+				BucketName: "bucket",
+				ObjectName: "wh/table/id",
 			},
 			expectedResult: true,
 			description:    "Should match with multiple S3 Tables actions in statement",
@@ -2388,8 +2388,8 @@ func TestS3TablesActionsWithImplicitMatching(t *testing.T) {
 			policyJSON: policy3JSON,
 			args: Args{
 				Action:     PutObjectAction,
-				BucketName: "wh",
-				ObjectName: "id",
+				BucketName: "bucket",
+				ObjectName: "wh/table/id",
 			},
 			expectedResult: true,
 			description:    "Should match PutObject when both GetTableData and PutTableData are allowed",
@@ -2409,11 +2409,11 @@ func TestS3TablesActionsWithImplicitMatching(t *testing.T) {
 			policyJSON: policy5JSON,
 			args: Args{
 				Action:     DeleteObjectAction,
-				BucketName: "del-warehouse",
-				ObjectName: "uuid-789",
+				BucketName: "bucket",
+				ObjectName: "del-warehouse/table/uuid-789",
 			},
 			expectedResult: true,
-			description:    "DeleteObject (implicit from DeleteTableData) should match with resource conversion",
+			description:    "DeleteObject (implicit from DeleteTableData) should match on the table resource",
 		},
 		{
 			name:       "DeleteTableData implicit DeleteObject with extra path",
@@ -2423,8 +2423,8 @@ func TestS3TablesActionsWithImplicitMatching(t *testing.T) {
 				BucketName: "del-warehouse",
 				ObjectName: "uuid-789/data/file.parquet",
 			},
-			expectedResult: true,
-			description:    "DeleteObject should match objects under the table prefix",
+			expectedResult: false,
+			description:    "a plain object path is not table data until the server presents it in tables form",
 		},
 		{
 			name:       "DeleteTableData wrong table uuid - should not match",
@@ -2464,19 +2464,19 @@ func TestS3TablesActionsWithImplicitMatching(t *testing.T) {
 			policyJSON: policy4JSON,
 			args: Args{
 				Action:     GetObjectAction,
-				BucketName: "all-warehouse",
-				ObjectName: "all-uuid",
+				BucketName: "bucket",
+				ObjectName: "all-warehouse/table/all-uuid",
 			},
 			expectedResult: true,
-			description:    "s3tables:* should allow GetObject through implicit matching with resource conversion",
+			description:    "s3tables:* should allow GetObject through implicit matching on the table resource",
 		},
 		{
 			name:       "s3tables:* allows PutObject implicitly",
 			policyJSON: policy4JSON,
 			args: Args{
 				Action:     PutObjectAction,
-				BucketName: "all-warehouse",
-				ObjectName: "all-uuid",
+				BucketName: "bucket",
+				ObjectName: "all-warehouse/table/all-uuid",
 			},
 			expectedResult: true,
 			description:    "s3tables:* should allow PutObject through implicit matching",
@@ -2486,8 +2486,8 @@ func TestS3TablesActionsWithImplicitMatching(t *testing.T) {
 			policyJSON: policy4JSON,
 			args: Args{
 				Action:     ListMultipartUploadPartsAction,
-				BucketName: "all-warehouse",
-				ObjectName: "all-uuid",
+				BucketName: "bucket",
+				ObjectName: "all-warehouse/table/all-uuid",
 			},
 			expectedResult: true,
 			description:    "s3tables:* should allow ListMultipartUploadParts through implicit matching",
@@ -2497,8 +2497,8 @@ func TestS3TablesActionsWithImplicitMatching(t *testing.T) {
 			policyJSON: policy4JSON,
 			args: Args{
 				Action:     AbortMultipartUploadAction,
-				BucketName: "all-warehouse",
-				ObjectName: "all-uuid",
+				BucketName: "bucket",
+				ObjectName: "all-warehouse/table/all-uuid",
 			},
 			expectedResult: true,
 			description:    "s3tables:* should allow AbortMultipartUpload through implicit matching",
@@ -2508,8 +2508,8 @@ func TestS3TablesActionsWithImplicitMatching(t *testing.T) {
 			policyJSON: policy4JSON,
 			args: Args{
 				Action:     DeleteObjectAction,
-				BucketName: "all-warehouse",
-				ObjectName: "all-uuid",
+				BucketName: "bucket",
+				ObjectName: "all-warehouse/table/all-uuid",
 			},
 			expectedResult: true,
 			description:    "s3tables:* should allow DeleteObject through implicit matching",
@@ -2520,10 +2520,10 @@ func TestS3TablesActionsWithImplicitMatching(t *testing.T) {
 			args: Args{
 				Action:     GetObjectAction,
 				BucketName: "all-warehouse",
-				ObjectName: "all-uuid/extra/path/data.parquet",
+				ObjectName: "all-uuid/data/file.parquet",
 			},
-			expectedResult: true,
-			description:    "s3tables:* should match with extra path segments discarded",
+			expectedResult: false,
+			description:    "a plain object path is not table data until the server presents it in tables form",
 		},
 		{
 			name:       "s3tables:* wrong warehouse should not match",

policy/statement.go

@@ -90,25 +90,17 @@ func (statement Statement) isAllowedFor(args *Args, resource string) bool {
 
 		class := statement.classify()
 
-		if class.has(classTable) && !TableAction(args.Action).IsValid() {
-			// When a tables policy statement (for example
-			//   "Action":   ["s3tables:GetTableData"],
-			//   "Resource": ["arn:aws:s3tables:::bucket/wh/table/uuid"]
-			// ) is evaluated for a plain S3 data-path action such as
-			// GetObject on (BucketName "wh", ObjectName "uuid[/...]"), the
-			// action match succeeds via implicitActions. However, the
-			// resource string built from Args ("wh/uuid[/...]") does not
-			// look like a tables ARN suffix ("bucket/wh/table/uuid"), so a
-			// direct string match against the S3 Tables resource
-			// would fail. In this specific case we know:
-			//   - the statement is a tables statement,
-			//   - the incoming action is covered implicitly (not a table API),
-			//   - and the stored policy resource is S3 Tables style.
-			// To allow GetObject/ListMultipartUploadParts/etc. when
-			// s3tables:GetTableData (or similar) is granted, normalize the
-			// S3 data-path resource into the canonical tables form before
-			// running the usual resource match.
-			if !isTableResourceString(resource) {
+		if class.has(classTable) && !TableAction(args.Action).IsValid() && !isTableResourceString(resource) {
+			// A tables action implies the S3 data actions Iceberg uses on table
+			// files (s3tables:GetTableData implies s3:GetObject). The server
+			// presents a warehouse object in tables form, so an implied Allow
+			// never reaches an ordinary bucket, while an S3 action the statement
+			// names itself matches the request as it is. A Deny matches the
+			// object's tables form, so denying a table still denies its files.
+			switch {
+			case statement.Effect == Allow && !statement.Actions.matchesNamed(args.Action):
+				return false
+			case statement.Effect == Deny:
 				if args.BucketName == "" || args.ObjectName == "" {
 					return false
 				}

policy/table_implied_scope_test.go

@@ -0,0 +1,112 @@
+// Copyright (c) 2015-2026 MinIO, Inc.
+//
+// This file is part of MinIO Object Storage stack
+//
+// This program is free software: you can redistribute it and/or modify
+// it under the terms of the GNU Affero General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// This program is distributed in the hope that it will be useful
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU Affero General Public License for more details.
+//
+// You should have received a copy of the GNU Affero General Public License
+// along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+package policy
+
+import (
+	"strings"
+	"testing"
+
+	"github.com/minio/pkg/v3/policy/condition"
+)
+
+// TestS3TablesImpliedActionsReachTableDataOnly verifies the S3 data actions a
+// tables grant implies reach only the table resource the server presents for a
+// warehouse object, never an object in an ordinary bucket, while a tables Deny
+// still refuses the files of the tables it names.
+func TestS3TablesImpliedActionsReachTableDataOnly(t *testing.T) {
+	doc := func(statements ...string) string {
+		return `{"Version":"2012-10-17","Statement":[` + strings.Join(statements, ",") + `]}`
+	}
+	plain := func(action Action, bucket, object string) Args {
+		return Args{Action: action, BucketName: bucket, ObjectName: object}
+	}
+	table := func(action Action, warehouse, id string) Args {
+		return Args{Action: action, BucketName: "bucket", ObjectName: warehouse + "/table/" + id}
+	}
+
+	everyWarehouse := doc(`{"Effect":"Allow","Action":["s3tables:*"],"Resource":["arn:aws:s3tables:::bucket/*"]}`)
+	wildcard := doc(`{"Effect":"Allow","Action":["s3tables:GetTableData","s3tables:PutTableData"],"Resource":["arn:aws:s3tables:::*"]}`)
+	oneWarehouse := doc(`{"Effect":"Allow","Action":["s3tables:GetTableData"],"Resource":["arn:aws:s3tables:::bucket/wh/table/*"]}`)
+	legacyMixedDeny := Policy{Version: DefaultVersion, Statements: []Statement{
+		NewStatement("", Allow, NewActionSet(GetObjectAction), NewResourceSet(NewResource("wh/*")), condition.NewFunctions()),
+		NewStatement("", Deny, NewActionSet(GetObjectAction, Action(S3TablesGetTableDataAction)),
+			NewResourceSet(NewS3TablesResource("bucket/wh/table/*")), condition.NewFunctions()),
+	}}
+	deniedTable := doc(
+		`{"Effect":"Allow","Action":["s3:GetObject"],"Resource":["arn:aws:s3:::wh/*"]}`,
+		`{"Effect":"Deny","Action":["s3tables:GetTableData"],"Resource":["arn:aws:s3tables:::bucket/wh/*"]}`,
+	)
+
+	for _, tc := range []struct {
+		name   string
+		policy string
+		args   Args
+		want   bool
+	}{
+		{"every warehouse: GetObject in an ordinary bucket", everyWarehouse, plain(GetObjectAction, "data", "x/y"), false},
+		{"every warehouse: PutObject in an ordinary bucket", everyWarehouse, plain(PutObjectAction, "data", "x/y"), false},
+		{"every warehouse: DeleteObject in an ordinary bucket", everyWarehouse, plain(DeleteObjectAction, "data", "x/y"), false},
+		{"every warehouse: multipart in an ordinary bucket", everyWarehouse, plain(AbortMultipartUploadAction, "data", "x/y"), false},
+		{"every warehouse: table data", everyWarehouse, table(PutObjectAction, "wh", "abc"), true},
+		{"wildcard resource: GetObject in an ordinary bucket", wildcard, plain(GetObjectAction, "data", "x/y"), false},
+		{"wildcard resource: PutObject in an ordinary bucket", wildcard, plain(PutObjectAction, "data", "x/y"), false},
+		{"wildcard resource: table data", wildcard, table(GetObjectAction, "wh", "abc"), true},
+		{"one warehouse: an object presented without its table", oneWarehouse, plain(GetObjectAction, "wh", "abc/data/f.parquet"), false},
+		{"one warehouse: table data", oneWarehouse, table(GetObjectAction, "wh", "abc"), true},
+		{"one warehouse: another warehouse's table", oneWarehouse, table(GetObjectAction, "other", "abc"), false},
+		{"denied table: its file is refused over plain S3", deniedTable, plain(GetObjectAction, "wh", "abc/data/f.parquet"), false},
+		{"denied table: its file is refused in tables form", deniedTable, table(GetObjectAction, "wh", "abc"), false},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			p, err := ParseConfig(strings.NewReader(tc.policy))
+			if err != nil {
+				t.Fatalf("parse: %v", err)
+			}
+			if got := p.IsAllowed(tc.args); got != tc.want {
+				t.Fatalf("IsAllowed(%+v) = %v, want %v", tc.args, got, tc.want)
+			}
+		})
+	}
+
+	// A Deny stored before mixing action types was refused, naming both an S3
+	// and a tables action, still denies the table's files over plain S3.
+	t.Run("a legacy mixed Deny still denies the table's files", func(t *testing.T) {
+		if legacyMixedDeny.IsAllowed(plain(GetObjectAction, "wh", "abc/data/f.parquet")) {
+			t.Fatal("the mixed Deny must refuse GetObject on a file of the table it names")
+		}
+		allowOnly := Policy{Version: DefaultVersion, Statements: legacyMixedDeny.Statements[:1]}
+		if !allowOnly.IsAllowed(plain(GetObjectAction, "wh", "abc/data/f.parquet")) {
+			t.Fatal("control: without the Deny, the S3 Allow reaches that file")
+		}
+	})
+
+	// A statement stored before mixing action types was refused keeps the S3
+	// actions it names; only the ones it implies are confined to table data.
+	t.Run("a legacy mixed statement keeps its named S3 actions", func(t *testing.T) {
+		st := NewStatement("", Allow,
+			NewActionSet(GetObjectAction, Action(S3TablesGetTableDataAction)),
+			NewResourceSet(NewResource("data/*")),
+			condition.NewFunctions())
+		if !st.IsAllowed(plain(GetObjectAction, "data", "x")) {
+			t.Fatal("a named s3:GetObject must still match its bucket")
+		}
+		if st.IsAllowed(plain(ListMultipartUploadPartsAction, "data", "x")) {
+			t.Fatal("an action only GetTableData implies must not reach an ordinary bucket")
+		}
+	})
+}