diff --git a/docs/adr-032-windows-reparse-point-same-handle-open.md b/docs/adr-032-windows-reparse-point-same-handle-open.md new file mode 100644 index 000000000..0736e825e --- /dev/null +++ b/docs/adr-032-windows-reparse-point-same-handle-open.md @@ -0,0 +1,349 @@ +# Architectural decision record (ADR) 032: 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, 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. + +### 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 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. + +### 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. +- A junction cannot be *traversed* through a capability at all, under either + 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. + +## 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 + +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 +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 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. + +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 tests above are Windows-only, and because this +section previously recorded the opposite conclusion. + +`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`, +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 +`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. 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 +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 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, 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/contents.md b/docs/contents.md index 7bfc208f2..101ab4fc9 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-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 e2203ef17..d09f0ddcd 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -5911,18 +5911,34 @@ 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 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-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; `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. -All of them, like the invalid-UTF-8 diagnostic that `contents` and `linecount` -raise for undecodable input, are MiniJinja `InvalidOperation` errors. See +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 boundary. diff --git a/docs/netsuke-design.md b/docs/netsuke-design.md index dafbdbdda..6f9aa9933 100644 --- a/docs/netsuke-design.md +++ b/docs/netsuke-design.md @@ -1515,18 +1515,21 @@ 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 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-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, 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 - 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. + 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. - 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`, diff --git a/docs/security-network-command-audit.md b/docs/security-network-command-audit.md index 8ed6837c6..046a0e9a6 100644 --- a/docs/security-network-command-audit.md +++ b/docs/security-network-command-audit.md @@ -138,15 +138,21 @@ 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. 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 f27099916..d131a5f69 100644 --- a/docs/stdlib-yaml-and-jinja-guide.md +++ b/docs/stdlib-yaml-and-jinja-guide.md @@ -166,18 +166,21 @@ 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 -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: `{{ '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 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 diff --git a/docs/users-guide.md b/docs/users-guide.md index 2e45fdb19..ff0bea4f1 100644 --- a/docs/users-guide.md +++ b/docs/users-guide.md @@ -1838,8 +1838,21 @@ 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. + +`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/fs_utils.rs b/src/stdlib/path/fs_utils.rs index 88b3dadc3..774a04811 100644 --- a/src/stdlib/path/fs_utils.rs +++ b/src/stdlib/path/fs_utils.rs @@ -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..b641acecb --- /dev/null +++ b/src/stdlib/path/windows_reparse.rs @@ -0,0 +1,101 @@ +//! 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 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}; +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; + +/// The `dwFlagsAndAttributes` bits the policy passes to the open. +/// +/// `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; + } + 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. +/// +/// `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 is_prohibited_reparse_point(metadata.file_attributes()) { + return Err(not_regular_file_error(path)); + } + Ok(()) +} + +#[cfg(test)] +#[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..dbfb24acc --- /dev/null +++ b/src/stdlib/path/windows_reparse_tests.rs @@ -0,0 +1,350 @@ +//! 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"; + +// 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. +#[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`'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 +/// 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(()) +} 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..6e66117c2 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 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 + /// 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,