Authorize source access before purification - #38564
Draft
tonydu-mz wants to merge 1 commit into
Draft
Conversation
tonydu-mz
force-pushed
the
tonydu/sql-660-create-table-from-source-and-alter-source-drive-the-sources
branch
2 times, most recently
from
September 2, 2026 18:39
6dbe163 to
c8dd2ba
Compare
Fixes SQL-660. ### Motivation `CREATE TABLE ... FROM SOURCE` and `ALTER SOURCE` are purified off-thread before they are planned. The only privilege check that ran before purification was `rbac::check_usage(.., &CREATE_ITEM_USAGE)`, which requires `USAGE` on the `Secret`, `Connection` and `Type` items the statement names. These statements name neither a secret nor a connection: they reach the upstream through an existing source, whose connection comes from its own `source_desc()` rather than from the statement. So nothing was required. Purification then opens that connection with the source owner's credentials and enumerates upstream objects. A role holding no privilege on the source could therefore make Materialize dial the source's upstream, and read upstream schema, table and column names out of the resulting purification errors. SQL-655 (#38480) closed the plan-time bypass, so no rows are readable; this is the residual that fix named. `CREATE SOURCE ... FROM CONNECTION` and `CREATE SINK ... INTO` are unaffected: they name their connection, so the existing usage requirement gates them. ### Description Replace the pre-purification `check_usage` call with `rbac::check_purification`, which builds a single `RbacRequirements` for the statement and delegates to the same validation path `check_plan` uses: * the existing `CREATE_ITEM_USAGE` usage requirements, unchanged; * for `ALTER SOURCE`: ownership of the named source; * for `CREATE TABLE ... FROM SOURCE`: read privileges on the source (`SELECT` plus schema `USAGE`), required of the owner too, since an owner's `SELECT` is an ordinary revocable grant and schema `USAGE` is separate from ownership. Both mirror what planning requires later, so a statement that passes here can still be rejected by `check_plan`, never the reverse. The source is resolved from the statement rather than from `resolved_ids`. `AlterSourceStatement::source_name` is an `UnresolvedItemName`, so name resolution never records it and a `resolved_ids`-based check is silently a no-op for `ALTER SOURCE`. Resolution mirrors purification exactly, so the check gates the item that would be dialed. The resolved source id is also added to the purified statement's dependency set. It was previously absent for `ALTER SOURCE`, so a source dropped concurrently with off-thread purification passed the validity check and then panicked the coordinator on the missing catalog entry ("catalog out of sync") during planning. With the id tracked, the drop is detected and the statement is repurified, ending in a clean unknown-item error. ### Verification `test/sqllogictest/rbac_create_table_from_source.slt` gains cases that pin the ordering, not just the denial. Each uses a statement whose purification fails for a non-permission reason, so a permission error can only come from the pre-purification check: * `CREATE TABLE ... FROM SOURCE` with an unresolvable reference: purification reports whether a reference exists upstream, so without the gate this leaks reference existence. * The same statement run by the source's owner after their `SELECT` is revoked, pinning that ownership does not stand in for read privileges. * `ALTER SOURCE ... ADD SUBSOURCE` on a load generator, which purification rejects with "does not support ALTER SOURCE": an ownership error can only come from the earlier gate. Plus: an owner-succeeds case, so the rule gates the caller rather than the syntax; a schema-`USAGE` denial, pinning the other half of the read requirement; and pass-through cases for a superuser and for an RBAC-disabled deployment, each reaching purification's own error on a source they do not own, pinning that the gate filters requirements the same way `check_plan` does.
tonydu-mz
force-pushed
the
tonydu/sql-660-create-table-from-source-and-alter-source-drive-the-sources
branch
from
September 3, 2026 01:55
c8dd2ba to
4e17335
Compare
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.
Authorize source access before purification
Fixes SQL-660.
Motivation
CREATE TABLE ... FROM SOURCEandALTER SOURCEare purified off-threadbefore they are planned. The only privilege check that ran before purification
was
rbac::check_usage(.., &CREATE_ITEM_USAGE), which requiresUSAGEon theSecret,ConnectionandTypeitems the statement names. These statementsname neither a secret nor a connection: they reach the upstream through an
existing source, whose connection comes from its own
source_desc()ratherthan from the statement. So nothing was required.
Purification then opens that connection with the source owner's credentials and
enumerates upstream objects. A role holding no privilege on the source could
therefore make Materialize dial the source's upstream, and read upstream schema,
table and column names out of the resulting purification errors. SQL-655
(#38480) closed the plan-time bypass, so no rows are readable; this is the
residual that fix named.
CREATE SOURCE ... FROM CONNECTIONandCREATE SINK ... INTOare unaffected:they name their connection, so the existing usage requirement gates them.
Description
Replace the pre-purification
check_usagecall withrbac::check_purification,which builds a single
RbacRequirementsfor the statement and delegates to thesame validation path
check_planuses:CREATE_ITEM_USAGEusage requirements, unchanged;ALTER SOURCE: ownership of the named source;CREATE TABLE ... FROM SOURCE: read privileges on the source (SELECTplus schema
USAGE), required of the owner too, since an owner'sSELECTisan ordinary revocable grant and schema
USAGEis separate from ownership.Both mirror what planning requires later, so a statement that passes here can
still be rejected by
check_plan, never the reverse.The source is resolved from the statement rather than from
resolved_ids.AlterSourceStatement::source_nameis anUnresolvedItemName, so nameresolution never records it and a
resolved_ids-based check is silently a no-opfor
ALTER SOURCE. Resolution mirrors purification exactly, so the check gatesthe item that would be dialed.
The resolved source id is also added to the purified statement's dependency set.
It was previously absent for
ALTER SOURCE, so a source dropped concurrentlywith off-thread purification passed the validity check and then panicked the
coordinator on the missing catalog entry ("catalog out of sync") during
planning. With the id tracked, the drop is detected and the statement is
repurified, ending in a clean unknown-item error.
Verification
test/sqllogictest/rbac_create_table_from_source.sltgains cases that pin theordering, not just the denial. Each uses a statement whose purification fails
for a non-permission reason, so a permission error can only come from the
pre-purification check:
CREATE TABLE ... FROM SOURCEwith an unresolvable reference: purificationreports whether a reference exists upstream, so without the gate this leaks
reference existence.
SELECTis revoked,pinning that ownership does not stand in for read privileges.
ALTER SOURCE ... ADD SUBSOURCEon a load generator, which purificationrejects with "does not support ALTER SOURCE": an ownership error can only
come from the earlier gate.
Plus: an owner-succeeds case, so the rule gates the caller rather than the
syntax; a schema-
USAGEdenial, pinning the other half of the readrequirement; and pass-through cases for a superuser and for an RBAC-disabled
deployment, each reaching purification's own error on a source they do not
own, pinning that the gate filters requirements the same way
check_plandoes.