fix: harden against confirmed review findings

[conorbronsdon]

From an automated multi-agent review (personal-context#62); implemented + verified by Claude Code. (PR auto-recovered — the agent completed and pushed this branch but a transient API error interrupted it before it could open the PR.)

---

fix: harden against confirmed review findings

Three verified findings from an automated multi-agent review, plus a
bundled build-prerequisite pin fix.

1. Parse-time recursion segfault (security/crash). The 256 depth cap
   existed only in the renderer; the recursive-descent parser had no
   bound, so a ~5000-deep nested {% if %} or a ~50k-long not chain
   overflowed the native stack and crashed the process (SIGSEGV). Thread
   a depth counter through both the block parser (parse_body / _parse_if
   / _parse_for) and the expression parser (_p_or..._p_primary), raising
   cleanly at 256 (matching the renderer's _MAX_DEPTH). Recursive _eval
   is transitively protected because the parser now caps expression
   nesting.

2. safe/escape divergence from Jinja (parity). Safety was a transient
   flag on the evaluation result, so x|escape|escape double-escaped and
   x|safe was lost through {% set %} and subsequent filters. Carry a
   safe flag on TemplateValue itself: safe marks the value, escape
   is idempotent on already-safe values, string-transform filters
   (upper/lower/title/trim/replace/truncate) preserve it, and {% set %}
   persists it. Now matches Jinja's Markup semantics.

3. {% set %} inside {% for %} leaked across iterations (parity). Loop
   bodies mutated the enclosing scope, so a counter rendered 3 where Jinja
   renders 0. Snapshot the scope at loop entry, reset to it before every
   iteration (no cross-iteration accumulation) and restore after the loop
   (no leak out), matching Jinja's loop scoping.

Build prerequisite: pixi.toml pinned mojo>=1.0.0b3, which excludes all
1.0.0b3.devN nightlies under PEP 440 ordering, so pixi install failed
to solve. Bumped to >=1.0.0b3.dev0,<2 (same fix as mojo-redis).

Adds 10 regression tests (deep-nesting guards, Markup semantics, loop
scoping). Full suite: 69 passed, 0 failed.

Co-Authored-By: Claude noreply@anthropic.com

[conorbronsdon]

🤖 Independent Claude review:

Verdict: Needs work — one blocking robustness gap in fix #1.

BLOCKING — the parse-depth cap does not close the stack-overflow class it claims to. The PR body states "Recursive _eval is transitively protected because the parser now caps expression nesting." That is not true. _MAX_PARSE_DEPTH only bounds constructs that recurse in the parser (not, unary -, parens, […]-index subexpr, filter-arg subexpr). But three common shapes are parsed iteratively (constant depth) and so sail past the guard, then overflow the native stack at render because _eval/_apply_filter have no depth bound of their own:

• Left-assoc operator chains — _p_add/_p_and/_p_or are while loops (src/template/parser.mojo:352,287,281). {{ 1 + 1 + 1 + … }} (~50k) parses fine, then _eval(EX_ADD) recurses down e.a (render.mojo:214) to full chain depth → SIGSEGV.
• Postfix chains — .attr / [idx] / |filter all loop in _p_postfix (parser.mojo:380). {{ x | upper | upper | … }} or {{ a.b.c.…​ }} build deeply nested EX_FILTER/EX_ATTR, and _apply_filter→_eval(e.a) (render.mojo:244) / _eval(EX_ATTR) (render.mojo:164) recurse to that depth → SIGSEGV.

These are the same crash class as the fixed not-chain, just via the eval spine instead of the parse spine — trivially reachable, unauthenticated DoS. Fix by bounding recursion in _eval/_apply_filter directly (thread a depth counter or cap chain length in the postfix/binary loops). Add regression tests for +/and/or/|/. chains — the current 10 tests cover only the parser-recursion vectors, so the suite passes while the hole remains.

Verified clean:

• Fix fix: harden against confirmed review findings #2 (safe/escape): idempotent escape on safe values, string-filter safe-propagation, and {% set %} persistence all correct; the _Eval.safe↔TemplateValue.safe split is reconciled at output (render.mojo:367) and at set (render.mojo:375-377).
• Fix Add API-doc site, format gate, recipe.yaml, bench #3 (loop scoping): snapshot/reset-per-iteration/restore matches Jinja's non-leaking {% set %} semantics; empty-loop and shadowed-name cases hold. (Non-blocking: scope = saved_scope.copy() every iteration is O(iterations × scope size) — fine for correctness, watch for large loops.)
• pixi pin >=1.0.0b3.dev0,<2 is correct under PEP 440 (admits 1.0.0b3.devN nightlies).

Note: mojo toolchain isn't installed in my environment, so the overflow is established by static call-path analysis, not a runtime crash repro.

[conorbronsdon]

Code review (opus, static): CHANGES-NEEDED

The safe-flag autoescape fix (#2) and loop-scoping fix are correct and well-tested. But the headline recursion-segfault fix is incomplete — the "transitively protected" claim is false:

• src/template/parser.mojo:379-438 (_p_postfix) — .attr/[idx]-base/|filter chains accumulate in a while loop with NO depth increment on the base spine (the depth+1 only guards sub-expressions). {{ a + .b×100000 }}, {{ x|upper×100000 }}, {{ a[0]×100000 }} all parse iteratively, uncapped. (HIGH)
• src/template/render.mojo:154,164,167,244 (_eval) — recurses on e.a for EX_ATTR/EX_ITEM/EX_FILTER with no depth guard → SIGSEGV at reachable depth. The parser cap doesn't bound these trees, so the segfault class the PR advertises closing stays open. (HIGH)
• Separate perf note: render.mojo:399-405 per-iteration scope = saved_scope.copy() is O(N × scope-size) deep-copies (10k-row loop copies the whole context 10k times). (low-med)

Fix: cap the postfix spine in _p_postfix + add a depth guard to _eval, with a test that a long postfix chain raises instead of crashing. Being applied to this branch now.