diff --git a/CHANGELOG.md b/CHANGELOG.md index 259ff75be40..08c2d426651 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -63,6 +63,7 @@ request adding CHANGELOG notes for breaking (!) changes and possibly other secti ### Deprecations ### Fixes +- Fixed a boundary condition in GCS downscoped credential generation (`GcpCredentialsStorageIntegration`). Locations without a trailing slash could previously grant access to sibling object prefixes via the generated CEL conditions for `resource.name` and list prefixes. Granted paths are now normalized to a directory prefix (with a trailing slash) before the CEL conditions are built, so sibling prefixes can no longer satisfy the `startsWith` checks. - Fixed `NullPointerException` during `dropEntity` when an entity referenced by a grant had been concurrently removed (or purged). `lookupEntities` can return null entries for dropped entities; these are now skipped safely. - `RateLimiterFilter` now returns an Iceberg-compatible `ErrorResponse` JSON body on HTTP 429, with `Content-Type: application/json`. Previously the body was empty, causing Iceberg REST clients to surface an opaque error. - The admin tool `purge` command now prints the underlying exception stack trace to stderr when a purge fails unexpectedly, matching the `bootstrap` command. Previously a failed purge printed only a generic message, giving operators no diagnostic information. diff --git a/polaris-core/src/main/java/org/apache/polaris/core/storage/gcp/GcpCredentialsStorageIntegration.java b/polaris-core/src/main/java/org/apache/polaris/core/storage/gcp/GcpCredentialsStorageIntegration.java index 8ed4a843d9a..fa5dc36dfe6 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/storage/gcp/GcpCredentialsStorageIntegration.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/storage/gcp/GcpCredentialsStorageIntegration.java @@ -18,6 +18,8 @@ */ package org.apache.polaris.core.storage.gcp; +import static org.apache.polaris.core.storage.StorageLocation.ensureTrailingSlash; + import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.json.JsonMapper; @@ -324,7 +326,11 @@ public static CredentialAccessBoundary generateAccessBoundaryRules( location -> { StorageUri uri = StorageUri.parse(location); String bucket = uri.authority(); - String path = uri.rawPath().substring(1); + // Treat the granted path as a directory prefix: a trailing slash is required so + // that the downstream startsWith() CEL conditions cannot be satisfied by sibling + // objects or list prefixes that merely share the granted path as a string prefix + // (e.g. a grant on "data/" must not authorize access to "data_foo/*)". + String path = ensureTrailingSlash(uri.rawPath().substring(1)); readConditionsByBucket .computeIfAbsent(bucket, key -> new LinkedHashSet<>()) .add(resourceNameStartsWithExpression(bucket, path)); @@ -334,7 +340,7 @@ public static CredentialAccessBoundary generateAccessBoundaryRules( location -> { StorageUri uri = StorageUri.parse(location); String bucket = uri.authority(); - String path = uri.rawPath().substring(1); + String path = ensureTrailingSlash(uri.rawPath().substring(1)); readConditionsByBucket .computeIfAbsent(bucket, key -> new LinkedHashSet<>()) .add(objectListPrefixStartsWithExpression(path)); @@ -345,7 +351,7 @@ public static CredentialAccessBoundary generateAccessBoundaryRules( location -> { StorageUri uri = StorageUri.parse(location); String bucket = uri.authority(); - String path = uri.rawPath().substring(1); + String path = ensureTrailingSlash(uri.rawPath().substring(1)); writeConditionsByBucket .computeIfAbsent(bucket, key -> new LinkedHashSet<>()) .add(resourceNameStartsWithExpression(bucket, path)); diff --git a/polaris-core/src/test/java/org/apache/polaris/core/storage/gcp/GcpCredentialsStorageIntegrationTest.java b/polaris-core/src/test/java/org/apache/polaris/core/storage/gcp/GcpCredentialsStorageIntegrationTest.java index cd8f824a0b3..fea1f994193 100644 --- a/polaris-core/src/test/java/org/apache/polaris/core/storage/gcp/GcpCredentialsStorageIntegrationTest.java +++ b/polaris-core/src/test/java/org/apache/polaris/core/storage/gcp/GcpCredentialsStorageIntegrationTest.java @@ -309,12 +309,12 @@ public void testGenerateAccessBoundaryQuotesCelLiteralCharacters() { .isEqualTo( "resource.name.startsWith('projects/_/buckets/bucket1/objects/" + "a\\'b\\\"c\\\\d" - + "') || api.getAttribute('storage.googleapis.com/objectListPrefix', '').startsWith('" + + "/') || api.getAttribute('storage.googleapis.com/objectListPrefix', '').startsWith('" + "a\\'b\\\"c\\\\d" - + "')"); + + "/')"); assertThat(expressionAt(parsedRules, 1)) .isEqualTo( - "resource.name.startsWith('projects/_/buckets/bucket1/objects/a\\'b\\\"c\\\\d')"); + "resource.name.startsWith('projects/_/buckets/bucket1/objects/a\\'b\\\"c\\\\d/')"); } @ParameterizedTest @@ -414,7 +414,7 @@ public void testGenerateAccessBoundaryPreservesLiteralQuestionMarksInPath() { .path("expression") .asText()) .contains("projects/_/buckets/bucket1/objects/path/to/data?with?question") - .contains("startsWith('path/to/data?with?question')"); + .contains("path/to/data?with?question"); } @Test @@ -494,6 +494,40 @@ public AccessToken refreshAccessToken(DownscopedCredentials credentials) { .equals(GcpCredentialsStorageIntegration.IMPERSONATION_SCOPE))); } + @Test + public void testGenerateAccessBoundaryNormalizesTrailingSlashConsistently() { + CredentialAccessBoundary noSlash = + GcpCredentialsStorageIntegration.generateAccessBoundaryRules( + Set.of("gs://bucket1/data"), Set.of("gs://bucket1/data"), Set.of("gs://bucket1/data")); + CredentialAccessBoundary withSlash = + GcpCredentialsStorageIntegration.generateAccessBoundaryRules( + Set.of("gs://bucket1/data/"), + Set.of("gs://bucket1/data/"), + Set.of("gs://bucket1/data/")); + + ObjectMapper mapper = JsonMapper.builder().build(); + assertThat(mapper.convertValue(noSlash, JsonNode.class)) + .isEqualTo(mapper.convertValue(withSlash, JsonNode.class)); + } + + @Test + public void testGenerateAccessBoundaryAppendsTrailingSlashToGuardAgainstSiblingAccess() { + CredentialAccessBoundary credentialAccessBoundary = + GcpCredentialsStorageIntegration.generateAccessBoundaryRules( + Set.of("gs://bucket1/data"), Set.of("gs://bucket1/data"), Set.of("gs://bucket1/data")); + + ObjectMapper mapper = JsonMapper.builder().build(); + JsonNode parsedRules = mapper.convertValue(credentialAccessBoundary, JsonNode.class); + + assertThat(expressionAt(parsedRules, 0)) + .isEqualTo( + "resource.name.startsWith('projects/_/buckets/bucket1/objects/data/')" + + " || api.getAttribute('storage.googleapis.com/objectListPrefix', '')" + + ".startsWith('data/')"); + assertThat(expressionAt(parsedRules, 1)) + .isEqualTo("resource.name.startsWith('projects/_/buckets/bucket1/objects/data/')"); + } + private static String expressionAt(JsonNode parsedRules, int ruleIndex) { return parsedRules .path("accessBoundaryRules") diff --git a/polaris-core/src/test/resources/org/apache/polaris/core/storage/gcp/gcp-testGenerateAccessBoundary.json b/polaris-core/src/test/resources/org/apache/polaris/core/storage/gcp/gcp-testGenerateAccessBoundary.json index 7d206cfff13..8fba5aac801 100644 --- a/polaris-core/src/test/resources/org/apache/polaris/core/storage/gcp/gcp-testGenerateAccessBoundary.json +++ b/polaris-core/src/test/resources/org/apache/polaris/core/storage/gcp/gcp-testGenerateAccessBoundary.json @@ -7,7 +7,7 @@ ], "availableResource": "//storage.googleapis.com/projects/_/buckets/bucket1", "availabilityCondition": { - "expression": "resource.name.startsWith('projects/_/buckets/bucket1/objects/path/to/data') || api.getAttribute('storage.googleapis.com/objectListPrefix', '').startsWith('path/to/data')" + "expression": "api.getAttribute('storage.googleapis.com/objectListPrefix', '').startsWith('path/to/data/') || resource.name.startsWith('projects/_/buckets/bucket1/objects/path/to/data/')" } }, { @@ -16,7 +16,7 @@ ], "availableResource": "//storage.googleapis.com/projects/_/buckets/bucket1", "availabilityCondition": { - "expression": "resource.name.startsWith('projects/_/buckets/bucket1/objects/path/to/data')" + "expression": "resource.name.startsWith('projects/_/buckets/bucket1/objects/path/to/data/')" } } ] diff --git a/polaris-core/src/test/resources/org/apache/polaris/core/storage/gcp/gcp-testGenerateAccessBoundaryWithMultipleBuckets.json b/polaris-core/src/test/resources/org/apache/polaris/core/storage/gcp/gcp-testGenerateAccessBoundaryWithMultipleBuckets.json index bec32428240..cd4260b765e 100644 --- a/polaris-core/src/test/resources/org/apache/polaris/core/storage/gcp/gcp-testGenerateAccessBoundaryWithMultipleBuckets.json +++ b/polaris-core/src/test/resources/org/apache/polaris/core/storage/gcp/gcp-testGenerateAccessBoundaryWithMultipleBuckets.json @@ -7,7 +7,7 @@ ], "availableResource": "//storage.googleapis.com/projects/_/buckets/bucket1", "availabilityCondition": { - "expression": "resource.name.startsWith('projects/_/buckets/bucket1/objects/awesome/path/to/data') || api.getAttribute('storage.googleapis.com/objectListPrefix', '').startsWith('awesome/path/to/data') || resource.name.startsWith('projects/_/buckets/bucket1/objects/normal/path/to/data') || api.getAttribute('storage.googleapis.com/objectListPrefix', '').startsWith('normal/path/to/data')" + "expression": "api.getAttribute('storage.googleapis.com/objectListPrefix', '').startsWith('awesome/path/to/data/') || api.getAttribute('storage.googleapis.com/objectListPrefix', '').startsWith('normal/path/to/data/') || resource.name.startsWith('projects/_/buckets/bucket1/objects/awesome/path/to/data/') || resource.name.startsWith('projects/_/buckets/bucket1/objects/normal/path/to/data/')" } }, { @@ -17,7 +17,7 @@ ], "availableResource": "//storage.googleapis.com/projects/_/buckets/bucket2", "availabilityCondition": { - "expression": "resource.name.startsWith('projects/_/buckets/bucket2/objects/a/super/path/to/data') || api.getAttribute('storage.googleapis.com/objectListPrefix', '').startsWith('a/super/path/to/data')" + "expression": "api.getAttribute('storage.googleapis.com/objectListPrefix', '').startsWith('a/super/path/to/data/') || resource.name.startsWith('projects/_/buckets/bucket2/objects/a/super/path/to/data/')" } }, { @@ -26,7 +26,7 @@ ], "availableResource": "//storage.googleapis.com/projects/_/buckets/bucket1", "availabilityCondition": { - "expression": "resource.name.startsWith('projects/_/buckets/bucket1/objects/normal/path/to/data')" + "expression": "resource.name.startsWith('projects/_/buckets/bucket1/objects/normal/path/to/data/')" } } ] diff --git a/polaris-core/src/test/resources/org/apache/polaris/core/storage/gcp/gcp-testGenerateAccessBoundaryWithoutList.json b/polaris-core/src/test/resources/org/apache/polaris/core/storage/gcp/gcp-testGenerateAccessBoundaryWithoutList.json index 2adedd74426..93d0a8076ee 100644 --- a/polaris-core/src/test/resources/org/apache/polaris/core/storage/gcp/gcp-testGenerateAccessBoundaryWithoutList.json +++ b/polaris-core/src/test/resources/org/apache/polaris/core/storage/gcp/gcp-testGenerateAccessBoundaryWithoutList.json @@ -6,7 +6,7 @@ ], "availableResource": "//storage.googleapis.com/projects/_/buckets/bucket1", "availabilityCondition": { - "expression": "resource.name.startsWith('projects/_/buckets/bucket1/objects/another/path/to/data') || resource.name.startsWith('projects/_/buckets/bucket1/objects/path/to/data')" + "expression": "resource.name.startsWith('projects/_/buckets/bucket1/objects/another/path/to/data/') || resource.name.startsWith('projects/_/buckets/bucket1/objects/path/to/data/')" } }, { @@ -15,7 +15,7 @@ ], "availableResource": "//storage.googleapis.com/projects/_/buckets/bucket1", "availabilityCondition": { - "expression": "resource.name.startsWith('projects/_/buckets/bucket1/objects/path/to/data')" + "expression": "resource.name.startsWith('projects/_/buckets/bucket1/objects/path/to/data/')" } } ] diff --git a/polaris-core/src/test/resources/org/apache/polaris/core/storage/gcp/gcp-testGenerateAccessBoundaryWithoutWrites.json b/polaris-core/src/test/resources/org/apache/polaris/core/storage/gcp/gcp-testGenerateAccessBoundaryWithoutWrites.json index 730c58c7649..84720e41ac8 100644 --- a/polaris-core/src/test/resources/org/apache/polaris/core/storage/gcp/gcp-testGenerateAccessBoundaryWithoutWrites.json +++ b/polaris-core/src/test/resources/org/apache/polaris/core/storage/gcp/gcp-testGenerateAccessBoundaryWithoutWrites.json @@ -6,7 +6,7 @@ ], "availableResource": "//storage.googleapis.com/projects/_/buckets/bucket1", "availabilityCondition": { - "expression": "resource.name.startsWith('projects/_/buckets/bucket1/objects/awesome/path/to/data') || resource.name.startsWith('projects/_/buckets/bucket1/objects/normal/path/to/data')" + "expression": "resource.name.startsWith('projects/_/buckets/bucket1/objects/awesome/path/to/data/') || resource.name.startsWith('projects/_/buckets/bucket1/objects/normal/path/to/data/')" } } ]