From fef5834ad0d189cf4e3b1be62b21cf7b273dfb1b Mon Sep 17 00:00:00 2001 From: Oliver Kieselbach Date: Fri, 24 Apr 2026 03:24:52 +0200 Subject: [PATCH] v2 single-rail 13: remove dead Registry-guard in CheckEnrollmentCompleteMarker MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The registry-based ghost-restart branch in CheckEnrollmentCompleteMarker (HKLM\SOFTWARE\AutopilotMonitor\Deployed present + !SessionExists) has never fired in production and was structurally unreachable: - session.id is created via SessionIdPersistence.GetOrCreate BEFORE the guard runs (both in V1 LoadConfiguration:131 and V2 Program.cs:216), so the !SessionExists() retry-cleanup arm is never reachable at runtime. - Even if the arm were reachable, the Deployed regkey cannot distinguish "fresh install" from "ghost restart": it is written by --install and intentionally preserved by CleanupService as a bootstrap-script reentry lock (read by scripts/Bootstrap/Install-AutopilotMonitor-v2.ps1 Guard 1 to prevent re-install on production devices). Both scenarios therefore present as "Deployed present + session.id absent". Remove the registry-guard block and its dead retry-cleanup path. Keep the file-based enrollment-complete.marker path intact — it is written by CheckSessionAgeEmergencyBreak before its own retry-cleanup attempt and serves the "cleanup-disabled emergency break" exit-cleanly case. Also drop the now-obsolete "Create SessionId BEFORE guards" justification comment in Program.cs and the matching boot-sequence doc wording — neither remaining guard (file-based marker, session-age watchdog) keys on session.id absence for ghost detection. CheckEnrollmentCompleteMarker loses the now-unused dataDirectory parameter; callsites updated. Tests: ProgramGuardsTests.cs keeps marker-absent / marker-without- selfdestruct / marker-with-selfdestruct-retry coverage; the first-run- after-install regression test is renamed to match its actual scope (returns_false_when_marker_file_is_absent). 630/630 V2.Core tests green. No behavior change — removed path was unreachable. TryWriteDeploymentMarker in Program.InstallMode.cs stays (writes the bootstrap-reentry-lock); CleanupService continues to intentionally leave the regkey in place. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../Program/ProgramGuardsTests.cs | 11 ++--- .../Program.Guards.cs | 46 ++++++------------- .../AutopilotMonitor.Agent.V2/Program.cs | 13 ++---- .../buildcounter.txt | 2 +- .../AutopilotMonitor.Agent/buildcounter.txt | 2 +- 5 files changed, 21 insertions(+), 53 deletions(-) diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Program/ProgramGuardsTests.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Program/ProgramGuardsTests.cs index c2a1b293..134a15a0 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Program/ProgramGuardsTests.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Program/ProgramGuardsTests.cs @@ -78,12 +78,10 @@ namespace AutopilotMonitor.Agent.V2.Core.Tests.Program // ================================================================= CheckEnrollmentCompleteMarker [Fact] - public void CheckEnrollmentCompleteMarker_returns_false_on_first_run_after_install_when_session_exists() + public void CheckEnrollmentCompleteMarker_returns_false_when_marker_file_is_absent() { - // Regression guard for the first-run-after-install bug: Program.cs must create the - // SessionId BEFORE invoking this guard, so a fresh install (Deployed registry marker - // written by --install, no prior session) does not get misdiagnosed as a ghost - // restart and trigger self-destruct before FetchConfigAsync. + // No enrollment-complete.marker present → guard must stay silent and leave the + // agent to proceed with normal startup, whether or not a SessionId exists. using var tmp = new TempDirectory(); var logger = NewLogger(tmp.Path); new SessionIdPersistence(tmp.Path).GetOrCreate(logger); @@ -91,7 +89,6 @@ namespace AutopilotMonitor.Agent.V2.Core.Tests.Program var factoryCalls = 0; var shouldExit = AutopilotMonitor.Agent.V2.Program.CheckEnrollmentCompleteMarker( - dataDirectory: tmp.Path, stateDirectory: stateDir, selfDestructOnComplete: true, // worst case — cleanup armed cleanupServiceFactory: () => @@ -121,7 +118,6 @@ namespace AutopilotMonitor.Agent.V2.Core.Tests.Program var factoryCalls = 0; var shouldExit = AutopilotMonitor.Agent.V2.Program.CheckEnrollmentCompleteMarker( - dataDirectory: tmp.Path, stateDirectory: stateDir, selfDestructOnComplete: false, cleanupServiceFactory: () => @@ -152,7 +148,6 @@ namespace AutopilotMonitor.Agent.V2.Core.Tests.Program var factoryCalls = 0; var shouldExit = AutopilotMonitor.Agent.V2.Program.CheckEnrollmentCompleteMarker( - dataDirectory: tmp.Path, stateDirectory: stateDir, selfDestructOnComplete: true, cleanupServiceFactory: () => diff --git a/src/Agent/AutopilotMonitor.Agent.V2/Program.Guards.cs b/src/Agent/AutopilotMonitor.Agent.V2/Program.Guards.cs index 6057ca7f..d5afa80c 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2/Program.Guards.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2/Program.Guards.cs @@ -184,47 +184,27 @@ namespace AutopilotMonitor.Agent.V2 // ----------------------------------------------------------------- Marker checks /// - /// true when an enrollment-complete.marker or the Deployed registry - /// marker indicates that this device already completed enrollment. Legacy parity — - /// triggers a self-destruct retry and the agent exits cleanly. + /// true when the file-based enrollment-complete.marker signals that a + /// previous session already completed enrollment — written by + /// right before it fires the cleanup retry, + /// so that a next-boot observing the marker exits cleanly even if the cleanup retry + /// failed to remove the Scheduled Task. Triggers another self-destruct attempt when + /// is set. + /// + /// The HKLM\SOFTWARE\AutopilotMonitor\Deployed registry value is NOT consulted + /// here: it is a bootstrap-script reentry lock (read by Install-AutopilotMonitor-v2.ps1 + /// Guard 1), set by --install and intentionally preserved across cleanup. It + /// therefore cannot distinguish a fresh install from a ghost restart and is unsuitable + /// as an agent-side ghost-restart signal. + /// /// internal static bool CheckEnrollmentCompleteMarker( - string dataDirectory, string stateDirectory, bool selfDestructOnComplete, Func cleanupServiceFactory, AgentLogger logger, bool consoleMode) { - // Registry guard: "Deployed" key set by --install but no active session → ghost restart. - try - { - using (var regKey = Microsoft.Win32.Registry.LocalMachine.OpenSubKey(DeploymentRegistryKey)) - { - var deployed = regKey?.GetValue(DeploymentRegistryValue) as string; - if (!string.IsNullOrEmpty(deployed)) - { - var persistence = new SessionIdPersistence(dataDirectory); - if (!persistence.SessionExists()) - { - logger.Info($"Ghost restart detected (Deployed={deployed}, no active session)."); - if (selfDestructOnComplete) - { - TryRetryCleanup(cleanupServiceFactory, logger, "ghost_restart"); - } - if (consoleMode) - Console.Out.WriteLine("Agent was previously deployed but no active session. Exiting."); - return true; - } - logger.Info($"Deployment marker present ({deployed}) with active session — continuing normally."); - } - } - } - catch (Exception regEx) - { - logger.Warning($"Registry deployment marker check failed: {regEx.Message}"); - } - // File-based enrollment-complete marker. var markerPath = Path.Combine(stateDirectory, EnrollmentCompleteMarkerFileName); if (!File.Exists(markerPath)) return false; diff --git a/src/Agent/AutopilotMonitor.Agent.V2/Program.cs b/src/Agent/AutopilotMonitor.Agent.V2/Program.cs index 54d1e5d5..781fa410 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2/Program.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2/Program.cs @@ -40,8 +40,8 @@ namespace AutopilotMonitor.Agent.V2 /// — reads markers + event log to classify last shutdown /// Resolve TenantId (registry → bootstrap-config.json fallback) /// Build (CLI args + persisted bootstrap / await-enrollment config) - /// Get/create SessionId via — before the guards, matching Legacy boot order. The ghost-restart guard keys on session.id absence to detect "self-destruct ran but Scheduled Task survived"; creating the session after the guard would misdiagnose every first-run-after-install as a ghost restart and trigger self-destruct before the remote-config fetch. - /// — ghost-restart detection + cleanup retry + /// Get/create SessionId via + /// — file-based enrollment-complete-marker detection + cleanup retry /// — absolute session-age watchdog /// (Optional) Wait for MDM certificate in --await-enrollment mode /// Build + → fetch config @@ -201,13 +201,6 @@ namespace AutopilotMonitor.Agent.V2 var agentConfig = BuildAgentConfiguration(args, tenantId, sessionId: null, bootstrapConfig, awaitConfig); - // Create/recover SessionId BEFORE the startup guards. Legacy-parity boot order — - // the ghost-restart guard below keys on session.id absence to detect - // "self-destruct ran but Scheduled Task survived". Creating the session after the - // guard would misdiagnose every first-run-after-install (Deployed registry marker - // set by --install, no session.id yet) as a ghost restart and trigger - // ExecuteSelfDestruct before the remote-config fetch, deleting the ProgramData - // directory on every fresh deployment. var sessionPersistence = new SessionIdPersistence(dataDirectory); if (args.Contains("--new-session")) { @@ -227,7 +220,7 @@ namespace AutopilotMonitor.Agent.V2 Func cleanupServiceFactory = () => new CleanupService(agentConfig, logger); if (CheckEnrollmentCompleteMarker( - dataDirectory, stateSubdir, + stateSubdir, agentConfig.SelfDestructOnComplete, cleanupServiceFactory, logger, consoleMode)) { logger.Info("Enrollment-complete marker handled — agent exiting."); diff --git a/src/Agent/AutopilotMonitor.Agent.V2/buildcounter.txt b/src/Agent/AutopilotMonitor.Agent.V2/buildcounter.txt index 9183bf03..a700e799 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2/buildcounter.txt +++ b/src/Agent/AutopilotMonitor.Agent.V2/buildcounter.txt @@ -1 +1 @@ -256 +257 diff --git a/src/Agent/AutopilotMonitor.Agent/buildcounter.txt b/src/Agent/AutopilotMonitor.Agent/buildcounter.txt index 48728527..b1e1a0c0 100644 --- a/src/Agent/AutopilotMonitor.Agent/buildcounter.txt +++ b/src/Agent/AutopilotMonitor.Agent/buildcounter.txt @@ -1 +1 @@ -1119 +1120