harden: treat all GitHub plugin data as untrusted + tests#2533
Open
kalepail wants to merge 1 commit into
Open
Conversation
Builds on the initial sanitization fix. The CLI plugin list is generated from GitHub repos anyone can opt into via the `stellar-cli-plugin` topic, so the name, description, and URL are all attacker-controlled and land in MDX (which renders Markdown, JSX, and raw HTML). HTML-entity escaping alone still left Markdown vectors live: a repo description could inject a phishing link `[text](url)`, a tracking-pixel image ``, a GFM autolink, or forged headings. Hardening: - sanitizeText now collapses whitespace to a single line (defeating block-level Markdown and multi-line attribute tricks) and backslash-escapes every ASCII punctuation character, so the value can only ever render as literal text. CommonMark/MDX guarantee `\` + punctuation renders as the literal char. - safeUrl allow-lists `https://github.com/...` only; other hosts and `javascript:` / `data:` schemes drop the item (allow-list, not deny-list). - Add a main-module guard so helpers are importable without the network fetch, and a node:test suite covering script/brace/link/image/autolink injection and URL validation. Run with: node --test scripts/stellar_cli_plugins.test.mjs Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Hardens the CLI plugin list generator against Markdown-based injection vectors by treating all GitHub-provided plugin metadata as untrusted when emitting MDX, and adds tests to lock in the sanitization/URL allow-list behavior.
Changes:
- Add
sanitizeText()that collapses whitespace and backslash-escapes ASCII punctuation to neutralize MDX/Markdown/HTML/JSX interpretation. - Add
safeUrl()allow-listing for GitHub URLs and drop plugins with non-allowed URLs. - Add
node:testcoverage and a main-module guard so helpers can be imported without triggering a network call.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| scripts/stellar_cli_plugins.mjs | Implements stricter sanitization + URL allow-listing and makes the script importable for tests via a main guard. |
| scripts/stellar_cli_plugins.test.mjs | Adds node:test suite to validate sanitization properties and URL rejection/acceptance cases. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+32
to
41
| function safeUrl(value) { | ||
| try { | ||
| const url = new URL(String(value)); | ||
| if (url.protocol !== "https:") return null; | ||
| if (url.hostname !== "github.com") return null; | ||
| return url.href; | ||
| } catch { | ||
| return null; | ||
| } | ||
| } |
Comment on lines
+95
to
+97
| res.on("end", () => { | ||
| exportMDX(JSON.parse(data)); | ||
| }); |
|
Preview is available here: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up hardening on top of #2532 (already merged). Closes the residual Markdown-injection vectors that HTML escaping alone left open.
Why more than #2532?
#2532 escaped HTML/JSX characters (
& < > { }), which stops<script>injection. But the generated page renders the repo description as Markdown, and that was still live. Cross-checked via deepwiki (againstfacebook/docusaurus), context7, and web sources — an attacker-controlled description could still inject:[Official — install now](https://evil.com)All plugin data (name, description, URL) comes from any repo that self-applies the
stellar-cli-plugintopic, so it is all untrusted input.Changes
sanitizeText— collapse all whitespace to a single line (defeats block-level Markdown like headings/lists/blockquotes/fences and multi-line attribute tricks), then backslash-escape every ASCII punctuation character. CommonMark guarantees\+ ASCII punctuation renders as the literal character, and MDX explicitly supports\</\{escaping (per the v3 migration docs), so the value can only ever render as literal text — links, images, emphasis, code, autolinks, and HTML/JSX are all neutralized in one uniform pass. Plain prose is visually unchanged (the renderer consumes the backslashes).safeUrl— allow-listhttps://github.com/...only; other hosts andjavascript:/data:schemes drop the item entirely. Allow-list rather than deny-list, per OWASP/Node-security guidance.node:testsuite (no new deps) covering script/brace/link/image/autolink injection and URL validation.Verification
Also ran the real generator against live GitHub data: output renders normally (e.g.
stellar-scaffold/cli), the known-bad repo stays excluded, and every URL is validated.🤖 Generated with Claude Code