From 42bf6e96f6ecec4f7cd61416ff6c26838012c019 Mon Sep 17 00:00:00 2001 From: Matthew John Cheetham Date: Tue, 31 Mar 2026 11:54:06 +0100 Subject: [PATCH 01/16] github/gitlab: use correct param order When using a custom credential UI for either GitHub or GitLab the commands are mixing up the optional URL and user name args. This is because the positional args of the `ExecuteAsync` methods was not the same as the `SetHandler`. Swap the args to fix this. Signed-off-by: Matthew John Cheetham --- src/shared/GitHub/UI/Commands/CredentialsCommand.cs | 2 +- src/shared/GitLab/UI/Commands/CredentialsCommand.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/shared/GitHub/UI/Commands/CredentialsCommand.cs b/src/shared/GitHub/UI/Commands/CredentialsCommand.cs index f14b3cb3e..45c6cfd7f 100644 --- a/src/shared/GitHub/UI/Commands/CredentialsCommand.cs +++ b/src/shared/GitHub/UI/Commands/CredentialsCommand.cs @@ -38,7 +38,7 @@ protected CredentialsCommand(ICommandContext context) this.SetHandler(ExecuteAsync, url, userName, basic, browser, device, pat, all); } - private async Task ExecuteAsync(string userName, string enterpriseUrl, + private async Task ExecuteAsync(string enterpriseUrl, string userName, bool basic, bool browser, bool device, bool pat, bool all) { var viewModel = new CredentialsViewModel(Context.SessionManager, Context.ProcessManager) diff --git a/src/shared/GitLab/UI/Commands/CredentialsCommand.cs b/src/shared/GitLab/UI/Commands/CredentialsCommand.cs index 1c1995a8d..02a0f7818 100644 --- a/src/shared/GitLab/UI/Commands/CredentialsCommand.cs +++ b/src/shared/GitLab/UI/Commands/CredentialsCommand.cs @@ -35,7 +35,7 @@ protected CredentialsCommand(ICommandContext context) this.SetHandler(ExecuteAsync, url, userName, basic, browser, pat, all); } - private async Task ExecuteAsync(string userName, string url, bool basic, bool browser, bool pat, bool all) + private async Task ExecuteAsync(string url, string userName, bool basic, bool browser, bool pat, bool all) { var viewModel = new CredentialsViewModel(Context.SessionManager) { From 51a237938fd41fe1663a05a6df32802df503c7c4 Mon Sep 17 00:00:00 2001 From: Matthew John Cheetham Date: Tue, 31 Mar 2026 11:56:26 +0100 Subject: [PATCH 02/16] streamextensions: fix a bug in multi-var reset handling The multi-dictionary writer normalises value lists to honor reset semantics (empty values clear prior entries). However, when only one normalised value remains, it writes the first element from the original list instead of the normalised list. If the list contains an empty reset marker followed by a single valid value, the output will incorrectly emit the pre-reset value (or an empty string), violating the protocol and potentially leaking stale data that should have been cleared. Fix the issue and extend the unit tests to cover this shape. Signed-off-by: Matthew John Cheetham --- src/shared/Core.Tests/StreamExtensionsTests.cs | 5 +++-- src/shared/Core/StreamExtensions.cs | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/shared/Core.Tests/StreamExtensionsTests.cs b/src/shared/Core.Tests/StreamExtensionsTests.cs index 09153ad26..b72874ba9 100644 --- a/src/shared/Core.Tests/StreamExtensionsTests.cs +++ b/src/shared/Core.Tests/StreamExtensionsTests.cs @@ -381,12 +381,13 @@ public void StreamExtensions_WriteDictionary_MultiEntriesWithEmpty_WritesKVPList { ["a"] = new[] {"1", "2", "", "3", "4"}, ["b"] = new[] {"5"}, - ["c"] = new[] {"6", "7", ""} + ["c"] = new[] {"6", "7", ""}, + ["d"] = new[] {"8", "", "9"} }; string output = WriteStringStream(input, StreamExtensions.WriteDictionary, newLine: LF); - Assert.Equal("a[]=3\na[]=4\nb=5\n\n", output); + Assert.Equal("a[]=3\na[]=4\nb=5\nd=9\n\n", output); } #endregion diff --git a/src/shared/Core/StreamExtensions.cs b/src/shared/Core/StreamExtensions.cs index 7ff338f5a..beb85699b 100644 --- a/src/shared/Core/StreamExtensions.cs +++ b/src/shared/Core/StreamExtensions.cs @@ -179,7 +179,7 @@ public static void WriteDictionary(this TextWriter writer, IDictionary Date: Tue, 31 Mar 2026 11:59:31 +0100 Subject: [PATCH 03/16] github: handle empty domain or enterprise hints If the WWW-Authenticate header from GitHub is missing a domain or enterprise hint we'd be hitting a null-reference exception when calculating a hash code for the `GitHubAuthChallenge`. Fix this. Signed-off-by: Matthew John Cheetham --- src/shared/GitHub/GitHubAuthChallenge.cs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/shared/GitHub/GitHubAuthChallenge.cs b/src/shared/GitHub/GitHubAuthChallenge.cs index de3afbdbd..1b33330c9 100644 --- a/src/shared/GitHub/GitHubAuthChallenge.cs +++ b/src/shared/GitHub/GitHubAuthChallenge.cs @@ -107,7 +107,15 @@ public override bool Equals(object obj) public override int GetHashCode() { - return Domain.GetHashCode() * 1019 ^ - Enterprise.GetHashCode() * 337; + int domainHash = Domain is null + ? 0 + : StringComparer.OrdinalIgnoreCase.GetHashCode(Domain); + + int enterpriseHash = Enterprise is null + ? 0 + : StringComparer.OrdinalIgnoreCase.GetHashCode(Enterprise); + + return (domainHash * 1019) ^ + (enterpriseHash * 337); } } From 453ae23fe265b2099f1705481001f6d7d7834ed4 Mon Sep 17 00:00:00 2001 From: Matthew John Cheetham Date: Tue, 31 Mar 2026 12:00:51 +0100 Subject: [PATCH 04/16] github: do not filter accounts outside of dotcom Outside of GitHub.com we should not filter accounts. The `FilterAccounts` method was detecting and logging this, but didn't actually stop filtering! Signed-off-by: Matthew John Cheetham --- src/shared/GitHub/GitHubHostProvider.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/shared/GitHub/GitHubHostProvider.cs b/src/shared/GitHub/GitHubHostProvider.cs index 06afd9592..07607dd4e 100644 --- a/src/shared/GitHub/GitHubHostProvider.cs +++ b/src/shared/GitHub/GitHubHostProvider.cs @@ -198,6 +198,7 @@ private bool FilterAccounts(Uri remoteUri, IEnumerable wwwAuth, ref ILis if (!IsGitHubDotCom(remoteUri)) { _context.Trace.WriteLine("No account filtering outside of GitHub.com."); + return false; } // Allow the user to disable account filtering until this feature stabilises. From 817f5e3609ac3a51718e485f5e506b3e77f3d745 Mon Sep 17 00:00:00 2001 From: Matthew John Cheetham Date: Tue, 31 Mar 2026 12:02:08 +0100 Subject: [PATCH 05/16] diagnose: fix network diag to await HTTP requests The network diagnostic failed to correctly await the test HTTP requests, and also did not return and await a `Task` (to capture exceptions). Signed-off-by: Matthew John Cheetham --- .../Core.Tests/Commands/DiagnoseCommandTests.cs | 13 +++++++------ src/shared/Core/Diagnostics/NetworkingDiagnostic.cs | 4 ++-- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/shared/Core.Tests/Commands/DiagnoseCommandTests.cs b/src/shared/Core.Tests/Commands/DiagnoseCommandTests.cs index 0118e9d85..42f5cedc7 100644 --- a/src/shared/Core.Tests/Commands/DiagnoseCommandTests.cs +++ b/src/shared/Core.Tests/Commands/DiagnoseCommandTests.cs @@ -2,6 +2,7 @@ using System.Net.Http; using System.Security.AccessControl; using System.Text; +using System.Threading.Tasks; using GitCredentialManager.Diagnostics; using GitCredentialManager.Tests.Objects; using Xunit; @@ -11,7 +12,7 @@ namespace Core.Tests.Commands; public class DiagnoseCommandTests { [Fact] - public void NetworkingDiagnostic_SendHttpRequest_Primary_OK() + public async Task NetworkingDiagnostic_SendHttpRequest_Primary_OK() { var primaryUriString = "http://example.com"; var sb = new StringBuilder(); @@ -24,14 +25,14 @@ public void NetworkingDiagnostic_SendHttpRequest_Primary_OK() httpHandler.Setup(HttpMethod.Head, primaryUri, httpResponse); - networkingDiagnostic.SendHttpRequest(sb, new HttpClient(httpHandler)); + await networkingDiagnostic.SendHttpRequestAsync(sb, new HttpClient(httpHandler)); httpHandler.AssertRequest(HttpMethod.Head, primaryUri, expectedNumberOfCalls: 1); Assert.Contains(expected, sb.ToString()); } [Fact] - public void NetworkingDiagnostic_SendHttpRequest_Backup_OK() + public async Task NetworkingDiagnostic_SendHttpRequest_Backup_OK() { var primaryUriString = "http://example.com"; var backupUriString = "http://httpforever.com"; @@ -48,7 +49,7 @@ public void NetworkingDiagnostic_SendHttpRequest_Backup_OK() httpHandler.Setup(HttpMethod.Head, primaryUri, httpResponse); httpHandler.Setup(HttpMethod.Head, backupUri, httpResponse); - networkingDiagnostic.SendHttpRequest(sb, new HttpClient(httpHandler)); + await networkingDiagnostic.SendHttpRequestAsync(sb, new HttpClient(httpHandler)); httpHandler.AssertRequest(HttpMethod.Head, primaryUri, expectedNumberOfCalls: 1); httpHandler.AssertRequest(HttpMethod.Head, backupUri, expectedNumberOfCalls: 1); @@ -56,7 +57,7 @@ public void NetworkingDiagnostic_SendHttpRequest_Backup_OK() } [Fact] - public void NetworkingDiagnostic_SendHttpRequest_No_Network() + public async Task NetworkingDiagnostic_SendHttpRequest_No_Network() { var primaryUriString = "http://example.com"; var backupUriString = "http://httpforever.com"; @@ -73,7 +74,7 @@ public void NetworkingDiagnostic_SendHttpRequest_No_Network() httpHandler.Setup(HttpMethod.Head, primaryUri, httpResponse); httpHandler.Setup(HttpMethod.Head, backupUri, httpResponse); - networkingDiagnostic.SendHttpRequest(sb, new HttpClient(httpHandler)); + await networkingDiagnostic.SendHttpRequestAsync(sb, new HttpClient(httpHandler)); httpHandler.AssertRequest(HttpMethod.Head, primaryUri, expectedNumberOfCalls: 1); httpHandler.AssertRequest(HttpMethod.Head, backupUri, expectedNumberOfCalls: 1); diff --git a/src/shared/Core/Diagnostics/NetworkingDiagnostic.cs b/src/shared/Core/Diagnostics/NetworkingDiagnostic.cs index 50ab5b4da..c49104ea8 100644 --- a/src/shared/Core/Diagnostics/NetworkingDiagnostic.cs +++ b/src/shared/Core/Diagnostics/NetworkingDiagnostic.cs @@ -29,7 +29,7 @@ protected override async Task RunInternalAsync(StringBuilder log, IList RunInternalAsync(StringBuilder log, IList { TestHttpUri, TestHttpUriFallback }) { From 774af8ea9ef83fbbee38e806c436425c412a5ae8 Mon Sep 17 00:00:00 2001 From: Matthew John Cheetham Date: Tue, 31 Mar 2026 12:03:24 +0100 Subject: [PATCH 06/16] macos: add die function to notarize.sh script Add the missing die function. Signed-off-by: Matthew John Cheetham --- src/osx/Installer.Mac/notarize.sh | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/osx/Installer.Mac/notarize.sh b/src/osx/Installer.Mac/notarize.sh index 9315d688a..f3aa55d00 100755 --- a/src/osx/Installer.Mac/notarize.sh +++ b/src/osx/Installer.Mac/notarize.sh @@ -1,4 +1,8 @@ #!/bin/bash +die () { + echo "$*" >&2 + exit 1 +} for i in "$@" do From 64cf7f208941e18ecc55bd33adf349d93d3c7aa4 Mon Sep 17 00:00:00 2001 From: Matthew John Cheetham Date: Tue, 31 Mar 2026 12:04:35 +0100 Subject: [PATCH 07/16] git: drain stderr on IsInsideRepository When suppressStreams is true, Git's stderr is redirected to a pipe but then we only read stdout before waiting for exit. If Ggit writes lots to stderr (for example when tracing is enabled), the stderr pipe can fill, causing the Git process to block and GCM to hang while checking repository state. Signed-off-by: Matthew John Cheetham --- src/shared/Core/Git.cs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/shared/Core/Git.cs b/src/shared/Core/Git.cs index 0c58e0159..e0ab82169 100644 --- a/src/shared/Core/Git.cs +++ b/src/shared/Core/Git.cs @@ -147,6 +147,13 @@ private string GetCurrentRepositoryInternal(bool suppressStreams) git.Start(Trace2ProcessClass.Git); string data = git.StandardOutput.ReadToEnd(); + + // Read all of standard error to prevent the process from hanging if it writes enough data to fill the buffer + if (suppressStreams) + { + git.StandardError.ReadToEnd(); + } + git.WaitForExit(); switch (git.ExitCode) From 4e29c3c5575b769bb39fa9a3742e5746565a3b35 Mon Sep 17 00:00:00 2001 From: Matthew John Cheetham Date: Tue, 31 Mar 2026 12:07:24 +0100 Subject: [PATCH 08/16] windows: fix layout.ps1 if symboloutput is not set If SymbolOutput is not set then there's a bug whereby we try and trim the end '/' and '\' characters on a null value. Guard against this. Signed-off-by: Matthew John Cheetham --- src/windows/Installer.Windows/layout.ps1 | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/windows/Installer.Windows/layout.ps1 b/src/windows/Installer.Windows/layout.ps1 index 3b1624896..53646764a 100644 --- a/src/windows/Installer.Windows/layout.ps1 +++ b/src/windows/Installer.Windows/layout.ps1 @@ -3,7 +3,10 @@ param ([Parameter(Mandatory)] $Configuration, [Parameter(Mandatory)] $Output, $R # Trim trailing slashes from output paths $Output = $Output.TrimEnd('\','/') -$SymbolOutput = $SymbolOutput.TrimEnd('\','/') + +if ($SymbolOutput) { + $SymbolOutput = $SymbolOutput.TrimEnd('\','/') +} Write-Output "Output: $Output" From f30679d05e5590bd73e10e864c56516b2befbaae Mon Sep 17 00:00:00 2001 From: Matthew John Cheetham Date: Tue, 31 Mar 2026 12:10:31 +0100 Subject: [PATCH 09/16] oauth: pass cancellation token to in-proc device code UI In the OAuth device-code flow, the in-proc UI path the calling logic creates a `CancellationTokenSource` and later calls `Cancel()` on that CTS to close the dialog once the token is obtained (or the user cancels). However, `ShowDeviceCodeViaUiAsync` disregards the provided cancellation token and passes `CancellationToken.None` into `AvaloniaUi.ShowViewAsync`. Pass the cancellation token correctly. Signed-off-by: Matthew John Cheetham --- src/shared/Core/Authentication/OAuthAuthentication.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/shared/Core/Authentication/OAuthAuthentication.cs b/src/shared/Core/Authentication/OAuthAuthentication.cs index a8de4ecb6..792ba40ec 100644 --- a/src/shared/Core/Authentication/OAuthAuthentication.cs +++ b/src/shared/Core/Authentication/OAuthAuthentication.cs @@ -247,7 +247,7 @@ private Task ShowDeviceCodeViaUiAsync(OAuth2DeviceCodeResult dcr, CancellationTo VerificationUrl = dcr.VerificationUri.ToString(), }; - return AvaloniaUi.ShowViewAsync(viewModel, GetParentWindowHandle(), CancellationToken.None); + return AvaloniaUi.ShowViewAsync(viewModel, GetParentWindowHandle(), ct); } private Task ShowDeviceCodeViaHelperAsync( From 0711d37ed040433b94c481b9966fa082dd74a442 Mon Sep 17 00:00:00 2001 From: Matthew John Cheetham Date: Tue, 31 Mar 2026 12:19:24 +0100 Subject: [PATCH 10/16] trace2: fix main thread identification The `Thread::ManagedTheadId` always starts with the entry thread as `1` and not `0`. https://github.com/dotnet/runtime/blob/790e8a525a0f76b8ad755c12e95b7f8770195d67/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/ManagedThreadId.cs#L181 Fix this in Trace2. Signed-off-by: Matthew John Cheetham --- src/shared/Core/Trace2.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/shared/Core/Trace2.cs b/src/shared/Core/Trace2.cs index 535812ea8..e7f048ca2 100644 --- a/src/shared/Core/Trace2.cs +++ b/src/shared/Core/Trace2.cs @@ -640,7 +640,7 @@ private void WriteMessage(Trace2Message message) private static string BuildThreadName() { // If this is the entry thread, call it "main", per Trace2 convention - if (Thread.CurrentThread.ManagedThreadId == 0) + if (Thread.CurrentThread.ManagedThreadId == 1) { return "main"; } From 26bbd326ece2c379aaffeae70d033a1bdf2ac036 Mon Sep 17 00:00:00 2001 From: Matthew John Cheetham Date: Tue, 31 Mar 2026 12:30:23 +0100 Subject: [PATCH 11/16] trace2: fix crash in perf format for large elapsed times BuildTimeSpan assumes an 11-character span and adjusts padding when the formatted elapsed time overflows. For values with 5+ digits before the decimal (>= 10000 seconds), the size difference exceeds the available padding budget, driving BeginPadding below zero. This causes `new string(' ', BeginPadding)` to throw an ArgumentOutOfRangeException. Since Trace2FileWriter does not catch exceptions, this crashes the credential helper when TRACE2 performance output is enabled. Fix the overflow check from `==` to `>=` so that values exceeding the full span (data + padding) zero out all padding rather than producing a negative value. Signed-off-by: Matthew John Cheetham --- src/shared/Core.Tests/Trace2MessageTests.cs | 2 ++ src/shared/Core/Trace2Message.cs | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/shared/Core.Tests/Trace2MessageTests.cs b/src/shared/Core.Tests/Trace2MessageTests.cs index 7e29a641f..82c1249ca 100644 --- a/src/shared/Core.Tests/Trace2MessageTests.cs +++ b/src/shared/Core.Tests/Trace2MessageTests.cs @@ -12,6 +12,8 @@ public class Trace2MessageTests [InlineData(26.316083, " 26.316083 ")] [InlineData(100.316083, "100.316083 ")] [InlineData(1000.316083, "1000.316083")] + [InlineData(10000.316083, "10000.316083")] + [InlineData(100000.31608, "100000.316080")] public void BuildTimeSpan_Match_Returns_Expected_String(double input, string expected) { var actual = Trace2Message.BuildTimeSpan(input); diff --git a/src/shared/Core/Trace2Message.cs b/src/shared/Core/Trace2Message.cs index 14327031f..78eb05a20 100644 --- a/src/shared/Core/Trace2Message.cs +++ b/src/shared/Core/Trace2Message.cs @@ -151,7 +151,7 @@ private static string BuildSpan(PerformanceFormatSpan component, string data) if (double.TryParse(data, out _)) { // Remove all padding for values that take up the entire span - if (Math.Abs(sizeDifference) == paddingTotal) + if (Math.Abs(sizeDifference) >= paddingTotal) { component.BeginPadding = 0; component.EndPadding = 0; From 33cf5e8b9356e98e504c3e8e84acba3c02607a6b Mon Sep 17 00:00:00 2001 From: Matthew John Cheetham Date: Tue, 31 Mar 2026 12:33:26 +0100 Subject: [PATCH 12/16] http: use correct http.sslAutoClientCert setting name We had been incorrectly looking for the `sslAutoClientCert` Git config option under the `credential` section, rather than `http`. Note: this is a Git for Windows only option. Signed-off-by: Matthew John Cheetham --- src/shared/Core/Settings.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/shared/Core/Settings.cs b/src/shared/Core/Settings.cs index af3dcf99c..480db7ea5 100644 --- a/src/shared/Core/Settings.cs +++ b/src/shared/Core/Settings.cs @@ -659,7 +659,7 @@ public bool IsCertificateVerificationEnabled } public bool AutomaticallyUseClientCertificates => - TryGetSetting(null, KnownGitCfg.Credential.SectionName, KnownGitCfg.Http.SslAutoClientCert, out string value) && value.ToBooleanyOrDefault(false); + TryGetSetting(null, KnownGitCfg.Http.SectionName, KnownGitCfg.Http.SslAutoClientCert, out string value) && value.ToBooleanyOrDefault(false); public string CustomCertificateBundlePath => TryGetPathSetting(KnownEnvars.GitSslCaInfo, KnownGitCfg.Http.SectionName, KnownGitCfg.Http.SslCaInfo, out string value) ? value : null; From 4d6cf6e79cc714b2539e97d4e54e6b12ba38343c Mon Sep 17 00:00:00 2001 From: Matthew John Cheetham Date: Tue, 31 Mar 2026 12:39:15 +0100 Subject: [PATCH 13/16] trace2: fix incomplete disposal of writers on cleanup ReleaseManagedResources iterates forward by index while removing elements from the same list. Each removal shifts remaining elements left, but the loop increments i, causing the next element to be skipped. As a result, only about half of the writers are disposed and removed, leaving file handles or buffers open. Fix by iterating in reverse so that removals do not shift any unvisited indices, and use RemoveAt(i) to avoid a redundant linear search. Signed-off-by: Matthew John Cheetham --- src/shared/Core/Trace2.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/shared/Core/Trace2.cs b/src/shared/Core/Trace2.cs index e7f048ca2..de6ca5822 100644 --- a/src/shared/Core/Trace2.cs +++ b/src/shared/Core/Trace2.cs @@ -460,11 +460,11 @@ protected override void ReleaseManagedResources() { try { - for (int i = 0; i < _writers.Count; i += 1) + for (int i = _writers.Count - 1; i >= 0; i--) { - using (var writer = _writers[i]) + using (_writers[i]) { - _writers.Remove(writer); + _writers.RemoveAt(i); } } } From e1354306bc9014cfcc70d5907c325faaa5e904a6 Mon Sep 17 00:00:00 2001 From: Matthew John Cheetham Date: Tue, 31 Mar 2026 13:32:27 +0100 Subject: [PATCH 14/16] git: fix crash when reading stderr from non-redirected processes ProcessManager.CreateProcess sets RedirectStandardError=false for all processes to avoid TRACE2 deadlocks. However, GetRemotes and CreateGitException unconditionally read StandardError, which throws InvalidOperationException when stderr is not redirected. Fix GetRemotes by explicitly redirecting stderr before starting the process, since it needs to check for 'not a git repository' errors. Guard CreateGitException defensively, as it is called from various contexts where stderr may or may not be redirected. Signed-off-by: Matthew John Cheetham --- src/shared/Core/Git.cs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/shared/Core/Git.cs b/src/shared/Core/Git.cs index e0ab82169..5f63f9918 100644 --- a/src/shared/Core/Git.cs +++ b/src/shared/Core/Git.cs @@ -174,6 +174,8 @@ public IEnumerable GetRemotes() { using (var git = CreateProcess("remote -v show")) { + // Redirect stderr so we can check for 'not a git repository' errors + git.StartInfo.RedirectStandardError = true; git.Start(Trace2ProcessClass.Git); // To avoid deadlocks, always read the output stream first and then wait // TODO: don't read in all the data at once; stream it @@ -274,7 +276,9 @@ public async Task> InvokeHelperAsync(string args, ID public static GitException CreateGitException(ChildProcess git, string message, ITrace2 trace2 = null) { - var gitMessage = git.StandardError.ReadToEnd(); + var gitMessage = git.StartInfo.RedirectStandardError + ? git.StandardError.ReadToEnd() + : null; if (trace2 != null) throw new Trace2GitException(trace2, message, git.ExitCode, gitMessage); From 6d85670e035b275b148104c17632b6e351fde153 Mon Sep 17 00:00:00 2001 From: Matthew John Cheetham Date: Tue, 31 Mar 2026 14:01:02 +0100 Subject: [PATCH 15/16] environment: check execute permission in TryLocateExecutable In f1a1ae5 (environment: manually scan $PATH on POSIX systems, 2022-05-31) the `which`-based lookup was replaced with a manual PATH scan that only checks FileExists, without verifying execute permissions. Unlike `which`, this means a non-executable file earlier in PATH can shadow a valid executable, causing process creation to fail when GCM later tries to run the located path. Add an IsExecutable check that verifies at least one execute bit is set on POSIX systems, matching the behaviour of `which`. On Windows, any existing file is considered executable. Guard the POSIX-specific File.GetUnixFileMode call with #if !NETFRAMEWORK for net472 compatibility. Signed-off-by: Matthew John Cheetham --- src/shared/Core.Tests/EnvironmentTests.cs | 29 +++++++++++++++++++ src/shared/Core/EnvironmentBase.cs | 3 +- src/shared/Core/FileSystem.cs | 25 ++++++++++++++++ .../Objects/TestFileSystem.cs | 27 +++++++++++++++++ 4 files changed, 83 insertions(+), 1 deletion(-) diff --git a/src/shared/Core.Tests/EnvironmentTests.cs b/src/shared/Core.Tests/EnvironmentTests.cs index d9b7cb67c..40c685c2a 100644 --- a/src/shared/Core.Tests/EnvironmentTests.cs +++ b/src/shared/Core.Tests/EnvironmentTests.cs @@ -94,6 +94,7 @@ public void PosixEnvironment_TryLocateExecutable_Exists_ReturnTrueAndPath() [expectedPath] = Array.Empty(), } }; + fs.SetExecutable(expectedPath); var envars = new Dictionary {["PATH"] = PosixPathVar}; var env = new PosixEnvironment(fs, envars); @@ -116,6 +117,32 @@ public void PosixEnvironment_TryLocateExecutable_ExistsMultiple_ReturnTrueAndFir ["/bin/foo"] = Array.Empty(), } }; + fs.SetExecutable(expectedPath); + fs.SetExecutable("/usr/local/bin/foo"); + fs.SetExecutable("/bin/foo"); + var envars = new Dictionary {["PATH"] = PosixPathVar}; + var env = new PosixEnvironment(fs, envars); + + bool actualResult = env.TryLocateExecutable(PosixExecName, out string actualPath); + + Assert.True(actualResult); + Assert.Equal(expectedPath, actualPath); + } + + [PosixFact] + public void PosixEnvironment_TryLocateExecutable_NotExecutable_SkipsToNextMatch() + { + string nonExecPath = "/home/john.doe/bin/foo"; + string expectedPath = "/usr/local/bin/foo"; + var fs = new TestFileSystem + { + Files = new Dictionary + { + [nonExecPath] = Array.Empty(), + [expectedPath] = Array.Empty(), + } + }; + fs.SetExecutable(expectedPath); var envars = new Dictionary {["PATH"] = PosixPathVar}; var env = new PosixEnvironment(fs, envars); @@ -142,6 +169,8 @@ public void MacOSEnvironment_TryLocateExecutable_Paths_Are_Ignored() [expectedPath] = Array.Empty(), } }; + fs.SetExecutable(pathsToIgnore.FirstOrDefault()); + fs.SetExecutable(expectedPath); var envars = new Dictionary {["PATH"] = PosixPathVar}; var env = new PosixEnvironment(fs, envars); diff --git a/src/shared/Core/EnvironmentBase.cs b/src/shared/Core/EnvironmentBase.cs index 6a3967193..39ed9dd03 100644 --- a/src/shared/Core/EnvironmentBase.cs +++ b/src/shared/Core/EnvironmentBase.cs @@ -138,7 +138,8 @@ internal virtual bool TryLocateExecutable(string program, ICollection pa { string candidatePath = Path.Combine(basePath, program); if (FileSystem.FileExists(candidatePath) && (pathsToIgnore is null || - !pathsToIgnore.Contains(candidatePath, StringComparer.OrdinalIgnoreCase))) + !pathsToIgnore.Contains(candidatePath, StringComparer.OrdinalIgnoreCase)) + && FileSystem.FileIsExecutable(candidatePath)) { path = candidatePath; return true; diff --git a/src/shared/Core/FileSystem.cs b/src/shared/Core/FileSystem.cs index aeacfd51d..c23f0faa1 100644 --- a/src/shared/Core/FileSystem.cs +++ b/src/shared/Core/FileSystem.cs @@ -34,6 +34,14 @@ public interface IFileSystem /// True if a file exists, false otherwise. bool FileExists(string path); + /// + /// Check if a file has execute permissions. + /// On Windows this always returns true. On POSIX it checks for any execute bit. + /// + /// Full path to file to test. + /// True if the file is executable, false otherwise. + bool FileIsExecutable(string path); + /// /// Check if a directory exists at the specified path. /// @@ -122,6 +130,23 @@ public abstract class FileSystem : IFileSystem public bool FileExists(string path) => File.Exists(path); +#if NETFRAMEWORK + public bool FileIsExecutable(string path) => true; +#else + public bool FileIsExecutable(string path) + { + if (!PlatformUtils.IsPosix()) + return true; + +#pragma warning disable CA1416 // Platform guard via PlatformUtils.IsPosix() + var mode = File.GetUnixFileMode(path); + return (mode & (UnixFileMode.UserExecute | + UnixFileMode.GroupExecute | + UnixFileMode.OtherExecute)) != 0; +#pragma warning restore CA1416 + } +#endif + public bool DirectoryExists(string path) => Directory.Exists(path); public string GetCurrentDirectory() => Directory.GetCurrentDirectory(); diff --git a/src/shared/TestInfrastructure/Objects/TestFileSystem.cs b/src/shared/TestInfrastructure/Objects/TestFileSystem.cs index 11dff8f1f..57a75f2b8 100644 --- a/src/shared/TestInfrastructure/Objects/TestFileSystem.cs +++ b/src/shared/TestInfrastructure/Objects/TestFileSystem.cs @@ -11,6 +11,7 @@ public class TestFileSystem : IFileSystem public string UserHomePath { get; set; } public string UserDataDirectoryPath { get; set; } public IDictionary Files { get; set; } = new Dictionary(); + public ISet ExecutableFiles { get; } = new HashSet(); public ISet Directories { get; set; } = new HashSet(); public string CurrentDirectory { get; set; } = Path.GetTempPath(); public bool IsCaseSensitive { get; set; } = false; @@ -36,6 +37,18 @@ bool IFileSystem.FileExists(string path) return Files.ContainsKey(path); } + bool IFileSystem.FileIsExecutable(string path) + { + if (!Files.ContainsKey(path)) + throw new FileNotFoundException("File not found", path); + + // On Windows, all files are considered executable. + if (!PlatformUtils.IsPosix()) + return true; + + return ExecutableFiles.Contains(path); + } + bool IFileSystem.DirectoryExists(string path) { return Directories.Contains(TrimSlash(path)); @@ -130,6 +143,20 @@ string[] IFileSystem.ReadAllLines(string path) #endregion + /// + /// Mark a test file as executable. File must exist in already. + /// + public void SetExecutable(string path, bool isExecutable = true) + { + if (!Files.ContainsKey(path)) + throw new FileNotFoundException("File not found", path); + + if (isExecutable) + ExecutableFiles.Add(path); + else + ExecutableFiles.Remove(path); + } + /// /// Trim trailing slashes from a path. /// From e437949d6f50c7acc143de64482d2ab3b8598e00 Mon Sep 17 00:00:00 2001 From: Matthew John Cheetham Date: Tue, 31 Mar 2026 14:04:58 +0100 Subject: [PATCH 16/16] windows: fix en-dash characters in installer Exec command The powershell.exe invocation in the Installer.Windows.csproj Exec task uses Unicode en dash characters (U+2013) instead of ASCII hyphens for the -NonInteractive and -ExecutionPolicy parameters. Windows PowerShell 5.1 does not recognise en dashes as parameter prefixes, so these flags are not applied correctly, which can cause the layout step to fail or run with an unexpected execution policy. Replace the en dash characters with ASCII hyphens. Signed-off-by: Matthew John Cheetham --- src/windows/Installer.Windows/Installer.Windows.csproj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/windows/Installer.Windows/Installer.Windows.csproj b/src/windows/Installer.Windows/Installer.Windows.csproj index ec678fe5f..36f5cd5f7 100644 --- a/src/windows/Installer.Windows/Installer.Windows.csproj +++ b/src/windows/Installer.Windows/Installer.Windows.csproj @@ -44,7 +44,7 @@