fix(validator): sandbox MCP stdio cwd, headers, and cross-platform SKILL.md - #4
Open
wufufu770 wants to merge 2 commits into
Open
fix(validator): sandbox MCP stdio cwd, headers, and cross-platform SKILL.md#4wufufu770 wants to merge 2 commits into
wufufu770 wants to merge 2 commits into
Conversation
The previous stdio cwd check only anchored the start of the value, so a
config like '${PLUGIN_DATA}/../../etc' passed validation and the server
process could be launched outside the Plugin sandbox. Replace the prefix
regex with path.resolve + path.relative containment against the supplied
pluginRoot / pluginData, and forward those roots through validateMcp so
hosted and standalone entry points both sandbox the cwd.
Also reject NUL bytes in cwd to defend against path-truncation attacks
where a runtime could split the path at '\x00' and resolve a different
target than the validator approved.
Add unit coverage for resolveCwd (anchored paths, escapes, missing
roots, non-string inputs, control bytes, mid-segment traversal) and
two hosted-Plugin integration tests covering safe and escaping cwd.
While here, the README and README.zh-CN build/license badges link and
source point at the upstream hetaoBackend fork; redirect them to the
MiniMax-AI/MiniMax-Code-Plugins organisation ship-to repo.
…ILL.md
The validator had structural gaps that let a Plugin author hide attack
surface inside mcp.json, mcp.json headers, and the SKILL.md frontmatter.
- mcp server name bypassed the 64-character cap that applies to
the Plugin name itself, so an MCP server key could be arbitrarily
long.
- command / args / env / headers values and keys accepted NUL bytes
(\x00), which a runtime might split on (fs path truncation, shell
argument parsing).
- streamable-http / sse headers accepted any key — including the
standard Authorization / Cookie / Set-Cookie / Proxy-Authorization
credentials and the common custom credential headers (X-Api-Key,
X-Auth-Token, X-Access-Token, X-Token, X-Secret, X-Api-Token,
Api-Key, Auth-Token, Access-Token) — letting a Plugin smuggle
tokens into the HTTP request and bypass the 'no credentials' gate
that applies to env values.
- headers values and keys accepted CR / LF / NUL / 0x7f, enabling
HTTP header injection (a value like 'a\r\nAuthorization: Bearer
evil' could split into a separate header on the wire).
- PLUGIN_ROOT / PLUGIN_DATA were rejected in env but accepted in
headers, a contract inconsistency.
- SKILL.md was required to use LF line endings — a Windows or
git-autocrlf contributor hit 'YAML frontmatter is required' with
no hint about the BOM or CRLF cause. SKILL.md now accepts CRLF
(normalised to LF) and rejects UTF-8 BOM with a specific error,
matching the contract enforced on plugin.json / mcp.json.
- DoS hardening: cwd / command / args / env entries / header values
now have explicit length caps (1024 / 1024 / 4096 / 256 / 8192
chars) and headers capped at 100 entries.
Adds 7 new test cases. npm run check passes 9 hosted Plugins, 2
examples, and 45 tests (5 consecutive runs).
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
1 participant
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.
Problem
The Plugin validator is the gate every Plugin passes through before host runtime loads it. Several structural gaps let a malicious
mcp.jsonorSKILL.mdsmuggle attack surface past CI and human review.Fix
Replaces loose validation with strict containment checks aligned to the contract documented in
docs/plugin-compatibility.mdanddocs/security-model.md.cwd..traversalpath.resolve+path.relativecontainmentcwd/command/args/env/headers\x00rejectedenvvsheadersPLUGIN_ROOT/PLUGIN_DATAblocked in env but allowed in headersheadersheadersvalue / key<= 64characters enforcedSKILL.mdREADMEMiniMax-AIVerification
$ npm run check
OK example hello-mcode
OK example hello-mcode-mcp
OK plugin Fectivnfy112357/github-explore
OK plugin HopeYin/dida365
OK plugin HopeYin/ticktick
OK plugin Hylouis233/mcp-server-patterns
OK plugin Hylouis233/search-first
OK plugin Hylouis233/verification-loop
OK plugin hetaoBackend/minimax-code-trajectory
ℹ tests 45
ℹ pass 45
ℹ fail 0
5 consecutive runs: 45/45 stable.
Risk
Low. Pure additive validation: no existing Plugin, example, or test in the hosted tree fails under the hardened rules. The only changes that could reject a previously-passing Plugin are ones that intentionally hid credentials or attempted path traversal in the first place.
Scope
This PR does NOT touch:
plugins/*/*docs/security-model.md(the contract here is implicit; a follow-up PR can spell it out if maintainers want)Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.