You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@polaris.apache.org by ad...@apache.org on 2026/04/18 17:22:07 UTC
(polaris) branch main updated: NoSQL: fix phantom grants for dropped/non-existent entities (#4233)
This is an automated email from the ASF dual-hosted git repository.
adutra pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/polaris.git
The following commit(s) were added to refs/heads/main by this push:
new bb72f62e4e NoSQL: fix phantom grants for dropped/non-existent entities (#4233)
bb72f62e4e is described below
commit bb72f62e4e25f2dc52ca87afdb98c2434cd4a8e4
Author: Alexandre Dutra <ad...@apache.org>
AuthorDate: Sat Apr 18 19:22:01 2026 +0200
NoSQL: fix phantom grants for dropped/non-existent entities (#4233)
The `loadGrants()` method never verified the anchor entity exists, always returning `SUCCESS` with a hardcoded version of 1. This violated the `PolarisGrantManager` contract which requires `ENTITY_NOT_FOUND` when the entity is missing, and allowed stale ACLs to surface as valid grants.
Additionally, the bidirectional ACL model produced self-referencing entries when `securable == grantee`, causing an entity to appear in its own grant list. These are now filtered out on read.
---
.../nosql/metastore/NoSqlMetaStore.java | 34 ++++++++++-
.../nosql/metastore/TestNoSqlMetaStoreManager.java | 65 ++++++++++++++++++++++
2 files changed, 96 insertions(+), 3 deletions(-)
diff --git a/persistence/nosql/persistence/metastore/src/main/java/org/apache/polaris/persistence/nosql/metastore/NoSqlMetaStore.java b/persistence/nosql/persistence/metastore/src/main/java/org/apache/polaris/persistence/nosql/metastore/NoSqlMetaStore.java
index 1cf6a02a62..eafb82f05b 100644
--- a/persistence/nosql/persistence/metastore/src/main/java/org/apache/polaris/persistence/nosql/metastore/NoSqlMetaStore.java
+++ b/persistence/nosql/persistence/metastore/src/main/java/org/apache/polaris/persistence/nosql/metastore/NoSqlMetaStore.java
@@ -1137,13 +1137,26 @@ class NoSqlMetaStore extends NonFunctionalBasePersistence {
}
LoadGrantsResult loadGrants(long catalogId, long id, int entityTypeCode, boolean onSecurable) {
+ PolarisEntityType entityType = PolarisEntityType.fromCode(entityTypeCode);
LOGGER.debug(
"loadGrants on {} for catalog:{}, id:{}, entityType:{}({})",
onSecurable ? "securable" : "grantee",
catalogId,
id,
- PolarisEntityType.fromCode(entityTypeCode),
+ entityType,
entityTypeCode);
+
+ var anchorEntity = lookupEntity(catalogId, id, entityTypeCode);
+ if (anchorEntity == null) {
+ LOGGER.trace(
+ "Anchor entity for loadGrants does not exist, returning ENTITY_NOT_FOUND: catalog:{}, id:{}, entityType:{}({})",
+ catalogId,
+ id,
+ entityType,
+ entityTypeCode);
+ return new LoadGrantsResult(ENTITY_NOT_FOUND, null);
+ }
+
var aclName = new GrantTriplet(true, catalogId, id, entityTypeCode).toRoleName();
var collector = new GrantRecordsCollector();
@@ -1165,6 +1178,20 @@ class NoSqlMetaStore extends NonFunctionalBasePersistence {
? securableAndGrantee.granteeTypeCode()
: securableAndGrantee.securableTypeCode();
+ // Skip self-referencing entries where the target is the anchor entity itself.
+ // The bidirectional ACL model produces these when securable == grantee.
+ if (targetId == id && targetTypeCode == entityTypeCode) {
+ if (LOGGER.isTraceEnabled()) {
+ LOGGER.trace(
+ " Skipping self-referencing grant entry for catalog:{}, id:{}, entityType:{}({})",
+ targetCatalogId,
+ targetId,
+ PolarisEntityType.fromCode(targetTypeCode),
+ targetTypeCode);
+ }
+ return;
+ }
+
var indexedAccess = memoizedIndexedAccess.indexedAccess(targetCatalogId, targetTypeCode);
var entityOptional =
indexedAccess.byId(targetId).map(o -> mapToEntity(o, targetCatalogId));
@@ -1190,10 +1217,11 @@ class NoSqlMetaStore extends NonFunctionalBasePersistence {
collector.grantRecords.size(),
catalogId,
id,
- PolarisEntityType.fromCode(entityTypeCode),
+ entityType,
entityTypeCode);
- return new LoadGrantsResult(1, collector.grantRecords, entities);
+ return new LoadGrantsResult(
+ anchorEntity.getGrantRecordsVersion(), collector.grantRecords, entities);
}
private List<PolarisGrantRecord> collectGrantRecords(
diff --git a/persistence/nosql/persistence/metastore/src/test/java/org/apache/polaris/persistence/nosql/metastore/TestNoSqlMetaStoreManager.java b/persistence/nosql/persistence/metastore/src/test/java/org/apache/polaris/persistence/nosql/metastore/TestNoSqlMetaStoreManager.java
index 60d26ec143..c251ab3af8 100644
--- a/persistence/nosql/persistence/metastore/src/test/java/org/apache/polaris/persistence/nosql/metastore/TestNoSqlMetaStoreManager.java
+++ b/persistence/nosql/persistence/metastore/src/test/java/org/apache/polaris/persistence/nosql/metastore/TestNoSqlMetaStoreManager.java
@@ -21,6 +21,7 @@ package org.apache.polaris.persistence.nosql.metastore;
import static org.apache.polaris.core.entity.PolarisEntityConstants.ENTITY_BASE_LOCATION;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatCode;
+import static org.assertj.core.api.Assertions.tuple;
import static org.assertj.core.api.InstanceOfAssertFactories.BOOLEAN;
import io.smallrye.common.annotation.Identifier;
@@ -39,6 +40,7 @@ import org.apache.polaris.core.entity.PolarisEntityConstants;
import org.apache.polaris.core.entity.PolarisEntityCore;
import org.apache.polaris.core.entity.PolarisEntitySubType;
import org.apache.polaris.core.entity.PolarisEntityType;
+import org.apache.polaris.core.entity.PolarisPrivilege;
import org.apache.polaris.core.persistence.BasePolarisMetaStoreManagerTest;
import org.apache.polaris.core.persistence.MetaStoreManagerFactory;
import org.apache.polaris.core.persistence.PolarisMetaStoreManager;
@@ -385,6 +387,69 @@ public class TestNoSqlMetaStoreManager extends BasePolarisMetaStoreManagerTest {
grantRecord -> assertThat(grantRecord.getGranteeId()).isEqualTo(principalRoleId));
}
+ @Test
+ public void testLoadGrantsReturnsEntityNotFoundForMissingAnchor() {
+ long missingId = metaStore.generateNewEntityId(callContext).getId();
+ PolarisBaseEntity missingPrincipalRole =
+ new PolarisBaseEntity(
+ PolarisEntityConstants.getNullId(),
+ missingId,
+ PolarisEntityType.PRINCIPAL_ROLE,
+ PolarisEntitySubType.NULL_SUBTYPE,
+ PolarisEntityConstants.getRootEntityId(),
+ "missingPrincipalRole");
+
+ LoadGrantsResult grantsToMissing =
+ metaStore.loadGrantsToGrantee(callContext, missingPrincipalRole);
+ assertThat(grantsToMissing.getReturnStatus())
+ .isEqualTo(BaseResult.ReturnStatus.ENTITY_NOT_FOUND);
+
+ LoadGrantsResult grantsOnMissing =
+ metaStore.loadGrantsOnSecurable(callContext, missingPrincipalRole);
+ assertThat(grantsOnMissing.getReturnStatus())
+ .isEqualTo(BaseResult.ReturnStatus.ENTITY_NOT_FOUND);
+ }
+
+ @Test
+ public void testLoadGrantsSkipsSelfReferencingEntries() {
+ PolarisBaseEntity principalRole =
+ new PolarisBaseEntity(
+ PolarisEntityConstants.getNullId(),
+ metaStore.generateNewEntityId(callContext).getId(),
+ PolarisEntityType.PRINCIPAL_ROLE,
+ PolarisEntitySubType.NULL_SUBTYPE,
+ PolarisEntityConstants.getRootEntityId(),
+ "selfReferencingPrincipalRole");
+
+ EntityResult createResult = metaStore.createEntityIfNotExists(callContext, null, principalRole);
+ assertThat(createResult.isSuccess()).isTrue();
+ PolarisBaseEntity createdPrincipalRole = createResult.getEntity();
+ assertThat(createdPrincipalRole).isNotNull();
+
+ assertThat(
+ metaStore
+ .grantPrivilegeOnSecurableToRole(
+ callContext,
+ createdPrincipalRole,
+ null,
+ createdPrincipalRole,
+ PolarisPrivilege.PRINCIPAL_ROLE_USAGE)
+ .isSuccess())
+ .isTrue();
+
+ assertThat(
+ List.of(
+ metaStore.loadGrantsOnSecurable(callContext, createdPrincipalRole),
+ metaStore.loadGrantsToGrantee(callContext, createdPrincipalRole)))
+ .extracting(
+ LoadGrantsResult::isSuccess,
+ LoadGrantsResult::getGrantRecords,
+ LoadGrantsResult::getEntities)
+ .containsExactly( //
+ tuple(true, List.of(), List.of()), //
+ tuple(true, List.of(), List.of()));
+ }
+
@Override
@Test
@Disabled(