Skip to content

Commit 79a6f00

Browse files
theFongtheFong
committed
fix: address PR feedback - ControlPath separator and ssh-agent reuse
- Use `-` instead of `:` in ControlPath to avoid issues on filesystems that don't support colons (SMB mounts, WSL edge cases) - Only spawn ssh-agent if SSH_AUTH_SOCK is not already set, preventing orphaned agent processes on rapid repeated exec calls
1 parent 82c267f commit 79a6f00

4 files changed

Lines changed: 26 additions & 25 deletions

File tree

‎pkg/cmd/exec/exec.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -297,8 +297,9 @@ func runSSHWithTimeout(sshAlias string, command string, connectTimeoutSecs int)
297297
// Escape the command for passing to SSH
298298
escapedCmd := strings.ReplaceAll(command, "'", "'\\''")
299299
// -T disables pseudo-terminal allocation (no "Pseudo-terminal will not be allocated" warning)
300-
// ssh-agent stdout is suppressed to /dev/null to hide "Agent pid NNNNN"
301-
cmd := fmt.Sprintf("eval $(ssh-agent -s) > /dev/null && ssh -T -o ConnectTimeout=%d -o LogLevel=ERROR %s '%s'", connectTimeoutSecs, sshAlias, escapedCmd)
300+
// Only start ssh-agent if one isn't already running (avoids orphaned agent processes)
301+
agentCmd := `if [ -z "$SSH_AUTH_SOCK" ]; then eval $(ssh-agent -s) > /dev/null; fi`
302+
cmd := fmt.Sprintf("%s && ssh -T -o ConnectTimeout=%d -o LogLevel=ERROR %s '%s'", agentCmd, connectTimeoutSecs, sshAlias, escapedCmd)
302303

303304
sshCmd := exec.Command("bash", "-c", cmd) //nolint:gosec //cmd is user input
304305
sshCmd.Stderr = os.Stderr

‎pkg/ssh/ssh.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ const workspaceSSHConfigTemplate = `Host {{ .Host }}
3939
ForwardAgent yes
4040
AddKeysToAgent yes
4141
ControlMaster auto
42-
ControlPath ~/.ssh/brev-control-%r@%h:%p
42+
ControlPath ~/.ssh/brev-control-%r@%h-%p
4343
ControlPersist 10m
4444
RemoteCommand cd {{ .Dir }}; $SHELL
4545

‎pkg/ssh/sshconfigurer.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -234,7 +234,7 @@ const SSHConfigEntryTemplateV2 = `Host {{ .Alias }}
234234
ForwardAgent yes
235235
RequestTTY yes
236236
ControlMaster auto
237-
ControlPath ~/.ssh/brev-control-%r@%h:%p
237+
ControlPath ~/.ssh/brev-control-%r@%h-%p
238238
ControlPersist 10m
239239
{{ if .RunRemoteCMD }}
240240
RemoteCommand cd {{ .Dir }}; $SHELL
@@ -254,7 +254,7 @@ const SSHConfigEntryTemplateV3 = `Host {{ .Alias }}
254254
ForwardAgent yes
255255
RequestTTY yes
256256
ControlMaster auto
257-
ControlPath ~/.ssh/brev-control-%r@%h:%p
257+
ControlPath ~/.ssh/brev-control-%r@%h-%p
258258
ControlPersist 10m
259259
Port {{ .Port }}
260260
{{ if .RunRemoteCMD }}

‎pkg/ssh/sshconfigurer_test.go‎

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ Host %s
151151
ForwardAgent yes
152152
RequestTTY yes
153153
ControlMaster auto
154-
ControlPath ~/.ssh/brev-control-%%r@%%h:%%p
154+
ControlPath ~/.ssh/brev-control-%%r@%%h-%%p
155155
ControlPersist 10m
156156
Port 22
157157
@@ -168,7 +168,7 @@ Host %s-host
168168
ForwardAgent yes
169169
RequestTTY yes
170170
ControlMaster auto
171-
ControlPath ~/.ssh/brev-control-%%r@%%h:%%p
171+
ControlPath ~/.ssh/brev-control-%%r@%%h-%%p
172172
ControlPersist 10m
173173
Port 22
174174
@@ -185,7 +185,7 @@ Host %s
185185
ForwardAgent yes
186186
RequestTTY yes
187187
ControlMaster auto
188-
ControlPath ~/.ssh/brev-control-%%r@%%h:%%p
188+
ControlPath ~/.ssh/brev-control-%%r@%%h-%%p
189189
ControlPersist 10m
190190
Port 22
191191
@@ -202,7 +202,7 @@ Host %s-host
202202
ForwardAgent yes
203203
RequestTTY yes
204204
ControlMaster auto
205-
ControlPath ~/.ssh/brev-control-%%r@%%h:%%p
205+
ControlPath ~/.ssh/brev-control-%%r@%%h-%%p
206206
ControlPersist 10m
207207
Port 22
208208
@@ -323,7 +323,7 @@ func Test_makeSSHConfigEntryV2(t *testing.T) { //nolint:funlen // test
323323
ForwardAgent yes
324324
RequestTTY yes
325325
ControlMaster auto
326-
ControlPath ~/.ssh/brev-control-%r@%h:%p
326+
ControlPath ~/.ssh/brev-control-%r@%h-%p
327327
ControlPersist 10m
328328
Port 20
329329
@@ -340,7 +340,7 @@ Host testName2-host
340340
ForwardAgent yes
341341
RequestTTY yes
342342
ControlMaster auto
343-
ControlPath ~/.ssh/brev-control-%r@%h:%p
343+
ControlPath ~/.ssh/brev-control-%r@%h-%p
344344
ControlPersist 10m
345345
Port 2022
346346
@@ -378,7 +378,7 @@ Host testName2-host
378378
ForwardAgent yes
379379
RequestTTY yes
380380
ControlMaster auto
381-
ControlPath ~/.ssh/brev-control-%r@%h:%p
381+
ControlPath ~/.ssh/brev-control-%r@%h-%p
382382
ControlPersist 10m
383383
Port 22
384384
@@ -395,7 +395,7 @@ Host testName2-host
395395
ForwardAgent yes
396396
RequestTTY yes
397397
ControlMaster auto
398-
ControlPath ~/.ssh/brev-control-%r@%h:%p
398+
ControlPath ~/.ssh/brev-control-%r@%h-%p
399399
ControlPersist 10m
400400
Port 22
401401
@@ -434,7 +434,7 @@ Host testName2-host
434434
ForwardAgent yes
435435
RequestTTY yes
436436
ControlMaster auto
437-
ControlPath ~/.ssh/brev-control-%r@%h:%p
437+
ControlPath ~/.ssh/brev-control-%r@%h-%p
438438
ControlPersist 10m
439439
Port 2022
440440
@@ -451,7 +451,7 @@ Host testName2-host
451451
ForwardAgent yes
452452
RequestTTY yes
453453
ControlMaster auto
454-
ControlPath ~/.ssh/brev-control-%r@%h:%p
454+
ControlPath ~/.ssh/brev-control-%r@%h-%p
455455
ControlPersist 10m
456456
Port 22
457457
@@ -488,7 +488,7 @@ Host testName2-host
488488
ForwardAgent yes
489489
RequestTTY yes
490490
ControlMaster auto
491-
ControlPath ~/.ssh/brev-control-%r@%h:%p
491+
ControlPath ~/.ssh/brev-control-%r@%h-%p
492492
ControlPersist 10m
493493
494494
`,
@@ -524,7 +524,7 @@ Host testName2-host
524524
ForwardAgent yes
525525
RequestTTY yes
526526
ControlMaster auto
527-
ControlPath ~/.ssh/brev-control-%r@%h:%p
527+
ControlPath ~/.ssh/brev-control-%r@%h-%p
528528
ControlPersist 10m
529529
530530
`,
@@ -564,7 +564,7 @@ Host testName2-host
564564
ForwardAgent yes
565565
RequestTTY yes
566566
ControlMaster auto
567-
ControlPath ~/.ssh/brev-control-%r@%h:%p
567+
ControlPath ~/.ssh/brev-control-%r@%h-%p
568568
ControlPersist 10m
569569
570570
Host testName2-host
@@ -580,7 +580,7 @@ Host testName2-host
580580
ForwardAgent yes
581581
RequestTTY yes
582582
ControlMaster auto
583-
ControlPath ~/.ssh/brev-control-%r@%h:%p
583+
ControlPath ~/.ssh/brev-control-%r@%h-%p
584584
ControlPersist 10m
585585
586586
`,
@@ -699,7 +699,7 @@ Host testName1
699699
ForwardAgent yes
700700
RequestTTY yes
701701
ControlMaster auto
702-
ControlPath ~/.ssh/brev-control-%r@%h:%p
702+
ControlPath ~/.ssh/brev-control-%r@%h-%p
703703
ControlPersist 10m
704704
Port 22
705705
@@ -716,7 +716,7 @@ Host testName1-host
716716
ForwardAgent yes
717717
RequestTTY yes
718718
ControlMaster auto
719-
ControlPath ~/.ssh/brev-control-%r@%h:%p
719+
ControlPath ~/.ssh/brev-control-%r@%h-%p
720720
ControlPersist 10m
721721
Port 22
722722
@@ -765,7 +765,7 @@ Host testName1
765765
ForwardAgent yes
766766
RequestTTY yes
767767
ControlMaster auto
768-
ControlPath ~/.ssh/brev-control-%r@%h:%p
768+
ControlPath ~/.ssh/brev-control-%r@%h-%p
769769
ControlPersist 10m
770770
Port 22
771771
@@ -782,7 +782,7 @@ Host testName1-host
782782
ForwardAgent yes
783783
RequestTTY yes
784784
ControlMaster auto
785-
ControlPath ~/.ssh/brev-control-%r@%h:%p
785+
ControlPath ~/.ssh/brev-control-%r@%h-%p
786786
ControlPersist 10m
787787
Port 22
788788
@@ -802,7 +802,7 @@ Host testName1
802802
ForwardAgent yes
803803
RequestTTY yes
804804
ControlMaster auto
805-
ControlPath ~/.ssh/brev-control-%r@%h:%p
805+
ControlPath ~/.ssh/brev-control-%r@%h-%p
806806
ControlPersist 10m
807807
Port 22
808808
@@ -819,7 +819,7 @@ Host testName1-host
819819
ForwardAgent yes
820820
RequestTTY yes
821821
ControlMaster auto
822-
ControlPath ~/.ssh/brev-control-%r@%h:%p
822+
ControlPath ~/.ssh/brev-control-%r@%h-%p
823823
ControlPersist 10m
824824
Port 22
825825

0 commit comments

Comments
 (0)