From 9f3ee5eedbbe080986f550069ab253f169f384e3 Mon Sep 17 00:00:00 2001 From: Oliver Kieselbach Date: Thu, 30 Apr 2026 15:59:29 +0200 Subject: [PATCH] fix(agent): V1 bails cleanly when TenantId missing instead of crashing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Customer gardnermedia (2026-04-30) hit a fatal V1 startup crash: "InvalidOperationException: Invalid agent configuration" thrown from MonitoringService..ctor. Root cause: LoadConfiguration returned a config with TenantId=null because HKLM\SOFTWARE\Microsoft\Enrollments did not yet carry an EnrollmentType=6\AADTenantID entry and no bootstrap-config .json was present. Without a guard, control reached MonitoringService construction and IsValid() threw an unhandled exception → crash log → "exception_crash" classification on the next start. V2 already bails cleanly in this case (AgentBootstrap.Run line 123-127). This commit gives V1 the same safety net plus a more diagnostic message so any future occurrence is immediately analyzable from the crash log. Changes: * Program.cs: pre-flight TenantId guard right after the --await-enrollment block. Logs an Error and returns cleanly so the ProcessExit handler writes clean-exit.marker — no spurious crash log, Scheduled Task simply retries on the next trigger. * MonitoringService..ctor: replace the opaque "Invalid agent configuration" throw with one that names the missing fields (ApiBaseUrl / SessionId / TenantId). * MonitoringServiceConfigValidationTests: 5 new xUnit cases covering each missing-field path plus the multi-missing case. * V1 buildcounter bump. V2 unchanged — its existing AgentBootstrap guard already covers this class of crash. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../MonitoringServiceConfigValidationTests.cs | 112 ++++++++++++++++++ .../Monitoring/Runtime/MonitoringService.cs | 9 +- src/Agent/AutopilotMonitor.Agent/Program.cs | 14 +++ .../AutopilotMonitor.Agent/buildcounter.txt | 2 +- 4 files changed, 134 insertions(+), 3 deletions(-) create mode 100644 src/Agent/AutopilotMonitor.Agent.Core.Tests/Core/MonitoringServiceConfigValidationTests.cs diff --git a/src/Agent/AutopilotMonitor.Agent.Core.Tests/Core/MonitoringServiceConfigValidationTests.cs b/src/Agent/AutopilotMonitor.Agent.Core.Tests/Core/MonitoringServiceConfigValidationTests.cs new file mode 100644 index 00000000..55a725b5 --- /dev/null +++ b/src/Agent/AutopilotMonitor.Agent.Core.Tests/Core/MonitoringServiceConfigValidationTests.cs @@ -0,0 +1,112 @@ +using System; +using System.IO; +using AutopilotMonitor.Agent.Core.Configuration; +using AutopilotMonitor.Agent.Core.Logging; +using AutopilotMonitor.Agent.Core.Monitoring.Runtime; +using Xunit; + +namespace AutopilotMonitor.Agent.Core.Tests.Core +{ + /// + /// Verifies that MonitoringService rejects an incomplete AgentConfiguration with a + /// diagnostic message that names the missing fields. Customer-impact regression + /// (gardnermedia, 2026-04-30): a TenantId-less startup crashed with an opaque + /// "Invalid agent configuration" message that defied forensic analysis. + /// + public class MonitoringServiceConfigValidationTests : IDisposable + { + private readonly string _tempDir; + + public MonitoringServiceConfigValidationTests() + { + _tempDir = Path.Combine(Path.GetTempPath(), "autopilot-cfg-validation-" + Guid.NewGuid().ToString("N")); + Directory.CreateDirectory(_tempDir); + } + + public void Dispose() + { + try { Directory.Delete(_tempDir, true); } catch { } + } + + private AgentConfiguration BuildValidConfig() => new AgentConfiguration + { + ApiBaseUrl = "https://example.invalid", + SessionId = Guid.NewGuid().ToString(), + TenantId = Guid.NewGuid().ToString(), + SpoolDirectory = Path.Combine(_tempDir, "spool"), + LogDirectory = Path.Combine(_tempDir, "logs") + }; + + private AgentLogger BuildLogger() => new AgentLogger(Path.Combine(_tempDir, "logs")); + + [Fact] + public void Constructor_throws_with_field_name_when_TenantId_missing() + { + var config = BuildValidConfig(); + config.TenantId = null; + + var ex = Assert.Throws( + () => new MonitoringService(config, BuildLogger(), agentVersion: "test")); + + Assert.Contains("missing", ex.Message); + Assert.Contains(nameof(AgentConfiguration.TenantId), ex.Message); + } + + [Fact] + public void Constructor_throws_with_field_name_when_SessionId_missing() + { + var config = BuildValidConfig(); + config.SessionId = ""; + + var ex = Assert.Throws( + () => new MonitoringService(config, BuildLogger(), agentVersion: "test")); + + Assert.Contains(nameof(AgentConfiguration.SessionId), ex.Message); + } + + [Fact] + public void Constructor_throws_with_field_name_when_ApiBaseUrl_missing() + { + var config = BuildValidConfig(); + config.ApiBaseUrl = null; + + var ex = Assert.Throws( + () => new MonitoringService(config, BuildLogger(), agentVersion: "test")); + + Assert.Contains(nameof(AgentConfiguration.ApiBaseUrl), ex.Message); + } + + [Fact] + public void Constructor_throws_with_all_field_names_when_multiple_missing() + { + var config = new AgentConfiguration + { + ApiBaseUrl = null, + SessionId = null, + TenantId = null, + SpoolDirectory = Path.Combine(_tempDir, "spool"), + LogDirectory = Path.Combine(_tempDir, "logs") + }; + + var ex = Assert.Throws( + () => new MonitoringService(config, BuildLogger(), agentVersion: "test")); + + Assert.Contains(nameof(AgentConfiguration.ApiBaseUrl), ex.Message); + Assert.Contains(nameof(AgentConfiguration.SessionId), ex.Message); + Assert.Contains(nameof(AgentConfiguration.TenantId), ex.Message); + } + + [Fact] + public void Constructor_message_does_not_change_for_valid_config() + { + // Sanity: a fully-populated config should NOT throw the validation exception. + // It may throw later (cert load, spool init) — we only assert the validation + // path itself does not fire on a complete config. + var config = BuildValidConfig(); + + var ex = Record.Exception(() => new MonitoringService(config, BuildLogger(), agentVersion: "test")); + if (ex is InvalidOperationException ioe) + Assert.DoesNotContain("Invalid agent configuration:", ioe.Message); + } + } +} diff --git a/src/Agent/AutopilotMonitor.Agent.Core/Monitoring/Runtime/MonitoringService.cs b/src/Agent/AutopilotMonitor.Agent.Core/Monitoring/Runtime/MonitoringService.cs index b6804718..69854fdc 100644 --- a/src/Agent/AutopilotMonitor.Agent.Core/Monitoring/Runtime/MonitoringService.cs +++ b/src/Agent/AutopilotMonitor.Agent.Core/Monitoring/Runtime/MonitoringService.cs @@ -101,9 +101,14 @@ namespace AutopilotMonitor.Agent.Core.Monitoring.Runtime _previousCrashException = previousCrashException; _lastBootTimeUtc = lastBootTimeUtc; - if (!_configuration.IsValid()) + var missingFields = new List(); + if (string.IsNullOrEmpty(_configuration.ApiBaseUrl)) missingFields.Add(nameof(_configuration.ApiBaseUrl)); + if (string.IsNullOrEmpty(_configuration.SessionId)) missingFields.Add(nameof(_configuration.SessionId)); + if (string.IsNullOrEmpty(_configuration.TenantId)) missingFields.Add(nameof(_configuration.TenantId)); + if (missingFields.Count > 0) { - throw new InvalidOperationException("Invalid agent configuration"); + throw new InvalidOperationException( + "Invalid agent configuration: missing " + string.Join(", ", missingFields)); } _spool = new EventSpool(_configuration.SpoolDirectory); diff --git a/src/Agent/AutopilotMonitor.Agent/Program.cs b/src/Agent/AutopilotMonitor.Agent/Program.cs index bec4aa02..f92ec6ac 100644 --- a/src/Agent/AutopilotMonitor.Agent/Program.cs +++ b/src/Agent/AutopilotMonitor.Agent/Program.cs @@ -255,6 +255,20 @@ namespace AutopilotMonitor.Agent } } + // Pre-flight: bail cleanly if TenantId is missing instead of crashing inside + // MonitoringService.IsValid(). Mirrors V2's AgentBootstrap guard. Common cause: + // agent scheduled before MDM enrollment populated HKLM\SOFTWARE\Microsoft\Enrollments + // and no bootstrap-config.json present. The Scheduled Task will retry on next trigger. + if (string.IsNullOrEmpty(config.TenantId)) + { + logger.Error("Agent cannot start: TenantId not available (registry empty + no bootstrap config). " + + "This usually means the agent was scheduled before MDM enrollment populated the registry. " + + "Will retry on next scheduled trigger."); + if (consoleMode) + Console.WriteLine("ERROR: TenantId not available. Agent will exit cleanly (will retry on next trigger)."); + return; + } + // Evaluate previous exit signals before starting the monitoring service. // clean-exit.marker present → previous run exited cleanly (OS shutdown, reboot, normal exit) // crash_*.log present → unhandled exception crash (with stack trace) diff --git a/src/Agent/AutopilotMonitor.Agent/buildcounter.txt b/src/Agent/AutopilotMonitor.Agent/buildcounter.txt index 9e3410c0..9d63033f 100644 --- a/src/Agent/AutopilotMonitor.Agent/buildcounter.txt +++ b/src/Agent/AutopilotMonitor.Agent/buildcounter.txt @@ -1 +1 @@ -1219 +1222