Skip to content

Fix k8s execute command argv handling #67

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Devamparikh wants to merge 1 commit into
base: main
Choose a base branch
from
+93 −10
Open

Fix k8s execute command argv handling #67

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 57 additions & 5 deletions pkg/k8s/k8s.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,12 @@ func (k *K8sTool) handleGetEvents(ctx context.Context, request mcp.CallToolReque
func (k *K8sTool) handleExecCommand(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) {
podName := mcp.ParseString(request, "pod_name", "")
namespace := mcp.ParseString(request, "namespace", "default")
container := mcp.ParseString(request, "container", "")
command := mcp.ParseString(request, "command", "")
commandArgs, err := parseStringSliceArgument(request, "args")
if err != nil {
return mcp.NewToolResultError(fmt.Sprintf("Invalid args: %v", err)), nil
}

if podName == "" || command == "" {
return mcp.NewToolResultError("pod_name and command parameters are required"), nil
Expand All @@ -332,16 +337,62 @@ func (k *K8sTool) handleExecCommand(ctx context.Context, request mcp.CallToolReq
return mcp.NewToolResultError(fmt.Sprintf("Invalid namespace: %v", err)), nil
}

// Validate command input for security
if err := security.ValidateCommandInput(command); err != nil {
return mcp.NewToolResultError(fmt.Sprintf("Invalid command: %v", err)), nil
commandParts := append(strings.Fields(command), commandArgs...)
if len(commandParts) == 0 {
return mcp.NewToolResultError("command parameter is required"), nil
}

for _, part := range commandParts {
if err := security.ValidateCommandInput(part); err != nil {
return mcp.NewToolResultError(fmt.Sprintf("Invalid command: %v", err)), nil
}
}

if container != "" {
if err := security.ValidateK8sResourceName(container); err != nil {
return mcp.NewToolResultError(fmt.Sprintf("Invalid container name: %v", err)), nil
}
}

args := []string{"exec", podName, "-n", namespace, "--", command}
args := []string{"exec", podName, "-n", namespace}
if container != "" {
args = append(args, "-c", container)
}
args = append(args, "--")
args = append(args, commandParts...)

return k.runKubectlCommand(ctx, request.Header, args...)
}

func parseStringSliceArgument(request mcp.CallToolRequest, name string) ([]string, error) {
arguments, ok := request.Params.Arguments.(map[string]any)
if !ok {
return nil, nil
}

raw, ok := arguments[name]
if !ok || raw == nil {
return nil, nil
}

switch value := raw.(type) {
case []string:
return value, nil
case []any:
values := make([]string, 0, len(value))
for _, item := range value {
text, ok := item.(string)
if !ok {
return nil, fmt.Errorf("%s must only contain strings", name)
}
values = append(values, text)
}
return values, nil
default:
return nil, fmt.Errorf("%s must be an array of strings", name)
}
}

// Get available API resources
func (k *K8sTool) handleGetAvailableAPIResources(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) {
return k.runKubectlCommand(ctx, request.Header, "api-resources")
Expand Down Expand Up @@ -765,7 +816,8 @@ func RegisterTools(s *server.MCPServer, llm llms.Model, kubeconfig string, readO
mcp.WithString("pod_name", mcp.Description("Name of the pod to execute in"), mcp.Required()),
mcp.WithString("namespace", mcp.Description("Namespace of the pod (default: default)")),
mcp.WithString("container", mcp.Description("Container name (for multi-container pods)")),
mcp.WithString("command", mcp.Description("Command to execute"), mcp.Required()),
mcp.WithString("command", mcp.Description("Command executable to run. For backward compatibility, simple whitespace-separated commands are split into argv tokens."), mcp.Required()),
mcp.WithArray("args", mcp.Description("Command arguments to pass after command. Prefer this for flags and arguments, for example command='uname', args=['-a']."), mcp.WithStringItems()),
), telemetry.AdaptToolHandler(telemetry.WithTracing("k8s_execute_command", k8sTool.handleExecCommand)))

s.AddTool(mcp.NewTool("k8s_rollout",
Expand Down
41 changes: 36 additions & 5 deletions pkg/k8s/k8s_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -669,14 +669,13 @@ spec:

func TestHandleExecCommand(t *testing.T) {
ctx := context.Background()
t.Run("exec command in pod", func(t *testing.T) {
t.Run("exec command in pod splits legacy command string", func(t *testing.T) {
mock := cmd.NewMockShellExecutor()
expectedOutput := `total 8
drwxr-xr-x 1 root root 4096 Jan 1 12:00 .
drwxr-xr-x 1 root root 4096 Jan 1 12:00 ..`

// The implementation passes the command as a single string after --
mock.AddCommandString("kubectl", []string{"exec", "mypod", "-n", "default", "--", "ls -la"}, expectedOutput, nil)
mock.AddCommandString("kubectl", []string{"exec", "mypod", "-n", "default", "--", "ls", "-la"}, expectedOutput, nil)
ctx := cmd.WithShellExecutor(ctx, mock)

k8sTool := newTestK8sTool()
Expand All @@ -701,7 +700,39 @@ drwxr-xr-x 1 root root 4096 Jan 1 12:00 ..`
callLog := mock.GetCallLog()
require.Len(t, callLog, 1)
assert.Equal(t, "kubectl", callLog[0].Command)
assert.Equal(t, []string{"exec", "mypod", "-n", "default", "--", "ls -la"}, callLog[0].Args)
assert.Equal(t, []string{"exec", "mypod", "-n", "default", "--", "ls", "-la"}, callLog[0].Args)
})

t.Run("exec command in pod with explicit args and container", func(t *testing.T) {
mock := cmd.NewMockShellExecutor()
expectedOutput := `Linux test-node 6.12.0`

mock.AddCommandString("kubectl", []string{"exec", "mypod", "-n", "default", "-c", "app", "--", "uname", "-a"}, expectedOutput, nil)
ctx := cmd.WithShellExecutor(ctx, mock)

k8sTool := newTestK8sTool()

req := mcp.CallToolRequest{}
req.Params.Arguments = map[string]interface{}{
"pod_name": "mypod",
"namespace": "default",
"container": "app",
"command": "uname",
"args": []interface{}{"-a"},
}

result, err := k8sTool.handleExecCommand(ctx, req)
assert.NoError(t, err)
assert.NotNil(t, result)
assert.False(t, result.IsError)

content := getResultText(result)
assert.Contains(t, content, "Linux test-node")

callLog := mock.GetCallLog()
require.Len(t, callLog, 1)
assert.Equal(t, "kubectl", callLog[0].Command)
assert.Equal(t, []string{"exec", "mypod", "-n", "default", "-c", "app", "--", "uname", "-a"}, callLog[0].Args)
})

t.Run("missing required parameters", func(t *testing.T) {
Expand Down Expand Up @@ -1390,7 +1421,7 @@ log line 2`
t.Run("exec command with bearer token", func(t *testing.T) {
mock := cmd.NewMockShellExecutor()
expectedOutput := `total 8`
mock.AddCommandString("kubectl", []string{"exec", "mypod", "-n", "default", "--", "ls -la", "--token", "exec-token"}, expectedOutput, nil)
mock.AddCommandString("kubectl", []string{"exec", "mypod", "-n", "default", "--", "ls", "-la", "--token", "exec-token"}, expectedOutput, nil)
ctx := cmd.WithShellExecutor(ctx, mock)

k8sTool := newTestK8sToolWithPassthrough(true)
Expand Down