fix(agent): harden single hardware read + reuse it for the session row

The agent read device hardware twice: once at startup via HardwareInfo (feeds the security headers + telemetry) and again at registration via DeviceInfoProvider (feeds the Sessions row). When WMI was transiently unavailable during OOBE, the body read returned "Unknown" while the header read had already captured real values — producing session rows with Manufacturer/Model/SerialNumber = "Unknown" even though the backend validated (and passed) real values from the headers. - HardwareInfo.GetHardwareInfo: retry once (~1.5s) when any field is still unresolved, per-field merge across attempts, and isolate each WMI query so a BIOS-serial failure no longer discards manufacturer/model. Healthy devices resolve on attempt 1 and never sleep. - SessionRegistrationHelper.BuildRegistration: accept the already-read hardware tuple and use it instead of re-querying WMI (falls back to DeviceInfoProvider only when none supplied, e.g. unit tests). - BackendSessionRegistration: pass the auth bundle's hardware through so the session row matches the headers the backend validated against. Net: one hardened WMI read instead of two, and the Sessions row can no longer diverge from the security headers. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-10-24 04:32:32 +00:00 · 2026-07-14 16:32:10 +02:00
parent b0105b123a
commit c649d80c86
4 changed files with 142 additions and 25 deletions
@@ -139,6 +139,30 @@ namespace AutopilotMonitor.Agent.V2.Core.Tests.Security
            Assert.True(captured.IsUserDriven);
        }

+        [Fact]
+        public async Task Uses_supplied_device_hardware_instead_of_querying_wmi()
+        {
+            using var tmp = new TempDirectory();
+            var logger = NewLogger(tmp);
+            var config = NewConfig();
+
+            SessionRegistration captured = null!;
+            var apiClient = new FakeApiClient(
+                _ => new RegisterSessionResponse { Success = true, SessionId = config.SessionId },
+                onRegister: reg => captured = reg);
+
+            // Simulates the auth bundle's already-hardened hardware read being passed through.
+            // The body must carry these verbatim rather than re-reading WMI on the test host.
+            await SessionRegistrationHelper.RegisterWithRetryAsync(
+                apiClient, config, agentVersion: "9.9.9", logger: logger,
+                deviceHardware: ("Contoso Devices", "Contoso Book 3", "SN-TEST-12345"));
+
+            Assert.NotNull(captured);
+            Assert.Equal("Contoso Devices", captured.Manufacturer);
+            Assert.Equal("Contoso Book 3", captured.Model);
+            Assert.Equal("SN-TEST-12345", captured.SerialNumber);
+        }
+
        // Subclasses the real BackendApiClient so we inherit its public API but intercept the
        // single virtual entry point that matters. BackendApiClient's ctor is protected-less in
        // V2 (`public BackendApiClient(string baseUrl, ...)` + `protected BackendApiClient()`);
@@ -1,5 +1,6 @@
 using System;
 using System.Management;
+using System.Threading;
 using AutopilotMonitor.Agent.V2.Core.Logging;

 namespace AutopilotMonitor.Agent.V2.Core.Security
@@ -9,22 +10,82 @@ namespace AutopilotMonitor.Agent.V2.Core.Security
    /// </summary>
    public static class HardwareInfo
    {
+        /// <summary>Sentinel returned per field when WMI cannot supply a value.</summary>
+        private const string Unknown = "Unknown";
+
        /// <summary>
-        /// Gets the device manufacturer and model from WMI
+        /// Gets the device manufacturer, model and serial number from WMI.
+        /// <para>
+        /// The Winmgmt (WMI) service is occasionally unavailable for a brief window during OOBE —
+        /// exactly when the agent first starts and reads hardware for the security headers AND the
+        /// registration row. A single failed read would poison both with "Unknown" (observed:
+        /// session rows with Manufacturer/Model/SerialNumber = "Unknown" while the registry-based
+        /// OS fields were correct). This one read is the single source of truth (headers, telemetry
+        /// and — via the auth bundle — the registration body all reuse it), so it retries once after
+        /// a short delay when any field is still unresolved. Per-field merge keeps a value that
+        /// resolved on an earlier attempt even if a later query transiently fails.
+        /// </para>
        /// </summary>
        /// <param name="logger">Logger for diagnostic output</param>
+        /// <param name="maxAttempts">Total read attempts (default 2 → one retry). Overridable for tests.</param>
+        /// <param name="retryDelayMs">Delay between attempts in milliseconds. Overridable for tests.</param>
        /// <returns>Tuple of (Manufacturer, Model, SerialNumber)</returns>
-        public static (string Manufacturer, string Model, string SerialNumber) GetHardwareInfo(AgentLogger logger = null)
+        public static (string Manufacturer, string Model, string SerialNumber) GetHardwareInfo(
+            AgentLogger logger = null,
+            int maxAttempts = 2,
+            int retryDelayMs = 1500)
        {
-            string manufacturer = "Unknown";
-            string model = "Unknown";
-            string serialNumber = "Unknown";
+            string manufacturer = Unknown;
+            string model = Unknown;
+            string serialNumber = Unknown;

+            for (int attempt = 1; attempt <= maxAttempts; attempt++)
+            {
+                var read = TryReadHardware(logger, attempt);
+
+                // Per-field merge: never overwrite an already-resolved value with a later "Unknown".
+                if (IsResolved(read.Manufacturer)) manufacturer = read.Manufacturer;
+                if (IsResolved(read.Model)) model = read.Model;
+                if (IsResolved(read.SerialNumber)) serialNumber = read.SerialNumber;
+
+                if (IsResolved(manufacturer) && IsResolved(model) && IsResolved(serialNumber))
+                {
+                    if (attempt > 1)
+                        logger?.Info($"Hardware fully resolved on attempt {attempt}/{maxAttempts}.");
+                    return (manufacturer, model, serialNumber);
+                }
+
+                if (attempt < maxAttempts)
+                {
+                    logger?.Warning(
+                        $"Hardware read incomplete (Manufacturer={manufacturer}, Model={model}, SerialNumber={serialNumber}) — " +
+                        $"WMI may not be ready yet. Retrying in {retryDelayMs}ms (attempt {attempt}/{maxAttempts}).");
+                    if (retryDelayMs > 0)
+                        Thread.Sleep(retryDelayMs);
+                }
+            }
+
+            logger?.Warning(
+                $"Hardware read still incomplete after {maxAttempts} attempt(s): " +
+                $"Manufacturer={manufacturer}, Model={model}, SerialNumber={serialNumber}.");
+            return (manufacturer, model, serialNumber);
+        }
+
+        /// <summary>
+        /// A single best-effort WMI read. Never throws. Each query is isolated so a failure reading
+        /// the BIOS serial does not discard a manufacturer/model that was already read successfully.
+        /// </summary>
+        private static (string Manufacturer, string Model, string SerialNumber) TryReadHardware(AgentLogger logger, int attempt)
+        {
+            string manufacturer = Unknown;
+            string model = Unknown;
+            string serialNumber = Unknown;
+
+            // Manufacturer + Model from Win32_ComputerSystem.
+            // Lenovo reports the marketing model name in Win32_ComputerSystemProduct.Version
+            // instead of Win32_ComputerSystem.Model (which contains a generic platform string).
            try
            {
-                // Get Manufacturer and Model from Win32_ComputerSystem
-                // Lenovo reports the marketing model name in Win32_ComputerSystemProduct.Version
-                // instead of Win32_ComputerSystem.Model (which contains a generic platform string)
                using (var searcher = new ManagementObjectSearcher("SELECT Manufacturer, Model FROM Win32_ComputerSystem"))
                using (var collection = searcher.Get())
                {
@@ -32,7 +93,7 @@ namespace AutopilotMonitor.Agent.V2.Core.Security
                    {
                        using (obj)
                        {
-                            manufacturer = obj["Manufacturer"]?.ToString() ?? "Unknown";
+                            manufacturer = obj["Manufacturer"]?.ToString() ?? Unknown;
                            if (manufacturer.IndexOf("lenovo", StringComparison.OrdinalIgnoreCase) >= 0)
                            {
                                using (var lenovoSearcher = new ManagementObjectSearcher("SELECT Version FROM Win32_ComputerSystemProduct"))
@@ -42,7 +103,7 @@ namespace AutopilotMonitor.Agent.V2.Core.Security
                                    {
                                        using (lenovoObj)
                                        {
-                                            model = lenovoObj["Version"]?.ToString() ?? "Unknown";
+                                            model = lenovoObj["Version"]?.ToString() ?? Unknown;
                                        }
                                        break;
                                    }
@@ -50,13 +111,20 @@ namespace AutopilotMonitor.Agent.V2.Core.Security
                            }
                            else
                            {
-                                model = obj["Model"]?.ToString() ?? "Unknown";
+                                model = obj["Model"]?.ToString() ?? Unknown;
                            }
                        }
                    }
                }
+            }
+            catch (Exception ex)
+            {
+                logger?.Debug($"Hardware read: Win32_ComputerSystem query failed (attempt {attempt}): {ex.Message}");
+            }

-                // Get Serial Number from Win32_BIOS
+            // Serial number from Win32_BIOS — isolated so a failure here keeps manufacturer/model.
+            try
+            {
                using (var searcher = new ManagementObjectSearcher("SELECT SerialNumber FROM Win32_BIOS"))
                using (var collection = searcher.Get())
                {
@@ -64,19 +132,27 @@ namespace AutopilotMonitor.Agent.V2.Core.Security
                    {
                        using (obj)
                        {
-                            serialNumber = obj["SerialNumber"]?.ToString() ?? "Unknown";
+                            serialNumber = obj["SerialNumber"]?.ToString() ?? Unknown;
                        }
                    }
                }
-
-                logger?.Debug($"Hardware detected: Manufacturer={manufacturer}, Model={model}, SerialNumber={serialNumber}");
-                return (manufacturer, model, serialNumber);
            }
            catch (Exception ex)
            {
-                logger?.Error("Error retrieving hardware information", ex);
-                return ("Unknown", "Unknown", "Unknown");
+                logger?.Debug($"Hardware read: Win32_BIOS query failed (attempt {attempt}): {ex.Message}");
            }
+
+            logger?.Debug($"Hardware detected (attempt {attempt}): Manufacturer={manufacturer}, Model={model}, SerialNumber={serialNumber}");
+            return (manufacturer, model, serialNumber);
        }
+
+        /// <summary>
+        /// A field counts as resolved when it is non-empty and not the "Unknown" fallback sentinel.
+        /// "Unknown" is only ever produced by this class's own fallbacks, so it is safe to treat as
+        /// "not yet read" for retry purposes.
+        /// </summary>
+        private static bool IsResolved(string value)
+            => !string.IsNullOrWhiteSpace(value)
+               && !string.Equals(value, Unknown, StringComparison.OrdinalIgnoreCase);
    }
 }
@@ -55,14 +55,15 @@ namespace AutopilotMonitor.Agent.V2.Core.Security
            AuthFailureTracker authFailureTracker = null,
            EmergencyReporter emergencyReporter = null,
            Func<int, Task> backoffDelay = null,
-            Func<Exception, Task> onTerminalTransportFailure = null)
+            Func<Exception, Task> onTerminalTransportFailure = null,
+            (string Manufacturer, string Model, string SerialNumber)? deviceHardware = null)
        {
            if (apiClient == null) throw new ArgumentNullException(nameof(apiClient));
            if (agentConfig == null) throw new ArgumentNullException(nameof(agentConfig));
            if (logger == null) throw new ArgumentNullException(nameof(logger));

            backoffDelay ??= DefaultBackoffDelay;
-            var registration = BuildRegistration(agentConfig, agentVersion);
+            var registration = BuildRegistration(agentConfig, agentVersion, deviceHardware);
            string lastError = null;
            Exception lastException = null;

@@ -142,15 +143,28 @@ namespace AutopilotMonitor.Agent.V2.Core.Security
        private static Task DefaultBackoffDelay(int attempt)
            => Task.Delay(((int)Math.Pow(2, attempt)) * 1000);

-        private static SessionRegistration BuildRegistration(AgentConfiguration agentConfig, string agentVersion)
+        private static SessionRegistration BuildRegistration(
+            AgentConfiguration agentConfig,
+            string agentVersion,
+            (string Manufacturer, string Model, string SerialNumber)? deviceHardware = null)
        {
+            // Reuse the single hardened hardware read (HardwareInfo.GetHardwareInfo) that already
+            // populated the security headers instead of re-querying WMI here. This saves a WMI
+            // round-trip AND — because that read retries on transient WMI unavailability during
+            // OOBE — keeps the session row's Manufacturer/Model/SerialNumber consistent with the
+            // headers the backend validated against. Falls back to a fresh DeviceInfoProvider read
+            // only when no hardware was supplied (e.g. unit tests exercising the helper directly).
+            string manufacturer = deviceHardware?.Manufacturer ?? DeviceInfoProvider.GetManufacturer() ?? string.Empty;
+            string model = deviceHardware?.Model ?? DeviceInfoProvider.GetModel() ?? string.Empty;
+            string serialNumber = deviceHardware?.SerialNumber ?? DeviceInfoProvider.GetSerialNumber() ?? string.Empty;
+
            return new SessionRegistration
            {
                SessionId = agentConfig.SessionId,
                TenantId = agentConfig.TenantId,
-                SerialNumber = DeviceInfoProvider.GetSerialNumber() ?? string.Empty,
-                Manufacturer = DeviceInfoProvider.GetManufacturer() ?? string.Empty,
-                Model = DeviceInfoProvider.GetModel() ?? string.Empty,
+                SerialNumber = serialNumber,
+                Manufacturer = manufacturer,
+                Model = model,
                DeviceName = Environment.MachineName,
                OsName = DeviceInfoProvider.GetOsName() ?? string.Empty,
                OsBuild = DeviceInfoProvider.GetOsBuild() ?? string.Empty,
@@ -40,7 +40,10 @@ namespace AutopilotMonitor.Agent.V2.Runtime
                logger: logger,
                authFailureTracker: auth.AuthFailureTracker,
                emergencyReporter: auth.EmergencyReporter,
-                onTerminalTransportFailure: ex => DiagnoseTerminalTransportFailure(ex, auth, logger))
+                onTerminalTransportFailure: ex => DiagnoseTerminalTransportFailure(ex, auth, logger),
+                // Reuse the single hardened hardware read from the auth bundle so the session row
+                // matches the security headers the backend validated against (no second WMI query).
+                deviceHardware: (auth.Manufacturer, auth.Model, auth.SerialNumber))
                .GetAwaiter().GetResult();

            if (registrationResult.Outcome != SessionRegistrationOutcome.Succeeded)