Skip to content

Commit

Permalink
remote: Disable ControlPersist for master ssh connection (zed-industr…
Browse files Browse the repository at this point in the history
…ies#19193)

remote: Disable ControlPersist for master ssh connection

`ControlPersist=yes` combined with `ControlMaster=yes` silently forces
`ForkAfterAuthentication=yes` (even when the user has explicitly set it
to `no` - reported upstream in [0]) - and the latter makes the ssh
subprocess disappear, which makes us think that the connection died

(This is only an issue for people who have `ControlPersist=yes` in their
`ssh_config`, and perhaps the answer is "if that option breaks things,
don't use that option?" - but it's an option that makes sense _most_ of
the time, it's just in this edge-case of "creating an ssh connection
with -N and expecting the process to stay in the foreground" where it
_must_ be set to no)

I think the alternative approach is to tell people "if you want to use
persistent connections, have a separate ~/.ssh/config entry for
servername (to ssh into) and servername-no-persist (to zed into)", which
is possible, but ugh. Kind of a messy situation >.<


Tests:

- Before: Connections to my server result in "Failed to connect: ." (The
error message is attempting to show stderr, but stderr is empty)

- After: Connections to my server work reliably


[0] https://bugzilla.mindrot.org/show_bug.cgi?id=3743


Release Notes:

- N/A
  • Loading branch information
shish authored Oct 14, 2024
1 parent 792f583 commit 7d5fe66
Showing 1 changed file with 8 additions and 1 deletion.
9 changes: 8 additions & 1 deletion crates/remote/src/ssh_session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1144,7 +1144,14 @@ impl SshRemoteConnection {
.stderr(Stdio::piped())
.env("SSH_ASKPASS_REQUIRE", "force")
.env("SSH_ASKPASS", &askpass_script_path)
.args(["-N", "-o", "ControlMaster=yes", "-o"])
.args([
"-N",
"-o",
"ControlPersist=no",
"-o",
"ControlMaster=yes",
"-o",
])
.arg(format!("ControlPath={}", socket_path.display()))
.arg(&url)
.spawn()?;
Expand Down

0 comments on commit 7d5fe66

Please sign in to comment.