sql: search-path-correct pg_get_viewdef and upstream search_path pin

[def-]

Motivation

Part of SQL-450.

A review of the SQL builtin catalog and the PostgreSQL source utilities surfaced
two name-resolution hazards. This PR addresses two of them; a third (the
mz_connection_oid / mz_secret_oid helpers) is left for a focused follow-up.

Description

1. Search-path-correct pg_get_viewdef (src/sql/src/func.rs).

The text overloads matched the bare view name against mz_views across every
schema of every database. That threw more than one record produced in subquery whenever two views shared a name anywhere in the environment, resolved
views in other databases, and rejected schema-qualified names. They now resolve
through the search-path-aware text -> regclass cast, matching PostgreSQL's
regclass semantics (current database, search path, first match). One behavior
change: pg_get_viewdef('missing') now errors like PostgreSQL instead of
returning NULL. The OID overloads were already correct and are unchanged.

2. Pin search_path on upstream PostgreSQL connections (src/postgres-util/src/tunnel.rs).

Our source-metadata queries reference pg_catalog relations and functions by
unqualified name. An upstream role default such as
ALTER ROLE mz_user SET search_path = attacker, pg_catalog could shadow them
with attacker-owned relations or functions, letting a hostile upstream feed
Materialize forged schema, publication, or RLS-policy rows. Rather than
qualifying every reference by hand (brittle, easy to miss one), pin
search_path=pg_catalog via the connection startup options at the single
connection chokepoint. The startup-options GUC source PGC_S_CLIENT outranks
the PGC_S_USER and PGC_S_DATABASE defaults set by ALTER ROLE /
ALTER DATABASE, so it holds even against a hostile per-role default, and it
covers every upstream connection (normal and replication) and every current and
future query. Data queries already reference user tables by fully-qualified
name, so ingestion is unaffected.

Not in this PR

The mz_connection_oid / mz_secret_oid helpers (reached from
has_connection_privilege / has_secret_privilege) have the same bare-name
global-resolution issue as pg_get_viewdef, but a correct fix requires inlining
current-database + search-path resolution because those object types have no OID
alias to reuse the existing resolver. That is a larger, standalone change and
will land in a follow-up so it can be reviewed on its own.

Verification

• bin/sqllogictest -- test/sqllogictest/pg_get_viewdef.slt passes (27/27), with
  new regression coverage for duplicate names, schema-qualified names, and
  error-on-missing.
• test/sqllogictest/rbac_mcp_agent.slt updated for the restricted-role error
  message that shifts from mz_views to mz_databases, since regclass
  resolution reaches mz_databases first.
• cargo check -p mz-sql, cargo check -p mz-postgres-util, and bin/fmt clean.
• The upstream connection path (including the pin) is exercised by the pg-cdc
  integration suite in CI, so any regression in ingestion would fail there.

🤖 Generated with Claude Code

[aljoscha]

looks good, I had one inline question where I couldn't immediately see why a test change was needed, would be good to resolve that before merging

[aljoscha]

why did this one change?

[def-]

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

[ggevay]

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.

[ggevay]

I'm looking at this, will have some comments shortly.

[ggevay]

So, one issue is that now the text overloads of pg_get_viewdef become unmaterializable (because the regclass cast expands to mz_resolve_object_name, whose body calls current_database and current_schemas, which are UnmaterializableFunc). This means that if a user currently has an MV or index on it, they'll be in a panic-loop when trying to upgrade. (I've confirmed this locally.)

I'm leaning towards just accepting this risk, because it seems unlikely that anybody has an index or MV with pg_get_viewdef. But we might want to at least check whether any cloud users have this (e.g. by doing the "run a SQL command at all users" runbook). What do you think?

[ggevay]

Some more comments from Claude. About 3., we might want to ask @MaterializeInc/sources-sinks team whether a Postgres source connecting though something like pgbouncer is a thing that we want to support. If we don't want to support that, then 3. would be a non-issue.

2. Temporary views break, and can silently return the wrong definition

src/sql/src/func.rs:2707

Temp views get rows in mz_views (StateUpdateKind::TemporaryItem in
src/adapter/src/catalog/apply.rs:1533 feeds pack_item_update, and
pack_view_update packs an MZ_VIEWS row unconditionally), so the old
WHERE name = $1 matched them. The new path resolves through
mz_internal.mz_resolve_object_name, which uses
pg_catalog.current_schemas(true), and that is evaluated via
state.effective_search_path(&search_path, false)
(src/adapter/src/optimize/dataflows.rs:626-628), i.e. with
include_temp_schema=false. The temp schema is never searched.

Consequences:

• CREATE TEMPORARY VIEW v AS ...; SELECT pg_get_viewdef('v') now errors
  relation "v" does not exist where it previously returned the definition.
• Worse: if a permanent view named v is on the search path,
  pg_get_viewdef('v') returns the permanent view's definition while
  SELECT * FROM v reads the temp view (FROM-clause resolution uses
  effective_search_path(true), src/adapter/src/catalog.rs:1944-1950).
  The reported definition does not match the relation the name resolves to.
• PostgreSQL resolves temp views here (pg_temp is implicitly on the path),
  so this also diverges from the PG semantics the PR claims to match.

Caveat: the old behavior was only reliable when the name was globally unique
(mz_views spans all databases and other sessions' temp views), so this is a
divergence-from-PG and self-inconsistency finding more than a strict
regression. Worth at least acknowledging as a known gap.

3. The search_path pin can be silently discarded by poolers, and is never verified

src/postgres-util/src/tunnel.rs:206

The pin exists only as the options startup parameter. A pooler/proxy with
ignore_startup_parameters = options (the standard PgBouncer workaround,
officially supported per pgbouncer/pgbouncer#908) accepts the connection and
silently drops the parameter. The session then keeps the attacker-influenceable
role/database default search_path, reinstating exactly the shadowing attack
this change prevents, with no error or log signal. There is no post-connect
verification anywhere (SHOW search_path / current_schemas is never
checked).

Notes on exposure:

• Outright rejection by a strict PgBouncer (unsupported startup parameter)
  is largely pre-existing: src/storage-types/src/connections.rs:2097
  already unconditionally sends an options startup parameter (empty string),
  and tokio-postgres puts it in the startup packet whenever set.
• Replication connections generally cannot traverse PgBouncer/RDS Proxy, so a
  fully pooled source fails fast anyway. The exposed slice is topologies where
  non-replication traffic (metadata, validation, snapshot) crosses a
  parameter-aware middlebox. Pure TCP proxies (HAProxy tcp mode, NLB) pass the
  pin untouched and are unaffected.
• The operator playbook connects the two failure modes: the loud
  unsupported startup parameter error leads admins straight to adding
  options to ignore_startup_parameters, converting a visible outage into a
  silent security bypass.

Suggested hardening, at the same chokepoint in connect_internal:

1. Execute SET search_path = pg_catalog after connect. Travels as a normal
   query, cannot be stripped, and PGC_S_SESSION also outranks role/database
   defaults. (Caveat: does not stick under transaction-mode pooling.)
2. Fail closed: verify once post-connect, e.g.
   SELECT current_schemas(false), and refuse the connection if the pin is
   not in effect. This catches any mechanism that defeated the pin.

Keeping the startup-parameter pin is still worthwhile (it covers walsender
connections where running SET is not always possible).

Btw. no ingestion doc states that Postgres sources need a direct (non-pooled)
connection. The only mention is one bullet in
doc/user/content/ingest-data/postgres/connection-closed.md:202. If this PR
lands as-is, a strict PgBouncer in front of an upstream now fails earlier and
differently (unsupported startup parameter: options at validation). Worth:

• stating the direct-connection requirement in the Postgres source guides
  (especially neon.md, since Neon hands out both direct and -pooler
  connection strings),
• adding the unsupported startup parameter symptom to the troubleshooting
  page with "you are connecting through a pooler; use the direct endpoint" as
  the resolution.

4. Missing-name error carries SQLSTATE 22000, not PostgreSQL's 42P01

src/sql/src/func.rs:2707 — CONFIRMED

The error for an unresolvable name comes from mz_unsafe.mz_error_if_null
inside the cast template (src/sql/src/plan/typeconv.rs:147, which carries a
TODO about the wrong error code at line 154). It evals to
EvalError::IfNullError, which maps to SqlState::DATA_EXCEPTION (22000) in
src/adapter/src/error.rs:466. PostgreSQL raises 42P01 (undefined_table)
for both 'x'::regclass and pg_get_viewdef('x'). Clients that catch
UndefinedTable to probe view existence will not match.

Pre-existing for all text -> reg* casts (e.g. has_table_privilege), but
this PR extends the surface to pg_get_viewdef while advertising PG parity,
and the new slt tests assert only the message text, so the wrong code is
invisible to them. Worth a mention in the PR description at minimum.

5. Materialized views return NULL where PostgreSQL returns the definition

src/sql/src/func.rs:2705 — CONFIRMED, pre-existing

The regclass alias covers materialized-view
(mz_object_oid_alias, src/catalog/src/builtin/mz_internal.rs:3430), so
pg_get_viewdef('my_matview') resolves to an OID, then misses in mz_views
and yields NULL. PostgreSQL documents pg_get_viewdef as working on views and
materialized views (returns the definition); for plain tables it errors
... is not a view where Materialize returns NULL.

The gap predates this PR (the OID overloads share it), but the new comment
claiming PG-matching semantics sits directly above it. Both mz_views and
mz_materialized_views expose oid and definition, so a UNION is a small
fix. At minimum add a TODO or list it next to the
mz_connection_oid/mz_secret_oid deferral in the PR description. No slt
test covers pg_get_viewdef on a materialized view.

6. Raw options append relies on an undocumented cross-crate invariant

src/postgres-util/src/tunnel.rs:203 — PLAUSIBLE, latent

PostgreSQL parses the options startup parameter with whitespace splitting
and backslash escapes (pg_split_opts). An existing options value ending in a
backslash would escape the separating space, merging the trailing token with
-c, silently disabling the pin. Unreachable today: the only producer
(src/storage-types/src/connections.rs:2067-2097) renders typed
u32/Duration values into --flag=<integer> strings. But that invariant
lives in a different crate with nothing tying it to the append site. A
one-line comment at the append site is enough; escaping logic would be
overkill.

7. Cleanup: duplicated bodies and non-idiomatic cast syntax

src/sql/src/func.rs:2705-2715

• The two text overloads carry verbatim-identical multi-line SQL bodies, and
  the Oid-family overloads repeat the mz_views lookup three more times (five
  copies total).
• The file's idiom for this conversion is $1::regclass::oid
  (has_table_privilege, func.rs:2314/2322) rather than
  CAST(CAST($1 AS pg_catalog.regclass) AS pg_catalog.oid). :: and CAST
  produce the same AST node, so this is purely stylistic.
• The file also has a self-delegation idiom (has_table_privilege
  func.rs:2330, has_database_privilege, has_schema_privilege). The text
  overloads could be
  "pg_get_viewdef($1::pg_catalog.regclass::pg_catalog.oid)" and
  "pg_get_viewdef($1::pg_catalog.regclass::pg_catalog.oid, $2)", which
  resolves unambiguously to the Oid overloads and collapses the duplication.