From 519e1a4e8cc3cab8c38ad6d9f085f411838af15f Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 26 Oct 2026 04:28:46 +0000 Subject: [PATCH 1/7] test(proxy): show SQL EXECUTE runs a statement prepared without mapping A statement prepared with SQL PREPARE while UNSAFE_DISABLE_MAPPING is on is never mapped. Once mapping is enabled again, EXECUTE of it is forwarded with its arguments as written, so only PostgreSQL's domain check stops plaintext reaching an encrypted column. The Bind settings check covers only protocol-level statements. These tests fail on main: Proxy forwards the EXECUTE instead of refusing it. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../src/disable_mapping.rs | 50 ++++++++++++++ .../src/postgresql/middleware/frontend.rs | 68 ++++++++++++++++++- 2 files changed, 117 insertions(+), 1 deletion(-) diff --git a/packages/cipherstash-proxy-integration/src/disable_mapping.rs b/packages/cipherstash-proxy-integration/src/disable_mapping.rs index bdc59738..427ee5fa 100644 --- a/packages/cipherstash-proxy-integration/src/disable_mapping.rs +++ b/packages/cipherstash-proxy-integration/src/disable_mapping.rs @@ -290,4 +290,54 @@ mod tests { .unwrap(); } } + + /// A statement prepared with SQL `PREPARE` while mapping is disabled was + /// never mapped. Executing it after mapping is enabled again must be + /// refused by Proxy, rather than forwarded with its arguments as written. + #[tokio::test] + async fn sql_execute_of_a_statement_prepared_without_mapping_is_refused() { + trace(); + + clear().await; + + let client = connect_with_tls(*PROXY).await; + let id = random_id(); + + client + .simple_query("SET CIPHERSTASH.UNSAFE_DISABLE_MAPPING = true") + .await + .unwrap(); + client + .simple_query( + "PREPARE insert_unmapped AS INSERT INTO encrypted (id, encrypted_text) VALUES ($1, $2)", + ) + .await + .unwrap(); + client + .simple_query("SET CIPHERSTASH.UNSAFE_DISABLE_MAPPING = false") + .await + .unwrap(); + + // Valid JSON, so only PostgreSQL's domain check would stop it otherwise + let err = client + .simple_query(&format!( + r#"EXECUTE insert_unmapped({id}, '{{"plaintext": "hello"}}')"# + )) + .await + .err() + .unwrap(); + let message = err.as_db_error().unwrap().message().to_owned(); + assert!( + message.starts_with( + "The prepared statement or bound values no longer match confirmed connection state" + ), + "{message}" + ); + + let rows = client + .query("SELECT id FROM encrypted WHERE id = $1", &[&id]) + .await + .unwrap(); + assert!(rows.is_empty()); + } } diff --git a/packages/cipherstash-proxy/src/postgresql/middleware/frontend.rs b/packages/cipherstash-proxy/src/postgresql/middleware/frontend.rs index 1c28d041..59780e6c 100644 --- a/packages/cipherstash-proxy/src/postgresql/middleware/frontend.rs +++ b/packages/cipherstash-proxy/src/postgresql/middleware/frontend.rs @@ -3833,6 +3833,12 @@ mod unresolvable_table_tests { /// a native `notes (body)`, and when `encrypted` is set, `legacy (data)` with /// `data` a legacy EQL v2 column. fn client(config: TandemConfig, encrypted: bool) -> (Frontend, Counter) { + let (context, counter) = client_context(config, encrypted); + (Frontend::new(context), counter) + } + + /// The [`Context`] behind [`client`]. + fn client_context(config: TandemConfig, encrypted: bool) -> (Context, Counter) { let mut schema = eql_mapper::Schema::new("public"); let mut table = eql_mapper::Table::new(sqltk::parser::ast::Ident::new("encrypted")); table.add_column(Arc::new(eql_mapper::Column::native( @@ -3881,7 +3887,7 @@ mod unresolvable_table_tests { counter.clone(), sender, ); - (Frontend::new(context), counter) + (context, counter) } fn encrypted_client() -> (Frontend, Counter) { @@ -4356,6 +4362,66 @@ mod unresolvable_table_tests { } } + /// A statement prepared with SQL `PREPARE` while mapping is disabled was + /// never mapped, so executing it once mapping is enabled again would write + /// its arguments as written. + #[tokio::test] + async fn sql_execute_of_a_statement_prepared_without_mapping_is_refused() { + let mut config = TandemConfig::for_testing(); + config.disable_mapping_for_testing(); + config.development.as_mut().unwrap().disable_mapping = false; + let (context, _) = client_context(config, true); + let mut frontend = Frontend::new(context.clone()); + let mut backend = crate::postgresql::middleware::Backend::new(context); + for (sql, tag) in [ + ("SET CIPHERSTASH.UNSAFE_DISABLE_MAPPING = true", "SET"), + ( + "PREPARE p (text) AS INSERT INTO encrypted (secret) VALUES ($1)", + "PREPARE", + ), + ("SET CIPHERSTASH.UNSAFE_DISABLE_MAPPING = false", "SET"), + ] { + let operation = operation_id(); + frontend + .intercept( + operation, + FrontendMessage::Query(Bytes::copy_from_slice(sql.as_bytes())), + ) + .await + .unwrap(); + backend + .intercept( + Some(operation), + pg_proto::BackendMessage::CommandComplete(Bytes::copy_from_slice( + tag.as_bytes(), + )), + ) + .await + .unwrap(); + backend + .intercept( + Some(operation), + pg_proto::BackendMessage::ReadyForQuery(pg_proto::TransactionStatus::Idle), + ) + .await + .unwrap(); + } + let reprepared = query( + &mut frontend, + "PREPARE q (text) AS INSERT INTO encrypted (secret) VALUES ($1)", + ) + .await; + assert!( + reprepared.contains("RAISE EXCEPTION"), + "mapping is still disabled: {reprepared}" + ); + let forwarded = query(&mut frontend, "EXECUTE p('plaintext')").await; + assert!( + forwarded.contains("RAISE EXCEPTION") && !forwarded.contains("plaintext"), + "{forwarded}" + ); + } + #[tokio::test] async fn extended_read_on_an_unresolvable_table_is_forwarded() { let (mut frontend, _) = encrypted_client(); From 2cd3cff3979918bc9a696901861459efabacc713 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 26 Oct 2026 04:33:11 +0000 Subject: [PATCH 2/7] fix(proxy): refuse SQL EXECUTE of a statement prepared without mapping Proxy now records each SQL PREPARE that writes data and is forwarded while UNSAFE_DISABLE_MAPPING is on. Once mapping is enabled again, EXECUTE of one is refused with PreparedSettingsChanged, as Bind is for a statement parsed with the extended protocol. The name is released when PostgreSQL confirms a DEALLOCATE or DISCARD ALL that removes it. Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 2 + .../src/postgresql/context/mod.rs | 37 +++++++- .../src/postgresql/middleware/frontend.rs | 92 +++++++++++++++++-- 3 files changed, 122 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 37c446dd..0c5a8610 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -47,6 +47,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### Fixed +- **SQL `EXECUTE` after `UNSAFE_DISABLE_MAPPING`**: Proxy now refuses `EXECUTE` of a statement that writes data and was prepared with SQL `PREPARE` while `CIPHERSTASH.UNSAFE_DISABLE_MAPPING` was on, once mapping is enabled again. It was forwarded with its arguments as written, so only PostgreSQL's type check stopped plaintext reaching an encrypted column. `DEALLOCATE` the statement and prepare it again. This matches the check Proxy already makes for statements prepared with the extended protocol. + - **`--db-user`**: the flag now sets the database username. It previously set an unused variable, so it had no effect. - **Unknown column error message**: the type-checking error for a column a table does not have now names the column and the table the right way round, for example `Column: email not found for table: users`. It previously swapped them. diff --git a/packages/cipherstash-proxy/src/postgresql/context/mod.rs b/packages/cipherstash-proxy/src/postgresql/context/mod.rs index 44077828..3ebe8049 100644 --- a/packages/cipherstash-proxy/src/postgresql/context/mod.rs +++ b/packages/cipherstash-proxy/src/postgresql/context/mod.rs @@ -32,7 +32,7 @@ use pg_proto::{Describe, DescribeTarget, DiagnosticResponse, TransactionStatus}; use serde_json::json; pub use statement_metadata::StatementMetadata; use std::{ - collections::{HashMap, VecDeque}, + collections::{HashMap, HashSet, VecDeque}, sync::{ atomic::{AtomicU64, Ordering}, Arc, RwLock, @@ -160,6 +160,11 @@ struct ConnectionProtocolState { prepared_statements: PreparedStatements, portals: HashMap>, portal_operations: HashMap>, + /// SQL `PREPARE` statements that write data, forwarded while mapping was + /// disabled. They were never mapped, so `EXECUTE` of one is refused once + /// mapping is enabled again. A name leaves only when PostgreSQL confirms a + /// `DEALLOCATE` or `DISCARD ALL` that removes it. + unmapped_sql_statements: HashSet, } /// The Bind operation that created a portal, and the SQL deallocation it runs when executed @@ -180,6 +185,7 @@ impl Default for ConnectionProtocolState { prepared_statements: PreparedStatements::default(), portals: HashMap::new(), portal_operations: HashMap::new(), + unmapped_sql_statements: HashSet::new(), } } } @@ -810,6 +816,28 @@ where Ok(()) } + /// Record that the SQL prepared statement `name`, which writes data, was + /// forwarded without mapping. + pub fn set_unmapped_sql_statement(&self, name: Name) -> Result<(), Error> { + self.protocol_state + .write() + .map_err(|_| crate::error::ContextError::ProtocolStateUnavailable)? + .unmapped_sql_statements + .insert(name); + Ok(()) + } + + /// Whether the SQL prepared statement `name` writes data and was forwarded + /// without mapping. + pub fn is_unmapped_sql_statement(&self, name: &Name) -> Result { + Ok(self + .protocol_state + .read() + .map_err(|_| crate::error::ContextError::ProtocolStateUnavailable)? + .unmapped_sql_statements + .contains(name)) + } + /// Carry the deallocation of `statement`, if any, onto the portal just bound from it. pub fn bind_statement_effects(&self, portal: &Name, statement: &Name) -> Result<(), Error> { let mut state = self @@ -888,6 +916,13 @@ where debug!(target: CONTEXT, client_id = self.client_id, msg = "Deallocate", ?deallocation); + match &deallocation { + Deallocation::Statement(name) => { + state.unmapped_sql_statements.remove(name); + } + Deallocation::AllStatements => state.unmapped_sql_statements.clear(), + } + let names = state.prepared_statements.deallocation_names(deallocation); for name in &names { // Also when a later Parse prepared the name again, so that Parse's rollback diff --git a/packages/cipherstash-proxy/src/postgresql/middleware/frontend.rs b/packages/cipherstash-proxy/src/postgresql/middleware/frontend.rs index 59780e6c..8dd90dcc 100644 --- a/packages/cipherstash-proxy/src/postgresql/middleware/frontend.rs +++ b/packages/cipherstash-proxy/src/postgresql/middleware/frontend.rs @@ -1252,8 +1252,31 @@ impl Frontend { resolver: &Arc, ) -> Result, Error> { if self.context.unsafe_disable_mapping()? { + if let ast::Statement::Prepare { name, .. } = statement { + if eql_mapper::writes_data(statement) { + self.context + .set_unmapped_sql_statement(sql_statement_name(name))?; + } + } return Ok(Route::Forward(Forward::MappingDisabled)); } + // Like the Bind settings check, for a statement prepared with SQL. + if let ast::Statement::Execute { + name: Some(name), .. + } = statement + { + if let [part] = name.0.as_slice() { + if let Some(name) = part.as_ident() { + if self.context.has_encrypted_columns() + && self + .context + .is_unmapped_sql_statement(&sql_statement_name(name))? + { + return Err(MappingError::PreparedSettingsChanged.into()); + } + } + } + } if let Some((table_name, columns)) = eql_mapper::copies_into_table(statement) { if self.context.has_encrypted_columns() { check_copy_into(table_name, columns, resolver)?; @@ -1460,14 +1483,8 @@ fn to_deallocation(statement: &ast::Statement) -> Option { ast::Statement::Deallocate { name, .. } => { if name.quote_style.is_none() && name.value.eq_ignore_ascii_case("all") { Some(Deallocation::AllStatements) - } else if name.quote_style.is_none() { - // Unquoted identifiers are folded to lower case by PostgreSQL - // (ASCII letters only, as `downcase_identifier` does for UTF-8) - Some(Deallocation::Statement(Name::from( - name.value.to_ascii_lowercase(), - ))) } else { - Some(Deallocation::Statement(Name::from(name.value.clone()))) + Some(Deallocation::Statement(sql_statement_name(name))) } } ast::Statement::Discard { @@ -1477,6 +1494,17 @@ fn to_deallocation(statement: &ast::Statement) -> Option { } } +/// The name PostgreSQL gives a SQL prepared statement named `name`. +fn sql_statement_name(name: &ast::Ident) -> Name { + if name.quote_style.is_none() { + // Unquoted identifiers are folded to lower case by PostgreSQL + // (ASCII letters only, as `downcase_identifier` does for UTF-8) + Name::from(name.value.to_ascii_lowercase()) + } else { + Name::from(name.value.clone()) + } +} + fn quote_literal(value: &str) -> String { let escaped = value.replace('\\', "\\\\").replace('\'', "''"); if value.contains('\\') { @@ -4417,11 +4445,59 @@ mod unresolvable_table_tests { ); let forwarded = query(&mut frontend, "EXECUTE p('plaintext')").await; assert!( - forwarded.contains("RAISE EXCEPTION") && !forwarded.contains("plaintext"), + forwarded.contains("RAISE EXCEPTION") + && forwarded.contains("no longer match confirmed connection state") + && !forwarded.contains("plaintext"), "{forwarded}" ); } + /// Once PostgreSQL confirms the statement is gone, its name is free again. + #[tokio::test] + async fn sql_execute_after_deallocating_a_statement_prepared_without_mapping_is_forwarded() { + let mut config = TandemConfig::for_testing(); + config.disable_mapping_for_testing(); + config.development.as_mut().unwrap().disable_mapping = false; + let (context, _) = client_context(config, true); + let mut frontend = Frontend::new(context.clone()); + let mut backend = crate::postgresql::middleware::Backend::new(context); + for (sql, tag) in [ + ("SET CIPHERSTASH.UNSAFE_DISABLE_MAPPING = true", "SET"), + ( + "PREPARE p (text) AS INSERT INTO encrypted (secret) VALUES ($1)", + "PREPARE", + ), + ("DEALLOCATE p", "DEALLOCATE"), + ("SET CIPHERSTASH.UNSAFE_DISABLE_MAPPING = false", "SET"), + ] { + let operation = operation_id(); + frontend + .intercept( + operation, + FrontendMessage::Query(Bytes::copy_from_slice(sql.as_bytes())), + ) + .await + .unwrap(); + backend + .intercept( + Some(operation), + pg_proto::BackendMessage::CommandComplete(Bytes::copy_from_slice( + tag.as_bytes(), + )), + ) + .await + .unwrap(); + backend + .intercept( + Some(operation), + pg_proto::BackendMessage::ReadyForQuery(pg_proto::TransactionStatus::Idle), + ) + .await + .unwrap(); + } + assert_eq!(query(&mut frontend, "EXECUTE p(1)").await, "EXECUTE p(1)"); + } + #[tokio::test] async fn extended_read_on_an_unresolvable_table_is_forwarded() { let (mut frontend, _) = encrypted_client(); From 0f48c2685b86062ef4469ab175333ac8b16da113 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 26 Oct 2026 05:07:48 +0000 Subject: [PATCH 3/7] test(proxy): show the schema loader misses encrypted columns off its search path The loader reads only the schemas in its connection's current_schemas. When every encrypted column is in a schema outside them, Proxy sees no encrypted columns, so a write to a table it cannot resolve falls back and is forwarded as written. The test runs the loader as a role whose search path leaves out the schema holding an encrypted column. It fails on main. SchemaManager is exported so the integration tests can run it. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../cipherstash-proxy-integration/src/lib.rs | 1 + .../src/schema_loader.rs | 63 +++++++++++++++++++ packages/cipherstash-proxy/src/proxy/mod.rs | 3 +- 3 files changed, 66 insertions(+), 1 deletion(-) create mode 100644 packages/cipherstash-proxy-integration/src/schema_loader.rs diff --git a/packages/cipherstash-proxy-integration/src/lib.rs b/packages/cipherstash-proxy-integration/src/lib.rs index ad4b3493..7b7e5b3f 100644 --- a/packages/cipherstash-proxy-integration/src/lib.rs +++ b/packages/cipherstash-proxy-integration/src/lib.rs @@ -31,6 +31,7 @@ mod pipeline; mod pooler; /// Database-backed transaction-aware schema middleware regressions. mod schema_change; +mod schema_loader; mod select; mod set_keyset_error; mod simple_protocol; diff --git a/packages/cipherstash-proxy-integration/src/schema_loader.rs b/packages/cipherstash-proxy-integration/src/schema_loader.rs new file mode 100644 index 00000000..c93dbe38 --- /dev/null +++ b/packages/cipherstash-proxy-integration/src/schema_loader.rs @@ -0,0 +1,63 @@ +//! Proxy's schema loader, run directly against PostgreSQL. + +#[cfg(test)] +mod tests { + use crate::common::{connect_with_tls, get_database_port, trace}; + use cipherstash_proxy::{proxy::SchemaManager, DatabaseConfig}; + + const ROLE: &str = "schema_loader_narrow_path"; + + /// The loader reads only the schemas on its own search path. An encrypted + /// column outside them must still count, or Proxy forwards a write to a + /// table it cannot find as if no column were encrypted. + #[tokio::test] + async fn encrypted_columns_off_the_loaders_search_path_are_counted() { + trace(); + + let postgres = connect_with_tls(get_database_port()).await; + postgres + .batch_execute(&format!( + " + DROP SCHEMA IF EXISTS schema_loader_visible CASCADE; + DROP SCHEMA IF EXISTS schema_loader_hidden CASCADE; + DROP ROLE IF EXISTS {ROLE}; + CREATE SCHEMA schema_loader_visible; + CREATE SCHEMA schema_loader_hidden; + CREATE TABLE schema_loader_visible.notes (id bigint, body text); + CREATE TABLE schema_loader_hidden.secrets (id bigint, secret eql_v3_text_search); + CREATE ROLE {ROLE} LOGIN PASSWORD 'p@ssword'; + ALTER ROLE {ROLE} SET search_path = schema_loader_visible; + GRANT USAGE ON SCHEMA schema_loader_visible, schema_loader_hidden TO {ROLE}; + GRANT SELECT ON ALL TABLES IN SCHEMA schema_loader_visible TO {ROLE}; + GRANT SELECT ON ALL TABLES IN SCHEMA schema_loader_hidden TO {ROLE}; + " + )) + .await + .unwrap(); + + let config: DatabaseConfig = serde_json::from_value(serde_json::json!({ + "host": "localhost", + "port": get_database_port(), + "name": "cipherstash", + "username": ROLE, + "password": "p@ssword", + })) + .unwrap(); + let snapshot = SchemaManager::init(&config).await.unwrap().load(); + let has_encrypted_columns = + !snapshot.encrypt_config().is_empty() || snapshot.schema().has_encrypted_columns(); + + postgres + .batch_execute(&format!( + " + DROP SCHEMA schema_loader_visible CASCADE; + DROP SCHEMA schema_loader_hidden CASCADE; + DROP ROLE {ROLE}; + " + )) + .await + .unwrap(); + + assert!(has_encrypted_columns); + } +} diff --git a/packages/cipherstash-proxy/src/proxy/mod.rs b/packages/cipherstash-proxy/src/proxy/mod.rs index 43222968..31917b7e 100644 --- a/packages/cipherstash-proxy/src/proxy/mod.rs +++ b/packages/cipherstash-proxy/src/proxy/mod.rs @@ -5,7 +5,6 @@ use crate::{ connect, error::{ConfigError, Error, ErrorChain, TlsConfigError}, postgresql::{Column, Context, KeysetIdentifier}, - proxy::schema::SchemaManager, tls, }; use cipherstash_client::encryption::Plaintext; @@ -19,6 +18,8 @@ pub(crate) mod schema; mod zerokms; pub use encrypt_config::EncryptConfig; +/// The schema loader, for tests that run it against PostgreSQL. +pub use schema::SchemaManager; pub use zerokms::ZeroKms; pub type ReloadSender = UnboundedSender; From 2294057e424d78fff386b54ec9cb5bca5140e405 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 26 Oct 2026 05:09:36 +0000 Subject: [PATCH 4/7] refactor(proxy): flatten the SQL EXECUTE settings check Name the statements a SQL PREPARE and EXECUTE refer to with to_writing_preparation and to_execution, beside to_deallocation, so route reads as one check. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../src/postgresql/middleware/frontend.rs | 50 ++++++++++++------- 1 file changed, 31 insertions(+), 19 deletions(-) diff --git a/packages/cipherstash-proxy/src/postgresql/middleware/frontend.rs b/packages/cipherstash-proxy/src/postgresql/middleware/frontend.rs index 8dd90dcc..4ebefd09 100644 --- a/packages/cipherstash-proxy/src/postgresql/middleware/frontend.rs +++ b/packages/cipherstash-proxy/src/postgresql/middleware/frontend.rs @@ -1252,29 +1252,17 @@ impl Frontend { resolver: &Arc, ) -> Result, Error> { if self.context.unsafe_disable_mapping()? { - if let ast::Statement::Prepare { name, .. } = statement { - if eql_mapper::writes_data(statement) { - self.context - .set_unmapped_sql_statement(sql_statement_name(name))?; - } + if let Some(name) = to_writing_preparation(statement) { + self.context.set_unmapped_sql_statement(name)?; } return Ok(Route::Forward(Forward::MappingDisabled)); } // Like the Bind settings check, for a statement prepared with SQL. - if let ast::Statement::Execute { - name: Some(name), .. - } = statement - { - if let [part] = name.0.as_slice() { - if let Some(name) = part.as_ident() { - if self.context.has_encrypted_columns() - && self - .context - .is_unmapped_sql_statement(&sql_statement_name(name))? - { - return Err(MappingError::PreparedSettingsChanged.into()); - } - } + if let Some(name) = to_execution(statement) { + if self.context.has_encrypted_columns() + && self.context.is_unmapped_sql_statement(&name)? + { + return Err(MappingError::PreparedSettingsChanged.into()); } } if let Some((table_name, columns)) = eql_mapper::copies_into_table(statement) { @@ -1494,6 +1482,30 @@ fn to_deallocation(statement: &ast::Statement) -> Option { } } +/// The name of a SQL `PREPARE` that prepares a statement that writes data, if any +fn to_writing_preparation(statement: &ast::Statement) -> Option { + match statement { + ast::Statement::Prepare { name, .. } if eql_mapper::writes_data(statement) => { + Some(sql_statement_name(name)) + } + _ => None, + } +} + +/// The name of the SQL prepared statement an `EXECUTE` runs, if any +fn to_execution(statement: &ast::Statement) -> Option { + match statement { + ast::Statement::Execute { + name: Some(ast::ObjectName(parts)), + .. + } => match parts.as_slice() { + [ast::ObjectNamePart::Identifier(name)] => Some(sql_statement_name(name)), + _ => None, + }, + _ => None, + } +} + /// The name PostgreSQL gives a SQL prepared statement named `name`. fn sql_statement_name(name: &ast::Ident) -> Name { if name.quote_style.is_none() { From 22583cbb4c1506108a32744904cb3965b84dbb29 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 26 Oct 2026 05:13:42 +0000 Subject: [PATCH 5/7] fix(proxy): count encrypted columns off the loader's search path The schema query now reads every non-system schema and reports whether each table's schema is on the loader's search path. Only those tables are loaded, as before. Encrypted columns in the others set Schema::encrypted_columns_elsewhere, so has_encrypted_columns is true and a write to a table Proxy cannot resolve fails closed. Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 2 + .../src/proxy/schema/manager.rs | 40 +++++++++++++++++++ .../src/proxy/sql/select_table_schemas.sql | 24 ++++++----- packages/eql-mapper/src/model/schema.rs | 19 +++++---- 4 files changed, 69 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0c5a8610..f39cb211 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -47,6 +47,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### Fixed +- **Encrypted tables off Proxy's search path**: Proxy now counts encrypted columns in schemas that are not on its database user's search path. When every encrypted table was in such a schema, Proxy saw no encrypted columns, so a write to a table it could not resolve was forwarded as written instead of refused. Tables in those schemas are still not loaded. + - **SQL `EXECUTE` after `UNSAFE_DISABLE_MAPPING`**: Proxy now refuses `EXECUTE` of a statement that writes data and was prepared with SQL `PREPARE` while `CIPHERSTASH.UNSAFE_DISABLE_MAPPING` was on, once mapping is enabled again. It was forwarded with its arguments as written, so only PostgreSQL's type check stopped plaintext reaching an encrypted column. `DEALLOCATE` the statement and prepare it again. This matches the check Proxy already makes for statements prepared with the extended protocol. - **`--db-user`**: the flag now sets the database username. It previously set an unused variable, so it had no effect. diff --git a/packages/cipherstash-proxy/src/proxy/schema/manager.rs b/packages/cipherstash-proxy/src/proxy/schema/manager.rs index 9ff56acf..c34ab32d 100644 --- a/packages/cipherstash-proxy/src/proxy/schema/manager.rs +++ b/packages/cipherstash-proxy/src/proxy/schema/manager.rs @@ -348,6 +348,13 @@ fn is_legacy_eql_v2(column_type_name: Option<&str>, column_domain_name: Option<& || column_domain_name == Some(EQL_V2_ENCRYPTED_TYPE) } +/// Whether a column is encrypted, as [`classify_column`] decides, without +/// logging: for a table that is not loaded. +fn is_encrypted(column_type_name: Option<&str>, column_domain_name: Option<&str>) -> bool { + column_domain_name.and_then(eql_domains::resolve).is_some() + || is_legacy_eql_v2(column_type_name, column_domain_name) +} + /// Decides what a single catalog row means for the type checker. /// /// Split out from [`load_schema`] so the classification — the security-relevant @@ -389,6 +396,9 @@ fn classify_column( /// One row of [`SCHEMA_QUERY`]: a table and its columns. struct TableRow { table_schema: String, + /// Whether `table_schema` is on the loader's search path. Only those + /// tables are loaded. + on_search_path: bool, table_name: String, columns: Vec, column_type_names: Vec>, @@ -405,6 +415,18 @@ fn build_snapshot(tables: impl IntoIterator) -> (Schema, Encryp let mut conflicting: HashSet = HashSet::new(); for row in tables { + if !row.on_search_path { + // Not loaded, but its encrypted columns still count, so that a + // write to a table Proxy cannot resolve fails closed. + schema.encrypted_columns_elsewhere |= row + .column_type_names + .iter() + .zip(&row.column_domain_names) + .any(|(column_type_name, column_domain_name)| { + is_encrypted(column_type_name.as_deref(), column_domain_name.as_deref()) + }); + continue; + } let table_name = row.table_name; let mut table = Table::in_schema( Ident::with_quote('"', &row.table_schema), @@ -475,6 +497,7 @@ async fn load_snapshot(config: &DatabaseConfig) -> Result<(Schema, EncryptConfig let (mut schema, encrypt_config) = build_snapshot(tables.into_iter().map(|table| TableRow { table_schema: table.get("table_schema"), + on_search_path: table.get("on_search_path"), table_name: table.get("table_name"), columns: table.get("columns"), column_type_names: table.get("column_type_names"), @@ -565,6 +588,7 @@ mod test { fn users_row(table_schema: &str, email_domain: Option<&str>) -> TableRow { TableRow { table_schema: table_schema.to_owned(), + on_search_path: true, table_name: "users".to_owned(), columns: vec!["id".to_owned(), "email".to_owned()], column_type_names: vec![Some("int4".to_owned()), Some("jsonb".to_owned())], @@ -572,6 +596,22 @@ mod test { } } + /// A table off the loader's search path is not loaded, but its encrypted + /// columns still count. + #[test] + fn table_off_the_search_path_is_not_loaded_but_its_encrypted_columns_count() { + for (domain, expected) in [(Some("eql_v3_text_search"), true), (None, false)] { + let (schema, encrypt_config) = build_snapshot([TableRow { + on_search_path: false, + ..users_row("hidden", domain) + }]); + + assert!(schema.tables.is_empty()); + assert!(encrypt_config.is_empty()); + assert_eq!(schema.has_encrypted_columns(), expected, "{domain:?}"); + } + } + fn object_name(parts: &[&str]) -> sqltk::parser::ast::ObjectName { sqltk::parser::ast::ObjectName( parts diff --git a/packages/cipherstash-proxy/src/proxy/sql/select_table_schemas.sql b/packages/cipherstash-proxy/src/proxy/sql/select_table_schemas.sql index c6a2f62a..7eda93d4 100644 --- a/packages/cipherstash-proxy/src/proxy/sql/select_table_schemas.sql +++ b/packages/cipherstash-proxy/src/proxy/sql/select_table_schemas.sql @@ -1,5 +1,6 @@ SELECT t.table_schema, + t.table_schema::text = ANY (current_schemas(false)::text[]) AS on_search_path, t.table_name, array_agg(c.column_name)::text[] AS columns, array_agg(c.udt_name)::text[] AS column_type_names, @@ -18,21 +19,26 @@ LEFT JOIN AND c.table_name = t.table_name WHERE t.table_type = 'BASE TABLE' - -- Only the schemas this connection resolves unqualified names in. + -- Every schema but the system ones (`pg_` is reserved for them). -- - -- The schema map records each table's schema, but the encrypt config is - -- keyed on the bare table name, so a same-named table in another schema - -- (another tenant's, a staging copy, a leftover eql_v2) would share or - -- conflict with the served table's column config. + -- Only tables `on_search_path`, the schemas this connection resolves + -- unqualified names in, are loaded. The schema map records each table's + -- schema, but the encrypt config is keyed on the bare table name, so a + -- same-named table in another schema (another tenant's, a staging copy, a + -- leftover eql_v2) would share or conflict with the served table's column + -- config. `false` excludes the implicitly-searched pg_catalog, leaving + -- exactly the schemas an unqualified `users` could mean. -- - -- `false` excludes the implicitly-searched pg_catalog, leaving exactly the - -- schemas an unqualified `users` could mean. - AND t.table_schema::text = ANY (current_schemas(false)::text[]) + -- Tables in other schemas only say whether the database has encrypted + -- columns at all, so that a write to a table Proxy cannot resolve fails + -- closed even when every encrypted table is off the search path. + AND t.table_schema::text NOT LIKE 'pg\_%' + AND t.table_schema::text <> 'information_schema' GROUP BY t.table_schema, t.table_name ORDER BY -- Search-path order, so rows for a name come in the order PostgreSQL - -- would resolve them. Consumers keep every row: a bare name found in two + -- would resolve them; tables off the search path come last. Consumers keep every row: a bare name found in two -- schemas stays ambiguous, and qualified names resolve to their schema. array_position(current_schemas(false)::text[], t.table_schema::text), t.table_name; diff --git a/packages/eql-mapper/src/model/schema.rs b/packages/eql-mapper/src/model/schema.rs index 4f983136..a2306b2a 100644 --- a/packages/eql-mapper/src/model/schema.rs +++ b/packages/eql-mapper/src/model/schema.rs @@ -21,6 +21,9 @@ pub struct Schema { pub name: Ident, pub tables: Vec>, pub aggregates: Vec>, + /// Whether the database has encrypted columns in tables this schema does + /// not hold, such as tables in schemas off the loader's search path. + pub encrypted_columns_elsewhere: bool, } /// A table (or view). @@ -142,14 +145,15 @@ pub enum UnresolvedTable<'a> { impl Schema { /// Whether any column holds encrypted data, including one this build - /// cannot map. + /// cannot map and one in a table this schema does not hold. pub fn has_encrypted_columns(&self) -> bool { - self.tables.iter().any(|table| { - table - .columns - .iter() - .any(|column| column.kind != ColumnKind::Native) - }) + self.encrypted_columns_elsewhere + || self.tables.iter().any(|table| { + table + .columns + .iter() + .any(|column| column.kind != ColumnKind::Native) + }) } /// Creates a new named empty schema. @@ -163,6 +167,7 @@ impl Schema { name, tables: Default::default(), aggregates: Default::default(), + encrypted_columns_elsewhere: false, } } From 3b04f64e3c70c634f07b57f735e0ad61c88c45a5 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 26 Oct 2026 06:32:27 +0000 Subject: [PATCH 6/7] fix(proxy): keep unmapped SQL preparations that follow a deallocation Routing records a SQL PREPARE before PostgreSQL runs the statements of the Query, so a DEALLOCATE earlier in the same Query, confirmed later, removed the new preparation's record and let EXECUTE through. Each unmapped preparation now records the operation that ran it, and a confirmed deallocation removes only names recorded by an earlier operation. A PREPARE parsed with the extended protocol is recorded again when its portal is executed, so a pipelined deallocation executed before it does not remove it either. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../src/postgresql/context/mod.rs | 79 ++++-- .../postgresql/context/prepared_statements.rs | 12 + .../src/postgresql/middleware/frontend.rs | 252 +++++++++++++----- 3 files changed, 255 insertions(+), 88 deletions(-) diff --git a/packages/cipherstash-proxy/src/postgresql/context/mod.rs b/packages/cipherstash-proxy/src/postgresql/context/mod.rs index 3ebe8049..248f4bcf 100644 --- a/packages/cipherstash-proxy/src/postgresql/context/mod.rs +++ b/packages/cipherstash-proxy/src/postgresql/context/mod.rs @@ -32,7 +32,7 @@ use pg_proto::{Describe, DescribeTarget, DiagnosticResponse, TransactionStatus}; use serde_json::json; pub use statement_metadata::StatementMetadata; use std::{ - collections::{HashMap, HashSet, VecDeque}, + collections::{HashMap, VecDeque}, sync::{ atomic::{AtomicU64, Ordering}, Arc, RwLock, @@ -161,10 +161,11 @@ struct ConnectionProtocolState { portals: HashMap>, portal_operations: HashMap>, /// SQL `PREPARE` statements that write data, forwarded while mapping was - /// disabled. They were never mapped, so `EXECUTE` of one is refused once - /// mapping is enabled again. A name leaves only when PostgreSQL confirms a - /// `DEALLOCATE` or `DISCARD ALL` that removes it. - unmapped_sql_statements: HashSet, + /// disabled, with the latest operation that ran each one. They were never + /// mapped, so `EXECUTE` of one is refused once mapping is enabled again. A + /// name leaves only when PostgreSQL confirms a `DEALLOCATE` or + /// `DISCARD ALL` that removes it, run by a later operation. + unmapped_sql_statements: HashMap, } /// The Bind operation that created a portal, and the SQL deallocation it runs when executed @@ -172,6 +173,8 @@ struct ConnectionProtocolState { pub(crate) struct PortalOwner { operation: K, deallocation: Option, + /// The unmapped SQL prepared statement it prepares when executed + preparation: Option, } impl Default for ConnectionProtocolState { @@ -185,7 +188,7 @@ impl Default for ConnectionProtocolState { prepared_statements: PreparedStatements::default(), portals: HashMap::new(), portal_operations: HashMap::new(), - unmapped_sql_statements: HashSet::new(), + unmapped_sql_statements: HashMap::new(), } } } @@ -213,6 +216,7 @@ impl ConnectionProtocolState { PortalOwner { operation, deallocation: None, + preparation: None, }, ); self.portals.insert(name, Arc::new(portal)); @@ -273,6 +277,16 @@ impl ConnectionProtocolState { } impl ConnectionProtocolState { + fn set_unmapped_sql_statement(&mut self, name: Name, operation: OperationId) { + let prepared = self + .unmapped_sql_statements + .entry(name) + .or_insert(operation); + if *prepared < operation { + *prepared = operation; + } + } + /// Everything Proxy keeps for the named statement, for a rollback to restore fn statement_snapshot(&self, name: &Name) -> settings_protocol::MetadataSnapshot { settings_protocol::MetadataSnapshot { @@ -701,10 +715,14 @@ where state.statement_metrics.insert(execution_id, metrics); ExecuteContext::new(name.clone(), portal, Some(execution_id)) }); - let deallocation = state + let (deallocation, preparation) = state .portal_operations .get(&name) - .and_then(|owner| owner.deallocation.clone()); + .map(|owner| (owner.deallocation.clone(), owner.preparation.clone())) + .unwrap_or_default(); + if let Some(prepares) = preparation { + state.set_unmapped_sql_statement(prepares, operation); + } let context = state.operations.entry(operation).or_default(); context.execute = Some(execute); context.deallocations.extend(deallocation); @@ -816,14 +834,29 @@ where Ok(()) } - /// Record that the SQL prepared statement `name`, which writes data, was - /// forwarded without mapping. - pub fn set_unmapped_sql_statement(&self, name: Name) -> Result<(), Error> { + /// Record that `operation` forwards the SQL prepared statement `name`, + /// which writes data, without mapping. + pub fn set_unmapped_sql_statement( + &self, + name: Name, + operation: OperationId, + ) -> Result<(), Error> { self.protocol_state .write() .map_err(|_| crate::error::ContextError::ProtocolStateUnavailable)? - .unmapped_sql_statements - .insert(name); + .set_unmapped_sql_statement(name, operation); + Ok(()) + } + + /// Record that the SQL of statement `name` is a `PREPARE` of `prepares`, an + /// unmapped statement that writes data, so that executing it through the + /// extended protocol records `prepares` in execution order. + pub fn set_statement_preparation(&self, name: Name, prepares: Name) -> Result<(), Error> { + self.protocol_state + .write() + .map_err(|_| crate::error::ContextError::ProtocolStateUnavailable)? + .prepared_statements + .set_preparation(name, prepares); Ok(()) } @@ -835,18 +868,21 @@ where .read() .map_err(|_| crate::error::ContextError::ProtocolStateUnavailable)? .unmapped_sql_statements - .contains(name)) + .contains_key(name)) } - /// Carry the deallocation of `statement`, if any, onto the portal just bound from it. + /// Carry the deallocation and preparation of `statement`, if any, onto the + /// portal just bound from it. pub fn bind_statement_effects(&self, portal: &Name, statement: &Name) -> Result<(), Error> { let mut state = self .protocol_state .write() .map_err(|_| crate::error::ContextError::ProtocolStateUnavailable)?; let deallocation = state.prepared_statements.deallocation(statement); + let preparation = state.prepared_statements.preparation(statement); if let Some(owner) = state.portal_operations.get_mut(portal) { owner.deallocation = deallocation; + owner.preparation = preparation; } Ok(()) } @@ -916,11 +952,20 @@ where debug!(target: CONTEXT, client_id = self.client_id, msg = "Deallocate", ?deallocation); + // Not a statement a later operation prepared again match &deallocation { Deallocation::Statement(name) => { - state.unmapped_sql_statements.remove(name); + if state + .unmapped_sql_statements + .get(name) + .is_some_and(|prepared| *prepared < operation) + { + state.unmapped_sql_statements.remove(name); + } } - Deallocation::AllStatements => state.unmapped_sql_statements.clear(), + Deallocation::AllStatements => state + .unmapped_sql_statements + .retain(|_, prepared| *prepared >= operation), } let names = state.prepared_statements.deallocation_names(deallocation); diff --git a/packages/cipherstash-proxy/src/postgresql/context/prepared_statements.rs b/packages/cipherstash-proxy/src/postgresql/context/prepared_statements.rs index c9fc8609..62c8b082 100644 --- a/packages/cipherstash-proxy/src/postgresql/context/prepared_statements.rs +++ b/packages/cipherstash-proxy/src/postgresql/context/prepared_statements.rs @@ -12,6 +12,9 @@ struct PreparedStatement { parse: Option, /// Set when the statement's SQL is a `DEALLOCATE` or `DISCARD ALL` deallocation: Option, + /// Set when the statement's SQL is a `PREPARE` of an unmapped statement + /// that writes data: the name it prepares + preparation: Option, } impl Default for PreparedStatement { @@ -21,6 +24,7 @@ impl Default for PreparedStatement { metrics_scope: None, parse: None, deallocation: None, + preparation: None, } } } @@ -82,6 +86,14 @@ where self.statements.entry(name).or_default().deallocation = Some(deallocation); } + pub fn preparation(&self, name: &Name) -> Option { + self.statements.get(name)?.preparation.clone() + } + + pub fn set_preparation(&mut self, name: Name, prepares: Name) { + self.statements.entry(name).or_default().preparation = Some(prepares); + } + /// Remove the statement, returning its metrics scope. pub fn remove(&mut self, name: &Name) -> Option { self.statements.remove(name)?.metrics_scope diff --git a/packages/cipherstash-proxy/src/postgresql/middleware/frontend.rs b/packages/cipherstash-proxy/src/postgresql/middleware/frontend.rs index 4ebefd09..c84fc559 100644 --- a/packages/cipherstash-proxy/src/postgresql/middleware/frontend.rs +++ b/packages/cipherstash-proxy/src/postgresql/middleware/frontend.rs @@ -425,6 +425,11 @@ impl Frontend { .filter_map(to_deallocation) .collect(), )?; + if self.context.unsafe_disable_mapping()? { + for name in parsed_statements.iter().filter_map(to_writing_preparation) { + self.context.set_unmapped_sql_statement(name, operation)?; + } + } self.context.prepare_schema_for_statement().await?; if self .context @@ -819,6 +824,15 @@ impl Frontend { self.context .set_statement_deallocation(message.statement.to_owned(), deallocation)?; } + if self.context.unsafe_disable_mapping()? { + if let Some(prepares) = to_writing_preparation(&statement) { + // Recorded again when it is executed, in execution order + self.context + .set_unmapped_sql_statement(prepares.clone(), operation)?; + self.context + .set_statement_preparation(message.statement.to_owned(), prepares)?; + } + } self.context .prepare_parse(operation, &statement, &statement_text) @@ -1252,9 +1266,6 @@ impl Frontend { resolver: &Arc, ) -> Result, Error> { if self.context.unsafe_disable_mapping()? { - if let Some(name) = to_writing_preparation(statement) { - self.context.set_unmapped_sql_statement(name)?; - } return Ok(Route::Forward(Forward::MappingDisabled)); } // Like the Bind settings check, for a statement prepared with SQL. @@ -2158,7 +2169,7 @@ mod tests { frontend .intercept( operation_id(), - FrontendMessage::Bind(Bind { + FrontendMessage::Bind(pg_proto::Bind { portal: name.clone(), statement: name.clone(), parameter_formats: vec![], @@ -2244,7 +2255,7 @@ mod tests { let output = frontend .intercept( operation_id(), - FrontendMessage::Bind(Bind { + FrontendMessage::Bind(pg_proto::Bind { portal: name.clone(), statement: name.clone(), parameter_formats: vec![], @@ -2290,7 +2301,7 @@ mod tests { frontend .intercept( bind, - FrontendMessage::Bind(Bind { + FrontendMessage::Bind(pg_proto::Bind { portal: bytes::Bytes::copy_from_slice(name), statement: bytes::Bytes::copy_from_slice(name), parameter_formats: vec![], @@ -2442,7 +2453,7 @@ mod tests { let output = frontend .intercept( bind, - FrontendMessage::Bind(Bind { + FrontendMessage::Bind(pg_proto::Bind { portal: name.clone(), statement: name.clone(), parameter_formats: vec![], @@ -2614,7 +2625,7 @@ mod tests { let output = frontend .intercept( bind, - FrontendMessage::Bind(Bind { + FrontendMessage::Bind(pg_proto::Bind { portal: name.clone(), statement: name.clone(), parameter_formats: vec![], @@ -2710,7 +2721,7 @@ mod tests { frontend .intercept( bind, - FrontendMessage::Bind(Bind { + FrontendMessage::Bind(pg_proto::Bind { portal: name.clone(), statement: name.clone(), parameter_formats: vec![], @@ -2786,7 +2797,7 @@ mod tests { frontend .intercept( operation_id(), - FrontendMessage::Bind(Bind { + FrontendMessage::Bind(pg_proto::Bind { portal: name.clone(), statement: name.clone(), parameter_formats: Vec::new(), @@ -2942,7 +2953,7 @@ mod tests { frontend .intercept( operation_id(), - FrontendMessage::Bind(Bind { + FrontendMessage::Bind(pg_proto::Bind { portal: portal.clone(), statement: statement.clone(), parameter_formats: Vec::new(), @@ -2992,7 +3003,7 @@ mod tests { frontend .intercept( operation_id(), - FrontendMessage::Bind(Bind { + FrontendMessage::Bind(pg_proto::Bind { portal, statement: statement.clone(), parameter_formats: Vec::new(), @@ -3080,7 +3091,7 @@ mod tests { frontend .intercept( operation_id(), - FrontendMessage::Bind(Bind { + FrontendMessage::Bind(pg_proto::Bind { portal: bytes::Bytes::from_static(b"portal"), statement: statement_name, parameter_formats: Vec::new(), @@ -3131,7 +3142,7 @@ mod tests { frontend .intercept( operation, - FrontendMessage::Bind(Bind { + FrontendMessage::Bind(pg_proto::Bind { portal: bytes::Bytes::from_static(b"portal"), statement: bytes::Bytes::from_static(b"statement"), parameter_formats: Vec::new(), @@ -3183,7 +3194,7 @@ mod tests { frontend .intercept( operation_id(), - FrontendMessage::Bind(Bind { + FrontendMessage::Bind(pg_proto::Bind { portal: portal.clone(), statement: bytes::Bytes::from_static(b"unknown_statement"), parameter_formats: Vec::new(), @@ -4402,25 +4413,29 @@ mod unresolvable_table_tests { } } - /// A statement prepared with SQL `PREPARE` while mapping is disabled was - /// never mapped, so executing it once mapping is enabled again would write - /// its arguments as written. - #[tokio::test] - async fn sql_execute_of_a_statement_prepared_without_mapping_is_refused() { + /// A connection with encrypted columns that may set `UNSAFE_DISABLE_MAPPING`. + fn unsafe_mapping_client() -> ( + Frontend, + crate::postgresql::middleware::Backend, + ) { let mut config = TandemConfig::for_testing(); config.disable_mapping_for_testing(); config.development.as_mut().unwrap().disable_mapping = false; let (context, _) = client_context(config, true); - let mut frontend = Frontend::new(context.clone()); - let mut backend = crate::postgresql::middleware::Backend::new(context); - for (sql, tag) in [ - ("SET CIPHERSTASH.UNSAFE_DISABLE_MAPPING = true", "SET"), - ( - "PREPARE p (text) AS INSERT INTO encrypted (secret) VALUES ($1)", - "PREPARE", - ), - ("SET CIPHERSTASH.UNSAFE_DISABLE_MAPPING = false", "SET"), - ] { + ( + Frontend::new(context.clone()), + crate::postgresql::middleware::Backend::new(context), + ) + } + + /// Sends each simple Query, and completes each of its statements in + /// PostgreSQL with the given command tags. + async fn complete_queries( + frontend: &mut Frontend, + backend: &mut crate::postgresql::middleware::Backend, + queries: &[(&str, &[&str])], + ) { + for (sql, tags) in queries { let operation = operation_id(); frontend .intercept( @@ -4429,15 +4444,17 @@ mod unresolvable_table_tests { ) .await .unwrap(); - backend - .intercept( - Some(operation), - pg_proto::BackendMessage::CommandComplete(Bytes::copy_from_slice( - tag.as_bytes(), - )), - ) - .await - .unwrap(); + for tag in *tags { + backend + .intercept( + Some(operation), + pg_proto::BackendMessage::CommandComplete(Bytes::copy_from_slice( + tag.as_bytes(), + )), + ) + .await + .unwrap(); + } backend .intercept( Some(operation), @@ -4446,6 +4463,34 @@ mod unresolvable_table_tests { .await .unwrap(); } + } + + const UNSAFE_ON: (&str, &[&str]) = ("SET CIPHERSTASH.UNSAFE_DISABLE_MAPPING = true", &["SET"]); + const UNSAFE_OFF: (&str, &[&str]) = + ("SET CIPHERSTASH.UNSAFE_DISABLE_MAPPING = false", &["SET"]); + const PREPARE_INSERT: &str = "PREPARE p (text) AS INSERT INTO encrypted (secret) VALUES ($1)"; + + fn assert_execute_refused(forwarded: &str) { + assert!( + forwarded.contains("RAISE EXCEPTION") + && forwarded.contains("no longer match confirmed connection state") + && !forwarded.contains("plaintext"), + "{forwarded}" + ); + } + + /// A statement prepared with SQL `PREPARE` while mapping is disabled was + /// never mapped, so executing it once mapping is enabled again would write + /// its arguments as written. + #[tokio::test] + async fn sql_execute_of_a_statement_prepared_without_mapping_is_refused() { + let (mut frontend, mut backend) = unsafe_mapping_client(); + complete_queries( + &mut frontend, + &mut backend, + &[UNSAFE_ON, (PREPARE_INSERT, &["PREPARE"]), UNSAFE_OFF], + ) + .await; let reprepared = query( &mut frontend, "PREPARE q (text) AS INSERT INTO encrypted (secret) VALUES ($1)", @@ -4455,58 +4500,123 @@ mod unresolvable_table_tests { reprepared.contains("RAISE EXCEPTION"), "mapping is still disabled: {reprepared}" ); - let forwarded = query(&mut frontend, "EXECUTE p('plaintext')").await; - assert!( - forwarded.contains("RAISE EXCEPTION") - && forwarded.contains("no longer match confirmed connection state") - && !forwarded.contains("plaintext"), - "{forwarded}" - ); + assert_execute_refused(&query(&mut frontend, "EXECUTE p('plaintext')").await); } - /// Once PostgreSQL confirms the statement is gone, its name is free again. + /// A deallocation removes only statements prepared before it, even when a + /// later statement in the same Query prepares the name again. #[tokio::test] - async fn sql_execute_after_deallocating_a_statement_prepared_without_mapping_is_forwarded() { - let mut config = TandemConfig::for_testing(); - config.disable_mapping_for_testing(); - config.development.as_mut().unwrap().disable_mapping = false; - let (context, _) = client_context(config, true); - let mut frontend = Frontend::new(context.clone()); - let mut backend = crate::postgresql::middleware::Backend::new(context); - for (sql, tag) in [ - ("SET CIPHERSTASH.UNSAFE_DISABLE_MAPPING = true", "SET"), - ( - "PREPARE p (text) AS INSERT INTO encrypted (secret) VALUES ($1)", - "PREPARE", - ), + async fn sql_execute_of_a_statement_prepared_after_a_deallocation_in_the_same_query_is_refused() + { + for (deallocation, tag) in [ + ("DEALLOCATE ALL", "DEALLOCATE ALL"), ("DEALLOCATE p", "DEALLOCATE"), - ("SET CIPHERSTASH.UNSAFE_DISABLE_MAPPING = false", "SET"), ] { - let operation = operation_id(); + let (mut frontend, mut backend) = unsafe_mapping_client(); + let both = format!("{deallocation}; {PREPARE_INSERT}"); + complete_queries( + &mut frontend, + &mut backend, + &[ + UNSAFE_ON, + (PREPARE_INSERT, &["PREPARE"]), + (&both, &[tag, "PREPARE"]), + UNSAFE_OFF, + ], + ) + .await; + assert_execute_refused(&query(&mut frontend, "EXECUTE p('plaintext')").await); + } + } + + /// The extended protocol runs a `PREPARE` when its portal is executed, so + /// a pipelined deallocation executed before it does not remove it. + #[tokio::test] + async fn sql_execute_of_a_statement_prepared_after_a_pipelined_deallocation_is_refused() { + let (mut frontend, mut backend) = unsafe_mapping_client(); + complete_queries(&mut frontend, &mut backend, &[UNSAFE_ON]).await; + for (name, sql) in [("d", "DEALLOCATE ALL"), ("pp", PREPARE_INSERT)] { frontend .intercept( - operation, - FrontendMessage::Query(Bytes::copy_from_slice(sql.as_bytes())), + operation_id(), + FrontendMessage::Parse(Parse { + statement: Bytes::copy_from_slice(name.as_bytes()), + query: Bytes::copy_from_slice(sql.as_bytes()), + parameter_types: vec![], + }), ) .await .unwrap(); - backend + } + let mut executions = vec![]; + for name in ["d", "pp"] { + let name = Bytes::copy_from_slice(name.as_bytes()); + frontend .intercept( - Some(operation), - pg_proto::BackendMessage::CommandComplete(Bytes::copy_from_slice( - tag.as_bytes(), - )), + operation_id(), + FrontendMessage::Bind(pg_proto::Bind { + portal: name.clone(), + statement: name.clone(), + parameter_formats: vec![], + parameters: vec![], + result_formats: vec![], + }), + ) + .await + .unwrap(); + let execute = operation_id(); + frontend + .intercept( + execute, + FrontendMessage::Execute(pg_proto::Execute { + portal: name, + max_rows: 0, + }), ) .await .unwrap(); + executions.push(execute); + } + for (execute, tag) in executions.into_iter().zip(["DEALLOCATE ALL", "PREPARE"]) { backend .intercept( - Some(operation), - pg_proto::BackendMessage::ReadyForQuery(pg_proto::TransactionStatus::Idle), + Some(execute), + pg_proto::BackendMessage::CommandComplete(Bytes::from_static(tag.as_bytes())), ) .await .unwrap(); } + let sync = operation_id(); + frontend + .intercept(sync, FrontendMessage::Sync) + .await + .unwrap(); + backend + .intercept( + Some(sync), + pg_proto::BackendMessage::ReadyForQuery(pg_proto::TransactionStatus::Idle), + ) + .await + .unwrap(); + complete_queries(&mut frontend, &mut backend, &[UNSAFE_OFF]).await; + assert_execute_refused(&query(&mut frontend, "EXECUTE p('plaintext')").await); + } + + /// Once PostgreSQL confirms the statement is gone, its name is free again. + #[tokio::test] + async fn sql_execute_after_deallocating_a_statement_prepared_without_mapping_is_forwarded() { + let (mut frontend, mut backend) = unsafe_mapping_client(); + complete_queries( + &mut frontend, + &mut backend, + &[ + UNSAFE_ON, + (PREPARE_INSERT, &["PREPARE"]), + ("DEALLOCATE p", &["DEALLOCATE"]), + UNSAFE_OFF, + ], + ) + .await; assert_eq!(query(&mut frontend, "EXECUTE p(1)").await, "EXECUTE p(1)"); } From dfcb8c1c6988a8cef00ca06b9502bd640611c0bd Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 26 Oct 2026 22:41:58 +0000 Subject: [PATCH 7/7] fix(proxy): refuse EXPLAIN of a SQL EXECUTE prepared without mapping EXPLAIN ANALYZE runs the statement it explains, so EXPLAIN ANALYZE EXECUTE of a statement prepared while UNSAFE_DISABLE_MAPPING was on wrote its arguments as written. to_execution now looks through EXPLAIN. Co-Authored-By: Claude Opus 5.5 --- .../src/postgresql/middleware/frontend.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/cipherstash-proxy/src/postgresql/middleware/frontend.rs b/packages/cipherstash-proxy/src/postgresql/middleware/frontend.rs index c84fc559..9befad26 100644 --- a/packages/cipherstash-proxy/src/postgresql/middleware/frontend.rs +++ b/packages/cipherstash-proxy/src/postgresql/middleware/frontend.rs @@ -1503,9 +1503,11 @@ fn to_writing_preparation(statement: &ast::Statement) -> Option { } } -/// The name of the SQL prepared statement an `EXECUTE` runs, if any +/// The name of the SQL prepared statement an `EXECUTE` runs, if any, including +/// one under `EXPLAIN`, which runs it with `ANALYZE` fn to_execution(statement: &ast::Statement) -> Option { match statement { + ast::Statement::Explain { statement, .. } => to_execution(statement), ast::Statement::Execute { name: Some(ast::ObjectName(parts)), .. @@ -4501,6 +4503,10 @@ mod unresolvable_table_tests { "mapping is still disabled: {reprepared}" ); assert_execute_refused(&query(&mut frontend, "EXECUTE p('plaintext')").await); + // `EXPLAIN ANALYZE` runs the statement it explains + assert_execute_refused( + &query(&mut frontend, "EXPLAIN ANALYZE EXECUTE p('plaintext')").await, + ); } /// A deallocation removes only statements prepared before it, even when a