From 297bad0e6720f578f3e6bb646711ed2b95e131a7 Mon Sep 17 00:00:00 2001 From: leynos Date: Fri, 18 Sep 2026 21:27:48 +0200 Subject: [PATCH 01/21] Harden the Windows final component through a same-handle reparse open Replace the pre-open symlink_metadata check in open_file_checked with an open that does not traverse a reparse point and a judgement taken from the handle it returns, so the policy decision and the read share one handle on Windows as they already do on Unix through O_NOFOLLOW. The new windows_reparse submodule sets FILE_FLAG_OPEN_REPARSE_POINT via cap_std's Windows-only OpenOptionsExt::custom_flags and refuses an opened handle carrying FILE_ATTRIBUTE_REPARSE_POINT. Testing the attribute bit rather than the tag also rejects junctions and volume mount points, which std does not report as symlinks. No unsafe, no new dependency, and no lint relaxation is required. Record the decision in ADR-026 and update the design and developers guides, which described reject_windows_symlink and its known race. Refs #703 --- ...-windows-reparse-point-same-handle-open.md | 190 ++++++++++++++++++ docs/contents.md | 2 + docs/developers-guide.md | 27 ++- docs/netsuke-design.md | 15 +- src/stdlib/path/fs_utils.rs | 65 +++--- src/stdlib/path/mod.rs | 2 + src/stdlib/path/windows_reparse.rs | 74 +++++++ 7 files changed, 324 insertions(+), 51 deletions(-) create mode 100644 docs/adr-026-windows-reparse-point-same-handle-open.md create mode 100644 src/stdlib/path/windows_reparse.rs diff --git a/docs/adr-026-windows-reparse-point-same-handle-open.md b/docs/adr-026-windows-reparse-point-same-handle-open.md new file mode 100644 index 000000000..e52281a8f --- /dev/null +++ b/docs/adr-026-windows-reparse-point-same-handle-open.md @@ -0,0 +1,190 @@ +# Architectural decision record (ADR) 026: Validate the Windows final component through a same-handle reparse-point open + +## Status + +Accepted. + +## Date + +2026-09-18. + +## Context and problem statement + +The four file-reading filters — `contents`, `linecount`, `hash`, and `digest` — +share one safety policy, enforced once in +`src/stdlib/path/fs_utils.rs::open_file_checked`. That function decides what may +be opened and then reads only through the handle it opened. + +On Unix the decision and the read share one open. `apply_unix_open_flags` adds +`O_NOFOLLOW` to the open itself when the default policy is in force, so the +kernel refuses a symlink final component as part of the same call that produces +the handle. There is no window between the check and the read. + +The Windows implementation had no equivalent. `reject_windows_symlink` ran a +`symlink_metadata` call *before* the open and then opened the path as a separate +step. Those two operations are not atomic: a final component that is a regular +file at check time can be replaced before the open, so a caller able to write to +the containing directory can win the race and have the read follow a prohibited +reparse point. The pre-open check was the platform's best available guard, but +it was never race-free. + +The gap is a hardening concern rather than a regression, and it does not defeat +the ordinary case. It matters because the same policy is load-bearing for all +four filters, and a check-then-open race is exactly the defect the Unix path was +built to avoid. + +## Decision Drivers + +- The Windows default path must reject a symlink, mount point (junction), and + any other prohibited reparse point without relying on a separate pre-open + metadata check. +- The Unix `O_NOFOLLOW` path must be unchanged, and `open_file_checked` must + remain the single shared entry point for all four filters. +- The open and the policy decision must not be able to diverge. +- The fix should not weaken the capability-based filesystem posture that the + rest of `src/stdlib/path/` is built on (`cap_std` directory handles rather + than ambient absolute paths), nor widen the crate's `unsafe` surface. + +## Requirements + +### Functional requirements + +- A final component that is a reparse point is rejected by the default policy on + Windows, whether it is a file symlink, a directory symlink, a junction, or a + volume mount point. +- A final component that is not a regular file is rejected on every platform. +- `follow_symlinks=true` continues to resolve a link to its target and read it. + +### Technical requirements + +- The reparse-point judgement and the regular-file judgement are both taken from + the handle that the read will use. +- The `unsafe_code` workspace lint stays at `forbid`; no new direct dependency + is added for this change. +- Ambient filesystem access stays out of the module, so Whitaker's + `no_std_fs_operations` policy needs no new exclusion. + +## Options considered + +### Option A: Raw Win32 FFI in a dedicated Windows submodule + +Open the final component with `CreateFileW`, passing +`FILE_FLAG_OPEN_REPARSE_POINT` so the open does not follow the reparse point, +then inspect the handle with `GetFileInformationByHandleEx` and +`FileAttributeTagInfo`, and adopt the handle with `FromRawHandle`. + +This closes the race and is the literal reading of the original plan. It was +rejected on three grounds. + +- It opens an **ambient absolute path** (`parent.dir_path.join(entry)`), because + `CreateFileW` has no directory-handle-relative form. That discards the + `cap_std` capability handle the rest of the module is built on, and it + requires a `dylint.toml` `excluded_paths` entry to exempt the new module from + Whitaker's `no_std_fs_operations` lint — an exemption that weakens the policy + for a check the repository can already perform inside the sandbox. +- It forces a workspace-wide downgrade of `unsafe_code` from `forbid` to + `deny`, and adds a direct `cfg(windows)` `windows-sys` dependency, for what + amounts to three `u32` constants and two calls. +- It is unnecessary, because Option B delivers the same guarantee through + existing public safe APIs. + +### Option B: `cap_std` Windows `OpenOptionsExt::custom_flags` and `MetadataExt::file_attributes` (chosen) + +`cap_std` re-exports the Windows-only `OpenOptionsExt` trait, whose +`custom_flags` method is OR-ed straight into the `dwFlagsAndAttributes` +argument of the open, and the Windows-only `MetadataExt` trait, whose +`file_attributes` method is populated from `BY_HANDLE_FILE_INFORMATION` read +from **the open handle itself**. Both are safe, public, and already reachable +through the `cap_std::fs_utf8` re-exports the module uses. + +The Windows no-follow branch therefore sets +`FILE_FLAG_OPEN_REPARSE_POINT | FILE_FLAG_BACKUP_SEMANTICS` on an ordinary +`parent.handle.open_with(...)` call, and then reads `file_attributes()` from the +resulting handle. The open and the judgement share one handle, exactly as the +Unix path does. + +This route is notable for what it does *not* need: no `unsafe`, no new +dependency, no lint relaxation, and no dylint exclusion. It is also the same +`OpenOptionsExt` trait the Unix branch of `open_file_checked` already uses to +set `O_NOFOLLOW`, so the two platforms reach their guarantees through parallel +mechanisms rather than one privileged path. + +### Option C: Keep the pre-open check and document the race + +Take no code change and record the residual risk. Rejected: the repository +would keep a check-then-open window in a security-relevant policy when a +same-handle alternative is available through an already-vendored dependency. + +## Decision Outcome + +Adopt Option B. + +`src/stdlib/path/fs_utils.rs::open_file_checked` keeps its shape. Its Windows +no-follow branch now applies `FILE_FLAG_OPEN_REPARSE_POINT` through +`OpenOptionsExt::custom_flags` before a single `open_with` call, and then +rejects the opened handle when either + +- the handle's `file_attributes()` carries `FILE_ATTRIBUTE_REPARSE_POINT`, or +- the handle's metadata reports anything other than a regular file. + +The first test rejects **every** reparse tag, not only the name-surrogate tags +that `std` reports as symlinks. Junctions and volume mount points carry +`IO_REPARSE_TAG_MOUNT_POINT`, which is not a name surrogate, so a check phrased +in terms of "is a symlink" would have missed exactly the file types this change +exists to reject. Testing the attribute bit instead makes the policy express +"reject all reparse points" directly. + +`reject_windows_symlink` is deleted; its pre-open `symlink_metadata` call is +gone, and nothing replaces it. + +### Why the race is closed by construction + +The two facts the policy needs — "is this a reparse point?" and "is this a +regular file?" — are both read from the handle that was opened, and that same +handle is what the caller then reads bytes from. Windows resolves the path once, +when the handle is created; subsequent queries on the handle cannot be +redirected by a concurrent rename or replace in the containing directory. There +is therefore no interval between the decision and the read in which the +filesystem object can change identity. This is the same argument the Unix path +relies on, where `O_NOFOLLOW` is a property of the one `open` call. + +## Known Risks and Limitations + +- `FILE_FLAG_BACKUP_SEMANTICS` is required to open a directory, which the + regular-file check must be able to do in order to reject directories with the + documented diagnostic rather than an open failure. +- The regular-file test is unchanged and platform-shared: it still runs after + the platform-specific open, so directories and other non-regular objects are + rejected identically everywhere. +- Reparse points outside the final component are out of scope. A symlinked or + junctioned parent directory is resolved when the parent handle is opened by + `parent_dir`, which is the pre-existing behaviour on both platforms and is not + changed here. +- The Unix path is untouched. `apply_unix_open_flags` and `restore_blocking` + keep their current behaviour byte for byte. + +## Architectural Rationale + +The change keeps the policy boundary where the design already puts it. All four +filters still enter through one `open_file_checked`, which still resolves a +`ParentDir` capability handle and still rejects non-regular files from the +opened handle. The Windows branch simply stops approximating `O_NOFOLLOW` from +outside the open and starts requesting the platform's own equivalent as part of +it, through the same trait family the Unix branch uses. + +Choosing the in-sandbox route over raw FFI also preserves the dependency +direction the module is built on. `src/stdlib/path/` reaches the filesystem +through `cap_std` handles; an FFI open of an ambient absolute path would have +introduced a second, weaker access path into the same module and required a +lint exemption to permit it. + +## Verification + +A Windows-only regression test asserts that a junction fixture — created with +`mklink /J`, which needs no privilege — is rejected by all four filters under +the default policy. The fixture follows the repository's "create the requested +file type or skip because that file type is unavailable" rule: the test asserts +the entry really carries `FILE_ATTRIBUTE_REPARSE_POINT` before rendering, so it +cannot pass against a plain directory. The existing symlink test continues to +cover the file-symlink reparse case, and the `follow_symlinks` opt-in test +covers the retained `open_with` path. diff --git a/docs/contents.md b/docs/contents.md index 7bfc208f2..e0ae5e0fd 100644 --- a/docs/contents.md +++ b/docs/contents.md @@ -173,6 +173,8 @@ operator, user, and contributor references are easier to find. - [ADR-028](adr-028-defer-split-build-dir-harness-trim.md): Deferred trim of the split-build-dir harness test, with the serialized-lane measurements that made the figure unstable and the ten-run gate that reopens the question. +- [ADR-026](adr-026-windows-reparse-point-same-handle-open.md): Windows final + component validation through a same-handle reparse-point open. ## Proposals diff --git a/docs/developers-guide.md b/docs/developers-guide.md index e2203ef17..16c0dfef2 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -5911,16 +5911,31 @@ and the `fcntl_getfl`/`fcntl_setfl` calls come from `rustix::fs::OFlags` and `rustix::fs`, a production dependency (1.0.8, `fs` feature, per `Cargo.toml`) used for this Unix flag handling. Once the opened handle is confirmed to be a regular file, `restore_blocking` clears `O_NONBLOCK`. The regular-file check -runs on the opened handle, so devices and FIFOs are rejected race-free. Windows -has no `O_NOFOLLOW` through cap-std, so `reject_windows_symlink` checks -`symlink_metadata` before the open; that check is not race-free and is tracked -as issue #703. +runs on the opened handle, so devices and FIFOs are rejected race-free. + +Windows has no `O_NOFOLLOW` through cap-std, but `windows_reparse` reaches the +same guarantee through `cap_std`'s Windows-only `OpenOptionsExt::custom_flags`, +which is OR-ed into the `dwFlagsAndAttributes` argument of the open. +`apply_open_flags` sets `FILE_FLAG_OPEN_REPARSE_POINT` while symlinks are not +followed, so the open does not traverse a reparse point and the returned handle +refers to the point itself, and it always sets `FILE_FLAG_BACKUP_SEMANTICS` so a +directory can be opened and then rejected by the shared regular-file check +rather than by the open failing. The decision is then taken from that same +handle: `reject_reparse_point` reads `file_attributes()` — populated from +`BY_HANDLE_FILE_INFORMATION` on the open handle — and refuses anything carrying +`FILE_ATTRIBUTE_REPARSE_POINT`. Testing the attribute bit rather than the tag +rejects junctions and volume mount points too, which `std` does not report as +symlinks because `IO_REPARSE_TAG_MOUNT_POINT` is not a name surrogate. Because +the judgement and the read share one handle, there is no check-then-open window +between them; see +[ADR-026](adr-026-windows-reparse-point-same-handle-open.md). Two diagnostics come out of the boundary. `bounded_read.rs` raises `file_too_large_error`, which quotes the path and the limit that was exceeded; `fs_utils.rs` raises `not_regular_file_error`, which quotes only the path and -is what rejects an opened FIFO or device (and a Windows symlink). On Unix a -symlink refused by `O_NOFOLLOW` instead surfaces through the mapped open error. +is what rejects an opened FIFO or device (and, on Windows, a reparse point that +`reject_reparse_point` refuses). On Unix a symlink refused by `O_NOFOLLOW` +instead surfaces through the mapped open error. All of them, like the invalid-UTF-8 diagnostic that `contents` and `linecount` raise for undecodable input, are MiniJinja `InvalidOperation` errors. See [Digest rendering](#digest-rendering) for the hashing loop that consumes this diff --git a/docs/netsuke-design.md b/docs/netsuke-design.md index dafbdbdda..6f408d4b2 100644 --- a/docs/netsuke-design.md +++ b/docs/netsuke-design.md @@ -1515,14 +1515,17 @@ Implementation notes: final component. Blocking mode is restored once the opened object is confirmed to be a regular file, and the regular-file check runs on the opened handle, so special files are rejected without a check-then-open window. On - Windows cap-std exposes no `O_NOFOLLOW`, so the pre-open `symlink_metadata` - check is the platform's best available guard; it is not race-free, and that - residual risk is tracked separately (issue #703). + Windows the default policy asks the open itself not to traverse a reparse + point and then refuses the opened handle when it carries + `FILE_ATTRIBUTE_REPARSE_POINT`; that rejects symlinks, junctions, and volume + mount points alike, and the judgement and the read share one handle. See + [ADR-026](adr-026-windows-reparse-point-same-handle-open.md). - An over-budget read fails with `stdlib.path.contents.file_too_large`, which quotes the path and the byte limit. An opened object that is not a regular - file (a FIFO, device, or a Windows symlink refused ahead of the open) fails - with `stdlib.path.contents.not_regular_file`, which quotes the path alone. A - Unix symlink refused by `O_NOFOLLOW` surfaces as the mapped open error for + file (a FIFO, a device, or a Windows reparse point refused on the opened + handle) fails with `stdlib.path.contents.not_regular_file`, which quotes the + path alone. A Unix symlink refused by `O_NOFOLLOW` surfaces as the mapped + open error for the action (the `stdlib.path.io.failed` family) instead of the regular-file diagnostic, because the refusal happens while opening. `linecount` validates UTF-8 incrementally as it counts, so a file that is not text is rejected diff --git a/src/stdlib/path/fs_utils.rs b/src/stdlib/path/fs_utils.rs index 88b3dadc3..2665a80f8 100644 --- a/src/stdlib/path/fs_utils.rs +++ b/src/stdlib/path/fs_utils.rs @@ -7,7 +7,7 @@ use std::io; use camino::{Utf8Path, Utf8PathBuf}; -#[cfg(unix)] +#[cfg(any(unix, windows))] use cap_std::fs_utf8::OpenOptionsExt; use cap_std::{ ambient_authority, fs, @@ -20,6 +20,8 @@ use rustix::fs::OFlags; use crate::localization::{self, keys}; use super::path_utils::normalise_parent; +#[cfg(windows)] +use super::windows_reparse; use crate::stdlib::io_helpers::io_to_error; /// An ambient handle to a path's parent directory and the entry name within it. @@ -53,20 +55,25 @@ pub(crate) fn not_regular_file_error(path: &Utf8Path) -> Error { /// Open `path` for reading under the file-reading safety policy. /// -/// On Unix the open is non-blocking, so a FIFO or device final component +/// The default policy opens the final path component without following a link +/// on either platform, and the decision is taken from the handle the caller +/// then reads: Unix asks for that in the open itself with `O_NOFOLLOW`, and +/// Windows asks the open not to traverse a reparse point and then judges the +/// returned handle's attributes (see `windows_reparse`). Neither platform +/// consults a separate path lookup, so a concurrent replace of the final +/// component cannot make the decision and the read disagree. +/// +/// On Unix the open is also non-blocking, so a FIFO or device final component /// cannot wedge the render worker inside `open` even when the caller opted /// into following symlinks; blocking mode is restored once the opened object -/// is confirmed to be a regular file. The final path component is opened -/// without following symlinks unless `limits.follow_symlinks` opts in, and the -/// opened object must be a regular file, checked on the opened handle so -/// devices and FIFOs are rejected race-free. +/// is confirmed to be a regular file. /// /// # Errors /// /// Returns a template error when the parent directory cannot be opened, the -/// target cannot be opened, the final component is a symlink while following -/// is disabled, the opened object is not a regular file, or blocking mode -/// cannot be restored. +/// target cannot be opened, the final component is a link while following is +/// disabled, the opened object is not a regular file, or blocking mode cannot +/// be restored. pub(crate) fn open_file_checked(path: &Utf8Path, limits: &FileReadLimits) -> Result { let parent = open_parent_dir(path)?; let mut options = OpenOptions::new(); @@ -76,10 +83,11 @@ pub(crate) fn open_file_checked(path: &Utf8Path, limits: &FileReadLimits) -> Res // default policy, which rejects a symlink final component. #[cfg(unix)] apply_unix_open_flags(&mut options, limits.follow_symlinks, path)?; + // On Windows directory opens must stay permitted under both policies so + // the shared regular-file check below reports the documented rejection + // rather than the open failing. #[cfg(windows)] - if !limits.follow_symlinks { - reject_windows_symlink(&parent, path)?; - } + windows_reparse::apply_open_flags(&mut options, limits.follow_symlinks); let file = parent .handle .open_with(Utf8Path::new(&parent.entry), &options) @@ -97,6 +105,12 @@ pub(crate) fn open_file_checked(path: &Utf8Path, limits: &FileReadLimits) -> Res err, ) })?; + // Judged from the handle just opened, never from the path, so the policy + // decision and the read cannot diverge. + #[cfg(windows)] + if !limits.follow_symlinks { + windows_reparse::reject_reparse_point(&metadata, path)?; + } if !metadata.is_file() { return Err(not_regular_file_error(path)); } @@ -135,33 +149,6 @@ fn apply_unix_open_flags( Ok(()) } -/// Reject a symlink final component ahead of an open on Windows. -/// -/// Windows exposes no `O_NOFOLLOW` through cap-std, so the pre-open -/// `symlink_metadata` check is the platform's best available guard. -/// -/// # Errors -/// -/// Returns a template error when the metadata cannot be read or names a -/// symlink. -#[cfg(windows)] -fn reject_windows_symlink(parent: &ParentDir, path: &Utf8Path) -> Result<(), Error> { - let metadata = parent - .handle - .symlink_metadata(Utf8Path::new(&parent.entry)) - .map_err(|err| { - io_to_error( - path, - &localization::message(keys::STDLIB_PATH_ACTION_STAT), - err, - ) - })?; - if metadata.file_type().is_symlink() { - return Err(not_regular_file_error(path)); - } - Ok(()) -} - /// Clear `O_NONBLOCK` from `file` after a non-blocking policy open. /// /// # Errors diff --git a/src/stdlib/path/mod.rs b/src/stdlib/path/mod.rs index a292adfb3..ef5d215d0 100644 --- a/src/stdlib/path/mod.rs +++ b/src/stdlib/path/mod.rs @@ -8,6 +8,8 @@ mod fs_utils; mod hash_utils; mod path_utils; mod read_telemetry; +#[cfg(windows)] +mod windows_reparse; #[cfg(test)] mod home_metrics_tests; diff --git a/src/stdlib/path/windows_reparse.rs b/src/stdlib/path/windows_reparse.rs new file mode 100644 index 000000000..e84240404 --- /dev/null +++ b/src/stdlib/path/windows_reparse.rs @@ -0,0 +1,74 @@ +//! Windows final-component policy: open the reparse point itself and judge the +//! handle that the read will use. +//! +//! Unix reaches this guarantee inside `open` with `O_NOFOLLOW`, so the policy +//! decision and the read share one call. Windows has no flag of that name, but +//! `FILE_FLAG_OPEN_REPARSE_POINT` has the same effect: the handle that comes +//! back refers to the reparse point rather than to whatever it points at. The +//! judgement then reads that handle's own attributes, so nothing between the +//! decision and the read can redirect either one — a concurrent rename in the +//! containing directory changes which entry the path names, but it cannot +//! change what an already-open handle refers to. +//! +//! The policy rejects **every** reparse point, not only the tags `std` reports +//! as symlinks. A junction or volume mount point carries +//! `IO_REPARSE_TAG_MOUNT_POINT`, which is not a name surrogate, so a test +//! phrased as "is this a symlink" would accept exactly the file types this +//! policy exists to refuse. Testing the attribute bit makes the policy express +//! "reject all reparse points" directly. +use camino::Utf8Path; +use cap_std::fs::MetadataExt as _; +use cap_std::fs_utf8::{Metadata, OpenOptions, OpenOptionsExt}; +use minijinja::Error; + +use super::fs_utils::not_regular_file_error; + +/// `FILE_FLAG_OPEN_REPARSE_POINT`: open the reparse point instead of following +/// it. The flag is ignored when the entry is not a reparse point, so it costs +/// an ordinary file nothing. +const FILE_FLAG_OPEN_REPARSE_POINT: u32 = 0x0020_0000; + +/// `FILE_FLAG_BACKUP_SEMANTICS`: permit a directory to be opened at all. +/// +/// Without it an open of a directory fails outright, and the caller would +/// report an open error where Unix reports the regular-file diagnostic. The +/// flag exists to let the shared `is_file` check reject a directory on its +/// attributes, exactly as it does on Unix. +const FILE_FLAG_BACKUP_SEMANTICS: u32 = 0x0200_0000; + +/// `FILE_ATTRIBUTE_REPARSE_POINT`: the entry carries a reparse tag. +/// +/// This is the attribute the policy consumes; the tag value itself is +/// deliberately never inspected, so a tag this build has never heard of is +/// refused rather than waved through. +const FILE_ATTRIBUTE_REPARSE_POINT: u32 = 0x0000_0400; + +/// Apply the Windows half of the open policy to `options`. +/// +/// Mirrors `apply_unix_open_flags`: the default policy asks the open itself not +/// to follow a reparse point, and the opt-in policy leaves the open to resolve +/// the link as usual. Directory opens are always permitted, so the shared +/// regular-file check can reject a directory by its attributes. +pub(super) fn apply_open_flags(options: &mut OpenOptions, follow_symlinks: bool) { + let mut flags = FILE_FLAG_BACKUP_SEMANTICS; + if !follow_symlinks { + flags |= FILE_FLAG_OPEN_REPARSE_POINT; + } + options.custom_flags(flags); +} + +/// Reject an opened handle whose entry is a reparse point. +/// +/// `metadata` must come from the handle the caller will read, never from a +/// separate path lookup: that is what makes the refusal race-free. +/// +/// # Errors +/// +/// Returns the non-regular-file diagnostic when the handle carries the +/// reparse-point attribute. +pub(super) fn reject_reparse_point(metadata: &Metadata, path: &Utf8Path) -> Result<(), Error> { + if metadata.file_attributes() & FILE_ATTRIBUTE_REPARSE_POINT != 0 { + return Err(not_regular_file_error(path)); + } + Ok(()) +} From b1490be2b6505de7de6025dd56c7502b96a1bf7d Mon Sep 17 00:00:00 2001 From: leynos Date: Fri, 18 Sep 2026 21:51:36 +0200 Subject: [PATCH 02/21] Correct the reparse-tag rationale in the design documents MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The prohibition was justified by a claim that junctions are not name surrogates. They are: `IO_REPARSE_TAG_MOUNT_POINT` is `0xA000_0003`, so bit 29 is set, and `std` reports a junction as a symlink. The same holds for a volume mount point. The claim mattered because it was doing load-bearing work in the argument for testing the attribute bit. That argument survives, but on the real gap: a tag that is *not* a name surrogate — a deduplication or cloud placeholder — is missed by `FileType::is_symlink` and, worse, is reported by `std` as `is_file() == true`, so the shared regular-file check would accept it. That is what makes `reject_reparse_point` load-bearing rather than redundant with the check beside it. No change in behaviour; the policy already tested the attribute bit. The prose now states the reason the code is written that way. Refs #703 --- ...-windows-reparse-point-same-handle-open.md | 21 ++++++++++++------- docs/developers-guide.md | 9 ++++---- docs/netsuke-design.md | 6 ++++-- 3 files changed, 22 insertions(+), 14 deletions(-) diff --git a/docs/adr-026-windows-reparse-point-same-handle-open.md b/docs/adr-026-windows-reparse-point-same-handle-open.md index e52281a8f..b10ac8c45 100644 --- a/docs/adr-026-windows-reparse-point-same-handle-open.md +++ b/docs/adr-026-windows-reparse-point-same-handle-open.md @@ -50,8 +50,9 @@ built to avoid. ### Functional requirements - A final component that is a reparse point is rejected by the default policy on - Windows, whether it is a file symlink, a directory symlink, a junction, or a - volume mount point. + Windows, whether it is a file symlink, a directory symlink, a junction, a + volume mount point, or any other tag such as a deduplication or cloud + placeholder. - A final component that is not a regular file is rejected on every platform. - `follow_symlinks=true` continues to resolve a link to its target and read it. @@ -127,12 +128,16 @@ rejects the opened handle when either - the handle's `file_attributes()` carries `FILE_ATTRIBUTE_REPARSE_POINT`, or - the handle's metadata reports anything other than a regular file. -The first test rejects **every** reparse tag, not only the name-surrogate tags -that `std` reports as symlinks. Junctions and volume mount points carry -`IO_REPARSE_TAG_MOUNT_POINT`, which is not a name surrogate, so a check phrased -in terms of "is a symlink" would have missed exactly the file types this change -exists to reject. Testing the attribute bit instead makes the policy express -"reject all reparse points" directly. +The first test rejects **every** reparse tag, not only the ones `std` reports +as symlinks. `FileType::is_symlink` is a test on the tag *value*: it is true +only for name-surrogate tags (bit 29 set), which covers file symlinks, +directory symlinks, junctions, and volume mount points — but is false for +every other tag, such as a deduplication or cloud placeholder. For those, an +open that follows the point succeeds and returns the target's handle, while a +check phrased as "is this a symlink" sees nothing to refuse. Testing the +attribute bit asks "is this a reparse point at all", which is the policy the +callers actually want, and it needs no knowledge of which tags a future +Windows release may mint. `reject_windows_symlink` is deleted; its pre-open `symlink_metadata` call is gone, and nothing replaces it. diff --git a/docs/developers-guide.md b/docs/developers-guide.md index 16c0dfef2..f9e761fb1 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -5924,10 +5924,11 @@ rather than by the open failing. The decision is then taken from that same handle: `reject_reparse_point` reads `file_attributes()` — populated from `BY_HANDLE_FILE_INFORMATION` on the open handle — and refuses anything carrying `FILE_ATTRIBUTE_REPARSE_POINT`. Testing the attribute bit rather than the tag -rejects junctions and volume mount points too, which `std` does not report as -symlinks because `IO_REPARSE_TAG_MOUNT_POINT` is not a name surrogate. Because -the judgement and the read share one handle, there is no check-then-open window -between them; see +rejects every reparse point, including tags `std` does not report as symlinks: +`FileType::is_symlink` is true only for name-surrogate tags, so a +deduplication or cloud placeholder would slip past a symlink-shaped check and +be followed. Because the judgement and the read share one handle, there is no +check-then-open window between them; see [ADR-026](adr-026-windows-reparse-point-same-handle-open.md). Two diagnostics come out of the boundary. `bounded_read.rs` raises diff --git a/docs/netsuke-design.md b/docs/netsuke-design.md index 6f408d4b2..6d3c98785 100644 --- a/docs/netsuke-design.md +++ b/docs/netsuke-design.md @@ -1517,8 +1517,10 @@ Implementation notes: handle, so special files are rejected without a check-then-open window. On Windows the default policy asks the open itself not to traverse a reparse point and then refuses the opened handle when it carries - `FILE_ATTRIBUTE_REPARSE_POINT`; that rejects symlinks, junctions, and volume - mount points alike, and the judgement and the read share one handle. See + `FILE_ATTRIBUTE_REPARSE_POINT`; that refuses symlinks, junctions, volume + mount points, and every other tag alike, because the test is on the + attribute bit rather than on the tag value. The judgement and the read share + one handle. See [ADR-026](adr-026-windows-reparse-point-same-handle-open.md). - An over-budget read fails with `stdlib.path.contents.file_too_large`, which quotes the path and the byte limit. An opened object that is not a regular From 9facc58f76680f69fcf0b00d3c4851c49c0bc53e Mon Sep 17 00:00:00 2001 From: leynos Date: Fri, 18 Sep 2026 21:51:43 +0200 Subject: [PATCH 03/21] Test the reparse policy against real tag values and add a junction test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The policy was only exercised through `open_flags` and the three ABI constants. Nothing asserted the property the policy actually turns on: that testing the attribute bit refuses the tags a name-surrogate test would miss. Extract `is_prohibited_reparse_point` as a pure predicate and pin it against real tag values — symlink and mount point for the surrogate case, NFS, deduplication and cloud for the non-surrogate case — cross-checking each fixture's own bit 29 so the table cannot drift from what it claims to test. A plain file's attributes must not be refused. Add a Windows integration test that creates a real directory junction with `mklink /J` and asserts all four filters refuse it. A junction needs no privilege, so unlike the symlink fixture there is nothing to skip on an ordinary host: the fixture reports `Ok(None)` only when `cmd.exe` is absent, and every other failure propagates rather than passing green over an unexercised policy. `require_real_junction` re-reads the reparse attribute without following the link, so a fixture that degraded to a plain directory cannot invert the assertions. The junction is refused by `reject_reparse_point` and, independently, by the shared regular-file check, since `std` reports a junction as a symlink. Driving both would need a non-surrogate reparse point, which no unprivileged fixture can create; the unit test above covers that gap instead. Refs #703 --- src/stdlib/path/windows_reparse.rs | 148 ++++++++++++++++-- .../read_policy_filters/file_type_tests.rs | 69 ++++++++ .../read_policy_filters/mod.rs | 37 +++++ tests/std_filter_tests/support.rs | 74 +++++++++ 4 files changed, 314 insertions(+), 14 deletions(-) diff --git a/src/stdlib/path/windows_reparse.rs b/src/stdlib/path/windows_reparse.rs index e84240404..ab5ba72ef 100644 --- a/src/stdlib/path/windows_reparse.rs +++ b/src/stdlib/path/windows_reparse.rs @@ -10,12 +10,15 @@ //! containing directory changes which entry the path names, but it cannot //! change what an already-open handle refers to. //! -//! The policy rejects **every** reparse point, not only the tags `std` reports -//! as symlinks. A junction or volume mount point carries -//! `IO_REPARSE_TAG_MOUNT_POINT`, which is not a name surrogate, so a test -//! phrased as "is this a symlink" would accept exactly the file types this -//! policy exists to refuse. Testing the attribute bit makes the policy express -//! "reject all reparse points" directly. +//! The policy rejects **every** reparse point, not only the ones `std` reports +//! as symlinks. `FileType::is_symlink` is a test on the tag *value*: it is true +//! only when the tag is a name surrogate (bit 29 set), which covers symlinks, +//! junctions, and volume mount points, but is false for every other tag — a +//! deduplication or cloud placeholder, for example. An open that follows one of +//! those would hand back the target's handle while a symlink-shaped check saw +//! nothing to refuse. Testing the attribute bit instead asks "is this a reparse +//! point at all", which is the policy the callers actually want and needs no +//! knowledge of which tags a future Windows release may mint. use camino::Utf8Path; use cap_std::fs::MetadataExt as _; use cap_std::fs_utf8::{Metadata, OpenOptions, OpenOptionsExt}; @@ -43,18 +46,38 @@ const FILE_FLAG_BACKUP_SEMANTICS: u32 = 0x0200_0000; /// refused rather than waved through. const FILE_ATTRIBUTE_REPARSE_POINT: u32 = 0x0000_0400; -/// Apply the Windows half of the open policy to `options`. +/// The `dwFlagsAndAttributes` bits the policy passes to the open. /// -/// Mirrors `apply_unix_open_flags`: the default policy asks the open itself not -/// to follow a reparse point, and the opt-in policy leaves the open to resolve -/// the link as usual. Directory opens are always permitted, so the shared -/// regular-file check can reject a directory by its attributes. -pub(super) fn apply_open_flags(options: &mut OpenOptions, follow_symlinks: bool) { +/// `FILE_FLAG_BACKUP_SEMANTICS` is unconditional, so a directory can be opened +/// and then rejected by the shared regular-file check rather than by the open +/// failing. `FILE_FLAG_OPEN_REPARSE_POINT` is added only by the default policy, +/// which must not traverse a reparse point. +pub(super) const fn open_flags(follow_symlinks: bool) -> u32 { let mut flags = FILE_FLAG_BACKUP_SEMANTICS; if !follow_symlinks { flags |= FILE_FLAG_OPEN_REPARSE_POINT; } - options.custom_flags(flags); + flags +} + +/// Apply the Windows half of the open policy to `options`. +/// +/// Mirrors `apply_unix_open_flags`: the default policy asks the open itself not +/// to follow a reparse point, and the opt-in policy leaves the open to resolve +/// the link as usual. +pub(super) fn apply_open_flags(options: &mut OpenOptions, follow_symlinks: bool) { + options.custom_flags(open_flags(follow_symlinks)); +} + +/// Whether `file_attributes` describes a reparse point the policy refuses. +/// +/// The test is on the attribute bit alone, and deliberately never on the tag +/// value. A tag-value test would have to enumerate acceptable tags, and a +/// name-surrogate test — the shape `FileType::is_symlink` uses — misses every +/// tag that is not a name surrogate, such as a deduplication or cloud +/// placeholder. +pub(super) const fn is_prohibited_reparse_point(file_attributes: u32) -> bool { + file_attributes & FILE_ATTRIBUTE_REPARSE_POINT != 0 } /// Reject an opened handle whose entry is a reparse point. @@ -67,8 +90,105 @@ pub(super) fn apply_open_flags(options: &mut OpenOptions, follow_symlinks: bool) /// Returns the non-regular-file diagnostic when the handle carries the /// reparse-point attribute. pub(super) fn reject_reparse_point(metadata: &Metadata, path: &Utf8Path) -> Result<(), Error> { - if metadata.file_attributes() & FILE_ATTRIBUTE_REPARSE_POINT != 0 { + if is_prohibited_reparse_point(metadata.file_attributes()) { return Err(not_regular_file_error(path)); } Ok(()) } + +#[cfg(test)] +mod tests { + use super::*; + + /// The default policy must ask the open not to traverse a reparse point, + /// and both policies must permit a directory open so the shared + /// regular-file check can report the documented rejection. + #[test] + fn default_policy_opens_without_traversing_and_permits_directories() { + let default = open_flags(false); + assert_eq!( + default & FILE_FLAG_OPEN_REPARSE_POINT, + FILE_FLAG_OPEN_REPARSE_POINT, + "the default policy must not traverse a reparse point" + ); + assert_eq!( + default & FILE_FLAG_BACKUP_SEMANTICS, + FILE_FLAG_BACKUP_SEMANTICS, + "the default policy must still permit opening a directory" + ); + + let following = open_flags(true); + assert_eq!( + following & FILE_FLAG_OPEN_REPARSE_POINT, + 0, + "the opt-in policy must let the open resolve the link" + ); + assert_eq!( + following & FILE_FLAG_BACKUP_SEMANTICS, + FILE_FLAG_BACKUP_SEMANTICS, + "the opt-in policy must still permit opening a directory" + ); + } + + /// The three constants are Windows ABI values, so a transcription slip + /// would compile and misbehave. Pin them against the documented values. + #[test] + fn constants_match_the_documented_abi_values() { + assert_eq!(FILE_FLAG_OPEN_REPARSE_POINT, 0x0020_0000); + assert_eq!(FILE_FLAG_BACKUP_SEMANTICS, 0x0200_0000); + assert_eq!(FILE_ATTRIBUTE_REPARSE_POINT, 0x0000_0400); + } + + /// The policy refuses every reparse tag, including the ones a + /// name-surrogate test would miss. + /// + /// `FileType::is_symlink` is true only when the tag is a name surrogate + /// (bit 29 set) — symlinks, junctions, and volume mount points — and + /// false for every other tag, for which `std` also reports + /// `is_file() == true`. So a tag-value test would both miss the + /// non-surrogate tags below and gain nothing for the surrogate ones: the + /// shared regular-file check already refuses a junction. The attribute + /// bit is the one test that covers both. + #[test] + fn policy_refuses_surrogate_and_non_surrogate_tags_alike() { + /// `IO_REPARSE_TAG_SYMLINK` + const SYMLINK: u32 = 0xA000_000C; + /// `IO_REPARSE_TAG_MOUNT_POINT` + const MOUNT_POINT: u32 = 0xA000_0003; + /// `IO_REPARSE_TAG_NFS` + const NFS: u32 = 0x8000_0014; + /// `IO_REPARSE_TAG_DEDUP` + const DEDUP: u32 = 0x8000_0013; + /// `IO_REPARSE_TAG_CLOUD` + const CLOUD: u32 = 0x9000_001A; + + /// Bit 29: the name-surrogate flag. + const NAME_SURROGATE: u32 = 0x2000_0000; + /// Every tag stamped onto an entry carries the attribute bit. + const ON_DISK: u32 = FILE_ATTRIBUTE_REPARSE_POINT; + /// `FILE_ATTRIBUTE_ARCHIVE`: what an ordinary file carries instead. + const FILE_ATTRIBUTE_ARCHIVE: u32 = 0x0000_0020; + + for (tag, is_surrogate) in [ + (SYMLINK, true), + (MOUNT_POINT, true), + (NFS, false), + (DEDUP, false), + (CLOUD, false), + ] { + assert_eq!( + tag & NAME_SURROGATE != 0, + is_surrogate, + "test fixture for tag {tag:#010x} misstates the surrogate flag" + ); + assert!( + is_prohibited_reparse_point(tag | ON_DISK), + "tag {tag:#010x} (name surrogate: {is_surrogate}) must be refused" + ); + } + assert!( + !is_prohibited_reparse_point(FILE_ATTRIBUTE_ARCHIVE), + "an ordinary file must not be refused" + ); + } +} diff --git a/tests/std_filter_tests/read_policy_filters/file_type_tests.rs b/tests/std_filter_tests/read_policy_filters/file_type_tests.rs index 185fe0c8e..ef82cd515 100644 --- a/tests/std_filter_tests/read_policy_filters/file_type_tests.rs +++ b/tests/std_filter_tests/read_policy_filters/file_type_tests.rs @@ -13,6 +13,8 @@ use test_support::fluent::normalize_fluent_isolates; #[cfg(unix)] use super::create_fifo; use super::fallible; +#[cfg(windows)] +use super::require_real_junction; use super::{ CONTENTS, DIGEST, FilterCase, HASH, LINECOUNT, ReadTarget, rejection, render_case, require_real_symlink, @@ -36,6 +38,24 @@ fn skip_without_symlink_support(case: FilterCase) { ); } +/// Report that this host cannot provide the junction fixture, then end the case. +/// +/// A junction needs no privilege, so `Ok(None)` here means only that `cmd.exe` +/// is absent. The case says so rather than passing silently: a suite that +/// reports green while quietly skipping its subject would hide the same +/// regression on a host that can create the fixture. +#[cfg(windows)] +#[expect( + clippy::print_stderr, + reason = "test harness: an unavailable fixture must be visible in the captured test output instead of passing silently" +)] +fn skip_without_junction_support(case: FilterCase) { + eprintln!( + "skipped: {} — this host cannot create a junction fixture", + case.name + ); +} + #[rstest] #[case::contents(CONTENTS)] #[case::linecount(LINECOUNT)] @@ -103,6 +123,55 @@ fn follow_symlinks_opt_in_reads_the_link_target(#[case] case: FilterCase) -> Res Ok(()) } +/// The Windows default policy must refuse a junction through the same open +/// that yields the handle. +/// +/// A junction is a directory-shaped reparse point: reading it back as text +/// renders the target's file names, so a policy that let the open traverse it +/// would silently expose a directory the caller never asked for. This case is +/// the Windows half of the same-handle guarantee, where the decision comes +/// from the opened handle's reparse-point attribute rather than from a +/// pre-open `symlink_metadata` lookup that a concurrent replace could defeat. +#[cfg(windows)] +#[rstest] +#[case::contents(CONTENTS)] +#[case::linecount(LINECOUNT)] +#[case::hash(HASH)] +#[case::digest(DIGEST)] +fn reading_filters_reject_a_junction(#[case] case: FilterCase) -> Result<()> { + let (_temp, root) = fallible::filter_workspace()?; + let Some(junction) = fallible::junction_fixture(&root)? else { + skip_without_junction_support(case); + return Ok(()); + }; + require_real_junction(&root, &junction)?; + let err = rejection( + case, + render_case( + case, + "junction", + ReadTarget::new(&root, &junction, 1024), + &case.template(), + )?, + )?; + ensure!( + err.kind() == ErrorKind::InvalidOperation, + "{}: should report InvalidOperation for a junction but was {:?}", + case.name, + err.kind() + ); + let message = normalize_fluent_isolates(&err.to_string()); + // The refusal happens on the opened handle, so it surfaces as the + // regular-file diagnostic; a traversal would have rendered the target + // directory's entries instead of erroring at all. + ensure!( + message.contains("not a regular file"), + "{}: error should explain the rejection: {message}", + case.name + ); + Ok(()) +} + #[cfg(unix)] #[rstest] #[case::contents(CONTENTS)] diff --git a/tests/std_filter_tests/read_policy_filters/mod.rs b/tests/std_filter_tests/read_policy_filters/mod.rs index 49ac7457a..64502b8b7 100644 --- a/tests/std_filter_tests/read_policy_filters/mod.rs +++ b/tests/std_filter_tests/read_policy_filters/mod.rs @@ -138,6 +138,43 @@ pub(super) fn require_real_symlink(root: &Utf8Path, link: &Utf8Path) -> Result<( Ok(()) } +/// Assert that `link` really is a reparse point, failing setup when it is not. +/// +/// This is the Windows counterpart of [`require_real_symlink`]. `mklink /J` +/// can silently produce something other than a junction — on a filesystem +/// without reparse-point support it fails, and a fixture that degraded to a +/// plain empty directory would still render, inverting the assertion: the +/// default policy rejects the files all four filters read, so a directory +/// would make every case pass for the wrong reason. The attribute is read +/// without following the link, so the check cannot be satisfied by the target. +/// +/// A special-file policy test must create the requested file type or skip +/// because that file type is unavailable; it must not substitute one. +#[cfg(windows)] +pub(super) fn require_real_junction(root: &Utf8Path, link: &Utf8Path) -> Result<()> { + use cap_std::fs::MetadataExt as _; + + /// `FILE_ATTRIBUTE_REPARSE_POINT`: the entry carries a reparse tag, which + /// is what distinguishes a junction from the plain directory it points at. + const FILE_ATTRIBUTE_REPARSE_POINT: u32 = 0x0000_0400; + + let dir = Dir::open_ambient_dir(root, ambient_authority()) + .with_context(|| format!("open workspace root {root} to stat the junction fixture"))?; + let name = link + .file_name() + .with_context(|| format!("junction fixture {link} has no file name"))?; + let metadata = dir + .symlink_metadata(Utf8Path::new(name)) + .with_context(|| format!("stat junction fixture {link}"))?; + ensure!( + metadata.file_attributes() & FILE_ATTRIBUTE_REPARSE_POINT != 0, + "fixture {link} is not a reparse point; the reparse-point policy cannot be \ + exercised without one, and substituting a plain directory would invert \ + the assertions" + ); + Ok(()) +} + /// Write `contents` to `name` inside `root`, returning the fixture's path. pub(super) fn write_fixture( root: &Utf8Path, diff --git a/tests/std_filter_tests/support.rs b/tests/std_filter_tests/support.rs index e628235be..3706ce7a0 100644 --- a/tests/std_filter_tests/support.rs +++ b/tests/std_filter_tests/support.rs @@ -22,6 +22,8 @@ pub(crate) mod fallible { //! file where a symlink was requested inverts the assertion outright. use super::{Workspace, stdlib}; + #[cfg(windows)] + use anyhow::ensure; use anyhow::{Context, Result, anyhow}; use camino::{Utf8Path, Utf8PathBuf}; use cap_std::{ambient_authority, fs_utf8::Dir}; @@ -168,6 +170,78 @@ pub(crate) mod fallible { err.raw_os_error() == Some(ERROR_PRIVILEGE_NOT_HELD) } + /// Create the workspace's real directory junction, `/junc` -> + /// `/junction_target`, and report the junction's path. + /// + /// A junction is the directory-shaped reparse point this policy refuses. + /// Unlike a symlink it needs no privilege and no Developer Mode, so the + /// fixture is available on every ordinary Windows host; it is built with + /// `cmd /C mklink /J`, which is the only unprivileged route to one. + /// + /// `Ok(None)` means this host has no `cmd.exe`, so the fixture cannot be + /// created at all. That is the only environmental unavailability this + /// fixture recognises. Every other failure — including a non-zero `mklink` + /// exit — is a setup fault and propagates, because junction creation needs + /// no privilege and reparse points are supported on every filesystem this + /// suite runs on: skipping there would report green over a policy that went + /// unexercised. + /// + /// Callers assert the entry really carries the reparse-point attribute + /// before rendering. They must not fall back to a plain directory: a + /// special-file policy test must create the requested file type or skip + /// because that file type is unavailable. It must not substitute one. + /// + /// # Errors + /// + /// Returns the setup error when `cmd` is present but the junction was not + /// created, quoting `mklink`'s own diagnostics. + #[cfg(windows)] + pub(crate) fn junction_fixture(root: &Utf8Path) -> Result> { + use std::os::windows::process::CommandExt as _; + use std::process::Command; + + /// Name of the junction the fixture creates. + const LINK: &str = "junc"; + /// Name of the directory the junction points at. + const TARGET: &str = "junction_target"; + + let dir = Dir::open_ambient_dir(root, ambient_authority()) + .context("open filter workspace for the junction fixture")?; + dir.create_dir(TARGET) + .context("create the junction fixture target directory")?; + let link = root.join(LINK); + let target = root.join(TARGET); + ensure!( + !link.as_str().contains('"') && !target.as_str().contains('"'), + "workspace path contains a quote and cannot be passed to cmd: \ + link {link}, target {target}" + ); + + // `mklink` is a `cmd` built-in, so it is reachable only through + // `cmd /C`. `raw_arg` passes the command line verbatim because `cmd` + // parses that line itself, while Rust's standard argument quoting + // targets the C runtime's rules rather than `cmd`'s. + let output = match Command::new("cmd") + .arg("/C") + .raw_arg(format!(r#"mklink /J "{link}" "{target}""#)) + .output() + { + Ok(output) => output, + Err(err) if err.kind() == std::io::ErrorKind::NotFound => return Ok(None), + Err(err) => { + return Err(err).context("run 'cmd /C mklink /J' for the junction fixture"); + } + }; + let stderr = String::from_utf8_lossy(&output.stderr); + ensure!( + output.status.success(), + "create junction fixture {link} -> {target}: cmd exited with {}: {}", + output.status, + stderr.trim() + ); + Ok(Some(link)) + } + pub(crate) fn render<'a>( env: &mut Environment<'a>, name: &'a str, From 3bc4cbc9af1f288f87585aeab3323a7c60132f44 Mon Sep 17 00:00:00 2001 From: leynos Date: Fri, 18 Sep 2026 21:54:38 +0200 Subject: [PATCH 04/21] Reflow the ADR-026 prose to the mdtablefix canonical form `make check-fmt` runs `mdtablefix --wrap` over every tracked Markdown file, and the hand-wrapped paragraphs in the three touched documents did not match its line breaking. Rewrapped in place with the same flags the Makefile passes. The reflow is cosmetic: a token-stream comparison of each file before and after shows the two sequences identical, so no wording changed. Refs #703 --- ...-windows-reparse-point-same-handle-open.md | 48 +++++++++---------- docs/developers-guide.md | 16 +++---- docs/netsuke-design.md | 16 +++---- 3 files changed, 39 insertions(+), 41 deletions(-) diff --git a/docs/adr-026-windows-reparse-point-same-handle-open.md b/docs/adr-026-windows-reparse-point-same-handle-open.md index b10ac8c45..b01ca159c 100644 --- a/docs/adr-026-windows-reparse-point-same-handle-open.md +++ b/docs/adr-026-windows-reparse-point-same-handle-open.md @@ -12,8 +12,8 @@ Accepted. The four file-reading filters — `contents`, `linecount`, `hash`, and `digest` — share one safety policy, enforced once in -`src/stdlib/path/fs_utils.rs::open_file_checked`. That function decides what may -be opened and then reads only through the handle it opened. +`src/stdlib/path/fs_utils.rs::open_file_checked`. That function decides what +may be opened and then reads only through the handle it opened. On Unix the decision and the read share one open. `apply_unix_open_flags` adds `O_NOFOLLOW` to the open itself when the default policy is in force, so the @@ -21,17 +21,17 @@ kernel refuses a symlink final component as part of the same call that produces the handle. There is no window between the check and the read. The Windows implementation had no equivalent. `reject_windows_symlink` ran a -`symlink_metadata` call *before* the open and then opened the path as a separate -step. Those two operations are not atomic: a final component that is a regular -file at check time can be replaced before the open, so a caller able to write to -the containing directory can win the race and have the read follow a prohibited -reparse point. The pre-open check was the platform's best available guard, but -it was never race-free. +`symlink_metadata` call *before* the open and then opened the path as a +separate step. Those two operations are not atomic: a final component that is a +regular file at check time can be replaced before the open, so a caller able to +write to the containing directory can win the race and have the read follow a +prohibited reparse point. The pre-open check was the platform's best available +guard, but it was never race-free. The gap is a hardening concern rather than a regression, and it does not defeat the ordinary case. It matters because the same policy is load-bearing for all -four filters, and a check-then-open race is exactly the defect the Unix path was -built to avoid. +four filters, and a check-then-open race is exactly the defect the Unix path +was built to avoid. ## Decision Drivers @@ -100,9 +100,9 @@ through the `cap_std::fs_utf8` re-exports the module uses. The Windows no-follow branch therefore sets `FILE_FLAG_OPEN_REPARSE_POINT | FILE_FLAG_BACKUP_SEMANTICS` on an ordinary -`parent.handle.open_with(...)` call, and then reads `file_attributes()` from the -resulting handle. The open and the judgement share one handle, exactly as the -Unix path does. +`parent.handle.open_with(...)` call, and then reads `file_attributes()` from +the resulting handle. The open and the judgement share one handle, exactly as +the Unix path does. This route is notable for what it does *not* need: no `unsafe`, no new dependency, no lint relaxation, and no dylint exclusion. It is also the same @@ -131,13 +131,13 @@ rejects the opened handle when either The first test rejects **every** reparse tag, not only the ones `std` reports as symlinks. `FileType::is_symlink` is a test on the tag *value*: it is true only for name-surrogate tags (bit 29 set), which covers file symlinks, -directory symlinks, junctions, and volume mount points — but is false for -every other tag, such as a deduplication or cloud placeholder. For those, an -open that follows the point succeeds and returns the target's handle, while a -check phrased as "is this a symlink" sees nothing to refuse. Testing the -attribute bit asks "is this a reparse point at all", which is the policy the -callers actually want, and it needs no knowledge of which tags a future -Windows release may mint. +directory symlinks, junctions, and volume mount points — but is false for every +other tag, such as a deduplication or cloud placeholder. For those, an open +that follows the point succeeds and returns the target's handle, while a check +phrased as "is this a symlink" sees nothing to refuse. Testing the attribute +bit asks "is this a reparse point at all", which is the policy the callers +actually want, and it needs no knowledge of which tags a future Windows release +may mint. `reject_windows_symlink` is deleted; its pre-open `symlink_metadata` call is gone, and nothing replaces it. @@ -146,8 +146,8 @@ gone, and nothing replaces it. The two facts the policy needs — "is this a reparse point?" and "is this a regular file?" — are both read from the handle that was opened, and that same -handle is what the caller then reads bytes from. Windows resolves the path once, -when the handle is created; subsequent queries on the handle cannot be +handle is what the caller then reads bytes from. Windows resolves the path +once, when the handle is created; subsequent queries on the handle cannot be redirected by a concurrent rename or replace in the containing directory. There is therefore no interval between the decision and the read in which the filesystem object can change identity. This is the same argument the Unix path @@ -163,8 +163,8 @@ relies on, where `O_NOFOLLOW` is a property of the one `open` call. rejected identically everywhere. - Reparse points outside the final component are out of scope. A symlinked or junctioned parent directory is resolved when the parent handle is opened by - `parent_dir`, which is the pre-existing behaviour on both platforms and is not - changed here. + `parent_dir`, which is the pre-existing behaviour on both platforms and is + not changed here. - The Unix path is untouched. `apply_unix_open_flags` and `restore_blocking` keep their current behaviour byte for byte. diff --git a/docs/developers-guide.md b/docs/developers-guide.md index f9e761fb1..11fc2209d 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -5918,16 +5918,16 @@ same guarantee through `cap_std`'s Windows-only `OpenOptionsExt::custom_flags`, which is OR-ed into the `dwFlagsAndAttributes` argument of the open. `apply_open_flags` sets `FILE_FLAG_OPEN_REPARSE_POINT` while symlinks are not followed, so the open does not traverse a reparse point and the returned handle -refers to the point itself, and it always sets `FILE_FLAG_BACKUP_SEMANTICS` so a -directory can be opened and then rejected by the shared regular-file check +refers to the point itself, and it always sets `FILE_FLAG_BACKUP_SEMANTICS` so +a directory can be opened and then rejected by the shared regular-file check rather than by the open failing. The decision is then taken from that same handle: `reject_reparse_point` reads `file_attributes()` — populated from `BY_HANDLE_FILE_INFORMATION` on the open handle — and refuses anything carrying `FILE_ATTRIBUTE_REPARSE_POINT`. Testing the attribute bit rather than the tag rejects every reparse point, including tags `std` does not report as symlinks: -`FileType::is_symlink` is true only for name-surrogate tags, so a -deduplication or cloud placeholder would slip past a symlink-shaped check and -be followed. Because the judgement and the read share one handle, there is no +`FileType::is_symlink` is true only for name-surrogate tags, so a deduplication +or cloud placeholder would slip past a symlink-shaped check and be followed. +Because the judgement and the read share one handle, there is no check-then-open window between them; see [ADR-026](adr-026-windows-reparse-point-same-handle-open.md). @@ -5936,9 +5936,9 @@ Two diagnostics come out of the boundary. `bounded_read.rs` raises `fs_utils.rs` raises `not_regular_file_error`, which quotes only the path and is what rejects an opened FIFO or device (and, on Windows, a reparse point that `reject_reparse_point` refuses). On Unix a symlink refused by `O_NOFOLLOW` -instead surfaces through the mapped open error. -All of them, like the invalid-UTF-8 diagnostic that `contents` and `linecount` -raise for undecodable input, are MiniJinja `InvalidOperation` errors. See +instead surfaces through the mapped open error. All of them, like the +invalid-UTF-8 diagnostic that `contents` and `linecount` raise for undecodable +input, are MiniJinja `InvalidOperation` errors. See [Digest rendering](#digest-rendering) for the hashing loop that consumes this boundary. diff --git a/docs/netsuke-design.md b/docs/netsuke-design.md index 6d3c98785..7a9337904 100644 --- a/docs/netsuke-design.md +++ b/docs/netsuke-design.md @@ -1518,20 +1518,18 @@ Implementation notes: Windows the default policy asks the open itself not to traverse a reparse point and then refuses the opened handle when it carries `FILE_ATTRIBUTE_REPARSE_POINT`; that refuses symlinks, junctions, volume - mount points, and every other tag alike, because the test is on the - attribute bit rather than on the tag value. The judgement and the read share - one handle. See - [ADR-026](adr-026-windows-reparse-point-same-handle-open.md). + mount points, and every other tag alike, because the test is on the attribute + bit rather than on the tag value. The judgement and the read share one + handle. See [ADR-026](adr-026-windows-reparse-point-same-handle-open.md). - An over-budget read fails with `stdlib.path.contents.file_too_large`, which quotes the path and the byte limit. An opened object that is not a regular file (a FIFO, a device, or a Windows reparse point refused on the opened handle) fails with `stdlib.path.contents.not_regular_file`, which quotes the path alone. A Unix symlink refused by `O_NOFOLLOW` surfaces as the mapped - open error for - the action (the `stdlib.path.io.failed` family) instead of the regular-file - diagnostic, because the refusal happens while opening. `linecount` validates - UTF-8 incrementally as it counts, so a file that is not text is rejected - rather than silently counted as opaque bytes. + open error for the action (the `stdlib.path.io.failed` family) instead of the + regular-file diagnostic, because the refusal happens while opening. + `linecount` validates UTF-8 incrementally as it counts, so a file that is not + text is rejected rather than silently counted as opaque bytes. - Each of the four filter closures records its call through `src/stdlib/path/read_telemetry.rs`: one sample of the bounded counter `netsuke_stdlib_file_read_total`, labelled `filter` (`contents`, `linecount`, From d241138e80d3d1d64a78a249176a675f7c8111ce Mon Sep 17 00:00:00 2001 From: leynos Date: Fri, 18 Sep 2026 23:06:07 +0200 Subject: [PATCH 05/21] Use sentence case for the ADR-026 section headings The documentation style guide requires sentence case for headings, and the ADR template spells the sections that way: "Decision drivers", "Decision outcome", "Known risks and limitations", "Architectural rationale". ADR-026 was the only ADR in the tree that used title case for all four. The date keeps its trailing period. The same template writes the field as `YYYY-MM-DD.`, and ADR-025 and ADR-022 follow it, so the formatting review suggestion to drop the period conflicts with the documented convention and is not applied. No behavioural change; headings only, and nothing links to these anchors. Refs #703 --- docs/adr-026-windows-reparse-point-same-handle-open.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/adr-026-windows-reparse-point-same-handle-open.md b/docs/adr-026-windows-reparse-point-same-handle-open.md index b01ca159c..d95abaad5 100644 --- a/docs/adr-026-windows-reparse-point-same-handle-open.md +++ b/docs/adr-026-windows-reparse-point-same-handle-open.md @@ -33,7 +33,7 @@ the ordinary case. It matters because the same policy is load-bearing for all four filters, and a check-then-open race is exactly the defect the Unix path was built to avoid. -## Decision Drivers +## Decision drivers - The Windows default path must reject a symlink, mount point (junction), and any other prohibited reparse point without relying on a separate pre-open @@ -116,7 +116,7 @@ Take no code change and record the residual risk. Rejected: the repository would keep a check-then-open window in a security-relevant policy when a same-handle alternative is available through an already-vendored dependency. -## Decision Outcome +## Decision outcome Adopt Option B. @@ -153,7 +153,7 @@ is therefore no interval between the decision and the read in which the filesystem object can change identity. This is the same argument the Unix path relies on, where `O_NOFOLLOW` is a property of the one `open` call. -## Known Risks and Limitations +## Known risks and limitations - `FILE_FLAG_BACKUP_SEMANTICS` is required to open a directory, which the regular-file check must be able to do in order to reject directories with the @@ -168,7 +168,7 @@ relies on, where `O_NOFOLLOW` is a property of the one `open` call. - The Unix path is untouched. `apply_unix_open_flags` and `restore_blocking` keep their current behaviour byte for byte. -## Architectural Rationale +## Architectural rationale The change keeps the policy boundary where the design already puts it. All four filters still enter through one `open_file_checked`, which still resolves a From a99b9bce637f94af1ad1aaca5c307cb5b38cbca4 Mon Sep 17 00:00:00 2001 From: leynos Date: Sat, 19 Sep 2026 01:52:01 +0200 Subject: [PATCH 06/21] Test the reparse policy on the handle and narrow a Windows-only import Add a handle-level test that only passes if FILE_FLAG_OPEN_REPARSE_POINT reaches the open. The integration test cannot detect that: std reports a junction as a symlink, so metadata.is_file() refuses the resolved directory regardless, and the policy would look correct while the flag went unapplied. Asserting the property on the opened handle's attributes is the only place the distinction shows, so a dropped flag now fails a test instead of silently widening the policy. The fixture is extracted into a helper, which also takes the test from 69 to 41 counted lines against the 70-line threshold: it had a single line of headroom on a lane only Windows CI compiles, so any small future edit would have broken that job with no local signal. Narrow the OpenOptionsExt import in fs_utils back to #[cfg(unix)]. Its only call site is inside #[cfg(unix)] apply_unix_open_flags; the Windows arm reaches custom_flags through windows_reparse, which has its own import. Commit 77c99e7c had widened it to any(unix, windows), which leaves the import live but unused on Windows and fails lint-clippy under -D warnings. Linux gates cannot see this, because the file's Windows code is cfg-gated. Both findings come from a local cross-compile probe: the main crate cannot be built for x86_64-pc-windows-msvc here because ring needs MSVC lib.exe, so the probe mirrors the module tree with file modules (Clippy resets its nesting counter at a file module and counts it through an inline one), copies windows_reparse byte for byte, and compiles the Windows-gated integration fixtures that the Linux gates never reach. Refs #703 --- src/stdlib/path/fs_utils.rs | 2 +- src/stdlib/path/windows_reparse.rs | 118 +++++++++++++++++++++++++++++ 2 files changed, 119 insertions(+), 1 deletion(-) diff --git a/src/stdlib/path/fs_utils.rs b/src/stdlib/path/fs_utils.rs index 2665a80f8..774a04811 100644 --- a/src/stdlib/path/fs_utils.rs +++ b/src/stdlib/path/fs_utils.rs @@ -7,7 +7,7 @@ use std::io; use camino::{Utf8Path, Utf8PathBuf}; -#[cfg(any(unix, windows))] +#[cfg(unix)] use cap_std::fs_utf8::OpenOptionsExt; use cap_std::{ ambient_authority, fs, diff --git a/src/stdlib/path/windows_reparse.rs b/src/stdlib/path/windows_reparse.rs index ab5ba72ef..b60cb43e9 100644 --- a/src/stdlib/path/windows_reparse.rs +++ b/src/stdlib/path/windows_reparse.rs @@ -99,6 +99,19 @@ pub(super) fn reject_reparse_point(metadata: &Metadata, path: &Utf8Path) -> Resu #[cfg(test)] mod tests { use super::*; + #[cfg(windows)] + use camino::Utf8PathBuf; + #[cfg(windows)] + use cap_std::{ambient_authority, fs_utf8::Dir}; + #[cfg(windows)] + use tempfile::tempdir; + + /// Name of the junction the handle tests create. + #[cfg(windows)] + const LINK: &str = "junc"; + /// Name of the directory that junction points at. + #[cfg(windows)] + const TARGET: &str = "junction_target"; /// The default policy must ask the open not to traverse a reparse point, /// and both policies must permit a directory open so the shared @@ -191,4 +204,109 @@ mod tests { "an ordinary file must not be refused" ); } + + /// Create the workspace the handle tests share, returning its temporary + /// directory, the capability handle, and the link and target paths. + /// + /// The `TempDir` is returned so the caller keeps it alive: dropping it + /// removes the tree, and the junction with it. + #[cfg(windows)] + fn junction_fixture() -> (tempfile::TempDir, Dir, Utf8PathBuf, Utf8PathBuf) { + use std::os::windows::process::CommandExt as _; + use std::process::Command; + + let temp = tempdir().expect("create the junction fixture workspace"); + let root = Utf8PathBuf::from_path_buf(temp.path().to_path_buf()) + .expect("fixture workspace path is valid UTF-8"); + let dir = Dir::open_ambient_dir(&root, ambient_authority()) + .expect("open the fixture workspace as a capability"); + dir.create_dir(TARGET) + .expect("create the junction fixture target directory"); + let link = root.join(LINK); + let target = root.join(TARGET); + assert!( + !link.as_str().contains('"') && !target.as_str().contains('"'), + "workspace path contains a quote and cannot be passed to cmd" + ); + + // `mklink` is a `cmd` built-in, so it is reachable only through + // `cmd /C`. `raw_arg` passes the line verbatim because `cmd` parses it + // itself, rather than by the C runtime's argument-quoting rules. + let output = Command::new("cmd") + .arg("/C") + .raw_arg(format!(r#"mklink /J "{link}" "{target}""#)) + .output() + .expect("run 'cmd /C mklink /J' for the junction fixture"); + assert!( + output.status.success(), + "create junction fixture {link} -> {target}: cmd exited with {}: {}", + output.status, + String::from_utf8_lossy(&output.stderr).trim() + ); + (temp, dir, link, target) + } + + /// The default policy's handle *is* the reparse point, and the opt-in + /// policy's handle is the directory it points at. + /// + /// This is the property that makes the same-handle judgement possible, and + /// the only test that fails if `FILE_FLAG_OPEN_REPARSE_POINT` stops being + /// passed: without it both opens return the target directory, whose + /// attributes carry no reparse bit, and `reject_reparse_point` would wave + /// the junction through. The integration test cannot detect that, because + /// `std` reports a junction as a symlink, so `metadata.is_file()` refuses + /// the directory anyway. The distinction only shows on the handle. + /// + /// A junction is used rather than a symlink because it needs no privilege, + /// and it is created with `mklink /J` before the directory is opened + /// through `cap_std`. Every step is a genuine filesystem operation; none + /// of it substitutes an ordinary file for the reparse point under test. + #[cfg(windows)] + #[test] + fn the_default_handle_is_the_junction_and_the_opt_in_handle_is_its_target() { + let (_temp, dir, link, target) = junction_fixture(); + + // The default policy: the handle must be the reparse point itself. + let mut refusing_options = OpenOptions::new(); + refusing_options.read(true); + apply_open_flags(&mut refusing_options, false); + let refusing_handle = dir + .open_with(Utf8Path::new(LINK), &refusing_options) + .expect("the default policy must be able to open the junction itself"); + let refusing_metadata = refusing_handle + .metadata() + .expect("read metadata from the default-policy handle"); + assert!( + is_prohibited_reparse_point(refusing_metadata.file_attributes()), + "the default handle must be the reparse point, so the policy has \ + something to refuse; a handle to the target directory means \ + FILE_FLAG_OPEN_REPARSE_POINT was not passed" + ); + assert!( + reject_reparse_point(&refusing_metadata, &link).is_err(), + "the default policy must refuse the junction it just opened" + ); + + // The opt-in policy: the handle must be the target directory, which is + // what makes the opt-in a genuine follow rather than a second refusal. + let mut following_options = OpenOptions::new(); + following_options.read(true); + apply_open_flags(&mut following_options, true); + let following_handle = dir + .open_with(Utf8Path::new(LINK), &following_options) + .expect("the opt-in policy must be able to open through the junction"); + let following_metadata = following_handle + .metadata() + .expect("read metadata from the opt-in handle"); + assert!( + !is_prohibited_reparse_point(following_metadata.file_attributes()), + "the opt-in handle must be the target directory, not the reparse \ + point; a handle carrying the reparse bit means the flag was \ + applied under the opt-in policy too" + ); + assert!( + reject_reparse_point(&following_metadata, &target).is_ok(), + "the opt-in policy must not refuse the target it resolved" + ); + } } From 704aa78df39a579c7087e25d8747436bdd0917b4 Mon Sep 17 00:00:00 2001 From: leynos Date: Sat, 19 Sep 2026 02:21:23 +0200 Subject: [PATCH 07/21] Renumber ADR-026 to ADR-027 after main claimed 026 main gained adr-026-manifest-environment-access-policy.md (PR #666) while this branch was in review, so the Windows reparse-point ADR moves to 027. Also drops the trailing full stop from the Date line, matching the two most recent ADRs on main (024, 026), which write the bare YYYY-MM-DD. --- ...n.md => adr-027-windows-reparse-point-same-handle-open.md} | 4 ++-- docs/contents.md | 2 +- docs/developers-guide.md | 2 +- docs/netsuke-design.md | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) rename docs/{adr-026-windows-reparse-point-same-handle-open.md => adr-027-windows-reparse-point-same-handle-open.md} (99%) diff --git a/docs/adr-026-windows-reparse-point-same-handle-open.md b/docs/adr-027-windows-reparse-point-same-handle-open.md similarity index 99% rename from docs/adr-026-windows-reparse-point-same-handle-open.md rename to docs/adr-027-windows-reparse-point-same-handle-open.md index d95abaad5..450befef5 100644 --- a/docs/adr-026-windows-reparse-point-same-handle-open.md +++ b/docs/adr-027-windows-reparse-point-same-handle-open.md @@ -1,4 +1,4 @@ -# Architectural decision record (ADR) 026: Validate the Windows final component through a same-handle reparse-point open +# Architectural decision record (ADR) 027: Validate the Windows final component through a same-handle reparse-point open ## Status @@ -6,7 +6,7 @@ Accepted. ## Date -2026-09-18. +2026-09-18 ## Context and problem statement diff --git a/docs/contents.md b/docs/contents.md index e0ae5e0fd..48014c2f4 100644 --- a/docs/contents.md +++ b/docs/contents.md @@ -173,7 +173,7 @@ operator, user, and contributor references are easier to find. - [ADR-028](adr-028-defer-split-build-dir-harness-trim.md): Deferred trim of the split-build-dir harness test, with the serialized-lane measurements that made the figure unstable and the ten-run gate that reopens the question. -- [ADR-026](adr-026-windows-reparse-point-same-handle-open.md): Windows final +- [ADR-027](adr-027-windows-reparse-point-same-handle-open.md): Windows final component validation through a same-handle reparse-point open. ## Proposals diff --git a/docs/developers-guide.md b/docs/developers-guide.md index 11fc2209d..62d1f0bde 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -5929,7 +5929,7 @@ rejects every reparse point, including tags `std` does not report as symlinks: or cloud placeholder would slip past a symlink-shaped check and be followed. Because the judgement and the read share one handle, there is no check-then-open window between them; see -[ADR-026](adr-026-windows-reparse-point-same-handle-open.md). +[ADR-027](adr-027-windows-reparse-point-same-handle-open.md). Two diagnostics come out of the boundary. `bounded_read.rs` raises `file_too_large_error`, which quotes the path and the limit that was exceeded; diff --git a/docs/netsuke-design.md b/docs/netsuke-design.md index 7a9337904..c3ca97151 100644 --- a/docs/netsuke-design.md +++ b/docs/netsuke-design.md @@ -1520,7 +1520,7 @@ Implementation notes: `FILE_ATTRIBUTE_REPARSE_POINT`; that refuses symlinks, junctions, volume mount points, and every other tag alike, because the test is on the attribute bit rather than on the tag value. The judgement and the read share one - handle. See [ADR-026](adr-026-windows-reparse-point-same-handle-open.md). + handle. See [ADR-027](adr-027-windows-reparse-point-same-handle-open.md). - An over-budget read fails with `stdlib.path.contents.file_too_large`, which quotes the path and the byte limit. An opened object that is not a regular file (a FIFO, a device, or a Windows reparse point refused on the opened From 30e7c3492be8cf1749c63513b125fcefd4747791 Mon Sep 17 00:00:00 2001 From: leynos Date: Sat, 19 Sep 2026 03:57:46 +0200 Subject: [PATCH 08/21] test(windows): skip the junction fixture when cmd.exe is absent The unit-test junction fixture spawned `cmd.exe` unconditionally and `expect`ed the spawn to succeed, so a host without `cmd.exe` failed the test instead of reporting that its subject was unavailable. That is both a false negative for a valid Windows host and an inconsistency with the sibling integration fixture, `fallible::junction_fixture` in tests/std_filter_tests/support.rs, which maps `ErrorKind::NotFound` to a skip. Map that one error kind to `None` from `junction_fixture`, and return early from the caller with a skip notice on captured test output. Every other spawn error and a non-zero `mklink` exit still assert: a junction needs no privilege, so a missing `cmd.exe` is the only unavailability that is a skip, and a real fault must not be masked as one. The skip is printed rather than silent so a green run cannot quietly hide the regression on a host that can build a junction; the `#[expect(clippy::print_stderr, ...)]` is fulfilled by that branch. --- src/stdlib/path/windows_reparse.rs | 34 +++++++++++++++++++++++++----- 1 file changed, 29 insertions(+), 5 deletions(-) diff --git a/src/stdlib/path/windows_reparse.rs b/src/stdlib/path/windows_reparse.rs index b60cb43e9..222f493ac 100644 --- a/src/stdlib/path/windows_reparse.rs +++ b/src/stdlib/path/windows_reparse.rs @@ -208,10 +208,17 @@ mod tests { /// Create the workspace the handle tests share, returning its temporary /// directory, the capability handle, and the link and target paths. /// + /// `None` means this host has no `cmd.exe` to reach `mklink` through, so + /// the fixture cannot be built; the caller reports that as a skip rather + /// than a failure, exactly as `fallible::junction_fixture` does in the + /// integration suite. A junction needs no privilege, so a missing + /// `cmd.exe` is the only unavailability that is a skip — every other + /// failure is a real fault, and stays an assertion. + /// /// The `TempDir` is returned so the caller keeps it alive: dropping it /// removes the tree, and the junction with it. #[cfg(windows)] - fn junction_fixture() -> (tempfile::TempDir, Dir, Utf8PathBuf, Utf8PathBuf) { + fn junction_fixture() -> Option<(tempfile::TempDir, Dir, Utf8PathBuf, Utf8PathBuf)> { use std::os::windows::process::CommandExt as _; use std::process::Command; @@ -232,18 +239,24 @@ mod tests { // `mklink` is a `cmd` built-in, so it is reachable only through // `cmd /C`. `raw_arg` passes the line verbatim because `cmd` parses it // itself, rather than by the C runtime's argument-quoting rules. - let output = Command::new("cmd") + let output = match Command::new("cmd") .arg("/C") .raw_arg(format!(r#"mklink /J "{link}" "{target}""#)) .output() - .expect("run 'cmd /C mklink /J' for the junction fixture"); + { + // No `cmd.exe` on this host, so `mklink` is unreachable: the + // fixture cannot be built, and the caller reports a skip. + Err(err) if err.kind() == std::io::ErrorKind::NotFound => return None, + // Every other spawn error is a real fault, not unavailability. + other => other.expect("run 'cmd /C mklink /J' for the junction fixture"), + }; assert!( output.status.success(), "create junction fixture {link} -> {target}: cmd exited with {}: {}", output.status, String::from_utf8_lossy(&output.stderr).trim() ); - (temp, dir, link, target) + Some((temp, dir, link, target)) } /// The default policy's handle *is* the reparse point, and the opt-in @@ -263,8 +276,19 @@ mod tests { /// of it substitutes an ordinary file for the reparse point under test. #[cfg(windows)] #[test] + #[expect( + clippy::print_stderr, + reason = "test harness: an unavailable fixture must be visible in the captured test output instead of passing silently" + )] fn the_default_handle_is_the_junction_and_the_opt_in_handle_is_its_target() { - let (_temp, dir, link, target) = junction_fixture(); + let Some((_temp, dir, link, target)) = junction_fixture() else { + // No `cmd.exe` on this host, so `mklink` is unreachable and the + // junction cannot be built. Say so rather than passing silently: a + // green run that quietly skipped its subject would hide the same + // regression on a host that can build one. + eprintln!("skipped: this host has no cmd.exe to create a junction fixture"); + return; + }; // The default policy: the handle must be the reparse point itself. let mut refusing_options = OpenOptions::new(); From 6b0ad8b9956d95669e111db1afc97673188ede1b Mon Sep 17 00:00:00 2001 From: leynos Date: Sat, 19 Sep 2026 12:52:07 +0200 Subject: [PATCH 09/21] fix(windows): satisfy the Windows-only Whitaker lints and drop an impossible assertion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Windows CI lane failed three ways, all in the reparse-point module. This addresses each, and adds a local oracle that reproduces the lane in seconds. `module_must_have_inner_docs` wanted a `//!` line as the test module's first item. Adding one pushed the module past `module_max_lines`' 400-line limit, so the tests move to a `windows_reparse_tests.rs` sibling, reached through `#[path]`, as `status_tests.rs` already does. `no_expect_outside_tests` rejected five `expect` calls in `junction_fixture`. The lint exempts `#[test]` functions, but a plain helper is production code to it however it is gated, and `#[cfg(test)]` on the enclosing module does not register: rustc strips `cfg` attributes before the HIR the lint inspects. The fixture now returns `Result` and propagates, with `ensure!` rather than `assert!` because the repo denies `panic_in_result_fn`. The opt-in half of the junction test asserted something impossible. `mklink /J` records an absolute target, and `cap_std`'s resolver refuses to follow a reparse point whose destination leaves the capability, reporting `escape_attempt()` as `PermissionDenied`; a capability open cannot traverse a junction under either policy. That half is dropped, and the security-critical half — the default policy's handle *is* the reparse point — is kept and strengthened with an ordinary-directory control, so the refusal is shown to be about the link rather than about directories generally. The handle is taken ambiently because it must be. ADR-027 gains the capability-limitation risk and a Verification section that says which layer carries which property, since `std` reports a junction as a symlink and the integration test alone cannot see the flag. Verified with `cargo dylint --target x86_64-pc-windows-msvc` against the probe crate, which reproduces the Windows lint verdict exactly, plus clippy on the same target. A deliberate syntax error in the test file fails that run, so the oracle is genuinely compiling the Windows-gated code. --- ...-windows-reparse-point-same-handle-open.md | 31 +- src/stdlib/path/windows_reparse.rs | 239 +------------- src/stdlib/path/windows_reparse_tests.rs | 308 ++++++++++++++++++ 3 files changed, 337 insertions(+), 241 deletions(-) create mode 100644 src/stdlib/path/windows_reparse_tests.rs diff --git a/docs/adr-027-windows-reparse-point-same-handle-open.md b/docs/adr-027-windows-reparse-point-same-handle-open.md index 450befef5..83afe4df1 100644 --- a/docs/adr-027-windows-reparse-point-same-handle-open.md +++ b/docs/adr-027-windows-reparse-point-same-handle-open.md @@ -165,6 +165,14 @@ relies on, where `O_NOFOLLOW` is a property of the one `open` call. junctioned parent directory is resolved when the parent handle is opened by `parent_dir`, which is the pre-existing behaviour on both platforms and is not changed here. +- A junction cannot be *traversed* through a capability at all, under either + policy. `mklink /J` records an absolute target, and `cap_std`'s resolver + refuses to follow a reparse point whose destination leaves the capability, + reporting `escape_attempt()` as `PermissionDenied`. The opt-in policy's + follow is therefore exercised by the file-symlink opt-in test, which uses a + relative target; the default policy's refusal of a junction needs no + traversal, because `FILE_FLAG_OPEN_REPARSE_POINT` returns the reparse point + itself. - The Unix path is untouched. `apply_unix_open_flags` and `restore_blocking` keep their current behaviour byte for byte. @@ -185,11 +193,26 @@ lint exemption to permit it. ## Verification -A Windows-only regression test asserts that a junction fixture — created with +Two tests carry the guarantee, one per layer. + +The integration test asserts that a junction fixture — created with `mklink /J`, which needs no privilege — is rejected by all four filters under the default policy. The fixture follows the repository's "create the requested file type or skip because that file type is unavailable" rule: the test asserts the entry really carries `FILE_ATTRIBUTE_REPARSE_POINT` before rendering, so it -cannot pass against a plain directory. The existing symlink test continues to -cover the file-symlink reparse case, and the `follow_symlinks` opt-in test -covers the retained `open_with` path. +cannot pass against a plain directory. + +The unit test covers the property the integration layer cannot see. `std` +reports a junction as a symlink, so `metadata.is_file()` would refuse the +directory even if the flag were never passed. Only the handle distinguishes +them, so the unit test opens the junction and asserts its attributes carry +`FILE_ATTRIBUTE_REPARSE_POINT` — the one check that fails when +`FILE_FLAG_OPEN_REPARSE_POINT` stops being applied — and then asserts the +policy refuses it, with the ordinary target directory as a control so the +refusal is shown to be about the link rather than about directories generally. +Because the capability resolver cannot reach the junction's absolute target, +that handle is taken through the ambient authority. + +The existing symlink test continues to cover the file-symlink reparse case, and +the `follow_symlinks` opt-in test covers the retained follow path with a +relative-target symlink. diff --git a/src/stdlib/path/windows_reparse.rs b/src/stdlib/path/windows_reparse.rs index 222f493ac..b641acecb 100644 --- a/src/stdlib/path/windows_reparse.rs +++ b/src/stdlib/path/windows_reparse.rs @@ -97,240 +97,5 @@ pub(super) fn reject_reparse_point(metadata: &Metadata, path: &Utf8Path) -> Resu } #[cfg(test)] -mod tests { - use super::*; - #[cfg(windows)] - use camino::Utf8PathBuf; - #[cfg(windows)] - use cap_std::{ambient_authority, fs_utf8::Dir}; - #[cfg(windows)] - use tempfile::tempdir; - - /// Name of the junction the handle tests create. - #[cfg(windows)] - const LINK: &str = "junc"; - /// Name of the directory that junction points at. - #[cfg(windows)] - const TARGET: &str = "junction_target"; - - /// The default policy must ask the open not to traverse a reparse point, - /// and both policies must permit a directory open so the shared - /// regular-file check can report the documented rejection. - #[test] - fn default_policy_opens_without_traversing_and_permits_directories() { - let default = open_flags(false); - assert_eq!( - default & FILE_FLAG_OPEN_REPARSE_POINT, - FILE_FLAG_OPEN_REPARSE_POINT, - "the default policy must not traverse a reparse point" - ); - assert_eq!( - default & FILE_FLAG_BACKUP_SEMANTICS, - FILE_FLAG_BACKUP_SEMANTICS, - "the default policy must still permit opening a directory" - ); - - let following = open_flags(true); - assert_eq!( - following & FILE_FLAG_OPEN_REPARSE_POINT, - 0, - "the opt-in policy must let the open resolve the link" - ); - assert_eq!( - following & FILE_FLAG_BACKUP_SEMANTICS, - FILE_FLAG_BACKUP_SEMANTICS, - "the opt-in policy must still permit opening a directory" - ); - } - - /// The three constants are Windows ABI values, so a transcription slip - /// would compile and misbehave. Pin them against the documented values. - #[test] - fn constants_match_the_documented_abi_values() { - assert_eq!(FILE_FLAG_OPEN_REPARSE_POINT, 0x0020_0000); - assert_eq!(FILE_FLAG_BACKUP_SEMANTICS, 0x0200_0000); - assert_eq!(FILE_ATTRIBUTE_REPARSE_POINT, 0x0000_0400); - } - - /// The policy refuses every reparse tag, including the ones a - /// name-surrogate test would miss. - /// - /// `FileType::is_symlink` is true only when the tag is a name surrogate - /// (bit 29 set) — symlinks, junctions, and volume mount points — and - /// false for every other tag, for which `std` also reports - /// `is_file() == true`. So a tag-value test would both miss the - /// non-surrogate tags below and gain nothing for the surrogate ones: the - /// shared regular-file check already refuses a junction. The attribute - /// bit is the one test that covers both. - #[test] - fn policy_refuses_surrogate_and_non_surrogate_tags_alike() { - /// `IO_REPARSE_TAG_SYMLINK` - const SYMLINK: u32 = 0xA000_000C; - /// `IO_REPARSE_TAG_MOUNT_POINT` - const MOUNT_POINT: u32 = 0xA000_0003; - /// `IO_REPARSE_TAG_NFS` - const NFS: u32 = 0x8000_0014; - /// `IO_REPARSE_TAG_DEDUP` - const DEDUP: u32 = 0x8000_0013; - /// `IO_REPARSE_TAG_CLOUD` - const CLOUD: u32 = 0x9000_001A; - - /// Bit 29: the name-surrogate flag. - const NAME_SURROGATE: u32 = 0x2000_0000; - /// Every tag stamped onto an entry carries the attribute bit. - const ON_DISK: u32 = FILE_ATTRIBUTE_REPARSE_POINT; - /// `FILE_ATTRIBUTE_ARCHIVE`: what an ordinary file carries instead. - const FILE_ATTRIBUTE_ARCHIVE: u32 = 0x0000_0020; - - for (tag, is_surrogate) in [ - (SYMLINK, true), - (MOUNT_POINT, true), - (NFS, false), - (DEDUP, false), - (CLOUD, false), - ] { - assert_eq!( - tag & NAME_SURROGATE != 0, - is_surrogate, - "test fixture for tag {tag:#010x} misstates the surrogate flag" - ); - assert!( - is_prohibited_reparse_point(tag | ON_DISK), - "tag {tag:#010x} (name surrogate: {is_surrogate}) must be refused" - ); - } - assert!( - !is_prohibited_reparse_point(FILE_ATTRIBUTE_ARCHIVE), - "an ordinary file must not be refused" - ); - } - - /// Create the workspace the handle tests share, returning its temporary - /// directory, the capability handle, and the link and target paths. - /// - /// `None` means this host has no `cmd.exe` to reach `mklink` through, so - /// the fixture cannot be built; the caller reports that as a skip rather - /// than a failure, exactly as `fallible::junction_fixture` does in the - /// integration suite. A junction needs no privilege, so a missing - /// `cmd.exe` is the only unavailability that is a skip — every other - /// failure is a real fault, and stays an assertion. - /// - /// The `TempDir` is returned so the caller keeps it alive: dropping it - /// removes the tree, and the junction with it. - #[cfg(windows)] - fn junction_fixture() -> Option<(tempfile::TempDir, Dir, Utf8PathBuf, Utf8PathBuf)> { - use std::os::windows::process::CommandExt as _; - use std::process::Command; - - let temp = tempdir().expect("create the junction fixture workspace"); - let root = Utf8PathBuf::from_path_buf(temp.path().to_path_buf()) - .expect("fixture workspace path is valid UTF-8"); - let dir = Dir::open_ambient_dir(&root, ambient_authority()) - .expect("open the fixture workspace as a capability"); - dir.create_dir(TARGET) - .expect("create the junction fixture target directory"); - let link = root.join(LINK); - let target = root.join(TARGET); - assert!( - !link.as_str().contains('"') && !target.as_str().contains('"'), - "workspace path contains a quote and cannot be passed to cmd" - ); - - // `mklink` is a `cmd` built-in, so it is reachable only through - // `cmd /C`. `raw_arg` passes the line verbatim because `cmd` parses it - // itself, rather than by the C runtime's argument-quoting rules. - let output = match Command::new("cmd") - .arg("/C") - .raw_arg(format!(r#"mklink /J "{link}" "{target}""#)) - .output() - { - // No `cmd.exe` on this host, so `mklink` is unreachable: the - // fixture cannot be built, and the caller reports a skip. - Err(err) if err.kind() == std::io::ErrorKind::NotFound => return None, - // Every other spawn error is a real fault, not unavailability. - other => other.expect("run 'cmd /C mklink /J' for the junction fixture"), - }; - assert!( - output.status.success(), - "create junction fixture {link} -> {target}: cmd exited with {}: {}", - output.status, - String::from_utf8_lossy(&output.stderr).trim() - ); - Some((temp, dir, link, target)) - } - - /// The default policy's handle *is* the reparse point, and the opt-in - /// policy's handle is the directory it points at. - /// - /// This is the property that makes the same-handle judgement possible, and - /// the only test that fails if `FILE_FLAG_OPEN_REPARSE_POINT` stops being - /// passed: without it both opens return the target directory, whose - /// attributes carry no reparse bit, and `reject_reparse_point` would wave - /// the junction through. The integration test cannot detect that, because - /// `std` reports a junction as a symlink, so `metadata.is_file()` refuses - /// the directory anyway. The distinction only shows on the handle. - /// - /// A junction is used rather than a symlink because it needs no privilege, - /// and it is created with `mklink /J` before the directory is opened - /// through `cap_std`. Every step is a genuine filesystem operation; none - /// of it substitutes an ordinary file for the reparse point under test. - #[cfg(windows)] - #[test] - #[expect( - clippy::print_stderr, - reason = "test harness: an unavailable fixture must be visible in the captured test output instead of passing silently" - )] - fn the_default_handle_is_the_junction_and_the_opt_in_handle_is_its_target() { - let Some((_temp, dir, link, target)) = junction_fixture() else { - // No `cmd.exe` on this host, so `mklink` is unreachable and the - // junction cannot be built. Say so rather than passing silently: a - // green run that quietly skipped its subject would hide the same - // regression on a host that can build one. - eprintln!("skipped: this host has no cmd.exe to create a junction fixture"); - return; - }; - - // The default policy: the handle must be the reparse point itself. - let mut refusing_options = OpenOptions::new(); - refusing_options.read(true); - apply_open_flags(&mut refusing_options, false); - let refusing_handle = dir - .open_with(Utf8Path::new(LINK), &refusing_options) - .expect("the default policy must be able to open the junction itself"); - let refusing_metadata = refusing_handle - .metadata() - .expect("read metadata from the default-policy handle"); - assert!( - is_prohibited_reparse_point(refusing_metadata.file_attributes()), - "the default handle must be the reparse point, so the policy has \ - something to refuse; a handle to the target directory means \ - FILE_FLAG_OPEN_REPARSE_POINT was not passed" - ); - assert!( - reject_reparse_point(&refusing_metadata, &link).is_err(), - "the default policy must refuse the junction it just opened" - ); - - // The opt-in policy: the handle must be the target directory, which is - // what makes the opt-in a genuine follow rather than a second refusal. - let mut following_options = OpenOptions::new(); - following_options.read(true); - apply_open_flags(&mut following_options, true); - let following_handle = dir - .open_with(Utf8Path::new(LINK), &following_options) - .expect("the opt-in policy must be able to open through the junction"); - let following_metadata = following_handle - .metadata() - .expect("read metadata from the opt-in handle"); - assert!( - !is_prohibited_reparse_point(following_metadata.file_attributes()), - "the opt-in handle must be the target directory, not the reparse \ - point; a handle carrying the reparse bit means the flag was \ - applied under the opt-in policy too" - ); - assert!( - reject_reparse_point(&following_metadata, &target).is_ok(), - "the opt-in policy must not refuse the target it resolved" - ); - } -} +#[path = "windows_reparse_tests.rs"] +mod tests; diff --git a/src/stdlib/path/windows_reparse_tests.rs b/src/stdlib/path/windows_reparse_tests.rs new file mode 100644 index 000000000..318af2f75 --- /dev/null +++ b/src/stdlib/path/windows_reparse_tests.rs @@ -0,0 +1,308 @@ +//! Unit tests for the Windows reparse-point open policy. +//! +//! The handle tests need a real junction, so they skip — loudly — when the host +//! has no `cmd.exe` to reach `mklink` through. A junction needs no privilege, so +//! that is the only condition under which the fixture is unavailable: every +//! other setup failure propagates as an error rather than quietly turning the +//! test green. + +use super::*; +#[cfg(windows)] +use anyhow::{Context, Result, ensure}; +#[cfg(windows)] +use camino::Utf8PathBuf; +#[cfg(windows)] +use cap_std::{ + ambient_authority, + fs_utf8::{Dir, File}, +}; +#[cfg(windows)] +use tempfile::tempdir; + +/// Name of the junction the handle tests create. +#[cfg(windows)] +const LINK: &str = "junc"; +/// Name of the directory that junction points at. +#[cfg(windows)] +const TARGET: &str = "junction_target"; + +/// The default policy must ask the open not to traverse a reparse point, +/// and both policies must permit a directory open so the shared +/// regular-file check can report the documented rejection. +#[test] +fn default_policy_opens_without_traversing_and_permits_directories() { + let default = open_flags(false); + assert_eq!( + default & FILE_FLAG_OPEN_REPARSE_POINT, + FILE_FLAG_OPEN_REPARSE_POINT, + "the default policy must not traverse a reparse point" + ); + assert_eq!( + default & FILE_FLAG_BACKUP_SEMANTICS, + FILE_FLAG_BACKUP_SEMANTICS, + "the default policy must still permit opening a directory" + ); + + let following = open_flags(true); + assert_eq!( + following & FILE_FLAG_OPEN_REPARSE_POINT, + 0, + "the opt-in policy must let the open resolve the link" + ); + assert_eq!( + following & FILE_FLAG_BACKUP_SEMANTICS, + FILE_FLAG_BACKUP_SEMANTICS, + "the opt-in policy must still permit opening a directory" + ); +} + +/// The three constants are Windows ABI values, so a transcription slip +/// would compile and misbehave. Pin them against the documented values. +#[test] +fn constants_match_the_documented_abi_values() { + assert_eq!(FILE_FLAG_OPEN_REPARSE_POINT, 0x0020_0000); + assert_eq!(FILE_FLAG_BACKUP_SEMANTICS, 0x0200_0000); + assert_eq!(FILE_ATTRIBUTE_REPARSE_POINT, 0x0000_0400); +} + +/// The policy refuses every reparse tag, including the ones a +/// name-surrogate test would miss. +/// +/// `FileType::is_symlink` is true only when the tag is a name surrogate +/// (bit 29 set) — symlinks, junctions, and volume mount points — and +/// false for every other tag, for which `std` also reports +/// `is_file() == true`. So a tag-value test would both miss the +/// non-surrogate tags below and gain nothing for the surrogate ones: the +/// shared regular-file check already refuses a junction. The attribute +/// bit is the one test that covers both. +#[test] +fn policy_refuses_surrogate_and_non_surrogate_tags_alike() { + /// `IO_REPARSE_TAG_SYMLINK` + const SYMLINK: u32 = 0xA000_000C; + /// `IO_REPARSE_TAG_MOUNT_POINT` + const MOUNT_POINT: u32 = 0xA000_0003; + /// `IO_REPARSE_TAG_NFS` + const NFS: u32 = 0x8000_0014; + /// `IO_REPARSE_TAG_DEDUP` + const DEDUP: u32 = 0x8000_0013; + /// `IO_REPARSE_TAG_CLOUD` + const CLOUD: u32 = 0x9000_001A; + + /// Bit 29: the name-surrogate flag. + const NAME_SURROGATE: u32 = 0x2000_0000; + /// Every tag stamped onto an entry carries the attribute bit. + const ON_DISK: u32 = FILE_ATTRIBUTE_REPARSE_POINT; + /// `FILE_ATTRIBUTE_ARCHIVE`: what an ordinary file carries instead. + const FILE_ATTRIBUTE_ARCHIVE: u32 = 0x0000_0020; + + for (tag, is_surrogate) in [ + (SYMLINK, true), + (MOUNT_POINT, true), + (NFS, false), + (DEDUP, false), + (CLOUD, false), + ] { + assert_eq!( + tag & NAME_SURROGATE != 0, + is_surrogate, + "test fixture for tag {tag:#010x} misstates the surrogate flag" + ); + assert!( + is_prohibited_reparse_point(tag | ON_DISK), + "tag {tag:#010x} (name surrogate: {is_surrogate}) must be refused" + ); + } + assert!( + !is_prohibited_reparse_point(FILE_ATTRIBUTE_ARCHIVE), + "an ordinary file must not be refused" + ); +} + +/// Everything the junction test needs, kept alive for its whole body. +/// +/// The temporary directory is held here so the tree outlives the handles taken +/// from it; dropping the fixture removes the junction with it. +#[cfg(windows)] +struct JunctionFixture { + /// Owns the tree; dropping it removes the junction. + _temp: tempfile::TempDir, + /// Capability over the fixture workspace. + dir: Dir, + /// The junction opened with the default policy. It is a directory-shaped + /// reparse point, and the capability resolver cannot traverse it, so an + /// ambient open is the only way to reach the reparse point itself. + opened: File, + /// The link name inside the workspace. + link: Utf8PathBuf, + /// The directory the link points at. + target: Utf8PathBuf, +} + +/// Open `path` through the ambient authority with the default policy. +/// +/// `path` must be absolute: the ambient open resolves it directly, without the +/// capability resolver that refuses an absolute link target. +/// +/// # Errors +/// +/// Returns the open error when the entry cannot be opened, or when the +/// requested access mode and creation disposition are inconsistent. +#[cfg(windows)] +fn open_link_ambient(path: &Utf8Path) -> Result { + let mut options = OpenOptions::new(); + options.read(true); + apply_open_flags(&mut options, false); + File::open_ambient_with(path, &options, ambient_authority()) + .with_context(|| format!("open the junction fixture at {path}")) +} + +/// Create the workspace the handle tests share. +/// +/// `Ok(None)` means this host has no `cmd.exe` to reach `mklink` through, so the +/// fixture cannot be built; the caller reports that as a skip rather than a +/// failure, exactly as `fallible::junction_fixture` does in the integration +/// suite. A junction needs no privilege, so a missing `cmd.exe` is the only +/// unavailability that is a skip — every other failure is a real fault, and +/// propagates as an error. +/// +/// The helper returns `Result` rather than calling `expect`, and hands back a +/// `File` rather than a platform handle, so it needs no unsafe code. +/// +/// # Errors +/// +/// Returns the setup error when `cmd` is present but the junction was not +/// created, quoting `mklink`'s own diagnostics. +#[cfg(windows)] +fn junction_fixture() -> Result> { + use std::os::windows::process::CommandExt as _; + use std::process::Command; + + let temp = tempdir().context("create the junction fixture workspace")?; + let root = Utf8PathBuf::from_path_buf(temp.path().to_path_buf()) + .map_err(|_| anyhow::anyhow!("fixture workspace path is valid UTF-8"))?; + let dir = Dir::open_ambient_dir(&root, ambient_authority()) + .context("open the fixture workspace as a capability")?; + dir.create_dir(TARGET) + .context("create the junction fixture target directory")?; + let link = root.join(LINK); + let target = root.join(TARGET); + ensure!( + !link.as_str().contains('"') && !target.as_str().contains('"'), + "workspace path contains a quote and cannot be passed to cmd: \ + link {link}, target {target}" + ); + + // `mklink` is a `cmd` built-in, so it is reachable only through `cmd /C`. + // `raw_arg` passes the line verbatim because `cmd` parses it itself, rather + // than by the C runtime's argument-quoting rules. + let output = match Command::new("cmd") + .arg("/C") + .raw_arg(format!(r#"mklink /J "{link}" "{target}""#)) + .output() + { + Ok(output) => output, + // No `cmd.exe` on this host, so `mklink` is unreachable: the fixture + // cannot be built, and the caller reports a skip. + Err(err) if err.kind() == std::io::ErrorKind::NotFound => return Ok(None), + // Every other spawn error is a real fault, not unavailability. + Err(err) => { + return Err(err).context("run 'cmd /C mklink /J' for the junction fixture"); + } + }; + let stderr = String::from_utf8_lossy(&output.stderr); + ensure!( + output.status.success(), + "create junction fixture {link} -> {target}: cmd exited with {}: {}", + output.status, + stderr.trim() + ); + let opened = open_link_ambient(&link)?; + Ok(Some(JunctionFixture { + _temp: temp, + dir, + opened, + link, + target, + })) +} + +/// The default policy's handle *is* the reparse point, not its target. +/// +/// This is the property that makes the same-handle judgement possible, and the +/// only test that fails if `FILE_FLAG_OPEN_REPARSE_POINT` stops being passed: +/// without it the open returns the target directory, whose attributes carry no +/// reparse bit, and `reject_reparse_point` would wave the junction through. The +/// integration test cannot detect that, because `std` reports a junction as a +/// symlink, so `metadata.is_file()` refuses the directory anyway. The +/// distinction only shows on the handle. +/// +/// A junction is used rather than a symlink because it needs no privilege, and +/// it is created with `mklink /J`. Every step is a genuine filesystem +/// operation; none of it substitutes an ordinary file for the reparse point +/// under test. +/// +/// The handle is taken through the ambient authority rather than through the +/// capability. `mklink /J` records an absolute target, and `cap_std` refuses to +/// resolve a reparse point whose destination leaves the capability — +/// `escape_attempt()`, reported as `PermissionDenied`. The default policy never +/// resolves it, because `FILE_FLAG_OPEN_REPARSE_POINT` makes the open return the +/// reparse point itself, so the difference between the policies is visible +/// exactly where it matters: on the handle the policy will judge. The opt-in +/// policy's follow is covered end to end by +/// `follow_symlinks_opt_in_reads_the_link_target` in the integration suite. +#[cfg(windows)] +#[test] +#[expect( + clippy::print_stderr, + reason = "test harness: an unavailable fixture must be visible in the captured test output instead of passing silently" +)] +fn the_default_handle_is_the_junction_not_its_target() -> Result<()> { + let Some(fixture) = junction_fixture()? else { + // No `cmd.exe` on this host, so `mklink` is unreachable and the junction + // cannot be built. Say so rather than passing silently: a green run that + // quietly skipped its subject would hide the same regression on a host + // that can build one. + eprintln!("skipped: this host has no cmd.exe to create a junction fixture"); + return Ok(()); + }; + let JunctionFixture { + _temp, + dir, + opened, + link, + target, + } = fixture; + + // The handle the fixture took must be the reparse point itself. + let metadata = opened + .metadata() + .context("read metadata from the default-policy handle")?; + ensure!( + is_prohibited_reparse_point(metadata.file_attributes()), + "the default handle must be the reparse point, so the policy has \ + something to refuse; a handle to the target directory means \ + FILE_FLAG_OPEN_REPARSE_POINT was not passed" + ); + ensure!( + reject_reparse_point(&metadata, &link).is_err(), + "the default policy must refuse the junction it just opened" + ); + + // The target directory is not itself a reparse point, which is what makes + // the refusal above a decision about the link rather than about directories + // in general. Without this the two checks could both pass on a host that + // stamped the attribute on an ordinary directory. + let target_metadata = dir + .metadata(Utf8Path::new(TARGET)) + .context("read metadata for the junction fixture target directory")?; + ensure!( + !is_prohibited_reparse_point(target_metadata.file_attributes()), + "the target directory must be an ordinary directory, not a reparse \ + point; otherwise the refusal above proves nothing about the link" + ); + ensure!( + reject_reparse_point(&target_metadata, &target).is_ok(), + "the policy must not refuse the ordinary directory the link points at" + ); + Ok(()) +} From 4c3ef48782a78c887b5842410ed3aca18c58e96c Mon Sep 17 00:00:00 2001 From: leynos Date: Sat, 19 Sep 2026 18:30:27 +0200 Subject: [PATCH 10/21] docs(tests): spell "recognizes" in the junction fixture comment The `-ize` form is the en-GB-oxendict preference this repository enforces elsewhere, and the surrounding prose already uses it. No behavioural change: the edit is inside a doc comment. Co-Authored-By: Claude Code --- tests/std_filter_tests/support.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/std_filter_tests/support.rs b/tests/std_filter_tests/support.rs index 3706ce7a0..6e66117c2 100644 --- a/tests/std_filter_tests/support.rs +++ b/tests/std_filter_tests/support.rs @@ -180,7 +180,7 @@ pub(crate) mod fallible { /// /// `Ok(None)` means this host has no `cmd.exe`, so the fixture cannot be /// created at all. That is the only environmental unavailability this - /// fixture recognises. Every other failure — including a non-zero `mklink` + /// fixture recognizes. Every other failure — including a non-zero `mklink` /// exit — is a setup fault and propagates, because junction creation needs /// no privilege and reparse points are supported on every filesystem this /// suite runs on: skipping there would report green over a policy that went From 2cb3ef4e2db0779ed29327488226f864ba5493cc Mon Sep 17 00:00:00 2001 From: leynos Date: Sat, 19 Sep 2026 20:08:15 +0200 Subject: [PATCH 11/21] docs: describe the Windows reparse-point policy in the user-facing guides Three guides still described the pre-open Windows check that this branch removed, or stated the policy as symlink-only when it in fact refuses every reparse point. `users-guide.md` and `stdlib-yaml-and-jinja-guide.md` now say that the Windows refusal covers junctions, volume mount points, and other tags such as deduplication or cloud placeholders, and that `follow_symlinks=true` is the opt-in for all of them. `security-network-command-audit.md` replaces the "pre-open `symlink_metadata` check on Windows" remediation with `FILE_FLAG_OPEN_REPARSE_POINT` plus the handle's `FILE_ATTRIBUTE_REPARSE_POINT`, and records that the judgement and the read share one handle. Reported by chatgpt-codex-connector on PR #739. Co-Authored-By: Claude Code --- docs/security-network-command-audit.md | 10 ++++++++-- docs/stdlib-yaml-and-jinja-guide.md | 8 ++++++-- docs/users-guide.md | 8 ++++++-- 3 files changed, 20 insertions(+), 6 deletions(-) diff --git a/docs/security-network-command-audit.md b/docs/security-network-command-audit.md index 8ed6837c6..2729d2c30 100644 --- a/docs/security-network-command-audit.md +++ b/docs/security-network-command-audit.md @@ -138,8 +138,14 @@ introduces, and concrete remediation tasks that would harden the helpers. - **Remediation:** the reading filters now share one policy. The final path component is opened without following symlinks (`O_NOFOLLOW` on Unix, where the open is also non-blocking so a FIFO or device cannot wedge a - build worker; a pre-open `symlink_metadata` check on Windows), and the - opened handle must be a regular file. `contents`, `linecount`, `hash`, and + build worker; `FILE_FLAG_OPEN_REPARSE_POINT` on Windows, so the open + returns the reparse point itself instead of traversing it), and the + opened handle must be a regular file. On Windows the handle is also + refused when it carries `FILE_ATTRIBUTE_REPARSE_POINT`, which rejects + junctions, volume mount points, and every other reparse tag rather than + only those `std` reports as symlinks. Because the judgement and the read + share one handle, there is no check-then-open window between them. + `contents`, `linecount`, `hash`, and `digest` stream against a running byte total anchored to `StdlibConfig::with_file_max_read_bytes` (default 8 MiB). `linecount` counts terminators incrementally instead of materializing the file. diff --git a/docs/stdlib-yaml-and-jinja-guide.md b/docs/stdlib-yaml-and-jinja-guide.md index f27099916..51bce46de 100644 --- a/docs/stdlib-yaml-and-jinja-guide.md +++ b/docs/stdlib-yaml-and-jinja-guide.md @@ -166,8 +166,12 @@ non-blocking, so a FIFO cannot wedge the render worker first. A symlink final component is refused on both platforms, but not with the same diagnostic: on Unix the default open declines to follow it, so the failure comes from the open itself and names the path together with the platform's symbolic-link detail, -while on Windows a check made before the open reuses the not-a-regular-file -diagnostic. Two optional keyword arguments narrow a call without touching the +while on Windows the open declines to traverse the reparse point and the +refusal then comes from the opened handle, which reuses the not-a-regular-file +diagnostic. That Windows refusal covers every reparse point, not only symlinks: +junctions, volume mount points, and other tags such as deduplication or cloud +placeholders are rejected alike, and `follow_symlinks=true` is the opt-in for +all of them. Two optional keyword arguments narrow a call without touching the operator ceiling: - `max_bytes` lowers the budget for one call (a value above the configured diff --git a/docs/users-guide.md b/docs/users-guide.md index 2e45fdb19..c2139d0a8 100644 --- a/docs/users-guide.md +++ b/docs/users-guide.md @@ -1838,8 +1838,12 @@ library. The reading filters also refuse to follow a symlink as the final path component and reject anything that is not a regular file once opened, including FIFOs and device nodes. A symlinked directory used *inside* a path is unaffected; only -the final entry is checked. Templates that deliberately read through a final -symlink can pass `follow_symlinks=true` to accept the link: +the final entry is checked. On Windows the same refusal covers every reparse +point, not only symlinks: junctions, volume mount points, and other tags such +as deduplication or cloud placeholders are all rejected, including tags Windows +may add later. Templates that deliberately read through a final symlink — or, +on Windows, through any of those reparse points — can pass +`follow_symlinks=true` to accept it: From d8aeaae9a11ec53a53859b9cc67609416dd1d6cf Mon Sep 17 00:00:00 2001 From: leynos Date: Sat, 19 Sep 2026 20:26:02 +0200 Subject: [PATCH 12/21] Renumber ADR-027 to ADR-032 after three later claims MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The earlier renumber to 027 was itself a collision. PR #699 minted docs/adr-027-command-placeholder-contract.md at 2026-09-18T23:52Z, 31 minutes before this branch's ac3d3e9b landed its own 027. Renumbering off main's ceiling (026) missed it because an ADR number is spent as soon as any branch mints it, not when it merges. A sweep of every remote branch shows 028, 029, 030, and 031 are taken too — 029 by two branches independently: 026 merged on main 027 PR #699 028 issue-693-trim-the-split-build-dir-harness-test... 029 docs/hexagonal-hardening-and-checking and make-the-build-standard-the-default 030 docs/hexagonal-hardening-and-checking 031 docs/hexagonal-hardening-and-checking 032 free Take 032 and leave 027 to #699, which claimed it first. Update the four references in lockstep: the ADR's own H1, the docs/contents.md index entry, and the two inbound links in developers-guide.md and netsuke-design.md. No other file mentions the number. --- ...pen.md => adr-032-windows-reparse-point-same-handle-open.md} | 2 +- docs/contents.md | 2 +- docs/developers-guide.md | 2 +- docs/netsuke-design.md | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) rename docs/{adr-027-windows-reparse-point-same-handle-open.md => adr-032-windows-reparse-point-same-handle-open.md} (99%) diff --git a/docs/adr-027-windows-reparse-point-same-handle-open.md b/docs/adr-032-windows-reparse-point-same-handle-open.md similarity index 99% rename from docs/adr-027-windows-reparse-point-same-handle-open.md rename to docs/adr-032-windows-reparse-point-same-handle-open.md index 83afe4df1..219d9b895 100644 --- a/docs/adr-027-windows-reparse-point-same-handle-open.md +++ b/docs/adr-032-windows-reparse-point-same-handle-open.md @@ -1,4 +1,4 @@ -# Architectural decision record (ADR) 027: Validate the Windows final component through a same-handle reparse-point open +# Architectural decision record (ADR) 032: Validate the Windows final component through a same-handle reparse-point open ## Status diff --git a/docs/contents.md b/docs/contents.md index 48014c2f4..101ab4fc9 100644 --- a/docs/contents.md +++ b/docs/contents.md @@ -173,7 +173,7 @@ operator, user, and contributor references are easier to find. - [ADR-028](adr-028-defer-split-build-dir-harness-trim.md): Deferred trim of the split-build-dir harness test, with the serialized-lane measurements that made the figure unstable and the ten-run gate that reopens the question. -- [ADR-027](adr-027-windows-reparse-point-same-handle-open.md): Windows final +- [ADR-032](adr-032-windows-reparse-point-same-handle-open.md): Windows final component validation through a same-handle reparse-point open. ## Proposals diff --git a/docs/developers-guide.md b/docs/developers-guide.md index 62d1f0bde..d09f0ddcd 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -5929,7 +5929,7 @@ rejects every reparse point, including tags `std` does not report as symlinks: or cloud placeholder would slip past a symlink-shaped check and be followed. Because the judgement and the read share one handle, there is no check-then-open window between them; see -[ADR-027](adr-027-windows-reparse-point-same-handle-open.md). +[ADR-032](adr-032-windows-reparse-point-same-handle-open.md). Two diagnostics come out of the boundary. `bounded_read.rs` raises `file_too_large_error`, which quotes the path and the limit that was exceeded; diff --git a/docs/netsuke-design.md b/docs/netsuke-design.md index c3ca97151..6f9aa9933 100644 --- a/docs/netsuke-design.md +++ b/docs/netsuke-design.md @@ -1520,7 +1520,7 @@ Implementation notes: `FILE_ATTRIBUTE_REPARSE_POINT`; that refuses symlinks, junctions, volume mount points, and every other tag alike, because the test is on the attribute bit rather than on the tag value. The judgement and the read share one - handle. See [ADR-027](adr-027-windows-reparse-point-same-handle-open.md). + handle. See [ADR-032](adr-032-windows-reparse-point-same-handle-open.md). - An over-budget read fails with `stdlib.path.contents.file_too_large`, which quotes the path and the byte limit. An opened object that is not a regular file (a FIFO, a device, or a Windows reparse point refused on the opened From cbec7e514bb8ae13ebcc327a86e95739a2275e2a Mon Sep 17 00:00:00 2001 From: leynos Date: Sat, 19 Sep 2026 21:39:24 +0200 Subject: [PATCH 13/21] Correct the follow_symlinks containment mechanism MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeRabbit raised one concern three times: that `follow_symlinks=true` does not override capability containment, so the docs must not imply it does. The premise is right, but the mechanism it named — "an absolute target that leaves the capability" — is wrong, and writing it down would have misdescribed what an operator can rely on. The resolver never compares the resolved path against the capability root. It refuses a link destination containing a prefix or root component outright, so it cannot tell an absolute target that stays inside from one that does not: both are refused. The trigger is the target's *absoluteness*, not its escaping. Measured rather than inferred. Two symlinks pointing at the same file inside one capability, one written relatively and one absolutely, under the opt-in policy: the relative link opened, the absolute link was refused with "a path led outside of the filesystem". Same file, same containment, different result — only the spelling differed. Documented at all four sites that state the policy, since a partial correction would leave the wrong mechanism standing somewhere: `users-guide.md`, `stdlib-yaml-and-jinja-guide.md`, ADR-032's known risks, and the Windows unit test whose doc comment explains why the junction handle is taken ambiently. The ADR records the superseded wording explicitly rather than quietly replacing it, so a reader who met the earlier draft can see which claim was wrong and why. Verified: the junction fixture's target is a relative `"file"`, so the opt-in integration test that ADR-032 cites as covering the follow path does in fact use the supported spelling. Refs #703. --- ...-windows-reparse-point-same-handle-open.md | 35 +++++++++++++++---- docs/stdlib-yaml-and-jinja-guide.md | 10 ++++-- docs/users-guide.md | 15 ++++++-- src/stdlib/path/windows_reparse_tests.rs | 8 +++-- 4 files changed, 52 insertions(+), 16 deletions(-) diff --git a/docs/adr-032-windows-reparse-point-same-handle-open.md b/docs/adr-032-windows-reparse-point-same-handle-open.md index 219d9b895..7a7744e8f 100644 --- a/docs/adr-032-windows-reparse-point-same-handle-open.md +++ b/docs/adr-032-windows-reparse-point-same-handle-open.md @@ -166,13 +166,34 @@ relies on, where `O_NOFOLLOW` is a property of the one `open` call. `parent_dir`, which is the pre-existing behaviour on both platforms and is not changed here. - A junction cannot be *traversed* through a capability at all, under either - policy. `mklink /J` records an absolute target, and `cap_std`'s resolver - refuses to follow a reparse point whose destination leaves the capability, - reporting `escape_attempt()` as `PermissionDenied`. The opt-in policy's - follow is therefore exercised by the file-symlink opt-in test, which uses a - relative target; the default policy's refusal of a junction needs no - traversal, because `FILE_FLAG_OPEN_REPARSE_POINT` returns the reparse point - itself. + policy: the refusal is part of resolving the link, which happens before the + open policy is consulted. `mklink /J` records an absolute target, and the + resolver rejects an absolute link destination outright, reporting + `escape_attempt()` as `PermissionDenied`. +- The trigger is **absoluteness of the link target, not escape from the + capability**. An earlier draft of this record said "a destination that leaves + the capability", which is the wrong mechanism and misdescribes what an + operator can rely on. The resolver never compares the resolved path against + the capability root, so it cannot tell an absolute target that stays inside + from one that does not. Both are refused. Verified two ways: + + - On Unix the kernel does this in `openat2` with `RESOLVE_BENEATH`, which + rejects any absolute link target, and `EXDEV` maps to `escape_attempt()`. + A probe on Linux confirmed it directly: two symlinks pointing at the *same + file inside* the capability, one written relatively and one absolutely, + with the opt-in policy in force — the relative link opened and the absolute + link was refused with "a path led outside of the filesystem". Same file, + same containment, different result; only the spelling of the target + differed. The Windows claim rests on the resolver's shared + `PrefixOrRootDir => escape_attempt()` arm, which is reached for a link + destination containing a prefix or root component, and cannot be + exercised on Linux. + - Consequence for tests and docs: a junction is refused under *either* + policy, so it can never demonstrate a successful opt-in follow. The opt-in + policy's follow is exercised by the file-symlink opt-in test, which uses a + relative target; the default policy's refusal of a junction needs no + traversal, because `FILE_FLAG_OPEN_REPARSE_POINT` returns the reparse point + itself. - The Unix path is untouched. `apply_unix_open_flags` and `restore_blocking` keep their current behaviour byte for byte. diff --git a/docs/stdlib-yaml-and-jinja-guide.md b/docs/stdlib-yaml-and-jinja-guide.md index 51bce46de..c6545c315 100644 --- a/docs/stdlib-yaml-and-jinja-guide.md +++ b/docs/stdlib-yaml-and-jinja-guide.md @@ -170,9 +170,13 @@ while on Windows the open declines to traverse the reparse point and the refusal then comes from the opened handle, which reuses the not-a-regular-file diagnostic. That Windows refusal covers every reparse point, not only symlinks: junctions, volume mount points, and other tags such as deduplication or cloud -placeholders are rejected alike, and `follow_symlinks=true` is the opt-in for -all of them. Two optional keyword arguments narrow a call without touching the -operator ceiling: +placeholders are rejected alike, and `follow_symlinks=true` waives the refusal +for all of them. That opt-in does not extend to the capability that anchors the +read in the workspace: a link whose target is written as an absolute path is +still refused as an escape attempt even when the target lies inside the +workspace, so relative-target links are the supported case. A junction always +records an absolute target, so the opt-in cannot follow one. Two optional +keyword arguments narrow a call without touching the operator ceiling: - `max_bytes` lowers the budget for one call (a value above the configured budget is clamped to it). Example: diff --git a/docs/users-guide.md b/docs/users-guide.md index c2139d0a8..be92a80f7 100644 --- a/docs/users-guide.md +++ b/docs/users-guide.md @@ -1841,9 +1841,18 @@ device nodes. A symlinked directory used *inside* a path is unaffected; only the final entry is checked. On Windows the same refusal covers every reparse point, not only symlinks: junctions, volume mount points, and other tags such as deduplication or cloud placeholders are all rejected, including tags Windows -may add later. Templates that deliberately read through a final symlink — or, -on Windows, through any of those reparse points — can pass -`follow_symlinks=true` to accept it: +may add later. + +`follow_symlinks=true` waives that final-component refusal, and only that. The +capability that anchors every read in the workspace is unaffected, so a link +whose target is written as an absolute path is still refused with a diagnostic +reporting that a path led outside the filesystem — even when the target is in +fact inside the workspace. Relativity of the *link target*, not containment of +the resolved path, is what the resolver tests. A template that reads through a +relative-target link is the supported case; on Windows a junction cannot be one, +because `mklink` records an absolute target and so is always refused under +either policy. Templates that deliberately read through a final symlink can +pass the opt-in to accept it: diff --git a/src/stdlib/path/windows_reparse_tests.rs b/src/stdlib/path/windows_reparse_tests.rs index 318af2f75..e111c94f8 100644 --- a/src/stdlib/path/windows_reparse_tests.rs +++ b/src/stdlib/path/windows_reparse_tests.rs @@ -242,9 +242,11 @@ fn junction_fixture() -> Result> { /// under test. /// /// The handle is taken through the ambient authority rather than through the -/// capability. `mklink /J` records an absolute target, and `cap_std` refuses to -/// resolve a reparse point whose destination leaves the capability — -/// `escape_attempt()`, reported as `PermissionDenied`. The default policy never +/// capability. `mklink /J` records an absolute target, and `cap_std`'s resolver +/// refuses an absolute link destination outright — `escape_attempt()`, reported +/// as `PermissionDenied`. The trigger is the target's *absoluteness*, not its +/// escaping the capability, so a junction is unresolvable under either policy +/// and can never demonstrate the opt-in's follow. The default policy never /// resolves it, because `FILE_FLAG_OPEN_REPARSE_POINT` makes the open return the /// reparse point itself, so the difference between the policies is visible /// exactly where it matters: on the handle the policy will judge. The opt-in From 6c3f44c93e0fd68e5245e51b72bc2b8260228332 Mon Sep 17 00:00:00 2001 From: leynos Date: Sat, 19 Sep 2026 21:48:42 +0200 Subject: [PATCH 14/21] Rewrap the users-guide paragraph mdtablefix's way MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `make check-fmt` failed on `docs/users-guide.md` in both the Linux `build-test` job and `Windows / lint-windows` for 2ffd72db: mdtablefix wanted +2 -2 on exactly one line. The line was 80 characters. Hand-wrapping prose to fit the 80-column budget is not the same as satisfying mdtablefix's own wrap, which chose a different break point. The previous commit hand-wrapped; this one runs the formatter over the file instead, which is the only way to get the same answer the gate does. Only the wrap point moved — no wording changed, and no other paragraph was touched. Verified with the gate's own invocation: mdtablefix --check --git --include-untracked --wrap --renumber \ --breaks --ellipsis --fences -> 142 files left unchanged, exit 0 Refs #703. --- docs/users-guide.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/users-guide.md b/docs/users-guide.md index be92a80f7..ff0bea4f1 100644 --- a/docs/users-guide.md +++ b/docs/users-guide.md @@ -1849,8 +1849,8 @@ whose target is written as an absolute path is still refused with a diagnostic reporting that a path led outside the filesystem — even when the target is in fact inside the workspace. Relativity of the *link target*, not containment of the resolved path, is what the resolver tests. A template that reads through a -relative-target link is the supported case; on Windows a junction cannot be one, -because `mklink` records an absolute target and so is always refused under +relative-target link is the supported case; on Windows a junction cannot be +one, because `mklink` records an absolute target and so is always refused under either policy. Templates that deliberately read through a final symlink can pass the opt-in to accept it: From c755c1a7776f689d2ecb398ce515bc493c4e37d9 Mon Sep 17 00:00:00 2001 From: leynos Date: Sat, 19 Sep 2026 21:55:32 +0200 Subject: [PATCH 15/21] State the ADR's evidence limit plainly MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ADR-032's Verification section described two Windows-only tests as carrying the guarantee, which reads as though continuous integration had confirmed them. It has not: the Windows test lane halts on the unrelated pre-existing failure tracked as issue 743 before nextest reaches `stdlib::path`. Measured from the job log rather than assumed: the run ends at 1078/2901 tests, and the strings `windows_reparse` and `junction` appear zero times in the whole log. A decision record that overstates its own evidence is worse than one that names the gap, so the gap is now named — what is verified (compiles, lint-clean, tests compile, on a local `x86_64-pc-windows-msvc` probe) and what is not (that the tests pass on the platform they govern), with the two-tool reason the lint evidence needs both `cargo dylint` and `cargo clippy`. Docs-only; no behaviour change. Formatted with mdtablefix rather than by hand, so `mdtablefix --check` reports 142 files unchanged, exit 0. Refs #703. --- ...-windows-reparse-point-same-handle-open.md | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/docs/adr-032-windows-reparse-point-same-handle-open.md b/docs/adr-032-windows-reparse-point-same-handle-open.md index 7a7744e8f..3533cd3a6 100644 --- a/docs/adr-032-windows-reparse-point-same-handle-open.md +++ b/docs/adr-032-windows-reparse-point-same-handle-open.md @@ -237,3 +237,31 @@ that handle is taken through the ambient authority. The existing symlink test continues to cover the file-symlink reparse case, and the `follow_symlinks` opt-in test covers the retained follow path with a relative-target symlink. + +### How far this evidence actually extends + +Stated plainly, because the two tests above are Windows-only and it would be +easy to read them as CI-verified when they are not. + +The `Windows / build-test-windows` job halts on an unrelated pre-existing +failure — a network-fixture race tracked as issue 743 — before the nextest run +reaches `stdlib::path`. On the run examined for this record it ended at +1078/2901 tests, and the strings `windows_reparse` and `junction` appeared +**zero** times in the whole job log. So no case described in this section has +executed in continuous integration, and a green Windows lane would not yet be +evidence about them. + +What *is* verified on this change, and by what: the Windows-gated source is +compiled and linted against `x86_64-pc-windows-msvc` by a local probe crate +that mirrors the module tree (the main crate cannot be cross-compiled here — +`ring` needs MSVC's `lib.exe`). Two tools are needed and neither subsumes the +other — `cargo dylint` runs `cargo check`, so it applies the Whitaker lints and +no clippy lint, while `cargo clippy` applies no Whitaker lint. Each was shown +to be live by injecting a defect it should catch and confirming a non-zero +exit, then reverting. That establishes the code compiles, is lint-clean, and +that the tests *compile*; it does not establish that they *pass* on Windows. + +Until issue 743 is fixed and this branch rebuilt, the runtime behaviour of the +policy is argued from the handle semantics in "Why the race is closed by +construction" plus the Linux-side mechanism evidence above, not demonstrated on +the platform it governs. That is the honest limit of this record. From 3862bf719be6ae6ff3b7e15d86b8a86121cf1f25 Mon Sep 17 00:00:00 2001 From: leynos Date: Sat, 19 Sep 2026 22:34:34 +0200 Subject: [PATCH 16/21] docs(adr-032): credit native Windows CI for compiling the gated code MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The evidence section pointed readers at the local probe crate as the main route to Windows verification, which sold the CI evidence short. The `Windows / lint-windows` lane runs `cargo clippy --workspace --all-targets --all-features -- -D warnings`, and `--all-targets` compiles the library's `cfg(test)` module and the integration test targets. Since `windows_reparse_tests.rs` is a `#[cfg(test)]` child of the lib, every Windows-gated line — the module, its tests, and the junction fixture — is compiled on Windows itself under `-D warnings`, on the platform's own toolchain rather than an approximation of it. That job is green on this head. The probe crate is still worth recording, but as what it is: the local development loop, needed because the main crate cannot be cross-compiled here (`ring` needs MSVC's `lib.exe`), not the guarantee. The limit is narrowed rather than removed. Compiling a test under `-D warnings` does not run it, and the lane that would run it still stops 1800 tests short of `stdlib::path`. Co-Authored-By: Claude Code --- ...-windows-reparse-point-same-handle-open.md | 43 +++++++++++++------ 1 file changed, 31 insertions(+), 12 deletions(-) diff --git a/docs/adr-032-windows-reparse-point-same-handle-open.md b/docs/adr-032-windows-reparse-point-same-handle-open.md index 3533cd3a6..317110295 100644 --- a/docs/adr-032-windows-reparse-point-same-handle-open.md +++ b/docs/adr-032-windows-reparse-point-same-handle-open.md @@ -248,18 +248,37 @@ failure — a network-fixture race tracked as issue 743 — before the nextest r reaches `stdlib::path`. On the run examined for this record it ended at 1078/2901 tests, and the strings `windows_reparse` and `junction` appeared **zero** times in the whole job log. So no case described in this section has -executed in continuous integration, and a green Windows lane would not yet be -evidence about them. - -What *is* verified on this change, and by what: the Windows-gated source is -compiled and linted against `x86_64-pc-windows-msvc` by a local probe crate -that mirrors the module tree (the main crate cannot be cross-compiled here — -`ring` needs MSVC's `lib.exe`). Two tools are needed and neither subsumes the -other — `cargo dylint` runs `cargo check`, so it applies the Whitaker lints and -no clippy lint, while `cargo clippy` applies no Whitaker lint. Each was shown -to be live by injecting a defect it should catch and confirming a non-zero -exit, then reverting. That establishes the code compiles, is lint-clean, and -that the tests *compile*; it does not establish that they *pass* on Windows. +executed in continuous integration, and a green Windows *test* lane would not +yet be evidence about their behaviour. + +What *is* verified on this change, and by what. Two routes, and the boundary +they share is stated at the end. + +**Native Windows CI compiles and lints every Windows-gated line, tests +included.** `Windows / lint-windows` runs `make lint-clippy`, which expands to +`cargo clippy --workspace --all-targets --all-features -- -D warnings`, and then +Whitaker's dylint suite over the same target and feature selection. +`--all-targets` pulls in the library's `cfg(test)` module and the integration +test targets, so `windows_reparse.rs`, `windows_reparse_tests.rs`, and the +junction fixture in `file_type_tests.rs` are all compiled on Windows itself, +under `-D warnings`. That job is green on this head. This is the widest +compile-and-lint evidence in the record, and it comes from the platform's own +toolchain rather than an approximation of it. + +**A local probe crate covers the development loop.** The main crate cannot be +cross-compiled on this host — `ring` needs MSVC's `lib.exe` — so Windows-gated +edits were iterated against a throwaway crate mirroring the module tree. Two +tools are needed there and neither subsumes the other: `cargo dylint` runs +`cargo check`, so it applies the Whitaker lints and no clippy lint, while +`cargo clippy` applies no Whitaker lint. Each was shown to be live by injecting +a defect it should catch, confirming a non-zero exit, and reverting. This is +what made the intermediate commits CI-worthwhile; it is a convenience, not the +guarantee. + +**What neither route shows is the tests running.** Compilation under +`-D warnings` is a strong statement about the code and a weak one about its +behaviour: the first route compiles the junction tests without executing them, +and on this head the lane that would execute them stops 1800 tests short. Until issue 743 is fixed and this branch rebuilt, the runtime behaviour of the policy is argued from the handle semantics in "Why the race is closed by From 306f1d266206b0f6bd24ae070a10dfc3713ce7b9 Mon Sep 17 00:00:00 2001 From: leynos Date: Sat, 19 Sep 2026 22:38:21 +0200 Subject: [PATCH 17/21] style(adr-032): let mdtablefix choose the wrap point My previous commit hand-wrapped a paragraph in this file and `check-fmt` failed in `build-test`: `docs/adr-032-...md +2 -2`, `1 file would be reformatted`, `make: *** [Makefile:314: check-fmt] Error 1`. The lines were within 80 columns; the break position was wrong. That is the trap recorded in the formatter memory, and I walked into it a second time in this same file. Fixed by running the tool rather than the ruler: mdtablefix --in-place --wrap --renumber --breaks --ellipsis --fences \ docs/adr-032-windows-reparse-point-same-handle-open.md only the break between "-- all-features" and "Whitaker's" moved. Verified with the invocation the gate itself uses, `mdtablefix --check --git --include-untracked --wrap --renumber --breaks --ellipsis --fences` -> `142 files left unchanged`, exit 0. Markdown-only; no code or behaviour touched. Co-Authored-By: Claude Code --- docs/adr-032-windows-reparse-point-same-handle-open.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/adr-032-windows-reparse-point-same-handle-open.md b/docs/adr-032-windows-reparse-point-same-handle-open.md index 317110295..ce909ba67 100644 --- a/docs/adr-032-windows-reparse-point-same-handle-open.md +++ b/docs/adr-032-windows-reparse-point-same-handle-open.md @@ -256,8 +256,8 @@ they share is stated at the end. **Native Windows CI compiles and lints every Windows-gated line, tests included.** `Windows / lint-windows` runs `make lint-clippy`, which expands to -`cargo clippy --workspace --all-targets --all-features -- -D warnings`, and then -Whitaker's dylint suite over the same target and feature selection. +`cargo clippy --workspace --all-targets --all-features -- -D warnings`, and +then Whitaker's dylint suite over the same target and feature selection. `--all-targets` pulls in the library's `cfg(test)` module and the integration test targets, so `windows_reparse.rs`, `windows_reparse_tests.rs`, and the junction fixture in `file_type_tests.rs` are all compiled on Windows itself, From 241b5289f68eb860ab8aa990e4b3dcfdb9f41391 Mon Sep 17 00:00:00 2001 From: leynos Date: Sat, 19 Sep 2026 23:00:21 +0200 Subject: [PATCH 18/21] docs(adr-032): pin the test-lane figure to its commit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `1078/2901 (1077 passed, 1 failed, 2 skipped)` is the one claim in this record most likely to age — it moves the moment issue 743 is fixed and a green Windows lane starts running these tests. Cite it against commit `2d8e5305` and name the test it died on, so a later reader can tell a stale number from a contradicted one. Re-measured on `2d8e5305` rather than carried over from the earlier run: same figures, and `windows_reparse`/`junction` still appear zero times in the job log. Verified with `mdtablefix --check --git --include-untracked --wrap --renumber --breaks --ellipsis --fences` -> `142 files left unchanged`, exit 0, and the file was formatted with the tool rather than by hand. Co-Authored-By: Claude Code --- docs/adr-032-windows-reparse-point-same-handle-open.md | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/docs/adr-032-windows-reparse-point-same-handle-open.md b/docs/adr-032-windows-reparse-point-same-handle-open.md index ce909ba67..9c0114cca 100644 --- a/docs/adr-032-windows-reparse-point-same-handle-open.md +++ b/docs/adr-032-windows-reparse-point-same-handle-open.md @@ -245,9 +245,12 @@ easy to read them as CI-verified when they are not. The `Windows / build-test-windows` job halts on an unrelated pre-existing failure — a network-fixture race tracked as issue 743 — before the nextest run -reaches `stdlib::path`. On the run examined for this record it ended at -1078/2901 tests, and the strings `windows_reparse` and `junction` appeared -**zero** times in the whole job log. So no case described in this section has +reaches `stdlib::path`. On commit `2d8e5305` the run ended at 1078/2901 tests +(1077 passed, 1 failed, 2 skipped), dying on +`stdlib::network::redirect::error_tests::protocol_failures_are_classified_from_a_live_response`, +and the strings `windows_reparse` and `junction` appeared **zero** times in +the whole job log. The figure is cited with its commit because it is the one +claim here most likely to age. So no case described in this section has executed in continuous integration, and a green Windows *test* lane would not yet be evidence about their behaviour. From 7b4599ba630238acd030c4d18d6f0fe2f24fc255 Mon Sep 17 00:00:00 2001 From: leynos Date: Sat, 19 Sep 2026 23:33:09 +0200 Subject: [PATCH 19/21] docs(stdlib guide): trim the reparse detail to a cross-link MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeRabbit finding 2 was the one finding in its batch with a valid basis: this guide sat at 397 lines on `main` and my earlier prose took it to 405. The other four findings cite a file-length rule that `AGENTS.md:31` scopes to "code file" and that only pylint enforces (`pyproject.toml:175`, `max-module-lines`), against documents already 2000-7300 lines on `main`; those are not this PR's to fix and are not defects. The cap claim is still worth honouring where this branch caused the crossing, so the added prose is trimmed from 12 inserted lines to 5. The dropped material enumerated reparse tags and restated the `follow_symlinks` containment behaviour, both of which `users-guide.md#configure-file-reading-limits` already carries in full — and this guide already cross-linked to that manual once, at line 387, so the pattern is established. Sibling guides updated alongside cover the security audit rather than the user-facing policy. Result: 400 lines, no longer over the threshold. Verified with `mdtablefix --check --git --include-untracked --wrap --renumber --breaks --ellipsis --fences` -> `142 files left unchanged`, exit 0; the file was formatted with the tool. Co-Authored-By: Claude Code --- docs/stdlib-yaml-and-jinja-guide.md | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/docs/stdlib-yaml-and-jinja-guide.md b/docs/stdlib-yaml-and-jinja-guide.md index c6545c315..a24062075 100644 --- a/docs/stdlib-yaml-and-jinja-guide.md +++ b/docs/stdlib-yaml-and-jinja-guide.md @@ -166,17 +166,11 @@ non-blocking, so a FIFO cannot wedge the render worker first. A symlink final component is refused on both platforms, but not with the same diagnostic: on Unix the default open declines to follow it, so the failure comes from the open itself and names the path together with the platform's symbolic-link detail, -while on Windows the open declines to traverse the reparse point and the -refusal then comes from the opened handle, which reuses the not-a-regular-file -diagnostic. That Windows refusal covers every reparse point, not only symlinks: -junctions, volume mount points, and other tags such as deduplication or cloud -placeholders are rejected alike, and `follow_symlinks=true` waives the refusal -for all of them. That opt-in does not extend to the capability that anchors the -read in the workspace: a link whose target is written as an absolute path is -still refused as an escape attempt even when the target lies inside the -workspace, so relative-target links are the supported case. A junction always -records an absolute target, so the opt-in cannot follow one. Two optional -keyword arguments narrow a call without touching the operator ceiling: +while on Windows the open declines to traverse the reparse point, so the +refusal comes from the opened handle and reuses the not-a-regular-file +diagnostic. That Windows refusal covers every reparse tag, not only symlinks, +and a relative-target link is the supported opt-in case. Two optional keyword +arguments narrow a call: - `max_bytes` lowers the budget for one call (a value above the configured budget is clamped to it). Example: @@ -184,8 +178,9 @@ keyword arguments narrow a call without touching the operator ceiling: - `follow_symlinks=true` permits the final component to be a symlink. Example: `{{ 'link/version.txt' | contents(follow_symlinks=true) }}`. -See the users' guide section on file reading limits for the defaults, the -symlink policy, and the trust model these limits assume. +See +[Configure file reading limits](users-guide.md#configure-file-reading-limits) +for the defaults, the full policy, and the trust model. MD5 and SHA-1 are available only in builds compiled with Cargo feature `legacy-digests`. Without that feature, `hash('md5')`, `hash('sha1')`, and their From 523290848f1ea1312f879f60b722a0195ddd1971 Mon Sep 17 00:00:00 2001 From: leynos Date: Sun, 20 Sep 2026 06:07:05 +0200 Subject: [PATCH 20/21] test(windows): assert the reparse policy at compile time MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Windows-gated tests only run on a Windows host, and the Windows test lane has been known to stop short of `stdlib::path` (ADR-032 records the figure), so the policy's two load-bearing branches had no evidence that ran on every push. A `const` assertion does not depend on a test lane reaching the module: rustc evaluates it whenever the module is compiled, and `Windows / lint-windows` compiles every Windows-gated line through `cargo clippy --all-targets`, which is green on every head of this branch. Both `pub(super)` functions are now pinned in `const` contexts: `open_flags(false)` must set `FILE_FLAG_OPEN_REPARSE_POINT`, `open_flags(true)` must not, either policy must keep `FILE_FLAG_BACKUP_SEMANTICS` so a directory can still be opened and rejected by the shared regular-file check, and `is_prohibited_reparse_point` must accept an attribute value carrying `FILE_ATTRIBUTE_REPARSE_POINT` and refuse an ordinary one. No production behaviour changes, no dependency is added, no function's visibility changes, and the runtime tests are untouched — they still cover the handle behaviour and the diagnostics that a compile-time assertion cannot see. The oracle was shown live rather than assumed. Against a probe crate mirroring the module tree, the true code compiles clean (exit 0); dropping the flag from the default branch aborts the build with `E0080: evaluation panicked: the default policy must not traverse a reparse point` and exit 101; and a broken attribute test fails independently on its own assertion, so neither branch is merely riding on its neighbour's assertion. Co-Authored-By: Claude Code --- src/stdlib/path/windows_reparse_tests.rs | 40 ++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/src/stdlib/path/windows_reparse_tests.rs b/src/stdlib/path/windows_reparse_tests.rs index e111c94f8..dbfb24acc 100644 --- a/src/stdlib/path/windows_reparse_tests.rs +++ b/src/stdlib/path/windows_reparse_tests.rs @@ -26,6 +26,46 @@ const LINK: &str = "junc"; #[cfg(windows)] const TARGET: &str = "junction_target"; +// Compile-time half of the policy contract. +// +// The runtime tests in this file only run on a Windows host, and the Windows +// test lane has been known to stop short of `stdlib::path` (see ADR-032). A +// `const` assertion has no such dependency: rustc evaluates it whenever this +// module is compiled, and `Windows / lint-windows` compiles it on every push +// through `cargo clippy --all-targets`. A regression in either policy branch +// therefore fails the Windows build, not just a test run. +// +// `open_flags` must ask the default open not to traverse a reparse point +// while either policy keeps a directory openable, and +// `is_prohibited_reparse_point` must test the attribute bit rather than the +// tag value. +const _: () = { + assert!( + open_flags(false) & FILE_FLAG_OPEN_REPARSE_POINT != 0, + "the default policy must not traverse a reparse point", + ); + assert!( + open_flags(true) & FILE_FLAG_OPEN_REPARSE_POINT == 0, + "the opt-in policy must let the open resolve the link", + ); + assert!( + open_flags(false) & FILE_FLAG_BACKUP_SEMANTICS != 0, + "the default policy must still permit opening a directory", + ); + assert!( + open_flags(true) & FILE_FLAG_BACKUP_SEMANTICS != 0, + "the opt-in policy must still permit opening a directory", + ); + assert!( + is_prohibited_reparse_point(FILE_ATTRIBUTE_REPARSE_POINT), + "an entry carrying FILE_ATTRIBUTE_REPARSE_POINT must be refused", + ); + assert!( + !is_prohibited_reparse_point(0), + "an entry without the attribute must not be refused", + ); +}; + /// The default policy must ask the open not to traverse a reparse point, /// and both policies must permit a directory open so the shared /// regular-file check can report the documented rejection. From 108fad1c41226f2e4a050771b5c15390bb79d455 Mon Sep 17 00:00:00 2001 From: leynos Date: Sun, 20 Sep 2026 06:55:13 +0200 Subject: [PATCH 21/21] docs: correct the opt-in wording and the ADR evidence record MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two documents described the `follow_symlinks` opt-in in symlink-only terms while on Windows it governs the final component's reparse point as a whole, which is the finding CodeRabbit raised on `docs/stdlib-yaml-and-jinja-guide.md` line 178 and `docs/security-network-command-audit.md` line 153. The premise is sound even though the wording of the finding inverted the mechanism: neither file mentions a pre-open metadata check, and `users-guide.md` already carries the requested content in full. Fixing the narrower defect the premise points at: - guide: `permits the final component to be a symlink` -> `waives that final-component refusal`; net-zero on line count (400). - audit: the same correction, and the sentence is reflowed rather than patch-edited, so the clause order stays readable (163 lines). - ADR-032: `## Verification` now records three layers — the compile-time assertion added in `5d2dab68`, the unit test, and the integration test — and `### How far this evidence actually extends` is rewritten, because the section it replaces concluded that none of these tests had ever run in CI. That was true when it was written and issue 743 is now fixed on `main`. The rewritten section also corrects a claim of mine that was wrong, not merely stale: I had argued that zero occurrences of the fixtures' skip line proved the junction was built, because nextest captures a passing test's stderr. It does not. A probe crate shows the marker is absent from nextest output under the default profile, and the `success-output = "immediate"` entries in `.config/nextest.toml` cover three unrelated test groups with no `--success-output` in CI. The argument now rests on the fixtures' single narrow quiet arm (`ErrorKind::NotFound` from spawning `cmd`, every other outcome failing) and on `require_real_junction` checking the reparse attribute before use, with the unmeasured premise — that the runner image can spawn `cmd.exe` — stated as such. The record of the error is left in place rather than tidied away. Docs only; no code, test, or build configuration changes. Applied with `mdtablefix --in-place` and verified as a no-op against the hand-written wording, so the result is formatter-canonical rather than hand-wrapped. Co-Authored-By: Claude Code --- ...-windows-reparse-point-same-handle-open.md | 112 ++++++++++++++---- docs/security-network-command-audit.md | 6 +- docs/stdlib-yaml-and-jinja-guide.md | 2 +- 3 files changed, 90 insertions(+), 30 deletions(-) diff --git a/docs/adr-032-windows-reparse-point-same-handle-open.md b/docs/adr-032-windows-reparse-point-same-handle-open.md index 9c0114cca..0736e825e 100644 --- a/docs/adr-032-windows-reparse-point-same-handle-open.md +++ b/docs/adr-032-windows-reparse-point-same-handle-open.md @@ -214,7 +214,19 @@ lint exemption to permit it. ## Verification -Two tests carry the guarantee, one per layer. +The guarantee is carried at three layers: a compile-time assertion, a unit +test, and an integration test. + +The compile-time layer is a `const _: () = { ... }` block in +`windows_reparse_tests.rs`. Runtime tests in that file execute only on a +Windows host, so a regression could reach a merge on the strength of a green +Linux run; a `const` assertion has no such dependency, because rustc evaluates +it whenever the module is compiled, and `Windows / lint-windows` compiles it on +every push through `cargo clippy --all-targets`. It pins both branches of +`open_flags` and both outcomes of `is_prohibited_reparse_point`, so a change to +either policy fails the Windows build rather than only a test run. (Before +issue 743 was fixed this layer mattered most, because the test lane did not +reach the module at all; it remains the layer that fails fastest.) The integration test asserts that a junction fixture — created with `mklink /J`, which needs no privilege — is rejected by all four filters under @@ -238,24 +250,70 @@ The existing symlink test continues to cover the file-symlink reparse case, and the `follow_symlinks` opt-in test covers the retained follow path with a relative-target symlink. +The compile-time assertions duplicate four decisions the unit test also makes, +which is deliberate rather than redundancy: the unit test decides them on a +Windows host, and the assertions decide them on the way to a Windows build. + ### How far this evidence actually extends -Stated plainly, because the two tests above are Windows-only and it would be -easy to read them as CI-verified when they are not. +Stated plainly, because the tests above are Windows-only, and because this +section previously recorded the opposite conclusion. -The `Windows / build-test-windows` job halts on an unrelated pre-existing -failure — a network-fixture race tracked as issue 743 — before the nextest run -reaches `stdlib::path`. On commit `2d8e5305` the run ended at 1078/2901 tests -(1077 passed, 1 failed, 2 skipped), dying on +`Windows / build-test-windows` was red repository-wide until `061182b1` landed +on `main` with the write-side shutdown fix for the network-fixture race tracked +as issue 743. Before that, and on every head of this branch, the lane halted +inside `stdlib::network` before the nextest run reached `stdlib::path`: on +commit `2d8e5305` the run ended at 1078/2901 tests (1077 passed, 1 failed, 2 +skipped), dying on `stdlib::network::redirect::error_tests::protocol_failures_are_classified_from_a_live_response`, -and the strings `windows_reparse` and `junction` appeared **zero** times in -the whole job log. The figure is cited with its commit because it is the one -claim here most likely to age. So no case described in this section has -executed in continuous integration, and a green Windows *test* lane would not -yet be evidence about their behaviour. - -What *is* verified on this change, and by what. Two routes, and the boundary -they share is stated at the end. +with the strings `windows_reparse` and `junction` appearing **zero** times in +the whole job log. So no case described in this section had ever executed in +continuous integration. + +**That is no longer true, and the tests now run.** This branch was rebased onto +`061182b1`, and on head `5d2dab68` the lane completes: +`Summary [ 341.726s] 2906 tests run: 2906 passed (1 slow), 2 skipped`. That +total is 2898 plus this branch's 8 — four unit tests and four +`reading_filters_reject_a_junction` cases. +`the_default_handle_is_the_junction_not_its_target` passes, as do all four +junction cases, and `windows_reparse` appears four times where it previously +appeared zero. The figure is cited with its commit because it is the claim here +most likely to age. + +A Windows-only test can pass without testing anything, by skipping its own +fixture, and this repository's skip convention returns `Ok(())` — so a skipped +fixture is recorded as a **pass**, not as a skip. Neither the skip total nor +the pass total can therefore distinguish "asserted against a junction" from +"quietly did nothing". Two things can. + +First, the fixtures have exactly one quiet arm, and it is narrow. Both +`junction_fixture` variants return `None` only on `ErrorKind::NotFound` from +`Command::new("cmd")`. Every other outcome is a failure: a `mklink` that exits +non-zero trips an `ensure!` quoting its stderr, and a spawn that fails for any +other reason propagates. So on a host where `cmd.exe` can be spawned, the only +ways to finish are "the junction was created" or "the test failed" — there is +no third way to pass. `cmd.exe` ships with the `windows-latest` image, which is +what makes that arm unreachable here. + +Second, a junction that was created is checked before it is used. +`require_real_junction` reads the entry's attributes *without following the +link* and fails unless `FILE_ATTRIBUTE_REPARSE_POINT` is set, precisely so that +a plain directory cannot stand in for a reparse point. A fixture that succeeded +but produced an ordinary directory fails the test rather than silently +inverting the assertions. + +The residual uncertainty is that the first step reasons about the runner image +rather than measuring it: the log does not record that `cmd` was spawnable, +because nextest hides the captured output of passing tests and the CI lane +passes no `--success-output`. (The `success-output = "immediate"` entries in +`.config/nextest.toml` cover three unrelated test groups.) So the absence of +the fixtures' skip lines proves nothing on its own, and is not relied on here. +What the run count does establish is that the cases ran at all: `main` reports +2898 tests and this head 2906, the difference being exactly the eight new +cases, all of which appear as `PASS`. + +What *is* verified on this change, and by what. Native CI and a local probe, +and the boundary between them is stated at the end. **Native Windows CI compiles and lints every Windows-gated line, tests included.** `Windows / lint-windows` runs `make lint-clippy`, which expands to @@ -264,9 +322,9 @@ then Whitaker's dylint suite over the same target and feature selection. `--all-targets` pulls in the library's `cfg(test)` module and the integration test targets, so `windows_reparse.rs`, `windows_reparse_tests.rs`, and the junction fixture in `file_type_tests.rs` are all compiled on Windows itself, -under `-D warnings`. That job is green on this head. This is the widest -compile-and-lint evidence in the record, and it comes from the platform's own -toolchain rather than an approximation of it. +under `-D warnings`. That job is green on this head. It is the only route that +compiles the Windows-gated lines with the platform's own toolchain rather than +an approximation of it. **A local probe crate covers the development loop.** The main crate cannot be cross-compiled on this host — `ring` needs MSVC's `lib.exe` — so Windows-gated @@ -278,12 +336,14 @@ a defect it should catch, confirming a non-zero exit, and reverting. This is what made the intermediate commits CI-worthwhile; it is a convenience, not the guarantee. -**What neither route shows is the tests running.** Compilation under +**What the Windows lane shows, and what it does not.** Compilation under `-D warnings` is a strong statement about the code and a weak one about its -behaviour: the first route compiles the junction tests without executing them, -and on this head the lane that would execute them stops 1800 tests short. - -Until issue 743 is fixed and this branch rebuilt, the runtime behaviour of the -policy is argued from the handle semantics in "Why the race is closed by -construction" plus the Linux-side mechanism evidence above, not demonstrated on -the platform it governs. That is the honest limit of this record. +behaviour, so the two lanes are described separately above rather than as one +result: the lint lane compiles the junction tests, and the test lane executes +them. What no test covers is the adversarial case the race analysis turns on. A +test cannot force a rename to land between two filesystem calls, because the +change removed the second call: the policy decision is read from the handle the +read uses, and this section is written as a construction argument for exactly +that reason. The runtime behaviour is now demonstrated on the platform it +governs; the *absence* of a window between the decision and the read remains an +argument from handle semantics rather than something a test could schedule. diff --git a/docs/security-network-command-audit.md b/docs/security-network-command-audit.md index 2729d2c30..046a0e9a6 100644 --- a/docs/security-network-command-audit.md +++ b/docs/security-network-command-audit.md @@ -150,9 +150,9 @@ introduces, and concrete remediation tasks that would harden the helpers. `StdlibConfig::with_file_max_read_bytes` (default 8 MiB). `linecount` counts terminators incrementally instead of materializing the file. Per-call `max_bytes` may narrow the ceiling and a named - `follow_symlinks=true` opt-in permits link following; budget rejections - name the path and the applicable limit, file-type rejections name only the - path, and neither discloses file contents. + `follow_symlinks=true` opt-in waives that final-component refusal; budget + rejections name the path and the applicable limit, and file-type + rejections name only the path; neither discloses file contents. ## Next steps diff --git a/docs/stdlib-yaml-and-jinja-guide.md b/docs/stdlib-yaml-and-jinja-guide.md index a24062075..d131a5f69 100644 --- a/docs/stdlib-yaml-and-jinja-guide.md +++ b/docs/stdlib-yaml-and-jinja-guide.md @@ -175,7 +175,7 @@ arguments narrow a call: - `max_bytes` lowers the budget for one call (a value above the configured budget is clamped to it). Example: `{{ 'fixtures/big.bin' | contents(max_bytes=1024) }}`. -- `follow_symlinks=true` permits the final component to be a symlink. Example: +- `follow_symlinks=true` waives that final-component refusal. Example: `{{ 'link/version.txt' | contents(follow_symlinks=true) }}`. See