Make POSIX stale endpoint recovery liveness-only

Stale UDS socket and SHM region recovery was gated on run-directory
permissions: automatic unlink was refused unless the run dir was owned
by the process euid and not group/other-writable. netdata's systemd
unit ships RuntimeDirectoryMode=0775, so on every standard install a
crashed server could never reclaim its endpoint on restart — the
service stayed AddrInUse until manual cleanup or reboot. The guard
came from a CodeQL TOCTOU alert and assumed hostile writers in a
shared directory; the run dir is the embedding service's private
runtime directory, so that threat model does not apply.

Liveness is now the only criterion, in C, Rust, and Go, for both
transports: a live server's endpoint (SHM owner PID alive with
non-zero generation; UDS connect probe succeeds) is never deleted and
a second server gets address-in-use. Everything else found at an
endpoint name — dead server leftovers, foreign files, symlinks,
unreadable entries, empty directories — is silently reclaimed so the
service starts. The one exception: fd exhaustion (EMFILE/ENFILE)
during the liveness check keeps the entry, since liveness could not
be evaluated and deleting could remove a live endpoint.

Descriptor-relative fstatat/unlinkat cleanup mechanics are kept.
Tests updated across all three languages, with new coverage for
crash recovery in 0775 run dirs and foreign-file reclaim; the public
specs and the integrator skill describe the new contract. Tracked as
SOW-0018; supersedes the SOW-0017 test-side umask workaround.

Author: ktsaou

.agents/sow/done/SOW-0018-20260610-liveness-only-stale-recovery.md

@@ -246,20 +246,24 @@ When a session closes (graceful or broken):
 Both `server_create` (per-session) and `cleanup_stale` (startup scan)
 use the same stale detection logic:
 
-1. Verify `{run_dir}` is safe for automatic stale unlink: owned by the
-   effective process user and not group- or world-writable. If the directory is
-   unsafe, stale entries are left in place and treated as in-use.
-2. Open the file and mmap the header. If `open()` fails with a
-   permission error (EACCES, EPERM), the file is left in place — it
-   may belong to another user or process.
-3. Validate magic. If invalid or file is undersized: stale — unlink only when
-   `{run_dir}` passed the safety check.
-4. Check `owner_pid` and `owner_generation`:
+1. Open the file and mmap the header. If `open()` fails with ENOENT,
+   nothing is there. If it fails with EMFILE/ENFILE (fd exhaustion),
+   liveness cannot be evaluated — the entry is kept and treated as
+   in-use. Any other open failure (unreadable file, symlink, foreign
+   junk at the endpoint name) marks the entry as junk — silently unlink.
+2. Validate magic. If invalid, or the file is undersized or not a
+   regular file: junk — silently unlink.
+3. Check `owner_pid` and `owner_generation`:
    - If `owner_pid` is alive AND `owner_generation` is non-zero:
-     the region is live — leave it.
+     the region is live — leave it, report address-in-use.
    - Otherwise (PID dead, or generation is zero indicating an
-     uninitialized/legacy region): stale — unlink only when `{run_dir}` passed
-     the safety check.
+     uninitialized/legacy region): stale — silently unlink.
+
+Liveness is the only criterion: a live server's endpoint is never
+deleted, and everything else found at an endpoint name is reclaimed so
+the service can start. Run-directory permissions do not gate stale
+recovery — the run dir is expected to be the embedding service's
+private runtime directory.
 
 The `owner_generation` check catches PID reuse: a new process that
 reuses an old PID will not have the same generation value. A zero

docs/level1-posix-shm.md

@@ -118,19 +118,25 @@ and therefore a different SHM file path.
 - Server checks if the socket path exists before bind.
 - If it exists: attempt to connect to it. If connection succeeds, a
   live server owns it — fail with address-in-use. If connection fails
-  with ECONNREFUSED or ENOENT, the socket is stale — unlink and
-  recreate. Other connect errors (EACCES, EPERM, etc.) are not treated
-  as stale — the file is left in place and the server fails.
+  with ENOENT, nothing is there. Any other connect failure
+  (ECONNREFUSED, EACCES, a foreign file squatting on the socket path,
+  etc.) means the endpoint is not a live server — it is silently
+  unlinked and recreated.
+- The only exception is fd exhaustion (EMFILE/ENFILE) while creating
+  the probe socket: liveness cannot be evaluated, so the endpoint is
+  kept and the server fails with address-in-use rather than risk
+  deleting a live socket.
 
 ### SHM file
 
 - Server checks the `.ipcshm` file for ownership metadata (PID and
   generation) stored in the SHM region header.
 - If the owner PID is alive and the generation matches, the region is
   active — fail with address-in-use.
-- If the owner PID is dead or the region is invalid/undersized, the
-  region is stale — unlink and recreate. If the file cannot be opened
-  due to permission errors (EACCES, EPERM), it is left in place.
+- If the owner PID is dead or the region is invalid/undersized/unreadable,
+  the region is stale or junk — silently unlink and recreate. Only fd
+  exhaustion (EMFILE/ENFILE) keeps the file in place, because liveness
+  could not be evaluated.
 - Clients that open an undersized/unpopulated SHM file (server has
   created it but not yet initialized the header) treat it as a
   retryable protocol-not-ready condition.

docs/level1-posix-uds.md

@@ -531,12 +531,17 @@ stale if a server process crashes without cleanup. Level 1 handles this:
   it (unlinks and recreates).
 - On listen: if an active endpoint exists from a live process, Level 1 fails
   with an appropriate error (address already in use).
-- On POSIX: automatic stale unlink is allowed only when `run_dir` is owned by
-  the effective process user and is not group- or world-writable. In unsafe
-  shared directories, stale entries are treated as in-use so Level 1 does not
-  unlink paths that another local user could race or replace.
+- Liveness is the only criterion: anything found at an endpoint name that is
+  not a live server (dead server's endpoint, foreign file accidentally placed
+  there, invalid or unreadable entry) is silently reclaimed so the service
+  can start. The one exception is fd exhaustion during the liveness check
+  itself — then the entry is kept and reported as in-use, so a live endpoint
+  is never deleted on an unverifiable check.
 - Stale detection uses process ownership metadata (PID and generation) to
   avoid false reclamation due to PID reuse.
+- `run_dir` is expected to be the embedding service's private runtime
+  directory (e.g. netdata's run dir); its permissions do not gate stale
+  recovery.
 
 ## Testing requirements
 

docs/level1-transport.md

@@ -286,9 +286,9 @@ Netdata-specific integration rules:
 - use `os_run_dir(true)` on the provider side
 - use `os_run_dir(false)` on the consumer side
 - treat these as a matched pair
-- keep the provider runtime directory service-owned and not group- or
-  world-writable; POSIX stale cleanup will not unlink old socket/SHM paths from
-  unsafe shared directories
+- stale socket/SHM paths from dead processes are reclaimed automatically on
+  server start, regardless of run-dir permissions; only a live server's
+  endpoint blocks a new server (address-in-use)
 - if provider and consumer do not agree on auth token or run dir, the client
   will stay in `AUTH_FAILED` or `NOT_FOUND`
 

docs/netipc-integrator-skill.md

@@ -14,11 +14,7 @@ const RESPONSE_BUF_SIZE: usize = 65536;
 static SERVICE_COUNTER: AtomicU64 = AtomicU64::new(0);
 
 fn ensure_run_dir() {
-    use std::os::unix::fs::PermissionsExt;
     let _ = std::fs::create_dir_all(TEST_RUN_DIR);
-    // The stale-unlink guard refuses group/other-writable run dirs; pin the
-    // mode so the process umask cannot decide test outcomes.
-    let _ = std::fs::set_permissions(TEST_RUN_DIR, std::fs::Permissions::from_mode(0o700));
 }
 
 fn unique_service(prefix: &str) -> String {

src/crates/netipc/src/service/cgroups_unix_tests.rs

@@ -20,11 +20,7 @@ const RESPONSE_BUF_SIZE: usize = 65536;
 static RAW_SERVICE_COUNTER: std::sync::atomic::AtomicU64 = std::sync::atomic::AtomicU64::new(0);
 
 fn ensure_run_dir() {
-    use std::os::unix::fs::PermissionsExt;
     let _ = std::fs::create_dir_all(TEST_RUN_DIR);
-    // The stale-unlink guard refuses group/other-writable run dirs; pin the
-    // mode so the process umask cannot decide test outcomes.
-    let _ = std::fs::set_permissions(TEST_RUN_DIR, std::fs::Permissions::from_mode(0o700));
 }
 
 fn cleanup_all(service: &str) {
@@ -1160,7 +1156,10 @@ fn test_server_falls_back_to_baseline_when_linux_shm_prepare_fails() {
 
     let shm_path = format!("{TEST_RUN_DIR}/{svc}-{:016x}.ipcshm", 1u64);
     let _ = std::fs::remove_dir_all(&shm_path);
+    // A non-empty directory cannot be reclaimed by stale recovery, so SHM
+    // prepare keeps failing and the server must fall back to baseline.
     std::fs::create_dir(&shm_path).expect("create SHM obstruction directory");
+    std::fs::write(format!("{shm_path}/keep"), b"x").expect("populate obstruction");
 
     let mut server = TestServer::start_with(
         svc,

src/crates/netipc/src/service/raw_unix_tests.rs

@@ -12,7 +12,6 @@ use crate::protocol::{
 use std::collections::HashSet;
 use std::ffi::CString;
 use std::io;
-use std::os::unix::fs::MetadataExt;
 use std::os::unix::io::RawFd;
 use std::path::{Path, PathBuf};
 use std::sync::atomic::{AtomicU64, Ordering};
@@ -602,7 +601,7 @@ impl UdsListener {
         let path = build_socket_path(run_dir, service_name)?;
 
         // Stale recovery
-        match check_and_recover_stale(&path, run_dir_allows_stale_unlink(run_dir)) {
+        match check_and_recover_stale(&path) {
             StaleResult::LiveServer => return Err(UdsError::AddrInUse),
             StaleResult::Stale | StaleResult::NotExist => { /* proceed */ }
         }
@@ -919,25 +918,17 @@ enum StaleResult {
     LiveServer,
 }
 
-fn run_dir_allows_stale_unlink(run_dir: &str) -> bool {
-    let metadata = match std::fs::metadata(run_dir) {
-        Ok(m) => m,
-        Err(_) => return false,
-    };
-    metadata.is_dir()
-        && metadata.uid() == unsafe { libc::geteuid() }
-        && metadata.mode() & 0o022 == 0
-}
-
-fn check_and_recover_stale(path: &str, allow_stale_unlink: bool) -> StaleResult {
+fn check_and_recover_stale(path: &str) -> StaleResult {
     if !Path::new(path).exists() {
         return StaleResult::NotExist;
     }
 
     // Try connecting to see if a live server is there
     let probe = unsafe { libc::socket(libc::AF_UNIX, libc::SOCK_SEQPACKET, 0) };
     if probe < 0 {
-        return StaleResult::NotExist;
+        // Cannot probe liveness (fd exhaustion) — keep the endpoint; the
+        // subsequent bind() fails instead of risking a live socket.
+        return StaleResult::LiveServer;
     }
 
     let result = match connect_unix(probe, path) {
@@ -946,23 +937,21 @@ fn check_and_recover_stale(path: &str, allow_stale_unlink: bool) -> StaleResult
             StaleResult::LiveServer
         }
         Err(UdsError::Connect(e)) if e == libc::ENOENT => StaleResult::NotExist,
-        Err(UdsError::Connect(e)) if e == libc::ECONNREFUSED => {
-            if !allow_stale_unlink {
-                StaleResult::LiveServer
-            } else {
-                // Connection refused means stale; unlink only in a private run dir.
-                match std::fs::remove_file(path) {
-                    Ok(()) => StaleResult::Stale,
-                    Err(err) if err.kind() == io::ErrorKind::NotFound => StaleResult::NotExist,
-                    Err(_) => StaleResult::LiveServer,
+        Err(_) => {
+            // Nothing accepted the connection: a dead server's socket, or a
+            // foreign file squatting on the endpoint path. Reclaim it.
+            match std::fs::remove_file(path) {
+                Ok(()) => StaleResult::Stale,
+                Err(err) if err.kind() == io::ErrorKind::NotFound => StaleResult::NotExist,
+                Err(err) if err.raw_os_error() == Some(libc::EISDIR) => {
+                    match std::fs::remove_dir(path) {
+                        Ok(()) => StaleResult::Stale,
+                        Err(_) => StaleResult::LiveServer,
+                    }
                 }
+                Err(_) => StaleResult::LiveServer,
             }
         }
-        Err(_) => {
-            // Other errors (EACCES, etc.) — can't determine ownership,
-            // treat as live to prevent overwriting
-            StaleResult::LiveServer
-        }
     };
 
     unsafe {

src/crates/netipc/src/transport/posix.rs

@@ -9,11 +9,7 @@ const TEST_RUN_DIR: &str = "/tmp/nipc_rust_test";
 const AUTH_TOKEN: u64 = 0xDEADBEEFCAFEBABE;
 
 fn ensure_run_dir() {
-    use std::os::unix::fs::PermissionsExt;
     let _ = std::fs::create_dir_all(TEST_RUN_DIR);
-    // The stale-unlink guard refuses group/other-writable run dirs; pin the
-    // mode so the process umask cannot decide test outcomes.
-    let _ = std::fs::set_permissions(TEST_RUN_DIR, std::fs::Permissions::from_mode(0o700));
 }
 
 fn cleanup_socket(service: &str) {
@@ -477,20 +473,14 @@ fn test_request_payload_over_cap() {
 //  Test 7: Stale socket recovery
 // -----------------------------------------------------------------------
 
-#[test]
-fn test_stale_recovery() {
-    ensure_run_dir();
-    let svc = unique_service("rs_stale");
-    cleanup_socket(&svc);
-
-    let path = format!("{TEST_RUN_DIR}/{svc}.sock");
-
-    // Create a stale socket file (bound but not listening)
+/// Create a socket file that is bound but not listening — what a crashed
+/// server leaves behind.
+fn create_stale_socket_file(path: &str) {
     unsafe {
         let sock = libc::socket(libc::AF_UNIX, libc::SOCK_SEQPACKET, 0);
         assert!(sock >= 0);
 
-        let c_path = CString::new(path.as_str()).unwrap();
+        let c_path = CString::new(path).unwrap();
         let mut addr: libc::sockaddr_un = std::mem::zeroed();
         addr.sun_family = libc::AF_UNIX as libc::sa_family_t;
         let path_bytes = c_path.as_bytes_with_nul();
@@ -505,7 +495,16 @@ fn test_stale_recovery() {
         // Close without unlink => stale
         libc::close(sock);
     }
+}
 
+#[test]
+fn test_stale_recovery() {
+    ensure_run_dir();
+    let svc = unique_service("rs_stale");
+    cleanup_socket(&svc);
+
+    let path = format!("{TEST_RUN_DIR}/{svc}.sock");
+    create_stale_socket_file(&path);
     assert!(Path::new(&path).exists(), "stale socket should exist");
 
     // listen should recover it
@@ -515,6 +514,41 @@ fn test_stale_recovery() {
     cleanup_socket(&svc);
 }
 
+#[test]
+fn test_stale_recovery_in_group_writable_run_dir() {
+    use std::os::unix::fs::PermissionsExt;
+    // Crash recovery must work regardless of run-dir permissions
+    // (netdata's systemd unit ships RuntimeDirectoryMode=0775).
+    let dir = format!("{TEST_RUN_DIR}_0775");
+    let _ = std::fs::create_dir_all(&dir);
+    std::fs::set_permissions(&dir, std::fs::Permissions::from_mode(0o775)).expect("chmod 0775");
+
+    let svc = unique_service("rs_stale_gw");
+    let path = format!("{dir}/{svc}.sock");
+    let _ = std::fs::remove_file(&path);
+    create_stale_socket_file(&path);
+
+    let listener = UdsListener::bind(&dir, &svc, default_server_config())
+        .expect("stale recovery must work in a group-writable run dir");
+    drop(listener);
+    let _ = std::fs::remove_file(&path);
+}
+
+#[test]
+fn test_foreign_file_at_socket_path_is_reclaimed() {
+    ensure_run_dir();
+    let svc = unique_service("rs_foreign");
+    cleanup_socket(&svc);
+
+    let path = format!("{TEST_RUN_DIR}/{svc}.sock");
+    std::fs::write(&path, b"accidentally copied file").expect("write foreign file");
+
+    let listener = UdsListener::bind(TEST_RUN_DIR, &svc, default_server_config())
+        .expect("listen should silently replace a foreign file at the socket path");
+    drop(listener);
+    cleanup_socket(&svc);
+}
+
 // -----------------------------------------------------------------------
 //  Test 8: Disconnect detection
 // -----------------------------------------------------------------------