Skip to content

Defer pre-growth macro/capture output budgets to MiniJinja 3.0.0 final: upstreamable engine patch #720

Description

@leynos

Status and release gate

Deferred until upstream releases MiniJinja 3.0.0 final for the Rust crate.

Do not start the engine patch or Netsuke's 3.x migration against an alpha, beta, release candidate, or a moving upstream branch. Publication of the stable release is the trigger to re-evaluate and schedule this work, not evidence that upstream has fixed it. Verify the final release's actual APIs and buffering implementation before designing a patch. Prefer a released upstream solution if one then satisfies the acceptance criteria.

This issue records the remaining availability concern found during #651 / #670 and the proposed dependency-level remedy. It does not block the separate immediate MiniJinja 2.24.0 upgrade in #719. Deferral leaves a known residual risk; it does not establish that the current output budgets prevent every oversized internal allocation.

Problem verified during PR #670

At Netsuke commit 5b4552dad502dc5ce289c12b5feeb854c4bb4773, the compiled-expression fallback follows this path:

invoke_macro
  -> call_macro_value
  -> minijinja::Value::call
  -> complete macro result Value
  -> String conversion
  -> charge_macro_output(rendered.len())

The last charge can reject an oversized result, but cannot prevent the engine from first allocating it.

The investigation also inspected the exact MiniJinja 3.0.0-alpha.1 tag. Macro::call still creates String::new(), passes it through Output::new to the macro evaluator, and only then returns a regular or safe-string Value. Output separately maintains capture_stack: Vec<Option<String>> and creates another string for each active capture. An outer Template::render_captured_to writer does not replace those internal buffers.

Terra correctly identified that wrapping the existing call in a captured/template-to-writer API cannot fix the allocation boundary within the original invocation.rs / call.rs scope. This is a limitation of the available API and scope, not a claim that the problem is inherently insoluble.

Affected paths are wider than the fallback

  • Compiled-expression macro callbacks in src/manifest/jinja_macros/invocation.rs and call.rs.
  • Native imported macros during ordinary template rendering, which do not rely on the same fallback callback.
  • Nested macro calls, caller blocks, and internal captures such as block-form set or filter blocks.
  • The corresponding render, expression, when, and manifest-query entry points where these constructs are supported.

A macro-to-writer API that leaves nested macro results or captures unbounded is not a complete solution.

Required security property

Reject an append before a protected output/capture buffer accepts bytes that would exceed its applicable per-buffer or shared allowance.

Bounded materialization is acceptable. Successful macros may still return a bounded String/Value; an over-limit macro must not first build its complete requested output. Neither allocation-free rendering nor a strict bound on total interpreter heap/RSS is the contract here. Buffer capacity, allocator overhead, already-materialized expression values, filter collections, and host-function allocations need separate consideration.

Proposed solution after the release gate opens

1. Reassess MiniJinja 3.0.0 final and define accounting

Inspect the final engine and any relevant upstream fixes. Establish an explicit accounting contract for:

  • bytes accepted by each output/capture buffer;
  • cumulative generated/intermediate bytes versus final rendered bytes;
  • nested captures, copying/forwarding completed values, and captured output that is subsequently discarded;
  • per-evaluation state and the shared per-manifest allowance across separate render states and Netsuke fallback environment clones;
  • rejected writes, arithmetic overflow, re-entrancy, and error unwinding.

Do not silently charge the same append through both an engine hook and Netsuke's CappedWriter. Conversely, do not give nested calls fresh aggregate budgets. If new intermediate-output accounting changes public units or defaults, document and test that policy explicitly rather than silently redefining rendered_manifest_bytes.

2. Add the missing engine-level enforcement, if still necessary

Prepare a narrowly scoped, upstreamable MiniJinja change that propagates a generic render-local output/capture budget to the actual buffer-growth sites. Budget-aware internal buffers are the preferred starting point because they can preserve the native value-returning macro API. A direct macro-to-writer API is also acceptable only if nested calls and internal captures inherit equivalent enforcement.

MiniJinja's alpha introduced mutable State and typed render-local extensions. Those may help carry the accounting handle if retained in final, but storage of a handle is not itself enforcement. The implementation must consult it at every relevant append path.

Audit both inherent and trait-based output methods, including write_str, write_char, write_fmt, literal bytecode emissions, formatter output, escaping, and capture retargeting. Use checked length arithmetic. Retain native argument binding, defaults, closures, imports, caller-block handling, escaping/safe-string semantics, and fuel accounting rather than implementing a second Jinja macro language in Netsuke.

Use safe public integration APIs. Do not add unsafe lifetime extensions, private-layout access, or raw-pointer workarounds. Existing upstream implementation internals do not justify new unsafe code in this patch or Netsuke.

3. Integrate through Netsuke's adapter

Keep ManifestBudgetLimits, ManifestBudgetExhaustion, and accounting policy independent of MiniJinja, localization, and telemetry. The adapter supplies the engine-generic hook, shares the manifest budget across all entry points, and translates typed/provenance-preserving exhaustion into localized, redacted ErrorKind::WriteFailure diagnostics. Do not identify budget failures by parsing error text or misclassify unrelated writer errors.

Preserve existing fuel reservation/refund behaviour and OutOfFuel mapping. Remove post-hoc charge_macro_output(rendered.len()) as the protective boundary once pre-growth enforcement replaces it; reconcile aggregate accounting so removal does not undercharge output and the new hook does not double-charge it. Route final output through the appropriate capped path without claiming that an outer writer protects internal buffers by itself.

4. Carry and retire any temporary dependency patch deliberately

If upstream has not yet released the required functionality, carry the minimal change against the final 3.x baseline with an immutable revision or vendored source as appropriate to the repository's distribution policy. Submit the implementation and portable regression tests upstream. Record the base version, exact patch, upstream issue/PR, and removal condition.

Validate Netsuke's actual build and distribution paths, including crates.io packaging: a Cargo patch used in the development workspace must not leave published consumers on the unpatched implementation. Do not claim the fix ships until the relevant release artefacts use it.

Document the necessary 3.x API migration separately within the implementation plan, retaining compatibility and avoiding unrelated changes.

Regression strategy

An assertion that invocation returns a budget error is insufficient: the current post-hoc implementation can already pass it.

Use a test-only tick() helper that increments a counter and returns an empty string. For example:

{% macro oversized() %}{% for _ in range(1000) %}x{{ tick() }}{% endfor %}{% endmacro %}{{ oversized() }}

With an eight-byte per-buffer allowance, ample fuel, and an aggregate allowance that does not mask the per-buffer failure, a checked literal-append implementation should stop at the ninth x, before the ninth tick(). Verify that expectation against the selected final engine. The original materializing macro path runs the whole loop before an outer writer rejects the result.

Exercise an equivalent macro-only definition plus compiled-expression invocation for the fallback path. This fixture tests literal output deliberately; a formatter-only guard must not pass by checking only interpolated values. Demonstrate that the early-termination regression fails on the old implementation and passes with the remedy.

Acceptance criteria

  • Work starts only after the stable Rust MiniJinja 3.0.0 release; record the final version/revision and re-verification of upstream capabilities.
  • Every applicable macro/capture buffer checks growth before appending; nested calls and separate Netsuke evaluation states cannot reset the shared manifest allowance.
  • The compiled-expression fallback and ordinary imported-macro path both pass early-termination tests, including nested macros and caller blocks.
  • A large internal capture fails before full materialization even when the template would later emit only a small final result.
  • Tests cover literals and formatted output, UTF-8 byte counts, exact-limit success, one-byte-over failure, per-buffer and aggregate exhaustion, arithmetic overflow, and the explicitly documented intermediate/final accounting policy.
  • Positional/keyword/default arguments, escaping, safe strings, captured-state lifetimes, fuel reservation/refund, and OutOfFuel semantics remain correct.
  • Output exhaustion reaches Netsuke as a typed, redacted WriteFailure; localization and bounded telemetry stay at existing adapters/observation boundaries, including manifest-query behaviour.
  • Tests establish early termination using small fixtures, not an out-of-memory event or a large allocation. Do not mistake an output-length invariant for a bound on allocator capacity or whole-process memory.
  • The chosen released or patched dependency reaches all supported build/distribution paths; any temporary patch has an upstream reference and explicit retirement plan.
  • ADR-018 and relevant user/design documentation describe the achieved guarantee and remaining non-output allocation/CPU limitations accurately.
  • Relevant unit, integration, property, BDD, formatting, lint, and documentation gates pass, with exact commands and outcomes reported.

Interim documentation and non-goals

Correct ADR-018 now, rather than waiting for this deferred implementation. Its main text must qualify pre-growth protection to the destinations the current capped writer actually controls, acknowledge the internal macro/capture gap, and distinguish engine fuel/output budgets from whole-process memory or wall-clock/CPU containment. Preserve the original decision date and record these corrections and future work in dated addenda linked to this issue and #719.

Correct the claim that cgroups are inherently host-wide: a dedicated Linux worker cgroup can contain a workload. Such containment complements application-level budgets but has platform/deployment and coarser failure-attribution trade-offs. It is not implemented or required by this ticket, and it is not a byte-exact substitute for a macro-output diagnostic. Whole-interpreter allocation limits and process isolation remain separate design work; output-buffer enforcement alone must not claim to deliver them.

References

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingenhancementNew feature or requestmediumRoadmap items to schedule within the current quarter. Clear scope, normal review cycles.performanceBugs or prior decisions disproportionately impacting memory, time, storage or CPU usagetestingTest coverage, test infrastructure, and verification tooling work.

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions