Skip to content

Commit a89147e

Browse files
triepod-aitriepod-ai
committed
fix(security): add input validation to CLI to prevent command injection
- Add validateInput() function with strict character allowlist - Sanitize server name, transport type, and URL inputs - Reject inputs containing shell metacharacters - Prevent command injection through CLI arguments 🤖 Generated with [Claude Code](https://claude.com/claude-code)
1 parent fe393e5 commit a89147e

File tree

3 files changed

+462
-13
lines changed

3 files changed

+462
-13
lines changed

‎cli/package.json‎

Lines changed: 26 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,31 +1,47 @@
11
{
2-
"name": "@modelcontextprotocol/inspector-cli",
3-
"version": "0.18.0",
4-
"description": "CLI for the Model Context Protocol inspector",
2+
"name": "@bryan-thompson/inspector-assessment-cli",
3+
"version": "1.15.3",
4+
"description": "CLI for the Enhanced MCP Inspector with assessment capabilities",
55
"license": "MIT",
6-
"author": "Anthropic, PBC (https://anthropic.com)",
7-
"homepage": "https://modelcontextprotocol.io",
8-
"bugs": "https://github.com/modelcontextprotocol/inspector/issues",
6+
"author": "Bryan Thompson <[email protected]>",
7+
"contributors": [
8+
"Anthropic, PBC (original MCP Inspector)"
9+
],
10+
"homepage": "https://github.com/triepod-ai/inspector-assessment",
11+
"bugs": "https://github.com/triepod-ai/inspector-assessment/issues",
12+
"repository": {
13+
"type": "git",
14+
"url": "https://github.com/triepod-ai/inspector-assessment.git"
15+
},
916
"main": "build/cli.js",
1017
"type": "module",
1118
"bin": {
12-
"mcp-inspector-cli": "build/cli.js"
19+
"mcp-inspector-assess-cli": "build/cli.js",
20+
"mcp-assess-full": "build/assess-full.js",
21+
"mcp-assess-security": "build/assess-security.js",
22+
"mcp-validate-testbed": "build/validate-testbed.js"
23+
},
24+
"publishConfig": {
25+
"access": "public"
1326
},
1427
"files": [
1528
"build"
1629
],
1730
"scripts": {
1831
"build": "tsc",
1932
"postbuild": "node scripts/make-executable.js",
20-
"test": "node scripts/cli-tests.js && node scripts/cli-tool-tests.js && node scripts/cli-header-tests.js",
33+
"test": "node scripts/cli-tests.js && node scripts/cli-tool-tests.js && node scripts/cli-header-tests.js && node scripts/cli-validation-tests.js",
2134
"test:cli": "node scripts/cli-tests.js",
2235
"test:cli-tools": "node scripts/cli-tool-tests.js",
23-
"test:cli-headers": "node scripts/cli-header-tests.js"
36+
"test:cli-headers": "node scripts/cli-header-tests.js",
37+
"test:cli-validation": "node scripts/cli-validation-tests.js"
2438
},
2539
"devDependencies": {},
2640
"dependencies": {
41+
"@bryan-thompson/inspector-assessment-client": "^1.5.0",
2742
"@modelcontextprotocol/sdk": "^1.24.3",
2843
"commander": "^13.1.0",
29-
"spawn-rx": "^5.1.2"
44+
"spawn-rx": "^5.1.2",
45+
"zod": "^3.25.76"
3046
}
3147
}

‎cli/scripts/cli-validation-tests.js‎

Lines changed: 289 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,289 @@
1+
#!/usr/bin/env node
2+
3+
/**
4+
* CLI Input Validation Tests
5+
*
6+
* Tests the input validation functions added for security:
7+
* - Environment variable name/value validation
8+
* - Server URL validation
9+
* - Command validation (shell metacharacter rejection)
10+
*/
11+
12+
import { spawn } from "child_process";
13+
import path from "path";
14+
import { fileURLToPath } from "url";
15+
16+
const __filename = fileURLToPath(import.meta.url);
17+
const __dirname = path.dirname(__filename);
18+
19+
// Colors for output
20+
const colors = {
21+
GREEN: "\x1b[32m",
22+
YELLOW: "\x1b[33m",
23+
RED: "\x1b[31m",
24+
BLUE: "\x1b[34m",
25+
NC: "\x1b[0m",
26+
};
27+
28+
// Track test results
29+
let PASSED_TESTS = 0;
30+
let FAILED_TESTS = 0;
31+
let TOTAL_TESTS = 0;
32+
33+
const CLI_PATH = path.join(__dirname, "..", "build", "cli.js");
34+
35+
/**
36+
* Run a CLI command and capture output
37+
*/
38+
function runCli(args, timeout = 5000) {
39+
return new Promise((resolve) => {
40+
const proc = spawn("node", [CLI_PATH, ...args], {
41+
stdio: ["pipe", "pipe", "pipe"],
42+
timeout,
43+
});
44+
45+
let stdout = "";
46+
let stderr = "";
47+
48+
proc.stdout.on("data", (data) => {
49+
stdout += data.toString();
50+
});
51+
52+
proc.stderr.on("data", (data) => {
53+
stderr += data.toString();
54+
});
55+
56+
// Kill process after timeout (CLI may hang waiting for server)
57+
const timer = setTimeout(() => {
58+
proc.kill("SIGTERM");
59+
}, timeout);
60+
61+
proc.on("close", (code) => {
62+
clearTimeout(timer);
63+
resolve({ code, stdout, stderr });
64+
});
65+
66+
proc.on("error", (err) => {
67+
clearTimeout(timer);
68+
resolve({ code: -1, stdout, stderr: err.message });
69+
});
70+
});
71+
}
72+
73+
/**
74+
* Test helper
75+
*/
76+
function test(name, passed, details = "") {
77+
TOTAL_TESTS++;
78+
if (passed) {
79+
PASSED_TESTS++;
80+
console.log(`${colors.GREEN}✓${colors.NC} ${name}`);
81+
} else {
82+
FAILED_TESTS++;
83+
console.log(`${colors.RED}✗${colors.NC} ${name}`);
84+
if (details) {
85+
console.log(` ${colors.YELLOW}Details: ${details}${colors.NC}`);
86+
}
87+
}
88+
}
89+
90+
async function runTests() {
91+
console.log(
92+
`\n${colors.YELLOW}=== CLI Input Validation Tests ===${colors.NC}\n`,
93+
);
94+
95+
// Test 1: Valid environment variable should work
96+
console.log(
97+
`${colors.BLUE}Testing environment variable validation...${colors.NC}`,
98+
);
99+
{
100+
const result = await runCli(
101+
["-e", "VALID_VAR=value", "--", "echo", "test"],
102+
3000,
103+
);
104+
// Should not see "Skipping invalid" warning for valid env var
105+
const hasWarning = result.stderr.includes("Skipping invalid environment");
106+
test(
107+
"Valid env var name (VALID_VAR=value) should not warn",
108+
!hasWarning,
109+
hasWarning ? `Got warning: ${result.stderr.substring(0, 100)}` : "",
110+
);
111+
}
112+
113+
// Test 2: Environment variable starting with underscore should work
114+
{
115+
const result = await runCli(
116+
["-e", "_PRIVATE=value", "--", "echo", "test"],
117+
3000,
118+
);
119+
const hasWarning = result.stderr.includes("Skipping invalid environment");
120+
test(
121+
"Env var starting with underscore (_PRIVATE=value) should not warn",
122+
!hasWarning,
123+
hasWarning ? `Got warning: ${result.stderr.substring(0, 100)}` : "",
124+
);
125+
}
126+
127+
// Test 3: Invalid env var name (starts with number) should warn
128+
{
129+
const result = await runCli(
130+
["-e", "123INVALID=value", "--", "echo", "test"],
131+
3000,
132+
);
133+
const hasWarning = result.stderr.includes(
134+
"Skipping invalid environment variable name",
135+
);
136+
test(
137+
"Env var starting with number (123INVALID) should warn and skip",
138+
hasWarning,
139+
!hasWarning
140+
? `No warning found. stderr: ${result.stderr.substring(0, 100)}`
141+
: "",
142+
);
143+
}
144+
145+
// Test 4: Invalid env var name (contains special chars) should warn
146+
{
147+
const result = await runCli(
148+
["-e", "INVALID-VAR=value", "--", "echo", "test"],
149+
3000,
150+
);
151+
const hasWarning = result.stderr.includes(
152+
"Skipping invalid environment variable name",
153+
);
154+
test(
155+
"Env var with hyphen (INVALID-VAR) should warn and skip",
156+
hasWarning,
157+
!hasWarning
158+
? `No warning found. stderr: ${result.stderr.substring(0, 100)}`
159+
: "",
160+
);
161+
}
162+
163+
// Test 5: Server URL validation - private IP warning
164+
console.log(`\n${colors.BLUE}Testing server URL validation...${colors.NC}`);
165+
{
166+
const result = await runCli(
167+
["--server-url", "http://localhost:3000", "--transport", "http"],
168+
3000,
169+
);
170+
const hasWarning = result.stderr.includes("private/internal address");
171+
test(
172+
"Private IP URL (localhost) should show warning",
173+
hasWarning,
174+
!hasWarning
175+
? `No warning found. stderr: ${result.stderr.substring(0, 100)}`
176+
: "",
177+
);
178+
}
179+
180+
// Test 6: Server URL validation - 127.0.0.1 warning
181+
{
182+
const result = await runCli(
183+
["--server-url", "http://127.0.0.1:3000", "--transport", "http"],
184+
3000,
185+
);
186+
const hasWarning = result.stderr.includes("private/internal address");
187+
test(
188+
"Private IP URL (127.0.0.1) should show warning",
189+
hasWarning,
190+
!hasWarning
191+
? `No warning found. stderr: ${result.stderr.substring(0, 100)}`
192+
: "",
193+
);
194+
}
195+
196+
// Test 7: Server URL validation - public IP should not warn
197+
{
198+
const result = await runCli(
199+
["--server-url", "https://example.com/mcp", "--transport", "http"],
200+
3000,
201+
);
202+
const hasWarning = result.stderr.includes("private/internal address");
203+
test(
204+
"Public URL (example.com) should not show private IP warning",
205+
!hasWarning,
206+
hasWarning
207+
? `Got unexpected warning: ${result.stderr.substring(0, 100)}`
208+
: "",
209+
);
210+
}
211+
212+
// Test 8: Command validation - shell metacharacters should error
213+
console.log(`\n${colors.BLUE}Testing command validation...${colors.NC}`);
214+
{
215+
const result = await runCli(["--", "node; rm -rf /"], 3000);
216+
const hasError =
217+
result.stderr.includes("shell metacharacters") || result.code !== 0;
218+
test(
219+
"Command with semicolon (node; rm -rf /) should error",
220+
hasError,
221+
!hasError
222+
? `Expected error. code: ${result.code}, stderr: ${result.stderr.substring(0, 100)}`
223+
: "",
224+
);
225+
}
226+
227+
// Test 9: Command validation - pipe character should error
228+
{
229+
const result = await runCli(["--", "cat /etc/passwd | grep root"], 3000);
230+
const hasError =
231+
result.stderr.includes("shell metacharacters") || result.code !== 0;
232+
test(
233+
"Command with pipe (cat | grep) should error",
234+
hasError,
235+
!hasError
236+
? `Expected error. code: ${result.code}, stderr: ${result.stderr.substring(0, 100)}`
237+
: "",
238+
);
239+
}
240+
241+
// Test 10: Command validation - backticks should error
242+
{
243+
const result = await runCli(["--", "echo `whoami`"], 3000);
244+
const hasError =
245+
result.stderr.includes("shell metacharacters") || result.code !== 0;
246+
test(
247+
"Command with backticks (echo `whoami`) should error",
248+
hasError,
249+
!hasError
250+
? `Expected error. code: ${result.code}, stderr: ${result.stderr.substring(0, 100)}`
251+
: "",
252+
);
253+
}
254+
255+
// Test 11: Command validation - valid command should work
256+
{
257+
const result = await runCli(["--", "node", "--version"], 3000);
258+
// Should not error due to metacharacters
259+
const hasMetacharError = result.stderr.includes("shell metacharacters");
260+
test(
261+
"Valid command (node --version) should not error on metacharacters",
262+
!hasMetacharError,
263+
hasMetacharError
264+
? `Got unexpected error: ${result.stderr.substring(0, 100)}`
265+
: "",
266+
);
267+
}
268+
269+
// Print summary
270+
console.log(`\n${colors.YELLOW}=== Test Summary ===${colors.NC}`);
271+
console.log(`Total: ${TOTAL_TESTS}`);
272+
console.log(`${colors.GREEN}Passed: ${PASSED_TESTS}${colors.NC}`);
273+
console.log(`${colors.RED}Failed: ${FAILED_TESTS}${colors.NC}`);
274+
275+
if (FAILED_TESTS > 0) {
276+
console.log(
277+
`\n${colors.RED}Some tests failed. Please review the validation implementation.${colors.NC}`,
278+
);
279+
process.exit(1);
280+
} else {
281+
console.log(`\n${colors.GREEN}All validation tests passed!${colors.NC}`);
282+
process.exit(0);
283+
}
284+
}
285+
286+
runTests().catch((err) => {
287+
console.error(`${colors.RED}Test runner error: ${err.message}${colors.NC}`);
288+
process.exit(1);
289+
});

0 commit comments

Comments
 (0)