rbac: improve permission denied error messages by maheshwarip · Pull Request #35880 · MaterializeInc/materialize

maheshwarip · 2026-04-06T13:54:54Z

Summary

Improves "permission denied" error messages to provide more actionable information to users:

Direct errors (role lacks privileges on an object):

Shows which role needs which privileges
Shows who owns the object
Provides HINT with suggested GRANT command

Indirect errors (MV/view owner lacks privileges on dependency):

Explains the dependency relationship
Shows which object in the lineage is causing the problem
Provides HINT targeting the correct role (the owner who needs access)

Example - Direct Error

SELECT * FROM some_table;

ERROR: permission denied for TABLE "schema.some_table"
DETAIL: The 'user1' role needs SELECT privileges on TABLE "schema.some_table"
The owner of TABLE "schema.some_table" is 'user2'
HINT: The owner ('user2') of TABLE "schema.some_table" or a superuser can grant access: GRANT SELECT ON TABLE "schema.some_table" TO 'user1'

Example - Indirect Error (querying MV where owner lost access to dependency)

SELECT * FROM some_mv;

ERROR: permission denied for TABLE "schema.source_table"
DETAIL: The 'mv_owner' role needs SELECT privileges on TABLE "schema.source_table"
The owner of TABLE "schema.source_table" is 'source_owner'
MATERIALIZED VIEW "schema.my_mv" references TABLE "schema.source_table"
HINT: The owner ('source_owner') of TABLE "schema.source_table" or a superuser can grant access: GRANT SELECT ON TABLE "schema.source_table" TO 'mv_owner'

Motivation

Users encountering permission denied errors often don't know:

Who owns the object they're trying to access
What GRANT command would fix the issue
Why they can't access an MV when the real problem is the MV owner lost access to a dependency

This PR addresses all three by enriching error messages with owner information and actionable HINT suggestions.

Test plan

Run bin/sqllogictest -- test/sqllogictest/permission_denied_errors.slt
Run bin/sqllogictest -- --rewrite-results test/sqllogictest/privilege_checks.slt to update remaining tests for new error format
Verify error messages display correctly in psql client

Enhance permission denied error messages to provide more actionable information to users: 1. Direct errors (role lacks privileges on an object): - Show which role needs which privileges - Show who owns the object - Provide HINT with suggested GRANT command 2. Indirect errors (MV/view owner lacks privileges on dependency): - Explain the dependency relationship - Show which object in the lineage is causing the problem - Provide HINT targeting the correct role (the owner who needs access) Example direct error: ERROR: permission denied for TABLE "schema.my_table" DETAIL: The 'user1' role needs SELECT privileges on TABLE "schema.my_table" The owner of TABLE "schema.my_table" is 'user2' HINT: The owner ('user2') of TABLE "schema.my_table" or a superuser can grant access: GRANT SELECT ON TABLE "schema.my_table" TO 'user1' Example indirect error (querying an MV where owner lost access to dependency): ERROR: permission denied for TABLE "schema.source_table" DETAIL: The 'mv_owner' role needs SELECT privileges on TABLE "schema.source_table" The owner of TABLE "schema.source_table" is 'source_owner' MATERIALIZED VIEW "schema.my_mv" references TABLE "schema.source_table" HINT: The owner ('source_owner') of TABLE "schema.source_table" or a superuser can grant access: GRANT SELECT ON TABLE "schema.source_table" TO 'mv_owner' Related to discussion: https://materializeinc.slack.com/archives/CU7ELJ6E9/p1775481281751919 https://claude.ai/code/session_01FLZQqa7mRprzCTQT6oQ3Ss

github-actions · 2026-04-06T13:55:04Z

Thanks for opening this PR! Here are a few tips to help make the review process smooth for everyone.

PR title guidelines

Use imperative mood: "Fix X" not "Fixed X" or "Fixes X"
Be specific: "Fix panic in catalog sync when controller restarts" not "Fix bug" or "Update catalog code"
Prefix with area if helpful: compute: , storage: , adapter: , sql:

Pre-merge checklist

The PR title is descriptive and will make sense in the git log.
This PR has adequate test coverage / QA involvement has been duly considered. (trigger-ci for additional test/nightly runs)
If this PR includes major user-facing behavior changes, I have pinged the relevant PM to schedule a changelog post.
This PR has an associated up-to-date design doc, is a design doc (template), or is sufficiently small to not require a design.
If this PR evolves an existing $T ⇔ Proto$T mapping (possibly in a backwards-incompatible way), then it is tagged with a T-proto label.
If this PR will require changes to cloud orchestration or tests, there is a companion cloud PR to account for those changes that is tagged with the release-blocker label (example).

maheshwarip · 2026-04-06T15:36:09Z

There's a chance this is slop - I'm not familiar with this part of our repo! At a glance, if this is wrong, feel free to just close the PR

antiguru · 2026-04-07T07:59:37Z

+    match object_id {
+        SystemObjectId::System => None,
+        SystemObjectId::Object(ObjectId::Cluster(id)) => {
+            Some(catalog.get_role(&catalog.get_cluster(*id).owner_id()).name().to_string())


Should probably have a function owner_id on SessionCatalog that returns Option<RoleId>. Similarly, consider making get_object_owner a function on SessionCatalog. The goal is to remove duplication and separate concerns.

…thod Address review feedback by using the existing `get_owner_id` method on `SessionCatalog` instead of duplicating the logic. https://claude.ai/code/session_01FLZQqa7mRprzCTQT6oQ3Ss

def-

Do we still plan to merge this?

def- · 2026-04-18T01:31:03Z

+                    "{} can grant access: GRANT {} ON {} TO '{}'",
+                    granter, privileges, object_description, grant_target


Suggested change

"{} can grant access: GRANT {} ON {} TO '{}'",

granter, privileges, object_description, grant_target

"{} can grant access: GRANT {} ON {} TO {}",

granter, privileges, object_description, Ident::new_unchecked(grant_target)

Done - updated to use Ident::new_unchecked(grant_target) for proper identifier formatting.

Generated by Claude Code

def- · 2026-04-18T01:32:13Z

 db error: ERROR: permission denied for SCHEMA "materialize.public"
 DETAIL: The 'joe' role needs CREATE privileges on SCHEMA "materialize.public"
+The owner of SCHEMA "materialize.public" is 'mz_system'
+HINT: The owner ('mz_system') of SCHEMA "materialize.public" or a superuser can grant access: GRANT CREATE ON SCHEMA "materialize.public" TO 'joe'


Suggested change

HINT: The owner ('mz_system') of SCHEMA "materialize.public" or a superuser can grant access: GRANT CREATE ON SCHEMA "materialize.public" TO 'joe'

HINT: The owner ('mz_system') of SCHEMA "materialize.public" or a superuser can grant access: GRANT CREATE ON SCHEMA "materialize.public" TO joe

'' is wrong, it should be "", or no quoting when not required. Same in other tests

Fixed - updated all test expectations to use proper identifier quoting (no single quotes for simple identifiers).

Generated by Claude Code

Address review feedback: Use `Ident::new_unchecked` for role names in GRANT hint messages instead of single quotes. This produces properly formatted SQL identifiers. https://claude.ai/code/session_01FLZQqa7mRprzCTQT6oQ3Ss

maheshwarip requested a review from a team as a code owner April 6, 2026 13:54

maheshwarip requested a review from SangJunBak April 6, 2026 13:54

antiguru reviewed Apr 7, 2026

View reviewed changes

rbac: simplify get_object_owner_name using existing SessionCatalog me…

35469fd

…thod Address review feedback by using the existing `get_owner_id` method on `SessionCatalog` instead of duplicating the logic. https://claude.ai/code/session_01FLZQqa7mRprzCTQT6oQ3Ss

def- requested changes Apr 18, 2026

View reviewed changes

Use proper identifier quoting in permission denied hints

6e7bf02

Address review feedback: Use `Ident::new_unchecked` for role names in GRANT hint messages instead of single quotes. This produces properly formatted SQL identifiers. https://claude.ai/code/session_01FLZQqa7mRprzCTQT6oQ3Ss

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

rbac: improve permission denied error messages#35880

rbac: improve permission denied error messages#35880
maheshwarip wants to merge 3 commits intomainfrom
claude/slack-session-2VjyP

maheshwarip commented Apr 6, 2026

Uh oh!

github-actions Bot commented Apr 6, 2026

Uh oh!

maheshwarip commented Apr 6, 2026

Uh oh!

antiguru Apr 7, 2026

Uh oh!

def- left a comment

Uh oh!

def- Apr 18, 2026

Uh oh!

maheshwarip Apr 18, 2026

Uh oh!

def- Apr 18, 2026

Uh oh!

maheshwarip Apr 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

		"{} can grant access: GRANT {} ON {} TO '{}'",
		granter, privileges, object_description, grant_target

	HINT: The owner ('mz_system') of SCHEMA "materialize.public" or a superuser can grant access: GRANT CREATE ON SCHEMA "materialize.public" TO 'joe'
	HINT: The owner ('mz_system') of SCHEMA "materialize.public" or a superuser can grant access: GRANT CREATE ON SCHEMA "materialize.public" TO joe

Conversation

maheshwarip commented Apr 6, 2026

Summary

Example - Direct Error

Example - Indirect Error (querying MV where owner lost access to dependency)

Motivation

Test plan

Related

Uh oh!

github-actions Bot commented Apr 6, 2026

PR title guidelines

Pre-merge checklist

Uh oh!

maheshwarip commented Apr 6, 2026

Uh oh!

antiguru Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

def- left a comment

Choose a reason for hiding this comment

Uh oh!

def- Apr 18, 2026

Choose a reason for hiding this comment

Uh oh!

maheshwarip Apr 18, 2026

Choose a reason for hiding this comment

Uh oh!

def- Apr 18, 2026

Choose a reason for hiding this comment

Uh oh!

maheshwarip Apr 18, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants