diff --git a/internal/cmd/vm/cmd_container.go b/internal/cmd/vm/cmd_container.go index fa9246fe..d2fafb54 100644 --- a/internal/cmd/vm/cmd_container.go +++ b/internal/cmd/vm/cmd_container.go @@ -16,6 +16,34 @@ import ( "github.com/spf13/cobra" ) +var ( + // allowedExecCommands is a whitelist of commands allowed to be executed in containers. + allowedExecCommands = map[string]bool{ + "ls": true, + "ps": true, + "cat": true, + "top": true, + "df": true, + "du": true, + "ifconfig": true, + "ip": true, + "ping": true, + "netstat": true, + "date": true, + "uptime": true, + "whoami": true, + "id": true, + "uname": true, + "echo": true, + "tail": true, + "head": true, + "grep": true, + "sleep": true, + "sh": true, + "bash": true, + } +) + var ( runName string runDetach bool @@ -330,6 +358,16 @@ func addVMExecCommand(parent *cobra.Command) { } func execInContainer(id string, cmd []string) error { + if len(cmd) == 0 { + return errors.New(i18n.T("cmd.vm.error.id_and_cmd_required")) + } + + // Validate against whitelist + baseCmd := cmd[0] + if !allowedExecCommands[baseCmd] { + return errors.New(i18n.T("cmd.vm.error.command_not_allowed", map[string]interface{}{"Command": baseCmd})) + } + manager, err := container.NewLinuxKitManager(io.Local) if err != nil { return fmt.Errorf(i18n.T("i18n.fail.init", "container manager")+": %w", err) diff --git a/internal/cmd/vm/cmd_container_test.go b/internal/cmd/vm/cmd_container_test.go new file mode 100644 index 00000000..3ac6a338 --- /dev/null +++ b/internal/cmd/vm/cmd_container_test.go @@ -0,0 +1,51 @@ +package vm + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestExecInContainer_Whitelist(t *testing.T) { + tests := []struct { + name string + cmd []string + expected string // Expected error substring + }{ + { + "Allowed command", + []string{"ls", "-la"}, + "", // Will fail later with "failed to determine state path" or similar, but NOT whitelist error + }, + { + "Disallowed command", + []string{"rm", "-rf", "/"}, + "command not allowed: rm", + }, + { + "Injection attempt in first arg", + []string{"ls; rm", "-rf", "/"}, + "command not allowed: ls; rm", + }, + { + "Empty command", + []string{}, + "container ID and command required", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := execInContainer("test-id", tt.cmd) + if tt.expected == "" { + // Should NOT be a whitelist error + if err != nil { + assert.NotContains(t, err.Error(), "command not allowed") + } + } else { + assert.Error(t, err) + assert.Contains(t, err.Error(), tt.expected) + } + }) + } +} diff --git a/pkg/container/exec_security_test.go b/pkg/container/exec_security_test.go new file mode 100644 index 00000000..b4016aa0 --- /dev/null +++ b/pkg/container/exec_security_test.go @@ -0,0 +1,28 @@ +package container + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestEscapeShellArg(t *testing.T) { + tests := []struct { + input string + expected string + }{ + {"ls", "'ls'"}, + {"foo bar", "'foo bar'"}, + {"it's", "'it'\\''s'"}, + {"; rm -rf /", "'; rm -rf /'"}, + {"$(whoami)", "'$(whoami)'"}, + {"`whoami`", "'`whoami`'"}, + {"\"quoted\"", "'\"quoted\"'"}, + } + + for _, tt := range tests { + t.Run(tt.input, func(t *testing.T) { + assert.Equal(t, tt.expected, escapeShellArg(tt.input)) + }) + } +} diff --git a/pkg/container/linuxkit.go b/pkg/container/linuxkit.go index d3bba481..8d413167 100644 --- a/pkg/container/linuxkit.go +++ b/pkg/container/linuxkit.go @@ -7,6 +7,7 @@ import ( goio "io" "os" "os/exec" + "strings" "syscall" "time" @@ -416,6 +417,13 @@ func (f *followReader) Close() error { return f.file.Close() } +// escapeShellArg safely quotes a string for use as a shell argument. +func escapeShellArg(arg string) string { + // Wrap in single quotes and escape existing single quotes. + // For example: 'it'\''s' + return "'" + strings.ReplaceAll(arg, "'", "'\\''") + "'" +} + // Exec executes a command inside the container via SSH. func (m *LinuxKitManager) Exec(ctx context.Context, id string, cmd []string) error { if err := ctx.Err(); err != nil { @@ -441,7 +449,11 @@ func (m *LinuxKitManager) Exec(ctx context.Context, id string, cmd []string) err "-o", "LogLevel=ERROR", "root@localhost", } - sshArgs = append(sshArgs, cmd...) + + // Escape each command argument for the remote shell + for _, c := range cmd { + sshArgs = append(sshArgs, escapeShellArg(c)) + } sshCmd := exec.CommandContext(ctx, "ssh", sshArgs...) sshCmd.Stdin = os.Stdin diff --git a/pkg/i18n/locales/en_GB.json b/pkg/i18n/locales/en_GB.json index 4f6d8f4f..702241b7 100644 --- a/pkg/i18n/locales/en_GB.json +++ b/pkg/i18n/locales/en_GB.json @@ -633,6 +633,8 @@ "stop.short": "Stop a running VM", "logs.short": "View VM logs", "exec.short": "Execute a command in a VM", + "error.id_and_cmd_required": "container ID and command required", + "error.command_not_allowed": "command not allowed: {{.Command}}", "templates.short": "Manage LinuxKit templates" }, "monitor": {