diff --git a/CHANGELOG.md b/CHANGELOG.md index b55114f..a0b82a1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,10 @@ Pre-1.0 note: while `pg_durable` is in major version `0`, minor releases may inc - **`df.grant_usage()` / `df.revoke_usage()`:** dropped the explicit per-function `EXECUTE` allowlist. Schema `USAGE` on `df` is the real access gate for ordinary `df.*` functions, so the helpers now grant/revoke schema `USAGE`, the table privileges, and `EXECUTE` only on the sensitive functions (`df.http`, `df.grant_usage`, `df.revoke_usage`). Function signatures are unchanged and existing privileges are unaffected (#242). +### Removed + +- **`df.debug_connection()`:** removed from the SQL surface as non-security, surface-reduction cleanup (#110). The function returned the worker connection string (`postgres://role@host:port/db`) with no password or credential, and the worker role is already visible through native PostgreSQL channels (the world-readable `pg_durable.worker_role` GUC and `pg_stat_activity.usename`) — so issue #110 is reclassified from a security finding to cleanup. Fresh installs no longer create the function and the `0.2.3 → 0.2.4` upgrade drops it; a binary-compatibility shim retains the underlying C symbol so pre-0.2.4 schemas keep resolving the function until `ALTER EXTENSION pg_durable UPDATE` runs. + ## [0.2.3] - 2026-06-17 Provider-line note: v0.2.3 stays in the `duroxide-pg` provider compatibility line started in v0.2.2, so the upgrade source is v0.2.2 (`sql/pg_durable--0.2.2--0.2.3.sql`). diff --git a/docs/security-review/security-review.md b/docs/security-review/security-review.md index db17460..8c5927c 100644 --- a/docs/security-review/security-review.md +++ b/docs/security-review/security-review.md @@ -138,7 +138,7 @@ The extension demonstrates strong security design for its core threat model: | I-4 | **HTTP headers (incl. auth tokens) stored in df.nodes query column**: When df.http() is called with Authorization headers, the full config JSON including credentials is stored in df.nodes. RLS-protected but no encryption. | High | ⛔ Recommend credential separation | | I-5 | **TLS not enforced on PostgreSQL connections**: Wire protocol uses whatever pg_hba.conf specifies. Extension does not enforce TLS. | Medium | ⚠️ Document requirement | | I-6 | **Worker role (GUC) visible via current_setting()**: pg_durable.worker_role is readable by any user | Low | ✅ Acceptable — not a secret | -| I-7 | **df.debug_connection() exposes connection string**: This function returns the duroxide connection string. Should be restricted in production. | Medium | ⚠️ Recommend REVOKE or restrict | +| I-7 | **df.debug_connection() exposed connection string**: This function returned the worker connection string (`postgres://role@host:port/db`) — no password or credential. Reclassified as **non-security**: the worker role is already exposed to any role via native PostgreSQL channels (world-readable `pg_durable.worker_role` GUC and `pg_stat_activity.usename`, per I-6), and the remaining fields (database, host/port, fixed schema) are connection-topology metadata, not credentials — the host is read from `PGHOST` (defaults to loopback). | Info | ✅ Resolved — function removed in v0.2.4 (#110) as surface-reduction cleanup, not a vulnerability | | I-8 | **Superuser bypasses RLS**: By design, superuser sees all users' data. Appropriate for single-tenant. | Info | ✅ Accepted | ### 3.5 Denial of Service @@ -248,7 +248,7 @@ The extension demonstrates strong security design for its core threat model: | 3 | **Parameterize activity SQL**: Convert update_instance_status and update_node_status to use sqlx bind parameters instead of format!() | Low | T-2 | | 4 | **Add statement_timeout to per-user connections**: Set `statement_timeout` on user SQL connections to prevent runaway queries | Low | D-4 | | 5 | **Add rate limiting on df.http()**: Implement `df.max_http_requests_per_instance` or per-user HTTP rate limit | Medium | D-3 | -| 6 | **Restrict df.debug_connection()**: REVOKE EXECUTE from PUBLIC or gate behind superuser check | Low | I-7 | +| 6 | ~~**Restrict df.debug_connection()**: REVOKE EXECUTE from PUBLIC or gate behind superuser check~~ → **Superseded.** Reclassified non-security (see I-7); function **removed** in v0.2.4 (#110) rather than restricted. | Low | I-7 | ### 5.3 Medium Priority — Before GA (P2) diff --git a/docs/security-review/workbook-data.md b/docs/security-review/workbook-data.md index d2e2bd8..1cd57db 100644 --- a/docs/security-review/workbook-data.md +++ b/docs/security-review/workbook-data.md @@ -65,7 +65,6 @@ pg_durable does not use token-based authentication. All identity is PostgreSQL r | `df.join(a, b)` / `df.join3(a, b, c)` | Durofut args | None (in-memory) | N/A | Validates Durofut JSON | Low | | `df.race(a, b)` | Two Durofut args | None (in-memory) | N/A | Validates Durofut JSON | Low | | `df.explain(fut_or_id)` | Durofut JSON or instance ID | PostgreSQL role | SELECT df.nodes (for existing instances) | JSON parse | Low | -| `df.debug_connection()` | None | PostgreSQL role | None | None | **Medium** — exposes connection info | | `df.version()` | None | PostgreSQL role | None | None | Low | | `df.target_database()` | None | PostgreSQL role | None | None | Low | diff --git a/docs/upgrade-testing.md b/docs/upgrade-testing.md index 175dae9..586e359 100644 --- a/docs/upgrade-testing.md +++ b/docs/upgrade-testing.md @@ -172,7 +172,7 @@ Each PR that changes the extension schema or modifies SQL queries in Rust code s **Minor release** (e.g. 0.2.0 → 0.3.0): 1. Create empty `sql/pg_durable----.sql` upgrade script 2. Bump `Cargo.toml` version to `` -3. If this release starts a new provider compatibility line, update the `PROVIDER_COMPAT_START_VERSION` default in `scripts/test-upgrade.sh`, check in an install SQL fixture (`sql/pg_durable--.sql`) at that boundary so reconstruction never chains across it, and document the boundary under "Version-Specific Changes". Downstream forks can instead override `PROVIDER_COMPAT_START_VERSION` in CI to keep the script shared. +3. If this release starts a new provider compatibility line, update the `PROVIDER_COMPAT_START_VERSION` default in `scripts/test-upgrade.sh`, check in an install SQL fixture (`sql/pg_durable--.sql`) at that boundary so reconstruction never chains across it, and document the boundary under "Version-Specific Changes". Downstream forks can instead override `PROVIDER_COMPAT_START_VERSION` in CI to keep the script shared. When you advance `PROVIDER_COMPAT_START_VERSION`, also audit for binary-compatibility shims annotated `#[pg_extern(sql = false)]` (e.g. `df.debug_connection()` in `src/dsl.rs`, retained for #110) and delete any whose C symbol is no longer referenced by a supported schema in the new line. If this is the first minor after a new major (e.g. 1.0.0 → 1.1.0), also: @@ -221,6 +221,14 @@ what the upgrade script handles, and any backward compatibility considerations. - **Scenario B1 considerations:** The new `.so` remains compatible with v0.2.3 schemas that have not run `ALTER EXTENSION UPDATE`: existing catalog entries still bind `df.wait_for_completion` to `wait_for_completion_wrapper`, which is retained as a Rust shim to `df.await_instance`. - **Scenario B2 considerations:** No data migration. Existing instances are unaffected; the upgrade only adds a SQL function binding. +#### #110 Remove df.debug_connection() (reclassified non-security cleanup) +- **DDL change (df schema):** The upgrade script `sql/pg_durable--0.2.3--0.2.4.sql` runs `DROP FUNCTION IF EXISTS df.debug_connection();`. Fresh v0.2.4 installs never create the function: its `#[pg_extern]` in `src/dsl.rs` is annotated `#[pg_extern(sql = false)]`, so pgrx emits no `CREATE FUNCTION` for it (the generated schema records `-- Skipped due to #[pgrx(sql = false)]`). The function returned the worker connection string (no credential) and is dropped as surface-reduction, because the worker role is already exposed to any role via native PostgreSQL channels — the world-readable `pg_durable.worker_role` GUC and `pg_stat_activity.usename` (see security-review item I-6); the remaining fields (database, host/port, schema) are connection-topology metadata, not secrets (the host comes from `PGHOST`, defaulting to loopback). Reclassified from security to cleanup; see issue #110. +- **Interaction with the `df.grant_usage()` simplification:** Earlier in this release `df.grant_usage()` carried `'df.debug_connection()'` in its explicit per-function allowlist (`func_sigs`), so dropping the function would have required editing that allowlist. The grant_usage simplification above (#242) removed the allowlist entirely in this same release, so the upgrade no longer needs any `grant_usage` change to account for the removed function — it simply drops `df.debug_connection()`. +- **Scenario A considerations:** `df.debug_connection()` is absent on both the fresh-install and upgrade paths after this release, keeping the `df` schema shapes equivalent. +- **Scenario B1 considerations (symbol retention):** Removing `df.debug_connection()` from the SQL surface required care to preserve binary backward compatibility. Pre-0.2.4 schemas (0.2.2, 0.2.3) define the function as `AS 'MODULE_PATHNAME','debug_connection_wrapper'`, and PostgreSQL validates that C symbol at `CREATE FUNCTION` time (`check_function_bodies = on` by default). Fully deleting the `#[pg_extern]` would drop the `debug_connection_wrapper` symbol from the `.so`, so the new binary could no longer instantiate any previously shipped schema — failing Scenario B1. The fix is `#[pg_extern(sql = false)]`: pgrx still compiles the C wrapper symbol into the binary (verified with `nm`) but emits no SQL, so old schemas keep resolving the symbol while fresh installs omit the function. The retained Rust body still returns the same non-secret connection string, so a binary-only swap (no `ALTER EXTENSION UPDATE`) leaves any pre-existing `df.debug_connection()` working until the customer upgrades. The shim is a temporary binary-compat detail — remove it once `PROVIDER_COMPAT_START_VERSION` advances past 0.2.3 and no supported schema references the symbol. +- **Scenario B2 considerations:** No data migration. Existing instances, nodes, and vars are untouched. After `ALTER EXTENSION UPDATE`, `df.debug_connection()` no longer exists; the simplified `df.grant_usage()` never references it. +- **Dependent-object note:** The upgrade runs `DROP FUNCTION IF EXISTS df.debug_connection()` with PostgreSQL's default `RESTRICT` behavior. If a customer created their own object that depends on the function (e.g. a view or SQL function that calls it), `ALTER EXTENSION UPDATE` aborts with a dependency error and the customer must drop or repoint that object first. This is intentional for a removed debug helper — the script deliberately does not `CASCADE`, to avoid silently dropping customer-owned objects. The fresh-install (`tests/e2e/sql/18_delegated_grants.sql`) and upgrade (`scripts/test-upgrade.sh` B2 grant test) suites assert the function is absent and that `df.grant_usage()` still works after the drop. + ### v0.2.2 → v0.2.3 #### Rename duroxide provider schema to `_duroxide` for fresh installs diff --git a/scripts/test-upgrade.sh b/scripts/test-upgrade.sh index dfef191..1778543 100755 --- a/scripts/test-upgrade.sh +++ b/scripts/test-upgrade.sh @@ -968,11 +968,42 @@ test_b2_new_data_after_upgrade() { assert_sql_equals "SELECT msg FROM test_upgrade_b2_log WHERE kind = 'post' ORDER BY id DESC LIMIT 1;" "new_value" } +test_b2_grant_usage_after_upgrade() { + # Regression guard for #110: after ALTER EXTENSION UPDATE, df.debug_connection() + # must be gone from the catalog. Scenario A only compares function name/args/ + # result (not bodies), and the other B2 tests never exercise df.grant_usage(), + # so this test independently confirms (a) the upgrade dropped the function and + # (b) the simplified df.grant_usage() (the per-function allowlist was removed in + # #242) still runs cleanly against the upgraded schema and actually grants a + # privilege. + local probe_role="durable_b2_grant_probe" + local out + + # Idempotent pre-clean (DROP OWNED errors harmlessly if the role is absent). + run_sql_capture "DROP OWNED BY ${probe_role};" >/dev/null 2>&1 || true + run_sql_capture "DROP ROLE IF EXISTS ${probe_role};" >/dev/null 2>&1 || true + out=$(run_sql_capture "CREATE ROLE ${probe_role} LOGIN;") || { echo "$out"; return 1; } + + # The dropped function is absent from the catalog after upgrade. + assert_sql_equals "SELECT to_regprocedure('df.debug_connection()') IS NULL;" "t" || return 1 + + # The rewritten grant_usage() runs against the upgraded schema and grants + # schema USAGE (plus the table privileges) without error. + out=$(run_sql_capture "SELECT df.grant_usage('${probe_role}');") || { echo "$out"; return 1; } + + # ... and a representative privilege was actually granted. + assert_sql_equals "SELECT has_function_privilege('${probe_role}', 'df.start(text, text, text)', 'EXECUTE');" "t" || return 1 + + # Clean up the probe role. + run_sql_capture "DROP OWNED BY ${probe_role}; DROP ROLE IF EXISTS ${probe_role};" >/dev/null 2>&1 || true +} + if [ "$HAS_COMPAT_PREV" = true ]; then run_test "B2: Pre-upgrade data survives ALTER EXTENSION UPDATE" test_b2_data_survives_upgrade run_test "B2: Pre-upgrade instance remains queryable" test_b2_pre_upgrade_instance_after_upgrade run_test "B2: In-flight work completes after upgrade" test_b2_inflight_work_after_upgrade run_test "B2: New data and execution after upgrade" test_b2_new_data_after_upgrade + run_test "B2: df.grant_usage() works and df.debug_connection() is gone after upgrade" test_b2_grant_usage_after_upgrade fi # ============================================================================ diff --git a/sql/pg_durable--0.2.3--0.2.4.sql b/sql/pg_durable--0.2.3--0.2.4.sql index fd76eb1..efaab86 100644 --- a/sql/pg_durable--0.2.3--0.2.4.sql +++ b/sql/pg_durable--0.2.3--0.2.4.sql @@ -6,6 +6,28 @@ -- See docs/upgrade-testing.md for the upgrade-script and backward-compatibility -- requirements (Scenario A / B1 / B2). +-- ============================================================================ +-- Remove df.debug_connection() (issue #110, reclassified non-security cleanup). +-- +-- The function returned the worker connection string (postgres://role@host:port/db) +-- — no password or credential. The worker role is already exposed to any role +-- through native PostgreSQL channels (the world-readable pg_durable.worker_role +-- GUC and pg_stat_activity.usename — see security-review item I-6); the remaining +-- fields (database, host/port, schema) are connection-topology metadata, not +-- secrets (the host comes from PGHOST, defaulting to loopback). It is dropped +-- purely to shrink the public function surface and future-proof against the +-- connection builder ever gaining a secret. +-- +-- The background worker builds its connection from the internal Rust helper, not +-- this SQL function, so dropping it changes no runtime behavior. The new .so +-- keeps the underlying C symbol (debug_connection_wrapper) compiled in via a +-- #[pg_extern(sql = false)] shim, so pre-0.2.4 schemas still resolve the function +-- until ALTER EXTENSION UPDATE runs (Scenario B1). df.grant_usage() no longer +-- references this function — its per-function allowlist is removed in this same +-- release (see below) — so the drop needs no further grant_usage change. +-- ============================================================================ +DROP FUNCTION IF EXISTS df.debug_connection(); + -- ============================================================================ -- Simplify df.grant_usage(): drop the explicit per-function allowlist. -- diff --git a/src/dsl.rs b/src/dsl.rs index f314663..62695ab 100644 --- a/src/dsl.rs +++ b/src/dsl.rs @@ -44,8 +44,20 @@ pub fn version() -> String { ) } -/// Debug function to see what duroxide connection is being used -#[pg_extern(schema = "df")] +/// Binary backward-compatibility shim for issue #110. +/// +/// `df.debug_connection()` was removed from the SQL surface in v0.2.4: it is no +/// longer emitted on fresh installs (`sql = false`) and is dropped by the +/// `0.2.3 -> 0.2.4` upgrade script. Pre-0.2.4 schemas, however, still define the +/// function with `AS 'MODULE_PATHNAME','debug_connection_wrapper'`, and +/// PostgreSQL validates that C symbol at `CREATE FUNCTION` time. We therefore +/// keep the wrapper symbol compiled into the binary so the new `.so` can still +/// load every previously shipped schema (upgrade-test Scenario B1). The body +/// intentionally mirrors the old, non-secret output so a binary-only swap +/// (without `ALTER EXTENSION UPDATE`) keeps working until the customer upgrades. +/// +/// Remove once `PROVIDER_COMPAT_START_VERSION` advances past 0.2.3. +#[pg_extern(sql = false)] pub fn debug_connection() -> String { use crate::types::{backend_duroxide_schema, postgres_connection_string}; format!( diff --git a/src/lib.rs b/src/lib.rs index 60f82ec..42727b1 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1355,10 +1355,14 @@ mod tests { } #[pg_test] - fn test_debug_connection_returns_info() { - let conn_info = crate::dsl::debug_connection(); - assert!(!conn_info.is_empty()); - assert!(conn_info.contains("duroxide")); // Should contain schema name + fn test_connection_info_builders() { + use crate::types::{backend_duroxide_schema, postgres_connection_string}; + let conn = postgres_connection_string(); + assert!(!conn.is_empty()); + assert!(conn.contains("postgres://")); + // Fresh installs use the "_duroxide" provider schema; upgraded installs + // use the legacy "duroxide". Both contain "duroxide" as a substring. + assert!(backend_duroxide_schema().contains("duroxide")); } // ======================================================================== diff --git a/tests/e2e/sql/18_delegated_grants.sql b/tests/e2e/sql/18_delegated_grants.sql index 720d40a..0be5547 100644 --- a/tests/e2e/sql/18_delegated_grants.sql +++ b/tests/e2e/sql/18_delegated_grants.sql @@ -385,4 +385,16 @@ BEGIN DROP ROLE dg_http_target; END $cleanup$; +-- === Fresh-install surface check (issue #110): df.debug_connection() removed === +-- v0.2.4 drops df.debug_connection() from the SQL surface. On a fresh install it +-- is never created (its #[pg_extern] is annotated sql = false). Assert it is +-- absent so a future change that re-emits the function is caught here. +DO $surface$ +BEGIN + IF to_regprocedure('df.debug_connection()') IS NOT NULL THEN + RAISE EXCEPTION 'TEST FAILED: df.debug_connection() should not exist on a fresh install (#110)'; + END IF; + RAISE NOTICE 'SURFACE CHECK PASSED: df.debug_connection() absent on fresh install'; +END $surface$; + SELECT 'TEST PASSED: 18_delegated_grants' AS result;