From 22241f920ce0b1cb929c7158b7b5c20759b43e61 Mon Sep 17 00:00:00 2001 From: Mario Montoya Date: Thu, 17 Oct 2024 13:31:22 -0500 Subject: [PATCH] RLS: Adding a new filter! macro (#1849) Signed-off-by: Mario Montoya Co-authored-by: joshua-spacetime --- Cargo.lock | 1 + crates/bindings-macro/Cargo.toml | 1 + crates/bindings-macro/src/lib.rs | 72 +++++++++++++++++++ crates/bindings/src/lib.rs | 2 +- crates/bindings/src/rt.rs | 13 ++++ ...ps__spacetimedb_bindings_dependencies.snap | 17 +++-- crates/core/src/db/update.rs | 20 +++++- .../src/host/wasm_common/module_host_actor.rs | 24 +++++-- crates/core/src/sql/mod.rs | 1 + crates/core/src/sql/parser.rs | 35 +++++++++ crates/expr/src/errors.rs | 12 +++- crates/expr/src/expr.rs | 14 +++- crates/lib/src/db/raw_def/v9.rs | 4 +- crates/schema/src/auto_migrate.rs | 46 +++++++++++- modules/sdk-test/src/lib.rs | 2 + smoketests/tests/auto_migration.py | 27 +++++++ 16 files changed, 269 insertions(+), 22 deletions(-) create mode 100644 crates/core/src/sql/parser.rs diff --git a/Cargo.lock b/Cargo.lock index cbff17da75e..eabb1b81455 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4315,6 +4315,7 @@ dependencies = [ "proc-macro2", "quote", "spacetimedb-primitives", + "spacetimedb-sql-parser", "syn 2.0.79", ] diff --git a/crates/bindings-macro/Cargo.toml b/crates/bindings-macro/Cargo.toml index 2860071576e..2c4ecdab953 100644 --- a/crates/bindings-macro/Cargo.toml +++ b/crates/bindings-macro/Cargo.toml @@ -13,6 +13,7 @@ bench = false [dependencies] spacetimedb-primitives.workspace = true +spacetimedb-sql-parser.workspace = true bitflags.workspace = true humantime.workspace = true diff --git a/crates/bindings-macro/src/lib.rs b/crates/bindings-macro/src/lib.rs index 3a447a541ce..bc4ba11a2fb 100644 --- a/crates/bindings-macro/src/lib.rs +++ b/crates/bindings-macro/src/lib.rs @@ -17,6 +17,7 @@ use proc_macro::TokenStream as StdTokenStream; use proc_macro2::{Span, TokenStream}; use quote::{format_ident, quote, quote_spanned, ToTokens}; use std::borrow::Cow; +use std::hash::{DefaultHasher, Hash, Hasher}; use std::time::Duration; use syn::ext::IdentExt; use syn::meta::ParseNestedMeta; @@ -1237,3 +1238,74 @@ pub fn schema_type(input: proc_macro::TokenStream) -> proc_macro::TokenStream { .unwrap_or_else(syn::Error::into_compile_error) .into() } + +fn parse_sql(input: ParseStream) -> syn::Result { + use spacetimedb_sql_parser::parser::sub; + + let lookahead = input.lookahead1(); + let sql = if lookahead.peek(syn::LitStr) { + let s = input.parse::()?; + // Checks the query is syntactically valid + let _ = sub::parse_subscription(&s.value()).map_err(|e| syn::Error::new(s.span(), format_args!("{e}")))?; + + s.value() + } else { + return Err(lookahead.error()); + }; + + Ok(sql) +} + +/// Generates code for registering a row-level security `SQL` function. +/// +/// A row-level security function takes a `SQL` query expression that is used to filter rows. +/// +/// The query follows the same syntax as a subscription query. +/// +/// **Example:** +/// +/// ```rust,ignore +/// /// Players can only see what's in their chunk +/// spacetimedb::filter!(" +/// SELECT * FROM LocationState WHERE chunk_index IN ( +/// SELECT chunk_index FROM LocationState WHERE entity_id IN ( +/// SELECT entity_id FROM UserState WHERE identity = @sender +/// ) +/// ) +/// "); +/// ``` +/// +/// **NOTE:** The `SQL` query expression is pre-parsed at compile time, but only check is a valid +/// subscription query *syntactically*, not that the query is valid when executed. +/// +/// For example, it could refer to a non-existent table. +#[proc_macro] +pub fn filter(input: proc_macro::TokenStream) -> proc_macro::TokenStream { + let rls_sql = syn::parse_macro_input!(input with parse_sql); + + let mut hasher = DefaultHasher::new(); + rls_sql.hash(&mut hasher); + let rls_name = format_ident!("rls_{}", hasher.finish()); + + let register_rls_symbol = format!("__preinit__20_register_{rls_name}"); + + let generated_describe_function = quote! { + #[export_name = #register_rls_symbol] + extern "C" fn __register_rls() { + spacetimedb::rt::register_row_level_security::<#rls_name>() + } + }; + + let emission = quote! { + const _: () = { + #generated_describe_function + }; + #[allow(non_camel_case_types)] + struct #rls_name { _never: ::core::convert::Infallible } + impl spacetimedb::rt::RowLevelSecurityInfo for #rls_name { + const SQL: &'static str = #rls_sql; + } + }; + + emission.into() +} diff --git a/crates/bindings/src/lib.rs b/crates/bindings/src/lib.rs index f3b62eba6a6..8a14cd760db 100644 --- a/crates/bindings/src/lib.rs +++ b/crates/bindings/src/lib.rs @@ -26,7 +26,7 @@ pub use rng::StdbRng; pub use sats::SpacetimeType; #[doc(hidden)] pub use spacetimedb_bindings_macro::__TableHelper; -pub use spacetimedb_bindings_macro::{duration, reducer, table}; +pub use spacetimedb_bindings_macro::{duration, filter, reducer, table}; pub use spacetimedb_bindings_sys as sys; pub use spacetimedb_lib; pub use spacetimedb_lib::de::{Deserialize, DeserializeOwned}; diff --git a/crates/bindings/src/rt.rs b/crates/bindings/src/rt.rs index 79a715f3a78..a77b3696273 100644 --- a/crates/bindings/src/rt.rs +++ b/crates/bindings/src/rt.rs @@ -290,6 +290,12 @@ impl RepeaterArgs for (Timestamp,) { } } +/// A trait for types that can *describe* a row-level security policy. +pub trait RowLevelSecurityInfo { + /// The SQL expression for the row-level security policy. + const SQL: &'static str; +} + /// Registers into `DESCRIBERS` a function `f` to modify the module builder. fn register_describer(f: fn(&mut ModuleBuilder)) { DESCRIBERS.lock().unwrap().push(f) @@ -352,6 +358,13 @@ pub fn register_reducer<'a, A: Args<'a>, I: ReducerInfo>(_: impl Reducer<'a, A>) }) } +/// Registers a row-level security policy. +pub fn register_row_level_security() { + register_describer(|module| { + module.inner.add_row_level_security(R::SQL); + }) +} + /// A builder for a module. #[derive(Default)] struct ModuleBuilder { diff --git a/crates/bindings/tests/snapshots/deps__spacetimedb_bindings_dependencies.snap b/crates/bindings/tests/snapshots/deps__spacetimedb_bindings_dependencies.snap index 8019dd6c391..7dfef03b9af 100644 --- a/crates/bindings/tests/snapshots/deps__spacetimedb_bindings_dependencies.snap +++ b/crates/bindings/tests/snapshots/deps__spacetimedb_bindings_dependencies.snap @@ -2,7 +2,7 @@ source: crates/bindings/tests/deps.rs expression: "cargo tree -p spacetimedb -f {lib} -e no-dev" --- -total crates: 62 +total crates: 64 spacetimedb ├── bytemuck ├── derive_more @@ -48,6 +48,15 @@ spacetimedb │ │ ├── itertools │ │ │ └── either │ │ └── nohash_hasher +│ ├── spacetimedb_sql_parser +│ │ ├── derive_more (*) +│ │ ├── sqlparser +│ │ │ └── log +│ │ └── thiserror +│ │ └── thiserror_impl +│ │ ├── proc_macro2 (*) +│ │ ├── quote (*) +│ │ └── syn (*) │ └── syn (*) ├── spacetimedb_bindings_sys │ └── spacetimedb_primitives (*) @@ -75,11 +84,7 @@ spacetimedb │ │ │ └── equivalent │ │ ├── nohash_hasher │ │ ├── smallvec -│ │ └── thiserror -│ │ └── thiserror_impl -│ │ ├── proc_macro2 (*) -│ │ ├── quote (*) -│ │ └── syn (*) +│ │ └── thiserror (*) │ ├── spacetimedb_primitives (*) │ ├── spacetimedb_sats │ │ ├── arrayvec diff --git a/crates/core/src/db/update.rs b/crates/core/src/db/update.rs index 9d88b99045a..46ba4835447 100644 --- a/crates/core/src/db/update.rs +++ b/crates/core/src/db/update.rs @@ -2,8 +2,10 @@ use super::datastore::locking_tx_datastore::MutTxId; use super::relational_db::RelationalDB; use crate::database_logger::SystemLogger; use crate::execution_context::ExecutionContext; +use crate::sql::parser::RowLevelExpr; use spacetimedb_data_structures::map::HashMap; use spacetimedb_lib::db::auth::StTableType; +use spacetimedb_lib::identity::AuthCtx; use spacetimedb_lib::AlgebraicValue; use spacetimedb_primitives::ColSet; use spacetimedb_schema::auto_migrate::{AutoMigratePlan, ManualMigratePlan, MigratePlan}; @@ -24,6 +26,7 @@ use std::sync::Arc; pub fn update_database( stdb: &RelationalDB, tx: &mut MutTxId, + auth_ctx: AuthCtx, plan: MigratePlan, system_logger: &SystemLogger, ) -> anyhow::Result<()> { @@ -44,7 +47,7 @@ pub fn update_database( match plan { MigratePlan::Manual(plan) => manual_migrate_database(stdb, tx, plan, system_logger, existing_tables), - MigratePlan::Auto(plan) => auto_migrate_database(stdb, tx, plan, system_logger, existing_tables), + MigratePlan::Auto(plan) => auto_migrate_database(stdb, tx, auth_ctx, plan, system_logger, existing_tables), } } @@ -63,6 +66,7 @@ fn manual_migrate_database( fn auto_migrate_database( stdb: &RelationalDB, tx: &mut MutTxId, + auth_ctx: AuthCtx, plan: AutoMigratePlan, system_logger: &SystemLogger, existing_tables: Vec>, @@ -119,6 +123,7 @@ fn auto_migrate_database( system_logger.info(&format!("Creating table `{}`", table_name)); log::info!("Creating table `{}`", table_name); + stdb.create_table(tx, table_schema)?; } spacetimedb_schema::auto_migrate::AutoMigrateStep::AddIndex(index_name) => { @@ -221,6 +226,19 @@ fn auto_migrate_database( spacetimedb_schema::auto_migrate::AutoMigrateStep::RemoveSchedule(_) => { anyhow::bail!("Removing schedules is not yet implemented"); } + spacetimedb_schema::auto_migrate::AutoMigrateStep::AddRowLevelSecurity(sql_rls) => { + system_logger.info(&format!("Adding row-level security `{sql_rls}`")); + log::info!("Adding row-level security `{sql_rls}`"); + let rls = plan.new.lookup_expect(sql_rls); + let rls = RowLevelExpr::build_row_level_expr(stdb, tx, &auth_ctx, rls)?; + + stdb.create_row_level_security(tx, rls.def)?; + } + spacetimedb_schema::auto_migrate::AutoMigrateStep::RemoveRowLevelSecurity(sql_rls) => { + system_logger.info(&format!("Removing-row level security `{sql_rls}`")); + log::info!("Removing row-level security `{sql_rls}`"); + stdb.drop_row_level_security(tx, sql_rls.clone())?; + } } } diff --git a/crates/core/src/host/wasm_common/module_host_actor.rs b/crates/core/src/host/wasm_common/module_host_actor.rs index 9f22d7a42e7..852b7e2bb74 100644 --- a/crates/core/src/host/wasm_common/module_host_actor.rs +++ b/crates/core/src/host/wasm_common/module_host_actor.rs @@ -9,9 +9,6 @@ use spacetimedb_schema::schema::{Schema, TableSchema}; use std::sync::Arc; use std::time::Duration; -use spacetimedb_lib::buffer::DecodeError; -use spacetimedb_lib::{bsatn, Address, RawModuleDef}; - use super::instrumentation::CallTimes; use crate::database_logger::SystemLogger; use crate::db::datastore::locking_tx_datastore::MutTxId; @@ -28,10 +25,14 @@ use crate::identity::Identity; use crate::messages::control_db::HostType; use crate::module_host_context::ModuleCreationContext; use crate::replica_context::ReplicaContext; +use crate::sql::parser::RowLevelExpr; use crate::subscription::module_subscription_actor::WriteConflict; use crate::util::const_unwrap; use crate::util::prometheus_handle::HistogramExt; use crate::worker_metrics::WORKER_METRICS; +use spacetimedb_lib::buffer::DecodeError; +use spacetimedb_lib::identity::AuthCtx; +use spacetimedb_lib::{bsatn, Address, RawModuleDef}; use super::*; @@ -263,6 +264,7 @@ impl ModuleInstance for WasmModuleInstance { let timestamp = Timestamp::now(); let stdb = &*self.replica_context().relational_db; let ctx = ExecutionContext::internal(stdb.address()); + let auth_ctx = AuthCtx::for_current(self.replica_context().database.owner_identity); let tx = stdb.begin_mut_tx(IsolationLevel::Serializable); let (tx, ()) = stdb .with_auto_rollback(&ctx, tx, |tx| { @@ -276,6 +278,19 @@ impl ModuleInstance for WasmModuleInstance { stdb.create_table(tx, schema) .with_context(|| format!("failed to create table {table_name}"))?; } + // Insert the late-bound row-level security expressions. + for rls in self.info.module_def.row_level_security() { + self.system_logger() + .info(&format!("Creating row level security `{}`", rls.sql)); + + let rls = RowLevelExpr::build_row_level_expr(stdb, tx, &auth_ctx, rls) + .with_context(|| format!("failed to create row-level security: `{}`", rls.sql))?; + let table_id = rls.def.table_id; + let sql = rls.def.sql.clone(); + stdb.create_row_level_security(tx, rls.def).with_context(|| { + format!("failed to create row-level security for table `{table_id}`: `{sql}`",) + })?; + } stdb.set_initialized(tx, HostType::Wasm, program)?; @@ -335,7 +350,8 @@ impl ModuleInstance for WasmModuleInstance { let (mut tx, _) = stdb.with_auto_rollback(&ctx, tx, |tx| stdb.update_program(tx, HostType::Wasm, program))?; self.system_logger().info(&format!("Updated program to {program_hash}")); - let res = crate::db::update::update_database(stdb, &mut tx, plan, self.system_logger()); + let auth_ctx = AuthCtx::for_current(self.replica_context().database.owner_identity); + let res = crate::db::update::update_database(stdb, &mut tx, auth_ctx, plan, self.system_logger()); match res { Err(e) => { diff --git a/crates/core/src/sql/mod.rs b/crates/core/src/sql/mod.rs index 59aada0d554..77bedc08261 100644 --- a/crates/core/src/sql/mod.rs +++ b/crates/core/src/sql/mod.rs @@ -1,4 +1,5 @@ pub mod ast; pub mod compiler; pub mod execute; +pub mod parser; mod type_check; diff --git a/crates/core/src/sql/parser.rs b/crates/core/src/sql/parser.rs new file mode 100644 index 00000000000..af2289be3a7 --- /dev/null +++ b/crates/core/src/sql/parser.rs @@ -0,0 +1,35 @@ +use crate::db::datastore::locking_tx_datastore::MutTxId; +use crate::db::relational_db::RelationalDB; +use crate::sql::ast::SchemaViewer; +use spacetimedb_expr::check::parse_and_type_sub; +use spacetimedb_expr::errors::TypingError; +use spacetimedb_expr::expr::RelExpr; +use spacetimedb_expr::ty::TyCtx; +use spacetimedb_lib::db::raw_def::v9::RawRowLevelSecurityDefV9; +use spacetimedb_lib::identity::AuthCtx; +use spacetimedb_schema::schema::RowLevelSecuritySchema; + +pub struct RowLevelExpr { + pub sql: RelExpr, + pub def: RowLevelSecuritySchema, +} + +impl RowLevelExpr { + pub fn build_row_level_expr( + stdb: &RelationalDB, + tx: &mut MutTxId, + auth_ctx: &AuthCtx, + rls: &RawRowLevelSecurityDefV9, + ) -> Result { + let mut ctx = TyCtx::default(); + let sql = parse_and_type_sub(&mut ctx, &rls.sql, &SchemaViewer::new(stdb, tx, auth_ctx))?; + + Ok(Self { + def: RowLevelSecuritySchema { + table_id: sql.table_id(&mut ctx)?, + sql: rls.sql.clone(), + }, + sql, + }) + } +} diff --git a/crates/expr/src/errors.rs b/crates/expr/src/errors.rs index 44ceb4eb8cb..471263b38df 100644 --- a/crates/expr/src/errors.rs +++ b/crates/expr/src/errors.rs @@ -1,10 +1,10 @@ -use spacetimedb_sql_parser::{ast::BinOp, parser::errors::SqlParseError}; -use thiserror::Error; - use super::{ statement::InvalidVar, ty::{InvalidTypeId, TypeWithCtx}, }; +use spacetimedb_sql_parser::ast::BinOp; +use spacetimedb_sql_parser::parser::errors::SqlParseError; +use thiserror::Error; #[derive(Error, Debug)] pub enum Unresolved { @@ -134,6 +134,10 @@ impl UnexpectedType { #[error("Duplicate name `{0}`")] pub struct DuplicateName(pub String); +#[derive(Debug, Error)] +#[error("`filter!` does not support column projections; Must return table rows")] +pub struct FilterReturnType; + #[derive(Error, Debug)] pub enum TypingError { #[error(transparent)] @@ -163,4 +167,6 @@ pub enum TypingError { Wildcard(#[from] InvalidWildcard), #[error(transparent)] DuplicateName(#[from] DuplicateName), + #[error(transparent)] + FilterReturnType(#[from] FilterReturnType), } diff --git a/crates/expr/src/expr.rs b/crates/expr/src/expr.rs index 42b7feb68a2..edcf25e7c25 100644 --- a/crates/expr/src/expr.rs +++ b/crates/expr/src/expr.rs @@ -1,12 +1,13 @@ use std::sync::Arc; +use crate::errors::{FilterReturnType, TypingError}; +use crate::static_assert_size; use spacetimedb_lib::AlgebraicValue; +use spacetimedb_primitives::TableId; use spacetimedb_schema::schema::TableSchema; use spacetimedb_sql_parser::ast::BinOp; -use crate::static_assert_size; - -use super::ty::{InvalidTypeId, Symbol, TyCtx, TyId, TypeWithCtx}; +use super::ty::{InvalidTypeId, Symbol, TyCtx, TyId, Type, TypeWithCtx}; /// A logical relational expression #[derive(Debug)] @@ -54,6 +55,13 @@ impl RelExpr { pub fn ty<'a>(&self, ctx: &'a TyCtx) -> Result, InvalidTypeId> { ctx.try_resolve(self.ty_id()) } + + pub fn table_id(&self, ctx: &mut TyCtx) -> Result { + match &*self.ty(ctx)? { + Type::Var(id, _) => Ok(*id), + _ => Err(FilterReturnType.into()), + } + } } /// A relational select operation or filter diff --git a/crates/lib/src/db/raw_def/v9.rs b/crates/lib/src/db/raw_def/v9.rs index 88251ae56c0..1be6f034d5f 100644 --- a/crates/lib/src/db/raw_def/v9.rs +++ b/crates/lib/src/db/raw_def/v9.rs @@ -339,9 +339,9 @@ pub struct RawUniqueConstraintDataV9 { } /// Data for the `RLS` policy on a table. -#[derive(Debug, Clone, SpacetimeType)] +#[derive(Debug, Clone, PartialEq, Eq, SpacetimeType)] #[sats(crate = crate)] -#[cfg_attr(feature = "test", derive(PartialEq, Eq, PartialOrd, Ord))] +#[cfg_attr(feature = "test", derive(PartialOrd, Ord))] pub struct RawRowLevelSecurityDefV9 { /// The `sql` expression to use for row-level security. pub sql: RawSql, diff --git a/crates/schema/src/auto_migrate.rs b/crates/schema/src/auto_migrate.rs index 67da3f94067..e3fec7026d4 100644 --- a/crates/schema/src/auto_migrate.rs +++ b/crates/schema/src/auto_migrate.rs @@ -3,7 +3,7 @@ use spacetimedb_data_structures::{ error_stream::{CollectAllErrors, CombineErrors, ErrorStream}, map::HashSet, }; -use spacetimedb_lib::db::raw_def::v9::TableType; +use spacetimedb_lib::db::raw_def::v9::{RawRowLevelSecurityDefV9, TableType}; use spacetimedb_sats::WithTypespace; pub type Result = std::result::Result>; @@ -87,6 +87,10 @@ pub enum AutoMigrateStep<'def> { AddSchedule(::Key<'def>), /// Remove a schedule annotation from a table. RemoveSchedule(::Key<'def>), + /// Add a row-level security query. + AddRowLevelSecurity(::Key<'def>), + /// Remove a row-level security query. + RemoveRowLevelSecurity(::Key<'def>), } /// Something that might prevent an automatic migration. @@ -170,8 +174,12 @@ pub fn ponder_auto_migrate<'def>(old: &'def ModuleDef, new: &'def ModuleDef) -> let indexes_ok = auto_migrate_indexes(&mut plan, &new_tables); let sequences_ok = auto_migrate_sequences(&mut plan, &new_tables); let constraints_ok = auto_migrate_constraints(&mut plan, &new_tables); + // IMPORTANT: RLS auto-migrate steps must come last, + // since they assume that any schema changes, like adding or dropping tables, + // have already been reflected in the database state. + let rls_ok = auto_migrate_row_level_security(&mut plan); - let ((), (), (), ()) = (tables_ok, indexes_ok, sequences_ok, constraints_ok).combine_errors()?; + let ((), (), (), (), ()) = (tables_ok, indexes_ok, sequences_ok, constraints_ok, rls_ok).combine_errors()?; Ok(plan) } @@ -218,6 +226,7 @@ fn auto_migrate_tables(plan: &mut AutoMigratePlan<'_>) -> Result<()> { plan.steps.push(AutoMigrateStep::AddTable(new.key())); Ok(()) } + // TODO: When we remove tables, we should also remove their dependencies, including row-level security. Diff::Remove { old } => Err(AutoMigrateError::RemoveTable { table: old.name.clone(), } @@ -408,6 +417,19 @@ fn auto_migrate_constraints(plan: &mut AutoMigratePlan, new_tables: &HashSet<&Id .collect_all_errors() } +// Because we can refer to many tables and fields on the row level-security query, we need to remove all of them, +// then add the new ones, instead of trying to track the graph of dependencies. +fn auto_migrate_row_level_security(plan: &mut AutoMigratePlan) -> Result<()> { + for rls in plan.old.row_level_security() { + plan.steps.push(AutoMigrateStep::RemoveRowLevelSecurity(rls.key())); + } + for rls in plan.new.row_level_security() { + plan.steps.push(AutoMigrateStep::AddRowLevelSecurity(rls.key())); + } + + Ok(()) +} + #[cfg(test)] mod tests { use super::*; @@ -490,6 +512,8 @@ mod tests { ) .finish(); + old_builder.add_row_level_security("SELECT * FROM Apples"); + let old_def: ModuleDef = old_builder .finish() .try_into() @@ -597,6 +621,8 @@ mod tests { .with_primary_key(0) .finish(); + new_builder.add_row_level_security("SELECT * FROM Bananas"); + let new_def: ModuleDef = new_builder .finish() .try_into() @@ -620,6 +646,13 @@ mod tests { plan.prechecks[0], AutoMigratePrecheck::CheckAddSequenceRangeValid(&bananas_sequence) ); + let sql_old = RawRowLevelSecurityDefV9 { + sql: "SELECT * FROM Apples".into(), + }; + + let sql_new = RawRowLevelSecurityDefV9 { + sql: "SELECT * FROM Bananas".into(), + }; assert!(plan.steps.contains(&AutoMigrateStep::RemoveSequence(&apples_sequence))); assert!(plan @@ -641,6 +674,11 @@ mod tests { assert!(plan .steps .contains(&AutoMigrateStep::AddSchedule(&inspections_schedule))); + + assert!(plan + .steps + .contains(&AutoMigrateStep::RemoveRowLevelSecurity(&sql_old.sql))); + assert!(plan.steps.contains(&AutoMigrateStep::AddRowLevelSecurity(&sql_new.sql))); } #[test] @@ -710,6 +748,10 @@ mod tests { .with_type(TableType::System) // change type .finish(); + // Invalid row-level security queries can't be detected in the ponder_auto_migrate function, they + // are detected when executing the plan because they depend on the database state. + // new_builder.add_row_level_security("SELECT wrong"); + // remove Bananas let new_def: ModuleDef = new_builder .finish() diff --git a/modules/sdk-test/src/lib.rs b/modules/sdk-test/src/lib.rs index 252ae7592d2..c370b65dc3c 100644 --- a/modules/sdk-test/src/lib.rs +++ b/modules/sdk-test/src/lib.rs @@ -639,3 +639,5 @@ define_tables! { #[spacetimedb::reducer] fn no_op_succeeds(_ctx: &ReducerContext) {} + +spacetimedb::filter!("SELECT * FROM one_u8"); diff --git a/smoketests/tests/auto_migration.py b/smoketests/tests/auto_migration.py index 90c09e5b00c..57900167797 100644 --- a/smoketests/tests/auto_migration.py +++ b/smoketests/tests/auto_migration.py @@ -36,6 +36,8 @@ class AddTableAutoMigration(Smoketest): x: f64, y: f64, } + +spacetimedb::filter!("SELECT * FROM person"); """ MODULE_CODE_UPDATED = ( @@ -57,12 +59,28 @@ class AddTableAutoMigration(Smoketest): println!("{}: {}", prefix, book.isbn); } } + +spacetimedb::filter!("SELECT * FROM book"); """ ) + def assertSql(self, sql, expected): + self.maxDiff = None + sql_out = self.spacetime("sql", self.address, sql) + sql_out = "\n".join([line.rstrip() for line in sql_out.splitlines()]) + expected = "\n".join([line.rstrip() for line in expected.splitlines()]) + self.assertMultiLineEqual(sql_out, expected) + def test_add_table_auto_migration(self): """This tests uploading a module with a schema change that should not require clearing the database.""" + # Check the row-level SQL filter is created correctly + self.assertSql("SELECT sql FROM st_row_level_security", """\ + sql +------------------------ + "SELECT * FROM person" +""") + logging.info("Initial publish complete") # initial module code is already published by test framework @@ -83,6 +101,15 @@ def test_add_table_auto_migration(self): self.publish_module(self.address, clear=False) logging.info("Updated") + + # Check the row-level SQL filter is added correctly + self.assertSql("SELECT sql FROM st_row_level_security", """\ + sql +------------------------ + "SELECT * FROM person" + "SELECT * FROM book" +""") + self.logs(100) self.call("add_person", "Husserl")