remote: Disable ControlPersist for master ssh connection (#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
This commit is contained in:
Shish 2024-10-14 14:36:31 +01:00 committed by GitHub
parent 792f583b97
commit 7d5fe66b54
No known key found for this signature in database
GPG key ID: B5690EEEBB952194

View file

@ -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()?;