From 830edb98e7fa2835233304f020c79dd10152d059 Mon Sep 17 00:00:00 2001 From: Oliver Kieselbach Date: Mon, 10 Aug 2026 13:42:43 +0200 Subject: [PATCH] agent: suppress distress dispatch when the auth failure came from a dead endpoint Follow-up to 13dd6723: a platform-403 (stopped/retired app, HTML error page) now skips the DeviceNotRegistered/AuthCertificateRejected distress report - it would be sent to the very endpoint that just answered with a platform page, and its classification would be wrong. Counting and the shutdown thresholds are unchanged, so a dead endpoint still drives the agent to its soft shutdown instead of retrying forever. The one-report-per-streak gate moves from "count == 1" to a streak-scoped flag: an endpoint-unavailable failure no longer consumes the streak's report slot, so a later genuine 403 from a live backend still dispatches. Co-Authored-By: Claude Fable 5 --- .../Security/AuthFailureTrackerTests.cs | 54 +++++++++++++++++++ .../Configuration/RemoteConfigService.cs | 2 +- .../Security/AuthFailureTracker.cs | 43 +++++++++++---- .../Security/SessionRegistrationHelper.cs | 2 +- 4 files changed, 88 insertions(+), 13 deletions(-) diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Security/AuthFailureTrackerTests.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Security/AuthFailureTrackerTests.cs index e2d0c856..f8896aaa 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Security/AuthFailureTrackerTests.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Security/AuthFailureTrackerTests.cs @@ -260,6 +260,60 @@ namespace AutopilotMonitor.Agent.V2.Core.Tests.Security Assert.Contains("agent/config", fakeDistress.Calls[0].Message); } + [Fact] + public void RecordFailure_endpoint_unavailable_suppresses_distress_but_still_counts() + { + var clock = new FakeClock(new DateTime(2026, 4, 21, 10, 0, 0, DateTimeKind.Utc)); + var fakeDistress = new FakeDistressReporter(); + var tracker = new AuthFailureTracker(maxFailures: 2, timeoutMinutes: 0, clock, NewLogger(), fakeDistress); + + int events = 0; + tracker.ThresholdExceeded += (_, _) => events++; + + // Platform-403s from a stopped endpoint: a distress report cannot reach it and the + // DeviceNotRegistered classification would be wrong — nothing must be dispatched. + tracker.RecordFailure(403, "agent/config", endpointUnavailable: true); + tracker.RecordFailure(403, "agent/register-session", endpointUnavailable: true); + + Assert.Empty(fakeDistress.Calls); + // The shutdown thresholds still apply — a dead endpoint must not retry forever. + Assert.Equal(2, tracker.ConsecutiveFailures); + Assert.Equal(1, events); + } + + [Fact] + public void RecordFailure_endpoint_unavailable_does_not_consume_streak_report_slot() + { + var clock = new FakeClock(new DateTime(2026, 4, 21, 10, 0, 0, DateTimeKind.Utc)); + var fakeDistress = new FakeDistressReporter(); + var tracker = new AuthFailureTracker(maxFailures: 50, timeoutMinutes: 0, clock, NewLogger(), fakeDistress); + + // The streak opens with a platform-403 (no dispatch), then a live backend answers + // with a genuine 403 — that one must still produce the streak's single report. + tracker.RecordFailure(403, "agent/config", endpointUnavailable: true); + tracker.RecordFailure(403, "agent/config"); + tracker.RecordFailure(403, "agent/config"); + + Assert.Single(fakeDistress.Calls); + Assert.Equal(DistressErrorType.DeviceNotRegistered, fakeDistress.Calls[0].ErrorType); + } + + [Fact] + public void RecordSuccess_rearms_distress_dispatch_for_the_next_streak() + { + var clock = new FakeClock(new DateTime(2026, 4, 21, 10, 0, 0, DateTimeKind.Utc)); + var fakeDistress = new FakeDistressReporter(); + var tracker = new AuthFailureTracker(maxFailures: 50, timeoutMinutes: 0, clock, NewLogger(), fakeDistress); + + tracker.RecordFailure(401, "agent/config"); + tracker.RecordSuccess(); + tracker.RecordFailure(403, "agent/config"); + + Assert.Equal(2, fakeDistress.Calls.Count); + Assert.Equal(DistressErrorType.AuthCertificateRejected, fakeDistress.Calls[0].ErrorType); + Assert.Equal(DistressErrorType.DeviceNotRegistered, fakeDistress.Calls[1].ErrorType); + } + [Fact] public void RecordFailure_without_distress_reporter_is_silent_but_still_tracks() { diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core/Configuration/RemoteConfigService.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core/Configuration/RemoteConfigService.cs index 6b6e9245..c21b8d08 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2.Core/Configuration/RemoteConfigService.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core/Configuration/RemoteConfigService.cs @@ -155,7 +155,7 @@ namespace AutopilotMonitor.Agent.V2.Core.Configuration // see the AuthCertificateRejected/DeviceNotRegistered distress signal // without 100 s of needless retry delay. _logger.Warning($"Config fetch authentication failed: {ex.Message}"); - _authFailureTracker?.RecordFailure(ex.StatusCode, "agent/config"); + _authFailureTracker?.RecordFailure(ex.StatusCode, "agent/config", ex.EndpointUnavailable); lastException = ex; LastFetchAuthStatusCode = ex.StatusCode; break; diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core/Security/AuthFailureTracker.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core/Security/AuthFailureTracker.cs index cfa07083..6c5cf5c8 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2.Core/Security/AuthFailureTracker.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core/Security/AuthFailureTracker.cs @@ -46,6 +46,7 @@ namespace AutopilotMonitor.Agent.V2.Core.Security private int _consecutiveFailures; private DateTime? _firstFailureUtc; private int _thresholdFired; // 0/1 — Interlocked exchange makes the event single-shot + private int _distressDispatched; // 0/1 per failure streak — reset by RecordSuccess public AuthFailureTracker( int maxFailures, @@ -77,18 +78,27 @@ namespace AutopilotMonitor.Agent.V2.Core.Security public void RecordSuccess() { if (Interlocked.Exchange(ref _consecutiveFailures, 0) == 0) return; // no-op if already zero + Interlocked.Exchange(ref _distressDispatched, 0); lock (_windowLock) { _firstFailureUtc = null; } } /// - /// Record a 401/403 from the given . V1 parity — on the FIRST - /// failure this fires a single distress report via the constructor-injected - /// (401 → AuthCertificateRejected, 403 → DeviceNotRegistered). - /// Subsequent failures are logged + counted but do not dispatch further distress signals. - /// Emits the first time either ceiling is crossed; - /// subsequent calls after termination is armed are no-ops. + /// Record a 401/403 from the given . V1 parity — the first + /// qualifying failure of a streak fires a single distress report via the + /// constructor-injected (401 → AuthCertificateRejected, + /// 403 → DeviceNotRegistered). Subsequent failures are logged + counted but do not + /// dispatch further distress signals. Emits the first + /// time either ceiling is crossed; subsequent calls after termination is armed are no-ops. /// - public void RecordFailure(int statusCode, string operation) + /// + /// True when the 401/403 was a platform-level HTML response (stopped/retired app, edge + /// proxy — see ). Suppresses the + /// distress dispatch: the report would go to the very endpoint that just answered with a + /// platform error page, and its DeviceNotRegistered classification would be wrong anyway. + /// Counting and the shutdown thresholds are unaffected. Should a later failure in the + /// same streak come from a live backend, that one still dispatches the streak's report. + /// + public void RecordFailure(int statusCode, string operation, bool endpointUnavailable = false) { if (Volatile.Read(ref _thresholdFired) == 1) return; @@ -102,10 +112,21 @@ namespace AutopilotMonitor.Agent.V2.Core.Security firstFailureAt = _firstFailureUtc.Value; } - // V1 parity — only the first consecutive failure produces a distress report. Repeat - // hits at 2..N use the local log only; the backend already knows from the first - // report that the device is unauthorized. - if (count == 1 && _distressReporter != null) + // V1 parity — only one distress report per failure streak. Repeat hits use the local + // log only; the backend already knows from the first report that the device is + // unauthorized. Endpoint-unavailable failures never dispatch (nothing to send to) but + // also never consume the streak's report slot. + if (endpointUnavailable) + { + if (_distressReporter != null && Volatile.Read(ref _distressDispatched) == 0) + { + _logger.Info( + $"AuthFailureTracker: distress report suppressed for {operation} (http {statusCode}) — " + + "endpoint unavailable (platform error page), a report cannot reach a stopped endpoint."); + } + } + else if (_distressReporter != null && + Interlocked.CompareExchange(ref _distressDispatched, 1, 0) == 0) { var distressType = statusCode == 403 ? DistressErrorType.DeviceNotRegistered diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core/Security/SessionRegistrationHelper.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core/Security/SessionRegistrationHelper.cs index 578a5763..7a867bef 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2.Core/Security/SessionRegistrationHelper.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core/Security/SessionRegistrationHelper.cs @@ -89,7 +89,7 @@ namespace AutopilotMonitor.Agent.V2.Core.Security // Feed the central auth-failure tracker so the first-failure distress dispatches // and the shutdown threshold advances. V1 parity: no retry on auth-failure — // backend has definitively rejected the cert/token. - authFailureTracker?.RecordFailure(ex.StatusCode, "agent/register-session"); + authFailureTracker?.RecordFailure(ex.StatusCode, "agent/register-session", ex.EndpointUnavailable); return SessionRegistrationResult.AuthFailed(ex.StatusCode, ex.Message); } catch (Exception ex)