Validate the Windows final component through a same-handle reparse-point open (#703)

[leynos]

Summary

Closes #703.

open_file_checked decided the Windows file-type and symlink policy from a
pre-open symlink_metadata lookup and then opened the path as a separate step.
Those two operations are not atomic: a final component that is an ordinary 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. Unix never had this window, because O_NOFOLLOW is applied to
the open itself.

The Windows path now reaches the same guarantee from inside the open. The final
component is opened with FILE_FLAG_OPEN_REPARSE_POINT, so the open does not
traverse the reparse point and the handle that comes back is the reparse
point; the policy decision is then read from that handle's own attributes, and
the caller reads bytes through the same handle. A concurrent rename changes
which entry the path names, but cannot change what an already-open handle
refers to, so the decision and the read cannot diverge.

Two details worth review:

• The policy rejects every reparse tag, not just the ones std calls
  symlinks. FileType::is_symlink tests the tag value: it is true only for
  name-surrogate tags (bit 29 set) — symlinks, junctions, volume mount points —
  and false for every other tag, such as a deduplication or cloud placeholder,
  for which std also reports is_file() == true. Testing
  FILE_ATTRIBUTE_REPARSE_POINT instead asks "is this a reparse point at all",
  which needs no enumeration of tags and refuses ones a future Windows release
  may mint.
• FILE_FLAG_BACKUP_SEMANTICS is unconditional, so a directory can be
  opened and then rejected by the existing shared regular-file check with the
  documented diagnostic, rather than the open failing with a different error
  than Unix reports.

reject_windows_symlink is deleted; nothing replaces its pre-open lookup.

Why no unsafe, no new dependency, no lint relaxation

The original plan anticipated raw Win32 FFI (CreateFileW +
GetFileInformationByHandleEx), a windows-sys dependency, an unsafe_code
downgrade, and a dylint.toml exclusion. None proved necessary: cap_std's
OpenOptionsExt::custom_flags is OR-ed straight into dwFlagsAndAttributes,
and MetadataExt::file_attributes() is populated from
BY_HANDLE_FILE_INFORMATION read from the open handle. Both are safe, public,
and already reachable through the cap_std::fs_utf8 re-exports the module
uses. This is also the same trait family the Unix arm already uses to set
O_NOFOLLOW, so the two platforms reach their guarantees through parallel
mechanisms. CreateFileW additionally has no directory-handle-relative form,
so an FFI route would have discarded the capability sandbox for an ambient
absolute path. Recorded in ADR-032.

Testing

Four layers carry the guarantee: a compile-time assertion, a unit test, an
integration test, and the opt-in test.

Compile time, on every Windows build. A const _: () = { ... } block in
windows_reparse_tests.rs pins both branches of open_flags and both outcomes
of is_prohibited_reparse_point. Runtime tests in that file execute only on a
Windows host, so a regression could have reached a merge on the strength of a
green Linux run; a const assertion does not, because rustc evaluates it
whenever the module is compiled and Windows / lint-windows compiles it on
every push through cargo clippy --all-targets. It fails the Windows build in
seconds rather than a test run in minutes. Added in response to the Codex
review's "Compile-Time / UI" finding.

Integration, all four filters. A Windows-only test asserts that a junction
fixture — created with mklink /J, which needs no privilege — is rejected by
contents, linecount, hash, and digest under the default policy. The
fixture follows the repository's "create the requested file type or skip
because that file type is unavailable" rule: require_real_junction asserts
the entry really carries FILE_ATTRIBUTE_REPARSE_POINT before rendering, so
the test cannot pass against a plain directory. Because the refusal happens on
the opened handle, it surfaces as the regular-file diagnostic; a traversal
would have rendered the target directory's entries instead of erroring at all.

Unit, on the handle. The integration test alone cannot detect a dropped
FILE_FLAG_OPEN_REPARSE_POINT, because std reports a junction as a symlink
and the shared regular-file check refuses the resolved directory anyway. A
handle-level test therefore asserts the property where it actually shows: the
default policy's open of a junction returns a handle carrying the reparse bit,
and the ordinary target directory is asserted not to carry one, so the
refusal is shown to be about the link rather than about directories generally.
That is the only test that fails if the flag stops being passed.

Opt-in. follow_symlinks_opt_in_reads_the_link_target covers the retained
follow path end to end using a relative-target file symlink. A junction
cannot serve here: mklink /J records an absolute target, and cap_std's
resolver refuses an absolute link destination outright (escape_attempt(),
reported as PermissionDenied) — under either policy. The handle-level test
therefore takes the junction through the ambient authority, which is the only
way to reach the reparse point itself. This is recorded in ADR-032 under "Known
risks and limitations".

Note the mechanism precisely, because an earlier revision of this body and of
the docs stated it wrongly. The resolver does not compare the resolved path
against the capability root; it refuses a link destination containing a prefix
or root component, and so cannot distinguish an absolute target that stays
inside from one that does not. Relativity of the link target, not containment
of the resolved path, is what is tested. Measured on Linux rather than
inferred: two symlinks pointing at the same file inside one capability, one
written relatively and one absolutely, under follow_symlinks=true — 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.

Windows verification: what CI already does, and what the probe crate is for

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, then
Whitaker's dylint suite over the same target and feature selection.
--all-targets compiles 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 built on Windows itself under
-D warnings. That job is green on this head.

For most of this branch's life that was the widest evidence available, because
the test lane never reached the module. It is no longer the widest: the test
lane now executes the cases as well, lower on this page. It remains the half
that compiles every Windows-gated line, and the two are described separately
because compiling a test under -D warnings is not running it.

The local probe crate below is therefore the development loop, not the
guarantee. It exists because the main crate cannot be cross-compiled on this
host (ring needs MSVC lib.exe), so Windows-gated edits were iterated
against it between CI runs. Its value is that it returns a Windows lint verdict
in seconds; passing --target x86_64-pc-windows-msvc after dylint's --
separator reaches the same lints the CI job applies:

cargo dylint --all --no-deps -- -p probe-capst --target x86_64-pc-windows-msvc --all-targets --all-features
cargo clippy --target x86_64-pc-windows-msvc --lib --tests --all-features -- -D warnings

Both are clean, and both are required, because each is blind to what the
other checks. cargo dylint shells out to cargo check, so it applies the
Whitaker lints and no clippy lint at all; cargo clippy does the reverse.
Running only the first leaves every clippy lint on Windows-only code
unvalidated. Neither direction is inferred — each was falsified by injecting a
defect into the mirrored test module and confirming a non-zero exit:

Injected defect	cargo dylint	cargo clippy
Some(1u32).unwrap()	passed, exit 0	failed, unwrap_used
std::fs::metadata(".")	failed, no_std_fs_operations	passed

Both tools were re-run clean on this head after the defects were reverted. The
first row also corrects this body's earlier claim that clippy is the weaker
tool: on this codebase the relationship is genuinely complementary, and clippy
alone is what let two Whitaker findings reach CI earlier on this branch
(module_must_have_inner_docs and five no_expect_outside_tests), both now
fixed.

The Windows test lane executes this PR's junction tests, and they pass.
This section said the opposite until the rebase, because
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 #743. Before that fix the
lane halted inside stdlib::network and never reached stdlib::path: on
2d8e5305 and on 34553e03 alike the run ended at 1078/2901 tests and the
string windows_reparse appeared zero times in the job log. This branch is
now rebased onto 061182b1, and on head 5d2dab68 the lane completes:

Summary [ 341.726s] 2906 tests run: 2906 passed (1 slow), 2 skipped

2898 of those are main's; the extra 8 are this branch's. The new cases:

PASS netsuke-build stdlib::path::windows_reparse::tests::constants_match_the_documented_abi_values
PASS netsuke-build stdlib::path::windows_reparse::tests::default_policy_opens_without_traversing_and_permits_directories
PASS netsuke-build stdlib::path::windows_reparse::tests::policy_refuses_surrogate_and_non_surrogate_tags_alike
PASS netsuke-build stdlib::path::windows_reparse::tests::the_default_handle_is_the_junction_not_its_target
PASS read_policy_filters::file_type_tests::reading_filters_reject_a_junction::case_1_contents
PASS read_policy_filters::file_type_tests::reading_filters_reject_a_junction::case_2_linecount
PASS read_policy_filters::file_type_tests::reading_filters_reject_a_junction::case_3_hash
PASS read_policy_filters::file_type_tests::reading_filters_reject_a_junction::case_4_digest

The result easiest to fake is the one worth checking hardest: these tests skip
their own fixture when the host has no cmd.exe to reach mklink through, so a
green run does not by itself prove a junction was built. It is worth being
exact about this, because my first draft of this section claimed the log proved
it and it does not.

This repository's skip convention returns Ok(()), so a skipped fixture is
recorded as a pass, not a skip. Neither total can distinguish "asserted
against a junction" from "quietly did nothing". Two things can.

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"); a mklink that exits non-zero trips an ensure! quoting
its stderr, and any other spawn error propagates. On a host where cmd.exe can
be spawned, the only ways to finish are "the junction was created" or "the test
failed". cmd.exe ships with windows-latest, so that arm is not reached.

A created junction 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 a plain directory cannot
stand in for a reparse point.

The residual uncertainty is that the first point reasons about the runner image
rather than measuring it: nextest hides the captured output of passing tests by
default — the success-output = "immediate" entries in .config/nextest.toml
cover three unrelated test groups, and the CI lane passes no --success-output
— so the log never records that cmd was spawnable. The absence of the skip
lines proves nothing on its own and is not relied on. What the run count does
establish is that the cases ran at all: main reports 2898 tests, this head
2906, and all eight new cases appear as PASS.

Windows / lint-windows is green on the same head; it is the compile-time half,
and build-test-windows is now the runtime half.

What the Windows lane still cannot show

A test cannot force a rename to land between two filesystem calls, because the
change removed the second call. The decision is read from the handle the read
uses, so the absence of a window is an argument from handle semantics, not
something a test can schedule. The runtime behaviour is now demonstrated on the
platform it governs; the construction argument is what covers the race itself.

Pre-existing Windows failure that the rebase carries the fix for

Recorded because this branch's earlier heads carried it, not because it is live.
Windows / build-test-windows failed for every PR and for main itself,
from a race in a test introduced by
8e09a3a4 Bump ureq from 2.12.1 to 3.4.0 (#438):

stdlib::network::redirect::error_tests::protocol_failures_are_classified_from_a_live_response
Error: a malformed status line should surface as a protocol error,
       got io: An established connection was aborted by the software in your host machine. (os error 10053)

malformed_status_line_failure wrote a malformed status line from a server
thread and joined it, but did not drain the request or shut the write side down
first, so the client's connection was reset before the malformed response was
read. Fixed on main by 061182b1 Complete local HTTP fixture responses with a write-side shutdown (#743) (#749), which this branch is rebased onto.

Checklist

• Windows default path rejects symlinks, mount points, and other prohibited
  reparse points without a separate pre-open metadata check
• Unix O_NOFOLLOW path unchanged; open_file_checked remains the single
  shared entry point for all four filters
• Regression tests accompany the change, with the special-file fixture
  creating the requested file type rather than substituting a regular file
• unsafe_code stays at forbid; no new dependency; no lint relaxation
• Local commit gates pass on head 5d2dab68: check-fmt (142 files
  unchanged), lint, typecheck, markdownlint (143 files, 0 errors),
  nixie (all diagrams validated), doc-coverage (98.79% vs an 80%
  threshold), doctest (123 passed, 0 failed), and — for the first time on
  this branch — make test end to end.
• make test — green, 3225 tests run: 3225 passed, 5 skipped on
  head 5d2dab68, a full run with no cancellation. This is the first head
  of the branch on which the gate passes whole: earlier heads failed
  locale_stub_ui_tests::harness_compiles_under_a_split_build_dir against
  nextest's 300s budget. That test passed here in 57.9s. main did not
  touch it (none of the six incoming commits names it), so the difference is
  load: the earlier 449s standalone / 333s near-idle measurements were this
  host's load sensitivity, not a defect. Recorded because the earlier heads'
  checklist entries said the opposite, and a reader comparing heads deserves
  to know which way it moved and why.
• Windows / lint-windows passes on CI, green on head 5d2dab68 as it was
  on 34553e03, 87c41ee9, 4eb1f08e, 2d8e5305, and 0b233a16. It is
  not only a lint result: it runs cargo clippy --workspace --all-targets --all-features -- -D warnings, and --all-targets compiles the
  library's #[cfg(test)] children, so the Windows-gated module, its
  tests, and the junction fixture are all built natively on Windows under
  -D warnings.
• Windows test coverage for the new cases — achieved on head
  5d2dab68. Windows / build-test-windows is green and the eight new
  cases execute there, including the four reading_filters_reject_a_junction
  cases and the_default_handle_is_the_junction_not_its_target. This was
  the last open item on the branch; it closed when the rebase onto
  061182b1 brought in the Windows: ureq 3.4.0 classifies a malformed status line as a connection abort, failing redirect error_tests and halting the suite at 1080/2891 #743 fix. See "The Windows test lane executes
  this PR's junction tests, and they pass".

Defects found after the PR opened

Recorded so the history is readable rather than tidy. All three were caught by
CI on this branch rather than by local gates, and the third was found while
reading a job log for an unrelated reason.

1. A wrong mechanism in the docs. Three review findings said
   follow_symlinks=true does not override capability containment. The premise
   was right and the wording I had shipped was wrong in a specific way: it said
   a link is refused when its destination leaves the capability, when in fact
   the resolver refuses an absolute link destination outright and never
   compares the resolved path to the capability root. Corrected in all four
   places that stated it, with the superseded wording quoted in ADR-032 so a
   reader who met the earlier claim can see which was wrong and why.
2. Non-canonical Markdown. A hand-wrapped paragraph in users-guide.md was
   under 80 columns but broke at the wrong position, so make check-fmt failed
   in both build-test and Windows / lint-windows. Fixed by running
   mdtablefix over the file rather than wrapping by hand — the tool's answer
   is the only one the gate accepts. Only the wrap point moved.
3. The same Markdown defect again, in the same file, two commits later, and
   the most useful entry here. Reviewing the CI job logs for an unrelated
   reason, I found this PR had understated its own Windows verification: the
   record pointed readers at a throwaway /tmp probe crate as the main route,
   when Windows / lint-windows already runs cargo clippy --workspace --all-targets --all-features -- -D warnings, and --all-targets compiles
   the library's #[cfg(test)] children — so the Windows-gated module, its
   tests, and the junction fixture are all built on Windows natively under
   -D warnings. Correcting that in ADR-032 and the PR body was worth doing.
   The correction itself then failed check-fmt at the identical
   make: *** [Makefile:314: check-fmt] Error 1, because I hand-wrote the new
   prose. Nothing interesting about that failure; the point is that knowing the
   rule did not prevent it, twice.

None of the three changed behaviour, and all three are fixed on the current
head. The third narrowed a claim as well as widening one: compiling a test
under -D warnings does not run it, so this PR still claims no Windows CI
execution of the new tests.

Review findings, and what happened to each

The final coderabbit review --agent pass on this PR returned five findings and
posted no PR comments, so they are recorded here rather than left invisible.
All five are the same class — "this document exceeds the repository's 400-line
limit, decompose it" — and four have a premise the repository does not support.

The cap is a code-file rule. AGENTS.md:31 reads "No single code file
may be longer than 400 lines". Its only enforcement is
pyproject.toml:175, max-module-lines = 400 under [tool.pylint.main] —
pylint is a Python linter and never reads Markdown. No Markdown gate imposes a
line cap (.markdownlint-cli2.jsonc sets only MD004/MD010/MD013/MD029, and no
Makefile target counts lines in a .md).

Four findings point at documents this PR did not push over any limit:

File	base	reviewed head	this PR's net
developers-guide.md	7299	7315	+16
netsuke-design.md	4018	4021	+3
users-guide.md	2010	2010	0

The only file here this PR lengthened is developers-guide.md, by 16 lines,
in a 7300-line document. All three were far over 400 long before this branch
existed, and none of the three is shorter than its base. Decomposing a
7000-line guide is a real piece of work, but it is not this PR's change and
doing it here would bury a filesystem hardening diff under thousands of moved
lines. Not actioned, deliberately.

One finding was right, and is fixed. stdlib-yaml-and-jinja-guide.md sat at
397 lines on main; my prose took it to 405. That is a threshold this
branch actually crossed, so the added text was trimmed back: the policy
paragraph went from 13 text lines on main to 21, then to 15 after the trim,
and the file landed at 400. The dropped material enumerated reparse tags and
restated the follow_symlinks containment behaviour that
users-guide.md#configure-file-reading-limits already documents in full. The
guide already pointed readers at the same manual once on main, at line 379
(#configure-network-access, now line 382), so citing the manual from this
paragraph follows the file's existing idiom; the new #configure-file-reading-limits
anchor is what this branch adds. The guide is now at 400.

The distinction matters for reviewability rather than for line counts: a rule
whose scope is "code file" does not become a defect when it is aimed at prose
the PR did not lengthen.

Two notes on the review process itself. The findings visible via
coderabbit review findings included three symlink-related items restating "an
absolute target that leaves the capability" — the exact wrong mechanism this
branch corrects. Those are stale cache from an earlier session (timestamped
before either review run, carrying no git.json); both runs made against this
branch persisted zero comments, and the wording they quote is no longer in the
tree. They are not output about this head and were not acted on.

The one actionable review finding, and how it finally closed

The Codex finding above ("Update user docs for the reparse-point policy") was
the only one that survived scrutiny, and it was right about all three files.
Recording the reasoning here because my first response to it was wrong, and the
error is more instructive than the fix.

The finding was made against 4eee40e7, which is not an ancestor of the
current head. That commit belongs to an earlier epoch of this branch, forked
from a273fad3; the branch was later rebuilt on a different base. Reading a
finding against the current tree therefore answers a different question than
the one the reviewer asked, and I initially did exactly that. Against
4eee40e7, all three sub-claims hold:

users-guide.md  "reparse" appears 0 times — described symlink rejection only
guide:169       "while on Windows a check made before the open reuses the
                 not-a-regular-file diagnostic"
audit:141       "a pre-open `symlink_metadata` check on Windows"

The implementation had already moved to the same-handle design at that commit
(reject_windows_symlink is gone from fs_utils.rs), and the review's own
subject is a compile-time finding against that commit — so the docs were
stale relative to the code, exactly as the reviewer said. The fix landed later
the same day in e8ebc5f4 (docs: describe the Windows reparse-point policy in the user-facing guides), which added the all-tags paragraph to
users-guide.md and replaced the pre-open wording in the other two files. The
review was submitted at 16:21Z and that commit is timestamped 18:08Z, so the
ordering is consistent with the finding having prompted the fix.

One sub-claim was still narrower than it looked, and CodeRabbit's second pass
found the remainder: two sentences described the opt-in in symlink-only
terms, while on Windows it governs every reparse tag —

stdlib-yaml-and-jinja-guide.md:178  `follow_symlinks=true` permits the final component to be a symlink
security-network-command-audit.md:153  `follow_symlinks=true` opt-in permits link following

That is a genuine defect: a reader on a cloud-placeholder or deduplicated file
would conclude the opt-in does not apply to them. Both now say the opt-in
"waives that final-component refusal", which is the users-guide.md phrasing,
and each paragraph's antecedent defines the refusal precisely — every reparse
tag on Windows — a few lines above. The change is net-zero on line count in
both files, so the branch does not cross the 400-line threshold it just came
under.

This is the useful shape to remember, in the opposite direction to the one I
first wrote down: a finding can be correct and still look wrong, because the
tree in front of you has moved past it. Check the commit the finding was made
against before deciding its premise is inverted — especially after a rebase,
where the reviewed commit may not be in the lineage at all.

References

• Issue: #703
• Parent policy: #648 — bound
  stdlib file filters and define a safe symlink/file-type policy
• Pre-existing Windows failure:
  #743 —
  Windows / build-test-windows red repository-wide since the ureq 3.4.0 bump
• Decision record:
  docs/adr-032-windows-reparse-point-same-handle-open.md —
  renumbered twice. It began at 026, moved to 027 when main merged its own
  026, then to 032: #699 had
  minted adr-027 thirty-one minutes earlier, and a sweep of every remote
  branch showed 028-031 spent as well (029 twice). Document the command placeholder contract in the README (4.4.1) #699 keeps 027.
• Lody session:
  https://lody.ai/leynos/sessions/5a6c2be3-7231-444e-86e7-a74daf140eb8

Summary by Sourcery

Harden Windows file-reading policy by opening final components without traversing reparse points and validating the resulting handle before reading.

Bug Fixes:

• Close the Windows final-component check-then-open race by validating reparse-point status from the same handle used for reading.
• Reject all Windows reparse points under the default file-reading policy while preserving opt-in support for supported relative-target symlinks.

Enhancements:

• Preserve capability-based filesystem access and existing regular-file diagnostics by using safe cap_std APIs and unconditional directory-open support.
• Add Windows-specific unit and integration coverage for junction handling across all four file-reading filters.

Documentation:

• Document the Windows same-handle reparse-point policy, its capability-resolution limitations, and the architectural decision in ADR-032.

Tests:

• Add handle-level tests proving the default Windows open returns the junction itself rather than its target.
• Add Windows integration tests verifying contents, linecount, hash, and digest reject junctions.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Team

Run ID: e5be07af-a458-48b8-b6d4-21919cb4e3c3

📥 Commits

Reviewing files that changed from the base of the PR and between a273fad and 4eee40e.

📒 Files selected for processing (11)

• docs/adr-027-windows-reparse-point-same-handle-open.md
• docs/contents.md
• docs/developers-guide.md
• docs/netsuke-design.md
• src/stdlib/path/fs_utils.rs
• src/stdlib/path/mod.rs
• src/stdlib/path/windows_reparse.rs
• src/stdlib/path/windows_reparse_tests.rs
• tests/std_filter_tests/read_policy_filters/file_type_tests.rs
• tests/std_filter_tests/read_policy_filters/mod.rs
• tests/std_filter_tests/support.rs

🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

• leynos/monotony (auto-detected)
• leynos/whitaker (auto-detected)
• leynos/rstest-bdd (auto-detected)
• leynos/mdtablefix (auto-detected)
• leynos/typos-config-builder (auto-detected)
• leynos/ortho-config (auto-detected)
• leynos/lading (auto-detected)
• leynos/shared-actions (auto-detected)
• leynos/nixie (auto-detected)
• leynos/ansible (auto-detected)

Included review availability: 0 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 1 review per hour.

---

Summary

• Replace the Windows pre-open symlink_metadata check with same-handle validation.
• Open the final path component with FILE_FLAG_OPEN_REPARSE_POINT.
• Reject reparse points and non-regular files using metadata from the opened handle.
• Preserve the Unix O_NOFOLLOW path and the shared open_file_checked entry point.
• Add Windows coverage for symlinks, junctions, reparse tags, and all four read filters.
• Add ADR-027 and update the developer and design documentation.
• Skip junction tests when the required Windows fixture is unavailable.
• Record the existing Windows test-lane limitation caused by a pre-existing network test failure.

Related issue

• #703 — Validate the Windows final component through a same-handle reparse-point open.

Walkthrough

Changes

Windows reparse-point validation

Layer / File(s)	Summary
Handle-based reparse policy src/stdlib/path/windows_reparse.rs, src/stdlib/path/mod.rs	Apply Windows reparse-aware flags. Detect prohibited reparse points from opened-handle attributes.
File-opening integration src/stdlib/path/fs_utils.rs	Use same-handle validation and remove the separate symlink_metadata check.
Junction verification and documentation src/stdlib/path/windows_reparse_tests.rs, tests/std_filter_tests/..., docs/...	Test symlink flags, reparse tags, junctions, filter errors, and document the revised behaviour.

Sequence Diagram(s)

sequenceDiagram
  participant Filter
  participant open_file_checked
  participant WindowsHandle
  participant Reader
  Filter->>open_file_checked: request file open
  open_file_checked->>WindowsHandle: open with reparse-aware flags
  WindowsHandle-->>open_file_checked: return handle attributes
  open_file_checked->>WindowsHandle: reject prohibited reparse point
  open_file_checked->>Reader: provide validated handle

Loading

Suggested labels: Issue

Priority: ➖ Normal

Change: Bug fix · Severity of issue fixed: Medium

🚥 Pre-merge checks | ✅ 13 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
User-Facing Documentation	⚠️ Warning	Treat the user-facing documentation as incomplete. The pull request changes Windows contents, linecount, hash, and digest behaviour: the default policy now rejects every final-component `FILE_…	Update docs/users-guide.md in the file-reading section. State that, on Windows, the default policy rejects every final-component reparse point, including symlinks, junctions, volume mount points, and other reparse tags, and that only the …
Testing (Compile-Time / Ui)	⚠️ Warning	Require a compile-time test. The PR adds the Windows pub(super) const fn functions open_flags and is_prohibited_reparse_point in src/stdlib/path/windows_reparse.rs, which introduces compile-ti…	Add a Windows-gated trybuild compile-pass test, or a clear Rust compile-time equivalent, that evaluates open_flags and is_prohibited_reparse_point in const contexts and verifies both policy branches and representative attribute values…

✅ Passed checks (13 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Accept the implementation for issue #703. open_file_checked applies FILE_FLAG_OPEN_REPARSE_POINT on the Windows default path, reads attributes from the returned handle, rejects every reparse-point…
Out of Scope Changes check	✅ Passed	Keep the documentation, ADR, unit tests, integration tests, and fixture helpers. These changes document or verify the same-handle Windows reparse-point policy required by issue #703. No unrelated prod…
Docstring Coverage	✅ Passed	Docstring coverage is 95.45% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 22 functions across 7 files. (4 skipped: 4 …
Testing (Overall)	✅ Passed	Pass the testing check. The pull request adds substantive Windows coverage. the_default_handle_is_the_junction_not_its_target creates a real mklink /J junction, opens it with the default flags, ch…
Developer Documentation	✅ Passed	PASS. The changed Windows file-opening boundary is documented in docs/developers-guide.md, including the windows_reparse abstraction, Windows open flags, handle-based reparse validation, diagnosti…
Module-Level Documentation	✅ Passed	Pass the module-level documentation check. The new windows_reparse and windows_reparse_tests modules start with clear //! documentation that states their purpose, utility, and relationship to th…
Testing (Unit And Behavioural)	✅ Passed	Mark this check PASS. The unit tests cover flag selection, ABI constants, surrogate and non-surrogate reparse tags, ordinary-file attributes, and rejection from a real junction handle. The Windows beh…
Testing (Property / Proof)	✅ Passed	Keep the parameterized tests. The introduced pure invariants have small, auditable domains or a direct bitmask contract: open_flags has only the two follow_symlinks states, and `is_prohibited_repa…
Unit Architecture	✅ Passed	PASS. The change preserves clear boundaries. windows_reparse::open_flags and is_prohibited_reparse_point are pure queries. apply_open_flags makes its mutation explicit through &mut OpenOptions…
Domain Architecture	✅ Passed	Keep the change in the existing filesystem adapter boundary. The authoritative diff changes only src/stdlib/path, its Windows-specific filesystem helper, documentation, and integration tests. `src/s…
Observability	✅ Passed	PASS — the pull request introduces Windows file-policy behaviour, but it keeps the decision inside the existing file-read boundary. open_file_checked returns the existing InvalidOperation diagnost…
Title check	✅ Passed	The title accurately describes the main change and references issue #703, which the description identifies as the issue being closed.
Description check	✅ Passed	The description directly explains the Windows same-handle reparse-point fix, its tests, documentation, and issue context.

Full details: User-Facing Documentation

Explanation

Treat the user-facing documentation as incomplete. The pull request changes Windows contents, linecount, hash, and digest behaviour: the default policy now rejects every final-component FILE_ATTRIBUTE_REPARSE_POINT, including junctions, mount points, and non-name-surrogate tags. The review-scoped diff does not change docs/users-guide.md. That guide still documents only final symlinks and says that follow_symlinks=true accepts the link, while the new Windows-specific reparse-point behaviour is documented only in developer and design documents. The changed implementation and ADR explicitly establish this broader policy.

Resolution

Update docs/users-guide.md in the file-reading section. State that, on Windows, the default policy rejects every final-component reparse point, including symlinks, junctions, volume mount points, and other reparse tags, and that only the final component is covered. Explain that follow_symlinks=true opts into following the final link when the target remains within the capability and is a regular file. Add the stricter Windows rejection to the applicable n+1 migration guide if the release treats this behaviour as a breaking change.

Full details: Testing (Compile-Time / Ui)

Explanation

Require a compile-time test. The PR adds the Windows pub(super) const fn functions open_flags and is_prohibited_reparse_point in src/stdlib/path/windows_reparse.rs, which introduces compile-time-capable Rust behaviour. The new tests in src/stdlib/path/windows_reparse_tests.rs call both functions through ordinary runtime locals and loops. The changed files add no trybuild fixture or other compile-time equivalent. The junction and error checks provide useful runtime coverage, and no new structured output requires a snapshot because the assertions target the stable diagnostic meaning.

Resolution

Add a Windows-gated trybuild compile-pass test, or a clear Rust compile-time equivalent, that evaluates open_flags and is_prohibited_reparse_point in const contexts and verifies both policy branches and representative attribute values. Keep the existing runtime handle and integration tests. If a snapshot is added for the diagnostic, redact temporary paths and other variable data.

---

_{A guarded handle opens bright
Reparse paths stay out of sight
Junction tests mark the way
Safe reads follow rules each day
Windows checks the handle right}

_{Comment @coderabbitai help to get the list of available commands.}

[sourcery-ai]

Reviewer's Guide

Windows file reads now open the final component without traversing reparse points, validate attributes from that same handle, and read through it, eliminating the pre-open check race while preserving capability-based access, shared regular-file diagnostics, and the symlink-following opt-in. New Windows tests and ADR/documentation updates cover the behavior and rationale.

Sequence diagram for race-free Windows file validation

sequenceDiagram
    participant Filter
    participant OpenFileChecked
    participant ParentHandle
    participant FileHandle

    Filter->>OpenFileChecked: open_file_checked(path, limits)
    OpenFileChecked->>ParentHandle: open_with(entry, custom_flags)
    Note over ParentHandle: FILE_FLAG_OPEN_REPARSE_POINT
    Note over ParentHandle: FILE_FLAG_BACKUP_SEMANTICS
    ParentHandle-->>OpenFileChecked: FileHandle
    OpenFileChecked->>FileHandle: metadata()
    FileHandle-->>OpenFileChecked: file_attributes()
    alt FILE_ATTRIBUTE_REPARSE_POINT set
        OpenFileChecked-->>Filter: not_regular_file_error(path)
    else ordinary regular file
        Filter->>FileHandle: read bytes
        FileHandle-->>Filter: file contents
    end

Loading

Flow diagram for Windows reparse-point policy modes

flowchart TD
    A["open_file_checked"] --> B["apply_open_flags"]
    B --> C{"follow_symlinks?"}
    C -->|No| D["OPEN_REPARSE_POINT + BACKUP_SEMANTICS"]
    C -->|Yes| E["BACKUP_SEMANTICS"]
    D --> F["open_with"]
    E --> F
    F --> G["metadata from opened handle"]
    G --> H{"Default policy and reparse attribute?"}
    H -->|Yes| I["not_regular_file_error"]
    H -->|No| J["shared is_file check and read"]

Loading

File-Level Changes

Change	Details	Files
Replace the Windows check-then-open symlink validation with same-handle reparse-point enforcement.	Apply FILE_FLAG_OPEN_REPARSE_POINT during the default-policy open. Always apply FILE_FLAG_BACKUP_SEMANTICS so directories reach the shared regular-file rejection. Inspect FILE_ATTRIBUTE_REPARSE_POINT and regular-file status from the handle used for reading. Remove the pre-open symlink_metadata validation and isolate Windows flag/attribute logic in a dedicated module.	src/stdlib/path/fs_utils.rs src/stdlib/path/mod.rs src/stdlib/path/windows_reparse.rs
Add Windows regression coverage proving both policy behavior and handle identity.	Test junction rejection across contents, linecount, hash, and digest filters. Create junction fixtures with mklink /J and verify they are genuine reparse points. Verify default-policy handles retain the reparse attribute while opt-in handles resolve to the target. Cover flag values and rejection of both surrogate and non-surrogate reparse tags.	tests/std_filter_tests/read_policy_filters/file_type_tests.rs tests/std_filter_tests/read_policy_filters/mod.rs tests/std_filter_tests/support.rs src/stdlib/path/windows_reparse.rs
Document the Windows hardening decision and updated file-reading policy.	Record the chosen safe cap_std implementation and rejected raw-FFI alternative in ADR-026. Update developer and design documentation to describe same-handle validation and all-tag reparse rejection. Add ADR-026 to the documentation index.	docs/adr-026-windows-reparse-point-same-handle-open.md docs/contents.md docs/developers-guide.md docs/netsuke-design.md

Assessment against linked issues

Issue	Objective	Addressed	Explanation
#703	Validate the Windows final path component through the same open handle used for reading, using FILE_FLAG_OPEN_REPARSE_POINT and rejecting any prohibited reparse point.	✅
#703	Continue rejecting non-regular files based on metadata from the opened handle while preserving the Unix O_NOFOLLOW path and the shared open_file_checked entry point for all four filters.	✅
#703	Add regression coverage demonstrating the Windows default policy rejects real reparse-point file types, including junctions, without substituting a regular file or silently masking unavailable fixtures.	✅

Possibly linked issues

• Validate the Windows final component through a same-handle reparse-point open #703: The PR implements the issue’s required same-handle Windows reparse-point validation, flags, metadata checks, and regression tests.

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it. You can also reply to a
  review comment with @sourcery-ai issue to create an issue from it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time. You can also comment
  @sourcery-ai title on the pull request to (re-)generate the title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time exactly where you
  want it. You can also comment @sourcery-ai summary on the pull request to
  (re-)generate the summary at any time.
• Generate reviewer's guide: Comment @sourcery-ai guide on the pull
  request to (re-)generate the reviewer's guide at any time.
• Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
  pull request to resolve all Sourcery comments. Useful if you've already
  addressed all the comments and don't want to see them anymore.
• Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
  request to dismiss all existing Sourcery reviews. Especially useful if you
  want to start fresh with a new review - don't forget to comment
  @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[sourcery-ai]

Sorry @leynos, you've used your own review budget of 250,000 diff characters for the last 7 days.

You can request another review in 4 days and 19 hours by commenting @sourcery-ai review. Upgrade to get a review now.

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-09-19T16:21:53.919316Z	4eee40e	Draft marked ready

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4eee40e77c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[leynos]

CodeRabbit review — dispositions

Ran coderabbit review --agent --base main on 4eee40e7; 6 findings, all
minor, collapsing to two distinct issues.

Fixed — spelling (tests/std_filter_tests/support.rs:183). CodeRabbit
flagged recognises. Confirmed against the house rule: en-GB-oxendict requires
-ize, and the repository uses recognizes overwhelmingly (31 occurrences,
including four in docs/developers-guide.md). Line changed to recognizes in
6370e3b6. The four other -ise-looking words I added (exercised,
unexercised, raise, otherwise) are correct as spelled.

Declined — skip-on-missing-cmd.exe (support.rs:230 and
windows_reparse_tests.rs:260-266). CodeRabbit asked that the
Ok(None) branch produce a "real setup error or an explicit test-runner skip"
instead of reporting the case as passed. Declined: the current behaviour is a
faithful copy of the repository's own established convention, which
predates this branch. origin/main already ships, in the same file:

let Some(link) = fallible::file_symlink_fixture(&root)? else {
    skip_without_symlink_support(case);
    return Ok(());
};

with

fn skip_without_symlink_support(case: FilterCase) {
    eprintln!(
        "skipped: {} — this host cannot create a symlink fixture",
        case.name
    );
}

The parallel skip_without_junction_support added here is deliberately
identical in shape so the two skip paths read the same way. No runtime-skip
mechanism exists for plain #[test]/#[rstest] in this repository — the only
rstest_bdd::skip! call site is in tests/bdd/steps/fs.rs, where it is valid
only because it is a BDD step. Changing this would make the junction skip
inconsistent with the symlink skip directly beside it, and would require
inventing a mechanism the repo does not have. The tests do not pass silently
in any case: the skip path writes an explicit skipped: ... line to stderr,
which is asserted to be visible in captured output.

Also declined — the same finding restated three times against different line
ranges. Same reasoning as above.

[leynos]

Addressed in 40150c13. All three premises checked out and all three files are
updated:

• docs/users-guide.md — the reading-filters section now states that the
  Windows refusal covers every reparse point, not only symlinks: junctions,
  volume mount points, and other tags such as deduplication or cloud
  placeholders, "including tags Windows may add later". It also says
  follow_symlinks=true is the opt-in for those, not just for symlinks.
• docs/stdlib-yaml-and-jinja-guide.md — the diagnostic comparison no longer
  says "a check made before the open"; it now describes the open declining to
  traverse the reparse point and the refusal coming from the opened handle. The
  same non-symlink reparse coverage and the opt-in note were added.
• docs/security-network-command-audit.md — the remediation bullet replaces
  "a pre-open symlink_metadata check on Windows" with
  FILE_FLAG_OPEN_REPARSE_POINT, the handle's
  FILE_ATTRIBUTE_REPARSE_POINT refusal, and the fact that the judgement and
  the read share one handle.

One note on scope: the symlink_metadata mention at
docs/netsuke-design.md:1424 was checked and left alone. It describes the
dir/file/symlink test predicates, which inspect an operand's FileType
through Dir::symlink_metadata and do not follow links — a different feature
from the reading filters' open policy, and unaffected by this change.

Verified: make check-fmt, make markdownlint (which includes the typos
gate), and the documentation_examples_tests contract test (31/31) all pass.

[leynos]

ADR renumbered 027 → 032 (87c41ee9, docs-only).

A reviewer opening docs/contents.md will see the new entry jump from 026 to
032, so here is why.

The ADR was originally written as 026, renumbered to 027 when main merged its
own 026, and now moves to 032. The 027 step 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 adr-027. Renumbering
off main's ceiling 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 (not just origin/main) shows the range is gone:

Number	Held by
026	merged on main
027	#699 — keeps it, claimed first
028	issue-693-trim-the-split-build-dir-harness-test…
029	docs/hexagonal-hardening-and-checking and make-the-build-standard-the-default (two-way collision)
030, 031	docs/hexagonal-hardening-and-checking
032	free — taken here

The commit is a git mv plus its four references updated in lockstep: the ADR's
own H1, the docs/contents.md index entry, and the inbound links in
docs/developers-guide.md and docs/netsuke-design.md. A whole-tree grep for
adr-027-windows / ADR-027 now returns nothing, and the docs/contents.md
link target exists. The documentation style guide specifies a "sequence number"
without a contiguity requirement, and the tree already carries gaps and
duplicate numbers (003, 004, 014, 018) predating this change.

make check-fmt and make markdownlint both pass on 87c41ee9; the full gate
suite and CI are being re-run for the new head.

[buzzybee-df12]

@coderabbitai approve

[coderabbitai]

✅ Action performed

Comments resolved and changes approved.