fix(lxc): lxc tty

src/Cargo.toml

@@ -63,7 +63,7 @@ tokio = { version = "1", features = ["full"] }
 uuid = { version = "1", features = ["v4"] }
 getrandom = "0.2"
 libc = "0.2"
-nix = { version = "0.29", features = ["mount", "sched", "signal", "net", "process", "user"] }
+nix = { version = "0.29", features = ["mount", "sched", "signal", "net", "process", "user", "term"] }
 lxc_common = { path = "lxc_common" }
 wslc_common = { path = "wslc_common" }
 isolation_session_bindings = { path = "isolation_session_bindings" }

src/lxc/src/main.rs

@@ -12,6 +12,7 @@ use wxc_common::models::{CodexRequest, ContainmentBackend, ScriptResponse};
 use wxc_common::script_runner::{handle_dry_run_exit, ScriptRunner};
 
 use lxc_common::lxc_runner::LxcScriptRunner;
+use lxc_common::signal_cleanup;
 #[cfg(all(feature = "hyperlight", target_arch = "x86_64"))]
 use wxc_common::hyperlight_runner::HyperlightScriptRunner;
 
@@ -107,6 +108,16 @@ fn delete_lxc_container(name: &str, logger: &mut Logger) -> bool {
 }
 
 fn main() {
+    // Install before spawning any other threads so the signal mask propagates.
+    // Failure here is fatal: install() either succeeds with the watchdog
+    // running, or restores the original signal mask and returns Err. We
+    // refuse to continue without it because containers leaked on SIGTERM/INT
+    // are exactly the failure mode this code exists to prevent.
+    if let Err(e) = signal_cleanup::install() {
+        eprintln!("Error: failed to install signal cleanup handler: {}", e);
+        process::exit(1);
+    }
+
     let cli = Cli::parse();
 
     // --setup-hyperlight: eagerly warm up the snapshot and exit. Runs

src/lxc_common/src/lib.rs

@@ -5,3 +5,4 @@ pub mod filesystem_mounts;
 pub mod lxc_bindings;
 pub mod lxc_runner;
 pub mod network_iptables;
+pub mod signal_cleanup;

src/lxc_common/src/lxc_bindings.rs

@@ -51,15 +51,16 @@ where
 /// See [`resolve_lxcpath_with_env`] for the exact algorithm. This wrapper
 /// reads the real environment and effective UID.
 pub fn resolve_default_lxcpath() -> String {
-    // `geteuid` only exists on Unix; on other targets the function is never
-    // invoked in production (lxc-exec is Linux-only) but the crate still
-    // needs to compile workspace-wide, so fall back to a non-root EUID.
-    #[cfg(unix)]
+    // lxc-exec is Linux-only at runtime, but the crate has to compile
+    // workspace-wide (clippy runs on windows-latest, and macOS dev builds
+    // pull lxc_common in transitively). On non-Linux targets the function
+    // is never invoked in production, so fall back to a non-root EUID.
+    #[cfg(target_os = "linux")]
     // SAFETY: `geteuid` is a thread-safe, side-effect-free libc call.
     fn current_euid() -> u32 {
         unsafe { libc::geteuid() as u32 }
     }
-    #[cfg(not(unix))]
+    #[cfg(not(target_os = "linux"))]
     fn current_euid() -> u32 {
         1
     }
@@ -207,24 +208,185 @@ impl LxcContainer {
         ))
     }
 
-    /// Execute a command inside a running container using lxc-attach.
+    /// Execute a command inside a running container using lxc-attach, with
+    /// the inner process attached to a freshly-allocated pty.
+    ///
+    /// This bridges the host's stdin/stdout/stderr to the inner pty, but
+    /// **defers forwarding host stdin to the inner process until bash has
+    /// produced its first output byte**. That delay is essential: an
+    /// interactive shell calls `tcsetattr` on its stdin during readline
+    /// init, which can flush any bytes the parent buffered into the pty
+    /// before the shell got there. Without the delay, anything the CLI
+    /// pre-buffers (e.g. its shell-init wrapper) is silently swallowed.
+    ///
+    /// Stdout/stderr are streamed live via the master fd; the returned
+    /// strings are always empty. Callers needing captured output should run
+    /// a self-contained `commandLine` and read it back from a file.
+    ///
+    /// Only built on Linux — the implementation depends on `pre_exec`,
+    /// `openpty`, and `TIOCSCTTY`. The crate still has to compile
+    /// workspace-wide on Windows (the `wxc-exec-lint` CI job runs
+    /// `cargo clippy --workspace` on `windows-latest`) and on macOS dev
+    /// machines, so a stub is provided below for non-Linux targets.
+    #[cfg(target_os = "linux")]
     pub fn attach_run(
         &self,
         command: &str,
         _working_directory: &str,
     ) -> Result<(i32, String, String), String> {
+        use nix::pty::openpty;
+        use std::io::{Read, Write};
+        use std::process::Stdio;
+        use std::sync::mpsc;
+        use std::thread;
+        use std::time::Duration;
+
+        // Allocate an inner pty pair. The slave goes to lxc-attach (and thus
+        // bash inside the container) so the inner process sees a real tty;
+        // we keep the master and bridge it to our own stdio.
+        let pty_pair = openpty(None, None).map_err(|e| format!("openpty failed: {}", e))?;
+
+        // Three fd duplicates of the slave so each Stdio takes ownership of
+        // its own handle; otherwise std::process::Stdio::from would consume
+        // the single OwnedFd and the rest of the spawn calls would fail.
+        let slave_in: Stdio = pty_pair
+            .slave
+            .try_clone()
+            .map_err(|e| format!("dup slave for stdin: {}", e))?
+            .into();
+        let slave_out: Stdio = pty_pair
+            .slave
+            .try_clone()
+            .map_err(|e| format!("dup slave for stdout: {}", e))?
+            .into();
+        let slave_err: Stdio = pty_pair.slave.into();
+
         let mut cmd = self.lxc_command("lxc-attach");
         cmd.args(["--", "/bin/sh", "-c", command]);
+        cmd.stdin(slave_in).stdout(slave_out).stderr(slave_err);
+
+        // Drop the inherited controlling terminal in the child and make the
+        // slave end of our pty its new controlling tty. Without this,
+        // lxc-attach detects that it has a controlling tty (the outer pty
+        // from node-pty) and forwards the inner pty's I/O to `/dev/tty`
+        // directly, bypassing the slave fds we wired into stdio. Our master
+        // would then see no data at all.
+        //
+        // Also unblock SIGHUP/SIGTERM/SIGINT in the child: `signal_cleanup::install`
+        // blocks them in this process so the watchdog thread can sigwait on
+        // them, but that mask is inherited across fork+exec. Without this
+        // reset the inner shell (and anything it spawns) would silently
+        // ignore Ctrl-C and termination signals.
+        unsafe {
+            use std::os::unix::process::CommandExt;
+            cmd.pre_exec(|| {
+                // Become a new session leader, detaching from the inherited
+                // controlling terminal.
+                nix::unistd::setsid().map_err(std::io::Error::from)?;
+                // SAFETY: ioctl on fd 0 (the slave we just dup2'd in via
+                // stdin) to make it the new controlling tty. Errors are
+                // non-fatal because setsid above already cleared the ctty
+                // state, which is what actually matters for lxc-attach.
+                let _ = libc::ioctl(0, libc::TIOCSCTTY as _, 0);
+
+                // Restore the default signal mask so the inner process
+                // doesn't inherit signal_cleanup's blocked set.
+                let mut empty = nix::sys::signal::SigSet::empty();
+                empty.add(nix::sys::signal::Signal::SIGHUP);
+                empty.add(nix::sys::signal::Signal::SIGTERM);
+                empty.add(nix::sys::signal::Signal::SIGINT);
+                empty.thread_unblock().map_err(std::io::Error::from)?;
+                Ok(())
+            });
+        }
 
-        let output = cmd
-            .output()
-            .map_err(|e| format!("Failed to run lxc-attach: {}", e))?;
+        let mut child = cmd
+            .spawn()
+            .map_err(|e| format!("Failed to spawn lxc-attach: {}", e))?;
+
+        drop(cmd);
+
+        // The child inherited all three slave handles and the parent's
+        // copies have been moved into Stdio. The slave will be fully closed
+        // when the child exits, which makes our master read return EOF.
+
+        // OwnedFd -> File via std::convert::From; no `unsafe` needed.
+        let master: std::fs::File = pty_pair.master.into();
+        let mut master_writer = master
+            .try_clone()
+            .map_err(|e| format!("dup master: {}", e))?;
+        let mut master_reader = master;
+
+        // Output forwarder: master -> host stdout. Signals "ready" on the
+        // first byte from inside the container — at that point bash has
+        // finished its readline/tcsetattr init and is safe to feed.
+        let (ready_tx, ready_rx) = mpsc::channel::<()>();
+        let output_thread = thread::spawn(move || {
+            let mut buf = [0u8; 4096];
+            let mut signaled = false;
+            let mut stdout = std::io::stdout();
+            loop {
+                match master_reader.read(&mut buf) {
+                    Ok(0) => break,
+                    Ok(n) => {
+                        if !signaled {
+                            let _ = ready_tx.send(());
+                            signaled = true;
+                        }
+                        let _ = stdout.write_all(&buf[..n]);
+                        let _ = stdout.flush();
+                    }
+                    Err(_) => break,
+                }
+            }
+        });
+
+        // Wait for bash to print its first byte before forwarding host stdin.
+        // Cap the wait so a wedged shell doesn't block the stdin forwarder
+        // forever; the inner process itself still runs to completion below
+        // (request.script_timeout is not yet enforced — see the TODO in
+        // lxc_runner::run_internal where attach_run is called).
+        let _ = ready_rx.recv_timeout(Duration::from_secs(5));
+
+        // Input forwarder: host stdin -> master. Detached; exits when stdin
+        // closes (which happens when our parent process closes the pty).
+        thread::spawn(move || {
+            let mut buf = [0u8; 4096];
+            let mut stdin = std::io::stdin();
+            loop {
+                match stdin.read(&mut buf) {
+                    Ok(0) => break,
+                    Ok(n) => {
+                        if master_writer.write_all(&buf[..n]).is_err() {
+                            break;
+                        }
+                    }
+                    Err(_) => break,
+                }
+            }
+        });
 
-        Ok((
-            output.status.code().unwrap_or(-1),
-            String::from_utf8_lossy(&output.stdout).to_string(),
-            String::from_utf8_lossy(&output.stderr).to_string(),
-        ))
+        let status = child
+            .wait()
+            .map_err(|e| format!("wait on lxc-attach: {}", e))?;
+
+        // Drain remaining output before returning. The slave fds are closed
+        // on child exit, so master_reader will hit EOF and the thread exits.
+        let _ = output_thread.join();
+
+        Ok((status.code().unwrap_or(-1), String::new(), String::new()))
+    }
+
+    /// Non-Linux stub. `lxc-exec` is Linux-only at runtime, but the
+    /// workspace still builds on Windows (clippy CI) and macOS (dev), so
+    /// the signature has to exist on every target.
+    #[cfg(not(target_os = "linux"))]
+    pub fn attach_run(
+        &self,
+        _command: &str,
+        _working_directory: &str,
+    ) -> Result<(i32, String, String), String> {
+        Err("LxcContainer::attach_run is only supported on Linux".to_string())
     }
 
     /// Stop the container.
@@ -233,10 +395,14 @@ impl LxcContainer {
     }
 
     /// Destroy the container (removes rootfs and config).
+    ///
+    /// `lxc-destroy -f` already force-stops a running container; we used to
+    /// call `lxc-stop` first, but plain `lxc-stop` waits up to 60 s for a
+    /// graceful shutdown — fatal for distros with systemd as PID 1 in
+    /// unprivileged userns where init never cleanly responds to SIGPWR.
+    /// Forcing the stop via destroy keeps this fast for both alpine and
+    /// ubuntu-class images.
     pub fn destroy(&self) -> Result<(), String> {
-        if self.is_running() {
-            let _ = self.stop();
-        }
         let mut cmd = self.lxc_command("lxc-destroy");
         cmd.arg("-f");
         Self::run_status(cmd, "lxc-destroy")

src/lxc_common/src/lxc_runner.rs

@@ -18,6 +18,7 @@ use wxc_common::script_runner::ScriptRunner;
 use crate::filesystem_mounts;
 use crate::lxc_bindings::LxcContainer;
 use crate::network_iptables::NetworkIptablesManager;
+use crate::signal_cleanup;
 
 /// Script runner that executes commands inside an LXC container.
 pub struct LxcScriptRunner {
@@ -97,6 +98,14 @@ impl LxcScriptRunner {
         }
 
         let container_name = self.resolve_container_name();
+        // Make the name visible to the signal-cleanup watchdog so a fatal
+        // signal during create/start/attach still tears the container down —
+        // but only when the caller actually wants the container destroyed at
+        // exit. With `destroyOnExit = false` the normal completion path
+        // preserves the container, so the signal path must too.
+        if self.destroy_on_exit {
+            signal_cleanup::set_active(&container_name);
+        }
         let _ = writeln!(logger, "Container name: {}", container_name);
         let _ = writeln!(
             logger,
@@ -174,6 +183,11 @@ impl LxcScriptRunner {
         if let Some(veth) = NetworkIptablesManager::discover_veth_interface(&container_name) {
             let _ = writeln!(logger, "Discovered veth interface: {}", veth);
             fw_manager.set_veth_interface(&veth);
+            if self.destroy_on_exit {
+                // Tell the watchdog about the veth so signal-time cleanup
+                // can also remove the FORWARD hook, not just the chain.
+                signal_cleanup::set_active_veth(&veth);
+            }
         }
 
         match fw_manager.apply_firewall_rules(&request.policy, logger) {

src/lxc_common/src/network_iptables.rs

@@ -256,6 +256,24 @@ impl NetworkIptablesManager {
         self.rules_applied = false;
         Ok(())
     }
+
+    /// Best-effort cleanup of any iptables state the runner may have
+    /// installed for a container, used when the original
+    /// `NetworkIptablesManager` instance isn't reachable (e.g. signal-time
+    /// cleanup from the watchdog thread). Builds a fresh manager pointed at
+    /// the same chain name so `remove_firewall_rules` does its work
+    /// regardless of whether rules were actually installed; iptables itself
+    /// is the source of truth.
+    pub fn force_cleanup(container_name: &str, veth_interface: Option<&str>, logger: &mut Logger) {
+        let mut mgr = Self::new(container_name);
+        if let Some(v) = veth_interface {
+            mgr.set_veth_interface(v);
+        }
+        // Bypass the rules_applied gate; if there's nothing to remove the
+        // iptables `-D`/`-F`/`-X` calls just no-op.
+        mgr.rules_applied = true;
+        let _ = mgr.remove_firewall_rules(logger);
+    }
 }
 
 impl Drop for NetworkIptablesManager {