feat: Support GRANT/REVOKE on views with security semantics and tests

[gouhongshen]

User description

What type of PR is this?

• API-change
• BUG
• Improvement
• Documentation
• Feature
• Test and CI
• Code Refactoring

Which issue(s) this PR fixes:

issue ##23232

What this PR does / why we need it:

• Add view as a first-class privilege object and wire it through auth checks and show grants.
• Record view lineage in the plan and enforce view privileges with DEFINER/INVOKER semantics.
• Fix privilege extraction for INSERT/UPDATE/DELETE paths and keep system view behavior consistent.
• Update BVT cases and expected results for view grants and related errors.
• Update RFC to keep only the final design and implementation summary.

---

PR Type

Enhancement, Tests

---

Description

• Add VIEW as first-class privilege object with GRANT/REVOKE support

• Implement view lineage tracking in plan nodes for authorization checks

• Enforce SQL SECURITY DEFINER/INVOKER semantics for view access

• Fix privilege extraction for INSERT/UPDATE/DELETE statement paths

• Add comprehensive BVT test cases for view grant/revoke operations

---

Diagram Walkthrough

flowchart LR
  A["View Privilege Request"] --> B["Extract View Lineage<br/>from Plan Nodes"]
  B --> C["Check View Privilege"]
  C --> D{SQL SECURITY<br/>Type?}
  D -->|DEFINER| E["Verify Definer<br/>Base Table Privileges"]
  D -->|INVOKER| F["Verify Invoker<br/>Base Table Privileges"]
  E --> G["Grant/Revoke<br/>Execution"]
  F --> G
  H["System Views<br/>information_schema/mysql"] --> I["Skip View Checks<br/>Read-Only"]

Loading

File Walkthrough

	Relevant files
Enhancement	10 files authenticate.go Add view privilege object type and enforcement logic          +566/-59 authenticate2.go Implement view privilege verification with security semantics +7/-1      build_ddl.go Record view security type in plan metadata                              +18/-0    query_builder.go Track view lineage in plan nodes during binding                    +19/-0    types.go Add view lineage fields to plan structures                              +5/-0      bind_context.go Propagate view chain context through bind contexts              +4/-0      deepcopy.go Deep copy view lineage fields in plan nodes                            +2/-0      variables.go Add view_security_type session variable                                    +8/-0      revoke.go Add VIEW to object type string representation                        +2/-0      plan.proto Add origin_views and direct_view fields to plan nodes        +2/-0
Tests	12 files authenticate_test.go Add unit tests for GRANT/REVOKE on views                                  +308/-4  build_ddl_test.go Update test fixtures with security type field                        +6/-4      query_builder_test.go Update test fixtures with security type field                        +3/-2      mock.go Add security type to mock view definitions                              +1/-0      grant_view.sql Add BVT tests for basic view grant/revoke operations          +120/-0  grant_view.result Add expected results for view grant BVT tests                        +70/-0    grant_view_non_sys.sql Add BVT tests for view grants on non-system tenants            +48/-0    grant_view_non_sys.result Add expected results for non-system tenant view grant tests +24/-0    grant_view_complex.sql Add complex BVT tests for multi-level role inheritance      +84/-0    grant_view_complex.result Add expected results for complex view grant scenarios        +54/-0    grant_privs_role.result Update error messages to include view object type                +3/-3      revoke_privs_role.result Update error messages to include view object type                +2/-2
Formatting	1 files task.pb.go Fix protobuf comment formatting                                                    +4/-3
Documentation	1 files view_grant_support_design.md Document final design and implementation of view grants    +79/-0
Additional files	1 files plan.pb.go +937/-818

---

[qodo-code-review]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
⚪	SQL injection risk Description: SQL statements are built via fmt.Sprintf(...) using identifiers (dbName, viewName) (e.g., checkDatabaseViewFormat, getViewMetaFormat), which could allow SQL injection if inputNameIsInvalid(...) does not strictly and correctly constrain allowed characters for identifiers. authenticate.go [1324-2086] Referred Code checkDatabaseTableFormat = `select rel_logical_id from mo_catalog.mo_tables where relname = "%s" and reldatabase = "%s" and account_id = %d;` checkDatabaseViewFormat = `select rel_logical_id from mo_catalog.mo_tables where relname = "%s" and reldatabase = "%s" and relkind = "v" and account_id = %d;` getViewMetaFormat = `select viewdef, owner from mo_catalog.mo_tables where relname = "%s" and reldatabase = "%s" and relkind = "v" and account_id = %d;` //TODO:fix privilege_level string and obj_type string //For object_type : table, privilege_level : *.* checkWithGrantOptionForTableStarStar = `select rp.privilege_id,rp.with_grant_option from mo_catalog.mo_database d, mo_catalog.mo_tables t, mo_catalog.mo_role_privs rp where d.dat_id = t.reldatabase_id and rp.obj_id = 0 and rp.obj_type = "%s" and rp.role_id = %d and rp.privilege_id = %d and rp.privilege_level = "%s" and rp.with_grant_option = true;` //For object_type : table, privilege_level : db.* checkWithGrantOptionForTableDatabaseStar = `select rp.privilege_id,rp.with_grant_option from mo_catalog.mo_database d, mo_catalog.mo_tables t, mo_catalog.mo_role_privs rp ... (clipped 742 lines)
Ticket Compliance
🟡	🎫 #23232 🟢 Support GRANT ... ON VIEW (views should be recognized as a supported privilege object type, not "Unknown ObjectType"). Support `REVOKE ... ON VIEW`. ⚪ End-to-end verification that GRANT/REVOKE ... ON VIEW works for all privilege levels (*.*, db.*, db.view, view) and that SHOW GRANTS output matches expectations in real runtime environments (not just unit/BVT tests).
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
🔴	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Unsafe ctx cast: New code type-asserts ctx.Value(defines.TenantIDKey{}) to uint32 without an ok check, which can panic and bypass graceful error handling if the context value has an unexpected type. Referred Code func getSqlForCheckDatabaseView( ctx context.Context, dbName string, viewName string, ) (string, error) { err := inputNameIsInvalid(ctx, dbName, viewName) if err != nil { return "", err } var ( account uint32 ) if v := ctx.Value(defines.TenantIDKey{}); v != nil { account = v.(uint32) } else { return "", moerr.NewInternalErrorNoCtx("no account id found in the ctx") } ... (clipped 27 lines) Learn more about managing compliance generic rules or creating your own custom rules
⚪	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: No audit logging: The PR adds/extends GRANT/REVOKE behavior for views but the diff shows no explicit audit event emission for permission changes, so it is unclear whether privilege changes are comprehensively logged with actor/outcome. Referred Code func doRevokePrivilege(ctx context.Context, ses FeSession, rp *tree.RevokePrivilege, bh BackgroundExec) (err error) { var vr *verifiedRole var objType objectType var objId int64 var privType PrivilegeType var sql string err = normalizeNamesOfRoles(ctx, rp.Roles) if err != nil { return err } account := ses.GetTenantInfo() verifiedRoles := make([]*verifiedRole, len(rp.Roles)) checkedPrivilegeTypes := make([]PrivilegeType, len(rp.Privileges)) //handle "IF EXISTS" //step 1: check roles. exists or not. for i, user := range rp.Roles { //check Revoke privilege on xxx yyy from moadmin(accountadmin) if account.IsNameOfAdminRoles(user.UserName) { ... (clipped 2783 lines) Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: User-facing internals: The PR introduces/uses moerr.NewInternalErrorf messages containing object identifiers (e.g., view and database names) which may surface to end users as "internal error" and could inadvertently disclose resource existence depending on error propagation rules. Referred Code func getViewId(ctx context.Context, bh BackgroundExec, dbName, viewName string) (int64, error) { sql, err := getSqlForCheckDatabaseView(ctx, dbName, viewName) if err != nil { return 0, err } bh.ClearExecResultSet() err = bh.Exec(ctx, sql) if err != nil { return 0, err } erArray, err := getResultSet(ctx, bh) if err != nil { return 0, err } if execResultArrayHasData(erArray) { id, err := erArray[0].GetInt64(ctx, 0, 0) if err != nil { return 0, err } ... (clipped 74 lines) Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Dynamic SQL format: The PR adds additional fmt.Sprintf-built SQL strings for view metadata/ID checks and while inputNameIsInvalid is called, it is not visible in the diff whether its sanitization fully prevents identifier-based injection across all supported name formats. Referred Code checkDatabaseTableFormat = `select rel_logical_id from mo_catalog.mo_tables where relname = "%s" and reldatabase = "%s" and account_id = %d;` checkDatabaseViewFormat = `select rel_logical_id from mo_catalog.mo_tables where relname = "%s" and reldatabase = "%s" and relkind = "v" and account_id = %d;` getViewMetaFormat = `select viewdef, owner from mo_catalog.mo_tables where relname = "%s" and reldatabase = "%s" and relkind = "v" and account_id = %d;` //TODO:fix privilege_level string and obj_type string //For object_type : table, privilege_level : *.* checkWithGrantOptionForTableStarStar = `select rp.privilege_id,rp.with_grant_option from mo_catalog.mo_database d, mo_catalog.mo_tables t, mo_catalog.mo_role_privs rp where d.dat_id = t.reldatabase_id and rp.obj_id = 0 and rp.obj_type = "%s" and rp.role_id = %d and rp.privilege_id = %d and rp.privilege_level = "%s" and rp.with_grant_option = true;` //For object_type : table, privilege_level : db.* checkWithGrantOptionForTableDatabaseStar = `select rp.privilege_id,rp.with_grant_option from mo_catalog.mo_database d, mo_catalog.mo_tables t, mo_catalog.mo_role_privs rp ... (clipped 742 lines) Learn more about managing compliance generic rules or creating your own custom rules
Update

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

[qodo-code-review]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
High-level	Use standard SQL syntax for views Instead of using a non-standard session variable view_security_type, the CREATE VIEW statement should be extended to support the standard SQL SECURITY {DEFINER | INVOKER} clause for better SQL compatibility. Examples: pkg/frontend/variables.go [1709-1716] "view_security_type": { Name: "view_security_type", Scope: ScopeSession, Dynamic: true, SetVarHintApplies: false, Type: InitSystemSystemEnumType("view_security_type", "DEFINER", "INVOKER"), Default: "DEFINER", }, pkg/sql/plan/build_ddl.go [104-118] func getViewSecurityTypeFromContext(ctx CompilerContext) string { securityType := "" val, err := ctx.ResolveVariable("view_security_type", true, false) if err == nil { if s, ok := val.(string); ok { securityType = s } } securityType = strings.TrimSpace(strings.ToUpper(securityType)) if securityType == "INVOKER" { ... (clipped 5 lines) Solution Walkthrough: Before: // User SQL SET view_security_type = 'INVOKER'; CREATE VIEW my_view AS SELECT * FROM my_table; // Go code in pkg/sql/plan/build_ddl.go func genViewTableDef(ctx CompilerContext, stmt *tree.Select) (*plan.TableDef, error) { // ... viewData, err := json.Marshal(ViewData{ Stmt: viewSql, DefaultDatabase: ctx.DefaultDatabase(), SecurityType: getViewSecurityTypeFromContext(ctx), }) // ... } func getViewSecurityTypeFromContext(ctx CompilerContext) string { val, err := ctx.ResolveVariable("view_security_type", true, false) // ... logic to get string value, defaults to DEFINER return securityType } After: // User SQL CREATE VIEW my_view SQL SECURITY INVOKER AS SELECT * FROM my_table; // Go code in pkg/sql/parsers/tree/ddl.go (conceptual change) type CreateView struct { // ... SecurityType string // e.g., "DEFINER" or "INVOKER" } // Go code in pkg/sql/plan/build_ddl.go func Build(stmt *tree.CreateTable, ctx CompilerContext) (*Plan, error) { // ... if stmt.IsView { // ... securityType := stmt.View.SecurityType // Get from parsed AST if securityType == "" { securityType = "DEFINER" // Default } viewData, err := json.Marshal(ViewData{ // ... SecurityType: securityType, }) // ... } } Suggestion importance[1-10]: 9 __ Why: The suggestion correctly identifies a major deviation from standard SQL syntax by using a session variable (view_security_type) instead of the standard SQL SECURITY clause in CREATE VIEW, which significantly impacts usability and compatibility.	High
Possible issue	Regenerate via protoc after proto update Instead of manually editing the generated .pb.go file, declare the new fields in the plan.proto file and regenerate the Go code using protoc. pkg/pb/plan/plan.pb.go [6619-6620] -OriginViews []string `protobuf:"bytes,71,rep,name=origin_views,json=originViews,proto3" json:"origin_views,omitempty"` -DirectView string `protobuf:"bytes,72,opt,name=direct_view,json=directView,proto3" json:"direct_view,omitempty"` +// NOTE: DO NOT EDIT. Fields are defined in plan.proto and this file +// is auto-generated. Update plan.proto and rerun protoc to apply changes. Apply / Chat Suggestion importance[1-10]: 9 __ Why: The suggestion correctly identifies that a generated protobuf file (.pb.go) is being manually edited, which is a critical process error that can lead to serialization issues and maintenance problems.	High
	✅ Remove incorrect view key parsing Suggestion Impact: The parseViewKey implementation no longer shows (and thus appears to have removed) the dot-based fallback splitting logic; instead it now only splits using KeySep (after additionally stripping an optional snapshot suffix). This aligns with the suggestion’s intent to prevent '.' from being treated as a view-key separator. code diff: @@ -5067,6 +5089,9 @@ if key == "" { return "", "" } + if baseKey, _, ok := splitViewSnapshotSuffix(key); ok { + key = baseKey + } if strings.Contains(key, KeySep) { return splitKey(key) } @@ -5076,39 +5101,67 @@ return "", key } Remove the fallback logic that parses view keys using a dot (.) separator in parseViewKey to prevent incorrect parsing when view names contain dots. pkg/frontend/authenticate.go [5066-5077] func parseViewKey(key string) (string, string) { if key == "" { return "", "" } if strings.Contains(key, KeySep) { return splitKey(key) } - if dotIdx := strings.LastIndex(key, "."); dotIdx != -1 { - return key[:dotIdx], key[dotIdx+1:] - } return "", key } [Suggestion processed] Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies that parsing with a '.' separator can lead to bugs if view names contain dots, and removing this fallback logic improves the robustness of view key parsing.	Medium
General	Use constant key separator Replace the hardcoded "#" separator with the KeySep constant when constructing viewKey to improve maintainability. pkg/sql/plan/query_builder.go [4662] -viewKey := schema + "#" + table +viewKey := schema + KeySep + table Apply / Chat Suggestion importance[1-10]: 5 __ Why: The suggestion correctly points out the use of a hardcoded separator, and replacing it with the KeySep constant improves code maintainability and consistency.	Low
Update