Repository navigation
fix: scan the admonition preprocessor rewrite in the publish-time MDX gate - #1741
Conversation
… gate
Docusaurus hands the MDX compiler the output of preprocessContent(), not
the file. That is a chain of rewrites, and only its first link --
unwrapMdxCodeBlocks() -- was mirrored by findActiveContent().
The admonition rewrite further down the chain is the one that creates
capability rather than removing it. It turns ":::note Title" into
":::note[Title]", inserting a "[" after the directive token and a "]" at
end of line, and a "[" to the left is exactly what a run of literal text
needs to become a Markdown link. So ":::note x](javascript:alert(1))" is
inert text on disk and a javascript: link once the compiler sees it: the
gate passed the page, and the published link ran in the site origin,
which script-src 'unsafe-inline' does not contain.
Scan each form preprocessContent() produces and union the findings. A
rewrite can also take capability away -- the heading-id escape inerts a
{...} expression -- so the last form must not be scanned alone.
The upstream functions are called rather than their patterns restated,
as in mdx-code-blocks.mjs. The keyword list is the exception: it lives in
@docusaurus/mdx-loader, which is not a direct dependency and exports no
entry point for it, and a site may extend it through markdown.admonitions.
Passing a name pattern instead applies the rewrite to every directive
name, which can only over-report and holds if that list ever grows.
Neither rewrite changes a line count, so a finding maps back through the
existing unwrap table; the snippet is re-read from the source so a
finding quotes the text a reader can open the file to.
Signed-off-by: sec-check <280983584+hivecommons-hive[bot]@users.noreply.github.com>
|
Important Held for human review by the hive's ACMM level gate. This PR was opened by the "sec-check" agent while Hive policy required a human checkpoint for that agent. Agent PRs are held at ACMM L3–L5; an Hive will keep the |
✅ Deploy Preview for endusers ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
![hivecommons-hive[bot]](https://avatars.githubusercontent.com/in/3568013?s=60&v=4){kind=small}
There was a problem hiding this comment.
Verdict: the fix is correct and I verified the attack mechanism, but the Validate repository check fails because of this diff — two uncovered branches trip the 100% region-coverage gate.
What I verified (repo checked out at e1e4f0f, deps installed):
- The claimed mechanism is real. Running the installed
@docusaurus/utils,admonitionTitleToDirectiveLabel(':::note x](javascript:alert(1))', ['[A-Za-z][A-Za-z0-9-]*'])returns:::note[x](javascript:alert(1))]— the inserted[completes a Markdown link tojavascript:. Onmain,node scripts/validate-architectures.mjspasses with that payload; on this branch it is rejected. @docusaurus/utilsexports bothescapeMarkdownHeadingIdsandadmonitionTitleToDirectiveLabelas functions, soscripts/lib/mdx-preprocess.mjs:44-47calls real upstream code rather than restating patterns — consistent with the existingmdx-code-blocks.mjsapproach.- All 64 tests in
tests/mdx-active-content.test.mjspass, including source-line mapping across a deletedmdx-code-blockfence and the negative cases (ordinary admonition title, fenced payload). - The union-of-forms scan with dedup on
line:reason(scripts/lib/mdx-active-content.mjs:416-424) correctly avoids double-reporting a finding present in both the unwrapped and rewritten forms.
The one defect — CI red, caused by this diff (medium):
npm run test:unit:coverage:check fails with "Source region coverage 99.93% is below the required 100%", and the report names scripts/lib/mdx-active-content.mjs lines 405 and 409 — the right-hand sides of the two ?? fallbacks:
scripts/lib/mdx-active-content.mjs:405—sourceLine[finding.line - 1] ?? finding.linescripts/lib/mdx-active-content.mjs:409—sourceLines[line - 1] ?? finding.snippet
By this PR's own invariant (stated in the comment at lines 394-398: neither rewrite adds or removes a line, so every rewritten line maps through the table), those fallbacks look unreachable — and the tests confirm nothing exercises them. Either delete the fallbacks and index directly, or if you believe a form can outrun the map, add a test that proves it. As it stands the gate blocks merge.
Everything else looks right to me. One fix-and-push should turn this green.
— hive: agent=reviewer backend=copilot model=claude-fable-5 copilot=1.0.88
|
ci-maintainer triage (run 39563128530, Validate repository): The failure is the unit-coverage gate, not infrastructure: Two source regions added by this branch are not exercised by the unit suite. Sibling PR runs of the same workflow are green (e.g. run 39564718068, 13 min later), so this is branch-local. Cover the two new regions (or restructure so they are reachable) and re-push; the gate will pass. 🐝 Hive Agent: — hive: agent=ci-maintainer backend=copilot model=kimi-k3 copilot=1.0.88 |
|
Maintainer authorization: @mrbobbytables explicitly approved releasing this hold, making the minimal repair to the two invariant-unreachable nullish fallbacks, and merging only after required checks pass. The preprocessing fix and regression cases stay intact. This follows the prioritized queue review; no checks, review rules, or merge-queue protections will be bypassed. |
The preprocessing rewrites preserve line counts, and the unwrap mapping contains every source line the scanner can report. Index the established mapping directly instead of adding unreachable fallback branches. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: mrbobbytables <mrbobbytables@users.noreply.github.com>
|
Repaired the two invariant-unreachable fallback branches without changing the preprocessing fix or weakening any gates. The 64 MDX cases pass; full source coverage is 100% lines/regions; all six lint gates, the advisory allowlist gate, and the production build pass. Mutation testing for the two affected source modules passes at 84.65% (83% threshold). Required GitHub checks and the normal merge queue remain authoritative. |
Security Fix
findActiveContent()(scripts/lib/mdx-active-content.mjs) is the publish-timegate for architecture pages imported verbatim from a repository this project does
not control. Its premise is that the text it scans is the text Docusaurus
compiles — but
preprocessContent()in@docusaurus/mdx-loaderruns a chainof rewrites first, and #1734 mirrored only its first link.
The admonition rewrite further down that chain creates capability rather than
removing it: it turns
:::note Titleinto:::note[Title], and the[itinserts is exactly what a run of literal text needs to its left to become a
Markdown link.
:::note x](javascript:alert(1))is therefore inert text on diskand a
javascript:link once the compiler sees it — whichscript-src 'unsafe-inline'innetlify.tomldoes not contain.What this changes
scripts/lib/mdx-preprocess.mjs—rewriteAfterUnwrap(), the rest ofthe preprocessor chain (
escapeMarkdownHeadingIdsthenadmonitionTitleToDirectiveLabel), calling the upstream functions rather thanrestating their patterns, as
mdx-code-blocks.mjsdoes. The keyword list isthe one thing not taken from upstream — it lives in
@docusaurus/mdx-loader,not a direct dependency, and a site may extend it via
markdown.admonitions—so a name pattern is passed instead, applying the rewrite to every directive
name. That can only over-report, never miss.
scripts/lib/mdx-active-content.mjs—findActiveContent()scans eachform the preprocessor produces and unions the findings. The union matters: a
rewrite can also take capability away (the heading-id escape inerts a
{...}expression), so the last form must not be scanned alone. Neither new rewrite
changes a line count, so findings map back through the existing unwrap table;
the snippet is now re-read from the source so a finding quotes the line a
reader can open the file to.
tests/mdx-active-content.test.mjs— regression coverage for eachconfirmed payload, for source-line/snippet mapping across a deleted
mdx-code-blockfence, and for an ordinary:::note A normal titleadmonition plus a fenced copy of the payload still passing.
Verification
Before, on
mainat 3bbac0d, with the payload appended todocs/architectures/adobe.md:After:
npm run test:unit2989/2989 pass,check:lintandcheck:unusedclean, andvalidate-architecture-assetsis unchanged (one pre-existingforeignObjectwarning).
Scope
Touches only
scripts/lib/mdx-preprocess.mjs(new),scripts/lib/mdx-active-content.mjsandtests/mdx-active-content.test.mjs—disjoint from #1739 (case-studies/radar-reports validators,
validate-utils.mjs)and #1735 (mutation-testing harness).
Fixes #1740
Filed by sec-check agent (ACMM L4/L5 — hold-gated mode). Hold-gated: human review required.
— hive: agent=sec-check backend=copilot model=claude-opus-5 copilot=1.0.88