Fail conflicting describe_module exports for wasm and v8

crates/core/Cargo.toml

@@ -164,6 +164,7 @@ pretty_assertions.workspace = true
 jsonwebtoken.workspace = true
 axum.workspace = true
 fs_extra.workspace = true
+wasmbin.workspace = true
 
 [lints]
 workspace = true

crates/core/src/host/mod.rs

@@ -16,6 +16,8 @@ mod module_common;
 #[allow(clippy::too_many_arguments)]
 pub mod module_host;
 pub mod scheduler;
+#[cfg(test)]
+pub(crate) mod test_utils;
 pub mod wasmtime;
 
 // Visible for integration testing.

crates/core/src/host/test_utils.rs

@@ -0,0 +1,45 @@
+use crate::database_logger::DatabaseLogger;
+use crate::db::relational_db::tests_utils::TestDB;
+use crate::energy::NullEnergyMonitor;
+use crate::host::Scheduler;
+use crate::messages::control_db::{Database, HostType};
+use crate::module_host_context::ModuleCreationContext;
+use crate::replica_context::ReplicaContext;
+use crate::subscription::module_subscription_actor::ModuleSubscriptions;
+use spacetimedb_lib::{Hash, Identity};
+use std::sync::Arc;
+
+pub(crate) fn module_creation_context_for_test(
+    host_type: HostType,
+) -> (ModuleCreationContext, tokio::runtime::Runtime) {
+    let TestDB { db, .. } = TestDB::in_memory().expect("failed to make test db");
+    let (subscriptions, runtime) = ModuleSubscriptions::for_test_new_runtime(db.clone());
+    let logger = {
+        let _rt = runtime.enter();
+        Arc::new(DatabaseLogger::in_memory(64 * 1024))
+    };
+    let replica_ctx = Arc::new(ReplicaContext {
+        database: Database {
+            id: 0,
+            database_identity: Identity::ZERO,
+            owner_identity: Identity::ZERO,
+            host_type,
+            initial_program: Hash::ZERO,
+        },
+        replica_id: 0,
+        logger,
+        subscriptions,
+        relational_db: db,
+    });
+    let (scheduler, _starter) = Scheduler::open(replica_ctx.relational_db.clone());
+
+    (
+        ModuleCreationContext {
+            replica_ctx,
+            scheduler,
+            program_hash: Hash::ZERO,
+            energy_monitor: Arc::new(NullEnergyMonitor),
+        },
+        runtime,
+    )
+}

crates/core/src/host/v8/mod.rs

@@ -942,12 +942,15 @@ fn extract_description<'scope>(
 mod test {
     use super::to_value::test::with_scope;
     use super::*;
+    use crate::host::test_utils::module_creation_context_for_test;
     use crate::host::v8::error::{ErrorOrException, ExceptionThrown};
     use crate::host::wasm_common::module_host_actor::ReducerOp;
     use crate::host::ArgsTuple;
+    use crate::messages::control_db::HostType;
     use spacetimedb_lib::{ConnectionId, Identity};
     use spacetimedb_primitives::ReducerId;
     use spacetimedb_schema::reducer_name::ReducerName;
+    use std::sync::Arc;
 
     fn with_module_catch<T>(
         code: &str,
@@ -1054,4 +1057,51 @@ js error Uncaught Error: foobar
         .map_err(|e| e.to_string());
         assert_eq!(raw_mod, Ok(RawModuleDef::V9(<_>::default())));
     }
+
+    #[test]
+    fn fail_multiple_describe_module_exports() {
+        let code = r#"
+            import { register_hooks } from "spacetime:sys@1.0";
+            register_hooks({
+                __call_reducer__: function() {},
+                __describe_module__: function() {
+                    return new Uint8Array([0]);
+                },
+                __describe_module_v10__: function() {
+                    return new Uint8Array([0]);
+                },
+            })
+        "#;
+        let err = with_scope(|scope| eval_user_module_catch(scope, code))
+            .expect_err("register_hooks should fail")
+            .to_string();
+        assert!(err.contains("cannot register both `__describe_module__` and `__describe_module_v10__` hooks"));
+    }
+
+    #[test]
+    fn startup_worker_fails_multiple_describe_module_exports() {
+        let code: Arc<str> = Arc::from(
+            r#"
+            import { register_hooks } from "spacetime:sys@1.0";
+            register_hooks({
+                __call_reducer__: function() {},
+                __describe_module__: function() {
+                    return new Uint8Array([0]);
+                },
+                __describe_module_v10__: function() {
+                    return new Uint8Array([0]);
+                },
+            })
+        "#,
+        );
+
+        let err = with_scope(|scope| {
+            let (mcc, _runtime) = module_creation_context_for_test(HostType::Js);
+            match startup_instance_worker(scope, code, Either::Right(mcc)) {
+                Ok(_) => panic!("startup should fail"),
+                Err(err) => err.to_string(),
+            }
+        });
+        assert!(err.contains("module cannot register both `__describe_module__` and `__describe_module_v10__` hooks"));
+    }
 }

crates/core/src/host/v8/syscall/hooks.rs

@@ -11,6 +11,10 @@ use crate::host::v8::error::Throwable;
 use crate::host::v8::error::TypeError;
 use crate::host::v8::from_value::cast;
 use crate::host::v8::string::StringConst;
+use crate::host::wasm_common::{DESCRIBE_MODULE_DUNDER, DESCRIBE_MODULE_DUNDER_V10};
+
+const DESCRIBE_MODULE_DUNDER_STR: &StringConst = &StringConst::new(DESCRIBE_MODULE_DUNDER);
+const DESCRIBE_MODULE_DUNDER_V10_STR: &StringConst = &StringConst::new(DESCRIBE_MODULE_DUNDER_V10);
 
 /// Returns the hook function `name` on `hooks_obj`.
 pub(super) fn get_hook_function<'scope>(
@@ -23,6 +27,23 @@ pub(super) fn get_hook_function<'scope>(
     cast!(scope, object, Function, "module function hook `{}`", name.as_str()).map_err(|e| e.throw(scope))
 }
 
+/// Ensures a module doesn't define both `__describe_module__` and `__describe_module_v10__`.
+pub(super) fn validate_describe_hooks<'scope>(
+    scope: &mut PinScope<'scope, '_>,
+    hooks_obj: Local<'_, Object>,
+) -> ExcResult<()> {
+    let has_v9_or_earlier = !property(scope, hooks_obj, DESCRIBE_MODULE_DUNDER_STR.string(scope))?.is_undefined();
+    let has_v10 = !property(scope, hooks_obj, DESCRIBE_MODULE_DUNDER_V10_STR.string(scope))?.is_undefined();
+    if has_v9_or_earlier && has_v10 {
+        return Err(TypeError(format!(
+            "module cannot register both `{}` and `{}` hooks",
+            DESCRIBE_MODULE_DUNDER, DESCRIBE_MODULE_DUNDER_V10
+        ))
+        .throw(scope));
+    }
+    Ok(())
+}
+
 /// Registers all the module function `hooks`
 /// and sets the given `AbiVersion` to `abi`.
 pub(super) fn set_hook_slots(

crates/core/src/host/v8/syscall/v1.rs

@@ -14,7 +14,7 @@ use super::common::{
     procedure_start_mut_tx, row_iter_bsatn_close, table_id_from_name, volatile_nonatomic_schedule_immediate,
 };
 use super::hooks::HookFunctions;
-use super::hooks::{get_hook_function, set_hook_slots};
+use super::hooks::{get_hook_function, set_hook_slots, validate_describe_hooks};
 use super::{AbiVersion, FnRet, ModuleHookKey};
 use crate::host::instance_env::InstanceEnv;
 use crate::host::wasm_common::instrumentation::span;
@@ -311,9 +311,11 @@ fn with_span<'scope, T, E: From<ExceptionThrown>>(
 ///
 /// Throws a `TypeError` if:
 /// - `hooks` is not an object that has functions `__describe_module__` and `__call_reducer__`.
+/// - `hooks` contains both `__describe_module__` and `__describe_module_v10__`.
 fn register_hooks_v1_0<'scope>(scope: &mut PinScope<'scope, '_>, args: FunctionCallbackArguments<'_>) -> FnRet<'scope> {
     // Convert `hooks` to an object.
     let hooks = cast!(scope, args.get(0), Object, "hooks object").map_err(|e| e.throw(scope))?;
+    validate_describe_hooks(scope, hooks)?;
 
     let describe_module = get_hook_function(scope, hooks, str_from_ident!(__describe_module__))?;
     let call_reducer = get_hook_function(scope, hooks, str_from_ident!(__call_reducer__))?;

crates/core/src/host/v8/syscall/v2.rs

@@ -16,7 +16,7 @@ use super::common::{
     procedure_start_mut_tx, row_iter_bsatn_close, table_id_from_name, volatile_nonatomic_schedule_immediate,
 };
 use super::hooks::HookFunctions;
-use super::hooks::{get_hook_function, set_hook_slots};
+use super::hooks::{get_hook_function, set_hook_slots, validate_describe_hooks};
 use super::{AbiVersion, ModuleHookKey};
 use crate::host::instance_env::InstanceEnv;
 use crate::host::wasm_common::instrumentation::span;
@@ -280,9 +280,11 @@ fn with_nothing<'scope, O: JsReturnValue>(
 ///
 /// Throws a `TypeError` if:
 /// - `hooks` is not an object that has functions `__describe_module__` and `__call_reducer__`.
+/// - `hooks` contains both `__describe_module__` and `__describe_module_v10__`.
 fn register_hooks_v2_0<'scope>(scope: &mut PinScope<'scope, '_>, args: FunctionCallbackArguments<'_>) -> ExcResult<()> {
     // Convert `hooks` to an object.
     let hooks = cast!(scope, args.get(0), Object, "hooks object").map_err(|e| e.throw(scope))?;
+    validate_describe_hooks(scope, hooks)?;
 
     let describe_module = get_hook_function(scope, hooks, str_from_ident!(__describe_module__))?;
     let call_reducer = get_hook_function(scope, hooks, str_from_ident!(__call_reducer__))?;

crates/core/src/host/wasm_common.rs

@@ -42,16 +42,28 @@ pub fn detect_raw_def_version<M>(module: &M) -> Result<RawModuleDefVersion, modu
 where
     M: module_host_actor::WasmModule,
 {
-    if module.get_export(DESCRIBE_MODULE_DUNDER).is_some() {
-        Ok(RawModuleDefVersion::V9OrEarlier)
-    } else if module.get_export(DESCRIBE_MODULE_DUNDER_V10).is_some() {
-        Ok(RawModuleDefVersion::V10)
-    } else {
-        Err(module_host_actor::DescribeError::Signature(anyhow::anyhow!(
+    detect_raw_def_version_from_export(|name| module.get_export(name))
+}
+
+fn detect_raw_def_version_from_export<T>(
+    get_export: impl Fn(&str) -> Option<T>,
+) -> Result<RawModuleDefVersion, module_host_actor::DescribeError> {
+    let has_v9_or_earlier = get_export(DESCRIBE_MODULE_DUNDER).is_some();
+    let has_v10 = get_export(DESCRIBE_MODULE_DUNDER_V10).is_some();
+
+    match (has_v9_or_earlier, has_v10) {
+        (true, false) => Ok(RawModuleDefVersion::V9OrEarlier),
+        (false, true) => Ok(RawModuleDefVersion::V10),
+        (false, false) => Err(module_host_actor::DescribeError::Signature(anyhow::anyhow!(
             "module does not export a {} or {} function",
             DESCRIBE_MODULE_DUNDER,
             DESCRIBE_MODULE_DUNDER_V10
-        )))
+        ))),
+        (true, true) => Err(module_host_actor::DescribeError::Signature(anyhow::anyhow!(
+            "module exports both {} and {}; expected exactly one describe function",
+            DESCRIBE_MODULE_DUNDER,
+            DESCRIBE_MODULE_DUNDER_V10
+        ))),
     }
 }
 /// Returns the describe dunder symbol for a given module version.

crates/core/src/host/wasmtime/wasmtime_module.rs

@@ -684,6 +684,14 @@ fn get_memory_size(store: &Store<WasmInstanceEnv>) -> usize {
 mod tests {
     use super::*;
     use crate::energy::EnergyQuanta;
+    use crate::host::test_utils::module_creation_context_for_test;
+    use crate::host::wasm_common::module_host_actor::WasmModuleHostActor;
+    use crate::host::wasm_common::{DESCRIBE_MODULE_DUNDER, DESCRIBE_MODULE_DUNDER_V10};
+    use crate::messages::control_db::HostType;
+    use wasmbin::builtins::Blob;
+    use wasmbin::indices::{FuncId, TypeId};
+    use wasmbin::sections::{Export, ExportDesc, FuncBody};
+    use wasmbin::types::{FuncType, ValueType};
 
     #[test]
     fn test_fuel() {
@@ -698,4 +706,59 @@ mod tests {
         let used = EnergyQuanta::from(budget) - remaining;
         assert_eq!(used, EnergyQuanta::new(10));
     }
+
+    fn module_with_both_describe_exports_wasm() -> Vec<u8> {
+        let module = wasmbin::Module {
+            sections: vec![
+                vec![FuncType {
+                    params: vec![ValueType::I32],
+                    results: vec![],
+                }]
+                .into(),
+                vec![TypeId::from(0), TypeId::from(0)].into(),
+                vec![
+                    Export {
+                        name: DESCRIBE_MODULE_DUNDER.to_owned(),
+                        desc: ExportDesc::Func(FuncId::from(0)),
+                    },
+                    Export {
+                        name: DESCRIBE_MODULE_DUNDER_V10.to_owned(),
+                        desc: ExportDesc::Func(FuncId::from(1)),
+                    },
+                ]
+                .into(),
+                vec![
+                    Blob::from(FuncBody {
+                        locals: vec![],
+                        expr: vec![],
+                    }),
+                    Blob::from(FuncBody {
+                        locals: vec![],
+                        expr: vec![],
+                    }),
+                ]
+                .into(),
+            ],
+        };
+        module.encode_into(Vec::new()).expect("failed to encode test module")
+    }
+
+    #[test]
+    fn wasm_module_host_actor_fails_multiple_describe_module_exports() {
+        let wasm = module_with_both_describe_exports_wasm();
+        let engine = wasmtime::Engine::new(&wasmtime::Config::new()).unwrap();
+        let module = wasmtime::Module::new(&engine, &wasm).expect("failed to compile test module");
+        let linker = wasmtime::Linker::new(&engine);
+        let instance_pre = linker
+            .instantiate_pre(&module)
+            .expect("failed to prepare instantiation");
+        let module = WasmtimeModule::new(instance_pre);
+        let (mcc, _runtime) = module_creation_context_for_test(HostType::Wasm);
+
+        let err = match WasmModuleHostActor::new(mcc, module) {
+            Ok(_) => panic!("initialization should fail"),
+            Err(err) => err.to_string(),
+        };
+        assert!(err.contains("module exports both __describe_module__ and __describe_module_v10__"));
+    }
 }