Optimize TRX reparse point confinement checks

[Amaury Levé (Evangelink)]

Summary

• replace the TRX confinement walk's Directory.Exists plus File.GetAttributes pair with one attribute read per component
• preserve missing-path behavior while treating I/O, access, and .NET Framework security failures as unsafe
• keep attachment-file reparse checks unchanged and cover one-call, missing, inaccessible, regular-file, and dangling-link semantics
• keep the new probe private; tests use the repository's established reflection pattern, so no internal API is added

Security and efficiency

Existing path components now require one filesystem metadata probe instead of two. Missing components still require one probe. This narrows, but does not eliminate, the existing TOCTOU window; the merge path continues to revalidate destination ancestors and copy without overwrite.

Only FileNotFoundException and DirectoryNotFoundException mean an entry is absent. IOException, UnauthorizedAccessException, and SecurityException fail closed. An unreadable merged In root is never deleted.

Validation

• Microsoft.Testing.Extensions unit tests: net8.0, net9.0, net462, net472
• repository pack
• focused merge-trx acceptance tests: 3 passed
• independent security and correctness reviews, including final reviews after removing the internal API test seam

Closes #10643

[Copilot]

Pull request overview

Optimizes TRX attachment confinement checks while preserving fail-closed security behavior.

Changes:

• Reduces filesystem metadata probes to one per path component.
• Handles missing and inaccessible entries distinctly.
• Adds focused tests for status and dangling-link behavior.

Show a summary per file

File	Description
TrxReportEngine.Merge.PathHelpers.cs	Adds single-probe reparse-point detection.
TrxReportEngine.Merge.Attachments.cs	Safely handles inaccessible merged roots.
InternalAPI.Unshipped.txt	Records the new internal helper.
TrxReportEngineMergeTests.cs	Tests detection and exception semantics.

Review details

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

• Files reviewed: 4/4 changed files
• Comments generated: 0
• Review effort level: Balanced

[Copilot]

Review details

• Files reviewed: 3/3 changed files
• Comments generated: 2
• Review effort level: Balanced

[github-actions]

🧵 Parallel-safety audit — PR #10648

Parallelization — assembly audited:

Test assembly	Scope	Workers	Analyzer coverage
Microsoft.Testing.Extensions.UnitTests	MethodLevel (via [assembly: Parallelize(Scope = ExecutionScope.MethodLevel, Workers = 0)] in Program.cs, unchanged by this PR)	CPU count (Workers = 0)	coverable once the parallel-safety analyzers ship (attribute-based opt-in)

Findings: A (global-state) 0 · B (paths) 0 · C (declaration) 0 · D (over-serialization) 0 — by severity: Critical 0 · High 0 · Warning 0 · Info 0.

This PR only touches production code in TrxReportEngine.Merge.Attachments.cs / TrxReportEngine.Merge.PathHelpers.cs (adding a getAttributes delegate seam and a null-status branch for unreadable reparse-point attributes) plus one new/changed test file, TrxReportEngineMergeTests.cs, adding several [TestMethod]s.

Reviewed every added test method for the category A–D taxonomy:

• All temp paths are constructed with Path.Combine(Path.GetTempPath(), $"trx-merge-{Guid.NewGuid():N}") — each test gets a unique directory, so there is no cross-test filesystem collision (category B). Deletion happens in a finally per test.
• No test sets environment variables, current directory, console state, culture, or any static/shared mutable field (category A).
• No [ResourceLock] or [DoNotParallelize] was added, removed, or changed anywhere in the diff (category C) — no near-miss or under/over-declaration to reconcile.
• No over-serialization introduced (category D).
• The reflection-based invocation helpers (GetReparsePointStatusMethod, etc.) are static readonly MethodInfo fields — these are read-only reflection handles resolved once, not mutable shared state, so they are not a hazard.
• Tests that inject a throwing/failing getAttributes delegate operate purely on local variables and unique paths; no shared resource is touched.

Nothing in this PR is parallel-unsafe. No action needed.

_{Advisory only — heuristic, non-blocking. Re-run with /parallel-audit. This audit answers "is it parallel-safe?"; for testability, smells, or flakiness see the detect-static-dependencies / test-smell-detection / test-anti-patterns analyses.}

🤖 Automated content by GitHub Copilot. Generated by the Parallel-safety audit on PR (on open / sync) workflow. · auto · 43.7 AIC · ⌖ 4.45 AIC · ⊞ 24.8K · [◷]( · ◷)

[Copilot]

Review details

• Files reviewed: 3/3 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[github-actions]

🧪 Expert test review — PR #10648

Grade	Test	Mutation	Notes	How to improve
A (90–100)	new TrxReportEngineMergeTests. GetReparsePointStatus_ ReadsAttributesOnceAndDetectsReparsePoint	2/2 killed	Asserts result and call count, killing both the true/false and once-only mutations.	—
A (90–100)	new TrxReportEngineMergeTests. GetReparsePointStatus_ WhenEntryIsARegularFile_ ReturnsFalse	1/1 killed	Focused, clear assertion on the non-reparse-point path.	—
A (90–100)	new TrxReportEngineMergeTests. GetReparsePointStatus_ WhenEntryIsMissing_ ReturnsFalse	2/2 killed	Covers both missing-file and missing-directory exception branches.	—
A (90–100)	new TrxReportEngineMergeTests. GetReparsePointStatus_ WhenAttributesCannotBeRead_ ReturnsNull	3/3 killed	Exercises all three unreadable-attribute exception types and asserts null.	—
A (90–100)	new TrxReportEngineMergeTests. GetReparsePointStatus_ WhenUnexpectedExceptionOccurs_ Propagates	1/1 killed	Confirms unrelated exceptions are not swallowed by the catch filter.	—
A (90–100)	new TrxReportEngineMergeTests. HasReparsePointComponent_ WhenAttributesCannotBeRead_ RejectsComponent	2/2 killed	Kills both the fail-closed (is not false) mutation and a call-count regression.	—
A (90–100)	new TrxReportEngineMergeTests. RelocateAttachments_ WhenMergedInRootAttributesCannotBeRead_ DropsReferencesWithoutDeletingRoot	4/4 killed	Materialized fixture proves the null-guard drops references and preserves the existing root/marker; body runs ~55 lines for one clear behavior.	—
A (90–100)	new TrxReportEngineMergeTests. GetReparsePointStatus_ WhenDirectoryLinkIsDangling_ ReturnsTrue	1/1 killed	Real symlink assertion with an appropriate platform-support skip guard.	—

All eight new tests target the GetReparsePointStatus/HasReparsePointComponent/RelocateAttachments fail-closed hardening added in this PR (unreadable file attributes must not be treated as safe). Each test isolates one meaningful production branch (missing entry, unreadable entry, unexpected exception, reparse-point detection, and the consumer-level drop-references path) with assertions that would fail if the corresponding guard were removed or inverted. No high-confidence actionable findings were identified, so no inline suggestions were posted.

_{This advisory comment was generated automatically. Grades are heuristic
and informational — they do not block merging. Re-run with
/review-tests.}

🤖 Automated content by GitHub Copilot. Generated by the Test Reviewer on PR (on open / sync) workflow. · auto · 118 AIC · ⌖ 3.01 AIC · ⊞ 16.9K · [◷]( · ◷)