diff --git a/CHANGELOG.md b/CHANGELOG.md index 7a06b943..c388a150 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,24 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Security + +- **Frontmatter YAML parsing is bounded (#162).** Adversarial frontmatter can no + longer exhaust the host through the YAML parser (an abort the JS/Python FFI + boundary cannot catch). A block over 1 MiB, or one that would expand to more + than 200,000 YAML nodes (an `&anchor` referenced by many `*alias`es), or one + nesting flow collections deeper than 1024 levels, is rejected with + `mds::resource_limit` before the parser materialises or deep-scans it — on the + CLI, the Rust API, napi, wasm and Python. The deep-nesting guard is checked + before the parser runs because the parser's own depth limits are reported only + after an O(depth²) scan is already paid. `mds::compile_str`/`check_str`/`lint_str_with` + now also reject sources over `MAX_FILE_SIZE` (10 MiB) with `mds::resource_limit`; + the bindings and the CLI already did. `mds lint` and `scan_imports` no longer + swallow these errors (and `scan_imports` now reports more than 256 frontmatter + `imports` entries as `mds::resource_limit` instead of silently omitting them); + plain YAML syntax errors in those two paths stay best-effort as before. + Duplicate-key and syntax-error messages are unchanged. + ### Changed - **`mds watch --debounce` is now a quiet period with a hard cap (#379).** diff --git a/SECURITY.md b/SECURITY.md index 1acc5af4..7c2d7a04 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -51,7 +51,11 @@ input. The compiler enforces several defense-in-depth controls: | Limit | Value | Location | |-------|-------|----------| -| Max file size | 10 MB per source file | `limits.rs` (`MAX_FILE_SIZE`) | +| Max file size | 10 MB per source file (file, virtual module, or in-memory string) | `limits.rs` (`MAX_FILE_SIZE`) | +| Max frontmatter size | 1 MiB per block | `limits.rs` (`MAX_FRONTMATTER_SIZE`) | +| Max frontmatter YAML nodes | 200,000 per block (alias expansion counted) | `limits.rs` (`MAX_FRONTMATTER_NODES`) | +| Max frontmatter flow-nesting depth | 1024 (checked pre-parse) | `limits.rs` (`MAX_FRONTMATTER_FLOW_DEPTH`) | +| YAML parser nesting depth | 128 | serde_yaml_ng (reported as `mds::yaml`) | | Max `mds.json` size | 1 MB | `mds-cli/src/main.rs` (`MAX_CONFIG_SIZE`) | | Max call depth | 128 | `evaluator.rs` (`MAX_CALL_DEPTH`) | | Max iterations per loop | 100,000 | `evaluator.rs` (`MAX_LOOP_ITERATIONS`) | diff --git a/crates/mds-cli/tests/cli_watch.rs b/crates/mds-cli/tests/cli_watch.rs index f4c8dd85..cf016283 100644 --- a/crates/mds-cli/tests/cli_watch.rs +++ b/crates/mds-cli/tests/cli_watch.rs @@ -1260,79 +1260,107 @@ fn watch_debounce_single_rebuild_from_burst() { /// since the loop never reaches `TickClock::recv_next` while a window is open. #[test] fn watch_debounce_cap_rebuilds_while_writes_never_stop() { - let dir = tempfile::tempdir().unwrap(); - let src = dir.path().join("hot.mds"); - std::fs::write(&src, "---\nname: v0\n---\nHot {{name}}!\n").unwrap(); - let out = dir.path().join("hot.md"); - - // --debounce 200 => cap = max(10 x 200ms, 1s) = 2s. - let (mut child, stderr_tap) = spawn_ready( - mds_bin() - .args([ - "watch", - src.to_str().unwrap(), - "--debounce", - "200", - "--poll-interval", - "0", - ]) - .stdout(Stdio::null()), - ); + // The writer thread must keep the stream denser than the 200ms quiet-period window, + // so a rebuild seen WHILE writing provably comes from the cap and not from a quiet + // period that ended on its own. That is a HARNESS precondition, not a property of + // the code under test: on a loaded runner the writer thread can itself be + // descheduled past the window (a 747ms inter-write gap was observed on CI), which + // makes the sample inconclusive rather than failing. Retry the whole measurement a + // bounded number of times, gated ONLY on that precondition — every behaviour + // assertion below still fails hard on the first attempt, so a real regression is + // never retried away. + const MAX_ATTEMPTS: u32 = 4; + const WINDOW: Duration = Duration::from_millis(200); + + for attempt in 1..=MAX_ATTEMPTS { + let dir = tempfile::tempdir().unwrap(); + let src = dir.path().join("hot.mds"); + std::fs::write(&src, "---\nname: v0\n---\nHot {{name}}!\n").unwrap(); + let out = dir.path().join("hot.md"); + + // --debounce 200 => cap = max(10 x 200ms, 1s) = 2s. + let (mut child, stderr_tap) = spawn_ready( + mds_bin() + .args([ + "watch", + src.to_str().unwrap(), + "--debounce", + "200", + "--poll-interval", + "0", + ]) + .stdout(Stdio::null()), + ); - assert!( - wait_for_file_contains(&out, "Hot v0!", TIMEOUT), - "initial compile should produce Hot v0!" - ); + assert!( + wait_for_file_contains(&out, "Hot v0!", TIMEOUT), + "initial compile should produce Hot v0!" + ); - let writing = std::sync::Arc::new(std::sync::atomic::AtomicBool::new(true)); - let writer_flag = std::sync::Arc::clone(&writing); - let writer_src = src.clone(); - let writer = std::thread::spawn(move || { - let stop_at = Instant::now() + Duration::from_secs(3); - let mut max_gap = Duration::ZERO; - let mut last = Instant::now(); - // Doubly bounded: <= 3s of wall clock AND <= 2000 iterations. - for i in 1..=2_000u32 { - if Instant::now() >= stop_at { - break; + let writing = std::sync::Arc::new(std::sync::atomic::AtomicBool::new(true)); + let writer_flag = std::sync::Arc::clone(&writing); + let writer_src = src.clone(); + let writer = std::thread::spawn(move || { + let stop_at = Instant::now() + Duration::from_secs(3); + let mut max_gap = Duration::ZERO; + let mut last = Instant::now(); + // Doubly bounded: <= 3s of wall clock AND <= 2000 iterations. + for i in 1..=2_000u32 { + if Instant::now() >= stop_at { + break; + } + write_atomic( + &writer_src, + format!("---\nname: v{i}\n---\nHot {{{{name}}}}!\n"), + ); + let now = Instant::now(); + max_gap = max_gap.max(now.duration_since(last)); + last = now; + std::thread::sleep(Duration::from_millis(5)); } - write_atomic( - &writer_src, - format!("---\nname: v{i}\n---\nHot {{{{name}}}}!\n"), + writer_flag.store(false, std::sync::atomic::Ordering::SeqCst); + max_gap + }); + + // The cap is 2s; allow the compile that follows it to land inside the bound. + wait_for_stderr_contains_str(&stderr_tap, "Recompiled ", Duration::from_millis(3500)); + let rebuilt_while_writing = writing.load(std::sync::atomic::Ordering::SeqCst); + + let max_gap = writer.join().expect("writer thread panicked"); + + // Harness precondition, checked before any behaviour assertion: if the writer + // thread could not sustain a sub-window cadence, this run cannot tell a cap + // rebuild from a quiet-period one. Discard it and retry rather than reporting a + // scheduling hiccup as a product failure. + if max_gap >= WINDOW { + drop(child); + assert!( + attempt < MAX_ATTEMPTS, + "the writer thread could not sustain a sub-{WINDOW:?} write cadence in \ + {MAX_ATTEMPTS} attempts (largest gap {max_gap:?}); the runner is too \ + loaded to exercise the cap deterministically" ); - let now = Instant::now(); - max_gap = max_gap.max(now.duration_since(last)); - last = now; - std::thread::sleep(Duration::from_millis(5)); + continue; } - writer_flag.store(false, std::sync::atomic::Ordering::SeqCst); - max_gap - }); - // The cap is 2s; allow the compile that follows it to land inside the bound. - wait_for_stderr_contains_str(&stderr_tap, "Recompiled ", Duration::from_millis(3500)); - let rebuilt_while_writing = writing.load(std::sync::atomic::Ordering::SeqCst); + assert!( + rebuilt_while_writing, + "a rebuild must happen WHILE the writes are still arriving — that is what the \ + cap is for; nothing was seen until the stream stopped" + ); - let max_gap = writer.join().expect("writer thread panicked"); - assert!( - max_gap < Duration::from_millis(200), - "precondition: no gap in the write stream may reach the 200ms window, else a \ - quiet period could legitimately have ended it; largest gap was {max_gap:?}" - ); - assert!( - rebuilt_while_writing, - "a rebuild must happen WHILE the writes are still arriving — that is what the \ - cap is for; nothing was seen until the stream stopped" - ); + let stderr = stderr_tap.finish_text(&mut child); + let rebuilds = count_occurrences(&stderr, "Recompiled "); + assert!( + (1..=4).contains(&rebuilds), + "3s of writes under a 200ms window with a 2s cap is one capped rebuild plus \ + the quiet-period rebuild that follows the last write; a fixed 200ms window \ + would give ~15. Got {rebuilds}; stderr was:\n{stderr}" + ); + return; + } - let stderr = stderr_tap.finish_text(&mut child); - let rebuilds = count_occurrences(&stderr, "Recompiled "); - assert!( - (1..=4).contains(&rebuilds), - "3s of writes under a 200ms window with a 2s cap is one capped rebuild plus \ - the quiet-period rebuild that follows the last write; a fixed 200ms window \ - would give ~15. Got {rebuilds}; stderr was:\n{stderr}" - ); + unreachable!("the loop returns on a conclusive attempt or asserts on the last one"); } // ── AC-F10: Watch no-arg auto-detect ───────────────────────────────────── diff --git a/crates/mds-cli/tests/common/mod.rs b/crates/mds-cli/tests/common/mod.rs index 35c38c5f..aa552941 100644 --- a/crates/mds-cli/tests/common/mod.rs +++ b/crates/mds-cli/tests/common/mod.rs @@ -24,6 +24,53 @@ pub fn mds_bin() -> std::process::Command { cmd } +// ── Frontmatter YAML bounds builders (#162) ────────────────────────────────── + +/// Frontmatter size cap (1 MiB) — mirrors `mds-core`'s `MAX_FRONTMATTER_SIZE`. +#[allow(dead_code)] +pub const MAX_FRONTMATTER_SIZE: usize = 1 << 20; + +/// Wrap a YAML frontmatter body in `---` fences with a one-line body. +#[allow(dead_code)] +pub fn wrap(yaml: &str) -> String { + format!("---\n{yaml}---\nHi\n") +} + +/// A single `k: \n` line whose total byte length is EXACTLY `bytes`. +/// +/// The `ZZSENTINELZZ` marker lets the size-cap tests assert the rejection message never +/// echoes the (arbitrarily large) frontmatter content back to the user. +#[allow(dead_code)] +pub fn fm_of_size(bytes: usize) -> String { + const PREFIX: &str = "k: ZZSENTINELZZ"; + assert!(bytes > PREFIX.len() + 1, "requested size too small"); + let pad = bytes - PREFIX.len() - 1; + let out = format!("{PREFIX}{}\n", "x".repeat(pad)); + assert_eq!( + out.len(), + bytes, + "fm_of_size must produce EXACTLY `bytes` bytes" + ); + out +} + +/// An alias-fan-out bomb: `a: &a [x, x, ...(n)]`, `b: [*a, *a, ...(m)]`. Each `*a` +/// re-expands the `n`-element anchor at deserialise time, so the materialised tree far +/// exceeds the node budget while the SOURCE stays small (~700 KB for n = m = 100 000). +#[allow(dead_code)] +pub fn alias_bomb(n: usize, m: usize) -> String { + let xs = vec!["x"; n].join(", "); + let refs = vec!["*a"; m].join(", "); + format!("a: &a [{xs}]\nb: [{refs}]\n") +} + +/// `k: [[[...x...]]]` with `d` nested flow sequences around a scalar (a deep-nest DoS +/// repro; the pre-parse flow-depth guard rejects any `d > 1024`). +#[allow(dead_code)] +pub fn nested_flow_seq(d: usize) -> String { + format!("k: {}x{}\n", "[".repeat(d), "]".repeat(d)) +} + // ── Duplicate --vars file key warnings (#326) ──────────────────────────────── /// USER-FACING CONTRACT (#326). `{key}` = dotted/bracketed path, `{path}` = the diff --git a/crates/mds-cli/tests/security.rs b/crates/mds-cli/tests/security.rs index 08f982e9..d59eb5eb 100644 --- a/crates/mds-cli/tests/security.rs +++ b/crates/mds-cli/tests/security.rs @@ -1,5 +1,8 @@ mod common; -use common::{assert_no_control_chars, fixture, mds_bin}; +use common::{ + alias_bomb, assert_no_control_chars, fixture, fm_of_size, mds_bin, nested_flow_seq, wrap, + MAX_FRONTMATTER_SIZE, +}; use std::collections::HashMap; #[test] @@ -393,6 +396,268 @@ fn exit_code_resource_limit() { ); } +// ── Frontmatter YAML DoS bounds across the CLI (#162) ──────────────────────── +// +// The alias bomb is the sub-1 MiB memory-amplification repro (n = m = 100 000, +// ~700 KB source): the source is under the 1 MiB size cap, so the node budget is what +// rejects it — fast, without materialising the tree. Every rejection is exit code 3 +// (`mds::resource_limit`), never echoes the raw hostile bytes (`*a`, the sentinel, or a +// bracket run), and stays free of control characters. + +/// The realistic memory-amplification repro, wrapped as a full `.mds` document. +fn bomb_doc() -> String { + wrap(&alias_bomb(100_000, 100_000)) +} + +#[test] +fn cli_1_check_stdin_alias_bomb_is_resource_limit() { + use std::io::Write; + let mut child = mds_bin() + .args(["check", "-"]) + .stdin(std::process::Stdio::piped()) + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()) + .spawn() + .unwrap(); + if let Some(mut stdin) = child.stdin.take() { + let _ = stdin.write_all(bomb_doc().as_bytes()); + } + let output = child.wait_with_output().unwrap(); + assert_eq!( + output.status.code(), + Some(3), + "alias bomb via `check -` must exit 3" + ); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + stderr.contains("mds::resource_limit"), + "stderr must name the resource limit: {stderr}" + ); + assert!( + !stderr.contains("*a"), + "stderr must not echo bomb content: {stderr}" + ); + assert_no_control_chars(&stderr, "cli_1 check stdin bomb stderr"); +} + +#[test] +fn cli_2_check_file_alias_bomb_is_resource_limit() { + let dir = tempfile::tempdir().unwrap(); + let src = dir.path().join("bomb.mds"); + std::fs::write(&src, bomb_doc()).unwrap(); + let output = mds_bin().arg("check").arg(&src).output().unwrap(); + assert_eq!( + output.status.code(), + Some(3), + "alias bomb via `check ` must exit 3" + ); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + stderr.contains("mds::resource_limit"), + "stderr must name the resource limit: {stderr}" + ); + assert!( + !stderr.contains("*a"), + "stderr must not echo bomb content: {stderr}" + ); + assert_no_control_chars(&stderr, "cli_2 check file bomb stderr"); +} + +#[test] +fn cli_3_check_over_size_cap_is_resource_limit_no_echo() { + // Frontmatter one byte over the 1 MiB cap → rejected before any YAML work, and the + // message must never echo the (arbitrarily large) frontmatter content. + let dir = tempfile::tempdir().unwrap(); + let src = dir.path().join("over.mds"); + std::fs::write(&src, wrap(&fm_of_size(MAX_FRONTMATTER_SIZE + 1))).unwrap(); + let output = mds_bin().arg("check").arg(&src).output().unwrap(); + assert_eq!( + output.status.code(), + Some(3), + "over-cap frontmatter must exit 3" + ); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + stderr.contains("mds::resource_limit"), + "stderr must name the resource limit: {stderr}" + ); + assert!( + !stderr.contains("ZZSENTINELZZ"), + "stderr must not echo the frontmatter content: {stderr}" + ); + assert_no_control_chars(&stderr, "cli_3 over-cap stderr"); +} + +#[test] +fn cli_3c_check_at_size_cap_is_accepted() { + // At-cap control (PF-013): exactly 1 MiB of frontmatter is admitted. + let dir = tempfile::tempdir().unwrap(); + let src = dir.path().join("atcap.mds"); + std::fs::write(&src, wrap(&fm_of_size(MAX_FRONTMATTER_SIZE))).unwrap(); + let status = mds_bin() + .arg("check") + .arg(&src) + .stdout(std::process::Stdio::null()) + .stderr(std::process::Stdio::null()) + .status() + .unwrap(); + assert_eq!( + status.code(), + Some(0), + "at-cap frontmatter must be accepted" + ); +} + +#[test] +fn cli_4_lint_alias_bomb_is_resource_limit() { + let dir = tempfile::tempdir().unwrap(); + let src = dir.path().join("bomb.mds"); + std::fs::write(&src, bomb_doc()).unwrap(); + let output = mds_bin().arg("lint").arg(&src).output().unwrap(); + assert_eq!( + output.status.code(), + Some(3), + "lint on the bomb must exit 3" + ); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + stderr.contains("mds::resource_limit"), + "stderr must name the resource limit: {stderr}" + ); + assert!( + !stderr.contains("*a"), + "stderr must not echo bomb content: {stderr}" + ); + assert_no_control_chars(&stderr, "cli_4 lint bomb stderr"); +} + +#[test] +fn cli_4c_lint_legit_aliases_is_clean() { + // Positive control (PF-013): a legitimate anchor/alias with every frontmatter key + // referenced in the body lints clean (exit 0) — the bounds do not over-reject. + let dir = tempfile::tempdir().unwrap(); + let src = dir.path().join("legit.mds"); + std::fs::write(&src, "---\na: &a [1, 2]\nb: *a\n---\n{{a}} {{b}}\n").unwrap(); + let status = mds_bin() + .arg("lint") + .arg(&src) + .stdout(std::process::Stdio::null()) + .stderr(std::process::Stdio::null()) + .status() + .unwrap(); + assert_eq!(status.code(), Some(0), "legit aliases must lint clean"); +} + +#[test] +fn cli_5_fmt_check_alias_bomb_leaves_file_byte_identical() { + // `mds fmt --check` compiles the source to prove formatting equivalence. Post-fix the + // compile rejects the bomb cleanly, the formatter falls back and reattaches the + // frontmatter verbatim, so `--check` reports the file unchanged (exit 0) and the + // on-disk bytes are untouched. + let dir = tempfile::tempdir().unwrap(); + let src = dir.path().join("bomb.mds"); + let original = bomb_doc(); + std::fs::write(&src, &original).unwrap(); + let status = mds_bin() + .args(["fmt", "--check"]) + .arg(&src) + .stdout(std::process::Stdio::null()) + .stderr(std::process::Stdio::null()) + .status() + .unwrap(); + assert_eq!( + status.code(), + Some(0), + "fmt --check on the bomb must exit 0" + ); + let after = std::fs::read(&src).unwrap(); + assert_eq!( + after, + original.as_bytes(), + "fmt --check must leave the bomb file byte-identical" + ); +} + +#[test] +fn cli_deep_check_deep_flow_nest_is_resource_limit() { + // The second DoS axis: pure deep flow-nesting (no anchors), a CPU hang in libyaml's + // flow scanner pre-fix. The pre-parse depth guard rejects depth > 1024 fast, before + // any scanner work, and the message names the flow-depth limit without echoing the + // bracket run. + let dir = tempfile::tempdir().unwrap(); + let src = dir.path().join("deep.mds"); + std::fs::write(&src, wrap(&nested_flow_seq(2000))).unwrap(); + let start = std::time::Instant::now(); + let output = mds_bin().arg("check").arg(&src).output().unwrap(); + let elapsed = start.elapsed(); + assert_eq!(output.status.code(), Some(3), "deep flow nest must exit 3"); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + stderr.contains("mds::resource_limit"), + "stderr must name the resource limit: {stderr}" + ); + // miette word-wraps the diagnostic and prefixes continuation lines with `│`; flatten + // that back to a single line before matching the full message. + let flat: String = stderr + .replace('│', " ") + .split_whitespace() + .collect::>() + .join(" "); + assert!( + flat.contains("flow nesting exceeds maximum depth of 1024"), + "stderr must name the flow-depth limit: {stderr}" + ); + assert!( + !stderr.contains("[[[") && !stderr.contains("]]]"), + "stderr must not echo the bracket run: {stderr}" + ); + assert_no_control_chars(&stderr, "cli_deep stderr"); + assert!( + elapsed < std::time::Duration::from_secs(5), + "the guard must reject the deep nest pre-parse (fast), took {elapsed:?}" + ); +} + +#[test] +fn cli_6_max_frontmatter_in_max_source_build_is_accepted() { + // Both caps at their boundaries compose: a 1 MiB (at-cap) frontmatter inside a + // source that is exactly 10 MiB (at MAX_FILE_SIZE) is accepted by `build -`. + use std::io::Write; + const MAX_FILE_SIZE: usize = 10 * 1024 * 1024; + let head = wrap_open(&fm_of_size(MAX_FRONTMATTER_SIZE)); + let body_len = MAX_FILE_SIZE - head.len(); + let mut doc = head; + doc.push_str(&"H".repeat(body_len - 1)); + doc.push('\n'); + assert_eq!( + doc.len(), + MAX_FILE_SIZE, + "source must be exactly at MAX_FILE_SIZE" + ); + + let mut child = mds_bin() + .args(["build", "-o", "-", "-"]) + .stdin(std::process::Stdio::piped()) + .stdout(std::process::Stdio::null()) + .stderr(std::process::Stdio::null()) + .spawn() + .unwrap(); + if let Some(mut stdin) = child.stdin.take() { + let _ = stdin.write_all(doc.as_bytes()); + } + let status = child.wait().unwrap(); + assert_eq!( + status.code(), + Some(0), + "1 MiB frontmatter inside a 10 MiB source must be accepted at both boundaries" + ); +} + +/// `---\n{yaml}---\n` — the fenced header without a body, for size-composition tests. +fn wrap_open(yaml: &str) -> String { + format!("---\n{yaml}---\n") +} + // ── AC-2: load_vars_file rejects symlinked vars paths (PF-004 fix) ─────────── #[test] diff --git a/crates/mds-core/src/lib.rs b/crates/mds-core/src/lib.rs index 7e378ad2..ada18ca3 100644 --- a/crates/mds-core/src/lib.rs +++ b/crates/mds-core/src/lib.rs @@ -347,6 +347,13 @@ pub fn compile( /// /// Warnings (e.g. empty `@include`) are printed to stderr. /// +/// # Limits +/// +/// The source must not exceed `MAX_FILE_SIZE` (10 MiB), and its YAML frontmatter +/// is bounded to 1 MiB, 200,000 nodes, and 1024 levels of flow-collection +/// nesting. Exceeding any of these fails with an `mds::resource_limit` error +/// before the frontmatter parser materialises or deep-scans the input. +/// /// # Examples /// /// ```rust @@ -364,6 +371,13 @@ pub fn compile_str(source: &str) -> Result { /// Warnings (e.g. empty `@include`) are printed to stderr. `base_dir` sets the /// root for resolving `@import` paths; defaults to the current directory. /// +/// # Limits +/// +/// The source must not exceed `MAX_FILE_SIZE` (10 MiB), and its YAML frontmatter +/// is bounded to 1 MiB, 200,000 nodes, and 1024 levels of flow-collection +/// nesting. Exceeding any of these fails with an `mds::resource_limit` error +/// before the frontmatter parser materialises or deep-scans the input. +/// /// # Examples /// /// ```rust,no_run @@ -420,6 +434,13 @@ pub fn check( /// /// Warnings (e.g. empty `@include`) are printed to stderr. /// +/// # Limits +/// +/// The source must not exceed `MAX_FILE_SIZE` (10 MiB), and its YAML frontmatter +/// is bounded to 1 MiB, 200,000 nodes, and 1024 levels of flow-collection +/// nesting. Exceeding any of these fails with an `mds::resource_limit` error +/// before the frontmatter parser materialises or deep-scans the input. +/// /// # Examples /// /// ```rust @@ -1360,10 +1381,17 @@ pub fn scan_imports(source: &str) -> Result, MdsError> { // Insert frontmatter import paths (they resolve before body imports). // Best-effort: ignore parse errors here (parse errors will surface at compile time). if let Some(fm) = module.frontmatter.as_ref() { - if let Ok(fm_imports) = resolver::parse_frontmatter_imports(&fm.raw) { - for imp in &fm_imports { - paths.insert(imp.path().to_owned()); + // Best-effort for plain parse/validation errors, but a resource limit (frontmatter + // size cap / node budget / too-many-imports) must fail closed rather than silently + // return only body imports (#162). + match resolver::parse_frontmatter_imports(&fm.raw) { + Ok(fm_imports) => { + for imp in &fm_imports { + paths.insert(imp.path().to_owned()); + } } + Err(e @ MdsError::ResourceLimit { .. }) => return Err(e), + Err(_) => {} } } @@ -1782,6 +1810,35 @@ mod tests { // ── scan_imports: frontmatter imports paths ─────────────────────────────── + /// #162: a frontmatter alias-fan-out bomb must make `scan_imports` fail closed with a + /// resource limit rather than silently return only body imports. + #[test] + fn scan_imports_frontmatter_bomb_propagates_resource_limit() { + let xs = vec!["x"; 1000].join(", "); + let refs = vec!["*a"; 400].join(", "); + let source = format!("---\na: &a [{xs}]\nb: [{refs}]\n---\nHi\n"); + let r = scan_imports(&source); + assert!( + matches!(r, Err(crate::MdsError::ResourceLimit { .. })), + "frontmatter bomb must propagate a resource limit, got {r:?}" + ); + } + + /// A plain frontmatter YAML syntax error stays best-effort: `scan_imports` swallows it + /// and still returns the body imports. + #[test] + fn scan_imports_frontmatter_syntax_error_is_lenient() { + let source = concat!( + "---\n", + "imports: [\n", // malformed YAML — never closes + "---\n", + "@import \"./x.mds\"\n", + "Hi\n", + ); + let paths = scan_imports(source).expect("syntax error in FM must be swallowed"); + assert_eq!(paths, vec!["./x.mds".to_string()]); + } + #[test] fn scan_imports_fm_alias() { let source = concat!( diff --git a/crates/mds-core/src/limits.rs b/crates/mds-core/src/limits.rs index aa3d8ae2..bc132006 100644 --- a/crates/mds-core/src/limits.rs +++ b/crates/mds-core/src/limits.rs @@ -59,6 +59,53 @@ pub(crate) const MAX_ARRAY_ELEMENTS: usize = 100_000; /// 256 entries is generous for any real template. pub(crate) const MAX_FRONTMATTER_IMPORTS: usize = 256; +/// Maximum byte length of one frontmatter YAML block (1 MiB). +/// +/// Checked by `resolver::frontmatter::parse_frontmatter_yaml` before any YAML work: the +/// `serde_yaml_ng` loader is eager (it drains the whole document into an event vector +/// before deserialising), so the cap must sit in front of it. Frontmatter is variable +/// data, not prose: 1 MiB is on the order of 50 000 `key: value` lines, while the body +/// keeps the 10 MiB `MAX_FILE_SIZE` bound. Exceeding this surfaces as +/// `mds::resource_limit` (CLI exit 3). See #162. +pub(crate) const MAX_FRONTMATTER_SIZE: usize = 1024 * 1024; + +/// Maximum number of YAML nodes one frontmatter block may materialise (200 000). +/// +/// Counted while `serde_yaml_ng` deserialises (every scalar, null, sequence, mapping, +/// mapping key and `!tag` wrapper is one node), so the parse fails before the tree is +/// built. This is the alias bound: an `&anchor` referenced by many `*alias`es expands at +/// deserialise time, so a block under `MAX_FRONTMATTER_SIZE` could otherwise demand on +/// the order of size^2/24 nodes (about 4 x 10^10 for 1 MiB). `serde_yaml_ng`'s own +/// repetition limit counts alias jumps, not nodes, and does not catch one large anchor +/// referenced a few thousand times. An alias-free block needs at least 2 bytes per node +/// (`[x,x,...]`), so under the size cap it stays around 525 000 nodes at most and a +/// realistic `key: value` block near 100 000; 200 000 rejects only amplification and +/// bounds the materialised tree at a few tens of MB per parse. Exceeding this surfaces +/// as `mds::resource_limit`. See #162. +pub(crate) const MAX_FRONTMATTER_NODES: usize = 200_000; + +/// Maximum running depth of flow-collection nesting (`[`/`{`) in one frontmatter +/// YAML block (1024). +/// +/// Checked by `resolver::frontmatter::parse_frontmatter_yaml` in a single O(n) pass over +/// the raw bytes, AFTER the size cap and BEFORE the budgeted parse. libyaml's flow scanner +/// is O(depth^2) in flow-collection nesting, and that cost is paid inside the scanner +/// UPSTREAM of deserialisation. The three existing depth limits are all post-hoc: serde's +/// recursion limit (128 parse frames, "recursion limit exceeded"), and `Value::from_yaml`'s +/// `MAX_VALUE_DEPTH` (64, "value nesting exceeds maximum depth of 64"). Every one of them +/// fires only AFTER the quadratic scan has already been paid, so a ~1 MiB pure deep +/// flow-nest (no anchors) burns 10+ s of CPU at ~32 MB RSS before any of them rejects it — +/// and the node budget cannot catch it (few nodes, trivial memory). A cheap pre-parse bound +/// on flow-nesting depth is the only thing that stops it before the scanner runs. +/// +/// 1024 is far above any legitimate frontmatter — flow collections are never nested even +/// 100 deep — yet it caps the worst admitted scan at ~1024^2 work (trivially fast). It sits +/// deliberately ABOVE serde's 128-frame recursion limit so the parser's own +/// recursion/value-depth errors stay reachable and unchanged for shallower inputs. +/// Block-style (indent) nesting has no quadratic cost and is not counted. Exceeding this +/// surfaces as `mds::resource_limit`. See #162. +pub(crate) const MAX_FRONTMATTER_FLOW_DEPTH: usize = 1024; + /// Maximum number of messages a `@message`-bearing template may produce. /// /// Prevents runaway memory use from adversarial inputs that generate thousands diff --git a/crates/mds-core/src/lint/facts.rs b/crates/mds-core/src/lint/facts.rs index e74ba013..ffd4556a 100644 --- a/crates/mds-core/src/lint/facts.rs +++ b/crates/mds-core/src/lint/facts.rs @@ -224,7 +224,7 @@ pub(super) fn collect_facts( // ── 1. Pre-collect frontmatter vars ───────────────────────────────────── if let Some(fm) = &module.frontmatter { - collect_frontmatter_vars(fm, source, &mut ctx); + collect_frontmatter_vars(fm, source, &mut ctx)?; } // ── 2. Build walk scope for shadow detection ───────────────────────────── @@ -255,19 +255,26 @@ pub(super) fn collect_facts( /// /// Reserved keys (imports, type, extends, prompt) are excluded. /// Approximate source offsets are computed via substring search in `source`. -fn collect_frontmatter_vars(fm: &crate::ast::Frontmatter, source: &str, ctx: &mut AnalysisContext) { +fn collect_frontmatter_vars( + fm: &crate::ast::Frontmatter, + source: &str, + ctx: &mut AnalysisContext, +) -> Result<(), MdsError> { // Reserved keys per Appendix A (unused-variable skip-set). const RESERVED: &[&str] = &["imports", "type", "extends", "prompt"]; - let yaml_result = serde_yaml_ng::from_str::(&fm.raw); - let yaml = match yaml_result { + // Route through the bounded choke point (#162). A resource limit (size cap / node + // budget) propagates so the lint pass fails closed on an amplification attack; a plain + // YAML syntax error stays swallowed (lenient) — the resolver surfaces the diagnostic. + let yaml = match crate::resolver::parse_frontmatter_yaml(&fm.raw) { Ok(v) => v, - Err(_) => return, // malformed YAML — skip; resolver would have caught this + Err(e @ MdsError::ResourceLimit { .. }) => return Err(e), + Err(_) => return Ok(()), }; let mapping = match &yaml { serde_yaml_ng::Value::Mapping(m) => m, - _ => return, + _ => return Ok(()), }; // Find the byte offset of the frontmatter content in the source. @@ -291,6 +298,8 @@ fn collect_frontmatter_vars(fm: &crate::ast::Frontmatter, source: &str, ctx: &mu approx_offset, }); } + + Ok(()) } /// Find the byte offset where frontmatter YAML content starts in `source`. @@ -725,6 +734,60 @@ mod tests { assert!(ctx.is_partial_or_extends); } + // ── Frontmatter YAML bounds propagation (#162) ────────────────────────── + + fn fm_doc(body: &str) -> String { + format!("---\n{body}---\nHi\n") + } + + /// An alias-fan-out bomb whose materialised tree (400 * 1001 nodes) far exceeds the + /// frontmatter node budget. + fn alias_bomb_fm() -> String { + let xs = vec!["x"; 1000].join(", "); + let refs = vec!["*a"; 400].join(", "); + format!("a: &a [{xs}]\nb: [{refs}]\n") + } + + /// Non-vacuity (checked FIRST): the collector really extracts frontmatter var names, + /// so the swallow tests below are not passing on an empty scan. + #[test] + fn collect_facts_frontmatter_vars_non_vacuous() { + let src = fm_doc("name: x\n"); + let module = parse(&src); + let ctx = collect_facts(&module, false, &src).unwrap(); + assert!( + ctx.frontmatter_vars.iter().any(|f| f.name == "name"), + "expected `name` frontmatter var, got {:?}", + ctx.frontmatter_vars + ); + } + + /// A plain YAML syntax error stays swallowed (lenient): no vars, no error — the + /// resolver surfaces the real diagnostic. + #[test] + fn collect_facts_swallows_syntax_error() { + let src = fm_doc("a: [\n"); + let module = parse(&src); + let ctx = collect_facts(&module, false, &src).unwrap(); + assert!( + ctx.frontmatter_vars.is_empty(), + "malformed frontmatter must yield no vars and no error" + ); + } + + /// A resource-limit error (node budget) is NOT swallowed — it propagates so the lint + /// pass fails closed on an amplification attack. + #[test] + fn collect_facts_propagates_resource_limit() { + let src = fm_doc(&alias_bomb_fm()); + let module = parse(&src); + let r = collect_facts(&module, false, &src); + assert!( + matches!(r, Err(MdsError::ResourceLimit { .. })), + "alias bomb must propagate a resource limit, got {r:?}" + ); + } + /// AC-PERF-04: Nesting deeper than MAX_NESTING_DEPTH (64) returns ResourceLimit. #[test] fn collect_facts_depth_limit_enforced() { diff --git a/crates/mds-core/src/resolver.rs b/crates/mds-core/src/resolver.rs index 4f874fef..9ceab923 100644 --- a/crates/mds-core/src/resolver.rs +++ b/crates/mds-core/src/resolver.rs @@ -14,7 +14,7 @@ use crate::evaluator::evaluate_with_map; use crate::evaluator::evaluate_with_map_seeded; use crate::fs::{FileSystem, NativeFs, VirtualFs}; use crate::lexer::tokenize; -use crate::limits::{MAX_BLOCKS_PER_MODULE, MAX_MODULE_COUNT}; +use crate::limits::{MAX_BLOCKS_PER_MODULE, MAX_FILE_SIZE, MAX_MODULE_COUNT}; use crate::parser::parse_with_ctx; use crate::scope::{FunctionDef, NamespaceScope, Scope}; // Import Origin from sourcemap.rs to avoid a scope→resolver import cycle. @@ -24,7 +24,8 @@ use crate::value::Value; use frontmatter::{build_scope_from_merged_mapping, deep_merge_yaml}; pub(crate) use frontmatter::{ - parse_frontmatter_imports, parse_frontmatter_imports_from_yaml, FrontmatterImport, + parse_frontmatter_imports, parse_frontmatter_imports_from_yaml, parse_frontmatter_yaml, + FrontmatterImport, }; use inheritance::{ apply_block_overrides, check_child_only_blocks, seed_effective_blocks, splice_skeleton, @@ -300,6 +301,24 @@ impl ModuleCache { self.modules.keys().cloned().collect() } + /// Enforce `MAX_FILE_SIZE` on a string entry source at the core string funnels. + /// + /// The binding layers (WASM/napi) and `NativeFs::read` carry their own size guards + /// with their own messages, but `compile_str`/`check_str`/`lint_str_with` and the + /// opts variants reach the resolver as an in-memory `&str` that never passes those + /// guards (PF-004). The message shape mirrors `fs.rs` so callers matching on + /// "too large" behave identically across paths. + fn check_source_size(source: &str) -> Result<(), MdsError> { + if source.len() as u64 > MAX_FILE_SIZE { + return Err(MdsError::resource_limit(format!( + "file too large ({} bytes, max {} bytes): {SOURCE_LABEL}", + source.len(), + MAX_FILE_SIZE, + ))); + } + Ok(()) + } + /// Guard against excessively deep import chains. fn check_import_depth(&self) -> Result<(), MdsError> { if self.resolving.len() >= MAX_IMPORT_DEPTH { @@ -503,6 +522,8 @@ impl ModuleCache { runtime_vars: &HashMap, warnings: &mut Vec, ) -> Result, MdsError> { + // Entry-size backstop for the string funnel (PF-004) — before any IO or parse. + Self::check_source_size(source)?; // Canonicalize base_dir via the FileSystem abstraction so that custom // or virtual backends can override this behaviour (fixes issue #21). let canonical_str = self.fs.canonicalize(base_dir)?; @@ -623,6 +644,7 @@ impl ModuleCache { runtime_vars: &HashMap, warnings: &mut Vec, ) -> Result { + Self::check_source_size(source)?; let canonical_str = self.fs.canonicalize(base_dir)?; self.fs.set_root(&canonical_str)?; self.check_import_depth()?; @@ -649,6 +671,7 @@ impl ModuleCache { opts: &crate::sourcemap::CompileOptions, warnings: &mut Vec, ) -> Result<(crate::CompiledOutput, Option), MdsError> { + Self::check_source_size(source)?; let canonical_str = self.fs.canonicalize(base_dir)?; self.fs.set_root(&canonical_str)?; self.check_import_depth()?; @@ -2301,9 +2324,8 @@ fn build_scope_from_frontmatter( let is_mds = !is_md || frontmatter.is_some_and(|fm| has_type_mds_frontmatter_raw(&fm.raw)); if let Some(fm) = frontmatter { - // Parse YAML once to avoid double-parsing - let yaml: serde_yaml_ng::Value = - serde_yaml_ng::from_str(&fm.raw).map_err(|e| MdsError::yaml_error(e.to_string()))?; + // Parse YAML once to avoid double-parsing (bounded — #162). + let yaml = parse_frontmatter_yaml(&fm.raw)?; if let serde_yaml_ng::Value::Mapping(map) = yaml { for (key, val) in map { @@ -2586,8 +2608,7 @@ fn parse_frontmatter_mapping( let Some(fm) = frontmatter else { return Ok(None); }; - let yaml: serde_yaml_ng::Value = - serde_yaml_ng::from_str(&fm.raw).map_err(|e| MdsError::yaml_error(e.to_string()))?; + let yaml = parse_frontmatter_yaml(&fm.raw)?; if let serde_yaml_ng::Value::Mapping(map) = yaml { Ok(Some(map)) } else { diff --git a/crates/mds-core/src/resolver/frontmatter.rs b/crates/mds-core/src/resolver/frontmatter.rs index cb87797c..299bdded 100644 --- a/crates/mds-core/src/resolver/frontmatter.rs +++ b/crates/mds-core/src/resolver/frontmatter.rs @@ -4,10 +4,17 @@ //! frontmatter mappings, building variable scopes, and parsing `imports:` declarations //! from YAML frontmatter. +use std::cell::Cell; use std::collections::HashMap; +use std::marker::PhantomData; + +use serde::de::{self, DeserializeSeed, EnumAccess, MapAccess, SeqAccess, VariantAccess, Visitor}; use crate::error::MdsError; -use crate::limits::{MAX_FRONTMATTER_IMPORTS, MAX_FRONTMATTER_MERGE_DEPTH}; +use crate::limits::{ + MAX_FRONTMATTER_FLOW_DEPTH, MAX_FRONTMATTER_IMPORTS, MAX_FRONTMATTER_MERGE_DEPTH, + MAX_FRONTMATTER_NODES, MAX_FRONTMATTER_SIZE, +}; use crate::parser::is_valid_identifier; use crate::scope::Scope; use crate::value::Value; @@ -128,6 +135,298 @@ pub(super) fn deep_merge_yaml( Ok(result) } +/// The single choke point for parsing frontmatter YAML into an untyped `Value` (#162). +/// +/// All four frontmatter parse sites route through here so the DoS bounds cannot be +/// bypassed on a parallel path. It returns a `Value` (not a `Mapping`) so each caller +/// keeps its own "not a mapping" handling. +/// +/// Three bounds are enforced ahead of / during the parse: +/// 1. A 1 MiB byte cap (`MAX_FRONTMATTER_SIZE`), checked before any YAML work because the +/// `serde_yaml_ng` loader is eager (it drains the whole document into an event vector). +/// 2. A flow-nesting depth guard (`MAX_FRONTMATTER_FLOW_DEPTH`), a single O(n) byte pass +/// before the parser. libyaml's flow scanner is O(depth^2) and runs UPSTREAM of the +/// node budget, so a deep flow-nest under the 1 MiB cap still burns seconds of CPU at +/// trivial RSS; the byte cap alone does not bound it. See [`check_flow_nesting_depth`]. +/// 3. A 200 000-node budget (`MAX_FRONTMATTER_NODES`), charged while deserialising, which +/// is what catches an `&anchor` referenced by many `*alias`es — the amplification +/// `serde_yaml_ng`'s own alias-jump limit does not catch. +/// +/// Our bounds surface as [`MdsError::resource_limit`]; everything `serde_yaml_ng` itself +/// rejects (syntax errors, its recursion/repetition limits, duplicate keys) surfaces as +/// [`MdsError::yaml_error`] with a message byte-identical to a plain +/// `serde_yaml_ng::from_str::`, because the same `Deserializer` drives both. +pub(crate) fn parse_frontmatter_yaml(raw: &str) -> Result { + parse_frontmatter_yaml_bounded(raw, MAX_FRONTMATTER_SIZE, MAX_FRONTMATTER_NODES) +} + +/// Bounds-parameterised core of [`parse_frontmatter_yaml`], so unit tests can pin node +/// accounting on tiny documents without allocating a real attack. +fn parse_frontmatter_yaml_bounded( + raw: &str, + max_bytes: usize, + max_nodes: usize, +) -> Result { + // 1. Size cap FIRST — before the eager loader touches the input. Bounds total work + // (the flow-nesting depth guard below bounds libyaml's O(depth^2) flow scanner, + // which the byte cap alone does not: a deep nest under 1 MiB still hangs). + if raw.len() > max_bytes { + return Err(MdsError::resource_limit(format!( + "frontmatter too large ({} bytes, max {max_bytes} bytes)", + raw.len() + ))); + } + + // 1b. Flow-nesting depth guard — a single O(n) byte pass BEFORE the parser, so + // libyaml's O(depth^2) flow scanner never runs on a pathological deep nest. This + // is a CPU bound the node budget cannot provide: the scanner runs UPSTREAM of + // deserialisation (at trivial RSS, few nodes), so a ~1 MiB pure deep flow-nest + // hangs for seconds before the budget or any downstream depth limit fires. #162. + check_flow_nesting_depth(raw, MAX_FRONTMATTER_FLOW_DEPTH)?; + + // 2. Budgeted deserialisation. `from_str::` is exactly + // `Value::deserialize(Deserializer::from_str(raw))`; driving the same deserializer + // with the budgeted seed keeps every non-budget error byte-identical. + let budget = NodeBudget::new(max_nodes); + let seed = BoundedYaml { budget: &budget }; + match seed.deserialize(serde_yaml_ng::Deserializer::from_str(raw)) { + Ok(value) => Ok(value), + // The budget latch is the discriminator — never the message text. + Err(_) if budget.tripped() => Err(MdsError::resource_limit(format!( + "frontmatter YAML node count exceeds maximum of {max_nodes} \ + (anchors expanded by aliases count once per expansion)" + ))), + Err(e) => Err(MdsError::yaml_error(e.to_string())), + } +} + +/// Reject frontmatter whose running flow-collection nesting depth ever exceeds +/// `max_depth`, in one O(n) pass over the raw bytes (#162). +/// +/// This is the pre-parse CPU bound: libyaml's flow scanner is O(depth^2) in flow nesting +/// and runs UPSTREAM of the node budget, so a ~1 MiB pure deep flow-nest hangs for seconds +/// before any downstream depth limit fires. The scan counts NET depth — flow openers +/// (`[`, `{`) increment, closers (`]`, `}`) decrement (saturating at 0) — not a total +/// bracket count, so a wide-but-shallow flow list (`[a, b, c, ...]`, depth 1) stays legal; +/// only nesting DEPTH is bounded. `[`/`]`/`{`/`}` are ASCII (< 0x80) and never occur inside +/// a UTF-8 multibyte sequence, so a byte scan is exact for them. +/// +/// The scan is deliberately naive: it does NOT skip brackets inside quoted scalars or +/// comments (that would require a YAML lexer). At a threshold of 1024 — 8x serde_yaml_ng's +/// own 128-frame recursion limit — a false rejection would need 1024+ net-unbalanced flow +/// openers inside scalar/comment content, which no legitimate frontmatter contains: a +/// document serde accepts has structural flow depth <= 64 (`MAX_VALUE_DEPTH`). The high +/// threshold, not a lexer, is the guard against false positives. +fn check_flow_nesting_depth(raw: &str, max_depth: usize) -> Result<(), MdsError> { + // Bounded by `raw.len()`, which the size cap has already bounded by MAX_FRONTMATTER_SIZE. + let mut depth: usize = 0; + for &byte in raw.as_bytes() { + match byte { + b'[' | b'{' => { + depth += 1; + if depth > max_depth { + // Never echo the (adversarial) raw input in the message. + return Err(MdsError::resource_limit(format!( + "frontmatter YAML flow nesting exceeds maximum depth of {max_depth}" + ))); + } + } + b']' | b'}' => depth = depth.saturating_sub(1), + _ => {} + } + } + Ok(()) +} + +/// A saturating-free node budget with a "tripped" latch, shared by reference across the +/// deserialise walk. `Cell` because the seed is `Copy` and threaded by value. +struct NodeBudget { + remaining: Cell, + tripped: Cell, +} + +impl NodeBudget { + fn new(max: usize) -> Self { + Self { + remaining: Cell::new(max), + tripped: Cell::new(false), + } + } + + /// Charge one node. On exhaustion, latch `tripped` and return a custom error so the + /// classifier in [`parse_frontmatter_yaml_bounded`] can attribute it to our bound. + /// `checked_sub` (never saturating) so the boundary is exact. + fn charge(&self) -> Result<(), E> { + match self.remaining.get().checked_sub(1) { + Some(rest) => { + self.remaining.set(rest); + Ok(()) + } + None => { + self.tripped.set(true); + Err(E::custom("frontmatter YAML node budget exhausted")) + } + } + } + + fn tripped(&self) -> bool { + self.tripped.get() + } +} + +/// A budgeted `DeserializeSeed`/`Visitor` that mirrors `serde_yaml_ng`'s own +/// `impl Deserialize for Value`, charging one node per scalar, sequence, mapping, mapping +/// key and `!tag` wrapper. It never pre-sizes from `size_hint` (an attacker controls it), +/// and it rejects duplicate keys with a message byte-identical to `serde_yaml_ng`'s. +#[derive(Clone, Copy)] +struct BoundedYaml<'b> { + budget: &'b NodeBudget, +} + +impl<'de, 'b> DeserializeSeed<'de> for BoundedYaml<'b> { + type Value = serde_yaml_ng::Value; + + fn deserialize(self, deserializer: D) -> Result + where + D: serde::Deserializer<'de>, + { + deserializer.deserialize_any(self) + } +} + +impl<'de, 'b> Visitor<'de> for BoundedYaml<'b> { + type Value = serde_yaml_ng::Value; + + fn expecting(&self, formatter: &mut std::fmt::Formatter) -> std::fmt::Result { + formatter.write_str("any YAML value") + } + + fn visit_bool(self, v: bool) -> Result { + self.budget.charge::()?; + Ok(serde_yaml_ng::Value::Bool(v)) + } + + fn visit_i64(self, v: i64) -> Result { + self.budget.charge::()?; + Ok(serde_yaml_ng::Value::Number(v.into())) + } + + fn visit_u64(self, v: u64) -> Result { + self.budget.charge::()?; + Ok(serde_yaml_ng::Value::Number(v.into())) + } + + fn visit_f64(self, v: f64) -> Result { + self.budget.charge::()?; + Ok(serde_yaml_ng::Value::Number(v.into())) + } + + fn visit_str(self, v: &str) -> Result { + self.budget.charge::()?; + Ok(serde_yaml_ng::Value::String(v.to_owned())) + } + + fn visit_string(self, v: String) -> Result { + self.budget.charge::()?; + Ok(serde_yaml_ng::Value::String(v)) + } + + fn visit_unit(self) -> Result { + self.budget.charge::()?; + Ok(serde_yaml_ng::Value::Null) + } + + fn visit_none(self) -> Result { + self.budget.charge::()?; + Ok(serde_yaml_ng::Value::Null) + } + + fn visit_some(self, deserializer: D) -> Result + where + D: serde::Deserializer<'de>, + { + deserializer.deserialize_any(self) + } + + fn visit_seq(self, mut seq: A) -> Result + where + A: SeqAccess<'de>, + { + // Charge the container, then each element as the seed visits it. Do NOT pre-size + // from `size_hint` — an alias expansion reports a large hint the attacker controls. + self.budget.charge::()?; + let mut out = serde_yaml_ng::Sequence::new(); + while let Some(elem) = seq.next_element_seed(self)? { + out.push(elem); + } + Ok(serde_yaml_ng::Value::Sequence(out)) + } + + fn visit_map(self, mut map: A) -> Result + where + A: MapAccess<'de>, + { + self.budget.charge::()?; + let mut out = serde_yaml_ng::Mapping::new(); + // Keys are nodes too. Reject a duplicate BEFORE reading its value, exactly as + // `serde_yaml_ng`'s `Mapping` visitor does, with a byte-identical message. + while let Some(key) = map.next_key_seed(self)? { + match out.entry(key) { + serde_yaml_ng::mapping::Entry::Occupied(entry) => { + return Err(::custom(duplicate_key_message( + entry.key(), + ))); + } + serde_yaml_ng::mapping::Entry::Vacant(entry) => { + let value = map.next_value_seed(self)?; + entry.insert(value); + } + } + } + Ok(serde_yaml_ng::Value::Mapping(out)) + } + + fn visit_enum(self, data: A) -> Result + where + A: EnumAccess<'de>, + { + // Charge the `!tag` wrapper, then the inner value via the seed. + self.budget.charge::()?; + let (tag, contents) = data.variant_seed(PhantomData::)?; + // `Tag::new` panics on an empty tag, so mirror `TagStringVisitor`'s guard and + // return the same error instead (never panic — #162 / never-panic contract). + if tag.is_empty() { + return Err(::custom( + "empty YAML tag is not allowed", + )); + } + let value = contents.newtype_variant_seed(self)?; + Ok(serde_yaml_ng::Value::Tagged(Box::new( + serde_yaml_ng::value::TaggedValue { + tag: serde_yaml_ng::value::Tag::new(tag), + value, + }, + ))) + } +} + +/// The `serde_yaml_ng` `DuplicateKeyError` `Display`, reproduced byte-for-byte (its type +/// is private). Mirrors `src/mapping.rs` in the vendored crate. +fn duplicate_key_message(key: &serde_yaml_ng::Value) -> String { + use serde_yaml_ng::Value; + let mut message = String::from("duplicate entry "); + match key { + Value::Null => message.push_str("with null key"), + Value::Bool(boolean) => message.push_str(&format!("with key `{boolean}`")), + Value::Number(number) => message.push_str(&format!("with key {number}")), + Value::String(string) => message.push_str(&format!("with key {string:?}")), + Value::Sequence(_) | Value::Mapping(_) | Value::Tagged(_) => { + message.push_str("in YAML map"); + } + } + message +} + /// Build a scope from a pre-merged `Mapping` and runtime variable overrides. /// /// Used by the template inheritance path after `deep_merge_yaml` has already @@ -292,8 +591,7 @@ fn parse_selective_entry( /// Returns an empty `Vec` if the `imports` key is absent. Propagates any /// parse or validation error from [`parse_frontmatter_imports_from_yaml`]. pub(crate) fn parse_frontmatter_imports(raw: &str) -> Result, MdsError> { - let yaml: serde_yaml_ng::Value = - serde_yaml_ng::from_str(raw).map_err(|e| MdsError::yaml_error(e.to_string()))?; + let yaml = parse_frontmatter_yaml(raw)?; let serde_yaml_ng::Value::Mapping(ref map) = yaml else { return Ok(vec![]); @@ -306,6 +604,10 @@ pub(crate) fn parse_frontmatter_imports(raw: &str) -> Result(r: &Result) -> bool { + matches!(r, Err(MdsError::ResourceLimit { .. })) +} + +fn is_yaml(r: &Result) -> bool { + matches!(r, Err(MdsError::YamlError { .. })) +} + +fn msg(r: &Result) -> String { + match r { + Err(e) => e.to_string(), + Ok(v) => panic!("expected Err, got Ok({v:?})"), + } +} + +// ── Builders ──────────────────────────────────────────────────────────────── + +/// Wrap a YAML frontmatter body in `---` fences with a one-line body. `\n` only, since +/// `fm.raw` is CR-stripped before it reaches the parser. +fn wrap(yaml: &str) -> String { + format!("---\n{yaml}---\nHi\n") +} + +/// A single `k: \n` line whose total byte length is EXACTLY `bytes`. +/// +/// The `ZZSENTINELZZ` marker lets the size-cap tests assert the rejection message never +/// echoes the (arbitrarily large) frontmatter content back to the user. +fn fm_of_size(bytes: usize) -> String { + const PREFIX: &str = "k: ZZSENTINELZZ"; + assert!(bytes > PREFIX.len() + 1, "requested size too small"); + // PREFIX + pad + '\n' == bytes. + let pad = bytes - PREFIX.len() - 1; + let out = format!("{PREFIX}{}\n", "x".repeat(pad)); + assert_eq!( + out.len(), + bytes, + "fm_of_size must produce EXACTLY `bytes` bytes" + ); + out +} + +/// Node count charged by the budgeted deserialiser for `alias_bomb(n, m, pad)`: +/// - top mapping container: 1 +/// - key `a` (1) + its anchored sequence of `n` scalars (1 + n) +/// - key `b` (1) + a sequence (1) of `m` aliases, each expanding to `1 + n` nodes +/// - when `pad > 0`: key `c` (1) + a sequence (1) of `pad` scalars (pad) +fn bomb_nodes(n: usize, m: usize, pad: usize) -> usize { + let core = 1 + 2 + n + 2 + m * (1 + n); + if pad > 0 { + core + 2 + pad + } else { + core + } +} + +/// An alias-fan-out bomb: `a: &a [x, x, ...(n)]`, `b: [*a, *a, ...(m)]`, and — when +/// `pad > 0` — `c: [x, x, ...(pad)]`. The materialised tree has `bomb_nodes(n, m, pad)` +/// nodes because each `*a` expands to the full `n`-element sequence at deserialise time. +fn alias_bomb(n: usize, m: usize, pad: usize) -> String { + let xs = vec!["x"; n].join(", "); + let refs = vec!["*a"; m].join(", "); + let mut out = format!("a: &a [{xs}]\nb: [{refs}]\n"); + if pad > 0 { + let ps = vec!["x"; pad].join(", "); + out.push_str(&format!("c: [{ps}]\n")); + } + out +} + +/// Build an alias bomb whose materialised node count is EXACTLY `target` (using n = 1000 +/// and always emitting the `c` padding sequence so `pad >= 1`). Asserts the count. +fn bomb_with_nodes(target: usize) -> String { + const N: usize = 1000; + // total = bomb_nodes(N, m, pad) = 1007 + 1001*m + pad (with pad > 0) + assert!(target > 1007 + 1001, "target too small to solve"); + let budget = target - 1007; + let mut m = budget / 1001; + let mut pad = budget - m * 1001; + if pad == 0 { + // Keep the `c` sequence non-empty so the (2 + pad) accounting term applies. + m -= 1; + pad += 1001; + } + let out = alias_bomb(N, m, pad); + assert_eq!( + bomb_nodes(N, m, pad), + target, + "bomb_with_nodes solved to the wrong count" + ); + out +} + +/// `k: [[[...x...]]]` with `d` nested flow sequences around a scalar. +fn nested_flow_seq(d: usize) -> String { + format!("k: {}x{}\n", "[".repeat(d), "]".repeat(d)) +} + +/// `d`-deep nested block mappings: `a:\n a:\n a: ...`. +fn nested_block_map(d: usize) -> String { + let mut out = String::new(); + for i in 0..d { + out.push_str(&" ".repeat(i)); + out.push_str("a:\n"); + } + out.push_str(&" ".repeat(d)); + out.push_str("v\n"); + out +} + +/// `d` nested tagged flow sequences around a scalar: `!t [!t [ ... 5 ... ]]`. Each level +/// is a tag on a one-element sequence (a node may carry only one tag, so tags cannot be +/// stacked directly). Every level adds a `Tagged` + a `Sequence` to the value tree. +fn tagged_nest(d: usize) -> String { + let mut inner = String::from("5"); + for _ in 0..d { + inner = format!("!t [{inner}]"); + } + format!("k: {inner}\n") +} + +/// Billion-laughs style multi-level alias amplification with `levels` anchor levels each +/// referencing the previous one `fanout` times. `serde_yaml_ng`'s own repetition limit +/// (alias-jump count) is what stops this, not our node budget. +fn laughs(fanout: usize, levels: usize) -> String { + let mut out = String::new(); + // Level 0: a literal list of scalars. + let leaves = vec!["\"x\""; fanout].join(", "); + out.push_str(&format!("l0: &l0 [{leaves}]\n")); + for i in 1..levels { + let refs = vec![format!("*l{}", i - 1); fanout].join(", "); + out.push_str(&format!("l{i}: &l{i} [{refs}]\n")); + } + out +} + +// ── T0: constant pins ───────────────────────────────────────────────────────── + +#[test] +fn t0_constants() { + assert_eq!(MAX_FRONTMATTER_SIZE, 1 << 20); + assert_eq!(MAX_FRONTMATTER_NODES, 200_000); +} + +// ── Size cap (exact boundary, direct on the choke point) ──────────────────────── + +#[test] +fn c2_size_cap_exact_boundary() { + // At the cap: a single ~1 MiB string value parses fine (PF-013 at-cap Ok twin). + let at = parse_frontmatter_yaml(&fm_of_size(MAX_FRONTMATTER_SIZE)); + assert!(at.is_ok(), "at-cap frontmatter must parse: {at:?}"); + + // One byte over: rejected as a resource limit, BEFORE any YAML work. + let over = parse_frontmatter_yaml(&fm_of_size(MAX_FRONTMATTER_SIZE + 1)); + assert!( + is_rl(&over), + "over-cap must be a resource limit, got {over:?}" + ); + let m = msg(&over); + assert!( + m.contains("frontmatter"), + "message must mention frontmatter: {m}" + ); + assert!( + m.contains(&MAX_FRONTMATTER_SIZE.to_string()), + "message must state the limit: {m}" + ); + assert!( + !m.contains("ZZSENTINELZZ"), + "message must not echo the frontmatter content: {m}" + ); +} + +#[test] +fn c3_size_cap_precedes_yaml_parse() { + // Over the cap AND malformed (`k: [` never closes). The size guard must fire FIRST, + // so this is a resource limit, not a YAML syntax error (order proof). + let raw = format!("k: [{}", "x".repeat(MAX_FRONTMATTER_SIZE)); + let r = parse_frontmatter_yaml(&raw); + assert!(is_rl(&r), "size cap must precede the parse: {r:?}"); + assert!( + !is_yaml(&r), + "must NOT surface as a YAML syntax error: {r:?}" + ); +} + +#[test] +fn c1_at_cap_compiles_end_to_end() { + // End-to-end wiring: a valid, comfortably-under-cap frontmatter compiles and the body + // is preserved. + let doc = wrap("greeting: hello\n"); + let out = crate::compile_str(&doc) + .expect("valid frontmatter must compile") + .into_markdown() + .expect("markdown output"); + assert!(out.ends_with("Hi\n"), "body must be preserved: {out:?}"); +} + +#[test] +fn c2_size_cap_end_to_end() { + // The cap is enforced on the string-compile path, not only in a unit call. + let doc = wrap(&fm_of_size(MAX_FRONTMATTER_SIZE + 4096)); + let r = crate::check_str(&doc); + assert!( + is_rl(&r), + "oversized frontmatter must be rejected via check_str: {r:?}" + ); +} + +// ── Node budget (exact accounting on tiny documents) ──────────────────────────── + +#[test] +fn budget_scalar_accounting() { + // map(1) + key(1) + scalar(1) = 3 nodes. + let raw = "k: v\n"; + assert!(parse_frontmatter_yaml_bounded(raw, usize::MAX, 3).is_ok()); + assert!(is_rl(&parse_frontmatter_yaml_bounded(raw, usize::MAX, 2))); +} + +#[test] +fn budget_sequence_accounting() { + // map(1) + key(1) + seq(1) + 2 scalars = 5 nodes. + let raw = "k: [x, x]\n"; + assert!(parse_frontmatter_yaml_bounded(raw, usize::MAX, 5).is_ok()); + assert!(is_rl(&parse_frontmatter_yaml_bounded(raw, usize::MAX, 4))); +} + +#[test] +fn budget_keys_are_charged() { + // Two entries: map(1) + [key(1)+scalar(1)] + [key(1)+scalar(1)] = 5 nodes. If keys + // were NOT charged the count would be 3 and a budget of 3 would (wrongly) pass. + let raw = "a: 1\nb: 2\n"; + assert!(parse_frontmatter_yaml_bounded(raw, usize::MAX, 5).is_ok()); + assert!(is_rl(&parse_frontmatter_yaml_bounded(raw, usize::MAX, 4))); + assert!(is_rl(&parse_frontmatter_yaml_bounded(raw, usize::MAX, 3))); +} + +#[test] +fn budget_tagged_accounting() { + // tagged wrapper(1) + inner scalar(1) = 2 nodes (M11: the `!tag` wrapper is charged). + let raw = "k: !T 5\n"; + // top is map(1) + key(1) + tagged(1) + scalar(1) = 4. + assert!(parse_frontmatter_yaml_bounded(raw, usize::MAX, 4).is_ok()); + assert!(is_rl(&parse_frontmatter_yaml_bounded(raw, usize::MAX, 3))); +} + +#[test] +fn c4_c5_node_budget_real_boundary() { + // C-4: exactly at the node cap parses (at-cap Ok twin, PF-013). + let at = alias_bomb_at(MAX_FRONTMATTER_NODES); + let at_r = parse_frontmatter_yaml(&at); + assert!(at_r.is_ok(), "exactly at the node cap must parse: {at_r:?}"); + + // C-5: one node over the cap is rejected as a resource limit. + let over = alias_bomb_at(MAX_FRONTMATTER_NODES + 1); + let over_r = parse_frontmatter_yaml(&over); + assert!( + is_rl(&over_r), + "one node over the cap must be rejected: {over_r:?}" + ); + let m = msg(&over_r); + assert!( + m.contains(&MAX_FRONTMATTER_NODES.to_string()), + "message must state the node limit: {m}" + ); + assert!( + !m.contains("*a"), + "message must not echo the bomb content: {m}" + ); +} + +/// Exactly-`target`-node alias bomb (wrapper around `bomb_with_nodes`). +fn alias_bomb_at(target: usize) -> String { + bomb_with_nodes(target) +} + +#[test] +fn c6_alias_revisits_are_counted() { + // ~150 KB source, but each of the m aliases re-expands the n-element anchor, so the + // materialised tree far exceeds the node cap: the budget counts re-visits, not bytes. + let raw = alias_bomb(MAX_FRONTMATTER_NODES / 4, 4, 0); + assert!( + raw.len() < 400 * 1024, + "bomb source stays small: {} bytes", + raw.len() + ); + let r = parse_frontmatter_yaml(&raw); + assert!( + is_rl(&r), + "alias re-visits must be counted toward the budget: {r:?}" + ); +} + +#[test] +fn c8_merge_key_is_a_plain_key_and_bomb_is_budgeted() { + // `<<` is NOT applied as a merge key by serde_yaml_ng::from_str:: — it is a + // literal key. A `b: {<<: [*a, *a, ...]}` document is therefore just an alias vector, + // and it is bounded by the node budget like any other. + let anchor: Vec = (0..2000).map(|i| format!(" k{i}: {i}")).collect(); + let refs = vec!["*a"; 200].join(", "); + let raw = format!("a: &a\n{}\nb:\n <<: [{refs}]\n", anchor.join("\n")); + let r = parse_frontmatter_yaml(&raw); + assert!(is_rl(&r), "`<<` alias bomb must be budgeted: {r:?}"); +} + +// ── Realistic sub-1 MiB alias bombs (the DoS repros) ──────────────────────────── + +#[test] +fn c7_realistic_sub_mib_alias_bomb_rejected_by_node_budget() { + // The real-world memory-amplification repro: n = m = 100 000 aliases. The SOURCE is + // ~700 KB — comfortably UNDER the 1 MiB size cap, so the size cap does NOT catch it + // (asserted first, per PF-013). The node budget is what rejects it, and it does so + // fast: each `*a` re-expands the anchored 100 000-element sequence, so the budget + // trips long before the full ~10^10-node tree could be materialised. + let raw = alias_bomb(100_000, 100_000, 0); + assert!( + raw.len() < MAX_FRONTMATTER_SIZE, + "the bomb must be under the size cap so the NODE BUDGET (not the size cap) is \ + what rejects it: {} bytes", + raw.len() + ); + let start = std::time::Instant::now(); + let r = parse_frontmatter_yaml(&raw); + let elapsed = start.elapsed(); + assert!( + is_rl(&r), + "sub-1 MiB alias bomb must be a resource limit: {r:?}" + ); + let m = msg(&r); + assert!( + m.contains(&MAX_FRONTMATTER_NODES.to_string()), + "message must name the node limit: {m}" + ); + assert!( + !m.contains("*a"), + "message must not echo the bomb content: {m}" + ); + // Non-materialisation proof: rejecting the bomb must not take the wall-clock cost of + // building the full expansion (pre-fix this OOM'd / hung). + assert!( + elapsed < std::time::Duration::from_secs(10), + "node budget must reject without materialising the tree, took {elapsed:?}" + ); +} + +// ── C-9: alias bomb reached through an @extends base module ────────────────────── + +#[test] +fn c9_alias_bomb_in_extends_base_is_rejected() { + // The bomb lives in a BASE module's frontmatter, reached through the resolver when a + // child `@extends` it. The base's frontmatter is parsed via the same choke point, so + // the resource limit fires before any inheritance work. + use std::collections::HashMap; + let bomb = alias_bomb(100_000, 100_000, 0); + let modules = HashMap::from([ + ( + "main.mds".to_string(), + "@extends \"./base.mds\"\n@block persona:\nhi\n@end\n".to_string(), + ), + ("base.mds".to_string(), format!("---\n{bomb}---\nBASE\n")), + ]); + let r = crate::compile_virtual(modules, "main.mds", None); + assert!( + is_rl(&r), + "a bomb in an @extends base's frontmatter must be rejected: {r:?}" + ); +} + +#[test] +fn c9c_legit_anchor_alias_still_compiles() { + // Positive control (PF-013): a legitimate anchor/alias used in the body must still + // work — the bounds must not over-reject valid YAML aliasing. `b: *a` resolves to the + // `[1, 2]` sequence and renders in the body. + let doc = "---\na: &a [1, 2]\nb: *a\n---\n{{b}}\n"; + let out = crate::compile_str(doc) + .expect("legit anchor/alias must compile") + .into_markdown() + .expect("markdown output"); + assert!( + out.contains("1, 2"), + "legit alias must render the anchored sequence: {out:?}" + ); +} + +// ── Depth pins (existing serde / Value::from_yaml behaviour, via check_str) ────── + +#[test] +fn c10a_flow_depth_64_ok() { + let r = crate::check_str(&wrap(&nested_flow_seq(64))); + assert!(r.is_ok(), "64-deep flow nest must be accepted: {r:?}"); +} + +#[test] +fn c10b_flow_depth_65_value_nesting() { + let r = crate::check_str(&wrap(&nested_flow_seq(65))); + assert!(is_yaml(&r), "65-deep must be a YAML error: {r:?}"); + assert!( + msg(&r).contains("value nesting exceeds maximum depth of 64"), + "expected value-nesting message: {}", + msg(&r) + ); +} + +#[test] +fn c10c_flow_depth_127_value_nesting_not_recursion() { + let r = crate::check_str(&wrap(&nested_flow_seq(127))); + assert!(is_yaml(&r), "127-deep must be a YAML error: {r:?}"); + let m = msg(&r); + assert!( + m.contains("value nesting exceeds maximum depth of 64"), + "expected value-nesting message at 127: {m}" + ); + assert!( + !m.contains("recursion limit"), + "127 is below serde's recursion limit; must not mention it: {m}" + ); +} + +#[test] +fn c10d_flow_depth_128_recursion_limit() { + let r = crate::check_str(&wrap(&nested_flow_seq(128))); + assert!(is_yaml(&r), "128-deep must be a YAML error: {r:?}"); + assert!( + msg(&r).contains("recursion limit exceeded"), + "expected serde recursion-limit message at 128: {}", + msg(&r) + ); +} + +#[test] +fn c10e_block_map_depth_128_value_nesting() { + let r = crate::check_str(&wrap(&nested_block_map(128))); + assert!( + is_yaml(&r), + "128-deep block map must be a YAML error: {r:?}" + ); + assert!( + msg(&r).contains("value nesting exceeds maximum depth of 64"), + "expected value-nesting message: {}", + msg(&r) + ); +} + +#[test] +fn c10f_tagged_nest_shallow_ok() { + // Tagged values exercise visit_enum and, while comfortably under the depth-64 cap, + // are accepted. Each nested `!t [...]` adds two levels (tag + sequence), so depth 20 + // and 30 map to value depths 40 and 60 — both under 64. + assert!(crate::check_str(&wrap(&tagged_nest(20))).is_ok()); + assert!(crate::check_str(&wrap(&tagged_nest(30))).is_ok()); +} + +#[test] +fn c10g_flow_depth_10000_hits_flow_guard() { + // Reconciled for the pre-parse flow-depth guard (#162): a 10 000-deep flow nest now + // trips the guard (threshold 1024 < 10 000) BEFORE serde_yaml_ng's 128-frame recursion + // limit, so it surfaces as a resource limit rather than a YAML recursion error. The + // shallow depth pins above (<= 128) never reach the guard and stay YAML errors. + let r = crate::check_str(&wrap(&nested_flow_seq(10_000))); + assert!( + is_rl(&r), + "very deep flow nest must trip the flow-depth guard (resource limit): {r:?}" + ); +} + +// ── C-12: pre-parse flow-nesting depth guard (#162) ───────────────────────────── + +/// T0-style pin: the guard threshold is 1024. +#[test] +fn c12_flow_depth_guard_constant_pin() { + assert_eq!(MAX_FRONTMATTER_FLOW_DEPTH, 1024); +} + +/// One over the guard: rejected as a resource limit, the message names the flow-depth +/// limit (never echoes the raw bracket run), and it returns FAST — proving the guard runs +/// pre-parse, before libyaml's O(depth^2) flow scanner. +#[test] +fn c12a_flow_depth_over_guard_is_resource_limit_and_fast() { + let raw = nested_flow_seq(MAX_FRONTMATTER_FLOW_DEPTH + 1); + let start = std::time::Instant::now(); + let r = parse_frontmatter_yaml(&raw); + let elapsed = start.elapsed(); + assert!( + is_rl(&r), + "flow nest over the guard must be a resource limit: {r:?}" + ); + let m = msg(&r); + assert!( + m.contains("flow nesting exceeds maximum depth"), + "message must name the flow-depth limit: {m}" + ); + // The message must never echo the (adversarial) raw input — no bracket-run leakage. + assert!( + !m.contains('[') && !m.contains(']') && !m.contains('{') && !m.contains('}'), + "resource-limit message must not echo raw frontmatter bytes: {m}" + ); + assert!( + elapsed < std::time::Duration::from_secs(1), + "guard must reject pre-parse (fast), took {elapsed:?}" + ); +} + +/// Positive control (PF-013): one BELOW the guard (1023 deep) is NOT rejected by the +/// guard. It passes to serde_yaml_ng, which rejects it at its 128-frame recursion limit — +/// proving the guard threshold does not mask the parser's own errors, and that the +/// largest scan the guard admits is still cheap (returns FAST). +#[test] +fn c12b_flow_depth_under_guard_reaches_serde_recursion_limit_and_fast() { + let raw = nested_flow_seq(MAX_FRONTMATTER_FLOW_DEPTH - 1); + let start = std::time::Instant::now(); + let r = parse_frontmatter_yaml(&raw); + let elapsed = start.elapsed(); + assert!( + is_yaml(&r), + "just under the guard must reach serde (a YAML error), not the guard: {r:?}" + ); + assert!( + msg(&r).contains("recursion limit exceeded"), + "expected serde recursion-limit message just under the guard: {}", + msg(&r) + ); + assert!( + elapsed < std::time::Duration::from_secs(1), + "the largest scan the guard admits must be cheap, took {elapsed:?}" + ); +} + +/// The guard bounds DEPTH, not width: a wide-but-shallow flow list (depth 1, 20 000 +/// elements) must be accepted. +#[test] +fn c12c_wide_flow_list_is_accepted_guard_is_depth_not_width() { + let raw = format!("k: [{}]\n", vec!["1"; 20_000].join(", ")); + let r = parse_frontmatter_yaml(&raw); + assert!(r.is_ok(), "wide shallow flow list must be Ok: {r:?}"); +} + +// ── Billion-laughs: serde's repetition limit, not our budget ──────────────────── + +#[test] +fn c11_billion_laughs_repetition_limit() { + let r = parse_frontmatter_yaml(&laughs(5, 7)); + assert!( + is_yaml(&r), + "billion-laughs must surface as a YAML error: {r:?}" + ); + assert!( + msg(&r).contains("repetition limit exceeded"), + "expected serde repetition-limit message: {}", + msg(&r) + ); +} + +#[test] +fn c11c_shallow_laughs_ok() { + let r = parse_frontmatter_yaml(&laughs(5, 3)); + assert!(r.is_ok(), "shallow amplification must parse: {r:?}"); +} + +// ── Parity with the unbounded parser for non-attack inputs ────────────────────── + +/// The budgeted parser must produce the SAME value (Ok) or the SAME error message (Err) +/// as `serde_yaml_ng::from_str::` for any input that is not an amplification +/// attack. For errors, the message is compared byte-for-byte against the raw parser's. +fn assert_parity(raw: &str) { + let bounded = parse_frontmatter_yaml(raw); + let plain = serde_yaml_ng::from_str::(raw); + match (&bounded, &plain) { + (Ok(b), Ok(p)) => assert_eq!(b, p, "value parity for {raw:?}"), + (Err(MdsError::YamlError { message }), Err(e)) => { + assert_eq!(*message, e.to_string(), "error-message parity for {raw:?}"); + } + _ => panic!( + "parity mismatch for {raw:?}: bounded={bounded:?}, plain_is_err={}", + plain.is_err() + ), + } +} + +#[test] +fn c_par_matches_unbounded_parser() { + // syntax error, duplicate key, tagged scalar/sequence, `<<` literal key, non-mapping. + assert_parity("a: [\n"); + assert_parity("a: 1\na: 2\n"); + assert_parity("x: !Thing [1, 2]\n"); + assert_parity("x: !!str 5\n"); + assert_parity("a: &a 1\n<<: *a\n"); + assert_parity("just text\n"); +} + +#[test] +fn c_par_duplicate_key_message_is_byte_identical() { + let raw = "a: 1\na: 2\n"; + let err = parse_frontmatter_yaml(raw).expect_err("duplicate key must be a YAML error"); + let MdsError::YamlError { message } = err else { + panic!("duplicate key must be a YAML error, got {err:?}"); + }; + let plain_msg = serde_yaml_ng::from_str::(raw) + .unwrap_err() + .to_string(); + assert!( + plain_msg.contains("duplicate entry with key \"a\""), + "sanity: raw parser reports duplicate-key text: {plain_msg}" + ); + assert_eq!( + message, plain_msg, + "duplicate-key message must be byte-identical" + ); +} + +#[test] +fn c_par_merge_key_is_literal() { + // `<<` is not merged; it is a literal string key alongside the anchored value. + let v = parse_frontmatter_yaml("a: &a 1\n<<: *a\n").expect("parses"); + let map = v.as_mapping().expect("mapping"); + assert!( + map.contains_key(serde_yaml_ng::Value::String("<<".to_string())), + "`<<` must be preserved as a literal key: {map:?}" + ); +} diff --git a/crates/mds-core/tests/api_surface.rs b/crates/mds-core/tests/api_surface.rs index 0f4a1ee0..302e182f 100644 --- a/crates/mds-core/tests/api_surface.rs +++ b/crates/mds-core/tests/api_surface.rs @@ -1161,6 +1161,69 @@ fn compile_max_file_size_still_enforced() { ); } +// ── #162: MAX_FILE_SIZE backstop at the string funnels ──────────────────────── + +fn cwd_str() -> String { + std::env::current_dir() + .unwrap() + .to_str() + .unwrap() + .to_owned() +} + +/// Every string-source funnel that runs a resolve pass rejects a source over +/// MAX_FILE_SIZE with a resource limit — the binding-layer size guards are bypassed by +/// these core entry points (PF-004), so the check must live in core. +#[track_caller] +fn assert_oversize_rejected(name: &str, r: Result<(), MdsError>) { + assert!( + matches!(r, Err(MdsError::ResourceLimit { .. })), + "{name} must reject an oversize source with a resource limit, got {r:?}" + ); + let msg = r.unwrap_err().to_string(); + assert!( + msg.contains("too large"), + "{name} rejection must mention the size limit, got: {msg}" + ); +} + +#[test] +fn string_funnels_reject_oversize_source() { + let over = " ".repeat((MAX_FILE_SIZE + 1) as usize); + + assert_oversize_rejected("compile_str", mds::compile_str(&over).map(|_| ())); + assert_oversize_rejected("check_str", mds::check_str(&over)); + assert_oversize_rejected( + "lint_str_with", + mds::lint_str_with(&over, None, None, &LintConfig::default()).map(|_| ()), + ); + assert_oversize_rejected( + "compile_str_with_deps_opts", + mds::compile_str_with_deps_opts(&over, None, None, mds::CompileOptions::default()) + .map(|_| ()), + ); + + let mut cache = ModuleCache::new(); + let mut warnings = vec![]; + assert_oversize_rejected( + "ModuleCache::resolve_source", + cache + .resolve_source(&over, &cwd_str(), &HashMap::new(), &mut warnings) + .map(|_| ()), + ); +} + +/// PF-013 at-cap Ok twin: a source of EXACTLY MAX_FILE_SIZE bytes is accepted. +#[test] +fn string_funnel_accepts_source_at_cap() { + let at = " ".repeat(MAX_FILE_SIZE as usize); + let r = mds::check_str(&at); + assert!( + r.is_ok(), + "a source at exactly MAX_FILE_SIZE must be accepted: {r:?}" + ); +} + // ── Lint API surface pins (L-API-1/2/3/4/5) ────────────────────────────────── /// L-API-1: lint_* function signatures mirror check_* conventions. diff --git a/crates/mds-core/tests/yaml_funnel.rs b/crates/mds-core/tests/yaml_funnel.rs new file mode 100644 index 00000000..d54f1326 --- /dev/null +++ b/crates/mds-core/tests/yaml_funnel.rs @@ -0,0 +1,296 @@ +//! Funnel guard (#162): every frontmatter YAML parse in `mds-core` production code must +//! go through the single budgeted choke point in `resolver/frontmatter.rs`. +//! +//! A resource limit enforced on only one parse site is silently bypassed by any other +//! `serde_yaml_ng::from_str` / `serde_yaml_ng::Deserializer` call (PF-004 shape). This +//! test converts "did we remember every parse site?" into a machine-checked invariant: +//! the two parse entry points may appear ONLY in `resolver/frontmatter.rs`, anywhere else +//! in `src/**` is a violation. +//! +//! Scope: `crates/mds-core/src/**`, excluding `*_tests.rs` files and `#[cfg(test)]` +//! modules (test code legitimately calls the raw parser to assert parity). Comment and +//! string-literal text is masked so a needle named in rustdoc or a string does not count. + +use std::path::{Path, PathBuf}; + +/// The two `serde_yaml_ng` parse entry points that must be funnelled. +const NEEDLES: &[&str] = &["serde_yaml_ng::from_str", "serde_yaml_ng::Deserializer"]; + +/// The one file allowed to contain them (path suffix, OS-agnostic). +const CHOKE_POINT: &[&str] = &["resolver", "frontmatter.rs"]; + +#[test] +fn yaml_parse_sites_are_funnelled() { + let src_dir = Path::new(env!("CARGO_MANIFEST_DIR")).join("src"); + let files = rust_files(&src_dir); + + // Non-vacuity: the source tree was actually walked. + assert!( + files.len() >= 10, + "non-vacuity: expected to walk at least 10 source files under {}, found {}", + src_dir.display(), + files.len() + ); + + let mut violations: Vec = Vec::new(); + let mut scanned_needles = 0usize; + let mut choke_point_seen = false; + + for file in &files { + if file + .file_name() + .and_then(|n| n.to_str()) + .is_some_and(|n| n.ends_with("_tests.rs")) + { + continue; + } + let is_choke = is_choke_point(file); + let raw = std::fs::read_to_string(file).expect("source must be readable"); + let code = strip_cfg_test_mods(&mask_comments_and_strings(&raw)); + + for needle in NEEDLES { + if code.contains(needle) { + scanned_needles += 1; + if is_choke { + choke_point_seen = true; + } else { + violations.push(format!(" {}: contains `{needle}`", rel(&src_dir, file))); + } + } + } + } + + // Non-vacuity: the scanner really found the needles at the choke point — otherwise a + // needle-blind bug would make this test pass by finding nothing anywhere. + assert!( + choke_point_seen, + "non-vacuity: expected `resolver/frontmatter.rs` to contain a YAML parse entry \ + point, found none — the funnel guard is not actually matching anything" + ); + assert!( + scanned_needles >= 1, + "non-vacuity: no needles matched at all" + ); + + assert!( + violations.is_empty(), + "YAML parse sites outside the frontmatter choke point ({} found).\n{}\n\n\ + Route the parse through `resolver::frontmatter::parse_frontmatter_yaml` instead, \ + or (for test code) move it into a `*_tests.rs` file or a `#[cfg(test)]` module.", + violations.len(), + violations.join("\n") + ); +} + +fn is_choke_point(path: &Path) -> bool { + let comps: Vec = path + .components() + .map(|c| c.as_os_str().to_string_lossy().into_owned()) + .collect(); + comps.len() >= CHOKE_POINT.len() && comps[comps.len() - CHOKE_POINT.len()..] == CHOKE_POINT[..] +} + +fn rel(base: &Path, path: &Path) -> String { + path.strip_prefix(base) + .unwrap_or(path) + .to_string_lossy() + .into_owned() +} + +fn rust_files(dir: &Path) -> Vec { + let mut out = Vec::new(); + let mut stack = vec![dir.to_path_buf()]; + // Bounded: the source tree is finite and acyclic (read_dir does not traverse symlinks). + while let Some(d) = stack.pop() { + let Ok(rd) = std::fs::read_dir(&d) else { + continue; + }; + for entry in rd.flatten() { + let p = entry.path(); + let Ok(ft) = entry.file_type() else { continue }; + if ft.is_dir() { + stack.push(p); + } else if ft.is_file() && p.extension().and_then(|e| e.to_str()) == Some("rs") { + out.push(p); + } + } + } + out.sort(); + out +} + +/// Replace the CONTENT of line comments, block comments, string literals and char +/// literals with spaces, preserving overall length and newlines. This keeps needles that +/// appear in rustdoc or inside a string from counting, and makes the brace matching in +/// [`strip_cfg_test_mods`] safe (no braces hide inside strings/comments). +fn mask_comments_and_strings(src: &str) -> String { + let b = src.as_bytes(); + let mut out = vec![b' '; b.len()]; + // Copy newlines through so line structure is preserved. + for (i, &c) in b.iter().enumerate() { + if c == b'\n' { + out[i] = b'\n'; + } + } + let mut i = 0usize; + while i < b.len() { + // Line comment. + if b[i] == b'/' && b.get(i + 1) == Some(&b'/') { + while i < b.len() && b[i] != b'\n' { + i += 1; + } + continue; + } + // Block comment (not nested — Rust allows nesting, but the codebase does not rely + // on it here; a needle would have to sit inside a nested comment to escape). + if b[i] == b'/' && b.get(i + 1) == Some(&b'*') { + i += 2; + while i < b.len() && !(b[i] == b'*' && b.get(i + 1) == Some(&b'/')) { + i += 1; + } + i = (i + 2).min(b.len()); + continue; + } + // Raw string: r"...", r#"..."#, r##"..."##, ... — only when `r` starts a token. + if b[i] == b'r' + && matches!(b.get(i + 1), Some(&b'"') | Some(&b'#')) + && !prev_is_ident_byte(b, i) + { + let mut hashes = 0usize; + let mut j = i + 1; + while b.get(j) == Some(&b'#') { + hashes += 1; + j += 1; + } + if b.get(j) == Some(&b'"') { + j += 1; + // Scan to a `"` followed by exactly `hashes` `#`. + while j < b.len() { + if b[j] == b'"' && (0..hashes).all(|k| b.get(j + 1 + k) == Some(&b'#')) { + j += 1 + hashes; + break; + } + j += 1; + } + i = j.min(b.len()); + continue; + } + } + // Normal string literal. + if b[i] == b'"' { + let mut j = i + 1; + while j < b.len() { + if b[j] == b'\\' { + j += 2; + continue; + } + if b[j] == b'"' { + j += 1; + break; + } + j += 1; + } + i = j.min(b.len()); + continue; + } + // Char literal `'x'` / `'\n'` — but not a lifetime `'a`. Only treat as a literal + // when a closing quote follows within a few bytes. + if b[i] == b'\'' { + let mut j = i + 1; + let mut closed = false; + let mut steps = 0; + while j < b.len() && steps < 8 { + if b[j] == b'\\' { + j += 2; + steps += 1; + continue; + } + if b[j] == b'\'' { + closed = true; + j += 1; + break; + } + j += 1; + steps += 1; + } + if closed { + i = j.min(b.len()); + continue; + } + } + // Not a masked region: copy the byte through verbatim. + out[i] = b[i]; + i += 1; + } + String::from_utf8(out).expect("masking preserves UTF-8 boundaries on ASCII delimiters") +} + +/// Remove every `#[cfg(test)]`-guarded item from already-masked source. Handles both an +/// inline `mod name { ... }` (brace-matched) and a `mod name;` / `#[path=...] mod name;` +/// declaration (whose body is an external `*_tests.rs`, excluded separately). Individual +/// `#[cfg(test)] fn ...` items are left in place; the codebase places test parse calls +/// inside `mod tests` or `*_tests.rs`, both of which are handled. +fn strip_cfg_test_mods(code: &str) -> String { + let mut result = code.to_string(); + // Bounded: at most one removal per `#[cfg(test)]` occurrence, and each iteration + // either removes a block or blanks the attribute, so no occurrence is seen twice. + while let Some(attr) = result.find("#[cfg(test)]") { + // Find the next `mod` keyword after the attribute. + let after = attr + "#[cfg(test)]".len(); + let Some(mod_rel) = result[after..].find("mod ") else { + // No module follows (e.g. a cfg(test) fn) — blank the attribute and move on. + result.replace_range(attr..after, &" ".repeat(after - attr)); + continue; + }; + let mod_start = after + mod_rel; + // Look for the block-open `{` or the statement-terminating `;`. + let brace = result[mod_start..].find('{'); + let semi = result[mod_start..].find(';'); + match (brace, semi) { + (Some(bo), semi_opt) if semi_opt.is_none_or(|s| bo < s) => { + let open = mod_start + bo; + if let Some(close) = match_brace(&result, open) { + result.replace_range(attr..=close, ""); + } else { + result.replace_range(attr..open, ""); + } + } + (_, Some(so)) => { + // `mod name;` declaration — remove the attribute + statement. + let end = mod_start + so + 1; + result.replace_range(attr..end, ""); + } + _ => { + result.replace_range(attr..after, &" ".repeat(after - attr)); + } + } + } + result +} + +/// Is the byte before `i` part of an identifier (so `r` is a suffix, not a raw-string +/// prefix, e.g. `str` / `for`)? +fn prev_is_ident_byte(b: &[u8], i: usize) -> bool { + i > 0 && (b[i - 1].is_ascii_alphanumeric() || b[i - 1] == b'_') +} + +/// Index of the `}` matching the `{` at `open` in already-masked text. +fn match_brace(text: &str, open: usize) -> Option { + let b = text.as_bytes(); + let mut depth = 0i32; + let mut i = open; + while i < b.len() { + match b[i] { + b'{' => depth += 1, + b'}' => { + depth -= 1; + if depth == 0 { + return Some(i); + } + } + _ => {} + } + i += 1; + } + None +} diff --git a/crates/mds-napi/__test__/index.spec.mjs b/crates/mds-napi/__test__/index.spec.mjs index ca8397d2..61af7302 100644 --- a/crates/mds-napi/__test__/index.spec.mjs +++ b/crates/mds-napi/__test__/index.spec.mjs @@ -1725,3 +1725,83 @@ describe('unknown rule name warning (AC-224 D8)', () => { ); }); }); + +// ── Frontmatter YAML DoS bounds (#162) ───────────────────────────────────────── +// +// The alias bomb is the sub-1 MiB memory-amplification repro (n = m = 100 000, +// ~700 KB source): under the 1 MiB size cap, so the node budget is what rejects it — +// fast, without materialising the tree. Deep flow-nesting is the second axis, rejected +// by the pre-parse flow-depth guard. Every rejection throws with `err.code === +// 'mds::resource_limit'` and never echoes the raw hostile bytes (`*a`, the sentinel). +describe('resource limits', () => { + const MAX_FRONTMATTER_SIZE = 1 << 20; // 1 MiB + const wrapFm = (y) => `---\n${y}---\nHi\n`; + // Builders mirror the core/CLI ones via string repetition (no MiB-scale literals). + const aliasBomb = (n, m) => + 'a: &a [' + 'x, '.repeat(n) + ']\nb: [' + '*a, '.repeat(m) + ']\n'; + const fmOfSize = (bytes) => { + const PREFIX = 'k: ZZSENTINELZZ'; + return PREFIX + 'x'.repeat(bytes - PREFIX.length - 1) + '\n'; + }; + const nestedFlowSeq = (d) => 'k: ' + '['.repeat(d) + 'x' + ']'.repeat(d) + '\n'; + + const bombDoc = wrapFm(aliasBomb(100000, 100000)); + + /// Assert `fn()` throws a resource-limit error whose message does not echo hostile bytes. + const assertResourceLimit = (fn, label) => { + assert.throws( + fn, + (err) => { + assert.ok(err instanceof Error, `${label}: should be an Error`); + assert.equal(err.code, 'mds::resource_limit', `${label}: got code ${err.code}`); + assert.ok(!err.message.includes('*a'), `${label}: message echoes bomb: ${err.message}`); + assert.ok( + !err.message.includes('ZZSENTINELZZ'), + `${label}: message echoes sentinel: ${err.message}`, + ); + return true; + }, + label, + ); + }; + + test('R-4: compile rejects the sub-1 MiB alias bomb as a resource limit', () => { + assert.ok(bombDoc.length < MAX_FRONTMATTER_SIZE + 64, 'bomb source stays near ~700 KB'); + assertResourceLimit(() => compile(bombDoc), 'R-4 compile bomb'); + }); + + test('R-5: check rejects the alias bomb as a resource limit', () => { + assertResourceLimit(() => check(bombDoc), 'R-5 check bomb'); + }); + + test('R-6: lint rejects the alias bomb as a resource limit', () => { + assertResourceLimit(() => lint(bombDoc), 'R-6 lint bomb'); + }); + + test('R-7: frontmatter one byte over the size cap is a resource limit', () => { + assertResourceLimit( + () => compile(wrapFm(fmOfSize(MAX_FRONTMATTER_SIZE + 1))), + 'R-7 over-cap', + ); + }); + + test('R-8: frontmatter exactly at the size cap is accepted', () => { + // At-cap control (PF-013): exactly 1 MiB of frontmatter compiles. + const result = compile(wrapFm(fmOfSize(MAX_FRONTMATTER_SIZE))); + assert.equal(result.kind, 'markdown', 'at-cap frontmatter must compile'); + }); + + test('R-9: a legitimate anchor/alias still compiles', () => { + // Positive control (PF-013): the bounds must not over-reject valid YAML aliasing. + const result = compile('---\na: &a [1, 2]\nb: *a\n---\n{{b}}\n'); + assert.equal(result.kind, 'markdown'); + assert.ok( + result.output.endsWith('1, 2\n'), + `legit alias must render the anchored sequence; got: ${result.output}`, + ); + }); + + test('napi-DEEP: a deep flow nest trips the flow-depth guard', () => { + assertResourceLimit(() => compile(wrapFm(nestedFlowSeq(2000))), 'napi-DEEP'); + }); +}); diff --git a/crates/mds-python/tests/test_concurrency.py b/crates/mds-python/tests/test_concurrency.py index d66e2447..2a78d03a 100644 --- a/crates/mds-python/tests/test_concurrency.py +++ b/crates/mds-python/tests/test_concurrency.py @@ -144,6 +144,11 @@ def _compile_once(_: int) -> m.CompileResult: # ── PERF4: the off-GIL panic path is reserved for true panics ──────────────────── +# The sub-1 MiB frontmatter alias bomb (#162): our bounds must reject it as an MdsError +# (mds::resource_limit), never as mds::internal — proving the node budget returns a clean +# error rather than panicking under the off-GIL catch_unwind path. +_ALIAS_BOMB = "---\na: &a [" + "x, " * 100000 + "]\nb: [" + "*a, " * 100000 + "]\n---\nHi\n" + MALFORMED = [ "{", "}", @@ -160,6 +165,7 @@ def _compile_once(_: int) -> m.CompileResult: "———\nnot: yaml: [\n———\n", "@extends\n", "{fn(((((}\n", + _ALIAS_BOMB, ] diff --git a/crates/mds-python/tests/test_limits.py b/crates/mds-python/tests/test_limits.py index 45c81661..76a035bc 100644 --- a/crates/mds-python/tests/test_limits.py +++ b/crates/mds-python/tests/test_limits.py @@ -9,6 +9,34 @@ import markdown_script as m MAX = 10 * 1024 * 1024 # MAX_SOURCE_SIZE (10 MiB) +FM_MAX = 1 << 20 # MAX_FRONTMATTER_SIZE (1 MiB) + + +# ── Frontmatter YAML DoS bomb builders (#162) ──────────────────────────────────── +# Built by string repetition so there is no MiB-scale literal in the test source. + + +def wrap_fm(yaml: str) -> str: + return f"---\n{yaml}---\nHi\n" + + +def alias_bomb(n: int, mm: int) -> str: + # a: &a [x, x, ...(n)] / b: [*a, *a, ...(mm)] — each *a re-expands the anchor. + return "a: &a [" + "x, " * n + "]\nb: [" + "*a, " * mm + "]\n" + + +def fm_of_size(nbytes: int) -> str: + prefix = "k: ZZSENTINELZZ" # sentinel proves the message never echoes content + return prefix + "x" * (nbytes - len(prefix) - 1) + "\n" + + +def nested_flow_seq(d: int) -> str: + return "k: " + "[" * d + "x" + "]" * d + "\n" + + +# The sub-1 MiB memory-amplification repro: ~700 KB source (under the size cap), so the +# node budget is what rejects it. +BOMB_DOC = wrap_fm(alias_bomb(100_000, 100_000)) # ── L1: >10 MiB source → resource_limit (all string inputs) ───────────────────── @@ -174,3 +202,57 @@ def test_v3_nested_json_values_accepted() -> None: vars={"cfg": {"flag": True, "items": [1, 2], "n": None}}, ) assert r.output == "true 1, 2\n" + + +# ── L4: frontmatter YAML DoS bounds across every entry point (#162) ────────────── + + +@pytest.mark.parametrize( + "fn", + [m.compile, m.check, m.lint, m.scan_imports], + ids=["compile", "check", "lint", "scan_imports"], +) +def test_l4_alias_bomb_is_resource_limit(fn) -> None: # type: ignore[no-untyped-def] + # The bomb rejects with a resource limit on every surface, and the message never + # echoes the raw hostile bytes (`*a` or the sentinel). + with pytest.raises(m.MdsError) as ei: + fn(BOMB_DOC) + assert ei.value.code == "mds::resource_limit" + assert "*a" not in ei.value.message + assert "ZZSENTINELZZ" not in ei.value.message + + +def test_l4_frontmatter_over_size_cap_is_resource_limit() -> None: + with pytest.raises(m.MdsError) as ei: + m.compile(wrap_fm(fm_of_size(FM_MAX + 1))) + assert ei.value.code == "mds::resource_limit" + assert "ZZSENTINELZZ" not in ei.value.message + + +def test_l4_frontmatter_at_size_cap_is_accepted() -> None: + # At-cap control (PF-013): exactly 1 MiB of frontmatter compiles. + r = m.compile(wrap_fm(fm_of_size(FM_MAX))) + assert isinstance(r.output, str) + + +def test_l4_legit_anchor_alias_still_compiles() -> None: + # Positive control (PF-013): valid YAML aliasing must not be over-rejected. + r = m.compile("---\na: &a [1, 2]\nb: *a\n---\n{{b}}\n") + assert r.output.endswith("1, 2\n") + + +def test_l4_scan_imports_stays_lenient_for_frontmatter_syntax_errors() -> None: + # scan_imports swallows a plain frontmatter YAML SYNTAX error and still returns the + # body imports — proving the resource-limit propagation above is specific to the + # bound, not blanket strictness. (Bomb propagation is covered by the parametrised + # scan_imports case.) + src = '---\nimports: [\n---\n@import "./x.mds"\nHi\n' + assert m.scan_imports(src) == ["./x.mds"] + + +def test_l4_deep_flow_nest_is_resource_limit() -> None: + # The second DoS axis: a deep flow nest (depth 2000 > 1024) trips the pre-parse + # flow-depth guard. + with pytest.raises(m.MdsError) as ei: + m.compile(wrap_fm(nested_flow_seq(2000))) + assert ei.value.code == "mds::resource_limit" diff --git a/crates/mds-python/tests/test_parity.py b/crates/mds-python/tests/test_parity.py index af01b9e4..38a659f4 100644 --- a/crates/mds-python/tests/test_parity.py +++ b/crates/mds-python/tests/test_parity.py @@ -115,6 +115,16 @@ def test_par2_live_cli_messages_byte_parity( # Same inputs the napi __test__ suite asserts on must yield the same core error # code through the Python binding (messages/spans come from the shared core). +# Frontmatter YAML bomb builders (#162), mirroring the napi R-4 assertion. Built by +# string repetition so there is no MiB-scale literal here. +def wrap(yaml: str) -> str: + return f"---\n{yaml}---\nHi\n" + + +def alias_bomb(n: int, mm: int) -> str: + return "a: &a [" + "x, " * n + "]\nb: [" + "*a, " * mm + "]\n" + + NAPI_ERROR_PARITY = [ ("mds::undefined_var", lambda: m.compile("Hello {{undefined_var}}!\n")), ("mds::syntax", lambda: m.compile("@import\n")), @@ -125,6 +135,9 @@ def test_par2_live_cli_messages_byte_parity( # Frontmatter sets count to Number(3); comparing against string literal "3" is a # cross-type comparison → mds::type_mismatch (#152). ("mds::type_mismatch", lambda: m.compile('---\ncount: 3\n---\n@if count == "3":\nx\n@end\n')), + # The sub-1 MiB alias bomb (napi R-4): the node budget rejects it with a resource + # limit through the Python binding too — same shared-core code. + ("mds::resource_limit", lambda: m.compile(wrap(alias_bomb(100000, 100000)))), ] diff --git a/crates/mds-wasm/tests/web.rs b/crates/mds-wasm/tests/web.rs index 96aa82c2..0c29a4fa 100644 --- a/crates/mds-wasm/tests/web.rs +++ b/crates/mds-wasm/tests/web.rs @@ -567,6 +567,126 @@ fn scan_imports_handles_all_directive_forms() { assert_eq!(js_array_str(&result, 4), "./e.mds"); } +// ── Frontmatter YAML DoS bounds (#162) ───────────────────────────────────────── +// +// The alias bomb is the sub-1 MiB memory-amplification repro (n = m = 100 000, +// ~700 KB source): under the 1 MiB size cap, so the node budget is what rejects it — +// fast, without materialising the tree. Deep flow-nesting is the second axis, rejected +// by the pre-parse flow-depth guard. Every rejection surfaces `code == +// "mds::resource_limit"` and never echoes the raw hostile bytes (`*a`, the sentinel). + +const MAX_FRONTMATTER_SIZE: usize = 1 << 20; // 1 MiB + +fn wrap_fm(y: &str) -> String { + format!("---\n{y}---\nHi\n") +} + +/// An alias-fan-out bomb: `a: &a [x, ...(n)]`, `b: [*a, ...(m)]`. Built by repetition so +/// there is no MiB-scale literal in the test source. +fn alias_bomb(n: usize, m: usize) -> String { + let xs = vec!["x"; n].join(", "); + let refs = vec!["*a"; m].join(", "); + format!("a: &a [{xs}]\nb: [{refs}]\n") +} + +fn fm_of_size(bytes: usize) -> String { + const PREFIX: &str = "k: ZZSENTINELZZ"; + format!("{PREFIX}{}\n", "x".repeat(bytes - PREFIX.len() - 1)) +} + +fn nested_flow_seq(d: usize) -> String { + format!("k: {}x{}\n", "[".repeat(d), "]".repeat(d)) +} + +/// The rejection message must never echo the adversarial content. +fn assert_no_echo(err: &JsValue, label: &str) { + let m = get_str(err, "message"); + assert!(!m.contains("*a"), "{label}: message echoes bomb: {m}"); + assert!( + !m.contains("ZZSENTINELZZ"), + "{label}: message echoes sentinel: {m}" + ); +} + +fn bomb_doc() -> String { + wrap_fm(&alias_bomb(100_000, 100_000)) +} + +#[wasm_bindgen_test] +fn w1_compile_alias_bomb_is_resource_limit() { + let err = mds_wasm::compile(&bomb_doc(), JsValue::NULL).unwrap_err(); + assert_eq!(get_str(&err, "code"), "mds::resource_limit"); + assert_no_echo(&err, "W-1 compile bomb"); +} + +#[wasm_bindgen_test] +fn w2_check_alias_bomb_is_resource_limit() { + let err = mds_wasm::check(&bomb_doc(), JsValue::NULL).unwrap_err(); + assert_eq!(get_str(&err, "code"), "mds::resource_limit"); + assert_no_echo(&err, "W-2 check bomb"); +} + +#[wasm_bindgen_test] +fn w3_lint_alias_bomb_is_resource_limit() { + let err = mds_wasm::lint(&bomb_doc(), JsValue::NULL).unwrap_err(); + assert_eq!(get_str(&err, "code"), "mds::resource_limit"); + assert_no_echo(&err, "W-3 lint bomb"); +} + +#[wasm_bindgen_test] +fn w4_scan_imports_alias_bomb_propagates_resource_limit() { + let err = mds_wasm::scan_imports(&bomb_doc()).unwrap_err(); + assert_eq!(get_str(&err, "code"), "mds::resource_limit"); + assert_no_echo(&err, "W-4 scan_imports bomb"); +} + +#[wasm_bindgen_test] +fn w5_frontmatter_over_cap_rejected_at_cap_accepted() { + // Over the 1 MiB cap → resource limit, message never echoes the frontmatter content. + let over = mds_wasm::compile( + &wrap_fm(&fm_of_size(MAX_FRONTMATTER_SIZE + 1)), + JsValue::NULL, + ) + .unwrap_err(); + assert_eq!(get_str(&over, "code"), "mds::resource_limit"); + assert_no_echo(&over, "W-5 over-cap"); + + // At-cap control (PF-013): exactly 1 MiB of frontmatter compiles. + let at = mds_wasm::compile(&wrap_fm(&fm_of_size(MAX_FRONTMATTER_SIZE)), JsValue::NULL); + assert!(at.is_ok(), "at-cap frontmatter must compile: {at:?}"); +} + +#[wasm_bindgen_test] +fn w6_scan_imports_stays_lenient_for_frontmatter_syntax_errors() { + // Positive control: a plain frontmatter YAML SYNTAX error (never-closed flow) is + // swallowed by scan_imports, which still returns the body imports — proving the + // resource-limit propagation in W-4 is specific to the bound, not blanket strictness. + let source = "---\nimports: [\n---\n@import \"./x.mds\"\nHi\n"; + let result = mds_wasm::scan_imports(source).expect("syntax error in FM must be lenient"); + assert_eq!(js_array_len(&result), 1); + assert_eq!(js_array_str(&result, 0), "./x.mds"); +} + +#[wasm_bindgen_test] +fn w_legit_anchor_alias_still_compiles() { + // Positive control (PF-013): valid YAML aliasing must not be over-rejected. + let result = + mds_wasm::compile("---\na: &a [1, 2]\nb: *a\n---\n{{b}}\n", JsValue::NULL).unwrap(); + let output = get_str(&result, "output"); + assert!( + output.ends_with("1, 2\n"), + "legit alias must render the anchored sequence; got: {output}" + ); +} + +#[wasm_bindgen_test] +fn w_deep_flow_nest_is_resource_limit() { + // The second DoS axis: a deep flow nest (depth 2000 > 1024) trips the pre-parse + // flow-depth guard. + let err = mds_wasm::compile(&wrap_fm(&nested_flow_seq(2000)), JsValue::NULL).unwrap_err(); + assert_eq!(get_str(&err, "code"), "mds::resource_limit"); +} + // ── Template inheritance tests (@extends / @block) ─────────────────────────── /// Build a modules option for inheritance tests. diff --git a/spec.md b/spec.md index 6c409e85..618ea15c 100644 --- a/spec.md +++ b/spec.md @@ -53,6 +53,22 @@ config: - Object values support dot-notation field access: `{{config.key}}`, `{{a.b.c}}` - Objects cannot be interpolated directly; access a specific field instead +**Resource limits:** + +| Limit | Value | +|-------|-------| +| `MAX_FRONTMATTER_SIZE` | 1 MiB per frontmatter block | +| `MAX_FRONTMATTER_NODES` | 200,000 YAML nodes per block, counted during parsing so alias expansion stops before the tree is built | +| `MAX_FRONTMATTER_FLOW_DEPTH` | 1024 levels of flow-collection (`[`/`{`) nesting, checked before the parser scans (bounds libyaml's O(depth²) flow scan) | +| `MAX_FILE_SIZE` | 10 MiB per source, including strings passed to the string APIs | + +Exceeding one of these returns `mds::resource_limit` (exit 3). YAML the parser +itself refuses — syntax errors, duplicate keys, nesting deeper than 128 levels, +its alias-repetition limit — returns `mds::yaml`, as does value nesting deeper +than 64 levels. These bounds are pinned by the `parse_frontmatter_yaml` tests in +`crates/mds-core/src/resolver/frontmatter.rs` and by the +`yaml_parse_sites_are_funnelled` test. + --- ### 4.2 Interpolation @@ -1296,7 +1312,7 @@ Maximum config file size: 1 MB. | `0` | Success | | `1` | Template error (syntax, undefined variable, arity mismatch, recursion, etc.) | | `2` | I/O or file-system error (file not found, not an MDS file, I/O failure) | -| `3` | Resource limit exceeded (output too large, too many iterations, message count exceeds `MAX_MESSAGE_COUNT` (10,000), or cumulative message content exceeds 50 MB) | +| `3` | Resource limit exceeded (output too large, too many iterations, message count exceeds `MAX_MESSAGE_COUNT` (10,000), cumulative message content exceeds 50 MB, or frontmatter over 1 MiB, over 200,000 YAML nodes, or flow-nesting deeper than 1024 levels) | **`mds lint`** (see §7.5 for per-code meaning):