Strengthening assertions at grant time seems like a good approach to me. As Alex notes, most of the incorrect grants become noise and are ineffectual at authz.
Backward compatibility is something to consider, though I think that the fact that we might allow certain grants today that have no impact on actual authz gives us more room to just fix things. Mike On Thu, Apr 23, 2026 at 10:28 AM Yufei Gu <[email protected]> wrote: > Hi Alex, thanks for raising this, I agree the issues are valid and worth > addressing. > > +1 on enforcing semantic correctness at grant time. Without validation, > it’s easy to end up with inconsistent or meaningless grant records. With > proper validation, fields like securableType, securableSubTypes, and > granteeType would no longer be “dead” metadata, but instead help define and > enforce the semantics of privilege grants. That said, we should be > thoughtful about backward compatibility. It would be great to get more > input from the community. > > Yufei > > > On Mon, Apr 20, 2026 at 10:18 AM Dmitri Bourlatchkov <[email protected]> > wrote: > > > Hi Alex, > > > > Thanks for your diligent review! I think your points are quite valid and > > certainly worth fixing. > > > > Regarding the practical code changes, I'm personally a bit hesitant to > just > > start altering that code right away. > > > > I'd appreciate getting feedback on this from Dennis and Michael. > > > > Cheers, > > Dmitri. > > > > On Sun, Apr 19, 2026 at 2:37 PM Alexandre Dutra <[email protected]> > wrote: > > > > > Hi all, > > > > > > I found a few issues with PolarisPrivilege that I'd like to raise for > > > discussion. > > > > > > 1. Unused or incorrect metadata > > > > > > PolarisPrivilege has 3 unused fields: securableType, > > > securableSubTypes, and granteeType. They are effectively dead metadata > > > today. > > > > > > Moreover, the two-argument constructor PolarisPrivilege(code, > > > securableType) defaults granteeType to CATALOG_ROLE. This is wrong for > > > ROOT-level privileges: SERVICE_MANAGE_ACCESS, CATALOG_CREATE, > > > CATALOG_LIST, PRINCIPAL_CREATE, PRINCIPAL_LIST, PRINCIPAL_ROLE_CREATE, > > > and PRINCIPAL_ROLE_LIST all have securableType=ROOT but are silently > > > marked as granteeType=CATALOG_ROLE. > > > > > > 2. No validation at grant time > > > > > > Nothing in the metastore API or its implementations validates that a > > > grant is semantically meaningful. The only check today is > > > grantee.getType().isGrantee() in some impls. > > > > > > This means, for example: > > > > > > - You can grant TABLE_READ_DATA on a PRINCIPAL entity (mismatched > > > securable type). > > > - You can grant VIEW_DROP on an ICEBERG_TABLE entity (mismatched > > > securable sub-type). > > > - You can grant any privilege to a PRINCIPAL directly (grantee type > > > violation). > > > - You can grant a privilege on a PRINCIPAL_ROLE to itself (self-grant). > > > - You can grant a privilege on a securable in catalog A to a > > > CATALOG_ROLE in catalog B (cross-catalog grants). > > > > > > Such grant records become pure noise once written. Some impls filter > > > them out, some others don't. > > > > > > So, here are a few questions to the community: > > > > > > 1. Should we clean up / fix PolarisPrivilege? > > > 2. Should we enforce semantic meaningfulness when writing grants? And > > how? > > > > > > Thanks, > > > Alex > > > > > >
