Optimize TRX reparse point confinement checks - #10648

Merged
Amaury Levé (Evangelink) merged 3 commits into
mainfrom
dev/amauryleve/optimize-trx-path-checks
Aug 24, 2026
Merged

Optimize TRX reparse point confinement checks#10648
Amaury Levé (Evangelink) merged 3 commits into
mainfrom
dev/amauryleve/optimize-trx-path-checks

Conversation

@Evangelink

@EvangelinkAmaury Levé (Evangelink) commented Aug 19, 2026

Copy link
Copy Markdown
Member

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

Use a single fail-closed attribute probe for each confinement component and cover missing, inaccessible, regular-file, and dangling-link behavior.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
CopilotAI balanced review requested due to automatic review settings August 19, 2026 02:24
@EvangelinkAmaury Levé (Evangelink) added the state/needs-review Awaiting review from the team. label Aug 19, 2026

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
FileDescription
TrxReportEngine.Merge.PathHelpers.csAdds single-probe reparse-point detection.
TrxReportEngine.Merge.Attachments.csSafely handles inaccessible merged roots.
InternalAPI.Unshipped.txtRecords the new internal helper.
TrxReportEngineMergeTests.csTests 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

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

Use the repository's reflection test pattern instead of expanding the tracked internal API solely for unit tests.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings August 19, 2026 04:59
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

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

Inject attribute failures through private overloads and verify unreadable ancestors and merged roots are rejected without deleting or writing through them.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings August 19, 2026 06:44
@github-actions

Copy link
Copy Markdown
Contributor

🧵 Parallel-safety audit — PR #10648

Parallelization — assembly audited:

Test assemblyScopeWorkersAnalyzer coverage
Microsoft.Testing.Extensions.UnitTestsMethodLevel (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 readonlyMethodInfo 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 · [◷]( · )

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

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

@github-actions

Copy link
Copy Markdown
Contributor

🧪 Expert test review — PR #10648

GradeTestMutationNotesHow to improve
A (90–100)new TrxReportEngineMergeTests.
GetReparsePointStatus_
ReadsAttributesOnceAndDetectsReparsePoint
2/2 killedAsserts result and call count, killing both the true/false and once-only mutations.
A (90–100)new TrxReportEngineMergeTests.
GetReparsePointStatus_
WhenEntryIsARegularFile_
ReturnsFalse
1/1 killedFocused, clear assertion on the non-reparse-point path.
A (90–100)new TrxReportEngineMergeTests.
GetReparsePointStatus_
WhenEntryIsMissing_
ReturnsFalse
2/2 killedCovers both missing-file and missing-directory exception branches.
A (90–100)new TrxReportEngineMergeTests.
GetReparsePointStatus_
WhenAttributesCannotBeRead_
ReturnsNull
3/3 killedExercises all three unreadable-attribute exception types and asserts null.
A (90–100)new TrxReportEngineMergeTests.
GetReparsePointStatus_
WhenUnexpectedExceptionOccurs_
Propagates
1/1 killedConfirms unrelated exceptions are not swallowed by the catch filter.
A (90–100)new TrxReportEngineMergeTests.
HasReparsePointComponent_
WhenAttributesCannotBeRead_
RejectsComponent
2/2 killedKills both the fail-closed (is not false) mutation and a call-count regression.
A (90–100)new TrxReportEngineMergeTests.
RelocateAttachments_
WhenMergedInRootAttributesCannotBeRead_
DropsReferencesWithoutDeletingRoot
4/4 killedMaterialized 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 killedReal 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 · [◷]( · )

@Evangelink
Amaury Levé (Evangelink) merged commit 808198c into mainAug 24, 2026
53 checks passed
@Evangelink
Amaury Levé (Evangelink) deleted the dev/amauryleve/optimize-trx-path-checks branch August 24, 2026 08:21
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

state/needs-reviewAwaiting review from the team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[efficiency-improver] Reduce duplicate filesystem stat calls in TRX reparse-point detection

3 participants

@Evangelink@0101
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

Optimize TRX reparse point confinement checks - #10648

Merged
Amaury Levé (Evangelink) merged 3 commits into
mainfrom
dev/amauryleve/optimize-trx-path-checks
Aug 24, 2026
Merged

Optimize TRX reparse point confinement checks#10648
Amaury Levé (Evangelink) merged 3 commits into
mainfrom
dev/amauryleve/optimize-trx-path-checks

Conversation

@Evangelink

@EvangelinkAmaury Levé (Evangelink) commented Aug 19, 2026

Copy link
Copy Markdown
Member

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

Use a single fail-closed attribute probe for each confinement component and cover missing, inaccessible, regular-file, and dangling-link behavior.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
CopilotAI balanced review requested due to automatic review settings August 19, 2026 02:24
@EvangelinkAmaury Levé (Evangelink) added the state/needs-review Awaiting review from the team. label Aug 19, 2026

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
FileDescription
TrxReportEngine.Merge.PathHelpers.csAdds single-probe reparse-point detection.
TrxReportEngine.Merge.Attachments.csSafely handles inaccessible merged roots.
InternalAPI.Unshipped.txtRecords the new internal helper.
TrxReportEngineMergeTests.csTests 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

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

Use the repository's reflection test pattern instead of expanding the tracked internal API solely for unit tests.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings August 19, 2026 04:59
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

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

Inject attribute failures through private overloads and verify unreadable ancestors and merged roots are rejected without deleting or writing through them.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings August 19, 2026 06:44
@github-actions

Copy link
Copy Markdown
Contributor

🧵 Parallel-safety audit — PR #10648

Parallelization — assembly audited:

Test assemblyScopeWorkersAnalyzer coverage
Microsoft.Testing.Extensions.UnitTestsMethodLevel (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 readonlyMethodInfo 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 · [◷]( · )

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

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

@github-actions

Copy link
Copy Markdown
Contributor

🧪 Expert test review — PR #10648

GradeTestMutationNotesHow to improve
A (90–100)new TrxReportEngineMergeTests.
GetReparsePointStatus_
ReadsAttributesOnceAndDetectsReparsePoint
2/2 killedAsserts result and call count, killing both the true/false and once-only mutations.
A (90–100)new TrxReportEngineMergeTests.
GetReparsePointStatus_
WhenEntryIsARegularFile_
ReturnsFalse
1/1 killedFocused, clear assertion on the non-reparse-point path.
A (90–100)new TrxReportEngineMergeTests.
GetReparsePointStatus_
WhenEntryIsMissing_
ReturnsFalse
2/2 killedCovers both missing-file and missing-directory exception branches.
A (90–100)new TrxReportEngineMergeTests.
GetReparsePointStatus_
WhenAttributesCannotBeRead_
ReturnsNull
3/3 killedExercises all three unreadable-attribute exception types and asserts null.
A (90–100)new TrxReportEngineMergeTests.
GetReparsePointStatus_
WhenUnexpectedExceptionOccurs_
Propagates
1/1 killedConfirms unrelated exceptions are not swallowed by the catch filter.
A (90–100)new TrxReportEngineMergeTests.
HasReparsePointComponent_
WhenAttributesCannotBeRead_
RejectsComponent
2/2 killedKills both the fail-closed (is not false) mutation and a call-count regression.
A (90–100)new TrxReportEngineMergeTests.
RelocateAttachments_
WhenMergedInRootAttributesCannotBeRead_
DropsReferencesWithoutDeletingRoot
4/4 killedMaterialized 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 killedReal 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 · [◷]( · )

@Evangelink
Amaury Levé (Evangelink) merged commit 808198c into mainAug 24, 2026
53 checks passed
@Evangelink
Amaury Levé (Evangelink) deleted the dev/amauryleve/optimize-trx-path-checks branch August 24, 2026 08:21
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

state/needs-reviewAwaiting review from the team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[efficiency-improver] Reduce duplicate filesystem stat calls in TRX reparse-point detection

3 participants

@Evangelink@0101
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Optimize TRX reparse point confinement checks - #10648

Merged
Amaury Levé (Evangelink) merged 3 commits into
mainfrom
dev/amauryleve/optimize-trx-path-checks
Aug 24, 2026
Merged

Optimize TRX reparse point confinement checks#10648
Amaury Levé (Evangelink) merged 3 commits into
mainfrom
dev/amauryleve/optimize-trx-path-checks

Conversation

@Evangelink

@EvangelinkAmaury Levé (Evangelink) commented Aug 19, 2026

Copy link
Copy Markdown
Member

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

Use a single fail-closed attribute probe for each confinement component and cover missing, inaccessible, regular-file, and dangling-link behavior.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
CopilotAI balanced review requested due to automatic review settings August 19, 2026 02:24
@EvangelinkAmaury Levé (Evangelink) added the state/needs-review Awaiting review from the team. label Aug 19, 2026

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
FileDescription
TrxReportEngine.Merge.PathHelpers.csAdds single-probe reparse-point detection.
TrxReportEngine.Merge.Attachments.csSafely handles inaccessible merged roots.
InternalAPI.Unshipped.txtRecords the new internal helper.
TrxReportEngineMergeTests.csTests 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

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

Use the repository's reflection test pattern instead of expanding the tracked internal API solely for unit tests.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings August 19, 2026 04:59
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

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

Inject attribute failures through private overloads and verify unreadable ancestors and merged roots are rejected without deleting or writing through them.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings August 19, 2026 06:44
@github-actions

Copy link
Copy Markdown
Contributor

🧵 Parallel-safety audit — PR #10648

Parallelization — assembly audited:

Test assemblyScopeWorkersAnalyzer coverage
Microsoft.Testing.Extensions.UnitTestsMethodLevel (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 readonlyMethodInfo 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 · [◷]( · )

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

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

@github-actions

Copy link
Copy Markdown
Contributor

🧪 Expert test review — PR #10648

GradeTestMutationNotesHow to improve
A (90–100)new TrxReportEngineMergeTests.
GetReparsePointStatus_
ReadsAttributesOnceAndDetectsReparsePoint
2/2 killedAsserts result and call count, killing both the true/false and once-only mutations.
A (90–100)new TrxReportEngineMergeTests.
GetReparsePointStatus_
WhenEntryIsARegularFile_
ReturnsFalse
1/1 killedFocused, clear assertion on the non-reparse-point path.
A (90–100)new TrxReportEngineMergeTests.
GetReparsePointStatus_
WhenEntryIsMissing_
ReturnsFalse
2/2 killedCovers both missing-file and missing-directory exception branches.
A (90–100)new TrxReportEngineMergeTests.
GetReparsePointStatus_
WhenAttributesCannotBeRead_
ReturnsNull
3/3 killedExercises all three unreadable-attribute exception types and asserts null.
A (90–100)new TrxReportEngineMergeTests.
GetReparsePointStatus_
WhenUnexpectedExceptionOccurs_
Propagates
1/1 killedConfirms unrelated exceptions are not swallowed by the catch filter.
A (90–100)new TrxReportEngineMergeTests.
HasReparsePointComponent_
WhenAttributesCannotBeRead_
RejectsComponent
2/2 killedKills both the fail-closed (is not false) mutation and a call-count regression.
A (90–100)new TrxReportEngineMergeTests.
RelocateAttachments_
WhenMergedInRootAttributesCannotBeRead_
DropsReferencesWithoutDeletingRoot
4/4 killedMaterialized 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 killedReal 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 · [◷]( · )

@Evangelink
Amaury Levé (Evangelink) merged commit 808198c into mainAug 24, 2026
53 checks passed
@Evangelink
Amaury Levé (Evangelink) deleted the dev/amauryleve/optimize-trx-path-checks branch August 24, 2026 08:21
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

state/needs-reviewAwaiting review from the team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[efficiency-improver] Reduce duplicate filesystem stat calls in TRX reparse-point detection

3 participants

@Evangelink@0101
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Optimize TRX reparse point confinement checks - #10648

Merged
Amaury Levé (Evangelink) merged 3 commits into
mainfrom
dev/amauryleve/optimize-trx-path-checks
Aug 24, 2026
Merged

Optimize TRX reparse point confinement checks#10648
Amaury Levé (Evangelink) merged 3 commits into
mainfrom
dev/amauryleve/optimize-trx-path-checks

Conversation

@Evangelink

@EvangelinkAmaury Levé (Evangelink) commented Aug 19, 2026

Copy link
Copy Markdown
Member

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

Use a single fail-closed attribute probe for each confinement component and cover missing, inaccessible, regular-file, and dangling-link behavior.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
CopilotAI balanced review requested due to automatic review settings August 19, 2026 02:24
@EvangelinkAmaury Levé (Evangelink) added the state/needs-review Awaiting review from the team. label Aug 19, 2026

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
FileDescription
TrxReportEngine.Merge.PathHelpers.csAdds single-probe reparse-point detection.
TrxReportEngine.Merge.Attachments.csSafely handles inaccessible merged roots.
InternalAPI.Unshipped.txtRecords the new internal helper.
TrxReportEngineMergeTests.csTests 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

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

Use the repository's reflection test pattern instead of expanding the tracked internal API solely for unit tests.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings August 19, 2026 04:59
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

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

Inject attribute failures through private overloads and verify unreadable ancestors and merged roots are rejected without deleting or writing through them.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings August 19, 2026 06:44
@github-actions

Copy link
Copy Markdown
Contributor

🧵 Parallel-safety audit — PR #10648

Parallelization — assembly audited:

Test assemblyScopeWorkersAnalyzer coverage
Microsoft.Testing.Extensions.UnitTestsMethodLevel (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 readonlyMethodInfo 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 · [◷]( · )

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

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

@github-actions

Copy link
Copy Markdown
Contributor

🧪 Expert test review — PR #10648

GradeTestMutationNotesHow to improve
A (90–100)new TrxReportEngineMergeTests.
GetReparsePointStatus_
ReadsAttributesOnceAndDetectsReparsePoint
2/2 killedAsserts result and call count, killing both the true/false and once-only mutations.
A (90–100)new TrxReportEngineMergeTests.
GetReparsePointStatus_
WhenEntryIsARegularFile_
ReturnsFalse
1/1 killedFocused, clear assertion on the non-reparse-point path.
A (90–100)new TrxReportEngineMergeTests.
GetReparsePointStatus_
WhenEntryIsMissing_
ReturnsFalse
2/2 killedCovers both missing-file and missing-directory exception branches.
A (90–100)new TrxReportEngineMergeTests.
GetReparsePointStatus_
WhenAttributesCannotBeRead_
ReturnsNull
3/3 killedExercises all three unreadable-attribute exception types and asserts null.
A (90–100)new TrxReportEngineMergeTests.
GetReparsePointStatus_
WhenUnexpectedExceptionOccurs_
Propagates
1/1 killedConfirms unrelated exceptions are not swallowed by the catch filter.
A (90–100)new TrxReportEngineMergeTests.
HasReparsePointComponent_
WhenAttributesCannotBeRead_
RejectsComponent
2/2 killedKills both the fail-closed (is not false) mutation and a call-count regression.
A (90–100)new TrxReportEngineMergeTests.
RelocateAttachments_
WhenMergedInRootAttributesCannotBeRead_
DropsReferencesWithoutDeletingRoot
4/4 killedMaterialized 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 killedReal 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 · [◷]( · )

@Evangelink
Amaury Levé (Evangelink) merged commit 808198c into mainAug 24, 2026
53 checks passed
@Evangelink
Amaury Levé (Evangelink) deleted the dev/amauryleve/optimize-trx-path-checks branch August 24, 2026 08:21
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

state/needs-reviewAwaiting review from the team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[efficiency-improver] Reduce duplicate filesystem stat calls in TRX reparse-point detection

3 participants

@Evangelink@0101
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

Optimize TRX reparse point confinement checks - #10648

Merged
Amaury Levé (Evangelink) merged 3 commits into
mainfrom
dev/amauryleve/optimize-trx-path-checks
Aug 24, 2026
Merged

Optimize TRX reparse point confinement checks#10648
Amaury Levé (Evangelink) merged 3 commits into
mainfrom
dev/amauryleve/optimize-trx-path-checks

Conversation

@Evangelink

@EvangelinkAmaury Levé (Evangelink) commented Aug 19, 2026

Copy link
Copy Markdown
Member

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

Use a single fail-closed attribute probe for each confinement component and cover missing, inaccessible, regular-file, and dangling-link behavior.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
CopilotAI balanced review requested due to automatic review settings August 19, 2026 02:24
@EvangelinkAmaury Levé (Evangelink) added the state/needs-review Awaiting review from the team. label Aug 19, 2026

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
FileDescription
TrxReportEngine.Merge.PathHelpers.csAdds single-probe reparse-point detection.
TrxReportEngine.Merge.Attachments.csSafely handles inaccessible merged roots.
InternalAPI.Unshipped.txtRecords the new internal helper.
TrxReportEngineMergeTests.csTests 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

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

Use the repository's reflection test pattern instead of expanding the tracked internal API solely for unit tests.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings August 19, 2026 04:59
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

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

Inject attribute failures through private overloads and verify unreadable ancestors and merged roots are rejected without deleting or writing through them.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings August 19, 2026 06:44
@github-actions

Copy link
Copy Markdown
Contributor

🧵 Parallel-safety audit — PR #10648

Parallelization — assembly audited:

Test assemblyScopeWorkersAnalyzer coverage
Microsoft.Testing.Extensions.UnitTestsMethodLevel (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 readonlyMethodInfo 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 · [◷]( · )

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

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

@github-actions

Copy link
Copy Markdown
Contributor

🧪 Expert test review — PR #10648

GradeTestMutationNotesHow to improve
A (90–100)new TrxReportEngineMergeTests.
GetReparsePointStatus_
ReadsAttributesOnceAndDetectsReparsePoint
2/2 killedAsserts result and call count, killing both the true/false and once-only mutations.
A (90–100)new TrxReportEngineMergeTests.
GetReparsePointStatus_
WhenEntryIsARegularFile_
ReturnsFalse
1/1 killedFocused, clear assertion on the non-reparse-point path.
A (90–100)new TrxReportEngineMergeTests.
GetReparsePointStatus_
WhenEntryIsMissing_
ReturnsFalse
2/2 killedCovers both missing-file and missing-directory exception branches.
A (90–100)new TrxReportEngineMergeTests.
GetReparsePointStatus_
WhenAttributesCannotBeRead_
ReturnsNull
3/3 killedExercises all three unreadable-attribute exception types and asserts null.
A (90–100)new TrxReportEngineMergeTests.
GetReparsePointStatus_
WhenUnexpectedExceptionOccurs_
Propagates
1/1 killedConfirms unrelated exceptions are not swallowed by the catch filter.
A (90–100)new TrxReportEngineMergeTests.
HasReparsePointComponent_
WhenAttributesCannotBeRead_
RejectsComponent
2/2 killedKills both the fail-closed (is not false) mutation and a call-count regression.
A (90–100)new TrxReportEngineMergeTests.
RelocateAttachments_
WhenMergedInRootAttributesCannotBeRead_
DropsReferencesWithoutDeletingRoot
4/4 killedMaterialized 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 killedReal 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 · [◷]( · )

@Evangelink
Amaury Levé (Evangelink) merged commit 808198c into mainAug 24, 2026
53 checks passed
@Evangelink
Amaury Levé (Evangelink) deleted the dev/amauryleve/optimize-trx-path-checks branch August 24, 2026 08:21
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

state/needs-reviewAwaiting review from the team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[efficiency-improver] Reduce duplicate filesystem stat calls in TRX reparse-point detection

3 participants

@Evangelink@0101
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Optimize TRX reparse point confinement checks - #10648

Merged
Amaury Levé (Evangelink) merged 3 commits into
mainfrom
dev/amauryleve/optimize-trx-path-checks
Aug 24, 2026
Merged

Optimize TRX reparse point confinement checks#10648
Amaury Levé (Evangelink) merged 3 commits into
mainfrom
dev/amauryleve/optimize-trx-path-checks

Conversation

@Evangelink

@EvangelinkAmaury Levé (Evangelink) commented Aug 19, 2026

Copy link
Copy Markdown
Member

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

Use a single fail-closed attribute probe for each confinement component and cover missing, inaccessible, regular-file, and dangling-link behavior.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
CopilotAI balanced review requested due to automatic review settings August 19, 2026 02:24
@EvangelinkAmaury Levé (Evangelink) added the state/needs-review Awaiting review from the team. label Aug 19, 2026

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
FileDescription
TrxReportEngine.Merge.PathHelpers.csAdds single-probe reparse-point detection.
TrxReportEngine.Merge.Attachments.csSafely handles inaccessible merged roots.
InternalAPI.Unshipped.txtRecords the new internal helper.
TrxReportEngineMergeTests.csTests 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

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

Use the repository's reflection test pattern instead of expanding the tracked internal API solely for unit tests.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings August 19, 2026 04:59
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

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

Inject attribute failures through private overloads and verify unreadable ancestors and merged roots are rejected without deleting or writing through them.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings August 19, 2026 06:44
@github-actions

Copy link
Copy Markdown
Contributor

🧵 Parallel-safety audit — PR #10648

Parallelization — assembly audited:

Test assemblyScopeWorkersAnalyzer coverage
Microsoft.Testing.Extensions.UnitTestsMethodLevel (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 readonlyMethodInfo 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 · [◷]( · )

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

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

@github-actions

Copy link
Copy Markdown
Contributor

🧪 Expert test review — PR #10648

GradeTestMutationNotesHow to improve
A (90–100)new TrxReportEngineMergeTests.
GetReparsePointStatus_
ReadsAttributesOnceAndDetectsReparsePoint
2/2 killedAsserts result and call count, killing both the true/false and once-only mutations.
A (90–100)new TrxReportEngineMergeTests.
GetReparsePointStatus_
WhenEntryIsARegularFile_
ReturnsFalse
1/1 killedFocused, clear assertion on the non-reparse-point path.
A (90–100)new TrxReportEngineMergeTests.
GetReparsePointStatus_
WhenEntryIsMissing_
ReturnsFalse
2/2 killedCovers both missing-file and missing-directory exception branches.
A (90–100)new TrxReportEngineMergeTests.
GetReparsePointStatus_
WhenAttributesCannotBeRead_
ReturnsNull
3/3 killedExercises all three unreadable-attribute exception types and asserts null.
A (90–100)new TrxReportEngineMergeTests.
GetReparsePointStatus_
WhenUnexpectedExceptionOccurs_
Propagates
1/1 killedConfirms unrelated exceptions are not swallowed by the catch filter.
A (90–100)new TrxReportEngineMergeTests.
HasReparsePointComponent_
WhenAttributesCannotBeRead_
RejectsComponent
2/2 killedKills both the fail-closed (is not false) mutation and a call-count regression.
A (90–100)new TrxReportEngineMergeTests.
RelocateAttachments_
WhenMergedInRootAttributesCannotBeRead_
DropsReferencesWithoutDeletingRoot
4/4 killedMaterialized 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 killedReal 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 · [◷]( · )

@Evangelink
Amaury Levé (Evangelink) merged commit 808198c into mainAug 24, 2026
53 checks passed
@Evangelink
Amaury Levé (Evangelink) deleted the dev/amauryleve/optimize-trx-path-checks branch August 24, 2026 08:21
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

state/needs-reviewAwaiting review from the team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[efficiency-improver] Reduce duplicate filesystem stat calls in TRX reparse-point detection

3 participants

@Evangelink@0101
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Optimize TRX reparse point confinement checks - #10648

Merged
Amaury Levé (Evangelink) merged 3 commits into
mainfrom
dev/amauryleve/optimize-trx-path-checks
Aug 24, 2026
Merged

Optimize TRX reparse point confinement checks#10648
Amaury Levé (Evangelink) merged 3 commits into
mainfrom
dev/amauryleve/optimize-trx-path-checks

Conversation

@Evangelink

@EvangelinkAmaury Levé (Evangelink) commented Aug 19, 2026

Copy link
Copy Markdown
Member

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

Use a single fail-closed attribute probe for each confinement component and cover missing, inaccessible, regular-file, and dangling-link behavior.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
CopilotAI balanced review requested due to automatic review settings August 19, 2026 02:24
@EvangelinkAmaury Levé (Evangelink) added the state/needs-review Awaiting review from the team. label Aug 19, 2026

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
FileDescription
TrxReportEngine.Merge.PathHelpers.csAdds single-probe reparse-point detection.
TrxReportEngine.Merge.Attachments.csSafely handles inaccessible merged roots.
InternalAPI.Unshipped.txtRecords the new internal helper.
TrxReportEngineMergeTests.csTests 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

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

Use the repository's reflection test pattern instead of expanding the tracked internal API solely for unit tests.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings August 19, 2026 04:59
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

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

Inject attribute failures through private overloads and verify unreadable ancestors and merged roots are rejected without deleting or writing through them.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings August 19, 2026 06:44
@github-actions

Copy link
Copy Markdown
Contributor

🧵 Parallel-safety audit — PR #10648

Parallelization — assembly audited:

Test assemblyScopeWorkersAnalyzer coverage
Microsoft.Testing.Extensions.UnitTestsMethodLevel (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 readonlyMethodInfo 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 · [◷]( · )

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

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

@github-actions

Copy link
Copy Markdown
Contributor

🧪 Expert test review — PR #10648

GradeTestMutationNotesHow to improve
A (90–100)new TrxReportEngineMergeTests.
GetReparsePointStatus_
ReadsAttributesOnceAndDetectsReparsePoint
2/2 killedAsserts result and call count, killing both the true/false and once-only mutations.
A (90–100)new TrxReportEngineMergeTests.
GetReparsePointStatus_
WhenEntryIsARegularFile_
ReturnsFalse
1/1 killedFocused, clear assertion on the non-reparse-point path.
A (90–100)new TrxReportEngineMergeTests.
GetReparsePointStatus_
WhenEntryIsMissing_
ReturnsFalse
2/2 killedCovers both missing-file and missing-directory exception branches.
A (90–100)new TrxReportEngineMergeTests.
GetReparsePointStatus_
WhenAttributesCannotBeRead_
ReturnsNull
3/3 killedExercises all three unreadable-attribute exception types and asserts null.
A (90–100)new TrxReportEngineMergeTests.
GetReparsePointStatus_
WhenUnexpectedExceptionOccurs_
Propagates
1/1 killedConfirms unrelated exceptions are not swallowed by the catch filter.
A (90–100)new TrxReportEngineMergeTests.
HasReparsePointComponent_
WhenAttributesCannotBeRead_
RejectsComponent
2/2 killedKills both the fail-closed (is not false) mutation and a call-count regression.
A (90–100)new TrxReportEngineMergeTests.
RelocateAttachments_
WhenMergedInRootAttributesCannotBeRead_
DropsReferencesWithoutDeletingRoot
4/4 killedMaterialized 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 killedReal 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 · [◷]( · )

@Evangelink
Amaury Levé (Evangelink) merged commit 808198c into mainAug 24, 2026
53 checks passed
@Evangelink
Amaury Levé (Evangelink) deleted the dev/amauryleve/optimize-trx-path-checks branch August 24, 2026 08:21
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

state/needs-reviewAwaiting review from the team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[efficiency-improver] Reduce duplicate filesystem stat calls in TRX reparse-point detection

3 participants

@Evangelink@0101
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

Optimize TRX reparse point confinement checks - #10648

Merged
Amaury Levé (Evangelink) merged 3 commits into
mainfrom
dev/amauryleve/optimize-trx-path-checks
Aug 24, 2026
Merged

Optimize TRX reparse point confinement checks#10648
Amaury Levé (Evangelink) merged 3 commits into
mainfrom
dev/amauryleve/optimize-trx-path-checks

Conversation

@Evangelink

@EvangelinkAmaury Levé (Evangelink) commented Aug 19, 2026

Copy link
Copy Markdown
Member

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

Use a single fail-closed attribute probe for each confinement component and cover missing, inaccessible, regular-file, and dangling-link behavior.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
CopilotAI balanced review requested due to automatic review settings August 19, 2026 02:24
@EvangelinkAmaury Levé (Evangelink) added the state/needs-review Awaiting review from the team. label Aug 19, 2026

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
FileDescription
TrxReportEngine.Merge.PathHelpers.csAdds single-probe reparse-point detection.
TrxReportEngine.Merge.Attachments.csSafely handles inaccessible merged roots.
InternalAPI.Unshipped.txtRecords the new internal helper.
TrxReportEngineMergeTests.csTests 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

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

Use the repository's reflection test pattern instead of expanding the tracked internal API solely for unit tests.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings August 19, 2026 04:59
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

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

Inject attribute failures through private overloads and verify unreadable ancestors and merged roots are rejected without deleting or writing through them.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings August 19, 2026 06:44
@github-actions

Copy link
Copy Markdown
Contributor

🧵 Parallel-safety audit — PR #10648

Parallelization — assembly audited:

Test assemblyScopeWorkersAnalyzer coverage
Microsoft.Testing.Extensions.UnitTestsMethodLevel (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 readonlyMethodInfo 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 · [◷]( · )

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

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

@github-actions

Copy link
Copy Markdown
Contributor

🧪 Expert test review — PR #10648

GradeTestMutationNotesHow to improve
A (90–100)new TrxReportEngineMergeTests.
GetReparsePointStatus_
ReadsAttributesOnceAndDetectsReparsePoint
2/2 killedAsserts result and call count, killing both the true/false and once-only mutations.
A (90–100)new TrxReportEngineMergeTests.
GetReparsePointStatus_
WhenEntryIsARegularFile_
ReturnsFalse
1/1 killedFocused, clear assertion on the non-reparse-point path.
A (90–100)new TrxReportEngineMergeTests.
GetReparsePointStatus_
WhenEntryIsMissing_
ReturnsFalse
2/2 killedCovers both missing-file and missing-directory exception branches.
A (90–100)new TrxReportEngineMergeTests.
GetReparsePointStatus_
WhenAttributesCannotBeRead_
ReturnsNull
3/3 killedExercises all three unreadable-attribute exception types and asserts null.
A (90–100)new TrxReportEngineMergeTests.
GetReparsePointStatus_
WhenUnexpectedExceptionOccurs_
Propagates
1/1 killedConfirms unrelated exceptions are not swallowed by the catch filter.
A (90–100)new TrxReportEngineMergeTests.
HasReparsePointComponent_
WhenAttributesCannotBeRead_
RejectsComponent
2/2 killedKills both the fail-closed (is not false) mutation and a call-count regression.
A (90–100)new TrxReportEngineMergeTests.
RelocateAttachments_
WhenMergedInRootAttributesCannotBeRead_
DropsReferencesWithoutDeletingRoot
4/4 killedMaterialized 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 killedReal 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 · [◷]( · )

@Evangelink
Amaury Levé (Evangelink) merged commit 808198c into mainAug 24, 2026
53 checks passed
@Evangelink
Amaury Levé (Evangelink) deleted the dev/amauryleve/optimize-trx-path-checks branch August 24, 2026 08:21
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

state/needs-reviewAwaiting review from the team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[efficiency-improver] Reduce duplicate filesystem stat calls in TRX reparse-point detection

3 participants

@Evangelink@0101