Skip to content

[ci-fix] Needs review: Accept UnauthorizedAccessException or IOException in Tar chained-symlink traversal test on Windows (refs #130177)#130231

Draft
github-actions[bot] wants to merge 1 commit into
mainfrom
ci-fix/tar-symlink-traversal-130177-edddd211189bce97
Draft

[ci-fix] Needs review: Accept UnauthorizedAccessException or IOException in Tar chained-symlink traversal test on Windows (refs #130177)#130231
github-actions[bot] wants to merge 1 commit into
mainfrom
ci-fix/tar-symlink-traversal-130177-edddd211189bce97

Conversation

@github-actions

@github-actions github-actions Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Workflow artifact: ci-fix
Artifact kind: help
Linked KBE: #130177

Note

This is an AI/Copilot-generated best-effort fix attempt that I could not fully validate (no clr+libs baseline build was possible in this environment). It is a starting point for a maintainer, not a finished change. Please review the analysis below before merging.

Root cause (best analysis)

TarFile_ExtractToDirectory_File_Tests.ExtractToDirectory_RejectsChainedSymlinkDirectoryTraversal_WithNestedFile fails intermittently on Windows only (~0.25% of runs, seen across many unrelated PR builds), with:

Assert.Throws() Failure: Exception type was not an exact match
Expected: typeof(System.UnauthorizedAccessException)
Actual:   typeof(System.IO.IOException)
---- System.IO.IOException : Cannot create '...\dest\a\b' because a file or directory with the same name already exists.
   at System.IO.FileSystem.CreateDirectory(...)
   at System.Formats.Tar.TarHelpers.CreateDirectory(...)
   at System.Formats.Tar.TarEntry.ExtractRelativeToDirectory(...)

The test builds a malicious archive whose chained symlinks (a/b -> ., a/b/c -> ., a/b/c/d -> ../../outside) attempt to escape the destination, then asserts that extraction throws. On Windows the rejection normally surfaces as UnauthorizedAccessException (attempting to traverse a directory symlink), but depending on the order in which the colliding entries are materialized, the same rejection can surface from Directory.CreateDirectory as IOException ("a file or directory with the same name already exists"). Both exceptions mean the traversal was rejected before anything escaped — the post-extraction assertions (outside/ and pwned.txt not created) already guarantee the security property and are not part of the failure.

The Assert.Throws<UnauthorizedAccessException> is therefore over-specified: it pins one of two equally-safe rejection paths.

Attempted fix

On the Windows branch, accept either UnauthorizedAccessException or IOException (still requiring that extraction throws). The non-Windows branch and the security post-conditions are unchanged, so the test keeps validating that the traversal is blocked and that nothing escapes the destination.

What is unverified / where I need help

  • I could not reproduce the ~0.25% Windows-only flake locally (no Windows host, and the clr+libs baseline build required to run the test was not possible here), so I cannot empirically confirm the fix removes the intermittent failure.
  • Please confirm the IOException path is genuinely always a safe rejection (i.e. never a case where the malicious file is partially written) — the existing Assert.False(... outside/pwned.txt ...) checks suggest it is, but a Tar owner should confirm this is the intended contract rather than a symptom of a deeper extraction-ordering bug worth fixing in the product.

Validation

  • Command: dotnet build /t:test src/libraries/System.Formats.Tar/tests/System.Formats.Tar.Tests.csproj — not run because a full clr+libs baseline build (prerequisite for the test host) exceeds this environment's time budget and background builds are unavailable.
  • Result: not run. The change is test-only and uses APIs already present in the file (Assert.ThrowsAny, OperatingSystem.IsWindows, C# type patterns).

Evidence

  • Failing build: https://dev.azure.com/dnceng-public/public/_build/results?buildId=1492983
  • First build it occurred: not attributable to a single commit — this is a long-standing low-rate (~0.25%) Windows-only assertion-strictness flake observed across many independent PR builds (e.g. 1494685, 1494680, 1494601), not a regression from a specific change.
  • Suspected regressing change: none identified — the assertion has always pinned a single exception type.

Help wanted

  • Likely author: omitted (no single regressing commit; pre-existing over-strict assertion).
  • Area owners (area-System.Formats.Tar): @karelz, @dotnet/area-system-formats-tar

Filed by ci-failure-fix. Comment here or on the workflow file to suggest changes; ci-failure-scan-feedback reads in-scope feedback daily and opens (or updates) a PR with prompt edits.

Note

🔒 Integrity filter blocked 6 items

The following items were blocked because they don't meet the GitHub integrity level.

To allow these resources, lower min-integrity in your GitHub frontmatter:

tools:
  github:
    min-integrity: approved  # merged | approved | unapproved | none

Generated by CI Outer-Loop Failure Fixer · ● 9.2M ·

…ink test on Windows

The ExtractToDirectory_RejectsChainedSymlinkDirectoryTraversal_WithNestedFile
test intermittently fails on Windows (~0.25%) because the colliding directory
creation during a rejected traversal can surface as IOException instead of
UnauthorizedAccessException depending on extraction ordering. Both exceptions
mean the traversal was rejected; the post-extraction assertions still verify
nothing escaped the destination.

Refs #130177

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @dotnet/area-system-formats-tar
See info in area-owners.md if you want to be subscribed.

@github-actions github-actions Bot mentioned this pull request Jul 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0 participants