Skip to content

Prefer DOTNET_ROOT over directory traversal when finding muxer#53405

Open
JamieMagee wants to merge 1 commit intodotnet:mainfrom
JamieMagee:fix/muxer-respect-dotnet-root-over-symlink-traversal
Open

Prefer DOTNET_ROOT over directory traversal when finding muxer#53405
JamieMagee wants to merge 1 commit intodotnet:mainfrom
JamieMagee:fix/muxer-respect-dotnet-root-over-symlink-traversal

Conversation

@JamieMagee
Copy link
Member

The Muxer constructor walks up two directories from AppContext.BaseDirectory to find the dotnet host. The runtime resolves symlinks on that path, so on systems that compose multiple SDK versions into one directory via symlinks (NixOS, Guix, etc.), it ends up in the wrong root and can only see one runtime version.

Move the DOTNET_HOST_PATH and DOTNET_ROOT checks ahead of the directory traversal so package managers can set the root explicitly. When neither variable is set, the old heuristic still runs.

Also stops _muxerPath from being set to a non-muxer process path (e.g. testhost) when all other lookups fail.

Ref: NixOS/nixpkgs#464575
Ref: #51693

The Muxer constructor walks up two directories from
AppContext.BaseDirectory to find the dotnet host. The runtime resolves
symlinks on that path, so on systems that compose multiple SDK versions
into one directory via symlinks (Nix, Guix), it ends up in the wrong
root and can only see one runtime version.

Move the DOTNET_HOST_PATH and DOTNET_ROOT checks ahead of the directory
traversal so package managers can set the root explicitly. When neither
variable is set, the old heuristic still runs.

Also stops _muxerPath from being set to a non-muxer process path (e.g.
testhost) when all other lookups fail.

Ref: NixOS/nixpkgs#464575
Ref: dotnet#51693
Copilot AI review requested due to automatic review settings March 12, 2026 03:30
Copy link
Contributor

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 updates Muxer discovery to prefer explicit environment configuration (DOTNET_HOST_PATH, DOTNET_ROOT) over the existing directory-traversal heuristic, improving correctness for installations that rely on symlink composition (e.g., NixOS/Guix). It also avoids incorrectly treating the current process path (e.g., testhost) as the muxer when discovery fails.

Changes:

  • Prioritize DOTNET_HOST_PATH and DOTNET_ROOT checks before walking up from AppContext.BaseDirectory.
  • Keep the existing “two directories up” heuristic as a fallback when env vars are not usable.
  • Restrict the final fallback to only use Environment.ProcessPath when the current process is actually dotnet/dotnet.exe.
Comments suppressed due to low confidence (1)

src/Cli/Microsoft.DotNet.Cli.Utils/Muxer.cs:62

  • DOTNET_ROOT is accepted as long as it’s non-null, but an empty string (or relative path) can result in Path.Combine(dotnetRoot, muxerFileName) producing a relative dotnet/dotnet.exe lookup in the current working directory. To avoid accidentally binding to an unrelated dotnet in CWD, consider treating null/empty/whitespace as unset and (optionally) requiring Path.IsPathRooted(dotnetRoot) / Directory.Exists(dotnetRoot) before using it.
            var dotnetRoot = Environment.GetEnvironmentVariable("DOTNET_ROOT");
            if (dotnetRoot is not null)
            {
                string rootMuxer = Path.Combine(dotnetRoot, muxerFileName);
                if (File.Exists(rootMuxer))
                {
                    _muxerPath = rootMuxer;
                }
            }

Comment on lines +46 to +50
string? dotnetHostPath = Environment.GetEnvironmentVariable("DOTNET_HOST_PATH");
if (dotnetHostPath is not null && File.Exists(dotnetHostPath))
{
string muxerPathMaybe = Path.Combine(rootPath, muxerFileName);
if (File.Exists(muxerPathMaybe))
_muxerPath = dotnetHostPath;
}
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

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

The DOTNET_HOST_PATH branch only checks File.Exists, but doesn’t validate that the value actually points to the dotnet muxer (e.g., filename matches muxerFileName). If DOTNET_HOST_PATH is set to some other existing executable, this will set _muxerPath incorrectly and downstream ProcessStartInfo calls will try to run the wrong binary. Consider additionally validating Path.GetFileName(dotnetHostPath) against muxerFileName (and optionally requiring an absolute path) before accepting it.

This issue also appears on line 54 of the same file.

Copilot uses AI. Check for mistakes.
Comment on lines +41 to +79
// Check environment variables first to allow package managers and
// other tools to explicitly set the dotnet root. This is needed
// when the SDK is installed via symlinks (e.g. Nix, Guix) where
// the directory-traversal heuristic below would resolve symlinks
// and find the wrong root directory.
string? dotnetHostPath = Environment.GetEnvironmentVariable("DOTNET_HOST_PATH");
if (dotnetHostPath is not null && File.Exists(dotnetHostPath))
{
string muxerPathMaybe = Path.Combine(rootPath, muxerFileName);
if (File.Exists(muxerPathMaybe))
_muxerPath = dotnetHostPath;
}

if (_muxerPath is null)
{
var dotnetRoot = Environment.GetEnvironmentVariable("DOTNET_ROOT");
if (dotnetRoot is not null)
{
_muxerPath = muxerPathMaybe;
string rootMuxer = Path.Combine(dotnetRoot, muxerFileName);
if (File.Exists(rootMuxer))
{
_muxerPath = rootMuxer;
}
}
}

if (_muxerPath is null)
{
// Best-effort search for muxer.
// SDK sets DOTNET_HOST_PATH as absolute path to current dotnet executable
// Most scenarios are running dotnet.dll as the app
// Root directory with muxer should be two above app base: <root>/sdk/<version>
string? rootPath = Path.GetDirectoryName(Path.GetDirectoryName(AppContext.BaseDirectory.TrimEnd(Path.DirectorySeparatorChar)));
if (rootPath is not null)
{
string muxerPathMaybe = Path.Combine(rootPath, muxerFileName);
if (File.Exists(muxerPathMaybe))
{
_muxerPath = muxerPathMaybe;
}
}
}

Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

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

The muxer discovery behavior changed materially (env var precedence, and avoiding fallback to non-dotnet process paths). There are unit tests for this assembly, but none covering Muxer resolution. Adding tests that set DOTNET_HOST_PATH / DOTNET_ROOT to temp locations (and verifying the chosen path) would help prevent regressions, especially for the symlinked-root scenario described in the PR.

Copilot uses AI. Check for mistakes.
@JamieMagee
Copy link
Member Author

This Muxer change is compatible with the https://github.com/dotnet/designs/blob/main/accepted/2025/local-sdk-global-json.md design (implemented in PR #47664).

The design adds a paths property to global.json so the host resolver can search multiple locations for a compatible SDK (local .dotnet dirs, user-wide installs, machine-wide installs). That resolution happens in native code (hostfxr_resolve_sdk2), completely outside the Muxer class. This patch doesn't touch any of that.

The Muxer class is a separate thing. It finds the dotnet binary so the SDK can launch child processes (dotnet run, dotnet test, tools, etc). The original code (before 69ae64b) checked DOTNET_HOST_PATH and DOTNET_ROOT as fallbacks when the current process wasn't dotnet. That commit moved a directory-traversal heuristic (walk up two dirs from AppContext.BaseDirectory) to the front of the queue, ahead of the env vars. That's what broke symlink-based layouts.

This patch puts the env vars back in front. When they aren't set, the normal case for the local-sdk design, the directory traversal still runs and the behavior is identical to what PR #47664 shipped. The MSBuildSdkResolver also has its own TryResolveMuxerFromSdkResolution() that derives the muxer from the resolved SDK directory, which is independent of the Muxer class entirely.

One thing worth calling out: VSTestForwardingApp sets VSTEST_DOTNET_ROOT_PATH from Path.GetDirectoryName(new Muxer().MuxerPath). This was specifically broken in the nixpkgs issue (vstest would overwrite DOTNET_ROOT_X64 with the wrong store path). With this patch, when DOTNET_ROOT is set, MuxerPath points to the right binary and vstest gets the right root.

@marcpopMSFT
Copy link
Member

@agocke @baronfel DOTNET_ROOT is only applied in apphost scenarios, correct? Preferring it for the sdk lookup seems like we're overloading it beyond what that env variable is really meant to be. Not sure how to support the composite .net install as described though

@baronfel
Copy link
Member

baronfel commented Mar 17, 2026

Correct - DOTNET_ROOT is not used by the raw muxer to locate runtimes, it always looks next to itself.
This feels like a very fragile knob to be touching in general for the SDK, because if we do change this precedence, then when users start using dotnetup they'll start redirecting globally-installed dotnets to the DOTNET_ROOT-based, user-local hive. That's a breaking change in expectations from the documented traversal today.

I do think we may need SDK-tooling-specific knobs for these kinds of concepts - this is part of the reason I tried to unify the important path calculations in my other PR.

@JamieMagee
Copy link
Member Author

@marcpopMSFT @baronfel What about a dedicated variable instead? Something like DOTNET_SDK_ROOT.
The Muxer constructor would check it after DOTNET_HOST_PATH but before the directory traversal:

  1. DOTNET_HOST_PATH (already SDK-internal, most authoritative)
  2. DOTNET_SDK_ROOT + muxer filename (new)
  3. Directory traversal from AppContext.BaseDirectory (existing)
  4. Environment.ProcessPath if it's actually the dotnet binary (existing)

When DOTNET_SDK_ROOT isn't set, behavior is identical to today. Package managers set it to the composed root directory, everyone else never notices it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants