Test that the HTML post-processor refuses to merge through a symlink

[Jakub Jareš (nohwnd)]

HtmlArtifactPostProcessor calls ArtifactPostProcessingHelper.IsReparsePoint before it merges, so a symlink or junction planted at the fixed merged name cannot redirect the merged report outside the output directory the orchestrator gave it. Trx, JUnit and Ctrf all have a test at the call site proving they act on that check, the HTML one did not, so deleting the guard from HtmlArtifactPostProcessor.cs made nothing in the repository fail.

Adds the symlink case and the dangling symlink case to HtmlArtifactPostProcessorTests, copying the shape of the existing ones.

Verified by removing the guard and rerunning: the symlink test fails on Assert.IsNull(output) because the merge writes through the link, and the dangling one fails by throwing. Both pass again with the guard restored. Full Microsoft.Testing.Extensions.UnitTests run on net9.0 is 1202 tests, 0 failed, and the project still builds for net462, net472, net8.0 and net9.0.

The dangling variant is not added to the Trx and Ctrf files. Those two call Directory.CreateDirectory(mergedDirectory) without the try/catch that the HTML and JUnit processors have, and on Linux that call throws IOException on a dangling directory symlink instead of returning, so the test would pass on Windows and fail on Linux CI. It looks like those two do not hold the never-fail-the-run invariant that the comment in TrxArtifactPostProcessor.cs describes, but that is a behavior change rather than a test, so I left it out of this PR.

Related: #10710

🤖

[github-actions]

Note

🤖 Automated review by GitHub Copilot. Generated by the Expert Code Review workflow. To request a follow-up action, reply by tagging @copilot directly.

Review Summary

Two new unit tests covering the reparse-point guard in HtmlArtifactPostProcessor.ProcessAsync — one for a live symlink and one for a dangling symlink. Both verify the processor returns null and does not write through the link.

#	Dimension	Verdict
1	Algorithmic Correctness	✅ Clean
2	Behavioral Compatibility	N/A — test-only
3	Public API Surface	N/A — no API changes
4	Thread Safety / Concurrency	N/A
5	Resource Management	✅ try/finally cleanup matches existing pattern
6	Error Handling	✅ Assert.Inconclusive for privilege-gated symlink creation
7	Performance	N/A — test code
8	Cross-TFM Correctness	✅ Correctly gated behind #if NETCOREAPP (Directory.CreateSymbolicLink is .NET 6+)
9	Localization	N/A
10	Security	✅ Tests validate the security guard
11	Naming & Conventions	✅
12	Test Quality	✅ Good assertions: IsNull on output, IsEmpty/IsFalse on side-effect directories
13	Documentation	N/A
14	Build & Packaging	N/A
15	Diagnostics / Logging	N/A
16	IPC / Wire Compat	N/A
17	Configuration	N/A
18	Analyzers	N/A
19	Cancellation	N/A
20	Serialization	N/A
21	Code Duplication	⚠️ Minor — the two tests share ~80% structure; a helper could reduce duplication, but this matches the existing test style in the file. Not blocking.
22	Scope Discipline	✅ Focused on one concern

Overall: Clean. No blocking issues found. The tests are well-structured, correctly guard against privilege limitations with Assert.Inconclusive, and follow existing patterns in the file.

[github-actions]

🧪 Expert test review — PR #10731

Grade	Test	Mutation	Notes	How to improve
A (90–100)	new HtmlArtifactPostProcessorTests. ProcessAsync_ WhenMergedDirectoryIsAReparsePoint_ DoesNotMerge	2/2 killed	Asserts both the null return and that nothing was written through the symlink target, exactly mirroring the accepted Trx/JUnit/Ctrf sibling tests for the same guard.	—
A (90–100)	new HtmlArtifactPostProcessorTests. ProcessAsync_ WhenMergedDirectoryIsADanglingReparsePoint_ DoesNotMerge	2/2 killed	Covers the dangling-symlink edge case distinctly from the live-symlink case, asserting null return and that the missing target was not materialized.	—

Both tests target the ArtifactPostProcessingHelper.IsReparsePoint(mergedDirectory) guard in HtmlArtifactPostProcessor.ProcessAsync. Removing that guard (as verified by the author) makes the first test fail on Assert.IsNull(output) because the merge would write through the link, and the second fails with a thrown IOException. The tests are #if NETCOREAPP-gated (symbolic link creation), handle the platform-permission case via Assert.Inconclusive, and clean up their temp directories in finally — matching the existing, already-reviewed pattern used for the Trx, JUnit, and Ctrf post-processors byte-for-byte in structure. No correctness, isolation, or maintainability issues found; no inline suggestions warranted.

_{This advisory comment was generated automatically. Grades are heuristic
and informational — they do not block merging. Suggestions on the Files
changed tab can be applied with one click. Re-run with
/review-tests.}

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

[Copilot]

Pull request overview

Adds HTML report post-processor coverage ensuring merges cannot write through symlinks outside the output directory.

Changes:

• Tests valid and dangling merged directory symlinks.
• Verifies processing returns no artifact and performs no redirected writes.

---

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

[github-actions]

🧵 Parallel-safety audit — PR #10731

Parallelization

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	coverable once the parallel-safety analyzers ship (attribute-based opt-in)

This PR only touches HtmlArtifactPostProcessorTests.cs, adding two [TestMethod]s (ProcessAsync_WhenMergedDirectoryIsAReparsePoint_DoesNotMerge, ProcessAsync_WhenMergedDirectoryIsADanglingReparsePoint_DoesNotMerge) guarded by #if NETCOREAPP. No lifecycle members ([TestInitialize]/[ClassInitialize]/ctor/Dispose), no [ResourceLock] / [DoNotParallelize] / [Parallelize] declarations, and no .runsettings/testconfig.json/MSBuild parallelization settings were added, removed, or modified.

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.

Top actions (by expected value): none — the change is parallel-safe as written.

---

Both new tests call the existing CreateTemporaryDirectory() helper, which mints a fresh GUID-named directory under Path.GetTempPath() per invocation. Everything each test does — including the fixed, production-determined child name merged used by HtmlArtifactPostProcessor, the symbolic link creation, and the report files it writes — happens strictly under that unique root, and each test deletes its own root in a finally. Under the assembly's MethodLevel scope every test (including these two) runs as its own concurrent chunk, but since the root directories never collide across tests, there is no shared mutable resource for two concurrently-running tests to race on: category B is clean because no other test can reach the same path, not because the path is "safe" in isolation.

No process-global state (env vars, CWD, culture, console, AppContext, static fields) is touched by the new code (category A: none). No [ResourceLock]/[DoNotParallelize] declarations were added, removed, or need reconciling (category C: none — nothing to under/over-declare). No serialization was introduced or removed (category D: none).

One minor readiness note, Info only, not a finding: Directory.CreateSymbolicLink requires elevation/Developer Mode on Windows; both new tests already handle this by calling Assert.Inconclusive and returning, which is the correct pattern and needs no change.

_{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 · 80.3 AIC · ⌖ 2.9 AIC · ⊞ 24.8K · [◷]( · ◷)

[Amaury Levé (Evangelink)]

Reviewed against RFC 018 and the existing artifact post-processing design. The added tests close the HTML call-site coverage gap and correctly verify fail-closed behavior for both live and dangling directory links without introducing production or API changes.