Runtime Paths Checks as opt-in #935

Co-Authored-By: brekotis <93345790+brekotis@users.noreply.github.com>

src/daemon/mod.rs

@@ -29,6 +29,8 @@ pub struct DaemonOptions {
     pub daemonize: bool,
     /// Path to PID file.
     pub pid_file: Option<PathBuf>,
+    /// Require trusted, symlink-free PID and log parents. Disabled by default for compatibility.
+    pub strict_runtime_paths: bool,
     /// User to run as after binding sockets.
     pub user: Option<String>,
     /// Group to run as after binding sockets.
@@ -332,6 +334,7 @@ mod tests {
     fn test_daemon_options_default() {
         let opts = DaemonOptions::default();
         assert!(!opts.daemonize);
+        assert!(!opts.strict_runtime_paths);
         assert!(!opts.should_daemonize());
         assert_eq!(opts.pid_file_path(), Path::new(DEFAULT_PID_FILE));
     }

src/daemon/pid_file.rs

@@ -17,6 +17,7 @@ use crate::util::secure_fs::AnchoredPath;
 /// PID file manager backed by a persistent sibling lock file.
 pub struct PidFile {
     path: PathBuf,
+    strict_runtime_paths: bool,
     lock_path: PathBuf,
     pid_file: Option<File>,
     pid_identity: Option<FileIdentity>,
@@ -40,12 +41,13 @@ impl FileIdentity {
 }
 
 impl PidFile {
-    /// Creates a new PID file manager for the given path.
-    pub fn new<P: AsRef<Path>>(path: P) -> Self {
+    /// Creates a PID manager with explicit parent-path policy; `false` allows legacy parents.
+    pub fn new<P: AsRef<Path>>(path: P, strict_runtime_paths: bool) -> Self {
         let path = normalize_pid_path(path.as_ref());
         let lock_path = sibling_lock_path(&path);
         Self {
             path,
+            strict_runtime_paths,
             lock_path,
             pid_file: None,
             pid_identity: None,
@@ -56,7 +58,7 @@ impl PidFile {
 
     /// Checks whether the PID file names a running process without modifying either file.
     pub fn check_running(&self) -> Result<Option<i32>, DaemonError> {
-        let Some(pid) = read_pid_file_if_exists(&self.path)? else {
+        let Some(pid) = read_pid_file_if_exists(&self.path, self.strict_runtime_paths)? else {
             return Ok(None);
         };
         Ok(is_process_running(pid).then_some(pid))
@@ -67,9 +69,10 @@ impl PidFile {
     /// Fails if another owner holds the lock or the existing PID names a running process.
     pub fn acquire(&mut self) -> Result<(), DaemonError> {
         let anchor =
-            AnchoredPath::open_trusted_parent_or_create(&self.path, 0o755).map_err(|error| {
+            AnchoredPath::open_runtime_parent(&self.path, Some(0o755), self.strict_runtime_paths)
+                .map_err(|error| {
                 DaemonError::PidFile(format!(
-                    "cannot open trusted parent for {}: {}",
+                    "cannot open PID parent for {}: {}",
                     self.path.display(),
                     error
                 ))
@@ -245,13 +248,16 @@ fn open_file_at(anchor: &AnchoredPath, name: &OsStr, flags: OFlag, mode: u32) ->
     Ok(File::from(descriptor))
 }
 
-fn read_pid_file_if_exists(path: &Path) -> Result<Option<i32>, DaemonError> {
-    let anchor = match AnchoredPath::open_trusted_parent(path) {
+fn read_pid_file_if_exists(
+    path: &Path,
+    strict_runtime_paths: bool,
+) -> Result<Option<i32>, DaemonError> {
+    let anchor = match AnchoredPath::open_runtime_parent(path, None, strict_runtime_paths) {
         Ok(anchor) => anchor,
         Err(error) if error.kind() == ErrorKind::NotFound => return Ok(None),
         Err(error) => {
             return Err(DaemonError::PidFile(format!(
-                "cannot open trusted parent for {}: {}",
+                "cannot open PID parent for {}: {}",
                 path.display(),
                 error
             )));
@@ -352,11 +358,14 @@ fn validate_regular_single_link(file: &File, path: &Path) -> Result<fs::Metadata
     Ok(metadata)
 }
 
-/// Reads a PID from a PID file.
+/// Reads a PID using the selected parent-path policy; `false` allows legacy parents.
 #[allow(dead_code)]
-pub fn read_pid_file<P: AsRef<Path>>(path: P) -> Result<i32, DaemonError> {
+pub fn read_pid_file<P: AsRef<Path>>(
+    path: P,
+    strict_runtime_paths: bool,
+) -> Result<i32, DaemonError> {
     let path = normalize_pid_path(path.as_ref());
-    read_pid_file_if_exists(&path)?.ok_or_else(|| {
+    read_pid_file_if_exists(&path, strict_runtime_paths)?.ok_or_else(|| {
         DaemonError::PidFile(format!(
             "cannot read {}: file does not exist",
             path.display()
@@ -364,17 +373,18 @@ pub fn read_pid_file<P: AsRef<Path>>(path: P) -> Result<i32, DaemonError> {
     })
 }
 
-/// Sends a signal to the process specified in a PID file.
+/// Signals a lock-owning process using the same parent-path policy for PID and lock files.
 #[allow(dead_code)]
 pub fn signal_pid_file<P: AsRef<Path>>(
     path: P,
     signal: nix::sys::signal::Signal,
+    strict_runtime_paths: bool,
 ) -> Result<(), DaemonError> {
     let path = normalize_pid_path(path.as_ref());
-    let pid = read_pid_file(&path)?;
+    let pid = read_pid_file(&path, strict_runtime_paths)?;
     #[cfg(target_os = "linux")]
     let pidfd = open_pidfd(pid)?;
-    if !daemon_lock_is_held(&path)? {
+    if !daemon_lock_is_held(&path, strict_runtime_paths)? {
         return Err(DaemonError::PidFile(format!(
             "refusing to signal unlocked or stale PID file {}",
             path.display()
@@ -399,12 +409,15 @@ pub enum DaemonStatus {
     NotRunning,
 }
 
-/// Checks daemon status without modifying the PID or lock file.
+/// Checks daemon status read-only, applying the selected policy to both parent lookups.
 #[allow(dead_code)]
-pub fn check_status<P: AsRef<Path>>(path: P) -> DaemonStatus {
+pub fn check_status<P: AsRef<Path>>(path: P, strict_runtime_paths: bool) -> DaemonStatus {
     let path = normalize_pid_path(path.as_ref());
-    match read_pid_file_if_exists(&path) {
-        Ok(Some(pid)) if daemon_lock_is_held(&path).unwrap_or(false) && is_process_running(pid) => {
+    match read_pid_file_if_exists(&path, strict_runtime_paths) {
+        Ok(Some(pid))
+            if daemon_lock_is_held(&path, strict_runtime_paths).unwrap_or(false)
+                && is_process_running(pid) =>
+        {
             DaemonStatus::Running(pid)
         }
         Ok(Some(pid)) => DaemonStatus::Stale(pid),
@@ -412,14 +425,14 @@ pub fn check_status<P: AsRef<Path>>(path: P) -> DaemonStatus {
     }
 }
 
-fn daemon_lock_is_held(path: &Path) -> Result<bool, DaemonError> {
+fn daemon_lock_is_held(path: &Path, strict_runtime_paths: bool) -> Result<bool, DaemonError> {
     let lock_path = sibling_lock_path(path);
-    let anchor = match AnchoredPath::open_trusted_parent(path) {
+    let anchor = match AnchoredPath::open_runtime_parent(path, None, strict_runtime_paths) {
         Ok(anchor) => anchor,
         Err(error) if error.kind() == ErrorKind::NotFound => return Ok(false),
         Err(error) => {
             return Err(DaemonError::PidFile(format!(
-                "cannot open trusted parent for {}: {}",
+                "cannot open PID parent for {}: {}",
                 path.display(),
                 error
             )));

src/daemon/pid_file/tests.rs

@@ -38,6 +38,107 @@ fn pid_file_remains_send_and_sync() {
     assert_send_sync::<PidFile>();
 }
 
+#[test]
+fn compatibility_pid_lifecycle_accepts_writable_parent_directories() {
+    use std::os::unix::fs::PermissionsExt;
+
+    for mode in [0o770, 0o777, 0o1777] {
+        let directory = tempfile::tempdir().unwrap();
+        fs::set_permissions(directory.path(), fs::Permissions::from_mode(mode)).unwrap();
+        let pid_path = directory.path().join("telemt.pid");
+        let mut strict_owner = PidFile::new(&pid_path, true);
+        assert!(strict_owner.acquire().is_err());
+        assert!(!pid_path.exists());
+        assert!(!sibling_lock_path(&pid_path).exists());
+        let mut owner = PidFile::new(&pid_path, false);
+        owner.acquire().unwrap();
+
+        assert_eq!(
+            read_pid_file(&pid_path, false).unwrap(),
+            std::process::id() as i32
+        );
+        assert_eq!(
+            owner.check_running().unwrap(),
+            Some(std::process::id() as i32)
+        );
+        assert_eq!(
+            check_status(&pid_path, false),
+            DaemonStatus::Running(std::process::id() as i32)
+        );
+        assert!(read_pid_file(&pid_path, true).is_err());
+        assert!(strict_owner.check_running().is_err());
+        assert_eq!(check_status(&pid_path, true), DaemonStatus::NotRunning);
+
+        owner.release().unwrap();
+        assert!(!pid_path.exists());
+        assert!(sibling_lock_path(&pid_path).exists());
+        assert_eq!(check_status(&pid_path, false), DaemonStatus::NotRunning);
+    }
+}
+
+#[test]
+fn compatibility_pid_lifecycle_follows_symlinked_parents() {
+    let directory = tempfile::tempdir().unwrap();
+    let real = directory.path().join("tmp");
+    let linked = directory.path().join("var");
+    fs::create_dir(&real).unwrap();
+    symlink("tmp", &linked).unwrap();
+    let pid_path = linked.join("run/telemt.pid");
+    let mut strict_owner = PidFile::new(&pid_path, true);
+    assert!(strict_owner.acquire().is_err());
+    assert!(!real.join("run").exists());
+    let mut owner = PidFile::new(&pid_path, false);
+    owner.acquire().unwrap();
+
+    assert_eq!(
+        read_pid_file(&pid_path, false).unwrap(),
+        std::process::id() as i32
+    );
+    assert_eq!(
+        check_status(&pid_path, false),
+        DaemonStatus::Running(std::process::id() as i32)
+    );
+    assert!(real.join("run/telemt.pid.lock").exists());
+    assert!(read_pid_file(&pid_path, true).is_err());
+    assert_eq!(check_status(&pid_path, true), DaemonStatus::NotRunning);
+
+    owner.release().unwrap();
+    assert!(!real.join("run/telemt.pid").exists());
+}
+
+#[test]
+fn private_pid_parent_supports_both_policies() {
+    for strict_runtime_paths in [false, true] {
+        let directory = tempfile::tempdir().unwrap();
+        let pid_path = directory.path().join("nested/run/telemt.pid");
+        let mut owner = PidFile::new(&pid_path, strict_runtime_paths);
+        owner.acquire().unwrap();
+
+        for read_strict in [false, true] {
+            assert_eq!(
+                read_pid_file(&pid_path, read_strict).unwrap(),
+                std::process::id() as i32
+            );
+            assert_eq!(
+                check_status(&pid_path, read_strict),
+                DaemonStatus::Running(std::process::id() as i32)
+            );
+        }
+        assert_eq!(
+            fs::metadata(pid_path.parent().unwrap()).unwrap().mode() & 0o777,
+            0o755
+        );
+        let mut contender = PidFile::new(&pid_path, strict_runtime_paths);
+        assert!(matches!(
+            contender.acquire(),
+            Err(DaemonError::AlreadyRunning(_))
+        ));
+        owner.release().unwrap();
+        contender.acquire().unwrap();
+        contender.release().unwrap();
+    }
+}
+
 #[test]
 fn system_var_run_alias_keeps_the_default_pid_path_usable() {
     let Ok(metadata) = fs::symlink_metadata("/var/run") else {
@@ -52,7 +153,7 @@ fn system_var_run_alias_keeps_the_default_pid_path_usable() {
         return;
     }
 
-    let pid_file = PidFile::new("/var/run/telemt.pid");
+    let pid_file = PidFile::new("/var/run/telemt.pid", false);
 
     assert_eq!(pid_file.path(), Path::new("/run/telemt.pid"));
 }
@@ -64,7 +165,7 @@ fn lock_holder_subprocess() {
     };
     let ready_path = PathBuf::from(std::env::var_os(HELPER_READY_PATH).unwrap());
     let stop_path = PathBuf::from(std::env::var_os(HELPER_STOP_PATH).unwrap());
-    let mut pid_file = PidFile::new(PathBuf::from(pid_path));
+    let mut pid_file = PidFile::new(PathBuf::from(pid_path), false);
     pid_file.acquire().unwrap();
     fs::write(&ready_path, b"ready").unwrap();
     assert!(wait_for_path(&stop_path, Duration::from_secs(10)));
@@ -100,7 +201,7 @@ fn persistent_sibling_lock_serializes_processes_after_pid_unlink() {
     let lock_inode = fs::metadata(&lock_path).unwrap().ino();
     fs::remove_file(&pid_path).unwrap();
 
-    let mut contender = PidFile::new(&pid_path);
+    let mut contender = PidFile::new(&pid_path, false);
     assert!(contender.acquire().is_err());
 
     fs::write(&stop_path, b"stop").unwrap();
@@ -123,10 +224,13 @@ fn stale_pid_checks_are_read_only() {
     let directory = tempfile::tempdir().unwrap();
     let pid_path = directory.path().join("telemt.pid");
     fs::write(&pid_path, b"2000000000\n").unwrap();
-    let pid_file = PidFile::new(&pid_path);
+    let pid_file = PidFile::new(&pid_path, false);
 
     assert_eq!(pid_file.check_running().unwrap(), None);
-    assert_eq!(check_status(&pid_path), DaemonStatus::Stale(2_000_000_000));
+    assert_eq!(
+        check_status(&pid_path, false),
+        DaemonStatus::Stale(2_000_000_000)
+    );
     assert!(pid_path.exists());
 }
 
@@ -136,15 +240,15 @@ fn status_requires_live_lock_ownership() {
     let pid_path = directory.path().join("telemt.pid");
     fs::write(&pid_path, format!("{}\n", std::process::id())).unwrap();
     assert_eq!(
-        check_status(&pid_path),
+        check_status(&pid_path, false),
         DaemonStatus::Stale(std::process::id() as i32)
     );
 
     fs::remove_file(&pid_path).unwrap();
-    let mut owner = PidFile::new(&pid_path);
+    let mut owner = PidFile::new(&pid_path, false);
     owner.acquire().unwrap();
     assert_eq!(
-        check_status(&pid_path),
+        check_status(&pid_path, false),
         DaemonStatus::Running(std::process::id() as i32)
     );
     owner.release().unwrap();
@@ -155,7 +259,7 @@ fn unowned_release_does_not_remove_pid_file() {
     let directory = tempfile::tempdir().unwrap();
     let pid_path = directory.path().join("telemt.pid");
     fs::write(&pid_path, b"2000000000\n").unwrap();
-    let mut pid_file = PidFile::new(&pid_path);
+    let mut pid_file = PidFile::new(&pid_path, false);
 
     pid_file.release().unwrap();
 
@@ -169,10 +273,11 @@ fn acquire_rejects_pid_symlink_without_truncating_target() {
     let target_path = directory.path().join("target");
     fs::write(&target_path, b"preserve\n").unwrap();
     symlink(&target_path, &pid_path).unwrap();
-    let mut pid_file = PidFile::new(&pid_path);
-
-    assert!(pid_file.acquire().is_err());
-    assert_eq!(fs::read(&target_path).unwrap(), b"preserve\n");
+    for strict_runtime_paths in [false, true] {
+        let mut pid_file = PidFile::new(&pid_path, strict_runtime_paths);
+        assert!(pid_file.acquire().is_err());
+        assert_eq!(fs::read(&target_path).unwrap(), b"preserve\n");
+    }
 }
 
 #[test]
@@ -182,10 +287,11 @@ fn acquire_rejects_pid_hard_link_without_truncating_target() {
     let target_path = directory.path().join("target");
     fs::write(&target_path, b"preserve\n").unwrap();
     fs::hard_link(&target_path, &pid_path).unwrap();
-    let mut pid_file = PidFile::new(&pid_path);
-
-    assert!(pid_file.acquire().is_err());
-    assert_eq!(fs::read(&target_path).unwrap(), b"preserve\n");
+    for strict_runtime_paths in [false, true] {
+        let mut pid_file = PidFile::new(&pid_path, strict_runtime_paths);
+        assert!(pid_file.acquire().is_err());
+        assert_eq!(fs::read(&target_path).unwrap(), b"preserve\n");
+    }
 }
 
 #[test]
@@ -196,7 +302,7 @@ fn acquire_rejects_symlinked_parent_without_publishing_outside() {
     fs::create_dir(&real_parent).unwrap();
     symlink(&real_parent, &linked_parent).unwrap();
     let pid_path = linked_parent.join("telemt.pid");
-    let mut pid_file = PidFile::new(&pid_path);
+    let mut pid_file = PidFile::new(&pid_path, true);
 
     assert!(pid_file.acquire().is_err());
     assert!(!real_parent.join("telemt.pid").exists());
@@ -210,7 +316,7 @@ fn release_remains_anchored_after_parent_path_replacement() {
     let moved_parent = directory.path().join("moved");
     fs::create_dir(&active_parent).unwrap();
     let pid_path = active_parent.join("telemt.pid");
-    let mut pid_file = PidFile::new(&pid_path);
+    let mut pid_file = PidFile::new(&pid_path, false);
     pid_file.acquire().unwrap();
 
     fs::rename(&active_parent, &moved_parent).unwrap();
@@ -231,7 +337,7 @@ fn release_does_not_remove_replacement_path() {
     let directory = tempfile::tempdir().unwrap();
     let pid_path = directory.path().join("telemt.pid");
     let owned_path = directory.path().join("owned.pid");
-    let mut pid_file = PidFile::new(&pid_path);
+    let mut pid_file = PidFile::new(&pid_path, false);
     pid_file.acquire().unwrap();
     fs::rename(&pid_path, &owned_path).unwrap();
     fs::write(&pid_path, b"replacement\n").unwrap();
@@ -253,7 +359,7 @@ fn pid_parser_rejects_process_group_values() {
 
     for value in ["-1\n", "0\n", "1\n"] {
         fs::write(&pid_path, value).unwrap();
-        assert!(read_pid_file(&pid_path).is_err());
+        assert!(read_pid_file(&pid_path, false).is_err());
     }
 }
 
@@ -262,7 +368,7 @@ fn pid_file_release_keeps_lock_inode() {
     let directory = tempfile::tempdir().unwrap();
     let pid_path = directory.path().join("telemt.pid");
     let lock_path = sibling_lock_path(&pid_path);
-    let mut pid_file = PidFile::new(&pid_path);
+    let mut pid_file = PidFile::new(&pid_path, false);
 
     pid_file.acquire().unwrap();
     assert!(
@@ -271,7 +377,10 @@ fn pid_file_release_keeps_lock_inode() {
             .into_iter()
             .all(|file| file.is_some())
     );
-    assert_eq!(read_pid_file(&pid_path).unwrap(), std::process::id() as i32);
+    assert_eq!(
+        read_pid_file(&pid_path, false).unwrap(),
+        std::process::id() as i32
+    );
     let lock_inode = fs::metadata(&lock_path).unwrap().ino();
     pid_file.release().unwrap();