Repository navigation
fix(authorization): mark injected filter CTEs NOT MATERIALIZED - #522
Merged
dazzleduck-web merged 5 commits intoOct 29, 2026
Merged
dazzleduck-web merged 5 commits into
dazzleduck-web merged 5 commits into
Conversation
gtaneja-sentrinox
added a commit
to gtaneja-sentrinox/dazzleduck-sql-server
that referenced
this pull request
Oct 29, 2026
…ope; review follow-ups Table-function arguments (an RLS bypass, found reviewing dazzleduck-web#521 with dazzleduck-web#522). A table function's arguments can hold a subquery -- DuckDB evaluates range(0, (SELECT max(amount) FROM secrets)) -- but the collector read only the first argument's constant path and never walked the rest. Under a 'function' grant on range, that query returned secrets' 999 with no check on secrets. And a table function whose first argument is not a constant (range(..), unnest(..), read_parquet((SELECT ..))) was not reported at all, so only the injector's refusal stood between it and a grant. - collectAllTableReferences walks every field of every FROM node except the nested relation fields (left/right/subquery/source), as dazzleduck-web#521 already does for SELECT and expressions: table-function arguments and named parameters, join conditions, AT clauses, pivot aggregates, VALUES rows. - a table function with no constant path is still reported, with a path no path grant can match (NON_CONSTANT_PATH): a table grant refuses it, a function grant decides by name. - injectFilterCtes walks the same fields; it still refuses TABLE_FUNCTION, now only as a backstop. Nested CTE scope. walkNestedCtes removed every same-named name from a CTE body's scope; since the body's own name is added only after it, that removed only an OUTER CTE of the same name, which DuckDB does bind (verified: the inner `o` in `WITH o AS (..) .. (WITH o AS (SELECT .. FROM o) ..)` reads the outer CTE, not the table). The injector wrapped it as a base table and the query failed to bind; the collector already kept it. namesATopLevelCte (withUpdatedDatabaseSchema) matched a qualified `main.orders` against a CTE named orders and left it without the connection's catalog, so a redirect target could resolve it in its own default catalog. A qualified name is never a CTE reference, as isCteReference already holds. Its stray Javadoc moves back to withUpdatedDatabaseSchema. Generated filter-CTE names now also avoid every table name the query reads: a real table named ___orders beside orders took ___orders_1, whose body `SELECT * FROM ___orders` bound to orders' filter CTE and returned orders rows. The left-join pruner's foldCase uses DuckDB's ASCII fold (foldIdentifier), as the authorizer does. Tests (FilterAppliedToEveryReadTest), each failing before its fix: aSubqueryInATableFunctionArgumentIsAuthorized, aSubqueryInAGrantedFunctionsArgumentIsAuthorized (leaked [999]), aNestedCteReadingASameNamedOuterCteReadsTheOuterCte (Table o does not exist), aTableNamedLikeAGeneratedFilterCteKeepsItsOwnRows (u:1,u:3 for u:42), aQualifiedTopLevelReadUnderASameNamedCteStillGetsTheCatalog. Commons suite: 791 run, 0 failures, 8 skipped. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
injectFilterCtes emits one filter CTE per table, SELECT * FROM t WHERE <filter>, and every reference to t reads it. With CTE_MATERIALIZE_DEFAULT, DuckDB materializes a CTE referenced more than once, so any query naming a table twice -- a roster plus a measure, a summary plus a detail, a self-join -- buffers every row the grant allows, every column, before either reference's own projection or predicates apply. On a large table that is the whole granted slice in memory. The CTEs are now CTE_MATERIALIZE_NEVER: each reference inlines to a plain filtered scan with projection, filter and partition pushdown. The body is a filter over one table, so the rows every reference sees are unchanged. Callers cannot work around this themselves: json_serialize_sql records every CTE as CTE_MATERIALIZE_DEFAULT, so a user's own NOT MATERIALIZED never survives the round trip (DuckDB 1.5). Test: two references to one table plan with no CTE_SCAN and return the filtered counts; fails on CTE_MATERIALIZE_DEFAULT. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Inlining the filter CTE evaluates the grant filter once per reference. For a deterministic filter that returns the same rows to each reference; for a volatile one -- a sampling grant like random() < 0.1, or gen_random_uuid() -- a self-join of the granted table would pair two different row sets where the materialized CTE gave one. Such a filter keeps CTE_MATERIALIZE_DEFAULT (isVolatile: random, gen_random_uuid, uuid, uuidv4, uuidv7, nextval, setseed); every other filter stays CTE_MATERIALIZE_NEVER. Test: volatileFilter_keepsTheDefaultSoEveryReferenceSeesOneRowSet. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
gtaneja-sentrinox
force-pushed
the
fix/filter-cte-not-materialized
branch
from
October 29, 2026 19:02
b45c1b3 to
b8f2725
Compare
…olatile isVolatile matched only volatile built-in names, so a grant filter whose randomness came from a sampled subquery (USING SAMPLE / TABLESAMPLE) or a schema-qualified macro wrapping random() was inlined per reference. Both now keep CTE_MATERIALIZE_DEFAULT. A qualified deterministic built-in only keeps the old materialized behaviour. An unqualified user macro cannot be told from a built-in here; the Javadoc says so. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A subquery can be nondeterministic without a volatile call -- LIMIT without ORDER BY, any_value / first over an unordered input -- and an aggregate can only reach a WHERE filter through one. A grant filter with a subquery now keeps CTE_MATERIALIZE_DEFAULT, so every reference sees one row set; filters made of plain predicates are still inlined. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ubqueries are volatile
The previous rules kept ordinary grant filters materialized:
- DuckDB's parser writes list and struct literals as main.list_value /
main.struct_pack, and built-ins live in system.main and pg_catalog, so
"any qualified call" flagged `tenant_id = ANY(['a', 'b'])` and
`{'k': v}.k`. Only a call qualified with a non-default schema or
catalog counts now.
- `x = ANY([..])` parses as a FROM-less subquery over unnest(..), so
"any subquery" flagged it too. Only a subquery that reads a table or
table function counts now; one that reads nothing is walked like any
other expression.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
dazzleduck-web
pushed a commit
that referenced
this pull request
Oct 29, 2026
…521) * fix(authorization): a CTE cannot shadow a qualified table reference injectFilterCtes and the table-reference collector skipped any BASE_TABLE whose bare name matched a CTE in scope, ignoring its schema and catalog. DuckDB binds main.t / db.main.t to the catalog even when a CTE named t is in scope, so WITH t AS (SELECT 1) SELECT * FROM main.t read the real t with no filter CTE: every tenant's rows under a filtered grant, and under RESTRICT_READ_ONLY a table the grant does not list at all. Only an unqualified name now counts as a CTE reference, the rule referencesOuterCte already applied for join pruning. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(authorization): CTE scope follows DuckDB's binding rules Three more ways the CTE-name tracking disagreed with DuckDB, found reviewing the qualified-name fix: - injectFilterCtes made every name in a WITH visible to every body in it, but a body sees only the CTEs declared before it: in `WITH a AS (SELECT * FROM t), t AS (...) SELECT * FROM a` DuckDB binds a's `t` to the table, which was then read with no filter CTE. - collectAllTableReferences ignored a WITH hung on a set operation (`WITH x AS (...) SELECT ... UNION ...`): its bodies went unchecked and its names were reported as tables, so such queries were refused. - CTE names matched case-sensitively; DuckDB matches them case-insensitively. A body's own name is now dropped from its scope in any case, so it always takes the filtered path. The pruning-hint lookup also treats a qualified name as the table under a same-named CTE, so that read keeps its outer-WHERE filter copy. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(authorization): bind names as DuckDB does; walk every expression field From an independent review of the two previous commits, each reproduced against RESTRICT_READ_ONLY and RESTRICTED_DATASOURCE: - Identifier matching used Java's equalsIgnoreCase, which folds Unicode (U+017F long s to `s`); DuckDB folds ASCII only. A CTE named "orderſ" hid a read of orders, or of an ungranted table, from both walks. Added Transformations.sameIdentifier / foldIdentifier (ASCII fold) and used them for every table/CTE name comparison here and in SqlAuthorizer. Introduced by the previous commit's case-insensitive match. - A WITH RECURSIVE CTE's own name was treated as the CTE in both arms; DuckDB binds it to the base table in the anchor arm, which was read unfiltered. The name is now in scope for the recursive arm only, plus `recurring.<name>` when the CTE is USING KEY (previously that reference matched by bare name; qualified-name handling had started refusing it). - The expression walkers of both passes descended an allow-list of fields, missing subqueries under STAR replace_list, window default/offset/partitions/ orders/filter, aggregate order_bys, BETWEEN, DISTINCT ON and others, which then read their table unfiltered. They now walk every field (fail closed). - Generated filter-CTE names were reserved case-sensitively, so a declared ___ORDERS collided with the generated ___orders. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(authorization): keep USING KEY recurring names out of the CTE-name set The previous commit put `recurring.<name>` into the scope as a plain string beside the CTE names. A quoted CTE can be named anything, so `WITH "recurring.secrets" AS (...) ... FROM recurring.secrets` made the real table in a schema named `recurring` look like a CTE: read unfiltered, and past the access check. The recurring names now live in a separate field of a typed scope (CteScope) that user CTE names never reach. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(authorization): authorize table-function arguments; nested CTE scope; review follow-ups Table-function arguments (an RLS bypass, found reviewing #521 with #522). A table function's arguments can hold a subquery -- DuckDB evaluates range(0, (SELECT max(amount) FROM secrets)) -- but the collector read only the first argument's constant path and never walked the rest. Under a 'function' grant on range, that query returned secrets' 999 with no check on secrets. And a table function whose first argument is not a constant (range(..), unnest(..), read_parquet((SELECT ..))) was not reported at all, so only the injector's refusal stood between it and a grant. - collectAllTableReferences walks every field of every FROM node except the nested relation fields (left/right/subquery/source), as #521 already does for SELECT and expressions: table-function arguments and named parameters, join conditions, AT clauses, pivot aggregates, VALUES rows. - a table function with no constant path is still reported, with a path no path grant can match (NON_CONSTANT_PATH): a table grant refuses it, a function grant decides by name. - injectFilterCtes walks the same fields; it still refuses TABLE_FUNCTION, now only as a backstop. Nested CTE scope. walkNestedCtes removed every same-named name from a CTE body's scope; since the body's own name is added only after it, that removed only an OUTER CTE of the same name, which DuckDB does bind (verified: the inner `o` in `WITH o AS (..) .. (WITH o AS (SELECT .. FROM o) ..)` reads the outer CTE, not the table). The injector wrapped it as a base table and the query failed to bind; the collector already kept it. namesATopLevelCte (withUpdatedDatabaseSchema) matched a qualified `main.orders` against a CTE named orders and left it without the connection's catalog, so a redirect target could resolve it in its own default catalog. A qualified name is never a CTE reference, as isCteReference already holds. Its stray Javadoc moves back to withUpdatedDatabaseSchema. Generated filter-CTE names now also avoid every table name the query reads: a real table named ___orders beside orders took ___orders_1, whose body `SELECT * FROM ___orders` bound to orders' filter CTE and returned orders rows. The left-join pruner's foldCase uses DuckDB's ASCII fold (foldIdentifier), as the authorizer does. Tests (FilterAppliedToEveryReadTest), each failing before its fix: aSubqueryInATableFunctionArgumentIsAuthorized, aSubqueryInAGrantedFunctionsArgumentIsAuthorized (leaked [999]), aNestedCteReadingASameNamedOuterCteReadsTheOuterCte (Table o does not exist), aTableNamedLikeAGeneratedFilterCteKeepsItsOwnRows (u:1,u:3 for u:42), aQualifiedTopLevelReadUnderASameNamedCteStillGetsTheCatalog. Commons suite: 791 run, 0 failures, 8 skipped. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(authorization): check every path a table function names; review round 2 A path list that mixed a granted constant with a computed element was checked on the constant alone: under an unfiltered path claim, read_parquet(['<granted>/a.parquet', '/etc/' || 'passwd']) was allowed and DuckDB read the computed path. extractPathsFromTableFunctionChild now reports whether EVERY element was a constant, and the collector adds NON_CONSTANT_PATH (no path grant matches it) as soon as one is not. Row generators that read no data -- unnest, range, generate_series -- are not datasources: with a computed argument (a lateral unnest of a column, range(n)) they are not reported, as before the previous commit, so a table grant still allows `FROM t, unnest(t.tags)`. The injector now lets them through (their arguments are walked and filtered) where it used to throw; any other table function is still refused there. Fail closed in the generic walks: a query or FROM node met anywhere other than through a subquery expression or a relation field now throws instead of being descended into as an expression, where its tables would be neither authorized nor filtered. The collector and the injector walk SELECT and FROM nodes through one helper (forEachFieldExcept) with shared skip sets, so the two cannot drift. SplitPlanner rejects NON_CONSTANT_PATH with a clear message, and the sentinel is a readable string rather than one starting with NUL. Javadocs of collectAllTableReferences and walkNestedCtes describe the current walk and scoping. Tests: aPathGrantRefusesAComputedPathInAList (fails before: the list was allowed), aLateralUnnestOfTheGrantedTableStillWorks (errors before: unsupported FROM node type 'TABLE_FUNCTION'); unsupportedFromNode_failsClosed now uses a data-reading table function. Commons: 793 run, 0 failures. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(authorization): AT-clause subqueries, path traversal, filter placement; review round 3 AT clause. A filter CTE's body is a copy of the table reference, AT clause included (DuckDB ignores AT on a CTE reference), taken before the reference's fields were walked. A subquery in AT (VERSION => (SELECT .. FROM <table>)) therefore ran unfiltered inside the body. Such a subquery is now refused; the AT clause moves to the body and off the reference. Filter-CTE identity. One CTE per RESOLVED table (unqualified parts filled from the connection's defaults, case folded) and AT clause: orders and main.orders used to get two CTEs, so a volatile filter, kept materialized so that every read sees one row set, was evaluated once per spelling. A different AT version gets its own body. TABLESAMPLE. The body copied the first reference's sample, so every reference to the table -- including ones the query did not sample -- read a sampled set. The sample now stays on the reference that wrote it. Table functions. Every table function is reported again, row generators included, and the injector refuses all of them: a catalog table macro named range / unnest / generate_series shadows the built-in, even unqualified, so the name cannot make a call data-free. A table grant now refuses `FROM t, range(..)` whether the argument is a literal or computed; the select-list `SELECT unnest(col) FROM t` is a scalar call and works. Paths. hasAccessToPath refuses a "." or ".." segment ('<granted>/../secret/x.parquet' matched the prefix) and never matches NON_CONSTANT_PATH, a grant of "/" included. Path/function grant filter placement. requireFilterablePlacement refuses a SELECT over a CTE -- its columns are the user's, so `* REPLACE ('<value>' AS <filter column>)` made the filter pass every row -- and a positional UNION, whose column names come from the user-written left arm. UNION BY NAME, the shape clients and the planner generate, still passes. Cleanup: the two pass-through modifier wrappers are gone, and both expression walkers use forEachFieldExcept. Tests, each failing before its fix: aSubqueryInAnAtClauseIsRefused, aTablesampleStaysOnItsOwnReference, differentlyQualifiedReadsOfOneTableShareOneFilterCte, aPathGrantRefusesDotDotSegments, aPathGrantFilterRefusesShapesWhereTheUserDefinesTheColumns, aTableFunctionInFromIsRefusedUnderATableGrantButSelectListUnnestWorks. Commons: 798 run, 0 failures. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(authorization): path-grant filter placement, column aliases, CTE keying; review round 4 Path/function grant filters. requireFilterablePlacement now takes the grant's filter and, unless it is the constant true, also requires: - exactly one table-function read in the query: the filter lands on one SELECT, so a second read of a granted file (a scalar subquery, a WHERE, a join) ran unfiltered; - every UNION BY NAME arm that reads the function to be SELECT * FROM fn(..) with a bare star and no column aliases on the function: `SELECT * REPLACE ('k2' AS key) FROM read_parquet(..)`, a column list, or `fn(..) f(a, key, ..)` made the filter's column the user's and let every row through. A `true` filter (which DuckDB parses as CAST('t' AS BOOLEAN)) has nothing to bypass and is not restricted further. Column aliases. The shared filter-CTE body copied the first reference's column aliases: `orders AS o(a, b, c)` became `FROM orders(a, b, c)` in the body, which DuckDB reads as a table-function call, and the filter would bind to the renamed columns. Aliases now stay on the reference; the RESTRICTED pruning copy of the filter is skipped for an aliased read, where its column names do not bind. CTE keying. The AT-clause part of the key ignores query_location, so two identical AT (VERSION => 3) clauses share one CTE. The filter lookup folds case like the key does: a first reference spelled "ORDERS" no longer gives every reference the deny-all filter. Tests, each failing before its fix: aPathGrantFilterRefusesRenamedArmsAndASecondRead, columnAliasesOnAFilteredTableStillWork, aTableSpelledDifferentlyStillGetsItsFilter, identicalAtClausesShareOneFilterCte. Commons: 802 run, 0 failures. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
injectFilterCtesemits one filter CTE per table (___t AS (SELECT * FROM t WHERE <filter>)), and every reference totreads it. The CTE wasCTE_MATERIALIZE_DEFAULT, and DuckDB materializes a CTE that's referenced more than once. So any query that names a table twice (roster + measure, summary + detail, a self-join) buffers every row the grant allows, every column, before either reference's own projection or predicates apply. On a large table that means buffering the whole granted slice, enough to exhaust the process's memory.Callers can't work around it.
json_serialize_sqlrecords every CTE asCTE_MATERIALIZE_DEFAULT(DuckDB 1.5), so a user's ownNOT MATERIALIZEDnever survives the authorizer's round trip.Fix
buildFilterCteEntrynow emitsCTE_MATERIALIZE_NEVER. Each reference inlines to a plain filtered scan with projection, filter and partition pushdown. The left-join pruner's per-consumer clones copy the whole entry, so they inherit the setting.Inlining evaluates the grant filter once per reference. That returns the same rows to each reference as long as the filter is deterministic. A volatile filter (e.g. sampling with
random() < 0.1, orgen_random_uuid()) would give a self-join two different row sets, so such a filter keepsCTE_MATERIALIZE_DEFAULT(isVolatile).Tests
TransformationsInjectFilterTest#sharedFilterCte_isInlinedNotMaterialized: a table referenced twice plans with noCTE_SCANand returns the filtered counts. It fails onCTE_MATERIALIZE_DEFAULT.TransformationsInjectFilterTest#volatileFilter_keepsTheDefaultSoEveryReferenceSeesOneRowSet: arandom()filter keeps the default, and a deterministic one is inlined.dazzleduck-sql-commonssuite passes (JDK 25).🤖 Generated with Claude Code