Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions src/postgres-util/src/tunnel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,20 @@ impl Config {
let mut postgres_config = self.inner.clone();
configure(&mut postgres_config);

// Pin the search path to `pg_catalog` for every upstream connection. Our catalog queries
// reference `pg_catalog` relations and functions by unqualified name (and user tables by a
// fully-qualified name), so an upstream role default such as
// `ALTER ROLE mz_user SET search_path = attacker, pg_catalog` must not be able to shadow
// them with attacker-owned relations or functions. The `options` startup parameter carries
// GUC source `PGC_S_CLIENT`, which outranks the `PGC_S_USER`/`PGC_S_DATABASE` defaults set
// by `ALTER ROLE`/`ALTER DATABASE`, so this holds even against a hostile per-role default.
let search_path_option = "-c search_path=pg_catalog";
let options = match postgres_config.get_options() {
Some(existing) => format!("{existing} {search_path_option}"),
None => search_path_option.to_string(),
};
postgres_config.options(options);

let mut tls = mz_tls_util::make_tls(&postgres_config).map_err(|tls_err| match tls_err {
mz_tls_util::TlsError::Generic(e) => PostgresError::Generic(e),
mz_tls_util::TlsError::OpenSsl(e) => PostgresError::PostgresSsl(e),
Expand Down
11 changes: 9 additions & 2 deletions src/sql/src/func.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2697,14 +2697,21 @@ pub static PG_CATALOG_BUILTINS: LazyLock<BTreeMap<&'static str, Func>> = LazyLoc
// pg_get_viewdef returns the (query part of) the given view's definition.
// We currently don't support pretty-printing (the `Bool`/`Int32` parameters).
"pg_get_viewdef" => Scalar {
// The text overloads resolve the name through the search-path-aware
// text -> regclass cast, matching PostgreSQL's regclass semantics
// (current database, search path, first match). Matching on the bare
// `name` instead would resolve across every schema and database, so
// any same-named view anywhere would make the lookup ambiguous.
params!(String) => sql_impl_func(
"(SELECT definition FROM mz_catalog.mz_views WHERE name = $1)"
"(SELECT definition FROM mz_catalog.mz_views
WHERE oid = CAST(CAST($1 AS pg_catalog.regclass) AS pg_catalog.oid))"
) => String, 1640;
params!(Oid) => sql_impl_func(
"(SELECT definition FROM mz_catalog.mz_views WHERE oid = $1)"
) => String, 1641;
params!(String, Bool) => sql_impl_func(
"(SELECT definition FROM mz_catalog.mz_views WHERE name = $1)"
"(SELECT definition FROM mz_catalog.mz_views
WHERE oid = CAST(CAST($1 AS pg_catalog.regclass) AS pg_catalog.oid))"
) => String, 2505;
params!(Oid, Bool) => sql_impl_func(
"(SELECT definition FROM mz_catalog.mz_views WHERE oid = $1)"
Expand Down
50 changes: 41 additions & 9 deletions test/sqllogictest/pg_get_viewdef.slt
Original file line number Diff line number Diff line change
Expand Up @@ -25,16 +25,22 @@ CREATE VIEW t_view AS SELECT t.a, b FROM t

# Test pg_get_viewdef(view_name)

query T
# The text overload resolves through regclass, so an unknown name errors
# (matching PostgreSQL) rather than returning NULL.
query error relation "doesnotexist" does not exist
SELECT pg_get_viewdef('doesnotexist')
----
NULL

query T
SELECT pg_get_viewdef('t_view')
----
SELECT "t"."a", "b" FROM [u1 AS "materialize"."public"."t"];

# Schema-qualified names resolve (previously returned NULL).
query T
SELECT pg_get_viewdef('public.t_view')
----
SELECT "t"."a", "b" FROM [u1 AS "materialize"."public"."t"];

# Test pg_get_viewdef(view_oid)

query T
Expand All @@ -49,15 +55,11 @@ SELECT "t"."a", "b" FROM [u1 AS "materialize"."public"."t"];

# Test pg_get_viewdef(view_name, pretty)

query T
query error relation "doesnotexist" does not exist
SELECT pg_get_viewdef('doesnotexist', true)
----
NULL

query T
query error relation "doesnotexist" does not exist
SELECT pg_get_viewdef('doesnotexist', false)
----
NULL

query T
SELECT pg_get_viewdef('t_view', true)
Expand Down Expand Up @@ -112,3 +114,33 @@ query T
SELECT pg_get_viewdef('t_view'::regclass::oid)
----
SELECT "t2"."a", "b" FROM [u1 AS "materialize"."public"."t2"];

# A same-named view in another schema must not make the text overload ambiguous.
# The name resolves via the search path, not by matching the bare name globally.

statement ok
CREATE SCHEMA other

statement ok
CREATE VIEW other.t_view AS SELECT c FROM t2

query T
SELECT pg_get_viewdef('t_view')
----
SELECT "t2"."a", "b" FROM [u1 AS "materialize"."public"."t2"];

query T
SELECT pg_get_viewdef('other.t_view')
----
SELECT "c" FROM [u1 AS "materialize"."public"."t2"];

statement ok
SET search_path = other

query T
SELECT pg_get_viewdef('t_view')
----
SELECT "c" FROM [u1 AS "materialize"."public"."t2"];

statement ok
RESET search_path
2 changes: 1 addition & 1 deletion test/sqllogictest/rbac_mcp_agent.slt
Original file line number Diff line number Diff line change
Expand Up @@ -765,7 +765,7 @@ DETAIL: Access to system catalog objects is restricted for this role. Contact yo
simple conn=agent_restricted,user=agent
SELECT pg_get_viewdef('next_arrivals');
----
db error: ERROR: access to system object mz_views is restricted
db error: ERROR: access to system object mz_databases is restricted

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.

why did this one change?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Our error evaluation order is quite strange, so no idea!

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.

I guess it's just that the old code was only looking at mz_views, and now we also look at mz_databases as part of the lookup, so now either of these can trip the check.

DETAIL: Access to system catalog objects is restricted for this role. Contact your administrator if you need access.

# obj_description queries system tables; pg_class is the first blocked ID encountered
Expand Down
Loading