From 97397a9836e6bd98a42aa3da3fd9081bb28cd883 Mon Sep 17 00:00:00 2001 From: Oliver Kieselbach Date: Thu, 11 Jun 2026 16:28:06 +0200 Subject: [PATCH] fix(agent-v2): resolve desktop owner via WTS session query, WMI as fallback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Session 4d5a0b78: WMI Win32_Process.GetOwner failed on every single DAD poll (wmiErrorCount == pollCount from the first poll on), so the explorer.exe owner was never resolved, desktop_arrived never fired, and the Desktop half of the completion AND-gate starved — the session sat in AwaitingDesktop forever even though the gate-starvation synthesis (d3abbacd) had correctly opened the AccountSetup gate. Owner resolution is now WTSQuerySessionInformation(WTSUserName/WTSDomainName) keyed by the process's session id as the primary path — a single kernel round-trip with no WinMgmt dependency, and semantically the right question anyway (explorer.exe runs as the session's logged-on user). The previous WMI GetOwner path remains as fallback when WTS yields no user. - New WtsNativeMethods interop (wtsapi32) with buffer-safe QuerySessionString - DesktopArrivalDetector.ResolveOwner: WTS first, WMI fallback, injectable test seams (SessionOwnerResolver/ProcessOwnerResolver) - first_poll/no_candidate liveness payloads now carry wtsErrorCount alongside wmiErrorCount so the broken path is visible per device V2.Core 1327/1327 green; WTS path live-verified (AzureAD\user shape matches the existing IsExcludedUser contract). Co-Authored-By: Claude Fable 5 --- .../Monitoring/DesktopArrivalDetectorTests.cs | 88 +++++++++++++++++++ .../SystemSignals/DesktopArrivalDetector.cs | 84 +++++++++++++++++- .../Monitoring/Interop/WtsNativeMethods.cs | 61 +++++++++++++ 3 files changed, 229 insertions(+), 4 deletions(-) create mode 100644 src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Interop/WtsNativeMethods.cs diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Monitoring/DesktopArrivalDetectorTests.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Monitoring/DesktopArrivalDetectorTests.cs index 1722b3aa..02d4f1e7 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Monitoring/DesktopArrivalDetectorTests.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Monitoring/DesktopArrivalDetectorTests.cs @@ -178,6 +178,94 @@ namespace AutopilotMonitor.Agent.V2.Core.Tests.Monitoring detector.ResetForRealUserSwitch(); } + // ============================================================================ + // Owner resolution: WTS primary, WMI fallback (session 4d5a0b78 fix, 2026-06-11) + // ============================================================================ + // Session 4d5a0b78: WMI Win32_Process.GetOwner failed on EVERY poll on a device + // (wmiErrorCount == pollCount), the owner was never resolved and desktop_arrived + // never fired — the Desktop half of the completion AND-gate starved. WTS session + // queries are now the primary path; WMI is only consulted when WTS yields no user. + + private static DesktopArrivalDetector BuildDetector(TempDirectory tmp) => + new DesktopArrivalDetector(new AgentLogger(tmp.Path, AgentLogLevel.Info)); + + [Fact] + public void ResolveOwner_uses_WTS_result_without_consulting_WMI() + { + using var tmp = new TempDirectory(); + using var detector = BuildDetector(tmp); + var wmiCalled = false; + detector.SessionOwnerResolver = sessionId => "CONTOSO\\alice"; + detector.ProcessOwnerResolver = pid => { wmiCalled = true; return "CONTOSO\\bob"; }; + + var owner = detector.ResolveOwner(processId: 1234, sessionId: 2); + + Assert.Equal("CONTOSO\\alice", owner); + Assert.False(wmiCalled); + } + + [Fact] + public void ResolveOwner_falls_back_to_WMI_when_WTS_returns_null() + { + using var tmp = new TempDirectory(); + using var detector = BuildDetector(tmp); + detector.SessionOwnerResolver = sessionId => null; + detector.ProcessOwnerResolver = pid => "CONTOSO\\alice"; + + Assert.Equal("CONTOSO\\alice", detector.ResolveOwner(1234, 2)); + } + + [Fact] + public void ResolveOwner_falls_back_to_WMI_when_WTS_returns_empty() + { + // WTSQuerySessionInformation can succeed with an empty user name (no user + // associated with the session) — must be treated like a miss, not a real owner. + using var tmp = new TempDirectory(); + using var detector = BuildDetector(tmp); + detector.SessionOwnerResolver = sessionId => ""; + detector.ProcessOwnerResolver = pid => "CONTOSO\\alice"; + + Assert.Equal("CONTOSO\\alice", detector.ResolveOwner(1234, 2)); + } + + [Fact] + public void ResolveOwner_falls_back_to_WMI_when_WTS_throws() + { + using var tmp = new TempDirectory(); + using var detector = BuildDetector(tmp); + detector.SessionOwnerResolver = sessionId => throw new System.ComponentModel.Win32Exception(5); + detector.ProcessOwnerResolver = pid => "CONTOSO\\alice"; + + Assert.Equal("CONTOSO\\alice", detector.ResolveOwner(1234, 2)); + } + + [Fact] + public void ResolveOwner_returns_null_when_both_paths_fail() + { + using var tmp = new TempDirectory(); + using var detector = BuildDetector(tmp); + detector.SessionOwnerResolver = sessionId => null; + detector.ProcessOwnerResolver = pid => null; + + Assert.Null(detector.ResolveOwner(1234, 2)); + } + + [Fact] + public void ResolveOwner_passes_session_id_to_WTS_and_pid_to_WMI() + { + using var tmp = new TempDirectory(); + using var detector = BuildDetector(tmp); + int? seenSessionId = null; + int? seenPid = null; + detector.SessionOwnerResolver = sessionId => { seenSessionId = sessionId; return null; }; + detector.ProcessOwnerResolver = pid => { seenPid = pid; return null; }; + + detector.ResolveOwner(processId: 4711, sessionId: 3); + + Assert.Equal(3, seenSessionId); + Assert.Equal(4711, seenPid); + } + // ============================================================================ // DAD-liveness telemetry (2026-05-15) — Started / FirstPoll / NoCandidate // ============================================================================ diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Enrollment/SystemSignals/DesktopArrivalDetector.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Enrollment/SystemSignals/DesktopArrivalDetector.cs index db92dd1b..90dbfeb9 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Enrollment/SystemSignals/DesktopArrivalDetector.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Enrollment/SystemSignals/DesktopArrivalDetector.cs @@ -4,6 +4,7 @@ using System.Diagnostics; using System.Management; using System.Threading; using AutopilotMonitor.Agent.V2.Core.Logging; +using AutopilotMonitor.Agent.V2.Core.Monitoring.Interop; using SharedEventTypes = AutopilotMonitor.Shared.Constants.EventTypes; namespace AutopilotMonitor.Agent.V2.Core.Monitoring.Enrollment.SystemSignals @@ -14,6 +15,13 @@ namespace AutopilotMonitor.Agent.V2.Core.Monitoring.Enrollment.SystemSignals /// Used for enrollment completion in no-ESP scenarios (WDP v2, ESP disabled) and /// as a backup signal for AccountSetup phase correction. /// + /// Owner resolution (session 4d5a0b78 fix, 2026-06-11): primary path is a WTS session + /// query (WTSQuerySessionInformation with WTSUserName/WTSDomainName) — fast and + /// independent of the WinMgmt service; WMI Win32_Process.GetOwner is the fallback. + /// On the affected device class GetOwner failed on every poll, so the owner was never + /// resolved and the Desktop half of the completion AND-gate starved. + /// + /// /// Liveness telemetry (state-change-only, max 3 events per detector lifetime, NOT periodic): /// /// @@ -44,9 +52,16 @@ namespace AutopilotMonitor.Agent.V2.Core.Monitoring.Enrollment.SystemSignals private bool _explorerEverSeen; // any explorer.exe observed at all (session 0 OR not) private bool _anyNonZeroSessionEverSeen; // explorer.exe in a non-SYSTEM session observed private int _wmiErrorCountSinceStart; + private int _wtsErrorCountSinceStart; private bool _noCandidateFired; private readonly int _noCandidateTimeoutMinutes; + // Test seams (session 4d5a0b78 fix): owner resolution is injectable so unit tests can + // drive the WTS-primary / WMI-fallback order without a live session manager or WinMgmt. + // Null = production defaults (GetSessionOwnerViaWts / GetProcessOwnerViaWmi). + internal Func SessionOwnerResolver { get; set; } + internal Func ProcessOwnerResolver { get; set; } + /// /// System/service account names that should NOT be considered real users. /// @@ -132,6 +147,7 @@ namespace AutopilotMonitor.Agent.V2.Core.Monitoring.Enrollment.SystemSignals _explorerEverSeen = false; _anyNonZeroSessionEverSeen = false; _wmiErrorCountSinceStart = 0; + _wtsErrorCountSinceStart = 0; _noCandidateFired = false; ArmTimer(startReason: "real-user-switch-reset"); @@ -188,8 +204,8 @@ namespace AutopilotMonitor.Agent.V2.Core.Monitoring.Enrollment.SystemSignals _anyNonZeroSessionEverSeen = true; - var owner = GetProcessOwner(proc.Id); - if (owner == null) + var owner = ResolveOwner(proc.Id, proc.SessionId); + if (string.IsNullOrEmpty(owner)) continue; // Check against exclusion list @@ -302,6 +318,7 @@ namespace AutopilotMonitor.Agent.V2.Core.Monitoring.Enrollment.SystemSignals { "explorerProcessCount", explorerProcessCount }, { "anyNonZeroSessionObserved", _anyNonZeroSessionEverSeen }, { "wmiErrorCount", _wmiErrorCountSinceStart }, + { "wtsErrorCount", _wtsErrorCountSinceStart }, }); } catch (Exception ex) { _logger.Verbose($"DesktopArrivalDetector: first-poll emit failed: {ex.Message}"); } @@ -337,6 +354,7 @@ namespace AutopilotMonitor.Agent.V2.Core.Monitoring.Enrollment.SystemSignals { "explorerEverSeen", _explorerEverSeen }, { "anyNonZeroSessionEverSeen", _anyNonZeroSessionEverSeen }, { "wmiErrorCount", _wmiErrorCountSinceStart }, + { "wtsErrorCount", _wtsErrorCountSinceStart }, }); } catch (Exception ex) { _logger.Verbose($"DesktopArrivalDetector: no-candidate emit failed: {ex.Message}"); } @@ -365,7 +383,65 @@ namespace AutopilotMonitor.Agent.V2.Core.Monitoring.Enrollment.SystemSignals } /// - /// Gets the owner of a process using WMI (Win32_Process.GetOwner). + /// Resolves the owning user of an explorer.exe candidate. Session 4d5a0b78 fix + /// (2026-06-11): the WMI Win32_Process.GetOwner call failed on every poll on + /// some devices (wmiErrorCount == pollCount) while the agent ran for over an hour — + /// the owner was never resolved, desktop_arrived never fired, and the Desktop + /// half of the completion AND-gate starved. WTS is now the primary path: a single + /// kernel round-trip against the session manager (no WinMgmt dependency, much + /// cheaper than a WMI query), keyed by the process's session id — explorer.exe runs + /// as the session's logged-on user. WMI remains the fallback when WTS returns no + /// user for the session. + /// + /// Returns "DOMAIN\User" or "User", or null when both paths fail. Each path's + /// failures are counted separately ( / + /// ) and surfaced via the first_poll / + /// no_candidate liveness payloads. + /// + /// + internal string ResolveOwner(int processId, int sessionId) + { + string owner = null; + try + { + owner = (SessionOwnerResolver ?? GetSessionOwnerViaWts)(sessionId); + } + catch (Exception ex) + { + _logger.Debug($"DesktopArrivalDetector: WTS owner resolution threw for session {sessionId}: {ex.Message}"); + } + if (!string.IsNullOrEmpty(owner)) + return owner; + + _wtsErrorCountSinceStart++; + return (ProcessOwnerResolver ?? GetProcessOwnerViaWmi)(processId); + } + + /// + /// Primary owner resolution: WTSQuerySessionInformation(WTSUserName/WTSDomainName) + /// for the process's session. Returns null when the API fails or the session has no + /// associated user (caller counts it and falls back to WMI). + /// + private string GetSessionOwnerViaWts(int sessionId) + { + try + { + var user = WtsNativeMethods.QuerySessionString(sessionId, WtsNativeMethods.WTSUserName); + if (string.IsNullOrEmpty(user)) + return null; + + var domain = WtsNativeMethods.QuerySessionString(sessionId, WtsNativeMethods.WTSDomainName); + return string.IsNullOrEmpty(domain) ? user : $"{domain}\\{user}"; + } + catch (Exception ex) + { + _logger.Debug($"DesktopArrivalDetector: WTS session query failed for session {sessionId}: {ex.Message}"); + return null; + } + } + + /// + /// Fallback owner resolution: WMI (Win32_Process.GetOwner). /// Returns "DOMAIN\User" or "User" string, or null on failure. /// /// P3 fix (2026-05-15): WMI exceptions are caught here AND bumped onto @@ -375,7 +451,7 @@ namespace AutopilotMonitor.Agent.V2.Core.Monitoring.Enrollment.SystemSignals /// leaving GetOwner-specific failures invisible. /// /// - private string GetProcessOwner(int processId) + private string GetProcessOwnerViaWmi(int processId) { try { diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Interop/WtsNativeMethods.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Interop/WtsNativeMethods.cs new file mode 100644 index 00000000..61f02946 --- /dev/null +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Interop/WtsNativeMethods.cs @@ -0,0 +1,61 @@ +using System; +using System.Runtime.InteropServices; + +namespace AutopilotMonitor.Agent.V2.Core.Monitoring.Interop +{ + /// + /// Win32 P/Invoke declarations for WTS (Remote Desktop Services) session queries. + /// Used by DesktopArrivalDetector to resolve the logged-on user of an + /// interactive session directly from the session manager — a single fast kernel + /// round-trip, unlike the WMI Win32_Process.GetOwner path which depends on + /// the WinMgmt service and fails persistently on some devices (session 4d5a0b78: + /// GetOwner failed on every poll while WTS-level session data stayed available). + /// + internal static class WtsNativeMethods + { + /// WTS_INFO_CLASS.WTSUserName — user name associated with the session. + public const int WTSUserName = 5; + + /// WTS_INFO_CLASS.WTSDomainName — domain of the user associated with the session. + public const int WTSDomainName = 7; + + /// WTS_CURRENT_SERVER_HANDLE — query the local machine. + public static readonly IntPtr WTS_CURRENT_SERVER_HANDLE = IntPtr.Zero; + + [DllImport("wtsapi32.dll", SetLastError = true, CharSet = CharSet.Unicode)] + public static extern bool WTSQuerySessionInformation( + IntPtr hServer, + int sessionId, + int wtsInfoClass, + out IntPtr ppBuffer, + out int pBytesReturned); + + [DllImport("wtsapi32.dll")] + public static extern void WTSFreeMemory(IntPtr pMemory); + + /// + /// Queries a string-typed WTS session property ( / + /// ) for on the local + /// machine. Returns null when the API call fails; an empty string is a + /// successful query with no value (e.g. no user associated with the session). + /// Always frees the WTS buffer. + /// + public static string QuerySessionString(int sessionId, int wtsInfoClass) + { + var buffer = IntPtr.Zero; + try + { + if (!WTSQuerySessionInformation(WTS_CURRENT_SERVER_HANDLE, sessionId, wtsInfoClass, out buffer, out var bytesReturned)) + return null; + + // bytesReturned includes the trailing null terminator (2 bytes Unicode); + // <= 2 means "no characters". + return bytesReturned > 2 ? Marshal.PtrToStringUni(buffer) : string.Empty; + } + finally + { + if (buffer != IntPtr.Zero) WTSFreeMemory(buffer); + } + } + } +}