From bf0e81a8cd8ec5d89e2216e89d8cfee83f93859f Mon Sep 17 00:00:00 2001 From: tjgreen42 <1738591+tjgreen42@users.noreply.github.com> Date: Fri, 17 Jul 2026 03:31:54 +0000 Subject: [PATCH] fix(worker): handle Unix-socket PGHOST in connection URL (#266) postgres_connection_string() built a postgres://user@host:port/db URL with PGHOST as the host. A Unix-socket directory PGHOST (e.g. CloudNativePG's /controller/run) put a raw path in the URL authority, which the parser rejects as "empty host", so the worker never connected and instances stayed pending. build_connection_url() percent-encodes a socket-path host (one starting with /) with the NON_ALPHANUMERIC set sqlx uses in its own build_url(). sqlx percent-decodes it and connects over the socket; encoding the whole path keeps the directory opaque, so metacharacters cannot be reinterpreted as a TCP host or query parameters. TCP addresses and hostnames are used verbatim. get_host()/connect_as_user() already handle socket paths via PgConnectOptions::host(). Tests cover TCP, hostname, and socket-directory inputs, including paths with a space and URL metacharacters, asserting the URL parses to the expected socket/host via sqlx. Copilot-Session: b5f8cc43-8ac2-4355-90c8-18a10d488296 --- Cargo.lock | 1 + Cargo.toml | 1 + src/types.rs | 104 ++++++++++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 105 insertions(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index 8e07f322..c8632a26 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1899,6 +1899,7 @@ dependencies = [ "cron", "duroxide", "duroxide-pg", + "percent-encoding", "pgrx", "pgrx-tests", "reqwest", diff --git a/Cargo.toml b/Cargo.toml index 8b11990f..8c654714 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -32,6 +32,7 @@ pgrx = "=0.16.1" serde = { version = "1.0", features = ["derive"] } serde_json = { version = "1.0", features = ["preserve_order"] } uuid = { version = "1.0", features = ["v4", "serde"] } +percent-encoding = "2.3" # duroxide integration duroxide = "=0.1.29" diff --git a/src/types.rs b/src/types.rs index 4a6d78f4..6bead410 100644 --- a/src/types.rs +++ b/src/types.rs @@ -7,6 +7,7 @@ use pgrx::{pg_extern, Spi}; 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::CString; @@ -146,7 +147,21 @@ pub fn postgres_connection_string() -> String { let user = get_worker_role(); let database = get_database(); - format!("postgres://{user}@{host}:{port}/{database}") + build_connection_url(&user, &host, port, &database) +} + +/// Build the worker's `postgres://` connection URL. +/// +/// A Unix-socket `host` (one starting with `/`) is percent-encoded so the URL +/// parser keeps the whole path as the host; sqlx percent-decodes it and +/// connects over the socket. TCP addresses and hostnames are used verbatim. +fn build_connection_url(user: &str, host: &str, port: i32, database: &str) -> String { + if host.starts_with('/') { + let encoded = utf8_percent_encode(host, NON_ALPHANUMERIC).to_string(); + format!("postgres://{user}@{encoded}:{port}/{database}") + } else { + format!("postgres://{user}@{host}:{port}/{database}") + } } /// Get the PostgreSQL host for connections @@ -1404,6 +1419,93 @@ mod tests { assert!(err.contains("Invalid quoted role name")); } + #[test] + fn build_connection_url_tcp_host_unchanged() { + assert_eq!( + build_connection_url("postgres", "127.0.0.1", 5432, "postgres"), + "postgres://postgres@127.0.0.1:5432/postgres" + ); + } + + #[test] + fn build_connection_url_hostname_unchanged() { + assert_eq!( + build_connection_url("worker", "db.internal.example.com", 6432, "app"), + "postgres://worker@db.internal.example.com:6432/app" + ); + } + + #[test] + fn build_connection_url_unix_socket_percent_encoded() { + assert_eq!( + build_connection_url("postgres", "/controller/run", 5432, "postgres"), + "postgres://postgres@%2Fcontroller%2Frun:5432/postgres" + ); + } + + #[test] + fn build_connection_url_unix_socket_standard_dir() { + assert_eq!( + build_connection_url("postgres", "/var/run/postgresql", 5432, "postgres"), + "postgres://postgres@%2Fvar%2Frun%2Fpostgresql:5432/postgres" + ); + } + + #[test] + fn build_connection_url_socket_parses_to_socket() { + use sqlx::postgres::PgConnectOptions; + use std::str::FromStr; + + let url = build_connection_url("postgres", "/controller/run", 5432, "postgres"); + let opts = PgConnectOptions::from_str(&url).expect("socket URL should parse"); + assert_eq!( + opts.get_socket().map(|p| p.to_string_lossy().into_owned()), + Some("/controller/run".to_string()), + ); + } + + #[test] + fn build_connection_url_tcp_parses_to_host() { + use sqlx::postgres::PgConnectOptions; + use std::str::FromStr; + + let url = build_connection_url("postgres", "127.0.0.1", 5432, "postgres"); + let opts = PgConnectOptions::from_str(&url).expect("TCP URL should parse"); + assert_eq!(opts.get_host(), "127.0.0.1"); + assert!(opts.get_socket().is_none()); + } + + #[test] + fn build_connection_url_socket_with_special_chars_parses_to_socket() { + use sqlx::postgres::PgConnectOptions; + use std::str::FromStr; + + let path = "/var/run/pg data"; + let url = build_connection_url("postgres", path, 5432, "postgres"); + let opts = PgConnectOptions::from_str(&url).expect("socket URL should parse"); + assert_eq!( + opts.get_socket().map(|p| p.to_string_lossy().into_owned()), + Some(path.to_string()), + ); + } + + #[test] + fn build_connection_url_socket_path_is_opaque_no_tcp_injection() { + use sqlx::postgres::PgConnectOptions; + use std::str::FromStr; + + // A leading-`/` host with URL metacharacters must remain a single opaque + // socket path (no TCP host or query-parameter interpretation). + let path = "/@evil.com?sslmode=disable"; + let url = build_connection_url("postgres", path, 5432, "postgres"); + let opts = PgConnectOptions::from_str(&url).expect("socket URL should parse"); + assert_eq!( + opts.get_socket().map(|p| p.to_string_lossy().into_owned()), + Some(path.to_string()), + ); + assert_ne!(opts.get_host(), "evil.com"); + } + // ============================================================================ // Substitution Engine Tests // ============================================================================