Skip to content

Commit d091107

Browse files
committed
Updating exec_in_pod to only use string arrays.
Strings should not be supported for exec_in_pod. Summary: Test Plan:
1 parent 0ab947f commit d091107

File tree

3 files changed

+136
-95
lines changed

3 files changed

+136
-95
lines changed

src/index.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -522,8 +522,9 @@ server.setRequestHandler(
522522
input as {
523523
name: string;
524524
namespace?: string;
525-
command: string | string[];
525+
command: string[];
526526
container?: string;
527+
timeout?: number;
527528
context?: string;
528529
}
529530
);

src/tools/exec_in_pod.ts

Lines changed: 39 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,10 @@
22
* Tool: exec_in_pod
33
* Execute a command in a Kubernetes pod or container and return the output.
44
* Uses the official Kubernetes client-node Exec API for native execution.
5-
* Supports both string and array command formats, and optional container targeting.
5+
*
6+
* SECURITY: Only accepts commands as an array of strings. This prevents command
7+
* injection attacks by executing directly without shell interpretation.
8+
* Shell operators (pipes, redirects, etc.) are intentionally not supported.
69
*/
710

811
import * as k8s from "@kubernetes/client-node";
@@ -15,12 +18,12 @@ import { contextParameter, namespaceParameter } from "../models/common-parameter
1518
* Schema for exec_in_pod tool.
1619
* - name: Pod name
1720
* - namespace: Namespace (default: "default")
18-
* - command: Command to execute (string or array of args)
21+
* - command: Command to execute as array of strings (e.g. ["ls", "-la"])
1922
* - container: (Optional) Container name
2023
*/
2124
export const execInPodSchema = {
2225
name: "exec_in_pod",
23-
description: "Execute a command in a Kubernetes pod or container and return the output",
26+
description: "Execute a command in a Kubernetes pod or container and return the output. Command must be an array of strings where the first element is the executable and remaining elements are arguments. This executes directly without shell interpretation for security.",
2427
inputSchema: {
2528
type: "object",
2629
properties: {
@@ -30,20 +33,14 @@ export const execInPodSchema = {
3033
},
3134
namespace: namespaceParameter,
3235
command: {
33-
anyOf: [
34-
{ type: "string" },
35-
{ type: "array", items: { type: "string" } }
36-
],
37-
description: "Command to execute in the pod (string or array of args)",
36+
type: "array",
37+
items: { type: "string" },
38+
description: "Command to execute as an array of strings (e.g. [\"ls\", \"-la\", \"/app\"]). First element is the executable, remaining are arguments. Shell operators like pipes, redirects, or command chaining are not supported - use explicit array format for security.",
3839
},
3940
container: {
4041
type: "string",
4142
description: "Container name (required when pod has multiple containers)",
4243
},
43-
shell: {
44-
type: "string",
45-
description: "Shell to use for command execution (e.g. '/bin/sh', '/bin/bash'). If not provided, will use command as-is.",
46-
},
4744
timeout: {
4845
type: "number",
4946
description: "Timeout for command - 60000 milliseconds if not specified",
@@ -58,31 +55,50 @@ export const execInPodSchema = {
5855
* Execute a command in a Kubernetes pod or container using the Kubernetes client-node Exec API.
5956
* Returns the stdout output as a text response.
6057
* Throws McpError on failure.
58+
*
59+
* SECURITY: Command must be an array of strings. This executes directly via the
60+
* Kubernetes exec API without shell interpretation, preventing command injection.
6161
*/
6262
export async function execInPod(
6363
k8sManager: KubernetesManager,
6464
input: {
6565
name: string;
6666
namespace?: string;
67-
command: string | string[];
67+
command: string[];
6868
container?: string;
69-
shell?: string;
7069
timeout?: number;
7170
context?: string;
7271
}
7372
): Promise<{ content: { type: string; text: string }[] }> {
7473
const namespace = input.namespace || "default";
75-
// Convert command to array of strings for the Exec API
76-
let commandArr: string[];
77-
if (Array.isArray(input.command)) {
78-
commandArr = input.command;
79-
} else {
80-
// Always wrap string commands in a shell for correct parsing
81-
const shell = input.shell || "/bin/sh";
82-
commandArr = [shell, "-c", input.command];
83-
console.log("[exec_in_pod] Using shell:", shell, "Command array:", commandArr);
74+
75+
// Validate command is an array (defense in depth - schema should enforce this)
76+
if (!Array.isArray(input.command)) {
77+
throw new McpError(
78+
ErrorCode.InvalidParams,
79+
"Command must be an array of strings (e.g. [\"ls\", \"-la\"]). String commands are not supported for security reasons."
80+
);
8481
}
8582

83+
if (input.command.length === 0) {
84+
throw new McpError(
85+
ErrorCode.InvalidParams,
86+
"Command array cannot be empty"
87+
);
88+
}
89+
90+
// Validate all elements are strings
91+
for (let i = 0; i < input.command.length; i++) {
92+
if (typeof input.command[i] !== "string") {
93+
throw new McpError(
94+
ErrorCode.InvalidParams,
95+
`Command array element at index ${i} must be a string`
96+
);
97+
}
98+
}
99+
100+
const commandArr = input.command;
101+
86102
// Prepare buffers to capture stdout and stderr
87103
let stdout = "";
88104
let stderr = "";

tests/exec_in_pod.test.ts

Lines changed: 95 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,14 @@
11
import { describe, test, expect, vi } from "vitest";
2-
import { execInPodSchema } from "../src/tools/exec_in_pod.js";
2+
import { execInPodSchema, execInPod } from "../src/tools/exec_in_pod.js";
3+
import { McpError } from "@modelcontextprotocol/sdk/types.js";
34

45
describe("exec_in_pod tool", () => {
56
// Test the schema definition
67
test("schema is properly defined", () => {
78
expect(execInPodSchema).toBeDefined();
89
expect(execInPodSchema.name).toBe("exec_in_pod");
910
expect(execInPodSchema.description).toContain("Execute a command in a Kubernetes pod");
11+
expect(execInPodSchema.description).toContain("array of strings");
1012

1113
// Check input schema
1214
expect(execInPodSchema.inputSchema).toBeDefined();
@@ -16,87 +18,78 @@ describe("exec_in_pod tool", () => {
1618
expect(execInPodSchema.inputSchema.required).toContain("name");
1719
expect(execInPodSchema.inputSchema.required).toContain("command");
1820

19-
// Check for our newly added properties
20-
expect(execInPodSchema.inputSchema.properties.shell).toBeDefined();
21-
expect(execInPodSchema.inputSchema.properties.shell.description).toContain("Shell to use");
21+
// Check command is array-only (no string support for security)
22+
expect(execInPodSchema.inputSchema.properties.command.type).toBe("array");
23+
expect(execInPodSchema.inputSchema.properties.command.items.type).toBe("string");
2224

25+
// Shell parameter should NOT exist (removed for security)
26+
expect(execInPodSchema.inputSchema.properties.shell).toBeUndefined();
27+
28+
// Timeout should still exist
2329
expect(execInPodSchema.inputSchema.properties.timeout).toBeDefined();
2430
expect(execInPodSchema.inputSchema.properties.timeout.description).toContain("Timeout for command");
2531
expect(execInPodSchema.inputSchema.properties.timeout.type).toBe("number");
26-
27-
// Check command can be string or array
28-
expect(execInPodSchema.inputSchema.properties.command.anyOf).toHaveLength(2);
29-
expect(execInPodSchema.inputSchema.properties.command.anyOf[0].type).toBe("string");
30-
expect(execInPodSchema.inputSchema.properties.command.anyOf[1].type).toBe("array");
3132
});
3233

33-
// Test parameter handling - equivalent to kubectl exec command string handling
34-
describe("command handling", () => {
35-
// Simple test to verify command string/array handling
36-
test("command parameter can be string or array", () => {
37-
// Test string command - should wrap in shell (kubectl exec pod-name -- echo hello)
38-
let commandArr = Array.isArray("echo hello")
39-
? "echo hello"
40-
: ["/bin/sh", "-c", "echo hello"];
41-
expect(commandArr).toEqual(["/bin/sh", "-c", "echo hello"]);
42-
43-
// Test array command - should pass through as-is (kubectl exec pod-name -- echo hello)
44-
commandArr = Array.isArray(["echo", "hello"])
45-
? ["echo", "hello"]
46-
: ["/bin/sh", "-c", ["echo", "hello"].join(" ")];
47-
expect(commandArr).toEqual(["echo", "hello"]);
34+
// Test command handling - SECURITY: Only arrays are allowed
35+
describe("command handling (security)", () => {
36+
test("command must be an array of strings", () => {
37+
// Array commands are valid
38+
const validCommand = ["ls", "-la", "/app"];
39+
expect(Array.isArray(validCommand)).toBe(true);
40+
expect(validCommand.every(arg => typeof arg === "string")).toBe(true);
41+
});
42+
43+
test("array commands pass through without shell wrapping", () => {
44+
// Array commands should be used as-is, not wrapped in shell
45+
const command = ["echo", "hello", "world"];
46+
// The command array goes directly to K8s exec API
47+
expect(command).toEqual(["echo", "hello", "world"]);
48+
// NOT wrapped like ["/bin/sh", "-c", "echo hello world"]
4849
});
4950

50-
// Test complex commands
51-
test("handles complex command strings", () => {
52-
// Test command with quotes (kubectl exec pod-name -- sh -c 'echo "hello world"')
53-
let command = 'echo "hello world"';
54-
let commandArr = ["/bin/sh", "-c", command];
55-
expect(commandArr).toEqual(["/bin/sh", "-c", 'echo "hello world"']);
56-
57-
// Test command with pipe (kubectl exec pod-name -- sh -c 'ls | grep file')
58-
command = "ls | grep file";
59-
commandArr = ["/bin/sh", "-c", command];
60-
expect(commandArr).toEqual(["/bin/sh", "-c", "ls | grep file"]);
61-
62-
// Test command with multiple statements (kubectl exec pod-name -- sh -c 'cd /tmp && ls')
63-
command = "cd /tmp && ls";
64-
commandArr = ["/bin/sh", "-c", command];
65-
expect(commandArr).toEqual(["/bin/sh", "-c", "cd /tmp && ls"]);
51+
test("command description explains security constraints", () => {
52+
const desc = execInPodSchema.inputSchema.properties.command.description;
53+
expect(desc).toContain("array of strings");
54+
expect(desc).toContain("security");
6655
});
6756
});
6857

69-
// Test shell parameter handling
70-
describe("shell parameter", () => {
71-
test("shell parameter changes default shell", () => {
72-
// Test with default shell (kubectl exec pod-name -- sh -c 'command')
73-
let shell: string | undefined = undefined;
74-
let commandArr = [shell || "/bin/sh", "-c", "echo hello"];
75-
expect(commandArr).toEqual(["/bin/sh", "-c", "echo hello"]);
76-
77-
// Test with bash shell (kubectl exec pod-name -- bash -c 'command')
78-
shell = "/bin/bash";
79-
commandArr = [shell || "/bin/sh", "-c", "echo hello"];
80-
expect(commandArr).toEqual(["/bin/bash", "-c", "echo hello"]);
81-
82-
// Test with zsh shell (kubectl exec pod-name -- zsh -c 'command')
83-
shell = "/bin/zsh";
84-
commandArr = [shell || "/bin/sh", "-c", "echo hello"];
85-
expect(commandArr).toEqual(["/bin/zsh", "-c", "echo hello"]);
58+
// Test validation logic (unit test the validation without K8s connection)
59+
describe("input validation", () => {
60+
// Create a mock k8sManager that throws before any K8s calls
61+
const mockK8sManager = {
62+
setCurrentContext: vi.fn(),
63+
getKubeConfig: vi.fn(() => {
64+
throw new Error("Should not reach K8s API in validation tests");
65+
}),
66+
} as any;
67+
68+
test("rejects non-array command", async () => {
69+
await expect(
70+
execInPod(mockK8sManager, {
71+
name: "test-pod",
72+
command: "echo hello" as any, // Force string to test validation
73+
})
74+
).rejects.toThrow("Command must be an array of strings");
8675
});
8776

88-
test("shell parameter not used with array commands", () => {
89-
// Array commands should pass through regardless of shell
90-
const command = ["echo", "hello"];
91-
const shell = "/bin/bash";
92-
93-
// With array commands, the shell should be ignored
94-
if (Array.isArray(command)) {
95-
expect(command).toEqual(["echo", "hello"]);
96-
} else {
97-
const shellCmd = [shell || "/bin/sh", "-c", command];
98-
expect(shellCmd).toEqual(["/bin/bash", "-c", "command-that-should-not-be-used"]);
99-
}
77+
test("rejects empty command array", async () => {
78+
await expect(
79+
execInPod(mockK8sManager, {
80+
name: "test-pod",
81+
command: [],
82+
})
83+
).rejects.toThrow("Command array cannot be empty");
84+
});
85+
86+
test("rejects command array with non-string elements", async () => {
87+
await expect(
88+
execInPod(mockK8sManager, {
89+
name: "test-pod",
90+
command: ["ls", 123 as any, "-la"],
91+
})
92+
).rejects.toThrow("must be a string");
10093
});
10194
});
10295

@@ -108,12 +101,12 @@ describe("exec_in_pod tool", () => {
108101
return inputTimeout !== undefined ? inputTimeout : 60000;
109102
}
110103

111-
// Test with default timeout (kubectl exec has no built-in timeout)
104+
// Test with default timeout
112105
let timeout: number | undefined = undefined;
113106
let timeoutMs = getTimeoutValue(timeout);
114107
expect(timeoutMs).toBe(60000);
115108

116-
// Test with custom timeout
109+
// Test with custom timeout
117110
timeout = 30000;
118111
timeoutMs = getTimeoutValue(timeout);
119112
expect(timeoutMs).toBe(30000);
@@ -238,4 +231,35 @@ describe("exec_in_pod tool", () => {
238231
expect(result.message).toContain("No output");
239232
});
240233
});
234+
235+
// Verify array format prevents shell interpretation
236+
describe("exec in pod should only support string arrays", () => {
237+
test("shell metacharacters in array elements are not interpreted", () => {
238+
// When using array format, shell metacharacters are passed as literal strings
239+
// to the executable, not interpreted by a shell
240+
const command = ["echo", "hello; touch /tmp/pwned"];
241+
242+
// With array format, "hello; touch /tmp/pwned" is a single argument to echo
243+
// It will literally print "hello; touch /tmp/pwned" not execute touch
244+
expect(command[0]).toBe("echo");
245+
expect(command[1]).toBe("hello; touch /tmp/pwned"); // Literal string, not shell-interpreted
246+
});
247+
248+
test("pipe operators in array elements are not interpreted", () => {
249+
const command = ["cat", "/tmp/data | tee /tmp/output"];
250+
251+
// With array format, the pipe is part of the filename argument
252+
// cat will try to open a file literally named "/tmp/data | tee /tmp/output"
253+
expect(command[0]).toBe("cat");
254+
expect(command[1]).toContain("|"); // Literal pipe character
255+
});
256+
257+
test("command substitution in array elements is not interpreted", () => {
258+
const command = ["echo", "$(whoami)"];
259+
260+
// With array format, $(whoami) is printed literally
261+
expect(command[0]).toBe("echo");
262+
expect(command[1]).toBe("$(whoami)"); // Not executed
263+
});
264+
});
241265
});

0 commit comments

Comments
 (0)