This is an automated email from the ASF dual-hosted git repository.
roryqi pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/main by this push:
new dd84d33687 Just a minor
dd84d33687 is described below
commit dd84d3368747d3fd08e7f1e02eadd2f6bc4f0a1d
Author: roryqi <[email protected]>
AuthorDate: Mon May 25 11:57:11 2026 +0800
Just a minor
Just a minor
---
.../s3/credential/AwsIrsaCredentialGenerator.java | 20 ++++++-
.../gravitino/s3/credential/S3TokenGenerator.java | 20 ++++++-
.../s3/credential/TestS3PolicyPrefix.java | 65 ++++++++++++++++++++++
3 files changed, 99 insertions(+), 6 deletions(-)
diff --git
a/bundles/aws/src/main/java/org/apache/gravitino/s3/credential/AwsIrsaCredentialGenerator.java
b/bundles/aws/src/main/java/org/apache/gravitino/s3/credential/AwsIrsaCredentialGenerator.java
index dadeb27237..3e83c57e98 100644
---
a/bundles/aws/src/main/java/org/apache/gravitino/s3/credential/AwsIrsaCredentialGenerator.java
+++
b/bundles/aws/src/main/java/org/apache/gravitino/s3/credential/AwsIrsaCredentialGenerator.java
@@ -25,6 +25,7 @@ import java.nio.file.Files;
import java.nio.file.Paths;
import java.util.Arrays;
import java.util.HashMap;
+import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
@@ -212,9 +213,7 @@ public class AwsIrsaCredentialGenerator implements
CredentialGenerator<AwsIrsaCr
.addAction("s3:ListBucket")
.addResource(key))
.addConditions(
- IamConditionOperator.STRING_LIKE,
- "s3:prefix",
- Arrays.asList(rawPath, addWildcardToPath(rawPath)));
+ IamConditionOperator.STRING_LIKE, "s3:prefix",
listPrefixes(rawPath));
bucketGetLocationStatementBuilder.computeIfAbsent(
bucketArn,
@@ -253,6 +252,21 @@ public class AwsIrsaCredentialGenerator implements
CredentialGenerator<AwsIrsaCr
return path.endsWith("/") ? path + "*" : path + "/*";
}
+ /**
+ * Builds the {@code s3:prefix} condition values for a {@code ListBucket}
statement. The prefixes
+ * are restricted to the location and its descendants by requiring a
trailing slash, so a vended
+ * credential cannot enumerate keys in adjacent locations (e.g. {@code
path/to/table_new}) that
+ * merely share the location's string prefix. The empty path (bucket root)
is preserved so listing
+ * the whole bucket still works.
+ *
+ * @param rawPath the object key prefix of the location, without a leading
slash
+ * @return the allowed {@code s3:prefix} values
+ */
+ static List<String> listPrefixes(String rawPath) {
+ String dirPrefix = rawPath.isEmpty() || rawPath.endsWith("/") ? rawPath :
rawPath + "/";
+ return Arrays.asList(dirPrefix, addWildcardToPath(rawPath));
+ }
+
private static String removeSchemaFromS3Uri(URI uri) {
String bucket = uri.getHost();
String path = trimLeadingSlash(uri.getPath());
diff --git
a/bundles/aws/src/main/java/org/apache/gravitino/s3/credential/S3TokenGenerator.java
b/bundles/aws/src/main/java/org/apache/gravitino/s3/credential/S3TokenGenerator.java
index 9158a5bafe..e0ed31f45e 100644
---
a/bundles/aws/src/main/java/org/apache/gravitino/s3/credential/S3TokenGenerator.java
+++
b/bundles/aws/src/main/java/org/apache/gravitino/s3/credential/S3TokenGenerator.java
@@ -23,6 +23,7 @@ import java.io.IOException;
import java.net.URI;
import java.util.Arrays;
import java.util.HashMap;
+import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
@@ -147,9 +148,7 @@ public class S3TokenGenerator implements
CredentialGenerator<S3TokenCredential>
.addAction("s3:ListBucket")
.addResource(key))
.addConditions(
- IamConditionOperator.STRING_LIKE,
- "s3:prefix",
- Arrays.asList(rawPath, addWildcardToPath(rawPath)));
+ IamConditionOperator.STRING_LIKE, "s3:prefix",
listPrefixes(rawPath));
bucketGetLocationStatementBuilder.computeIfAbsent(
bucketArn,
@@ -203,6 +202,21 @@ public class S3TokenGenerator implements
CredentialGenerator<S3TokenCredential>
return path.endsWith("/") ? path + "*" : path + "/*";
}
+ /**
+ * Builds the {@code s3:prefix} condition values for a {@code ListBucket}
statement. The prefixes
+ * are restricted to the location and its descendants by requiring a
trailing slash, so a vended
+ * credential cannot enumerate keys in adjacent locations (e.g. {@code
path/to/table_new}) that
+ * merely share the location's string prefix. The empty path (bucket root)
is preserved so listing
+ * the whole bucket still works.
+ *
+ * @param rawPath the object key prefix of the location, without a leading
slash
+ * @return the allowed {@code s3:prefix} values
+ */
+ static List<String> listPrefixes(String rawPath) {
+ String dirPrefix = rawPath.isEmpty() || rawPath.endsWith("/") ? rawPath :
rawPath + "/";
+ return Arrays.asList(dirPrefix, addWildcardToPath(rawPath));
+ }
+
private static String removeSchemaFromS3Uri(URI uri) {
String bucket = uri.getHost();
String path = trimLeadingSlash(uri.getPath());
diff --git
a/bundles/aws/src/test/java/org/apache/gravitino/s3/credential/TestS3PolicyPrefix.java
b/bundles/aws/src/test/java/org/apache/gravitino/s3/credential/TestS3PolicyPrefix.java
new file mode 100644
index 0000000000..d9fcc068e1
--- /dev/null
+++
b/bundles/aws/src/test/java/org/apache/gravitino/s3/credential/TestS3PolicyPrefix.java
@@ -0,0 +1,65 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.gravitino.s3.credential;
+
+import java.util.List;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+
+/**
+ * Verifies that the {@code s3:prefix} condition for {@code ListBucket}
statements is scoped to the
+ * location and its descendants, so a vended credential cannot enumerate keys
in adjacent locations
+ * sharing the same string prefix (e.g. {@code path/to/table_new} for location
{@code
+ * path/to/table}).
+ */
+public class TestS3PolicyPrefix {
+
+ @Test
+ void testListPrefixesExcludeAdjacentLocations() {
+ // The bare path "path/to/table" must NOT be allowed, otherwise ListBucket
with that prefix
+ // would enumerate keys under the sibling "path/to/table_new".
+ List<String> prefixes = S3TokenGenerator.listPrefixes("path/to/table");
+ Assertions.assertEquals(2, prefixes.size());
+ Assertions.assertTrue(prefixes.contains("path/to/table/"));
+ Assertions.assertTrue(prefixes.contains("path/to/table/*"));
+ Assertions.assertFalse(prefixes.contains("path/to/table"));
+
+ // The IRSA generator shares the same scoping rule.
+ Assertions.assertEquals(prefixes,
AwsIrsaCredentialGenerator.listPrefixes("path/to/table"));
+ }
+
+ @Test
+ void testListPrefixesWithTrailingSlash() {
+ List<String> prefixes = S3TokenGenerator.listPrefixes("path/to/table/");
+ Assertions.assertEquals(2, prefixes.size());
+ Assertions.assertTrue(prefixes.contains("path/to/table/"));
+ Assertions.assertTrue(prefixes.contains("path/to/table/*"));
+ }
+
+ @Test
+ void testListPrefixesForBucketRoot() {
+ // For the bucket root the empty prefix must be preserved so listing the
whole bucket still
+ // works; "/*" alone would not match the empty list prefix.
+ List<String> prefixes = S3TokenGenerator.listPrefixes("");
+ Assertions.assertEquals(2, prefixes.size());
+ Assertions.assertTrue(prefixes.contains(""));
+ Assertions.assertTrue(prefixes.contains("/*"));
+ }
+}
