daemon: forward GUI envs; use user XDG config; (#333)

jamesmcm · web-flow · commit 59646e6fe835 · 2025-09-14T21:13:49.000+02:00
- Forward essential X/Wayland env vars from client to daemon-launched apps:
    DISPLAY, WAYLAND_DISPLAY, XAUTHORITY, XDG_RUNTIME_DIR, XDG_CONFIG_HOME,
    XDG_SESSION_TYPE, MOZ_ENABLE_WAYLAND, QT_QPA_PLATFORM, GTK_MODULES, GTK3_MODULES, I3SOCK.
    Default XDG_RUNTIME_DIR to /run/user/&lt;uid&gt; when missing.
  - Extend DaemonRequest to carry the forwarded env map; merge into child env in daemon path.
  - Respect XDG_CONFIG_HOME in daemon mode: add thread-local config_dir override
    and set it per client to forwarded XDG_CONFIG_HOME (if it exists) or fall back
    to ~/.config of the connecting user. Clear the override when the handler exits.
    This fixes false “config files … do not exist” when the daemon runs under systemd.
  - Demote noisy “Could not get PULSE_SERVER from host” to debug in daemon mode
    (still warn in non-daemon). Daemon continues to set PULSE_SERVER to
    unix:/run/user/&lt;uid&gt;/pulse/native for the connecting user.
  - Add daemon mode flag in core (set_daemon_mode/is_daemon_mode); mark daemon context
    in main and use it for conditional logging.
  - Avoid interactive auto-sync in daemon mode: setup_namespace now accepts
    auto_sync_if_missing; daemon passes false (bails with a clear message if configs
    are missing), CLI path passes true (preserving previous behavior).
  - Pass env and stdio consistently for both TTY and non-TTY client cases.

  Result:

  - GUI apps launched via the daemon get correct X/Wayland/desktop env and no longer fail with
  “no DISPLAY”.
  - Daemon resolves configs from the user’s XDG config dir instead of /root/.config under
  systemd.
  - Systemd logs are quieter; no spurious PULSE warnings or “not a terminal” from attempted
  interactive sync.
diff --git a/Cargo.toml b/Cargo.toml
@@ -1,7 +1,7 @@
 [package]
 name = "vopono"
 description = "Launch applications via VPN tunnels using temporary network namespaces"
-version = "0.10.15"
+version = "0.10.16"
 authors = ["James McMurray <jamesmcm03@gmail.com>"]
 edition = "2024"
 license = "GPL-3.0-or-later"
diff --git a/README.md b/README.md
@@ -90,7 +90,7 @@ For a smoother experience, run the privileged root daemon and keep using `vopono
 - Or run manually as root: `sudo vopono daemon`
 - Then use vopono normally as your user: `vopono exec --provider mullvad --server se firefox`
 
-See USERGUIDE.md for a ready‑to‑copy systemd unit.
+See [USERGUIDE.md](USERGUIDE.md) for a ready‑to‑copy systemd unit.
 
 vopono can handle up to 255 separate network namespaces (i.e. different VPN server
 connections - if your VPN provider allows it). Commands launched with
diff --git a/USERGUIDE.md b/USERGUIDE.md
@@ -31,7 +31,7 @@ vopono now supports a persistent root daemon that handles all privileged work. R
 
 Signals (e.g., Ctrl+C, Ctrl+Z) and interactive TTY behavior work cleanly via the daemon. The daemon listens on `/run/vopono.sock` and cleans it up on exit.
 
-Example systemd unit for the root daemon (`/etc/systemd/system/vopono-daemon.service`):
+Example systemd unit for the root daemon (`/etc/systemd/system/vopono.service`):
 
 ```
 [Unit]
@@ -54,8 +54,8 @@ WantedBy=multi-user.target
 Check status and logs:
 
 ```
-sudo systemctl status vopono-daemon
-sudo journalctl -u vopono-daemon -e
+sudo systemctl status vopono
+sudo journalctl -u vopono -e
 ```
 
 Note there is a known issue that when using tmux, etc. - sometimes the
diff --git a/src/daemon.rs b/src/daemon.rs
@@ -33,7 +33,10 @@ use std::thread;
 
 #[derive(Serialize, Deserialize, Debug)]
 pub enum DaemonRequest {
-    Execute(ExecCommand),
+    Execute {
+        cmd: ExecCommand,
+        env: std::collections::HashMap<String, String>,
+    },
     Control(DaemonControl),
 }
 
@@ -118,6 +121,8 @@ fn handle_client(mut conn: LocalSocketStream) -> anyhow::Result<()> {
         user.name, uid, group.name, gid
     );
 
+    // Note: Do not set config override yet; we may adopt client's XDG_CONFIG_HOME.
+
     // Read a framed request (length-prefixed u32 then payload)
     let mut len_bytes = [0u8; 4];
     conn.read_exact(&mut len_bytes)?;
@@ -132,7 +137,19 @@ fn handle_client(mut conn: LocalSocketStream) -> anyhow::Result<()> {
         recv_fds_over_unix_socket(&conn, 3)?;
 
     match request {
-        DaemonRequest::Execute(mut exec_command) => {
+        DaemonRequest::Execute {
+            cmd: mut exec_command,
+            env: forwarded_env,
+        } => {
+            // Set config override from client's XDG_CONFIG_HOME if present, falling back to ~/.config
+            let override_base = forwarded_env
+                .get("XDG_CONFIG_HOME")
+                .and_then(|p| {
+                    let pb = std::path::PathBuf::from(p);
+                    if pb.exists() { Some(pb) } else { None }
+                })
+                .unwrap_or_else(|| user.dir.join(".config"));
+            vopono_core::util::set_config_dir_override(Some(override_base));
             exec_command.user = Some(user.name);
             exec_command.group = Some(group.name);
 
@@ -158,6 +175,7 @@ fn handle_client(mut conn: LocalSocketStream) -> anyhow::Result<()> {
                     false,
                     Some((slave, slave, slave)),
                     true,
+                    Some(forwarded_env.clone()),
                 )?;
                 // Do not close the slave here: it's owned by the spawned child via Stdio::from_raw_fd
                 // and will be closed by the child/OS when appropriate.
@@ -168,6 +186,7 @@ fn handle_client(mut conn: LocalSocketStream) -> anyhow::Result<()> {
                     false,
                     Some((client_stdin_fd, client_stdout_fd, client_stderr_fd)),
                     false,
+                    Some(forwarded_env.clone()),
                 )?;
             }
 
@@ -356,6 +375,8 @@ fn handle_client(mut conn: LocalSocketStream) -> anyhow::Result<()> {
             // Ignore unexpected control frame sent as the first message
         }
     }
+    // Clear any thread-local override before exiting the handler
+    vopono_core::util::set_config_dir_override(None);
     Ok(())
 }
 
diff --git a/src/exec.rs b/src/exec.rs
@@ -50,7 +50,7 @@ pub fn execute_as_daemon(
         parsed_command,
         forwarder,
         host_env_vars,
-    } = setup_namespace(command, &uiclient, true)?;
+    } = setup_namespace(command, &uiclient, true, false)?; // daemon: do not auto-sync
 
     let ns = ns.write_lockfile(&parsed_command.application)?;
 
@@ -76,14 +76,15 @@ pub fn execute_as_daemon_with_stdio(
     pipe_io: bool,
     stdio_fds: Option<(RawFd, RawFd, RawFd)>,
     take_controlling_tty: bool,
+    forwarded_env: Option<std::collections::HashMap<String, String>>,
 ) -> anyhow::Result<(ApplicationWrapper, NetworkNamespace)> {
     let uiclient = CliClient {};
     let NamespaceConfig {
         ns,
         parsed_command,
         forwarder,
         host_env_vars,
-    } = setup_namespace(command, &uiclient, true)?;
+    } = setup_namespace(command, &uiclient, true, false)?; // daemon: do not auto-sync
 
     // In daemon mode, ensure PULSE_SERVER points to the connecting user's runtime
     // so apps can talk to the host Pulse/pipewire server.
@@ -93,6 +94,18 @@ pub fn execute_as_daemon_with_stdio(
     {
         let pulse = format!("unix:/run/user/{}/pulse/native", user.uid.as_raw());
         host_env_vars.insert("PULSE_SERVER".to_string(), pulse);
+        // Ensure XDG_RUNTIME_DIR points at the connecting user's runtime dir
+        host_env_vars
+            .entry("XDG_RUNTIME_DIR".to_string())
+            .or_insert_with(|| format!("/run/user/{}", user.uid.as_raw()));
+    }
+
+    // Merge all client-forwarded environment variables. Client side already whitelists
+    // which keys are forwarded; here we simply apply them.
+    if let Some(fwd) = forwarded_env.as_ref() {
+        for (k, v) in fwd {
+            host_env_vars.insert(k.clone(), v.clone());
+        }
     }
 
     let ns = ns.write_lockfile(&parsed_command.application)?;
@@ -125,7 +138,7 @@ pub fn exec(
         parsed_command,
         forwarder,
         host_env_vars,
-    } = setup_namespace(command, uiclient, verbose)?;
+    } = setup_namespace(command, uiclient, verbose, true)?; // CLI path: allow auto-sync if missing
     let ns = ns.write_lockfile(&parsed_command.application)?;
     run_application_and_wait(
         &parsed_command,
@@ -142,6 +155,7 @@ fn setup_namespace(
     command: ExecCommand,
     uiclient: &dyn UiClient,
     verbose: bool,
+    auto_sync_if_missing: bool,
 ) -> anyhow::Result<NamespaceConfig> {
     create_dir_all(vopono_dir()?)?;
     let vopono_config_settings = ArgsConfig::get_config_file(&command)?;
@@ -164,16 +178,28 @@ fn setup_namespace(
                 .wireguard_dir(),
             _ => unreachable!(),
         }?;
-        if !cdir.exists() || cdir.read_dir()?.next().is_none() {
-            info!(
-                "Config files for {} {} do not exist, running vopono sync",
-                parsed_command.provider, parsed_command.protocol
-            );
-            synch(
-                parsed_command.provider.clone(),
-                &Some(parsed_command.protocol.clone()),
-                uiclient,
-            )?;
+        let missing_configs = !cdir.exists() || cdir.read_dir()?.next().is_none();
+        if missing_configs {
+            if auto_sync_if_missing {
+                info!(
+                    "Config files for {} {} do not exist, running vopono sync",
+                    parsed_command.provider, parsed_command.protocol
+                );
+                synch(
+                    parsed_command.provider.clone(),
+                    &Some(parsed_command.protocol.clone()),
+                    uiclient,
+                )?;
+            } else {
+                // In daemon mode, avoid interactive sync and return a clear error.
+                anyhow::bail!(
+                    "Missing configuration for {} {}. Run 'vopono sync --provider {} --protocol {}' as your user to initialize.",
+                    parsed_command.provider,
+                    parsed_command.protocol,
+                    parsed_command.provider,
+                    parsed_command.protocol
+                );
+            }
         }
     }
 
diff --git a/src/main.rs b/src/main.rs
@@ -61,6 +61,8 @@ fn main() -> anyhow::Result<()> {
                 eprintln!("Error: The daemon command requires root privileges.");
                 std::process::exit(1);
             }
+            // Mark process context so libraries can adjust behavior (e.g., logging verbosity)
+            vopono_core::util::set_daemon_mode(true);
             info!("Starting vopono in daemon mode.");
             return daemon::start();
         }
@@ -116,7 +118,34 @@ fn forward_to_daemon(cmd: &ExecCommand) -> anyhow::Result<i32> {
     };
 
     debug!("Connected to daemon, forwarding command.");
-    let request = daemon::DaemonRequest::Execute(cmd.clone());
+    // Collect a small set of environment variables from the client session
+    // that are relevant for GUI/desktop integration.
+    let mut fwd_env: std::collections::HashMap<String, String> = Default::default();
+    for key in [
+        // X/Wayland basics
+        "DISPLAY",
+        "WAYLAND_DISPLAY",
+        "XAUTHORITY",
+        // Runtime/config roots for per-user sockets and configs
+        "XDG_RUNTIME_DIR",
+        "XDG_CONFIG_HOME",
+        // Toolkit/session hints (safe to forward)
+        "XDG_SESSION_TYPE",
+        "MOZ_ENABLE_WAYLAND",
+        "QT_QPA_PLATFORM",
+        "GTK_MODULES",
+        "GTK3_MODULES",
+        // Window manager IPC socket
+        "I3SOCK",
+    ] {
+        if let Ok(val) = std::env::var(key) {
+            fwd_env.insert(key.to_string(), val);
+        }
+    }
+    let request = daemon::DaemonRequest::Execute {
+        cmd: cmd.clone(),
+        env: fwd_env,
+    };
     let bytes = bincode::serde::encode_to_vec(&request, bincode::config::standard())?;
     conn.write_all(&(bytes.len() as u32).to_be_bytes())?;
     conn.write_all(&bytes)?;
diff --git a/vopono.service b/vopono.service
@@ -0,0 +1,15 @@
+[Unit]
+Description=Vopono root daemon
+After=network.target
+Requires=network.target
+
+[Service]
+Type=simple
+ExecStart=/usr/bin/vopono daemon
+Restart=on-failure
+RestartSec=2s
+# Optional: enable structured logs
+Environment=RUST_LOG=info
+
+[Install]
+WantedBy=multi-user.target
diff --git a/vopono_core/Cargo.toml b/vopono_core/Cargo.toml
@@ -1,7 +1,7 @@
 [package]
 name = "vopono_core"
 description = "Library code for running VPN connections in network namespaces"
-version = "0.1.15"
+version = "0.1.16"
 edition = "2024"
 authors = ["James McMurray <jamesmcm03@gmail.com>"]
 license = "GPL-3.0-or-later"
diff --git a/vopono_core/src/network/application_wrapper.rs b/vopono_core/src/network/application_wrapper.rs
@@ -261,13 +261,14 @@ impl ApplicationWrapper {
                         // If stdin is a TTY, take it as controlling terminal
                         // Use TIOCSCTTY with arg 1 to forcibly acquire if already in use
                         let fd0: i32 = 0;
-                        let is_tty = libc::isatty(fd0) == 1;
-                        if is_tty {
-                            // Let the compiler infer the proper ioctl request type per target
-                            let _ = libc::ioctl(fd0, libc::TIOCSCTTY as _, 1);
-                            // Set foreground process group to our own pgrp
-                            let pgrp = libc::getpgrp();
-                            let _ = libc::tcsetpgrp(fd0, pgrp);
+                        if libc::isatty(fd0) == 1 {
+                            // Attempt to acquire the TTY as controlling terminal. Only set
+                            // the foreground process group if that succeeded.
+                            let acquire_res = libc::ioctl(fd0, libc::TIOCSCTTY as _, 1);
+                            if acquire_res == 0 {
+                                let pgrp = libc::getpgrp();
+                                let _ = libc::tcsetpgrp(fd0, pgrp);
+                            }
                         }
                     }
 
diff --git a/vopono_core/src/util/env_vars.rs b/vopono_core/src/util/env_vars.rs
@@ -8,14 +8,21 @@ use crate::network::{netns::NetworkNamespace, port_forwarding::Forwarder};
 pub fn get_host_env_vars() -> HashMap<String, String> {
     let mut env_vars = HashMap::new();
 
+    // Best-effort: try to detect Pulse/pipewire server when available.
+    // Avoid noisy warnings in non-interactive contexts (e.g., systemd daemon) by logging at debug level.
     if which::which("pactl").is_ok() {
         match crate::util::pulseaudio::get_pulseaudio_server() {
             Ok(pa) => {
                 debug!("Found PULSE_SERVER on host: {}", &pa);
                 env_vars.insert("PULSE_SERVER".to_string(), pa);
             }
             Err(e) => {
-                warn!("Could not get PULSE_SERVER from host: {e:?}");
+                // Only warn in non-daemon contexts; daemon sets PULSE_SERVER explicitly per user.
+                if crate::util::is_daemon_mode() {
+                    debug!("Could not get PULSE_SERVER from host: {e:?}");
+                } else {
+                    warn!("Could not get PULSE_SERVER from host: {e:?}");
+                }
             }
         }
     } else {
diff --git a/vopono_core/src/util/mod.rs b/vopono_core/src/util/mod.rs
@@ -31,7 +31,39 @@ use users::{get_current_uid, get_user_by_uid};
 use walkdir::WalkDir;
 use which::which;
 
+thread_local! {
+    static CONFIG_DIR_OVERRIDE: std::cell::RefCell<Option<PathBuf>> = const { std::cell::RefCell::new(None) };
+}
+
+static DAEMON_MODE: std::sync::atomic::AtomicBool = std::sync::atomic::AtomicBool::new(false);
+
+pub fn set_daemon_mode(v: bool) {
+    DAEMON_MODE.store(v, std::sync::atomic::Ordering::Relaxed);
+}
+
+pub fn is_daemon_mode() -> bool {
+    DAEMON_MODE.load(std::sync::atomic::Ordering::Relaxed)
+}
+
+/// Set a thread-local override for the base config directory (i.e., the parent of `vopono/`).
+/// When set, `config_dir()` will return this path instead of detecting from env/XDG.
+/// Use `None` to clear. This is safe for the daemon where each client runs on its own thread.
+pub fn set_config_dir_override(path: Option<PathBuf>) {
+    CONFIG_DIR_OVERRIDE.with(|ov| *ov.borrow_mut() = path);
+}
+
 pub fn config_dir() -> anyhow::Result<PathBuf> {
+    // Respect thread-local override first (used by daemon to select the connecting user's config).
+    if let Some(override_path) = CONFIG_DIR_OVERRIDE.with(|ov| ov.borrow().clone())
+        && override_path.exists()
+    {
+        debug!(
+            "Using config dir from override: {}",
+            override_path.to_string_lossy()
+        );
+        return Ok(override_path);
+    }
+
     let path: Option<PathBuf> = None
         .or_else(|| {
             if let Ok(home) = std::env::var("HOME") {
