Require read privileges for CREATE TABLE ... FROM SOURCE - #38480
Conversation
598170c to
6d29cb0
Compare
6d29cb0 to
1d929f7
Compare
QA LLM Review1. MEDIUM -- Requiring read on every export of the reference lets one tenant's table block all other roles, including the source owner
Because the requirement is the conjunction over all cataloged exports of the reference, a single export sitting in a schema other roles cannot see denies the attach to everyone else, the source owner included, even when they can already read that reference through an export they are authorized for. In the deployment shape the PR description targets (platform team owns sources, app teams attach tables into their own schemas), the first app team to attach a reference locks out every other team and the platform team. Details
Concretely: the platform team owns source That is stricter than the stated goal of requiring "the privileges a direct read of that data requires" — reading The sound criterion is disjunctive rather than conjunctive: ownership of the parent, or 2. MEDIUM --
|
ff68319 to
4254e51
Compare
QA LLM Review1. MEDIUM -- Column-name coverage is not a read-authorization proxy on Kafka sources, where the caller picks the column names
The coverage rule compares column name sets, but Details
Concretely, on the shape in CREATE TABLE app_schema.raw (key, f1, f2) FROM SOURCE avro_source (REFERENCE 'topic')
FORMAT BYTES ENVELOPE NONE INCLUDE HEADERS, PARTITION;The desc is Column names carry meaning on the connectors where they come from the upstream schema, and none on the connectors where the caller supplies them. Restricting the coverage branch to the reference types whose names are purification-derived, and falling through to the ownership requirement otherwise, would keep the rule sound; comparing the planned table's |
fb5c503 to
5c440d0
Compare
|
Thanks Dennis! I address all the comments |
There was a problem hiding this comment.
On reading this change, at a high level, it feel like it's a bit hacky/not principled. Although the motivation is clear, I don't like the implicitness of allowing access based on existing privileges of adjacent/similar looking tables.
Take the following example:
Team B has SELECT on source pg_src and on source table safe.customers, and their dbt run attaches source table public.customers fine on Monday. On Tuesday the platform team drops or narrows safe.customers during their own migration. Team B's identical dbt run now fails with must be owner of SOURCE pg_src.
Nothing else in our RBAC model follows this pattern. For the time being, I think I'd rather make the RBAC policy a bit dumb and either:
- Assert that the table creator must be an owner of the source (means separate teams can't export source tables separate from the platform team. Might be a breaking change)
- Assert that the table creator must also have SELECT privilege on the source, similar to sinks (means sub team A can access everything from sub team B). This would be my pick (also don't forget to update docs!!)
In the longer term, I think something really nice could be PostgreSQL column level SELECT grants where you can grant SELECT on certain columns of a relation. Syntax could look like:
GRANT SELECT (name, department)
ON pg_src (REFERENCE "public"."customers")
TO analyst;
but this would be a larger feature.
|
Thanks Jun. That's a fair point. I'll take a look at this more tonight |
5c440d0 to
1699d8a
Compare
### Motivation `CREATE TABLE ... FROM SOURCE` populates the new table from an existing ingestion, so creating it reads that source's data. #38480 makes the privilege requirements match that, fixing a case where `CREATE` on the destination schema alone was enough to read a source the role had been denied (SQL-655, 2026 penetration test finding H02). The privileges include for `CREATE TABLE` still lists only the schema and type requirements, so it understates what the statement needs. ### Description Add the read requirement to `create-table.md`, the single include used by all five `CREATE TABLE` pages and by the generated privileges appendix: `SELECT` on the source plus `USAGE` on its schema. The note about scope is the part worth reading. The source is the authorization boundary, so `SELECT` on it permits attaching any reference that source ingests, including references with no existing table and columns some existing table omitted. An admin deciding whether to grant it needs that sentence. Split out from #38480 so that fix, which is an urgent security finding, is not gated on a second CODEOWNERS scope. It is accurate to merge this either before or after #38480: before, it documents a requirement that is about to exist; after, it closes a gap where the docs understate what is enforced. ### Verification Prose only. Rendering is unchanged in shape, four bullets where there were three.
|
Thanks @SangJunBak ! Your instability point make a lot of sense. Authorization that depends on which other exports happen to exist means one team narrowing its own table silently breaks another team's unchanged dbt run, with an error naming the source rather than the table that actually changed. That's not a tradeoff worth the finer granularity. |
| # `CREATE TABLE ... FROM SOURCE` must require the same read privileges as | ||
| # reading the source directly. | ||
| # | ||
| # The statement plans to a generic `Plan::CreateTable`, so its RBAC arm has to |
There was a problem hiding this comment.
no need to talk about implementation details here, these will rot
There was a problem hiding this comment.
updated - thanks Ajoscha
| // `CREATE TABLE ... FROM SOURCE` populates the new table from an | ||
| // existing ingestion, so creating it reads that source's data. | ||
| // Require what reading the source requires, the same way | ||
| // `CREATE SINK` requires read on the relation it reads. | ||
| // | ||
| // The source is the authorization boundary: `SELECT` on it permits | ||
| // attaching any reference it ingests. Per-column or per-reference | ||
| // granularity would need real column-level grants rather than being | ||
| // inferred from which exports happen to exist, which would make the | ||
| // outcome depend on unrelated catalog state. |
There was a problem hiding this comment.
Just saying sth like create table ... from sources requires select priviledges on the source should be enough here. The rest is the history of how we arrived at the current setup and noise
There was a problem hiding this comment.
updated. Thanks Aljsocha
…38491) Document the read privileges CREATE TABLE ... FROM SOURCE requires ### Motivation `CREATE TABLE ... FROM SOURCE` populates the new table from an existing ingestion, so creating it reads that source's data. #38480 makes the privilege requirements match that, fixing a case where `CREATE` on the destination schema alone was enough to read a source the role had been denied ([SQL-655](https://linear.app/materializeinc/issue/SQL-655), 2026 penetration test finding H02). The privileges include for `CREATE TABLE` still lists only the schema and type requirements, so it understates what the statement needs. ### Description Add the read requirement to `create-table.md`, the single include used by all five `CREATE TABLE` pages and by the generated privileges appendix: `SELECT` on the source plus `USAGE` on its schema. The note about scope is the part worth reading. The source is the authorization boundary, so `SELECT` on it permits attaching any reference that source ingests, including references with no existing table and columns some existing table omitted. An admin deciding whether to grant it needs that sentence. Split out from #38480 so that fix, which is an urgent security finding, is not gated on a second CODEOWNERS scope. It is accurate to merge this either before or after #38480: before, it documents a requirement that is about to exist; after, it closes a gap where the docs understate what is enforced. ### Verification Prose only. Rendering is unchanged in shape, four bullets where there were three.
| # A reference nothing has exported yet is authorized the same way, so the rule | ||
| # does not change with the shape of the catalog. | ||
|
|
||
| simple conn=mz_system,user=mz_system | ||
| CREATE SOURCE victim_schema.unexported FROM LOAD GENERATOR AUCTION (AS OF 300, UP TO 301); | ||
| ---- | ||
| COMPLETE 0 | ||
|
|
||
| simple conn=attacker,user=attacker | ||
| CREATE TABLE attacker_schema.first_attach FROM SOURCE victim_schema.unexported (REFERENCE "auction"."users"); | ||
| ---- | ||
| db error: ERROR: permission denied for SOURCE "materialize.victim_schema.unexported" | ||
| DETAIL: The 'attacker' role needs SELECT privileges on SOURCE "materialize.victim_schema.unexported" | ||
|
|
||
| simple conn=mz_system,user=mz_system | ||
| GRANT SELECT ON victim_schema.unexported TO attacker; | ||
| ---- | ||
| COMPLETE 0 | ||
|
|
||
| simple conn=attacker,user=attacker | ||
| CREATE TABLE attacker_schema.first_attach FROM SOURCE victim_schema.unexported (REFERENCE "auction"."users"); | ||
| ---- |
There was a problem hiding this comment.
nit: This test feels a bit redundant given we're testing that RBAC privileges of one object shouldn't affect another in the same schema. I'd remove it!
There was a problem hiding this comment.
Thank you! I updated
1699d8a to
1bde35e
Compare
Fixes SQL-655 (2026 Refactor penetration test, finding H02, High). ### Motivation `CREATE TABLE ... FROM SOURCE` bypassed source authorization: it required only `CREATE` on the destination schema, and nothing about the source it reads. Because the new 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 `CREATE TABLE ... FROM SOURCE` now requires `SELECT` on the source and `USAGE` on its schema, the same read privileges `CREATE SINK` requires on the relation it reads. **The source is the authorization boundary.** `SELECT` on a source permits attaching any reference that source ingests, including references that have no table yet, and including columns some existing table omitted. If a reference should not be reachable, it should not be in the source's publication or topic set. Pentest remediation item 2 (additionally requiring `SELECT` on an existing table for the same reference) is intentionally not implemented: making the outcome depend on which other tables happen to exist would let one team's migration revoke another team's ability to run an unchanged statement. Per-column granularity belongs in column-level grants. ### 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. 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. ### Verification New `test/sqllogictest/rbac_create_table_from_source.slt`: a direct read is denied; the attach is denied without `SELECT` on the source; the attach is denied for a role that has `SELECT` on the source but no `CREATE` on the destination schema; it succeeds with both; and a second role with the same grants can attach the same reference after another team has already attached it. 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, and its gate covers secrets, connections and types but not sources. The upstream connection also comes from the source's own definition rather than the statement, so an unauthorized role can still cause an outbound connection to the source's upstream using the source owner's credentials, and use purification errors as an upstream existence and column-name oracle. Shared with `CREATE SOURCE` and `CREATE SINK` rather than introduced here; closing it needs a pre-purification privilege gate. Filed separately.
1bde35e to
bdb9f10
Compare
QA LLM Review1. MEDIUM -- The new
|
I'm aware of this and per my conversation with @jasonhernandez . This can be done as a follow up |
let's keep this open as a follow up, but merge what we have today! |
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 was not detected as invalidating the result. ### 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.
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 was not detected as invalidating the result. ### 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.
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.
Require read privileges for CREATE TABLE ... FROM SOURCE
Fixes SQL-655 (2026 Refactor penetration test, finding H02, High).
Motivation
CREATE TABLE ... FROM SOURCEbypassed source authorization: it required onlyCREATEon the destination schema, and nothing about the source it reads.Because the new table is owned by whoever creates it,
CREATEon any schema arole 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
CREATE TABLE ... FROM SOURCEnow requiresSELECTon the source andUSAGEon its schema, the same read privileges
CREATE SINKrequires on the relationit reads.
The source is the authorization boundary.
SELECTon a source permitsattaching any reference that source ingests, including references that have no
table yet, and including columns some existing table omitted. If a reference
should not be reachable, it should not be in the source's publication or topic
set.
Pentest remediation item 2 (additionally requiring
SELECTon an existing tablefor the same reference) is intentionally not implemented: making the outcome
depend on which other tables happen to exist would let one team's migration
revoke another team's ability to run an unchanged statement. Per-column
granularity belongs in column-level grants.
User-visible effect
This tightens an existing privilege requirement, and
enable_create_table_from_sourceis on by default, so it applies to everyone onupgrade. A role that previously needed only
CREATEon its own schema plusUSAGEon the source's schema now also needsSELECTon the source.Deployments where a platform team owns sources and application teams attach
tables into their own schemas will need those
SELECTgrants added. Theuser-facing privileges include for
CREATE TABLEis updated in a follow-up PR,kept separate so this fix is not gated on a second CODEOWNERS scope.
Verification
New
test/sqllogictest/rbac_create_table_from_source.slt: a direct read isdenied; the attach is denied without
SELECTon the source; the attach isdenied for a role that has
SELECTon the source but noCREATEon thedestination schema; it succeeds with both; and a second role with the same
grants can attach the same reference after another team has already attached it.
Registered in
tests_without_viewsalongside the other RBAC suites, since--auto-index-selectsview-wrapping would change privilege semantics.Residual, not fixed here
Purification runs before authorization, and its gate covers secrets, connections
and types but not sources. The upstream connection also comes from the source's
own definition rather than the statement, so an unauthorized role can still
cause an outbound connection to the source's upstream using the source owner's
credentials, and use purification errors as an upstream existence and
column-name oracle. Shared with
CREATE SOURCEandCREATE SINKrather thanintroduced here; closing it needs a pre-purification privilege gate. Filed
separately.