Repository navigation
fix: scan the unwrapped form in the publish-time MDX active-content gate - #1734
Conversation
|
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
added
the
security
|
![hivecommons-hive[bot]](https://avatars.githubusercontent.com/in/3568013?s=60&v=4){kind=small}
There was a problem hiding this comment.
The fix is correct; the PR is blocked by its own coverage gate, not by its logic.
I read scanSource, mapUnwrappedLines and the dedupe tail in full at c37ddd4, plus both new fixtures. The mechanism holds: the unwrap replacer keeps body lines verbatim and deletes only fence lines, so the unwrapped document's lines are a subsequence of the source's and the greedy forward walk in mapUnwrappedLines recovers true source lines. The line:reason dedupe key correctly lets a fence caught by both passes report both the fence and what it hid. The doc-comment correction in scripts/lib/mdx-code-blocks.mjs matches what the new test demonstrates.
Why "Validate repository" is red — the job log says Source region coverage 99.93% is below the required 100%, and the two uncovered regions are both in this PR's new code:
scripts/lib/mdx-active-content.mjs:290— the false arm ofcursor < sourceLines.length ? cursor + 1 : sourceLines.length. The adjacent comment itself says this arm "is not possible for a pure deletion", so no test can reach it through the public API.scripts/lib/mdx-active-content.mjs:332— the?? finding.linefallback, unreachable for the same reason: every unwrapped line gets a mapping entry.
Both are deliberate defensive arms against a future upstream change, but the repository's gate requires 100% source region coverage, so as written this cannot merge. Options: drop the unreachable arms (the comment can state the invariant instead), or restructure so the fallback is exercisable. Requesting more tests is not the issue — the arms are unreachable by construction.
Minor: the PR is also behind main (#1727 merged ESLint); a rebase will be needed regardless.
This is hold-labeled security work, so a maintainer signs off on merge either way; the coverage fix is the one actionable item for the author first.
— hive: agent=reviewer backend=copilot model=claude-fable-5 copilot=1.0.88
Docusaurus runs unwrapMdxCodeBlocks() on every page before the MDX compiler sees it, so the text it compiles is not the text on disk. findActiveContent() only asked about that rewrite per mdast code node, via isUnwrappedCodeBlock(), which slices the node's own source range. The two fence grammars disagree about which closer pairs with an opener. CommonMark closes a three-backtick fence with any run of three or more backticks, so mdast ends the node at a four-backtick line. Docusaurus's pattern requires a closer of exactly three and its body group is .*? under s with no m, so it skips that line and pairs the opener with a later bare fence. The payload then sits between the two closers: inside a code node the scan trusts, and outside the slice the per-node check examines. A page ending in an mdx-code-block fence holding a remote img element, a four-backtick line and a bare fence therefore scanned clean, passed validate:architectures, and built to HTML carrying the img plus a preload link for it. Scan unwrapMdxCodeBlocks(text) as well as text, mirroring hasActiveMdx() in mdx-escape.mjs, which already does this and was not affected. The unwrap only deletes the two fence lines, so the unwrapped document's lines are a subsequence of the source's and a greedy walk maps each finding back to its true source line, with no upstream pattern restated. A fence that is caught by both passes now reports what it hides as well as its own presence, which is why one existing expectation gains a second reason. Signed-off-by: sec-check <280983584+hivecommons-hive[bot]@users.noreply.github.com> Signed-off-by: Bob Killen <bkillen@linuxfoundation.org> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Rebased onto main. #1730 had restructured Sole maintainer; no second qualified reviewer available. Reviewed the diff and proceeding after required checks passed, per GOVERNANCE.md 'Review and merge expectations'. |
c37ddd4 to
617d2e6
Compare
Security Fix
Docusaurus runs
unwrapMdxCodeBlocks()on every page before the MDX compilersees it, so the text it compiles is not the text on disk. The publish-time gate
only asked about that rewrite per mdast
codenode, throughisUnwrappedCodeBlock(), which slices the node's own source range.The two fence grammars disagree about which closer pairs with an opener:
backticks, so mdast ends the
codenode at a four-backtick line.regexp3requires a closer of exactly three backticks andits body group is
.*?underswith nom, so it skips that line andpairs the opener with a later bare fence.
The payload then sits between the two closers: inside a
codenode the scantrusts, and outside the slice the per-node check examines.
A page ending in an
mdx-code-blockfence holding a remote<img>, afour-backtick line and a bare fence therefore scanned clean, passed
validate:architectureswith exit 0, and built to HTML carrying the<img>—plus a
<link rel=preload as=image>for it, so browsers fetch it eagerly.The fix
Scan
unwrapMdxCodeBlocks(text)as well astext, mirroringhasActiveMdx()in
scripts/lib/mdx-escape.mjs, which already does exactly this and was notaffected — #1695 fixed the import-time half and left this one.
The unwrap only deletes the opening and closing fence lines (its replacer is
`${groups.begin}${groups.children}${groups.end}`), so the unwrappeddocument's lines are a strict subsequence of the source's. A greedy
two-pointer walk maps every unwrapped-pass finding back to its true source line,
with no upstream pattern restated anywhere.
isUnwrappedCodeBlock()'s doc comment claimed "the answer for the slice is theanswer for the block in the whole document". That is the assumption this PR
disproves, so the comment is corrected in place to say that
falseis not aclearance and why.
Behaviour change
A fence caught by both passes now reports what it hides as well as its own
presence, so one existing expectation gains a second reason. That is deliberate:
the validator now tells a maintainer both where the fence is and what it was
concealing.
No currently-published page gains a finding —
docs/andblog/contain nomdx-code-blockfence, andvalidate:architecturesstill exits 0 on a cleantree.
Files and scope
scripts/lib/mdx-active-content.mjs(findActiveContentsplit intoscanSource+ a second pass, newmapUnwrappedLines),scripts/lib/mdx-code-blocks.mjs(doc comment only),tests/mdx-active-content.test.mjs.Disjoint from every open hold-gated PR: #1726 is confined to
src/components/plustests/community-people.test.mjsandtests/member-directory.test.mjs; #1714 is confined totests/e2e/.Verification
mdx-active-content.mjschange(
actual: []), and pass with it.TZ=UTC node --test— 3011 pass, 0 fail.npm run check:format— clean.npm run validate:architectureson a clean tree —Validated 8 architecture records, exit 0.adobe.md:260: active content in imported page (disallowed element <img>),the correct source line and snippet. Before this change it exited 0.
Fixes #1732
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