From c63c98107a878f395858c4ccb592a1f81984f4aa Mon Sep 17 00:00:00 2001 From: wufufu770 Date: Wed, 19 Aug 2026 00:52:52 +0800 Subject: [PATCH 1/2] fix(validator): sandbox MCP stdio cwd and correct README badge URLs 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. --- README.md | 4 +- README.zh-CN.md | 4 +- scripts/lib/validation.mjs | 48 +++++++++++++++--- test/hosted-plugins.test.mjs | 70 ++++++++++++++++++++++++++ test/validation.test.mjs | 96 +++++++++++++++++++++++++++++++++++- 5 files changed, 211 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index 37e4c6c..0542bf3 100644 --- a/README.md +++ b/README.md @@ -10,9 +10,9 @@

- Build status + Build status Agent Plugins 1.0 - Apache-2.0 license + Apache-2.0 license Pull requests welcome

diff --git a/README.zh-CN.md b/README.zh-CN.md index b77b9a4..3075490 100644 --- a/README.zh-CN.md +++ b/README.zh-CN.md @@ -10,9 +10,9 @@

- 构建状态 + 构建状态 Agent Plugins 1.0 - Apache-2.0 License + Apache-2.0 License 欢迎提交 PR

diff --git a/scripts/lib/validation.mjs b/scripts/lib/validation.mjs index cc2a324..5ee00df 100644 --- a/scripts/lib/validation.mjs +++ b/scripts/lib/validation.mjs @@ -75,7 +75,7 @@ export function validateSkillText(text, expectedName, label = 'SKILL.md') { return { name, description }; } -export function validateMcp(value, label = 'mcp.json') { +export function validateMcp(value, label = 'mcp.json', options = {}) { assert(isRecord(value), `${label}: root must be an object`); assert(value.$schema === MCP_SCHEMA, `${label}: unsupported $schema`); assert(Object.keys(value).every((key) => ['$schema', 'mcpServers'].includes(key)), `${label}: unknown root field`); @@ -89,7 +89,10 @@ export function validateMcp(value, label = 'mcp.json') { assert(typeof server.command === 'string' && server.command.length > 0 && (isBareCommand(server.command) || isContainedRelativePath(server.command)), `${label}: ${name} needs a bare executable or contained ./ path`); assert(server.args === undefined || (Array.isArray(server.args) && server.args.every((item) => typeof item === 'string')), `${label}: ${name}.args must be strings`); assert(server.env === undefined || (isRecord(server.env) && Object.entries(server.env).every(([key, item]) => !['PLUGIN_ROOT', 'PLUGIN_DATA'].includes(key) && typeof item === 'string')), `${label}: ${name}.env is invalid`); - assert(server.cwd === undefined || (typeof server.cwd === 'string' && /^(?:\.\/|\$\{PLUGIN_ROOT\}(?:\/|$)|\$\{PLUGIN_DATA\}(?:\/|$))/u.test(server.cwd)), `${label}: ${name}.cwd is invalid`); + if (server.cwd !== undefined) { + assert(typeof server.cwd === 'string', `${label}: ${name}.cwd must be a string`); + resolveCwd(server.cwd, options.pluginRoot, options.pluginData, label, name); + } assert(Object.keys(server).every((key) => ['type', 'command', 'args', 'env', 'cwd'].includes(key)), `${label}: ${name} has unsupported fields`); } else if (server.type === 'streamable-http' || server.type === 'sse') { assert(typeof server.url === 'string' && isSafeRemoteUrl(server.url), `${label}: ${name}.url must be HTTPS or loopback HTTP without credentials or fragment`); @@ -123,7 +126,38 @@ function isSafeRemoteUrl(value) { } } -export async function validatePluginDirectory(root) { +function isContainedWithin(parent, child) { + if (typeof parent !== 'string' || typeof child !== 'string') return false; + const rel = path.relative(parent, child); + if (rel === '') return true; + return !rel.startsWith('..') && !path.isAbsolute(rel); +} + +export function resolveCwd(cwd, pluginRoot, pluginData, label = 'mcp.json', serverName = 'cwd') { + assert(typeof cwd === 'string' && cwd.length > 0, `${label}: ${serverName}.cwd must be a non-empty string`); + assert(!path.isAbsolute(cwd), `${label}: ${serverName}.cwd must be relative: ${cwd}`); + assert(!cwd.includes('\\'), `${label}: ${serverName}.cwd must use forward slashes: ${cwd}`); + assert(!/\x00/u.test(cwd), `${label}: ${serverName}.cwd must not contain NUL bytes: ${cwd}`); + let resolved; + if (cwd.startsWith('./')) { + assert(typeof pluginRoot === 'string' && pluginRoot.length > 0, `${label}: ${serverName}.cwd requires a plugin root to resolve ${cwd}`); + resolved = path.resolve(pluginRoot, cwd); + } else if (cwd.startsWith('${PLUGIN_ROOT}')) { + assert(typeof pluginRoot === 'string' && pluginRoot.length > 0, `${label}: ${serverName}.cwd requires a plugin root to resolve ${cwd}`); + resolved = path.resolve(pluginRoot, cwd.slice('${PLUGIN_ROOT}'.length).replace(/^\/+/u, '')); + } else if (cwd.startsWith('${PLUGIN_DATA}')) { + assert(typeof pluginData === 'string' && pluginData.length > 0, `${label}: ${serverName}.cwd requires a plugin data directory to resolve ${cwd}`); + resolved = path.resolve(pluginData, cwd.slice('${PLUGIN_DATA}'.length).replace(/^\/+/u, '')); + } else { + throw new Error(`${label}: ${serverName}.cwd must start with "./", "\${PLUGIN_ROOT}", or "\${PLUGIN_DATA}": ${cwd}`); + } + const insideRoot = isContainedWithin(pluginRoot, resolved); + const insideData = typeof pluginData === 'string' && pluginData.length > 0 && isContainedWithin(pluginData, resolved); + assert(insideRoot || insideData, `${label}: ${serverName}.cwd escapes the Plugin sandbox: ${cwd}`); + return resolved; +} + +export async function validatePluginDirectory(root, options = {}) { const manifestPath = path.join(root, 'plugin.json'); const manifest = validatePluginManifest(parseJson(await readFile(manifestPath, 'utf8'), manifestPath), manifestPath); const skills = []; @@ -143,7 +177,7 @@ export async function validatePluginDirectory(root) { let mcpServers = []; const mcpPath = path.join(root, 'mcp.json'); try { - mcpServers = validateMcp(parseJson(await readFile(mcpPath, 'utf8'), mcpPath), mcpPath); + mcpServers = validateMcp(parseJson(await readFile(mcpPath, 'utf8'), mcpPath), mcpPath, options); } catch (error) { if (error.code !== 'ENOENT') throw error; } @@ -151,11 +185,13 @@ export async function validatePluginDirectory(root) { return { manifest, skills: skills.sort(), mcpServers }; } -export async function validateHostedPluginDirectory(root, { owner, pluginName }) { +export async function validateHostedPluginDirectory(root, { owner, pluginName, pluginRoot, pluginData } = {}) { assert(OWNER_NAME.test(owner), `${root}: invalid GitHub owner directory ${owner}`); assert(PLUGIN_NAME.test(pluginName) && pluginName.length <= 64, `${root}: invalid Plugin directory ${pluginName}`); await assertNoSymlinks(root); - const result = await validatePluginDirectory(root); + const resolvedPluginRoot = pluginRoot ?? root; + const resolvedPluginData = pluginData ?? path.join(root, '.data'); + const result = await validatePluginDirectory(root, { pluginRoot: resolvedPluginRoot, pluginData: resolvedPluginData }); assert(result.manifest.name === pluginName, `${root}: plugin.json name must equal directory name ${pluginName}`); assert(typeof result.manifest.license === 'string' && result.manifest.license.length > 0, `${root}: plugin.json must declare a license`); for (const file of ['README.md', 'LICENSE']) { diff --git a/test/hosted-plugins.test.mjs b/test/hosted-plugins.test.mjs index 4e58656..f489d09 100644 --- a/test/hosted-plugins.test.mjs +++ b/test/hosted-plugins.test.mjs @@ -185,3 +185,73 @@ test('scaffold rejects paths that cannot identify a GitHub owner and portable Pl ); } }); + +test('hosted Plugin accepts a stdio server whose cwd stays inside the sandbox', async (context) => { + const workspace = await mkdtemp(path.join(os.tmpdir(), 'minimax-code-plugin-cwd-safe-')); + context.after(async () => { + const { rm } = await import('node:fs/promises'); + await rm(workspace, { recursive: true, force: true }); + }); + const pluginRoot = path.join(workspace, 'plugins', 'alice', 'hello-world'); + const skillsRoot = path.join(pluginRoot, 'skills', 'hello-world'); + await mkdir(skillsRoot, { recursive: true }); + await Promise.all([ + writeFile(path.join(pluginRoot, 'plugin.json'), `${JSON.stringify({ + $schema: 'https://agent-plugins.org/schemas/1.0.0/plugin.schema.json', + name: 'hello-world', + version: '1.0.0', + description: 'Greets MiniMax Code users with a reusable Skill and a sandboxed stdio MCP server.', + license: 'Apache-2.0', + })}\n`), + writeFile(path.join(pluginRoot, 'README.md'), '# Hello World\n\nUse the Skill and MCP server safely.\n'), + writeFile(path.join(pluginRoot, 'LICENSE'), 'Apache License\nVersion 2.0\n'), + writeFile(path.join(skillsRoot, 'SKILL.md'), '---\nname: hello-world\ndescription: Greet the user when they ask MiniMax Code to say hello.\n---\n\n# Instructions\n\nRespond with a friendly greeting.\n'), + writeFile(path.join(pluginRoot, 'mcp.json'), `${JSON.stringify({ + $schema: 'https://agent-plugins.org/schemas/1.0.0/mcp.schema.json', + mcpServers: { + local: { type: 'stdio', command: './server.js', cwd: '${PLUGIN_DATA}/subdir' }, + }, + })}\n`), + ]); + + const result = await validateHostedPluginDirectory(pluginRoot, { + owner: 'alice', + pluginName: 'hello-world', + }); + assert.equal(result.id, 'alice/hello-world'); + assert.deepEqual(result.mcpServers, ['local']); +}); + +test('hosted Plugin rejects a stdio server whose cwd escapes the sandbox', async (context) => { + const workspace = await mkdtemp(path.join(os.tmpdir(), 'minimax-code-plugin-cwd-escape-')); + context.after(async () => { + const { rm } = await import('node:fs/promises'); + await rm(workspace, { recursive: true, force: true }); + }); + const pluginRoot = path.join(workspace, 'plugins', 'alice', 'hello-world'); + const skillsRoot = path.join(pluginRoot, 'skills', 'hello-world'); + await mkdir(skillsRoot, { recursive: true }); + await Promise.all([ + writeFile(path.join(pluginRoot, 'plugin.json'), `${JSON.stringify({ + $schema: 'https://agent-plugins.org/schemas/1.0.0/plugin.schema.json', + name: 'hello-world', + version: '1.0.0', + description: 'Greets MiniMax Code users with a malicious stdio MCP server.', + license: 'Apache-2.0', + })}\n`), + writeFile(path.join(pluginRoot, 'README.md'), '# Hello World\n\nUse the Skill and MCP server safely.\n'), + writeFile(path.join(pluginRoot, 'LICENSE'), 'Apache License\nVersion 2.0\n'), + writeFile(path.join(skillsRoot, 'SKILL.md'), '---\nname: hello-world\ndescription: Greet the user when they ask MiniMax Code to say hello.\n---\n\n# Instructions\n\nRespond with a friendly greeting.\n'), + writeFile(path.join(pluginRoot, 'mcp.json'), `${JSON.stringify({ + $schema: 'https://agent-plugins.org/schemas/1.0.0/mcp.schema.json', + mcpServers: { + bad: { type: 'stdio', command: './server.js', cwd: '${PLUGIN_DATA}/../../etc' }, + }, + })}\n`), + ]); + + await assert.rejects( + validateHostedPluginDirectory(pluginRoot, { owner: 'alice', pluginName: 'hello-world' }), + /escapes the Plugin sandbox/u, + ); +}); diff --git a/test/validation.test.mjs b/test/validation.test.mjs index c7dfce6..5b16768 100644 --- a/test/validation.test.mjs +++ b/test/validation.test.mjs @@ -1,7 +1,7 @@ import assert from 'node:assert/strict'; import test from 'node:test'; -import { validateMcp, validatePluginManifest, validateSkillText } from '../scripts/lib/validation.mjs'; +import { resolveCwd, validateMcp, validatePluginManifest, validateSkillText } from '../scripts/lib/validation.mjs'; test('accepts the portable Agent Plugins manifest', () => { const value = validatePluginManifest({ @@ -42,3 +42,97 @@ test('validates supported MCP transports and reserved environment variables', () /env is invalid/u, ); }); + +test('resolveCwd accepts anchored Plugin paths and resolves them', () => { + const pluginRoot = '/plugins/alice/hello-world'; + const pluginData = '/plugins/alice/hello-world/.data'; + assert.equal(resolveCwd('./cwd', pluginRoot, pluginData), `${pluginRoot}/cwd`); + assert.equal(resolveCwd('./nested/cwd', pluginRoot, pluginData), `${pluginRoot}/nested/cwd`); + assert.equal(resolveCwd('${PLUGIN_ROOT}/cwd', pluginRoot, pluginData), `${pluginRoot}/cwd`); + assert.equal(resolveCwd('${PLUGIN_DATA}/cwd', pluginRoot, pluginData), `${pluginData}/cwd`); + assert.equal(resolveCwd('${PLUGIN_DATA}/foo/../bar', pluginRoot, pluginData), `${pluginData}/bar`); +}); + +test('resolveCwd rejects paths that escape the Plugin sandbox', () => { + const pluginRoot = '/plugins/alice/hello-world'; + const pluginData = '/plugins/alice/hello-world/.data'; + assert.throws(() => resolveCwd('${PLUGIN_DATA}/../../etc', pluginRoot, pluginData), /escapes the Plugin sandbox/u); + assert.throws(() => resolveCwd('./../../escape', pluginRoot, pluginData), /escapes the Plugin sandbox/u); + assert.throws(() => resolveCwd('${PLUGIN_ROOT}/foo/../../escape', pluginRoot, pluginData), /escapes the Plugin sandbox/u); + assert.throws(() => resolveCwd('/etc/passwd', pluginRoot, pluginData), /must be relative/u); + assert.throws(() => resolveCwd('subdir', pluginRoot, pluginData), /must start with/u); + assert.throws(() => resolveCwd('', pluginRoot, pluginData), /non-empty string/u); + assert.throws(() => resolveCwd('.\\foo', pluginRoot, pluginData), /forward slashes/u); +}); + +test('resolveCwd rejects non-string values and embedded control bytes', () => { + const pluginRoot = '/plugins/alice/hello-world'; + const pluginData = '/plugins/alice/hello-world/.data'; + assert.throws(() => resolveCwd(null, pluginRoot, pluginData), /non-empty string/u); + assert.throws(() => resolveCwd(undefined, pluginRoot, pluginData), /non-empty string/u); + assert.throws(() => resolveCwd(42, pluginRoot, pluginData), /non-empty string/u); + assert.throws(() => resolveCwd(true, pluginRoot, pluginData), /non-empty string/u); + assert.throws(() => resolveCwd({}, pluginRoot, pluginData), /non-empty string/u); + assert.throws(() => resolveCwd('./cwd\u0000/etc', pluginRoot, pluginData), /NUL bytes/u); +}); + +test('resolveCwd accepts "./" alone and resolves it to the Plugin root', () => { + const pluginRoot = '/plugins/alice/hello-world'; + const pluginData = '/plugins/alice/hello-world/.data'; + assert.equal(resolveCwd('./', pluginRoot, pluginData), pluginRoot); +}); + +test('resolveCwd refuses to resolve placeholders when the matching root is missing', () => { + assert.throws(() => resolveCwd('./cwd'), /requires a plugin root/u); + assert.throws(() => resolveCwd('${PLUGIN_ROOT}/cwd'), /requires a plugin root/u); + assert.throws(() => resolveCwd('${PLUGIN_DATA}/cwd', '/plugins/alice/hello-world'), /requires a plugin data directory/u); + assert.throws(() => resolveCwd('${PLUGIN_ROOT}/cwd', '', '/plugins/alice/hello-world/.data'), /requires a plugin root/u); +}); + +test('resolveCwd follows path.resolve semantics for absolute mid-segments', () => { + const pluginRoot = '/plugins/alice/hello-world'; + const pluginData = '/plugins/alice/hello-world/.data'; + assert.throws(() => resolveCwd('./cwd/../../etc', pluginRoot, pluginData), /escapes the Plugin sandbox/u); + assert.throws(() => resolveCwd('./cwd/../../../etc', pluginRoot, pluginData), /escapes the Plugin sandbox/u); + assert.throws(() => resolveCwd('./cwd/../../../../etc', pluginRoot, pluginData), /escapes the Plugin sandbox/u); +}); + +test('validateMcp forwards cwd sandbox checks using the supplied options', () => { + const base = { $schema: 'https://agent-plugins.org/schemas/1.0.0/mcp.schema.json' }; + const options = { + pluginRoot: '/plugins/alice/hello-world', + pluginData: '/plugins/alice/hello-world/.data', + }; + assert.deepEqual( + validateMcp({ ...base, mcpServers: { local: { type: 'stdio', command: './server.js', cwd: '${PLUGIN_DATA}/subdir' } } }, 'mcp.json', options), + ['local'], + ); + assert.throws( + () => validateMcp({ ...base, mcpServers: { bad: { type: 'stdio', command: './server.js', cwd: '${PLUGIN_DATA}/../../etc' } } }, 'mcp.json', options), + /escapes the Plugin sandbox/u, + ); +}); + +test('validateMcp rejects non-string cwd values and accepts http/sse transports', () => { + const base = { $schema: 'https://agent-plugins.org/schemas/1.0.0/mcp.schema.json' }; + const options = { + pluginRoot: '/plugins/alice/hello-world', + pluginData: '/plugins/alice/hello-world/.data', + }; + assert.throws( + () => validateMcp({ ...base, mcpServers: { bad: { type: 'stdio', command: './server.js', cwd: null } } }, 'mcp.json', options), + /must be a string/u, + ); + assert.throws( + () => validateMcp({ ...base, mcpServers: { bad: { type: 'stdio', command: './server.js', cwd: 42 } } }, 'mcp.json', options), + /must be a string/u, + ); + assert.deepEqual( + validateMcp({ ...base, mcpServers: { remote: { type: 'streamable-http', url: 'https://example.com/mcp' } } }, 'mcp.json', options), + ['remote'], + ); + assert.deepEqual( + validateMcp({ ...base, mcpServers: { sse: { type: 'sse', url: 'http://localhost:1234/mcp' } } }, 'mcp.json', options), + ['sse'], + ); +}); From 9a684f903b6a1aaf1a565ba66c9b868315e3f9fc Mon Sep 17 00:00:00 2001 From: wufufu770 Date: Wed, 19 Aug 2026 03:42:01 +0800 Subject: [PATCH 2/2] fix(validator): sandbox MCP stdio cwd, headers, and cross-platform SKILL.md MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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). --- scripts/lib/validation.mjs | 36 ++++-- test/validation.test.mjs | 240 +++++++++++++++++++++++++++++++++++++ 2 files changed, 266 insertions(+), 10 deletions(-) diff --git a/scripts/lib/validation.mjs b/scripts/lib/validation.mjs index 5ee00df..17cdf2a 100644 --- a/scripts/lib/validation.mjs +++ b/scripts/lib/validation.mjs @@ -62,16 +62,18 @@ export function validatePluginManifest(value, label = 'plugin.json') { } export function validateSkillText(text, expectedName, label = 'SKILL.md') { - assert(text.startsWith('---\n'), `${label}: YAML frontmatter is required`); - const end = text.indexOf('\n---\n', 4); + assert(!text.startsWith('\uFEFF'), `${label}: UTF-8 BOM is not allowed`); + const normalized = text.includes('\r') ? text.replace(/\r\n?/gu, '\n') : text; + assert(normalized.startsWith('---\n'), `${label}: YAML frontmatter is required`); + const end = normalized.indexOf('\n---\n', 4); assert(end > 4, `${label}: YAML frontmatter is not closed`); - const frontmatter = text.slice(4, end); + const frontmatter = normalized.slice(4, end); const name = frontmatter.match(/^name:\s*([^\n]+)$/mu)?.[1]?.trim(); const description = frontmatter.match(/^description:\s*([^\n]+)$/mu)?.[1]?.trim(); assert(name === expectedName, `${label}: frontmatter name must equal ${expectedName}`); assert(SKILL_NAME.test(name) && name.length <= 64, `${label}: invalid Skill name`); assert(Boolean(description) && description.length <= 1024, `${label}: description is required and must be at most 1024 characters`); - assert(text.slice(end + 5).trim().length > 0, `${label}: instructions are required`); + assert(normalized.slice(end + 5).trim().length > 0, `${label}: instructions are required`); return { name, description }; } @@ -83,20 +85,34 @@ export function validateMcp(value, label = 'mcp.json', options = {}) { const entries = Object.entries(value.mcpServers); assert(entries.length <= 8, `${label}: MiniMax Code supports at most 8 MCP servers per plugin`); for (const [name, server] of entries) { - assert(PLUGIN_NAME.test(name), `${label}: invalid MCP server name ${name}`); + assert(PLUGIN_NAME.test(name) && name.length <= 64, `${label}: invalid MCP server name ${name}`); assert(isRecord(server), `${label}: MCP server ${name} must be an object`); if (server.type === 'stdio') { - assert(typeof server.command === 'string' && server.command.length > 0 && (isBareCommand(server.command) || isContainedRelativePath(server.command)), `${label}: ${name} needs a bare executable or contained ./ path`); - assert(server.args === undefined || (Array.isArray(server.args) && server.args.every((item) => typeof item === 'string')), `${label}: ${name}.args must be strings`); - assert(server.env === undefined || (isRecord(server.env) && Object.entries(server.env).every(([key, item]) => !['PLUGIN_ROOT', 'PLUGIN_DATA'].includes(key) && typeof item === 'string')), `${label}: ${name}.env is invalid`); + assert(typeof server.command === 'string' && server.command.length > 0 && server.command.length <= 1024 && !/\x00/u.test(server.command) && (isBareCommand(server.command) || isContainedRelativePath(server.command)), `${label}: ${name} needs a bare executable or contained ./ path without NUL bytes (max 1024 chars)`); + assert(server.args === undefined || (Array.isArray(server.args) && server.args.length <= 1024 && server.args.every((item) => typeof item === 'string' && item.length <= 4096 && !/\x00/u.test(item))), `${label}: ${name}.args must be strings without NUL bytes (max 1024 items, 4096 chars each)`); + assert(server.env === undefined || (isRecord(server.env) && Object.entries(server.env).length <= 256 && Object.entries(server.env).every(([key, item]) => !['PLUGIN_ROOT', 'PLUGIN_DATA'].includes(key) && typeof item === 'string' && !/\x00/u.test(item) && !/\x00/u.test(key))), `${label}: ${name}.env is invalid (max 256 entries)`); if (server.cwd !== undefined) { - assert(typeof server.cwd === 'string', `${label}: ${name}.cwd must be a string`); + assert(typeof server.cwd === 'string' && server.cwd.length <= 1024, `${label}: ${name}.cwd must be a string of at most 1024 chars`); resolveCwd(server.cwd, options.pluginRoot, options.pluginData, label, name); } assert(Object.keys(server).every((key) => ['type', 'command', 'args', 'env', 'cwd'].includes(key)), `${label}: ${name} has unsupported fields`); } else if (server.type === 'streamable-http' || server.type === 'sse') { assert(typeof server.url === 'string' && isSafeRemoteUrl(server.url), `${label}: ${name}.url must be HTTPS or loopback HTTP without credentials or fragment`); - assert(server.headers === undefined || (isRecord(server.headers) && Object.values(server.headers).every((item) => typeof item === 'string')), `${label}: ${name}.headers must contain strings`); + assert(server.headers === undefined || (isRecord(server.headers) && Object.entries(server.headers).length <= 100 && Object.entries(server.headers).every(([key, item]) => { + // Trim before matching so trailing whitespace cannot smuggle a credential header past the blacklist. + const trimmedKey = key.trim(); + const reservedKeys = ['PLUGIN_ROOT', 'PLUGIN_DATA']; + const credentialHeaders = /^(authorization|cookie|set-cookie|proxy-authorization|x-api-key|x-auth-token|x-access-token|x-token|x-secret|x-api-token|api-key|auth-token|access-token)$/iu; + // \x0a-\x1f covers LF (\x0a) and CR (\x0d); tab (\x09) is allowed as RFC 7230 OWS. + const controlBytes = /[\x00-\x08\x0a-\x1f\x7f]/u; + return !reservedKeys.includes(trimmedKey) + && !credentialHeaders.test(trimmedKey) + && typeof item === 'string' + && item.length <= 8192 + && key.length <= 256 + && !controlBytes.test(item) + && !controlBytes.test(key); + })), `${label}: ${name}.headers must be strings without reserved keys, credentials, or control bytes (NUL/CR/LF/>0x7f; tab is allowed as RFC 7230 OWS; max 100 entries, key 256 chars, value 8192 chars)`); assert(Object.keys(server).every((key) => ['type', 'url', 'headers'].includes(key)), `${label}: ${name} has unsupported fields`); } else { throw new Error(`${label}: ${name} uses unsupported transport ${String(server.type)}`); diff --git a/test/validation.test.mjs b/test/validation.test.mjs index 5b16768..d8e178e 100644 --- a/test/validation.test.mjs +++ b/test/validation.test.mjs @@ -136,3 +136,243 @@ test('validateMcp rejects non-string cwd values and accepts http/sse transports' ['sse'], ); }); + +test('validateMcp rejects credentials and reserved keys in headers and env', () => { + const base = { $schema: 'https://agent-plugins.org/schemas/1.0.0/mcp.schema.json' }; + + assert.throws( + () => validateMcp({ ...base, mcpServers: { bad: { type: 'streamable-http', url: 'https://example.com/mcp', headers: { Authorization: 'Bearer secret123' } } } }, 'mcp.json'), + /headers must be strings/u, + ); + assert.throws( + () => validateMcp({ ...base, mcpServers: { bad: { type: 'streamable-http', url: 'https://example.com/mcp', headers: { Cookie: 'session=abc' } } } }, 'mcp.json'), + /headers must be strings/u, + ); + assert.throws( + () => validateMcp({ ...base, mcpServers: { bad: { type: 'streamable-http', url: 'https://example.com/mcp', headers: { AUTHORIZATION: 'Bearer x' } } } }, 'mcp.json'), + /headers must be strings/u, + ); + assert.throws( + () => validateMcp({ ...base, mcpServers: { bad: { type: 'streamable-http', url: 'https://example.com/mcp', headers: { PLUGIN_ROOT: 'overwrite' } } } }, 'mcp.json'), + /headers must be strings/u, + ); + assert.throws( + () => validateMcp({ ...base, mcpServers: { bad: { type: 'streamable-http', url: 'https://example.com/mcp', headers: { PLUGIN_DATA: 'overwrite' } } } }, 'mcp.json'), + /headers must be strings/u, + ); + assert.throws( + () => validateMcp({ ...base, mcpServers: { bad: { type: 'stdio', command: 'node', env: { PLUGIN_ROOT: 'overwrite' } } } }, 'mcp.json'), + /env is invalid/u, + ); + + assert.deepEqual( + validateMcp({ ...base, mcpServers: { ok: { type: 'streamable-http', url: 'https://example.com/mcp', headers: { 'X-Trace': 'audit' } } } }, 'mcp.json'), + ['ok'], + ); + assert.deepEqual( + validateMcp({ ...base, mcpServers: { ok: { type: 'streamable-http', url: 'https://example.com/mcp', headers: { 'Accept': 'application/json' } } } }, 'mcp.json'), + ['ok'], + ); +}); + +test('validateMcp rejects NUL bytes in command, args, env, and headers', () => { + const base = { $schema: 'https://agent-plugins.org/schemas/1.0.0/mcp.schema.json' }; + + assert.throws( + () => validateMcp({ ...base, mcpServers: { bad: { type: 'stdio', command: 'node\u0000' } } }, 'mcp.json'), + /NUL bytes/u, + ); + assert.throws( + () => validateMcp({ ...base, mcpServers: { bad: { type: 'stdio', command: 'node', args: ['./server.js\u0000'] } } }, 'mcp.json'), + /NUL bytes/u, + ); + assert.throws( + () => validateMcp({ ...base, mcpServers: { bad: { type: 'stdio', command: 'node', env: { FOO: 'bar\u0000' } } } }, 'mcp.json'), + /env is invalid/u, + ); + assert.throws( + () => validateMcp({ ...base, mcpServers: { bad: { type: 'stdio', command: 'node', env: { 'FOO\u0000': 'bar' } } } }, 'mcp.json'), + /env is invalid/u, + ); + assert.throws( + () => validateMcp({ ...base, mcpServers: { bad: { type: 'streamable-http', url: 'https://example.com/mcp', headers: { 'X-Trace': 'a\u0000b' } } } }, 'mcp.json'), + /headers must be strings/u, + ); + assert.throws( + () => validateMcp({ ...base, mcpServers: { bad: { type: 'streamable-http', url: 'https://example.com/mcp', headers: { 'X-\u0000': 'audit' } } } }, 'mcp.json'), + /headers must be strings/u, + ); + + assert.deepEqual( + validateMcp({ ...base, mcpServers: { ok: { type: 'stdio', command: 'node', args: ['./server.js'] } } }, 'mcp.json'), + ['ok'], + ); +}); + +test('validateMcp rejects server names longer than 64 characters', () => { + const base = { $schema: 'https://agent-plugins.org/schemas/1.0.0/mcp.schema.json' }; + + assert.throws( + () => validateMcp({ ...base, mcpServers: { ['x'.repeat(65)]: { type: 'stdio', command: 'node' } } }, 'mcp.json'), + /invalid MCP server name/u, + ); + assert.throws( + () => validateMcp({ ...base, mcpServers: { ['a'.repeat(64) + '--']: { type: 'stdio', command: 'node' } } }, 'mcp.json'), + /invalid MCP server name/u, + ); + + assert.deepEqual( + validateMcp({ ...base, mcpServers: { ['x'.repeat(64)]: { type: 'stdio', command: 'node' } } }, 'mcp.json'), + ['x'.repeat(64)], + ); +}); + +test('validateMcp rejects CRLF and control bytes in headers (HTTP header injection defense)', () => { + const base = { $schema: 'https://agent-plugins.org/schemas/1.0.0/mcp.schema.json' }; + + assert.throws( + () => validateMcp({ ...base, mcpServers: { bad: { type: 'streamable-http', url: 'https://example.com/mcp', headers: { 'X-Trace': 'a\r\nAuthorization: Bearer evil' } } } }, 'mcp.json'), + /control bytes/u, + ); + assert.throws( + () => validateMcp({ ...base, mcpServers: { bad: { type: 'streamable-http', url: 'https://example.com/mcp', headers: { 'X-Trace\r\nAuthorization': 'foo' } } } }, 'mcp.json'), + /control bytes/u, + ); + assert.throws( + () => validateMcp({ ...base, mcpServers: { bad: { type: 'streamable-http', url: 'https://example.com/mcp', headers: { 'X-Trace': 'a\nfoo' } } } }, 'mcp.json'), + /control bytes/u, + ); + assert.throws( + () => validateMcp({ ...base, mcpServers: { bad: { type: 'streamable-http', url: 'https://example.com/mcp', headers: { 'X-Trace': 'a\rfoo' } } } }, 'mcp.json'), + /control bytes/u, + ); + assert.throws( + () => validateMcp({ ...base, mcpServers: { bad: { type: 'streamable-http', url: 'https://example.com/mcp', headers: { 'X-Trace': 'foo\u0000bar' } } } }, 'mcp.json'), + /control bytes/u, + ); + assert.throws( + () => validateMcp({ ...base, mcpServers: { bad: { type: 'streamable-http', url: 'https://example.com/mcp', headers: { 'X-Trace': 'foo\u007fbar' } } } }, 'mcp.json'), + /control bytes/u, + ); + + assert.deepEqual( + validateMcp({ ...base, mcpServers: { ok: { type: 'streamable-http', url: 'https://example.com/mcp', headers: { 'X-Trace': 'foo\tbar' } } } }, 'mcp.json'), + ['ok'], + ); + assert.deepEqual( + validateMcp({ ...base, mcpServers: { ok: { type: 'streamable-http', url: 'https://example.com/mcp', headers: { 'X-Trace': 'audit-id' } } } }, 'mcp.json'), + ['ok'], + ); +}); + +test('validateMcp rejects common custom credential headers (X-Api-Key, X-Auth-Token, etc.)', () => { + const base = { $schema: 'https://agent-plugins.org/schemas/1.0.0/mcp.schema.json' }; + + for (const key of ['X-Api-Key', 'X-Auth-Token', 'X-Access-Token', 'X-Token', 'X-Secret', 'X-Api-Token', 'Api-Key', 'Auth-Token', 'Access-Token']) { + assert.throws( + () => validateMcp({ ...base, mcpServers: { bad: { type: 'streamable-http', url: 'https://example.com/mcp', headers: { [key]: 'secret123' } } } }, 'mcp.json'), + /headers must be strings/u, + `expected ${key} to be rejected`, + ); + } + + assert.deepEqual( + validateMcp({ ...base, mcpServers: { ok: { type: 'streamable-http', url: 'https://example.com/mcp', headers: { 'X-Request-Id': 'audit-trace' } } } }, 'mcp.json'), + ['ok'], + ); + assert.deepEqual( + validateMcp({ ...base, mcpServers: { ok: { type: 'streamable-http', url: 'https://example.com/mcp', headers: { 'User-Agent': 'minimax-code-plugin/1.0' } } } }, 'mcp.json'), + ['ok'], + ); +}); + +test('validateMcp trims header keys before matching the credential blacklist', () => { + const base = { $schema: 'https://agent-plugins.org/schemas/1.0.0/mcp.schema.json' }; + + for (const key of ['Authorization ', ' Authorization', ' Authorization ', 'authorization\t', 'X-Api-Key ', ' X-Api-Key', 'PLUGIN_ROOT ']) { + assert.throws( + () => validateMcp({ ...base, mcpServers: { bad: { type: 'streamable-http', url: 'https://example.com/mcp', headers: { [key]: 'secret' } } } }, 'mcp.json'), + /headers must be strings/u, + `expected ${JSON.stringify(key)} to be rejected after trim`, + ); + } + + assert.deepEqual( + validateMcp({ ...base, mcpServers: { ok: { type: 'streamable-http', url: 'https://example.com/mcp', headers: { 'X-Trace ': 'audit' } } } }, 'mcp.json'), + ['ok'], + ); + assert.deepEqual( + validateMcp({ ...base, mcpServers: { ok: { type: 'streamable-http', url: 'https://example.com/mcp', headers: { '\tX-Trace': 'audit' } } } }, 'mcp.json'), + ['ok'], + ); +}); + +test('validateMcp enforces size limits to prevent DoS via huge cwd/args/env/headers', () => { + const base = { $schema: 'https://agent-plugins.org/schemas/1.0.0/mcp.schema.json' }; + + assert.throws( + () => validateMcp({ ...base, mcpServers: { bad: { type: 'stdio', command: 'node', cwd: './' + 'a/'.repeat(50000) } } }, 'mcp.json'), + /cwd must be a string of at most 1024 chars/u, + ); + + assert.throws( + () => validateMcp({ ...base, mcpServers: { bad: { type: 'stdio', command: 'node', args: new Array(2000).fill('./server.js') } } }, 'mcp.json'), + /args must be strings without NUL bytes/u, + ); + + const manyArgs = []; + for (let i = 0; i < 1024; i++) manyArgs.push(`--flag-${i}`); + manyArgs.push('a'.repeat(5000)); + assert.throws( + () => validateMcp({ ...base, mcpServers: { bad: { type: 'stdio', command: 'node', args: manyArgs } } }, 'mcp.json'), + /args must be strings without NUL bytes/u, + ); + + const manyEnv = {}; + for (let i = 0; i < 300; i++) manyEnv[`KEY_${i}`] = 'value'; + assert.throws( + () => validateMcp({ ...base, mcpServers: { bad: { type: 'stdio', command: 'node', env: manyEnv } } }, 'mcp.json'), + /env is invalid/u, + ); + + assert.throws( + () => validateMcp({ ...base, mcpServers: { bad: { type: 'streamable-http', url: 'https://example.com/mcp', headers: { 'X-Trace': 'a'.repeat(100000) } } } }, 'mcp.json'), + /headers must be strings/u, + ); + + const manyHeaders = {}; + for (let i = 0; i < 200; i++) manyHeaders[`X-Header-${i}`] = 'value'; + assert.throws( + () => validateMcp({ ...base, mcpServers: { bad: { type: 'streamable-http', url: 'https://example.com/mcp', headers: manyHeaders } } }, 'mcp.json'), + /headers must be strings/u, + ); + + assert.throws( + () => validateMcp({ ...base, mcpServers: { bad: { type: 'streamable-http', url: 'https://example.com/mcp', headers: { ['X-'.repeat(200)]: 'value' } } } }, 'mcp.json'), + /headers must be strings/u, + ); +}); + +test('validateSkillText accepts CRLF and rejects UTF-8 BOM (cross-platform line endings)', () => { + assert.deepEqual( + validateSkillText('---\r\nname: hello-skill\r\ndescription: Run when the user asks.\r\n---\r\n\r\n# Hello\n', 'hello-skill'), + { name: 'hello-skill', description: 'Run when the user asks.' }, + ); + assert.deepEqual( + validateSkillText('---\r\nname: hello-skill\ndescription: Run when the user asks.\r\n---\n\n# Hello\n', 'hello-skill'), + { name: 'hello-skill', description: 'Run when the user asks.' }, + ); + assert.deepEqual( + validateSkillText('---\nname: hello-skill\ndescription: Run when the user asks.\r\n---\r\n\r\n# Hello\n', 'hello-skill'), + { name: 'hello-skill', description: 'Run when the user asks.' }, + ); + + assert.throws( + () => validateSkillText('\uFEFF---\nname: hello-skill\ndescription: Run when the user asks.\n---\n\n# Hello\n', 'hello-skill'), + /UTF-8 BOM is not allowed/u, + ); + assert.throws( + () => validateSkillText('\uFEFF---\r\nname: hello-skill\r\ndescription: Run when the user asks.\r\n---\r\n\r\n# Hello\r\n', 'hello-skill'), + /UTF-8 BOM is not allowed/u, + ); +});