Merge branch 'dev' into fix/ssh-security-13442055821003769195

.github/workflows/auto-merge.yml

@@ -1,3 +1,6 @@
+# This workflow is localized from host-uk/.github/.github/workflows/auto-merge.yml@dev
+# because the reusable version is currently failing due to missing git context.
+# See: https://github.com/host-uk/core/actions/runs/21697467567/job/62570690752
 name: Auto Merge
 
 on:

docs/adr/0000-template.md

@@ -0,0 +1,49 @@
+# ADR Template
+
+* Status: [proposed | rejected | accepted | deprecated | superseded by [ADR-NNNN](NNNN-example.md)]
+* Deciders: [list of names or roles involved in the decision]
+* Date: [YYYY-MM-DD when the decision was last updated]
+
+## Context and Problem Statement
+
+[Describe the context and problem statement, e.g., in free form using several paragraphs or bullet points. Explain why this decision is needed now.]
+
+## Decision Drivers
+
+* [driver 1, e.g., a force, facing concern, ...]
+* [driver 2, e.g., a force, facing concern, ...]
+
+## Considered Options
+
+* [option 1]
+* [option 2]
+
+## Decision Outcome
+
+Chosen option: "[option 1]", because [justification. e.g., only option, which meets currently debated acceptance criteria].
+
+### Positive Consequences
+
+* [e.g., improvement in quality attribute, follow-up decisions required, ...]
+* ...
+
+### Negative Consequences
+
+* [e.g., compromising quality attribute, follow-up decisions required, ...]
+* ...
+
+## Pros and Cons of the Options
+
+### [option 1]
+
+[example | description | pointer to more information | ...]
+
+* Good, because [argument 1]
+* Bad, because [argument 2]
+
+### [option 2]
+
+[example | description | pointer to more information | ...]
+
+* Good, because [argument 1]
+* Bad, because [argument 2]

docs/adr/0001-use-wails-v3.md

@@ -0,0 +1,39 @@
+# ADR 0001: Use Wails v3 for GUI
+
+* Status: accepted
+* Deciders: Project Maintainers
+* Date: 2025-05-15
+
+## Context and Problem Statement
+
+The project needs a way to build cross-platform desktop applications with a modern UI. Historically, Electron has been the go-to choice, but it is known for its high resource consumption and large binary sizes.
+
+## Decision Drivers
+
+* Performance and resource efficiency.
+* Smaller binary sizes.
+* Tight integration with Go.
+* Native look and feel.
+
+## Considered Options
+
+* Electron
+* Wails (v2)
+* Wails (v3)
+* Fyne
+
+## Decision Outcome
+
+Chosen option: "Wails (v3)", because it provides the best balance of using web technologies for the UI while keeping the backend in Go with minimal overhead. Wails v3 specifically offers improvements in performance and features over v2.
+
+### Positive Consequences
+
+* Significantly smaller binary sizes compared to Electron.
+* Reduced memory usage.
+* Ability to use any frontend framework (Vue, React, Svelte, etc.).
+* Direct Go-to-JS bindings.
+
+### Negative Consequences
+
+* Wails v3 is still in alpha/beta, which might lead to breaking changes or bugs.
+* Smaller ecosystem compared to Electron.

docs/adr/0002-ipc-bridge-pattern.md

@@ -0,0 +1,37 @@
+# ADR 0002: IPC Bridge Pattern
+
+* Status: accepted
+* Deciders: Project Maintainers
+* Date: 2025-05-15
+
+## Context and Problem Statement
+
+Wails allows direct binding of Go methods to the frontend. However, as the number of services and methods grows, managing individual bindings for every service becomes complex. We need a way to decouple the frontend from the internal service structure.
+
+## Decision Drivers
+
+* Decoupling services from the frontend runtime.
+* Simplified binding generation.
+* Centralized message routing.
+* Uniform internal and external communication.
+
+## Considered Options
+
+* Direct Wails Bindings for all services.
+* IPC Bridge Pattern (Centralized ACTION handler).
+
+## Decision Outcome
+
+Chosen option: "IPC Bridge Pattern", because it allows services to remain agnostic of the frontend runtime. Only the `Core` service is registered with Wails, and it exposes a single `ACTION` method that routes messages to the appropriate service based on an IPC handler.
+
+### Positive Consequences
+
+* Only one Wails service needs to be registered.
+* Services can be tested independently of Wails.
+* Adding new functionality to a service doesn't necessarily require regenerating frontend bindings.
+* Consistency between frontend-to-backend and backend-to-backend communication.
+
+### Negative Consequences
+
+* Less type safety out-of-the-box in the frontend for specific service methods (though this can be improved with manual type definitions or codegen).
+* Requires services to implement `HandleIPCEvents`.

docs/adr/0003-soa-dual-constructor-di.md

@@ -0,0 +1,37 @@
+# ADR 0003: Service-Oriented Architecture with Dual-Constructor DI
+
+* Status: accepted
+* Deciders: Project Maintainers
+* Date: 2025-05-15
+
+## Context and Problem Statement
+
+The application consists of many components (config, crypt, workspace, etc.) that depend on each other. We need a consistent way to manage these dependencies and allow for easy testing.
+
+## Decision Drivers
+
+* Testability.
+* Modularity.
+* Ease of service registration.
+* Clear lifecycle management.
+
+## Considered Options
+
+* Global variables/singletons.
+* Dependency Injection (DI) container.
+* Manual Dependency Injection.
+
+## Decision Outcome
+
+Chosen option: "Service-Oriented Architecture with Dual-Constructor DI". Each service follows a pattern where it provides a `New()` constructor for standalone use/testing (static DI) and a `Register()` function for registration with the `Core` service container (dynamic DI).
+
+### Positive Consequences
+
+* Easy to unit test services by passing mock dependencies to `New()`.
+* Automatic service discovery and lifecycle management via `Core`.
+* Decoupled components.
+
+### Negative Consequences
+
+* Some boilerplate required for each service (`New` and `Register`).
+* Dependency on `pkg/core` for `Register`.

docs/adr/0004-storage-abstraction-medium.md

@@ -0,0 +1,36 @@
+# ADR 0004: Storage Abstraction via Medium Interface
+
+* Status: accepted
+* Deciders: Project Maintainers
+* Date: 2025-05-15
+
+## Context and Problem Statement
+
+The application needs to support different storage backends (local file system, SFTP, WebDAV, etc.) for its workspace data. Hardcoding file system operations would make it difficult to support remote storage.
+
+## Decision Drivers
+
+* Flexibility in storage backends.
+* Ease of testing (mocking storage).
+* Uniform API for file operations.
+
+## Considered Options
+
+* Standard `os` package.
+* Interface abstraction (`Medium`).
+* `spf13/afero` library.
+
+## Decision Outcome
+
+Chosen option: "Interface abstraction (`Medium`)". We defined a custom `Medium` interface in `pkg/io` that abstracts common file operations.
+
+### Positive Consequences
+
+* Application logic is agnostic of where files are actually stored.
+* Easy to implement new backends (SFTP, WebDAV).
+* Simplified testing using `MockMedium`.
+
+### Negative Consequences
+
+* Small overhead of interface calls.
+* Need to ensure all file operations go through the `Medium` interface.

docs/adr/README.md

@@ -0,0 +1,32 @@
+# Architecture Decision Records (ADR)
+
+This directory contains the Architecture Decision Records for the Core project.
+
+## What is an ADR?
+
+An Architecture Decision Record (ADR) is a document that captures an important architectural decision made along with its context and consequences.
+
+## Why use ADRs?
+
+- **Context:** Helps new contributors understand *why* certain decisions were made.
+- **History:** Provides a historical record of the evolution of the project's architecture.
+- **Transparency:** Makes the decision-making process transparent and open for discussion.
+
+## ADR Process
+
+1. **Identify a Decision:** When an architectural decision needs to be made, start a new ADR.
+2. **Use the Template:** Copy `0000-template.md` to a new file named `NNNN-short-title.md` (e.g., `0001-use-wails-v3.md`).
+3. **Draft the ADR:** Fill in the context, drivers, and considered options.
+4. **Propose:** Set the status to `proposed` and open a Pull Request for discussion.
+5. **Accept/Reject:** Once a consensus is reached, update the status to `accepted` or `rejected` and merge.
+6. **Supersede:** If a later decision changes an existing one, update the status of the old ADR to `superseded` and point to the new one.
+
+## ADR Index
+
+| ID | Title | Status | Date |
+|---|---|---|---|
+| 0000 | [ADR Template](0000-template.md) | N/A | 2025-05-15 |
+| 0001 | [Use Wails v3 for GUI](0001-use-wails-v3.md) | accepted | 2025-05-15 |
+| 0002 | [IPC Bridge Pattern](0002-ipc-bridge-pattern.md) | accepted | 2025-05-15 |
+| 0003 | [Service-Oriented Architecture with Dual-Constructor DI](0003-soa-dual-constructor-di.md) | accepted | 2025-05-15 |
+| 0004 | [Storage Abstraction via Medium Interface](0004-storage-abstraction-medium.md) | accepted | 2025-05-15 |

docs/index.md

@@ -85,6 +85,7 @@ And `repos.yaml` in workspace root for multi-repo management.
 ## Reference
 
 - [Configuration](configuration.md) - All config options
+- [Architecture Decisions (ADR)](adr/README.md) - Key architectural decisions
 - [Glossary](glossary.md) - Term definitions
 
 ## Claude Code Skill

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)

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)
+			}
+		})
+	}
+}

mkdocs.yml

@@ -68,6 +68,12 @@ nav:
   - GUI Application:
     - Overview: gui/overview.md
     - MCP Bridge: gui/mcp-bridge.md
+  - Architecture Decisions:
+    - Overview: adr/README.md
+    - "ADR 0001: Use Wails v3": adr/0001-use-wails-v3.md
+    - "ADR 0002: IPC Bridge Pattern": adr/0002-ipc-bridge-pattern.md
+    - "ADR 0003: Service Pattern": adr/0003-soa-dual-constructor-di.md
+    - "ADR 0004: Storage Abstraction": adr/0004-storage-abstraction-medium.md
   - API Reference:
     - Core: api/core.md
     - Display: api/display.md

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))
+		})
+	}
+}

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

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": {