From c613a431ec73b41ca432fd7515c788419c94ff92 Mon Sep 17 00:00:00 2001 From: tabversion Date: Wed, 6 May 2026 14:37:36 +0800 Subject: [PATCH] fix(frontend): harden show object visibility Make SHOW object lists and known-name metadata probes honor object visibility, and redact hidden secret refs in connection output. Move connector-backed SHOW visibility coverage into source/sink/connection e2e suites so the DDL metadata-only test does not depend on external schema registry validation or madsim secret materialization. Constraint: User requested the approved SHOW object visibility hardening plan. Confidence: high Scope-risk: narrow Directive: Avoid leaking unauthorized object and secret names through SHOW commands. Tested: git diff --check -- e2e_test/ddl/show_object_privilege.slt e2e_test/source_inline/connection/schema_registry.slt e2e_test/source_inline/kafka/secret_dep.slt e2e_test/sink/blackhole_sink.slt Tested: cargo fmt --all -- --check Tested: cargo check -p risingwave_frontend Not-tested: local SLT because no RisingWave server is running on 127.0.0.1:4566. Co-authored-by: OmX --- e2e_test/ddl/show_object_privilege.slt | 85 +++++++++++++++++++ e2e_test/sink/blackhole_sink.slt | 41 ++++++++- .../connection/schema_registry.slt | 44 ++++++++++ e2e_test/source_inline/kafka/secret_dep.slt | 48 +++++++++++ .../rw_catalog/rw_connections.rs | 14 +-- src/frontend/src/handler/create_connection.rs | 43 +++++++++- src/frontend/src/handler/show.rs | 63 ++++++++++---- 7 files changed, 311 insertions(+), 27 deletions(-) create mode 100644 e2e_test/ddl/show_object_privilege.slt diff --git a/e2e_test/ddl/show_object_privilege.slt b/e2e_test/ddl/show_object_privilege.slt new file mode 100644 index 0000000000000..e29dc6aac0e15 --- /dev/null +++ b/e2e_test/ddl/show_object_privilege.slt @@ -0,0 +1,85 @@ +# Verify SHOW object-list commands don't expose objects that are only schema-visible. + +statement ok +CREATE USER show_acl_user WITH PASSWORD 'password'; + +statement ok +CREATE SCHEMA show_acl; + +statement ok +GRANT USAGE ON SCHEMA show_acl TO show_acl_user; + +statement ok +CREATE TABLE show_acl.hidden_table (v1 int, v2 varchar); + +statement ok +CREATE INDEX hidden_idx ON show_acl.hidden_table (v1); + +statement ok +CREATE MATERIALIZED VIEW show_acl.hidden_mv AS SELECT v1 FROM show_acl.hidden_table; + +statement ok +CREATE VIEW show_acl.hidden_view AS SELECT v1 FROM show_acl.hidden_table; + +statement ok +CREATE SECRET show_acl.hidden_secret WITH (backend = 'meta') AS 'http://example.invalid'; + +# Schema USAGE alone should not reveal object names. +system ok +bash -c 'set -euo pipefail; out=$(PGPASSWORD=password psql -U show_acl_user -d dev -Atc "SHOW SECRETS FROM show_acl;"); ! grep -q "hidden_secret" <<<"$out"' + +system ok +bash -c 'set -euo pipefail; out=$(PGPASSWORD=password psql -U show_acl_user -d dev -Atc "SHOW SECRETS FROM show_acl LIKE '\''hidden_secret'\'';"); ! grep -q "hidden_secret" <<<"$out"' + +system ok +bash -c 'set -euo pipefail; out=$(PGPASSWORD=password psql -U show_acl_user -d dev -Atc "SHOW TABLES FROM show_acl;"); ! grep -q "hidden_table" <<<"$out"' + +system ok +bash -c 'set -euo pipefail; out=$(PGPASSWORD=password psql -U show_acl_user -d dev -Atc "SHOW VIEWS FROM show_acl;"); ! grep -q "hidden_view" <<<"$out"' + +system ok +bash -c 'set -euo pipefail; out=$(PGPASSWORD=password psql -U show_acl_user -d dev -Atc "SHOW MATERIALIZED VIEWS FROM show_acl;"); ! grep -q "hidden_mv" <<<"$out"' + +system ok +bash -c 'set -euo pipefail; out=$(PGPASSWORD=password psql -U show_acl_user -d dev -Atc "SHOW INTERNAL TABLES FROM show_acl;"); test -z "$out"' + +# Known-name probes should not return metadata for hidden tables. +system ok +bash -c 'set -euo pipefail; out=$(PGPASSWORD=password psql -U show_acl_user -d dev -Atc "SHOW COLUMNS FROM show_acl.hidden_table;" 2>&1 || true); ! grep -q "v1.*integer" <<<"$out"' + +system ok +bash -c 'set -euo pipefail; out=$(PGPASSWORD=password psql -U show_acl_user -d dev -Atc "SHOW INDEXES FROM show_acl.hidden_table;" 2>&1 || true); ! grep -q "hidden_idx" <<<"$out"' + +# Once the object itself becomes visible, SHOW should include it. +statement ok +GRANT USAGE ON SECRET show_acl.hidden_secret TO show_acl_user; + +system ok +bash -c 'set -euo pipefail; out=$(PGPASSWORD=password psql -U show_acl_user -d dev -Atc "SHOW SECRETS FROM show_acl;"); grep -q "show_acl.hidden_secret" <<<"$out"' + +statement ok +GRANT SELECT ON TABLE show_acl.hidden_table TO show_acl_user; + +system ok +bash -c 'set -euo pipefail; out=$(PGPASSWORD=password psql -U show_acl_user -d dev -Atc "SHOW TABLES FROM show_acl;"); grep -q "show_acl.hidden_table" <<<"$out"' + +system ok +bash -c 'set -euo pipefail; out=$(PGPASSWORD=password psql -U show_acl_user -d dev -Atc "SHOW COLUMNS FROM show_acl.hidden_table;"); grep -q "v1.*integer" <<<"$out"' + +system ok +bash -c 'set -euo pipefail; out=$(PGPASSWORD=password psql -U show_acl_user -d dev -Atc "SHOW INDEXES FROM show_acl.hidden_table;"); grep -q "hidden_idx" <<<"$out"' + +statement ok +GRANT SELECT ON VIEW show_acl.hidden_view TO show_acl_user; + +system ok +bash -c 'set -euo pipefail; out=$(PGPASSWORD=password psql -U show_acl_user -d dev -Atc "SHOW VIEWS FROM show_acl;"); grep -q "show_acl.hidden_view" <<<"$out"' + +statement ok +DROP SECRET show_acl.hidden_secret; + +statement ok +DROP SCHEMA show_acl CASCADE; + +statement ok +DROP USER show_acl_user; diff --git a/e2e_test/sink/blackhole_sink.slt b/e2e_test/sink/blackhole_sink.slt index 01b744a37b8c7..def835d6f47a5 100644 --- a/e2e_test/sink/blackhole_sink.slt +++ b/e2e_test/sink/blackhole_sink.slt @@ -36,4 +36,43 @@ statement ok DROP TABLE t5; statement ok -FLUSH; \ No newline at end of file +FLUSH; + +# Verify SHOW SINKS visibility in the sink e2e environment. +statement ok +CREATE USER show_acl_sink_user WITH PASSWORD 'password'; + +statement ok +CREATE SCHEMA show_acl_sink; + +statement ok +GRANT USAGE ON SCHEMA show_acl_sink TO show_acl_sink_user; + +statement ok +CREATE TABLE show_acl_sink.hidden_table (v1 int primary key, v2 int); + +statement ok +CREATE SINK show_acl_sink.hidden_sink AS SELECT * FROM show_acl_sink.hidden_table WITH ( + connector = 'blackhole' +); + +system ok +bash -c 'set -euo pipefail; out=$(PGPASSWORD=password psql -U show_acl_sink_user -d dev -Atc "SHOW SINKS FROM show_acl_sink;"); ! grep -q "hidden_sink" <<<"$out"' + +statement ok +GRANT SELECT ON SINK show_acl_sink.hidden_sink TO show_acl_sink_user; + +system ok +bash -c 'set -euo pipefail; out=$(PGPASSWORD=password psql -U show_acl_sink_user -d dev -Atc "SHOW SINKS FROM show_acl_sink;"); grep -q "hidden_sink" <<<"$out"' + +statement ok +DROP SINK show_acl_sink.hidden_sink; + +statement ok +DROP TABLE show_acl_sink.hidden_table; + +statement ok +DROP SCHEMA show_acl_sink; + +statement ok +DROP USER show_acl_sink_user; diff --git a/e2e_test/source_inline/connection/schema_registry.slt b/e2e_test/source_inline/connection/schema_registry.slt index e97f868cfec88..ddfd4e1f7858e 100644 --- a/e2e_test/source_inline/connection/schema_registry.slt +++ b/e2e_test/source_inline/connection/schema_registry.slt @@ -53,4 +53,48 @@ drop table t; statement ok drop connection schema_registry_conn; +# Verify connection SHOW visibility and secret-ref redaction in the schema-registry docker environment. +statement ok +CREATE USER show_acl_conn_user WITH PASSWORD 'password'; + +statement ok +CREATE SCHEMA show_acl_conn; + +statement ok +GRANT USAGE ON SCHEMA show_acl_conn TO show_acl_conn_user; + +statement ok +CREATE SECRET show_acl_conn.hidden_registry WITH (backend = 'meta') AS '${RISEDEV_SCHEMA_REGISTRY_URL}'; + +statement ok +CREATE CONNECTION show_acl_conn.visible_conn WITH ( + type = 'schema_registry', + schema.registry = secret show_acl_conn.hidden_registry +); + +system ok +bash -c 'set -euo pipefail; out=$(PGPASSWORD=password psql -U show_acl_conn_user -d dev -Atc "SHOW CONNECTIONS FROM show_acl_conn;"); ! grep -q "visible_conn" <<<"$out"' + +statement ok +GRANT USAGE ON CONNECTION show_acl_conn.visible_conn TO show_acl_conn_user; +system ok +bash -c 'set -euo pipefail; out=$(PGPASSWORD=password psql -U show_acl_conn_user -d dev -Atc "SHOW CONNECTIONS FROM show_acl_conn;"); grep -q "visible_conn" <<<"$out"; grep -q "" <<<"$out"; ! grep -q "hidden_registry" <<<"$out"' + +statement ok +GRANT USAGE ON SECRET show_acl_conn.hidden_registry TO show_acl_conn_user; + +system ok +bash -c 'set -euo pipefail; out=$(PGPASSWORD=password psql -U show_acl_conn_user -d dev -Atc "SHOW CONNECTIONS FROM show_acl_conn;"); grep -q "hidden_registry" <<<"$out"' + +statement ok +DROP CONNECTION show_acl_conn.visible_conn; + +statement ok +DROP SECRET show_acl_conn.hidden_registry; + +statement ok +DROP SCHEMA show_acl_conn; + +statement ok +DROP USER show_acl_conn_user; diff --git a/e2e_test/source_inline/kafka/secret_dep.slt b/e2e_test/source_inline/kafka/secret_dep.slt index 8b3b31441e056..605519381fe58 100644 --- a/e2e_test/source_inline/kafka/secret_dep.slt +++ b/e2e_test/source_inline/kafka/secret_dep.slt @@ -59,3 +59,51 @@ drop secret test_secret_dep_schema.sec_1; statement ok set streaming_use_shared_source to true; + +# Verify SHOW SOURCES visibility in the Kafka docker environment. +system ok +rpk topic create show_acl_source_topic -p 1 || true + +statement ok +CREATE USER show_acl_source_user WITH PASSWORD 'password'; + +statement ok +CREATE SCHEMA show_acl_source; + +statement ok +GRANT USAGE ON SCHEMA show_acl_source TO show_acl_source_user; + +statement ok +CREATE SECRET show_acl_source.hidden_topic WITH (backend = 'meta') AS 'show_acl_source_topic'; + +statement ok +CREATE SOURCE show_acl_source.hidden_source(x varchar) +WITH( + ${RISEDEV_KAFKA_WITH_OPTIONS_COMMON}, + topic = secret show_acl_source.hidden_topic, + scan.startup.mode = 'earliest' +) FORMAT PLAIN ENCODE JSON; + +system ok +bash -c 'set -euo pipefail; out=$(PGPASSWORD=password psql -U show_acl_source_user -d dev -Atc "SHOW SOURCES FROM show_acl_source;"); ! grep -q "hidden_source" <<<"$out"' + +statement ok +GRANT SELECT ON SOURCE show_acl_source.hidden_source TO show_acl_source_user; + +system ok +bash -c 'set -euo pipefail; out=$(PGPASSWORD=password psql -U show_acl_source_user -d dev -Atc "SHOW SOURCES FROM show_acl_source;"); grep -q "hidden_source" <<<"$out"' + +statement ok +DROP SOURCE show_acl_source.hidden_source; + +statement ok +DROP SECRET show_acl_source.hidden_topic; + +statement ok +DROP SCHEMA show_acl_source; + +statement ok +DROP USER show_acl_source_user; + +system ok +rpk topic delete show_acl_source_topic || true; diff --git a/src/frontend/src/catalog/system_catalog/rw_catalog/rw_connections.rs b/src/frontend/src/catalog/system_catalog/rw_catalog/rw_connections.rs index f00e8c81e6c50..c8e1db9c55718 100644 --- a/src/frontend/src/catalog/system_catalog/rw_catalog/rw_connections.rs +++ b/src/frontend/src/catalog/system_catalog/rw_catalog/rw_connections.rs @@ -18,7 +18,7 @@ use risingwave_frontend_macro::system_catalog; use crate::catalog::system_catalog::{SysCatalogReaderImpl, get_acl_items}; use crate::error::Result; -use crate::handler::create_connection::print_connection_params; +use crate::handler::create_connection::print_connection_params_with_secret_visibility; #[derive(Fields)] struct RwConnection { @@ -64,11 +64,13 @@ fn read_rw_connections(reader: &SysCatalogReaderImpl) -> Result { - rw_connection.connection_params = print_connection_params( - &reader.auth_context.database, - params, - &catalog_reader, - ); + rw_connection.connection_params = + print_connection_params_with_secret_visibility( + &reader.auth_context.database, + params, + &catalog_reader, + current_user, + ); } }; diff --git a/src/frontend/src/handler/create_connection.rs b/src/frontend/src/handler/create_connection.rs index 43575e2b03187..5cb944bcc0501 100644 --- a/src/frontend/src/handler/create_connection.rs +++ b/src/frontend/src/handler/create_connection.rs @@ -37,6 +37,8 @@ use crate::error::ErrorCode::ProtocolError; use crate::error::{ErrorCode, Result, RwError}; use crate::handler::HandlerArgs; use crate::session::SessionImpl; +use crate::user::has_access_to_object; +use crate::user::user_catalog::UserCatalog; use crate::utils::{resolve_privatelink_in_with_option, resolve_secret_ref_in_with_options}; pub(crate) const CONNECTION_TYPE_PROP: &str = "type"; @@ -153,17 +155,50 @@ pub fn print_connection_params( db_name: &str, params: &PbConnectionParams, catalog_reader: &CatalogReadGuard, +) -> String { + print_connection_params_impl(db_name, params, catalog_reader, None) +} + +pub fn print_connection_params_with_secret_visibility( + db_name: &str, + params: &PbConnectionParams, + catalog_reader: &CatalogReadGuard, + current_user: &UserCatalog, +) -> String { + print_connection_params_impl(db_name, params, catalog_reader, Some(current_user)) +} + +fn print_connection_params_impl( + db_name: &str, + params: &PbConnectionParams, + catalog_reader: &CatalogReadGuard, + current_user: Option<&UserCatalog>, ) -> String { let print_secret_ref = |secret_ref: &SecretRef| -> String { - // the lookup across all schemas in the database but should guarantee the secret exists - let (schema_name, secret_name) = catalog_reader - .find_schema_secret_by_secret_id(db_name, SecretId::from(secret_ref.secret_id)) - .unwrap(); let maybe_print_as = match secret_ref.get_ref_as().unwrap() { RefAsType::Text => "", RefAsType::File => " AS FILE", RefAsType::Unspecified => "", }; + + let secret_id = SecretId::from(secret_ref.secret_id); + let (schema_name, secret) = catalog_reader + .iter_schemas(db_name) + .unwrap() + .find_map(|schema| { + schema + .get_secret_by_id(secret_id) + .map(|secret| (schema.name.clone(), secret.clone())) + }) + .unwrap(); + + if let Some(current_user) = current_user + && !has_access_to_object(current_user, secret.id, secret.owner) + { + return format!("SECRET {}", maybe_print_as); + } + + let secret_name = secret.name.clone(); format!("SECRET {}.{}{}", schema_name, secret_name, maybe_print_as,) }; let deref_secrets = params diff --git a/src/frontend/src/handler/show.rs b/src/frontend/src/handler/show.rs index a355e6a1eca0e..f34ff9cab4d07 100644 --- a/src/frontend/src/handler/show.rs +++ b/src/frontend/src/handler/show.rs @@ -46,7 +46,7 @@ use crate::catalog::schema_catalog::SchemaCatalog; use crate::catalog::{CatalogError, IndexCatalog}; use crate::error::{Result, RwError}; use crate::handler::HandlerArgs; -use crate::handler::create_connection::print_connection_params; +use crate::handler::create_connection::print_connection_params_with_secret_visibility; use crate::session::{SessionImpl, WorkerProcessId}; use crate::user::user_catalog::UserCatalog; use crate::user::{has_access_to_object, has_schema_usage_privilege}; @@ -55,7 +55,7 @@ pub fn get_columns_from_table( session: &SessionImpl, table_name: ObjectName, ) -> Result> { - let mut binder = Binder::new_for_system(session); + let mut binder = Binder::new_for_batch(session); let relation = binder.bind_relation_by_name(&table_name, None, None, false)?; let column_catalogs = match relation { Relation::Source(s) => s.catalog.columns, @@ -75,6 +75,13 @@ pub fn get_columns_from_sink( ) -> Result> { let binder = Binder::new_for_system(session); let sink = binder.bind_sink_by_name(sink_name)?; + let user_reader = session.env().user_info_reader().read_guard(); + let current_user = user_reader + .get_user_by_name(&session.user_name()) + .expect("user not found"); + if !has_access_to_object(current_user, sink.sink_catalog.id, sink.sink_catalog.owner) { + return Err(CatalogError::not_found("sink", sink.sink_catalog.name.clone()).into()); + } Ok(sink.sink_catalog.full_columns().to_vec()) } @@ -84,6 +91,15 @@ pub fn get_columns_from_view( ) -> Result> { let binder = Binder::new_for_system(session); let view = binder.bind_view_by_name(view_name)?; + let user_reader = session.env().user_info_reader().read_guard(); + let current_user = user_reader + .get_user_by_name(&session.user_name()) + .expect("user not found"); + if !view.view_catalog.is_system_view() + && !has_access_to_object(current_user, view.view_catalog.id, view.view_catalog.owner) + { + return Err(CatalogError::not_found("view", view.view_catalog.name.clone()).into()); + } Ok(view .view_catalog @@ -101,7 +117,7 @@ pub fn get_indexes_from_table( session: &SessionImpl, table_name: ObjectName, ) -> Result>> { - let mut binder = Binder::new_for_system(session); + let mut binder = Binder::new_for_batch(session); let relation = binder.bind_relation_by_name(&table_name, None, None, false)?; let indexes = match relation { Relation::BaseTable(t) => t.table_indexes, @@ -504,7 +520,7 @@ pub async fn handle_show_object( let (reader, current_user) = get_catalog_reader(); iter_schema_items(&session, &schema, &reader, ¤t_user, |schema| { schema - .iter_user_table() + .iter_user_table_with_acl(¤t_user) .map(|t| with_schema_name(&schema.name, &t.name)) .map(|name| ShowObjectRow { name }) .collect() @@ -514,7 +530,7 @@ pub async fn handle_show_object( let (reader, current_user) = get_catalog_reader(); iter_schema_items(&session, &schema, &reader, ¤t_user, |schema| { schema - .iter_internal_table() + .iter_internal_table_with_acl(¤t_user) .map(|t| with_schema_name(&schema.name, &t.name)) .map(|name| ShowObjectRow { name }) .collect() @@ -544,7 +560,7 @@ pub async fn handle_show_object( let (reader, current_user) = get_catalog_reader(); iter_schema_items(&session, &schema, &reader, ¤t_user, |schema| { schema - .iter_view() + .iter_view_with_acl(¤t_user) .map(|t| with_schema_name(&schema.name, &t.name)) .map(|name| ShowObjectRow { name }) .collect() @@ -554,7 +570,7 @@ pub async fn handle_show_object( let (reader, current_user) = get_catalog_reader(); iter_schema_items(&session, &schema, &reader, ¤t_user, |schema| { schema - .iter_created_mvs() + .iter_created_mvs_with_acl(¤t_user) .map(|t| with_schema_name(&schema.name, &t.name)) .map(|name| ShowObjectRow { name }) .collect() @@ -565,7 +581,7 @@ pub async fn handle_show_object( let mut sources = iter_schema_items(&session, &schema, &reader, ¤t_user, |schema| { schema - .iter_source() + .iter_source_with_acl(¤t_user) .map(|t| with_schema_name(&schema.name, &t.name)) .map(|name| ShowObjectRow { name }) .collect() @@ -584,7 +600,7 @@ pub async fn handle_show_object( let (reader, current_user) = get_catalog_reader(); iter_schema_items(&session, &schema, &reader, ¤t_user, |schema| { schema - .iter_sink() + .iter_sink_with_acl(¤t_user) .map(|t| with_schema_name(&schema.name, &t.name)) .map(|name| ShowObjectRow { name }) .collect() @@ -594,7 +610,7 @@ pub async fn handle_show_object( let (reader, current_user) = get_catalog_reader(); let rows = iter_schema_items(&session, &schema, &reader, ¤t_user, |schema| { schema - .iter_subscription() + .iter_subscription_with_acl(¤t_user) .map(|t| ShowSubscriptionRow { name: with_schema_name(&schema.name, &t.name), retention_seconds: t.retention_seconds as i64, @@ -609,7 +625,7 @@ pub async fn handle_show_object( let (reader, current_user) = get_catalog_reader(); iter_schema_items(&session, &schema, &reader, ¤t_user, |schema| { schema - .iter_secret() + .iter_secret_with_acl(¤t_user) .map(|t| with_schema_name(&schema.name, &t.name)) .map(|name| ShowObjectRow { name }) .collect() @@ -641,7 +657,7 @@ pub async fn handle_show_object( ShowObject::Connection { schema } => { let (reader, current_user) = get_catalog_reader(); let rows = iter_schema_items(&session, &schema, &reader, ¤t_user, |schema| { - schema.iter_connections() + schema.iter_connections_with_acl(¤t_user) .map(|c| { let name = c.name.clone(); let r#type = match &c.info { @@ -657,13 +673,23 @@ pub async fn handle_show_object( .get_source_ids_by_connection(c.id) .unwrap_or_default() .into_iter() - .filter_map(|sid| schema.get_source_by_id(sid).map(|catalog| catalog.name.as_str())) + .filter_map(|sid| { + schema.get_source_by_id(sid).and_then(|catalog| { + has_access_to_object(¤t_user, catalog.id, catalog.owner) + .then_some(catalog.name.as_str()) + }) + }) .collect_vec(); let sink_names = schema .get_sink_ids_by_connection(c.id) .unwrap_or_default() .into_iter() - .filter_map(|sid| schema.get_sink_by_id(sid).map(|catalog| catalog.name.as_str())) + .filter_map(|sid| { + schema.get_sink_by_id(sid).and_then(|catalog| { + has_access_to_object(¤t_user, catalog.id, catalog.owner) + .then_some(catalog.name.as_str()) + }) + }) .collect_vec(); let properties = match &c.info { #[expect(deprecated)] @@ -680,7 +706,12 @@ pub async fn handle_show_object( } connection::Info::ConnectionParams(params) => { // todo: show dep relations - print_connection_params(&session.database(), params, &reader) + print_connection_params_with_secret_visibility( + &session.database(), + params, + &reader, + ¤t_user, + ) } }; ShowConnectionRow { @@ -698,7 +729,7 @@ pub async fn handle_show_object( let (reader, current_user) = get_catalog_reader(); let rows = iter_schema_items(&session, &schema, &reader, ¤t_user, |schema| { schema - .iter_function() + .iter_function_with_acl(¤t_user) .map(|t| ShowFunctionRow { name: with_schema_name(&schema.name, &t.name), arguments: t.arg_types.iter().map(|t| t.to_string()).join(", "),