Fix several small bugs that an AI agent detected (#2303)

A collection of small bug fixes found with the help of an AI agent
scanning the codebase. None of these are known to be actively exploited
or causing user-reported issues, but each is a latent defect that could
cause crashes, hangs, or incorrect behaviour under the right conditions.

## Fixes

**Credential UI** — `github/gitlab: use correct param order`
- Custom credential UI commands had swapped URL and username arguments.

**Stream extensions** — `streamextensions: fix a bug in multi-var reset
handling`
- Multi-dictionary writer emitted the wrong value when a reset marker
was followed by a single entry.

**GitHub auth** — `github: handle empty domain or enterprise hints`
- Null-reference exception when WWW-Authenticate header is missing
domain/enterprise hints.

**GitHub account filtering** — `github: do not filter accounts outside
of dotcom`
- Account filtering was applied outside of GitHub.com despite detecting
it shouldn't be.

**Network diagnostics** — `diagnose: fix network diag to await HTTP
requests`
- HTTP requests in the network diagnostic were not properly awaited.

**macOS notarize script** — `macos: add die function to notarize.sh
script`
- Missing `die` function.

**Git stderr handling** — `git: drain stderr on IsInsideRepository`
- Potential hang when Git writes enough to stderr to fill the pipe
buffer.

**Windows layout script** — `windows: fix layout.ps1 if symboloutput is
not set`
- Null-reference trimming `SymbolOutput` when the variable is unset.

**OAuth device code UI** — `oauth: pass cancellation token to in-proc
device code UI`
- Cancellation token was not forwarded, so dismissing the dialog didn't
work.

**TRACE2 thread ID** — `trace2: fix main thread identification`
- Main thread ID starts at 1, not 0.

**TRACE2 perf format** — `trace2: fix crash in perf format for large
elapsed times`
- `ArgumentOutOfRangeException` crash when elapsed time exceeds 9999
seconds.

**HTTP config** — `http: use correct http.sslAutoClientCert setting
name`
- Setting was read from the wrong Git config section.

**TRACE2 writer cleanup** — `trace2: fix incomplete disposal of writers
on cleanup`
- Forward iteration with removal skipped every other writer, leaking
file handles.

**Git stderr redirect** — `git: fix crash when reading stderr from
non-redirected processes`
- `InvalidOperationException` when `GetRemotes`/`CreateGitException`
read non-redirected stderr.

**Executable lookup** — `environment: check execute permission in
TryLocateExecutable`
- PATH scan didn't verify execute bits on POSIX, unlike the `which` it
replaced.

**Installer Exec command** — `windows: fix en-dash characters in
installer Exec command`
- Unicode en-dash characters instead of ASCII hyphens broke PowerShell
5.1 parameter parsing.

Author: mjcheetham

src/osx/Installer.Mac/notarize.sh

@@ -1,4 +1,8 @@
 #!/bin/bash
+die () {
+    echo "$*" >&2
+    exit 1
+}
 
 for i in "$@"
 do

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,15 +49,15 @@ 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);
         Assert.Contains(expected, sb.ToString());
     }
 
     [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);

src/shared/Core.Tests/EnvironmentTests.cs

@@ -94,6 +94,7 @@ public void PosixEnvironment_TryLocateExecutable_Exists_ReturnTrueAndPath()
                     [expectedPath] = Array.Empty<byte>(),
                 }
             };
+            fs.SetExecutable(expectedPath);
             var envars = new Dictionary<string, string> {["PATH"] = PosixPathVar};
             var env = new PosixEnvironment(fs, envars);
 
@@ -116,6 +117,32 @@ public void PosixEnvironment_TryLocateExecutable_ExistsMultiple_ReturnTrueAndFir
                     ["/bin/foo"] = Array.Empty<byte>(),
                 }
             };
+            fs.SetExecutable(expectedPath);
+            fs.SetExecutable("/usr/local/bin/foo");
+            fs.SetExecutable("/bin/foo");
+            var envars = new Dictionary<string, string> {["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<string, byte[]>
+                {
+                    [nonExecPath] = Array.Empty<byte>(),
+                    [expectedPath] = Array.Empty<byte>(),
+                }
+            };
+            fs.SetExecutable(expectedPath);
             var envars = new Dictionary<string, string> {["PATH"] = PosixPathVar};
             var env = new PosixEnvironment(fs, envars);
 
@@ -142,6 +169,8 @@ public void MacOSEnvironment_TryLocateExecutable_Paths_Are_Ignored()
                     [expectedPath] = Array.Empty<byte>(),
                 }
             };
+            fs.SetExecutable(pathsToIgnore.FirstOrDefault());
+            fs.SetExecutable(expectedPath);
             var envars = new Dictionary<string, string> {["PATH"] = PosixPathVar};
             var env = new PosixEnvironment(fs, envars);
 

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

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);

src/shared/Core/Authentication/OAuthAuthentication.cs

@@ -247,7 +247,7 @@ private Task ShowDeviceCodeViaUiAsync(OAuth2DeviceCodeResult dcr, CancellationTo
                 VerificationUrl = dcr.VerificationUri.ToString(),
             };
 
-            return AvaloniaUi.ShowViewAsync<DeviceCodeView>(viewModel, GetParentWindowHandle(), CancellationToken.None);
+            return AvaloniaUi.ShowViewAsync<DeviceCodeView>(viewModel, GetParentWindowHandle(), ct);
         }
 
         private Task ShowDeviceCodeViaHelperAsync(

src/shared/Core/Diagnostics/NetworkingDiagnostic.cs

@@ -29,7 +29,7 @@ protected override async Task<bool> RunInternalAsync(StringBuilder log, IList<st
             bool hasNetwork = NetworkInterface.GetIsNetworkAvailable();
             log.AppendLine($"IsNetworkAvailable: {hasNetwork}");
 
-            SendHttpRequest(log, httpClient);
+            await SendHttpRequestAsync(log, httpClient);
 
             log.Append($"Sending HEAD request to {TestHttpsUri}...");
             using var httpsResponse = await httpClient.HeadAsync(TestHttpsUri);
@@ -98,7 +98,7 @@ protected override async Task<bool> RunInternalAsync(StringBuilder log, IList<st
             return true;
         }
 
-        internal /* For testing purposes */ async void SendHttpRequest(StringBuilder log, HttpClient httpClient)
+        internal /* For testing purposes */ async Task SendHttpRequestAsync(StringBuilder log, HttpClient httpClient)
         {
             foreach (var uri in new List<string> { TestHttpUri, TestHttpUriFallback })
             {

src/shared/Core/EnvironmentBase.cs

@@ -138,7 +138,8 @@ internal virtual bool TryLocateExecutable(string program, ICollection<string> 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;

src/shared/Core/FileSystem.cs

@@ -34,6 +34,14 @@ public interface IFileSystem
         /// <returns>True if a file exists, false otherwise.</returns>
         bool FileExists(string path);
 
+        /// <summary>
+        /// Check if a file has execute permissions.
+        /// On Windows this always returns true. On POSIX it checks for any execute bit.
+        /// </summary>
+        /// <param name="path">Full path to file to test.</param>
+        /// <returns>True if the file is executable, false otherwise.</returns>
+        bool FileIsExecutable(string path);
+
         /// <summary>
         /// Check if a directory exists at the specified path.
         /// </summary>
@@ -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();

src/shared/Core/Git.cs

@@ -146,6 +146,15 @@ private string GetCurrentRepositoryInternal(bool suppressStreams)
                 }
 
                 git.Start(Trace2ProcessClass.Git);
+
+                // Drain and throw away stderr asynchronously to avoid a deadlock
+                // if the child process fills the stderr pipe buffer.
+                if (suppressStreams)
+                {
+                    git.Process.ErrorDataReceived += (_, _) => { };
+                    git.Process.BeginErrorReadLine();
+                }
+
                 string data = git.StandardOutput.ReadToEnd();
                 git.WaitForExit();
 
@@ -167,6 +176,8 @@ public IEnumerable<GitRemote> 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
@@ -267,7 +278,9 @@ public async Task<IDictionary<string, string>> 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);