fix(mods): sandbox reentrancy, transaction-scope reset, tolerant mod headers

Audit wave 2026-07-11, findings 1/8/14:
- LuaCsExecutionGuard keeps a per-LuaState hook stack; nested mods_call restores
  the outer hook instead of disarming it (direct and indirect cycles covered)
- LuaCsModRuntime resets the shared ILuaTransactionScope in finally around every
  handler/timer/load, closing the open-transaction leak
- LuaModHeader skips unknown capability tokens (fail-closed to None);
  ResourcesBundledModSource isolates per-mod load failures

Author: NeoXider

Assets/CoreAIMods/Runtime/Infrastructure/LuaCsModRuntimeFactory.cs

@@ -158,7 +158,11 @@ public static LuaCsModStack Create(LuaCsModStackOptions options)
                 options.AutoPersistMods,
                 // Share the version store the gameplay bindings already use: the runtime keys mod history
                 // under a "mod:" prefix, so it never collides with the coreai_lua_* script slots.
-                options.LuaScriptVersions);
+                options.LuaScriptVersions,
+                // WHY: The bindings are the shared transaction scope of both surfaces; handing them to the
+                // runtime lets it reset a leaked coreai_world_begin per guarded call, exactly as the
+                // one-off executor resets around every chunk.
+                transactionScope: bindings);
 
             LuaCsGameToolExecutor executor = new(
                 new LuaCsSecureEnvironment(),

Assets/CoreAIMods/Runtime/Infrastructure/ResourcesBundledModSource.cs

@@ -48,14 +48,24 @@ public IReadOnlyList<BundledMod> Load()
                     continue;
                 }
 
-                LuaModHeader header = LuaModHeader.Parse(asset.text, asset.name);
-                string id = (header.Id ?? "").Trim();
-                if (id.Length == 0 || !seen.Add(id))
+                // WHY: Per-mod isolation: one asset with a malformed header must not throw out of Load and
+                // kill the seeding of every other bundled mod (and the Hub list with it).
+                try
                 {
-                    continue; // no id, or a duplicate id already taken by an earlier asset
-                }
+                    LuaModHeader header = LuaModHeader.Parse(asset.text, asset.name);
+                    string id = (header.Id ?? "").Trim();
+                    if (id.Length == 0 || !seen.Add(id))
+                    {
+                        continue; // no id, or a duplicate id already taken by an earlier asset
+                    }
 
-                mods.Add(new BundledMod(id, asset.text, header.Version));
+                    mods.Add(new BundledMod(id, asset.text, header.Version));
+                }
+                catch (System.Exception ex)
+                {
+                    Debug.LogWarning(
+                        $"[ResourcesBundledModSource] Skipping bundled mod asset '{asset.name}': {ex.Message}");
+                }
             }
 
             return mods;

Assets/CoreAIMods/Runtime/LuaExecution/LuaCsModRuntime.cs

@@ -132,6 +132,7 @@ private sealed class Mod
         private readonly ILuaModStore _store;
         private readonly ILuaModSourceStore _sourceStore;
         private readonly ILuaScriptVersionStore _versionStore;
+        private readonly ILuaTransactionScope _transactionScope;
         private readonly bool _autoPersistMods;
         private readonly ILog _log;
         private readonly List<Mod> _tickScratch = new();
@@ -212,6 +213,13 @@ private sealed class Mod
         /// <see cref="TryRevertMod"/>. Null falls back to <see cref="NullLuaScriptVersionStore"/> (no history —
         /// the prior behaviour).
         /// </param>
+        /// <param name="transactionScope">
+        /// Optional shared transaction scope of the gameplay bindings behind <paramref name="gameplayBindings"/>.
+        /// When supplied, the runtime resets it around every load chunk and guarded handler/timer call —
+        /// mirroring <see cref="LuaCsGameToolExecutor"/> — so a handler that dies between
+        /// <c>coreai_world_begin</c> and commit cannot leave a stale transaction silently buffering the
+        /// world commands of later handlers/timers.
+        /// </param>
         public LuaCsModRuntime(
             Action<LuaCsApiRegistry, LuaCapabilities> gameplayBindings = null,
             ILuaModStore store = null,
@@ -220,14 +228,16 @@ public LuaCsModRuntime(
             long handlerMaxSteps = DefaultHandlerMaxSteps,
             ILuaModSourceStore sourceStore = null,
             bool autoPersistMods = true,
-            ILuaScriptVersionStore versionStore = null)
+            ILuaScriptVersionStore versionStore = null,
+            ILuaTransactionScope transactionScope = null)
         {
             _gameplayBindings = gameplayBindings;
             _store = store;
             _log = log;
             _sourceStore = sourceStore ?? NullLuaModSourceStore.Instance;
             _versionStore = versionStore ?? new NullLuaScriptVersionStore();
             _autoPersistMods = autoPersistMods;
+            _transactionScope = transactionScope;
             _handlerGuard = new LuaCsExecutionGuard(handlerTimeoutMs, handlerMaxSteps);
         }
 
@@ -406,11 +416,18 @@ private Mod BuildMod(string modId, string luaCode, LuaCapabilities capabilities)
             // during load resolve correctly.
             mod.State = _env.Create(registry);
 
-            // TODO(migration): reset the ported world/unity transaction scope around the load chunk
-            // WHY: (the MoonSharp runtime calls ILuaTransactionScope.ResetTransactions() before/after the
-            // chunk so a transaction left open by a failing load cannot bleed into later scripts).
-            // There is no ported transaction surface yet, so this is a no-op seam for now.
-            _env.RunChunk(mod.State, luaCode);
+            // WHY: Reset the shared transaction scope before/after the load chunk (mirroring the MoonSharp
+            // runtime) so a transaction left open by a failing load cannot bleed into later scripts —
+            // and a transaction leaked elsewhere cannot swallow this chunk's world commands.
+            ResetTransactionScope();
+            try
+            {
+                _env.RunChunk(mod.State, luaCode);
+            }
+            finally
+            {
+                ResetTransactionScope();
+            }
 
             return mod;
         }
@@ -833,10 +850,31 @@ private void InvokeGuarded(Mod mod, LuaFunction fn, params object[] args)
             }
             finally
             {
-                // TODO(migration): reset the ported world/unity transaction scope here (the MoonSharp
-                // WHY: runtime resets ILuaTransactionScope per guarded call so a transaction opened inside one
-                // invocation cannot leak into the next handler/timer/tick). No-op until gameplay bindings
-                // are ported.
+                // WHY: Reset the shared transaction scope per guarded call (mirroring the MoonSharp runtime
+                // and LuaCsGameToolExecutor) so a transaction opened inside one invocation cannot leak
+                // into the next handler/timer/tick and silently buffer its world commands.
+                ResetTransactionScope();
+            }
+        }
+
+        /// <summary>
+        /// Discards any unfinished world/data transaction on the shared gameplay-binding scope. Best-effort:
+        /// it runs inside finally blocks on the tick path, so a throwing scope must not derail the tick.
+        /// </summary>
+        private void ResetTransactionScope()
+        {
+            if (_transactionScope == null)
+            {
+                return;
+            }
+
+            try
+            {
+                _transactionScope.ResetTransactions();
+            }
+            catch (Exception ex)
+            {
+                _log?.Error($"[LuaCsModRuntime] ILuaTransactionScope.ResetTransactions() failed: {ex}");
             }
         }
 

Assets/CoreAIMods/Runtime/LuaExecution/LuaModHeader.cs

@@ -309,7 +309,13 @@ private static string NormalizeCapabilities(string value)
                 LuaCapabilities capabilities = LuaCapabilities.None;
                 for (int i = 0; i < parts.Length; i++)
                 {
-                    capabilities |= (LuaCapabilities)Enum.Parse(typeof(LuaCapabilities), parts[i], true);
+                    // WHY: Tolerant parse (mirrors HubModServiceBase.ParseCaps): an unknown token in one
+                    // LLM/hand-written header must not throw out of Parse and take down whole-directory
+                    // loads such as ResourcesBundledModSource; unknown tokens are simply skipped.
+                    if (Enum.TryParse(parts[i], true, out LuaCapabilities parsed))
+                    {
+                        capabilities |= parsed;
+                    }
                 }
 
                 return capabilities.ToString();

Assets/CoreAIMods/Runtime/Sandbox/LuaCsExecutionGuard.cs

@@ -1,5 +1,7 @@
 using System;
+using System.Collections.Generic;
 using System.Diagnostics;
+using System.Runtime.CompilerServices;
 using System.Threading;
 using Lua;
 using Lua.Runtime;
@@ -26,6 +28,14 @@ public sealed class LuaCsExecutionGuard
         /// <summary>Default per-execution GC allocation budget enforced between VM instructions.</summary>
         public const long DefaultMaxAllocatedBytesBudget = 256 * 1024 * 1024;
 
+        // WHY: Guarded execution can re-enter on the SAME LuaState (mods_call: a self-call, or A calls B
+        // which calls back into A): a nested finally that simply cleared the hook would disarm the
+        // limits the still-running outer call depends on, letting the rest of the outer chunk run
+        // unlimited (sandbox escape). Track the installed hooks per state — shared across guard
+        // instances via this static table — so a nested guard restores the previous hook on exit and
+        // the hook is only fully uninstalled when the outermost guarded call unwinds.
+        private static readonly ConditionalWeakTable<LuaState, Stack<LuaFunction>> InstalledHooks = new();
+
         private readonly int _timeoutMs;
         private readonly long _maxSteps;
         private readonly long _maxAllocatedBytes;
@@ -148,9 +158,11 @@ private LuaValue[] ExecuteGuarded(
                 return new System.Threading.Tasks.ValueTask<int>(ctx.Return());
             });
 
-            state.SetHook(hook, string.Empty, 1);
+            Stack<LuaFunction> installed = InstalledHooks.GetOrCreateValue(state);
+            installed.Push(hook);
             try
             {
+                state.SetHook(hook, string.Empty, 1);
                 return body(cancellationToken);
             }
             catch (LuaRuntimeException)
@@ -159,9 +171,19 @@ private LuaValue[] ExecuteGuarded(
             }
             finally
             {
+                installed.Pop();
                 try
                 {
-                    state.SetHook(null, string.Empty, 0);
+                    if (installed.Count > 0)
+                    {
+                        // WHY: An enclosing guarded call is still running on this state: re-arm ITS hook
+                        // instead of clearing, so the outer step/time/alloc limits stay live.
+                        state.SetHook(installed.Peek(), string.Empty, 1);
+                    }
+                    else
+                    {
+                        state.SetHook(null, string.Empty, 0);
+                    }
                 }
                 catch
                 {

Assets/CoreAIMods/Tests/EditMode/LuaCsModRuntimeEditModeTests.cs

@@ -304,6 +304,98 @@ public void LuaCs_InterMod_ValueAndFunctionExport_ConsumerReadsAndCalls_Function
                 "The error should steer the author toward mods_call.");
         }
 
+        [Test]
+        [Timeout(15000)]
+        public void LuaCs_ModsCall_SelfCall_CannotDisarmHandlerGuard()
+        {
+            MemoryStore store = new();
+            LuaCsModStack stack = BuildStack(store);
+            stack.Runtime.LoadMod("m", @"
+                mods_export('noop', function() return 1 end)
+                hooks_on('go', function()
+                    mods_call(mod_id(), 'noop')
+                    local x = 0
+                    for i = 1, 1000000 do x = x + 1 end
+                    store_set('escaped', 'yes')
+                end)");
+
+            stack.Runtime.EmitEvent("go", "");
+            stack.Runtime.Tick(0);
+
+            Assert.AreEqual("", store.Get("m", "escaped"),
+                "A self mods_call must not disarm the outer guard: the over-budget loop after it must be cut.");
+            Assert.IsNotEmpty(stack.Runtime.GetRecentHandlerErrors("m"),
+                "The over-budget handler must fail with a recorded error, not run to completion unlimited.");
+        }
+
+        [Test]
+        [Timeout(15000)]
+        public void LuaCs_ModsCall_IndirectCycle_CannotDisarmHandlerGuard()
+        {
+            MemoryStore store = new();
+            LuaCsModStack stack = BuildStack(store);
+            stack.Runtime.LoadMod("a", @"
+                mods_export('noop', function() return 1 end)
+                hooks_on('go', function()
+                    mods_call('b', 'pong')
+                    local x = 0
+                    for i = 1, 1000000 do x = x + 1 end
+                    store_set('escaped', 'yes')
+                end)");
+            stack.Runtime.LoadMod("b", "mods_export('pong', function() return mods_call('a', 'noop') end)");
+
+            stack.Runtime.EmitEvent("go", "");
+            stack.Runtime.Tick(0);
+
+            Assert.AreEqual("", store.Get("a", "escaped"),
+                "An A->B->A mods_call cycle must not disarm A's outer guard (self-call bans cannot catch this).");
+            Assert.IsNotEmpty(stack.Runtime.GetRecentHandlerErrors("a"),
+                "The over-budget handler must fail with a recorded error.");
+        }
+
+        [Test]
+        public void LuaCs_HandlerDiesInsideWorldTransaction_NextHandlerCommandStillReachesSink()
+        {
+            MemoryStore store = new();
+            FakeCommandSink sink = new();
+            LuaCsModStack stack = BuildStack(store, sink);
+
+            stack.Runtime.LoadMod("t", @"
+                hooks_on('leak', function()
+                    coreai_world_begin()
+                    error('dies before commit')
+                end)
+                hooks_on('later', function() coreai_world_destroy('victim') end)",
+                LuaCapabilities.WorldEdit);
+
+            stack.Runtime.EmitEvent("leak", "");
+            stack.Runtime.Tick(0);
+            Assert.IsNotEmpty(stack.Runtime.GetRecentHandlerErrors("t"), "The leaking handler must fail.");
+            Assert.AreEqual(0, sink.Commands.Count,
+                "The aborted transaction's buffered commands must never reach the sink.");
+
+            stack.Runtime.EmitEvent("later", "");
+            stack.Runtime.Tick(0);
+            Assert.AreEqual(1, sink.Commands.Count,
+                "A transaction leaked by a dead handler must not silently swallow the next handler's world command.");
+        }
+
+        [Test]
+        public void LuaCs_LoadChunkDiesInsideWorldTransaction_NextLoadCommandStillReachesSink()
+        {
+            FakeCommandSink sink = new();
+            LuaCsModStack stack = BuildStack(new MemoryStore(), sink);
+
+            Assert.Catch(() => stack.Runtime.LoadMod("dying",
+                "coreai_world_begin()\nerror('dies before commit')",
+                LuaCapabilities.WorldEdit));
+            Assert.IsFalse(stack.Runtime.IsLoaded("dying"));
+
+            stack.Runtime.LoadMod("healthy", "coreai_world_destroy('victim')", LuaCapabilities.WorldEdit);
+            Assert.AreEqual(1, sink.Commands.Count,
+                "A transaction leaked by a failing load chunk must not swallow the next load's world command.");
+        }
+
         [Test]
         public void LuaCs_OneOff_ExecuteReturnsOutput()
         {

Assets/CoreAIMods/Tests/EditMode/LuaCsSecureSandboxEditModeTests.cs

@@ -72,6 +72,39 @@ public void AllocationBomb_TableConcat_CapEnforced()
                 $"Expected the table.concat cap to fire, got: {ex.Message}");
         }
 
+        [Test]
+        [Timeout(15000)]
+        public void NestedGuardedCall_SameState_OuterBudgetStaysArmed()
+        {
+            LuaCsSecureEnvironment env = new();
+            LuaCsApiRegistry registry = new();
+            LuaCsExecutionGuard nestedGuard = new(timeoutMs: 2000, maxSteps: 10_000);
+            LuaState state = null;
+            LuaFunction noop = null;
+            registry.Register("nested", new System.Func<double>(() =>
+            {
+                // Mirrors mods_call: a guarded call re-entering the guard on the SAME LuaState.
+                LuaValue[] r = nestedGuard.Execute(state, noop, CancellationToken.None);
+                return r.Length > 0 ? r[0].Read<double>() : 0d;
+            }));
+            state = env.Create(registry);
+            noop = env.RunChunk(state, "return function() return 1 end")[0].Read<LuaFunction>();
+
+            // The nested guard's cleanup must restore the outer hook instead of clearing it; otherwise
+            // the over-budget loop after nested() runs unlimited and the chunk returns normally.
+            LuaCsExecutionGuard outerGuard = new(timeoutMs: 2000, maxSteps: 5_000);
+            LuaRuntimeException ex = Assert.Throws<LuaRuntimeException>(() =>
+                env.RunChunk(state,
+                    "nested()\n" +
+                    "local x = 0\n" +
+                    "for i = 1, 100000 do x = x + 1 end\n" +
+                    "return x",
+                    outerGuard));
+
+            Assert.IsTrue(ex.Message.Contains("EXCEEDED_HARD_LIMIT_STEPS"),
+                $"Expected the OUTER step budget to stay armed after a nested guarded call, got: {ex.Message}");
+        }
+
         [Test]
         public void AllocationBomb_NormalHundredKbString_StillPasses()
         {

Assets/CoreAIMods/Tests/EditMode/LuaModHeaderEditModeTests.cs

@@ -76,6 +76,31 @@ public void Parse_CapabilitiesCommaList_RoundTripsViaLuaCapabilities()
             Assert.AreEqual("Read, Gameplay, WorldEdit", header.Capabilities);
         }
 
+        [Test]
+        public void Parse_UnknownCapabilityToken_IsSkippedInsteadOfThrowing()
+        {
+            LuaModHeader header = LuaModHeader.Parse(@"--[[@coreai
+id: tolerant
+capabilities: Read, Superpowers, WorldEdit
+]]", "fallback");
+
+            Assert.AreEqual("tolerant", header.Id);
+            Assert.AreEqual("Read, WorldEdit", header.Capabilities,
+                "Unknown capability tokens must be skipped, keeping the known ones.");
+        }
+
+        [Test]
+        public void Parse_OnlyUnknownCapabilityTokens_FallsBackToNone()
+        {
+            LuaModHeader header = LuaModHeader.Parse(@"--[[@coreai
+id: tolerant
+capabilities: Superpowers
+]]", "fallback");
+
+            Assert.AreEqual("None", header.Capabilities,
+                "A capabilities line with no known token must fail closed to None, not throw.");
+        }
+
         [Test]
         public void Parse_NoHeader_ReturnsFallbackIdAndDefaultActive()
         {