Skip to content

Commit f3e42ee

Browse files
DParikh_flexeraDParikh_flexera
authored andcommitted
Fix k8s exec argv handling
Signed-off-by: DParikh_flexera <Dparikh@flexera.com>
1 parent 33fa633 commit f3e42ee

2 files changed

Lines changed: 93 additions & 10 deletions

File tree

pkg/k8s/k8s.go

Lines changed: 57 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -316,7 +316,12 @@ func (k *K8sTool) handleGetEvents(ctx context.Context, request mcp.CallToolReque
316316
func (k *K8sTool) handleExecCommand(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) {
317317
podName := mcp.ParseString(request, "pod_name", "")
318318
namespace := mcp.ParseString(request, "namespace", "default")
319+
container := mcp.ParseString(request, "container", "")
319320
command := mcp.ParseString(request, "command", "")
321+
commandArgs, err := parseStringSliceArgument(request, "args")
322+
if err != nil {
323+
return mcp.NewToolResultError(fmt.Sprintf("Invalid args: %v", err)), nil
324+
}
320325

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

335-
// Validate command input for security
336-
if err := security.ValidateCommandInput(command); err != nil {
337-
return mcp.NewToolResultError(fmt.Sprintf("Invalid command: %v", err)), nil
340+
commandParts := append(strings.Fields(command), commandArgs...)
341+
if len(commandParts) == 0 {
342+
return mcp.NewToolResultError("command parameter is required"), nil
343+
}
344+
345+
for _, part := range commandParts {
346+
if err := security.ValidateCommandInput(part); err != nil {
347+
return mcp.NewToolResultError(fmt.Sprintf("Invalid command: %v", err)), nil
348+
}
349+
}
350+
351+
if container != "" {
352+
if err := security.ValidateK8sResourceName(container); err != nil {
353+
return mcp.NewToolResultError(fmt.Sprintf("Invalid container name: %v", err)), nil
354+
}
338355
}
339356

340-
args := []string{"exec", podName, "-n", namespace, "--", command}
357+
args := []string{"exec", podName, "-n", namespace}
358+
if container != "" {
359+
args = append(args, "-c", container)
360+
}
361+
args = append(args, "--")
362+
args = append(args, commandParts...)
341363

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

367+
func parseStringSliceArgument(request mcp.CallToolRequest, name string) ([]string, error) {
368+
arguments, ok := request.Params.Arguments.(map[string]any)
369+
if !ok {
370+
return nil, nil
371+
}
372+
373+
raw, ok := arguments[name]
374+
if !ok || raw == nil {
375+
return nil, nil
376+
}
377+
378+
switch value := raw.(type) {
379+
case []string:
380+
return value, nil
381+
case []any:
382+
values := make([]string, 0, len(value))
383+
for _, item := range value {
384+
text, ok := item.(string)
385+
if !ok {
386+
return nil, fmt.Errorf("%s must only contain strings", name)
387+
}
388+
values = append(values, text)
389+
}
390+
return values, nil
391+
default:
392+
return nil, fmt.Errorf("%s must be an array of strings", name)
393+
}
394+
}
395+
345396
// Get available API resources
346397
func (k *K8sTool) handleGetAvailableAPIResources(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) {
347398
return k.runKubectlCommand(ctx, request.Header, "api-resources")
@@ -765,7 +816,8 @@ func RegisterTools(s *server.MCPServer, llm llms.Model, kubeconfig string, readO
765816
mcp.WithString("pod_name", mcp.Description("Name of the pod to execute in"), mcp.Required()),
766817
mcp.WithString("namespace", mcp.Description("Namespace of the pod (default: default)")),
767818
mcp.WithString("container", mcp.Description("Container name (for multi-container pods)")),
768-
mcp.WithString("command", mcp.Description("Command to execute"), mcp.Required()),
819+
mcp.WithString("command", mcp.Description("Command executable to run. For backward compatibility, simple whitespace-separated commands are split into argv tokens."), mcp.Required()),
820+
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()),
769821
), telemetry.AdaptToolHandler(telemetry.WithTracing("k8s_execute_command", k8sTool.handleExecCommand)))
770822

771823
s.AddTool(mcp.NewTool("k8s_rollout",

pkg/k8s/k8s_test.go

Lines changed: 36 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -669,14 +669,13 @@ spec:
669669

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

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

682681
k8sTool := newTestK8sTool()
@@ -701,7 +700,39 @@ drwxr-xr-x 1 root root 4096 Jan 1 12:00 ..`
701700
callLog := mock.GetCallLog()
702701
require.Len(t, callLog, 1)
703702
assert.Equal(t, "kubectl", callLog[0].Command)
704-
assert.Equal(t, []string{"exec", "mypod", "-n", "default", "--", "ls -la"}, callLog[0].Args)
703+
assert.Equal(t, []string{"exec", "mypod", "-n", "default", "--", "ls", "-la"}, callLog[0].Args)
704+
})
705+
706+
t.Run("exec command in pod with explicit args and container", func(t *testing.T) {
707+
mock := cmd.NewMockShellExecutor()
708+
expectedOutput := `Linux test-node 6.12.0`
709+
710+
mock.AddCommandString("kubectl", []string{"exec", "mypod", "-n", "default", "-c", "app", "--", "uname", "-a"}, expectedOutput, nil)
711+
ctx := cmd.WithShellExecutor(ctx, mock)
712+
713+
k8sTool := newTestK8sTool()
714+
715+
req := mcp.CallToolRequest{}
716+
req.Params.Arguments = map[string]interface{}{
717+
"pod_name": "mypod",
718+
"namespace": "default",
719+
"container": "app",
720+
"command": "uname",
721+
"args": []interface{}{"-a"},
722+
}
723+
724+
result, err := k8sTool.handleExecCommand(ctx, req)
725+
assert.NoError(t, err)
726+
assert.NotNil(t, result)
727+
assert.False(t, result.IsError)
728+
729+
content := getResultText(result)
730+
assert.Contains(t, content, "Linux test-node")
731+
732+
callLog := mock.GetCallLog()
733+
require.Len(t, callLog, 1)
734+
assert.Equal(t, "kubectl", callLog[0].Command)
735+
assert.Equal(t, []string{"exec", "mypod", "-n", "default", "-c", "app", "--", "uname", "-a"}, callLog[0].Args)
705736
})
706737

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

13961427
k8sTool := newTestK8sToolWithPassthrough(true)

0 commit comments

Comments
 (0)