Skip to content

fix(authorization): mark injected filter CTEs NOT MATERIALIZED - #522

Merged
dazzleduck-web merged 5 commits into
dazzleduck-web:mainfrom
gtaneja-sentrinox:fix/filter-cte-not-materialized
Oct 29, 2026
Merged

dazzleduck-web merged 5 commits into
dazzleduck-web:mainfrom
gtaneja-sentrinox:fix/filter-cte-not-materialized

Conversation

@gtaneja-sentrinox

@gtaneja-sentrinox gtaneja-sentrinox commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

Problem

injectFilterCtes emits one filter CTE per table (___t AS (SELECT * FROM t WHERE <filter>)), and every reference to t reads it. The CTE was CTE_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_sql records every CTE as CTE_MATERIALIZE_DEFAULT (DuckDB 1.5), so a user's own NOT MATERIALIZED never survives the authorizer's round trip.

Fix

buildFilterCteEntry now emits CTE_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, or gen_random_uuid()) would give a self-join two different row sets, so such a filter keeps CTE_MATERIALIZE_DEFAULT (isVolatile).

Tests

  • TransformationsInjectFilterTest#sharedFilterCte_isInlinedNotMaterialized: a table referenced twice plans with no CTE_SCAN and returns the filtered counts. It fails on CTE_MATERIALIZE_DEFAULT.
  • TransformationsInjectFilterTest#volatileFilter_keepsTheDefaultSoEveryReferenceSeesOneRowSet: a random() filter keeps the default, and a deterministic one is inlined.
  • Full dazzleduck-sql-commons suite passes (JDK 25).

🤖 Generated with Claude Code

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>
gtaneja-sentrinox and others added 2 commits October 29, 2026 12:01
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
gtaneja-sentrinox force-pushed the fix/filter-cte-not-materialized branch from b45c1b3 to b8f2725 Compare October 29, 2026 19:02
gtaneja-sentrinox and others added 3 commits October 29, 2026 12:08
…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
dazzleduck-web merged commit c952b1b into dazzleduck-web:main Oct 29, 2026
2 checks passed
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants