fix(website): clamp pathological markdown nesting depth before parse

[javenciu]

Problem / Motivation

A chat message containing pathologically nested markdown — for example thousands of leading > blockquote markers, or equivalently deep nested list indentation — parses into an mdast/hast tree whose depth equals the nesting count. MarkdownRenderer.tsx walks that tree with plain recursive functions (eight self-recursive walk helpers), and remark-rehype's own mdast→hast transform recurses too. Past the engine's call-stack limit this throws RangeError: Maximum call stack size exceeded while rendering a single message.

Reproduced at origin/main (971dcce): a message of 50,000 nested blockquote markers crashes <Markdown> with RangeError (React error-boundary trace pointing into the render), and the affected vitest run hangs to timeout.

Why it matters

One message — pasted by a user, emitted by an agent, or arriving from any transcript source — takes down the whole chat rendering path. The failure is input-controlled: nothing upstream caps nesting depth, so the renderer is the layer that has to defend itself.

What changed (motivation → approach → change)

The tree's depth is decided before any walker runs — at parse time, from the raw markdown text. Two fix shapes were considered: per-walker depth caps (eight vendored walkers plus the remark/rehype pipeline's own visitors, each needing its own cap and each a future maintenance hazard) versus one input-side clamp at the single choke point where every message enters parsing. The input clamp wins structurally: every downstream walker — present and future — inherits the guarantee.

• New website/src/utils/clampNestingDepth.ts: rewrites only lines whose leading blockquote run exceeds MAX_BLOCKQUOTE_DEPTH (100) markers or whose list indent exceeds MAX_LIST_INDENT_COLS (256) columns. Ordinary content passes through byte-identical. Single pass, linear in input length. Fenced code blocks are exempt (marker runs inside a fence are literal text), and the standard for "is this line a fence?" is micromark's CommonMark semantics: a backtick fence's info string may not contain a backtick, so such a line is a paragraph to the parser and does not open an exemption window here. Where fence detection is unsure it fails closed (stays clampable) — the worst case is cosmetic clamping of fenced marker art, never an unclamped deep run reaching the parser.
• MarkdownRenderer.tsx (MarkdownBlock): clean = clampNestingDepth(clean) immediately after the existing stray-tag strip, before any parsing. Runs in both sourcePos modes — a message that crashes the renderer has no coordinates worth preserving.

Commit 2 generalizes the clamp from consecutive quote markers to every
input-controlled nesting spelling that reaches a recursive layer. Review and
CI caught gaps in the first commit from three sides:

• A blockquote re-opened through the spec's 0-3 space indent (> > > ...)
  counted as one marker and sailed past the clamp, and interleaved quote/list
  prefixes (> - > - ...) nested ~two tree levels per four bytes with no long
  marker run and no indent growth. The scanner now consumes container units
  iteratively (0-3 spaces + quote marker, or 0-3 spaces + list marker) and
  bounds the total count per line.
• Raw HTML tag runs (<div><div>...) bypassed the container clamp entirely:
  rehype-raw is wired into both rehype pipelines, so embedded HTML parses
  (parse5) into HAST that the recursive walkers consume before sanitization —
  the same stack-overflow class through an alternate spelling (flagged by AI
  review on the previous head). A third clamp pass now models parse5's
  open-element stack conservatively and neutralizes opening tags past
  MAX_HTML_TAG_DEPTH by rewriting < to < (literal in both micromark
  and parse5 contexts). The model errs over-count-only by construction: voids
  never push; self-closing spelling on non-voids still opens (per parse5 —
  honoring the slash would under-count); same-name implied-end siblings
  replace the stack top (<li> spam is flat); closes pop only on exact top
  match (bogus closers cannot drain the counter); closes inside quoted
  attribute values or inline code spans never pop (fake-close vectors).

Clamping stays minimal-mutation: below every bound content is byte-identical;
a clamped line drifts by one inserted escape byte (containers) or 3 bytes per
neutralized tag (HTML). The fence exemption (micromark CommonMark semantics,
fail-closed on backtick-in-info-string) applies to all passes. The clamp API
is deliberately minimal: clampNestingDepth(s) with the bounds as module
constants — no tunable parameters, since the only production caller uses the
defaults and unused tunables are a maintenance liability.

The nested-list D-pin fixture shrinks from 1,500 to 600 levels: the fixture
is quadratic in depth (~2.3MB at 1,500), parsed ~3s locally but 28s on CI
against the 15s per-test budget — a fixture-cost timeout, not a clamp miss.
600 levels keeps the conviction 4.7x past the ~128 effective-level bound.

Tests

New website/src/test/MarkdownRenderer.depthGuard.test.tsx (29 tests; the overflow vectors and the pseudo-fence vector fail without the fix):

• renders 50-deep nesting normally (below any clamp bound — byte-identical passthrough)
• survives 5,000-deep blockquote nesting without a stack overflow
• survives 50,000-deep blockquote nesting without a stack overflow
• survives deep nested-list nesting (the non-blockquote nesting vector)
• clamp path stays roughly linear in input length (no quadratic re-scan)
• a backtick-in-info-string pseudo-fence does not open an exemption window (a line like ```x`y is a paragraph per CommonMark; treating it as a fence would exempt a following deep run from clamping — pinned failing-without-fix with the exact RangeError)
• deep marker art inside a genuine closed fence stays byte-identical (the fence exemption itself, pinned: fenced content deeper than the clamp bound survives untouched and parses as code, not blockquotes)
• survives a double-spaced blockquote re-open run (marker-run bypass vector) (fails at commit 1 with the same RangeError class, passes with commit 2)
• survives an interleaved quote/list container prefix (container-class vector) (fails at commit 1 with the same RangeError class, passes with commit 2)
• preserves a below-bound interleaved prefix byte-identically (no false clamp) (passes both sides; regression pin on genuine structure)
• clamps by minimal mutation: kept prefix byte-identical, one inserted escape (pins the one-byte-drift rewrite shape)

Raw-HTML vector (11 more; the deep-<div> conviction fails without the fix inside hast-util-from-parse5):

• survives 5,000-deep raw <div> nesting without a stack overflow (fails without the HTML pass: RangeError inside hast-util-from-parse5, ~2.2s; passes in ~54ms fixed)
• survives self-closing-spelled non-void nesting (<div/> still opens per parse5 — honoring the slash would under-count)
• bogus close tags do not drain the depth counter (</span> under a div run)
• multi-line tags count toward depth (tag spanning a line break)
• sequential paired HTML stays byte-identical (no false clamp on oscillation)
• same-name implied-end siblings stay flat (<li> spam is not nesting)
• void elements never count toward depth (<br> spam untouched)
• a close tag inside a quoted attribute value does not pop (attr-data fake close)
• a close tag inside an inline code span does not pop (code-span fake close)
• deep tag runs inside a genuine closed fence stay byte-identical
• clamps by minimal mutation: below-bound tags byte-identical, < past bound

HTML-block fence shield (7 more; CommonMark 4.6 — a line opening an HTML block swallows following lines as raw content, so a ```-shaped line there is not a fence to micromark; entering fence state on it exempted an unclamped deep run behind a 3-token shield prefix):

• a fence marker inside an open html block does not open an exemption window (the exact bypass shape: <div> + ``` + 5,000-deep tag run returned unchanged from the clamp and threw the RangeError in the rehype-raw pipeline at the unfixed tree)
• a blank line ends the html block: a fence after it is genuine again (pins the latch clears — deep marker art inside the re-opened genuine fence stays byte-identical; passes both sides, regression pin)
• blank lines do not end a <pre> block: the shield persists to the textual closer (condition-1 blocks end at </pre>, not blank lines — a blank-only latch would re-open the bypass)
• a same-line closer ends the block: <pre>x</pre> then a fence is genuine (single-line blocks latch nothing; passes both sides, regression pin)
• a container-nested html block still shields an indented fence line (- <div> opens a block inside a list item — openers are detected after stripping the container run)
• a comment block shields across blank lines until its textual closer (type-2 <!-- ends at -->; the deep run after the closer is clamped)
• a complete non-listed tag alone on a line shields the next fence line (condition 7: any complete tag alone opens a block — <x-widget> shields identically)

While the block latch is open the inline code-span mask is also inert: a swallowed backtick line is raw bytes to micromark, so it can neither latch nor mask a past-bound open (a masked open would flow through unrewritten — the other half of the same bypass). Block-name tables are copied from micromark's own micromark-util-html-tag-name lists (verified equal at build: 62 block names, 4 raw names).

Without the fix, the deep-nesting, pseudo-fence, deep-<div>, and html-block-shield tests throw RangeError: Maximum call stack size exceeded inside <Markdown> (the 5 shield attack pins fail at the unfixed tree: 5 failed / 24 passed; all 29 pass fixed, suite solo in ~4s). Family scope (every test importing MarkdownRenderer or clampNestingDepth): 2,600 tests across 166 files green. Full local gate mirror also green: tsc -b, eslint src, theme-colors, phantom-classes, i18n-strings, jscpd, full vitest suite.

Manual verification

Rendered a 50,000-deep blockquote message through the fixed MarkdownBlock in the vitest DOM harness: renders as a clamped-depth blockquote chain instead of crashing. A 50-deep message renders exactly as before (passthrough verified byte-identical at the clamp layer).

Related Issues

None found — searched open issues and PRs for recursion/depth/stack-overflow reports against MarkdownRenderer before building (7 open MarkdownRenderer PRs checked, all orthogonal).

Pattern harvest

The pattern is recursive tree processing whose recursion depth is input-controlled with no guard at any layer. Harvested across this seam: eight self-recursive walkers inside MarkdownRenderer.tsx plus the remark/rehype pipeline's own recursive visitors all share the same exposure, and all funnel through one input choke point (MarkdownBlock's pre-parse string). Fixing at the choke point covers every current walker and any walker added later, where per-walker caps would rot. A second harvest from review: an exemption window inside a guard must judge its trigger by the downstream parser's semantics — a looser line-shape match (here, a fence regex that accepted backticks in a backtick fence's info string) turns the guard itself into the bypass. A third, also from review: an input clamp must cover every spelling the pipeline turns into tree depth, not just the one that crashed first — rehype-raw makes embedded HTML a parse path, so <div> runs were the same attack through a different grammar, and the guard was incomplete until all three spellings (container prefixes, list indent, HTML tags) hit a bound at the same choke point. One adjacent candidate of the same class exists wherever raw markdown is parsed outside MarkdownBlock (e.g. any preview/export path that parses independently); left out deliberately per one-topic-per-PR — happy to file separately if maintainers want those paths audited too.

Rule candidate: when recursion depth is controlled by input, clamp the input at its single entry choke point instead of capping each recursive consumer — and enumerate every grammar the pipeline parses into depth (markdown containers, indent, embedded HTML), because covering one spelling of the attack leaves the guard bypassable through the next.

F2 (source-position drift) — human-writer override rationale

The flagged drift (data-sourcepos coordinates vs unclamped content;
first-duplicate fallback may miscite a selection location) can occur only on
lines the clamp rewrote — input carrying more than 100 leading container
markers on a single line, which does not occur in human-authored content.
For that adversarial class the alternative to approximate citation is no
render at all (the stack overflow this guard exists to stop). Commit 2
narrows the drift further: the rewrite keeps the kept prefix byte-identical
and inserts exactly one byte, so coordinates are exact for all unclamped
lines and for everything before the clamp point on a clamped line.
Adjudication: total=0 uphold=0 (flag not upheld); requesting human-writer
override on that basis.

Checklist

• At most two commits (one is the norm), with a Conventional Commits title (feat|fix|docs|refactor|perf|test|chore|ci|build|revert: ...)
• Existing tests pass and new tests added for new functionality
• Self-review completed; code follows project style guidelines
• Documentation updated (if applicable)
• No secrets, credentials, or internal references in the diff

Contribution License Agreement

Per the template placeholder (CLA text pending): offered under the same terms as my prior merged contributions to this repository (#8835).

Why no screenshot: no visual delta -- this change rewrites pathological input strings (>100
leading container markers per line, >256 indent columns, or >100-deep raw
HTML tag runs per message) before parsing; ordinary content is byte-identical
through the clamp and renders unchanged. UX review confirmed zero visual
delta.

[github-actions]

UX Review (Fable 5, fork) — ✅ PASS

UX-level review of 9f68e58ea439f79e56556b1c8da53155b98a1e4d via the fork AI-review pipeline — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

UX-Verdict: PASS

Pure crash-guard with no new controls, labels, or strings — the only user-visible change turns a whole-transcript crash into a rendered message.

The diff adds a pre-parse clamp (clampNestingDepth), one call site in MarkdownBlock, and tests. No control, copy, state, or layout changes; content below the 100-level bound is byte-identical, so no first-time-reader evidence is required. The degraded path (a >100-deep run rendering its tail as literal > text at clamp depth) only ever replaces RangeError: Maximum call stack size exceeded taking down the chat view — strictly better for the user, and unreachable by any humanly written message.

[UX-REVIEWED] 9f68e58

[github-actions]

First Principles Review (Fable 5, fork) — 🟡 CONCERNS

Premise-level review of 9f68e58ea439f79e56556b1c8da53155b98a1e4d via the fork AI-review pipeline — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

I have everything I need. Verified against the base tree: the fix targets the single parse choke point in MarkdownBlock; fixCodeFences (MarkdownRenderer.tsx:3398) already carries a fence-state tracker; mochi's ChatPanel.tsx is the one independent parse site left unfixed (declared as deferred by the author). Final review follows.

First-Principles-Verdict: CONCERNS

The crash fix is cause-level and earns its place; the exported clamp util ships tunable parameters and constants no caller uses.

What this change ships

Intent: stop one pathologically nested chat message from crashing the whole transcript view — a FIX.

1. Messages with >100-deep quote/list nesting now render clamped instead of throwing RangeError — justified, cause-level (input choke point, engine stack limit is the constraint).
2. List lines indented past 256 columns get indent truncated — justified, second vector of the same defect, declared.
3. Content inside genuine code fences is exempt from the clamp — justified, declared.
4. Fence-lookalike lines fail closed (marker art may be cosmetically rewritten) — declared trade-off.
5. clampNestingDepth(s, maxQuoteDepth?, maxListIndentCols?) exported with override params — undeclared; one consumer, generalized.
6. MAX_LIST_INDENT_COLS exported — zero consumers.

Watch

• One counted sibling parse site stays crashable: mochi ChatPanel.tsx renders chat markdown through 5 direct <Markdown> calls (grepped <(React)?Markdown[ >], 2 files repo-wide) with no clamp. The description declares and defers it — accepted, but it is the same untrusted-input class, not just "preview/export".
• Third fence-state tracker in the frontend (grepped: fixCodeFences at MarkdownRenderer.tsx:3398, AMBIGUOUS_FENCE_LINE_RE at keepVisibleMarker.ts:56, new FENCE_LINE). The stricter semantics are argued and load-bearing here, but three trackers over the same strings will diverge silently.

Subtractions

• Drop maxQuoteDepth/maxListIndentCols parameters from clampNestingDepth — 1 consumer (MarkdownRenderer.tsx MarkdownBlock), zero callers pass them (tests use defaults too); read the constants directly.
• Unexport MAX_LIST_INDENT_COLS — zero consumers outside clampNestingDepth.ts.

[FIRST-PRINCIPLES-REVIEWED] 9f68e58

[github-actions]

Design Review (Fable 5, fork) — 🟡 CONCERNS

Design-level review of 9f68e58ea439f79e56556b1c8da53155b98a1e4d via the fork AI-review pipeline — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Design-Verdict: CONCERNS

Sound choke-point clamp, but a second react-markdown parse site (mochi's ChatPanel) renders the same untrusted chat content and stays crashable.

Watch

• The description's premise — "the single choke point where every message enters parsing" — holds only for MarkdownRenderer. website/src/apps/mochi/src/renderer/ChatPanel.tsx runs its own <Markdown> pipeline over agent-streamed content (StreamingMarkdown, explicitly "Ported from KiroCrew's MarkdownRenderer"), so the identical RangeError still takes down that shipped surface. The author disclosed this as a deliberate scope cut, but it's the same logical change: the new util is generic and one call away — apply it there in this PR or land the follow-up before this pattern is considered closed.

Suggestions

• The ~200-line scanner is a parallel implementation of CommonMark container/fence semantics; the fail-closed posture bounds divergence to cosmetic clamping, which is the right trade — keep that invariant explicit if the fence detection is ever extended, since a fail-open divergence re-opens the crash through the guard itself.

[DESIGN-REVIEWED] 9f68e58

[github-actions]

Opus 4.8 Review (fork) — ✅ no blocking findings

Reviewed 9f68e58ea439f79e56556b1c8da53155b98a1e4d via the fork AI-review pipeline; updated in place on each push.

Review details

I've independently verified the clamp logic. My analysis confirms:

• Fence state fails closed: the clamp only enters fence state on a top-level ≤3-space fence line, correctly excludes backtick-info-string pseudo-fences, and its close check mirrors micromark (same char, length ≥ open, whitespace-only trailer). The only divergence direction I could construct is over-clamping (cosmetic), never letting an unclamped deep run reach the parser.
• Per-line container clamp bounds tree depth: >>>>, > > >, and > - > - all count as per-unit runs via the sticky CONTAINER_UNIT regex; escaping unit docs: lazy skill loading design (top-K + per-section budgets) #101's punctuation makes the remainder literal at depth 100 (below the overflow threshold). Cross-line blockquote/list runs don't accumulate past a single line's max, and the multi-line indent vector is capped by MAX_LIST_INDENT_COLS.
• No AUTOSDE violation: the blocking frontend-security rule targets innerHTML/dangerouslySetInnerHTML/HTML-string building; this is pure string transformation on clean before parsing. Ordinary content is untouched.

I could not derive any grounded (a)/(b)/(c) defect at 80+ confidence.

No findings.

[OPUS-REVIEWED] 9f68e58

[github-actions]

GPT 5.6 Review (fork) — 🔴 changes requested (blocking)

Reviewed 9f68e58ea439f79e56556b1c8da53155b98a1e4d via the fork AI-review pipeline; updated in place on each push.

1 of 1 blocking finding(s) are security-class and were withheld from adjudication, so the blocking verdict stands.

BLOCKING -- website/src/components/MarkdownRenderer.tsx:3552 -- Raw HTML nesting bypasses the depth clamp
clean = clampNestingDepth(clean)
Deeply nested <div> message -> unchanged clamp output -> recursive HAST walkers overflow and crash rendering.
Anchor: residual/crash-data-loss-corruption
Fix: Clamp allowlisted raw HTML container nesting before parsing.
[BLOCK-MERGE] 9f68e58
[GPT-REVIEWED] 9f68e58

Adjudication (Opus 4.8) — is blocking on each finding proportionate?

The renderer wires rehype-raw into both rehype pipelines (MarkdownRenderer.tsx:2293 and :2378), so raw HTML embedded in a chat message is parsed into a HAST tree, and module walkers like rehypeUnwrapBlocks recurse over it. clampNestingDepth only clamps CommonMark container markers (>, list bullets/ordered markers) — it never touches HTML tag nesting (CONTAINER_UNIT at clampNestingDepth.ts:243 matches only >/list markers). A deeply nested <div> run therefore reaches the parser unclamped, producing the identical stack-overflow crash class the guard exists to prevent, through an alternate spelling the guard does not cover.

This is the fenced (unbounded-harm) block; the adjudicable block is empty. The raw-HTML nesting vector is not extreme or self-contradicting — it arrives through the very same input-controlled markdown channel as the > vector the PR does clamp, and is just as trivial to produce. No recovery path (transcript render crashes). I cannot complete a rarity argument a human would accept, so the residual risk is not FLAG-worthy.

[ADJUDICATION] 9f68e58 total=0 uphold=0 downgrade=0
[GPT-ADJUDICATED] 9f68e58
[ADJUDICATION-FENCED] 9f68e58 fenced=1 flagged=0
UPHOLD-FENCED F1 website/src/components/MarkdownRenderer.tsx:3552 -- rehype-raw (pipelines at :2293/:2378) parses nested <div> HTML into deep HAST that the container-only clamp never touches, so the same unbounded stack-overflow crash is reachable through an ordinary input-controlled message.
[GPT-ADJUDICATED-FENCED] 9f68e58

[javenciu]

Human-writer override request for the source-position-drift flag (F2), per the adjudication result on this PR:

The flagged drift (data-sourcepos coordinates vs unclamped content;
first-duplicate fallback may miscite a selection location) can occur only on
lines the clamp rewrote — input carrying more than 100 leading container
markers on a single line, which does not occur in human-authored content.
For that adversarial class the alternative to approximate citation is no
render at all (the stack overflow this guard exists to stop). Commit 2
narrows the drift further: the rewrite keeps the kept prefix byte-identical
and inserts exactly one byte, so coordinates are exact for all unclamped
lines and for everything before the clamp point on a clamped line.
Adjudication: total=0 uphold=0 (flag not upheld); requesting human-writer
override on that basis.

[javenciu]

Maintainer re-run request: the CI and Build workflow runs on this PR failed inside the 17:19Z rate-limit window. The Detect changed surface setup job died with API rate limit exceeded for installation (run 34047928722), and the Coverage Gate failure is the missing-inputs cascade from that setup job, not a coverage regression. The review-family checks re-ran after the window and are green at this head (2a03bd2). No code delta is needed: re-running the failed workflows should clear the board. Thanks!