ARCADIA-Studio/harness/release-analysis-decoder-review-20260911.md

Bounded analysis decoding review — 2026-09-11

Codex-956atf /root/semantics_critic; report-only assignment. Reviewed current persistence, language, source-project, REST and CLI boundaries, plus the isolated schema8 candidate at .local/analysis-core-worktree-20260911. No decoder/dependency/production changes, Cargo, native input or Git operations were performed. Gitea issue 1 comment 246 records ownership and the early decision sent to root and requirements_delta.

Decision

Retain normal serde_json recursion rejection for the current core staging slice. Do not enable unbounded_depth on the strength of a 512-container scan plus serde_stacker alone. This permits the worker's bounded type/admission work to continue; it does not complete the ratified logical-depth128 transport contract.

For the complete transport implementation, prefer one bounded codec using the existing Serde schema, with a nonrecursive raw structural preflight, explicit aggregate allocation/work limits, bounded typed expression admission and protected lifecycle operations. serde_stacker is an appropriate maintained component for the recursive Serde calls after those gates. A second general JSON parser would duplicate string/number/schema semantics without fixing recursive ownership afterward. A flat iterative internal expression representation is a stronger alternative if the current boxed representation cannot pass lifecycle gates; it is a larger change and is not needed to unblock this staging slice.

This is a concrete choice of the existing conservative behavior now and a gated Serde codec next, not a claim that logical depth128 is inherently safe for every Rust operation or target. While the conservative reader remains, a save/export must reject values outside its explicit transport profile or remain an unfinished staging capability; a deep in-memory admission pass does not make a later save reopenable. No stack/RSS result was measured in this review.

Observed boundaries

Actual source	What it establishes; what it does not
crates/oa-core/src/persistence.rs:139–223	Checks caller byte limit, builds a complete serde_json::Value header, then deserializes the original bytes again into the typed schema. Model counts and validation run afterward. Preserving the original-byte second parse is important for duplicate rejection. A depth cap does not bound the header's width.
crates/oa-core/src/language.rs:26–104	ParseLimits counts bytes, graph objects, records and step references. It is not a JSON-value or Expr allocation budget. UNLIMITED uses usize::MAX; it must not silently become authority for unbounded recursive JSON.
crates/oa-server/src/store.rs:20–36, routes.rs:131, lib.rs:65	REST has an 8MiB body cap and separate model/source-project limits. Generic typed request decoding is a separate serde_json::from_slice path. Store envelopes, commands, model import and responses all need an explicit codec audit; changing core persistence alone does not change them.
crates/oa-cli/src/lib.rs:13–23, source_project.rs:51–70	CLI source projects have a 64MiB aggregate source budget, 256 files and bounded include traversal. This is source-only CLI admission, not evidence that every JSON entry point has a 64MiB cap.
persistence.rs:860–885	Native load_with_report reads the file and calls the unlimited helper. Save validates, then allocates the complete pretty JSON or formatted source before its existing atomic-file transaction. An input budget does not limit this allocation.
language.rs:811, 1297–1303, language/semantic_source.rs:13	Generic source values use a recursive AST and a separate64-level parser cap; resolution/formatting and historical conversion also recurse. The600-declaration/128KiB test in language_nesting_tests.rs:266 exercises iterative declaration handling, not128-deep typed Expr JSON/source.
Isolated analysis/types.rs:102–129	Expr is a closed externally tagged enum containing Box<Expr>, Vec<Expr> and UUID-keyed Call arguments. Debug, Clone, PartialEq, Serialize and Deserialize are derived; ownership also drops recursively. Call argument decoding reuses unique_id_map.
Isolated analysis/admission.rs::Info::read, typecheck.rs::Checker::expression	The structural expression walk is iterative and checks depth128, nodes65536 and list1024; type checking is recursively bounded afterward. The1,000,000 static-event ceiling runs after allocation and is a different budget from wire decoding. Latest inspected admit and public validation run this analysis admission before architectural visitors that might clone an element.

The integrated persistence/language observations were read at SHA256 1C01474F906A63E552C0D590B272C95821C00A100B3E79E58E67DBBEAC341A14 and 638F08A42E9CF4996B5D07F69B4D6F517A518369F3604D25BE1006E40E080765; isolated analysis/types.rs was 43A41FE1E1B151E252AB9BBFF031E24CDBAF4EAAFCB0FD33975E760ED22E5BD5. The isolated work is active staging, not a frozen compiled artifact. The installed lock contains serde_json1.0.151 and serde1.0.229.

Concrete falsifiers

1. Logical depth and JSON depth differ. Start with {"Boolean":true} and wrap it127 times in {"Call":{"definition":{"Local":"00000000-0000-0000-0000-000000000001"},"arguments":{"00000000-0000-0000-0000-000000000001":CHILD}}}. The resulting128-node wire shape is16,018 ASCII bytes and382 containers deep before its document envelope. This is a shape/count example, not an admitted call graph. Default serde_json starts with a128-container recursion counter, so it cannot provide full logical-depth128 support. The proposed512 is plausible for this current shape but must be checked against every actual document/envelope/constructor; it is not the semantic depth limit.

2. Flat malformed JSON defeats a depth-only admission gate. {"schema_version":8,"junk":[0,0,...]} has container depth2 and exactly 2*N + 29 bytes. With4,190,000 zeros it is8,380,029 bytes, below8MiB; with33,000,000 zeros it is66,000,029 bytes, below64MiB. The current header path allocates all those Value entries before the typed schema rejects junk. These are deterministic byte counts, not measured allocations or Rust executions. The65536 expression-node and model-record limits do not run soon enough to protect this path.

3. Accepted node/depth limits do not bound pretty output to the input profile. Build a balanced Boolean Or tree with14 branching levels (32767 nodes), then wrap it in113 Unary Not nodes. It has32880 expression nodes, logical depth128 and255 JSON containers. Its standalone compact JSON is904,810 ASCII bytes; equivalent two-space pretty JSON is67,336,988 bytes, already above64MiB before Project wrapping. The typed Boolean operations are appropriate for this shape; no Rust admission run is claimed. These lengths were computed by a deterministic object/indent recurrence, checked against actual JSON.stringify on a small same-shape fixture; no67MB string or file was created. Existing to_vec_pretty means a future deeper decoder can expose a large output allocation and a readback-profile mismatch even for an otherwise bounded expression.

4. A DOM intermediate can erase invalid input. Two operand fields in a Unary payload must be rejected, as must duplicate Call parameter IDs and repeated enum constructors. A Value object collapses repeated decoded keys; converting that Value into Expr can no longer establish the original duplicate error. Escaped and literal spellings such as "operand" and "\u006fperand" are the same decoded key. Continue typed decoding from the original bytes, never from a normalized header DOM. Current unique_id_map (model.rs:1639) deserializes the second value before detecting a duplicate with insert; aggregate admission must count the work/allocation of both, including the displaced subtree's drop.

5. Decode protection ends before all dangerous operations do. Test the deepest accepted tree through clone, equality, Debug/error reporting, source conversion, compact/pretty serialization and ordinary drop. Also reject an unknown field or duplicate key after a deep subtree has already been constructed, then verify partial-state destruction. A postdecode depth error does not protect the drop needed to return that error. A manually constructed100,000-deep public Expr is outside every wire preflight; a hostile in-memory test must use safe teardown or a separate process, rather than accidentally demonstrating its own recursive destructor.

6. A string-aware scan is essential, but need not become another JSON parser. Braces/brackets inside strings, \u005b, escaped quotes, and odd/even runs of backslashes must not change structural depth. Test EOF after a backslash/quote, mismatched closers, invalid UTF-8, unpaired surrogate escapes, trailing non-whitespace and multiple root values. The scanner bounds resources; serde_json remains responsible for valid JSON and decoded keys. Manual use of Deserialize must finish with Deserializer::end() so trailing input is rejected.

Minimal next codec gate

1. Require a finite, explicit ingress profile at every external boundary. Apply bytes before reading beyond the limit or allocating a DOM. Scan original bytes with a fixed512-entry container stack, quote/escape state and checked aggregate counters. Count scalar values and object keys/members as well as opening containers; counting only { and [ misses the flat-zero falsifier. Bound decoded string storage and token/entry counts across the entire document, including rejected/duplicate payloads. Raw string-span bytes can be charged conservatively without implementing a second Unicode decoder: valid JSON escapes shrink or preserve the byte bound when decoded into UTF-8. A new aggregate JSON ceiling needs an explicit profile and measurement; do not silently reuse the static analysis1,000,000-event number as if it already measured decoding.

2. Avoid the full current-schema header DOM. A small version probe can deserialize only schema_version while ignoring other fields, then dispatch the original bounded bytes to the correct closed typed reader. Installed serde_json1.0.151 de.rs:1102 skips ignored values iteratively, using its scratch stack; preflight still bounds that stack and skipped bytes. Keep old-schema field-rejection/migration behavior intact. RawValue can preserve original subranges if needed, but neither it nor IgnoredAny is a resource-admission policy.

3. Place guarded disable_recursion_limit() and serde_stacker::Deserializer in the private codec, not scattered callers. Use bounded Serde visitors/seeds for Expr and its collections where necessary to reject depth129, oversized lists/maps and aggregate node allocation before constructing their children. Reuse the established tagged schema, duplicate UUID checks and Exact canonical decoder; do not make a permissive parallel Expr grammar or a Value-to-Expr normalization path. Migration/header/commands/source-bundle paths must retain their own applicable limits.

4. Close lifecycle handling before exposing newly accepted deep values. Prefer iterative structural/reference walks and bounded source/format traversal. Use the Serde serializer adapter plus a capped output writer at controlled JSON output boundaries; a Vec length check after serialization is too late. Audit direct to_value, generic response serialization, history/command clones and error formatting, not only save_checked. Decide how accepted and partially decoded recursive owners are safely destroyed. If boxed Expr's ordinary lifecycle cannot pass the target stack gate, implement iterative ownership/lifecycle operations or a flat internal arena while keeping the wire schema; a recursive custom JSON decoder does not solve that problem.

5. Keep the current conservative rejection until root has separate-process negative tests and measured valid-boundary lifecycle tests on actual Windows and Linux targets, debug and release. Include a small-stack test comparable to the existing128KiB declaration probe, max-depth Unary/Binary/Call combinations, malformed/duplicate late failure, depth/width just over each ceiling, escaped strings, and capped output failure preserving the existing destination. Record compile profile, effective budgets, peak memory, exit status and exact round-trip bytes. A small-stack pass is target/build evidence, not a language-level proof for arbitrary Rust callers or unsupported platforms.

Comparison and upstream evidence

Approach	Decision and reason
Existing default rejection	Use now. Preserves an established recursion guard without a new dependency or lifecycle claim. It intentionally leaves some depth128 inputs unsupported and cannot be advertised as full schema8 transport completion. It does not repair the existing shallow-header allocation issue.
Raw depth512 + stacker only	Reject as sufficient. Solves recursive Serde stack growth, but not aggregate allocations, duplicate normalization, output expansion or recursive teardown/clone/debug.
Bounded preflight + typed Serde codec + lifecycle gates	Preferred complete route. Retains the actual schema and maintained parsing semantics, adds the missing independent resource boundaries, and is narrower than replacing JSON/language parsing. Enable deeper input only when all lifecycle gates pass.
Custom recursive decoder	Do not choose. Reimplements semantics while retaining the stack/Drop problem.
Fully iterative decoder with flat owned AST	Fallback for unmet lifecycle guarantees. Can make stack use explicit throughout, but the benefit requires flat/iterative ownership, cloning and serialization too; merely building a recursive Value/Expr iteratively is insufficient.

The maintained serde_stacker0.1.14 documentation explicitly separates its Serde adapters from other recursive operations and demonstrates iterative destruction even after protected parsing. Its Deserializer API documents stack checks per nesting level and defaults of a64KiB red zone and2MiB growth segment. These are stack-growth settings, not total-memory or work limits.

serde_json1.0.151's recursion-limit API requires the unbounded_depth feature and warns about later recursive operations; enabling the feature does not itself disable every deserializer's default guard. RawValue preserves a JSON byte range and can avoid a subtree DOM, but still requires correct admission and eventual typed decoding.

stacker's official repository documents platform-dependent implementation, Windows Fibers, and no stack growth on unsupported platforms. Consequently a successful dependency build alone is not proof of effective protection on every release target. The stacker API describes explicitly instrumented growth points; a single call around deserialization cannot protect unrelated later recursive code.

No decoder safety, memory ceiling, complete depth128 round-trip, or dependency/platform execution acceptance is claimed by this report. Root received the early staging decision and the measured output-expansion counterexample; implementation and dependency admission remain separate owned work.