diff --git a/src/commands/init.rs b/src/commands/init.rs index 01cfcd7..fa26fc0 100644 --- a/src/commands/init.rs +++ b/src/commands/init.rs @@ -45,7 +45,7 @@ pub fn is_devproxy_process(pid: i32) -> bool { comm.trim() .rsplit('/') .next() - .map(|name| name == "devproxy") + .map(|name| name == "devproxy" || name == "devproxy-daemon") .unwrap_or(false) } _ => false, @@ -56,7 +56,7 @@ pub fn is_devproxy_process(pid: i32) -> bool { // On Linux, read /proc//exe symlink. let exe = std::fs::read_link(format!("/proc/{pid}/exe")); match exe { - Ok(path) => path.file_name().map(|n| n == "devproxy").unwrap_or(false), + Ok(path) => path.file_name().map(|n| n == "devproxy" || n == "devproxy-daemon").unwrap_or(false), Err(_) => false, } } @@ -315,6 +315,23 @@ fn spawn_daemon_directly(exe: &std::path::Path, port: u16, domain: &str) -> Resu Ok(()) } +/// Copy the current binary to the dedicated daemon binary path. +/// Creates the parent directory if needed, then delegates to +/// `update::prepare_binary` for permissions and codesigning. +pub fn install_daemon_binary(src: &std::path::Path, dest: &std::path::Path) -> Result<()> { + if let Some(parent) = dest.parent() { + std::fs::create_dir_all(parent) + .with_context(|| format!("could not create {}", parent.display()))?; + } + std::fs::copy(src, dest) + .with_context(|| format!("could not copy binary to {}", dest.display()))?; + + super::update::prepare_binary(dest)?; + + eprintln!("{} daemon binary installed at {}", "ok:".green(), dest.display()); + Ok(()) +} + pub fn run(domain: &str, port: u16, no_daemon: bool) -> Result<()> { validate_domain(domain)?; let config = Config { @@ -410,6 +427,12 @@ pub fn run(domain: &str, port: u16, no_daemon: bool) -> Result<()> { eprintln!("starting daemon on port {port}..."); let exe = std::env::current_exe().context("could not determine binary path")?; + // Copy the current binary to a dedicated daemon path so that + // launchd's KeepAlive monitoring of the daemon binary does not + // SIGKILL normal CLI invocations at the main binary path. + let daemon_bin = Config::daemon_binary_path()?; + install_daemon_binary(&exe, &daemon_bin)?; + // Try platform-specific socket activation (launchd on macOS, // systemd on Linux). The daemon receives pre-bound sockets from // the OS, so it runs as the current user — no sudo needed for @@ -421,7 +444,7 @@ pub fn run(domain: &str, port: u16, no_daemon: bool) -> Result<()> { // Pass DEVPROXY_CONFIG_DIR if set — the plist/unit file must // embed it so the daemon uses the correct config directory. let config_dir_override = std::env::var("DEVPROXY_CONFIG_DIR").ok(); - match crate::platform::install_daemon(&exe, port, config_dir_override.as_deref()) { + match crate::platform::install_daemon(&daemon_bin, port, config_dir_override.as_deref()) { Ok(()) => { // Wait for daemon to become responsive (socket activation // startup may be slower than direct spawn) @@ -468,7 +491,7 @@ pub fn run(domain: &str, port: u16, no_daemon: bool) -> Result<()> { } if !activated { - spawn_daemon_directly(&exe, port, domain)?; + spawn_daemon_directly(&daemon_bin, port, domain)?; } } diff --git a/src/commands/update.rs b/src/commands/update.rs index 342f9cc..69c214b 100644 --- a/src/commands/update.rs +++ b/src/commands/update.rs @@ -136,12 +136,13 @@ fn download_file(url: &str, dest: &Path) -> Result<()> { } /// Set permissions and codesign the binary at `path`. -fn prepare_binary(path: &Path) -> Result<()> { +/// Used by both update (for downloaded binaries) and init (for the daemon binary copy). +pub(crate) fn prepare_binary(path: &Path) -> Result<()> { #[cfg(unix)] { use std::os::unix::fs::PermissionsExt; std::fs::set_permissions(path, std::fs::Permissions::from_mode(0o755)) - .context("failed to set permissions on downloaded binary")?; + .context("failed to set binary permissions")?; } #[cfg(target_os = "macos")] @@ -320,6 +321,13 @@ fn do_update(download_url: &str, exe_path: &Path, tmpfile: &Path) -> Result<()> replace_binary(tmpfile, exe_path)?; + // Also update the dedicated daemon binary so that the launchd/systemd + // managed daemon picks up the new version on restart. + let daemon_bin = crate::config::Config::daemon_binary_path()?; + if daemon_bin.exists() { + super::init::install_daemon_binary(exe_path, &daemon_bin)?; + } + Ok(()) } diff --git a/src/config.rs b/src/config.rs index 4946423..81682be 100644 --- a/src/config.rs +++ b/src/config.rs @@ -57,6 +57,32 @@ impl Config { Ok(Self::config_dir()?.join("daemon.log")) } + /// Returns the path to the dedicated daemon binary. + /// + /// The daemon binary is stored separately from the CLI binary so that + /// launchd's KeepAlive monitoring of the daemon path does not interfere + /// with normal CLI invocations (e.g., `devproxy --version`). + /// + /// Default path (via `dirs::data_dir()`): + /// - macOS: `~/Library/Application Support/devproxy/devproxy-daemon` + /// - Linux: `~/.local/share/devproxy/devproxy-daemon` + /// + /// Respects `DEVPROXY_DATA_DIR` env var for test isolation. + /// + /// Note: this path is only resolved by CLI commands (`init`, `update`) + /// to install/update the daemon binary. The daemon process itself never + /// calls this function, so `DEVPROXY_DATA_DIR` does not need to be + /// propagated into plist/unit environment variables. + pub fn daemon_binary_path() -> Result { + if let Ok(dir) = std::env::var("DEVPROXY_DATA_DIR") { + return Ok(PathBuf::from(dir).join("devproxy-daemon")); + } + let dir = dirs::data_dir() + .context("could not determine data directory")? + .join("devproxy"); + Ok(dir.join("devproxy-daemon")) + } + pub fn save(&self) -> Result<()> { let dir = Self::config_dir()?; std::fs::create_dir_all(&dir)?; diff --git a/tests/e2e.rs b/tests/e2e.rs index 973224e..24f9a4b 100644 --- a/tests/e2e.rs +++ b/tests/e2e.rs @@ -1201,6 +1201,14 @@ fn test_launchd_socket_activation() { plist_content.contains(&daemon_port.to_string()), "plist should contain port {daemon_port}" ); + + // Verify the plist references the daemon binary path (not the CLI binary) + assert!( + plist_content.contains("devproxy-daemon"), + "plist should reference the dedicated daemon binary (devproxy-daemon), \ + not the CLI binary. This prevents launchd KeepAlive from SIGKILL'ing \ + normal CLI invocations." + ); } // Cleanup: bootout the agent and remove plist @@ -1235,3 +1243,192 @@ fn test_launchd_socket_activation() { let _ = std::fs::remove_dir_all(&config_dir); } + +/// Verify that `devproxy --version` works even when a daemon is running. +/// +/// This is the core regression test for the launchd SIGKILL bug: launchd's +/// KeepAlive=true causes it to SIGKILL any non-managed process at the +/// managed binary path. By using a separate daemon binary path, the CLI +/// binary is free from launchd interference. +/// +/// This test verifies the structural fix: init installs a daemon binary at +/// a separate path, and --version runs cleanly from the CLI binary. +#[test] +#[ignore] // Run with: cargo test --test e2e -- --ignored test_version_works_with_daemon_running +fn test_version_works_with_daemon_running() { + let config_dir = create_test_config_dir("ver-daemon"); + let port = find_free_port(); + + // Start a daemon in the background. The daemon needs TLS certs which + // create_test_config_dir already generates via `init --no-daemon`. + let _guard = start_test_daemon(&config_dir, port); + + // Run --version — this must succeed even while a daemon is running. + // In the launchd SIGKILL bug, running the binary at the launchd-managed + // path would get killed. With the daemon binary path separation fix, + // the CLI binary path is not managed by launchd. + let output = Command::new(devproxy_bin()) + .args(["--version"]) + .env("DEVPROXY_CONFIG_DIR", &config_dir) + .env("DEVPROXY_NO_SOCKET_ACTIVATION", "1") + .output() + .expect("failed to run devproxy --version"); + + assert!( + output.status.success(), + "devproxy --version should succeed (exit {}): stderr={}", + output.status.code().unwrap_or(-1), + String::from_utf8_lossy(&output.stderr) + ); + + let stdout = String::from_utf8_lossy(&output.stdout); + assert!( + stdout.contains("devproxy"), + "--version should print version info: {stdout}" + ); +} + +/// Verify that init with daemon installs the daemon binary at the +/// DEVPROXY_DATA_DIR path and that the daemon binary path differs +/// from the CLI binary path. +#[test] +fn test_init_daemon_binary_path_separation() { + let config_dir = std::env::temp_dir().join(format!( + "devproxy-config-path-sep-{}", + std::process::id() + )); + let data_dir = std::env::temp_dir().join(format!( + "devproxy-data-path-sep-{}", + std::process::id() + )); + if config_dir.exists() { + std::fs::remove_dir_all(&config_dir).unwrap(); + } + if data_dir.exists() { + std::fs::remove_dir_all(&data_dir).unwrap(); + } + std::fs::create_dir_all(&config_dir).unwrap(); + std::fs::create_dir_all(&data_dir).unwrap(); + + let port = find_free_port(); + + // Run init with daemon (uses fallback spawn since DEVPROXY_NO_SOCKET_ACTIVATION=1) + let output = Command::new(devproxy_bin()) + .args([ + "init", + "--domain", + TEST_DOMAIN, + "--port", + &port.to_string(), + ]) + .env("DEVPROXY_CONFIG_DIR", &config_dir) + .env("DEVPROXY_DATA_DIR", &data_dir) + .env("DEVPROXY_NO_SOCKET_ACTIVATION", "1") + .output() + .expect("failed to run devproxy init"); + + let stderr = String::from_utf8_lossy(&output.stderr); + eprintln!("init stderr: {stderr}"); + + assert!( + output.status.success(), + "init should succeed: {stderr}" + ); + + // Verify daemon binary was installed at the data dir path + let daemon_bin = data_dir.join("devproxy-daemon"); + assert!( + daemon_bin.exists(), + "daemon binary should be installed at {}: {stderr}", + daemon_bin.display() + ); + + // Verify daemon binary path is different from CLI binary path + let cli_bin = devproxy_bin(); + assert_ne!( + cli_bin, daemon_bin, + "daemon binary path should differ from CLI binary path" + ); + + // Verify --version works from CLI binary while daemon is running + let version_output = Command::new(&cli_bin) + .args(["--version"]) + .env("DEVPROXY_CONFIG_DIR", &config_dir) + .output() + .expect("failed to run --version"); + + assert!( + version_output.status.success(), + "--version should succeed while daemon is running (exit {}): {}", + version_output.status.code().unwrap_or(-1), + String::from_utf8_lossy(&version_output.stderr) + ); + + // Clean up daemon + let pid_path = config_dir.join("daemon.pid"); + if pid_path.exists() { + if let Ok(pid_str) = std::fs::read_to_string(&pid_path) { + if let Ok(pid) = pid_str.trim().parse::() { + unsafe { + libc::kill(pid, libc::SIGTERM); + } + std::thread::sleep(Duration::from_millis(500)); + unsafe { + libc::kill(pid, libc::SIGKILL); + } + } + } + } + + let _ = std::fs::remove_dir_all(&config_dir); + let _ = std::fs::remove_dir_all(&data_dir); +} + +/// Verify that the daemon binary at the data dir path can run --version +/// and produce the same output as the CLI binary. +#[test] +fn test_daemon_binary_matches_cli_binary() { + let data_dir = std::env::temp_dir().join(format!( + "devproxy-data-match-{}", + std::process::id() + )); + if data_dir.exists() { + std::fs::remove_dir_all(&data_dir).unwrap(); + } + std::fs::create_dir_all(&data_dir).unwrap(); + + let cli_bin = devproxy_bin(); + let daemon_bin = data_dir.join("devproxy-daemon"); + + // Copy CLI binary to daemon path (simulating what init does) + std::fs::copy(&cli_bin, &daemon_bin).unwrap(); + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + std::fs::set_permissions(&daemon_bin, std::fs::Permissions::from_mode(0o755)).unwrap(); + } + + let cli_output = Command::new(&cli_bin) + .args(["--version"]) + .output() + .expect("CLI --version failed"); + let daemon_output = Command::new(&daemon_bin) + .args(["--version"]) + .output() + .expect("daemon --version failed"); + + assert!(cli_output.status.success(), "CLI --version should succeed"); + assert!( + daemon_output.status.success(), + "daemon binary --version should succeed" + ); + + let cli_version = String::from_utf8_lossy(&cli_output.stdout); + let daemon_version = String::from_utf8_lossy(&daemon_output.stdout); + assert_eq!( + cli_version, daemon_version, + "CLI and daemon binaries should report the same version" + ); + + let _ = std::fs::remove_dir_all(&data_dir); +}