From 5cfebf59d92f51b703c34bf18f9fc5a4aebf6898 Mon Sep 17 00:00:00 2001 From: Pino de Candia <32303022+pinodeca@users.noreply.github.com> Date: Mon, 22 Jun 2026 19:57:17 +0000 Subject: [PATCH] Security: gate df.metrics() behind explicit EXECUTE grants df.metrics() returns system-wide aggregate counts (total instances, running/completed/failed, total executions/events) from the duroxide store without per-user filtering. Previously any role granted access via df.grant_usage() could observe activity patterns across all users (security review Finding 6, issue #126). Rather than hard-coding a superuser-only runtime check, control access with ordinary PostgreSQL function privileges: - Fresh installs REVOKE EXECUTE on df.metrics() FROM PUBLIC. - An ordinary df.grant_usage() does not grant df.metrics(). - df.grant_usage(role, with_grant => true) marks a pg_durable admin and grants df.metrics() WITH GRANT OPTION (admins both view and re-delegate cluster-wide metrics). - Admins can also grant EXECUTE on df.metrics() explicitly to the roles that may view cluster-wide activity. - df.revoke_usage() revokes df.metrics() so admins can revoke and re-grant ordinary access without it, cleaning up installs that granted it under older helper bodies. Fresh installs make df.metrics() private by default. The 0.2.3->0.2.4 upgrade does NOT revoke its PUBLIC EXECUTE: that is a posture change for new installs, and existing admins who want it locked down have already revoked the PUBLIC grant themselves. Updates USER_GUIDE.md and docs/rls.md, and adds tests/e2e/sql/50_metrics_grants.sql covering grant omission, the with_grant admin grant, revoke_usage() cleanup, and an explicit-grant metrics call by a non-superuser. Closes #126 --- USER_GUIDE.md | 15 +++-- docs/rls.md | 4 +- sql/pg_durable--0.2.3--0.2.4.sql | 35 +++++++--- src/lib.rs | 32 +++++++--- src/monitoring.rs | 5 ++ tests/e2e/sql/50_metrics_grants.sql | 99 +++++++++++++++++++++++++++++ 6 files changed, 165 insertions(+), 25 deletions(-) create mode 100644 tests/e2e/sql/50_metrics_grants.sql diff --git a/USER_GUIDE.md b/USER_GUIDE.md index e67f4383..7fca2eca 100644 --- a/USER_GUIDE.md +++ b/USER_GUIDE.md @@ -1471,14 +1471,17 @@ SELECT * FROM df.instance_nodes('a1b2c3d4', 10); **Columns:** `execution_id`, `node_id`, `node_type`, `query`, `result_name`, `left_node`, `right_node`, `status`, `result` -### System Metrics +### System Metrics (Explicit Grant Required) ```sql +-- Requires a direct admin grant; df.grant_usage() does not include it. SELECT * FROM df.metrics(); ``` **Columns:** `total_instances`, `running_instances`, `completed_instances`, `failed_instances`, `total_executions`, `total_events` +> **Note:** `df.metrics()` returns system-wide aggregate counts across all users and is omitted from an ordinary `df.grant_usage('role')`. It is granted automatically to pg_durable admins via `df.grant_usage('role', with_grant => true)`, or you can grant EXECUTE on `df.metrics()` directly to any role that may view cluster-wide pg_durable activity. Other users can call `df.list_instances()` to view a summary of their own workflows. + ### Quick Status Check ```sql @@ -1667,12 +1670,12 @@ This function is purely additive — it never issues REVOKE. To downgrade a role |-----------|---------|-------------| | `p_role` | *(required)* | Target role name | | `include_http` | `false` | Grant EXECUTE on `df.http()` (opt-in — makes outbound network requests) | -| `with_grant` | `false` | Grant all privileges WITH GRANT OPTION and allow the role to call `df.grant_usage()` / `df.revoke_usage()` to manage other roles' access. The caller must hold each underlying privilege WITH GRANT OPTION (automatically true for superusers and delegated admins). | +| `with_grant` | `false` | Grant all privileges WITH GRANT OPTION and allow the role to call `df.grant_usage()` / `df.revoke_usage()` to manage other roles' access. Also grants EXECUTE on `df.metrics()` (system-wide aggregate counts), since `with_grant => true` designates a pg_durable admin. The caller must hold each underlying privilege WITH GRANT OPTION (automatically true for superusers and delegated admins). |
Equivalent manual grants (for reference) -The ordinary DSL functions (`df.sql`, `df.start`, `df.status`, etc.) keep PostgreSQL's default `PUBLIC EXECUTE`, so granting `USAGE ON SCHEMA df` is the single access gate that makes them callable — no per-function `GRANT EXECUTE` is required. Only the **sensitive** functions (`df.http`, `df.grant_usage`, `df.revoke_usage`) have `PUBLIC EXECUTE` revoked at install time and must be granted explicitly. +The ordinary DSL functions (`df.sql`, `df.start`, `df.status`, etc.) keep PostgreSQL's default `PUBLIC EXECUTE`, so granting `USAGE ON SCHEMA df` is the single access gate that makes them callable — no per-function `GRANT EXECUTE` is required. Only the **sensitive** functions (`df.http`, `df.metrics`, `df.grant_usage`, `df.revoke_usage`) have `PUBLIC EXECUTE` revoked at install time and must be granted explicitly. ```sql -- Access gate: schema USAGE makes every ordinary df.* function callable @@ -1680,6 +1683,10 @@ GRANT USAGE ON SCHEMA df TO app_role; -- Optional: HTTP access (include_http => true) -- GRANT EXECUTE ON FUNCTION df.http(text, text, text, jsonb, integer) TO app_role; +-- Optional: system-wide metrics access (also granted automatically by +-- df.grant_usage(role, with_grant => true)) +-- GRANT EXECUTE ON FUNCTION df.metrics() TO app_role; + -- Optional: delegated administration (with_grant => true) -- GRANT EXECUTE ON FUNCTION df.grant_usage(text, boolean, boolean) TO app_role; -- GRANT EXECUTE ON FUNCTION df.revoke_usage(text) TO app_role; @@ -1742,7 +1749,7 @@ To remove a role's access to pg_durable: SELECT df.revoke_usage('app_role'); ``` -This revokes all privileges previously granted by `df.grant_usage()`. It removes schema `USAGE`, EXECUTE on the sensitive functions (`df.http`, `df.grant_usage`, `df.revoke_usage`), and the table privileges — the mirror image of what `df.grant_usage()` grants. +This revokes all privileges previously granted by `df.grant_usage()`. It removes schema `USAGE`, EXECUTE on the sensitive functions (`df.http`, `df.metrics`, `df.grant_usage`, `df.revoke_usage`), and the table privileges. `df.metrics()` is granted only by `df.grant_usage('role', with_grant => true)` (or a direct admin GRANT); `df.revoke_usage()` always removes it, which also cleans up roles that received it from older grant helper bodies before re-granting ordinary access. There is no explicit self-revoke guard, and none is needed: PostgreSQL's `REVOKE` only removes grants made by the current role. A non-superuser therefore cannot revoke privileges another role (e.g. a superuser) granted to it, so calling `df.revoke_usage()` on your own role is harmless — it cannot lock you out of grants you didn't issue yourself. diff --git a/docs/rls.md b/docs/rls.md index bb527438..433d0684 100644 --- a/docs/rls.md +++ b/docs/rls.md @@ -341,7 +341,7 @@ ALTER TABLE df.vars ENABLE ROW LEVEL SECURITY; - `df.instance_info()`: Already queries `df.instances` via SPI for label — RLS filters automatically. Add ownership check before calling duroxide client - `df.instance_executions()`: Add ownership check (SPI query on `df.instances`) before calling duroxide client - `df.instance_nodes()`: Already queries `df.nodes` via SPI — RLS filters automatically - - `df.metrics()`: No change — returns aggregate counts, OK for any user + - `df.metrics()`: System-wide metrics remain private by default. An ordinary `df.grant_usage()` omits it, fresh installs revoke PUBLIC EXECUTE, and admins can grant EXECUTE explicitly to trusted roles. `df.grant_usage('role', with_grant => true)` (a pg_durable admin) grants it automatically. ✅ Implemented in v0.2.4. 5. **Update E2E tests** - Remove manual grants from `00_setup_playground.sql` (now automatic) @@ -371,7 +371,7 @@ All decisions have been resolved. No open questions remain. - **Decision 7 (cancel/signal ownership)**: Explicit ownership check before duroxide client call. ✅ - **Decision 8 (auto-grants)**: No automatic grants to PUBLIC. Admins must explicitly grant privileges to application roles after `CREATE EXTENSION`. ✅ Revised (was auto-grant to PUBLIC in v0.1.1). - **Monitoring functions**: Rework `df.list_instances()`, `df.instance_info()`, `df.instance_executions()`, and `df.instance_nodes()` to only show the calling user's own instances. Currently these functions fetch instance IDs from the duroxide client (which returns ALL instances via the worker's connection), then join with `df.instances` for labels. With RLS, the SPI label query is already filtered — but the duroxide client still returns other users' instance IDs, causing a mismatch. Fix: query `df.instances` via SPI first (RLS-filtered), then use only those IDs when calling the duroxide client. ✅ -- **`df.metrics()`**: OK for any user — returns aggregate system-wide counts with no per-instance data. No RLS consideration needed. ✅ +- **`df.metrics()`**: Controlled by explicit EXECUTE grants (security review Finding 6, v0.2.4). `df.metrics()` exposes system-wide aggregate counts (total instances, running/completed/failed counts, total executions and events) from the duroxide store without per-user filtering. Fresh installs revoke PUBLIC EXECUTE, an ordinary `df.grant_usage()` does not grant it, `df.grant_usage('role', with_grant => true)` (a pg_durable admin) grants it WITH GRANT OPTION, and `df.revoke_usage()` removes explicit metrics grants so admins can revoke and re-grant ordinary access without metrics. Roles without explicit metrics access should use `df.list_instances()` to view a summary of their own workflows. ✅ --- 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 efaab866..5d86e52f 100644 --- a/sql/pg_durable--0.2.3--0.2.4.sql +++ b/sql/pg_durable--0.2.3--0.2.4.sql @@ -38,12 +38,18 @@ DROP FUNCTION IF EXISTS df.debug_connection(); -- access boundary while requiring maintenance on every new function and -- masquerading as a security allowlist. -- --- The sensitive functions (df.http, df.grant_usage, df.revoke_usage) had their --- PUBLIC EXECUTE revoked at install time and were never in the list; they are --- still granted explicitly here, so their protection is unchanged. +-- The sensitive functions (df.http, df.grant_usage, df.revoke_usage) have +-- PUBLIC EXECUTE revoked; df.http and the admin helpers are granted explicitly +-- here when requested. The updated body also grants df.metrics() (system-wide +-- aggregate counts) to with_grant => true admins. -- --- This CREATE OR REPLACE brings pre-existing installs in line with fresh --- 0.2.4 installs (see src/lib.rs). The new body works against the existing +-- Unlike a fresh 0.2.4 install, this upgrade does NOT revoke df.metrics()'s +-- PUBLIC EXECUTE. Making df.metrics() private by default is a posture change for +-- new installs; existing admins who want it locked down have already revoked the +-- PUBLIC grant themselves, so we leave this install's grants as they are. +-- +-- This CREATE OR REPLACE otherwise brings pre-existing installs in line with +-- fresh 0.2.4 installs (see src/lib.rs). The new body works against the existing -- schema and changes no privileges already granted. -- ============================================================================ CREATE OR REPLACE FUNCTION df.grant_usage( @@ -70,10 +76,13 @@ BEGIN EXECUTE pg_catalog.format('GRANT EXECUTE ON FUNCTION df.http(text, text, text, jsonb, integer) TO %I', p_role) OPERATOR(pg_catalog.||) grant_opt; END IF; - -- Admin helpers — only for delegated administrators. + -- Admin helpers and system-wide metrics — with_grant => true marks a + -- pg_durable admin, so it also grants df.metrics() (cluster-wide aggregate + -- counts). IF with_grant THEN EXECUTE pg_catalog.format('GRANT EXECUTE ON FUNCTION df.grant_usage(text, boolean, boolean) TO %I', p_role) OPERATOR(pg_catalog.||) grant_opt; EXECUTE pg_catalog.format('GRANT EXECUTE ON FUNCTION df.revoke_usage(text) TO %I', p_role) OPERATOR(pg_catalog.||) grant_opt; + EXECUTE pg_catalog.format('GRANT EXECUTE ON FUNCTION df.metrics() TO %I', p_role) OPERATOR(pg_catalog.||) grant_opt; END IF; -- Table privileges @@ -100,10 +109,11 @@ $fn$; -- the role out of every ordinary df.* function. -- -- The new body undoes exactly what grant_usage() grants: schema USAGE, EXECUTE --- on the sensitive functions, and the table privileges. Note: a role granted --- under the OLD grant_usage() (explicit per-function EXECUTE) may retain inert --- EXECUTE entries on ordinary functions after this revoke; they are harmless --- because schema USAGE is gone, and clear on the next drop/regrant cycle. +-- on the sensitive functions (including df.metrics(), which grant_usage() grants +-- to with_grant admins), and the table privileges. Note: a role granted under +-- the OLD grant_usage() (explicit per-function EXECUTE) may retain inert EXECUTE +-- entries on ordinary functions after this revoke; they are harmless because +-- schema USAGE is gone. -- ============================================================================ CREATE OR REPLACE FUNCTION df.revoke_usage(p_role TEXT) RETURNS VOID @@ -123,6 +133,11 @@ BEGIN EXCEPTION WHEN insufficient_privilege THEN NULL; END; + BEGIN + EXECUTE pg_catalog.format('REVOKE EXECUTE ON FUNCTION df.metrics() FROM %I CASCADE', p_role); + EXCEPTION WHEN insufficient_privilege THEN + NULL; + END; BEGIN EXECUTE pg_catalog.format('REVOKE EXECUTE ON FUNCTION df.grant_usage(text, boolean, boolean) FROM %I CASCADE', p_role); EXCEPTION WHEN insufficient_privilege THEN diff --git a/src/lib.rs b/src/lib.rs index 76b26a32..92379da3 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -365,12 +365,15 @@ CREATE POLICY vars_user_isolation ON df.vars -- -- Access gate: schema USAGE makes the ordinary df.* functions callable (they -- keep PostgreSQL's default PUBLIC EXECUTE). Sensitive functions (df.http, --- df.grant_usage, df.revoke_usage) have PUBLIC EXECUTE revoked at install time --- and are granted explicitly below — keep a new private function private the --- same way (REVOKE ... FROM PUBLIC in rls_and_grants, then grant it here). +-- df.metrics, df.grant_usage, df.revoke_usage) have PUBLIC EXECUTE revoked at +-- install time and are granted explicitly below when appropriate — keep a new +-- private function private the same way (REVOKE ... FROM PUBLIC in +-- rls_and_grants, then grant it here). -- include_http => true also grants EXECUTE on df.http() (opt-in: network). --- with_grant => true grants everything WITH GRANT OPTION and lets the role --- call df.grant_usage()/df.revoke_usage() for others. +-- with_grant => true marks a pg_durable admin: grants everything WITH GRANT +-- OPTION, lets the role call df.grant_usage()/ +-- df.revoke_usage() for others, and grants df.metrics() +-- (system-wide aggregate counts). CREATE OR REPLACE FUNCTION df.grant_usage( p_role TEXT, include_http boolean DEFAULT false, @@ -395,10 +398,13 @@ BEGIN EXECUTE pg_catalog.format('GRANT EXECUTE ON FUNCTION df.http(text, text, text, jsonb, integer) TO %I', p_role) OPERATOR(pg_catalog.||) grant_opt; END IF; - -- Admin helpers — only for delegated administrators. + -- Admin helpers and system-wide metrics — with_grant => true marks a + -- pg_durable admin, so it also grants df.metrics() (cluster-wide aggregate + -- counts). IF with_grant THEN EXECUTE pg_catalog.format('GRANT EXECUTE ON FUNCTION df.grant_usage(text, boolean, boolean) TO %I', p_role) OPERATOR(pg_catalog.||) grant_opt; EXECUTE pg_catalog.format('GRANT EXECUTE ON FUNCTION df.revoke_usage(text) TO %I', p_role) OPERATOR(pg_catalog.||) grant_opt; + EXECUTE pg_catalog.format('GRANT EXECUTE ON FUNCTION df.metrics() TO %I', p_role) OPERATOR(pg_catalog.||) grant_opt; END IF; -- Table privileges @@ -434,6 +440,11 @@ BEGIN EXCEPTION WHEN insufficient_privilege THEN NULL; END; + BEGIN + EXECUTE pg_catalog.format('REVOKE EXECUTE ON FUNCTION df.metrics() FROM %I CASCADE', p_role); + EXCEPTION WHEN insufficient_privilege THEN + NULL; + END; BEGIN EXECUTE pg_catalog.format('REVOKE EXECUTE ON FUNCTION df.grant_usage(text, boolean, boolean) FROM %I CASCADE', p_role); EXCEPTION WHEN insufficient_privilege THEN @@ -483,10 +494,13 @@ BEGIN END IF; END $$; --- df.http(), df.grant_usage() and df.revoke_usage() are sensitive (network --- access / privilege management), so revoke PostgreSQL's default PUBLIC --- EXECUTE. df.grant_usage() re-grants them explicitly to authorized roles. +-- df.http(), df.metrics(), df.grant_usage() and df.revoke_usage() are sensitive +-- (network access / system-wide monitoring / privilege management), so revoke +-- PostgreSQL's default PUBLIC EXECUTE. df.grant_usage() re-grants the helper +-- functions explicitly to authorized roles; df.metrics() is granted to +-- with_grant => true admins or by a direct administrator GRANT. REVOKE EXECUTE ON FUNCTION df.http(text, text, text, jsonb, integer) FROM PUBLIC; +REVOKE EXECUTE ON FUNCTION df.metrics() FROM PUBLIC; REVOKE EXECUTE ON FUNCTION df.grant_usage(text, boolean, boolean) FROM PUBLIC; REVOKE EXECUTE ON FUNCTION df.revoke_usage(text) FROM PUBLIC; "#, diff --git a/src/monitoring.rs b/src/monitoring.rs index 97c583b7..30471859 100644 --- a/src/monitoring.rs +++ b/src/monitoring.rs @@ -291,6 +291,11 @@ pub fn instance_executions( } /// Get system-wide durable function metrics. +/// +/// Access is controlled by PostgreSQL function privileges. Roles with ordinary +/// df usage can call `df.list_instances()` to see counts scoped to their own +/// workflows; `df.metrics()` should be granted only to roles that may see +/// system-wide aggregate counts. #[pg_extern(schema = "df")] pub fn metrics() -> TableIterator< 'static, diff --git a/tests/e2e/sql/50_metrics_grants.sql b/tests/e2e/sql/50_metrics_grants.sql new file mode 100644 index 00000000..0ea8893c --- /dev/null +++ b/tests/e2e/sql/50_metrics_grants.sql @@ -0,0 +1,99 @@ +-- Tests: df.metrics() access is controlled by PostgreSQL EXECUTE grants. +-- +-- Verifies that: +-- 1. An ordinary df.grant_usage() does NOT grant EXECUTE on df.metrics(). +-- 2. df.grant_usage(with_grant => true) grants EXECUTE WITH GRANT OPTION +-- (with_grant => true designates a pg_durable admin). +-- 3. df.revoke_usage() removes the df.metrics() grant. +-- 4. A non-superuser with an explicit GRANT can actually call df.metrics(). +-- +-- Runs as postgres throughout (creates/drops roles, uses SET SESSION AUTHORIZATION). + +-- === Setup === +DO $setup$ +BEGIN + PERFORM pg_terminate_backend(pid) + FROM pg_stat_activity + WHERE usename = 'metrics_test_user' + AND pid <> pg_backend_pid(); + + BEGIN DROP OWNED BY metrics_test_user; EXCEPTION WHEN undefined_object THEN NULL; END; + BEGIN DROP ROLE metrics_test_user; EXCEPTION WHEN undefined_object THEN NULL; END; +END $setup$; + +CREATE ROLE metrics_test_user LOGIN; +GRANT TEMPORARY ON DATABASE postgres TO metrics_test_user; +GRANT USAGE, CREATE ON SCHEMA public TO metrics_test_user; + +-- Helper: assert metrics_test_user's EXECUTE privilege on df.metrics() matches +-- the expected value, failing with a labelled message otherwise. +CREATE FUNCTION pg_temp.assert_metrics_exec(expected BOOLEAN, test_label TEXT) +RETURNS VOID LANGUAGE plpgsql AS $fn$ +DECLARE + has_execute BOOLEAN; +BEGIN + SELECT has_function_privilege('metrics_test_user', 'df.metrics()', 'EXECUTE') + INTO has_execute; + + IF has_execute IS DISTINCT FROM expected THEN + RAISE EXCEPTION '% FAILED: expected EXECUTE on df.metrics() = %, got %', + test_label, expected, has_execute; + END IF; + + RAISE NOTICE '% PASSED', test_label; +END $fn$; + +-- === Test 1: ordinary grant_usage() does NOT grant EXECUTE on df.metrics() === +SELECT df.grant_usage('metrics_test_user'); +SELECT pg_temp.assert_metrics_exec(false, 'TEST 1 (ordinary grant_usage omits df.metrics())'); + +-- === Test 2: grant_usage(with_grant => true) grants EXECUTE WITH GRANT OPTION === +SELECT df.grant_usage('metrics_test_user', with_grant => true); +SELECT pg_temp.assert_metrics_exec(true, 'TEST 2 (with_grant admin gets df.metrics())'); + +DO $$ +BEGIN + IF NOT has_function_privilege( + 'metrics_test_user', 'df.metrics()', 'EXECUTE WITH GRANT OPTION' + ) THEN + RAISE EXCEPTION 'TEST 2 FAILED: with_grant => true should grant df.metrics() WITH GRANT OPTION'; + END IF; + RAISE NOTICE 'TEST 2 PASSED (WITH GRANT OPTION)'; +END $$; + +-- === Test 3: revoke_usage() removes the df.metrics() grant === +-- Reuses the with_grant grant from Test 2 — no separate setup needed. +SELECT df.revoke_usage('metrics_test_user'); +SELECT pg_temp.assert_metrics_exec(false, 'TEST 3 (revoke_usage removes df.metrics())'); + +-- === Test 4: a non-superuser with an explicit GRANT can call df.metrics() === +-- Re-grant ordinary usage (schema USAGE is the access gate) and add an explicit +-- df.metrics() grant on top. +SELECT df.grant_usage('metrics_test_user'); +GRANT EXECUTE ON FUNCTION df.metrics() TO metrics_test_user; + +SET SESSION AUTHORIZATION metrics_test_user; +DO $$ +DECLARE + total_instances BIGINT; +BEGIN + SELECT m.total_instances INTO total_instances FROM df.metrics() m; + RAISE NOTICE 'TEST 4 PASSED: explicit GRANT allows df.metrics() (total_instances = %)', total_instances; +END $$; +RESET SESSION AUTHORIZATION; + +-- === Cleanup === +DROP FUNCTION pg_temp.assert_metrics_exec(BOOLEAN, TEXT); + +DO $cleanup$ +BEGIN + PERFORM pg_terminate_backend(pid) + FROM pg_stat_activity + WHERE usename = 'metrics_test_user' + AND pid <> pg_backend_pid(); + + DROP OWNED BY metrics_test_user; + DROP ROLE metrics_test_user; +END $cleanup$; + +SELECT 'TEST PASSED: 50_metrics_grants' AS result;