Skip to content

Commit 52dfc56

Browse files
committed
📝 Add implementation plan for Windows junction support
📝 Document junction handling in --allow-unsafe-links The extract path gates Windows junction creation on --allow-unsafe-links (see `cli/src/command/extract.rs` junction arm), but the flag's help text still advertised only "symbolic links and hard links". A user reading `pna extract --help` or `pna compat bsdtar --help` today would not know the flag also covers NTFS junctions. Rewrite the four help strings (positive/negative x extract/bsdtar) to list symbolic links, hard links, and Windows junctions together, and regenerate the committed CLI reference so docs stay in sync. 🐛 Pass --unstable alongside --keep-permission on Windows Windows CI surfaced that `extract_junction_does_not_mutate_external_target` panics with `MissingRequiredArgument: --unstable` on the `--no-default-features` build of the `extract` subcommand. On Windows, `--keep-permission` (and related metadata-restoration flags) is gated behind `--unstable` per `cli/src/command/extract.rs:149-150` (`#[cfg_attr(windows, arg(requires = "unstable", ...))]`). Add `#[cfg(windows)] "--unstable"` to the test's CLI args, matching the peer pattern in `cli/tests/cli/keep_all.rs` and `cli/tests/cli/diff/metadata.rs`. Plan mirrored. 🐛 Load-bearing I2 fence: stamp Permission chunk in junction fixture The Task 4.4 spec-compliance review empirically proved the I2 security fence was decorative: a regression that swaps `restore_link_timestamps_no_follow` for the default `restore_metadata` call slipped through the `extract_junction_does_not_mutate_external_target` assertion. Root cause: `build_junction_fixture` produced a junction entry with no Permission chunk, so `restore_metadata`'s chmod/chown branch (gated on `item.metadata().permission().is_some()`) never entered — no follow-link syscall ever touched the target, no matter what the extract code did. Extend the fixture helper surface: - `build_junction_fixture_with_permission(target, mode)` stamps a Permission chunk with the given mode so extract's `restore_mode` actually fires when `--keep-permission` is passed. - `build_junction_fixture` continues to produce a Permission-less entry for the other two tests that don't exercise the mode path. - Both delegate to a shared `build_junction_fixture_with_optional_permission` to avoid duplication. Retarget the I2 test to `build_junction_fixture_with_permission(..., 0o755)` while still pre-setting the external directory to 0o700. Under a regression that removes Task 4.1's early return, extract now calls `chmod(link, 0o755)` which follows the symlink and changes the external target's mode to 0o755, firing `baseline_perms != after_perms` with the "I2 violation" message. Empirically verified by reverting Task 4.1's early return in a throwaway experiment: the fence fires with `left 0o040700, right 0o040755`. With the early return restored, the fence passes. Plan Task 4.4 Step 1 code block mirrors the new helper surface. ✅ Pin junction extract does-not-mutate-target invariant 🎨 Hoist file_type() call in junction round-trip test Match the neighboring `extract_junction_with_allow_unsafe_links_creates_link` test's `let ft = meta.file_type();` hoist, avoiding three separate `file_type()` calls inside a single `assert!`. Pure style cleanup flagged by the Task 4.3 code-quality review. ✅ End-to-end junction round trip via CLI 🐛 Gate junction round-trip test off WASM CI surfaced that `extract_junction_with_allow_unsafe_links_creates_link` panics under `wasm32-wasip1` with "Operation not permitted (os error 63)" because wasi-preview1 does not expose symlink creation. The fallback path `pna::fs::symlink` therefore fails when the test tries to materialize the non-Windows junction-fallback symlink. Add `#[cfg(not(target_family = "wasm"))]` to the test, matching the pattern established by `cli/tests/cli/extract/hardlink.rs:95` and peers which gate off WASM for the same reason. The skip-case test (`extract_junction_without_allow_unsafe_links_skips`) is not gated because it never reaches the symlink-creation path. Plan Task 4.2 Step 1 code block mirrors the gate. ✅ Cover junction extract round-trip across platforms 🎨 Polish Task 4.1 extract branch per code-quality review Address all four Minor nits flagged by the Task 4.1 code-quality review; none affect Invariant I2 correctness. - Junction skip warning: drop the internal encoding term "HardLink+fLTP=Directory" and match the peer symlink/hardlink voice ("If you need to extract it, use `--allow-unsafe-links`."). - Non-Windows fallback warning: demote to `log::debug!`. The per-entry `warn!` would spam on large Windows-origin archives extracted on Unix; the degradation (junction → symlink) is a known platform limitation, best surfaced at debug level. - `create_junction_or_fallback` canonicalize: replace `unwrap_or` with an explicit match that logs the underlying error at `debug!` level before falling back to the raw join. Prevents permission-denied / ELOOP failures from being hidden behind `create_junction`'s generic "target must be absolute" message. - Test pattern: switch `extract_junction_without_allow_unsafe_links_skips` to the peer CLI test convention (`utils::setup()` + relative `"{base}/{base}.pna"` paths + in-process `cli::Cli::try_parse_from`). Drops the subprocess invocation and the macOS `canonicalize` workaround, and aligns with existing peer tests in `cli/tests/cli/extract/`. The stderr assertion is dropped because the filesystem assertion ("link_dir does not exist") is the load-bearing check. Plan Task 4.1 Step 2 and Step 4 mirror the final code. ✨ Extract HardLink+fLTP=Directory as junction or symlink fallback ♻️ Route junction target through PathnameEditor::edit_junction The Task 3.1 code-quality review flagged that the `StoreAs::Junction` arm in `create_entry` built the `EntryReference` directly via `EntryReference::from_utf8_preserve_root(target.to_str().unwrap())`, bypassing the `PathnameEditor::edit_junction` method added in Task 2.1. The consequence was that user `-s` / `--transform` substitutions applied to symlink targets but silently NOT to junction targets, and `edit_junction` itself was dead code on every platform. Call `pathname_editor.edit_junction(target)` instead. The method applies substitutions and returns an `EntryReference` via the shared helper that `edit_symlink` also uses, so junction and symlink targets behave consistently under user transforms. Invariant I1 (spec §7) guarantees the target is valid UTF-8, so the helper's internal `to_string_lossy` is effectively lossless for real inputs. Updates spec §6 / §8 so the "load-bearing unwrap" paragraph is replaced with the `edit_junction` rationale, and plan Task 3.1 Step 7 mirrors the final code. Remove the now-unused `EntryReference` import from `cli/src/command/core.rs`. ✨ Detect and archive Windows junctions as HardLink entries 🐛 Terminate junction reparse names with UTF-16 NUL Windows CI revealed that every `create_junction_round_trip` run was hitting `ERROR_INVALID_REPARSE_DATA` (4392) — the kernel rejected our IO_REPARSE_TAG_MOUNT_POINT buffer. Microsoft's REPARSE_DATA_BUFFER contract requires both SubstituteName and PrintName inside PathBuffer to be UTF-16 NUL-terminated, with the length fields excluding the NULs and PrintNameOffset set after SubstituteNameLength plus the NUL. Our writer omitted both terminators and pointed PrintNameOffset at `subst_bytes_len` (should be `subst_bytes_len + 2`). The parser never noticed because it slices by offset/length and never reads the NULs, so `read_reparse_point_on_junction` (which reads a `mklink /J`-built junction) passed all along while `create_junction_round_trip` failed. Append a u16 0x0000 after each name, bump `PrintNameOffset` by 2, and make `ReparseDataLength` account for the four extra NUL bytes. The u16-overflow guard now also checks the two extra NUL bytes so oversized targets are still caught before any filesystem mutation. Plan Task 1.3 Step 4 mirrors the fix. 🎨 Align edit_junction test names and doc comment with conventions The Task 2.1 code-quality review flagged two stylistic gaps against the file's existing conventions: - The four new unit tests used an `edit_junction_*` prefix while every other test in `cli/src/command/core/path.rs` uses an `editor_*` prefix. Rename to `editor_junction_*` so the test list stays consistently grep-able. - The `edit_junction` doc comment captured the design rationale (why a separate method exists) but not the contract (substitutions applied; leading `/` and --strip-components NOT applied). Mirror the contract lines from `edit_symlink` so the two methods read as siblings, not as rationale + cross-reference. ✨ Add PathnameEditor::edit_junction delegating to shared helper ✨ Add detect_junction helper for CLI 🔒 Reject oversized junction targets before mutating filesystem The Task 1.3 code-quality review flagged a silent `u16` truncation pitfall: the reparse-buffer header stores SubstituteName and PrintName byte lengths as `u16`, and `create_junction` was casting `(wide.len() * 2) as u16` plus computing `data_len = 8 + subst + print` as `u16` — a path exceeding ~32 KB of UTF-16 bytes would silently wrap and emit a malformed reparse buffer. The 16 KB reparse-buffer kernel cap acts as a backstop, but relying on that to backstop silent corruption is weaker than the helper's contract should be. Move the SubstituteName / PrintName construction and the u16 overflow check ahead of `std::fs::create_dir`, so an oversized target now returns `InvalidInput` at the helper boundary without ever leaving a half-created junction directory behind. Use `checked_add` for the 8-byte offset-header total so the check also catches the 32_760-plus-offset-header wrap case. Plan Task 1.3 Step 4 mirrors the reorder. ✨ Create NTFS junctions via FSCTL_SET_REPARSE_POINT ♻️ Align read_reparse_point with CLI FFI conventions The Task 1.2 code-quality review flagged three consistency gaps against the rest of the crate's Windows FFI: - Reuse `crate::utils::str::encode_wide`, which rejects embedded NUL bytes before null-terminating. The previous inline `encode_wide().collect() + push(0)` would let a path with an embedded NUL reach CreateFileW and open a silently-truncated different file. All nine sibling call sites in `cli/src/utils/os/windows/` already use this helper. - Use `PCWSTR::from_raw` instead of the tuple-struct constructor, again matching the nine sibling call sites. - Downgrade `read_reparse_point` from `pub` to `pub(crate)` to match the spec §5 commitment that these primitives stay CLI-private. Promotion into the `pna` crate behind a `windows-fs` feature is captured as an explicit follow-up in spec §11. Plan Task 1.2 Step 4 and spec §5 are updated to mirror the source so they don't drift again. ✨ Read NTFS reparse points via DeviceIoControl ✅ Cover symlink reparse buffer parsing The Task 1.1 code-quality review flagged that the IO_REPARSE_TAG_SYMLINK arm of parse_reparse_buffer shipped without direct unit coverage. Add three tests and the sample_symlink_buffer helper that exercises the symlink layout (with the extra 4-byte Flags word): one for an absolute target where \\??\\ is stripped, one for a relative target where the raw string is preserved, and one for a truncated buffer that should error with InvalidData before the flags word is read. Updates the plan's Task 1.1 Step 3 test module to mirror the final reparse.rs file so plan and implementation stay in sync. 🎨 Sort windows feature list alphabetically ASCII lexicographic order places Win32_System_IO before Win32_System_Ioctl ('O' 0x4F < 'o' 0x6F). The plan originally placed them in the opposite order, which the Task 1.1 code-quality review flagged as the only sortable-by-ASCII anomaly in the list. Swap the two entries in both Cargo.toml and the plan so the list is strictly sorted — the stable-diff rationale the plan cites now actually holds. No functional impact; feature ordering does not affect compilation. ✨ Add Windows reparse buffer parser in CLI 📝 Slim junction plan to reference the spec The spec `docs/superpowers/specs/2026-04-19-windows-junction-support-design.md` now owns the WHY — problem, architecture, interfaces, invariants, rejected alternatives, follow-ups. The plan keeps the HOW — Task 1.1 through Task 6.2 with concrete code snippets, commit commands, and verification steps. Removed duplicated sections from the plan (Goal, Architecture, Tech Stack narrative, full In-scope/Out-of-scope list, Risk Summary table, Testing Matrix detailed rows, File Structure tables) and replaced them with short cross-references plus plan-specific anchors: where each of T1-T19 lands, which task enforces which safety invariant, which files are touched. The Task List and its embedded implementation details are untouched. 📝 Formalize Windows junction support design spec Writing-plans skipped the brainstorming→spec step and produced a plan directly, with all design decisions iterated inside the plan through grill and Codex adversarial review rounds. This commits the spec retroactively so architectural intent lives separately from task-level steps. Captures locked decisions (HardLink+fLTP=Directory encoding, cli-only module placement, PathnameEditor::edit_junction with shared helper, Option A metadata safety with early return, HRESULT→Win32 error translation, non-UTF-8 invariant from UTF-16 gate) alongside the rejected alternatives (cases B'/C', pna-crate module placement, option B/C metadata approaches, direct edit_symlink reuse, HRESULT-form error matching) and the deferred follow-ups (relative-path optimization, full junction-aware no-follow metadata, public reparse-point API). 📝 Translate HRESULT to Win32 code in reparse error wrapping Codex adversarial review flagged that `detect_junction`'s `raw_os_error() == Some(4390)` check against ERROR_NOT_A_REPARSE_POINT would never fire because `read_reparse_point` and `create_junction` round-trip `windows::core::Error` through `io::Error::from_raw_os_error(e.code().0)`. The windows crate encodes Win32 failures as HRESULT via `HRESULT_FROM_WIN32` (0x80070000 | code), so the stored raw_os_error is the HRESULT form (e.g. 0x80071126), not the canonical Win32 code (4390). Introduce `io_error_from_win32` in the reparse module. It inspects the HRESULT facility bits and, when FACILITY_WIN32 is set, extracts the low 16 bits as the Win32 code; other HRESULTs pass through as raw i32. Every `CreateFileW` / `DeviceIoControl` error path in read_reparse_point and create_junction now goes through this helper, so downstream consumers such as `detect_junction` see Microsoft-documented Win32 codes. Adds two pure-Rust unit tests that verify the FACILITY_WIN32 extraction and the non-Win32 pass-through, so the helper is regression-fenced without needing a Windows CI round-trip. 📝 Close junction external-target mutation hole in plan Codex adversarial review flagged that `DataKind::HardLink + fLTP=Directory` (junction or non-Windows fallback symlink) still went through the default `restore_metadata()` path in extract.rs, which would chmod/chown/ACL/xattr/ fflag the junction via follow-link syscalls and therefore mutate the external directory outside the extraction root. Option (A) from the review discussion closes the hole by bypassing `restore_metadata()` for junction entries and applying only no-follow timestamp restoration via a new `restore_link_timestamps_no_follow` helper. A `TODO: junction-aware no-follow metadata (deferred follow-up)` block on that helper and a new Out-of-Scope entry document the full option (C) path for a follow-up plan. Task 4.4 locks the invariant with a security regression test (T19) that pre-sets a distinctive mode on the external target, runs extract with `--keep-permission --allow-unsafe-links`, and asserts the external mode is unchanged. Self-review, risk summary, testing matrix and type-consistency sections are updated accordingly. 📝 Align junction plan with existing Windows module tree Codex adversarial review flagged that the plan instructed implementers to create cli/src/utils/os/windows/fs/mod.rs and edit cli/src/utils/os/windows/mod.rs, but the repository uses the sibling layout: cli/src/utils/os/windows.rs declares `pub(crate) mod fs;` and cli/src/utils/os/windows/fs.rs already holds FileHandle, chmod, lchown, open_read_metadata, and pub(crate) mod owner. Creating a new fs/mod.rs would conflict with fs.rs and strand those exports. Reparse and junction now live as additional submodules of the existing fs module: Task 1.1 adds `pub(crate) mod reparse;` to windows/fs.rs and creates windows/fs/reparse.rs; Task 1.4 adds `pub(crate) mod junction;` and creates windows/fs/junction.rs. The existing contents of windows/fs.rs are left untouched. 📝 Rework junction plan after grill review Move the Windows reparse-point FFI out of pna into cli so pna stays platform-dependency-free. Add PathnameEditor::edit_junction delegating to a helper shared with edit_symlink. Declare StoreAs::Junction as unconditional for exhaustive matching. Accept both absolute and relative stored targets at extract for forward compatibility. Correct detect_junction's error-kind mapping, switch to OsStr::encode_wide for non-lossy path encoding in create_junction, and fix the push branch name.
1 parent 3f7aa7c commit 52dfc56

14 files changed

Lines changed: 3554 additions & 19 deletions

File tree

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

cli/Cargo.toml

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,10 +65,14 @@ nix = { version = "0.31.3", features = ["user", "fs", "ioctl"] }
6565

6666
[target.'cfg(windows)'.dependencies]
6767
windows = { version = "0.62.2", features = [
68-
"Win32_Storage_FileSystem",
68+
"Win32_Foundation",
69+
"Win32_Security",
6970
"Win32_Security_Authorization",
70-
"Win32_System_WindowsProgramming",
71+
"Win32_Storage_FileSystem",
72+
"Win32_System_IO",
73+
"Win32_System_Ioctl",
7174
"Win32_System_Threading",
75+
"Win32_System_WindowsProgramming",
7276
] }
7377
field-offset = { version = "0.3.6", optional = true }
7478

@@ -77,6 +81,7 @@ maplit = "1.0.2"
7781
path-slash = "0.2.1"
7882
rust-embed = { version = "8.11.0", features = ["debug-embed"] }
7983
scopeguard = "1.2.0"
84+
tempfile = "3"
8085
walkdir = "2.5.0"
8186
criterion = { version = "0.8.2", default-features = false, features = ["cargo_bench_support", "plotters"] }
8287

cli/src/command/bsdtar.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -601,12 +601,12 @@ pub(crate) struct BsdtarCommand {
601601
to_stdout: bool,
602602
#[arg(
603603
long,
604-
help = "Allow extracting symbolic links and hard links that contain root or parent paths (default)"
604+
help = "Allow extracting symbolic links, hard links, or Windows junctions whose target points outside the extraction root (default)"
605605
)]
606606
allow_unsafe_links: bool,
607607
#[arg(
608608
long,
609-
help = "Do not allow extracting symbolic links and hard links that contain root or parent paths"
609+
help = "Do not allow extracting symbolic links, hard links, or Windows junctions whose target points outside the extraction root"
610610
)]
611611
no_allow_unsafe_links: bool,
612612
#[arg(

cli/src/command/core.rs

Lines changed: 43 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -383,6 +383,11 @@ pub(crate) enum StoreAs {
383383
Dir,
384384
Symlink(LinkTargetType),
385385
Hardlink(PathBuf),
386+
/// Windows NTFS junction. The inner `PathBuf` is the **external** target
387+
/// path (typically absolute). This variant is only produced on Windows
388+
/// but is declared unconditionally so that `match` arms remain exhaustive
389+
/// on every platform.
390+
Junction(PathBuf),
386391
}
387392

388393
/// Source of an archive to include (file path or stdin).
@@ -781,8 +786,12 @@ pub(crate) fn collect_items_with_state(
781786
// Classify entry and maybe add it to output
782787
let store = if is_symlink {
783788
let meta = fs::symlink_metadata(entry_path)?;
784-
let link_target_type = detect_symlink_target_type(entry_path, &meta)?;
785-
Some((StoreAs::Symlink(link_target_type), meta))
789+
if let Some(target) = classify_junction(entry_path)? {
790+
Some((StoreAs::Junction(target), meta))
791+
} else {
792+
let link_target_type = detect_symlink_target_type(entry_path, &meta)?;
793+
Some((StoreAs::Symlink(link_target_type), meta))
794+
}
786795
} else if is_file {
787796
if let Some(linked) = hardlink_resolver.resolve(entry_path).ok().flatten() {
788797
Some((StoreAs::Hardlink(linked), fs::symlink_metadata(entry_path)?))
@@ -883,6 +892,26 @@ fn detect_symlink_target_type(
883892
}
884893
}
885894

895+
/// Returns the junction target if `path` is a Windows junction, or `None` for
896+
/// any non-junction path (including regular directories, symlinks, and
897+
/// unknown reparse tags). Errors inside the probe are swallowed to a debug
898+
/// log so classification still falls through to the existing symlink handler.
899+
#[cfg(windows)]
900+
fn classify_junction(path: &Path) -> io::Result<Option<PathBuf>> {
901+
match crate::utils::os::windows::fs::junction::detect_junction(path) {
902+
Ok(v) => Ok(v),
903+
Err(e) => {
904+
log::debug!("Failed to inspect reparse point {}: {}", path.display(), e);
905+
Ok(None)
906+
}
907+
}
908+
}
909+
910+
#[cfg(not(windows))]
911+
fn classify_junction(_path: &Path) -> io::Result<Option<PathBuf>> {
912+
Ok(None)
913+
}
914+
886915
pub(crate) fn collect_split_archives(first: impl AsRef<Path>) -> io::Result<Vec<fs::File>> {
887916
let first = first.as_ref();
888917
let mut archives = Vec::new();
@@ -980,6 +1009,18 @@ pub(crate) fn create_entry(
9801009
let entry = EntryBuilder::new_dir(entry_name);
9811010
apply_metadata(entry, path, keep_options, metadata)?.build()
9821011
}
1012+
StoreAs::Junction(target) => {
1013+
// Route the target through PathnameEditor::edit_junction so user-
1014+
// specified `-s` / `--transform` substitutions apply to junction
1015+
// targets on the same footing as symlink targets. Invariant I1
1016+
// (spec §7) guarantees the target is valid UTF-8, so the shared
1017+
// helper's `from_path_lossy_preserve_root` is effectively
1018+
// lossless for real inputs.
1019+
let reference = pathname_editor.edit_junction(target);
1020+
let mut entry = EntryBuilder::new_hard_link(entry_name, reference)?;
1021+
entry.link_target_type(LinkTargetType::Directory);
1022+
apply_metadata(entry, path, keep_options, metadata)?.build()
1023+
}
9831024
}
9841025
.map(Some)
9851026
}

cli/src/command/core/path.rs

Lines changed: 56 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -77,11 +77,11 @@ impl PathnameEditor {
7777
Some((sanitized, had_root))
7878
}
7979

80-
/// Edit a symlink target path.
81-
///
82-
/// Only user-specified substitutions (`-s`) are applied.
83-
/// Leading `/` and `--strip-components` are NOT applied, matching bsdtar.
84-
pub(crate) fn edit_symlink(&self, target: &Path) -> EntryReference {
80+
/// Apply user-specified substitutions to a link target while preserving
81+
/// absolute path components and skipping `--strip-components`, matching
82+
/// bsdtar symlink semantics. Shared between [`edit_symlink`](Self::edit_symlink)
83+
/// and [`edit_junction`](Self::edit_junction).
84+
fn transform_link_target_preserving_root(&self, target: &Path) -> EntryReference {
8585
let transformed: Cow<'_, Path> = if let Some(t) = &self.transformers {
8686
Cow::Owned(PathBuf::from(t.apply(
8787
target.to_string_lossy(),
@@ -94,6 +94,28 @@ impl PathnameEditor {
9494
EntryReference::from_path_lossy_preserve_root(&transformed)
9595
}
9696

97+
/// Edit a symlink target path.
98+
///
99+
/// Only user-specified substitutions (`-s`) are applied.
100+
/// Leading `/` and `--strip-components` are NOT applied, matching bsdtar.
101+
pub(crate) fn edit_symlink(&self, target: &Path) -> EntryReference {
102+
self.transform_link_target_preserving_root(target)
103+
}
104+
105+
/// Edit a Windows-junction target path.
106+
///
107+
/// Only user-specified substitutions (`-s`) are applied.
108+
/// Leading `/` and `--strip-components` are NOT applied, matching bsdtar
109+
/// symlink semantics.
110+
///
111+
/// Semantically identical to [`edit_symlink`](Self::edit_symlink) for the
112+
/// moment. A separate public method is introduced so that any future
113+
/// divergence between symlink-target and junction-target handling can be
114+
/// added without touching every call site.
115+
pub(crate) fn edit_junction(&self, target: &Path) -> EntryReference {
116+
self.transform_link_target_preserving_root(target)
117+
}
118+
97119
/// Apply substitution transforms and strip leading components.
98120
///
99121
/// Returns `None` when the path becomes empty after transformation or stripping.
@@ -1156,4 +1178,33 @@ mod tests {
11561178
// NOT parent dir: "..." is a normal component
11571179
assert!(!has_parent_dir_component("a/.../b"));
11581180
}
1181+
1182+
#[test]
1183+
fn editor_junction_preserves_unix_absolute() {
1184+
let editor = PathnameEditor::new(None, None, false, false);
1185+
let out = editor.edit_junction(Path::new("/abs/target"));
1186+
assert_eq!(out.as_str(), "/abs/target");
1187+
}
1188+
1189+
#[test]
1190+
fn editor_junction_preserves_windows_absolute() {
1191+
let editor = PathnameEditor::new(None, None, false, false);
1192+
let out = editor.edit_junction(Path::new("C:\\abs\\target"));
1193+
assert_eq!(out.as_str(), "C:\\abs\\target");
1194+
}
1195+
1196+
#[test]
1197+
fn editor_junction_preserves_relative_unchanged() {
1198+
let editor = PathnameEditor::new(None, None, false, false);
1199+
let out = editor.edit_junction(Path::new("rel/target"));
1200+
assert_eq!(out.as_str(), "rel/target");
1201+
}
1202+
1203+
#[test]
1204+
fn editor_junction_does_not_apply_strip_components() {
1205+
let editor = PathnameEditor::new(Some(1), None, false, false);
1206+
let out = editor.edit_junction(Path::new("/abs/target"));
1207+
// strip_components does NOT apply to junction targets, matching symlink semantics.
1208+
assert_eq!(out.as_str(), "/abs/target");
1209+
}
11591210
}

cli/src/command/extract.rs

Lines changed: 138 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -366,12 +366,12 @@ pub(crate) struct ExtractCommand {
366366
no_same_owner: bool,
367367
#[arg(
368368
long,
369-
help = "Allow extracting symbolic links and hard links that contain root or parent paths"
369+
help = "Allow extracting symbolic links, hard links, or Windows junctions whose target points outside the extraction root"
370370
)]
371371
allow_unsafe_links: bool,
372372
#[arg(
373373
long,
374-
help = "Do not allow extracting symbolic links and hard links that contain root or parent paths (default)"
374+
help = "Do not allow extracting symbolic links, hard links, or Windows junctions whose target points outside the extraction root (default)"
375375
)]
376376
no_allow_unsafe_links: bool,
377377
#[arg(
@@ -1421,6 +1421,41 @@ where
14211421
DataKind::HardLink => {
14221422
let reader = item.reader(ReadOptions::with_password(password))?;
14231423
let original = io::read_to_string(reader)?;
1424+
let is_directory_link = matches!(
1425+
item.metadata().link_target_type(),
1426+
Some(LinkTargetType::Directory)
1427+
);
1428+
1429+
if is_directory_link {
1430+
// Encoded junction: apply user transforms but preserve absolute
1431+
// paths and do NOT sanitize (no edit_hardlink).
1432+
let transformed = pathname_editor.edit_junction(Path::new(original.as_str()));
1433+
let target_str = transformed.as_str();
1434+
1435+
if !allow_unsafe_links {
1436+
log::warn!(
1437+
"Skipped extracting a Windows junction. If you need to extract it, use `--allow-unsafe-links`."
1438+
);
1439+
return Ok(());
1440+
}
1441+
if *safe_writes || remove_existing {
1442+
utils::io::ignore_not_found(utils::fs::remove_path(&path))?;
1443+
}
1444+
create_junction_or_fallback(&path, target_str)?;
1445+
1446+
// SAFETY (Invariant I2): the default `restore_metadata()` call
1447+
// at the end of this function would apply chmod/chown/ACL/
1448+
// xattr/fflags to the junction via follow-link syscalls,
1449+
// mutating the EXTERNAL directory the junction points at
1450+
// (outside the extraction root). For the MVP we bypass the
1451+
// full metadata restore and only apply no-follow timestamp
1452+
// restoration. See `restore_link_timestamps_no_follow` for
1453+
// the follow-up that would properly restore junction-owned
1454+
// metadata.
1455+
restore_link_timestamps_no_follow(&path, item.metadata(), keep_options)?;
1456+
return Ok(());
1457+
}
1458+
14241459
let Some((original, had_root)) = pathname_editor.edit_hardlink(original.as_ref())
14251460
else {
14261461
log::warn!(
@@ -2016,6 +2051,107 @@ fn symlink_with_type<P: AsRef<Path>, Q: AsRef<Path>>(
20162051
pna::fs::symlink(original, link)
20172052
}
20182053

2054+
/// Create the link for a HardLink+fLTP=Directory entry.
2055+
///
2056+
/// On Windows, builds a real junction. On non-Windows, falls back to a
2057+
/// symbolic link per the PNA spec `chunk_specifications/index.md:332-336`
2058+
/// MAY clause. Accepts both absolute and relative stored targets:
2059+
///
2060+
/// - On Windows, relative targets are resolved against `link`'s parent then
2061+
/// canonicalized to an absolute path (kernel requires absolute for
2062+
/// junctions). If canonicalization fails (e.g. the target does not exist),
2063+
/// the join result is passed through; `create_junction` will then fail with
2064+
/// a descriptive I/O error.
2065+
/// - On non-Windows, the raw stored string is passed to `symlink` verbatim
2066+
/// so the resulting symlink is identical to what the archive encoded.
2067+
fn create_junction_or_fallback(link: &Path, target: &str) -> io::Result<()> {
2068+
#[cfg(windows)]
2069+
{
2070+
let raw = Path::new(target);
2071+
let absolute = if raw.is_absolute() {
2072+
raw.to_path_buf()
2073+
} else {
2074+
let base = link.parent().unwrap_or_else(|| Path::new("."));
2075+
let joined = base.join(raw);
2076+
match std::fs::canonicalize(&joined) {
2077+
Ok(canon) => canon,
2078+
Err(e) => {
2079+
log::debug!(
2080+
"Failed to canonicalize junction target {}: {}; using raw join",
2081+
joined.display(),
2082+
e
2083+
);
2084+
joined
2085+
}
2086+
}
2087+
};
2088+
crate::utils::os::windows::fs::reparse::create_junction(link, &absolute)
2089+
}
2090+
#[cfg(not(windows))]
2091+
{
2092+
log::debug!(
2093+
"Creating symbolic link instead of Windows junction: {} -> {}",
2094+
link.display(),
2095+
target
2096+
);
2097+
pna::fs::symlink(target, link)
2098+
}
2099+
}
2100+
2101+
/// Restore only the timestamps of a junction or fallback-symlink entry,
2102+
/// without following the link.
2103+
///
2104+
/// # Why not the full `restore_metadata()` path?
2105+
///
2106+
/// `DataKind::HardLink + fLTP=Directory` encodes a Windows junction (or a
2107+
/// fallback symlink on non-Windows). The default `restore_metadata()` uses
2108+
/// `chmod`, `chown`, `set_facl(follow_links=true)`, `setxattr`, `set_flags`,
2109+
/// and macOS `copyfile` — every one of those follows links. If we let them
2110+
/// run against the junction path, they would mutate the **external**
2111+
/// directory the junction points at (outside the extraction root), which is
2112+
/// a security hole (Invariant I2, spec §7).
2113+
///
2114+
/// For the MVP we bypass the full metadata path for junction entries and
2115+
/// apply only timestamps through the existing `restore_path_timestamps`
2116+
/// helper, which already uses `filetime::set_symlink_file_times` and thus
2117+
/// opens the reparse point itself with `FILE_FLAG_OPEN_REPARSE_POINT` on
2118+
/// Windows / uses `utimensat(AT_SYMLINK_NOFOLLOW)` / `lutimes` on Unix.
2119+
/// Delegation is sufficient — no separate `utils::fs` helper is required.
2120+
///
2121+
/// # TODO: junction-aware no-follow metadata (deferred follow-up, spec §11)
2122+
///
2123+
/// A full implementation should restore mode/owner/ACL/xattr/fflags on the
2124+
/// junction itself using no-follow APIs:
2125+
/// - Unix: `lchmod` (BSD), `lchown`, `lsetxattr`, `lremovexattr`.
2126+
/// - Linux: mode-on-symlink is not supported by the kernel; either skip
2127+
/// silently or gate behind `#[cfg(target_os = "linux")]` with a `warn!`.
2128+
/// - Windows: open the reparse point via `FILE_FLAG_OPEN_REPARSE_POINT` and
2129+
/// apply ACL/security info with `SetSecurityInfo` on that handle; mode is
2130+
/// expressed via the Windows security descriptor, not `chmod`.
2131+
/// - ACL restoration must pass `follow_links = false` into `restore_acls`.
2132+
///
2133+
/// When implemented, replace this helper with a junction-aware branch of
2134+
/// `restore_metadata` that keeps Invariant I2 but preserves all attributes.
2135+
#[cfg(not(target_family = "wasm"))]
2136+
#[inline]
2137+
fn restore_link_timestamps_no_follow(
2138+
path: &Path,
2139+
metadata: &pna::Metadata,
2140+
keep_options: &KeepOptions,
2141+
) -> io::Result<()> {
2142+
restore_path_timestamps(path, metadata, keep_options)
2143+
}
2144+
2145+
#[cfg(target_family = "wasm")]
2146+
#[inline]
2147+
fn restore_link_timestamps_no_follow(
2148+
_path: &Path,
2149+
_metadata: &pna::Metadata,
2150+
_keep_options: &KeepOptions,
2151+
) -> io::Result<()> {
2152+
Ok(())
2153+
}
2154+
20192155
#[cfg(test)]
20202156
mod tests {
20212157
use super::*;

cli/src/utils/os/windows/fs.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
1+
pub(crate) mod junction;
12
pub(crate) mod owner;
3+
pub(crate) mod reparse;
24

35
use super::security::{Sid, apply_security_info};
46
use crate::utils::str::encode_wide;

0 commit comments

Comments
 (0)