From fc10575b08bbf9434941db46fbe6a841f103b5c4 Mon Sep 17 00:00:00 2001 From: mrhid6 Date: Tue, 15 Sep 2026 14:09:34 +0000 Subject: [PATCH] fix: require a 2 minute boot time change before counting a patch reboot Windows reports boot time as now minus uptime, which drifts by a second or so between reports. A static report sent in the grace period before the reboot could read as a changed boot and mark a server still owing a reboot as failed. --- server/internal/patchrun/patchrun.go | 7 ++++++- server/internal/patchrun/patchrun_test.go | 19 +++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/server/internal/patchrun/patchrun.go b/server/internal/patchrun/patchrun.go index 02ae013..04a53fc 100644 --- a/server/internal/patchrun/patchrun.go +++ b/server/internal/patchrun/patchrun.go @@ -203,7 +203,7 @@ func VerifyReboot(s models.PatchServerRun, bootTime time.Time, rebootRequired bo } proven := bootTime.After(*s.RebootedAt) if s.BootTimeBefore != nil { - proven = bootTime.After(*s.BootTimeBefore) + proven = bootTime.After(s.BootTimeBefore.Add(BootTimeTolerance)) } if !proven { return s, false @@ -219,6 +219,11 @@ func VerifyReboot(s models.PatchServerRun, bootTime time.Time, rebootRequired bo return s, true } +// BootTimeTolerance is how far a reported boot time must move before it counts +// as a reboot. Windows reports boot time as now minus uptime, which drifts by a +// second or so between reports; a real reboot moves it by far more than this. +const BootTimeTolerance = 2 * time.Minute + // Finalize says whether the run is over and how it ended. Only succeeded // counts as success: unsupported, agent_too_old and the window outcomes did // not patch anything. diff --git a/server/internal/patchrun/patchrun_test.go b/server/internal/patchrun/patchrun_test.go index 6af11b6..32c6a28 100644 --- a/server/internal/patchrun/patchrun_test.go +++ b/server/internal/patchrun/patchrun_test.go @@ -278,6 +278,25 @@ func TestVerifyRebootChangedBoot(t *testing.T) { } } +// Windows derives boot time from now minus uptime, so it drifts by a second or +// so between reports. A report sent in the grace period before the reboot +// must not read as a reboot, or a server still owing one is marked failed. +func TestVerifyRebootIgnoresBootTimeDrift(t *testing.T) { + s := srv("a", models.PatchSrvRebooting) + s.RebootedAt = tp(t0) + s.BootTimeBefore = tp(t0.Add(-10 * 24 * time.Hour)) + now := t0.Add(30 * time.Second) + for _, drift := range []time.Duration{time.Second, BootTimeTolerance} { + if _, ok := VerifyReboot(s, s.BootTimeBefore.Add(drift), true, now); ok { + t.Errorf("a boot time drifted by %s is not proof of a reboot", drift) + } + } + got, ok := VerifyReboot(s, s.BootTimeBefore.Add(BootTimeTolerance+time.Second), false, now) + if !ok || got.Status != models.PatchSrvSucceeded { + t.Fatalf("a boot time past the tolerance is a reboot: %+v ok=%v", got, ok) + } +} + func TestFinalize(t *testing.T) { mk := func(statuses ...string) models.PatchRun { r := windowRun(0)