diff --git a/CHANGELOG.md b/CHANGELOG.md index b28c14a8..6fe6bd88 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ Pre-1.0 note: while `pg_durable` is in major version `0`, minor releases may inc ### Fixed +- **Worker connection role names:** catalog role names are now passed verbatim when opening workflow connections, preventing quote-wrapped names from being reinterpreted as a different role. - **Restricted HTTP transport (#342):** restricted allow-list builds now require HTTPS so credentials and request bodies cannot be sent over plaintext HTTP; development-only `http-allow-all` builds continue to permit HTTP. ## [0.2.6] - 2026-08-23 diff --git a/src/types.rs b/src/types.rs index 65902ac1..f967fe62 100644 --- a/src/types.rs +++ b/src/types.rs @@ -9,7 +9,6 @@ use chrono::{DateTime, Utc}; use cron::Schedule as CronSchedule; use percent_encoding::{utf8_percent_encode, NON_ALPHANUMERIC}; use serde::{Deserialize, Serialize}; -use std::borrow::Cow; use std::ffi::{CStr, CString}; use std::str::FromStr; use std::sync::{Arc, OnceLock}; @@ -238,49 +237,6 @@ pub fn target_database() -> String { get_database() } -fn normalize_role_name_for_connection(user: &str) -> Result, String> { - if !user.starts_with('"') { - if user.ends_with('"') { - return Err(format!( - "Invalid role name '{}': unexpected trailing double quote in connection username", - user - )); - } - return Ok(Cow::Borrowed(user)); - } - - if !user.ends_with('"') || user.len() < 2 { - return Err(format!( - "Invalid role name '{}': unterminated quoted identifier in connection username", - user - )); - } - - let inner = &user[1..user.len() - 1]; - let mut normalized = String::with_capacity(inner.len()); - let mut chars = inner.chars().peekable(); - - while let Some(ch) = chars.next() { - if ch != '"' { - normalized.push(ch); - continue; - } - - if chars.peek() == Some(&'"') { - normalized.push('"'); - chars.next(); - continue; - } - - return Err(format!( - "Invalid quoted role name '{}': expected doubled double quotes inside identifier", - user - )); - } - - Ok(Cow::Owned(normalized)) -} - /// Create a single PostgreSQL connection authenticated as `user`. pub async fn connect_as_user( user: &str, @@ -308,11 +264,10 @@ async fn connect_as_user_with_application_name( /// Connection timeout for per-user SQL connections (seconds). const CONNECT_TIMEOUT_SECS: u64 = 30; - let normalized_user = normalize_role_name_for_connection(user)?; let default_db = target_database(); let db = database.unwrap_or(&default_db); let mut options = PgConnectOptions::new() - .username(normalized_user.as_ref()) + .username(user) .database(db) .port(get_port()) .application_name(application_name); @@ -328,17 +283,13 @@ async fn connect_as_user_with_application_name( .map_err(|_| { format!( "Connection to database '{}' as '{}' timed out after {}s", - db, - normalized_user.as_ref(), - CONNECT_TIMEOUT_SECS + db, user, CONNECT_TIMEOUT_SECS ) })? .map_err(|e| { format!( "Failed to connect to database '{}' as '{}'. Error: {}", - db, - normalized_user.as_ref(), - e + db, user, e ) })?; @@ -1981,36 +1932,6 @@ mod tests { } } - #[test] - fn normalize_role_name_keeps_raw_rolname() { - let normalized = normalize_role_name_for_connection("plain_role").unwrap(); - assert_eq!(normalized.as_ref(), "plain_role"); - } - - #[test] - fn normalize_role_name_keeps_mixed_case_rolname() { - let normalized = normalize_role_name_for_connection("labUser").unwrap(); - assert_eq!(normalized.as_ref(), "labUser"); - } - - #[test] - fn normalize_role_name_unquotes_regrole_text_output() { - let normalized = normalize_role_name_for_connection("\"Role Name\"").unwrap(); - assert_eq!(normalized.as_ref(), "Role Name"); - } - - #[test] - fn normalize_role_name_unescapes_embedded_quotes() { - let normalized = normalize_role_name_for_connection("\"Role \"\"Name\"\"\"").unwrap(); - assert_eq!(normalized.as_ref(), "Role \"Name\""); - } - - #[test] - fn normalize_role_name_rejects_malformed_quoted_identifier() { - let err = normalize_role_name_for_connection("\"bad\"name\"").unwrap_err(); - assert!(err.contains("Invalid quoted role name")); - } - #[test] fn configured_host_takes_precedence_over_pghost() { assert_eq!( diff --git a/tests/e2e/sql/17_superuser_guc.sql b/tests/e2e/sql/17_superuser_guc.sql index 2e164e12..b3485fc6 100644 --- a/tests/e2e/sql/17_superuser_guc.sql +++ b/tests/e2e/sql/17_superuser_guc.sql @@ -14,6 +14,7 @@ -- 3. Forgery caught by load_function_graph (instance-level rejection). -- 4. Forgery caught by execute_sql (node-level rejection, post-cache tamper). -- 5. Cross-iteration forgery neutralized by the frozen graph snapshot. +-- 6. A literal quoted catalog role cannot authenticate as postgres. -- -- "GUC on + superuser succeeds" is implicitly covered by every other E2E test -- (standard phase runs as postgres with enable_superuser_instances = on). @@ -341,6 +342,80 @@ END $$; DROP TABLE _su_guc_t5; +-- ============================================================ +-- Test 6: Quoted catalog role names cannot authenticate as another role +-- +-- PostgreSQL permits a role whose literal name is "postgres" (including +-- quote bytes). The worker must pass that catalog name unchanged to libpq. +-- Normalizing it as regrole display text would authenticate this workflow as +-- the real postgres superuser and write the protected sentinel. +-- ============================================================ +DROP TABLE IF EXISTS su_guc_quoted_role_sentinel; +CREATE TABLE su_guc_quoted_role_sentinel (marker TEXT); + +DO $setup_quoted_role$ +BEGIN + IF EXISTS (SELECT 1 FROM pg_roles WHERE rolname = chr(34) || 'postgres' || chr(34)) THEN + EXECUTE 'DROP OWNED BY """postgres"""'; + EXECUTE 'DROP ROLE """postgres"""'; + END IF; +END $setup_quoted_role$; +CREATE ROLE """postgres""" NOLOGIN; + +DROP TABLE IF EXISTS _su_guc_t6; +CREATE TABLE _su_guc_t6 (instance_id TEXT); +GRANT ALL ON _su_guc_t6 TO df_e2e_user, su_guc_forger; + +SET SESSION AUTHORIZATION df_e2e_user; +INSERT INTO _su_guc_t6 +SELECT df.start( + 'INSERT INTO su_guc_quoted_role_sentinel VALUES (''unexpected superuser identity'')', + 'su-guc-test6-quoted-role-identity' +); +RESET SESSION AUTHORIZATION; + +SET SESSION AUTHORIZATION su_guc_forger; +DO $forge_quoted_role$ +DECLARE + inst TEXT; +BEGIN + SELECT instance_id INTO inst FROM _su_guc_t6; + UPDATE df.instances + SET submitted_by = ( + SELECT oid::regrole FROM pg_roles WHERE rolname = chr(34) || 'postgres' || chr(34) + ) + WHERE id = inst; + UPDATE df.nodes + SET submitted_by = ( + SELECT oid::regrole FROM pg_roles WHERE rolname = chr(34) || 'postgres' || chr(34) + ) + WHERE instance_id = inst; +END $forge_quoted_role$; +RESET SESSION AUTHORIZATION; + +DO $assert_quoted_role$ +DECLARE + inst_id TEXT; + status TEXT; +BEGIN + SELECT instance_id INTO inst_id FROM _su_guc_t6; + SELECT df.await_instance(inst_id, 30) INTO status; + + IF lower(status) != 'failed' THEN + RAISE EXCEPTION 'TEST 6 FAILED: expected quoted-role workflow to fail, got: %', status; + END IF; + + IF EXISTS (SELECT 1 FROM su_guc_quoted_role_sentinel) THEN + RAISE EXCEPTION 'TEST 6 FAILED: quoted catalog role authenticated as postgres'; + END IF; + + RAISE NOTICE 'TEST 6 PASSED: quoted catalog role did not authenticate as postgres'; +END $assert_quoted_role$; + +DROP TABLE _su_guc_t6; +DROP TABLE su_guc_quoted_role_sentinel; +DROP ROLE """postgres"""; + -- ============================================================ -- Cleanup -- ============================================================ diff --git a/tests/e2e/sql/49_quoted_role_names.sql b/tests/e2e/sql/49_quoted_role_names.sql index 36ce0dff..7ca0e242 100644 --- a/tests/e2e/sql/49_quoted_role_names.sql +++ b/tests/e2e/sql/49_quoted_role_names.sql @@ -31,6 +31,29 @@ BEGIN END LOOP; END $setup$; +DO $boundary_setup$ +BEGIN + PERFORM pg_terminate_backend(pid) + FROM pg_stat_activity + WHERE usename IN ('quoteedge', '"quoteedge"') + AND pid <> pg_backend_pid(); + BEGIN + EXECUTE format('DROP OWNED BY %I', '"quoteedge"'); + EXCEPTION + WHEN undefined_object THEN NULL; + END; + EXECUTE format('DROP ROLE IF EXISTS %I', '"quoteedge"'); + EXECUTE 'DROP ROLE IF EXISTS quoteedge'; + EXECUTE 'CREATE ROLE quoteedge LOGIN'; + EXECUTE format('CREATE ROLE %I LOGIN', '"quoteedge"'); + PERFORM df.grant_usage('"quoteedge"'); + EXECUTE format( + 'GRANT TEMPORARY ON DATABASE %I TO %I', + current_database(), + '"quoteedge"' + ); +END $boundary_setup$; + SET SESSION AUTHORIZATION "labUser"; CREATE TEMP TABLE _test_state_quoted_1 (instance_id TEXT); INSERT INTO _test_state_quoted_1 @@ -49,6 +72,12 @@ INSERT INTO _test_state_quoted_3 SELECT df.start('SELECT 1 AS ok', 'quoted-role-embedded-quote'); RESET SESSION AUTHORIZATION; +SET SESSION AUTHORIZATION """quoteedge"""; +CREATE TEMP TABLE _test_state_quoted_4 (instance_id TEXT); +INSERT INTO _test_state_quoted_4 +SELECT df.start('SELECT current_user AS who', 'quoted-role-boundary'); +RESET SESSION AUTHORIZATION; + DO $$ DECLARE inst_id TEXT; @@ -112,9 +141,40 @@ BEGIN END IF; END $$; +DO $$ +DECLARE + inst_id TEXT; + final_status TEXT; + node_role TEXT; + ran_as TEXT; +BEGIN + SELECT instance_id INTO inst_id FROM _test_state_quoted_4; + SELECT df.await_instance(inst_id, 30) INTO final_status; + IF final_status != 'completed' THEN + RAISE EXCEPTION 'TEST FAILED (boundary "quoteedge"): expected completed, got %', final_status; + END IF; + SELECT r.rolname INTO node_role + FROM df.nodes n + JOIN pg_catalog.pg_roles r ON r.oid = n.submitted_by::oid + WHERE n.instance_id = inst_id + LIMIT 1; + IF node_role != '"quoteedge"' THEN + RAISE EXCEPTION 'TEST FAILED (boundary): expected submitted_by rolname "quoteedge" (quotes included), got %', node_role; + END IF; + SELECT (result->'rows'->0->>'who') INTO ran_as + FROM df.nodes + WHERE instance_id = inst_id + AND result IS NOT NULL + LIMIT 1; + IF ran_as != '"quoteedge"' THEN + RAISE EXCEPTION 'TEST FAILED (boundary): workflow executed as %, expected literal "quoteedge"', ran_as; + END IF; +END $$; + DROP TABLE _test_state_quoted_1; DROP TABLE _test_state_quoted_2; DROP TABLE _test_state_quoted_3; +DROP TABLE _test_state_quoted_4; DO $cleanup$ DECLARE @@ -135,4 +195,19 @@ BEGIN END LOOP; END $cleanup$; +DO $boundary_cleanup$ +BEGIN + PERFORM pg_terminate_backend(pid) + FROM pg_stat_activity + WHERE usename IN ('quoteedge', '"quoteedge"') + AND pid <> pg_backend_pid(); + BEGIN + EXECUTE format('DROP OWNED BY %I', '"quoteedge"'); + EXCEPTION + WHEN undefined_object THEN NULL; + END; + EXECUTE format('DROP ROLE IF EXISTS %I', '"quoteedge"'); + EXECUTE 'DROP ROLE IF EXISTS quoteedge'; +END $boundary_cleanup$; + SELECT 'TEST PASSED' AS result;