From 6e3c1e2615a4f621cf2776172f914e40daa2ce94 Mon Sep 17 00:00:00 2001 From: Toby Hede Date: Fri, 18 Sep 2026 12:01:09 +1000 Subject: [PATCH 1/4] fix(deps): bump tokio-postgres to 0.7.18 Addresses GHSA-3gjw-f78c-vvpw (affected >= 0.4.0, < 0.7.18). Lockfile-only change. --- Cargo.lock | 84 ++++++++++++++++++++++++++++++++++++------------------ 1 file changed, 57 insertions(+), 27 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e5abf3f2..254a2144 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2691,11 +2691,10 @@ checksum = "8355be11b20d696c8f18f6cc018c4e372165b1fa8126cef092399c9951984ffa" [[package]] name = "libredox" -version = "0.1.3" +version = "0.1.24" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c0ff37bd590ca25063e35af745c343cb7a0271906fb7b37e4813e8f79f00268d" +checksum = "6480ccc157a1389bb2e4891b24751b0f798ba640d22386f23143fbcc89da195a" dependencies = [ - "bitflags", "libc", ] @@ -3030,6 +3029,24 @@ dependencies = [ "autocfg", ] +[[package]] +name = "objc2-core-foundation" +version = "0.3.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2a180dd8642fa45cdb7dd721cd4c11b1cadd4929ce112ebd8b9f5803cc79d536" +dependencies = [ + "bitflags", +] + +[[package]] +name = "objc2-system-configuration" +version = "0.3.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7216bd11cbda54ccabcab84d523dc93b858ec75ecfb3a7d89513fa22464da396" +dependencies = [ + "objc2-core-foundation", +] + [[package]] name = "oid-registry" version = "0.8.1" @@ -3220,18 +3237,19 @@ dependencies = [ [[package]] name = "phf" -version = "0.11.3" +version = "0.13.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1fd6780a80ae0c52cc120a26a1a42c1ae51b247a253e4e06113d23d2c2edd078" +checksum = "c1562dc717473dbaa4c1f85a36410e03c047b2e7df7f45ee938fbef64ae7fadf" dependencies = [ "phf_shared", + "serde", ] [[package]] name = "phf_shared" -version = "0.11.3" +version = "0.13.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "67eabc2ef2a60eb7faa00097bd1ffdb5bd28e62bf39990626a582201b7a754e5" +checksum = "e57fef6bc5981e38c2ce2d63bfa546861309f875b8a75f092d1d54ae2d64f266" dependencies = [ "siphasher", ] @@ -3274,9 +3292,9 @@ checksum = "350e9b48cbc6b0e028b0473b114454c6316e57336ee184ceab6e53f72c178b3e" [[package]] name = "postgres-derive" -version = "0.4.6" +version = "0.4.9" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "69700ea4603c5ef32d447708e6a19cd3e8ac197a000842e97f527daea5e4175f" +checksum = "4d9d9089bb0ce62f4b5d52a0be0f4acfb35738b979380670d3dea85fe38d6ddd" dependencies = [ "heck", "proc-macro2", @@ -3304,16 +3322,16 @@ dependencies = [ [[package]] name = "postgres-types" -version = "0.2.9" +version = "0.2.14" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "613283563cd90e1dfc3518d548caee47e0e725455ed619881f5cf21f36de4b48" +checksum = "851ca9db4932932d69f3ea811b1abe63087a0f740a47692619dd40d4899b68be" dependencies = [ "bytes", "chrono", "fallible-iterator", "postgres-derive", "postgres-protocol", - "serde", + "serde_core", "serde_json", "uuid", ] @@ -4021,7 +4039,7 @@ dependencies = [ "errno", "libc", "linux-raw-sys 0.11.0", - "windows-sys 0.52.0", + "windows-sys 0.59.0", ] [[package]] @@ -4100,7 +4118,7 @@ dependencies = [ "security-framework", "security-framework-sys", "webpki-root-certs 1.0.5", - "windows-sys 0.61.2", + "windows-sys 0.59.0", ] [[package]] @@ -4325,14 +4343,15 @@ dependencies = [ [[package]] name = "serde_json" -version = "1.0.140" +version = "1.0.151" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "20068b6e96dc6c9bd23e01df8827e6c7e1f2fddd43c21810382803c136b99373" +checksum = "c841b55ecdae098c80dcae9cf767f6f8a0c2cdb3416bbef72181df4d0fe73f14" dependencies = [ "itoa", "memchr", - "ryu", "serde", + "serde_core", + "zmij", ] [[package]] @@ -4800,7 +4819,7 @@ dependencies = [ "getrandom 0.4.2", "once_cell", "rustix 1.1.3", - "windows-sys 0.52.0", + "windows-sys 0.59.0", ] [[package]] @@ -4969,9 +4988,9 @@ dependencies = [ [[package]] name = "tokio-postgres" -version = "0.7.13" +version = "0.7.18" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6c95d533c83082bb6490e0189acaa0bbeef9084e60471b696ca6988cd0541fb0" +checksum = "a528f7d280f6d5b9cd149635c8705b0dd049754bc67d81d31fa25169a93809d3" dependencies = [ "async-trait", "byteorder", @@ -4986,8 +5005,8 @@ dependencies = [ "pin-project-lite", "postgres-protocol", "postgres-types", - "rand 0.9.3", - "socket2 0.5.8", + "rand 0.10.2", + "socket2 0.6.5", "tokio", "tokio-util", "whoami", @@ -5627,9 +5646,12 @@ dependencies = [ [[package]] name = "wasite" -version = "0.1.0" +version = "1.0.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b8dad83b4f25e74f184f64c43b150b91efe7647395b42289f38e50566d82855b" +checksum = "66fe902b4a6b8028a753d5424909b764ccf79b7a209eac9bf97e59cda9f71a42" +dependencies = [ + "wasi 0.14.2+wasi-0.2.4", +] [[package]] name = "wasm-bindgen" @@ -5773,11 +5795,13 @@ dependencies = [ [[package]] name = "whoami" -version = "1.6.0" +version = "2.1.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6994d13118ab492c3c80c1f81928718159254c53c472bf9ce36f8dae4add02a7" +checksum = "626c4bac6755d76ffc12cb01b2eac751db1996b9e0041de9aa02c8c211ddc82c" dependencies = [ - "redox_syscall", + "libc", + "libredox", + "objc2-system-configuration", "wasite", "web-sys", ] @@ -6583,3 +6607,9 @@ dependencies = [ "quote", "syn 2.0.117", ] + +[[package]] +name = "zmij" +version = "1.0.23" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "29666d0abbfad1e3dc4dcf6144730dd3a3ab225bbbdac83319345b1b44ccfc1b" From 9461b8c2efac18ecdd0a5bc9c2138391ac020a8f Mon Sep 17 00:00:00 2001 From: Toby Hede Date: Mon, 21 Sep 2026 11:30:58 +1000 Subject: [PATCH 2/4] test(proxy): assert structured errors after tokio-postgres bump tokio-postgres 0.7.18 no longer appends the cause to Error's Display, so integration assertions on err.to_string() saw a bare "db error" and failed on all four PostgreSQL versions. Assert on the structured error instead: assert_db_error reads severity and message off as_db_error(), assert_client_error reads the kind off Display and the detail off source(). The exact customer-visible message text, including the docs/errors.md links, stays pinned. ConfigError::Database is no longer transparent: it renders the cause so Proxy's own logs keep the server's message rather than logging "db error". --- CHANGELOG.md | 4 ++ .../src/common.rs | 46 ++++++++++++++++++ .../src/extended_protocol_error_messages.rs | 47 ++++++++----------- .../src/multitenant/set_keyset_id.rs | 16 +++---- .../src/multitenant/set_keyset_name.rs | 16 +++---- .../src/set_keyset_error.rs | 13 +++-- packages/cipherstash-proxy/src/error.rs | 13 ++++- 7 files changed, 102 insertions(+), 53 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f3bb7d32..b5aa3bdb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### Fixed +- **Database error detail in Proxy's logs**: logged database errors again include the message PostgreSQL returned, not just the error kind. The upgraded PostgreSQL client library stopped appending the underlying cause when an error is rendered as text, which left entries such as `Database connection error` reading only `db error`. Proxy now renders the cause itself, so the server's message is back in the log line. + - **Extended-protocol execution lifecycle regressions**: statement duration and slow-statement metrics now describe each execution rather than the lifetime of its cached prepared statement; distinct portals keep isolated Bind measurements; suspended executions retain their metrics until completion; correlated stale responses and inaccessible connection protocol state close the connection instead of silently omitting metadata transitions; uncorrelated responses retain PostgreSQL passthrough behavior; decryption failures no longer report pending schema changes as successful; and disabling mapping no longer creates empty statement metrics. - **Query cancellation through Proxy**: Cancellation requests now reach the matching PostgreSQL connection, and their routing entries are removed when the client connection exits. Previously cancellation requests arrived on a separate connection that could not find the original route; retaining those routes globally without cleanup could also leak memory and eventually reject a new connection if PostgreSQL reused a cancellation key. @@ -30,6 +32,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### Security +- **Updated PostgreSQL client library**: Proxy is now built against `tokio-postgres` 0.7.18, which resolves [GHSA-3gjw-f78c-vvpw](https://github.com/advisories/GHSA-3gjw-f78c-vvpw). No configuration change is required. + - **DDL now updates encryption metadata transactionally**: Proxy applies schema changes only after PostgreSQL confirms execution, keeps successful changes connection-local until commit, and atomically publishes schema and EQL domain metadata before reporting idle readiness. Extended-protocol DDL, explicit transactions, savepoints, rollbacks, one-`Sync` pipelining, and already-open connections now observe the correct schema generation. Unmodelled DDL, simple-query batches whose DDL may change encryption metadata before a dependent statement, and failed catalog publication fail closed instead of risking plaintext writes through stale metadata; encryption-neutral DDL and native temporary-table batches remain compatible. ## [3.0.1] - 2026-08-05 diff --git a/packages/cipherstash-proxy-integration/src/common.rs b/packages/cipherstash-proxy-integration/src/common.rs index 9b13eaba..e81cbac8 100644 --- a/packages/cipherstash-proxy-integration/src/common.rs +++ b/packages/cipherstash-proxy-integration/src/common.rs @@ -596,6 +596,52 @@ pub fn interleaved_indices(len: usize) -> Vec { indices } +/// Asserts that `result` failed with a PostgreSQL `ErrorResponse` carrying +/// exactly this severity and this message. +/// +/// Read the message off the `DbError` rather than off `err.to_string()`. +/// `tokio_postgres::Error`'s `Display` renders only the error *kind* — since +/// 0.7.18 it no longer appends its cause — so `to_string()` yields a bare +/// `"db error"` and pins nothing. The server's text is unchanged and still +/// reachable through `as_db_error()`, which is where `Display` used to read it +/// from, so asserting there keeps the exact customer-visible wording pinned. +/// +/// `message` is the primary message field only: the `"db error: "` kind prefix +/// and the `"ERROR: "`/`"FATAL: "` severity prefix that `Display` used to +/// compose are asserted as `severity`, not as part of the text. +pub fn assert_db_error(result: Result, severity: &str, message: &str) { + let Err(err) = result else { + panic!("expected a database error, got a successful result"); + }; + + let db_error = err + .as_db_error() + .unwrap_or_else(|| panic!("expected a database error, got: {err:?}")); + + assert_eq!(db_error.severity(), severity); + assert_eq!(db_error.message(), message); +} + +/// Asserts that `result` failed in the client, before the statement reached the +/// server, with exactly this error kind and this underlying cause. +/// +/// The counterpart to [`assert_db_error`] for errors that never become a +/// PostgreSQL `ErrorResponse` — a `ToSql` conversion failure, for example. The +/// kind is what `tokio_postgres::Error` itself renders; the detail is the cause +/// it no longer appends, read back through `source()`. +pub fn assert_client_error(result: Result, kind: &str, cause: &str) { + let Err(err) = result else { + panic!("expected a client error, got a successful result"); + }; + + assert_eq!(err.to_string(), kind); + + let source = std::error::Error::source(&err) + .unwrap_or_else(|| panic!("expected the error to carry a cause, got: {err:?}")); + + assert_eq!(source.to_string(), cause); +} + /// /// Configure the client TLS settings. /// These are the settings for connecting to the database with TLS. diff --git a/packages/cipherstash-proxy-integration/src/extended_protocol_error_messages.rs b/packages/cipherstash-proxy-integration/src/extended_protocol_error_messages.rs index a1f205fd..1e135d4a 100644 --- a/packages/cipherstash-proxy-integration/src/extended_protocol_error_messages.rs +++ b/packages/cipherstash-proxy-integration/src/extended_protocol_error_messages.rs @@ -1,8 +1,11 @@ #[cfg(test)] mod tests { - use tracing::{debug, info}; + use tracing::debug; - use crate::common::{clear, connect_with_tls, random_id, reset_schema, trace, PROXY}; + use crate::common::{ + assert_client_error, assert_db_error, clear, connect_with_tls, random_id, reset_schema, + trace, PROXY, + }; /// A statement that always fails inside the proxy, at Parse, in every /// configuration: the proxy's SQL parser rejects it before it reaches the @@ -45,14 +48,11 @@ mod tests { let sql = "INSERT INTO encrypted (id, encrypted_unconfigured) VALUES ($1, $2)"; let result = client.query(sql, &[&id, &encrypted_text]).await; - assert!(result.is_err()); - - if let Err(err) = result { - let msg = err.to_string(); - assert_eq!(msg, "db error: ERROR: column \"encrypted_unconfigured\" of relation \"encrypted\" does not exist"); - } else { - unreachable!(); - } + assert_db_error( + result, + "ERROR", + "column \"encrypted_unconfigured\" of relation \"encrypted\" does not exist", + ); } /// A storage-only encrypted column round-trips. @@ -110,14 +110,11 @@ mod tests { let sql = "INSERT INTO encrypted (id, encrypted_date) VALUES ($1, $2)"; let result = client.query(sql, &[&id, &encrypted_date]).await; - assert!(result.is_err()); - - if let Err(err) = result { - let msg = err.to_string(); - assert_eq!(msg, "error serializing parameter 1: cannot convert between the Rust type `i32` and the Postgres type `date`"); - } else { - unreachable!(); - } + assert_client_error( + result, + "error serializing parameter 1", + "cannot convert between the Rust type `i32` and the Postgres type `date`", + ); } /// CIP-3678 regression: a statement that fails inside the proxy on a @@ -210,14 +207,10 @@ mod tests { let sql = "INSERT INTO encrypted id, encrypted_text VALUES ($1, $2)"; let result = client.query(sql, &[&id, &encrypted_text]).await; - assert!(result.is_err()); - - if let Err(err) = result { - let msg = err.to_string(); - info!("{}", msg); - assert_eq!(msg, "db error: ERROR: sql parser error: Expected: SELECT, VALUES, or a subquery in the query body, found: id at Line: 1, Column: 23. For help visit https://github.com/cipherstash/proxy/blob/main/docs/errors.md#mapping-invalid-sql-statement"); - } else { - unreachable!(); - } + assert_db_error( + result, + "ERROR", + "sql parser error: Expected: SELECT, VALUES, or a subquery in the query body, found: id at Line: 1, Column: 23. For help visit https://github.com/cipherstash/proxy/blob/main/docs/errors.md#mapping-invalid-sql-statement", + ); } } diff --git a/packages/cipherstash-proxy-integration/src/multitenant/set_keyset_id.rs b/packages/cipherstash-proxy-integration/src/multitenant/set_keyset_id.rs index 85b4e428..8877ad3c 100644 --- a/packages/cipherstash-proxy-integration/src/multitenant/set_keyset_id.rs +++ b/packages/cipherstash-proxy-integration/src/multitenant/set_keyset_id.rs @@ -8,7 +8,8 @@ #[cfg(test)] mod tests { use crate::common::{ - clear, connect_with_tls, random_id, rows_to_vec, simple_query_with_client, trace, PROXY, + assert_db_error, clear, connect_with_tls, random_id, rows_to_vec, simple_query_with_client, + trace, PROXY, }; use uuid::Uuid; @@ -292,15 +293,12 @@ mod tests { let insert_sql = "INSERT INTO encrypted (id, encrypted_text) VALUES ($1, $2)"; let result = client.query(insert_sql, &[&id, &text]).await; - assert!(result.is_err()); - - if let Err(err) = result { - let msg = err.to_string(); - assert_eq!(msg, "db error: FATAL: Unknown keyset name or id '2cace9db-3a2a-4b46-a184-ba412b3e0730'. Check the configured credentials. For help visit https://github.com/cipherstash/proxy/blob/main/docs/errors.md#encrypt-unknown-keyset"); - } else { - unreachable!(); - } + assert_db_error( + result, + "FATAL", + "Unknown keyset name or id '2cace9db-3a2a-4b46-a184-ba412b3e0730'. Check the configured credentials. For help visit https://github.com/cipherstash/proxy/blob/main/docs/errors.md#encrypt-unknown-keyset", + ); // -------- // Switch back to TENANT_1 diff --git a/packages/cipherstash-proxy-integration/src/multitenant/set_keyset_name.rs b/packages/cipherstash-proxy-integration/src/multitenant/set_keyset_name.rs index e00739e7..8267bab9 100644 --- a/packages/cipherstash-proxy-integration/src/multitenant/set_keyset_name.rs +++ b/packages/cipherstash-proxy-integration/src/multitenant/set_keyset_name.rs @@ -8,7 +8,8 @@ #[cfg(test)] mod tests { use crate::common::{ - clear, connect_with_tls, random_id, rows_to_vec, simple_query_with_client, trace, PROXY, + assert_db_error, clear, connect_with_tls, random_id, rows_to_vec, simple_query_with_client, + trace, PROXY, }; /// @@ -356,15 +357,12 @@ mod tests { let insert_sql = "INSERT INTO encrypted (id, encrypted_text) VALUES ($1, $2)"; let result = client.query(insert_sql, &[&id, &text]).await; - assert!(result.is_err()); - - if let Err(err) = result { - let msg = err.to_string(); - assert_eq!(msg, "db error: FATAL: Unknown keyset name or id 'BLAHVTHA'. Check the configured credentials. For help visit https://github.com/cipherstash/proxy/blob/main/docs/errors.md#encrypt-unknown-keyset"); - } else { - unreachable!(); - } + assert_db_error( + result, + "FATAL", + "Unknown keyset name or id 'BLAHVTHA'. Check the configured credentials. For help visit https://github.com/cipherstash/proxy/blob/main/docs/errors.md#encrypt-unknown-keyset", + ); // -------- // Switch back to TENANT_1 diff --git a/packages/cipherstash-proxy-integration/src/set_keyset_error.rs b/packages/cipherstash-proxy-integration/src/set_keyset_error.rs index 30f63633..4eb9007f 100644 --- a/packages/cipherstash-proxy-integration/src/set_keyset_error.rs +++ b/packages/cipherstash-proxy-integration/src/set_keyset_error.rs @@ -11,16 +11,15 @@ mod tests { use tracing::info; - use crate::common::{connect_with_tls, trace, PROXY}; + use crate::common::{assert_db_error, connect_with_tls, trace, PROXY}; /// Helper function to assert that a result contains the expected "Cannot SET CIPHERSTASH.KEYSET" error fn assert_keyset_error(result: Result) { - if let Err(err) = result { - let msg = err.to_string(); - assert_eq!(msg, "db error: FATAL: Cannot SET CIPHERSTASH.KEYSET if a default keyset has been configured. For help visit https://github.com/cipherstash/proxy/blob/main/docs/errors.md#encrypt-unexpected-set-keyset"); - } else { - unreachable!(); - } + assert_db_error( + result, + "FATAL", + "Cannot SET CIPHERSTASH.KEYSET if a default keyset has been configured. For help visit https://github.com/cipherstash/proxy/blob/main/docs/errors.md#encrypt-unexpected-set-keyset", + ); } /// Tests error handling of unknown keyset id diff --git a/packages/cipherstash-proxy/src/error.rs b/packages/cipherstash-proxy/src/error.rs index 58b12944..7ec66d77 100644 --- a/packages/cipherstash-proxy/src/error.rs +++ b/packages/cipherstash-proxy/src/error.rs @@ -171,7 +171,18 @@ pub enum ConfigError { #[error(transparent)] Certificate(#[from] rustls_pki_types::pem::Error), - #[error(transparent)] + /// Renders the tokio-postgres error *and* its cause. + /// + /// `tokio_postgres::Error`'s own `Display` is only the error kind — since + /// 0.7.18 it no longer appends the cause — so forwarding it transparently + /// would log a bare `db error` and drop the server's message, which is the + /// only part an operator can act on. Append the cause here so the logged + /// string stays as informative as it was. + #[error( + "{}{}", + _0, + std::error::Error::source(_0).map(|cause| format!(": {cause}")).unwrap_or_default() + )] Database(#[from] tokio_postgres::Error), #[error(transparent)] From b31fbb967e02f8c34b056aaafa2f3b5743680ff8 Mon Sep 17 00:00:00 2001 From: Toby Hede Date: Mon, 21 Sep 2026 13:57:56 +1000 Subject: [PATCH 3/4] fix(proxy): log the full error source chain tokio-postgres 0.7.18 follows the convention that an error's Display describes only that error, with the cause reached through source(). The previous fix worked against that by re-embedding the cause in ConfigError::Database's Display, and it only covered errors converted into that variant: sites logging a raw tokio_postgres::Error still dropped the server's message. Add ErrorChain, a Display wrapper that walks source(), and use it at every log site that can carry a database error. ConfigError::Database is transparent again. Recording the error as a dyn Error field is not enough: the Structured (JSON) format, the default off a terminal, renders only Display. ErrorChain skips a cause the message already ends with, so variants that embed their cause in Display are not printed twice. --- .../cipherstash-proxy/src/cli/migrate/mod.rs | 8 +- packages/cipherstash-proxy/src/connect/mod.rs | 8 +- packages/cipherstash-proxy/src/error.rs | 101 +++++++++++++++--- packages/cipherstash-proxy/src/main.rs | 8 +- packages/cipherstash-proxy/src/proxy/mod.rs | 4 +- .../src/proxy/schema/manager.rs | 4 +- 6 files changed, 106 insertions(+), 27 deletions(-) diff --git a/packages/cipherstash-proxy/src/cli/migrate/mod.rs b/packages/cipherstash-proxy/src/cli/migrate/mod.rs index 4a694330..f82b50e2 100644 --- a/packages/cipherstash-proxy/src/cli/migrate/mod.rs +++ b/packages/cipherstash-proxy/src/cli/migrate/mod.rs @@ -1,4 +1,4 @@ -use crate::error::Error; +use crate::error::{Error, ErrorChain}; use crate::log::MIGRATE; use crate::tls::NoCertificateVerification; use crate::TandemConfig; @@ -136,7 +136,7 @@ impl Migrate { let rows = match client.simple_query(&sql).await { Ok(rows) => rows, Err(err) => { - error!(target: MIGRATE, msg = "Error fetching records", table = self.table, error = err.to_string()); + error!(target: MIGRATE, msg = "Error fetching records", table = self.table, error = %ErrorChain(&err)); std::process::exit(exitcode::SOFTWARE); } }; @@ -270,7 +270,7 @@ pub async fn connect_with_tls( tokio::spawn(async move { if let Err(err) = connection.await { - error!("Connection error: {}", err); + error!("Connection error: {}", ErrorChain(&err)); } }); Ok(client) @@ -285,7 +285,7 @@ pub async fn connect_with_no_tls(connection_string: &str) -> Result Result { tokio::spawn(async move { if let Err(err) = connection.await { - error!(msg = "Connection error", error = err.to_string()); + error!(msg = "Connection error", error = %ErrorChain(&err)); } }); Ok(client) diff --git a/packages/cipherstash-proxy/src/error.rs b/packages/cipherstash-proxy/src/error.rs index 7ec66d77..e2a7f59a 100644 --- a/packages/cipherstash-proxy/src/error.rs +++ b/packages/cipherstash-proxy/src/error.rs @@ -2,7 +2,7 @@ use crate::{postgresql::Column, Identifier}; use cipherstash_client::{encryption, schema::ColumnType}; use eql_mapper::{EqlMapperError, EqlTermVariant}; use metrics_exporter_prometheus::BuildError; -use std::{io, time::Duration}; +use std::{fmt, io, time::Duration}; use thiserror::Error; pub(crate) const ERROR_DOC_BASE_URL: &str = @@ -171,18 +171,7 @@ pub enum ConfigError { #[error(transparent)] Certificate(#[from] rustls_pki_types::pem::Error), - /// Renders the tokio-postgres error *and* its cause. - /// - /// `tokio_postgres::Error`'s own `Display` is only the error kind — since - /// 0.7.18 it no longer appends the cause — so forwarding it transparently - /// would log a bare `db error` and drop the server's message, which is the - /// only part an operator can act on. Append the cause here so the logged - /// string stays as informative as it was. - #[error( - "{}{}", - _0, - std::error::Error::source(_0).map(|cause| format!(": {cause}")).unwrap_or_default() - )] + #[error(transparent)] Database(#[from] tokio_postgres::Error), #[error(transparent)] @@ -663,6 +652,39 @@ impl From for Error { } } +/// Renders an error and its full `source()` chain on one line: `a: b: c`. +/// +/// Use this wherever an error is logged. `Display` on its own is not enough: +/// by convention an error's `Display` describes only that error, and the cause +/// is reached through `source()`. `tokio_postgres::Error` follows that +/// convention from 0.7.18, so `err.to_string()` logs a bare `db error` and +/// drops the server's message. +/// +/// Recording the error as a `dyn Error` field does not help either: the +/// `Structured` (JSON) log format renders only `Display`, and it is the default +/// whenever stdout is not a terminal. +/// +/// A cause whose text the rendered message already ends with is skipped, so an +/// error that embeds its cause in `Display` (`"...: {0}"` with `#[from]`) is not +/// printed twice. +pub struct ErrorChain<'a>(pub &'a (dyn std::error::Error + 'static)); + +impl fmt::Display for ErrorChain<'_> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let mut rendered = self.0.to_string(); + let mut source = self.0.source(); + while let Some(cause) = source { + let cause_text = cause.to_string(); + if !rendered.ends_with(&cause_text) { + rendered.push_str(": "); + rendered.push_str(&cause_text); + } + source = cause.source(); + } + f.write_str(&rendered) + } +} + #[cfg(test)] mod tests { use super::*; @@ -682,4 +704,57 @@ mod tests { }; assert_eq!(error.to_string(), "Connection timed out after 5000 ms"); } + + #[derive(Debug, Error)] + #[error("{message}")] + struct Leaf { + message: &'static str, + } + + #[derive(Debug, Error)] + #[error("{message}")] + struct Wrapper { + message: &'static str, + #[source] + source: Leaf, + } + + #[test] + fn error_chain_renders_an_error_without_a_source() { + let error = Leaf { + message: "db error", + }; + + assert_eq!(ErrorChain(&error).to_string(), "db error"); + } + + #[test] + fn error_chain_appends_each_cause() { + let error = Wrapper { + message: "db error", + source: Leaf { + message: "ERROR: permission denied for table encrypted", + }, + }; + + assert_eq!( + ErrorChain(&error).to_string(), + "db error: ERROR: permission denied for table encrypted" + ); + } + + #[test] + fn error_chain_skips_a_cause_already_in_the_message() { + let error = Wrapper { + message: "Invalid encryption configuration: bad column", + source: Leaf { + message: "bad column", + }, + }; + + assert_eq!( + ErrorChain(&error).to_string(), + "Invalid encryption configuration: bad column" + ); + } } diff --git a/packages/cipherstash-proxy/src/main.rs b/packages/cipherstash-proxy/src/main.rs index 5cf22a72..b79a428a 100644 --- a/packages/cipherstash-proxy/src/main.rs +++ b/packages/cipherstash-proxy/src/main.rs @@ -1,6 +1,6 @@ use cipherstash_proxy::config::TandemConfig; use cipherstash_proxy::connect; -use cipherstash_proxy::error::{ConfigError, Error}; +use cipherstash_proxy::error::{ConfigError, Error, ErrorChain}; use cipherstash_proxy::prometheus::CLIENTS_ACTIVE_CONNECTIONS; use cipherstash_proxy::proxy::Proxy; use cipherstash_proxy::{cli, log, postgresql as pg, prometheus, tls, Args}; @@ -43,7 +43,7 @@ fn main() -> Result<(), Box> { } Err(err) => { - error!(msg = "Error running command", error = err.to_string()); + error!(msg = "Error running command", error = %ErrorChain(&err)); std::process::exit(exitcode::USAGE); } } @@ -114,7 +114,7 @@ fn main() -> Result<(), Box> { warn!(msg = "Database connection timeout", error = err.to_string()); } _ => { - error!(msg = "Database connection error", error = err.to_string()); + error!(msg = "Database connection error", error = %ErrorChain(&err)); } } }, @@ -236,7 +236,7 @@ async fn init(mut config: TandemConfig) -> Proxy { Err(err) => { error!( msg = "Could not start CipherStash proxy", - error = err.to_string() + error = %ErrorChain(&err) ); std::process::exit(exitcode::UNAVAILABLE); } diff --git a/packages/cipherstash-proxy/src/proxy/mod.rs b/packages/cipherstash-proxy/src/proxy/mod.rs index 0ca35886..9731bc6d 100644 --- a/packages/cipherstash-proxy/src/proxy/mod.rs +++ b/packages/cipherstash-proxy/src/proxy/mod.rs @@ -3,7 +3,7 @@ use std::sync::Arc; use crate::{ config::TandemConfig, connect, - error::{ConfigError, Error, TlsConfigError}, + error::{ConfigError, Error, ErrorChain, TlsConfigError}, postgresql::{Column, Context, KeysetIdentifier}, proxy::schema::SchemaManager, tls, @@ -103,7 +103,7 @@ impl Proxy { Err(err) => { warn!( msg = "Could not query EQL version from database", - error = err.to_string() + error = %ErrorChain(&err) ); None } diff --git a/packages/cipherstash-proxy/src/proxy/schema/manager.rs b/packages/cipherstash-proxy/src/proxy/schema/manager.rs index c9175898..8dbf8659 100644 --- a/packages/cipherstash-proxy/src/proxy/schema/manager.rs +++ b/packages/cipherstash-proxy/src/proxy/schema/manager.rs @@ -2,7 +2,7 @@ use super::eql_domains; use crate::config::DatabaseConfig; -use crate::error::Error; +use crate::error::{Error, ErrorChain}; use crate::proxy::encrypt_config::from_domain::column_config_from_domain; use crate::proxy::EncryptConfig; use crate::proxy::{AGGREGATE_QUERY, SCHEMA_QUERY}; @@ -213,7 +213,7 @@ where Err(err) => { warn!( msg = "Error reloading committed schema snapshot", - error = err.to_string() + error = %ErrorChain(&err) ); false } From 9eaa4bc7fccddf7324a89e96dae4237c769ba241 Mon Sep 17 00:00:00 2001 From: Toby Hede Date: Mon, 21 Sep 2026 17:27:05 +1000 Subject: [PATCH 4/4] test(proxy): use assert_db_error for the passthrough relation error Replace the hand-rolled source() match with the shared helper. Same severity and message are pinned; failures now report what arrived instead of hitting a bare unreachable!(). --- .../src/passthrough.rs | 20 ++++--------------- 1 file changed, 4 insertions(+), 16 deletions(-) diff --git a/packages/cipherstash-proxy-integration/src/passthrough.rs b/packages/cipherstash-proxy-integration/src/passthrough.rs index 39730385..dee12a94 100644 --- a/packages/cipherstash-proxy-integration/src/passthrough.rs +++ b/packages/cipherstash-proxy-integration/src/passthrough.rs @@ -1,8 +1,9 @@ #[cfg(test)] mod tests { - use crate::common::{clear, connect_with_tls, random_id, random_string, PROXY}; + use crate::common::{ + assert_db_error, clear, connect_with_tls, random_id, random_string, PROXY, + }; use rand::Rng; - use std::error::Error; #[tokio::test] async fn passthrough_statement() { @@ -36,20 +37,7 @@ mod tests { let sql = "SELECT * FROM blahvtha"; let result = client.query(sql, &[]).await; - assert!(result.is_err()); - - match result { - Ok(_) => unreachable!(), - Err(error) => match error.source() { - Some(db_error) => { - assert_eq!( - db_error.to_string(), - "ERROR: relation \"blahvtha\" does not exist" - ); - } - None => unreachable!(), - }, - } + assert_db_error(result, "ERROR", "relation \"blahvtha\" does not exist"); } #[tokio::test]