fix(core): bound frontmatter YAML parsing — 1 MiB cap, 200k node budget, 1024 flow-depth guard, string-API MAX_FILE_SIZE; lint/scan_imports propagate resource_limit (#162) - #384
Merged
Conversation
…elled parse, lenient sites propagate resource_limit (#162) Adds the failing bounds tests for frontmatter YAML DoS hardening ahead of the fix. A stub `parse_frontmatter_yaml` (unbounded `from_str`) is present only so the test crate links and the behavioural assertions fail rather than the build. RED (13 behavioural failures observed, cargo nextest -p mds-core): - frontmatter_tests::budget_scalar_accounting / budget_sequence_accounting / budget_keys_are_charged / budget_tagged_accounting — node budget not enforced. - frontmatter_tests::c2_size_cap_exact_boundary / c3_size_cap_precedes_yaml_parse — 1 MiB size cap absent. - frontmatter_tests::c4_c5_node_budget_real_boundary / c6_alias_revisits_are_counted / c8_merge_key_is_a_plain_key_and_bomb_is_budgeted — 200k node budget absent. - lint::facts::tests::collect_facts_propagates_resource_limit — swallow site does not propagate ResourceLimit. - tests::scan_imports_frontmatter_bomb_propagates_resource_limit — scan_imports swallows ResourceLimit. - yaml_funnel::yaml_parse_sites_are_funnelled — parse sites not yet funnelled. - api_surface::string_funnels_reject_oversize_source — MAX_FILE_SIZE backstop absent at the string funnels. Passing regression pins: c_par_* parity, depth pins (c10*), c11 billion-laughs, c1/at-cap Ok twins, T0 constants.
… MAX_FILE_SIZE at the string funnels; lint facts and scan_imports propagate resource_limit (#162) Frontmatter YAML was parsed by four unbounded `serde_yaml_ng::from_str` sites. An `&anchor` referenced by many `*alias`es expands at deserialise time, so a sub-megabyte block could materialise multiple GiB — serde_yaml_ng's own alias-jump limit does not catch a single large anchor referenced a few thousand times. Fix (all four sites now funnel through resolver::frontmatter::parse_frontmatter_yaml): - 1 MiB size cap (MAX_FRONTMATTER_SIZE) checked before the eager loader runs. - 200k node budget (MAX_FRONTMATTER_NODES) charged by a DeserializeSeed that mirrors serde_yaml_ng's `impl Deserialize for Value`, counting each scalar, sequence, mapping, mapping key and `!tag` wrapper. Our bounds surface as mds::resource_limit; everything serde_yaml_ng rejects stays mds::yaml with a byte-identical message (same Deserializer drives both). Duplicate-key detection reproduces serde_yaml_ng's private DuplicateKeyError verbatim. - Core MAX_FILE_SIZE backstop (check_source_size) at resolve_source / resolve_source_intrinsic / resolve_source_intrinsic_opts, since compile_str / check_str / lint_str_with / opts-compile bypass the binding-side guards. - lint::facts::collect_frontmatter_vars and lib::scan_imports now PROPAGATE a ResourceLimit (fail closed) while still swallowing plain YAML syntax errors. SIDE EFFECT for CHANGELOG: scan_imports on a file with >256 frontmatter imports now returns Err(ResourceLimit) instead of silently returning only body imports. Seed shape: DeserializeSeed driving serde_yaml_ng::Deserializer::from_str (the primary design; no fallback needed). Mutation controls (scratch copy, reverted; observed-RED): - M1 size guard disabled -> c2_size_cap_exact_boundary RED. - M5 classify arm (budget trip -> yaml) -> c4_c5_node_budget_real_boundary RED. - M6 facts swallow restored -> collect_facts_propagates_resource_limit RED. - M7 scan_imports swallow restored -> scan_imports_frontmatter_bomb RED. Accounting mutations M2/M3/M4/M11 are caught by construction: budget_*_accounting assert BOTH the at-cap Ok and the one-node-over is_rl, so any missing/extra charge shifts the exact boundary and fails. M8 (backstop) is pinned by string_funnels_reject_oversize_source + its at-cap Ok twin; M9 by t0_constants; M10 (dup) by c_par parity; M12 by scan_imports bomb; M13 (build_scope revert) is weak — yaml_funnel is the real enforcer; M14 (empty-tag guard) is defence-in-depth, not reachable via libyaml, no test claimed. Gates: cargo fmt clean; clippy -D warnings clean; nextest -p mds-core 1438 passed (was 1406); cargo test --doc clean; rustdoc -D warnings clean. Step F (release, /usr/bin/time -l, 10s alarm): a deep flow-nest whose frontmatter is just UNDER 1 MiB (1,040,005 bytes) still HANGS — 10.01s real / 9.53s user CPU, RSS 32 MB — a CPU-bound hang in libyaml's flow scanner UPSTREAM of the node budget. Just OVER 1 MiB (1,060,005 bytes) is rejected instantly (0.01s, exit 3, "frontmatter too large"). OPEN FINDING for the orchestrator: 1 MiB is not tight enough for the deep-flow-nest CPU-scan class; not changed here (out of scope).
Add MAX_FRONTMATTER_FLOW_DEPTH (1024) and C-12 tests for a pre-parse guard that bounds flow-collection nesting depth before libyaml's O(depth^2) flow scanner runs. Reconcile C-10g: a 10000-deep flow nest now trips the guard (resource limit) instead of serde's recursion limit. RED: c12a (over guard -> resource_limit) and c10g (reconciled) fail with serde's "recursion limit exceeded"; positive controls c12/c12b/c12c pass. #162
…er to close the deep-nest CPU DoS (#162) libyaml's flow scanner is O(depth^2) in flow-collection nesting and runs upstream of deserialisation, so a ~1 MiB pure deep flow-nest (no anchors) burns 10+ s of CPU at ~32 MB RSS before the node budget or any depth limit fires. Add check_flow_nesting_depth: a single O(n) byte pass over the raw frontmatter, after the 1 MiB size cap and before the budgeted parse, that rejects when running flow-collection nesting depth exceeds MAX_FRONTMATTER_FLOW_DEPTH (1024) with mds::resource_limit. Counts NET depth so wide-but-shallow flow lists stay legal; threshold sits above serde's 128-frame recursion limit so shallower inputs keep their existing errors. #162
… core, CLI, napi, wasm and Python (#162)
… spec §4.1/§7.9 and SECURITY.md; CHANGELOG Security entry; compile_str rustdoc (#162)
…ion (#162) watch_debounce_cap_rebuilds_while_writes_never_stop asserts a HARNESS precondition — the writer thread must keep inter-write gaps under the 200ms quiet-period window so a rebuild seen while writing provably comes from the cap, not a quiet period. On a loaded CI runner the writer thread was itself descheduled for 747ms, tripping that precondition and reddening PR #384's CI (a scheduling hiccup, not a product failure; C3/#162 touches no watch code). Wrap the measurement in a bounded (4-attempt) retry gated ONLY on that precondition: an inconclusive sample (max gap >= window) is discarded and retried, while every behaviour assertion (rebuilt-while-writing, rebuild count 1..=4) still fails hard on the first attempt, so a real regression is never retried away.
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
This is the first explicitly security-scoped step since A1: it bounds frontmatter YAML parsing so adversarial frontmatter can no longer exhaust the host through the YAML parser — an abort that the JS/Python FFI boundary cannot catch. Both DoS axes of #162 are closed at a single core choke point,
parse_frontmatter_yaml(crates/mds-core/src/resolver/frontmatter.rs), before libyaml materialises or deep-scans anything. The Snyk MCP server failed to connect this session (ENOENT, missing macos-arm64 wrapper), so asnyk_code_scancould not run; thesecurity/snyk (dean0x)check is SCA-only and does not scan Rust source (per project history), so it would not have exercised this change regardless.What was wrong
The issue's original premise ("stack overflow") was incorrect — libyaml is iterative, so the failure mode is OOM / CPU hang, not stack overflow. There are two independent axes:
&anchorreferenced by many*aliases expands during deserialization. A ~70 KB bomb otherwise allocates ~8.3 GiB and completes. serde_yaml_ng's own repetition limit (jumpcount > events × 100) counts alias jumps, not materialised nodes, and provably does not fire for this shape.MAX_VALUE_DEPTH64) are post-hoc — they fire only after the quadratic scan has already been paid.Two further gaps: the eager loader plus
compile_str/check_str/lint_str_withwere uncapped on the Rust API (the bindings and CLI already capped file size — the PF-004 gap), and two lenient call sites (lint::facts::collect_frontmatter_vars,lib::scan_imports) swallowed the resulting resource-limit errors.Design
All bounds sit at the one choke point
parse_frontmatter_yaml, applied before libyaml materialises anything:MAX_FRONTMATTER_SIZE) →mds::resource_limit"frontmatter too large". Bounds the eager loader.MAX_FRONTMATTER_NODES) via aBoundedYamlDeserializeSeed→mds::resource_limit"frontmatter YAML node count exceeds maximum of 200000". Counting happens during parsing so alias expansion stops before the tree is built — this closes the alias-bomb memory axis. The 200,000 figure is generous headroom over any legitimate frontmatter (a legit ~1 MiB document ofkey: valuelines is ~41,600 nodes) while capping a bomb's retained tree to tens of MB.MAX_FRONTMATTER_FLOW_DEPTH) — a pre-parse O(n) raw-byte scan →mds::resource_limit"frontmatter YAML flow nesting exceeds maximum depth of 1024". Checked before the parser scans, because the parser's own depth error is only reported after the O(depth²) scan is paid. 1024 is 8× serde's own 128-frame recursion limit and ~10× any real flow nesting, so it caps the worst admitted scan at ~1024² (trivially fast) while sitting above serde's 128 limit — inputs shallower than 128 keep their existingmds::yamldepth-error behaviour unchanged.resolve_source/_intrinsic/_opts) →mds::resource_limit"file too large". Closes the gap wherecompile_str/check_str/lint_str_withbypassed the binding-side caps.collect_frontmatter_varsandscan_importsnow return the resource-limit error (still lenient for plain YAML syntax errors).Honest plan deviation: the original plan set "depth = tests only" on the premise that the 1 MiB size cap bounds the flow scan. The D0/step-F measurement refuted that premise — a just-under-1-MiB deep flow-nest still hung ~10 s at 32 MB RSS (the node budget never sees it; the byte cap admits it). The user then approved adding the pre-parse flow-depth guard (item 3) so both halves of #162 are actually closed.
Error codes:
mds::resource_limit= exit 3 (our bounds);mds::yaml= exit 1 (everything serde_yaml_ng itself rejects — syntax, duplicate keys, its recursion/repetition/value-depth messages — byte-identical).<<remains a literal key (no merge applied).Evidence
D0 measurement @c4f5880 (pre-fix): the ~70 KB alias bomb allocates ~8.3 GiB and completes; a ~2 MB deep flow-nest hangs 10 s in libyaml's scanner.
@Head (post-fix): the same ~1.04 MB deep flow-nest goes from 10.01 s → 0.01 s (exit 3, flow-depth message); a legitimate shallow ~1 MiB frontmatter still compiles (exit 0, 0.05 s); bombs reject in ~0.5 s in core.
Per-commit RED → GREEN was followed (RED test commits precede each GREEN fix). Per-surface matrix — each vector rejected at its intended layer across {core, CLI, napi, wasm, Python}:
Gate counts: mds-core 1406 → 1445, mds-cli 834 → 843 (2288 combined); doctests 53; npm workspaces all green incl. mds-napi 115; wasm-pack
--node69 /--release69; pytest 256;npm run test:gates212/0.test_perf4_malformed_input_never_yields_internalstays green with the bomb appended toMALFORMED(yieldsmds::resource_limit, nevermds::internal). Docs commit gates: rustdoc-D warningsclean,cargo test --doc -p mds-core53 passed, control-bytes + version gates clean.Known limitations
lint_str(no-arg) has no core-side 10 MiB check — it runs no resolver pass, socheck_source_sizedoes not cover it. The bindings and CLI cap it; the frontmatter bounds still apply viafacts.rs. Deliberately not adding a fourth funnel.MAX_MODULE_COUNT.depythonizewalk) is a separate binding-side hardening, out of scope.Changes
crates/mds-core/src/resolver/frontmatter.rs—parse_frontmatter_yamlchoke point,NodeBudget/BoundedYamlseed,check_flow_nesting_depth,duplicate_key_message.crates/mds-core/src/limits.rs—MAX_FRONTMATTER_SIZE,MAX_FRONTMATTER_NODES,MAX_FRONTMATTER_FLOW_DEPTH.crates/mds-core/src/resolver.rs—check_source_size+ funnelled parse sites.crates/mds-core/src/lint/facts.rs,crates/mds-core/src/lib.rs— propagateResourceLimit;compile_str/compile_str_with/check_strrustdoc# Limits.frontmatter_tests.rs,tests/yaml_funnel.rs,tests/api_surface.rs, CLItests/security.rs+tests/common/mod.rs, napi__test__/index.spec.mjs, wasmtests/web.rs, Pythontest_limits.py/test_parity.py/test_concurrency.py.spec.md§4.1 + §7.9,SECURITY.mdlimits table,CHANGELOG.md### Security.Related Issues
Closes #162
Refs #134 #161
Refs #383