Make daemon socket/PID paths environment-independent; fix rc service HOME

4 files changed, 68 insertions, 31 deletions

diff --git a/README.md b/README.md
index ef8ebaf..fbc5ffa 100644
--- a/README.md
+++ b/README.md
@@ -44,9 +44,11 @@ FreeBSD notes:
   as rsplayer_user via daemon(8); `service rsplayer status`/`stop`/`restart`
   work as expected. You can also run `rsp start-daemon` manually from your
   login shell — `start-daemon`/`stop-daemon`/`restart` behave the same.
-- XDG_RUNTIME_DIR is not set by default on FreeBSD, so the daemon socket and PID
-  file fall back to /tmp/rsplayer/. Set XDG_RUNTIME_DIR in your rc.d script or
-  shell profile if you prefer a private runtime dir.
+- The daemon socket and PID file live in ~/.local/state/rsplayer/ (the XDG
+  state dir), not in XDG_RUNTIME_DIR: the path is identical whether the daemon
+  was started by the service or manually. This matters on FreeBSD, where
+  XDG_RUNTIME_DIR is often unset (previously the fallback was the
+  world-writable /tmp/rsplayer).
 - Desktop notifications and MPRIS (playerctl etc.) require a running D-Bus session
   bus (`pkg install dbus`) and a notification daemon; without them the daemon runs
   fine, those features are just disabled.
diff --git a/rsplayer.rc b/rsplayer.rc
index 33a46b9..706387b 100755
--- a/rsplayer.rc
+++ b/rsplayer.rc
@@ -14,11 +14,10 @@
 #   rsplayer_group="wheel"     # group of that user
 #
 # rsplayer_user must be a regular user: the daemon socket and PID file live
-# in rsplayer_runtime_dir (default /tmp/rsplayer, matching the daemon's
-# fallback when XDG_RUNTIME_DIR is unset) and the `rsp` CLI connects to that
-# socket, so the daemon and the CLI must run as the same user.
-#
-# Optional: rsplayer_runtime_dir="/tmp/rsplayer"
+# in that user's XDG state directory (~/.local/state/rsplayer) and the `rsp`
+# CLI connects to that socket, so the daemon and the CLI must run as the
+# same user. This also means the socket path no longer depends on the
+# environment (XDG_RUNTIME_DIR), so the service and manual runs agree.
 
 # PROVIDE: rsplayer
 # REQUIRE: LOGIN dbus
@@ -33,11 +32,21 @@ load_rc_config $name
 
 : ${rsplayer_user:="root"}
 : ${rsplayer_group:="wheel"}
-: ${rsplayer_runtime_dir:="/tmp/rsplayer"}
+
+# daemon(8) does not set HOME when dropping privileges, and rsp derives its
+# config and runtime paths from it, so resolve the user's home explicitly.
+rsplayer_home=$(pw usershow "${rsplayer_user}" 2>/dev/null | awk -F: '{print $9}')
+if [ -z "${rsplayer_home}" ]; then
+    echo "Cannot determine home directory for user '${rsplayer_user}'."
+    echo "Set rsplayer_user in /etc/rc.conf."
+    exit 1
+fi
 
 command="/usr/local/bin/rsp"
 procname="${command}"
-pidfile="${rsplayer_runtime_dir}/rsplayer.pid"
+state_dir="${rsplayer_home}/.local/state/rsplayer"
+pidfile="${state_dir}/rsplayer.pid"
+socket="${state_dir}/daemon.sock"
 
 start_cmd="rsplayer_start"
 stop_cmd="rsplayer_stop"
@@ -54,19 +63,23 @@ rsplayer_start()
         echo "         connect to the daemon socket. Set rsplayer_user in rc.conf."
     fi
 
-    install -d -o "${rsplayer_user}" -g "${rsplayer_group}" -m 700 "${rsplayer_runtime_dir}"
-    echo "Starting rsplayer as ${rsplayer_user}."
-    /usr/sbin/daemon -u "${rsplayer_user}" -o "${rsplayer_runtime_dir}/rsplayer.log" \
+    echo "Starting rsplayer as ${rsplayer_user} (HOME=${rsplayer_home})."
+    /usr/bin/env HOME="${rsplayer_home}" \
+        /usr/sbin/daemon -u "${rsplayer_user}" -o "${state_dir}/rsplayer.log" \
         "${command}" start-daemon
 }
 
 rsplayer_stop()
 {
     echo "Stopping rsplayer."
-    if ! su -m "${rsplayer_user}" -c "${command} stop-daemon"; then
-        echo "rsplayer is not running; cleaning up stale files."
-        rm -f "${pidfile}" "${rsplayer_runtime_dir}/daemon.sock"
+    if ! su -m "${rsplayer_user}" -c "env HOME='${rsplayer_home}' ${command} stop-daemon"; then
+        # stop-daemon could not reach the daemon (stale/missing PID file or
+        # socket, e.g. a daemon started under an older runtime dir): kill any
+        # leftover rsp start-daemon processes for this user.
+        echo "Graceful stop failed; killing leftover daemons."
+        pkill -u "${rsplayer_user}" -f "${command} start-daemon" 2>/dev/null || true
     fi
+    rm -f "${pidfile}" "${socket}"
 }
 
 run_rc_command "$1"
diff --git a/src/config.rs b/src/config.rs
index 67cb366..f20fb3a 100644
--- a/src/config.rs
+++ b/src/config.rs
@@ -303,9 +303,39 @@ impl Config {
     }
 
     pub fn get_socket_path() -> PathBuf {
-        let runtime_dir = std::env::var("XDG_RUNTIME_DIR")
-            .map(PathBuf::from)
-            .unwrap_or_else(|_| PathBuf::from("/tmp"));
-        runtime_dir.join("rsplayer").join("daemon.sock")
+        runtime_dir().join("daemon.sock")
     }
 }
+
+/// Directory holding the daemon socket and PID file.
+///
+/// Uses the user's XDG state directory (~/.local/state/rsplayer) so the path
+/// is identical no matter how the daemon was started (rc.d service, login
+/// shell, ...). The previous XDG_RUNTIME_DIR-based path depended on the
+/// environment and broke the service/CLI handshake on FreeBSD, where
+/// XDG_RUNTIME_DIR is often unset (falling back to the world-writable
+/// /tmp/rsplayer).
+pub fn runtime_dir() -> PathBuf {
+    if let Some(base) = directories::BaseDirs::new()
+        && let Some(state) = base.state_dir()
+    {
+        return state.join("rsplayer");
+    }
+    // Fallback when no home/state dir is resolvable
+    std::env::var("XDG_RUNTIME_DIR")
+        .map(PathBuf::from)
+        .unwrap_or_else(|_| PathBuf::from("/tmp"))
+        .join("rsplayer")
+}
+
+/// Create the runtime directory (private, 0700) and return its path.
+pub fn ensure_runtime_dir() -> std::io::Result<PathBuf> {
+    let dir = runtime_dir();
+    std::fs::create_dir_all(&dir)?;
+    #[cfg(unix)]
+    {
+        use std::os::unix::fs::PermissionsExt;
+        std::fs::set_permissions(&dir, std::fs::Permissions::from_mode(0o700))?;
+    }
+    Ok(dir)
+}
diff --git a/src/daemon.rs b/src/daemon.rs
index 279620d..21f1a3b 100644
--- a/src/daemon.rs
+++ b/src/daemon.rs
@@ -203,13 +203,8 @@ pub fn start_daemon() -> Result<(), Box<dyn std::error::Error>> {
         std::fs::remove_file(&socket_path).ok();
     }
 
-    // Create runtime directory
-    if let Some(parent) = socket_path.parent() {
-        std::fs::create_dir_all(parent)?;
-    }
-    if let Some(parent) = pid_path.parent() {
-        std::fs::create_dir_all(parent)?;
-    }
+    // Create runtime directory (private, 0700)
+    crate::config::ensure_runtime_dir()?;
 
     // Write PID file
     let pid = std::process::id();
@@ -1609,10 +1604,7 @@ fn find_audio_in_dir(dir: &Path) -> Option<Vec<std::path::PathBuf>> {
 
 /// Get the PID file path for the daemon
 fn get_pid_path(_config: &Config) -> std::path::PathBuf {
-    let runtime_dir = std::env::var("XDG_RUNTIME_DIR")
-        .map(std::path::PathBuf::from)
-        .unwrap_or_else(|_| std::path::PathBuf::from("/tmp"));
-    runtime_dir.join("rsplayer").join("rsplayer.pid")
+    crate::config::runtime_dir().join("rsplayer.pid")
 }
 
 /// Check if a process with the given PID is running