SESSION-EMPTY-OBJECT: refuse {} in the JSON preflight so a malformed document cannot abort the host - #390
Open
tamashi095 wants to merge 4 commits into
Open
SESSION-EMPTY-OBJECT: refuse {} in the JSON preflight so a malformed document cannot abort the host#390tamashi095 wants to merge 4 commits into
tamashi095 wants to merge 4 commits into
Conversation
…document cannot abort the host
json-syntax 0.12.5 (and jstrict 0.14.0 before it) never calls `end_fragment` for an empty JSON
object, so the reserved `CodeMap` entry keeps `volume = 0` and `IterMapped::next` reads every later
member one slot off. `session::parse::Parser::keys` hits `Option::unwrap()` on the resulting `None`
(`json-syntax-0.12.5/src/object/mod.rs:795:67`), reachable from untrusted input through the C API
(`miso_engine_v1_compile_session`) and the wasm boot path (`miso_engine_web_v1_boot`). The
workspace's release profile is `panic = "abort"`, so this aborts the host process / traps the
AudioWorklet rather than returning a diagnostic.
No V1 schema position ever admits `{}` (every object rejects unknown keys and requires every field
explicit; empty arrays are legal, empty objects never are). This implements option 1 from #387: a
preflight refusal in `crates/session/src/json_preflight.rs` that rejects an empty object anywhere
in the document, before the typed walk and before the dependency's `Value` tree is built, with a
proper source span and `DiagnosticCode::JsonSyntax` -- the same code the existing duplicate-key and
depth-129 preflight refusals already use, since a `json.syntax` diagnostic is documented (see
`tools/session-validator/src/lib.rs`) to be produced only by the grammar stage and returned alone.
No new diagnostic code was needed. Factored the three refusal sites' path materialization into one
`Scanner::current_path` helper.
Options 2 (trusting `volume == 0` in the typed walk) and 3 (patching json-syntax upstream) are not
done here, per the issue's ordering; upstream still has the bug.
Tests:
- `crates/session/tests/json_grammar.rs`: every `{}` placement in `canonical.json`
(`render_profile`, `output_profile`, a track's `builtins`) refuses with an exact span and path;
`canonical-minimal.json` with an empty `render_profile` reports `json.syntax` at `$.render_profile`
instead of the previously bogus `numeric.out_of_schema_range` at `$.output_profile.channels` with
degenerate `151..151` spans; a document content-equivalent to the fuzz crash the #385 verifier
found (`crash-52d9c906ce5ad7f1d1e67dad91b13ec69e2caab5`, not present on disk in this worktree) no
longer panics; the minimal `{"a":{},"b":1}` CodeMap illustration from the issue.
- `crates/capi/src/ffi.rs` (`ffi::tests::empty_object_document_is_diagnosed_not_aborted`):
`miso_engine_v1_compile_session` on the nine-track fixture with an empty `render_profile` returns
`RESULT_COMPILE_REJECTED` with `json.syntax\t$.render_profile\n`, not an abort.
- `hosts/host-web/src/tests.rs` (`tests::empty_object_document_boots_to_a_diagnostic_not_a_trap`):
`AudioWorkletEngineHost::boot` and the raw `miso_engine_web_v1_boot` C ABI entry both return
`RESULT_REFUSED_DOCUMENT` with the same diagnostic instead of trapping.
Re-pinned the AudioWorklet artifact (`crates/session` ships in the wasm build):
`22e4c25cba7f97b66db720ad8ac8cf653de0afcabe84101693f4fa166b90d4e6` ->
`439ba6b513c5b1c9c2d1ff462d5cdd43136b0d70df540d4a21c238886d165958`
(`MISO_ENGINE_WEB_AUDIOWORKLET_REPIN=1 bash scripts/build-web-audioworklet.sh <dir>`).
Commands run:
- cargo test --locked -p session
- cargo test --locked -p capi
- cargo test --locked -p host-web
- bash scripts/check-session-policy.sh
- bash scripts/check-realtime-policy.sh
- bash scripts/check-workspace-policy.sh
- bash scripts/build-web-audioworklet.sh <dir>
- bash scripts/check-web-audioworklet.sh <dir>
- python3 scripts/check-browser-expected-resources.py
- cargo clippy --locked -p session -p capi -p host-web --all-targets -- -D warnings
- cargo fmt --all -- --check
- git diff --check
Refs #387
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EwL1uTcxsmopHtamG6bMko
- npm run qualify -- --artifacts <dir> --browser all --record-matrix --candidate-commit 90109f6 --self-test-mutations: chromium 151.0.7922.34, firefox 153.0, webkit 26.5 all passed. - npm run qualify -- --artifacts <dir> --browser all --check-matrix --self-test-mutations: PASS against the recorded results.json. candidateCommit fc01c53... -> 90109f6... (this branch's #387 fix commit); wasmSha256 22e4c25c... -> 439ba6b5... (matches the re-pinned artifact digest recorded in the parent commit). Refs #387 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EwL1uTcxsmopHtamG6bMko
…comment, and seed the CI fuzz corpus Verifier feedback on PR #390 (REQUEST CHANGES, documentation and PR-truthfulness only; the fix itself was verified correct: 186 {} placements, 48 scanner attacks, fuzz clean, all CI job commands green locally): 1. docs/SESSION_SCHEMA_V1.md: added the empty-object refusal to the preflight paragraph that already documents the duplicate-key and depth-129 refusals -- code, path/span shape, and the json-syntax 0.12.5 CodeMap defect it works around (issue #387). Empty arrays are called out as unaffected. 2. crates/session/src/json_preflight.rs:31-32: the comment claimed this pass "only returns the two refusals for which the value parser does not expose the required contract information." That was already inaccurate for the empty-object refusal added in the parent commit: the value parser normally *would* diagnose an empty object correctly (missing schema fields etc.); this preflight refuses it first specifically to keep the corrupted json-syntax CodeMap state (no `end_fragment` call on the empty-object branch) from ever being reached. Rewritten to name all three refusals and state that reason accurately. 3. .github/workflows/fuzz.yml:117 / fixtures/session/v1/fuzz-seeds/empty-object.json: the issue asked to add `{}` to the session_parse fuzz corpus seeds. Added a small committed fixture (canonical.json with render_profile set to {}) and appended it to the session_parse job's `-seed_inputs=` list (libFuzzer accepts a comma-separated list). Verified directly: running the compiled session_parse fuzz binary once against this exact seed on this branch executes cleanly (no crash); the identical byte content was already proven to panic session::parse::Parser::keys on the unmodified pre-#387 tree (session-level repro captured before implementing the fix, and independently by the #385 verifier's own fuzzer, which is how #387 was found in the first place) -- so this seed would crash main's session_parse target on its first execution. Re-pin note (not requested, but required): editing json_preflight.rs's comment shifts every subsequent line number in that file, including its one slicing panic site (`self.source[start..self.cursor]` in `Scanner::string`). Rust embeds a `core::panic::Location` (file/line/column) constant at that call site regardless of the release profile's `-C strip=debuginfo` (that strip level removes DWARF sections, not these compiled-in panic-location strings), so a comment-only edit to a shipped source file still moves the wasm artifact's bytes. Confirmed by rebuilding: digest changed 439ba6b5... -> f1f27d8d... at the identical byte count (2,635,719), i.e. exactly a debug-metadata-only delta, not a behavior change. Re-pinned the artifact and regenerated the qualification lineage against this commit's own hash, same as the prior two commits did. Per the coordinator's explicit instruction, crates/capi/tests/resource_lifecycle.rs is untouched; its pre-existing, already-diagnosed-separately failure on main since #385 is out of scope here. Commands run: - cargo test --locked -p session - bash scripts/check-session-policy.sh - RUSTDOCFLAGS="-D warnings" cargo doc --locked -p session -p capi -p host-web --no-deps - cargo fmt --all -- --check - git diff --check - cargo clippy --locked -p session -p capi -p host-web --all-targets -- -D warnings - bash scripts/build-web-audioworklet.sh <dir> (re-pin, then normal build against the new pin) - bash scripts/check-web-audioworklet.sh <dir> - python3 scripts/check-browser-expected-resources.py - ./target/x86_64-unknown-linux-gnu/release/session_parse fixtures/session/v1/fuzz-seeds/empty-object.json (single-input run, no crash) Refs #387 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EwL1uTcxsmopHtamG6bMko
… commit The prior commit only edits comments/docs/a fuzz fixture/a workflow file, but one of those edits (crates/session/src/json_preflight.rs's comment) is inside a real shipped source file. Shifting its line numbers moved the compiled `core::panic::Location` constant for `Scanner::string`'s one slicing panic site, which changed the wasm artifact's bytes even though no behavior changed (2,635,719 bytes before and after; only the digest moved). See that commit's message for the full explanation. - MISO_ENGINE_WEB_AUDIOWORKLET_REPIN=1 bash scripts/build-web-audioworklet.sh <dir>: produced f1f27d8daa4c9f0828c9a3b2ca939bfdb507d041db6d7bc2a63ec6c826c1f74d (was 439ba6b513c5b1c9c2d1ff462d5cdd43136b0d70df540d4a21c238886d165958). - bash scripts/build-web-audioworklet.sh <dir> (normal build against the new pin): PASS, same 2,635,719 bytes. - bash scripts/check-web-audioworklet.sh <dir>: PASS. - python3 scripts/check-browser-expected-resources.py: PASS. - npm run qualify -- --artifacts <dir> --browser all --record-matrix --candidate-commit 1297393 --self-test-mutations: chromium 151.0.7922.34, firefox 153.0, webkit 26.5 all passed. - npm run qualify -- --artifacts <dir> --browser all --check-matrix --self-test-mutations: PASS against the recorded results.json. Refs #387 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EwL1uTcxsmopHtamG6bMko
tamashi095
added a commit
that referenced
this pull request
Sep 4, 2026
…acle arms (#392) ## Summary `crates/capi/tests/resource_lifecycle.rs`'s `exported_c_candidates_replay_render_and_both_destroy_orders_balance_exactly` asserts an exact allocation-call count against an exact deallocation-call count inside an armed window. After #385 replaced `jstrict` with upstream `json-syntax 0.12.5`, every object parse indexes through `hashbrown` 0.12's `DefaultHashBuilder`, which is `ahash` 0.7's `RandomState`. Its **first** construction in a process boxes three `once_cell::race::OnceBox` statics (`RAND_SOURCE`, its inner `Box<dyn RandomSource>`, and the `SEEDS` array: 8 + 16 + 64 = 88 bytes) that live until process exit and belong to no capi owner. `ahash`'s `build.rs` forces the `runtime-rng` feature on every hosted target, so no Cargo feature removes it. Those 88 bytes land on whichever thread parses the first `json-syntax` object in the process. Under the parallel test harness that's a race between this file's three tests; on the CI runner (4 vCPU) `exported_c_candidates_replay_render_and_both_destroy_orders_balance_exactly` loses the race and gets charged for an allocation with no matching deallocation inside its own window (`left: 2613 right: 2610`, deterministic with `RUST_TEST_THREADS=1`). **Fix (test-only):** parse a trivial JSON object once, before `begin()` arms the allocator oracle, so the lazy `ahash` statics are always initialized before any window starts observing. Uses `{"warm":0}`, not `{}` — PR #390 (landing soon) makes the preflight refuse an empty object before the parser runs, which would defeat the warm-up; `{"warm":0}` reaches `json_syntax::Value::parse_str` and constructs the hasher regardless of what the preflight or schema does with it afterward. **Shipped caps are unaffected.** They are explicit row sums computed at compile time (`crates/capi/src/runtime/compile.rs:185`), not derived from any allocator oracle, and this PR touches no shipped file — confirmed by rebuilding the AudioWorklet artifact and diffing its digest against the committed pin (unchanged, see Gate output). Also adds one sentence to `docs/REALTIME_DEPENDENCY_POLICY.md`'s #385 session-parser entry recording this lazy-allocation behavior, so a future reader of that policy (or a future allocator oracle) isn't surprised by it again.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
crates/session/src/json_preflight.rs: reject an empty JSON object{}anywhere in the document, before the typed walk and beforejson-syntax'sValuetree is built, with a proper source span,DiagnosticCode::JsonSyntax, and message"empty JSON object {} is never a valid V1 schema value". This is option 1 from session aborts the host on untrusted JSON containing an empty object {} (parser CodeMap volume bug, reachable from C API and wasm boot) #387.{}is never legal V1:docs/SESSION_SCHEMA_V1.md-- "Every object rejects unknown keys and every field is explicit, including empty arrays"; every schema object has required keys, so an object with none can never validate. Empty arrays stay legal and untouched (array()is unmodified).json-syntax0.12.5 never callsparser.end_fragment(i)for an empty object, so the reservedCodeMapentry keepsspan = p..p, volume = 0.IterMapped::nextthen advances2 + volumeinstead of2 + 1, so every object member after an empty object is read oneCodeMapslot off.session::parse::Parser::keys(crates/session/src/parse.rs:100) hitsOption::unwrap()on the resultingNoneatjson-syntax-0.12.5/src/object/mod.rs:795:67. The workspace's release profile ispanic = "abort"(Cargo.toml:132), so this aborts the host process / traps the AudioWorklet rather than returning a diagnostic, for any document reachable throughmiso_engine_v1_compile_sessionormiso_engine_web_v1_boot.DiagnosticCode::JsonSyntax("json.syntax") rather than adding a new code: it is exactly the code the preflight's other two refusals (duplicate key, depth-129) already use, andtools/session-validator/src/lib.rsdocuments (and tests) thatjson.syntaxis produced only by the grammar stage and is always returned alone -- our new refusal preserves that invariant (it also short-circuits the whole document, same as the other two). No conformance fixture or diagnostic-code enumeration needed updating (there is no registry doc or enumerated-code list beyond theDiagnosticCodeenum itself anddocs/SESSION_SCHEMA_V1.md's prose) -- butdocs/SESSION_SCHEMA_V1.md's own prose describing the preflight did need updating, and initially wasn't: see "Documentation and comment corrections" below, added after verifier review.Scanner::current_pathhelper in the same file, to avoid a third copy.crates/sessionships in the wasm build) and regenerated the qualification lineage, exactly as JSON-SYNTAX: replace the jstrict fork with upstream json-syntax in crates/session #385 did, against the latestorigin/main(65d83af5, includes TOOLING-DEDUPE: remove the private SHA-256 and the incomplete JSON escaper, fold duplicated helpers into bench-support #386) -- twice (see "Documentation and comment corrections").Documentation and comment corrections (verifier round 1: REQUEST CHANGES)
The fix itself was verified correct (186
{}placements, 48 scanner attacks, fuzz clean, all CI job commands green locally); the verifier's findings were documentation and PR-truthfulness only, addressed in two follow-up commits:docs/SESSION_SCHEMA_V1.mddid not mention the empty-object refusal at all, even though its opening paragraph already documents the sibling duplicate-key and depth-129 preflight refusals. Added two sentences there naming the code (json.syntax), the path/span shape (over the value's{...}bytes), the json-syntax 0.12.5CodeMapdefect it works around, and that empty arrays are unaffected.crates/session/src/json_preflight.rs:31-32's comment said this pass "only returns the two refusals for which the value parser does not expose the required contract information" -- stale from before the empty-object refusal existed, and actually wrong for it: the value parser would diagnose an empty object correctly on its own (missing schema fields etc.); the refusal exists to keep the corruptedCodeMapstate from ever being reached, not because of a contract-information gap. Rewritten to name all three refusals accurately.{}to thesession_parsefuzz corpus seeds; the original PR did not wire this into CI. Addedfixtures/session/v1/fuzz-seeds/empty-object.json(canonical.jsonwithrender_profileset to{}) and appended it to.github/workflows/fuzz.yml'ssession_parsejob-seed_inputs=list (libFuzzer accepts a comma-separated list). Verified directly: running the compiledsession_parsefuzz binary once against this exact seed on this branch executes cleanly with no crash; the identical byte content already reproduced the pre-fix panic in this PR's own session-level repro (captured before implementing the fix) and is how the JSON-SYNTAX: replace the jstrict fork with upstream json-syntax in crates/session #385 verifier found session aborts the host on untrusted JSON containing an empty object {} (parser CodeMap volume bug, reachable from C API and wasm boot) #387 in the first place -- so this seed would crashmain'ssession_parsetarget on its first execution, and CI will now catch a regression here going forward.json-syntaxfix is "in addition to 1, not instead" -- i.e. still owed, not forbidden. My original PR body incorrectly framed it as if the issue itself excluded it; that exclusion was actually my own task scope for this PR, not the issue's position. Corrected below; the coordinator will open the follow-up issue for the upstream PR +[patch].json_preflight.rs's comment (item 2) shifts every subsequent line number in that real, shipped source file, including its one slicing panic site (self.source[start..self.cursor]inScanner::string). Rust embeds acore::panic::Location(file/line/column) constant at that call site regardless of the release profile's-C strip=debuginfo(that strip level removes DWARF sections, not these compiled-in panic-location strings), so a comment-only edit to a shipped file still moves the wasm artifact's bytes. Confirmed by rebuilding: digest changed439ba6b5...->f1f27d8d...at the identical byte count (2,635,719) -- a debug-metadata-only delta, not a behavior change -- so the artifact was re-pinned and the qualification lineage regenerated a second time, against this branch's actual final commits.crates/capi/tests/resource_lifecycle.rsis untouched.exported_c_candidates_replay_render_and_both_destroy_orders_balance_exactly(left 2613, right 2610) already fails onmainitself since JSON-SYNTAX: replace the jstrict fork with upstream json-syntax in crates/session #385 and is being diagnosed separately; it is unrelated to this PR and out of scope here.Rows closed (#387)
{}placement now produce a diagnostic, not a panic, throughparse_session_json,miso_engine_v1_compile_session, andmiso_engine_web_v1_boot. See Before/after.cargo test --locked --workspacecount matchesmainplus the new regression tests (see Gate output / workspace counts below).session_parseran 15 minutes with{}seeded: no crashes (17,652,390 executions), and{}is now a permanent CI seed (fuzz.yml's-seed_inputs=), not just a one-off local run.Gate output
Original round, rerun fresh on
816957af(rebased ontoorigin/main65d83af5):Verifier-requested round, rerun fresh on the final head (
a52bdae0):Wasm/qualification, both rounds (
<dir>rebuilt fresh each time):Fuzz (
cargo-fuzz 0.13.2installed;+nightly-2026-08-20toolchain present):17,652,390 executions, 0 crashes, 0 new artifacts (
fuzz/artifacts/session_parse/empty). ~19,600 exec/s sustained. The general mutation corpus directory (fuzz/corpus/session_parse/, 2,150 files after the run) is not committed, matching this workspace's existing convention for this target (only the curated protocol golden seeds underfuzz/corpus/protocol_*are checked in). What is now committed and wired into CI is the one deliberate{}seed (fixtures/session/v1/fuzz-seeds/empty-object.json), appended tofuzz.yml's-seed_inputs=forsession_parse.Workspace-wide counts (unchanged by the verifier-requested round -- no test-affecting files touched, only comments/docs/a fixture/a workflow/the pin):
Before/after
Session-level (
{"render_profile": {}}substituted intofixtures/session/v1/canonical.json, unmodified pre-fix tree):After:
canonical-minimal.json(before, unmodified tree -- the bogus diagnostics the issue named):After:
(one clear diagnostic at the real fault, not a bogus range error plus two degenerate zero-width spans)
C API (
miso_engine_v1_compile_session, nine-track fixture,render_profileemptied):compile_children->parse_host_session->parse_session_json;catch_result'scatch_unwindcannot contain an abort underpanic = "abort").RESULT_COMPILE_REJECTED, diagnostics bytesjson.syntax\t$.render_profile\n. Verified inffi::tests::empty_object_document_is_diagnosed_not_aborted.Wasm boot path (
AudioWorkletEngineHost::boot/miso_engine_web_v1_boot, one-track fixture,render_profileemptied):--release(panic = "abort") build.RESULT_REFUSED_DOCUMENT, diagnosticjson.syntax\t$.render_profile\n, through both the safeAudioWorkletEngineHost::bootcall and the raw C ABImiso_engine_web_v1_bootentry point. Verified intests::empty_object_document_boots_to_a_diagnostic_not_a_trap.Wasm artifact digest and size (
scripts/build-web-audioworklet.sh,miso-engine-v1-audio-worklet.simd128.wasm):origin/main65d83af5, pre-#387)22e4c25cba7f97b66db720ad8ac8cf653de0afcabe84101693f4fa166b90d4e6816957af)439ba6b513c5b1c9c2d1ff462d5cdd43136b0d70df540d4a21c238886d165958a52bdae0)f1f27d8daa4c9f0828c9a3b2ca939bfdb507d041db6d7bc2a63ec6c826c1f74dRound 1: -457 bytes from the new preflight branch (a handful of straight-line instructions, smaller than whatever code-size noise the change otherwise perturbed in the fat-LTO build). Round 2: 0 byte-count change, digest-only -- see "Documentation and comment corrections" item 5 for why a comment edit still moves a compiled panic-location constant.
Seen, not done
json-syntaxstill has the bug (src/parse/object.rs:26-29never callsend_fragmentfor the empty-object branch; the empty-array branch does). No upstream release exists to bump to. Per session aborts the host on untrusted JSON containing an empty object {} (parser CodeMap volume bug, reachable from C API and wasm boot) #387 itself, the upstream PR + a[patch](option 3) is "in addition to 1, not instead" -- i.e. still owed, not excluded by the issue. It is not done in this PR because the coordinator scoped this PR to option 1 only and will open the follow-up issue for it. The bug is fully neutralized for this workspace by the preflight refusal in the meantime, since{}can never reach the parser'sValuetree fromparse_session_jsonany more, but a future direct consumer ofjson_syntax::Value::parse_stroutsidesession's preflight boundary would still hit it.volume == 0as 1 in the typed walk) not implemented, per this PR's scope (not touchingparse.rs's typed walk).crash-52d9c906ce5ad7f1d1e67dad91b13ec69e2caab5named in the issue was not present on disk in this worktree (checkedfuzz/artifacts/, the scratch directories named in the task, and did a filesystem-wide search). A content-equivalent input was found at the JSON-SYNTAX: replace the jstrict fork with upstream json-syntax in crates/session #385 verifier's own scratch path (render_profilereduced to whitespace-only content, still an empty object under the JSON grammar) and is used verbatim as thefuzz_crash_52d9c906_empty_render_profile_diagnoses_instead_of_abortingregression test and as the committed CI fuzz seed; it reproduces the identical pre-fix panic (verified on the unmodified tree before implementing the fix).session,capi, andhost-webare control-plane-only, and this PR touches no render arithmetic (docs/REALTIME_DEPENDENCY_POLICY.md's "Issue 004 control-plane parser dependencies" section already establishessessionis not render-reachable).crates/capi/tests/resource_lifecycle.rs'sexported_c_candidates_replay_render_and_both_destroy_orders_balance_exactlyfailure (left 2613, right 2610) predates this PR (present onmainsince JSON-SYNTAX: replace the jstrict fork with upstream json-syntax in crates/session #385) and is out of scope here per the coordinator's explicit instruction; it is being diagnosed separately.Skipped
Closes #387
🤖 Generated with Claude Code
https://claude.ai/code/session_01EwL1uTcxsmopHtamG6bMko
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.