-
Notifications
You must be signed in to change notification settings - Fork 805
Fix: Resolve correct pwsh path if installed via Windows Store #3246
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
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -32,7 +32,6 @@ public class PowerShellHostViewModel : ViewModelBase, IProfileManager | |||||||||||||||||||||||
| #region Variables | ||||||||||||||||||||||||
| private static readonly ILog Log = LogManager.GetLogger(typeof(PowerShellHostViewModel)); | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| private readonly IDialogCoordinator _dialogCoordinator; | ||||||||||||||||||||||||
| private readonly DispatcherTimer _searchDispatcherTimer = new(); | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| public IInterTabClient InterTabClient { get; } | ||||||||||||||||||||||||
|
|
@@ -307,16 +306,15 @@ public bool ProfileContextMenuIsOpen | |||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| #region Constructor, load settings | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| public PowerShellHostViewModel(IDialogCoordinator instance) | ||||||||||||||||||||||||
| public PowerShellHostViewModel() | ||||||||||||||||||||||||
| { | ||||||||||||||||||||||||
| _isLoading = true; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| _dialogCoordinator = instance; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| // Check if PowerShell executable is configured | ||||||||||||||||||||||||
| CheckExecutable(); | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| // Try to find PowerShell executable | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| if (!IsExecutableConfigured) | ||||||||||||||||||||||||
| TryFindExecutable(); | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
|
|
@@ -569,8 +567,20 @@ private void TryFindExecutable() | |||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| var applicationFilePath = ApplicationHelper.Find(PowerShell.PwshFileName); | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| if (string.IsNullOrEmpty(applicationFilePath)) | ||||||||||||||||||||||||
| // Workaround for: https://github.com/BornToBeRoot/NETworkManager/issues/3223 | ||||||||||||||||||||||||
| if (applicationFilePath.EndsWith("AppData\\Local\\Microsoft\\WindowsApps\\pwsh.exe")) | ||||||||||||||||||||||||
| { | ||||||||||||||||||||||||
| Log.Info("Found pwsh.exe in AppData (Microsoft Store installation). Trying to resolve real path..."); | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| var realPwshPath = FindRealPwshPath(applicationFilePath); | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| if (realPwshPath != null) | ||||||||||||||||||||||||
| applicationFilePath = realPwshPath; | ||||||||||||||||||||||||
|
Comment on lines
+577
to
+578
|
||||||||||||||||||||||||
| if (realPwshPath != null) | |
| applicationFilePath = realPwshPath; | |
| if (realPwshPath != null) | |
| { | |
| applicationFilePath = realPwshPath; | |
| } | |
| else | |
| { | |
| Log.Warn("Failed to resolve real pwsh path. Falling back to Windows PowerShell."); | |
| applicationFilePath = ApplicationHelper.Find(PowerShell.WindowsPowerShellFileName); | |
| } |
Copilot
AI
Nov 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential resource leak: The Process object is disposed via using, but if process.WaitForExit() hangs or takes too long, this could block indefinitely. Consider adding a timeout:
if (!process.WaitForExit(5000)) // 5 second timeout
{
process.Kill();
return null;
}| process.WaitForExit(); | |
| // Wait for up to 5 seconds for the process to exit | |
| if (!process.WaitForExit(5000)) | |
| { | |
| process.Kill(); | |
| return null; | |
| } |
Copilot
AI
Nov 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The string replacement logic appears redundant and potentially incorrect. Lines 632-637 perform multiple overlapping replacements for removing newlines and carriage returns:
Replace(@"\\", @"\")- converts double backslashes to single (this may corrupt valid Windows paths)Replace(@"\r", string.Empty)andReplace(@"\n", string.Empty)- these won't match literal\ror\nin the string- Multiple redundant
Replace("\r\n", ...),Replace("\n", ...),Replace("\r", ...)calls
Simplify to:
output = output.Trim();The output from (Get-Command pwsh).Source should already be a clean path without embedded newlines. The .Trim() call will handle any leading/trailing whitespace.
| output = output.Replace(@"\\", @"\") | |
| .Replace(@"\r", string.Empty) | |
| .Replace(@"\n", string.Empty) | |
| .Replace("\r\n", string.Empty) | |
| .Replace("\n", string.Empty) | |
| .Replace("\r", string.Empty) | |
| .Trim(); | |
| output = output.Trim(); |
Copilot
AI
Nov 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The catch block silently swallows all exceptions without logging. This makes debugging difficult if FindRealPwshPath fails. Consider logging the exception:
catch (Exception ex)
{
Log.Error($"Failed to resolve real pwsh path: {ex.Message}");
return null;
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The path check using
.EndsWith()is case-sensitive and may not handle all variations of the path (e.g., lowercase drive letters, forward slashes). Consider using a more robust check:This handles case variations and ensures we're matching the correct directory structure.