NodusRuntime applies no max_frames cap despite documenting MAX_STACK_DEPTH — runaway recursion hangs the host #350

Activity

1. Masterplanner25

   added

   bugSomething isn't workingSomething isn't working

   severity:highMajor feature broken or significantly wrongMajor feature broken or significantly wrong

   subsystem:embedding

   on Aug 6, 2026
2. Masterplanner25 commented on Aug 7, 2026

   

   Masterplanner25

   on Aug 7, 2026

   OwnerAuthor

   More actions

   Found the exact mechanism while verifying docs/security/SECURITY_MATRIX.md. My
   original report said MAX_STACK_DEPTH "appears nowhere else in embedding.py",
   which is true but understates it — the cap is applied and then thrown away.

   src/nodus/runtime/embedding.py:771:

   configure_vm_limits(vm, max_steps=resolved_steps, timeout_ms=resolved_timeout)
   resolved_frames = self.max_frames if max_frames is None else max_frames
   vm.max_frames = resolved_frames

   configure_vm_limits ends with vm.max_frames = MAX_STACK_DEPTH
   (src/nodus/tooling/sandbox.py:37), so the guard is correctly installed — and
   the very next two lines overwrite it with self.max_frames, which is None
   unless the caller passed one.

   So this is not a missing wire-up. It is a default-handling bug in three lines:
   the assignment is unconditional when it should only apply an explicit override.

   Suggested fix:

   configure_vm_limits(vm, max_steps=resolved_steps, timeout_ms=resolved_timeout)
   resolved_frames = self.max_frames if max_frames is None else max_frames
   if resolved_frames is not None:
       vm.max_frames = resolved_frames

   That honors the documented contract ("None means the VM default
   (MAX_STACK_DEPTH)") without changing behavior for anyone who passes an
   explicit value, and leaves configure_vm_limits as the single place the default
   lives — so CLI and embedded cannot drift again.

   A caller who genuinely wants no cap would then need an explicit sentinel; worth
   deciding whether that case should exist at all, given the API is the one used
   for untrusted scripts.

   Doc impact

   docs/security/SECURITY_MATRIX.md claimed "Stack overflow raises sandbox err —
   ✅ CLI ✅ EMB ✅ SRV". The EMB column was wrong, and the adjacent row called the
   depth limit "❌ hardcoded" in all three contexts when max_frames is in fact a
   constructor parameter. Both rows corrected, pointing here.
3. Masterplanner25

   added 2 commits that reference this issue on Aug 7, 2026

   docs(policy,security): correct two false enforcement claims in SECURI…

   ...

   c533aec

   docs(governance): record the sweep's nine findings; fix a repeated fa…

   ...

   f4beeb4
4. Masterplanner25 commented on Aug 14, 2026

   

   Masterplanner25

   on Aug 14, 2026

   OwnerAuthor

   More actions

   Two additions from the docs/runtime/ sweep.

   1. Root cause of the propagation: the docstring on the parameter itself.

   src/nodus/runtime/embedding.py documented the false behavior at the source, which is
   why five documents inherited it:

   max_frames:
       Maximum call stack depth.  Raises a sandbox error on overflow.  ``None``
       means the VM default (``MAX_STACK_DEPTH``).

   MAX_STACK_DEPTH appears in exactly one enforcement site in the whole tree —
   tooling/sandbox.py:37, the CLI path. embedding.py never applies it. Every doc that
   repeated the claim was faithfully copying the docstring. Fixed there as well as in the
   docs, or the next sweep re-derives it.

   Instances found and corrected: SECURITY_MATRIX.md, SECURITY_POSTURE.md,
   EMBEDDING.md (×2), OPERATOR_OR_EMBEDDER_RUNBOOK.md, and the docstring.

   2. The severity is higher than "no cap" in one specific configuration.

   With the default max_steps, runaway recursion still stops — on the step limit:

   {'kind': 'sandbox', 'message': 'Execution step limit exceeded'}
   

   But EMBEDDING.md §2 recommends max_steps=None, timeout_ms=None for long-lived
   hosts — servers, MCP/A2A, workflow engines. In that configuration there is no step
   limit, no deadline, and no frame cap:

   rt = NodusRuntime(max_steps=None, timeout_ms=None)   # the documented server config
   rt.run_source('fn f(n) { return f(n + 1i) }\nf(0i)\n')

   Killed at 60s, still running. No RecursionError — VM frames are heap-allocated, so
   Python's stack is never touched and its recursion limit never fires. The frame list
   grows until the process is OOM-killed.

   So the documented configuration for exactly the long-lived hosts that most need a guard
   is the one configuration with no guard at all. Both docs now carry an explicit warning
   and pass max_frames in the recommended snippet.

   FAILURE_AND_DEGRADATION_MODEL.md also claimed this path produced a Python
   RecursionError "caught as internal error" — it does not, and that row is corrected to
   the two real outcomes.
5. Masterplanner25

   added 2 commits that reference this issue on Aug 14, 2026

   docs(runtime): document 2 undocumented opcodes and 9 missing sandbox …

   ...

   254f5e8

   fix(embedding): apply MAX_STACK_DEPTH by default in NodusRuntime (#350)…

   ...

   8886b9e
6. Masterplanner25

   closed this as completedin #373on Aug 15, 2026
7. Masterplanner25 commented on Aug 15, 2026

   

   Masterplanner25

   on Aug 15, 2026

   OwnerAuthor

   More actions

   Fixed in #373 (8886b9e) — your three-line diagnosis in the first comment was exactly right.

   resolved_frames = self.max_frames if max_frames is None else max_frames
   if resolved_frames is not None:
       vm.max_frames = resolved_frames

   None now means MAX_STACK_DEPTH (10,000), matching the docstring and the CLI. The recommended long-lived-host config raises Call stack overflow in 0.2s instead of growing until OOM.

   All three contexts now take the cap from configure_vm_limits. I checked the SRV column while I was in there: services/server.py imports run_source from tooling/runner.py, which calls configure_vm_limits and never touches max_frames — so the server was correct all along, and the matrix's ✅ was accurate.

   On the open question — should "no cap" be expressible? I decided no. max_frames is the only guard still standing when max_steps and timeout_ms are both None, and this is the API used for untrusted scripts, so a sentinel meaning "disable the last guard" is a footgun with no clear use. A host that genuinely wants an unbounded stack passes a large integer; there is a test proving 20,001 frames deep works when asked for. Easy to revisit if a real need turns up.

   Behavior change worth recording: an embedded run recursing deeper than 10,000 frames previously succeeded and now raises. That is the documented contract being honored rather than a new restriction, but it is visible to anyone who was relying on the gap. Called out in the CHANGELOG.

   Tests — tests/test_max_frames_default.py, 9 tests across embedded and CLI per the security-boundary rule; the 4 asserting the embedded default fail against the unfixed code, verified by stashing the fix. The recursion cases run in subprocesses with timeouts on purpose: an uncapped run doesn't raise, it grows, so an in-process test would hang CI rather than fail it. That also closes TG-007 in TEST_GAP_BACKLOG.md — the untested invariant is what hid this.

   Docs swept back. The five documents that had been corrected on 2026-08-07 to describe the broken behavior now describe the fixed behavior, each keeping a note that v4.1.1 and earlier are affected. EXECUTION_INVARIANTS.md I-SAND-03 is unconditional again rather than "holds only when max_frames is passed".
8. Masterplanner25

   added 7 commits that reference this issue on Aug 15, 2026

   fix(cli): handle --help centrally so it never runs the command (#353, #…

   ...

   c629c2b

   fix(workflow): a sweeper must not adopt runs it does not own (#376) (#…

   ...

   546a2ea

   release: Gate 10 creator validation for v4.2.0; two findings filed

   ...

   910f416

   release: v4.2.0 (#414)

   ...

   1ad1a66

   docs(evals): post-publish eval for v4.2.0 -- clean, no findings

   ...

   9ef6699

   docs(evals): post-publish eval for v4.2.0 -- clean, no findings (#417)

   ...

   102c5d4

   fix: a directly constructed VM caps call depth by default (#458)

   ...

   24d1eda