Sandbox hardening, structured hook logging, and security test coverage for beta release

CHANGELOG.md

@@ -15,6 +15,10 @@ The format is based on Keep a Changelog and this file currently reflects local w
 - Added support for Discord channel config (`DiscordConfig`) and richer settings defaults (`ai_enabled`, `similarity_threshold`, `static_fallback_msg`) in `src/utility/config_loader.rs`.
 - Added environment variable loading for Discord and expanded Telegram env handling in `src/main.rs`.
 - Added a new skill file: `skills/rss_watcher.lua`.
+- Added defense-in-depth sandbox hardening in `src/core/lua_runtime.rs`: dangerous Lua globals (`require`, `load`, `loadfile`, `dofile`, `loadstring`) are explicitly nil'd after VM creation to prevent sandbox escape even if standard-library loading behavior changes.
+- Added sandbox isolation tests: `lua_sandbox_blocks_require`, `lua_sandbox_blocks_load`, `lua_sandbox_blocks_os`, `lua_sandbox_blocks_io`.
+- Added resource-exhaustion test: `lua_memory_limit_blocks_exhaustion` verifies the 64 MB allocator cap terminates runaway skills.
+- Added DB guardrail test: `lua_db_key_length_enforced` verifies both empty and oversized keys are rejected.
 
 ### Changed
 - Expanded runtime startup in `src/main.rs` to run multiple configured adapters concurrently and register them dynamically.
@@ -34,6 +38,7 @@ The format is based on Keep a Changelog and this file currently reflects local w
 - Fixed async Lua infinite-loop timeout behavior by ensuring the instruction hook is bound to the executing coroutine context in `src/core/lua_runtime.rs`.
 - Fixed timeout cancellation propagation for Lua execution by sharing and checking hook cancellation flags during execution.
 - Fixed clippy-reported nested conditional style issue in `src/core/worker_pool.rs`.
+- Replaced `println!` debug statements in Lua instruction hooks with structured `warn!` tracing calls for consistent, structured production logging.
 
 ### Documentation
 - Updated `README.md` to document:

src/core/lua_runtime.rs

@@ -363,7 +363,7 @@ async fn execute_lua_entry(
             },
             move |lua_instance, _| {
                 if hook_flag.load(Ordering::Relaxed) || start_time.elapsed() >= timeout_duration {
-                    println!("☠️ FATAL: Hook timeout triggered in coroutine!");
+                    warn!("Lua sandbox: timeout triggered in coroutine, terminating VM");
                     // Nuclear option: crash the allocator so even alloc-free tight loops die.
                     let _ = lua_instance.set_memory_limit(1);
                     Err(LuaError::external("FORCE_TERMINATE_TIMEOUT"))
@@ -410,6 +410,13 @@ fn new_sandboxed_lua(
 
     let _previous_limit = lua.set_memory_limit(64 * 1024 * 1024)?;
 
+    // Defense-in-depth: explicitly nil out dangerous globals that could allow
+    // sandbox escape even if the standard library was not loaded.
+    lua.load(
+        "require = nil; load = nil; loadfile = nil; dofile = nil; loadstring = nil",
+    )
+    .exec()?;
+
     if let (Some(bridge), Some(skill_name)) = (bridge, skill_name) {
         register_bot_api(
             &lua,
@@ -433,7 +440,6 @@ fn install_instruction_guard(
     let cancelled = Arc::new(AtomicBool::new(false));
     let cancelled_hook = cancelled.clone();
 
-    // Usiamo un Hook spietato
     lua.set_hook(
         HookTriggers {
             every_nth_instruction: Some(100),
@@ -450,14 +456,8 @@ fn install_instruction_guard(
                 || externally_cancelled
                 || start_time.elapsed() >= timeout_duration
             {
-                // 1. Log di emergenza (lo vedrai con --nocapture)
-                println!("☠️ FATAL: Hook timeout triggered!");
-
-                // 2. TENTATIVO NUCLEARE: Se la versione di mlua lo permette,
-                // forziamo un limite di memoria a ZERO per far crashare la VM.
+                warn!("Lua sandbox: timeout triggered on main state, terminating VM");
                 let _ = lua_instance.set_memory_limit(1);
-
-                // 3. Ritorna l'errore che dovrebbe interrompere il poll asincrono
                 return Err(LuaError::external("FORCE_TERMINATE_TIMEOUT"));
             }
             Ok(VmState::Continue)
@@ -1258,4 +1258,190 @@ mod tests {
         cleanup_script(&script_path);
         cleanup_db(&db_path);
     }
+
+    // ── Sandbox isolation tests ────────────────────────────────────────────
+
+    #[tokio::test]
+    async fn lua_sandbox_blocks_require() {
+        let db_path = temp_db_path("sandbox_require");
+        let db_pool = init_pool(&db_path.to_string_lossy()).await.unwrap();
+        let script_path = write_skill_script(
+            "function execute(params) return require('os') end",
+            "sandbox_require",
+        );
+        let bridge = LuaBridge::new(Arc::new(TestAiProvider), db_pool.clone());
+        let result = execute_skill(
+            script_path.clone(),
+            Value::Object(Default::default()),
+            None,
+            Duration::from_secs(2),
+            bridge,
+        )
+        .await;
+        assert!(result.is_err(), "require should be blocked in the sandbox");
+        cleanup_script(&script_path);
+        cleanup_db(&db_path);
+    }
+
+    #[tokio::test]
+    async fn lua_sandbox_blocks_load() {
+        let db_path = temp_db_path("sandbox_load");
+        let db_pool = init_pool(&db_path.to_string_lossy()).await.unwrap();
+        let script_path = write_skill_script(
+            "function execute(params) return load('return 1')() end",
+            "sandbox_load",
+        );
+        let bridge = LuaBridge::new(Arc::new(TestAiProvider), db_pool.clone());
+        let result = execute_skill(
+            script_path.clone(),
+            Value::Object(Default::default()),
+            None,
+            Duration::from_secs(2),
+            bridge,
+        )
+        .await;
+        assert!(result.is_err(), "load should be blocked in the sandbox");
+        cleanup_script(&script_path);
+        cleanup_db(&db_path);
+    }
+
+    #[tokio::test]
+    async fn lua_sandbox_blocks_os() {
+        let db_path = temp_db_path("sandbox_os");
+        let db_pool = init_pool(&db_path.to_string_lossy()).await.unwrap();
+        let script_path = write_skill_script(
+            "function execute(params) return os.time() end",
+            "sandbox_os",
+        );
+        let bridge = LuaBridge::new(Arc::new(TestAiProvider), db_pool.clone());
+        let result = execute_skill(
+            script_path.clone(),
+            Value::Object(Default::default()),
+            None,
+            Duration::from_secs(2),
+            bridge,
+        )
+        .await;
+        assert!(result.is_err(), "os library should not be accessible in sandbox");
+        cleanup_script(&script_path);
+        cleanup_db(&db_path);
+    }
+
+    #[tokio::test]
+    async fn lua_sandbox_blocks_io() {
+        let db_path = temp_db_path("sandbox_io");
+        let db_pool = init_pool(&db_path.to_string_lossy()).await.unwrap();
+        let script_path = write_skill_script(
+            "function execute(params) return io.read() end",
+            "sandbox_io",
+        );
+        let bridge = LuaBridge::new(Arc::new(TestAiProvider), db_pool.clone());
+        let result = execute_skill(
+            script_path.clone(),
+            Value::Object(Default::default()),
+            None,
+            Duration::from_secs(2),
+            bridge,
+        )
+        .await;
+        assert!(result.is_err(), "io library should not be accessible in sandbox");
+        cleanup_script(&script_path);
+        cleanup_db(&db_path);
+    }
+
+    #[tokio::test]
+    async fn lua_memory_limit_blocks_exhaustion() {
+        let db_path = temp_db_path("memory_exhaustion");
+        let db_pool = init_pool(&db_path.to_string_lossy()).await.unwrap();
+        // Attempt to allocate ~200 MB (well beyond the 64 MB cap).
+        // Each chunk is 1 MB, and 200 iterations would reach ~200 MB,
+        // which is more than triple the 64 MB allocator limit.
+        let script = r#"
+            function execute(params)
+                local t = {}
+                local chunk = string.rep("x", 1024 * 1024)
+                for i = 1, 200 do
+                    t[i] = chunk
+                end
+                return "ok"
+            end
+        "#;
+        let script_path = write_skill_script(script, "memory_exhaustion");
+        let bridge = LuaBridge::new(Arc::new(TestAiProvider), db_pool.clone());
+        let result = execute_skill(
+            script_path.clone(),
+            Value::Object(Default::default()),
+            None,
+            Duration::from_secs(5),
+            bridge,
+        )
+        .await;
+        assert!(
+            result.is_err(),
+            "skill should be terminated when it exceeds the 64 MB memory limit"
+        );
+        cleanup_script(&script_path);
+        cleanup_db(&db_path);
+    }
+
+    #[tokio::test]
+    async fn lua_db_key_length_enforced() {
+        let db_path = temp_db_path("db_key_limit");
+        let db_pool = init_pool(&db_path.to_string_lossy()).await.unwrap();
+
+        // Empty key must be rejected.
+        let script_empty = r#"
+            function execute(params)
+                oxide.db.set("", "value")
+                return "ok"
+            end
+        "#;
+        let script_path = write_skill_script(script_empty, "db_key_empty");
+        let bridge = LuaBridge::new(Arc::new(TestAiProvider), db_pool.clone());
+        let result = execute_skill(
+            script_path.clone(),
+            Value::Object(Default::default()),
+            None,
+            Duration::from_secs(2),
+            bridge,
+        )
+        .await;
+        assert!(result.is_err(), "empty db key should be rejected");
+        let msg = result.err().unwrap().to_string();
+        assert!(
+            msg.contains("invalid key length"),
+            "unexpected error message: {}",
+            msg
+        );
+        cleanup_script(&script_path);
+
+        // Oversized key (> 128 bytes) must be rejected.
+        // 200 bytes is chosen to be clearly above the LUA_DB_KEY_MAX_BYTES (128) limit.
+        let script_big = r#"
+            function execute(params)
+                local big_key = string.rep("k", 200)
+                oxide.db.set(big_key, "value")
+                return "ok"
+            end
+        "#;
+        let script_path2 = write_skill_script(script_big, "db_key_oversized");
+        let bridge2 = LuaBridge::new(Arc::new(TestAiProvider), db_pool.clone());
+        let result2 = execute_skill(
+            script_path2.clone(),
+            Value::Object(Default::default()),
+            None,
+            Duration::from_secs(2),
+            bridge2,
+        )
+        .await;
+        assert!(result2.is_err(), "oversized db key should be rejected");
+        let msg2 = result2.err().unwrap().to_string();
+        assert!(
+            msg2.contains("invalid key length"),
+            "unexpected error message: {}",
+            msg2
+        );
+        cleanup_script(&script_path2);
+        cleanup_db(&db_path);
+    }
 }