From 601b7ad3384747278f39f202f834abaf01764c3b Mon Sep 17 00:00:00 2001 From: Matthew John Cheetham Date: Mon, 5 Oct 2026 16:49:20 +0100 Subject: [PATCH 1/2] auth: share visible console parenting Console parenting currently lives inside the MSAL adapter, so other in-process authentication dialogs do not benefit from the visible terminal owner. Those prompts can appear disconnected from the Git operation that opened them. Resolve the parent at the shared authentication boundary so all in-process prompts follow the same policy. Explicit handles retain precedence, hidden console owners are ignored, and minimised owners remain eligible. Keep progress-window creation in the MSAL adapter: it is a fallback for broker flows that require a parent, not a default owner for ordinary authentication dialogs. Assisted-by: Claude Sonnet 5 Assisted-by: GPT-6.1 Sol Signed-off-by: Matthew John Cheetham --- docs/environment.md | 6 ++ .../Entra/MsalParentWindowAdapterTests.cs | 95 ++----------------- src/Core/Authentication/AuthenticationBase.cs | 9 +- src/Core/Authentication/Entra/MsalAdaptors.cs | 40 +------- src/Core/PlatformUtils.cs | 31 ++++++ 5 files changed, 54 insertions(+), 127 deletions(-) diff --git a/docs/environment.md b/docs/environment.md index 888e13adb..c6d258f52 100644 --- a/docs/environment.md +++ b/docs/environment.md @@ -177,6 +177,12 @@ Defaults to enabled. Specify the parent window handle (`HWND`) for modal dialogs on Windows. +When this variable is unset, GCM tries to parent authentication dialogs to the +console's visible root-owner window. Hidden or invalid windows are ignored. + +Explicitly setting a handle using this variable takes precedence and it is not +checked for visibility. + > [!NOTE] > This is intended for use by application developers who are calling Git and/or > GCM from their application and wish to correctly parent authentication dialogs diff --git a/src/Core.Tests/Authentication/Entra/MsalParentWindowAdapterTests.cs b/src/Core.Tests/Authentication/Entra/MsalParentWindowAdapterTests.cs index 831ae489f..b60a70558 100644 --- a/src/Core.Tests/Authentication/Entra/MsalParentWindowAdapterTests.cs +++ b/src/Core.Tests/Authentication/Entra/MsalParentWindowAdapterTests.cs @@ -10,60 +10,22 @@ public class MsalParentWindowAdapterTests [Theory] [InlineData(42, false)] [InlineData(42, true)] - [InlineData(-1, false)] - [InlineData(-1, true)] - public void GetWindow_ExplicitParent_TakesPrecedence(int parentValue, bool createIfMissing) + public void GetWindow_ExplicitParent_ReturnsParent(int parentValue, bool createIfMissing) { object parentWindow = new IntPtr(parentValue); using var adapter = new MsalParentWindowAdapter(parentWindow, createIfMissing, - () => throw new InvalidOperationException("Console lookup should not be called."), - _ => throw new InvalidOperationException("Visibility should not be checked."), _ => throw new InvalidOperationException("A progress window should not be created.")); object result = adapter.GetWindow(); Assert.Same(parentWindow, result); } - [Theory] - [InlineData(false)] - [InlineData(true)] - public void GetWindow_VisibleConsoleRoot_PrecedesProgressWindow(bool createIfMissing) - { - var consoleRoot = new IntPtr(42); - bool visibilityChecked = false; - using var adapter = new MsalParentWindowAdapter(IntPtr.Zero, createIfMissing, - () => consoleRoot, - hwnd => - { - Assert.Equal(consoleRoot, hwnd); - visibilityChecked = true; - return true; - }, - _ => throw new InvalidOperationException("A progress window should not be created.")); - - IntPtr result = Assert.IsType(adapter.GetWindow()); - - Assert.Equal(consoleRoot, result); - Assert.True(visibilityChecked); - } - - [Theory] - [InlineData(0)] - [InlineData(42)] - public void GetWindow_NoVisibleConsoleRoot_CreatesProgressWhenRequired(int consoleRootValue) + [Fact] + public void GetWindow_NoParent_Required_CreatesProgressWindow() { - var consoleRoot = new IntPtr(consoleRootValue); - var progressWindow = new IntPtr(84); - int visibilityChecks = 0; + var progressWindow = new IntPtr(42); var windowToken = CancellationToken.None; using (var adapter = new MsalParentWindowAdapter(IntPtr.Zero, true, - () => consoleRoot, - hwnd => - { - Assert.Equal(consoleRoot, hwnd); - visibilityChecks++; - return false; - }, ct => { windowToken = ct; @@ -73,7 +35,6 @@ public void GetWindow_NoVisibleConsoleRoot_CreatesProgressWhenRequired(int conso IntPtr result = Assert.IsType(adapter.GetWindow()); Assert.Equal(progressWindow, result); - Assert.Equal(consoleRootValue == 0 ? 0 : 1, visibilityChecks); Assert.True(windowToken.CanBeCanceled); Assert.False(windowToken.IsCancellationRequested); } @@ -81,58 +42,14 @@ public void GetWindow_NoVisibleConsoleRoot_CreatesProgressWhenRequired(int conso Assert.True(windowToken.IsCancellationRequested); } - [Theory] - [InlineData(0)] - [InlineData(42)] - public void GetWindow_NoVisibleConsoleRoot_ReturnsNullWhenParentNotRequired(int consoleRootValue) - { - var consoleRoot = new IntPtr(consoleRootValue); - int visibilityChecks = 0; - using var adapter = new MsalParentWindowAdapter(IntPtr.Zero, false, - () => consoleRoot, - hwnd => - { - Assert.Equal(consoleRoot, hwnd); - visibilityChecks++; - return false; - }, - _ => throw new InvalidOperationException("A progress window should not be created.")); - - object result = adapter.GetWindow(); - - Assert.Null(result); - Assert.Equal(consoleRootValue == 0 ? 0 : 1, visibilityChecks); - } - [Fact] - public void GetWindow_VisibilityCheckThrows_PropagatesException() + public void GetWindow_NoParent_NotRequired_ReturnsNull() { - var exception = new InvalidOperationException("Visibility check failed."); - using var adapter = new MsalParentWindowAdapter(IntPtr.Zero, true, - () => new IntPtr(42), - _ => throw exception, + using var adapter = new MsalParentWindowAdapter(IntPtr.Zero, false, _ => throw new InvalidOperationException("A progress window should not be created.")); - var thrown = Assert.Throws(adapter.GetWindow); - Assert.Same(exception, thrown); - } - - [Fact] - public void Create_ExplicitParent_IsReturned() - { - object parentWindow = new IntPtr(42); - using var adapter = MsalParentWindowAdapter.Create(parentWindow, true); - object result = adapter.GetWindow(); - Assert.Same(parentWindow, result); - } - - [PosixFact] - public void Create_NoExplicitParent_Posix_ReturnsNull() - { - using var adapter = MsalParentWindowAdapter.Create(IntPtr.Zero); - object result = adapter.GetWindow(); Assert.Null(result); } } diff --git a/src/Core/Authentication/AuthenticationBase.cs b/src/Core/Authentication/AuthenticationBase.cs index fb177d112..5bfc1a9d0 100644 --- a/src/Core/Authentication/AuthenticationBase.cs +++ b/src/Core/Authentication/AuthenticationBase.cs @@ -5,7 +5,6 @@ using System.Linq; using System.Threading; using System.Threading.Tasks; -using GitCredentialManager.Tty; using GitCredentialManager.UI.ViewModels; namespace GitCredentialManager.Authentication @@ -125,11 +124,19 @@ protected void ThrowIfWindowCancelled(WindowViewModel viewModel) protected IntPtr GetParentWindowHandle() { + // Check if the user has specified a parent window ID if (int.TryParse(Context.Settings.ParentWindowId, out int id)) { return new IntPtr(id); } + // Check if we can use the console window as a parent + IntPtr consoleParent = PlatformUtils.GetConsoleParentWindow(); + if (consoleParent != IntPtr.Zero) + { + return consoleParent; + } + return IntPtr.Zero; } diff --git a/src/Core/Authentication/Entra/MsalAdaptors.cs b/src/Core/Authentication/Entra/MsalAdaptors.cs index e169ee838..2f335649c 100644 --- a/src/Core/Authentication/Entra/MsalAdaptors.cs +++ b/src/Core/Authentication/Entra/MsalAdaptors.cs @@ -1,7 +1,6 @@ using System; using System.Net.Http; using System.Threading; -using GitCredentialManager.Interop.Windows.Native; using GitCredentialManager.UI.Controls; using Microsoft.Identity.Client; @@ -31,48 +30,32 @@ internal class MsalParentWindowAdapter : IDisposable private readonly bool _createIfMissing; private readonly CancellationTokenSource _cts = new(); - private readonly Func _getConsoleParentWindow; - private readonly Func _isWindowVisible; private readonly Func _createWindow; public static MsalParentWindowAdapter Create(object parentWindow, bool createIfMissing = false) { - return new MsalParentWindowAdapter(parentWindow, createIfMissing, - GetConsoleParentWindow, User32.IsWindowVisible, ProgressWindow.ShowAndGetHandle); + return new MsalParentWindowAdapter(parentWindow, createIfMissing, ProgressWindow.ShowAndGetHandle); } internal MsalParentWindowAdapter(object parentWindow, bool createIfMissing, - Func getConsoleParentWindow, Func isWindowVisible, Func createWindow) { - EnsureArgument.NotNull(getConsoleParentWindow, nameof(getConsoleParentWindow)); - EnsureArgument.NotNull(isWindowVisible, nameof(isWindowVisible)); EnsureArgument.NotNull(createWindow, nameof(createWindow)); _parentWindow = parentWindow; _createIfMissing = createIfMissing; - - _getConsoleParentWindow = getConsoleParentWindow; - _isWindowVisible = isWindowVisible; _createWindow = createWindow; } public object GetWindow() { + // Try to use the parent window we were given (if any) if (_parentWindow is IntPtr p && p != IntPtr.Zero) { return _parentWindow; } - // See if we can use the console window as a parent. - // We only consider the window if it is valid and visible. - IntPtr consoleParent = _getConsoleParentWindow(); - if (consoleParent != IntPtr.Zero && _isWindowVisible(consoleParent)) - { - return consoleParent; - } - - // Create a stub window to use as a parent + // Create a stub window to use as a parent instead (if the caller requires it) if (_createIfMissing) { return _createWindow(_cts.Token); @@ -81,23 +64,6 @@ public object GetWindow() return null; } - private static IntPtr GetConsoleParentWindow() - { - // On Windows we can try and get the console window parent handle if that exists - if (!PlatformUtils.IsWindows()) - { - return IntPtr.Zero; - } - - IntPtr consoleHandle = Kernel32.GetConsoleWindow(); - - // When the parent is using ConPTY (pseudo-terminals) the console - // window may be a fake/stub (as is the case with Windows Terminal). - // This means we need to walk up the ancestor chain to get the actual - // root owner (typically the terminal emulator's window). - return User32.GetAncestor(consoleHandle, GetAncestorFlags.GetRootOwner); - } - public void Dispose() { // Close and clean up any stub window we may have created diff --git a/src/Core/PlatformUtils.cs b/src/Core/PlatformUtils.cs index cba2c4e3c..6f8dc38e9 100644 --- a/src/Core/PlatformUtils.cs +++ b/src/Core/PlatformUtils.cs @@ -5,6 +5,7 @@ using System.Runtime.InteropServices; using System.Runtime.Versioning; using GitCredentialManager.Interop.Posix.Native; +using GitCredentialManager.Interop.Windows.Native; namespace GitCredentialManager { @@ -211,6 +212,36 @@ public static bool IsElevatedUser() return false; } + public static IntPtr GetConsoleParentWindow() + { + // On Windows we can try and get the console window parent handle if that exists + if (!IsWindows()) + { + return IntPtr.Zero; + } + + IntPtr consoleHandle = Kernel32.GetConsoleWindow(); + + // When the parent is using ConPTY (pseudo-terminals) the console + // window may be a fake/stub (as is the case with Windows Terminal). + // This means we need to walk up the ancestor chain to get the actual + // root owner (typically the terminal emulator's window). + IntPtr parent = User32.GetAncestor(consoleHandle, GetAncestorFlags.GetRootOwner); + + // On Windows the parent window may be hidden (not visible) if the console is + // running in a background process (like a service). + // We do not consider a hidden window as a valid parent. We do however consider + // a minimised window as a valid parent (and the user is able to restore it and + // see our child window too). + if (User32.IsWindowVisible(parent)) + { + return parent; + } + + // No visible console window available! + return IntPtr.Zero; + } + #region Platform Entry Path Utils /// From a3f1cea44e7073e0dabcac4c7264b4a8d5e9a99a Mon Sep 17 00:00:00 2001 From: Matthew John Cheetham Date: Mon, 5 Oct 2026 16:50:03 +0100 Subject: [PATCH 2/2] ui: use console fallback for helper parents Helper commands resolve their parent handle independently of the in-process authentication path. Keeping only the explicit-HWND case would leave their default parent-selection policy inconsistent with the shared authentication lookup. Use the same visible-console fallback when no handle is configured. Explicit handles still take precedence, and a missing or hidden console leaves the parent unset. Assisted-by: GPT-6.1 Sol Signed-off-by: Matthew John Cheetham --- src/Core/UI/HelperCommand.cs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/Core/UI/HelperCommand.cs b/src/Core/UI/HelperCommand.cs index f75096ddb..49492bf1f 100644 --- a/src/Core/UI/HelperCommand.cs +++ b/src/Core/UI/HelperCommand.cs @@ -16,11 +16,19 @@ public HelperCommand(ICommandContext context, string name, string description) protected IntPtr GetParentHandle() { + // Check if the user has specified a parent window ID if (int.TryParse(Context.Settings.ParentWindowId, out int id)) { return new IntPtr(id); } + // Check if we can use the console window as a parent + IntPtr consoleParent = PlatformUtils.GetConsoleParentWindow(); + if (consoleParent != IntPtr.Zero) + { + return consoleParent; + } + return IntPtr.Zero; }