Security: restrict df.debug_connection() and redact connection info from PG logs

[pinodeca]

Summary

df.debug_connection() exposes the full PostgreSQL connection string used by the background worker, including the superuser role name, host, port, and database. This information should not be accessible to customers in a PaaS deployment.

Additionally, the background worker logs the full connection string to the PostgreSQL log on every startup, which is visible to customers who receive log copies.

Findings

1. df.debug_connection() grants too broad access

The function returns:

postgres://azuresu@127.0.0.1:5432/postgres (schema: duroxide)

This reveals:

• The worker role name (useful for targeted attacks)
• The host/port (confirming network topology)
• The target database name
• The duroxide schema name

Current state: debug_connection() is included in the func_sigs array inside df.grant_usage() (src/lib.rs), meaning every role granted basic pg_durable access can call it. It was never designed for customer use — it is a developer convenience for the extension authors to verify BGW configuration.

Customers already have safe alternatives for debugging:

• df.target_database() — returns only the database name
• df.version() — returns extension version and build timestamp
• df.metrics() — returns system-wide execution statistics
• df.instance_info(id) — returns instance details

Recommendation: Either:

• (a) Remove debug_connection() entirely, or
• (b) Remove it from the func_sigs array in df.grant_usage() so it is never granted to non-superuser roles (treat it like df.http() or the admin helpers)

2. Connection string logged unredacted to PG logs

In src/worker.rs line ~92:

log!(
    "pg_durable: background worker connected to PostgreSQL at {} (schema: {})",
    pg_conn_str,
    DUROXIDE_SCHEMA
);

This emits the full connection string (including the worker role name) to the PostgreSQL log on every BGW start/restart. In our PaaS deployment, customers can obtain copies of PG logs.

Recommendation: Redact the role name from the log message. Log only the host:port/database, or mask the user component. The full connection string is useful for infrastructure owners, so consider a separate infrastructure-only log channel or redact only the sensitive portions.

Context

• Security review item: I-7 in docs/security-review/security-review.md
• Affected files: src/dsl.rs (function), src/worker.rs (log), src/lib.rs (grant_usage func_sigs)
• Severity: Medium
• Branch context: analysis performed on pinodeca/http-mode-guc

[crprashant]

Proposal: reclassify #110 as non-security cleanup and drop df.debug_connection()

After digging into this against the actual code, I'd like to reclassify this from a security finding to non-security cleanup, and remove df.debug_connection() rather than restrict it.

Why this isn't a security issue

df.debug_connection() returns postgres://{worker_role}@{host}:{port}/{database} (schema: duroxide) — no password, no credential. Every field is already obtainable by any unprivileged role through native PostgreSQL channels:

Exposed field	Already obtainable via	Privilege needed
Worker role (azuresu)	SELECT current_setting('pg_durable.worker_role') — GUC registered with GucFlags::default(), world-readable	none
Worker role (azuresu)	SELECT usename FROM pg_stat_activity — usename is visible to non-superusers for all sessions	none
Database	df.target_database() / current_database()	none
Host / port	loopback 127.0.0.1; current_setting('port') / inet_server_addr()	none
duroxide schema	fixed constant	n/a

Because the worker role name leaks through the GUC and pg_stat_activity.usename regardless of this extension, hiding it via debug_connection (or by redacting logs) achieves nothing. This is consistent with existing security-review item I-6 ("worker role … not a secret"). So I-7 / this issue is not a vulnerability — I'll drop the security label and Medium severity.

Why drop it anyway (the real reason)

Not security — least-surface + future-proofing:

• No production dependency. The background worker builds its connection from the internal Rust postgres_connection_string() (worker.rs), not the SQL function. Dropping it changes no runtime behavior.
• Redundant. Every field is reconstructable (table above).
• Future-proofing (the durable reason). It is built from postgres_connection_string(). Today that carries no credential — but if that builder ever gains a password/token, the function (and the startup log) would silently become a real secret leak. Removing it eliminates that latent foot-gun permanently.

What we lose

Practically nothing of unique value — only the one-call convenience of packaging five already-public fields. Binary backward-compat is preserved via a sql = false shim (see change 1 below).

---

What the PR does — now open as #250 (ready for review)

This is implemented in #250, rebased on top of #242 (the df.grant_usage() / df.revoke_usage() simplification, since merged). Actual change set:

1. src/dsl.rs — stop emitting df.debug_connection() as SQL. Rather than deleting the Rust function outright, it's annotated #[pg_extern(sql = false)]: fresh installs get no CREATE FUNCTION, but the underlying C wrapper symbol stays compiled into the .so so pre-0.2.4 schemas keep resolving it (B1 compat) until the customer runs ALTER EXTENSION pg_durable UPDATE.
2. src/lib.rs — retarget the unit test at the production helpers (postgres_connection_string() + backend_duroxide_schema()) instead of the removed wrapper. (No allowlist change — see the Simplify df.grant_usage/df.revoke_usage privilege helpers #242 note below.)
3. sql/pg_durable--0.2.3--0.2.4.sql — add DROP FUNCTION IF EXISTS df.debug_connection(); (default RESTRICT, no CASCADE).
4. docs/security-review/security-review.md — update, not delete (preserve audit trail): I-7 Medium → Info, Status "Resolved — function removed; reclassified non-security"; supersede Recommendation Update to duroxide 0.1.18 and duroxide-pg-opt v0.1.16 #6.
5. docs/security-review/workbook-data.md — remove the df.debug_connection() row from the function-surface inventory.
6. docs/upgrade-testing.md — add a 0.2.3 → 0.2.4 version note for the DROP (covers the retained C symbol + the RESTRICT dependency behavior).
7. CHANGELOG.md — new ### Removed entry under ## [0.2.4].
8. Tests — tests/e2e/sql/18_delegated_grants.sql adds a fresh-install assertion that df.debug_connection() is never created; scripts/test-upgrade.sh adds a B2 test confirming the function is gone after UPDATE and the simplified df.grant_usage() still grants cleanly.

Note on #242: an earlier draft of this plan also removed df.debug_connection() from df.grant_usage()'s func_sigs EXECUTE allowlist (and touched USER_GUIDE.md). #242 has since removed that allowlist entirely, so #250 rebases on top of #242 and needs no grant_usage change — it just drops the function. That's why src/lib.rs now carries only the unit-test change.

On the log-redaction half

Since the worker role name isn't a secret, redacting the BGW startup connection log is cosmetic, not a security fix. I'll either skip it or do it purely for log tidiness — but not bill it as security, so both halves of this issue stay consistent.

---

#250 is open and ready for review. If anyone disagrees with the reclassification or the decision to remove (vs. restrict) the function, please push back here — happy to discuss before it merges.

[pinodeca]

Good analysis and thanks for the simplification by dropping the function!