fix(cli): disclose orphaned temp path and correct --atomic help text (#537)

* fix(cli): correct stale --atomic help text for swap ordering and permissions

The atomic field's doc comment described the pre-#519 removal-before-rename
ordering and never mentioned #531's Unix parent-read-permission requirement.
Update it to match the current backup-then-swap-then-remove implementation.

Closes #532

* fix(cli): disclose surviving temp/backup directory after atomic-force failure

run_atomic_force_extraction creates its temp and backup directories with
path-based calls, and its best-effort cleanup on failure is also path-based.
If an intermediate component of the destination's parent is replaced with a
symlink while extraction is in progress, that cleanup can silently target a
decoy at the redirected location instead of the real directory, leaving
content behind with no indication of where.

Every failure site now captures each directory's (dev, ino) identity via
PinnedDir::entry_status right after creating it (fd-relative to the already-
pinned parent, so unaffected by the redirect), and re-checks that identity
after the cleanup attempt. When content survives, its current path is
resolved fresh from an fd opened on the entry itself rather than the
(possibly still-redirected) logical path, since the logical path does not
necessarily lead there anymore.

This does not, and cannot, discover content written directly to a
redirect-created decoy directory if the redirect reverts before this check
runs: exarch-core's own per-entry extraction writes are path-based, and
making them fd-relative is out of scope for this fix. That narrower case
remains a genuine, undisclosed orphan. Not a security escape and not new
data loss, only a disclosure gap in the error/warning text.

Closes #530

* docs(skills): sync exarch-cli SKILL.md with corrected --atomic behavior

The --atomic flag's skill table entry repeated the same stale
removal-before-rename description cli.rs's help text had (#532), and
didn't mention the temp/backup disclosure added for #530.

Refs #530, #532

Author: bug-ops

CHANGELOG.md

@@ -153,6 +153,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Changed
 
+- **`exarch-cli`: `extract --help`'s `--atomic` text described a stale swap ordering and omitted
+  the Unix permission requirement `--force` now has (#532)**: the `atomic` field's doc comment in
+  `cli.rs` said the existing destination is "removed after successful extraction (just before
+  rename), not before" — a description that predates the #519 fix and no longer matches
+  `run_atomic_force_extraction`'s actual swap ordering (old destination renamed aside first, new
+  content renamed into place second, old destination removed only once that swap succeeds). It also
+  never mentioned #531's Unix-only change requiring read permission on the destination's parent
+  directory. The doc comment now describes both accurately. No behavior change, `--help` text only.
 - **Duplicate-file, duplicate-symlink, and duplicate-hardlink skip paths now share a
   `common::checked_increment_files_skipped` helper (#518)** instead of repeating the same
   checked-add-with-overflow-guard increment inline in `formats::common::extract_file_with_permit`,
@@ -312,6 +320,43 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
+- **`exarch-cli`: `extract --atomic --force` did not disclose the location of a temp/backup
+  directory left behind by a parent-directory redirect mid-extraction (#530)**:
+  `run_atomic_force_extraction` creates its temp and backup directories with path-based calls
+  (`tempfile::tempdir_in`), and its best-effort cleanup on failure (`std::fs::remove_dir_all`) is
+  also path-based — if an intermediate component of the destination's parent is replaced with a
+  symlink while extraction is in progress, that cleanup call can silently target a decoy at the
+  redirected location instead of the real directory, leaving genuine content behind with no
+  indication of where. Every failure site in `run_atomic_force_extraction` now captures each
+  directory's `(dev, ino)` identity via `PinnedDir::entry_status` (fd-relative to the already-pinned
+  parent, so unaffected by the redirect) right after creating it, and after the best-effort cleanup
+  attempt, re-checks that identity: if the directory still exists and matches, its *current* path is
+  resolved fresh from a freshly opened fd on the entry itself (`PinnedDir::open_entry` +
+  `commands::atomic_swap::current_path`, using `/proc/self/fd` on Linux and `F_GETPATH` via
+  `rustix::fs::getpath` on macOS) and disclosed in the error message — critical, since the *logical*
+  path built from the (possibly still-redirected) parent does not necessarily lead there anymore,
+  confirmed by live-reproducing a persisting mid-extraction redirect and checking that the disclosed
+  path resolves to the real, surviving content rather than the decoy. If no fd-to-path facility is
+  available (any Unix other than Linux/macOS), falls back to naming the `(dev, ino)` identity with a
+  `find -inum` pointer instead of a path — Unix only, since `entry_status` always reports the
+  identity `(0, 0)` on other platforms, which would make that pointer meaningless there. Capturing the
+  backup directory's own identity (used only so a later failure to remove it can be disclosed) is
+  itself best-effort: by the time it runs, the destination has already been renamed into the backup's
+  place, so a failure there must not abort an otherwise-successful swap over and above what actually
+  failed — it degrades to skipping that one disclosure opportunity instead. In the ordinary
+  (non-redirected) case, cleanup succeeds and
+  the identity recheck correctly finds nothing, so no directory is disclosed and none is left behind
+  — this is deliberate: an earlier version of this fix instead persisted the temp directory
+  unconditionally, which left one behind on *every* failed `--atomic --force`, not just the redirect
+  race; the identity-recheck approach avoids that regression entirely. Non-Unix targets fall back to
+  naming the (possibly stale) logical path unconditionally when content survives, since
+  `PinnedDir::entry_status` cannot distinguish "our" directory from a replacement there (documented
+  residual, same as #526). This does not, and cannot, discover content written directly to a
+  redirect-created decoy directory if the redirect is reverted before this check runs — `exarch-core`'s
+  own per-entry extraction writes are path-based and out of scope for this fix to make fd-relative;
+  that narrower case remains a genuine, undisclosed orphan. Not a security escape (the fd-pinned swap
+  logic from #526/GHSA-x8wr-7ww2-c94x still confines renames/removes correctly) and not new data
+  loss, only a disclosure gap in the error/warning text.
 - **`exarch-core`: `verify`/`list` reported `PASS` for a TAR archive containing a non-UTF8 entry
   name that may fail to extract on filesystems requiring UTF-8 names (#528)**: TAR entry names are
   stored byte-exact (no lossy conversion) in `ArchiveEntry.path`, but

crates/exarch-cli/Cargo.toml

@@ -27,7 +27,7 @@ serde_json.workspace = true
 tempfile.workspace = true
 
 [target.'cfg(unix)'.dependencies]
-rustix = { workspace = true, features = ["fs"] }
+rustix = { workspace = true, features = ["fs", "alloc"] }
 
 [dev-dependencies]
 assert_cmd.workspace = true

crates/exarch-cli/src/cli.rs

@@ -114,10 +114,15 @@ pub struct ExtractArgs {
     pub force: bool,
 
     /// Extract atomically: use a temp dir, rename on success, clean up on
-    /// failure. When combined with --force, the existing destination is
-    /// removed after successful extraction (just before rename), not
-    /// before, to minimize the window where neither old nor new data
-    /// exists.
+    /// failure. When combined with --force to replace an existing
+    /// destination, the existing destination is renamed aside first, the
+    /// new content is renamed into its place second, and only once that
+    /// swap has succeeded is the old destination removed - never before, so
+    /// a failure at any point still leaves either the old or the new
+    /// content in place under the destination name. On Unix, --force also
+    /// requires read permission on the destination's parent directory (used
+    /// to pin it by file descriptor for the swap); the destination itself
+    /// does not need to be readable.
     #[arg(long)]
     pub atomic: bool,
 

crates/exarch-cli/src/commands/atomic_swap.rs

@@ -129,6 +129,63 @@ impl PinnedDir {
         rustix::fs::unlinkat(&self.0, name, rustix::fs::AtFlags::REMOVEDIR)?;
         Ok(())
     }
+
+    /// Opens the directory entry `name`, resolved relative to the pinned
+    /// directory, and returns an owned handle to it.
+    ///
+    /// Used by callers that need [`current_path`] on the specific entry
+    /// `name` identifies — resolving *its* current location in the
+    /// filesystem namespace, not `parent`'s — rather than anything this type
+    /// exposes about `name` itself. `NOFOLLOW` matches every other lookup in
+    /// this type: a symlink at `name` is rejected, not followed.
+    pub(super) fn open_entry(&self, name: &OsStr) -> io::Result<std::fs::File> {
+        let fd = rustix::fs::openat(
+            &self.0,
+            name,
+            rustix::fs::OFlags::RDONLY
+                | rustix::fs::OFlags::DIRECTORY
+                | rustix::fs::OFlags::CLOEXEC
+                | rustix::fs::OFlags::NOFOLLOW,
+            rustix::fs::Mode::empty(),
+        )?;
+        Ok(std::fs::File::from(fd))
+    }
+}
+
+/// Best-effort: `fd`'s current path in the filesystem namespace, resolved
+/// directly from the descriptor rather than any stored string — so it
+/// reflects wherever the entry actually is *now*, unaffected by a redirect
+/// of any ancestor path component since `fd` was opened. Used to disclose an
+/// accurate, walkable location for content [`PinnedDir::entry_status`] has
+/// already confirmed survives (issue #530): that confirmation alone doesn't
+/// guarantee the *logical* path built from `parent`'s (possibly still
+/// redirected) display path actually leads there — this does.
+///
+/// Returns `None` if this platform has no fd-to-path facility implemented
+/// here (anything other than Linux or macOS); callers must have a fallback,
+/// such as naming the logical path with a caveat, for that case.
+#[cfg(target_os = "linux")]
+pub(super) fn current_path(fd: &std::fs::File) -> Option<std::path::PathBuf> {
+    use std::os::fd::AsRawFd;
+    std::fs::read_link(format!("/proc/self/fd/{}", fd.as_raw_fd())).ok()
+}
+
+/// macOS implementation of [`current_path`] via `fcntl(fd, F_GETPATH)`, the
+/// platform's direct fd-to-path facility (Linux's `/proc/self/fd` has no
+/// macOS equivalent).
+#[cfg(target_os = "macos")]
+pub(super) fn current_path(fd: &std::fs::File) -> Option<std::path::PathBuf> {
+    use std::os::unix::ffi::OsStrExt;
+    rustix::fs::getpath(fd)
+        .ok()
+        .map(|c_path| std::path::PathBuf::from(std::ffi::OsStr::from_bytes(c_path.as_bytes())))
+}
+
+/// Fallback for Unix targets with no fd-to-path facility implemented here:
+/// no accurate current path is available.
+#[cfg(all(unix, not(any(target_os = "linux", target_os = "macos"))))]
+pub(super) fn current_path(_fd: &std::fs::File) -> Option<std::path::PathBuf> {
+    None
 }
 
 /// Non-Unix fallback: plain path-based operations.
@@ -188,6 +245,27 @@ impl PinnedDir {
     pub(super) fn remove_dir(&self, name: &OsStr) -> io::Result<()> {
         std::fs::remove_dir(self.parent.join(name))
     }
+
+    /// Opens the directory entry `name`, resolved by joining `self.parent`.
+    ///
+    /// Path-based, same residual as every other operation on this platform's
+    /// `PinnedDir`. Kept for signature parity with the Unix impl so callers
+    /// never need to branch on platform; its result feeds into
+    /// [`current_path`], which always returns `None` on this platform
+    /// regardless of whether this call succeeds.
+    pub(super) fn open_entry(&self, name: &OsStr) -> io::Result<std::fs::File> {
+        std::fs::File::open(self.parent.join(name))
+    }
+}
+
+/// Non-Unix fallback for [`current_path`]: no fd-to-path facility is
+/// implemented here, so this always returns `None` — the same "documented
+/// residual, not a false guarantee" pattern as [`PinnedDir`]'s non-Unix
+/// `entry_status` always reporting `(0, 0)`. Callers fall back to naming the
+/// logical path instead.
+#[cfg(not(unix))]
+pub(super) fn current_path(_fd: &std::fs::File) -> Option<std::path::PathBuf> {
+    None
 }
 
 /// Reports whether `metadata` carries `FILE_ATTRIBUTE_REPARSE_POINT`.
@@ -216,6 +294,7 @@ fn is_reparse_point(metadata: &std::fs::Metadata) -> bool {
 mod tests {
     use super::DestEntryKind;
     use super::PinnedDir;
+    use super::current_path;
     use std::ffi::OsStr;
     use std::os::unix::fs::PermissionsExt;
     use std::os::unix::fs::symlink;
@@ -413,4 +492,89 @@ mod tests {
             ),
         }
     }
+
+    /// Regression test for issue #530, round 3: `current_path` must resolve
+    /// an entry's *current* location from its own fd, not the path used to
+    /// open it. Proven directly here — no timing race needed, since the
+    /// redirect only has to happen *between* `open_entry` and `current_path`,
+    /// not concurrently with either — by the same
+    /// move-aside-and-symlink-over technique
+    /// `rename_follows_the_pinned_inode_not_a_planted_symlink` uses for
+    /// `rename`. This is the property `survival_clause` (in `extract.rs`)
+    /// depends on to disclose the real surviving location instead of a
+    /// decoy or nothing.
+    #[test]
+    #[cfg(any(target_os = "linux", target_os = "macos"))]
+    fn current_path_resolves_through_a_redirected_parent() {
+        let root = tempfile::tempdir().expect("tempdir");
+        let real_parent = root.path().join("real_parent");
+        std::fs::create_dir(&real_parent).expect("create real_parent");
+        std::fs::create_dir(real_parent.join("entry")).expect("create entry");
+
+        let pin = PinnedDir::open(&real_parent).expect("pin real_parent");
+        let fd = pin
+            .open_entry(OsStr::new("entry"))
+            .expect("open entry via pinned fd");
+
+        // Redirect the path `real_parent` used to name: move the real
+        // directory aside and put a symlink to a decoy in its place.
+        let decoy = root.path().join("decoy");
+        std::fs::create_dir(&decoy).expect("create decoy");
+        let moved_real_parent = root.path().join("real_parent_moved");
+        std::fs::rename(&real_parent, &moved_real_parent).expect("move real parent aside");
+        symlink(&decoy, &real_parent).expect("plant symlink at the old path");
+
+        let resolved = current_path(&fd).expect("current_path must resolve on this platform");
+
+        // `current_path` resolves through every symlink in the filesystem
+        // (e.g. macOS's `/tmp` -> `/private/tmp`), so the expected side must
+        // be canonicalized the same way before comparing — the property
+        // under test is "reflects the real, moved location", not "matches
+        // the exact string `root.path()` happened to be built from".
+        let expected = moved_real_parent
+            .join("entry")
+            .canonicalize()
+            .expect("canonicalize expected path");
+        assert_eq!(
+            resolved, expected,
+            "current_path must reflect the entry's real, moved location, not the stale logical \
+             path or the decoy"
+        );
+    }
+
+    /// `open_entry` must fail closed (`NOFOLLOW`) rather than follow a
+    /// symlink planted at `name`, the same rejection `entry_status` already
+    /// applies to its own lookup. An fd opened by following a symlink here
+    /// would make `current_path` resolve to wherever the symlink's target
+    /// currently is, not the entry `disclose_if_orphaned` is actually
+    /// tracking.
+    #[test]
+    fn open_entry_rejects_a_planted_symlink() {
+        let root = tempfile::tempdir().expect("tempdir");
+        let parent = root.path().join("parent");
+        std::fs::create_dir(&parent).expect("create parent");
+        let real_dir = root.path().join("real_dir");
+        std::fs::create_dir(&real_dir).expect("create real_dir");
+        let link = parent.join("link");
+        symlink(&real_dir, &link).expect("plant symlink");
+
+        let pin = PinnedDir::open(&parent).expect("pin parent");
+        let err = pin
+            .open_entry(OsStr::new("link"))
+            .expect_err("a symlink entry must be rejected, not followed");
+        // The specific errno is platform-dependent: combined with
+        // `O_DIRECTORY`, Linux's `open(2)` still reports `ELOOP` for a
+        // `NOFOLLOW`'d symlink, while macOS/BSD report `ENOTDIR` instead
+        // (`io::ErrorKind::FilesystemLoop` itself is also still unstable —
+        // `io_error_more`, rust-lang/rust#86442 — hence comparing the raw
+        // errno rather than the `ErrorKind`). Either is acceptable here: what
+        // matters is that the open fails closed rather than following the
+        // symlink into a real, followable directory fd.
+        let raw = err.raw_os_error();
+        assert!(
+            raw == Some(rustix::io::Errno::LOOP.raw_os_error())
+                || raw == Some(rustix::io::Errno::NOTDIR.raw_os_error()),
+            "expected NOFOLLOW to reject the symlink with ELOOP or ENOTDIR, got: {err}"
+        );
+    }
 }