Skip to content

fix: harden terminal-escape rendering across all MdsError variants (CWE-150) - #253

Open
dean0x wants to merge 41 commits into
mainfrom
fix/esc-injection-176
Open

fix: harden terminal-escape rendering across all MdsError variants (CWE-150)#253
dean0x wants to merge 41 commits into
mainfrom
fix/esc-injection-176

Conversation

@dean0x

@dean0x dean0x commented Jul 24, 2026

Copy link
Copy Markdown
Owner

Summary

  • CWE-150 (ESC-injection hardening): Raw C0/DEL/C1 control bytes in MDS source files could reach terminal stderr and JS/Python/WASM API error messages; closes Harden terminal-escape rendering across all MdsError variants (CWE-150) #176
  • Sanitization applied at all serialization and diagnostic-render boundaries — MdsError::serialize(), LintResult::to_canonical_json() (incl. "file" key), CompileResult::to_canonical_json() warnings, CLI eprint_error, and render_diag_human() — with a PF-014 redesign that sanitizes render inputs, not the rendered frame, so miette's own colour codes survive on TTY
  • T-1..T-15 test vectors across all surfaces: Rust unit tests, CLI e2e, output unit tests, and binding tests for napi (T-12/T-13, E-10..E-13), WASM (T-15, F5/F6), Python (T-14), and @mdscript/mds universal package (T-11)

Changes

Production (A-series)

Change File Description
A1 crates/mds-core/src/error.rs serialize() applies sanitize_control_chars to message and help — all three binding layers inherit this; adds MdsError::display_sanitized() public API
A2 crates/mds-cli/src/output.rs PF-014 redesign: neutralize_source_for_render sanitizes source excerpt inputs byte-length-preservingly; render_error_sanitized sanitizes message/help pre-render; eprint_error delegates to it
A3 crates/mds-cli/src/watch.rs Replace 11 bare eprintln!("{e:?}") calls with eprint_error(e) — watch mode now sanitizes on the same path as build/check/compile/fmt
A4 crates/mds-cli/src/lint.rs Route render_diag_human to sanitize inputs (message, help, source) before miette renders — not post-hoc on the rendered frame
B4 crates/mds-core/src/lint/diagnostic.rs to_canonical_json() sanitizes message, help, and the "file" group key at the JSON serialization boundary

Tests (B-series)

  • T-1..T-3 (error_tests.rs): serialize sanitizes ESC/DEL/C1 in message and help
  • T-4 (diagnostic.rs): to_canonical_json() sanitizes diagnostic message with ESC byte
  • T-5..T-9 (cli_lint.rs): CLI e2e — single-file, directory, stdin, DEL+C1, JSON output path
  • T-10a/b/c (output.rs): neutralize_source_for_render — C0 removal, byte-length preservation (span safety), and colour-code passthrough
  • T-11 (packages/mds/__test__/error.spec.mjs): @mdscript/mds universal package — U-E10 (ESC in error), U-E11 (DEL in error), U-E12 (C1/NEL in lint), U-E-DIFF (native/WASM parity)
  • T-12/T-13 + E-10..E-13 (crates/mds-napi/__test__/index.spec.mjs): napi — T-12/E-10 error path ESC, T-13/E-11 lint path ESC, E-12 error path DEL, E-13 lint path C1
  • T-14 (crates/mds-python/tests/test_errors.py): Python — T-14/E11 error-path, T-14/E12 typed LintDiagnostic.message and to_json()
  • T-15 (crates/mds-wasm/tests/web.rs): WASM — T-15/F5 error path, T-15/F6 lint path via lintVirtual

Stale reference fixes (D)

Fixed 4 stale "issue #5" cross-refs → "issue #176" in cli_build.rs, cli_lint.rs, cli_commands.rs, cli_fmt.rs.

Breaking Changes

Behavioral change (necessary for security): err.message, err.help, and lint diagnostic messages delivered via the JS/Python/WASM API now carry \uXXXX 6-char literals for any embedded C0/DEL/C1 bytes instead of the raw bytes. Consumers that grep for exact control byte sequences in error messages will need to update — they should be checking for sanitized literals anyway.

Bundler packages (zero plugin-side changes): @mdscript/bundler-utils (formatMdsError) and all plugin/loader packages (@mdscript/vite-plugin, @mdscript/rollup-plugin, @mdscript/webpack-loader, @mdscript/rspack-loader) inherit sanitized error messages transitively via core MdsError::serialize() — no changes were required in any plugin or bundler package.

Related Issues

Validation Results

Numbers as of original submission; resolve round not re-validated here — orchestrator re-validates before merge.

Check Result
cargo fmt --all --check PASS
cargo clippy --workspace --all-targets -- -D warnings PASS
cargo nextest run --workspace 1918/1918
cargo test --doc --workspace 36/36
WASM release binary (mds_wasm_bg.wasm) 816,800 bytes ≤ 850,000 budget
npm test --workspaces 96 pass
pytest crates/mds-python/tests -q 220 pass
Examples smoke: mds lint examples/linting/ exits 2 as designed (stress-test build OK)
Adversarial ANSI smoke stderr contains literal ^[, zero raw 0x1B bytes

Reviewer Focus Areas

  • A1 (error.rs:serialize) — verify that sanitize_control_chars is called on both message (from self.to_string()) and help (from Diagnostic::help(self)), and that span byte offsets are NOT touched (lines 826-832)
  • A4 (lint.rs:render_diag_human) — PF-014 redesign: verify that message/help and the source excerpt bytes are sanitized before miette renders; no post-hoc pass over the rendered frame; verify both with_source_code and plain Report::from branches
  • T-9 redesignserde_yaml_ng rejects raw ESC bytes in YAML frontmatter keys with mds::yaml; the test now verifies the JSON output wire contains no raw control bytes regardless of which path fires (regression guard); programmatic lint-message sanitization is covered by T-4
  • WASM budget — 816,800 bytes ≤ 850,000 byte limit; the sanitize_control_chars addition is <1KB

Review-resolution addendum

The resolve round (commits after 25e5e25) addressed the PF-014 defect identified in review:

PF-014 render redesign: The original A2 sanitized the already-rendered miette frame (post-hoc over the full string). miette's fancy renderer emits real SGR colour codes into that string, so the sanitizer was escaping miette's own ANSI sequences on TTY — mangling legitimate colour output on benign input, and misaligning caret columns by up to ~10 chars per control byte (because miette computed carets against raw source before the 1→6 char expansion). The fix moves sanitization to the renderer's inputs:

  • neutralize_source_for_render(src) — byte-length-preserving pass: C0/DEL → ? (U+003F, same byte width), C1 → NBSP (U+00C2 U+00A0, same 2-byte width as UTF-8 C1). Span offsets computed by miette stay accurate.
  • Message and help strings are sanitized to \uXXXX literals (same as the wire format) before being attached to the miette Report.
  • The rendered frame itself is passed through unchanged — miette's SGR codes survive on TTY.

New public APIs added:

  • MdsError::display_sanitized() -> String — returns the sanitized Display form; safe for terminal output. The raw Display impl is preserved with a documented unsafety contract.
  • neutralize_source_for_render(src: &str) -> Cow<str> — public for testing and downstream CLI tools.

Additional hardening in this round:

  • sanitize_control_chars returns Cow<'_, str> (fast path: no allocation when input is clean).
  • CompileResult::to_canonical_json() warnings and emit_warnings sanitized.
  • Python LintResult sanitization moved to constructor (eliminates double-sanitize on the wire-JSON path).
  • CLI status-line paths (filename display in Clean: … / Fixed: … / error lines) use a safe_path helper.
  • Strict cross-surface assertions and T-label renumbering (T-1..T-15, no gaps; napi uses T-12/T-13 + E-10..E-13; WASM uses T-15 F5/F6; Python uses T-14; universal package uses T-11 with U-E10..U-E-DIFF).

Fixes #176

Escalation-decision addendum (2026-07-26)

The 2026-07-25 review escalated 5 findings that were escape-map / wire-contract decisions, not bugs. All were decided by the owner and implemented on this branch.

Finding Decision Outcome
security-4 WIDEN Escape class extended to all 12 Unicode Bidi_Control members (U+061C, U+200E/F, U+202A–E, U+2066–9) plus U+2028/2029 and U+FEFF — Trojan Source / CVE-2021-42574
security-9 ESCAPE \n ON WIRE ONLY Wire surfaces escape newline (CWE-117 log forging); the CLI human render keeps real newlines so multi-line frames still render
security-10 + reliability-9 ACCEPT + DOCUMENT Escaping is one-way, lossy and non-injective; consumers MUST NOT un-escape. Round-tripping is a permanent non-goal (spec §7.5)
security-11 TTY-GATED NEUTRALIZE --fix --diff preview neutralizes control bytes on a terminal, stays byte-faithful when piped so diffs remain patch-applicable

Governing rule (re-ratified)

The four-boundary enumeration was superseded by a per-field rule: untrusted identifiers and filenames are WIRE-escaped on every surface that renders a diagnostic, human output included; prose (message/help bodies) stays HUMAN. A filename is never legitimately multi-line, so preserving \n in one only enables status-line forgery; a diagnostic body legitimately is multi-line.

Owner-approved scope additions

  • Raw-ESC on the CLI error pathMdsError message text reached stderr unescaped. Fixed at the eprint_error choke-point via a SanitizedReport wrapper that covers both MdsError and the CLI's own miette!() errors, and forwards an owned sanitized cause graph (the previous debug_assert!-only guard was compiled out in release — PF-005).
  • CI-enforced print disciplinecrates/mds-cli/tests/print_discipline.rs fails the build if any print macro in mds-cli/src interpolates a value that is not a sanitizer call. Three consecutive review rounds each found a new bare eprintln! after the previous was fixed; this converts an unbounded reviewer search into a bounded, enforced invariant. It traces let bindings one hop, refuses to resolve non-let binders, and fails closed on anything untraceable.

Declared residuals — specified, not hidden

  1. MdsError message bodies interpolate untrusted text as prose, so a \n survives inside the rendered miette frame. The forged text is -prefixed and indented, so it cannot masquerade as a bare status line. Not closed: there are 118 MdsError::*(format!(…)) construction sites, and fixing a handful would leave the claim false at the rest.
  2. Functional path references — Source Map v3 file/sources and CompileResult.dependencies are emitted verbatim. Escaping them would point the map at a path that does not exist, breaking resolution. Consumers MUST treat embedded paths as untrusted (spec §7.5, "Carve-out: functional path references").
  3. Five allowlisted eprint_warning sites whose safety rests on mds-core producer discipline that a CLI-side lexical guard cannot verify across the crate boundary. The one reachable producer is now covered by a test; the other two interpolate parser-validated identifiers.

Verification

cargo nextest 1980 (from 1943), 37 doctests, 229 pytest, 100 napi, 265 universal JS, 53 wasm-pack; clippy -D warnings and fmt --check clean. WASM 822,137 bytes vs the 850,000 CI guard.

Four independent adversarial alignment passes were run; each found and closed a claim the code had not yet earned.

Note: this PR also fixes a defect currently live on main — the pre-fix binary sanitized the rendered miette frame, so benign syntax errors rendered as literal [31m noise on any colour terminal. CI never saw it because CLI tests pin NO_COLOR=1.

dean0x added 6 commits July 25, 2026 01:21
…WE-150 / issue #176)

Sanitize C0/DEL/C1 control bytes at every serialization and render boundary
so raw ESC bytes in source can never reach stderr or the JS/Python/WASM API.

Changes (A-series: production, B-series: tests, D: stale refs):

A1 — error.rs serialize(): apply sanitize_control_chars to message and help;
     all three binding layers (napi/wasm/python) inherit this sanitization.
A2 — output.rs: extract render_error_sanitized(report) → String (testable
     choke-point); eprint_error delegates to it.
A3 — watch.rs: replace 11 bare eprintln!("{e:?}") calls with eprint_error(e);
     watch mode now sanitizes on the same path as build/check/compile/fmt.
A4 — lint.rs render_diag_human: build report in both branches, route both
     through crate::output::eprint_error for whole-frame sanitization.

B1 — error_tests.rs: T-1..T-3 (ESC/DEL/C1 in serialize message+help)
B2 — cli_lint.rs: T-4..T-9 (single-file/dir/stdin/DEL+C1/JSON output path)
B3 — output.rs tests: T-10 render_error_sanitized with source-frame report
B4 — binding tests T-11..T-15:
       error.spec.mjs (mds universal): T-12/U-E10 error path
       index.spec.mjs (napi): T-13a error path, T-13b lint-path YAML guard
       test_errors.py (python): T-14/E11 error path, T-14/E12 lint YAML guard
       web.rs (wasm): T-15/F5 error path, T-15/F6 lint-path YAML guard

D — fix 4 stale "issue #5" cross-refs → "issue #176":
      cli_build.rs:1091, cli_lint.rs:1554, cli_commands.rs:612, cli_fmt.rs:1152

Note: YAML rejects raw C0 bytes in frontmatter keys (serde_yaml_ng), so the
lint-path binding tests (T-13b/E12/F6) verify the YAML-error output path
rather than unused-variable message sanitization; the latter is covered by
the T-4 unit test in diagnostic.rs which constructs LintDiagnostic directly.

fix/esc-injection-176 (TASK_ID)
Closes #176
…#176]

Change #2 (AC-F4): apply sanitize_control_chars at the Python pyclass
population site (LintResult.files getter, ~line 775 of mds-python/src/lib.rs)
as defense-in-depth (PF-004/PF-007). The upstream to_canonical_json() already
sanitizes; this second guard ensures LintDiagnostic.as_json()/to_dict() can
never re-introduce raw C0/DEL/C1 bytes even if a future code path bypasses
the JSON round-trip.

test: replace vacuous E12 test with vector-1 lint_virtual scenario
- Use lint_virtual with ESC byte (U+001B) in module name to trigger
  duplicate-import rule that embeds the raw path in message
- Assert LintDiagnostic.message contains no raw control bytes (typed attr)
- Assert sanitized \\u001B literal present (explicit evidence of sanitization)
- Assert to_dict()["message"] == .message (cross-surface parity guard, PF-007)
- Proven working: pytest 220 pass, 0 fail

fix: clippy manual_range_contains in cli_lint.rs T-9 assertions
…n output.rs

- `render_error_sanitized` and `eprint_error` doc blocks were merged without
  a blank `///` separator — Rust attributed both to `render_error_sanitized`
  while `eprint_error` got no doc. Restored each function's own doc block.
- Remove `source_len` unused-intermediate and its `let _ = source_len;`
  suppression from the T-10 test; the variable served no functional purpose.
- Move `_assert_no_control_chars` helper before its first caller (test_e11)
  and replace the duplicate inline 6-line loop with a call to the helper,
  matching the pattern already used by test_e12.

No behaviour changes; 1918/1918 tests pass.
]

T-4 (to_canonical_json_sanitizes_diagnostic_message) now covers the
same behavior and confirms sanitization happens. The deleted test
contradicted shipped behavior — it passed only via its
raw_msg.contains('\x1B') || raw_msg.contains("\\u001B") disjunct,
meaning it never verified the old "raw message preserved" claim.

Per repo quality rule "leave the end-state, not the transition":
remove the residue test entirely.
… vector [#176]

Previous T-13b (napi) and F6 (wasm) used lint() with a YAML frontmatter
key containing a raw ESC byte. serde_yaml_ng rejects that vector with
mds::yaml, making the Ok-branch dead code -- no diagnostic message was
ever produced, so the absence assertions were vacuous.

Replace both with the proven Python E12 pattern: lintVirtual with a
module NAME containing U+001B (not rejected upstream), imported twice
so duplicate-import fires and embeds the raw path in its message.

Assertions now verify:
  (1) No raw C0/DEL/C1 bytes in any diagnostic message.
  (2) Sanitized \\u001B literal IS present (positive, non-vacuous).
  (3) Result shape: version 1, duplicate-import rule present.

Both new tests pass immediately since the sanitization was already
shipped -- they are evidence tests confirming the behavior is reachable.
The describe-block preamble at lines 1198-1199 described the OLD test vector
(frontmatter with unused-variable), but the actual T-13b test uses lintVirtual
with a module name containing U+001B imported twice to trigger duplicate-import.
Updated preamble to match the real test behavior described in the accurate
inner comment at lines 1235-1237.

@dean0x dean0x left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review: ESC-injection hardening (PR #253 / issue #176)

Recommendation: HOLD — 4 blocking defects require fixes before merge.


Findings Summary

# Severity Reviewers Finding
1 BLOCKING 5 (security 98%, regression 98%, reliability 92%, testing 90%, docs 85%) Post-render ANSI/caret desynchronization — inline at output.rs:496
2 BLOCKING 5 (security 95%, architecture 90%, reliability 85%, python 96%, docs 90%) files[].file left unsanitized — inline at diagnostic.rs:327
3 BLOCKING 2 (testing 95%, security) T-9 fully vacuous, passes pre-fix binary — inline at cli_lint.rs:1795
4 BLOCKING 3 (regression 95%, docs 97%, compliance 95%) Missing CHANGELOG entry for breaking security change — see below
8 BLOCKING 1 (python 97%) Python typed-vs-wire parity break — inline at lib.rs:780
5 SHOULD FIX 4 (complexity 93%, rust 93%, docs 96%, architecture 95%) Stale/contradictory doc blocks at diagnostic.rs:123 and :135 — inline at diagnostic.rs:7
6 SHOULD FIX 2 (consistency 92%, testing 85%) Lowercase \u001b fallback is dead code — inline at index.spec.mjs:1272
7 SHOULD FIX 1 (complexity 92%) C1 byte-range scan over raw UTF-8 bytes — inline at cli_lint.rs:1827

Finding #4 — BLOCKING: Missing CHANGELOG entry (summary-only; no file in diff)

No CHANGELOG entry exists for this PR despite it being a breaking behavioral change. Inputs that previously echoed raw control bytes verbatim in --format json output now emit \uXXXX literals. Any consumer that depended on raw bytes (e.g., byte-scans or exact string matching) silently breaks. This satisfies the project's "breaking behavioral change" threshold.

The compliance and documentation reviewers both flag that the project CHANGELOG has an [Unreleased] section and this change must appear in it before merge. The regression reviewer notes that the CHANGELOG is the only human-readable record linking CWE-150 hardening to the wire-format impact.

Fix: Add an entry to CHANGELOG.md under [Unreleased] such as:

### Security
- `mds lint --format json` now emits `\uXXXX` 6-char literals in `message` and `help`
  fields instead of raw C0/DEL/C1 control bytes (CWE-150 / issue #176). This is a
  wire-format change: consumers that byte-scan these fields must update their patterns.

Additional should-fix items (>=80% confidence, single-reviewer, not consolidated above)

  • Architecture (90%): CompileResult::to_canonical_json warnings[] field carries raw strings — the module doc "ALL boundaries" claim is still false after this PR for compile-error surfaces.
  • Architecture (90%): Five sibling eprint_error-like sites hand-roll eprintln!("+sanitize instead of calling the new eprint_error()` helper — PF-004 duplication that will drift.
  • Rust (95%): Display for MdsError still emits raw control bytes via fmt::Display while serialize() sanitizes — an unsafe ergonomic default; any .to_string() caller gets raw bytes.
  • Rust (90%): render_diag_human deep-clones the full fix_edits payload into the miette struct — miette only reads labels and help; the clone is pure waste on every human-render call.
  • Rust (85%): sanitize_control_chars returns String; a Cow<'_, str> fast-path would skip allocation on clean inputs (which are the overwhelmingly common case).
  • Performance (90%): render_diag_human clones the full source file per diagnostic — O(diagnostics × file_size); a shared Arc<str> would reduce this to O(1) per diagnostic.

Lower-confidence suggestions (60–79% — discussion only, not blocking)

  • Security (75%): \n is in the allowed pass-through set; a consumer that writes diagnostic messages to a log file without further escaping is vulnerable to log injection. Consider whether \n should be \u000A in the sanitization output for CLI-output-only use cases.
  • Security (70%): sanitize_control_chars is not injective — a downstream that unescapes \uXXXX literals recovers live ESC. Document the "sanitize for terminal rendering, not for round-trip" scope in the function doc.
  • Architecture (70%): render_error_sanitized naming — render_report_for_terminal or render_report_sanitized would communicate the terminal-context intent more precisely.
  • Architecture (68%): --diff / --check output echoes raw file paths; ESC-bearing paths would still survive to stderr on those code paths. Arguably by design (not a lint-output surface), but worth documenting the scope decision.
  • Consistency (65%): The vector-label accounting scheme (T-5a/T-5b vs T-5/T-6 numbering) is inconsistent between comments and the tracking doc; T-11 is referenced but absent.

Verified clean — do not re-litigate in follow-up review

The following axes were independently verified by multiple reviewers and are confirmed safe:

Axis Verdict Evidence
Span / offset integrity (PF-012) CLEAN sanitize_control_chars never touches span / fix_edits; T-4 pins byte offsets end-to-end
Idempotency CLEAN \uXXXX output contains no control chars; double-pass is a safe no-op
PF-005 (debug_assert-only guard) CLEAN All sanitization is unconditional runtime code; no debug_assert! guarding any security invariant
debug-panics feature CLEAN Zero Cargo.toml / package.json changes; debug-panics absent from all binding defaults
Supply-chain CLEAN Zero dependency additions or version bumps
Bundler zero-change CLEAN formatMdsError / bundler-utils treats message/help as opaque strings; spans taken from numbers
--fix machinery CLEAN diag_to_edits consumes LintDiagnostic structs directly, never re-parses JSON
CRLF sources CLEAN miette strips \r during line-splitting; CRLF inputs do not affect span accuracy
watch.rs A3 CLEAN 11 sites uniformly use eprint_error; import added; zero divergence
--format json shape CLEAN Keys, ordering, span, fix_edits byte-identical on benign input (no control bytes)
UTF-8 safety CLEAN sanitize_control_chars operates on char scalars via char_indices(); no byte-level truncation

Code review by Claude — 12 reviewers (security, architecture, performance, complexity, consistency, regression, testing, reliability, rust, python, documentation, compliance)

Comment thread crates/mds-cli/src/output.rs Outdated
/// embedded source frames (issue #176 / CWE-150). The render+sanitize pass is
/// idempotent: calling it a second time on already-sanitized output is a no-op.
pub(crate) fn render_error_sanitized(report: &miette::Report) -> String {
mds::sanitize_control_chars(&format!("{report:?}"))

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BLOCKING — Post-render sanitization breaks colour output and misaligns carets
Converging reviewers: security 98%, regression 98%, reliability 92%, testing 90%, docs 85% — 5 reviewers

render_error_sanitized formats the report with {report:?} — which on a colour TTY causes miette's GraphicalReportHandler to emit ANSI SGR sequences — then runs sanitize_control_chars over the already-rendered frame. miette's own \x1b[33m becomes literal [33m garbage. Every interactive mds lint and mds watch invocation is broken on benign input. CI stays green because piped stderr disables colour; none of the new T-5..T-9 tests use a pty or force-enable colour.

Second consequence: the post-render pass widens the source line by 5 columns per control char (1 → 6 chars) but leaves the caret row untouched, so carets misalign for any span whose error-triggering token appears after a control byte on the same source line. The comment at lint.rs:284 asserting "Span accuracy is preserved" is inverted — miette computes widths against the raw source, so post-render expansion of those bytes is exactly what breaks alignment.

Reproduced on a pty:

$ script -q /dev/null ./target/debug/mds lint plain.mds 2>&1 | head -3
[33mmds::lint::legacy-interpolation[0m   ← bare `[33m` not ANSI

Fix: sanitize untrusted inputs before miette renders them, not the rendered output. For the source string passed to NamedSource, use a byte-length-preserving substitution (1 byte → 1 byte, e.g. replace each control byte with 0x20 or a Unicode replacement char) so span offsets and caret columns stay exact. For the whole-frame path, either drop it entirely (the input is already clean) or make it colour-aware (allow ESC [m through, escape everything else).

Additionally: add a T-10 variant that places a span after a control byte and asserts caret column alignment, and add one in-process colour-forced test so the interactive TTY render path has coverage.

Co-Authored-By: Claude noreply@anthropic.com

Comment thread crates/mds-core/src/lint/diagnostic.rs Outdated
"severity": diag.severity.to_string(),
"message": diag.message,
"help": diag.help,
"message": sanitize_control_chars(&diag.message),

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BLOCKING — files[].file left unsanitized while sibling message/help fields are sanitized
Converging reviewers: security 95%, architecture 90%, reliability 85%, python 96%, docs 90% — 5 reviewers

This hunk sanitizes message and help, but the per-file group key approximately 14 lines below this hunk (line 341 in the current file, outside the diff):

// line 341 (not in this hunk — the gap is the defect):
"file": file,

...is emitted raw. serde_json escapes C0 bytes (< 0x20) in JSON strings, but C1 bytes (U+0080–U+009F) are ≥ 0x20 and emitted verbatim. A filename containing U+009B (CSI) reaches --format json output as a live control byte. Any consumer that reads json["files"][i]["file"] and writes it to a terminal gets a live CSI sequence.

This directly falsifies the module doc seven lines above this file's start that now claims sanitization at "ALL serialization and render boundaries."

Verified:

$ mds lint $'fo�o.mds' --format json | python3 -c   "import json,sys; r=json.load(sys.stdin); print(repr(r['files'][0]['file']))"
'fo�o.mds'   ← raw C1 in parsed JSON value

Same gap at crates/mds-python/src/lib.rs:800 (file: file_key unsanitized in LintFileReport construction) and LintFileReport.file typed getter.

Fix:

// diagnostic.rs line ~341
.map(|(file, diagnostics)| {
    serde_json::json!({
        "file": mds::sanitize_control_chars(&file),
        "diagnostics": diagnostics,
    })
})

Propagate to mds-python/src/lib.rs:800 for cross-surface parity. Add a T-10 variant with a control char in the filename asserting files[0].file is sanitized.

Co-Authored-By: Claude noreply@anthropic.com

/// diagnostic message that embeds the error context is sanitized before serialization.
/// ESC-in-diagnostic-message sanitization is unit-tested by T-4 (`to_canonical_json`).
#[test]
fn lint_json_hostile_source_output_contains_no_raw_control_bytes() {

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BLOCKING — T-9 is fully vacuous; --format json wire format has zero non-trivial ESC coverage
Converging reviewers: testing 95%, security — 2 reviewers

T-9 cannot fail against the pre-fix binary for three independent reasons:

  1. serde_yaml_ng rejects raw ESC upstream. The YAML frontmatter key source: with a raw ESC byte causes a parse error in serde_yaml_ng; the error envelope has a fixed generic message ("failed to parse") with no user-supplied content. sanitize_control_chars is never called on this path.

  2. Gate 3 binds None and silently skips. The error envelope from a YAML parse failure does not populate json["files"], so the if let Some(files_arr) = json.get("files") check at Gate 3 binds None and the entire per-diagnostic assertion is skipped.

  3. Gate 2 tests serde_json behaviour, not sanitize_control_chars. The byte scan for C0 bytes (< 0x20) only confirms that serde_json escapes control bytes in JSON strings — a guarantee that was always true. It proves nothing about what sanitize_control_chars does or whether it runs.

Verified: T-9 passes when run against the pre-fix binary.

Fix — use the duplicate-import vector with a module name containing U+001B (known to produce a user-visible message):

#[cfg(unix)]
#[test]
fn lint_json_esc_in_diagnostic_message_is_sanitized() {
    let dir = tempfile::tempdir().unwrap();
    fs::write(dir.path().join("fo\u{1B}o.mds"), b"hi\n").unwrap();
    let main = dir.path().join("main.mds");
    fs::write(&main, b"@import \"./fo\x1Bo.mds\"\n@import \"./fo\x1Bo.mds\"\nhi\n").unwrap();
    let out = mds_bin().env("NO_COLOR", "1")
        .args(["lint", "--format", "json"]).arg(&main)
        .stdout(std::process::Stdio::piped()).output().unwrap();
    let json: serde_json::Value = serde_json::from_slice(&out.stdout).expect("valid JSON");
    let files = json["files"].as_array().expect("files must be present: non-vacuity guard");
    let msg = files[0]["diagnostics"][0]["message"].as_str()
        .expect("duplicate-import diagnostic must exist");
    assert!(!msg.contains('\x1B'), "raw ESC in message: {msg:?}");
    assert!(msg.contains("\\u001B"), "sanitized literal missing: {msg:?}");
}

Also change Gate 3 from if let Some(...) to .expect(...) so future shape changes surface as test failures rather than silent skips.

Co-Authored-By: Claude noreply@anthropic.com

Comment thread crates/mds-core/src/lint/diagnostic.rs Outdated
//! intact for `LintResult::to_canonical_json()` (typed serialization is safe; C0/C1
//! bytes in JSON string values are legal and the consumer can handle them). Apply
//! `sanitize_control_chars` only at the CLI human-render step (mds-cli/src/lint.rs).
//! **Sanitization discipline**: `sanitize_control_chars` is applied at ALL

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SHOULD FIX — Two LintDiagnostic doc blocks 116–128 lines below directly contradict this rewritten module header
Converging reviewers: complexity 93%, rust 93%, docs 96%, architecture 95% — 4 reviewers

This PR correctly rewrote the module doc (lines 7–17) to say "ALL serialization and render boundaries." But the LintDiagnostic struct doc at lines 123 and 135 of this file was not updated and now says the opposite:

Line 123 (struct doc eprintln! usage example):

/// miette at the CLI boundary: `eprintln!("{:?}", miette::Report::from(diag))`

This is verbatim the anti-pattern that output.rs:501–503 now forbids with a MUST NOT comment, and exactly the CWE-150 vulnerability this PR closes. A contributor following this example re-introduces the vulnerability.

Line 135 (struct doc sanitization note):

/// **Sanitization**: apply `sanitize_control_chars` at the CLI render boundary only.

The word "only" encodes the pre-PR contract. This PR changed it to "ALL boundaries." The two docs are now contradictory.

These are the highest-traffic docs on this type (cargo doc / IDE hover on LintDiagnostic).

Fix — update lines 123 and 135 in this file:

// line 123: replace the eprintln! example with:
/// Render **only** via `mds_cli::output::eprint_error` — never a bare
/// `eprintln!("{report:?}")`, which leaks raw control bytes (issue #176 / CWE-150).

// line 135: replace the sanitization note with:
/// **Sanitization**: see module-level "Sanitization discipline" — applied at every
/// output boundary; raw bytes preserved here so span offsets and fix-edit byte
/// ranges stay accurate. Do not call `sanitize_control_chars` in constructors.

Co-Authored-By: Claude noreply@anthropic.com

Comment thread crates/mds-napi/__test__/index.spec.mjs Outdated
const hasSanitizedEsc = allDiags.some(
(d) =>
typeof d.message === 'string' &&
(d.message.includes('\\u001B') || d.message.includes('\\u001b')),

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SHOULD FIX — Lowercase \u001b fallback is dead code that weakens the assertion
Converging reviewers: consistency 92%, testing 85% — 2 reviewers

sanitize_control_chars uses {:04X} (uppercase hex). The || d.message.includes('\\u001b') branch (lowercase) is unreachable against the current implementation and any future implementation constrained by the test suite — because 13 other assertions in this PR (T-5a, T-5b, T-6, T-7, error-path tests across all three binding surfaces) correctly pin uppercase \\u001B.

The lowercase fallback means a casing regression at diagnostic.rs:426 (the format string) would fail 13 tests and silently pass this assertion. The same dead-code fallback appears at:

  • crates/mds-python/tests/test_errors.py:268: "\\u001B" in d.message or "\\u001b" in d.message
  • crates/mds-wasm/tests/web.rs:894: .contains("\\u001B") || m.contains("\\u001b")

Fix in all three files — drop the lowercase branch:

// napi T-13b (this file, line ~1272):
const hasSanitizedEsc = allDiags.some(
  (d) => typeof d.message === 'string' && d.message.includes('\\u001B'),
);
# Python test_errors.py:268:
esc_in_messages = [d for d in all_diags if "\\u001B" in d.message]
// WASM web.rs:894:
let has_sanitized = all_messages.iter().any(|m| m.contains("\\u001B"));

Co-Authored-By: Claude noreply@anthropic.com

Comment thread crates/mds-cli/tests/cli_lint.rs Outdated
for (i, byte) in stdout_str.bytes().enumerate() {
let is_c0 = byte < 0x20 && byte != b'\t' && byte != b'\n';
let is_del = byte == 0x7F;
let is_c1 = (0x80..=0x9F).contains(&byte);

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SHOULD FIX — Byte-level C1 range check on raw UTF-8 bytes causes spurious failures on ordinary non-ASCII text
Converging reviewers: complexity 92% — 1 reviewer

stdout_str.bytes() filtered by (0x80..=0x9F).contains(&byte) tests UTF-8 bytes, not codepoints. A C1 codepoint (U+0080–U+009F) is encoded as two bytes: 0xC2 0x80 through 0xC2 0x9F. But 0x80 is also the continuation byte of (em dash, E2 80 94), ' (right single quote, E2 80 99), and hundreds of other common characters. The moment any diagnostic message contains an em dash — pervasive in this repo's message strings — Gate 2 fires a false positive.

The same PR correctly uses char_indices() with codepoint comparison in crates/mds-wasm/tests/web.rs:790. Two implementations have already diverged within a single PR. Gate 3 at line 1845 repeats the same byte-level predicate.

Fix — hoist a codepoint-predicate helper into crates/mds-cli/tests/common/mod.rs:

pub fn assert_no_raw_control_chars(s: &str, label: &str) {
    for (i, ch) in s.char_indices() {
        let cp = ch as u32;
        let bad = (cp < 0x20 && cp != 0x09 && cp != 0x0A)
            || cp == 0x7F
            || (0x80..=0x9F).contains(&cp);
        assert!(!bad, "raw control char U+{cp:04X} at byte {i} in {label}; text: {s:?}");
    }
}

Gates 2 and 3 then collapse to:

assert_no_raw_control_chars(&stdout_str, "lint --format json stdout");

Co-Authored-By: Claude noreply@anthropic.com

Comment thread crates/mds-python/src/lib.rs Outdated
// The upstream to_canonical_json() also sanitizes, but this
// ensures as_json()/to_dict() parity for any future code path
// that bypasses the JSON round-trip (PF-004/PF-007).
message: mds::sanitize_control_chars(&json_str(d, "message")),

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BLOCKING — Python typed-vs-wire parity break violates a documented invariant (PF-007)
Converging reviewers: python 97% — 1 reviewer

message and help are sanitized when populating the typed pyclass here (lines 780/784), but LintResult.to_dict() at line 833 returns self.value verbatim via value_to_py. When self.value carries a raw control byte — which can happen via the public LintResult(canonical_dict) constructor (also the __reduce__ / unpickle path) — the typed getter and wire format diverge:

result.files()[0].diagnostics[0].message  → "...\u001B..."   (sanitized)
result.to_dict()["files"][0]["diagnostics"][0]["message"]  → "...\x1b..."  (raw)

Line 701 of this file is the class docstring that asserts the opposite: "typed getters and to_dict()/to_json() read from it so they can never diverge." test_par6_file_report_to_dict_parity_null_help_span (test_parity.py:206) is a PF-007 regression test for this invariant — it passes currently only because its fixture has no control bytes.

Additionally, LintFileReport.file at line ~800 is the field carrying raw bytes while sibling message/help are sanitized — PF-004 (alternate path bypasses control enforced on primary path) on a parallel field in the same object.

The comment on line 776–780 ("belt-and-suspenders defense-in-depth") describes the intent correctly but is applied to the wrong side of the split — sanitizing at read-time on the typed path leaves the backing store raw.

Fix: move sanitization into LintResult::new() so self.value carries sanitized bytes from the moment untrusted data enters (covers the LintResult(canonical) constructor and unpickle). Remove the sanitize calls at lines 780/784. Apply the same to LintFileReport.file at construction. Add test_e13_lintresult_from_canonical_sanitizes_message that constructs via LintResult(canonical) with a raw ESC byte and asserts both typed and to_dict() paths return the sanitized literal.

Co-Authored-By: Claude noreply@anthropic.com

dean0x and others added 9 commits July 25, 2026 18:21
…#176]

- sanitize_control_chars → Cow<'_, str>: borrowed fast-path (zero alloc)
  for clean strings; exact-capacity owned path for strings with controls.
  Escaping semantics unchanged: C0 ∖ {\\n,\\t}, DEL, C1 → uppercase \\uXXXX.
- Extract is_control_char() helper; add #[must_use] + doc examples (rust-7,
  rust-8, rust-9, complexity-8, architecture-8).
- fmt::write Result handled via .expect() (reliability-5).
- Sanitize "file" key in LintResult::to_canonical_json (security-3).
- Sanitize warnings in CompileResult::to_canonical_json and emit_warnings
  (compliance-2, security-8).
- All call sites updated with .into_owned() (mds-cli lint.rs + output.rs,
  mds-core error.rs, mds-python lib.rs).
- testing-3: to_canonical_json_sanitizes_diagnostic_message now embeds the
  hostile name in help too — non-vacuous assertion on help field.
- testing-7: sanitize_is_idempotent covers ESC, DEL, C1, NUL, empty, clean.

Co-Authored-By: Claude <noreply@anthropic.com>
python-1/architecture-4: Close the typed-vs-wire parity gap. The previous
belt-and-suspenders sanitize at the LintDiagnostic population site in
files() diverged from to_dict()/to_json() which read self.value verbatim,
breaking parity whenever self.value carried raw control bytes via the
LintResult(canonical) / pickle entry point.

Fix: add sanitize_lint_value() called in LintResult::new() so the backing
store is always sanitized before typed getters or to_dict() read it.
Revert population-site wraps to plain reads — no allocation on clean strings
(avoids PF-004, closes parity gap for PF-007).

python-5: Verify file/grouping key inherits core sanitization. After ed24492
sanitized the "file" key in to_canonical_json(), and with sanitize_lint_value()
covering new(), the files() getter reads sanitized values in all paths.

performance-4/rust-4: Belt-and-suspenders sanitize_control_chars(&json_str(...))
double-allocated on every diagnostic (json_str owns, sanitize copies). Now
plain reads; sanitize_lint_value() uses Cow so clean strings borrow (zero alloc).

python-4: Delete the stale comment at lines 775-779 that claimed the wrap
"ensures as_json()/to_dict() parity for any future bypass path" — it broke
that parity (python-1) and direct constructors were still raw.

python-2: Add test_par6_lint_result_constructor_sanitizes_typed_fields — the
regression anchor that FAILS if sanitize_lint_value() is removed from new().
E12 was vacuous for the Python wrap (to_canonical_json() did the work); test_par6
is the non-vacuous constructor-path test (avoids PF-013).

python-3: Add _assert_no_control_chars(fr.file, "LintFileReport.file") inside
the E12 for-fr loop — regression anchor for file-key sanitization.

python-6: Fix stale (b) comment placement — the comment block was inside the
for-diag loop but the (b) assertion was outside. Restructured to co-locate.

python-7: Parametrize E12 over ESC (U+001B), DEL (U+007F), and U+0085 NEL.
NEL is the reachable YAML vector (serde_yaml_ng rejects raw ESC/DEL in YAML
keys but U+0085 passes). lint_virtual uses plain Python strings so all three
reach the engine. Casing tolerance (lowercase \\u001b) removed — sanitize
emits uppercase {:04X} unconditionally.
consistency-1: Remove dead lowercase \\u001b alternatives — sanitize_control_chars
always emits uppercase {:04X}, so the OR branch was unreachable (napi T-13/E-11,
WASM F6).

consistency-2: Close the T-11 gap — renumber T-12 (universal) → T-11, T-13a
(napi error) → T-12/E-10, T-13b (napi lint) → T-13/E-11. Update KB cross-surface
anchor note accordingly.

consistency-5: Rename assertNoControlBytes → assertNoControlChars in index.spec.mjs
(helper uses charCodeAt/UTF-16 code units, not bytes).

consistency-6: Fix KB anchor — E12 is in test_errors.py (not test_lint.py), add
T-15 label for WASM, update T-11..T-15 count to "no gaps".

testing-11: Add DEL (U+007F) and U+0085 (NEL/C1) vectors to all three surfaces:
napi E-12/E-13, universal U-E11/U-E12, WASM T-15/F5-DEL + T-15/F6-C1.

complexity-3: New WASM tests use the existing assert_no_control_chars helper; no
inline predicates.

testing-6: Add U-E-DIFF differential test to packages/mds error.spec.mjs —
deepEqual of parsed canonical JSON from native and WASM backends for the same
ESC-injection input; skips gracefully when a backend is unavailable locally.
Root cause (PF-014): `render_error_sanitized` post-processed the fully-rendered
miette frame with `sanitize_control_chars`. On any colour TTY this corrupted
miette's own SGR colour codes into literal `[33m` garbage. CI was green only
because all CLI tests pin NO_COLOR=1 and pipe stderr.

Fix strategy: sanitize user-controlled INPUTS before miette renders them; never
post-process the rendered artifact.

New canonical function — `neutralize_source_for_render` (in mds-core, re-exported
to mds-cli): byte-length-preserving substitution so miette span byte-offsets and
caret columns remain valid after neutralization. C0/DEL (1-byte) → '?';
C1 U+0080–U+009F (2-byte UTF-8) → U+00A0 NBSP (2-byte).

All NamedSource creation sites now apply input-level sanitization:
- `mds-core/src/error.rs` `at()`: neutralize source + sanitize filename (PF-014)
- `mds-core/src/formatter.rs` Syntax error rebuild: same (PF-004 parallel path)
- `mds-cli/src/lint.rs` `render_diag_human`: neutralize source, sanitize filename,
  sanitize message/help, set fix_removals/fix_edits = None (architecture-3/rust-1),
  remove dead None arm with .expect() (reliability-4/testing-4)

`render_error_sanitized` simplified to `format!("{report:?}")` — no post-processing.

Tests:
- T-10 replaced by T-10a (byte-length invariant), T-10b (caret alignment with span
  AFTER control char), T-10c (colour path: miette SGR survives, hostile C1 removed)
- T-5/T-6/T-7: remove stale \\u001B literal assertions (ESC → '?' not '\\u001B')
- T-8: remove stale \\u007F/\\u0085 assertions; add NBSP positive assertion

Applying PF-013, PF-014, PF-004.
- Module doc: replace "ALL ... boundaries" with the precise closed set
  (MdsError::serialize, LintResult::to_canonical_json incl. file key,
  CompileResult::to_canonical_json, emit_warnings, Python pyclass, CLI
  input-level render); add disambiguation note distinguishing
  CompileResult::to_canonical_json (lib.rs) from LintResult::to_canonical_json
  (this file); add deliberate exclusions (raw Display, napi err.detail
  under debug-panics); list fields NOT sanitized (rule, span/fix_edits).

- LintDiagnostic doc: remove the eprintln!("{:?}") anti-pattern example;
  prescribe mds_cli::output::eprint_error as the required TTY render path;
  cite CWE-150/PF-014; state the input-level model (sanitize fields
  pre-render; do not post-process the rendered frame). Fix "CLI render
  boundary only" stale claim to defer to module doc.

- message field: append "(sanitized at output boundaries — see module docs)".

- sanitize_control_chars fn doc: replace the divergent boundary enumeration
  with a pointer to the module doc (single authoritative source; avoids
  consistency-3 divergence; fixes documentation-7 omission of serialize()).

- Doctests verified: cargo test --doc -p mds-core — 36 passed including the
  sanitize_control_chars doctest pinning uppercase \uXXXX format + idempotency.

Co-Authored-By: Claude <noreply@anthropic.com>
- Rewrites T-9 (lint_json_hostile_source_output_contains_no_raw_control_bytes)
  using a `duplicate-import` + U+0085 NEL vector so the diagnostic message
  actually carries a C1 byte to sanitize; the old ESC-in-YAML-key vector was
  vacuous (serde_yaml_ng rejects 0x1B before lint runs — PF-013 shape).
  Gates 1–3 are now fail-closed (unwrap_or_else/panic) and a positive assertion
  verifies the \\u0085 literal is present in the sanitized output (testing-9).
- Adds assert_no_control_chars(s, label) to common/mod.rs: char-based (not
  byte-based) so it doesn't false-positive on UTF-8 continuation bytes inside
  ordinary multi-byte codepoints (complexity-1, complexity-3-adjacent).
- Adds NO_COLOR=1 to mds_bin() in common/mod.rs so T-5..T-8 can use lint_path/
  lint_stdin directly without re-transcribing their bodies (complexity-2).
- Removes out.stderr.clone() in T-5..T-8; uses String::from_utf8_lossy borrow
  instead (complexity-9).
- Adds watch_esc_in_initial_compile_error_is_sanitized (T-Watch-ESC) testing
  the non-loop initial-compile-error path in run_watch_file via 500 ms sleep
  + kill; asserts stderr non-empty and ESC-free (testing-8).
All five architecture-6 hand-rolled sanitize_control_chars bypass sites
replaced with eprint_error (lint.rs:95, lint.rs:~1408, main.rs:232,
main.rs:348, build.rs:1548). Adds safe_path helper in output.rs and
applies it to every status-line path interpolation (Clean:, Fixed:,
Formatted:, Compiled to:, Source map written to:, error writing {}, etc.)
in lint.rs, fmt.rs, build.rs, and main.rs so hostile filenames containing
ANSI ESC sequences cannot inject terminal commands (CWE-150 / PF-014).

Additional hardening:
- render_error_sanitized visibility narrowed to fn (rust-3)
- eprint_error rustdoc updated: enforced single-choke-point contract
  (documentation-12)
- render_diag_human summary clarified: lists all sanitized inputs
  (documentation-14)
- idempotency note correctly attributed to sanitize_control_chars, not
  render_error_sanitized (reliability-7)
- eprint_error added to lint.rs use block; crate::output:: qualifiers
  removed from all call sites in lint.rs (consistency-4)
- sanitize_control_chars removed from build.rs mds import (no longer
  used directly after architecture-6 fix)
- T-11a / T-11b unit tests added to output.rs for safe_path (PF-013)
Add CHANGELOG entries under [Unreleased] for the CWE-150 hardening:
- Security bullet: all boundaries hardened, PF-014 render redesign,
  MdsError::display_sanitized(), deliberate exclusions (span/fix_edits/rule)
- BREAKING section: JS/Python/WASM API wire format now carries \uXXXX
  literals for C0/DEL/C1 bytes; span offsets and fix_edits unchanged

Add spec.md §7.5 "Sanitization invariant (v1)" subsection: normative
table documenting message/help/file sanitization vs span/fix_edits raw
byte offsets as a v1 contract, superseding prior raw-passthrough behavior.

Closes documentation-1, compliance-1, regression-2, documentation-5,
architecture-5 from the resolve-B8 triage ledger.
Remove the `Option` wrapper from `render_diag_human` and
`render_result_human`: both functions are called exclusively from the
Human-format path and always receive a source pair; the `.expect()` at
the bottom of `render_diag_human` was the only guard, which is now
unnecessary.  Move the invariant assertion to `emit_result` where the
Human/JSON split actually lives, and change the two direct callers
(`lint_one_file_human` and the stdin write path) to pass tuples
directly.  No functional change.
dean0x added 7 commits July 25, 2026 23:16
Closes review findings security-4, security-9 and security-10/reliability-9.

Widened class (both sanitizers, one predicate):
- U+200E/U+200F, U+202A-U+202E, U+2066-U+2069 - Unicode bidi controls
  (Trojan Source, CVE-2021-42574): a single RLO reverses how the rest of a
  diagnostic line renders in any bidi-aware terminal, IDE or review UI.
- U+2028/U+2029 - terminate a JavaScript string literal.
- U+FEFF - invisible BOM/ZWNBSP.

`neutralize_source_for_render` is widened symmetrically (avoids PF-004: an
alternate path that silently stops enforcing what the primary path does). The
new codepoints are all 3-byte UTF-8, so they map to U+FFFD (also 3 bytes),
preserving the byte-length invariant that T-10a pins.

Wire mode (`sanitize_control_chars_wire`) additionally escapes `\n`. A raw
newline in a diagnostic string is a line-forging vector: any consumer that
prints or line-splits it renders an attacker-authored line as a real finding.
Both modes share ONE escape map behind an EscapeMode flag - a forked second map
would be the same PF-004 drift in miniature. `\t` is preserved in both.

Boundaries: WIRE at MdsError::serialize(), LintResult::to_canonical_json()
(message/help/file key), CompileResult::to_canonical_json() warnings, and the
Python sanitize_lint_value() construction path (PF-004: it backs the typed
getters AND as_json). HUMAN unchanged at every render boundary.

Documents the one-way contract: escaping is lossy and non-injective, so
consumers MUST NOT un-escape \uXXXX back into bytes; span/fix_edits byte
offsets stay raw for callers that need the original bytes.
Every new test was written before the guard and confirmed RED against it
(PF-013: an absence-only security assertion is worth nothing if the vector
never reaches the guarded path). Each carries a reachable vector, a POSITIVE
assertion on the escaped form, and a non-vacuity guard on the expected rule.

Core (mds-core): bidi/separator/BOM escaping, byte-length-preserving
neutralization, RLO reversal + U+2028 + newline-forging vectors through
to_canonical_json, wire-vs-human mode divergence, serialize()/display_sanitized()
asymmetry.

Cross-surface (PF-007 - per-surface goldens cannot catch divergence, so all five
emit identical bytes): CLI T-9b/T-9c, napi E-14/E-15, WASM F6-BIDI, universal JS
U-E13/U-E14, Python RLO/LRI/LS/BOM params + E13 + par7 constructor path. The
U-E-DIFF differential vector now spans every escape class at once.

Reachability notes (both found by watching a test fail for the wrong reason):
- a newline inside an `@import "..."` path is rejected by the lexer with
  "unclosed quote in path", so that route is vacuous. The reachable vector is a
  YAML double-quoted frontmatter key, whose \n escape serde_yaml_ng decodes into
  a real newline that unused-variable embeds verbatim.
- U+202E survives the import path intact, so duplicate-import carries it.

The shared assert_no_control_chars helpers on all five surfaces now reject the
widened class too, which strengthens the pre-existing T-5..T-15 anchors.
spec.md 7.5: normative escaped-character table (what and why per class), the
human-vs-wire `\n` split, and a new "Escaping is one-way" subsection stating
that the mapping is lossy and non-injective, that consumers MUST NOT un-escape
\uXXXX back into bytes, and that round-tripping is an explicit non-goal - no
backslash-escaping will be added in this or a later wire version. Also records
the --diff/--check preview rule (TTY-neutralized, byte-faithful when piped);
that behaviour is implemented by the follow-up batch in the same PR.

CHANGELOG: extends the existing #176 Security entry with the widened class, the
BREAKING wire-format `\n` escape (with migration note for consumers that split
on newlines), the one-way contract, and the --diff/--check gating.
Add `preview_text_for(writer_is_tty, text) -> Cow` to output.rs:
neutralizes hostile ESC/control bytes in template source on TTY
(CWE-150), passes through unchanged when piped so diff output stays
byte-faithful for `patch`/tooling. Wire into `render_diff_lint`
(lint.rs) and `render_diff` (fmt.rs) — both paths share the one
helper (avoids PF-004). Unit tests: TTY neutralization with positive
assertion on neutralized form + byte-length preserved (PF-013),
piped Cow::Borrowed passthrough, clean string unchanged on TTY.
…f into output.rs [#176]

Both fmt.rs and lint.rs contained identical copies of colorize_unified_diff
and structurally identical render_diff/render_diff_lint functions — the latter
having become byte-for-byte equivalent after the TTY-gating added in the
preceding commit. Move the shared implementation to output.rs (the shared
output-path module) and remove the duplicates. Tests follow their
implementations.
The WASM surface was the only one of the five with no assertion pinning
the WIRE-mode \n escape — Rust core, CLI, napi, Python and universal JS
each had one. Per-surface goldens cannot catch that kind of divergence
(PF-007), so a WASM-only regression in to_canonical_json() would have
shipped green.

Adds wasm_lint_virtual_newline_in_frontmatter_key_is_escaped_on_the_wire,
mirroring napi E-15 / universal-JS U-E14 exactly (same YAML double-quoted
frontmatter vector, same four assertions) so the surfaces stay
differentially comparable.

Verified non-vacuous by mutation: reverting to_canonical_json()'s message
field from sanitize_control_chars_wire to sanitize_control_chars makes the
new test FAIL (web.rs:1137), confirming the vector reaches the guarded path.

Also corrects the files[].file assertion comment in the sibling bidi test:
the hostile codepoint is in the *imported* module name while that key is
the *entry* filename, so the assertion cannot fail via that vector (PF-013).
…176]

Three documentation-accuracy defects found during the final quality gate.

1. Raw control bytes embedded in comments (12 lines, 5 files). Comments that
   meant to name the escaped literal instead carried the raw byte itself —
   including public rustdoc on sanitize_control_chars, which shipped a raw
   ESC to docs.rs from the very function that exists to escape it. The DEL
   cases were worse than cosmetic: 'sanitizes DEL to <raw DEL>' renders as a
   truncated sentence. All test vectors already used proper escapes, so this
   is comment-only with no behaviour change.

2. The module doc claimed a 'complete closed boundary set' and listed
   MdsError::display_sanitized() as a live HUMAN boundary. It has zero
   production callers, and mds build/check render errors through
   eprint_error -> format!("{report:?}"), so an MdsError message that
   interpolates attacker-controlled text still reaches stderr raw. Verified:
   'mds build' on '@include "./x.mds" as fo<ESC>[31mBAD' emits a raw ESC.
   The source excerpt IS neutralized by MdsError::at(); only the message is
   affected. Documents the gap honestly rather than asserting false closure —
   closing it needs sanitization at MdsError construction (PF-014 forbids
   post-processing the rendered frame), which is out of scope for #176.

3. spec.md and CHANGELOG scoped the TTY-gated --diff preview to
   'mds lint --fix --diff' only; mds fmt --diff shares the same renderer.

Also corrects three more files[].file assertion comments (napi, universal
JS, Python) that claimed to cover the file-key escape but cannot fail via
their vector (PF-013), pointing to the two tests that genuinely cover it,
and fixes a six/seven character miscount.
dean0x and others added 19 commits July 26, 2026 02:29
`eprint_error` rendered a miette Report straight to stderr, so a diagnostic's
own message and help text reached the terminal with raw control bytes intact.
`MdsError::at()` neutralized the source excerpt and `serialize()` covered the
wire surfaces, but nothing covered the message. A hostile template could inject
ANSI escape sequences via any error that interpolates untrusted text, e.g.
`invalid include alias: '<alias>'` (CWE-150).

Both CLI error families were affected, not just `MdsError`: CLI-authored
`miette::miette!()` reports interpolate `mds.json` values and filesystem paths
and do not downcast to `MdsError` (PF-004). The fix therefore wraps at the
`Report` level — `SanitizedReport` overrides the Display message, the help text,
and each `LabeledSpan`'s label with HUMAN-mode escaped copies, and delegates
code, severity, url, source_code, and every byte span untouched. Any error type
added later inherits the guarantee without touching the boundary.

Sanitizing happens strictly on the renderer's INPUTS, before the Report is
built. Post-processing the rendered frame is PF-014: it escapes miette's own
ANSI SGR codes into literal noise on any colour-capable TTY and desyncs caret
alignment, on benign input, and CI cannot see it because the CLI tests pin
NO_COLOR=1 and pipe stderr. An earlier round of this PR shipped and reverted
exactly that defect. Renders for well-formed input are byte-identical.

Tests: two subprocess e2e vectors (one per error family) with paired
negative/positive/non-vacuity assertions, and seven in-process unit tests that
pin the colour path via an explicitly themed handler, since the e2e tests are
structurally blind to it. The pre-existing `@define \x1bfoo:` e2e vectors could
never have caught this — their message is a fixed string, so only the source
excerpt was ever exercised (PF-013).

Verified by guard-removal: disabling `sanitize_report` fails 5/7 unit tests and
both e2e tests; the 2 that still pass are the negative controls asserting the
boundary is inert (newline preservation, clean-input render equality).

Co-Authored-By: Claude <noreply@anthropic.com>
The boundary table in `lint/diagnostic.rs` documented `MdsError` message text on
the CLI terminal path as an open gap. The preceding commit closes it, so the
table now states the closed set and describes `eprint_error` as the covering
boundary for both CLI error families.

`MdsError::display_sanitized()` is no longer listed as a CLI boundary — it never
was one. It was added to answer review finding S-1, which was about the
*published crate's* API: `Display` is raw while `serialize()` is sanitized, so
downstream Rust consumers rendering an `MdsError` themselves need a safe
accessor. It keeps that role and its rustdoc now says so explicitly, including
why the CLI does not route through it (it cannot cover CLI-authored
`miette::miette!()` errors, which are not `MdsError`s).

spec.md §7.5 gains the normative statement that CLI human-render escaping
happens before the renderer runs, never on the rendered frame.

Co-Authored-By: Claude <noreply@anthropic.com>
Five sites in `mds build` and `mds check` called `*_collecting_warnings`
variants and printed warnings with a bare `eprintln!("{w}")`, bypassing
the `sanitize_control_chars` call that `mds-core`'s `emit_warnings`
applies on the primary code paths (PF-004 parallel-path gap / CWE-150).

Add `output::eprint_warning(w: &str)` — a single choke-point that applies
HUMAN-mode sanitization before printing — and migrate all five sites:

  main.rs  ~279  check stdin warnings
  main.rs  ~288  check single-file warnings
  main.rs  ~341  check directory per-file warnings
  build.rs ~694  compile_to_content single-file/stdin warnings
  build.rs ~1148 run_build stdin warnings

`crates/mds-core/**` is untouched; binding artifacts stay valid.

Tests (T-WARN-1/2/3, output::tests):
  - T-WARN-1: hostile ESC in warning → raw byte absent, \\u001B present
  - T-WARN-2: clean warning passes through unchanged
  - T-WARN-3: bidi U+202E is also escaped (widened class coverage)

Guard-removal evidence: replacing mds::sanitize_control_chars(&hostile)
with Cow::Borrowed(hostile.as_str()) in T-WARN-1 fails with:
  assertion failed: raw ESC must be absent from warning output
Update .devflow resolution-summary Escalations section with owner
decisions decided 2026-07-25 (implementation landed 2026-07-26):

- security-4: WIDEN — bidi/Trojan-Source + JS-hazard chars added to
  escape class (CVE-2021-42574 class; pass through C0/DEL/C1 untouched)
- security-9: ESCAPE \n ON WIRE ONLY — CWE-117 closed on wire surfaces;
  CLI human render keeps real newlines; one shared map behind a mode flag
- security-10 + reliability-9 (one decision): ACCEPT + DOCUMENT — one-way
  lossy non-injective escaping; round-tripping is a permanent non-goal;
  contract forbids un-escaping \uXXXX back to bytes; documented in spec
  §7.5 and sanitize_control_chars rustdoc
- security-11: TTY-GATED NEUTRALIZE — --fix --diff preview neutralizes
  when stdout is a tty, byte-faithful when piped; NO_COLOR does not gate
  this (safety, not styling); --check status lines are not TTY-gated

Also records: batch rationale (last free wire-format window, zero users,
pre-v0.4.0-tag), implementing commits, owner-approved mid-flight expansion
(MdsError stderr path + miette!/warning prints found by Scrutinizer), and
honest alignment-review finding that boundary-closure is still incomplete
(lint.rs:197 eprintln + fix-rejected MdsError Display — work in progress).
…176]

Four core-side gaps the #176 alignment review found between what the PR
claims and what it delivers:

- U+061C ARABIC LETTER MARK was missing from the bidi class (11 of the 12
  Unicode Bidi_Control=Yes codepoints were covered). It is 2 bytes in UTF-8
  while every other member is 3, so the format-hazard predicate is now split
  by UTF-8 width: neutralize_source_for_render routes it through the 2-byte
  NBSP branch, keeping the byte-length invariant its debug_assert_eq! pins.
  The byte-level fast path also had to learn 0xD8.

- A filename was sanitized in HUMAN mode, which preserves newlines by design
  so multi-line diagnostic messages keep rendering. POSIX allows a newline in
  a filename and directory mode discovers names by walking, so a hostile name
  could forge status lines and frame headers (CWE-117). Filenames now use WIRE
  mode. The three sites that built a NamedSource by hand (MdsError::at,
  formatter check_equivalence, and the lint renderer) now share one
  named_source_for_render builder, which applies the correct mode per half:
  WIRE for the single-line filename, byte-length-preserving neutralization for
  the span-indexed source.

- FixOutcome::Rejected.reason interpolated MdsError's deliberately-raw
  Display. reason is a public field of a public enum, so the number of print
  sites is unbounded; it is escaped at its single construction site instead
  (WIRE, because the CLI prints it as one unframed status line).

- Doc claims narrowed to what is true: the escape class, the per-field mode
  split, and the boundary table (which now names every in-tree site and
  explicitly excludes watch.rs's pre-existing raw status prints).

Tests: U+061C added to the bidi table shared by the escape and neutralize
tests, plus a byte-width test that fails if a member is filed under the wrong
UTF-8 width; named_source_for_render filename/source split; both rejection
reason construction paths.
…graph [#176]

- lint.rs printed the unknown-mds.json-rule warning with a bare eprintln!,
  bypassing both eprint_error and eprint_warning. A rule NAME is an arbitrary
  JSON object key and a JSON \uXXXX escape decodes to a real byte, so any
  repository could put a raw ESC on stderr just by being linted. This is the
  "sixth warning print" that eprint_warning's own rustdoc claimed was
  foreclosed; the rustdoc now states coverage rather than foreclosure.

- SanitizedReport reported source()/related()/diagnostic_source() as absent,
  guarded only by a debug_assert! that no CLI error populates them.
  debug_assert! is compiled out of release (PF-005), so the first error type to
  grow a #[source] field would have had its cause chain silently dropped from
  release stderr while CI stayed green. The wrapper now materialises the whole
  auxiliary graph into owned, escaped SanitizedNodes at construction and
  forwards those — preserving the diagnostic and the guarantee, in both
  profiles. The walk is depth-bounded so a cyclic source() cannot hang render.

- safe_path moves to WIRE mode so a newline-bearing filename cannot forge a
  status line, and the two Clean: printers that open-coded their own escape
  call now share safe_file_display rather than drifting to a different mode.

- --check was documented as a TTY-gated neutralized boundary; it is not.
  --check alone emits only status lines (unconditionally sanitized), and
  neutralization applies to --diff. Corrected in output.rs to match spec.md
  and CHANGELOG.md, which already said --diff only.

The shared assert_no_control_chars test helper now rejects U+061C, so every
existing e2e escape assertion covers the widened class.
- CHANGELOG: the warning entry said five sites; there were six. Adds the
  filename-forgery (CWE-117) and rejection-reason entries, and records U+061C
  plus the per-width neutralization substitutes.
- spec.md 7.5: escape class is the complete Bidi_Control=Yes set; the
  human/wire split is stated per FIELD (filenames are WIRE everywhere) rather
  than per surface; neutralization substitutes listed by UTF-8 width.
…176]

The header claimed the table listed "every in-tree site". It does not, and
saying so undercuts the scope note directly below it that excludes watch.rs's
pre-existing raw status prints. Points at that note instead. [#176]
`emit_warnings` prints warning strings to stderr in HUMAN mode, which
preserves `\n` by design so multi-line warning bodies keep rendering.
Three producers interpolated an untrusted single-line identifier into
that prose and relied on the HUMAN pass to make it safe:

- resolver.rs: the imported-module filename in the source-map segment-cap
  warning
- evaluator.rs: the `@include` alias, twice

Per the per-field rule re-ratified for #176 (spec 7.5), an identifier or
filename is WIRE on every surface including human terminal output — it is
never legitimately multi-line, so a raw newline in one only lets it forge
a standalone line byte-identical in form to genuine output (CWE-117).
Escaped at construction, so every consumer of the warning inherits it.

`sanitize_control_chars_wire` is idempotent, so the existing WIRE pass in
`CompileResult::to_canonical_json` is unaffected.

Co-Authored-By: Claude <noreply@anthropic.com>
…176]

Two reproduced gaps, plus the whole class they belong to.

M1 — `output.rs` printed the shared walker's depth-limit warning with a
bare `eprintln!` interpolating `dir.display()`. The name is discovered by
the walk, and every directory-mode subcommand shares that walker, so one
hostile directory put a raw ESC, a raw U+202E and a forged standalone
`Clean:` line on stderr for `build`, `check`, `fmt`, `lint` and `watch`
at once. Its sibling at `probe_and_remove_stale` printed a raw path and a
raw `io::Error` the same way.

M2 — `lint.rs`'s unknown-`mds.json`-rule warning already routed through
`eprint_warning`, but that is HUMAN mode, which preserves `\n`. A rule
name of `x\nClean: totally-real.mds\n0 problems found\n` still emitted
three standalone lines byte-identical to genuine status output.

Both are the same defect: the escape mode is a property of the FIELD, not
of the surface. Warning prose stays HUMAN so multi-line bodies render;
identifiers, filenames and error causes go WIRE everywhere, because none
of them is ever legitimately multi-line.

Applied across the crate rather than at the two reported sites:

- new `output::safe_inline` — the general WIRE helper for a single-line
  untrusted value (`io::Error` causes, `mds.json` rule names and config
  paths, `--format` arguments, fix-rejection reasons). `safe_path` and
  `safe_file_display` now delegate to it, so there is one escape call.
- `watch.rs` lifecycle status lines — previously carved out as a
  pre-existing gap — now route through `safe_path` / `safe_inline` /
  `eprint_warning` like everything else. Allowlisting them would have
  been a deliberate hole in the guard that follows.
- every remaining `{e}` / `{reason}` / `.display()` / CLI-argument
  interpolation in `build.rs`, `lint.rs`, `main.rs` is escaped.
- `build.rs` gains `kind_label`, hoisting a two-arm `&'static str` match
  out of a format argument so the print reads as data, not control flow.

The `eprint_warning` rustdoc no longer asserts coverage by listing sites.
Two revisions of that list were correct when written and stale by the
next review; the invariant is enforced by a test instead. Same for the
test-strategy comment in `output.rs`, which claimed the resolver warning
was the only one interpolating untrusted text and that an e2e vector
would be vacuous — both false, and the unreachability argument was the
load-bearing half.

Co-Authored-By: Claude <noreply@anthropic.com>
Three reviews of #176 each found a different bare `eprintln!` reaching a
terminal with an untrusted value. Each round fixed its findings correctly
and each time the next reviewer found another, because the property was
only ever asserted about the sites someone remembered to enumerate. The
search is unbounded; this makes it a bounded, machine-checked invariant.

`tests/print_discipline.rs` fails if any print macro under
`crates/mds-cli/src/**` interpolates a value that is not a call to one of
the escape helpers. It scans the crate's own sources with a small Rust
lexer — comments masked, string / raw-string / char literals skipped — so
it is fast, deterministic, and needs no external tools.

It also scans `format!` invocations nested inside `eprint_warning` calls.
That is what catches M2: HUMAN-mode escaping of the whole line does not
make an interpolated identifier safe, because HUMAN preserves newlines.
HUMAN `sanitize_control_chars` is deliberately NOT in the accepted-helper
list for that reason; `eprint_warning`'s own body is one narrow allowlist
entry instead of a blanket exemption.

Exceptions live in an explicit allowlist keyed by (file, expression) —
not by line, so it cannot rot as code moves — with a written per-entry
justification: the compiled artefact written to stdout (escaping it would
corrupt every redirect), a `&'static str` label, and integer counters.
`every_allowlist_entry_is_live` fails if an entry stops matching, so an
exemption cannot outlive the code that needed it.

Scope is documented honestly rather than overclaimed: `miette!()` message
construction is NOT covered and is recorded as a known residual.

Proven RED both ways before landing:
  - a fresh eprintln! interpolating entry.display() in watch.rs
    -> "watch.rs:926: eprintln! interpolates unsanitized `entry.display()`"
  - reverting the M2 fix to its HUMAN-only shape
    -> "lint.rs:209: eprint_warning(format!) interpolates unsanitized `name`"

Plus two e2e regression vectors:

  T-ESC-WALK-1 (new) pins the walker depth-limit warning on all three
  directory-mode subcommands — the raw ESC byte written into the directory
  name comes back as the uppercase six-character literal, proving genuine
  decode-then-escape rather than literal passthrough.

  T-ESC-RULE-1 gains newlines in its vector and a standalone-forged-line
  assertion. Its old vector had none, and `assert_no_control_chars`
  permits newlines by design, so it certified CWE-150 closure on that
  vector while CWE-117 stayed open on it.

Co-Authored-By: Claude <noreply@anthropic.com>
The owner re-ratified Decision 3 on 2026-07-26 in a stronger, per-field
form, superseding the "wire mode at exactly four boundaries" enumeration:

  Untrusted identifiers and filenames are WIRE-escaped on every surface,
  human output included. Prose (diagnostic message / help bodies) stays
  HUMAN so multi-line frames keep rendering.

The discriminator is whether the value is ever legitimately multi-line. A
filename or a config key never is, so preserving a raw newline in one only
enables status-line forgery (CWE-117); a diagnostic body genuinely is.
Stating it as a rule makes each remaining site decidable without
re-deriving a list of boundaries — which is what went stale twice.

- spec.md 7.5: the rule is now normative, with a per-value table, and
  explicitly supersedes the per-surface framing.
- diagnostic.rs module doc: same rule at the top; the `eprint_warning`
  boundary row now reads "prose HUMAN, interpolated identifiers/paths
  WIRE"; new rows for `safe_inline` and for the print-discipline guard;
  `emit_warnings` records that its producers escape identifiers at
  construction.
- diagnostic.rs "scope of the table": the `watch.rs` carve-out is deleted
  because those lines are now routed. The two things still outside the
  table — CLI `miette!()` message construction and compiled template
  output — are named, with why, and are not claimed closed.
- CHANGELOG: the warning-path bullet no longer counts sites. It states
  what is covered, that `watch.rs` is included, and that the property is
  CI-enforced rather than enumerated. A new bullet states the per-field
  rule and notes the two `mds-core` items that go public with it,
  `sanitize_control_chars_wire` and `named_source_for_render`.
- review ledger: records the re-ratification, the owner-approved systemic
  guard, and the three consequences that change earlier decisions.

Co-Authored-By: Claude <noreply@anthropic.com>
…#176]

The print-discipline guard scanned `format!` only where it appeared lexically
inside an `eprint_warning(...)` call, so hoisting the message into a local —
completely idiomatic — made the whole interpolation invisible and reintroduced
M2 verbatim. `eprint_warning(<bare identifier>)` produced no site at all; five
live sites (build.rs:711/1166, main.rs:279/288/341) passed a loop variable and
were trusted without anything checking them.

The helper argument is now classified in its own right: a string literal, a
whole-expression sanitizer call, or a `format!` whose interpolations are each
accepted. A bare local is traced one hop through its `let` binding in the same
file and judged by the same rule. Anything unresolved is REPORTED, not assumed
safe — a false positive costs one allowlist entry, a false negative costs
another review round. The five `w` sites now carry written justifications in a
separate ALLOWED_UNTRACED_HELPER_ARGS list, which exempts them only in the
helper-argument position and states plainly that their safety rests on mds-core
producer discipline this guard cannot verify.

Also:
- `is_sanitizer_call` now requires the call to be the WHOLE expression, so
  `safe_path(p) + &evil` and `safe_path(p).replace("a", &evil)` are rejected.
  The self-test previously asserted this property but only tested the prefix
  direction; it now covers the suffix direction too.
- `write!` / `writeln!` whose first argument names a terminal stream are
  scanned. Latent today (the crate has none) — pinned before the first one lands.
- `fn eprint_warning(w: &str)` is no longer scanned as a call to itself.
- The rustdoc claims only what the guard delivers, and names four accepted
  limits: name-matched sanitizers, anti-rot-not-anti-reuse allowlists, the
  one-hop single-file binding trace, and name-based stream detection. This is a
  lexical scanner; the bar it meets is accidental reintroduction, not
  deliberate circumvention.

Each bypass was proven by injection into real source, confirming the guard
fails naming the exact site, then reverted.

Co-Authored-By: Claude <noreply@anthropic.com>
…dual [#176]

spec §7.5 asserted that filenames, paths and causes are "WIRE everywhere". That
is false: mds-core `MdsError` message bodies interpolate exactly those values
and stay HUMAN on terminal surfaces, so a hostile path with a newline takes a
line of its own inside the rendered frame (`fs.rs:478` `cannot read
{normalized}: {e}`, `:487` invalid-UTF-8, `parser_helpers.rs:853` `invalid
import alias: '{alias}'`).

Chose to narrow the claim rather than chase it. Making it literally true means
WIRE-escaping every untrusted interpolation at every `MdsError` construction
site — 110+ `MdsError::*(format!(…))` across parser_helpers, evaluator,
resolver, builtins, fs and lib — and changing the public message text every
binding layer sees. Fixing only the two sites the reviewer named would leave
the claim false at ~100 others, which is precisely the overclaim that reopened
this issue twice.

So the rule is now stated per FIELD, exactly: a path in a `file` field (status
line, `[file:line:col]` header, JSON `file` key) is WIRE on every surface; a
path interpolated into a message BODY is prose and follows the message row.
The residual is named in all four places — spec §7.5 gains a "Residual" section,
the boundary table names the mds-core half beside the CLI `miette!()` half, and
the CHANGELOG carries a "Declared residual" paragraph. The reviewer's
characterization is preserved: frame content is `│`-prefixed and the prefix
survives `strip()`, so it cannot masquerade as a bare status line.

Other corrected claims:
- "All serialization and diagnostic-render boundaries are now hardened" was
  closed-set-by-enumeration in the file that retires enumeration; now an audit
  list that points at the per-field rule.
- "Human-render output is unchanged" was false under the per-field rule and
  self-contradicted twice below; now scoped to diagnostic prose.
- "sanitizes renderer inputs byte-length-preservingly" was over-broad; only
  source text is length-preserved, message/help are \uXXXX-escaped.
- "All five surfaces" after enumerating four.
- Two bidi rustdoc lists omitted U+061C (11 of 12); code was already correct.
- diagnostic.rs defined the class as "C0 except \n/\t" while the spec puts \n
  in the class with \t as the sole exemption. Same behaviour, two incompatible
  definitions; \n is in the class and HUMAN/WIRE is the mode choice.

Co-Authored-By: Claude <noreply@anthropic.com>
Guard hardening (B1-B4 fixed, B5/B6 accepted as stated limits), the
option-(b) decision on the WIRE-everywhere claim with its blast-radius
evidence, and the disposition of all five live bare-`w` helper arguments.

Co-Authored-By: Claude <noreply@anthropic.com>
Refresh sanitization discipline section for PR #176 (fix/esc-injection-176):
per-field HUMAN/WIRE rule, widened bidi class, SanitizedReport, print-discipline
guard, named_source_for_render, and decided open-design-questions.
…#176]

Two defects in the print-discipline guard's own security justification, both
found by a fourth adversarial pass.

1. Undocumented bypass. `let` bindings are matched file-wide, so a name also
   introduced by a `for` variable, function parameter or closure parameter was
   resolved against unrelated `let`s of the same name and accepted when all of
   them were safe — contradicting the guard's affirmative claim that such a
   binder is "reported, not assumed safe". Proven live: `lint.rs` carries three
   `let label = safe_path(...)` bindings, so

       fn atk_v12(rules: &[String]) { for label in rules { eprint_warning(label); } }

   injected into the real file produced ZERO sites.

   Fixed rather than merely documented. `collect_non_let_binders` collects every
   `for` / parameter / closure binder in a file; each poisons its own name, so
   `classify_helper_arg` reports such an argument instead of resolving it.
   Over-collection is the safe direction and is chosen deliberately. With the
   construct injected the guard now fails:

       lint.rs:1547: eprint_warning(untraced) interpolates unsanitized `label`

   Zero false positives on real source: no allowlist entry was added, and both
   real-source tests pass unchanged. A new non-vacuity assertion (>= 50 binders
   found) keeps the poison set from silently emptying. `if let` / `while let` /
   `match`-arm binders remain unmodelled and are now limit 5 in "Accepted
   limits" — the narrowed remnant, stated as such.

2. Two false claims. The module doc and an allowlist justification said the
   mds-core producer precondition was "upheld by ... mds-core's own tests"; no
   such test existed. Another named an `evaluator.rs` producer of
   "rejected-value text" that does not exist.

   Made true where it can be. mds-core has exactly three warning producers that
   interpolate a runtime value. `resolver.rs`'s imported-module filename is the
   only one whose input can carry a hostile character, and it is now pinned by
   `producer_discipline.rs`: an entry module importing a module named with a
   real ESC and U+202E, whose evaluation trips MAX_SOURCEMAP_SEGMENTS. PF-013 —
   positive (both escaped literals present), negative (no raw ESC, U+202E or
   newline), non-vacuity (the warning must exist), guard-removal proven by
   deleting the WIRE wrapper in resolver.rs. The two `evaluator.rs` sites
   interpolate an `@include` alias the parser restricts to
   `[A-Za-z_][A-Za-z0-9_]*`, so a test of them would be vacuous; they are stated
   as upheld by review, not claimed to be tested.

The test lives in mds-cli, not mds-core, so it asserts the precondition on the
exact value `build.rs`/`main.rs` hand to `eprint_warning` — and leaves mds-core
free of executable change.

Co-Authored-By: Claude <noreply@anthropic.com>
A fourth adversarial pass reproduced a real per-surface gap: `mds build
--source-map` on a file named `ev<LF>il<ESC>[31m.mds` writes a sidecar whose
decoded `file` / `sources` carry a real newline and a real ESC, while the CLI
status lines of the same run are correctly escaped. That falsified the shared
per-field sentence wherever it appeared.

DECISION: carve source maps out; do not escape them. Source Map v3 `file` /
`sources` are functional path references — devtools, bundlers and IDEs resolve
them against the filesystem. Escaping one to a `\uXXXX` literal would point the
map at a path that does not exist, breaking source-map resolution and dependency
tracking in order to defend against a pathological filename. It is the same
product-versus-display distinction that already keeps compiled stdout output
byte-faithful: escaping the artefact corrupts the artefact.

Stated as an explicit, NAMED third carve-out at every site where the per-field
rule appears — spec 7.5 (new "Carve-out: functional path references" subsection,
plus the `file` invariant row, the governing blockquote, the supersession
paragraph and a new mode-table row), `lint/diagnostic.rs` ("three categories
remain outside the table"), CHANGELOG ("Declared carve-out"), and the rustdoc a
consumer actually reads: `SourceMap`, `CompileResult::dependencies` and
`to_canonical_json`. Scope: source-map `file` / `sources` / `sourcesContent` in
both the sidecar and the embedded `sourceMap`, plus `CompileResult.dependencies`.
The contract is one-way and normative: consumers MUST treat these paths as
untrusted; JSON string encoding is not escaping, since a decoded "\n" is a real
newline again. The CLI does not depend on that contract — its `Compiled to` and
`Source map written to` lines print through `safe_path`.

The per-field sentence itself is narrowed everywhere from "on every surface" to
"on the diagnostic surfaces". CHANGELOG's "every wire boundary" becomes "the
diagnostic wire boundaries"; "escaped ... on every surface" becomes "on every
surface that renders one"; "display-hazardous, on every surface" becomes "on
every surface that escapes". No absolute claim survives.

Also corrects four inconsistencies:
- spec claimed both "C0 ... except `\t`" and "`\t` is the only character IN the
  class that is preserved". Unified on the framing the other documents use:
  `\t` is the sole exemption from the C0 range.
- `error.rs`'s `display_sanitized` rustdoc still used the retired "C0 except
  `\t` and `\n`" framing that `diagnostic.rs` forbids — the third site the
  round-3 unification missed.
- mds-python said "the other four surfaces", implying five; there are four in
  total, so from Python it is three, now named.
- the print guard's untraced allowlist is two entries covering five sites, not
  five entries.

Input-boundary rejection of control characters in filenames was assessed as a
stronger long-term defence and deliberately NOT implemented: it changes which
inputs compile at all and belongs in a separate issue.

Documentation only in mds-core / mds-wasm / mds-napi / mds-python: no
executable line changes, so the binding suites are unaffected.

Co-Authored-By: Claude <noreply@anthropic.com>
Adds a "Round 4" section covering the fourth adversarial alignment pass: the
reproduced source-map sidecar gap and the carve-out decision with its rationale
(functional path references resolve against the filesystem, so escaping them
breaks resolution rather than protecting anything), the two false statements in
the print guard's own justification and what replaced them, the non-`let` binder
bypass with the injection proof, and the four doc inconsistencies.

Closes the section with the two things previous rounds lacked: a round-4
verification table with exact counts, and an explicit enumeration of the five
residuals that remain after four adversarial passes — message-body prose,
the source-map carve-out, the guard's five lexical limits, the two
review-only `evaluator.rs` producers, and one-way non-injective escaping.

Also corrects "five bare-`w` sites allowlisted" to name the two entries that
cover them, and updates the mds-lint knowledge base with the carve-out, the
poison-set behaviour and the producer-precondition asymmetry.

Co-Authored-By: Claude <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Harden terminal-escape rendering across all MdsError variants (CWE-150)

1 participant