From 7675c99204d8f784a2f6070385110370faf65b6e Mon Sep 17 00:00:00 2001 From: James Sadler Date: Sat, 22 Aug 2026 22:23:39 +1000 Subject: [PATCH 1/4] docs(proxy): define transactional schema lifecycle Record the BUG-308 design for connection-local DDL visibility and authoritative schema publication. Define atomic committed snapshots, transaction overlays, savepoint behavior, protocol deferral, fail-closed reload handling, and the boundary of the standalone schema middleware. Signed-off-by: James Sadler --- packages/cipherstash-proxy/CONTEXT.md | 35 +++++++- ...001-transaction-aware-schema-middleware.md | 87 +++++++++++++++++++ 2 files changed, 120 insertions(+), 2 deletions(-) create mode 100644 packages/cipherstash-proxy/docs/adr/0001-transaction-aware-schema-middleware.md diff --git a/packages/cipherstash-proxy/CONTEXT.md b/packages/cipherstash-proxy/CONTEXT.md index 9289b0a09..70acc4dd3 100644 --- a/packages/cipherstash-proxy/CONTEXT.md +++ b/packages/cipherstash-proxy/CONTEXT.md @@ -103,8 +103,39 @@ Proxy's in-band control API, intercepted rather than forwarded — `KEYSET_ID`, that print `CIPHERSTASH.DISABLE_MAPPING` are wrong. **Reload**: -Re-reading state from the database after observed DDL. Two independent things reload: the -database schema, and the column encrypt config. +Re-reading authoritative schema state from PostgreSQL after observed DDL. A reload produces +one **committed schema snapshot**; it does not merge Proxy's inferred DDL effects into shared +state. + +**Committed schema snapshot**: +An immutable, monotonically versioned pair of database structure and column encryption +metadata loaded from PostgreSQL. The pair is published atomically because a table without its +encryption policy (or an encryption policy without its table) is not a valid observable state. + +**Transaction schema overlay**: +The confirmed effects of successful DDL executions in one connection's current transaction. +It is checkpointed by savepoints, restored by `ROLLBACK TO SAVEPOINT`, and discarded by a full +rollback. Parsed or prepared DDL is only intent; it enters the overlay after PostgreSQL reports +successful execution. + +**Effective schema**: +The committed schema snapshot pinned when a transaction starts, with that transaction's schema +overlay applied. EQL Mapper type-checks and transforms against this view. An idle connection +adopts the latest committed snapshot before its next transaction. + +**Schema publication**: +Atomically replacing the shared committed schema snapshot after the outermost transaction +containing DDL commits and an authoritative catalog reload succeeds. Proxy completes publication +before forwarding `ReadyForQuery(I)`, so a connection opened after readiness observes the new +schema and encryption metadata. Failed publication is fail-closed: the affected connection is +closed without forwarding readiness, and the dirty publication remains eligible for retry. + +**Schema middleware**: +The owner of transactional schema state. Frontend and Backend report protocol lifecycle events; +they do not directly change overlays, dirty flags, or reload managers. The middleware owns DDL +detection, prepared DDL effects, successful-execution activation, savepoint and transaction +transitions, effective-schema resolution, reload coordination, and schema publication. See +`docs/adr/0001-transaction-aware-schema-middleware.md`. ## Note on `session` diff --git a/packages/cipherstash-proxy/docs/adr/0001-transaction-aware-schema-middleware.md b/packages/cipherstash-proxy/docs/adr/0001-transaction-aware-schema-middleware.md new file mode 100644 index 000000000..416cf960a --- /dev/null +++ b/packages/cipherstash-proxy/docs/adr/0001-transaction-aware-schema-middleware.md @@ -0,0 +1,87 @@ +--- +status: accepted +issue: BUG-308 +--- + +# Transaction-aware schema middleware + +## Context + +Proxy must map a statement using the schema PostgreSQL makes visible to that statement. DDL is +transactional: its effects become visible inside the transaction after successful execution, +but other connections cannot observe them until the outermost transaction commits. + +The existing design marks schema change while parsing SQL and reloads through a separate database +connection. In the extended protocol, a client's preparation `Sync` can consume that marker before +the DDL is executed. Even moving the reload to every `ReadyForQuery` is insufficient: a +`ReadyForQuery(T)` occurs inside an explicit transaction, where the loader connection still cannot +see uncommitted DDL. Schema and column encryption configuration are also loaded and published +separately, while existing connections retain snapshots taken when their contexts were created. + +These behaviours can make later connections use stale mapping or encryption metadata. They also +make connection-local behaviour depend on speculative DDL inferred at `Parse`, even if execution +later fails. + +## Decision + +Introduce standalone schema middleware as the sole owner of the transactional schema lifecycle. +Frontend and Backend report protocol events to it; neither manipulates schema-change flags or +reload managers directly. + +### State model + +- A **committed schema snapshot** is immutable and monotonically versioned. It contains database + structure and encryption metadata derived from EQL domain types as one atomic value. +- A transaction pins the current committed snapshot. Existing idle connections adopt the latest + snapshot before starting their next transaction. +- A **transaction schema overlay** contains only effects confirmed by successful DDL execution. + The connection's **effective schema** is its pinned snapshot plus this overlay. +- Savepoints checkpoint the overlay. `ROLLBACK TO SAVEPOINT` restores its checkpoint, full rollback + discards it, and release preserves its effects in the enclosing transaction. +- Parsed or prepared DDL records intent with the prepared statement. Each successful execution + applies its effect; `Parse` alone never changes schema state. +- If a successful DDL cannot be modelled accurately, later schema-dependent statements in that + transaction fail closed. + +### Protocol ordering + +After a DDL `Execute` is forwarded, protocol-control messages required to complete that execution +continue to flow, but later schema-dependent operations wait until its success or failure is known. +This avoids both speculative mapping and a deadlock in the extended-protocol prepare flow. + +A simple-query message containing DDL followed by a schema-dependent statement fails closed for +the initial implementation. Supporting that case requires preserving PostgreSQL's response +semantics while introducing an execution boundary and will be tracked separately. + +### Publication + +When the outermost transaction containing successful DDL commits, one reload coordinator reads the +authoritative PostgreSQL catalog. Concurrent publication requests are coalesced, and generation +ordering prevents an older reload from replacing newer state. The coordinator atomically publishes +the combined schema and encryption snapshot before Proxy forwards `ReadyForQuery(I)`. + +Proxy does not merge its inferred overlay into shared state. PostgreSQL remains authoritative for +cascades, conditional DDL, server-side effects, and the final outcome of the transaction. + +If publication fails after PostgreSQL has committed, Proxy retains the dirty publication for retry, +does not forward successful readiness, and closes the affected client connection. The database +commit cannot be undone, but Proxy must not imply that stale encryption metadata is safe to use. + +## Consequences + +- DDL becomes visible to later statements on the same connection immediately after successful + execution, including within an explicit transaction. +- Other connections observe DDL only after commit and successful publication. +- Every transaction maps against a stable schema and encryption-policy generation. +- Frontend and Backend become protocol adapters around a testable schema state machine. +- Extended-protocol pipelining requires bounded deferral after DDL execution. +- Availability is intentionally sacrificed when committed schema state cannot be published safely. +- Schema and encryption managers can no longer publish independent observable states. + +## Verification + +State-machine tests cover successful execution, execution failure, explicit commit, full rollback, +savepoint rollback, generation ordering, deferral, unmodelled DDL, and reload failure. Database-backed +tests cover extended-protocol autocommit, explicit transactions, an already-open second connection, +pipelining, direct ciphertext verification, and failure before readiness. The existing simple-query +behaviour remains covered, with dependent post-DDL batches asserted to fail closed. From eec768dcd4762f266159ca2631c310abbf97a5be Mon Sep 17 00:00:00 2001 From: James Sadler Date: Sat, 22 Aug 2026 23:30:09 +1000 Subject: [PATCH 2/4] fix(proxy): make schema changes transaction aware Publish schema and encryption metadata as one versioned snapshot, with connection-local overlays pinned for transactions and savepoint-aware rollback semantics. Activate DDL only after backend success, defer pipelined statements behind in-flight DDL, and reload authoritative catalog state after the outermost commit using generation-safe coalescing. Fail closed for unmodelled DDL, dependent simple-query batches, and publication failures. Add unit and TLS-backed regressions proving immediate cross-connection encryption and ciphertext at rest. Signed-off-by: James Sadler --- CHANGELOG.md | 4 + docs/errors.md | 32 + .../src/schema_change.rs | 169 +++- packages/cipherstash-proxy/src/error.rs | 12 +- .../src/postgresql/backend.rs | 42 +- .../src/postgresql/context/mod.rs | 199 ++-- .../src/postgresql/frontend.rs | 74 +- .../cipherstash-proxy/src/postgresql/mod.rs | 1 + .../src/proxy/encrypt_config/manager.rs | 40 +- .../src/proxy/encrypt_config/mod.rs | 4 +- packages/cipherstash-proxy/src/proxy/mod.rs | 39 +- .../src/proxy/schema/manager.rs | 367 +++++++- .../src/proxy/schema/middleware.rs | 870 ++++++++++++++++++ .../cipherstash-proxy/src/proxy/schema/mod.rs | 4 +- packages/eql-mapper/src/model/schema_delta.rs | 29 +- 15 files changed, 1661 insertions(+), 225 deletions(-) create mode 100644 packages/cipherstash-proxy/src/proxy/schema/middleware.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 8ac6d4dbe..d45f42c94 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ## [Unreleased] +### Security + +- **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, pipelining, and already-open connections now observe the correct schema generation. Unmodelled DDL, dependent simple-query batches, and failed catalog publication fail closed instead of risking plaintext writes through stale metadata. + ## [3.0.1] - 2026-08-05 ### Added diff --git a/docs/errors.md b/docs/errors.md index 8f9ae535c..764362577 100644 --- a/docs/errors.md +++ b/docs/errors.md @@ -12,6 +12,8 @@ - [Invalid SQL statement](#mapping-invalid-sql-statement) - [Unsupported parameter type](#mapping-unsupported-parameter-type) - [Statement could not be type checked](#mapping-statement-could-not-be-type-checked) + - [Dependent statement after DDL](#mapping-dependent-statement-after-ddl) + - [Unmodelled DDL](#mapping-unmodelled-ddl) - [Unmappable encrypted column](#mapping-unmappable-encrypted-column) - [Internal Error](#mapping-internal-error) @@ -249,6 +251,36 @@ If the error persists, please contact CipherStash [support](https://cipherstash. + + + +## Dependent statement after DDL + +A simple-query batch contains a schema-dependent statement after DDL. Proxy cannot observe the +DDL execution result between statements in one simple-query message, so it refuses the complete +batch before PostgreSQL executes any part of it. + +### How to fix + +Send the DDL and the dependent statement as separate queries. Extended-protocol clients may +pipeline them; Proxy defers dependent mapping until PostgreSQL reports the DDL outcome. + + + + + +## Unmodelled DDL + +PostgreSQL successfully executed a schema change whose connection-local effect Proxy cannot model +safely, such as conditional or cascading DDL. Schema-dependent statements are refused for the +rest of that transaction. + +### How to fix + +Roll back the transaction, or commit it and wait for Proxy to publish an authoritative catalog +snapshot before issuing schema-dependent statements. + + diff --git a/packages/cipherstash-proxy-integration/src/schema_change.rs b/packages/cipherstash-proxy-integration/src/schema_change.rs index ff1fa0420..9f1340c27 100644 --- a/packages/cipherstash-proxy-integration/src/schema_change.rs +++ b/packages/cipherstash-proxy-integration/src/schema_change.rs @@ -1,25 +1,168 @@ #[cfg(test)] mod tests { - use crate::common::{connect_with_tls, random_id, PROXY}; + use crate::common::{connect, connect_with_tls, get_database_port, random_id, PROXY}; + use tokio_postgres::Client; + + async fn connect_for_test(port: u16) -> Client { + if std::env::var("CS_TEST_USE_TLS").as_deref() == Ok("false") { + connect(port).await + } else { + connect_with_tls(port).await + } + } + + fn table(prefix: &str) -> String { + format!("{prefix}_{}", random_id()) + } + + fn create_encrypted_table(table: &str) -> String { + format!("CREATE TABLE {table} (id bigint PRIMARY KEY, secret eql_v3_text_search NOT NULL)") + } + + async fn assert_ciphertext_at_rest(table: &str, id: i64, plaintext: &str) { + let postgres = connect_for_test(get_database_port()).await; + let sql = format!("SELECT secret::text FROM {table} WHERE id = $1"); + let stored: String = postgres.query_one(&sql, &[&id]).await.unwrap().get(0); + + assert!( + !stored.contains(plaintext), + "plaintext reached PostgreSQL: {stored}" + ); + let payload: serde_json::Value = serde_json::from_str(&stored).unwrap(); + assert!( + payload.get("c").is_some(), + "missing record ciphertext: {payload}" + ); + } + + async fn insert_secret(client: &Client, table: &str, id: i64, plaintext: &str) { + let sql = format!("INSERT INTO {table} (id, secret) VALUES ($1, $2)"); + assert_eq!(client.execute(&sql, &[&id, &plaintext]).await.unwrap(), 1); + } + + #[tokio::test] + async fn later_connection_encrypts_immediately_after_extended_protocol_ddl() { + let ddl_connection = connect_for_test(*PROXY).await; + let already_open_connection = connect_for_test(*PROXY).await; + let table = table("bug_308_extended"); + + ddl_connection + .execute(&create_encrypted_table(&table), &[]) + .await + .unwrap(); + + insert_secret(&already_open_connection, &table, 1, "classified").await; + assert_ciphertext_at_rest(&table, 1, "classified").await; + } #[tokio::test] - async fn schema_change_reloads_schema() { - let client = connect_with_tls(*PROXY).await; + async fn explicit_transaction_uses_successful_ddl_overlay_before_commit() { + let client = connect_for_test(*PROXY).await; + let table = table("bug_308_transaction"); - let id = random_id(); + client.batch_execute("BEGIN").await.unwrap(); + client + .execute(&create_encrypted_table(&table), &[]) + .await + .unwrap(); + insert_secret(&client, &table, 1, "inside transaction").await; + client.batch_execute("COMMIT").await.unwrap(); - let sql = format!( - "CREATE TABLE table_{id} ( - id bigint, - PRIMARY KEY(id) - );" + assert_ciphertext_at_rest(&table, 1, "inside transaction").await; + } + + #[tokio::test] + async fn pipelined_statement_waits_for_extended_ddl_activation() { + let client = connect_for_test(*PROXY).await; + let table = table("bug_308_pipeline"); + let create = create_encrypted_table(&table); + let insert = format!("INSERT INTO {table} (id, secret) VALUES ($1, $2)"); + let create = client.prepare(&create).await.unwrap(); + + let (created, inserted) = tokio::join!( + client.execute(&create, &[]), + client.execute(&insert, &[&1_i64, &"pipelined"]), ); + created.unwrap(); + assert_eq!(inserted.unwrap(), 1); + + assert_ciphertext_at_rest(&table, 1, "pipelined").await; + } + + #[tokio::test] + async fn rollback_discards_successful_ddl_overlay() { + let client = connect_for_test(*PROXY).await; + let postgres = connect_for_test(get_database_port()).await; + let table = table("bug_308_rollback"); + + client.batch_execute("BEGIN").await.unwrap(); + client + .execute(&create_encrypted_table(&table), &[]) + .await + .unwrap(); + client.batch_execute("ROLLBACK").await.unwrap(); - let _ = client.execute(&sql, &[]).await.unwrap(); + let exists: bool = postgres + .query_one("SELECT to_regclass($1) IS NOT NULL", &[&table]) + .await + .unwrap() + .get(0); + assert!(!exists); + } + + #[tokio::test] + async fn rollback_to_savepoint_restores_schema_and_encryption_overlay() { + let client = connect_for_test(*PROXY).await; + let postgres = connect_for_test(get_database_port()).await; + let retained = table("bug_308_retained"); + let reverted = table("bug_308_reverted"); + + client.batch_execute("BEGIN").await.unwrap(); + client + .execute(&create_encrypted_table(&retained), &[]) + .await + .unwrap(); + client + .batch_execute("SAVEPOINT before_reverted") + .await + .unwrap(); + client + .execute(&create_encrypted_table(&reverted), &[]) + .await + .unwrap(); + client + .batch_execute("ROLLBACK TO SAVEPOINT before_reverted") + .await + .unwrap(); + insert_secret(&client, &retained, 1, "savepoint secret").await; + client.batch_execute("COMMIT").await.unwrap(); + + assert_ciphertext_at_rest(&retained, 1, "savepoint secret").await; + let exists: bool = postgres + .query_one("SELECT to_regclass($1) IS NOT NULL", &[&reverted]) + .await + .unwrap() + .get(0); + assert!(!exists); + } + + #[tokio::test] + async fn simple_query_batch_with_dependent_post_ddl_statement_fails_closed() { + let client = connect_for_test(*PROXY).await; + let postgres = connect_for_test(get_database_port()).await; + let table = table("bug_308_simple_batch"); + let batch = format!( + "{}; INSERT INTO {table} (id, secret) VALUES (1, 'plaintext')", + create_encrypted_table(&table) + ); - let sql = format!("SELECT id FROM table_{id}"); - let rows = client.query(&sql, &[]).await.unwrap(); + assert!(client.simple_query(&batch).await.is_err()); - assert!(rows.is_empty()); + let exists: bool = postgres + .query_one("SELECT to_regclass($1) IS NOT NULL", &[&table]) + .await + .unwrap() + .get(0); + assert!(!exists); } } diff --git a/packages/cipherstash-proxy/src/error.rs b/packages/cipherstash-proxy/src/error.rs index dffc14935..965550c97 100644 --- a/packages/cipherstash-proxy/src/error.rs +++ b/packages/cipherstash-proxy/src/error.rs @@ -74,7 +74,11 @@ impl Error { // Forwarding a statement that references a legacy EQL v2 column // stores plaintext in a column its operator believes is encrypted // (CIP-3688). No configuration may turn that back on. - Error::Mapping(MappingError::UnmappableEncryptedColumn { .. }) + Error::Mapping( + MappingError::UnmappableEncryptedColumn { .. } + | MappingError::DependentStatementAfterDdl + | MappingError::UnmodelledDdl + ) ) } } @@ -99,6 +103,12 @@ pub enum ZeroKMSError { #[derive(Error, Debug)] pub enum MappingError { + #[error("A simple-query batch cannot contain a schema-dependent statement after DDL. Send the DDL and dependent statement as separate queries. For help visit {}#mapping-dependent-statement-after-ddl", ERROR_DOC_BASE_URL)] + DependentStatementAfterDdl, + + #[error("A successful schema change in this transaction cannot be modelled safely. Roll back the transaction before issuing schema-dependent statements. For help visit {}#mapping-unmodelled-ddl", ERROR_DOC_BASE_URL)] + UnmodelledDdl, + #[error("Invalid parameter for column '{}' of type '{}' in table '{}' (OID {}). For help visit {}#mapping-invalid-parameter", _0.column_name(), _0.cast_type(), _0.table_name(), _0.oid(), ERROR_DOC_BASE_URL)] InvalidParameter(Box), diff --git a/packages/cipherstash-proxy/src/postgresql/backend.rs b/packages/cipherstash-proxy/src/postgresql/backend.rs index 1e2a24f11..df33fea51 100644 --- a/packages/cipherstash-proxy/src/postgresql/backend.rs +++ b/packages/cipherstash-proxy/src/postgresql/backend.rs @@ -185,7 +185,12 @@ where // client opening its next connection after ReadyForQuery observes // the newly loaded schema and encrypt configuration. if matches!(code.into(), BackendCode::ReadyForQuery) { - self.context.reload_schema_if_changed().await; + if bytes.last() == Some(&b'I') { + self.context.publish_schema_if_changed().await?; + } + if let Some(status) = bytes.last().copied() { + self.context.schema_ready_for_query(status); + } } self.write_with_flush(bytes).await?; @@ -202,8 +207,13 @@ where match code.into() { BackendCode::CommandComplete | BackendCode::EmptyQueryResponse - | BackendCode::PortalSuspended - | BackendCode::ErrorResponse => { + | BackendCode::PortalSuspended => { + self.context.schema_execution_succeeded(); + self.context.complete_execution(); + self.context.finish_session(); + } + BackendCode::ErrorResponse => { + self.context.schema_execution_failed(); self.context.complete_execution(); self.context.finish_session(); } @@ -240,10 +250,12 @@ where } } + self.context.schema_execution_succeeded(); self.context.complete_execution(); self.context.finish_session(); } BackendCode::ErrorResponse => { + self.context.schema_execution_failed(); if let Some(b) = self.error_response_handler(&bytes)? { bytes = b } @@ -291,7 +303,12 @@ where client_id = self.context.client_id, msg = "ReadyForQuery" ); - self.context.reload_schema_if_changed().await; + if bytes.last() == Some(&b'I') { + self.context.publish_schema_if_changed().await?; + } + if let Some(status) = bytes.last().copied() { + self.context.schema_ready_for_query(status); + } } code => { @@ -757,6 +774,7 @@ mod tests { use crate::log; use crate::postgresql::context::KeysetIdentifier; use crate::postgresql::messages::Name; + use crate::postgresql::parser::SqlParser; use crate::proxy::{EncryptConfig, EncryptionService}; use eql_mapper::Schema; use std::io::Cursor; @@ -847,7 +865,7 @@ mod tests { } #[tokio::test] - async fn passthrough_reloads_changed_schema_before_ready_for_query() { + async fn publication_failure_closes_connection_before_idle_readiness() { let config = Arc::new(TandemConfig::for_testing()); let encrypt_config = Arc::new(EncryptConfig::default()); let schema = Arc::new(Schema::new("public")); @@ -860,7 +878,9 @@ mod tests { TestService {}, reload_sender, ); - context.set_schema_changed(); + let ddl = SqlParser::parse_statement("create table reports (id bigint)").unwrap(); + context.execute_simple_schema_statements(&[ddl]); + context.schema_execution_succeeded(); let reload_task = tokio::spawn(async move { let Some(crate::proxy::ReloadCommand::DatabaseSchema(responder)) = @@ -868,20 +888,16 @@ mod tests { else { panic!("expected a database schema reload command"); }; - responder.send(true).expect("reload receiver must be open"); + responder.send(false).unwrap(); }); let (client_sender, mut client_receiver) = mpsc::unbounded_channel(); let reader = Cursor::new(ready_for_query_bytes().to_vec()); let mut backend = Backend::new(client_sender, reader, context); - backend.rewrite().await.unwrap(); + assert!(backend.rewrite().await.is_err()); reload_task.await.unwrap(); - assert_eq!( - client_receiver.recv().await.unwrap(), - ready_for_query_bytes() - ); - assert!(!backend.context.take_schema_changed()); + assert!(client_receiver.try_recv().is_err()); } /// Regression test for BUG-300 (passthrough memory leak). diff --git a/packages/cipherstash-proxy/src/postgresql/context/mod.rs b/packages/cipherstash-proxy/src/postgresql/context/mod.rs index 3ea0cbd54..7301dca34 100644 --- a/packages/cipherstash-proxy/src/postgresql/context/mod.rs +++ b/packages/cipherstash-proxy/src/postgresql/context/mod.rs @@ -11,13 +11,16 @@ use super::{ }; use crate::{ config::TandemConfig, - error::{EncryptError, Error}, + error::{ConfigError, EncryptError, Error}, log::{CONTEXT, SLOW_STATEMENTS}, prometheus::{ SLOW_STATEMENTS_TOTAL, STATEMENTS_EXECUTION_DURATION_SECONDS, STATEMENTS_SESSION_DURATION_SECONDS, }, - proxy::{EncryptConfig, EncryptionService, ReloadCommand, ReloadSender}, + proxy::{ + schema::{CommittedSchemaStore, SchemaMiddleware}, + EncryptConfig, EncryptionService, ReloadCommand, ReloadSender, + }, }; use cipherstash_client::IdentifiedBy; use eql_mapper::{Schema, TableResolver}; @@ -28,7 +31,7 @@ pub use statement_metadata::StatementMetadata; use std::{ collections::{HashMap, VecDeque}, sync::{ - atomic::{AtomicBool, AtomicU64, Ordering}, + atomic::{AtomicU64, Ordering}, Arc, LazyLock, RwLock, }, time::{Duration, Instant}, @@ -61,18 +64,15 @@ where { pub client_id: i32, config: Arc, - encrypt_config: Arc, encryption: T, reload_sender: ReloadSender, - column_mapper: ColumnMapper, + schema_middleware: SchemaMiddleware, statements: Arc>>>, statement_sessions: Arc>>, portals: Arc>>, describe: Arc>, execute: Arc>, - schema_changed: Arc, session_metrics: Arc>, - table_resolver: Arc, unsafe_disable_mapping: bool, keyset_id: Arc>>, session_id_counter: Arc, @@ -177,7 +177,19 @@ where encryption: T, reload_sender: ReloadSender, ) -> Context { - let column_mapper = ColumnMapper::new(encrypt_config.clone()); + let schema_store = + CommittedSchemaStore::from_parts((*schema).clone(), (*encrypt_config).clone()); + Self::new_with_schema_store(client_id, config, schema_store, encryption, reload_sender) + } + + pub fn new_with_schema_store( + client_id: i32, + config: Arc, + schema_store: CommittedSchemaStore, + encryption: T, + reload_sender: ReloadSender, + ) -> Context { + let schema_middleware = SchemaMiddleware::from_store(schema_store); Context { statements: Arc::new(RwLock::new(HashMap::new())), @@ -185,13 +197,10 @@ where portals: Arc::new(RwLock::new(HashMap::new())), describe: Arc::new(RwLock::from(Queue::new())), execute: Arc::new(RwLock::from(Queue::new())), - schema_changed: Arc::new(AtomicBool::new(false)), session_metrics: Arc::new(RwLock::from(Queue::new())), - table_resolver: Arc::new(TableResolver::new_editable(schema)), client_id, config, - encrypt_config, - column_mapper, + schema_middleware, encryption, reload_sender, unsafe_disable_mapping: false, @@ -562,20 +571,69 @@ where Some(session_context.to_owned()) } - pub fn set_schema_changed(&self) { - debug!(target: CONTEXT, - client_id = self.client_id, - msg = "Schema changed" - ); - self.schema_changed.store(true, Ordering::Release); + pub fn get_table_resolver(&self) -> Arc { + self.schema_middleware.resolver() } - pub fn take_schema_changed(&self) -> bool { - self.schema_changed.swap(false, Ordering::AcqRel) + pub fn prepare_schema_statement(&self, name: Name, statement: sqltk::parser::ast::Statement) { + self.schema_middleware.prepare(name, statement); } - pub fn get_table_resolver(&self) -> Arc { - self.table_resolver.clone() + pub fn bind_schema_statement(&self, portal: Name, prepared_statement: &Name) { + self.schema_middleware.bind(portal, prepared_statement); + } + + pub fn execute_schema_portal(&self, portal: &Name) { + self.schema_middleware.execute(portal); + } + + pub fn execute_simple_schema_statements(&self, statements: &[sqltk::parser::ast::Statement]) { + self.schema_middleware.simple_query(statements); + } + + pub fn mark_schema_protocol_boundary(&self) { + self.schema_middleware.protocol_boundary(); + } + + pub fn schema_execution_succeeded(&self) { + self.schema_middleware.execution_succeeded(); + } + + pub fn schema_execution_failed(&self) { + self.schema_middleware.execution_failed(); + } + + pub async fn wait_for_schema_execution(&self) { + self.schema_middleware.wait_for_ddl().await; + } + + pub fn ensure_schema_modelled(&self) -> Result<(), Error> { + if self.schema_middleware.has_unmodelled_ddl() { + return Err(crate::error::MappingError::UnmodelledDdl.into()); + } + Ok(()) + } + + pub fn adopt_latest_schema(&self) { + self.schema_middleware.adopt_latest(); + } + + pub async fn prepare_schema_for_statement(&self) -> Result<(), Error> { + if self + .schema_middleware + .requires_publication_before_statement() + { + if !self.reload_schema().await { + return Err(ConfigError::SchemaCouldNotBeLoaded.into()); + } + self.schema_middleware.publication_succeeded(); + } + self.schema_middleware.before_statement(); + Ok(()) + } + + pub fn schema_ready_for_query(&self, status: u8) { + self.schema_middleware.ready_for_query(status); } /// Examines a [`sqltk::parser::ast::Statement`] and if it is precisely equal to `SET UNSAFE_DISABLE_MAPPING = {boolean};` @@ -791,16 +849,26 @@ where } /// Reload schema if it has changed since last check. - pub async fn reload_schema_if_changed(&self) { - if self.take_schema_changed() && !self.reload_schema().await { - // Preserve the dirty state when the reload task is unavailable so - // a later statement can retry instead of silently losing the DDL. - self.set_schema_changed(); + pub async fn publish_schema_if_changed(&self) -> Result<(), Error> { + if !self.schema_middleware.needs_publication() { + self.adopt_latest_schema(); + return Ok(()); } + + if self.schema_middleware.has_local_changes() { + self.schema_middleware.mark_publication_pending(); + } + + if !self.reload_schema().await { + return Err(ConfigError::SchemaCouldNotBeLoaded.into()); + } + + self.schema_middleware.publication_succeeded(); + Ok(()) } pub fn is_passthrough(&self) -> bool { - self.encrypt_config.is_empty() || self.config.mapping_disabled() + self.schema_middleware.encrypt_config().is_empty() || self.config.mapping_disabled() } // Column processing delegation methods @@ -808,28 +876,31 @@ where &self, typed_statement: &eql_mapper::TypeCheckedStatement<'_>, ) -> Result>, Error> { - self.column_mapper.get_projection_columns(typed_statement) + ColumnMapper::new(self.schema_middleware.encrypt_config()) + .get_projection_columns(typed_statement) } pub fn get_param_columns( &self, typed_statement: &eql_mapper::TypeCheckedStatement<'_>, ) -> Result>, Error> { - self.column_mapper.get_param_columns(typed_statement) + ColumnMapper::new(self.schema_middleware.encrypt_config()) + .get_param_columns(typed_statement) } pub fn get_output_param_columns( &self, plan: &eql_mapper::ParamPlan, ) -> Result>, Error> { - self.column_mapper.get_output_param_columns(plan) + ColumnMapper::new(self.schema_middleware.encrypt_config()).get_output_param_columns(plan) } pub fn get_literal_columns( &self, typed_statement: &eql_mapper::TypeCheckedStatement<'_>, ) -> Result>, Error> { - self.column_mapper.get_literal_columns(typed_statement) + ColumnMapper::new(self.schema_middleware.encrypt_config()) + .get_literal_columns(typed_statement) } // Direct config access methods @@ -1069,7 +1140,7 @@ mod tests { messages::{Name, Target}, Column, }, - proxy::{EncryptConfig, EncryptionService, ReloadCommand}, + proxy::{EncryptConfig, EncryptionService}, TandemConfig, }; use cipherstash_client::IdentifiedBy; @@ -1121,68 +1192,6 @@ mod tests { ) } - #[tokio::test] - async fn successful_schema_reload_consumes_change_flag_once() { - let config = Arc::new(TandemConfig::for_testing()); - let encrypt_config = Arc::new(EncryptConfig::default()); - let schema = Arc::new(Schema::new("public")); - let (reload_sender, mut reload_receiver) = mpsc::unbounded_channel(); - let context = Context::new( - 1, - config, - encrypt_config, - schema, - TestService {}, - reload_sender, - ); - let reload_task = tokio::spawn(async move { - let Some(ReloadCommand::DatabaseSchema(responder)) = reload_receiver.recv().await - else { - panic!("expected database schema reload"); - }; - responder.send(true).expect("reload receiver is alive"); - tokio::time::timeout(std::time::Duration::from_millis(20), reload_receiver.recv()) - .await - .is_err() - }); - - context.set_schema_changed(); - context.reload_schema_if_changed().await; - context.reload_schema_if_changed().await; - - assert!(!context.take_schema_changed()); - assert!(reload_task.await.expect("reload task did not panic")); - } - - #[tokio::test] - async fn failed_schema_reload_keeps_change_flag_for_retry() { - let config = Arc::new(TandemConfig::for_testing()); - let encrypt_config = Arc::new(EncryptConfig::default()); - let schema = Arc::new(Schema::new("public")); - let (reload_sender, mut reload_receiver) = mpsc::unbounded_channel(); - let context = Context::new( - 1, - config, - encrypt_config, - schema, - TestService {}, - reload_sender, - ); - let reload_task = tokio::spawn(async move { - let Some(ReloadCommand::DatabaseSchema(responder)) = reload_receiver.recv().await - else { - panic!("expected database schema reload"); - }; - responder.send(false).expect("reload receiver is alive"); - }); - - context.set_schema_changed(); - context.reload_schema_if_changed().await; - - reload_task.await.expect("reload task did not panic"); - assert!(context.take_schema_changed()); - } - fn statement() -> Statement { Statement { param_columns: vec![], diff --git a/packages/cipherstash-proxy/src/postgresql/frontend.rs b/packages/cipherstash-proxy/src/postgresql/frontend.rs index 0e5436428..2c0db166a 100644 --- a/packages/cipherstash-proxy/src/postgresql/frontend.rs +++ b/packages/cipherstash-proxy/src/postgresql/frontend.rs @@ -48,6 +48,19 @@ use std::time::Instant; use tokio::io::{AsyncRead, AsyncWrite, AsyncWriteExt}; use tracing::{debug, error, info, warn}; +fn is_schema_ddl(statement: &ast::Statement) -> bool { + matches!( + statement, + ast::Statement::CreateTable(_) + | ast::Statement::CreateView { .. } + | ast::Statement::AlterTable { .. } + | ast::Statement::Drop { + object_type: ast::ObjectType::Table | ast::ObjectType::View, + .. + } + ) +} + /// The PostgreSQL proxy frontend that handles client-to-server message processing. /// /// The Frontend intercepts messages from PostgreSQL clients, analyzes SQL statements for @@ -221,7 +234,10 @@ where // The server's ErrorResponse and ReadyForQuery answer // the client's Query in order. See handle_statement_error. match self.handle_statement_error(err)? { - Some(exception) => bytes = exception, + Some(exception) => { + self.context.mark_schema_protocol_boundary(); + bytes = exception; + } // FATAL error written directly to the client; the // simple protocol still expects a ReadyForQuery. None => { @@ -255,6 +271,7 @@ where // client's batch in order. See handle_statement_error. match self.handle_statement_error(err)? { Some(exception) => { + self.context.mark_schema_protocol_boundary(); self.error_state = Some(ErrorState::ExceptionInjected); bytes = exception; } @@ -283,6 +300,7 @@ where // client's batch in order. See handle_statement_error. match self.handle_statement_error(err)? { Some(exception) => { + self.context.mark_schema_protocol_boundary(); self.error_state = Some(ErrorState::ExceptionInjected); bytes = exception; } @@ -300,8 +318,6 @@ where ?code, ); - self.context.reload_schema_if_changed().await; - match self.error_state.take() { Some(ErrorState::ExceptionInjected) => { // The exception statement injected on failure already @@ -322,6 +338,7 @@ where } None => {} } + self.context.mark_schema_protocol_boundary(); } Code::Close => { self.close_handler(&bytes).await?; @@ -381,6 +398,7 @@ where async fn execute_handler(&mut self, bytes: &BytesMut) -> Result<(), Error> { let execute = Execute::try_from(bytes)?; debug!(target: PROTOCOL, client_id = self.context.client_id, ?execute); + self.context.execute_schema_portal(&execute.portal); self.context .set_execute_for_portal(execute.portal.to_owned()); Ok(()) @@ -433,6 +451,23 @@ where // Simple Query may contain many statements let parsed_statements = SqlParser::parse_statements(&query.statement)?; + self.context.prepare_schema_for_statement().await?; + if let Some(ddl_index) = parsed_statements.iter().position(is_schema_ddl) { + if parsed_statements + .iter() + .skip(ddl_index + 1) + .any(eql_mapper::requires_type_check) + { + return Err(MappingError::DependentStatementAfterDdl.into()); + } + } + if parsed_statements + .iter() + .any(eql_mapper::requires_type_check) + { + self.context.wait_for_schema_execution().await; + self.context.ensure_schema_modelled()?; + } let mut transformed_statements = vec![]; debug!(target: MAPPER, @@ -462,8 +497,6 @@ where self.handle_set_keyset(statement)?; - self.check_for_schema_change(statement); - if !eql_mapper::requires_type_check(statement) { counter!(STATEMENTS_PASSTHROUGH_TOTAL).increment(1); continue; @@ -571,6 +604,9 @@ where self.context.add_portal(Name::unnamed(), portal); self.context.set_execute(Name::unnamed(), Some(session_id)); + self.context + .execute_simple_schema_statements(&parsed_statements); + if encrypted { let transformed_statement = transformed_statements .iter() @@ -826,6 +862,13 @@ where .set_statement_session(message.name.to_owned(), session_id); let statement = SqlParser::parse_statement(&message.statement)?; + self.context.prepare_schema_for_statement().await?; + if eql_mapper::requires_type_check(&statement) { + self.context.wait_for_schema_execution().await; + self.context.ensure_schema_modelled()?; + } + self.context + .prepare_schema_statement(message.name.to_owned(), statement.clone()); if let Some(mapping_disabled) = self.context.maybe_set_unsafe_disable_mapping(&statement) { warn!( @@ -843,8 +886,6 @@ where self.handle_set_keyset(&statement)?; - self.check_for_schema_change(&statement); - if !eql_mapper::requires_type_check(&statement) { counter!(STATEMENTS_PASSTHROUGH_TOTAL).increment(1); return Ok(None); @@ -945,19 +986,6 @@ where } } - /// - /// Check the Statement AST for DDL - /// Sets a schema changed flag in the Context - /// - /// - fn check_for_schema_change(&self, statement: &ast::Statement) { - let schema_changed = eql_mapper::collect_ddl(self.context.get_table_resolver(), statement); - - if schema_changed { - self.context.set_schema_changed(); - } - } - /// /// Handles `SET CIPHERSTASH KEYSET_*` statements /// @@ -1085,6 +1113,10 @@ where /// - `Ok(None)` - No parameter encryption needed, forward original message /// - `Err(error)` - Processing failed, error should be sent to client async fn bind_handler(&mut self, bytes: &BytesMut) -> Result, Error> { + let mut bind = Bind::try_from(bytes)?; + self.context + .bind_schema_statement(bind.portal.to_owned(), &bind.prepared_statement); + if self.context.unsafe_disable_mapping() { warn!(msg = "Encrypted statement mapping is not enabled"); counter!(STATEMENTS_PASSTHROUGH_MAPPING_DISABLED_TOTAL).increment(1); @@ -1092,8 +1124,6 @@ where return Ok(None); } - let mut bind = Bind::try_from(bytes)?; - let session_id = self .context .get_statement_session_or_latest(&bind.prepared_statement); diff --git a/packages/cipherstash-proxy/src/postgresql/mod.rs b/packages/cipherstash-proxy/src/postgresql/mod.rs index 71c8df249..e6172da9f 100644 --- a/packages/cipherstash-proxy/src/postgresql/mod.rs +++ b/packages/cipherstash-proxy/src/postgresql/mod.rs @@ -16,6 +16,7 @@ pub use context::column::Column; pub use context::Context; pub use context::KeysetIdentifier; pub use handler::handler; +pub(crate) use messages::Name; pub const PROTOCOL_VERSION_NUMBER: i32 = 196608; diff --git a/packages/cipherstash-proxy/src/proxy/encrypt_config/manager.rs b/packages/cipherstash-proxy/src/proxy/encrypt_config/manager.rs index 6b923fc57..fffbea825 100644 --- a/packages/cipherstash-proxy/src/proxy/encrypt_config/manager.rs +++ b/packages/cipherstash-proxy/src/proxy/encrypt_config/manager.rs @@ -15,7 +15,7 @@ use tracing::{debug, error, info, warn}; /// type EncryptConfigMap = HashMap; -#[derive(Clone, Debug)] +#[derive(Clone, Debug, PartialEq)] pub struct EncryptConfig { config: EncryptConfigMap, } @@ -38,6 +38,44 @@ impl EncryptConfig { pub fn get_column_config(&self, identifier: &eql::Identifier) -> Option { self.config.get(identifier).cloned() } + + pub(crate) fn insert(&mut self, identifier: eql::Identifier, config: ColumnConfig) { + self.config.insert(identifier, config); + } + + pub(crate) fn remove_column(&mut self, table: &str, column: &str) { + self.config + .remove(&eql::Identifier::new(table.to_owned(), column.to_owned())); + } + + pub(crate) fn remove_table(&mut self, table: &str) { + self.config + .retain(|identifier, _| identifier.table != table); + } + + pub(crate) fn rename_column(&mut self, table: &str, from: &str, to: &str) { + let from = eql::Identifier::new(table.to_owned(), from.to_owned()); + if let Some(config) = self.config.remove(&from) { + self.config.insert( + eql::Identifier::new(table.to_owned(), to.to_owned()), + config, + ); + } + } + + pub(crate) fn rename_table(&mut self, from: &str, to: &str) { + let renamed = self + .config + .iter() + .filter(|(identifier, _)| identifier.table == from) + .map(|(identifier, config)| (identifier.column.clone(), config.clone())) + .collect::>(); + self.remove_table(from); + for (column, config) in renamed { + self.config + .insert(eql::Identifier::new(to.to_owned(), column), config); + } + } } impl Default for EncryptConfig { diff --git a/packages/cipherstash-proxy/src/proxy/encrypt_config/mod.rs b/packages/cipherstash-proxy/src/proxy/encrypt_config/mod.rs index 4edf5db8c..68e26e2c9 100644 --- a/packages/cipherstash-proxy/src/proxy/encrypt_config/mod.rs +++ b/packages/cipherstash-proxy/src/proxy/encrypt_config/mod.rs @@ -1,4 +1,4 @@ -mod from_domain; +pub(crate) mod from_domain; mod manager; -pub use manager::{EncryptConfig, EncryptConfigManager}; +pub use manager::EncryptConfig; diff --git a/packages/cipherstash-proxy/src/proxy/mod.rs b/packages/cipherstash-proxy/src/proxy/mod.rs index 091fcba18..c1c252565 100644 --- a/packages/cipherstash-proxy/src/proxy/mod.rs +++ b/packages/cipherstash-proxy/src/proxy/mod.rs @@ -5,7 +5,7 @@ use crate::{ connect, error::Error, postgresql::{Column, Context, KeysetIdentifier}, - proxy::{encrypt_config::EncryptConfigManager, schema::SchemaManager}, + proxy::schema::SchemaManager, }; use cipherstash_client::encryption::Plaintext; use tokio::sync::mpsc::{self, UnboundedReceiver, UnboundedSender}; @@ -13,7 +13,7 @@ use tokio::sync::oneshot::Sender; use tracing::{debug, warn}; mod encrypt_config; -mod schema; +pub(crate) mod schema; mod zerokms; pub use encrypt_config::EncryptConfig; @@ -37,7 +37,6 @@ const AGGREGATE_QUERY: &str = include_str!("./sql/select_aggregates.sql"); #[derive(Debug)] pub enum ReloadCommand { DatabaseSchema(ReloadResponder), - EncryptSchema(ReloadResponder), } /// @@ -45,7 +44,6 @@ pub enum ReloadCommand { /// pub struct Proxy { pub config: Arc, - pub encrypt_config_manager: EncryptConfigManager, pub schema_manager: SchemaManager, /// The EQL version installed in the database or `None` if it was not present pub eql_version: Option, @@ -61,24 +59,17 @@ impl Proxy { // Ensures error on start if credential or network issue zerokms.init_cipher(None).await?; - let encrypt_config_manager = EncryptConfigManager::init(&config.database).await?; - let schema_manager = SchemaManager::init(&config.database).await?; let eql_version = Proxy::eql_version(&config).await?; let (reload_sender, reload_receiver) = mpsc::unbounded_channel(); - Proxy::receive( - reload_receiver, - schema_manager.clone(), - encrypt_config_manager.clone(), - ); + Proxy::receive(reload_receiver, schema_manager.clone()); Ok(Proxy { config: Arc::new(config), zerokms, - encrypt_config_manager, schema_manager, eql_version, reload_sender, @@ -104,23 +95,16 @@ impl Proxy { Ok(version) } - pub fn receive( - mut reload_receiver: ReloadReceiver, - schema_manager: SchemaManager, - encrypt_config_manager: EncryptConfigManager, - ) { + pub fn receive(mut reload_receiver: ReloadReceiver, schema_manager: SchemaManager) { tokio::task::spawn(async move { while let Some(command) = reload_receiver.recv().await { debug!(msg = "ReloadCommand received", ?command); match command { ReloadCommand::DatabaseSchema(responder) => { - let schema_reloaded = schema_manager.reload().await; - let encrypt_config_reloaded = encrypt_config_manager.reload().await; - let _ = responder.send(schema_reloaded && encrypt_config_reloaded); - } - ReloadCommand::EncryptSchema(responder) => { - let reloaded = encrypt_config_manager.reload().await; - let _ = responder.send(reloaded); + let schema_manager = schema_manager.clone(); + tokio::task::spawn(async move { + let _ = responder.send(schema_manager.reload().await); + }); } } } @@ -132,16 +116,13 @@ impl Proxy { /// pub fn context(&self, client_id: i32) -> Context { let config = self.config.clone(); - let encrypt_config = self.encrypt_config_manager.load(); - let schema = self.schema_manager.load(); let reload_sender = self.reload_sender.clone(); let encryption = self.zerokms.clone(); - Context::new( + Context::new_with_schema_store( client_id, config, - encrypt_config, - schema, + self.schema_manager.store(), encryption, reload_sender, ) diff --git a/packages/cipherstash-proxy/src/proxy/schema/manager.rs b/packages/cipherstash-proxy/src/proxy/schema/manager.rs index 94eea3c59..ffef703f9 100644 --- a/packages/cipherstash-proxy/src/proxy/schema/manager.rs +++ b/packages/cipherstash-proxy/src/proxy/schema/manager.rs @@ -1,20 +1,117 @@ use super::eql_domains; use crate::config::DatabaseConfig; use crate::error::Error; +use crate::proxy::encrypt_config::from_domain::column_config_from_domain; +use crate::proxy::EncryptConfig; use crate::proxy::{AGGREGATE_QUERY, SCHEMA_QUERY}; use crate::{connect, log::SCHEMA}; use arc_swap::ArcSwap; +use cipherstash_client::eql::Identifier; use eql_mapper::{Column, Schema, Table}; use sqltk::parser::ast::Ident; -use std::sync::Arc; +use std::future::Future; +use std::sync::{ + atomic::{AtomicU64, Ordering}, + Arc, +}; use std::time::Duration; -use tokio::{task::JoinHandle, time}; +use tokio::{sync::Mutex, task::JoinHandle, time}; use tracing::{debug, info, warn}; +#[derive(Clone, Debug)] +pub struct CommittedSchemaSnapshot { + version: u64, + schema: Arc, + encrypt_config: Arc, +} + +#[derive(Clone, Debug)] +pub struct CommittedSchemaStore { + snapshot: Arc>, + requested_publication: Arc, + published_publication: Arc, +} + +impl CommittedSchemaStore { + pub(crate) fn from_parts(schema: Schema, encrypt_config: EncryptConfig) -> Self { + Self { + snapshot: Arc::new(ArcSwap::new(Arc::new(CommittedSchemaSnapshot::new( + 1, + schema, + encrypt_config, + )))), + requested_publication: Arc::new(AtomicU64::new(0)), + published_publication: Arc::new(AtomicU64::new(0)), + } + } + + pub fn load(&self) -> Arc { + self.snapshot.load().clone() + } + + pub fn publication_pending(&self) -> bool { + self.requested_publication.load(Ordering::Acquire) + > self.published_publication.load(Ordering::Acquire) + } + + pub fn mark_publication_pending(&self) { + self.requested_publication.fetch_add(1, Ordering::AcqRel); + } + + pub(crate) fn publication_succeeded(&self) { + self.published_publication.store( + self.requested_publication.load(Ordering::Acquire), + Ordering::Release, + ); + } + + #[cfg(test)] + pub fn for_testing(schema: Schema, encrypt_config: EncryptConfig) -> Self { + Self::from_parts(schema, encrypt_config) + } + + #[cfg(test)] + pub fn publish_for_testing(&self, schema: Schema, encrypt_config: EncryptConfig) { + let version = self.load().version() + 1; + self.snapshot.store(Arc::new(CommittedSchemaSnapshot::new( + version, + schema, + encrypt_config, + ))); + self.publication_succeeded(); + } +} + +impl CommittedSchemaSnapshot { + fn new(version: u64, schema: Schema, encrypt_config: EncryptConfig) -> Self { + Self { + version, + schema: Arc::new(schema), + encrypt_config: Arc::new(encrypt_config), + } + } + + pub fn version(&self) -> u64 { + self.version + } + + pub fn schema(&self) -> Arc { + self.schema.clone() + } + + pub fn encrypt_config(&self) -> Arc { + self.encrypt_config.clone() + } +} + #[derive(Clone, Debug)] pub struct SchemaManager { config: DatabaseConfig, - schema: Arc>, + snapshot: Arc>, + requested_generation: Arc, + requested_publication: Arc, + published_publication: Arc, + reload_lock: Arc>, _reload_handle: Arc>, } @@ -24,37 +121,110 @@ impl SchemaManager { init_reloader(config).await } - pub fn load(&self) -> Arc { - self.schema.load().clone() + pub fn load(&self) -> Arc { + self.snapshot.load().clone() + } + + pub fn store(&self) -> CommittedSchemaStore { + CommittedSchemaStore { + snapshot: self.snapshot.clone(), + requested_publication: self.requested_publication.clone(), + published_publication: self.published_publication.clone(), + } } pub async fn reload(&self) -> bool { - match load_schema_with_retry(&self.config).await { - Ok(reloaded) => { - debug!(target: SCHEMA, msg = "Reloaded database schema"); - self.schema.swap(Arc::new(reloaded)); - true - } - Err(err) => { - warn!( - msg = "Error reloading database schema", - error = err.to_string() - ); - false - } + coalesced_reload( + self.snapshot.clone(), + self.requested_generation.clone(), + self.requested_publication.clone(), + self.published_publication.clone(), + self.reload_lock.clone(), + || load_snapshot_with_retry(&self.config), + ) + .await + } +} + +async fn coalesced_reload( + snapshot: Arc>, + requested_generation: Arc, + requested_publication: Arc, + published_publication: Arc, + reload_lock: Arc>, + load: F, +) -> bool +where + F: FnOnce() -> Fut, + Fut: Future>, +{ + let requested = requested_generation.fetch_add(1, Ordering::AcqRel) + 1; + let _guard = reload_lock.lock().await; + let publication_generation = requested_publication.load(Ordering::Acquire); + + if snapshot.load().version() >= requested { + published_publication.fetch_max(publication_generation, Ordering::AcqRel); + return true; + } + + // A catalog read can only satisfy requests already visible when the read + // begins. A request arriving while it is in progress must trigger a later + // read, because PostgreSQL may have committed that DDL after this read's + // transaction snapshot was established. + let loaded_generation = requested_generation.load(Ordering::Acquire); + + match load().await { + Ok((schema, encrypt_config)) => { + debug!(target: SCHEMA, msg = "Reloaded committed schema snapshot", version = loaded_generation); + publish_if_newer( + &snapshot, + CommittedSchemaSnapshot::new(loaded_generation, schema, encrypt_config), + ); + published_publication.fetch_max(publication_generation, Ordering::AcqRel); + true + } + Err(err) => { + warn!( + msg = "Error reloading committed schema snapshot", + error = err.to_string() + ); + false } } } +fn publish_if_newer( + store: &ArcSwap, + candidate: CommittedSchemaSnapshot, +) -> bool { + if candidate.version() <= store.load().version() { + return false; + } + store.store(Arc::new(candidate)); + true +} + async fn init_reloader(config: DatabaseConfig) -> Result { // Skip retries on startup as the likely failure mode is configuration - let schema = load_schema(&config).await?; - info!(msg = "Loaded database schema"); + let (schema, encrypt_config) = load_snapshot(&config).await?; + info!(msg = "Loaded committed schema snapshot"); - let schema = Arc::new(ArcSwap::new(Arc::new(schema))); + let snapshot = Arc::new(ArcSwap::new(Arc::new(CommittedSchemaSnapshot::new( + 1, + schema, + encrypt_config, + )))); + let requested_generation = Arc::new(AtomicU64::new(1)); + let requested_publication = Arc::new(AtomicU64::new(0)); + let published_publication = Arc::new(AtomicU64::new(0)); + let reload_lock = Arc::new(Mutex::new(())); let config_ref = config.clone(); - let schema_ref = schema.clone(); + let snapshot_ref = snapshot.clone(); + let generation_ref = requested_generation.clone(); + let requested_publication_ref = requested_publication.clone(); + let published_publication_ref = published_publication.clone(); + let reload_lock_ref = reload_lock.clone(); let reload_handle = tokio::spawn(async move { let reload_interval = tokio::time::Duration::from_secs(config_ref.config_reload_interval); @@ -67,23 +237,25 @@ async fn init_reloader(config: DatabaseConfig) -> Result { loop { interval.tick().await; - match load_schema_with_retry(&config_ref).await { - Ok(reloaded) => { - schema_ref.swap(Arc::new(reloaded)); - } - Err(err) => { - warn!( - msg = "Error loading database schema", - error = err.to_string() - ); - } - } + coalesced_reload( + snapshot_ref.clone(), + generation_ref.clone(), + requested_publication_ref.clone(), + published_publication_ref.clone(), + reload_lock_ref.clone(), + || load_snapshot_with_retry(&config_ref), + ) + .await; } }); Ok(SchemaManager { config, - schema, + snapshot, + requested_generation, + requested_publication, + published_publication, + reload_lock, _reload_handle: Arc::new(reload_handle), }) } @@ -93,15 +265,17 @@ async fn init_reloader(config: DatabaseConfig) -> Result { /// When databases and the proxy start up at the same time they might not be ready to accept connections before the /// proxy tries to query the schema. To give the proxy the best chance of initialising correctly this method will /// retry the query a few times before passing on the error. -async fn load_schema_with_retry(config: &DatabaseConfig) -> Result { +async fn load_snapshot_with_retry( + config: &DatabaseConfig, +) -> Result<(Schema, EncryptConfig), Error> { let mut retry_count = 0; let max_retry_count = 10; let max_backoff = Duration::from_secs(2); loop { - match load_schema(config).await { - Ok(schema) => { - return Ok(schema); + match load_snapshot(config).await { + Ok(snapshot) => { + return Ok(snapshot); } Err(e) => { @@ -178,16 +352,23 @@ fn classify_column( } pub async fn load_schema(config: &DatabaseConfig) -> Result { + load_snapshot(config).await.map(|(schema, _)| schema) +} + +async fn load_snapshot(config: &DatabaseConfig) -> Result<(Schema, EncryptConfig), Error> { let client = connect::database(config).await?; + client + .batch_execute("BEGIN ISOLATION LEVEL REPEATABLE READ READ ONLY") + .await?; let tables = client.query(SCHEMA_QUERY, &[]).await?; let mut schema = Schema::new("public"); + let mut encrypt_config = EncryptConfig::new(); if tables.is_empty() { warn!(msg = "Database schema contains no tables"); - return Ok(schema); - }; + } for table in tables { let table_name: String = table.get("table_name"); @@ -210,6 +391,13 @@ pub async fn load_schema(config: &DatabaseConfig) -> Result { ); table.add_column(Arc::new(column)); + + if let Some(domain) = column_domain_name.as_deref() { + if let Some(config) = column_config_from_domain(&table_name, col, domain) { + encrypt_config + .insert(Identifier::new(table_name.clone(), col.clone()), config); + } + } }); schema.add_table(table); @@ -224,13 +412,16 @@ pub async fn load_schema(config: &DatabaseConfig) -> Result { }) .collect(); - Ok(schema) + client.batch_execute("COMMIT").await?; + Ok((schema, encrypt_config)) } #[cfg(test)] mod test { use super::*; use eql_mapper::ColumnKind; + use std::sync::atomic::AtomicUsize; + use tokio::sync::Notify; /// The shape `information_schema.columns` reports for a column declared with /// the EQL v2 composite type, verified against PostgreSQL 17: `udt_name` is @@ -292,4 +483,98 @@ mod test { ColumnKind::Native ); } + + #[test] + fn an_older_generation_cannot_replace_a_newer_snapshot() { + let snapshot = ArcSwap::new(Arc::new(CommittedSchemaSnapshot::new( + 3, + Schema::new("public"), + EncryptConfig::new(), + ))); + + assert!(!publish_if_newer( + &snapshot, + CommittedSchemaSnapshot::new(2, Schema::new("stale"), EncryptConfig::new(),), + )); + assert_eq!(snapshot.load().version(), 3); + } + + #[tokio::test] + async fn requests_arriving_during_a_reload_are_coalesced_into_one_follow_up() { + let snapshot = Arc::new(ArcSwap::new(Arc::new(CommittedSchemaSnapshot::new( + 1, + Schema::new("public"), + EncryptConfig::new(), + )))); + let requested_generation = Arc::new(AtomicU64::new(1)); + let requested_publication = Arc::new(AtomicU64::new(1)); + let published_publication = Arc::new(AtomicU64::new(0)); + let reload_lock = Arc::new(Mutex::new(())); + let loads = Arc::new(AtomicUsize::new(0)); + let started = Arc::new(Notify::new()); + let release = Arc::new(Notify::new()); + + let first = tokio::spawn(coalesced_reload( + snapshot.clone(), + requested_generation.clone(), + requested_publication.clone(), + published_publication.clone(), + reload_lock.clone(), + { + let loads = loads.clone(); + let started = started.clone(); + let release = release.clone(); + move || async move { + loads.fetch_add(1, Ordering::AcqRel); + started.notify_one(); + release.notified().await; + Ok((Schema::new("public"), EncryptConfig::new())) + } + }, + )); + + started.notified().await; + requested_publication.fetch_add(1, Ordering::AcqRel); + let second = tokio::spawn(coalesced_reload( + snapshot.clone(), + requested_generation.clone(), + requested_publication.clone(), + published_publication.clone(), + reload_lock.clone(), + { + let loads = loads.clone(); + move || async move { + loads.fetch_add(1, Ordering::AcqRel); + Ok((Schema::new("public"), EncryptConfig::new())) + } + }, + )); + + let third = tokio::spawn(coalesced_reload( + snapshot.clone(), + requested_generation.clone(), + requested_publication.clone(), + published_publication.clone(), + reload_lock.clone(), + { + let loads = loads.clone(); + move || async move { + loads.fetch_add(1, Ordering::AcqRel); + Ok((Schema::new("public"), EncryptConfig::new())) + } + }, + )); + + while requested_generation.load(Ordering::Acquire) < 4 { + tokio::task::yield_now().await; + } + release.notify_one(); + + assert!(first.await.unwrap()); + assert!(second.await.unwrap()); + assert!(third.await.unwrap()); + assert_eq!(loads.load(Ordering::Acquire), 2); + assert_eq!(snapshot.load().version(), 4); + assert_eq!(published_publication.load(Ordering::Acquire), 2); + } } diff --git a/packages/cipherstash-proxy/src/proxy/schema/middleware.rs b/packages/cipherstash-proxy/src/proxy/schema/middleware.rs new file mode 100644 index 000000000..d61175a9b --- /dev/null +++ b/packages/cipherstash-proxy/src/proxy/schema/middleware.rs @@ -0,0 +1,870 @@ +use super::eql_domains; +use super::manager::CommittedSchemaStore; +use crate::postgresql::Name; +use crate::proxy::encrypt_config::from_domain::column_config_from_domain; +use crate::proxy::EncryptConfig; +use cipherstash_client::eql::Identifier; +use eql_mapper::{ColumnKind, Schema, SchemaWithEdits, TableResolver}; +use sqltk::parser::ast::{ + AlterTableOperation, ColumnDef, DropBehavior, Ident, ObjectName, ObjectNamePart, ObjectType, + Statement, +}; +use std::collections::{HashMap, VecDeque}; +use std::sync::{ + atomic::{AtomicBool, AtomicUsize, Ordering}, + Arc, RwLock, +}; +use tokio::sync::Notify; + +#[derive(Clone, Debug)] +struct Intent { + statement: Statement, + ddl: bool, + modelled: bool, +} + +#[derive(Clone, Debug)] +enum PendingExecution { + Execute(Option>), + ReadyBoundary, +} + +#[derive(Clone, Debug)] +struct Savepoint { + name: Ident, + schema: SchemaWithEdits, + encrypt_config: EncryptConfig, + unmodelled: bool, +} + +/// Connection-local owner of the effective database schema. +/// +/// Protocol adapters report DDL execution outcomes through this interface; +/// parsing alone never changes the resolver visible to later statements. +#[derive(Clone, Debug)] +pub struct SchemaMiddleware { + store: CommittedSchemaStore, + base: Arc>>, + base_encrypt_config: Arc>>, + encrypt_config: Arc>>, + resolver: Arc>>, + prepared: Arc>>, + portals: Arc>>, + executions: Arc>>, + in_flight_ddl: Arc, + execution_finished: Arc, + dirty: Arc, + unmodelled: Arc, + transaction_active: Arc, + savepoints: Arc>>, +} + +impl SchemaMiddleware { + #[cfg(test)] + pub fn new(schema: Arc) -> Self { + Self::from_store(CommittedSchemaStore::for_testing( + (*schema).clone(), + EncryptConfig::new(), + )) + } + + pub fn from_store(store: CommittedSchemaStore) -> Self { + let snapshot = store.load(); + let schema = snapshot.schema(); + Self { + store, + base: Arc::new(RwLock::new(schema.clone())), + base_encrypt_config: Arc::new(RwLock::new(snapshot.encrypt_config())), + encrypt_config: Arc::new(RwLock::new(snapshot.encrypt_config())), + resolver: Arc::new(RwLock::new(Arc::new(TableResolver::new_editable(schema)))), + prepared: Arc::new(RwLock::new(HashMap::new())), + portals: Arc::new(RwLock::new(HashMap::new())), + executions: Arc::new(RwLock::new(VecDeque::new())), + in_flight_ddl: Arc::new(AtomicUsize::new(0)), + execution_finished: Arc::new(Notify::new()), + dirty: Arc::new(AtomicBool::new(false)), + unmodelled: Arc::new(AtomicBool::new(false)), + transaction_active: Arc::new(AtomicBool::new(false)), + savepoints: Arc::new(RwLock::new(Vec::new())), + } + } + + pub fn resolver(&self) -> Arc { + self.resolver.read().unwrap().clone() + } + + pub fn encrypt_config(&self) -> Arc { + self.encrypt_config.read().unwrap().clone() + } + + pub fn adopt_latest(&self) { + let snapshot = self.store.load(); + let schema = snapshot.schema(); + *self.base.write().unwrap() = schema.clone(); + let encrypt_config = snapshot.encrypt_config(); + *self.base_encrypt_config.write().unwrap() = encrypt_config.clone(); + *self.encrypt_config.write().unwrap() = encrypt_config; + *self.resolver.write().unwrap() = Arc::new(TableResolver::new_editable(schema)); + self.savepoints.write().unwrap().clear(); + self.unmodelled.store(false, Ordering::Release); + } + + pub fn needs_publication(&self) -> bool { + self.dirty.load(Ordering::Acquire) || self.store.publication_pending() + } + + pub fn has_local_changes(&self) -> bool { + self.dirty.load(Ordering::Acquire) + } + + pub fn mark_publication_pending(&self) { + self.store.mark_publication_pending(); + } + + pub fn requires_publication_before_statement(&self) -> bool { + !self.transaction_active.load(Ordering::Acquire) + && !self.has_local_changes() + && self.store.publication_pending() + } + + pub fn publication_succeeded(&self) { + self.dirty.store(false, Ordering::Release); + self.adopt_latest(); + } + + pub fn has_unmodelled_ddl(&self) -> bool { + self.unmodelled.load(Ordering::Acquire) + } + + pub fn before_statement(&self) { + if !self.transaction_active.load(Ordering::Acquire) + && !self.has_local_changes() + && !self.store.publication_pending() + && self.in_flight_ddl.load(Ordering::Acquire) == 0 + { + self.adopt_latest(); + } + } + + pub fn ready_for_query(&self, status: u8) { + self.discard_skipped_executions(); + self.transaction_active + .store(status != b'I', Ordering::Release); + if status == b'I' && !self.needs_publication() { + self.adopt_latest(); + } + } + + pub fn prepare(&self, name: Name, statement: Statement) { + self.prepared.write().unwrap().insert( + name, + Intent { + ddl: is_schema_ddl(&statement), + modelled: is_modelled_ddl(&statement), + statement, + }, + ); + } + + pub fn bind(&self, portal: Name, prepared_statement: &Name) { + let intent = self + .prepared + .read() + .unwrap() + .get(prepared_statement) + .cloned(); + let mut portals = self.portals.write().unwrap(); + match intent { + Some(intent) => { + portals.insert(portal, intent); + } + None => { + portals.remove(&portal); + } + } + } + + pub fn execute(&self, portal: &Name) { + let intent = self.portals.read().unwrap().get(portal).cloned(); + self.transaction_active.store(true, Ordering::Release); + if intent.as_ref().is_some_and(|intent| intent.ddl) { + self.in_flight_ddl.fetch_add(1, Ordering::AcqRel); + } + self.executions + .write() + .unwrap() + .push_back(PendingExecution::Execute(intent.map(Box::new))); + } + + pub fn simple_query(&self, statements: &[Statement]) { + self.transaction_active.store(true, Ordering::Release); + let mut executions = self.executions.write().unwrap(); + for statement in statements { + let intent = Intent { + ddl: is_schema_ddl(statement), + modelled: is_modelled_ddl(statement), + statement: statement.clone(), + }; + if intent.ddl { + self.in_flight_ddl.fetch_add(1, Ordering::AcqRel); + } + executions.push_back(PendingExecution::Execute(Some(Box::new(intent)))); + } + executions.push_back(PendingExecution::ReadyBoundary); + } + + /// Marks the `Sync` boundary whose `ReadyForQuery` terminates an extended + /// protocol batch. PostgreSQL skips the remaining executions in that batch + /// after an error, but may already have a later batch queued behind it. + pub fn protocol_boundary(&self) { + self.executions + .write() + .unwrap() + .push_back(PendingExecution::ReadyBoundary); + } + + pub async fn wait_for_ddl(&self) { + loop { + let notified = self.execution_finished.notified(); + if self.in_flight_ddl.load(Ordering::Acquire) == 0 { + return; + } + notified.await; + } + } + + #[cfg(test)] + pub fn execution_started(&self, statement: Statement) { + let ddl = is_schema_ddl(&statement); + if ddl { + self.in_flight_ddl.fetch_add(1, Ordering::AcqRel); + } + self.executions + .write() + .unwrap() + .push_back(PendingExecution::Execute(Some(Box::new(Intent { + modelled: is_modelled_ddl(&statement), + statement, + ddl, + })))); + } + + pub fn execution_succeeded(&self) { + if let Some(Some(intent)) = self.pop_execution() { + let statement = intent.statement; + match statement { + Statement::Savepoint { name } => { + let resolver = self.resolver(); + let overlay = resolver.as_schema_with_edits().unwrap(); + let checkpoint = overlay.read().unwrap().clone(); + self.savepoints.write().unwrap().push(Savepoint { + name, + schema: checkpoint, + encrypt_config: (*self.encrypt_config()).clone(), + unmodelled: self.has_unmodelled_ddl(), + }); + } + Statement::ReleaseSavepoint { name } => { + let mut savepoints = self.savepoints.write().unwrap(); + if let Some(index) = savepoints + .iter() + .rposition(|savepoint| savepoint.name == name) + { + savepoints.truncate(index); + } + } + Statement::Rollback { + savepoint: Some(name), + .. + } => { + let mut savepoints = self.savepoints.write().unwrap(); + if let Some(index) = savepoints + .iter() + .rposition(|savepoint| savepoint.name == name) + { + let checkpoint = savepoints[index].schema.clone(); + let encrypt_config = savepoints[index].encrypt_config.clone(); + let unmodelled = savepoints[index].unmodelled; + savepoints.truncate(index + 1); + let resolver = self.resolver(); + let overlay = resolver.as_schema_with_edits().unwrap(); + *overlay.write().unwrap() = checkpoint; + *self.encrypt_config.write().unwrap() = Arc::new(encrypt_config); + self.unmodelled.store(unmodelled, Ordering::Release); + self.dirty.store( + unmodelled || resolver.has_schema_changed(), + Ordering::Release, + ); + } + } + Statement::Rollback { + savepoint: None, .. + } => { + *self.resolver.write().unwrap() = Arc::new(TableResolver::new_editable( + self.base.read().unwrap().clone(), + )); + *self.encrypt_config.write().unwrap() = + self.base_encrypt_config.read().unwrap().clone(); + self.savepoints.write().unwrap().clear(); + self.dirty.store(false, Ordering::Release); + self.unmodelled.store(false, Ordering::Release); + } + statement => { + if intent.ddl && !intent.modelled { + self.unmodelled.store(true, Ordering::Release); + } else { + self.apply_ddl(&statement); + } + } + } + if intent.ddl { + self.dirty.store( + self.has_unmodelled_ddl() || self.resolver().has_schema_changed(), + Ordering::Release, + ); + self.in_flight_ddl.fetch_sub(1, Ordering::AcqRel); + self.execution_finished.notify_waiters(); + } + } + } + + fn apply_ddl(&self, statement: &Statement) { + eql_mapper::collect_ddl_with_column_kind(self.resolver(), statement, &|column| { + column_kind(column) + }); + + let mut config = (*self.encrypt_config()).clone(); + apply_encrypt_config(&mut config, statement); + *self.encrypt_config.write().unwrap() = Arc::new(config); + } + + pub fn execution_failed(&self) { + if let Some(Some(intent)) = self.pop_execution() { + if intent.ddl { + self.in_flight_ddl.fetch_sub(1, Ordering::AcqRel); + self.execution_finished.notify_waiters(); + } + } + } + + fn pop_execution(&self) -> Option> { + let mut executions = self.executions.write().unwrap(); + if matches!(executions.front(), Some(PendingExecution::Execute(_))) { + match executions.pop_front().unwrap() { + PendingExecution::Execute(intent) => Some(intent.map(|intent| *intent)), + PendingExecution::ReadyBoundary => unreachable!(), + } + } else { + None + } + } + + fn discard_skipped_executions(&self) { + let mut discarded_ddl = 0; + let mut executions = self.executions.write().unwrap(); + while let Some(execution) = executions.pop_front() { + match execution { + PendingExecution::Execute(Some(intent)) if intent.ddl => discarded_ddl += 1, + PendingExecution::Execute(_) => {} + PendingExecution::ReadyBoundary => break, + } + } + drop(executions); + + if discarded_ddl > 0 { + self.in_flight_ddl + .fetch_sub(discarded_ddl, Ordering::AcqRel); + self.execution_finished.notify_waiters(); + } + } +} + +fn column_domain(column: &ColumnDef) -> String { + column + .data_type + .to_string() + .split('.') + .next_back() + .unwrap_or_default() + .trim_matches('"') + .to_owned() +} + +fn column_kind(column: &ColumnDef) -> ColumnKind { + eql_domains::resolve(&column_domain(column)) + .map(|(identity, traits)| ColumnKind::Eql(traits, identity)) + .unwrap_or(ColumnKind::Native) +} + +fn object_name(name: &ObjectName) -> &str { + match name.0.last() { + Some(ObjectNamePart::Identifier(name)) => &name.value, + _ => "", + } +} + +fn add_column_config(config: &mut EncryptConfig, table: &str, column: &ColumnDef) { + let domain = column_domain(column); + if let Some(column_config) = column_config_from_domain(table, &column.name.value, &domain) { + config.insert( + Identifier::new(table.to_owned(), column.name.value.clone()), + column_config, + ); + } +} + +fn apply_encrypt_config(config: &mut EncryptConfig, statement: &Statement) { + match statement { + Statement::CreateTable(create) => { + let table = object_name(&create.name); + config.remove_table(table); + for column in &create.columns { + add_column_config(config, table, column); + } + } + Statement::AlterTable { + name, operations, .. + } => { + let table = object_name(name); + for operation in operations { + match operation { + AlterTableOperation::AddColumn { column_def, .. } => { + add_column_config(config, table, column_def); + } + AlterTableOperation::DropColumn { column_name, .. } => { + config.remove_column(table, &column_name.value); + } + AlterTableOperation::RenameColumn { + old_column_name, + new_column_name, + } => { + config.rename_column(table, &old_column_name.value, &new_column_name.value); + } + AlterTableOperation::RenameTable { table_name } => { + config.rename_table(table, object_name(table_name)); + } + _ => {} + } + } + } + Statement::Drop { + object_type: ObjectType::Table | ObjectType::View, + names, + .. + } => { + for name in names { + config.remove_table(object_name(name)); + } + } + _ => {} + } +} + +fn is_schema_ddl(statement: &Statement) -> bool { + matches!( + statement, + Statement::CreateTable(_) + | Statement::CreateView { .. } + | Statement::AlterTable { .. } + | Statement::Drop { + object_type: ObjectType::Table | ObjectType::View, + .. + } + ) +} + +fn is_modelled_ddl(statement: &Statement) -> bool { + match statement { + Statement::CreateTable(create) => { + !create.or_replace + && !create.if_not_exists + && !create.temporary + && create.query.is_none() + && create.like.is_none() + && create.clone.is_none() + && create.inherits.is_none() + && create.on_commit.is_none() + } + Statement::AlterTable { operations, .. } => { + operations.iter().all(|operation| match operation { + AlterTableOperation::AddColumn { if_not_exists, .. } => !if_not_exists, + AlterTableOperation::RenameColumn { .. } + | AlterTableOperation::RenameTable { .. } => true, + AlterTableOperation::DropColumn { drop_behavior, .. } => { + *drop_behavior != Some(DropBehavior::Cascade) + } + _ => false, + }) + } + Statement::Drop { + object_type: ObjectType::Table | ObjectType::View, + cascade, + .. + } => !cascade, + Statement::CreateView { .. } => false, + _ => true, + } +} + +#[cfg(test)] +mod tests { + use super::*; + use eql_mapper::Table; + use sqltk::parser::ast::{Ident, ObjectName, ObjectNamePart}; + use sqltk::parser::{dialect::PostgreSqlDialect, parser::Parser}; + + fn parse(sql: &str) -> Statement { + Parser::new(&PostgreSqlDialect {}) + .try_with_sql(sql) + .unwrap() + .parse_statement() + .unwrap() + } + + fn table(name: &str) -> ObjectName { + ObjectName(vec![ObjectNamePart::Identifier(Ident::new(name))]) + } + + #[test] + fn ddl_changes_effective_schema_only_after_successful_execution() { + let middleware = SchemaMiddleware::new(Arc::new(Schema::new("public"))); + let ddl = parse("create table reports (id bigint)"); + + assert!(middleware + .resolver() + .resolve_table(&table("reports")) + .is_err()); + + middleware.execution_started(ddl); + + assert!(middleware + .resolver() + .resolve_table(&table("reports")) + .is_err()); + + middleware.execution_succeeded(); + + assert!(middleware + .resolver() + .resolve_table(&table("reports")) + .is_ok()); + } + + #[test] + fn full_rollback_discards_successful_transaction_ddl() { + let middleware = SchemaMiddleware::new(Arc::new(Schema::new("public"))); + + middleware.execution_started(parse("create table reports (id bigint)")); + middleware.execution_succeeded(); + assert!(middleware + .resolver() + .resolve_table(&table("reports")) + .is_ok()); + + middleware.execution_started(parse("rollback")); + middleware.execution_succeeded(); + + assert!(middleware + .resolver() + .resolve_table(&table("reports")) + .is_err()); + } + + #[test] + fn explicit_commit_keeps_changes_dirty_until_authoritative_publication() { + let middleware = SchemaMiddleware::new(Arc::new(Schema::new("public"))); + + middleware.simple_query(&[parse("begin")]); + middleware.execution_succeeded(); + middleware.execution_started(parse("create table reports (id bigint)")); + middleware.execution_succeeded(); + middleware.ready_for_query(b'T'); + middleware.simple_query(&[parse("commit")]); + middleware.execution_succeeded(); + middleware.ready_for_query(b'I'); + + assert!(middleware.needs_publication()); + middleware.publication_succeeded(); + assert!(!middleware.needs_publication()); + } + + #[test] + fn rollback_to_savepoint_restores_the_overlay_checkpoint() { + let middleware = SchemaMiddleware::new(Arc::new(Schema::new("public"))); + + middleware.execution_started(parse("create table accounts (id bigint)")); + middleware.execution_succeeded(); + middleware.execution_started(parse("savepoint before_reports")); + middleware.execution_succeeded(); + middleware.execution_started(parse("create table reports (id bigint)")); + middleware.execution_succeeded(); + + middleware.execution_started(parse("rollback to savepoint before_reports")); + middleware.execution_succeeded(); + + assert!(middleware + .resolver() + .resolve_table(&table("accounts")) + .is_ok()); + assert!(middleware + .resolver() + .resolve_table(&table("reports")) + .is_err()); + } + + #[test] + fn release_savepoint_preserves_changes_in_the_enclosing_transaction() { + let middleware = SchemaMiddleware::new(Arc::new(Schema::new("public"))); + + middleware.execution_started(parse("create table accounts (id bigint)")); + middleware.execution_succeeded(); + middleware.execution_started(parse("savepoint before_reports")); + middleware.execution_succeeded(); + middleware.execution_started(parse("create table reports (id bigint)")); + middleware.execution_succeeded(); + middleware.execution_started(parse("release savepoint before_reports")); + middleware.execution_succeeded(); + + assert!(middleware + .resolver() + .resolve_table(&table("accounts")) + .is_ok()); + assert!(middleware + .resolver() + .resolve_table(&table("reports")) + .is_ok()); + assert!(middleware.needs_publication()); + } + + #[test] + fn rollback_to_savepoint_preserves_an_earlier_unmodelled_change() { + let middleware = SchemaMiddleware::new(Arc::new(Schema::new("public"))); + + middleware.execution_started(parse("alter table secrets alter column value type text")); + middleware.execution_succeeded(); + middleware.execution_started(parse("savepoint after_unmodelled")); + middleware.execution_succeeded(); + middleware.execution_started(parse("create table reports (id bigint)")); + middleware.execution_succeeded(); + middleware.execution_started(parse("rollback to savepoint after_unmodelled")); + middleware.execution_succeeded(); + + assert!(middleware.has_unmodelled_ddl()); + assert!(middleware.needs_publication()); + } + + #[test] + fn idle_connection_adopts_a_newly_published_snapshot() { + let store = CommittedSchemaStore::for_testing(Schema::new("public"), EncryptConfig::new()); + let middleware = SchemaMiddleware::from_store(store.clone()); + let mut published = Schema::new("public"); + published.add_table(Table::new(Ident::new("reports"))); + + store.publish_for_testing(published, EncryptConfig::new()); + middleware.adopt_latest(); + + assert!(middleware + .resolver() + .resolve_table(&table("reports")) + .is_ok()); + } + + #[test] + fn active_transaction_keeps_its_pinned_snapshot_until_idle() { + let store = CommittedSchemaStore::for_testing(Schema::new("public"), EncryptConfig::new()); + let middleware = SchemaMiddleware::from_store(store.clone()); + let begin = parse("begin"); + middleware.simple_query(&[begin]); + middleware.execution_succeeded(); + + let mut published = Schema::new("public"); + published.add_table(Table::new(Ident::new("reports"))); + store.publish_for_testing(published, EncryptConfig::new()); + middleware.before_statement(); + + assert!(middleware + .resolver() + .resolve_table(&table("reports")) + .is_err()); + middleware.ready_for_query(b'I'); + assert!(middleware + .resolver() + .resolve_table(&table("reports")) + .is_ok()); + } + + #[test] + fn ordinary_extended_execution_pins_the_snapshot_until_readiness() { + let store = CommittedSchemaStore::for_testing(Schema::new("public"), EncryptConfig::new()); + let middleware = SchemaMiddleware::from_store(store.clone()); + let statement = Name::from("statement"); + let portal = Name::from("portal"); + middleware.prepare(statement.clone(), parse("select 1")); + middleware.bind(portal.clone(), &statement); + middleware.execute(&portal); + + let mut published = Schema::new("public"); + published.add_table(Table::new(Ident::new("reports"))); + store.publish_for_testing(published, EncryptConfig::new()); + middleware.before_statement(); + + assert!(middleware + .resolver() + .resolve_table(&table("reports")) + .is_err()); + middleware.execution_succeeded(); + middleware.protocol_boundary(); + middleware.ready_for_query(b'I'); + assert!(middleware + .resolver() + .resolve_table(&table("reports")) + .is_ok()); + } + + #[test] + fn publication_failure_is_visible_to_other_connections() { + let store = CommittedSchemaStore::for_testing(Schema::new("public"), EncryptConfig::new()); + let publisher = SchemaMiddleware::from_store(store.clone()); + let idle_connection = SchemaMiddleware::from_store(store); + + publisher.execution_started(parse("create table reports (id bigint)")); + publisher.execution_succeeded(); + publisher.mark_publication_pending(); + + assert!(idle_connection.requires_publication_before_statement()); + } + + #[test] + fn successful_ddl_activates_schema_and_encryption_metadata_together() { + let middleware = SchemaMiddleware::new(Arc::new(Schema::new("public"))); + + middleware.execution_started(parse( + "create table secrets (id bigint, value eql_v3_text_search)", + )); + middleware.execution_succeeded(); + + let column = middleware + .resolver() + .resolve_table_column(&table("secrets"), &Ident::new("value")) + .unwrap(); + assert!(matches!(column.kind, ColumnKind::Eql(_, _))); + assert!(middleware + .encrypt_config() + .get_column_config(&Identifier::new("secrets".to_owned(), "value".to_owned())) + .is_some()); + } + + #[test] + fn successful_unmodelled_ddl_refuses_later_schema_use_until_rollback() { + let middleware = SchemaMiddleware::new(Arc::new(Schema::new("public"))); + + middleware.execution_started(parse("alter table secrets alter column value type text")); + middleware.execution_succeeded(); + + assert!(middleware.has_unmodelled_ddl()); + assert!(middleware.needs_publication()); + + middleware.execution_started(parse("rollback")); + middleware.execution_succeeded(); + assert!(!middleware.has_unmodelled_ddl()); + } + + #[test] + fn create_table_as_is_unmodelled() { + assert!(!is_modelled_ddl(&parse( + "create table archived_reports as select 1 as id", + ))); + } + + #[test] + fn cascading_column_drop_is_unmodelled() { + assert!(!is_modelled_ddl(&parse( + "alter table reports drop column account_id cascade", + ))); + } + + #[test] + fn conditional_create_and_add_are_unmodelled() { + assert!(!is_modelled_ddl(&parse( + "create table if not exists reports (id bigint)", + ))); + assert!(!is_modelled_ddl(&parse( + "alter table reports add column if not exists account_id bigint", + ))); + } + + #[tokio::test] + async fn dependent_mapping_waits_for_ddl_execution_outcome() { + let middleware = SchemaMiddleware::new(Arc::new(Schema::new("public"))); + middleware.execution_started(parse("create table reports (id bigint)")); + + assert!(tokio::time::timeout( + std::time::Duration::from_millis(20), + middleware.wait_for_ddl(), + ) + .await + .is_err()); + + middleware.execution_failed(); + + tokio::time::timeout( + std::time::Duration::from_millis(20), + middleware.wait_for_ddl(), + ) + .await + .unwrap(); + assert!(middleware + .resolver() + .resolve_table(&table("reports")) + .is_err()); + } + + #[tokio::test] + async fn readiness_discards_pipelined_executions_skipped_after_an_error() { + let middleware = SchemaMiddleware::new(Arc::new(Schema::new("public"))); + middleware.execution_started(parse("create table first_table (id bigint)")); + middleware.execution_started(parse("create table skipped_table (id bigint)")); + middleware.protocol_boundary(); + + middleware.execution_failed(); + middleware.ready_for_query(b'I'); + + tokio::time::timeout( + std::time::Duration::from_millis(20), + middleware.wait_for_ddl(), + ) + .await + .unwrap(); + assert!(middleware + .resolver() + .resolve_table(&table("skipped_table")) + .is_err()); + } + + #[tokio::test] + async fn readiness_does_not_discard_a_later_pipelined_batch() { + let middleware = SchemaMiddleware::new(Arc::new(Schema::new("public"))); + middleware.execution_started(parse("create table failed_table (id bigint)")); + middleware.execution_started(parse("create table skipped_table (id bigint)")); + middleware.protocol_boundary(); + middleware.execution_started(parse("create table later_table (id bigint)")); + middleware.protocol_boundary(); + + middleware.execution_failed(); + middleware.ready_for_query(b'I'); + + assert!(tokio::time::timeout( + std::time::Duration::from_millis(20), + middleware.wait_for_ddl(), + ) + .await + .is_err()); + middleware.execution_succeeded(); + middleware.ready_for_query(b'I'); + middleware.wait_for_ddl().await; + assert!(middleware + .resolver() + .resolve_table(&table("later_table")) + .is_ok()); + } +} diff --git a/packages/cipherstash-proxy/src/proxy/schema/mod.rs b/packages/cipherstash-proxy/src/proxy/schema/mod.rs index c34d83ce0..acb17f3fd 100644 --- a/packages/cipherstash-proxy/src/proxy/schema/mod.rs +++ b/packages/cipherstash-proxy/src/proxy/schema/mod.rs @@ -1,4 +1,6 @@ mod eql_domains; mod manager; +mod middleware; -pub use manager::SchemaManager; +pub use manager::{CommittedSchemaStore, SchemaManager}; +pub use middleware::SchemaMiddleware; diff --git a/packages/eql-mapper/src/model/schema_delta.rs b/packages/eql-mapper/src/model/schema_delta.rs index f78d953b2..22198bacb 100644 --- a/packages/eql-mapper/src/model/schema_delta.rs +++ b/packages/eql-mapper/src/model/schema_delta.rs @@ -23,7 +23,7 @@ use super::{ /// /// All table and column lookups during EQL mapping will go through via the overlay scheme, falling back to the /// loaded schema. -#[derive(Debug)] +#[derive(Clone, Debug)] pub struct SchemaWithEdits { schema: Arc, overlays: HashMap, @@ -114,7 +114,7 @@ impl SchemaWithEdits { } /// Acts like a mask over a table or an existing table that has been dropped in the current transaction. -#[derive(Debug)] +#[derive(Clone, Debug)] enum Overlay { /// Hides the existence of table in the main [`Schema`] causing resolution of that table to fail. Dropped, @@ -190,10 +190,24 @@ impl From<&OverlayTable> for Table { /// /// Returns `true` if `statement` contained relevant DDL (regardless of `TableResolver` variant). pub fn collect_ddl(table_resolver: Arc, statement: &Statement) -> bool { + collect_ddl_with_column_kind(table_resolver, statement, &|_| ColumnKind::Native) +} + +/// Applies DDL using the caller's classification for newly declared columns. +/// +/// The mapper itself defaults columns to native because PostgreSQL domain +/// identity belongs to Proxy. Proxy supplies the catalog-backed classifier so +/// a transaction-local EQL domain has the same type as its eventual catalog row. +pub fn collect_ddl_with_column_kind( + table_resolver: Arc, + statement: &Statement, + classify: &dyn Fn(&ColumnDef) -> ColumnKind, +) -> bool { if let Some(schema_with_edits) = table_resolver.as_schema_with_edits() { let mut visitor = DdlCollector { schema: schema_with_edits, changed: false, + classify, }; let _ = statement.accept(&mut visitor); return visitor.changed; @@ -202,12 +216,13 @@ pub fn collect_ddl(table_resolver: Arc, statement: &Statement) -> table_resolver.has_schema_changed() } -struct DdlCollector { +struct DdlCollector<'a> { schema: Arc>, changed: bool, + classify: &'a dyn Fn(&ColumnDef) -> ColumnKind, } -impl DdlCollector { +impl DdlCollector<'_> { fn capture_create_view(&self, name: &ObjectName, columns: &[ViewColumnDef]) { let name = name.clone(); let mut table = OverlayTable::new(name.clone()); @@ -228,7 +243,7 @@ impl DdlCollector { for def in columns { table.add_column(Column { name: def.name.clone(), - kind: ColumnKind::Native, + kind: (self.classify)(def), }); } @@ -244,7 +259,7 @@ impl DdlCollector { if let Overlay::Table(table) = overlay { table.add_column(Column { name: column_def.name.clone(), - kind: ColumnKind::Native, + kind: (self.classify)(column_def), }); } } @@ -317,7 +332,7 @@ impl DdlCollector { } } -impl<'ast> Visitor<'ast> for DdlCollector { +impl<'ast> Visitor<'ast> for DdlCollector<'_> { type Error = Infallible; fn enter(&mut self, node: &'ast N) -> ControlFlow> { From 5cd5ef4e0bb9fcc73a79e607c60dbe8e85e00208 Mon Sep 17 00:00:00 2001 From: James Sadler Date: Sun, 23 Aug 2026 21:36:06 +1000 Subject: [PATCH 3/4] fix(proxy): preserve native simple-query DDL batches Restrict the dependent post-DDL guard to schema changes that can alter encryption metadata, preserving pgx temporary-table setup batches while continuing to fail closed for encrypted DDL followed by mapped work. Document every BUG-308 production module, type, and function. Expand the schema middleware module docs with its authority, atomicity, protocol-ordering, transaction, publication, and failure invariants plus current and intrinsic limitations. Add focused regression coverage and verify the previously failing Go integration path against the rebuilt Proxy image. Signed-off-by: James Sadler --- packages/cipherstash-proxy/src/error.rs | 2 + .../src/postgresql/context/mod.rs | 23 +++ .../src/postgresql/frontend.rs | 26 +-- .../src/proxy/encrypt_config/from_domain.rs | 3 +- .../src/proxy/encrypt_config/manager.rs | 12 ++ .../src/proxy/encrypt_config/mod.rs | 1 + packages/cipherstash-proxy/src/proxy/mod.rs | 2 + .../src/proxy/schema/manager.rs | 21 ++- .../src/proxy/schema/middleware.rs | 173 ++++++++++++++++++ .../cipherstash-proxy/src/proxy/schema/mod.rs | 5 + 10 files changed, 245 insertions(+), 23 deletions(-) diff --git a/packages/cipherstash-proxy/src/error.rs b/packages/cipherstash-proxy/src/error.rs index 965550c97..39f70f3c0 100644 --- a/packages/cipherstash-proxy/src/error.rs +++ b/packages/cipherstash-proxy/src/error.rs @@ -103,9 +103,11 @@ pub enum ZeroKMSError { #[derive(Error, Debug)] pub enum MappingError { + /// A simple-query batch would map against schema changed earlier in the batch. #[error("A simple-query batch cannot contain a schema-dependent statement after DDL. Send the DDL and dependent statement as separate queries. For help visit {}#mapping-dependent-statement-after-ddl", ERROR_DOC_BASE_URL)] DependentStatementAfterDdl, + /// Confirmed DDL cannot be represented safely by the transaction overlay. #[error("A successful schema change in this transaction cannot be modelled safely. Roll back the transaction before issuing schema-dependent statements. For help visit {}#mapping-unmodelled-ddl", ERROR_DOC_BASE_URL)] UnmodelledDdl, diff --git a/packages/cipherstash-proxy/src/postgresql/context/mod.rs b/packages/cipherstash-proxy/src/postgresql/context/mod.rs index 7301dca34..52c4ea8b5 100644 --- a/packages/cipherstash-proxy/src/postgresql/context/mod.rs +++ b/packages/cipherstash-proxy/src/postgresql/context/mod.rs @@ -182,6 +182,7 @@ where Self::new_with_schema_store(client_id, config, schema_store, encryption, reload_sender) } + /// Constructs a connection context over the shared committed schema store. pub fn new_with_schema_store( client_id: i32, config: Arc, @@ -571,42 +572,61 @@ where Some(session_context.to_owned()) } + /// Returns the resolver for this connection's effective schema snapshot. pub fn get_table_resolver(&self) -> Arc { self.schema_middleware.resolver() } + /// Records schema intent for a parsed prepared statement. pub fn prepare_schema_statement(&self, name: Name, statement: sqltk::parser::ast::Statement) { self.schema_middleware.prepare(name, statement); } + /// Associates a bound portal with its prepared statement's schema intent. pub fn bind_schema_statement(&self, portal: Name, prepared_statement: &Name) { self.schema_middleware.bind(portal, prepared_statement); } + /// Records a portal execution awaiting its backend result. pub fn execute_schema_portal(&self, portal: &Name) { self.schema_middleware.execute(portal); } + /// Records statements in one simple-query protocol message. pub fn execute_simple_schema_statements(&self, statements: &[sqltk::parser::ast::Statement]) { self.schema_middleware.simple_query(statements); } + /// Returns whether a simple-query batch must be rejected to protect encryption. + pub fn simple_query_requires_fail_closed( + &self, + statements: &[sqltk::parser::ast::Statement], + ) -> bool { + self.schema_middleware + .simple_query_requires_fail_closed(statements) + } + + /// Records an extended-protocol synchronization boundary. pub fn mark_schema_protocol_boundary(&self) { self.schema_middleware.protocol_boundary(); } + /// Reports successful execution of the next queued statement. pub fn schema_execution_succeeded(&self) { self.schema_middleware.execution_succeeded(); } + /// Reports failed execution of the next queued statement. pub fn schema_execution_failed(&self) { self.schema_middleware.execution_failed(); } + /// Waits for preceding schema-changing executions to resolve. pub async fn wait_for_schema_execution(&self) { self.schema_middleware.wait_for_ddl().await; } + /// Refuses schema-dependent work after confirmed unmodelled DDL. pub fn ensure_schema_modelled(&self) -> Result<(), Error> { if self.schema_middleware.has_unmodelled_ddl() { return Err(crate::error::MappingError::UnmodelledDdl.into()); @@ -614,10 +634,12 @@ where Ok(()) } + /// Replaces idle connection-local state with the latest committed snapshot. pub fn adopt_latest_schema(&self) { self.schema_middleware.adopt_latest(); } + /// Publishes pending shared state, then prepares the effective schema for mapping. pub async fn prepare_schema_for_statement(&self) -> Result<(), Error> { if self .schema_middleware @@ -632,6 +654,7 @@ where Ok(()) } + /// Reports a readiness boundary and PostgreSQL transaction status. pub fn schema_ready_for_query(&self, status: u8) { self.schema_middleware.ready_for_query(status); } diff --git a/packages/cipherstash-proxy/src/postgresql/frontend.rs b/packages/cipherstash-proxy/src/postgresql/frontend.rs index 2c0db166a..87b61b093 100644 --- a/packages/cipherstash-proxy/src/postgresql/frontend.rs +++ b/packages/cipherstash-proxy/src/postgresql/frontend.rs @@ -48,19 +48,6 @@ use std::time::Instant; use tokio::io::{AsyncRead, AsyncWrite, AsyncWriteExt}; use tracing::{debug, error, info, warn}; -fn is_schema_ddl(statement: &ast::Statement) -> bool { - matches!( - statement, - ast::Statement::CreateTable(_) - | ast::Statement::CreateView { .. } - | ast::Statement::AlterTable { .. } - | ast::Statement::Drop { - object_type: ast::ObjectType::Table | ast::ObjectType::View, - .. - } - ) -} - /// The PostgreSQL proxy frontend that handles client-to-server message processing. /// /// The Frontend intercepts messages from PostgreSQL clients, analyzes SQL statements for @@ -452,14 +439,11 @@ where // Simple Query may contain many statements let parsed_statements = SqlParser::parse_statements(&query.statement)?; self.context.prepare_schema_for_statement().await?; - if let Some(ddl_index) = parsed_statements.iter().position(is_schema_ddl) { - if parsed_statements - .iter() - .skip(ddl_index + 1) - .any(eql_mapper::requires_type_check) - { - return Err(MappingError::DependentStatementAfterDdl.into()); - } + if self + .context + .simple_query_requires_fail_closed(&parsed_statements) + { + return Err(MappingError::DependentStatementAfterDdl.into()); } if parsed_statements .iter() diff --git a/packages/cipherstash-proxy/src/proxy/encrypt_config/from_domain.rs b/packages/cipherstash-proxy/src/proxy/encrypt_config/from_domain.rs index 1fc0958f7..d87130332 100644 --- a/packages/cipherstash-proxy/src/proxy/encrypt_config/from_domain.rs +++ b/packages/cipherstash-proxy/src/proxy/encrypt_config/from_domain.rs @@ -3,7 +3,8 @@ //! EQL v3 domain types are self-configuring: a column's Postgres domain (e.g. //! `eql_v3_text_search`) encodes both the plaintext token type and the //! searchable-encryption terms it stores. That is enough to build the -//! [`ColumnConfig`] the encrypt pipeline needs, so `eql_v2.add_search_config` +//! [`cipherstash_client::schema::ColumnConfig`] the encrypt pipeline needs, so +//! `eql_v2.add_search_config` //! and the `eql_v2_configuration` table are redundant. //! //! The SEM term → index mapping mirrors the client's indexers (verified against diff --git a/packages/cipherstash-proxy/src/proxy/encrypt_config/manager.rs b/packages/cipherstash-proxy/src/proxy/encrypt_config/manager.rs index fffbea825..47486f6d1 100644 --- a/packages/cipherstash-proxy/src/proxy/encrypt_config/manager.rs +++ b/packages/cipherstash-proxy/src/proxy/encrypt_config/manager.rs @@ -39,20 +39,31 @@ impl EncryptConfig { self.config.get(identifier).cloned() } + /// Returns whether any encrypted column belongs to `table`. + pub(crate) fn contains_table(&self, table: &str) -> bool { + self.config + .keys() + .any(|identifier| identifier.table == table) + } + + /// Inserts or replaces encryption metadata for one column. pub(crate) fn insert(&mut self, identifier: eql::Identifier, config: ColumnConfig) { self.config.insert(identifier, config); } + /// Removes encryption metadata for one column. pub(crate) fn remove_column(&mut self, table: &str, column: &str) { self.config .remove(&eql::Identifier::new(table.to_owned(), column.to_owned())); } + /// Removes all encryption metadata for a table. pub(crate) fn remove_table(&mut self, table: &str) { self.config .retain(|identifier, _| identifier.table != table); } + /// Moves encryption metadata to a renamed column identifier. pub(crate) fn rename_column(&mut self, table: &str, from: &str, to: &str) { let from = eql::Identifier::new(table.to_owned(), from.to_owned()); if let Some(config) = self.config.remove(&from) { @@ -63,6 +74,7 @@ impl EncryptConfig { } } + /// Moves all encryption metadata to a renamed table identifier. pub(crate) fn rename_table(&mut self, from: &str, to: &str) { let renamed = self .config diff --git a/packages/cipherstash-proxy/src/proxy/encrypt_config/mod.rs b/packages/cipherstash-proxy/src/proxy/encrypt_config/mod.rs index 68e26e2c9..673fb7548 100644 --- a/packages/cipherstash-proxy/src/proxy/encrypt_config/mod.rs +++ b/packages/cipherstash-proxy/src/proxy/encrypt_config/mod.rs @@ -1,3 +1,4 @@ +/// Derives encryption metadata from EQL domain declarations. pub(crate) mod from_domain; mod manager; diff --git a/packages/cipherstash-proxy/src/proxy/mod.rs b/packages/cipherstash-proxy/src/proxy/mod.rs index c1c252565..cd394df28 100644 --- a/packages/cipherstash-proxy/src/proxy/mod.rs +++ b/packages/cipherstash-proxy/src/proxy/mod.rs @@ -13,6 +13,7 @@ use tokio::sync::oneshot::Sender; use tracing::{debug, warn}; mod encrypt_config; +/// Transaction-aware schema snapshots, overlays, and publication. pub(crate) mod schema; mod zerokms; @@ -95,6 +96,7 @@ impl Proxy { Ok(version) } + /// Starts the asynchronous coordinator for schema reload requests. pub fn receive(mut reload_receiver: ReloadReceiver, schema_manager: SchemaManager) { tokio::task::spawn(async move { while let Some(command) = reload_receiver.recv().await { diff --git a/packages/cipherstash-proxy/src/proxy/schema/manager.rs b/packages/cipherstash-proxy/src/proxy/schema/manager.rs index ffef703f9..75c702094 100644 --- a/packages/cipherstash-proxy/src/proxy/schema/manager.rs +++ b/packages/cipherstash-proxy/src/proxy/schema/manager.rs @@ -19,6 +19,7 @@ use tokio::{sync::Mutex, task::JoinHandle, time}; use tracing::{debug, info, warn}; #[derive(Clone, Debug)] +/// An immutable, atomically published schema and encryption-metadata generation. pub struct CommittedSchemaSnapshot { version: u64, schema: Arc, @@ -26,6 +27,7 @@ pub struct CommittedSchemaSnapshot { } #[derive(Clone, Debug)] +/// Shared access to committed snapshots and their publication generations. pub struct CommittedSchemaStore { snapshot: Arc>, requested_publication: Arc, @@ -33,6 +35,7 @@ pub struct CommittedSchemaStore { } impl CommittedSchemaStore { + /// Creates the initial committed generation from aligned schema metadata. pub(crate) fn from_parts(schema: Schema, encrypt_config: EncryptConfig) -> Self { Self { snapshot: Arc::new(ArcSwap::new(Arc::new(CommittedSchemaSnapshot::new( @@ -45,19 +48,23 @@ impl CommittedSchemaStore { } } + /// Loads one internally consistent committed snapshot. pub fn load(&self) -> Arc { self.snapshot.load().clone() } + /// Returns whether a requested publication has not yet completed. pub fn publication_pending(&self) -> bool { self.requested_publication.load(Ordering::Acquire) > self.published_publication.load(Ordering::Acquire) } + /// Advances the requested publication generation. pub fn mark_publication_pending(&self) { self.requested_publication.fetch_add(1, Ordering::AcqRel); } + /// Marks all currently requested publication generations as satisfied. pub(crate) fn publication_succeeded(&self) { self.published_publication.store( self.requested_publication.load(Ordering::Acquire), @@ -66,11 +73,13 @@ impl CommittedSchemaStore { } #[cfg(test)] + /// Creates a committed store without connecting to PostgreSQL. pub fn for_testing(schema: Schema, encrypt_config: EncryptConfig) -> Self { Self::from_parts(schema, encrypt_config) } #[cfg(test)] + /// Publishes an aligned test snapshot as the next version. pub fn publish_for_testing(&self, schema: Schema, encrypt_config: EncryptConfig) { let version = self.load().version() + 1; self.snapshot.store(Arc::new(CommittedSchemaSnapshot::new( @@ -83,6 +92,7 @@ impl CommittedSchemaStore { } impl CommittedSchemaSnapshot { + /// Creates an immutable snapshot at an explicit monotonic version. fn new(version: u64, schema: Schema, encrypt_config: EncryptConfig) -> Self { Self { version, @@ -91,14 +101,17 @@ impl CommittedSchemaSnapshot { } } + /// Returns the monotonic snapshot version. pub fn version(&self) -> u64 { self.version } + /// Returns the structural schema from this generation. pub fn schema(&self) -> Arc { self.schema.clone() } + /// Returns encryption metadata from the same generation as the schema. pub fn encrypt_config(&self) -> Arc { self.encrypt_config.clone() } @@ -121,10 +134,12 @@ impl SchemaManager { init_reloader(config).await } + /// Loads the current atomic committed snapshot. pub fn load(&self) -> Arc { self.snapshot.load().clone() } + /// Returns a cloneable store for per-connection schema middleware. pub fn store(&self) -> CommittedSchemaStore { CommittedSchemaStore { snapshot: self.snapshot.clone(), @@ -146,6 +161,7 @@ impl SchemaManager { } } +/// Coalesces concurrent reload requests while preserving generation ordering. async fn coalesced_reload( snapshot: Arc>, requested_generation: Arc, @@ -193,6 +209,7 @@ where } } +/// Publishes a candidate only when it advances the committed version. fn publish_if_newer( store: &ArcSwap, candidate: CommittedSchemaSnapshot, @@ -204,6 +221,7 @@ fn publish_if_newer( true } +/// Loads the initial snapshot and starts periodic authoritative refreshes. async fn init_reloader(config: DatabaseConfig) -> Result { // Skip retries on startup as the likely failure mode is configuration let (schema, encrypt_config) = load_snapshot(&config).await?; @@ -260,7 +278,7 @@ async fn init_reloader(config: DatabaseConfig) -> Result { }) } -/// Fetch the dataset and retry on any error +/// Fetches an atomic snapshot and retries transient catalog-read failures. /// /// When databases and the proxy start up at the same time they might not be ready to accept connections before the /// proxy tries to query the schema. To give the proxy the best chance of initialising correctly this method will @@ -355,6 +373,7 @@ pub async fn load_schema(config: &DatabaseConfig) -> Result { load_snapshot(config).await.map(|(schema, _)| schema) } +/// Reads schema and encryption metadata in one repeatable-read transaction. async fn load_snapshot(config: &DatabaseConfig) -> Result<(Schema, EncryptConfig), Error> { let client = connect::database(config).await?; client diff --git a/packages/cipherstash-proxy/src/proxy/schema/middleware.rs b/packages/cipherstash-proxy/src/proxy/schema/middleware.rs index d61175a9b..4680bb925 100644 --- a/packages/cipherstash-proxy/src/proxy/schema/middleware.rs +++ b/packages/cipherstash-proxy/src/proxy/schema/middleware.rs @@ -1,3 +1,60 @@ +//! Transaction-aware coordination between PostgreSQL protocol events and schema mapping. +//! +//! # Principles of operation +//! +//! PostgreSQL is the authority. Proxy never publishes a schema inferred from client SQL to +//! other connections. Instead, [`CommittedSchemaStore`] exposes immutable, monotonically +//! versioned snapshots loaded from PostgreSQL. Each snapshot contains both structural schema +//! and encryption metadata, so a mapper can never observe a table definition from one catalog +//! generation and encryption rules from another. +//! +//! A connection adopts a committed snapshot while idle and pins it when work begins. Confirmed +//! DDL is applied to a connection-local overlay, producing the connection's effective schema: +//! the pinned committed snapshot plus successful changes in the current transaction. Savepoints +//! checkpoint both the structural overlay and encryption metadata. Full rollback discards the +//! overlay; rollback to a savepoint restores its checkpoint; release keeps the changes in the +//! enclosing transaction. +//! +//! Protocol events, rather than parsing alone, drive state changes: +//! +//! 1. `Parse` records DDL intent under the prepared-statement name. +//! 2. `Bind` associates that intent with a portal. +//! 3. `Execute` queues the intent and marks DDL as in flight. +//! 4. Backend success activates the change; backend failure discards it. +//! 5. `Sync` and simple-query boundaries segment the execution queue because PostgreSQL skips +//! the rest of an extended-protocol batch after an error. +//! +//! Schema-dependent work waits behind in-flight DDL, which makes pipelining correct without +//! guessing whether PostgreSQL will accept the change. At outermost commit, the connection asks +//! the schema manager to reload PostgreSQL before successful idle readiness reaches the client. +//! Publication requests are shared, coalesced, and generation ordered. If publication fails, +//! the committed database transaction cannot be undone; Proxy therefore retains the pending +//! publication, withholds successful readiness, and closes that client connection. +//! +//! # Trade-offs and limitations +//! +//! * The overlay intentionally models a small, deterministic DDL subset. Conditional DDL, +//! cascading changes, `CREATE TABLE AS`/`LIKE`/`CLONE`/`INHERITS`, views, and unsupported +//! `ALTER TABLE` operations are treated as unmodelled. After such DDL succeeds, later +//! schema-dependent statements fail closed until rollback or authoritative publication. +//! * A simple-query message has one PostgreSQL response boundary, so Proxy cannot safely remap a +//! later statement after observing an earlier statement's result. Batches that may change +//! encryption metadata and then perform schema-dependent work are rejected. Explicitly native +//! table DDL remains compatible because it cannot create an encryption obligation. +//! * Temporary and other connection-local catalog objects are invisible to the separate +//! publication connection. Native temporary-table batches may pass through, but encrypted +//! temporary objects cannot be represented or authoritatively published and are unsupported. +//! * SQL executed indirectly by procedures, extensions, or dynamic SQL cannot be inferred from +//! the wire statement. Periodic authoritative reloads eventually discover committed global +//! changes; a transaction still keeps its pinned view for consistency. +//! * Prepared-statement and portal intents are retained for the connection lifetime. Reusing a +//! protocol name replaces its entry, but closing many unique names can retain bounded metadata +//! until the client disconnects. +//! +//! For supported schema changes, these conservative refusals trade some PostgreSQL surface-area +//! compatibility for the core invariant: Proxy must never forward plaintext because it +//! speculated about uncommitted or incompletely modelled schema state. + use super::eql_domains; use super::manager::CommittedSchemaStore; use crate::postgresql::Name; @@ -17,6 +74,7 @@ use std::sync::{ use tokio::sync::Notify; #[derive(Clone, Debug)] +/// The schema-relevant meaning recorded for a parsed statement. struct Intent { statement: Statement, ddl: bool, @@ -24,12 +82,16 @@ struct Intent { } #[derive(Clone, Debug)] +/// One execution or synchronization boundary awaiting a backend outcome. enum PendingExecution { + /// An execution, optionally carrying schema intent. Execute(Option>), + /// The boundary terminated by the next `ReadyForQuery` message. ReadyBoundary, } #[derive(Clone, Debug)] +/// A transaction savepoint and its corresponding schema checkpoint. struct Savepoint { name: Ident, schema: SchemaWithEdits, @@ -61,6 +123,7 @@ pub struct SchemaMiddleware { impl SchemaMiddleware { #[cfg(test)] + /// Constructs middleware around a schema-only snapshot for unit tests. pub fn new(schema: Arc) -> Self { Self::from_store(CommittedSchemaStore::for_testing( (*schema).clone(), @@ -68,6 +131,7 @@ impl SchemaMiddleware { )) } + /// Constructs connection-local middleware backed by the shared committed store. pub fn from_store(store: CommittedSchemaStore) -> Self { let snapshot = store.load(); let schema = snapshot.schema(); @@ -89,14 +153,17 @@ impl SchemaMiddleware { } } + /// Returns the resolver for the connection's current effective schema. pub fn resolver(&self) -> Arc { self.resolver.read().unwrap().clone() } + /// Returns encryption metadata aligned with the current effective schema. pub fn encrypt_config(&self) -> Arc { self.encrypt_config.read().unwrap().clone() } + /// Replaces connection-local state with the latest committed snapshot. pub fn adopt_latest(&self) { let snapshot = self.store.load(); let schema = snapshot.schema(); @@ -109,33 +176,40 @@ impl SchemaMiddleware { self.unmodelled.store(false, Ordering::Release); } + /// Returns whether authoritative catalog publication is still required. pub fn needs_publication(&self) -> bool { self.dirty.load(Ordering::Acquire) || self.store.publication_pending() } + /// Returns whether this connection has confirmed, unpublished DDL changes. pub fn has_local_changes(&self) -> bool { self.dirty.load(Ordering::Acquire) } + /// Records a shared publication request for the connection's committed DDL. pub fn mark_publication_pending(&self) { self.store.mark_publication_pending(); } + /// Returns whether an idle connection must publish pending catalog state before use. pub fn requires_publication_before_statement(&self) -> bool { !self.transaction_active.load(Ordering::Acquire) && !self.has_local_changes() && self.store.publication_pending() } + /// Clears local dirty state and adopts the newly published snapshot. pub fn publication_succeeded(&self) { self.dirty.store(false, Ordering::Release); self.adopt_latest(); } + /// Returns whether confirmed DDL cannot be represented by the local overlay. pub fn has_unmodelled_ddl(&self) -> bool { self.unmodelled.load(Ordering::Acquire) } + /// Adopts a newer committed snapshot before an idle connection starts work. pub fn before_statement(&self) { if !self.transaction_active.load(Ordering::Acquire) && !self.has_local_changes() @@ -146,6 +220,7 @@ impl SchemaMiddleware { } } + /// Applies a PostgreSQL readiness boundary and its transaction status. pub fn ready_for_query(&self, status: u8) { self.discard_skipped_executions(); self.transaction_active @@ -155,6 +230,7 @@ impl SchemaMiddleware { } } + /// Records schema intent for a named prepared statement. pub fn prepare(&self, name: Name, statement: Statement) { self.prepared.write().unwrap().insert( name, @@ -166,6 +242,7 @@ impl SchemaMiddleware { ); } + /// Associates a portal with the schema intent of its prepared statement. pub fn bind(&self, portal: Name, prepared_statement: &Name) { let intent = self .prepared @@ -184,6 +261,7 @@ impl SchemaMiddleware { } } + /// Records a portal execution awaiting a backend success or failure response. pub fn execute(&self, portal: &Name) { let intent = self.portals.read().unwrap().get(portal).cloned(); self.transaction_active.store(true, Ordering::Release); @@ -196,6 +274,7 @@ impl SchemaMiddleware { .push_back(PendingExecution::Execute(intent.map(Box::new))); } + /// Records every statement and readiness boundary in a simple-query message. pub fn simple_query(&self, statements: &[Statement]) { self.transaction_active.store(true, Ordering::Release); let mut executions = self.executions.write().unwrap(); @@ -213,6 +292,64 @@ impl SchemaMiddleware { executions.push_back(PendingExecution::ReadyBoundary); } + /// Returns whether mapping a later statement in this simple-query batch + /// could observe encryption metadata changed by an earlier DDL statement. + pub fn simple_query_requires_fail_closed(&self, statements: &[Statement]) -> bool { + let mut encryption_changing_ddl_seen = false; + + for statement in statements { + if encryption_changing_ddl_seen && eql_mapper::requires_type_check(statement) { + return true; + } + encryption_changing_ddl_seen |= self.ddl_may_change_encryption(statement); + } + + false + } + + /// Conservatively classifies DDL that can change encryption metadata. + fn ddl_may_change_encryption(&self, statement: &Statement) -> bool { + match statement { + Statement::CreateTable(create) => { + create.query.is_some() + || create.like.is_some() + || create.clone.is_some() + || create.inherits.is_some() + || create + .columns + .iter() + .any(|column| matches!(column_kind(column), ColumnKind::Eql(_, _))) + } + Statement::AlterTable { + name, operations, .. + } => { + self.encrypt_config().contains_table(object_name(name)) + || operations.iter().any(|operation| match operation { + AlterTableOperation::AddColumn { column_def, .. } => { + matches!(column_kind(column_def), ColumnKind::Eql(_, _)) + } + AlterTableOperation::DropColumn { .. } + | AlterTableOperation::RenameColumn { .. } + | AlterTableOperation::RenameTable { .. } => false, + _ => true, + }) + } + Statement::Drop { + object_type: ObjectType::Table, + names, + .. + } => names + .iter() + .any(|name| self.encrypt_config().contains_table(object_name(name))), + Statement::CreateView { .. } + | Statement::Drop { + object_type: ObjectType::View, + .. + } => true, + _ => false, + } + } + /// Marks the `Sync` boundary whose `ReadyForQuery` terminates an extended /// protocol batch. PostgreSQL skips the remaining executions in that batch /// after an error, but may already have a later batch queued behind it. @@ -223,6 +360,7 @@ impl SchemaMiddleware { .push_back(PendingExecution::ReadyBoundary); } + /// Waits until all earlier schema-changing executions have resolved. pub async fn wait_for_ddl(&self) { loop { let notified = self.execution_finished.notified(); @@ -234,6 +372,7 @@ impl SchemaMiddleware { } #[cfg(test)] + /// Records a direct statement execution for state-machine tests. pub fn execution_started(&self, statement: Statement) { let ddl = is_schema_ddl(&statement); if ddl { @@ -249,6 +388,7 @@ impl SchemaMiddleware { })))); } + /// Applies the next queued execution after PostgreSQL confirms success. pub fn execution_succeeded(&self) { if let Some(Some(intent)) = self.pop_execution() { let statement = intent.statement; @@ -328,6 +468,7 @@ impl SchemaMiddleware { } } + /// Applies modelled DDL to schema and encryption overlays as one operation. fn apply_ddl(&self, statement: &Statement) { eql_mapper::collect_ddl_with_column_kind(self.resolver(), statement, &|column| { column_kind(column) @@ -338,6 +479,7 @@ impl SchemaMiddleware { *self.encrypt_config.write().unwrap() = Arc::new(config); } + /// Discards the next queued execution after PostgreSQL reports failure. pub fn execution_failed(&self) { if let Some(Some(intent)) = self.pop_execution() { if intent.ddl { @@ -347,6 +489,7 @@ impl SchemaMiddleware { } } + /// Removes the next execution without crossing a readiness boundary. fn pop_execution(&self) -> Option> { let mut executions = self.executions.write().unwrap(); if matches!(executions.front(), Some(PendingExecution::Execute(_))) { @@ -359,6 +502,7 @@ impl SchemaMiddleware { } } + /// Discards executions PostgreSQL skipped after an error up to readiness. fn discard_skipped_executions(&self) { let mut discarded_ddl = 0; let mut executions = self.executions.write().unwrap(); @@ -379,6 +523,7 @@ impl SchemaMiddleware { } } +/// Returns the unqualified PostgreSQL domain name declared for a column. fn column_domain(column: &ColumnDef) -> String { column .data_type @@ -390,12 +535,14 @@ fn column_domain(column: &ColumnDef) -> String { .to_owned() } +/// Classifies a declared column as native or as an EQL domain. fn column_kind(column: &ColumnDef) -> ColumnKind { eql_domains::resolve(&column_domain(column)) .map(|(identity, traits)| ColumnKind::Eql(traits, identity)) .unwrap_or(ColumnKind::Native) } +/// Returns the final identifier component of a PostgreSQL object name. fn object_name(name: &ObjectName) -> &str { match name.0.last() { Some(ObjectNamePart::Identifier(name)) => &name.value, @@ -403,6 +550,7 @@ fn object_name(name: &ObjectName) -> &str { } } +/// Adds inferred encryption metadata for one newly declared column. fn add_column_config(config: &mut EncryptConfig, table: &str, column: &ColumnDef) { let domain = column_domain(column); if let Some(column_config) = column_config_from_domain(table, &column.name.value, &domain) { @@ -413,6 +561,7 @@ fn add_column_config(config: &mut EncryptConfig, table: &str, column: &ColumnDef } } +/// Applies modelled DDL to a mutable encryption-metadata overlay. fn apply_encrypt_config(config: &mut EncryptConfig, statement: &Statement) { match statement { Statement::CreateTable(create) => { @@ -460,6 +609,7 @@ fn apply_encrypt_config(config: &mut EncryptConfig, statement: &Statement) { } } +/// Returns whether a statement changes schema state tracked by the middleware. fn is_schema_ddl(statement: &Statement) -> bool { matches!( statement, @@ -473,6 +623,7 @@ fn is_schema_ddl(statement: &Statement) -> bool { ) } +/// Returns whether a DDL statement can be represented exactly by the overlay. fn is_modelled_ddl(statement: &Statement) -> bool { match statement { Statement::CreateTable(create) => { @@ -793,6 +944,28 @@ mod tests { ))); } + #[test] + fn native_temporary_table_batch_does_not_fail_closed() { + let middleware = SchemaMiddleware::new(Arc::new(Schema::new("public"))); + let statements = vec![ + parse("create temporary table names (name text)"), + parse("insert into names (name) values ('Ada')"), + ]; + + assert!(!middleware.simple_query_requires_fail_closed(&statements)); + } + + #[test] + fn encrypted_table_batch_fails_closed_before_dependent_insert() { + let middleware = SchemaMiddleware::new(Arc::new(Schema::new("public"))); + let statements = vec![ + parse("create table secrets (value eql_v3_text_search)"), + parse("insert into secrets (value) values ('classified')"), + ]; + + assert!(middleware.simple_query_requires_fail_closed(&statements)); + } + #[tokio::test] async fn dependent_mapping_waits_for_ddl_execution_outcome() { let middleware = SchemaMiddleware::new(Arc::new(Schema::new("public"))); diff --git a/packages/cipherstash-proxy/src/proxy/schema/mod.rs b/packages/cipherstash-proxy/src/proxy/schema/mod.rs index acb17f3fd..c4f5bb9b9 100644 --- a/packages/cipherstash-proxy/src/proxy/schema/mod.rs +++ b/packages/cipherstash-proxy/src/proxy/schema/mod.rs @@ -1,5 +1,10 @@ +//! Schema loading, committed snapshots, and connection-local transactional overlays. + +/// Resolves EQL domain identities and capabilities. mod eql_domains; +/// Loads and atomically publishes authoritative committed snapshots. mod manager; +/// Coordinates schema state with PostgreSQL transaction and protocol events. mod middleware; pub use manager::{CommittedSchemaStore, SchemaManager}; From fa6bf25afe68f7dcbd37572588b0814705547b70 Mon Sep 17 00:00:00 2001 From: James Sadler Date: Mon, 24 Aug 2026 12:14:12 +1000 Subject: [PATCH 4/4] fix(proxy): harden transaction-aware schema handling Normalize PostgreSQL identifiers in domain prediction, overlays, and savepoint matching. Fail closed when savepoint state cannot be reconciled, while accepting encryption-neutral ALTER TABLE operations and ignoring safe connection-local temporary tables. Inject Flush after extended-protocol DDL Execute so clients can pipeline dependent Parse messages before a single Sync without deadlock. Track simple-query intents from the statements actually forwarded, including compatibility fallback paths, and centralize typed ReadyForQuery publication handling. Remove the obsolete encryption configuration reloader, restore empty-configuration startup warnings in SchemaManager, and document ownership, correctness principles, trade-offs, and intrinsic limitations. Add unit, Rust database, and pgx SendBatch regressions for the review findings. Signed-off-by: James Sadler --- CHANGELOG.md | 2 +- .../cipherstash-proxy-integration/src/lib.rs | 1 + .../src/schema_change.rs | 25 +- packages/cipherstash-proxy/CONTEXT.md | 9 +- ...001-transaction-aware-schema-middleware.md | 27 +- packages/cipherstash-proxy/src/error.rs | 4 +- .../src/postgresql/backend.rs | 29 +- .../src/postgresql/context/mod.rs | 8 +- .../src/postgresql/frontend.rs | 71 ++- .../src/proxy/encrypt_config/config.rs | 95 ++++ .../src/proxy/encrypt_config/manager.rs | 280 ----------- .../src/proxy/encrypt_config/mod.rs | 5 +- .../src/proxy/schema/manager.rs | 11 + .../src/proxy/schema/middleware.rs | 441 +++++++++++++++--- .../cipherstash-proxy/src/proxy/schema/mod.rs | 2 +- tests/integration/golang/pgx_test.go | 40 +- 16 files changed, 654 insertions(+), 396 deletions(-) create mode 100644 packages/cipherstash-proxy/src/proxy/encrypt_config/config.rs delete mode 100644 packages/cipherstash-proxy/src/proxy/encrypt_config/manager.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index d45f42c94..f6c149bc2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### Security -- **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, pipelining, and already-open connections now observe the correct schema generation. Unmodelled DDL, dependent simple-query batches, and failed catalog publication fail closed instead of risking plaintext writes through stale metadata. +- **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/lib.rs b/packages/cipherstash-proxy-integration/src/lib.rs index 756ab8d23..70921320a 100644 --- a/packages/cipherstash-proxy-integration/src/lib.rs +++ b/packages/cipherstash-proxy-integration/src/lib.rs @@ -24,6 +24,7 @@ mod multitenant; mod ore_order_helpers; mod passthrough; mod pipeline; +/// Database-backed transaction-aware schema middleware regressions. mod schema_change; mod select; mod set_keyset_error; diff --git a/packages/cipherstash-proxy-integration/src/schema_change.rs b/packages/cipherstash-proxy-integration/src/schema_change.rs index 9f1340c27..a60b82863 100644 --- a/packages/cipherstash-proxy-integration/src/schema_change.rs +++ b/packages/cipherstash-proxy-integration/src/schema_change.rs @@ -1,4 +1,5 @@ #[cfg(test)] +/// End-to-end schema-change tests through Proxy and directly against PostgreSQL. mod tests { use crate::common::{connect, connect_with_tls, get_database_port, random_id, PROXY}; use tokio_postgres::Client; @@ -71,6 +72,28 @@ mod tests { assert_ciphertext_at_rest(&table, 1, "inside transaction").await; } + #[tokio::test] + async fn encryption_neutral_alter_table_keeps_transaction_mappable() { + let client = connect_for_test(*PROXY).await; + let table = table("bug_308_safe_alter"); + + client + .execute(&create_encrypted_table(&table), &[]) + .await + .unwrap(); + client.batch_execute("BEGIN").await.unwrap(); + client + .batch_execute(&format!( + "ALTER TABLE {table} ALTER COLUMN secret SET NOT NULL" + )) + .await + .unwrap(); + insert_secret(&client, &table, 1, "after safe alter").await; + client.batch_execute("COMMIT").await.unwrap(); + + assert_ciphertext_at_rest(&table, 1, "after safe alter").await; + } + #[tokio::test] async fn pipelined_statement_waits_for_extended_ddl_activation() { let client = connect_for_test(*PROXY).await; @@ -123,7 +146,7 @@ mod tests { .await .unwrap(); client - .batch_execute("SAVEPOINT before_reverted") + .batch_execute("SAVEPOINT Before_Reverted") .await .unwrap(); client diff --git a/packages/cipherstash-proxy/CONTEXT.md b/packages/cipherstash-proxy/CONTEXT.md index 70acc4dd3..c74c9dc58 100644 --- a/packages/cipherstash-proxy/CONTEXT.md +++ b/packages/cipherstash-proxy/CONTEXT.md @@ -132,10 +132,11 @@ closed without forwarding readiness, and the dirty publication remains eligible **Schema middleware**: The owner of transactional schema state. Frontend and Backend report protocol lifecycle events; -they do not directly change overlays, dirty flags, or reload managers. The middleware owns DDL -detection, prepared DDL effects, successful-execution activation, savepoint and transaction -transitions, effective-schema resolution, reload coordination, and schema publication. See -`docs/adr/0001-transaction-aware-schema-middleware.md`. +they do not directly change overlays or dirty flags. The middleware owns DDL detection, prepared +DDL effects, successful-execution activation, savepoint and transaction transitions, +effective-schema resolution, and the decision that publication is required. `Context` performs the +authoritative reload round trip, while `SchemaManager` coalesces reloads and orders their +generations. See `docs/adr/0001-transaction-aware-schema-middleware.md`. ## Note on `session` diff --git a/packages/cipherstash-proxy/docs/adr/0001-transaction-aware-schema-middleware.md b/packages/cipherstash-proxy/docs/adr/0001-transaction-aware-schema-middleware.md index 416cf960a..2f0c55fab 100644 --- a/packages/cipherstash-proxy/docs/adr/0001-transaction-aware-schema-middleware.md +++ b/packages/cipherstash-proxy/docs/adr/0001-transaction-aware-schema-middleware.md @@ -47,11 +47,16 @@ reload managers directly. After a DDL `Execute` is forwarded, protocol-control messages required to complete that execution continue to flow, but later schema-dependent operations wait until its success or failure is known. -This avoids both speculative mapping and a deadlock in the extended-protocol prepare flow. +Proxy injects `Flush` after the DDL `Execute`, allowing PostgreSQL to return `CommandComplete` +without waiting for a client `Sync`. The client's `Sync` remains the sole synchronization boundary, +so it still receives exactly one `ReadyForQuery`. This supports clients that pipeline DDL and a +dependent `Parse` in one batch without speculative mapping or a protocol deadlock. -A simple-query message containing DDL followed by a schema-dependent statement fails closed for -the initial implementation. Supporting that case requires preserving PostgreSQL's response -semantics while introducing an execution boundary and will be tracked separately. +A simple-query message fails closed when DDL may change encryption metadata and a later statement +requires that metadata for mapping. Native DDL and native temporary-table batches continue to pass +through because they introduce no encryption obligation. Proxy derives execution intents from the +statements it actually forwards, including when a mapping error uses the compatibility passthrough +fallback, so backend outcomes cannot become misaligned with phantom intents. ### Publication @@ -75,6 +80,14 @@ commit cannot be undone, but Proxy must not imply that stale encryption metadata - Every transaction maps against a stable schema and encryption-policy generation. - Frontend and Backend become protocol adapters around a testable schema state machine. - Extended-protocol pipelining requires bounded deferral after DDL execution. +- Native temporary tables are connection-local and absent from authoritative reloads. Proxy ignores + them only when they cannot shadow an encrypted table or introduce EQL columns; unsafe cases fail + closed for the rest of the connection (unless rollback to an earlier savepoint removes the + object), because a global catalog reload cannot prove connection-local state disappeared. +- The local overlay deliberately models only deterministic schema changes. Encryption-neutral + constraints, defaults, nullability, ownership, trigger/rule state, and row-level-security state + are accepted, while conditional, cascading, table-rewriting, view, and type-changing operations + remain unmodelled within a transaction. - Availability is intentionally sacrificed when committed schema state cannot be published safely. - Schema and encryption managers can no longer publish independent observable states. @@ -83,5 +96,7 @@ commit cannot be undone, but Proxy must not imply that stale encryption metadata State-machine tests cover successful execution, execution failure, explicit commit, full rollback, savepoint rollback, generation ordering, deferral, unmodelled DDL, and reload failure. Database-backed tests cover extended-protocol autocommit, explicit transactions, an already-open second connection, -pipelining, direct ciphertext verification, and failure before readiness. The existing simple-query -behaviour remains covered, with dependent post-DDL batches asserted to fail closed. +pipelining with a single client `Sync`, direct ciphertext verification, safe `ALTER TABLE`, native +temporary tables, compatibility fallback bookkeeping, and failure before readiness. Simple-query +coverage proves both that encryption-dependent post-DDL batches fail closed and native batches +continue to work. diff --git a/packages/cipherstash-proxy/src/error.rs b/packages/cipherstash-proxy/src/error.rs index 39f70f3c0..32370a2df 100644 --- a/packages/cipherstash-proxy/src/error.rs +++ b/packages/cipherstash-proxy/src/error.rs @@ -103,8 +103,8 @@ pub enum ZeroKMSError { #[derive(Error, Debug)] pub enum MappingError { - /// A simple-query batch would map against schema changed earlier in the batch. - #[error("A simple-query batch cannot contain a schema-dependent statement after DDL. Send the DDL and dependent statement as separate queries. For help visit {}#mapping-dependent-statement-after-ddl", ERROR_DOC_BASE_URL)] + /// A simple-query batch would map against encryption metadata changed earlier in the batch. + #[error("A simple-query batch cannot contain a schema-dependent statement after DDL that may change encryption metadata. Send the DDL and dependent statement as separate queries. For help visit {}#mapping-dependent-statement-after-ddl", ERROR_DOC_BASE_URL)] DependentStatementAfterDdl, /// Confirmed DDL cannot be represented safely by the transaction overlay. diff --git a/packages/cipherstash-proxy/src/postgresql/backend.rs b/packages/cipherstash-proxy/src/postgresql/backend.rs index df33fea51..c081d80cc 100644 --- a/packages/cipherstash-proxy/src/postgresql/backend.rs +++ b/packages/cipherstash-proxy/src/postgresql/backend.rs @@ -18,6 +18,7 @@ use crate::prometheus::{ DECRYPTION_ERROR_TOTAL, DECRYPTION_REQUESTS_TOTAL, ROWS_ENCRYPTED_TOTAL, ROWS_PASSTHROUGH_TOTAL, ROWS_TOTAL, SERVER_BYTES_RECEIVED_TOTAL, }; +use crate::proxy::schema::TransactionStatus; use crate::proxy::EncryptionService; use crate::EqlCiphertext; use bytes::BytesMut; @@ -185,12 +186,7 @@ where // client opening its next connection after ReadyForQuery observes // the newly loaded schema and encrypt configuration. if matches!(code.into(), BackendCode::ReadyForQuery) { - if bytes.last() == Some(&b'I') { - self.context.publish_schema_if_changed().await?; - } - if let Some(status) = bytes.last().copied() { - self.context.schema_ready_for_query(status); - } + self.handle_ready_for_query(&bytes).await?; } self.write_with_flush(bytes).await?; @@ -303,12 +299,7 @@ where client_id = self.context.client_id, msg = "ReadyForQuery" ); - if bytes.last() == Some(&b'I') { - self.context.publish_schema_if_changed().await?; - } - if let Some(status) = bytes.last().copied() { - self.context.schema_ready_for_query(status); - } + self.handle_ready_for_query(&bytes).await?; } code => { @@ -325,6 +316,20 @@ where Ok(()) } + /// Publishes committed DDL before exposing idle readiness, then updates + /// connection-local transaction state from the same authoritative boundary. + async fn handle_ready_for_query(&self, bytes: &BytesMut) -> Result<(), Error> { + let Some(status) = bytes.last().copied().map(TransactionStatus::from) else { + return Ok(()); + }; + + if status.is_idle() { + self.context.publish_schema_if_changed().await?; + } + self.context.schema_ready_for_query(status); + Ok(()) + } + /// Handles PostgreSQL ErrorResponse messages from the server. /// /// ErrorResponse messages indicate that an error occurred during SQL execution. diff --git a/packages/cipherstash-proxy/src/postgresql/context/mod.rs b/packages/cipherstash-proxy/src/postgresql/context/mod.rs index 52c4ea8b5..e3b22e514 100644 --- a/packages/cipherstash-proxy/src/postgresql/context/mod.rs +++ b/packages/cipherstash-proxy/src/postgresql/context/mod.rs @@ -587,9 +587,9 @@ where self.schema_middleware.bind(portal, prepared_statement); } - /// Records a portal execution awaiting its backend result. - pub fn execute_schema_portal(&self, portal: &Name) { - self.schema_middleware.execute(portal); + /// Records a portal execution and returns whether its DDL needs an injected flush. + pub fn execute_schema_portal(&self, portal: &Name) -> bool { + self.schema_middleware.execute(portal) } /// Records statements in one simple-query protocol message. @@ -655,7 +655,7 @@ where } /// Reports a readiness boundary and PostgreSQL transaction status. - pub fn schema_ready_for_query(&self, status: u8) { + pub fn schema_ready_for_query(&self, status: crate::proxy::schema::TransactionStatus) { self.schema_middleware.ready_for_query(status); } diff --git a/packages/cipherstash-proxy/src/postgresql/frontend.rs b/packages/cipherstash-proxy/src/postgresql/frontend.rs index 87b61b093..73345639c 100644 --- a/packages/cipherstash-proxy/src/postgresql/frontend.rs +++ b/packages/cipherstash-proxy/src/postgresql/frontend.rs @@ -205,6 +205,7 @@ where } } + let mut flush_after_write = false; match code { Code::Query => { match self.query_handler(&bytes).await { @@ -239,7 +240,7 @@ where self.describe_handler(&bytes).await?; } Code::Execute => { - self.execute_handler(&bytes).await?; + flush_after_write = self.execute_handler(&bytes).await?; } Code::Parse => { match self.parse_handler(&bytes).await { @@ -340,6 +341,9 @@ where } self.write_to_server(bytes).await?; + if flush_after_write { + self.write_to_server(postgresql_flush_message()).await?; + } Ok(()) } @@ -382,13 +386,13 @@ where Ok(()) } - async fn execute_handler(&mut self, bytes: &BytesMut) -> Result<(), Error> { + async fn execute_handler(&mut self, bytes: &BytesMut) -> Result { let execute = Execute::try_from(bytes)?; debug!(target: PROTOCOL, client_id = self.context.client_id, ?execute); - self.context.execute_schema_portal(&execute.portal); + let executes_ddl = self.context.execute_schema_portal(&execute.portal); self.context .set_execute_for_portal(execute.portal.to_owned()); - Ok(()) + Ok(executes_ddl) } /// Handles PostgreSQL Query messages (simple query protocol). @@ -452,7 +456,7 @@ where self.context.wait_for_schema_execution().await; self.context.ensure_schema_modelled()?; } - let mut transformed_statements = vec![]; + let mut forwarded_statements = vec![]; debug!(target: MAPPER, client_id = self.context.client_id, @@ -476,6 +480,7 @@ where warn!(msg = "Encrypted statement mapping is not enabled"); counter!(STATEMENTS_PASSTHROUGH_MAPPING_DISABLED_TOTAL).increment(1); counter!(STATEMENTS_PASSTHROUGH_TOTAL).increment(1); + forwarded_statements.push(statement.clone()); continue; } @@ -483,6 +488,7 @@ where if !eql_mapper::requires_type_check(statement) { counter!(STATEMENTS_PASSTHROUGH_TOTAL).increment(1); + forwarded_statements.push(statement.clone()); continue; } @@ -497,6 +503,11 @@ where if self.context.mapping_errors_enabled() || err.must_fail_closed() { return Err(err); } else { + self.record_simple_schema_execution( + session_id, + Portal::passthrough(Some(session_id)), + &parsed_statements, + ); return Ok(None); }; } @@ -509,6 +520,7 @@ where msg = "Encryptable Statement", ); + let mut transformed = false; if typed_statement.requires_transform() { // Record parse duration before encryption work starts if !parse_duration_recorded { @@ -536,11 +548,16 @@ where // The simple protocol has no params, so the plan is // always empty here — only the SQL is needed. - transformed_statements.push(transformed_statement.statement); + forwarded_statements.push(transformed_statement.statement); encrypted = true; + transformed = true; } } + if !transformed { + forwarded_statements.push(typed_statement.statement.clone()); + } + counter!(STATEMENTS_ENCRYPTED_TOTAL).increment(1); // Set Encrypted portal and mark as mapped @@ -555,7 +572,7 @@ where msg = "Passthrough Statement" ); counter!(STATEMENTS_PASSTHROUGH_TOTAL).increment(1); - transformed_statements.push(statement.clone()); + forwarded_statements.push(statement.clone()); } }; } @@ -585,14 +602,10 @@ where m.set_query_fingerprint(&query.statement); }); - self.context.add_portal(Name::unnamed(), portal); - self.context.set_execute(Name::unnamed(), Some(session_id)); - - self.context - .execute_simple_schema_statements(&parsed_statements); + self.record_simple_schema_execution(session_id, portal, &forwarded_statements); if encrypted { - let transformed_statement = transformed_statements + let transformed_statement = forwarded_statements .iter() .map(|s| s.to_string()) .collect::>() @@ -631,6 +644,18 @@ where } } + /// Records the portal and schema intents for the statements actually sent to PostgreSQL. + fn record_simple_schema_execution( + &mut self, + session_id: SessionId, + portal: Portal, + statements: &[ast::Statement], + ) { + self.context.add_portal(Name::unnamed(), portal); + self.context.set_execute(Name::unnamed(), Some(session_id)); + self.context.execute_simple_schema_statements(statements); + } + /// Encrypts literal values found in SQL statements. /// /// Takes literal values extracted from SQL statements and encrypts those that @@ -1541,6 +1566,15 @@ where Ok(serde_json::to_string(literal).map(Value::SingleQuotedString)?) } +/// Builds a PostgreSQL `Flush` message. +/// +/// Proxy injects this after a DDL `Execute` so PostgreSQL sends its execution +/// result without waiting for the client's later `Sync`. The client still owns +/// the synchronization boundary and receives exactly one `ReadyForQuery`. +fn postgresql_flush_message() -> BytesMut { + BytesMut::from(&b"H\0\0\0\x04"[..]) +} + /// Implementation of PostgreSQL error handling for the Frontend component. impl PostgreSqlErrorHandler for Frontend where @@ -1579,3 +1613,14 @@ where Ok(()) } } + +#[cfg(test)] +/// Wire-shape regression tests for messages injected by the frontend. +mod tests { + use super::postgresql_flush_message; + + #[test] + fn injected_flush_has_the_postgresql_wire_shape() { + assert_eq!(&postgresql_flush_message()[..], b"H\0\0\0\x04"); + } +} diff --git a/packages/cipherstash-proxy/src/proxy/encrypt_config/config.rs b/packages/cipherstash-proxy/src/proxy/encrypt_config/config.rs new file mode 100644 index 000000000..5f3dbe6df --- /dev/null +++ b/packages/cipherstash-proxy/src/proxy/encrypt_config/config.rs @@ -0,0 +1,95 @@ +use cipherstash_client::eql; +use cipherstash_client::schema::ColumnConfig; +use std::collections::HashMap; + +/// +/// Column configuration keyed by table name and column name +/// - key: `{table_name}.{column_name}` +/// +type EncryptConfigMap = HashMap; + +#[derive(Clone, Debug, PartialEq)] +/// Encryption policies indexed by their resolved table and column names. +pub struct EncryptConfig { + config: EncryptConfigMap, +} + +impl EncryptConfig { + /// Constructs encryption metadata from an already indexed configuration map. + pub fn new_from_config(config: EncryptConfigMap) -> Self { + Self { config } + } + + /// Constructs an empty encryption configuration. + pub fn new() -> Self { + Self { + config: HashMap::new(), + } + } + + /// Returns whether the snapshot contains no encrypted columns. + pub fn is_empty(&self) -> bool { + self.config.is_empty() + } + + /// Returns the encryption policy for one resolved column. + pub fn get_column_config(&self, identifier: &eql::Identifier) -> Option { + self.config.get(identifier).cloned() + } + + /// Returns whether any encrypted column belongs to `table`. + pub(crate) fn contains_table(&self, table: &str) -> bool { + self.config + .keys() + .any(|identifier| identifier.table == table) + } + + /// Inserts or replaces encryption metadata for one column. + pub(crate) fn insert(&mut self, identifier: eql::Identifier, config: ColumnConfig) { + self.config.insert(identifier, config); + } + + /// Removes encryption metadata for one column. + pub(crate) fn remove_column(&mut self, table: &str, column: &str) { + self.config + .remove(&eql::Identifier::new(table.to_owned(), column.to_owned())); + } + + /// Removes all encryption metadata for a table. + pub(crate) fn remove_table(&mut self, table: &str) { + self.config + .retain(|identifier, _| identifier.table != table); + } + + /// Moves encryption metadata to a renamed column identifier. + pub(crate) fn rename_column(&mut self, table: &str, from: &str, to: &str) { + let from = eql::Identifier::new(table.to_owned(), from.to_owned()); + if let Some(config) = self.config.remove(&from) { + self.config.insert( + eql::Identifier::new(table.to_owned(), to.to_owned()), + config, + ); + } + } + + /// Moves all encryption metadata to a renamed table identifier. + pub(crate) fn rename_table(&mut self, from: &str, to: &str) { + let renamed = self + .config + .iter() + .filter(|(identifier, _)| identifier.table == from) + .map(|(identifier, config)| (identifier.column.clone(), config.clone())) + .collect::>(); + self.remove_table(from); + for (column, config) in renamed { + self.config + .insert(eql::Identifier::new(to.to_owned(), column), config); + } + } +} + +impl Default for EncryptConfig { + fn default() -> Self { + Self::new() + } +} diff --git a/packages/cipherstash-proxy/src/proxy/encrypt_config/manager.rs b/packages/cipherstash-proxy/src/proxy/encrypt_config/manager.rs deleted file mode 100644 index 47486f6d1..000000000 --- a/packages/cipherstash-proxy/src/proxy/encrypt_config/manager.rs +++ /dev/null @@ -1,280 +0,0 @@ -use super::from_domain::column_config_from_domain; -use crate::{ - config::DatabaseConfig, connect, error::Error, log::ENCRYPT_CONFIG, proxy::SCHEMA_QUERY, -}; -use arc_swap::ArcSwap; -use cipherstash_client::eql; -use cipherstash_client::schema::ColumnConfig; -use std::{collections::HashMap, sync::Arc, time::Duration}; -use tokio::{task::JoinHandle, time}; -use tracing::{debug, error, info, warn}; - -/// -/// Column configuration keyed by table name and column name -/// - key: `{table_name}.{column_name}` -/// -type EncryptConfigMap = HashMap; - -#[derive(Clone, Debug, PartialEq)] -pub struct EncryptConfig { - config: EncryptConfigMap, -} - -impl EncryptConfig { - pub fn new_from_config(config: EncryptConfigMap) -> Self { - Self { config } - } - - pub fn new() -> Self { - Self { - config: HashMap::new(), - } - } - - pub fn is_empty(&self) -> bool { - self.config.is_empty() - } - - pub fn get_column_config(&self, identifier: &eql::Identifier) -> Option { - self.config.get(identifier).cloned() - } - - /// Returns whether any encrypted column belongs to `table`. - pub(crate) fn contains_table(&self, table: &str) -> bool { - self.config - .keys() - .any(|identifier| identifier.table == table) - } - - /// Inserts or replaces encryption metadata for one column. - pub(crate) fn insert(&mut self, identifier: eql::Identifier, config: ColumnConfig) { - self.config.insert(identifier, config); - } - - /// Removes encryption metadata for one column. - pub(crate) fn remove_column(&mut self, table: &str, column: &str) { - self.config - .remove(&eql::Identifier::new(table.to_owned(), column.to_owned())); - } - - /// Removes all encryption metadata for a table. - pub(crate) fn remove_table(&mut self, table: &str) { - self.config - .retain(|identifier, _| identifier.table != table); - } - - /// Moves encryption metadata to a renamed column identifier. - pub(crate) fn rename_column(&mut self, table: &str, from: &str, to: &str) { - let from = eql::Identifier::new(table.to_owned(), from.to_owned()); - if let Some(config) = self.config.remove(&from) { - self.config.insert( - eql::Identifier::new(table.to_owned(), to.to_owned()), - config, - ); - } - } - - /// Moves all encryption metadata to a renamed table identifier. - pub(crate) fn rename_table(&mut self, from: &str, to: &str) { - let renamed = self - .config - .iter() - .filter(|(identifier, _)| identifier.table == from) - .map(|(identifier, config)| (identifier.column.clone(), config.clone())) - .collect::>(); - self.remove_table(from); - for (column, config) in renamed { - self.config - .insert(eql::Identifier::new(to.to_owned(), column), config); - } - } -} - -impl Default for EncryptConfig { - fn default() -> Self { - Self::new() - } -} - -#[derive(Clone, Debug)] -pub struct EncryptConfigManager { - config: DatabaseConfig, - encrypt_config: Arc>, - _reload_handle: Arc>, -} - -impl EncryptConfigManager { - pub async fn init(config: &DatabaseConfig) -> Result { - let config = config.clone(); - init_reloader(config).await - } - - pub fn load(&self) -> Arc { - self.encrypt_config.load().clone() - } - - pub fn is_empty(&self) -> bool { - self.encrypt_config.load().is_empty() - } - - pub async fn reload(&self) -> bool { - match load_encrypt_config_with_retry(&self.config).await { - Ok(reloaded) => { - debug!(target: ENCRYPT_CONFIG, msg = "Reloaded encrypt configuration"); - self.encrypt_config.swap(Arc::new(reloaded)); - true - } - Err(err) => { - warn!( - msg = "Error reloading encrypt configuration", - error = err.to_string() - ); - false - } - } - } -} - -async fn init_reloader(config: DatabaseConfig) -> Result { - // Skip retries on startup as the likely failure mode is configuration - // Only warn on startup, otherwise warning on every reload - let encrypt_config = match load_encrypt_config(&config).await { - Ok(encrypt_config) => encrypt_config, - Err(err) => { - // Encrypt config is inferred from the schema (EQL v3 self-configuring - // domains), so a load error here is a database/connection failure, not - // a missing config table. A schema with no encrypted columns is a - // successful (empty) load, warned about below. - error!( - msg = "Error loading Encrypt configuration", - error = err.to_string() - ); - return Err(err); - } - }; - - debug!(target: ENCRYPT_CONFIG, ?encrypt_config); - - if encrypt_config.is_empty() { - warn!(msg = "ENCRYPT CONFIGURATION NOT LOADED"); - warn!(msg = "No active Encrypt configuration found in database."); - warn!(msg = "Data is not protected with encryption"); - } else { - info!(msg = "Loaded Encrypt configuration"); - } - - let encrypt_config = Arc::new(ArcSwap::new(Arc::new(encrypt_config))); - - let config_ref = config.clone(); - - let dataset_ref = encrypt_config.clone(); - let reload_handle = tokio::spawn(async move { - let reload_interval = tokio::time::Duration::from_secs(config_ref.config_reload_interval); - - let mut interval = tokio::time::interval_at( - tokio::time::Instant::now() + reload_interval, - reload_interval, - ); - - loop { - interval.tick().await; - - match load_encrypt_config_with_retry(&config_ref).await { - Ok(reloaded) => { - debug!(target: ENCRYPT_CONFIG, msg = "Reloaded Encrypt configuration"); - dataset_ref.swap(Arc::new(reloaded)); - } - Err(err) => { - warn!( - msg = "Error reloading Encrypt configuration", - error = err.to_string() - ); - } - } - } - }); - - Ok(EncryptConfigManager { - config, - encrypt_config, - _reload_handle: Arc::new(reload_handle), - }) -} - -/// Fetch the dataset and retry on any error -/// -/// When databases and the proxy start up at the same time they might not be ready to accept connections before the -/// proxy tries to query the schema. To give the proxy the best chance of initialising correctly this method will -/// retry the query a few times before passing on the error. -async fn load_encrypt_config_with_retry(config: &DatabaseConfig) -> Result { - let mut retry_count = 0; - let max_retry_count = 10; - let max_backoff = Duration::from_secs(2); - - loop { - match load_encrypt_config(config).await { - Ok(encrypt_config) => { - return Ok(encrypt_config); - } - - Err(err) => { - if retry_count >= max_retry_count { - debug!( - ENCRYPT_CONFIG, - msg = "Encrypt configuration could not beloaded", - retries = retry_count, - error = err.to_string() - ); - return Err(err); - } - } - } - - let sleep_duration_ms = (100 * 2_u64.pow(retry_count)).min(max_backoff.as_millis() as _); - - time::sleep(Duration::from_millis(sleep_duration_ms)).await; - - retry_count += 1; - } -} - -/// Loads the encrypt configuration by inferring it from the database schema. -/// -/// EQL v3 columns are self-configuring domain types, so each encrypted column's -/// `ColumnConfig` is derived from its Postgres domain typname (see -/// [`column_config_from_domain`]). There is no `eql_v2_configuration` table or -/// `add_search_config` in v3 — the schema is the single source of truth. -pub async fn load_encrypt_config(config: &DatabaseConfig) -> Result { - let client = connect::database(config).await?; - - let tables = client.query(SCHEMA_QUERY, &[]).await?; - - let mut map = EncryptConfigMap::new(); - - for table in tables { - let table_name: String = table.get("table_name"); - let columns: Vec = table.get("columns"); - let column_domain_names: Vec> = table.get("column_domain_names"); - - for (column, domain) in columns.iter().zip(column_domain_names) { - let Some(domain) = domain else { continue }; - if let Some(column_config) = column_config_from_domain(&table_name, column, &domain) { - debug!( - target: ENCRYPT_CONFIG, - msg = "Encrypted column", - table = table_name, - column = column, - domain = domain - ); - // First wins. The query returns the search path in precedence - // order, so if a table name does appear in more than one - // searched schema this keeps the one PostgreSQL itself would - // resolve to, rather than whichever happened to come last. - map.entry(eql::Identifier::new(table_name.clone(), column.clone())) - .or_insert(column_config); - } - } - } - - Ok(EncryptConfig::new_from_config(map)) -} diff --git a/packages/cipherstash-proxy/src/proxy/encrypt_config/mod.rs b/packages/cipherstash-proxy/src/proxy/encrypt_config/mod.rs index 673fb7548..be7203b1d 100644 --- a/packages/cipherstash-proxy/src/proxy/encrypt_config/mod.rs +++ b/packages/cipherstash-proxy/src/proxy/encrypt_config/mod.rs @@ -1,5 +1,6 @@ +/// Immutable encryption-policy snapshots. +mod config; /// Derives encryption metadata from EQL domain declarations. pub(crate) mod from_domain; -mod manager; -pub use manager::EncryptConfig; +pub use config::EncryptConfig; diff --git a/packages/cipherstash-proxy/src/proxy/schema/manager.rs b/packages/cipherstash-proxy/src/proxy/schema/manager.rs index 75c702094..e6bd39f8c 100644 --- a/packages/cipherstash-proxy/src/proxy/schema/manager.rs +++ b/packages/cipherstash-proxy/src/proxy/schema/manager.rs @@ -1,3 +1,5 @@ +#![deny(missing_docs)] + use super::eql_domains; use crate::config::DatabaseConfig; use crate::error::Error; @@ -118,6 +120,7 @@ impl CommittedSchemaSnapshot { } #[derive(Clone, Debug)] +/// Loads, versions, and atomically publishes authoritative committed snapshots. pub struct SchemaManager { config: DatabaseConfig, snapshot: Arc>, @@ -129,6 +132,7 @@ pub struct SchemaManager { } impl SchemaManager { + /// Loads the initial snapshot and starts periodic authoritative refreshes. pub async fn init(config: &DatabaseConfig) -> Result { let config = config.clone(); init_reloader(config).await @@ -148,6 +152,7 @@ impl SchemaManager { } } + /// Requests a coalesced authoritative reload and reports whether it published safely. pub async fn reload(&self) -> bool { coalesced_reload( self.snapshot.clone(), @@ -225,6 +230,11 @@ fn publish_if_newer( async fn init_reloader(config: DatabaseConfig) -> Result { // Skip retries on startup as the likely failure mode is configuration let (schema, encrypt_config) = load_snapshot(&config).await?; + if encrypt_config.is_empty() { + warn!(msg = "ENCRYPT CONFIGURATION NOT LOADED"); + warn!(msg = "No active Encrypt configuration found in database."); + warn!(msg = "Data is not protected with encryption"); + } info!(msg = "Loaded committed schema snapshot"); let snapshot = Arc::new(ArcSwap::new(Arc::new(CommittedSchemaSnapshot::new( @@ -369,6 +379,7 @@ fn classify_column( Column::native(ident) } +/// Loads the current structural schema without exposing encryption metadata. pub async fn load_schema(config: &DatabaseConfig) -> Result { load_snapshot(config).await.map(|(schema, _)| schema) } diff --git a/packages/cipherstash-proxy/src/proxy/schema/middleware.rs b/packages/cipherstash-proxy/src/proxy/schema/middleware.rs index 4680bb925..7f824fb4a 100644 --- a/packages/cipherstash-proxy/src/proxy/schema/middleware.rs +++ b/packages/cipherstash-proxy/src/proxy/schema/middleware.rs @@ -44,6 +44,9 @@ //! * Temporary and other connection-local catalog objects are invisible to the separate //! publication connection. Native temporary-table batches may pass through, but encrypted //! temporary objects cannot be represented or authoritatively published and are unsupported. +//! A temporary table that could shadow encrypted metadata leaves the connection fail-closed +//! until an earlier savepoint removes it or the connection ends; an authoritative global reload +//! cannot prove that a connection-local object disappeared. //! * SQL executed indirectly by procedures, extensions, or dynamic SQL cannot be inferred from //! the wire statement. Periodic authoritative reloads eventually discover committed global //! changes; a transaction still keeps its pinned view for consistency. @@ -55,6 +58,8 @@ //! compatibility for the core invariant: Proxy must never forward plaintext because it //! speculated about uncommitted or incompletely modelled schema state. +#![deny(missing_docs)] + use super::eql_domains; use super::manager::CommittedSchemaStore; use crate::postgresql::Name; @@ -63,8 +68,8 @@ use crate::proxy::EncryptConfig; use cipherstash_client::eql::Identifier; use eql_mapper::{ColumnKind, Schema, SchemaWithEdits, TableResolver}; use sqltk::parser::ast::{ - AlterTableOperation, ColumnDef, DropBehavior, Ident, ObjectName, ObjectNamePart, ObjectType, - Statement, + AlterColumnOperation, AlterTableOperation, ColumnDef, DataType, DropBehavior, Ident, + ObjectName, ObjectNamePart, ObjectType, Statement, }; use std::collections::{HashMap, VecDeque}; use std::sync::{ @@ -97,6 +102,38 @@ struct Savepoint { schema: SchemaWithEdits, encrypt_config: EncryptConfig, unmodelled: bool, + local_unmodelled: bool, +} + +/// PostgreSQL's transaction state carried by a `ReadyForQuery` message. +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub enum TransactionStatus { + /// The connection is outside a transaction block. + Idle, + /// The connection is inside a transaction block that can accept commands. + InTransaction, + /// The connection is inside a failed transaction block awaiting rollback. + FailedTransaction, + /// An unrecognized protocol status, treated conservatively as non-idle. + Unknown(u8), +} + +impl TransactionStatus { + /// Returns whether PostgreSQL is outside a transaction block. + pub fn is_idle(self) -> bool { + self == Self::Idle + } +} + +impl From for TransactionStatus { + fn from(status: u8) -> Self { + match status { + b'I' => Self::Idle, + b'T' => Self::InTransaction, + b'E' => Self::FailedTransaction, + status => Self::Unknown(status), + } + } } /// Connection-local owner of the effective database schema. @@ -117,6 +154,7 @@ pub struct SchemaMiddleware { execution_finished: Arc, dirty: Arc, unmodelled: Arc, + local_unmodelled: Arc, transaction_active: Arc, savepoints: Arc>>, } @@ -148,6 +186,7 @@ impl SchemaMiddleware { execution_finished: Arc::new(Notify::new()), dirty: Arc::new(AtomicBool::new(false)), unmodelled: Arc::new(AtomicBool::new(false)), + local_unmodelled: Arc::new(AtomicBool::new(false)), transaction_active: Arc::new(AtomicBool::new(false)), savepoints: Arc::new(RwLock::new(Vec::new())), } @@ -206,7 +245,7 @@ impl SchemaMiddleware { /// Returns whether confirmed DDL cannot be represented by the local overlay. pub fn has_unmodelled_ddl(&self) -> bool { - self.unmodelled.load(Ordering::Acquire) + self.unmodelled.load(Ordering::Acquire) || self.local_unmodelled.load(Ordering::Acquire) } /// Adopts a newer committed snapshot before an idle connection starts work. @@ -221,11 +260,11 @@ impl SchemaMiddleware { } /// Applies a PostgreSQL readiness boundary and its transaction status. - pub fn ready_for_query(&self, status: u8) { + pub fn ready_for_query(&self, status: TransactionStatus) { self.discard_skipped_executions(); self.transaction_active - .store(status != b'I', Ordering::Release); - if status == b'I' && !self.needs_publication() { + .store(!status.is_idle(), Ordering::Release); + if status.is_idle() && !self.needs_publication() { self.adopt_latest(); } } @@ -235,7 +274,7 @@ impl SchemaMiddleware { self.prepared.write().unwrap().insert( name, Intent { - ddl: is_schema_ddl(&statement), + ddl: self.is_schema_ddl(&statement), modelled: is_modelled_ddl(&statement), statement, }, @@ -261,17 +300,20 @@ impl SchemaMiddleware { } } - /// Records a portal execution awaiting a backend success or failure response. - pub fn execute(&self, portal: &Name) { + /// Records a portal execution awaiting a backend result and returns whether + /// the protocol adapter must flush DDL immediately to unblock dependent work. + pub fn execute(&self, portal: &Name) -> bool { let intent = self.portals.read().unwrap().get(portal).cloned(); + let executes_ddl = intent.as_ref().is_some_and(|intent| intent.ddl); self.transaction_active.store(true, Ordering::Release); - if intent.as_ref().is_some_and(|intent| intent.ddl) { + if executes_ddl { self.in_flight_ddl.fetch_add(1, Ordering::AcqRel); } self.executions .write() .unwrap() .push_back(PendingExecution::Execute(intent.map(Box::new))); + executes_ddl } /// Records every statement and readiness boundary in a simple-query message. @@ -280,7 +322,7 @@ impl SchemaMiddleware { let mut executions = self.executions.write().unwrap(); for statement in statements { let intent = Intent { - ddl: is_schema_ddl(statement), + ddl: self.is_schema_ddl(statement), modelled: is_modelled_ddl(statement), statement: statement.clone(), }; @@ -311,7 +353,9 @@ impl SchemaMiddleware { fn ddl_may_change_encryption(&self, statement: &Statement) -> bool { match statement { Statement::CreateTable(create) => { - create.query.is_some() + self.encrypt_config() + .contains_table(&postgres_object_name(&create.name)) + || create.query.is_some() || create.like.is_some() || create.clone.is_some() || create.inherits.is_some() @@ -323,24 +367,37 @@ impl SchemaMiddleware { Statement::AlterTable { name, operations, .. } => { - self.encrypt_config().contains_table(object_name(name)) - || operations.iter().any(|operation| match operation { - AlterTableOperation::AddColumn { column_def, .. } => { - matches!(column_kind(column_def), ColumnKind::Eql(_, _)) - } - AlterTableOperation::DropColumn { .. } - | AlterTableOperation::RenameColumn { .. } - | AlterTableOperation::RenameTable { .. } => false, - _ => true, - }) + let table = postgres_object_name(name); + let encrypt_config = self.encrypt_config(); + operations.iter().any(|operation| match operation { + AlterTableOperation::AddColumn { column_def, .. } => { + matches!(column_kind(column_def), ColumnKind::Eql(_, _)) + } + operation if is_encryption_neutral_alter_operation(operation) => false, + AlterTableOperation::DropColumn { column_name, .. } + | AlterTableOperation::RenameColumn { + old_column_name: column_name, + .. + } => encrypt_config + .get_column_config(&Identifier::new( + table.clone(), + postgres_identifier(column_name), + )) + .is_some(), + AlterTableOperation::RenameTable { .. } => { + encrypt_config.contains_table(&table) + } + _ => true, + }) } Statement::Drop { object_type: ObjectType::Table, names, .. - } => names - .iter() - .any(|name| self.encrypt_config().contains_table(object_name(name))), + } => names.iter().any(|name| { + self.encrypt_config() + .contains_table(&postgres_object_name(name)) + }), Statement::CreateView { .. } | Statement::Drop { object_type: ObjectType::View, @@ -350,6 +407,21 @@ impl SchemaMiddleware { } } + /// Returns whether a statement changes tracked schema state for this connection. + /// + /// Native temporary tables are connection-local and cannot be published from the + /// manager's catalog connection, so they are ignored unless they could shadow an + /// encrypted table or introduce an encryption domain. Those unsafe cases remain DDL + /// and therefore fail closed as unmodelled changes after successful execution. + fn is_schema_ddl(&self, statement: &Statement) -> bool { + match statement { + Statement::CreateTable(create) if create.temporary => { + self.ddl_may_change_encryption(statement) + } + _ => is_catalog_schema_ddl(statement), + } + } + /// Marks the `Sync` boundary whose `ReadyForQuery` terminates an extended /// protocol batch. PostgreSQL skips the remaining executions in that batch /// after an error, but may already have a later batch queued behind it. @@ -374,7 +446,7 @@ impl SchemaMiddleware { #[cfg(test)] /// Records a direct statement execution for state-machine tests. pub fn execution_started(&self, statement: Statement) { - let ddl = is_schema_ddl(&statement); + let ddl = self.is_schema_ddl(&statement); if ddl { self.in_flight_ddl.fetch_add(1, Ordering::AcqRel); } @@ -401,16 +473,19 @@ impl SchemaMiddleware { name, schema: checkpoint, encrypt_config: (*self.encrypt_config()).clone(), - unmodelled: self.has_unmodelled_ddl(), + unmodelled: self.unmodelled.load(Ordering::Acquire), + local_unmodelled: self.local_unmodelled.load(Ordering::Acquire), }); } Statement::ReleaseSavepoint { name } => { let mut savepoints = self.savepoints.write().unwrap(); if let Some(index) = savepoints .iter() - .rposition(|savepoint| savepoint.name == name) + .rposition(|savepoint| identifiers_equal(&savepoint.name, &name)) { savepoints.truncate(index); + } else { + self.mark_unmodelled(false); } } Statement::Rollback { @@ -420,21 +495,26 @@ impl SchemaMiddleware { let mut savepoints = self.savepoints.write().unwrap(); if let Some(index) = savepoints .iter() - .rposition(|savepoint| savepoint.name == name) + .rposition(|savepoint| identifiers_equal(&savepoint.name, &name)) { let checkpoint = savepoints[index].schema.clone(); let encrypt_config = savepoints[index].encrypt_config.clone(); let unmodelled = savepoints[index].unmodelled; + let local_unmodelled = savepoints[index].local_unmodelled; savepoints.truncate(index + 1); let resolver = self.resolver(); let overlay = resolver.as_schema_with_edits().unwrap(); *overlay.write().unwrap() = checkpoint; *self.encrypt_config.write().unwrap() = Arc::new(encrypt_config); self.unmodelled.store(unmodelled, Ordering::Release); + self.local_unmodelled + .store(local_unmodelled, Ordering::Release); self.dirty.store( unmodelled || resolver.has_schema_changed(), Ordering::Release, ); + } else { + self.mark_unmodelled(false); } } Statement::Rollback { @@ -449,17 +529,22 @@ impl SchemaMiddleware { self.dirty.store(false, Ordering::Release); self.unmodelled.store(false, Ordering::Release); } - statement => { - if intent.ddl && !intent.modelled { - self.unmodelled.store(true, Ordering::Release); + statement if intent.ddl => { + if !intent.modelled { + let local_only = matches!( + statement, + Statement::CreateTable(ref create) if create.temporary + ); + self.mark_unmodelled(local_only); } else { self.apply_ddl(&statement); } } + _ => {} } if intent.ddl { self.dirty.store( - self.has_unmodelled_ddl() || self.resolver().has_schema_changed(), + self.unmodelled.load(Ordering::Acquire) || self.resolver().has_schema_changed(), Ordering::Release, ); self.in_flight_ddl.fetch_sub(1, Ordering::AcqRel); @@ -521,18 +606,24 @@ impl SchemaMiddleware { self.execution_finished.notify_waiters(); } } + + /// Prevents subsequent mapping when local state cannot be reconstructed exactly. + fn mark_unmodelled(&self, connection_local: bool) { + if connection_local { + self.local_unmodelled.store(true, Ordering::Release); + } else { + self.unmodelled.store(true, Ordering::Release); + self.dirty.store(true, Ordering::Release); + } + } } /// Returns the unqualified PostgreSQL domain name declared for a column. fn column_domain(column: &ColumnDef) -> String { - column - .data_type - .to_string() - .split('.') - .next_back() - .unwrap_or_default() - .trim_matches('"') - .to_owned() + match &column.data_type { + DataType::Custom(name, _) => postgres_object_name(name), + data_type => data_type.to_string().to_ascii_lowercase(), + } } /// Classifies a declared column as native or as an EQL domain. @@ -542,20 +633,35 @@ fn column_kind(column: &ColumnDef) -> ColumnKind { .unwrap_or(ColumnKind::Native) } -/// Returns the final identifier component of a PostgreSQL object name. -fn object_name(name: &ObjectName) -> &str { +/// Returns an identifier exactly as PostgreSQL stores it in the catalog. +fn postgres_identifier(identifier: &Ident) -> String { + if identifier.quote_style.is_some() { + identifier.value.clone() + } else { + identifier.value.to_ascii_lowercase() + } +} + +/// Returns the catalog spelling of the final component of an object name. +fn postgres_object_name(name: &ObjectName) -> String { match name.0.last() { - Some(ObjectNamePart::Identifier(name)) => &name.value, - _ => "", + Some(ObjectNamePart::Identifier(name)) => postgres_identifier(name), + _ => String::new(), } } +/// Compares identifiers according to PostgreSQL's quoted-identifier rules. +fn identifiers_equal(left: &Ident, right: &Ident) -> bool { + postgres_identifier(left) == postgres_identifier(right) +} + /// Adds inferred encryption metadata for one newly declared column. fn add_column_config(config: &mut EncryptConfig, table: &str, column: &ColumnDef) { let domain = column_domain(column); - if let Some(column_config) = column_config_from_domain(table, &column.name.value, &domain) { + let column_name = postgres_identifier(&column.name); + if let Some(column_config) = column_config_from_domain(table, &column_name, &domain) { config.insert( - Identifier::new(table.to_owned(), column.name.value.clone()), + Identifier::new(table.to_owned(), column_name), column_config, ); } @@ -565,32 +671,36 @@ fn add_column_config(config: &mut EncryptConfig, table: &str, column: &ColumnDef fn apply_encrypt_config(config: &mut EncryptConfig, statement: &Statement) { match statement { Statement::CreateTable(create) => { - let table = object_name(&create.name); - config.remove_table(table); + let table = postgres_object_name(&create.name); + config.remove_table(&table); for column in &create.columns { - add_column_config(config, table, column); + add_column_config(config, &table, column); } } Statement::AlterTable { name, operations, .. } => { - let table = object_name(name); + let table = postgres_object_name(name); for operation in operations { match operation { AlterTableOperation::AddColumn { column_def, .. } => { - add_column_config(config, table, column_def); + add_column_config(config, &table, column_def); } AlterTableOperation::DropColumn { column_name, .. } => { - config.remove_column(table, &column_name.value); + config.remove_column(&table, &postgres_identifier(column_name)); } AlterTableOperation::RenameColumn { old_column_name, new_column_name, } => { - config.rename_column(table, &old_column_name.value, &new_column_name.value); + config.rename_column( + &table, + &postgres_identifier(old_column_name), + &postgres_identifier(new_column_name), + ); } AlterTableOperation::RenameTable { table_name } => { - config.rename_table(table, object_name(table_name)); + config.rename_table(&table, &postgres_object_name(table_name)); } _ => {} } @@ -602,7 +712,7 @@ fn apply_encrypt_config(config: &mut EncryptConfig, statement: &Statement) { .. } => { for name in names { - config.remove_table(object_name(name)); + config.remove_table(&postgres_object_name(name)); } } _ => {} @@ -610,7 +720,7 @@ fn apply_encrypt_config(config: &mut EncryptConfig, statement: &Statement) { } /// Returns whether a statement changes schema state tracked by the middleware. -fn is_schema_ddl(statement: &Statement) -> bool { +fn is_catalog_schema_ddl(statement: &Statement) -> bool { matches!( statement, Statement::CreateTable(_) @@ -623,6 +733,38 @@ fn is_schema_ddl(statement: &Statement) -> bool { ) } +/// Returns whether an `ALTER TABLE` operation cannot affect encryption metadata. +fn is_encryption_neutral_alter_operation(operation: &AlterTableOperation) -> bool { + matches!( + operation, + AlterTableOperation::AddConstraint(_) + | AlterTableOperation::DropConstraint { + drop_behavior: None | Some(DropBehavior::Restrict), + .. + } + | AlterTableOperation::RenameConstraint { .. } + | AlterTableOperation::DisableRowLevelSecurity + | AlterTableOperation::DisableRule { .. } + | AlterTableOperation::DisableTrigger { .. } + | AlterTableOperation::EnableAlwaysRule { .. } + | AlterTableOperation::EnableAlwaysTrigger { .. } + | AlterTableOperation::EnableReplicaRule { .. } + | AlterTableOperation::EnableReplicaTrigger { .. } + | AlterTableOperation::EnableRowLevelSecurity + | AlterTableOperation::EnableRule { .. } + | AlterTableOperation::EnableTrigger { .. } + | AlterTableOperation::OwnerTo { .. } + | AlterTableOperation::AlterColumn { + op: AlterColumnOperation::SetNotNull + | AlterColumnOperation::DropNotNull + | AlterColumnOperation::SetDefault { .. } + | AlterColumnOperation::DropDefault + | AlterColumnOperation::AddGenerated { .. }, + .. + } + ) +} + /// Returns whether a DDL statement can be represented exactly by the overlay. fn is_modelled_ddl(statement: &Statement) -> bool { match statement { @@ -644,7 +786,7 @@ fn is_modelled_ddl(statement: &Statement) -> bool { AlterTableOperation::DropColumn { drop_behavior, .. } => { *drop_behavior != Some(DropBehavior::Cascade) } - _ => false, + operation => is_encryption_neutral_alter_operation(operation), }) } Statement::Drop { @@ -658,6 +800,7 @@ fn is_modelled_ddl(statement: &Statement) -> bool { } #[cfg(test)] +/// Unit coverage for transaction and protocol state transitions. mod tests { use super::*; use eql_mapper::Table; @@ -729,10 +872,10 @@ mod tests { middleware.execution_succeeded(); middleware.execution_started(parse("create table reports (id bigint)")); middleware.execution_succeeded(); - middleware.ready_for_query(b'T'); + middleware.ready_for_query(TransactionStatus::InTransaction); middleware.simple_query(&[parse("commit")]); middleware.execution_succeeded(); - middleware.ready_for_query(b'I'); + middleware.ready_for_query(TransactionStatus::Idle); assert!(middleware.needs_publication()); middleware.publication_succeeded(); @@ -837,7 +980,7 @@ mod tests { .resolver() .resolve_table(&table("reports")) .is_err()); - middleware.ready_for_query(b'I'); + middleware.ready_for_query(TransactionStatus::Idle); assert!(middleware .resolver() .resolve_table(&table("reports")) @@ -852,7 +995,7 @@ mod tests { let portal = Name::from("portal"); middleware.prepare(statement.clone(), parse("select 1")); middleware.bind(portal.clone(), &statement); - middleware.execute(&portal); + assert!(!middleware.execute(&portal)); let mut published = Schema::new("public"); published.add_table(Table::new(Ident::new("reports"))); @@ -865,13 +1008,24 @@ mod tests { .is_err()); middleware.execution_succeeded(); middleware.protocol_boundary(); - middleware.ready_for_query(b'I'); + middleware.ready_for_query(TransactionStatus::Idle); assert!(middleware .resolver() .resolve_table(&table("reports")) .is_ok()); } + #[test] + fn extended_ddl_execution_requests_an_immediate_flush() { + let middleware = SchemaMiddleware::new(Arc::new(Schema::new("public"))); + let statement = Name::from("statement"); + let portal = Name::from("portal"); + middleware.prepare(statement.clone(), parse("create table reports (id bigint)")); + middleware.bind(portal.clone(), &statement); + + assert!(middleware.execute(&portal)); + } + #[test] fn publication_failure_is_visible_to_other_connections() { let store = CommittedSchemaStore::for_testing(Schema::new("public"), EncryptConfig::new()); @@ -944,6 +1098,161 @@ mod tests { ))); } + #[test] + fn unquoted_domain_names_follow_postgresql_case_folding() { + let middleware = SchemaMiddleware::new(Arc::new(Schema::new("public"))); + let statements = vec![ + parse("create table secrets (value EQL_V3_TEXT_SEARCH)"), + parse("insert into secrets (value) values ('classified')"), + ]; + + assert!(matches!( + column_kind(match &statements[0] { + Statement::CreateTable(create) => &create.columns[0], + _ => unreachable!(), + }), + ColumnKind::Eql(_, _) + )); + assert!(middleware.simple_query_requires_fail_closed(&statements)); + } + + #[test] + fn encryption_overlay_uses_catalog_spelling_for_unquoted_identifiers() { + let middleware = SchemaMiddleware::new(Arc::new(Schema::new("public"))); + middleware.execution_started(parse("create table Secrets (Secret EQL_V3_TEXT_SEARCH)")); + middleware.execution_succeeded(); + + assert!(middleware + .encrypt_config() + .get_column_config(&Identifier::new("secrets", "secret")) + .is_some()); + } + + #[test] + fn quoted_domain_names_preserve_case() { + let column = match parse("create table secrets (value \"EQL_V3_TEXT_SEARCH\")") { + Statement::CreateTable(create) => create.columns.into_iter().next().unwrap(), + _ => unreachable!(), + }; + + assert!(matches!(column_kind(&column), ColumnKind::Native)); + } + + #[test] + fn savepoint_names_follow_postgresql_case_folding() { + let middleware = SchemaMiddleware::new(Arc::new(Schema::new("public"))); + middleware.execution_started(parse( + "create table users (id bigint, secret eql_v3_text_search)", + )); + middleware.execution_succeeded(); + middleware.execution_started(parse("savepoint Foo")); + middleware.execution_succeeded(); + middleware.execution_started(parse("alter table users drop column secret")); + middleware.execution_succeeded(); + middleware.execution_started(parse("rollback to savepoint foo")); + middleware.execution_succeeded(); + + assert!(middleware + .resolver() + .resolve_table_column(&table("users"), &Ident::new("secret")) + .is_ok()); + } + + #[test] + fn savepoint_state_desynchronization_fails_closed() { + let middleware = SchemaMiddleware::new(Arc::new(Schema::new("public"))); + middleware.execution_started(parse("rollback to savepoint missing")); + middleware.execution_succeeded(); + + assert!(middleware.has_unmodelled_ddl()); + assert!(middleware.needs_publication()); + } + + #[test] + fn encryption_neutral_alter_table_operations_are_modelled() { + for sql in [ + "alter table users add constraint users_email_uq unique (email)", + "alter table users alter column email set not null", + "alter table users alter column email drop not null", + "alter table users alter column email set default 'unknown'", + "alter table users alter column email drop default", + "alter table users owner to current_user", + "alter table users enable row level security", + ] { + assert!(is_modelled_ddl(&parse(sql)), "not modelled: {sql}"); + } + } + + #[test] + fn encryption_neutral_alter_on_encrypted_table_allows_a_dependent_batch_statement() { + let mut encrypt_config = EncryptConfig::new(); + let column = match parse("create table users (secret eql_v3_text_search)") { + Statement::CreateTable(create) => create.columns.into_iter().next().unwrap(), + _ => unreachable!(), + }; + add_column_config(&mut encrypt_config, "users", &column); + let store = CommittedSchemaStore::for_testing(Schema::new("public"), encrypt_config); + let middleware = SchemaMiddleware::from_store(store); + let statements = vec![ + parse("alter table users alter column secret set not null"), + parse("insert into users (secret) values ('classified')"), + ]; + + assert!(!middleware.simple_query_requires_fail_closed(&statements)); + } + + #[test] + fn native_temporary_table_execution_does_not_dirty_schema_state() { + let middleware = SchemaMiddleware::new(Arc::new(Schema::new("public"))); + middleware.execution_started(parse("create temporary table names (name text)")); + middleware.execution_succeeded(); + + assert!(!middleware.has_unmodelled_ddl()); + assert!(!middleware.needs_publication()); + assert!(middleware + .resolver() + .resolve_table(&table("names")) + .is_err()); + } + + #[test] + fn temporary_table_cannot_shadow_an_encrypted_table_in_a_batch() { + let middleware = SchemaMiddleware::new(Arc::new(Schema::new("public"))); + middleware.execution_started(parse( + "create table users (id bigint, email eql_v3_text_search)", + )); + middleware.execution_succeeded(); + let statements = vec![ + parse("create temporary table users (id bigint, email text)"), + parse("insert into users (id, email) values (1, 'alice@example.com')"), + ]; + + assert!(middleware.simple_query_requires_fail_closed(&statements)); + } + + #[test] + fn temporary_shadowing_remains_fail_closed_after_global_publication() { + let mut encrypt_config = EncryptConfig::new(); + let column = match parse("create table users (email eql_v3_text_search)") { + Statement::CreateTable(create) => create.columns.into_iter().next().unwrap(), + _ => unreachable!(), + }; + add_column_config(&mut encrypt_config, "users", &column); + let store = CommittedSchemaStore::for_testing(Schema::new("public"), encrypt_config); + let middleware = SchemaMiddleware::from_store(store); + + middleware.execution_started(parse("create temporary table users (email text)")); + middleware.execution_succeeded(); + assert!(middleware.has_unmodelled_ddl()); + assert!(!middleware.needs_publication()); + + middleware.protocol_boundary(); + middleware.ready_for_query(TransactionStatus::Idle); + middleware.publication_succeeded(); + + assert!(middleware.has_unmodelled_ddl()); + } + #[test] fn native_temporary_table_batch_does_not_fail_closed() { let middleware = SchemaMiddleware::new(Arc::new(Schema::new("public"))); @@ -1000,7 +1309,7 @@ mod tests { middleware.protocol_boundary(); middleware.execution_failed(); - middleware.ready_for_query(b'I'); + middleware.ready_for_query(TransactionStatus::Idle); tokio::time::timeout( std::time::Duration::from_millis(20), @@ -1024,7 +1333,7 @@ mod tests { middleware.protocol_boundary(); middleware.execution_failed(); - middleware.ready_for_query(b'I'); + middleware.ready_for_query(TransactionStatus::Idle); assert!(tokio::time::timeout( std::time::Duration::from_millis(20), @@ -1033,7 +1342,7 @@ mod tests { .await .is_err()); middleware.execution_succeeded(); - middleware.ready_for_query(b'I'); + middleware.ready_for_query(TransactionStatus::Idle); middleware.wait_for_ddl().await; assert!(middleware .resolver() diff --git a/packages/cipherstash-proxy/src/proxy/schema/mod.rs b/packages/cipherstash-proxy/src/proxy/schema/mod.rs index c4f5bb9b9..2696a1128 100644 --- a/packages/cipherstash-proxy/src/proxy/schema/mod.rs +++ b/packages/cipherstash-proxy/src/proxy/schema/mod.rs @@ -8,4 +8,4 @@ mod manager; mod middleware; pub use manager::{CommittedSchemaStore, SchemaManager}; -pub use middleware::SchemaMiddleware; +pub use middleware::{SchemaMiddleware, TransactionStatus}; diff --git a/tests/integration/golang/pgx_test.go b/tests/integration/golang/pgx_test.go index 18f0cd8cf..6d2348f6b 100644 --- a/tests/integration/golang/pgx_test.go +++ b/tests/integration/golang/pgx_test.go @@ -69,6 +69,39 @@ INSERT INTO t (name) VALUES require.Equal("Ada", result) } +func TestPgxBatchPipelinesEncryptedDDLAndDependentInsert(t *testing.T) { + t.Parallel() + require := require.New(t) + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + config, err := pgx.ParseConfig(os.Getenv("DATABASE_URL")) + require.NoError(err) + config.DefaultQueryExecMode = pgx.QueryExecModeExec + conn, err := pgx.ConnectConfig(ctx, config) + require.NoError(err) + defer conn.Close(ctx) + + table := fmt.Sprintf("bug_308_pgx_pipeline_%d", rand.Int()) + batch := &pgx.Batch{} + batch.Queue(fmt.Sprintf( + "CREATE TABLE %s (id bigint PRIMARY KEY, secret EQL_V3_TEXT_SEARCH NOT NULL)", + table, + )) + batch.Queue( + fmt.Sprintf("INSERT INTO %s (id, secret) VALUES ($1, $2)", table), + 1, + "batched", + ) + + results := conn.SendBatch(ctx, batch) + _, err = results.Exec() + require.NoError(err) + command, err := results.Exec() + require.NoError(err) + require.Equal(int64(1), command.RowsAffected()) + require.NoError(results.Close()) +} + func TestPgxEncryptedMapText(t *testing.T) { t.Parallel() conn := setupPgxConnection(t) @@ -261,9 +294,9 @@ func TestPgxInsertEncryptedWithStructScan(t *testing.T) { // EncryptedRowWithJsonb represents a row with id, encrypted_text and encrypted_jsonb fields type EncryptedRowWithJsonb struct { - ID int `db:"id"` - EncryptedText string `db:"encrypted_text"` - EncryptedJsonb map[string]interface{} `db:"encrypted_jsonb"` + ID int `db:"id"` + EncryptedText string `db:"encrypted_text"` + EncryptedJsonb map[string]interface{} `db:"encrypted_jsonb"` } // Scan implements the sql.Scanner interface for EncryptedRowWithJsonb @@ -304,4 +337,3 @@ func TestPgxInsertEncryptedWithJsonbStructScan(t *testing.T) { }) } } -