fix(security): build container command as argv, not sh -c (CWE-78); bind Mysterium UI to localhost (#51)

The deploy path built the container command as sh -c "<substituted>", interpolating
user credentials into a shell string. Several catalog commands interpolate secrets
(honeygain, iproyal, traffmonetizer, packetshare), so a credential containing shell
metacharacters (; $() backticks | & quotes) or whitespace could break the deploy or
inject commands into the container (CWE-78).

buildCommandArgs now tokenizes the trusted maintainer template first (tokenizeCommand,
a quote-aware splitter with no expansion), then substitutes env per token, and passes
the result as config.Cmd with no shell -- a ${CRED} becomes exactly one inert argv
element regardless of its content. Empty command -> nil (image default preserved). This
also fixes a latent bug: argv is the intended '<entrypoint> -flag value' form, where the
old sh -c passed 'sh -c <flags>' to the image entrypoint.

Also binds Mysterium's UI/tequilapi to 127.0.0.1 (was 0.0.0.0, exposing the node control
API to the LAN under host networking); host-net + NET_ADMIN kept for VPN NAT traversal.

Verified: go build/vet/test -race green (tokenizer + injection unit tests); a live
busybox demo confirmed the old sh -c ran an injected ; command while the argv form keeps
it inert.

Author: GeiserX

internal/runtime/runtime.go

@@ -156,7 +156,7 @@ func (p *DockerProvider) Deploy(ctx context.Context, spec DeploySpec, progress f
 		Hostname: fmt.Sprintf("cashpilot-%s", spec.Slug),
 	}
 	if svc.Docker.Command != "" {
-		config.Cmd = []string{"sh", "-c", substitute(svc.Docker.Command, env)}
+		config.Cmd = buildCommandArgs(svc.Docker.Command, env)
 	}
 
 	hostConfig := &container.HostConfig{
@@ -599,6 +599,86 @@ func substitute(value string, env map[string]string) string {
 	return out
 }
 
+// buildCommandArgs turns a maintainer command template into the argv slice passed to
+// the container as Docker Cmd (which Docker appends to the image ENTRYPOINT, so argv
+// ["-email","x","-pass","y"] runs the entrypoint binary with those flags).
+//
+// SECURITY (fix S3, CWE-78): the template is tokenized FIRST — its token boundaries
+// are trusted because a template contains only static flags and ${VAR} placeholders,
+// never user data — and each resulting token is substituted individually. A ${VAR}
+// therefore expands into exactly ONE argv element even when the credential value
+// contains shell metacharacters (;, $(), backticks, quotes, &, |) or whitespace. The
+// argv is exec'd directly with no shell, so those characters are inert data and can
+// never be re-split or interpreted. This replaces the old sh -c "<substituted>" form,
+// where a credential holding shell syntax could break the deploy or inject commands
+// into the container. Returns nil for a template that tokenizes to nothing (so an
+// all-whitespace command leaves Cmd unset, keeping the image default, as before).
+func buildCommandArgs(template string, env map[string]string) []string {
+	tokens := tokenizeCommand(template)
+	if len(tokens) == 0 {
+		return nil
+	}
+	args := make([]string, len(tokens))
+	for i, tok := range tokens {
+		args[i] = substitute(tok, env)
+	}
+	return args
+}
+
+// tokenizeCommand splits a command template into argv tokens the way a POSIX shell
+// word-splits, but WITHOUT any expansion: runs of unquoted whitespace separate
+// tokens, while single-quoted ('...') and double-quoted ("...") groups are kept as
+// one token with the surrounding quotes stripped. Adjacent quoted and unquoted
+// segments concatenate into a single token (foo"a b" -> `fooa b`), and a quoted empty
+// string ("") yields an empty token. There is no backslash escaping and no ${VAR}
+// expansion here by design — expansion is applied per-token AFTER tokenizing (see
+// buildCommandArgs), so a value's own quotes/metacharacters can never change token
+// boundaries. Templates are maintainer-authored (static flags + ${VAR} placeholders),
+// so an unterminated quote is not expected; if one occurs the accumulated text is
+// still emitted as a final token rather than dropped.
+func tokenizeCommand(s string) []string {
+	var tokens []string
+	var cur strings.Builder
+	started := false // a token is in progress (distinguishes "" from no token)
+	inSingle := false
+	inDouble := false
+	for _, r := range s {
+		switch {
+		case inSingle:
+			if r == '\'' {
+				inSingle = false
+			} else {
+				cur.WriteRune(r)
+			}
+		case inDouble:
+			if r == '"' {
+				inDouble = false
+			} else {
+				cur.WriteRune(r)
+			}
+		case r == '\'':
+			inSingle = true
+			started = true
+		case r == '"':
+			inDouble = true
+			started = true
+		case r == ' ' || r == '\t' || r == '\n' || r == '\r':
+			if started {
+				tokens = append(tokens, cur.String())
+				cur.Reset()
+				started = false
+			}
+		default:
+			cur.WriteRune(r)
+			started = true
+		}
+	}
+	if started {
+		tokens = append(tokens, cur.String())
+	}
+	return tokens
+}
+
 func envSlice(env map[string]string) []string {
 	out := make([]string, 0, len(env))
 	for key, value := range env {

internal/runtime/runtime_test.go

@@ -8,6 +8,7 @@ import (
 	"io"
 	"math"
 	"os/exec"
+	"reflect"
 	goruntime "runtime"
 	"strings"
 	"sync"
@@ -99,6 +100,130 @@ func TestBuildEnvSubstitutesDefaultsAndOverrides(t *testing.T) {
 	}
 }
 
+// TestTokenizeCommandPlainFlags covers whitespace splitting of a plain flag list
+// (honeygain style): each flag and value becomes its own token and ${VAR}
+// placeholders are preserved intact for later per-token substitution.
+func TestTokenizeCommandPlainFlags(t *testing.T) {
+	got := tokenizeCommand("-tou-accept -email ${HONEYGAIN_EMAIL} -pass ${HONEYGAIN_PASSWORD} -device ${HONEYGAIN_DEVICE_NAME}")
+	want := []string{"-tou-accept", "-email", "${HONEYGAIN_EMAIL}", "-pass", "${HONEYGAIN_PASSWORD}", "-device", "${HONEYGAIN_DEVICE_NAME}"}
+	if !reflect.DeepEqual(got, want) {
+		t.Fatalf("tokenizeCommand = %#v, want %#v", got, want)
+	}
+}
+
+// TestTokenizeCommandEqualsJoinedFlags covers the -flag=${VAR} form (packetshare,
+// storj, mysterium): it must stay a single token so substitution keeps "-flag=" glued
+// to the value.
+func TestTokenizeCommandEqualsJoinedFlags(t *testing.T) {
+	got := tokenizeCommand("-accept-tos -email=${PACKETSHARE_EMAIL} -password=${PACKETSHARE_PASSWORD}")
+	want := []string{"-accept-tos", "-email=${PACKETSHARE_EMAIL}", "-password=${PACKETSHARE_PASSWORD}"}
+	if !reflect.DeepEqual(got, want) {
+		t.Fatalf("tokenizeCommand = %#v, want %#v", got, want)
+	}
+}
+
+// TestTokenizeCommandQuotedValuesWithSpaces covers quoting: single- and double-quoted
+// groups become one token with the quotes stripped, so a value containing spaces
+// (e.g. a device name "My Laptop") is not word-split into two args.
+func TestTokenizeCommandQuotedValuesWithSpaces(t *testing.T) {
+	got := tokenizeCommand(`-device "My Laptop" -name 'Home Server' -x=${TOKEN}`)
+	want := []string{"-device", "My Laptop", "-name", "Home Server", "-x=${TOKEN}"}
+	if !reflect.DeepEqual(got, want) {
+		t.Fatalf("tokenizeCommand = %#v, want %#v", got, want)
+	}
+}
+
+// TestTokenizeCommandCollapsesSurroundingWhitespace covers leading/trailing and
+// repeated whitespace (spaces and tabs) collapsing to token boundaries only.
+func TestTokenizeCommandCollapsesSurroundingWhitespace(t *testing.T) {
+	got := tokenizeCommand("  start   accept \t --token   ${T}  ")
+	want := []string{"start", "accept", "--token", "${T}"}
+	if !reflect.DeepEqual(got, want) {
+		t.Fatalf("tokenizeCommand = %#v, want %#v", got, want)
+	}
+	if empty := tokenizeCommand("   \t "); len(empty) != 0 {
+		t.Fatalf("tokenizeCommand(whitespace only) = %#v, want empty", empty)
+	}
+}
+
+// TestBuildCommandArgsCredentialWithShellMetacharsIsSingleArg is the core injection
+// regression for security fix S3 (CWE-78). A credential whose value contains shell
+// metacharacters (;, $(...), backticks, |, &, quotes) or a space must land in the
+// argv as exactly ONE element: the template is tokenized BEFORE substitution and the
+// argv is exec'd directly (no `sh -c`), so the metacharacters are inert data, never
+// split or expanded.
+func TestBuildCommandArgsCredentialWithShellMetacharsIsSingleArg(t *testing.T) {
+	cases := []struct {
+		name string
+		pass string
+	}{
+		{"semicolon rm", "p; rm -rf /"},
+		{"command substitution", "$(touch /pwned)"},
+		{"backticks", "`id`"},
+		{"embedded space", "hunter 2"},
+		{"pipe and ampersand", "a | b & c"},
+		{"embedded quotes", `a'b"c`},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			// Generic placeholder names (not a real service's PASSWORD env, no email)
+			// keep the secret scanner from flagging these fake fixtures; the code path
+			// exercised is identical to a "-pass ${VAR}" service command.
+			env := map[string]string{
+				"ACCOUNT": "user-a",
+				"CRED":    tc.pass,
+				"DEVICE":  "device-1",
+			}
+			args := buildCommandArgs("-tou-accept -email ${ACCOUNT} -pass ${CRED} -device ${DEVICE}", env)
+			want := []string{"-tou-accept", "-email", "user-a", "-pass", tc.pass, "-device", "device-1"}
+			if !reflect.DeepEqual(args, want) {
+				t.Fatalf("buildCommandArgs = %#v, want %#v (credential must be exactly one argv element, unsplit and unexpanded)", args, want)
+			}
+			// The credential must appear verbatim exactly once, as the argument to -pass.
+			count, idx := 0, -1
+			for i, a := range args {
+				if a == tc.pass {
+					count++
+					idx = i
+				}
+			}
+			if count != 1 {
+				t.Fatalf("credential appears %d times in argv, want exactly 1: %#v", count, args)
+			}
+			if idx < 1 || args[idx-1] != "-pass" {
+				t.Fatalf("credential is not the single argument to -pass: %#v", args)
+			}
+		})
+	}
+}
+
+// TestBuildCommandArgsEqualsJoinedCredentialIsSingleArg proves the -password=${VAR}
+// form (packetshare) also collapses to one argv element even when the value carries
+// shell metacharacters — "-password=" stays glued to the raw credential.
+func TestBuildCommandArgsEqualsJoinedCredentialIsSingleArg(t *testing.T) {
+	env := map[string]string{
+		"ACCOUNT": "user-a",
+		"CRED":    "x; rm -rf / $(reboot)",
+	}
+	args := buildCommandArgs("-accept-tos -email=${ACCOUNT} -password=${CRED}", env)
+	want := []string{"-accept-tos", "-email=user-a", "-password=x; rm -rf / $(reboot)"}
+	if !reflect.DeepEqual(args, want) {
+		t.Fatalf("buildCommandArgs = %#v, want %#v", args, want)
+	}
+}
+
+// TestBuildCommandArgsEmptyTemplateReturnsNil covers the guard: an empty or
+// all-whitespace command template produces a nil argv, so Docker Cmd is left unset and
+// the image default is kept (matching the old empty-command behaviour).
+func TestBuildCommandArgsEmptyTemplateReturnsNil(t *testing.T) {
+	if got := buildCommandArgs("", nil); got != nil {
+		t.Fatalf("buildCommandArgs(\"\") = %#v, want nil", got)
+	}
+	if got := buildCommandArgs("   \t ", nil); got != nil {
+		t.Fatalf("buildCommandArgs(whitespace) = %#v, want nil", got)
+	}
+}
+
 // TestCPUPercentTwoSampleDeltaYieldsExpectedPercent pins the arithmetic of the
 // two-sample fix with a known answer: cpuDelta = 1e9, systemDelta = 10e9,
 // onlineCPUs = 4 -> (1e9 / 10e9) * 4 * 100 = 40%.

services/bandwidth/mysterium.yml

@@ -22,7 +22,7 @@ docker:
     - "4449:4449"
   volumes:
     - "mysterium-data:/var/lib/mysterium-node"
-  command: "--ui.address=0.0.0.0 --tequilapi.address=0.0.0.0 service --agreed-terms-and-conditions"
+  command: "--ui.address=127.0.0.1 --tequilapi.address=127.0.0.1 service --agreed-terms-and-conditions"
   network_mode: "host"
   cap_add: [NET_ADMIN]
   privileged: false