From bac40c877b07aa5edaac0cb029e62b6ebe5b71b4 Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Mon, 17 Aug 2026 15:31:31 -0700 Subject: [PATCH] Fix PATH caching in `CommandDiscovery` code (#27809) The method `GetLookupDirectoryPaths` always returns the cached instance of path collection. However, the returned collection gets mutated in `CommandPathSearch.ResolveCurrentDirectoryInLookupPaths` to resolve the relative paths such as `.\tools` based on the user's current working directory `$PWD`, so for example, `.\tools` gets replaced with `cwd-1\tools` in the cached instance. Then, when the user changes to a different working directory `cwd-2`, command discovery won't find the executable or ps1 script under `cwd-2/tools` as expected, but those executables under `cwd-1\tools` will always be discoverable no matter what the `$PWD` is. That behavior is incorrect. This pull request improves the caching logic of `GetLookupDirectoryPaths` and makes it return a copy of the path collection, so the mutation happens to the returned copy, and the cached instance is kept intact. So, for every command search, the `CommandPathSearch` will resolve relative paths against the `$PWD` that the user is located at that time. --- .../engine/CommandDiscovery.cs | 79 ++++++++----------- .../Get-Command.Tests.ps1 | 51 ++++++++++++ 2 files changed, 84 insertions(+), 46 deletions(-) diff --git a/src/System.Management.Automation/engine/CommandDiscovery.cs b/src/System.Management.Automation/engine/CommandDiscovery.cs index 9ca87f4c834..bd88024b240 100644 --- a/src/System.Management.Automation/engine/CommandDiscovery.cs +++ b/src/System.Management.Automation/engine/CommandDiscovery.cs @@ -1199,81 +1199,68 @@ internal void UnregisterLookupCommandInfoAction(string currentAction, string com /// internal LookupPathCollection GetLookupDirectoryPaths() { - LookupPathCollection result = new LookupPathCollection(); - string path = Environment.GetEnvironmentVariable("PATH"); + discoveryTracer.WriteLine("PATH: {0}", path); - discoveryTracer.WriteLine( - "PATH: {0}", - path); - - bool isPathCacheValid = - path != null && - string.Equals(_pathCacheKey, path, StringComparison.OrdinalIgnoreCase) && - _cachedPath != null; + bool isPathCacheValid = _cachedLookupPaths is not null + && string.Equals(_pathCacheKey, path, StringComparison.OrdinalIgnoreCase); if (!isPathCacheValid) { - // Reset the cached lookup paths - _cachedLookupPaths = null; - - // Tokenize the path and cache it - _pathCacheKey = path; + _cachedLookupPaths = null; - if (_pathCacheKey != null) + if (string.IsNullOrEmpty(path)) { + // Cache an empty collection when PATH is null (unset) or an empty string. + _cachedLookupPaths = new List(); + } + else + { + // Tokenize the path and cache it string[] tokenizedPath = _pathCacheKey.Split(Path.PathSeparator, StringSplitOptions.RemoveEmptyEntries); - _cachedPath = new Collection(); + _cachedLookupPaths = new List(capacity: tokenizedPath.Length); foreach (string directory in tokenizedPath) { - string tempDir = directory.TrimStart(); - if (tempDir.EqualsOrdinalIgnoreCase("~")) - { - tempDir = Environment.GetFolderPath( - Environment.SpecialFolder.UserProfile, - Environment.SpecialFolderOption.DoNotVerify); - } - else if (tempDir.StartsWith("~" + Path.DirectorySeparatorChar)) + string tempDir = directory.Trim(); + if (tempDir.StartsWith('~')) { - tempDir = Environment.GetFolderPath( - Environment.SpecialFolder.UserProfile, - Environment.SpecialFolderOption.DoNotVerify) - + Path.DirectorySeparatorChar - + tempDir.Substring(2); + if (tempDir.Length is 1) + { + tempDir = Environment.GetFolderPath( + Environment.SpecialFolder.UserProfile, + Environment.SpecialFolderOption.DoNotVerify); + } + else if (tempDir[1] == Path.DirectorySeparatorChar) + { + string homeDir = Environment.GetFolderPath( + Environment.SpecialFolder.UserProfile, + Environment.SpecialFolderOption.DoNotVerify); + tempDir = $"{homeDir}{Path.DirectorySeparatorChar}{tempDir.AsSpan(2)}"; + } } - _cachedPath.Add(tempDir); - result.Add(tempDir); + _cachedLookupPaths.Add(tempDir); } } } - else - { - result.AddRange(_cachedPath); - } - // Cache the new lookup paths - return _cachedLookupPaths ??= result; + // The returned instance will be mutated in 'CommandPathSearch.ResolveCurrentDirectoryInLookupPaths' when resolving relative paths, + // which depends on user's current working directory. So, we need to return a copy of the lookup paths to keep the cache intact. + return new LookupPathCollection(_cachedLookupPaths); } /// - /// The cached list of lookup paths. It can be invalidated by - /// the PATH changing. + /// The cached list of lookup paths. It can be invalidated by the PATH changing. /// - private LookupPathCollection _cachedLookupPaths; + private List _cachedLookupPaths; /// /// The key that determines if the cached PATH can be used. /// private string _pathCacheKey; - /// - /// The cache of the tokenized PATH directories. - /// - private Collection _cachedPath; - #endregion internal members #region environment variable helpers diff --git a/test/powershell/Modules/Microsoft.PowerShell.Core/Get-Command.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Core/Get-Command.Tests.ps1 index 1fa33efbf0d..46f49e87bc0 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Core/Get-Command.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Core/Get-Command.Tests.ps1 @@ -275,3 +275,54 @@ Describe "Get-Command Tests" -Tags "CI" { $result | Should -Be $null } } + +Describe "Test relative path in PATH env var" -Tags "CI" { + + BeforeAll { + $originalPath = $env:PATH + $originalLocation = Get-Location + + $subDir1 = Join-Path $TestDrive -ChildPath "subdir1" + $subDir2 = Join-Path $TestDrive -ChildPath "subdir2" + $toolsUnderSubDir1 = Join-Path $subDir1 -ChildPath "tools" + $toolsUnderSubDir2 = Join-Path $subDir2 -ChildPath "tools" + + $null = New-Item -Path $subDir1 -ItemType Directory -Force + $null = New-Item -Path $subDir2 -ItemType Directory -Force + $null = New-Item -Path $toolsUnderSubDir1 -ItemType Directory -Force + $null = New-Item -Path $toolsUnderSubDir2 -ItemType Directory -Force + + $helloScript = Join-Path $toolsUnderSubDir1 -ChildPath "hello.ps1" + $byeScript = Join-Path $toolsUnderSubDir2 -ChildPath "bye.ps1" + $null = New-Item -Path $helloScript -ItemType File -Force + $null = New-Item -Path $byeScript -ItemType File -Force + } + + AfterAll { + Set-Location $originalLocation + $env:PATH = $originalPath + } + + It "Get-Command should resolve relative path in PATH env var based on user's current working directory" { + $dirSep = [System.IO.Path]::DirectorySeparatorChar + $pathSep = [System.IO.Path]::PathSeparator + + Set-Location $subDir1 + $env:PATH = ".${dirSep}tools${pathSep}$env:PATH" + + $result = Get-Command "hello.ps1" -ErrorAction silentlycontinue + $result | Should -Not -BeNullOrEmpty -Because "CWD is $subDir1, so '.${dirSep}tools' in PATH should be resolved to '$toolsUnderSubDir1', which contains 'hello.ps1'" + $result.Path | Should -BeExactly $helloScript + + $result = Get-Command "bye.ps1" -ErrorAction silentlycontinue + $result | Should -BeNullOrEmpty -Because "'bye.ps1' is not in '$toolsUnderSubDir1'" + + Set-Location $subDir2 + $result = Get-Command "hello.ps1" -ErrorAction silentlycontinue + $result | Should -BeNullOrEmpty -Because "CWD is $subDir2, so '.${dirSep}tools' in PATH should be resolved to '$toolsUnderSubDir2', which contains 'bye.ps1'" + + $result = Get-Command "bye.ps1" -ErrorAction silentlycontinue + $result | Should -Not -BeNullOrEmpty + $result.Path | Should -BeExactly $byeScript + } +}