Hacktoberfest 2026: the issues maintainers tagged for October, open and beginner-friendly. Browse Hacktoberfest issues

[sec-check] MDX active-content gate resolves duplicate link definitions last-wins, bypassing the remote-image check

Open
  • Is something wrong?
#1,672 1 comment 0 reactions 0 assignees View on GitHub

Maintainers usually reply within 1 day

@hivecommons-hive is already working on this.

Since Oct 20, 2026.

  • #1673 by @hivecommons-hive — open

Assessment

Difficulty
2/5
Estimated time
1-3 hours
Newbie friendliness
25/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Stale
Tech stack
javascript
Domain
security

Research direction

Read scripts/lib/mdx-active-content.mjs and scripts/lib/architecture-content.mjs, then inspect the duplicate-definition handling described in the issue. Run the named tests in tests/mdx-active-content.test.mjs and tests/architecture-content-clean-markdown.test.mjs; done means both maps use the first definition and the tests cover remote/local ordering and cleanMarkdown() rewriting.

Written by the indexing model from the issue text.

Description

agent/sec-check hive/claimed-by-sec-check security stage/040-covered-by-pr

Security Finding

Severity: medium
Type: unsafe-pattern (security-control bypass)

scripts/lib/mdx-active-content.mjs and scripts/lib/architecture-content.mjs each build their link-reference definition map with a bare Map.set(), so a repeated label resolves to the last definition in the document:

  • scripts/lib/mdx-active-content.mjs:159 — if (node.type === 'definition') definitions.set(node.identifier, node.url);
  • scripts/lib/architecture-content.mjs:81 — if (node.type === 'definition') definitions.set(node.identifier, node.url);

CommonMark resolves a repeated label to the first definition, and the renderer this site actually publishes through does the same. node_modules/mdast-util-to-hast/lib/state.js:227-237 builds its map under if (!map.has(id)) with the comment "Mimick CM behavior of link definitions.", and @mdx-js/mdx runs that transform. So for a document carrying two definitions of one label, the gate inspects a URL the published page never uses.

Reproduction (run at a621619, Node 26, repo node_modules installed)
import { findActiveContent } from './scripts/lib/mdx-active-content.mjs';
import { compile } from '@mdx-js/mdx';

const body = `# Imported architecture

![diagram][d]

[d]: https://beacon.evil.example/px.png
[d]: /img/architectures/local.png
`;

console.log('findActiveContent =>', JSON.stringify(findActiveContent(body)));
console.log(String(await compile(body)).includes('beacon.evil.example'));

Output:

findActiveContent => []
true            <-- compiled page contains src: "https://beacon.evil.example/px.png"

The gate reports zero findings while the compiled page loads the third-party image.

Impact

Bounded, and deliberately stated narrowly:

  • Not XSS. Every definition node is checked for a script-capable scheme regardless of which one wins, because URL_NODES in mdx-active-content.mjs:100 includes definition and that arm runs per node rather than through the map. A javascript: destination is still caught on both copies.
  • The remote-image beacon check is bypassed. reportRemoteImage() is the only check that resolves a reference through the map, so a remote <img> destination reaches the published page unflagged. That is precisely the control the module's own header documents: "this is the gate on what is published, and it has to hold when a page reaches docs/ without passing the importer." A remote image is fetched by every visitor's browser with no user action, disclosing their IP, User-Agent and Referer to a host the page author chose.
  • Reachable at the gate that matters. scripts/validate-architectures.mjs:135 runs findActiveContent() over every .md on disk under docs/architectures/, driven by the filesystem rather than the catalog, so this is the check standing between a page added by a pull request and publication.
  • architecture-content.mjs is a correctness bug, not an exposure. rewriteImages() replaces the whole imageReference node with an inline image, so whichever definition it picks the emitted destination is mirrored, rebased or demoted — the output stays safe. But it applies the image policy to a URL the renderer would not have used, which means the published page can show the wrong diagram, and it leaves the two modules disagreeing about reference resolution when they are written to agree.

Recommendation

Make both maps first-wins, matching CommonMark and mdast-util-to-hast:

scripts/lib/mdx-active-content.mjs

  eachNode(tree, (node) => {
    // First definition wins, as CommonMark specifies and as
    // mdast-util-to-hast implements for the published page; a later
    // duplicate of the same label is dead text the renderer ignores.
    if (node.type === 'definition' && !definitions.has(node.identifier))
      definitions.set(node.identifier, node.url);
  });

scripts/lib/architecture-content.mjs

    if (node.type === 'definition' && !definitions.has(node.identifier))
      definitions.set(node.identifier, node.url);

Both identifier keys stay consistent with the renderer: mdast normalizes identifier, and mdast-util-to-hast upper-cases that normalized value, which is injective, so the two collapse the same labels.

Add regression fixtures so a future refactor cannot reintroduce last-wins:

  • tests/mdx-active-content.test.mjs — a duplicate-label document whose first definition is remote and second is local must report remote image destination …, and the mirror case (first local, second remote) must report nothing.
  • tests/architecture-content-clean-markdown.test.mjs — cleanMarkdown() must rewrite a duplicated reference from its first definition.

Completion criteria

  • mdx-active-content.mjs resolves a repeated label to the first definition
  • architecture-content.mjs resolves a repeated label to the first definition
  • Regression tests cover both orderings in tests/mdx-active-content.test.mjs
  • Regression test covers cleanMarkdown() duplicate-label rewriting

No workflow file is involved; the whole fix is pushable.

Filed by sec-check agent (ACMM L4/L5 — hold-gated mode)

🐝 Hive Agent: security | Instance: hosted-available-lke648397-260827-5n31 | SHA: a621619

— hive: agent=sec-check backend=copilot model=claude-opus-5 copilot=1.0.88

Dominant language
JavaScript
Stars
0
Forks
3
Avg merge
7h 20m
Merged PRs (30d)
641

Getting set up

Open in Codespaces

Starts the project's dev container in your browser, under your own GitHub account.

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from cncf/endusers

All issues in cncf/endusers

Similar issues

More JavaScript issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.