Skip to content

Commit 7791915

Browse files
committed
fix(console): shell-escape zmx session name
Defense-in-depth against shell injection via the --session flag. Although validSessionName already restricts the value to a safe character set, escaping at the point of use protects against future regex changes or callers that bypass the regex. Fixes #9
1 parent 3b32043 commit 7791915

2 files changed

Lines changed: 46 additions & 1 deletion

File tree

cmd/console.go

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"regexp"
77
"time"
88

9+
"al.essio.dev/pkg/shellescape"
910
"github.com/briandowns/spinner"
1011
"github.com/spf13/cobra"
1112

@@ -104,7 +105,14 @@ func zmxRemoteCmdViaSandbox(ctx context.Context, sb sandbox.Sandbox, name, sessi
104105
Cmd: []string{"sh", "-c", "command -v zmx >/dev/null 2>&1"},
105106
})
106107
if err == nil && code == 0 {
107-
return []string{"sh", "-lc", "unset XDG_RUNTIME_DIR && zmx attach " + session + " bash -l"}
108+
return zmxAttachCmd(session)
108109
}
109110
return nil
110111
}
112+
113+
// zmxAttachCmd builds the shell command that attaches to a zmx session.
114+
// The session name is shell-escaped for defense-in-depth even though
115+
// validSessionName already restricts it to a safe character set.
116+
func zmxAttachCmd(session string) []string {
117+
return []string{"sh", "-lc", "unset XDG_RUNTIME_DIR && zmx attach " + shellescape.Quote(session) + " bash -l"}
118+
}

cmd/console_test.go

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
package cmd
2+
3+
import (
4+
"strings"
5+
"testing"
6+
)
7+
8+
func TestZmxAttachCmdEscapesSession(t *testing.T) {
9+
tests := []struct {
10+
name string
11+
session string
12+
contains string
13+
}{
14+
{"plain", "console", "zmx attach console bash -l"},
15+
{"with-dash", "my-session", "zmx attach my-session bash -l"},
16+
{"with-dot", "test.1", "zmx attach test.1 bash -l"},
17+
{"semicolon", "foo;rm -rf /", "zmx attach 'foo;rm -rf /' bash -l"},
18+
{"backtick", "foo`id`", "zmx attach 'foo`id`' bash -l"},
19+
{"dollar", "foo$(id)", "zmx attach 'foo$(id)' bash -l"},
20+
{"single-quote", "foo'bar", `zmx attach 'foo'"'"'bar' bash -l`},
21+
{"space", "foo bar", "zmx attach 'foo bar' bash -l"},
22+
}
23+
for _, tt := range tests {
24+
t.Run(tt.name, func(t *testing.T) {
25+
got := zmxAttachCmd(tt.session)
26+
if len(got) != 3 {
27+
t.Fatalf("zmxAttachCmd returned %d args, want 3", len(got))
28+
}
29+
if got[0] != "sh" || got[1] != "-lc" {
30+
t.Errorf("zmxAttachCmd prefix = %q %q, want sh -lc", got[0], got[1])
31+
}
32+
if !strings.Contains(got[2], tt.contains) {
33+
t.Errorf("zmxAttachCmd(%q)[2] = %q, want substring %q", tt.session, got[2], tt.contains)
34+
}
35+
})
36+
}
37+
}

0 commit comments

Comments
 (0)