From e9888f00f783110aabafd3532ba1703824e8e51e Mon Sep 17 00:00:00 2001 From: seawinde Date: Fri, 10 Apr 2026 17:00:54 +0800 Subject: [PATCH 1/4] [fix](auth) Fix CTE privilege bypass due to shared privChecked flag on StatementContext MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ### What problem does this PR solve? Issue Number: N/A Related PR: #44621 Problem Summary: When a query uses a Common Table Expression (CTE) that references an unauthorized table, the privilege check is incorrectly skipped, allowing the query to succeed without proper authorization. **Root cause:** `CheckPrivileges.rewriteRoot()` uses `StatementContext.isPrivChecked()` as a guard to run only once. However, CTE processing (`RewriteCteChildren.visitLogicalCTEAnchor()`) creates separate `CascadesContext` subtrees for consumer and producer that **share** the same `StatementContext`. The consumer subtree is processed first, its `CheckPrivileges` sets `privChecked=true` on the shared `StatementContext`, and when the producer subtree runs, `CheckPrivileges` sees the flag and skips — leaving unauthorized tables in the CTE body unchecked. ``` StatementContext (shared) ├─ CascadesContext (consumer) ── CheckPrivileges runs ──> sets privChecked=true └─ CascadesContext (producer) ── CheckPrivileges sees true ──> SKIPPED (BUG) ``` **Fix:** Move the `privChecked` flag from `StatementContext` to `CascadesContext`. Since `newSubtreeContext()` creates a **new** `CascadesContext` instance for each CTE subtree, each subtree now has its own independent flag. This ensures both producer and consumer run their own privilege checks. Within the same `CascadesContext`, the flag still prevents duplicate checks (e.g., after view inlining — preserving #44621 semantics). ``` StatementContext (shared, no privChecked) ├─ CascadesContext (consumer) ── privChecked=false ── CheckPrivileges runs ── privChecked=true └─ CascadesContext (producer) ── privChecked=false ── CheckPrivileges runs ── privChecked=true ✓ ``` | File | Change Description | |------|-------------------| | `CascadesContext.java` | Add `privChecked` boolean field + getter/setter | | `StatementContext.java` | Remove `privChecked` field and accessors | | `CheckPrivileges.java` | Read/write `privChecked` on `CascadesContext` instead of `StatementContext` | | `TestCheckPrivileges.java` | Add `testCtePrivilegeCheck()` with 4 CTE authorization scenarios | | `TestCtePrivCheckGranularity.java` (new) | 4 unit tests verifying per-CascadesContext flag independence | | `test_cte_privilege_check.groovy` (new) | 6 regression test cases covering CTE privilege bypass | ### Release note Fixed a privilege bypass where queries using CTE (Common Table Expression) could access unauthorized tables without being denied. The privilege check was incorrectly shared across CTE subtrees, causing the producer's check to be skipped after the consumer's check ran. ### Check List (For Author) - Test - [x] Regression test - [x] Unit Test - Behavior changed: - [x] No. - Does this need documentation? - [x] No. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../apache/doris/nereids/CascadesContext.java | 10 ++ .../doris/nereids/StatementContext.java | 10 -- .../rules/rewrite/CheckPrivileges.java | 12 +- .../privileges/TestCheckPrivileges.java | 70 ++++++++ .../TestCtePrivCheckGranularity.java | 97 +++++++++++ .../auth_p0/test_cte_privilege_check.groovy | 151 ++++++++++++++++++ 6 files changed, 336 insertions(+), 14 deletions(-) create mode 100644 fe/fe-core/src/test/java/org/apache/doris/nereids/privileges/TestCtePrivCheckGranularity.java create mode 100644 regression-test/suites/auth_p0/test_cte_privilege_check.groovy diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/CascadesContext.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/CascadesContext.java index aa98abdd4b0c22..56b37025433c05 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/CascadesContext.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/CascadesContext.java @@ -120,6 +120,8 @@ public class CascadesContext implements ScheduleContext { private boolean isLeadingDisableJoinReorder = false; + private boolean privChecked = false; + private final Map hintMap = Maps.newLinkedHashMap(); private final ThreadLocal showPlanProcess = new ThreadLocal<>(); @@ -520,6 +522,14 @@ public void setLeadingDisableJoinReorder(boolean leadingDisableJoinReorder) { isLeadingDisableJoinReorder = leadingDisableJoinReorder; } + public boolean isPrivChecked() { + return privChecked; + } + + public void setPrivChecked(boolean privChecked) { + this.privChecked = privChecked; + } + public Map getHintMap() { return hintMap; } diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/StatementContext.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/StatementContext.java index 520ef17fb0bb18..e530706b3fa000 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/StatementContext.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/StatementContext.java @@ -266,8 +266,6 @@ public enum TableFrom { private final Map snapshots = Maps.newHashMap(); - private boolean privChecked; - // if greater than 0 means the duration has used private long materializedViewRewriteDuration = 0L; @@ -1029,14 +1027,6 @@ public void setGroupCommitMergeBackend( this.groupCommitMergeBackend = groupCommitMergeBackend; } - public boolean isPrivChecked() { - return privChecked; - } - - public void setPrivChecked(boolean privChecked) { - this.privChecked = privChecked; - } - public List getUsedBackendsDistributing() { return usedBackendsDistributing; } diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/CheckPrivileges.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/CheckPrivileges.java index b65f7245ca7603..e1532edfe15ef0 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/CheckPrivileges.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/CheckPrivileges.java @@ -46,20 +46,24 @@ /** * CheckPrivileges - * This rule should only check once, because after check would set setPrivChecked in statementContext + * This rule should only check once per CascadesContext, because after check would set privChecked + * on the CascadesContext. Using CascadesContext-level (instead of StatementContext-level) flag + * ensures CTE producer and consumer subtrees are checked independently, while still preventing + * duplicate checks within the same subtree (e.g., after view inlining). */ public class CheckPrivileges extends ColumnPruning { private JobContext jobContext; @Override public Plan rewriteRoot(Plan plan, JobContext jobContext) { - // Only enter once, if repeated, the permissions of the table in the view will be checked - if (jobContext.getCascadesContext().getStatementContext().isPrivChecked()) { + // Only enter once per CascadesContext. If repeated within the same context, + // the permissions of the table in the view will be checked incorrectly. + if (jobContext.getCascadesContext().isPrivChecked()) { return plan; } this.jobContext = jobContext; super.rewriteRoot(plan, jobContext); - jobContext.getCascadesContext().getStatementContext().setPrivChecked(true); + jobContext.getCascadesContext().setPrivChecked(true); // don't rewrite plan, because Reorder expect no LogicalProject on LogicalJoin return plan; } diff --git a/fe/fe-core/src/test/java/org/apache/doris/nereids/privileges/TestCheckPrivileges.java b/fe/fe-core/src/test/java/org/apache/doris/nereids/privileges/TestCheckPrivileges.java index 658f3a96128fd5..5b5e5bef420fbe 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/nereids/privileges/TestCheckPrivileges.java +++ b/fe/fe-core/src/test/java/org/apache/doris/nereids/privileges/TestCheckPrivileges.java @@ -223,6 +223,76 @@ public void testPrivilegesAndPolicies() throws Exception { }); } + @Test + public void testCtePrivilegeCheck() throws Exception { + FeConstants.runningUnitTest = true; + // Use the same external catalog for privilege control infrastructure + String catalogProvider + = "org.apache.doris.nereids.privileges.TestCheckPrivileges$CustomCatalogProvider"; + String accessControllerFactory + = "org.apache.doris.nereids.privileges.CustomAccessControllerFactory"; + String catalog = "custom_catalog_cte"; + + createCatalog("create catalog " + catalog + " properties(" + + " \"type\"=\"test\"," + + " \"catalog_provider.class\"=\"" + catalogProvider + "\"," + + " \"" + CatalogMgr.ACCESS_CONTROLLER_CLASS_PROP + "\"=\"" + accessControllerFactory + "\"" + + ")"); + + // Create internal OLAP tables (LogicalOlapScan supports withRelationId needed by CTE) + String cteDb = "cte_priv_db"; + createDatabase(cteDb); + useDatabase(cteDb); + createTable("create table allowed_tbl (id int, name varchar(50)) " + + "distributed by hash(id) buckets 1 " + + "properties('replication_num' = '1')"); + createTable("create table denied_tbl (id int, name varchar(50)) " + + "distributed by hash(id) buckets 1 " + + "properties('replication_num' = '1')"); + + String user = "test_cte_privilege_user"; + addUser(user, true); + useUser(user); + + List privileges = ImmutableList.of( + MakePrivileges.table("internal", cteDb, "allowed_tbl").allowSelectTable(user) + ); + + AccessControllerManager accessManager = Env.getCurrentEnv().getAccessManager(); + CatalogAccessController catalogAccessController = accessManager.getAccessControllerOrDefault(catalog); + new Expectations(accessManager) { + { + accessManager.getAccessControllerOrDefault("internal"); + minTimes = 0; + result = catalogAccessController; + } + }; + + withPrivileges(privileges, () -> { + // CTE with authorized table should succeed + query("with cte as (select * from allowed_tbl) select * from cte"); + + // CTE with unauthorized table should fail (this was the bug: + // consumer was checked first, setting privChecked=true on shared StatementContext, + // so producer's CheckPrivileges was skipped) + Assertions.assertThrows(AnalysisException.class, () -> + query("with cte as (select * from denied_tbl) select * from cte") + ); + + // CTE referenced twice forces no-inline path through RewriteCteChildren + Assertions.assertThrows(AnalysisException.class, () -> + query("with cte as (select * from denied_tbl) " + + "select * from cte union all select * from cte") + ); + + // CTE authorized + direct unauthorized table should fail + Assertions.assertThrows(AnalysisException.class, () -> + query("with cte as (select * from allowed_tbl) " + + "select * from cte union all select * from denied_tbl") + ); + }); + } + private void query(String sql) { PlanChecker.from(connectContext) .parse(sql) diff --git a/fe/fe-core/src/test/java/org/apache/doris/nereids/privileges/TestCtePrivCheckGranularity.java b/fe/fe-core/src/test/java/org/apache/doris/nereids/privileges/TestCtePrivCheckGranularity.java new file mode 100644 index 00000000000000..11004de28d903b --- /dev/null +++ b/fe/fe-core/src/test/java/org/apache/doris/nereids/privileges/TestCtePrivCheckGranularity.java @@ -0,0 +1,97 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package org.apache.doris.nereids.privileges; + +import org.apache.doris.nereids.CascadesContext; +import org.apache.doris.nereids.properties.PhysicalProperties; + +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +import java.util.Optional; + +/** + * Tests that privChecked flag is per-CascadesContext, not per-StatementContext. + * This ensures CTE producer and consumer subtrees check privileges independently. + */ +public class TestCtePrivCheckGranularity { + + @Test + public void testPrivCheckedDefaultFalse() { + CascadesContext ctx = CascadesContext.initTempContext(); + Assertions.assertFalse(ctx.isPrivChecked(), "privChecked should default to false"); + } + + @Test + public void testPrivCheckedSetAndGet() { + CascadesContext ctx = CascadesContext.initTempContext(); + + ctx.setPrivChecked(true); + Assertions.assertTrue(ctx.isPrivChecked()); + + ctx.setPrivChecked(false); + Assertions.assertFalse(ctx.isPrivChecked()); + } + + @Test + public void testSubtreeContextHasIndependentPrivChecked() { + CascadesContext parentCtx = CascadesContext.initTempContext(); + + // Simulate CTE: create subtree context (shares StatementContext but different CascadesContext) + CascadesContext subtreeCtx = CascadesContext.newSubtreeContext( + Optional.empty(), parentCtx, parentCtx.getRewritePlan(), PhysicalProperties.ANY); + + // Both should share the same StatementContext + Assertions.assertSame(parentCtx.getStatementContext(), subtreeCtx.getStatementContext(), + "Parent and subtree should share the same StatementContext"); + + // Both should start with privChecked = false + Assertions.assertFalse(parentCtx.isPrivChecked()); + Assertions.assertFalse(subtreeCtx.isPrivChecked()); + + // Setting privChecked on subtree should NOT affect parent + subtreeCtx.setPrivChecked(true); + Assertions.assertTrue(subtreeCtx.isPrivChecked(), "subtree privChecked should be true"); + Assertions.assertFalse(parentCtx.isPrivChecked(), "parent privChecked should still be false"); + + // Setting privChecked on parent should NOT affect subtree + parentCtx.setPrivChecked(true); + Assertions.assertTrue(parentCtx.isPrivChecked()); + Assertions.assertTrue(subtreeCtx.isPrivChecked()); // still true from before + } + + @Test + public void testTwoSubtreeContextsAreIndependent() { + CascadesContext parentCtx = CascadesContext.initTempContext(); + + // Simulate CTE consumer and producer: two subtree contexts from same parent + CascadesContext consumerCtx = CascadesContext.newSubtreeContext( + Optional.empty(), parentCtx, parentCtx.getRewritePlan(), PhysicalProperties.ANY); + CascadesContext producerCtx = CascadesContext.newSubtreeContext( + Optional.empty(), parentCtx, parentCtx.getRewritePlan(), PhysicalProperties.ANY); + + // Consumer runs CheckPrivileges first, sets its flag + consumerCtx.setPrivChecked(true); + + // Producer should NOT see consumer's flag — this is the bug fix + Assertions.assertFalse(producerCtx.isPrivChecked(), + "Producer CascadesContext should have independent privChecked from consumer"); + Assertions.assertFalse(parentCtx.isPrivChecked(), + "Parent CascadesContext should not be affected by consumer's privChecked"); + } +} diff --git a/regression-test/suites/auth_p0/test_cte_privilege_check.groovy b/regression-test/suites/auth_p0/test_cte_privilege_check.groovy new file mode 100644 index 00000000000000..ec5af48af1d7c4 --- /dev/null +++ b/regression-test/suites/auth_p0/test_cte_privilege_check.groovy @@ -0,0 +1,151 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +suite("test_cte_privilege_check","p0,auth") { + String suiteName = "test_cte_privilege_check" + String user = "${suiteName}_user" + String pwd = 'C123_567p' + String dbName = "${suiteName}_db" + String tableName1 = "${suiteName}_table1" + String tableName2 = "${suiteName}_table2" + String viewName = "${suiteName}_view" + + try_sql("drop user ${user}") + try_sql """drop view if exists ${dbName}.${viewName}""" + try_sql """drop table if exists ${dbName}.${tableName1}""" + try_sql """drop table if exists ${dbName}.${tableName2}""" + sql """drop database if exists ${dbName}""" + + sql """create user '${user}' IDENTIFIED by '${pwd}'""" + + //cloud-mode + if (isCloudMode()) { + def clusters = sql " SHOW CLUSTERS; " + assertTrue(!clusters.isEmpty()) + def validCluster = clusters[0][0] + sql """GRANT USAGE_PRIV ON CLUSTER `${validCluster}` TO ${user}"""; + } + + sql """create database ${dbName}""" + sql("""use ${dbName}""") + + sql """ + CREATE TABLE IF NOT EXISTS ${dbName}.`${tableName1}` ( + id BIGINT, + username VARCHAR(20) + ) + DISTRIBUTED BY HASH(id) BUCKETS 2 + PROPERTIES ( + "replication_num" = "1" + ); + """ + + sql """ + CREATE TABLE IF NOT EXISTS ${dbName}.`${tableName2}` ( + id BIGINT, + username VARCHAR(20) + ) + DISTRIBUTED BY HASH(id) BUCKETS 2 + PROPERTIES ( + "replication_num" = "1" + ); + """ + + sql """insert into ${dbName}.${tableName1} values (1, 'alice'), (2, 'bob')""" + sql """insert into ${dbName}.${tableName2} values (3, 'charlie'), (4, 'dave')""" + + // Grant select only on tableName1, NOT on tableName2 + sql """grant select_priv on ${dbName}.${tableName1} to ${user}""" + + // Case 1: CTE references unauthorized table -> should fail + connect(user, "${pwd}", context.config.jdbcUrl) { + try { + sql """ + with cte as (select * from ${dbName}.${tableName2}) + select * from cte + """ + assertTrue(false, "Should have thrown privilege denied exception") + } catch (Exception e) { + log.info("Case 1 expected error: " + e.getMessage()) + assertTrue(e.getMessage().contains("denied"), "Expected denied but got: " + e.getMessage()) + } + } + + // Case 2: CTE references authorized table -> should succeed + connect(user, "${pwd}", context.config.jdbcUrl) { + sql """ + with cte as (select * from ${dbName}.${tableName1}) + select * from cte + """ + } + + // Case 3: Direct query unauthorized table -> should fail (baseline) + connect(user, "${pwd}", context.config.jdbcUrl) { + try { + sql "select * from ${dbName}.${tableName2}" + assertTrue(false, "Should have thrown privilege denied exception") + } catch (Exception e) { + log.info("Case 3 expected error: " + e.getMessage()) + assertTrue(e.getMessage().contains("denied"), "Expected denied but got: " + e.getMessage()) + } + } + + // Case 4: CTE referenced twice (no inline) + unauthorized table -> should fail + connect(user, "${pwd}", context.config.jdbcUrl) { + try { + sql """ + with cte as (select * from ${dbName}.${tableName2}) + select * from cte + union all + select * from cte + """ + assertTrue(false, "Should have thrown privilege denied exception") + } catch (Exception e) { + log.info("Case 4 expected error: " + e.getMessage()) + assertTrue(e.getMessage().contains("denied"), "Expected denied but got: " + e.getMessage()) + } + } + + // Case 5: CTE authorized + direct unauthorized table -> should fail + connect(user, "${pwd}", context.config.jdbcUrl) { + try { + sql """ + with cte as (select * from ${dbName}.${tableName1}) + select * from cte + union all + select * from ${dbName}.${tableName2} + """ + assertTrue(false, "Should have thrown privilege denied exception") + } catch (Exception e) { + log.info("Case 5 expected error: " + e.getMessage()) + assertTrue(e.getMessage().contains("denied"), "Expected denied but got: " + e.getMessage()) + } + } + + // Case 6: View + UNION auth non-regression (PR #44621) + sql """create view ${dbName}.${viewName} as select * from ${dbName}.${tableName1} union select * from ${dbName}.${tableName2};""" + sql """grant select_priv on ${dbName}.${viewName} to ${user}""" + connect(user, "${pwd}", context.config.jdbcUrl) { + sql "select * from ${dbName}.${viewName}" + } + + try_sql("drop user ${user}") + try_sql """drop view if exists ${dbName}.${viewName}""" + try_sql """drop table if exists ${dbName}.${tableName1}""" + try_sql """drop table if exists ${dbName}.${tableName2}""" + sql """drop database if exists ${dbName}""" +} From 4efdbf93d62c469fafa6175b1114ddc312b1cb9a Mon Sep 17 00:00:00 2001 From: seawinde Date: Mon, 13 Apr 2026 14:54:50 +0800 Subject: [PATCH 2/4] [fix](auth) Fix test failures in CTE privilege check tests ### What problem does this PR solve? Issue Number: N/A Related PR: #62339 Problem Summary: Fix two test failures introduced in the CTE privilege bypass fix: 1. **test_cte_privilege_check.groovy**: The regression test user lacked database-level access to `regression_test`, causing `connect()` to fail before any CTE query was executed. Added `grant select_priv on regression_test` to the test setup. 2. **TestCheckPrivileges.testCtePrivilegeCheck**: The new test method called `useUser("test_cte_privilege_user")` on the shared `connectContext` but never restored it. When `testPrivilegesAndPolicies` ran afterward, the unprivileged user caused `createCatalog` to throw `AnalysisException` instead of the expected `DdlException`. Wrapped the test body in try-finally to restore root user. ### Release note None ### Check List (For Author) - Test: Unit Test / Regression test fix - Behavior changed: No - Does this need documentation: No Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../privileges/TestCheckPrivileges.java | 74 ++++++++++--------- .../auth_p0/test_cte_privilege_check.groovy | 1 + 2 files changed, 40 insertions(+), 35 deletions(-) diff --git a/fe/fe-core/src/test/java/org/apache/doris/nereids/privileges/TestCheckPrivileges.java b/fe/fe-core/src/test/java/org/apache/doris/nereids/privileges/TestCheckPrivileges.java index 5b5e5bef420fbe..9b0835e39de83e 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/nereids/privileges/TestCheckPrivileges.java +++ b/fe/fe-core/src/test/java/org/apache/doris/nereids/privileges/TestCheckPrivileges.java @@ -254,43 +254,47 @@ public void testCtePrivilegeCheck() throws Exception { addUser(user, true); useUser(user); - List privileges = ImmutableList.of( - MakePrivileges.table("internal", cteDb, "allowed_tbl").allowSelectTable(user) - ); - - AccessControllerManager accessManager = Env.getCurrentEnv().getAccessManager(); - CatalogAccessController catalogAccessController = accessManager.getAccessControllerOrDefault(catalog); - new Expectations(accessManager) { - { - accessManager.getAccessControllerOrDefault("internal"); - minTimes = 0; - result = catalogAccessController; - } - }; - - withPrivileges(privileges, () -> { - // CTE with authorized table should succeed - query("with cte as (select * from allowed_tbl) select * from cte"); - - // CTE with unauthorized table should fail (this was the bug: - // consumer was checked first, setting privChecked=true on shared StatementContext, - // so producer's CheckPrivileges was skipped) - Assertions.assertThrows(AnalysisException.class, () -> - query("with cte as (select * from denied_tbl) select * from cte") - ); - - // CTE referenced twice forces no-inline path through RewriteCteChildren - Assertions.assertThrows(AnalysisException.class, () -> - query("with cte as (select * from denied_tbl) " - + "select * from cte union all select * from cte") + try { + List privileges = ImmutableList.of( + MakePrivileges.table("internal", cteDb, "allowed_tbl").allowSelectTable(user) ); - // CTE authorized + direct unauthorized table should fail - Assertions.assertThrows(AnalysisException.class, () -> - query("with cte as (select * from allowed_tbl) " - + "select * from cte union all select * from denied_tbl") - ); - }); + AccessControllerManager accessManager = Env.getCurrentEnv().getAccessManager(); + CatalogAccessController catalogAccessController = accessManager.getAccessControllerOrDefault(catalog); + new Expectations(accessManager) { + { + accessManager.getAccessControllerOrDefault("internal"); + minTimes = 0; + result = catalogAccessController; + } + }; + + withPrivileges(privileges, () -> { + // CTE with authorized table should succeed + query("with cte as (select * from allowed_tbl) select * from cte"); + + // CTE with unauthorized table should fail (this was the bug: + // consumer was checked first, setting privChecked=true on shared StatementContext, + // so producer's CheckPrivileges was skipped) + Assertions.assertThrows(AnalysisException.class, () -> + query("with cte as (select * from denied_tbl) select * from cte") + ); + + // CTE referenced twice forces no-inline path through RewriteCteChildren + Assertions.assertThrows(AnalysisException.class, () -> + query("with cte as (select * from denied_tbl) " + + "select * from cte union all select * from cte") + ); + + // CTE authorized + direct unauthorized table should fail + Assertions.assertThrows(AnalysisException.class, () -> + query("with cte as (select * from allowed_tbl) " + + "select * from cte union all select * from denied_tbl") + ); + }); + } finally { + useUser("root"); + } } private void query(String sql) { diff --git a/regression-test/suites/auth_p0/test_cte_privilege_check.groovy b/regression-test/suites/auth_p0/test_cte_privilege_check.groovy index ec5af48af1d7c4..b9d9f566772759 100644 --- a/regression-test/suites/auth_p0/test_cte_privilege_check.groovy +++ b/regression-test/suites/auth_p0/test_cte_privilege_check.groovy @@ -69,6 +69,7 @@ suite("test_cte_privilege_check","p0,auth") { sql """insert into ${dbName}.${tableName2} values (3, 'charlie'), (4, 'dave')""" // Grant select only on tableName1, NOT on tableName2 + sql """grant select_priv on regression_test to ${user}""" sql """grant select_priv on ${dbName}.${tableName1} to ${user}""" // Case 1: CTE references unauthorized table -> should fail From fae07680321d57f3de12c174b741a9844a726246 Mon Sep 17 00:00:00 2001 From: seawinde Date: Mon, 13 Apr 2026 15:50:13 +0800 Subject: [PATCH 3/4] [fix](fe) Convert testCtePrivilegeCheck from JMockit to Mockito ### What problem does this PR solve? Problem Summary: After PR #62221 removed all JMockit usage from the codebase, the testCtePrivilegeCheck method still used JMockit Expectations pattern which causes compilation failure. Convert to Mockito spy pattern consistent with the rest of the test file. ### Release note None ### Check List (For Author) - Test: Unit Test (compilation verified) - Behavior changed: No - Does this need documentation: No Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../doris/nereids/privileges/TestCheckPrivileges.java | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/fe/fe-core/src/test/java/org/apache/doris/nereids/privileges/TestCheckPrivileges.java b/fe/fe-core/src/test/java/org/apache/doris/nereids/privileges/TestCheckPrivileges.java index 9b0835e39de83e..bec6b1299d01d6 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/nereids/privileges/TestCheckPrivileges.java +++ b/fe/fe-core/src/test/java/org/apache/doris/nereids/privileges/TestCheckPrivileges.java @@ -261,13 +261,10 @@ public void testCtePrivilegeCheck() throws Exception { AccessControllerManager accessManager = Env.getCurrentEnv().getAccessManager(); CatalogAccessController catalogAccessController = accessManager.getAccessControllerOrDefault(catalog); - new Expectations(accessManager) { - { - accessManager.getAccessControllerOrDefault("internal"); - minTimes = 0; - result = catalogAccessController; - } - }; + AccessControllerManager spyAccessManager = Mockito.spy(accessManager); + Mockito.doReturn(catalogAccessController).when(spyAccessManager) + .getAccessControllerOrDefault("internal"); + Deencapsulation.setField(Env.getCurrentEnv(), "accessManager", spyAccessManager); withPrivileges(privileges, () -> { // CTE with authorized table should succeed From e4facdf901f0edc0e58ed76786ea4a41e0fab2d2 Mon Sep 17 00:00:00 2001 From: seawinde Date: Mon, 13 Apr 2026 17:20:12 +0800 Subject: [PATCH 4/4] [fix](fe) Skip auth for DeleteFromUsingCommand fallback in DeleteFromCommand ### What problem does this PR solve? Problem Summary: Moving privChecked from StatementContext to CascadesContext (for the CTE privilege bypass fix) caused a regression in DELETE commands with complex WHERE clauses (e.g. NOT EXISTS subquery). DeleteFromCommand sets skipAuth=true during initial Nereids planning because DELETE does not need SELECT privilege. Previously, the privChecked flag on StatementContext was shared across planners, so when DeleteFromUsingCommand created a new planner, CheckPrivileges would see privChecked=true and skip. After moving privChecked to CascadesContext, the new planner gets a fresh CascadesContext with privChecked=false, causing CheckPrivileges to run auth checks without skipAuth=true, failing on tables the user only has LOAD privilege for. Fix: Wrap both DeleteFromUsingCommand.run() call sites in skipAuth=true, consistent with the initial planning phase's auth policy. ### Release note None ### Check List (For Author) - Test: Regression test (test_dml_delete_table_auth) - Behavior changed: No - Does this need documentation: No Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../plans/commands/DeleteFromCommand.java | 20 +++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/DeleteFromCommand.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/DeleteFromCommand.java index d939d278ddeff5..25fd4bfd3365a3 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/DeleteFromCommand.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/DeleteFromCommand.java @@ -207,8 +207,14 @@ public void run(ConnectContext ctx, StmtExecutor executor) throws Exception { } } catch (Exception e) { try { - new DeleteFromUsingCommand(nameParts, tableAlias, isTempPart, partitions, - logicalQuery, Optional.empty(), false).run(ctx, executor); + // delete not need select priv, skip auth for the fallback planner + ctx.setSkipAuth(true); + try { + new DeleteFromUsingCommand(nameParts, tableAlias, isTempPart, partitions, + logicalQuery, Optional.empty(), false).run(ctx, executor); + } finally { + ctx.setSkipAuth(originalIsSkipAuth); + } return; } catch (Exception e2) { LOG.warn("delete from command failed", e2); @@ -219,8 +225,14 @@ public void run(ConnectContext ctx, StmtExecutor executor) throws Exception { // if table's enable_mow_light_delete is false, use `DeleteFromUsingCommand` if (olapTable.getKeysType() == KeysType.UNIQUE_KEYS && olapTable.getEnableUniqueKeyMergeOnWrite() && !olapTable.getEnableMowLightDelete()) { - new DeleteFromUsingCommand(nameParts, tableAlias, isTempPart, partitions, logicalQuery, - Optional.empty(), false).run(ctx, executor); + // delete not need select priv, skip auth for the fallback planner + ctx.setSkipAuth(true); + try { + new DeleteFromUsingCommand(nameParts, tableAlias, isTempPart, partitions, logicalQuery, + Optional.empty(), false).run(ctx, executor); + } finally { + ctx.setSkipAuth(originalIsSkipAuth); + } return; }