fix: address PR review comments

- Copy fn pointer out of CUSTOM_MODULES lock before calling module
  declaration to prevent potential deadlock (spin::Mutex not re-entrant)
- Move cargo:rerun-if-env-changed=HYPERLIGHT_JS_RUNTIME_PATH before the
  match so Cargo tracks the env var even when unset; use canonical path
  for rerun-if-changed
- Lock down globalThis.console binding (non-writable/non-configurable)
  after Object.freeze to prevent handler code replacing it entirely
- Fix test doc comment: built-in overrides work except for require

Signed-off-by: Simon Davies <simongdavies@users.noreply.github.com>

Author: simongdavies

src/hyperlight-js-runtime/src/globals/freeze.rs

@@ -20,14 +20,19 @@ use rquickjs::Ctx;
 /// Called AFTER custom_globals! so extender crates can modify/extend
 /// globals first (e.g. adding console.warn/error/info/debug).
 ///
-/// Frozen: console (Object.freeze), print (non-writable).
+/// Frozen: console (Object.freeze + non-writable/non-configurable binding),
+///         print (non-writable/non-configurable binding).
 /// Already frozen: require (non-configurable from setup),
 ///                 String.bytesFrom (on frozen String constructor).
 pub fn setup(ctx: &Ctx<'_>) -> rquickjs::Result<()> {
     ctx.eval::<(), _>(
         r#"
         if (typeof globalThis.console === 'object') {
             Object.freeze(globalThis.console);
+            Object.defineProperty(globalThis, 'console', {
+                writable: false,
+                configurable: false
+            });
         }
         if (typeof globalThis.print === 'function') {
             Object.defineProperty(globalThis, 'print', {

src/hyperlight-js-runtime/src/modules/mod.rs

@@ -115,7 +115,9 @@ pub struct NativeModuleLoader;
 impl Resolver for NativeModuleLoader {
     fn resolve(&mut self, _ctx: &Ctx<'_>, base: &str, name: &str) -> Result<String> {
         ensure_custom_modules_init();
-        if CUSTOM_MODULES.lock().contains_key(name) || BUILTIN_MODULES.contains_key(name) {
+        // Copy result out before dropping the lock to avoid holding it during resolution
+        let found_custom = CUSTOM_MODULES.lock().contains_key(name);
+        if found_custom || BUILTIN_MODULES.contains_key(name) {
             Ok(name.to_string())
         } else {
             Err(rquickjs::Error::new_resolving(base, name))
@@ -126,8 +128,12 @@ impl Resolver for NativeModuleLoader {
 impl Loader for NativeModuleLoader {
     fn load<'js>(&mut self, ctx: &Ctx<'js>, name: &str) -> Result<Module<'js>> {
         ensure_custom_modules_init();
-        // Check custom modules first
-        if let Some(decl) = CUSTOM_MODULES.lock().get(name) {
+        // Copy the fn pointer out while holding the lock, then drop the guard
+        // before calling the declaration. This avoids deadlock if decl()
+        // triggers a nested module load that tries to lock CUSTOM_MODULES
+        // (spin::Mutex is not re-entrant).
+        let custom_decl = CUSTOM_MODULES.lock().get(name).copied();
+        if let Some(decl) = custom_decl {
             return decl(ctx.clone(), name);
         }
         // Fall back to built-in modules

src/hyperlight-js-runtime/tests/fixtures/extended_runtime/Cargo.lock

@@ -50,6 +50,13 @@ fn main() -> anyhow::Result<()> {
     use std::{env, fs};
 
     let args: Vec<String> = env::args().collect();
+    if args.len() < 3 {
+        let program = args.first().map(|s| s.as_str()).unwrap_or("extended_runtime");
+        eprintln!("Usage: {program} <handler_file> <event_json>");
+        return Err(anyhow::anyhow!(
+            "missing required arguments: <handler_file> and <event_json>"
+        ));
+    }
     let file = std::path::PathBuf::from(&args[1]);
     let event = &args[2];
 

src/hyperlight-js-runtime/tests/fixtures/extended_runtime/src/main.rs

@@ -20,7 +20,7 @@ limitations under the License.
 //! - `register_native_module` adds custom modules to the loader
 //! - Built-in modules always work
 //! - Custom modules can be imported from JS handlers
-//! - Built-in module names cannot be overridden (panics)
+//! - Overriding most built-in module names works; overriding `require` panics
 //! - The `native_modules!` macro generates correct `init_native_modules`
 //! - Full pipeline tests with the extended_runtime fixture binary
 
@@ -346,7 +346,12 @@ const EXTENDED_RUNTIME_MANIFEST: &str = concat!(
 
 static EXTENDED_RUNTIME_BINARY: LazyLock<PathBuf> = LazyLock::new(|| {
     let output = Command::new("cargo")
-        .args(["build", "--manifest-path", EXTENDED_RUNTIME_MANIFEST])
+        .args([
+            "build",
+            "--locked",
+            "--manifest-path",
+            EXTENDED_RUNTIME_MANIFEST,
+        ])
         .output()
         .expect("Failed to run cargo build for extended-runtime fixture");
 

src/hyperlight-js-runtime/tests/native_modules.rs

@@ -193,14 +193,25 @@ fn build_js_runtime() -> PathBuf {
 }
 
 fn bundle_runtime() {
+    // Always rerun if the environment variable changes, even if it's currently unset.
+    println!("cargo:rerun-if-env-changed=HYPERLIGHT_JS_RUNTIME_PATH");
+
     let js_runtime_resource = match env::var("HYPERLIGHT_JS_RUNTIME_PATH") {
         Ok(path) => {
-            println!("cargo:warning=Using custom JS runtime: {}", path);
-            println!("cargo:rerun-if-env-changed=HYPERLIGHT_JS_RUNTIME_PATH");
-            println!("cargo:rerun-if-changed={}", path);
-            PathBuf::from(path)
+            let canonical = PathBuf::from(&path)
                 .canonicalize()
-                .expect("HYPERLIGHT_JS_RUNTIME_PATH must point to a valid file")
+                .expect("HYPERLIGHT_JS_RUNTIME_PATH must point to a valid file");
+            assert!(
+                canonical.is_file(),
+                "HYPERLIGHT_JS_RUNTIME_PATH must point to a file, not a directory: {}",
+                canonical.display()
+            );
+            println!(
+                "cargo:warning=Using custom JS runtime: {}",
+                canonical.display()
+            );
+            println!("cargo:rerun-if-changed={}", canonical.display());
+            canonical
         }
         Err(_) => build_js_runtime(),
     };