Skip to content

adapter: authorize source access before purification - #38501

Closed
jasonhernandez wants to merge 2 commits into
MaterializeInc:tonydu/sql-655-create-table-from-source-bypasses-source-authorization-h02from
jasonhernandez:jason/sql-655-purification-source-gate
Closed

adapter: authorize source access before purification#38501
jasonhernandez wants to merge 2 commits into
MaterializeInc:tonydu/sql-655-create-table-from-source-bypasses-source-authorization-h02from
jasonhernandez:jason/sql-655-purification-source-gate

Conversation

@jasonhernandez

@jasonhernandezjasonhernandez commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Stacked on #38480. Review only the top commit until that merges; the base will be retargeted to main afterwards.

Motivation

#38480 closes the plan-time bug in SQL-655 but leaves a residual it calls out: purification runs before planning, and must_spawn_purification only authorizes the secrets, connections, and types in the statement's resolved ids. CREATE TABLE ... FROM SOURCE and ALTER SOURCE take the upstream connection from the named source's own source_desc(), which is not in the resolved ids, so a role the plan would reject can still make Materialize open an outbound connection to the upstream using the source owner's credentials, and read purification errors as an upstream existence and column-name oracle.

CREATE SOURCE ... FROM CONNECTION and CREATE SINK ... INTO are not affected: the connection they use is in their resolved ids and the existing check_usage(CREATE_ITEM_USAGE) call already gates it before purification. No new privilege is needed.

Description

Add rbac::check_purification_source_access and call it next to the existing check_usage in the purification task. For every Source item the statement names it requires:

  • ALTER SOURCE: ownership of the source.
  • every other purified statement: SELECT on the source (with schema USAGE), or ownership.

Each is a necessary condition of the corresponding plan-time requirement (ALTER SOURCE requires ownership; CREATE TABLE ... FROM SOURCE after #38480 requires SELECT on the parent or ownership; CREATE SINK FROM requires read on its input), so no statement the plan would accept is rejected here. Superuser and RBAC-off sessions are unaffected through the usual filter_requirements.

This is a necessary-condition gate, not a duplicate of the plan-time check. The column-coverage rule in #38480 needs the purified schema and stays where it is.

User-visible effect

A role without SELECT on a source now gets a SELECT denial from CREATE TABLE ... FROM SOURCE instead of an ownership denial, and gets it before any upstream I/O. No previously permitted statement becomes denied.

Verification

Extends test/sqllogictest/rbac_create_table_from_source.slt with the one case the gate is observable on: a non-owner with no SELECT on a never-exported source is denied for SELECT rather than for ownership. Confirmed the assertion fails without the command_handler.rs call and passes with it.

Part of SQL-655.

🤖 Generated with Claude Code

tonydu-mzand others added 2 commits August 26, 2026 13:56
Fixes SQL-655 (2026 Refactor penetration test, finding H02, High).
### Motivation
`CREATE TABLE ... FROM SOURCE` bypassed source authorization. The statement
plans to a generic `Plan::CreateTable`, whose RBAC arm required only `CREATE`
on the destination schema and never inspected `table.data_source`. Because the
resulting table is owned by whoever creates it, `CREATE` on any schema a role
controls was enough to read a source that role had been explicitly denied.
The assessment read 4,076 rows from an Auction load generator through an
attached table while direct reads of the same export stayed denied. The
behaviour has been reachable by default since v26.25.0 and was still present in
v26.35.0.
### Description
The `Plan::CreateTable` arm now inspects `table.data_source` and, for an
`IngestionExport`, applies a column-coverage rule: a role may attach a reference
only if it can already read every column the new table would expose. It can if
* it owns the source, or
* it can read an existing export of the reference (an export or subsource whose
`RelationDesc` projects at least the planned columns), requiring `SELECT` on
the source and on that covering export.
When neither holds, the attach opens a new or wider read path to upstream data,
so it requires ownership of the source, matching `ALTER SOURCE ... ADD
SUBSOURCE`.
Column names are only a read proxy where they come from the upstream schema:
postgres, mysql, sql_server, and multi-output load generators reject
caller-supplied names, so equal names mean the same data. Kafka is excluded from
the coverage path and always requires ownership, because the caller picks
`FORMAT`/`ENVELOPE`/`INCLUDE` and renames the resulting columns, so equal names
there can hide raw payload bytes, headers, and partition metadata a covering
export never projected.
Keying on column coverage rather than on the set of exports is what makes this
neither over-strict nor under-strict. A sibling table exporting the same
reference does not lock other roles out, because the requirement is a covering
export the role can read, not read on every export. And an export created with
`EXCLUDE COLUMNS` does not authorize a wider table for the same reference,
because it does not cover the excluded column, so that case falls through to the
ownership requirement.
Because "read via any covering export" is a disjunction the conjunctive
`RbacRequirements` cannot express, the arm evaluates the role's holdings at
requirement-generation time (a small `role_holds_privileges` helper) to pick a
covering export the role can read, and otherwise names one anyway so the denial
points at a `SELECT` grant that would authorize the attach rather than at
ownership. Superusers and RBAC-off deployments are unaffected:
`filter_to_mandatory_requirements` strips `AclMode::SELECT`.
### User-visible effect
This tightens an existing privilege requirement, and
`enable_create_table_from_source` is on by default, so it applies to everyone on
upgrade. A role that previously needed only `CREATE` on its own schema plus
`USAGE` on the source's schema now also needs `SELECT` on the source and on any
already-cataloged export of the reference being attached.
Deployments where a platform team owns sources and application teams attach
tables into their own schemas will need those `SELECT` grants added. The
user-facing privileges include for `CREATE TABLE` is updated in a follow-up PR,
kept separate so this fix is not gated on a second CODEOWNERS scope.
When nothing has yet exported the reference, there is no export and therefore no
existing item whose privileges say who may read it. Attaching it is creating a
new read path to upstream data, so that case requires **ownership** of the
source rather than `SELECT`. This matches `ALTER SOURCE ... ADD SUBSOURCE`, the
old-syntax operation it replaces. `SELECT` would be the wrong gate there: for a
multi-output source the parent carries progress and offset metadata rather than
user rows, so granting it reads as "let them see ingestion progress" and should
not also mean "let them replicate any table in the publication".
### Verification
New `test/sqllogictest/rbac_create_table_from_source.slt` walks the privilege
set one grant at a time: a direct read is denied; the attach is denied; the
attach is still denied after `SELECT` on the parent alone, which is what pins
the export requirement; and it succeeds once the full set is granted, so the
check does not block legitimate attachment. Deleting either half of the
requirement fails a different assertion, so neither passes vacuously.
Registered in `tests_without_views` alongside the other RBAC suites, since
`--auto-index-selects` view-wrapping would change privilege semantics.
### Residual, not fixed here
Purification runs before authorization. `must_spawn_purification` gates it on
`CREATE_ITEM_USAGE`, which covers secrets, connections and types but not
sources, and the upstream connection is taken from the source's own
`source_desc()` rather than from the statement's resolved ids. An unauthorized
role can therefore still cause an outbound connection to the source's upstream
using the source owner's credentials through a connection it holds no `USAGE`
on, and use purification errors as an upstream existence and column-name
oracle. This is shared with `CREATE SOURCE` and `CREATE SINK` rather than
introduced here, and closing it needs a pre-purification privilege gate. Filed
separately.
The column-restricted (`EXCLUDE COLUMNS`) escalation the coverage rule closes is
only reachable on the postgres, mysql, and sql_server connectors, which
sqllogictest cannot drive, so it is covered by the logic but not by an added
assertion. It should get a testdrive case against a network connector. The
load-generator test does cover the parent-plus-export requirement, the
no-existing-export ownership fallback, and that a sibling table does not lock
another role out.
Purification runs before planning. When a statement names an existing
source, purification borrows that source's upstream connection and secrets
to contact the upstream. The pre-purification check only covers the
secrets, connections, and types in the statement's resolved ids, so the
borrowed connection is never authorized against the caller, and a role the
plan would reject can still trigger an outbound connection with the owner's
credentials and read purification errors as an upstream oracle.
Gate purification on the sources a statement names: ownership for ALTER
SOURCE, and SELECT or ownership for every other purified statement. Both
are necessary conditions of the plan-time requirement, so no statement the
plan would accept is rejected.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@jasonhernandez
jasonhernandezforce-pushed the jason/sql-655-purification-source-gate branch from de26cb6 to 7139192CompareAugust 26, 2026 22:21
@jasonhernandez
jasonhernandez marked this pull request as ready for review August 26, 2026 22:24
@jasonhernandez
jasonhernandez requested a review from a team as a code ownerAugust 26, 2026 22:24
@jasonhernandez

Copy link
Copy Markdown
ContributorAuthor

@tonydu-mz I think #38480 is a sufficient immediate fix, but you may want to merge this into it. We could also address this later if there isn't time in this release cycle.

@tonydu-mz
tonydu-mzforce-pushed the tonydu/sql-655-create-table-from-source-bypasses-source-authorization-h02 branch from 5c440d0 to 1699d8aCompareAugust 27, 2026 01:37
@def-

def- commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

QA LLM Review

1. HIGH -- The ALTER SOURCE half of the new purification gate never checks anything

src/sql/src/rbac.rs:425

AlterSourceStatement::source_name is an UnresolvedItemName, not a T::ItemName, so name resolution never records the source in resolved_ids. The Statement::AlterSource arm therefore always builds an empty ownership set and the gate is a no-op, leaving ALTER SOURCE ... REFRESH REFERENCES / ... ADD SUBSOURCE purification reachable by any authenticated role with no privilege on the source at all. That is exactly the outbound-connection-with-the-owner's-credentials and upstream-schema-oracle path the commit message says it closes.

Details

The AST field is pub source_name: UnresolvedItemName (src/sql-parser/src/ast/defs/statement.rs:3152), and NameResolver does not override fold_unresolved_item_name, so nothing is inserted into NameResolver::ids for that field. Both consumers re-resolve the name themselves rather than reading a resolved id: purify_alter_source does scx.resolve_item(RawItemName::Name(unresolved_source_name.clone())) (src/sql/src/pure.rs:1378) and plan_alter_source does resolve_item_or_type(scx, ObjectType::Source, source_name.clone(), if_exists) (src/sql/src/plan/statement/ddl.rs:8016). The rest of an ALTER SOURCE statement contributes no items either. DropSubsources.names, TextColumns, and ExcludeColumns are all UnresolvedItemName, and SetOptions/ResetOptions carry only RETAIN HISTORY / TIMESTAMP INTERVAL scalars. So resolved_ids.items() is empty and sources yields nothing.

Reachability: must_spawn_purification returns true for every ALTER SOURCE except an options-only RETAIN HISTORY change, check_usage over an empty resolved_ids is vacuous, and purify_alter_source_refresh_references (src/sql/src/pure.rs:1665) connects straight to the upstream Postgres/MySQL/SQL Server instance with no feature gate. Name resolution performs no privilege check, so any role that can log in can drive it; the ownership denial only lands afterwards, at plan time, once the network I/O and any error-message schema disclosure have already happened. (ADD SUBSOURCE additionally exposes upstream table and column names through purify_source_exports errors, though it is short-circuited when force_source_table_syntax is set.)

The test added in this commit only covers CREATE TABLE ... FROM SOURCE, whose source field is a T::ItemName, so the working half is exercised and the broken half is not.

Fix: resolve the name in the gate the same way purification does, instead of relying on resolved_ids.

Statement::AlterSource(AlterSourceStatement{ source_name, .. }) => {// `source_name` is an `UnresolvedItemName`, so it never reaches// `resolved_ids`; mirror `purify_alter_source`'s own resolution.// A resolution failure is left to purification/planning to report.ifletOk(partial) = normalize::unresolved_item_name(source_name.clone()){ifletOk(item) = catalog.resolve_item(&partial){if item.item_type() == CatalogItemType::Source{
requirements.ownership = vec![ObjectId::Item(item.id())];}}}}

A regression test needs a network connector (REFRESH REFERENCES on a Postgres source, asserting the ownership denial arrives without the upstream being contacted), so it belongs in testdrive rather than the new .slt.

Sign up for freeto 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.

3 participants

@jasonhernandez@def-@tonydu-mz