Bound every guest transport against an unresponsive guest

CHANGELOG.md

@@ -83,6 +83,14 @@
 
 ### Fixes
 
+- **Guest transports fail instead of hanging when a VM stops responding** — A
+  paused VM, a wedged sshd, or a lost TAP device left `ssh`, `scp`, and `rsync`
+  calls blocked on a dead socket with no deadline, so lifecycle commands,
+  `coop exec`, and `coop push`/`pull` hung until interrupted. Every transport
+  now derives from one option list that sets `BatchMode`, a connect timeout,
+  and a liveness probe, so a guest whose sshd stops answering fails after ~90s
+  — the bound interactive sessions already had.
+
 - **Install Codex's complete runtime package** (#442) — Recent Codex releases
   use a companion `codex-code-mode-host` executable, but coop installed only
   the raw `codex` binary, causing Code Mode to fail closed at startup. Image

docs/trust-model.md

@@ -132,7 +132,11 @@ user `env_forward` entries, and the VM SSH key. The invariants:
 
 - coop connects to the guest with `StrictHostKeyChecking=no`,
   `UserKnownHostsFile=/dev/null`, `IdentitiesOnly=yes`
-  (`backend.rs:SshTarget::ssh_opts`, `workspace.rs:ssh_config_block`). This is
+  (`backend.rs:SshTarget::transport_opts` — the one list `ssh`, `scp`, and
+  rsync's `-e` all derive from — and `workspace.rs:ssh_config_block`). coop's
+  own transports add `BatchMode=yes`, so a rejected key fails instead of
+  falling back to a password prompt; the `~/.ssh/config` block written for the
+  user's own `ssh coop-<name>` deliberately does not. This is
   deliberate: guest keys are ephemeral and regenerated per VM, so there is no
   stable host key to pin. The trade-off is that a MITM on the path to the guest
   is not detected — acceptable because that path is loopback / a local TAP link

src/backend.rs

@@ -337,9 +337,31 @@ impl SshTarget {
         dir.join(format!("coop-{short:08x}.sock"))
     }
 
-    /// SSH options for commands.
-    pub fn ssh_opts(&self) -> Vec<String> {
+    /// Options every transport shares, up to the port flag.
+    ///
+    /// `ssh`, `scp`, and rsync's `-e` command all take these; only the port
+    /// flag differs (`-p` vs `-P`), so all three derive from here instead of
+    /// hand-copying a list that then drifts.
+    ///
+    /// The bounds, in the order they take effect. `ConnectTimeout` caps the
+    /// TCP connect and banner exchange. `BatchMode` refuses password and
+    /// passphrase prompts, so a key the guest rejects fails instead of
+    /// blocking on stdin where no interactive user exists (`coop up` in CI).
+    /// `ServerAlive*` then covers the established session: a paused VM, a
+    /// wedged sshd, or a lost TAP device otherwise leaves SSH blocked on a
+    /// dead socket with no deadline. It is answered by sshd itself, not by
+    /// the remote command, so a silent hour-long install is never at risk —
+    /// only a guest whose sshd cannot answer for 90s.
+    fn transport_opts(&self) -> Vec<String> {
         vec![
+            "-o".into(),
+            "BatchMode=yes".into(),
+            "-o".into(),
+            "ConnectTimeout=10".into(),
+            "-o".into(),
+            "ServerAliveInterval=30".into(),
+            "-o".into(),
+            "ServerAliveCountMax=3".into(),
             "-o".into(),
             "StrictHostKeyChecking=no".into(),
             "-o".into(),
@@ -350,11 +372,16 @@ impl SshTarget {
             "LogLevel=ERROR".into(),
             "-i".into(),
             self.key_path.display().to_string(),
-            "-p".into(),
-            self.port.to_string(),
         ]
     }
 
+    /// SSH options for commands.
+    pub fn ssh_opts(&self) -> Vec<String> {
+        let mut opts = self.transport_opts();
+        opts.extend(["-p".into(), self.port.to_string()]);
+        opts
+    }
+
     /// SSH options with connection multiplexing.
     ///
     /// Only used during boot probing (`wait_until_ready`) where rapid
@@ -376,21 +403,10 @@ impl SshTarget {
 
     /// SCP options (uses -P for port instead of -p).
     pub fn scp_opts(&self) -> Vec<String> {
-        vec![
-            "-q".into(),
-            "-o".into(),
-            "StrictHostKeyChecking=no".into(),
-            "-o".into(),
-            "UserKnownHostsFile=/dev/null".into(),
-            "-o".into(),
-            "IdentitiesOnly=yes".into(),
-            "-o".into(),
-            "LogLevel=ERROR".into(),
-            "-i".into(),
-            self.key_path.display().to_string(),
-            "-P".into(),
-            self.port.to_string(),
-        ]
+        let mut opts = vec!["-q".to_string()];
+        opts.extend(self.transport_opts());
+        opts.extend(["-P".into(), self.port.to_string()]);
+        opts
     }
 
     /// user@host address string.
@@ -529,13 +545,12 @@ impl SshTarget {
     }
 
     /// SSH command string for rsync's -e flag.
+    ///
+    /// Derived from [`Self::ssh_opts`] so a transfer inherits the same bounds
+    /// as any other guest command. rsync splits this string on whitespace, so
+    /// it stays unquoted — a key path containing spaces has never worked here.
     pub fn rsync_ssh_cmd(&self) -> String {
-        format!(
-            "ssh -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null \
-             -o IdentitiesOnly=yes -o LogLevel=ERROR -i {} -p {}",
-            self.key_path.display(),
-            self.port,
-        )
+        format!("ssh {}", self.ssh_opts().join(" "))
     }
 
     /// Run a command on the guest via SSH and capture stdout.
@@ -3267,6 +3282,61 @@ Filesystem     1M-blocks  Used Available Use% Mounted on
 /dev/vda1          20480  3200     16000  17% /
 ";
 
+    fn ssh_test_target() -> SshTarget {
+        SshTarget {
+            host: Hostname::new("192.0.2.1").unwrap(),
+            port: NonZeroU16::new(22).unwrap(),
+            user: SshUser::new("ubuntu").unwrap(),
+            key_path: PathBuf::from("/tmp/test-key"),
+        }
+    }
+
+    #[test]
+    fn every_transport_is_bounded_against_a_wedged_guest() {
+        let target = ssh_test_target();
+        let ssh = target.ssh_opts();
+        let scp = target.scp_opts();
+        let rsync = target.rsync_ssh_cmd();
+
+        for bound in [
+            "BatchMode=yes",
+            "ConnectTimeout=10",
+            "ServerAliveInterval=30",
+            "ServerAliveCountMax=3",
+        ] {
+            assert!(ssh.contains(&bound.to_string()), "{bound} missing from ssh");
+            assert!(scp.contains(&bound.to_string()), "{bound} missing from scp");
+            assert!(rsync.contains(bound), "{bound} missing from rsync -e");
+        }
+
+        // Exactly one pair per transport: OpenSSH honors the first value of a
+        // repeated `-o`, so a caller that appends its own `ServerAlive*` after
+        // these would be silently ignored rather than tightening the bound.
+        for (name, opts) in [
+            ("ssh", ssh),
+            ("scp", scp),
+            ("mux", target.ssh_opts_mux()),
+            ("rsync", rsync.split(' ').map(str::to_string).collect()),
+        ] {
+            let pairs = opts.iter().filter(|o| o.starts_with("ServerAlive")).count();
+            assert_eq!(pairs, 2, "{name} must carry one ServerAlive* pair");
+        }
+    }
+
+    #[test]
+    fn ssh_and_scp_options_differ_only_in_the_port_flag() {
+        // Two hand-copied lists is how the last hardening change reached only
+        // one of them; they now derive from `transport_opts`.
+        let target = ssh_test_target();
+        let ssh = target.ssh_opts();
+        let scp = target.scp_opts();
+
+        assert_eq!(scp.first().map(String::as_str), Some("-q"));
+        assert_eq!(scp[1..scp.len() - 2], ssh[..ssh.len() - 2]);
+        assert!(scp.ends_with(&["-P".to_string(), "22".to_string()]));
+        assert!(ssh.ends_with(&["-p".to_string(), "22".to_string()]));
+    }
+
     #[test]
     fn boot_preflight_fails_on_missing_config_dir() {
         // The boot choke point must reject a config whose custom

src/port_forward.rs

@@ -172,8 +172,8 @@ pub fn spawn_ssh_forwards(
     // ExitOnForwardFailure=yes: refuse to background if a -L fails to
     //     bind — turns races with `check_host_port_collisions` into
     //     loud errors rather than silently lost forwards.
-    // ServerAliveInterval=30: detect dead VMs so the forwarder doesn't
-    //     hang around eating descriptors after a hard reboot.
+    // The `ServerAlive*` bound that keeps this tunnel from outliving a dead
+    // VM comes from `SshTarget::transport_opts` via `ssh_opts` above.
     args.extend([
         "-f".into(),
         "-N".into(),
@@ -186,8 +186,6 @@ pub fn spawn_ssh_forwards(
         "ControlPersist=yes".into(),
         "-o".into(),
         "ExitOnForwardFailure=yes".into(),
-        "-o".into(),
-        "ServerAliveInterval=30".into(),
     ]);
 
     let mut spec_log: Vec<String> = Vec::new();

src/proxy.rs

@@ -379,8 +379,6 @@ fn spawn_reverse_forward(inst: &Instance, name: &str, target: &SshTarget, port:
         "-T".into(),
         "-o".into(),
         "ExitOnForwardFailure=yes".into(),
-        "-o".into(),
-        "ServerAliveInterval=30".into(),
         "-R".into(),
         format!("127.0.0.1:{port}:127.0.0.1:{port}"),
     ]);

src/ssh.rs

@@ -41,22 +41,6 @@ fn guest_term() -> String {
     }
 }
 
-/// Keepalive options for interactive sessions.
-///
-/// A paused/suspended VM or a dropped local connection leaves SSH
-/// blocked on a dead socket indefinitely. With these, SSH sends a probe
-/// every 30s and gives up after 3 unanswered probes — so an unreachable
-/// session terminates within ~90s instead of hanging. Scoped to
-/// interactive use; the short-lived non-interactive paths don't need it.
-fn keepalive_opts() -> [String; 4] {
-    [
-        "-o".into(),
-        "ServerAliveInterval=30".into(),
-        "-o".into(),
-        "ServerAliveCountMax=3".into(),
-    ]
-}
-
 /// Force a known OpenSSH escape character for emergency disconnects.
 ///
 /// OpenSSH only recognizes the escape at the start of a line, so users
@@ -68,7 +52,6 @@ fn escape_opts() -> [String; 2] {
 
 fn interactive_ssh_args(session: &SshSession, remote_cmd: String) -> Vec<String> {
     let mut args = session.ssh_opts();
-    args.extend(keepalive_opts());
     args.extend(escape_opts());
     args.extend(["-t".to_string(), session.target.addr(), remote_cmd]);
     args
@@ -201,19 +184,6 @@ mod tests {
         assert_eq!(render_remote(&cmd), "cd /workspace && 'echo' 'hi'");
     }
 
-    #[test]
-    fn keepalive_opts_set_interval_and_count() {
-        assert_eq!(
-            keepalive_opts(),
-            [
-                "-o",
-                "ServerAliveInterval=30",
-                "-o",
-                "ServerAliveCountMax=3",
-            ],
-        );
-    }
-
     #[test]
     fn escape_opts_force_tilde_escape() {
         assert_eq!(escape_opts(), ["-e", "~"]);
@@ -232,9 +202,20 @@ mod tests {
             env: crate::backend::EnvForward::default(),
         };
 
+        // The `ServerAlive*` pair comes from `SshTarget::transport_opts`, which
+        // every transport shares; this session must not add a second one, since
+        // OpenSSH honors the first value of a repeated `-o`.
         assert_eq!(
             interactive_ssh_args(&session, "cd /workspace && 'claude' 'agents'".into()),
             [
+                "-o",
+                "BatchMode=yes",
+                "-o",
+                "ConnectTimeout=10",
+                "-o",
+                "ServerAliveInterval=30",
+                "-o",
+                "ServerAliveCountMax=3",
                 "-o",
                 "StrictHostKeyChecking=no",
                 "-o",
@@ -247,10 +228,6 @@ mod tests {
                 "/tmp/coop-test-key",
                 "-p",
                 "1",
-                "-o",
-                "ServerAliveInterval=30",
-                "-o",
-                "ServerAliveCountMax=3",
                 "-e",
                 "~",
                 "-t",