fix(sandbox): strip " (deleted)" suffix from unlinked /proc/<pid>/exe paths

When a running binary is unlinked from its filesystem path — the common
case is a `docker cp` hot-swap of `/opt/openshell/bin/openshell-sandbox`
during the `cluster-deploy-fast` dev upgrade workflow — the Linux kernel
appends the literal string ` (deleted)` to the `/proc/<pid>/exe` readlink
target. The tainted `PathBuf` then flows into `collect_ancestor_binaries`
and on into `BinaryIdentityCache::verify_or_cache`, which tries to
`std::fs::metadata` the path. `stat()` fails with `ENOENT` because the
literal suffix isn't a real filesystem path, and the CONNECT proxy denies
every outbound request with:

    ancestor integrity check failed for \
      /opt/openshell/bin/openshell-sandbox (deleted): \
      Failed to stat ...: No such file or directory (os error 2)

Reproduced in production 2026-04-15: a cluster-deploy-fast-style hot-swap
of the supervisor binary caused every pod whose PID 1 held the now-deleted
inode to deny ALL outbound CONNECTs (slack.com, registry.npmjs.org,
169.254.169.254, etc.), breaking Slack REST delivery, npm installs, and
IMDS probes simultaneously. Existing pre-hot-swap TCP tunnels (e.g. Slack
Socket Mode WSS) kept working because they never re-evaluate the proxy.

Strip the suffix in `binary_path()` so downstream callers see the clean,
grep-friendly path. This aligns the cache key and log messages with the
original on-disk location.

Note: stripping the suffix does NOT by itself make the identity cache
tolerant of a legitimate binary replacement — `verify_or_cache` will now
`stat` and hash whatever currently lives at the stripped path, which is
the NEW binary, and surface a clearer `Binary integrity violation` error.
Fully unblocking the cluster-deploy-fast hot-swap workflow needs a
follow-up that either (a) reads running-binary content from
`/proc/<pid>/exe` directly via `File::open` (procfs resolves this to the
live in-memory executable even when the original inode has been unlinked),
or (b) keys the identity cache by exec dev+inode instead of path. Happy to
send that as a separate PR once the approach is decided — filing this
narrow fix first because it stands on its own: it fixes a concrete
misleading error and unblocks the obvious next step.

Added `binary_path_strips_deleted_suffix` test that copies `/bin/sleep`
to a temp path, spawns a child from it, unlinks the temp binary, verifies
the raw readlink contains the ` (deleted)` suffix, then asserts the public
API returns the stripped path.

Signed-off-by: mjamiv <michael.commack@gmail.com>

Author: mjamiv

crates/openshell-sandbox/src/procfs.rs

@@ -19,17 +19,43 @@ use tracing::debug;
 /// `/proc/{pid}/cmdline` because `argv[0]` is trivially spoofable by any
 /// process and must not be used as a trusted identity source.
 ///
-/// If this fails, ensure the proxy process has permission to read
-/// `/proc/<pid>/exe` (e.g. same user, or `CAP_SYS_PTRACE`).
+/// ### Unlinked binaries (`(deleted)` suffix)
+///
+/// When a running binary is unlinked from its filesystem path — the common
+/// case is a `docker cp` hot-swap of `/opt/openshell/bin/openshell-sandbox`
+/// during a `cluster-deploy-fast` dev upgrade — the kernel appends the
+/// literal string `" (deleted)"` to the `/proc/<pid>/exe` readlink target.
+/// The raw tainted path (e.g. `"/opt/openshell/bin/openshell-sandbox (deleted)"`)
+/// is not a real filesystem path: any downstream `stat()` fails with `ENOENT`.
+///
+/// We strip the suffix so callers see a clean, grep-friendly path suitable
+/// for cache keys and log messages. This does NOT claim the file at the
+/// stripped path is the same binary that the process is executing — the
+/// on-disk inode may now be arbitrary. Callers that need to verify the
+/// running binary's *contents* (for integrity checking) should read the
+/// magic `/proc/<pid>/exe` symlink directly via `File::open`, which procfs
+/// resolves to the live in-memory executable even when the original inode
+/// has been unlinked.
+///
+/// If the readlink itself fails, ensure the proxy process has permission
+/// to read `/proc/<pid>/exe` (e.g. same user, or `CAP_SYS_PTRACE`).
 #[cfg(target_os = "linux")]
 pub fn binary_path(pid: i32) -> Result<PathBuf> {
-    std::fs::read_link(format!("/proc/{pid}/exe")).map_err(|e| {
+    let link = format!("/proc/{pid}/exe");
+    let target = std::fs::read_link(&link).map_err(|e| {
         miette::miette!(
             "Failed to read /proc/{pid}/exe: {e}. \
              Cannot determine binary identity — denying request. \
              Hint: the proxy may need CAP_SYS_PTRACE or to run as the same user."
         )
-    })
+    })?;
+
+    // Strip the " (deleted)" suffix the kernel appends for unlinked binaries.
+    // See the doc comment above for the full rationale.
+    if let Some(stripped) = target.to_str().and_then(|s| s.strip_suffix(" (deleted)")) {
+        return Ok(PathBuf::from(stripped));
+    }
+    Ok(target)
 }
 
 /// Resolve the binary path of the TCP peer inside a sandbox network namespace.
@@ -391,6 +417,59 @@ mod tests {
         assert!(path.exists());
     }
 
+    /// Verify that an unlinked binary's path is returned without the
+    /// kernel's " (deleted)" suffix. This is the common case during a
+    /// `docker cp` hot-swap of the supervisor binary — before this strip,
+    /// callers that `stat()` the returned path get `ENOENT` and the
+    /// ancestor integrity check in the CONNECT proxy denies every request.
+    #[cfg(target_os = "linux")]
+    #[test]
+    fn binary_path_strips_deleted_suffix() {
+        use std::os::unix::fs::PermissionsExt;
+
+        // Copy /bin/sleep to a temp path we control so we can unlink it.
+        let tmp = tempfile::TempDir::new().unwrap();
+        let exe_path = tmp.path().join("deleted-sleep");
+        std::fs::copy("/bin/sleep", &exe_path).unwrap();
+        std::fs::set_permissions(&exe_path, std::fs::Permissions::from_mode(0o755)).unwrap();
+
+        // Spawn a child from the temp binary, then unlink it while the
+        // child is still running. The child keeps the exec mapping via
+        // `/proc/<pid>/exe`, but readlink will now return the tainted
+        // "<path> (deleted)" string.
+        let mut child = std::process::Command::new(&exe_path)
+            .arg("5")
+            .spawn()
+            .unwrap();
+        let pid: i32 = child.id().cast_signed();
+        std::fs::remove_file(&exe_path).unwrap();
+
+        // Sanity check: the raw readlink should contain " (deleted)".
+        let raw = std::fs::read_link(format!("/proc/{pid}/exe"))
+            .unwrap()
+            .to_string_lossy()
+            .into_owned();
+        assert!(
+            raw.ends_with(" (deleted)"),
+            "kernel should append ' (deleted)' to unlinked exe readlink; got {raw:?}"
+        );
+
+        // The public API should return the stripped path, not the tainted one.
+        let resolved = binary_path(pid).expect("binary_path should succeed for deleted binary");
+        assert_eq!(
+            resolved, exe_path,
+            "binary_path should strip the ' (deleted)' suffix"
+        );
+        let resolved_str = resolved.to_string_lossy();
+        assert!(
+            !resolved_str.contains("(deleted)"),
+            "stripped path must not contain '(deleted)'; got {resolved_str:?}"
+        );
+
+        let _ = child.kill();
+        let _ = child.wait();
+    }
+
     #[cfg(target_os = "linux")]
     #[test]
     fn collect_descendants_includes_self() {