Skip to content

Commit 7fb97ba

Browse files
deevusclaude
andcommitted
fix: switch SSH env forwarding from SetEnv to SendEnv, restrict AcceptEnv
SetEnv exposes secret values in process argv (visible via ps aux). SendEnv only puts key names in argv — values are read from the process environment by the SSH client. Also restricts AcceptEnv from wildcard (*) to only the specific variable names configured in env.forward, preventing arbitrary env injection by other SSH clients connecting to the container. Closes #8 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 0e7a41a commit 7fb97ba

13 files changed

Lines changed: 209 additions & 56 deletions

File tree

cmd/root.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package cmd
33
import (
44
"fmt"
55
"os"
6+
"sort"
67
"strconv"
78
"strings"
89

@@ -93,6 +94,14 @@ func sandboxConfig() map[string]string {
9394
if len(cfg.Defaults.DNS) > 0 {
9495
m["dns"] = strings.Join(cfg.Defaults.DNS, ",")
9596
}
97+
if len(cfg.EnvForward) > 0 {
98+
keys := make([]string, 0, len(cfg.EnvForward))
99+
for k := range cfg.EnvForward {
100+
keys = append(keys, k)
101+
}
102+
sort.Strings(keys)
103+
m["env_forward_keys"] = strings.Join(keys, ",")
104+
}
96105

97106
// Backend-specific keys.
98107
switch cfg.Backend {

internal/ssh/console_unix.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,5 +17,9 @@ func Console(cc ConnConfig, remoteCmd string) error {
1717
return fmt.Errorf("ssh binary not found: %w", err)
1818
}
1919
args := append([]string{"ssh"}, consoleArgs(cc, remoteCmd)...)
20-
return syscall.Exec(sshBin, args, os.Environ())
20+
environ := os.Environ()
21+
if len(cc.Env) > 0 {
22+
environ = EnvWithOverrides(environ, cc.Env)
23+
}
24+
return syscall.Exec(sshBin, args, environ)
2125
}

internal/ssh/console_windows.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,5 +19,8 @@ func Console(cc ConnConfig, remoteCmd string) error {
1919
cmd.Stdin = os.Stdin
2020
cmd.Stdout = os.Stdout
2121
cmd.Stderr = os.Stderr
22+
if len(cc.Env) > 0 {
23+
cmd.Env = EnvWithOverrides(os.Environ(), cc.Env)
24+
}
2225
return cmd.Run()
2326
}

internal/ssh/ssh.go

Lines changed: 39 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ type ConnConfig struct {
2222
Host string
2323
User string
2424
KeyPath string
25-
Env map[string]string // optional, for SetEnv forwarding
25+
Env map[string]string // optional, for SendEnv forwarding
2626
KnownHostsPath string // path to known_hosts file for accept-new verification
2727
}
2828

@@ -75,6 +75,7 @@ func Exec(ctx context.Context, cc ConnConfig, command []string) (int, error) {
7575
cmd.Stdin = os.Stdin
7676
cmd.Stdout = os.Stdout
7777
cmd.Stderr = os.Stderr
78+
applyEnv(cmd, cc)
7879

7980
if err := cmd.Run(); err != nil {
8081
var exitErr *exec.ExitError
@@ -91,6 +92,7 @@ func Exec(ctx context.Context, cc ConnConfig, command []string) (int, error) {
9192
func ExecQuiet(ctx context.Context, cc ConnConfig, command []string) (int, error) {
9293
args := append(Args(cc), command...)
9394
cmd := exec.CommandContext(ctx, "ssh", args...)
95+
applyEnv(cmd, cc)
9496

9597
if err := cmd.Run(); err != nil {
9698
var exitErr *exec.ExitError
@@ -107,6 +109,7 @@ func Output(ctx context.Context, cc ConnConfig, command []string) ([]byte, error
107109
args := append(Args(cc), command...)
108110
cmd := exec.CommandContext(ctx, "ssh", args...)
109111
cmd.Stderr = os.Stderr
112+
applyEnv(cmd, cc)
110113
return cmd.Output()
111114
}
112115

@@ -115,6 +118,7 @@ func Output(ctx context.Context, cc ConnConfig, command []string) ([]byte, error
115118
func OutputQuiet(ctx context.Context, cc ConnConfig, command []string) ([]byte, error) {
116119
args := append(Args(cc), command...)
117120
cmd := exec.CommandContext(ctx, "ssh", args...)
121+
applyEnv(cmd, cc)
118122
return cmd.Output()
119123
}
120124

@@ -123,9 +127,17 @@ func OutputQuiet(ctx context.Context, cc ConnConfig, command []string) ([]byte,
123127
func TestAuth(ctx context.Context, cc ConnConfig) error {
124128
args := append(Args(cc), "true")
125129
cmd := exec.CommandContext(ctx, "ssh", args...)
130+
applyEnv(cmd, cc)
126131
return cmd.Run()
127132
}
128133

134+
// applyEnv sets cmd.Env if the ConnConfig has env vars to forward via SendEnv.
135+
func applyEnv(cmd *exec.Cmd, cc ConnConfig) {
136+
if len(cc.Env) > 0 {
137+
cmd.Env = EnvWithOverrides(os.Environ(), cc.Env)
138+
}
139+
}
140+
129141
// Args builds the SSH command-line arguments for the given connection config.
130142
// It is exported for use by callers that need to construct custom exec.Cmd
131143
// with non-standard Stdin/Stdout/Stderr (e.g. sandbox backends).
@@ -145,30 +157,44 @@ func Args(cc ConnConfig) []string {
145157
}
146158

147159
// Forward env vars via SSH protocol (requires AcceptEnv on server).
148-
// All vars must be in a single SetEnv directive (multiple -o SetEnv
149-
// flags don't stack in OpenSSH — only the first takes effect).
160+
// SendEnv tells the client to read values from its own process
161+
// environment — only key names appear in argv, not secret values.
150162
if len(cc.Env) > 0 {
151163
keys := make([]string, 0, len(cc.Env))
152164
for k := range cc.Env {
153165
keys = append(keys, k)
154166
}
155167
sort.Strings(keys)
156-
157-
var setenv strings.Builder
158-
setenv.WriteString("SetEnv=")
159-
for i, k := range keys {
160-
if i > 0 {
161-
setenv.WriteByte(' ')
162-
}
163-
fmt.Fprintf(&setenv, "%s=%s", k, cc.Env[k])
164-
}
165-
args = append(args, "-o", setenv.String())
168+
args = append(args, "-o", "SendEnv="+strings.Join(keys, " "))
166169
}
167170

168171
args = append(args, cc.User+"@"+cc.Host)
169172
return args
170173
}
171174

175+
// EnvWithOverrides returns a copy of base with entries from overrides
176+
// added or replaced. Used to inject env vars into the SSH process
177+
// environment so that SendEnv can forward them.
178+
func EnvWithOverrides(base []string, overrides map[string]string) []string {
179+
env := make([]string, len(base))
180+
copy(env, base)
181+
for k, v := range overrides {
182+
prefix := k + "="
183+
found := false
184+
for i, e := range env {
185+
if strings.HasPrefix(e, prefix) {
186+
env[i] = prefix + v
187+
found = true
188+
break
189+
}
190+
}
191+
if !found {
192+
env = append(env, prefix+v)
193+
}
194+
}
195+
return env
196+
}
197+
172198
// RemoveKnownHost removes all entries for the given host from the known_hosts
173199
// file. This is used to clean up stale entries when containers are
174200
// created, destroyed, or restored from snapshots. It is a no-op if the

internal/ssh/ssh_test.go

Lines changed: 72 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package ssh
22

33
import (
44
"os"
5+
"slices"
56
"strings"
67
"testing"
78
)
@@ -92,7 +93,7 @@ func TestSSHArgs(t *testing.T) {
9293
}
9394
})
9495

95-
t.Run("SetEnv with env vars", func(t *testing.T) {
96+
t.Run("SendEnv with env vars", func(t *testing.T) {
9697
cc := ConnConfig{
9798
Host: "10.0.0.1",
9899
User: "pixel",
@@ -103,24 +104,27 @@ func TestSSHArgs(t *testing.T) {
103104
}
104105
args := Args(cc)
105106

106-
// All vars should be in a single SetEnv directive (space-separated,
107-
// sorted by key), preceded by -o. Multiple -o SetEnv flags don't
108-
// stack in OpenSSH — only the first takes effect.
109-
want := "SetEnv=API_KEY=sk-secret GITHUB_TOKEN=ghp_abc123"
107+
// SendEnv should contain only key names (sorted), no values.
108+
// Values are read from the process environment by the SSH client.
109+
want := "SendEnv=API_KEY GITHUB_TOKEN"
110110
var found bool
111111
for i, a := range args {
112-
if strings.HasPrefix(a, "SetEnv=") {
112+
if strings.HasPrefix(a, "SendEnv=") {
113113
if i == 0 || args[i-1] != "-o" {
114-
t.Errorf("SetEnv arg %q not preceded by -o", a)
114+
t.Errorf("SendEnv arg %q not preceded by -o", a)
115115
}
116116
if a != want {
117-
t.Errorf("SetEnv = %q, want %q", a, want)
117+
t.Errorf("SendEnv = %q, want %q", a, want)
118118
}
119119
found = true
120120
}
121+
// Ensure no SetEnv (secrets in argv).
122+
if strings.HasPrefix(a, "SetEnv=") {
123+
t.Errorf("unexpected SetEnv arg %q — should use SendEnv", a)
124+
}
121125
}
122126
if !found {
123-
t.Error("SetEnv not found in args")
127+
t.Error("SendEnv not found in args")
124128
}
125129

126130
// user@host should still be last.
@@ -129,20 +133,20 @@ func TestSSHArgs(t *testing.T) {
129133
}
130134
})
131135

132-
t.Run("nil env produces no SetEnv", func(t *testing.T) {
136+
t.Run("nil env produces no SendEnv", func(t *testing.T) {
133137
args := Args(ConnConfig{Host: "10.0.0.1", User: "pixel"})
134138
for _, a := range args {
135-
if strings.HasPrefix(a, "SetEnv=") {
136-
t.Errorf("unexpected SetEnv arg %q with nil env", a)
139+
if strings.HasPrefix(a, "SendEnv=") {
140+
t.Errorf("unexpected SendEnv arg %q with nil env", a)
137141
}
138142
}
139143
})
140144

141-
t.Run("empty env produces no SetEnv", func(t *testing.T) {
145+
t.Run("empty env produces no SendEnv", func(t *testing.T) {
142146
args := Args(ConnConfig{Host: "10.0.0.1", User: "pixel", Env: map[string]string{}})
143147
for _, a := range args {
144-
if strings.HasPrefix(a, "SetEnv=") {
145-
t.Errorf("unexpected SetEnv arg %q with empty env", a)
148+
if strings.HasPrefix(a, "SendEnv=") {
149+
t.Errorf("unexpected SendEnv arg %q with empty env", a)
146150
}
147151
}
148152
})
@@ -253,15 +257,18 @@ func TestConsoleArgs(t *testing.T) {
253257
}
254258
args := consoleArgs(cc, "zmx attach build")
255259

256-
// Verify SetEnv is present.
257-
var foundSetEnv bool
260+
// Verify SendEnv is present (not SetEnv).
261+
var foundSendEnv bool
258262
for _, a := range args {
263+
if strings.HasPrefix(a, "SendEnv=") {
264+
foundSendEnv = true
265+
}
259266
if strings.HasPrefix(a, "SetEnv=") {
260-
foundSetEnv = true
267+
t.Errorf("unexpected SetEnv arg %q — should use SendEnv", a)
261268
}
262269
}
263-
if !foundSetEnv {
264-
t.Error("SetEnv not found in args")
270+
if !foundSendEnv {
271+
t.Error("SendEnv not found in args")
265272
}
266273

267274
// Verify -t and command at end.
@@ -277,3 +284,48 @@ func TestConsoleArgs(t *testing.T) {
277284
}
278285
})
279286
}
287+
288+
func TestEnvWithOverrides(t *testing.T) {
289+
base := []string{"HOME=/home/user", "PATH=/usr/bin", "EXISTING=old"}
290+
291+
t.Run("overrides existing and adds new", func(t *testing.T) {
292+
result := EnvWithOverrides(base, map[string]string{
293+
"EXISTING": "new",
294+
"NEW_VAR": "value",
295+
})
296+
297+
var foundExisting, foundNew bool
298+
for _, e := range result {
299+
if e == "EXISTING=new" {
300+
foundExisting = true
301+
}
302+
if e == "NEW_VAR=value" {
303+
foundNew = true
304+
}
305+
if e == "EXISTING=old" {
306+
t.Error("old value should be overridden")
307+
}
308+
}
309+
if !foundExisting {
310+
t.Error("EXISTING not overridden")
311+
}
312+
if !foundNew {
313+
t.Error("NEW_VAR not added")
314+
}
315+
})
316+
317+
t.Run("does not mutate base", func(t *testing.T) {
318+
orig := slices.Clone(base)
319+
_ = EnvWithOverrides(base, map[string]string{"EXISTING": "new"})
320+
if !slices.Equal(base, orig) {
321+
t.Error("base slice was mutated")
322+
}
323+
})
324+
325+
t.Run("nil overrides returns copy", func(t *testing.T) {
326+
result := EnvWithOverrides(base, nil)
327+
if !slices.Equal(result, base) {
328+
t.Errorf("result = %v, want %v", result, base)
329+
}
330+
})
331+
}

sandbox/incus/backend.go

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,7 @@ func (i *Incus) provisionInstance(ctx context.Context, full string) error {
166166
i.mkdir(full, "/home/pixel", 0o755)
167167
i.mkdir(full, "/home/pixel/.ssh", 0o700)
168168
i.mkdir(full, "/etc/systemd/resolved.conf.d", 0o755)
169-
i.mkdir(full, "/etc/ssh/sshd_config.d", 0o755)
169+
170170
i.mkdir(full, "/etc/profile.d", 0o755)
171171
i.mkdir(full, "/etc/sudoers.d", 0o755)
172172

@@ -181,11 +181,6 @@ func (i *Incus) provisionInstance(ctx context.Context, full string) error {
181181
}
182182
}
183183

184-
// sshd AcceptEnv config.
185-
if err := i.pushFile(full, "/etc/ssh/sshd_config.d/pixels.conf", []byte("AcceptEnv *\n"), 0o644); err != nil {
186-
return fmt.Errorf("writing sshd config: %w", err)
187-
}
188-
189184
// Shell profile.
190185
if err := i.pushFile(full, "/etc/profile.d/pixels.sh", []byte(scripts.PixelsProfile), 0o644); err != nil {
191186
return fmt.Errorf("writing profile: %w", err)

sandbox/incus/config.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,7 @@ type incusCfg struct {
3838
allow []string
3939
dns []string
4040

41-
env map[string]string
42-
envForward map[string]string
41+
env map[string]string
4342
}
4443

4544
// parseCfg extracts an incusCfg from a flat key-value map.

sandbox/truenas/backend.go

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -94,12 +94,13 @@ func (t *TrueNAS) Create(ctx context.Context, opts sandbox.CreateOpts) (*sandbox
9494
steps := provision.Steps(t.cfg.egress, t.cfg.devtools)
9595

9696
provOpts := ProvisionOpts{
97-
SSHPubKey: pubKey,
98-
DNS: t.cfg.dns,
99-
Env: t.cfg.env,
100-
DevTools: t.cfg.devtools,
101-
Egress: t.cfg.egress,
102-
EgressAllow: t.cfg.allow,
97+
SSHPubKey: pubKey,
98+
DNS: t.cfg.dns,
99+
Env: t.cfg.env,
100+
EnvForwardKeys: t.cfg.envForwardKeys,
101+
DevTools: t.cfg.devtools,
102+
Egress: t.cfg.egress,
103+
EgressAllow: t.cfg.allow,
103104
}
104105
if len(steps) > 0 {
105106
provOpts.ProvisionScript = provision.Script(steps)

sandbox/truenas/client.go

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,7 @@ type ProvisionOpts struct {
9393
SSHPubKey string
9494
DNS []string // nameservers (e.g. ["1.1.1.1", "8.8.8.8"])
9595
Env map[string]string // environment variables to inject into /etc/environment
96+
EnvForwardKeys []string // env var names for sshd AcceptEnv restriction
9697
DevTools bool // whether to install dev tools (mise, claude-code, codex, opencode)
9798
Egress string // "unrestricted", "agent", or "allowlist"
9899
EgressAllow []string // custom domains (merged into agent, standalone for allowlist)
@@ -137,15 +138,21 @@ func (c *Client) Provision(ctx context.Context, name string, opts ProvisionOpts)
137138
logf("Wrote DNS config (%d nameservers)", len(opts.DNS))
138139
}
139140

140-
// Configure sshd to accept forwarded env vars via SSH SetEnv.
141+
// Configure sshd to accept only the specific forwarded env var names.
141142
sshdDropin := rootfs + "/etc/ssh/sshd_config.d/pixels.conf"
143+
var sshdContent string
144+
if len(opts.EnvForwardKeys) > 0 {
145+
sshdContent = "AcceptEnv " + strings.Join(opts.EnvForwardKeys, " ") + "\n"
146+
} else {
147+
sshdContent = "# No env forwarding configured\n"
148+
}
142149
if err := c.Filesystem.WriteFile(ctx, sshdDropin, truenas.WriteFileParams{
143-
Content: []byte("AcceptEnv *\n"),
150+
Content: []byte(sshdContent),
144151
Mode: 0o644,
145152
}); err != nil {
146153
return fmt.Errorf("writing sshd drop-in: %w", err)
147154
}
148-
logf("Wrote sshd AcceptEnv config")
155+
logf("Wrote sshd config")
149156

150157
// Shell alias for detaching zmx sessions.
151158
if err := c.Filesystem.WriteFile(ctx, rootfs+"/etc/profile.d/pixels.sh", truenas.WriteFileParams{

0 commit comments

Comments
 (0)