feat(query): add privilege check for system.procedures table#19406
feat(query): add privilege check for system.procedures table#19406TCeason wants to merge 3 commits intodatabendlabs:mainfrom
Conversation
Add RBAC visibility check to filter procedures based on user privileges when `enable_experimental_rbac_check` is enabled. - Add `Procedure` variant to `Object` enum in visibility_checker - Handle `Object::Procedure` in `get_visibility_checker` to load procedure ownerships - Filter procedures using `check_procedure_visibility` in procedures_table - Add `procedures` to SYSTEM_TABLES_ALLOW_LIST for query access - Add tests for system.procedures visibility in 18_0017_procedure_rbac.sh
github-actions
bot
added
the
pr-feature
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: de8f2aae17
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
TCeason
force-pushed
the
procedure_tb_priv
branch
from
de8f2aa to
bbef426
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7ec9dd94eb
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| "one", | ||
| "processes", | ||
| "user_functions", | ||
| "procedures", |
There was a problem hiding this comment.
Adding "procedures" to SYSTEM_TABLES_ALLOW_LIST makes SELECT on system.procedures skip table-level privilege checks in validate_table_access (privilege_access.rs), so any user can query this table. Because procedures_table only applies row filtering when enable_experimental_rbac_check is enabled, sessions with that setting off can now enumerate all procedures without AccessProcedure or ownership, which is a new privilege bypass introduced by this commit.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Setting default enable now. And will remove it in next version
1 participant
I hereby agree to the terms of the CLA available at: https://docs.databend.com/dev/policies/cla/
Summary
Add RBAC visibility check to filter procedures based on user privileges
when
enable_experimental_rbac_checkis enabled.Procedurevariant toObjectenum in visibility_checkerObject::Procedureinget_visibility_checkerto load procedure ownershipscheck_procedure_visibilityin procedures_tableproceduresto SYSTEM_TABLES_ALLOW_LIST for query accessTests
Type of change
This change is