From e00232deff52015d514f7bfd2548505daf5f4451 Mon Sep 17 00:00:00 2001 From: clockwork-labs-bot Date: Thu, 5 Mar 2026 13:28:25 -0500 Subject: [PATCH 1/8] Run client_connected hook for HTTP SQL requests The /sql endpoint now calls the module's client_connected reducer before executing SQL, and client_disconnected after. This allows module authors to accept or reject SQL connections based on the caller's identity, matching the behavior of the /call endpoint. If client_connected rejects the connection, the SQL request returns 403 Forbidden without executing the query. The sql_direct function is unchanged since it is also used by the pgwire server which has its own connection lifecycle. --- crates/client-api/src/routes/database.rs | 37 ++++++++++++++++++++++-- 1 file changed, 34 insertions(+), 3 deletions(-) diff --git a/crates/client-api/src/routes/database.rs b/crates/client-api/src/routes/database.rs index 20e6196c78b..0cc077362ed 100644 --- a/crates/client-api/src/routes/database.rs +++ b/crates/client-api/src/routes/database.rs @@ -538,15 +538,46 @@ where pub async fn sql( State(worker_ctx): State, - Path(name_or_identity): Path, - Query(params): Query, + Path(SqlParams { name_or_identity }): Path, + Query(SqlQueryParams { confirmed }): Query, Extension(auth): Extension, body: String, ) -> axum::response::Result where S: NodeDelegate + ControlStateDelegate + Authorization, { - let json = sql_direct(worker_ctx, name_or_identity, params, auth.claims.identity, body).await?; + let caller_identity = auth.claims.identity; + let connection_id = generate_random_connection_id(); + + let (host, database) = find_leader_and_database(&worker_ctx, name_or_identity).await?; + + // Run the module's client_connected reducer, if any. + // If it rejects the connection, bail before executing SQL. + let module = host.module().await.map_err(log_and_500)?; + module + .call_identity_connected(auth.into(), connection_id) + .await + .map_err(client_connected_error_to_response)?; + + let sql_auth = worker_ctx + .authorize_sql(caller_identity, database.database_identity) + .await?; + + let result = host + .exec_sql( + sql_auth, + database, + confirmed.unwrap_or(crate::DEFAULT_CONFIRMED_READS), + body, + ) + .await; + + module + .call_identity_disconnected(caller_identity, connection_id, false) + .await + .map_err(client_disconnected_error_to_response)?; + + let json = result?; let total_duration = json.iter().fold(0, |acc, x| acc + x.total_duration_micros); From 5b082e82e819bf2e89c1b5340ab64606a9370778 Mon Sep 17 00:00:00 2001 From: clockwork-labs-bot Date: Thu, 5 Mar 2026 14:10:35 -0500 Subject: [PATCH 2/8] Fix: always call identity_disconnected even if authorize_sql fails Wrap authorize_sql + exec_sql in an async block so the result is captured without early-returning. The disconnect call now runs unconditionally after the connected block completes. --- crates/client-api/src/routes/database.rs | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/crates/client-api/src/routes/database.rs b/crates/client-api/src/routes/database.rs index 0cc077362ed..fbe7c957626 100644 --- a/crates/client-api/src/routes/database.rs +++ b/crates/client-api/src/routes/database.rs @@ -559,19 +559,22 @@ where .await .map_err(client_connected_error_to_response)?; - let sql_auth = worker_ctx - .authorize_sql(caller_identity, database.database_identity) - .await?; + let result = async { + let sql_auth = worker_ctx + .authorize_sql(caller_identity, database.database_identity) + .await?; - let result = host - .exec_sql( + host.exec_sql( sql_auth, database, confirmed.unwrap_or(crate::DEFAULT_CONFIRMED_READS), body, ) - .await; + .await + } + .await; + // Always disconnect, even if authorization or execution failed. module .call_identity_disconnected(caller_identity, connection_id, false) .await From 84b72865fb4542760f35c7c9c5646c3b7e433b97 Mon Sep 17 00:00:00 2001 From: clockwork-labs-bot Date: Thu, 5 Mar 2026 14:47:52 -0500 Subject: [PATCH 3/8] Move connect hook into sql_direct, update pgwire and tests Address review: move client_connected/client_disconnected calls from the HTTP sql handler into sql_direct so both HTTP and pgwire SQL go through the module's onConnect hook. - sql_direct now takes a ConnectionAuthCtx parameter - pgwire stores ConnectionAuthCtx in Metadata (built from validated claims during auth handshake) - Update smoketests: SQL queries now create their own temporary connection, so the st_client table may contain 1 row (the SQL connection's own) when checking that a websocket's row was cleaned up. Tests now assert <= 1 row instead of == 0. --- Cargo.lock | 1 + crates/client-api/src/routes/database.rs | 47 ++++++++----------- crates/pg/Cargo.toml | 1 + crates/pg/src/pg_server.rs | 16 ++++++- .../smoketests/client_connection_errors.rs | 11 +++-- ...ient_connected_error_rejects_connection.py | 11 +++-- 6 files changed, 51 insertions(+), 36 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 60fbaac65ab..a8f8021bfa2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8193,6 +8193,7 @@ dependencies = [ "http 1.3.1", "log", "pgwire", + "spacetimedb-auth", "spacetimedb-client-api", "spacetimedb-client-api-messages", "spacetimedb-lib 2.0.3", diff --git a/crates/client-api/src/routes/database.rs b/crates/client-api/src/routes/database.rs index fbe7c957626..cc6e7954e4b 100644 --- a/crates/client-api/src/routes/database.rs +++ b/crates/client-api/src/routes/database.rs @@ -24,6 +24,7 @@ use futures::TryStreamExt; use http::StatusCode; use log::{info, warn}; use serde::Deserialize; +use spacetimedb::auth::identity::ConnectionAuthCtx; use spacetimedb::database_logger::DatabaseLogger; use spacetimedb::host::module_host::ClientConnectedError; use spacetimedb::host::{CallResult, UpdateDatabaseResult}; @@ -518,35 +519,12 @@ pub async fn sql_direct( SqlParams { name_or_identity }: SqlParams, SqlQueryParams { confirmed }: SqlQueryParams, caller_identity: Identity, + caller_auth: ConnectionAuthCtx, sql: String, ) -> axum::response::Result>> where S: NodeDelegate + ControlStateDelegate + Authorization, { - // Anyone is authorized to execute SQL queries. The SQL engine will determine - // which queries this identity is allowed to execute against the database. - - let (host, database) = find_leader_and_database(&worker_ctx, name_or_identity).await?; - - let auth = worker_ctx - .authorize_sql(caller_identity, database.database_identity) - .await?; - - host.exec_sql(auth, database, confirmed.unwrap_or(crate::DEFAULT_CONFIRMED_READS), sql) - .await -} - -pub async fn sql( - State(worker_ctx): State, - Path(SqlParams { name_or_identity }): Path, - Query(SqlQueryParams { confirmed }): Query, - Extension(auth): Extension, - body: String, -) -> axum::response::Result -where - S: NodeDelegate + ControlStateDelegate + Authorization, -{ - let caller_identity = auth.claims.identity; let connection_id = generate_random_connection_id(); let (host, database) = find_leader_and_database(&worker_ctx, name_or_identity).await?; @@ -555,7 +533,7 @@ where // If it rejects the connection, bail before executing SQL. let module = host.module().await.map_err(log_and_500)?; module - .call_identity_connected(auth.into(), connection_id) + .call_identity_connected(caller_auth, connection_id) .await .map_err(client_connected_error_to_response)?; @@ -568,7 +546,7 @@ where sql_auth, database, confirmed.unwrap_or(crate::DEFAULT_CONFIRMED_READS), - body, + sql, ) .await } @@ -580,7 +558,22 @@ where .await .map_err(client_disconnected_error_to_response)?; - let json = result?; + result +} + +pub async fn sql( + State(worker_ctx): State, + Path(name_or_identity): Path, + Query(params): Query, + Extension(auth): Extension, + body: String, +) -> axum::response::Result +where + S: NodeDelegate + ControlStateDelegate + Authorization, +{ + let caller_identity = auth.claims.identity; + let caller_auth: ConnectionAuthCtx = auth.into(); + let json = sql_direct(worker_ctx, name_or_identity, params, caller_identity, caller_auth, body).await?; let total_duration = json.iter().fold(0, |acc, x| acc + x.total_duration_micros); diff --git a/crates/pg/Cargo.toml b/crates/pg/Cargo.toml index dd49122dea0..a3e1f0c4f3f 100644 --- a/crates/pg/Cargo.toml +++ b/crates/pg/Cargo.toml @@ -7,6 +7,7 @@ license-file = "LICENSE" description = "Postgres wire protocol Server support for SpacetimeDB" [dependencies] +spacetimedb-auth.workspace = true spacetimedb-client-api-messages.workspace = true spacetimedb-client-api.workspace = true spacetimedb-lib.workspace = true diff --git a/crates/pg/src/pg_server.rs b/crates/pg/src/pg_server.rs index 10dae652a02..e49d88d55f6 100644 --- a/crates/pg/src/pg_server.rs +++ b/crates/pg/src/pg_server.rs @@ -22,6 +22,7 @@ use pgwire::messages::data::DataRow; use pgwire::messages::startup::Authentication; use pgwire::messages::{PgWireBackendMessage, PgWireFrontendMessage}; use pgwire::tokio::process_socket; +use spacetimedb_auth::identity::ConnectionAuthCtx; use spacetimedb_client_api::auth::validate_token; use spacetimedb_client_api::routes::database; use spacetimedb_client_api::routes::database::{SqlParams, SqlQueryParams}; @@ -64,6 +65,7 @@ impl From for PgWireError { struct Metadata { database: String, caller_identity: Identity, + caller_auth: ConnectionAuthCtx, } pub(crate) fn to_rows( @@ -163,6 +165,7 @@ where db, SqlQueryParams { confirmed: Some(true) }, params.caller_identity, + params.caller_auth.clone(), query.to_string(), ) .await, @@ -266,8 +269,8 @@ impl claims.identity, + let claims = match validate_token(&self.ctx, &pwd.password).await { + Ok(claims) => claims, Err(err) => { log::error!( "PG: Authentication failed for identity `{}` on database {database}: {err}", @@ -277,12 +280,21 @@ impl Date: Thu, 5 Mar 2026 14:59:03 -0500 Subject: [PATCH 4/8] Add smoketests for SQL connect hook Tests that would fail without the client_connected/client_disconnected hooks added in the /sql HTTP endpoint: - test_sql_rejected_when_client_connected_errors: SQL is rejected (403) when client_connected returns an error. Previously, SQL bypassed the connect hook entirely. - test_sql_triggers_connect_disconnect_hooks: Verifies that SQL requests trigger both client_connected and client_disconnected lifecycle reducers. - test_sql_returns_data_with_connect_hook: Ensures normal SQL queries still work when client_connected accepts the connection. --- crates/smoketests/modules/Cargo.toml | 1 + .../modules/sql-connect-hook/Cargo.toml | 12 ++++ .../modules/sql-connect-hook/src/lib.rs | 23 +++++++ crates/smoketests/tests/smoketests/mod.rs | 1 + .../tests/smoketests/sql_connect_hook.rs | 64 +++++++++++++++++++ 5 files changed, 101 insertions(+) create mode 100644 crates/smoketests/modules/sql-connect-hook/Cargo.toml create mode 100644 crates/smoketests/modules/sql-connect-hook/src/lib.rs create mode 100644 crates/smoketests/tests/smoketests/sql_connect_hook.rs diff --git a/crates/smoketests/modules/Cargo.toml b/crates/smoketests/modules/Cargo.toml index 450a4ea962e..05d68eb7dac 100644 --- a/crates/smoketests/modules/Cargo.toml +++ b/crates/smoketests/modules/Cargo.toml @@ -74,6 +74,7 @@ members = [ "delete-database", "client-connection-reject", "client-connection-disconnect-panic", + "sql-connect-hook", # Log filtering tests "logs-level-filter", diff --git a/crates/smoketests/modules/sql-connect-hook/Cargo.toml b/crates/smoketests/modules/sql-connect-hook/Cargo.toml new file mode 100644 index 00000000000..14c9cc23f2b --- /dev/null +++ b/crates/smoketests/modules/sql-connect-hook/Cargo.toml @@ -0,0 +1,12 @@ +[package] +name = "smoketest-module-sql-connect-hook" +version = "0.1.0" +edition = "2021" +publish = false + +[lib] +crate-type = ["cdylib"] + +[dependencies] +spacetimedb.workspace = true +log.workspace = true diff --git a/crates/smoketests/modules/sql-connect-hook/src/lib.rs b/crates/smoketests/modules/sql-connect-hook/src/lib.rs new file mode 100644 index 00000000000..7c0368003ad --- /dev/null +++ b/crates/smoketests/modules/sql-connect-hook/src/lib.rs @@ -0,0 +1,23 @@ +use spacetimedb::{log, ReducerContext, Table}; + +#[spacetimedb::table(accessor = person, public)] +pub struct Person { + name: String, +} + +#[spacetimedb::reducer(init)] +pub fn init(ctx: &ReducerContext) { + ctx.db.person().insert(Person { + name: "Alice".to_string(), + }); +} + +#[spacetimedb::reducer(client_connected)] +pub fn connected(ctx: &ReducerContext) { + log::info!("sql_connect_hook: client_connected caller={}", ctx.sender); +} + +#[spacetimedb::reducer(client_disconnected)] +pub fn disconnected(ctx: &ReducerContext) { + log::info!("sql_connect_hook: client_disconnected caller={}", ctx.sender); +} diff --git a/crates/smoketests/tests/smoketests/mod.rs b/crates/smoketests/tests/smoketests/mod.rs index 012d7988641..af3494a0826 100644 --- a/crates/smoketests/tests/smoketests/mod.rs +++ b/crates/smoketests/tests/smoketests/mod.rs @@ -33,6 +33,7 @@ mod rls; mod schedule_reducer; mod servers; mod sql; +mod sql_connect_hook; mod templates; mod timestamp_route; mod views; diff --git a/crates/smoketests/tests/smoketests/sql_connect_hook.rs b/crates/smoketests/tests/smoketests/sql_connect_hook.rs new file mode 100644 index 00000000000..898cceb412c --- /dev/null +++ b/crates/smoketests/tests/smoketests/sql_connect_hook.rs @@ -0,0 +1,64 @@ +use spacetimedb_smoketests::Smoketest; + +/// Test that SQL requests are rejected when client_connected returns an error. +/// +/// This verifies that the /sql HTTP endpoint now runs the module's +/// client_connected reducer and rejects the request if it errors. +/// Without PR #4563, this SQL query would succeed. +#[test] +fn test_sql_rejected_when_client_connected_errors() { + let test = Smoketest::builder() + .precompiled_module("client-connection-reject") + .build(); + + // SQL should fail because client_connected returns an error + let result = test.sql("SELECT * FROM all_u8s"); + assert!( + result.is_err(), + "Expected SQL query to be rejected when client_connected errors, but it succeeded" + ); +} + +/// Test that SQL requests trigger client_connected and client_disconnected hooks. +/// +/// This verifies that the /sql HTTP endpoint calls the module's lifecycle +/// reducers. Without PR #4563, no connect/disconnect logs would appear. +#[test] +fn test_sql_triggers_connect_disconnect_hooks() { + let test = Smoketest::builder() + .precompiled_module("sql-connect-hook") + .build(); + + // Run a SQL query + test.sql("SELECT * FROM person").unwrap(); + + // Check that both connect and disconnect hooks were called + let logs = test.logs(100).unwrap(); + assert!( + logs.iter().any(|l| l.contains("sql_connect_hook: client_connected")), + "Expected client_connected log from SQL request, got: {:?}", + logs + ); + assert!( + logs.iter().any(|l| l.contains("sql_connect_hook: client_disconnected")), + "Expected client_disconnected log from SQL request, got: {:?}", + logs + ); +} + +/// Test that SQL queries still return data when client_connected accepts. +/// +/// Ensures the connect hook doesn't break normal SQL functionality. +#[test] +fn test_sql_returns_data_with_connect_hook() { + let test = Smoketest::builder() + .precompiled_module("sql-connect-hook") + .build(); + + test.assert_sql( + "SELECT * FROM person", + r#" name +------- + Alice"#, + ); +} From 6b081b704499ff97b68359522ab4b457c13baaa0 Mon Sep 17 00:00:00 2001 From: Zeke Foppa Date: Fri, 6 Mar 2026 12:45:52 -0800 Subject: [PATCH 5/8] [bot/sql-connect-hook]: review --- crates/pg/src/pg_server.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/pg/src/pg_server.rs b/crates/pg/src/pg_server.rs index e49d88d55f6..138ab6430b2 100644 --- a/crates/pg/src/pg_server.rs +++ b/crates/pg/src/pg_server.rs @@ -284,6 +284,7 @@ impl Date: Fri, 6 Mar 2026 13:12:43 -0800 Subject: [PATCH 6/8] [bot/sql-connect-hook]: fix tests --- crates/smoketests/modules/Cargo.lock | 8 ++++++++ crates/smoketests/modules/sql-connect-hook/src/lib.rs | 4 ++-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/crates/smoketests/modules/Cargo.lock b/crates/smoketests/modules/Cargo.lock index e7260b578ca..d6de55331ec 100644 --- a/crates/smoketests/modules/Cargo.lock +++ b/crates/smoketests/modules/Cargo.lock @@ -849,6 +849,14 @@ dependencies = [ "spacetimedb", ] +[[package]] +name = "smoketest-module-sql-connect-hook" +version = "0.1.0" +dependencies = [ + "log", + "spacetimedb", +] + [[package]] name = "smoketest-module-sql-format" version = "0.1.0" diff --git a/crates/smoketests/modules/sql-connect-hook/src/lib.rs b/crates/smoketests/modules/sql-connect-hook/src/lib.rs index 7c0368003ad..5691fc89520 100644 --- a/crates/smoketests/modules/sql-connect-hook/src/lib.rs +++ b/crates/smoketests/modules/sql-connect-hook/src/lib.rs @@ -14,10 +14,10 @@ pub fn init(ctx: &ReducerContext) { #[spacetimedb::reducer(client_connected)] pub fn connected(ctx: &ReducerContext) { - log::info!("sql_connect_hook: client_connected caller={}", ctx.sender); + log::info!("sql_connect_hook: client_connected caller={}", ctx.sender()); } #[spacetimedb::reducer(client_disconnected)] pub fn disconnected(ctx: &ReducerContext) { - log::info!("sql_connect_hook: client_disconnected caller={}", ctx.sender); + log::info!("sql_connect_hook: client_disconnected caller={}", ctx.sender()); } From 60464e46835d3afdcaf2a9d87dd8bd259d817833 Mon Sep 17 00:00:00 2001 From: Zeke Foppa Date: Fri, 6 Mar 2026 13:15:09 -0800 Subject: [PATCH 7/8] [bot/sql-connect-hook]: fix tests --- .../smoketests/tests/smoketests/sql_connect_hook.rs | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/crates/smoketests/tests/smoketests/sql_connect_hook.rs b/crates/smoketests/tests/smoketests/sql_connect_hook.rs index 898cceb412c..2a69ba5e4e3 100644 --- a/crates/smoketests/tests/smoketests/sql_connect_hook.rs +++ b/crates/smoketests/tests/smoketests/sql_connect_hook.rs @@ -25,9 +25,7 @@ fn test_sql_rejected_when_client_connected_errors() { /// reducers. Without PR #4563, no connect/disconnect logs would appear. #[test] fn test_sql_triggers_connect_disconnect_hooks() { - let test = Smoketest::builder() - .precompiled_module("sql-connect-hook") - .build(); + let test = Smoketest::builder().precompiled_module("sql-connect-hook").build(); // Run a SQL query test.sql("SELECT * FROM person").unwrap(); @@ -51,14 +49,12 @@ fn test_sql_triggers_connect_disconnect_hooks() { /// Ensures the connect hook doesn't break normal SQL functionality. #[test] fn test_sql_returns_data_with_connect_hook() { - let test = Smoketest::builder() - .precompiled_module("sql-connect-hook") - .build(); + let test = Smoketest::builder().precompiled_module("sql-connect-hook").build(); test.assert_sql( "SELECT * FROM person", r#" name -------- - Alice"#, +--------- + "Alice""#, ); } From 00b3853ab08fae0194dd1189ff55ea2362c67f94 Mon Sep 17 00:00:00 2001 From: clockwork-labs-bot Date: Fri, 6 Mar 2026 17:18:03 -0500 Subject: [PATCH 8/8] Add smoketest: disconnect called even on SQL query error Verifies that call_identity_disconnected runs even when exec_sql fails (e.g., querying a nonexistent table). The authorize_sql and exec_sql errors are captured inside an async block, so disconnect always runs afterward. --- .../tests/smoketests/sql_connect_hook.rs | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/crates/smoketests/tests/smoketests/sql_connect_hook.rs b/crates/smoketests/tests/smoketests/sql_connect_hook.rs index 2a69ba5e4e3..17bf5373266 100644 --- a/crates/smoketests/tests/smoketests/sql_connect_hook.rs +++ b/crates/smoketests/tests/smoketests/sql_connect_hook.rs @@ -58,3 +58,29 @@ fn test_sql_returns_data_with_connect_hook() { "Alice""#, ); } + +/// Test that client_disconnected is still called even when the SQL query fails. +/// +/// The `authorize_sql` and `exec_sql` errors are captured inside an async block, +/// so `call_identity_disconnected` runs regardless of query success or failure. +#[test] +fn test_sql_disconnect_called_on_query_error() { + let test = Smoketest::builder().precompiled_module("sql-connect-hook").build(); + + // Run an invalid SQL query — this will fail in exec_sql + let result = test.sql("SELECT * FROM nonexistent_table"); + assert!(result.is_err(), "Expected invalid SQL to fail"); + + // Despite the query error, both connect and disconnect should have been called + let logs = test.logs(100).unwrap(); + assert!( + logs.iter().any(|l| l.contains("sql_connect_hook: client_connected")), + "Expected client_connected even on failed SQL, got: {:?}", + logs + ); + assert!( + logs.iter().any(|l| l.contains("sql_connect_hook: client_disconnected")), + "Expected client_disconnected even on failed SQL, got: {:?}", + logs + ); +}