Skip to content

[release/8.0-staging] Fix absolute path check when loading hostfxr/hostpolicy/coreclr #116776

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

elinor-fung
Copy link
Member

@elinor-fung elinor-fung commented Jun 18, 2025

Backport of #116597 to release/8.0-staging

On Windows, we were incorrectly only checking for <letter>: and incorrectly determining that UNC and device paths were not absolute. This updates pal::is_path_rooted to match the managed Path.IsPathRooted and adds a pal::is_path_absolute with the logic of Path.IsPartiallyQualified

cc @dotnet/appmodel @AaronRobinsonMSFT

Customer Impact

  • Customer reported
  • Found internally

Customers are unable to run self-contained apps from or point framework-dependent apps to use a .NET runtime from a UNC share or device path.

Issue: #116521

Regression

  • Yes
  • No

Regressed in last servicing release.
b33d4e3

Testing

Manual testing for UNC share and device path. Automated test added for device paths.

Risk

Medium. The fix has to be in a path that every app will exercise. The updated logic is a mirror of existing logic in managed APIs.

On Windows, we were incorrectly only checking for <letter>: and incorrectly determining that UNC and device paths were not absolute. This updates pal::is_path_rooted to match the managed Path.IsPathRooted and adds a pal::is_path_absolute with the logic of Path.IsPartiallyQualified
@Copilot Copilot AI review requested due to automatic review settings June 18, 2025 03:06
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refines how hostfxr/hostpolicy/coreclr resolver checks for absolute paths on Windows by replacing is_path_rooted with a new is_path_fully_qualified, and adds tests for UNC and device path scenarios.

  • Introduces pal::is_path_fully_qualified alongside updated is_path_rooted logic in pal.windows.cpp and a matching stub in pal.unix.cpp.
  • Updates various resolver components (coreclr_resolver, deps_resolver, fxr_resolver, hostpolicy_resolver, apphost, corehost) to use the new qualification check.
  • Adds Windows-specific tests for UNC (\\?\) and device (\\.\) paths, and an error trace on failure.

Reviewed Changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/native/corehost/hostpolicy/standalone/coreclr_resolver.cpp Use fully qualified path check instead of rooted check
src/native/corehost/hostpolicy/deps_resolver.cpp Update assertion to use fully qualified path check
src/native/corehost/hostmisc/pal.windows.cpp Implement is_path_fully_qualified and refine rooted logic
src/native/corehost/hostmisc/pal.unix.cpp Stub is_path_fully_qualified to Unix-rooted logic
src/native/corehost/hostmisc/pal.h Declare is_path_fully_qualified
src/native/corehost/fxr_resolver.h Switch to fully qualified check for hostfxr path
src/native/corehost/fxr/standalone/hostpolicy_resolver.cpp Switch to fully qualified check for hostpolicy path
src/native/corehost/corehost.cpp Add error trace when fxr resolution fails
src/native/corehost/apphost/standalone/hostfxr_resolver.cpp Switch to fully qualified check for apphost fxr path
src/installer/tests/HostActivation.Tests/StandaloneAppActivation.cs Add tests for UNC and device EXE paths
src/installer/tests/HostActivation.Tests/PortableAppActivation.cs Add tests for device paths in DotNetRoot setting
Comments suppressed due to low confidence (1)

src/native/corehost/hostmisc/pal.h:331

  • Add a brief comment above is_path_fully_qualified explaining its purpose and how it differs from is_path_rooted, to aid future maintainers.
    bool is_path_fully_qualified(const string_t& path);

@elinor-fung elinor-fung changed the title Fix absolute path check when loading hostfxr/hostpolicy/coreclr [release/8.0-staging] Fix absolute path check when loading hostfxr/hostpolicy/coreclr Jun 18, 2025
@elinor-fung elinor-fung added the Servicing-consider Issue for next servicing release review label Jun 18, 2025
@elinor-fung elinor-fung added this to the 8.0.x milestone Jun 18, 2025
Copy link
Member

@jeffschwMSFT jeffschwMSFT left a comment

Choose a reason for hiding this comment

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

lgtm. we will take for consideration in 8.0.x

Copy link
Contributor

Tagging subscribers to this area: @vitek-karas, @agocke, @VSadov
See info in area-owners.md if you want to be subscribed.

@rbhanda rbhanda modified the milestones: 8.0.x, 8.0.19 Jun 24, 2025
@rbhanda rbhanda added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Jun 24, 2025
@elinor-fung elinor-fung merged commit 7315722 into dotnet:release/8.0-staging Jun 25, 2025
188 of 200 checks passed
@elinor-fung elinor-fung deleted the host-fixAbsolutePathCheck-8.0 branch June 25, 2025 22:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-Host Servicing-approved Approved for servicing release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants