Skip to content

feat(auth): authorize query-auth reads and carry the grant on the split - #758

Open
plusplusjiajia wants to merge 1 commit into
apache:mainfrom
plusplusjiajia:query-auth-carry-grant
Open

feat(auth): authorize query-auth reads and carry the grant on the split#758
plusplusjiajia wants to merge 1 commit into
apache:mainfrom
plusplusjiajia:query-auth-carry-grant

Conversation

@plusplusjiajia

@plusplusjiajia plusplusjiajia commented Aug 27, 2026

Copy link
Copy Markdown
Member

Purpose

A query-auth.enabled table makes the server return a per-user row filter and column masking that the client is expected to apply. This client cannot apply them yet, so it refuses to read such a table at all — even for a user the server reports as unrestricted. This slice fetches the authorization at scan-plan time and carries it to the read, so that user can read. A user with rules gets the same refusal as before.

Brief change log

TableScan::plan authorizes once and stamps the result on every split, as Java wraps each split in a QueryAuthSplit. TableRead::to_arrow then decides from the splits: each must carry a grant, from this handle,
and unrestricted — per split, since split lists can be concatenated across plans.

Whether a table is query-auth comes from the server, not the loaded handle: the option can be set after a load, and a cached false would skip authorization entirely. Sync read boundaries cannot ask, so they
read a marker the splits carry. Unlike the grant it survives serialization, so a round-tripped split still demands authorization and, having lost the grant, fails closed.

Four refusals are deliberate. A restricted grant fails at planning, since a plan carries row counts and min/max that engines answer COUNT/MIN/MAX from without reading a row. A time-travelled, branch, or
decorated (db.t$branch_x, db.t$files) handle is refused, since the server rules on the current schema while those read other files. A plan is refused when a data file still carries statistics for a column the
current schema no longer has — value_stats and write_cols are public on every split. And the request names no columns, which the server expands to the real schema fields — Java sends the read type's names
instead. Naming a reserved system column would fail the server's permission check and deny an otherwise authorized user, so a read that reaches one is refused client-side instead; the cost is that a user
authorized for only a subset of columns is still refused, as today.

AuthTableQueryResponse rejects unknown fields, unlike every other response: an absent field reads as "no rule", so protocol drift would look like an unrestricted grant.

@plusplusjiajia
plusplusjiajia force-pushed the query-auth-carry-grant branch 11 times, most recently from e324972 to f67b381 Compare August 30, 2026 12:44
@plusplusjiajia
plusplusjiajia force-pushed the query-auth-carry-grant branch from f67b381 to 96e1832 Compare August 30, 2026 13:13
@plusplusjiajia
plusplusjiajia marked this pull request as ready for review August 30, 2026 14:20

/// Whether the server says this table is `query-auth.enabled` right now: the
/// handle's schema is a snapshot, and a cached `false` would skip the check.
pub(crate) async fn server_query_auth_enabled(&self) -> Result<bool> {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Apply the live server check to direct search APIs too

This helper closes the stale-handle gap for TableScan, but the direct scored/search entry points still call only CoreOptions::ensure_read_authorized() on the schema cached when the handle was loaded. In particular, BatchVectorSearchBuilder::execute reads the snapshot/index manifest directly, and VectorSearchBuilder::execute_scored, FullTextSearchBuilder::execute_scored, and HybridSearchBuilder::execute_scored reach those direct paths without an authorized TableScan.

Therefore: load a REST table while query auth is false, enable restricted query auth on the server, then reuse the handle for one of these searches. The cached guard passes and row IDs/scores derived from protected data are returned without the auth exchange. Please route every out-of-band search entry through this async server-state check and reject when query auth is enabled (these paths cannot apply masking/filtering), with stale-handle regressions analogous to the new scan test.

}
let canonical = schema_fields
.iter()
.any(|f| f.id() == field.id() && f.name() == field.name());

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Validate the full nested field shape, not only the top-level pair

Both this guard and the old-file check above compare only the top-level (id, name). That leaves a concrete disclosure path after nested schema evolution: suppose the authorized current schema contains profile ROW<public>, while a live older file has the same top-level field id/name but profile ROW<public, secret>. A caller can use the public ReadBuilder::with_read_type with that old Row type; this check passes, and data_file_reader::prune_data_type recursively selects the requested old child by id, so profile.secret is decoded even though it is absent from the schema/column set the server authorized. The planning check at lines 87-92 also passes the old file for the same reason.

Please validate canonical fields recursively (including Row children and nested Array/Map/Multiset element types, allowing only explicitly safe evolution), and make the old-file containment check recursive too. An end-to-end test with a dropped nested field and a crafted old read type should be rejected.

core_options.ensure_type_paimon_served(&self.table.identifier().full_name())?;
// The handle's flag is a snapshot; the marker survives a round-trip.
let required = core_options.query_auth_enabled()
|| data_splits.iter().any(|s| s.query_auth_required());

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Do not accept an unmarked split from a stale REST handle

When a REST handle was loaded while query auth was false, core_options.query_auth_enabled() stays false. If the server later enables restricted query auth, a caller that bypasses TableScan can still pass an old unmarked plan—or a split made through the public DataSplitBuilder—to this synchronous to_arrow boundary. required is false, so the method returns rows without any auth RPC or grant. This contradicts the PR’s stated stale-handle guarantee; the new test covers planning after the toggle, but not this public read path.

Please make REST reads require evidence that the split was planned after a live server-state check even when that check said query auth was disabled (for example, a catalog-session-bound checked-plan capability distinct from query_auth_required), or make materialization able to re-authorize. Unmarked splits should remain acceptable only where no REST authorization boundary exists. Add a regression for: load with false, retain/build an unmarked split, enable a restricted response, then call to_arrow and require refusal.

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