v2 single-rail 13: remove dead Registry-guard in CheckEnrollmentCompleteMarker

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) <noreply@anthropic.com>
This commit is contained in:
Oliver Kieselbach
2026-04-24 03:24:52 +02:00
parent 8fe75a0f4e
commit fef5834ad0
5 changed files with 21 additions and 53 deletions
@@ -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: () =>
@@ -184,47 +184,27 @@ namespace AutopilotMonitor.Agent.V2
// ----------------------------------------------------------------- Marker checks
/// <summary>
/// <c>true</c> when an <c>enrollment-complete.marker</c> or the <c>Deployed</c> registry
/// marker indicates that this device already completed enrollment. Legacy parity —
/// triggers a self-destruct retry and the agent exits cleanly.
/// <c>true</c> when the file-based <c>enrollment-complete.marker</c> signals that a
/// previous session already completed enrollment — written by
/// <see cref="CheckSessionAgeEmergencyBreak"/> 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
/// <paramref name="selfDestructOnComplete"/> is set.
/// <para>
/// The <c>HKLM\SOFTWARE\AutopilotMonitor\Deployed</c> registry value is NOT consulted
/// here: it is a bootstrap-script reentry lock (read by Install-AutopilotMonitor-v2.ps1
/// Guard 1), set by <c>--install</c> 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.
/// </para>
/// </summary>
internal static bool CheckEnrollmentCompleteMarker(
string dataDirectory,
string stateDirectory,
bool selfDestructOnComplete,
Func<CleanupService> 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;
+3 -10
View File
@@ -40,8 +40,8 @@ namespace AutopilotMonitor.Agent.V2
/// <item><see cref="DetectPreviousExit"/> — reads markers + event log to classify last shutdown</item>
/// <item>Resolve TenantId (registry → bootstrap-config.json fallback)</item>
/// <item>Build <see cref="AgentConfiguration"/> (CLI args + persisted bootstrap / await-enrollment config)</item>
/// <item>Get/create SessionId via <see cref="SessionIdPersistence"/> — <b>before</b> the guards, matching Legacy boot order. The ghost-restart guard keys on <c>session.id</c> 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.</item>
/// <item><see cref="CheckEnrollmentCompleteMarker"/> — ghost-restart detection + cleanup retry</item>
/// <item>Get/create SessionId via <see cref="SessionIdPersistence"/></item>
/// <item><see cref="CheckEnrollmentCompleteMarker"/> — file-based enrollment-complete-marker detection + cleanup retry</item>
/// <item><see cref="CheckSessionAgeEmergencyBreak"/> — absolute session-age watchdog</item>
/// <item>(Optional) Wait for MDM certificate in <c>--await-enrollment</c> mode</item>
/// <item>Build <see cref="BackendApiClient"/> + <see cref="RemoteConfigService"/> → fetch config</item>
@@ -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<CleanupService> cleanupServiceFactory = () => new CleanupService(agentConfig, logger);
if (CheckEnrollmentCompleteMarker(
dataDirectory, stateSubdir,
stateSubdir,
agentConfig.SelfDestructOnComplete, cleanupServiceFactory, logger, consoleMode))
{
logger.Info("Enrollment-complete marker handled — agent exiting.");
@@ -1 +1 @@
256
257
@@ -1 +1 @@
1119
1120