From 442c2cde9e0513233db72727a35018e062f492e6 Mon Sep 17 00:00:00 2001 From: Dan Draper Date: Sat, 30 May 2026 10:48:14 +1000 Subject: [PATCH 1/5] fix(cli): add --database-url/--db-port/--db-password/--no-tls, fix --db-user Makes the proxy usable as a plain CLI (e.g. `npx stash proxy`) without hand-setting env vars: - --database-url: parse a full postgres:// connection string (host, port, user, password, dbname) via tokio-postgres' parser; individual flags override. - --db-port / --db-password: the missing scalar connection flags. - --no-tls: listen for client connections in plaintext, overriding any CS_TLS__* env/config (inbound only; does not affect the proxy->database connection). Avoids a hard startup error when a stray CS_TLS__* points at a missing cert. Fixes a bug: --db-user set CS_DATABASE__USER, but the config field is `username` (CS_DATABASE__USERNAME), so --db-user was silently a no-op. Adds a regression test for the URL parsing + the USERNAME fix. Validated locally: `npx stash proxy --database-url postgres://.../db --no-tls` connects and listens. --- packages/cipherstash-proxy/src/cli/mod.rs | 26 +++++ .../cipherstash-proxy/src/config/tandem.rs | 110 +++++++++++++++++- 2 files changed, 134 insertions(+), 2 deletions(-) diff --git a/packages/cipherstash-proxy/src/cli/mod.rs b/packages/cipherstash-proxy/src/cli/mod.rs index f2341834..378f8767 100644 --- a/packages/cipherstash-proxy/src/cli/mod.rs +++ b/packages/cipherstash-proxy/src/cli/mod.rs @@ -21,11 +21,23 @@ const DEFAULT_CONFIG_FILE: &str = "cipherstash-proxy.toml"; /// CipherStash Proxy keeps your sensitive data in PostgreSQL encrypted and searchable, with no changes to SQL. /// pub struct Args { + /// Optional full PostgreSQL connection string, e.g. + /// "postgres://user:pass@host:5432/dbname". + /// Sets host, port, user, password and database name at once. + /// Individual flags below override matching parts of the URL. + #[arg(long, value_name = "URL", verbatim_doc_comment)] + pub database_url: Option, + /// Optional database host to connect to. /// Uses env or config file if not specified. #[arg(short = 'H', long)] pub db_host: Option, + /// Optional database port to connect to. + /// Uses env or config file if not specified. + #[arg(short = 'P', long)] + pub db_port: Option, + /// Optional database name to connect to. /// Uses env or config file if not specified. #[arg(value_name = "DBNAME")] @@ -36,6 +48,20 @@ pub struct Args { #[arg(short = 'u', long)] pub db_user: Option, + /// Optional database password. + /// Uses env or config file if not specified. + /// Prefer CS_DATABASE__PASSWORD or --database-url to avoid leaking the + /// password into shell history / the process list. + #[arg(short = 'W', long, verbatim_doc_comment)] + pub db_password: Option, + + /// Disable inbound (client-facing) TLS: the proxy listens for client + /// connections in plaintext. Overrides any CS_TLS__* env / config. + /// Use for local development only. Does not affect the connection from the + /// proxy to the database. + #[arg(long, verbatim_doc_comment)] + pub no_tls: bool, + /// Optional path to a CipherStash Proxy configuration file. /// /// Default is "cipherstash-proxy.toml". diff --git a/packages/cipherstash-proxy/src/config/tandem.rs b/packages/cipherstash-proxy/src/config/tandem.rs index f149144d..1a525b2a 100644 --- a/packages/cipherstash-proxy/src/config/tandem.rs +++ b/packages/cipherstash-proxy/src/config/tandem.rs @@ -108,15 +108,29 @@ impl TandemConfig { config.log.format = args.log_format; } + // --no-tls forces a plaintext inbound listener, ignoring any CS_TLS__* + // env / config. (Does not affect the proxy -> database connection.) + if args.no_tls { + if config.tls.is_some() { + println!("Disabling inbound TLS (--no-tls): the proxy will listen without TLS"); + } + config.tls = None; + config.server.require_tls = false; + } + Ok(config) } pub fn build_path(path: &str) -> Result { let args = Args { config_file_path: path.to_string(), + database_url: None, db_host: None, + db_port: None, db_name: None, db_user: None, + db_password: None, + no_tls: false, log_level: LogConfig::default_log_level(), log_format: LogConfig::default_log_format(), command: None, @@ -169,12 +183,24 @@ impl TandemConfig { env })); - // Command line arguments override env vars + // Command line arguments override env vars. + // A full connection string is applied first; individual flags below + // then override matching parts of it. + if let Some(url) = &args.database_url { + println!("Overriding database connection from --database-url"); + apply_database_url(url)?; + } + if let Some(db_host) = &args.db_host { println!("Overriding database host from command line argument"); env::set_var("CS_DATABASE__HOST", db_host); } + if let Some(db_port) = &args.db_port { + println!("Overriding database port from command line argument"); + env::set_var("CS_DATABASE__PORT", db_port.to_string()); + } + if let Some(dbname) = &args.db_name { println!("Overriding database name from command line argument"); env::set_var("CS_DATABASE__NAME", dbname); @@ -182,7 +208,12 @@ impl TandemConfig { if let Some(db_user) = &args.db_user { println!("Overriding database user from command line argument"); - env::set_var("CS_DATABASE__USER", db_user); + env::set_var("CS_DATABASE__USERNAME", db_user); + } + + if let Some(db_password) = &args.db_password { + println!("Overriding database password from command line argument"); + env::set_var("CS_DATABASE__PASSWORD", db_password); } // Source order is important! @@ -361,6 +392,47 @@ static RE: LazyLock = LazyLock::new(|| Regex::new(r"`([^`]+)`").unwrap()) /// /// Extracts a field name (if present) from a config::ConfigError string /// This is called in `build` if a ConfigError message contains the string `missing field` +/// Parse a PostgreSQL connection string (e.g. +/// "postgres://user:pass@host:5432/dbname") and set the corresponding +/// CS_DATABASE__* env vars for each component present. Individual --db-* flags +/// are applied after this and take precedence. +fn apply_database_url(url: &str) -> Result<(), Error> { + use std::str::FromStr; + + let pg = + tokio_postgres::Config::from_str(url).map_err(|err| ConfigError::InvalidParameter { + name: "database-url".to_string(), + value: err.to_string(), + })?; + + if let Some(host) = pg.get_hosts().iter().find_map(|h| match h { + tokio_postgres::config::Host::Tcp(host) => Some(host.clone()), + _ => None, + }) { + env::set_var("CS_DATABASE__HOST", host); + } + + if let Some(port) = pg.get_ports().first() { + env::set_var("CS_DATABASE__PORT", port.to_string()); + } + + if let Some(user) = pg.get_user() { + env::set_var("CS_DATABASE__USERNAME", user); + } + + if let Some(password) = pg.get_password() { + if let Ok(password) = std::str::from_utf8(password) { + env::set_var("CS_DATABASE__PASSWORD", password); + } + } + + if let Some(dbname) = pg.get_dbname() { + env::set_var("CS_DATABASE__NAME", dbname); + } + + Ok(()) +} + /// Expected string is in the forms: /// "missing field `{field}}` for key `{key}`" /// "missing field `{field}}`" @@ -419,6 +491,40 @@ mod tests { const CS_PREFIX: &str = "CS_TEST"; + #[test] + /// --database-url sets each CS_DATABASE__* var, including USERNAME (not USER + /// -- a previous bug set the wrong var so --db-user was a silent no-op). + fn database_url_sets_connection_env() { + use crate::config::tandem::apply_database_url; + with_no_cs_vars(|| { + temp_env::with_vars_unset( + [ + "CS_DATABASE__HOST", + "CS_DATABASE__PORT", + "CS_DATABASE__USERNAME", + "CS_DATABASE__USER", + "CS_DATABASE__PASSWORD", + "CS_DATABASE__NAME", + ], + || { + apply_database_url("postgres://alice:s3cret@db.example.com:5430/orders") + .unwrap(); + + assert_eq!( + std::env::var("CS_DATABASE__HOST").unwrap(), + "db.example.com" + ); + assert_eq!(std::env::var("CS_DATABASE__PORT").unwrap(), "5430"); + assert_eq!(std::env::var("CS_DATABASE__USERNAME").unwrap(), "alice"); + assert_eq!(std::env::var("CS_DATABASE__PASSWORD").unwrap(), "s3cret"); + assert_eq!(std::env::var("CS_DATABASE__NAME").unwrap(), "orders"); + // The old (buggy) variable name must not be set. + assert!(std::env::var("CS_DATABASE__USER").is_err()); + }, + ); + }); + } + #[test] /// the env vars from stash setup should be the preferred option /// File -> extended env (generated by the config struct layout) -> stash setup env From e1c5fd6c823f8d83bfa136a29a59c82077f26413 Mon Sep 17 00:00:00 2001 From: Dan Draper Date: Sat, 30 May 2026 11:19:49 +1000 Subject: [PATCH 2/5] feat(cli): quiet-by-default logging, random-port fallback, best-effort TLS DX improvements for running proxy as a CLI (e.g. npx stash proxy): - Quiet by default: log errors only unless --debug, an explicit --log-level / CS_LOG__LEVEL, or a config file is present (production/observability via env or config is unchanged). The listen address always prints. - Random-port fallback: if the listen port is the default (no CS_SERVER__PORT, no config file) and is already in use, bind an OS-assigned port and report it instead of failing. An explicitly configured port is still respected. - Best-effort inbound TLS: a TLS misconfiguration now falls back to a plaintext listener with a notice instead of exiting. --tls forces TLS (fatal if it can't be set up); --no-tls forces plaintext. Adds a regression test for the random-port fallback. Relates to BUG-300-adjacent CLI usability work. --- packages/cipherstash-proxy/src/cli/mod.rs | 13 ++- .../cipherstash-proxy/src/config/tandem.rs | 2 + packages/cipherstash-proxy/src/connect/mod.rs | 67 +++++++++++- packages/cipherstash-proxy/src/main.rs | 100 ++++++++++++------ 4 files changed, 147 insertions(+), 35 deletions(-) diff --git a/packages/cipherstash-proxy/src/cli/mod.rs b/packages/cipherstash-proxy/src/cli/mod.rs index 378f8767..5fbbbc16 100644 --- a/packages/cipherstash-proxy/src/cli/mod.rs +++ b/packages/cipherstash-proxy/src/cli/mod.rs @@ -59,9 +59,20 @@ pub struct Args { /// connections in plaintext. Overrides any CS_TLS__* env / config. /// Use for local development only. Does not affect the connection from the /// proxy to the database. - #[arg(long, verbatim_doc_comment)] + #[arg(long, verbatim_doc_comment, conflicts_with = "tls")] pub no_tls: bool, + /// Require inbound (client-facing) TLS. Startup fails if TLS is not + /// configured or the certificate/key are invalid. Without this flag the + /// proxy uses TLS when configured and falls back to plaintext otherwise. + #[arg(long, verbatim_doc_comment)] + pub tls: bool, + + /// Enable verbose (debug) logging. Without it the proxy logs errors only. + /// An explicit --log-level / CS_LOG__LEVEL or a config file takes precedence. + #[arg(long, verbatim_doc_comment)] + pub debug: bool, + /// Optional path to a CipherStash Proxy configuration file. /// /// Default is "cipherstash-proxy.toml". diff --git a/packages/cipherstash-proxy/src/config/tandem.rs b/packages/cipherstash-proxy/src/config/tandem.rs index 1a525b2a..053696e9 100644 --- a/packages/cipherstash-proxy/src/config/tandem.rs +++ b/packages/cipherstash-proxy/src/config/tandem.rs @@ -131,6 +131,8 @@ impl TandemConfig { db_user: None, db_password: None, no_tls: false, + tls: false, + debug: false, log_level: LogConfig::default_log_level(), log_format: LogConfig::default_log_format(), command: None, diff --git a/packages/cipherstash-proxy/src/connect/mod.rs b/packages/cipherstash-proxy/src/connect/mod.rs index d17d9796..0587a7ed 100644 --- a/packages/cipherstash-proxy/src/connect/mod.rs +++ b/packages/cipherstash-proxy/src/connect/mod.rs @@ -51,16 +51,35 @@ pub async fn database(config: &DatabaseConfig) -> Result { Ok(client) } -pub async fn bind_with_retry(server: &ServerConfig) -> TcpListener { - let address = &server.to_socket_address(); +pub async fn bind_with_retry(server: &ServerConfig, allow_random_fallback: bool) -> TcpListener { + let address = server.to_socket_address(); let mut retry_count = 0; loop { - match TcpListener::bind(address).await { + match TcpListener::bind(&address).await { Ok(listener) => { - info!(msg = "Server waiting for connections", address); + report_listening(&listener); return listener; } + // The configured port is in use, but it's only the default (not + // explicitly set), so fall back to an OS-assigned port and report it + // rather than failing. + Err(err) if err.kind() == std::io::ErrorKind::AddrInUse && allow_random_fallback => { + match TcpListener::bind(format!("{}:0", server.host)).await { + Ok(listener) => { + println!( + "Port {} is already in use; listening on an OS-assigned port instead.", + server.port + ); + report_listening(&listener); + return listener; + } + Err(err) => { + error!(msg = "Error binding connection", error = err.to_string()); + std::process::exit(exitcode::CONFIG); + } + } + } Err(err) => { if retry_count > MAX_RETRY_COUNT { error!( @@ -80,6 +99,21 @@ pub async fn bind_with_retry(server: &ServerConfig) -> TcpListener { } } +/// Report the address the proxy is listening on. Uses `println!` so the +/// listening address (including an OS-assigned fallback port) is always visible, +/// even when logging is quiet. +fn report_listening(listener: &TcpListener) { + match listener.local_addr() { + Ok(addr) => { + println!("CipherStash Proxy listening on {addr}"); + info!(msg = "Server waiting for connections", address = %addr); + } + Err(_) => { + info!(msg = "Server waiting for connections"); + } + } +} + pub async fn connect_with_retry(addr: &str) -> Result { let mut retry_count = 0; @@ -157,3 +191,28 @@ pub fn configure(stream: &TcpStream) { } } } + +#[cfg(test)] +mod tests { + use super::*; + + #[tokio::test] + async fn bind_falls_back_to_random_port_when_default_in_use() { + // Occupy a port, then ask bind_with_retry to bind the same one. + let occupied = TcpListener::bind("127.0.0.1:0").await.unwrap(); + let occupied_port = occupied.local_addr().unwrap().port(); + + let server = ServerConfig { + host: "127.0.0.1".to_string(), + port: occupied_port, + ..ServerConfig::default() + }; + + // With fallback allowed, it binds a different OS-assigned port. + let listener = bind_with_retry(&server, true).await; + let bound_port = listener.local_addr().unwrap().port(); + + assert_ne!(bound_port, occupied_port); + assert_ne!(bound_port, 0); + } +} diff --git a/packages/cipherstash-proxy/src/main.rs b/packages/cipherstash-proxy/src/main.rs index a11da143..a25d111f 100644 --- a/packages/cipherstash-proxy/src/main.rs +++ b/packages/cipherstash-proxy/src/main.rs @@ -1,4 +1,4 @@ -use cipherstash_proxy::config::TandemConfig; +use cipherstash_proxy::config::{LogConfig, LogLevel, TandemConfig}; use cipherstash_proxy::connect::{self, AsyncStream}; use cipherstash_proxy::error::{ConfigError, Error}; use cipherstash_proxy::prometheus::CLIENTS_ACTIVE_CONNECTIONS; @@ -23,7 +23,21 @@ fn main() -> Result<(), Box> { } }; - log::init(config.log.clone()); + // Quiet by default for CLI use: log errors only, unless --debug is passed, + // a log level was set explicitly (--log-level / CS_LOG__LEVEL), or a config + // file is present. This keeps `proxy` clean to run by hand while leaving + // production (which sets a level or ships a config) unchanged. + let log_explicitly_set = args.log_level != LogConfig::default_log_level() + || std::env::var("CS_LOG__LEVEL").is_ok() + || std::path::Path::new(&args.config_file_path).exists(); + let log_config = if args.debug { + LogConfig::with_level(LogLevel::Debug) + } else if log_explicitly_set { + config.log.clone() + } else { + LogConfig::with_level(LogLevel::Error) + }; + log::init(log_config); let runtime = tokio::runtime::Builder::new_multi_thread() .worker_threads(config.server.worker_threads) @@ -52,9 +66,14 @@ fn main() -> Result<(), Box> { runtime.block_on(async move { let shutdown_timeout = &config.server.shutdown_timeout(); - let mut proxy = init(config).await; + let mut proxy = init(config, args.tls).await; - let listener = connect::bind_with_retry(&proxy.config.server).await; + // If the listen port is just the default (not set via CS_SERVER__PORT or + // a config file) and it's already in use, fall back to an OS-assigned + // port rather than failing. An explicitly configured port is respected. + let port_configured = std::env::var("CS_SERVER__PORT").is_ok() + || std::path::Path::new(&args.config_file_path).exists(); + let listener = connect::bind_with_retry(&proxy.config.server, !port_configured).await; let tracker = TaskTracker::new(); let mut client_id = 0; @@ -144,7 +163,7 @@ fn main() -> Result<(), Box> { /// Validate various configuration options and /// Init the Proxy service /// -async fn init(mut config: TandemConfig) -> Proxy { +async fn init(mut config: TandemConfig, force_tls: bool) -> Proxy { if config.encrypt.default_keyset_id.is_none() { warn!(msg = "Default Keyset Id has not been configured"); warn!(msg = "A Keyset Identifier must be set using the `SET CIPHERSTASH.KEYSET_ID` or `SET CIPHERSTASH.KEYSET_NAME` commands"); @@ -182,32 +201,53 @@ async fn init(mut config: TandemConfig) -> Proxy { std::process::exit(exitcode::CONFIG); }); - match config.tls { - Some(ref mut tls) => { - _ = tls.check_cert().inspect_err(|err| { - error!(msg = err.to_string()); - std::process::exit(exitcode::CONFIG); - }); - - _ = tls.check_private_key().inspect_err(|err| { - error!(msg = err.to_string()); - std::process::exit(exitcode::CONFIG); - }); - - match tls::configure_server(tls) { - Ok(_) => { - info!(msg = "Server Transport Layer Security (TLS) configuration validated"); - } - Err(err) => { - error!( - msg = "Server Transport Layer Security (TLS) configuration error", - error = err.to_string() - ); - std::process::exit(exitcode::CONFIG); - } + // Validate inbound TLS if configured. By default, a TLS misconfiguration is + // non-fatal: we warn and fall back to a plaintext listener. Passing --tls + // makes any TLS problem fatal (no silent downgrade). + let tls_error: Option = match &config.tls { + Some(tls) => { + if let Err(err) = tls.check_cert() { + Some(err.to_string()) + } else if let Err(err) = tls.check_private_key() { + Some(err.to_string()) + } else if let Err(err) = tls::configure_server(tls) { + Some(err.to_string()) + } else { + None } } - None => { + None => None, + }; + + match (&config.tls, tls_error, force_tls) { + // TLS configured and valid. + (Some(_), None, _) => { + info!(msg = "Server Transport Layer Security (TLS) configuration validated"); + } + // TLS configured but invalid, and --tls forces it: fatal. + (Some(_), Some(err), true) => { + eprintln!("TLS is required (--tls) but the configuration is invalid: {err}"); + std::process::exit(exitcode::CONFIG); + } + // TLS configured but invalid, no force: fall back to plaintext. + (Some(_), Some(err), false) => { + eprintln!( + "TLS configuration is invalid ({err}); falling back to a plaintext listener. \ + Pass --tls to make this fatal, or --no-tls to silence this warning." + ); + config.tls = None; + config.server.require_tls = false; + } + // No TLS configured, but --tls forces it: fatal. + (None, _, true) => { + eprintln!( + "TLS is required (--tls) but no inbound TLS certificate is configured \ + (set CS_TLS__CERTIFICATE_PATH / CS_TLS__PRIVATE_KEY_PATH)." + ); + std::process::exit(exitcode::CONFIG); + } + // No TLS configured, no force: plaintext listener. + (None, _, false) => { warn!(msg = "Transport Layer Security (TLS) is not configured"); warn!(msg = "Listening on an unsafe connection"); } @@ -287,6 +327,6 @@ async fn reload_application_config(config: &TandemConfig, args: &Args) -> Result } info!(msg = "Configuration reloaded"); - let proxy = init(new_config).await; + let proxy = init(new_config, args.tls).await; Ok(proxy) } From 6ac4806559370cd228e7746d4fbec9bc3512c8a6 Mon Sep 17 00:00:00 2001 From: Dan Draper Date: Sat, 30 May 2026 11:23:52 +1000 Subject: [PATCH 3/5] chore(log): downgrade missing-EQL-config-table from error to warn When a target database has no eql_v2_configuration table, the proxy continues in passthrough -- it's only an error if you actually want encryption. Logging it at error made it show even with the new quiet-by-default logging; warn is the right level. --- .../cipherstash-proxy/src/proxy/encrypt_config/manager.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/cipherstash-proxy/src/proxy/encrypt_config/manager.rs b/packages/cipherstash-proxy/src/proxy/encrypt_config/manager.rs index 65dcaf0d..374ed23a 100644 --- a/packages/cipherstash-proxy/src/proxy/encrypt_config/manager.rs +++ b/packages/cipherstash-proxy/src/proxy/encrypt_config/manager.rs @@ -98,7 +98,9 @@ async fn init_reloader(config: DatabaseConfig) -> Result { - error!(msg = "No Encrypt configuration table in database."); + // Not fatal: the proxy continues in passthrough. EQL is only + // required if you want encryption, so log at warn, not error. + warn!(msg = "No Encrypt configuration table in database."); warn!(msg = "Encrypt requires the Encrypt Query Language (EQL) to be installed in the target database"); warn!(msg = "See https://github.com/cipherstash/encrypt-query-language"); } From e13f5b88e0d1deaa819e15bb178f03daac6628ea Mon Sep 17 00:00:00 2001 From: Dan Draper Date: Sat, 30 May 2026 13:34:25 +1000 Subject: [PATCH 4/5] fix(log): stop dumping raw ErrorResponse at error level for DB errors A PostgreSQL error forwarded to the client is normal proxy operation -- the client already surfaces it -- but the backend logged every one at ERROR level as the full Debug-formatted ErrorResponse struct. That is unreadable and, with quiet-by-default logging, was the only thing showing on a failed query (and it bled into --psql sessions). Log it at debug instead, concisely (SQLSTATE code + message) via a new ErrorResponse::field() accessor. Nothing on a SQL error by default; a clean one-line entry under --debug. Adds a unit test for the accessor. --- .../src/postgresql/backend.rs | 16 ++++++++++---- .../src/postgresql/messages/error_response.rs | 22 +++++++++++++++++++ 2 files changed, 34 insertions(+), 4 deletions(-) diff --git a/packages/cipherstash-proxy/src/postgresql/backend.rs b/packages/cipherstash-proxy/src/postgresql/backend.rs index 9e832e6e..ec3c94c4 100644 --- a/packages/cipherstash-proxy/src/postgresql/backend.rs +++ b/packages/cipherstash-proxy/src/postgresql/backend.rs @@ -2,7 +2,7 @@ use super::context::Context; use super::data::to_sql; use super::error_handler::PostgreSqlErrorHandler; use super::message_buffer::MessageBuffer; -use super::messages::error_response::ErrorResponse; +use super::messages::error_response::{ErrorResponse, ErrorResponseCode}; use super::messages::row_description::RowDescription; use super::messages::BackendCode; use super::Column; @@ -24,7 +24,7 @@ use cipherstash_client::eql::EqlCiphertext; use metrics::{counter, histogram}; use std::time::Instant; use tokio::io::AsyncRead; -use tracing::{debug, error, info, warn}; +use tracing::{debug, error, warn}; /// The PostgreSQL proxy backend that handles server-to-client message processing. /// @@ -316,8 +316,16 @@ where /// to forward to the client unchanged. fn error_response_handler(&mut self, bytes: &BytesMut) -> Result, Error> { let error_response = ErrorResponse::try_from(bytes)?; - error!(msg = "PostgreSQL Error", error = ?error_response); - info!(msg = "PostgreSQL Errors originate in the database"); + // A database error forwarded to the client is normal proxy operation -- + // the client surfaces it -- so log concisely at debug, not error. (The + // full message is available to the client; --debug shows it here too.) + debug!( + target: PROTOCOL, + client_id = self.context.client_id, + msg = "Database returned an error", + code = error_response.field(ErrorResponseCode::Code).unwrap_or("?"), + error = error_response.field(ErrorResponseCode::Message).unwrap_or(""), + ); Ok(Some(bytes.to_owned())) } diff --git a/packages/cipherstash-proxy/src/postgresql/messages/error_response.rs b/packages/cipherstash-proxy/src/postgresql/messages/error_response.rs index 7013fe1f..e2a2554d 100644 --- a/packages/cipherstash-proxy/src/postgresql/messages/error_response.rs +++ b/packages/cipherstash-proxy/src/postgresql/messages/error_response.rs @@ -60,6 +60,15 @@ pub enum ErrorResponseCode { } impl ErrorResponse { + /// Value of the first field with the given code (e.g. the SQLSTATE `Code` + /// or the human-readable `Message`), if present. + pub fn field(&self, code: ErrorResponseCode) -> Option<&str> { + self.fields + .iter() + .find(|f| f.code == code) + .map(|f| f.value.as_str()) + } + /// Create a FATAL error response for connection timeout. /// /// Uses PostgreSQL error code 57P05 (idle_session_timeout). While this code @@ -514,6 +523,19 @@ mod tests { assert_eq!(bytes, message); } + #[test] + pub fn field_returns_value_by_code() { + let message = to_message(b"E\0\0\0kSERROR\0VERROR\0C26000\0Mprepared statement \"a37\" does not exist\0Fprepare.c\0L454\0RFetchPreparedStatement\0\0Z\0\0\0\x05I"); + let error_response = ErrorResponse::try_from(&message).unwrap(); + + assert_eq!(error_response.field(ErrorResponseCode::Code), Some("26000")); + assert_eq!( + error_response.field(ErrorResponseCode::Message), + Some("prepared statement \"a37\" does not exist") + ); + assert_eq!(error_response.field(ErrorResponseCode::Detail), None); + } + #[test] pub fn sql_parse_error_response() { let response = ErrorResponse::invalid_sql_statement( From c928bdfc27229db1c632bbccfd84621abf4b011c Mon Sep 17 00:00:00 2001 From: Dan Draper Date: Mon, 1 Jun 2026 11:36:01 +1000 Subject: [PATCH 5/5] fix(cli): repair integration build and address review feedback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add the six new Args fields (database_url, db_port, db_password, no_tls, tls, debug) to the struct literal in the migrate integration test. The proxy crate compiled, but the integration crate's literal was stale, so the full-workspace test build failed E0063 — this is the CI failure. - Reject a non-UTF8 password from --database-url with a clear InvalidParameter error instead of silently dropping it (which would connect with no/stale credentials). Add a regression test; the branch is reachable because tokio-postgres stores the URL password as raw bytes without UTF-8 validation. - Remove a stale doc comment left over from another function above apply_database_url. --- .../src/migrate/mod.rs | 6 ++++ .../cipherstash-proxy/src/config/tandem.rs | 36 +++++++++++++++---- 2 files changed, 36 insertions(+), 6 deletions(-) diff --git a/packages/cipherstash-proxy-integration/src/migrate/mod.rs b/packages/cipherstash-proxy-integration/src/migrate/mod.rs index 701f3731..2847385b 100644 --- a/packages/cipherstash-proxy-integration/src/migrate/mod.rs +++ b/packages/cipherstash-proxy-integration/src/migrate/mod.rs @@ -42,9 +42,15 @@ mod tests { log_level: LogLevel::Debug, log_format: LogFormat::Pretty, command: None, + database_url: None, db_host: None, + db_port: None, db_name: None, db_user: None, + db_password: None, + no_tls: false, + tls: false, + debug: false, }; let config = match TandemConfig::load(&args) { diff --git a/packages/cipherstash-proxy/src/config/tandem.rs b/packages/cipherstash-proxy/src/config/tandem.rs index 053696e9..d545a13e 100644 --- a/packages/cipherstash-proxy/src/config/tandem.rs +++ b/packages/cipherstash-proxy/src/config/tandem.rs @@ -391,9 +391,6 @@ impl Default for PrometheusConfig { static RE: LazyLock = LazyLock::new(|| Regex::new(r"`([^`]+)`").unwrap()); -/// -/// Extracts a field name (if present) from a config::ConfigError string -/// This is called in `build` if a ConfigError message contains the string `missing field` /// Parse a PostgreSQL connection string (e.g. /// "postgres://user:pass@host:5432/dbname") and set the corresponding /// CS_DATABASE__* env vars for each component present. Individual --db-* flags @@ -423,9 +420,15 @@ fn apply_database_url(url: &str) -> Result<(), Error> { } if let Some(password) = pg.get_password() { - if let Ok(password) = std::str::from_utf8(password) { - env::set_var("CS_DATABASE__PASSWORD", password); - } + // The password is required downstream as a String. Rather than silently + // dropping a non-UTF8 password (which would leave the proxy connecting + // with no password, or a stale env/config one), fail clearly. + let password = + std::str::from_utf8(password).map_err(|_| ConfigError::InvalidParameter { + name: "database-url".to_string(), + value: "password contains invalid UTF-8".to_string(), + })?; + env::set_var("CS_DATABASE__PASSWORD", password); } if let Some(dbname) = pg.get_dbname() { @@ -527,6 +530,27 @@ mod tests { }); } + #[test] + /// A non-UTF8 password in --database-url is an error, not a silent no-op. + /// tokio-postgres percent-decodes the password to raw bytes without UTF-8 + /// validation, so `%FF` yields invalid UTF-8; we must reject it rather than + /// leave CS_DATABASE__PASSWORD unset (which would connect with no/stale + /// credentials). + fn database_url_rejects_non_utf8_password() { + use crate::config::tandem::apply_database_url; + with_no_cs_vars(|| { + temp_env::with_vars_unset(["CS_DATABASE__PASSWORD"], || { + let result = apply_database_url("postgres://alice:%FF@db.example.com/orders"); + assert!( + result.is_err(), + "expected a non-UTF8 password to be rejected" + ); + // And it must not have set a password from the bad URL. + assert!(std::env::var("CS_DATABASE__PASSWORD").is_err()); + }); + }); + } + #[test] /// the env vars from stash setup should be the preferred option /// File -> extended env (generated by the config struct layout) -> stash setup env