From 50987c96adec3ed89e619e96373e8cf78dcb7171 Mon Sep 17 00:00:00 2001 From: "benjamin.baker" Date: Sun, 30 Aug 2026 14:30:16 -0400 Subject: [PATCH] Harden endpoint agent command channel against request storms A misconfigured BaseUrl (plaintext http:// against the TLS port) made the command long-poll fail at connect in milliseconds. The worker misread that fast failure as a normal idle hold-window close and re-polled with no delay, spinning into a request storm. Gate the idle-close path on the poll having stayed open at least MinIdlePollDuration, so a sub-second transport failure takes the transient backoff instead; add a MinPollInterval floor so no path can exceed a safe request rate. Also reject a non-loopback http:// BaseUrl at startup with a single clear error, turning a silent runtime flood into a fast fail. Loopback http stays valid for local dev. --- agent/ShopDbEndpointAgent.Core/AgentConfig.cs | 33 +++++++++++++++ .../ShopDbEndpointAgent.Core/CommandWorker.cs | 42 ++++++++++++++++--- agent/ShopDbEndpointAgent/Program.cs | 7 ++++ 3 files changed, 76 insertions(+), 6 deletions(-) diff --git a/agent/ShopDbEndpointAgent.Core/AgentConfig.cs b/agent/ShopDbEndpointAgent.Core/AgentConfig.cs index f8a8b9f9..78bddb68 100644 --- a/agent/ShopDbEndpointAgent.Core/AgentConfig.cs +++ b/agent/ShopDbEndpointAgent.Core/AgentConfig.cs @@ -84,6 +84,39 @@ public sealed class AgentConfig => !string.IsNullOrWhiteSpace(BaseUrl) && !string.IsNullOrWhiteSpace(ApiKey) && ApiKey != "REPLACE_WITH_COLLECTOR_KEY"; + + // Validates the BaseUrl scheme before any request is made. A plaintext + // http:// URL aimed at a real (non-loopback) server is the classic + // misconfiguration that hits the TLS port and used to spin the command + // long-poll into a storm; refusing to start on it turns a silent, noisy + // runtime failure into a single clear startup error. Loopback over http is + // still allowed so a developer running the API on plain http locally works. + // Returns a human-readable problem, or null when the BaseUrl is acceptable. + public string? BaseUrlProblem() + { + if (string.IsNullOrWhiteSpace(BaseUrl)) + return null; + + if (!Uri.TryCreate(BaseUrl.Trim(), UriKind.Absolute, out var uri)) + return $"BaseUrl '{BaseUrl}' is not a valid absolute URL."; + + if (uri.Scheme == Uri.UriSchemeHttps) + return null; + + if (uri.Scheme == Uri.UriSchemeHttp) + { + if (uri.IsLoopback) + return null; + return $"BaseUrl '{BaseUrl}' uses plaintext http:// against a " + + "non-loopback host. The server requires https://; an http:// " + + "URL hits the TLS port and fails every request. Reprovision " + + "BaseUrl in HKLM\\SOFTWARE\\GE\\ShopDB (or appsettings.json) " + + "with an https:// URL."; + } + + return $"BaseUrl '{BaseUrl}' uses unsupported scheme '{uri.Scheme}'. " + + "Use https:// (or http:// only for a loopback dev server)."; + } } // Watched targets for this endpoint. Both lists carry entries with a wire name diff --git a/agent/ShopDbEndpointAgent.Core/CommandWorker.cs b/agent/ShopDbEndpointAgent.Core/CommandWorker.cs index 9389d805..ec6dea0e 100644 --- a/agent/ShopDbEndpointAgent.Core/CommandWorker.cs +++ b/agent/ShopDbEndpointAgent.Core/CommandWorker.cs @@ -24,6 +24,19 @@ public sealed class CommandWorker : BackgroundService // operator has to fix something, so re-polling fast changes nothing. private static readonly TimeSpan PermanentRetryDelay = TimeSpan.FromSeconds(300); + // A genuine idle long-poll close only happens after the server has held the + // request open for most of its hold window (~25s). A connection that drops + // in well under this did not idle out - it failed at connect/TLS (e.g. an + // http:// BaseUrl hitting the TLS port), which must NOT be treated as a + // normal close and fast re-polled. Below this threshold we take the + // transient-failure backoff instead. + private static readonly TimeSpan MinIdlePollDuration = TimeSpan.FromSeconds(5); + + // Hard floor between poll starts. Even if some unforeseen path returns + // immediately every iteration, this caps the loop at a safe request rate so + // a misconfigured agent can never turn into a request storm. + private static readonly TimeSpan MinPollInterval = TimeSpan.FromSeconds(2); + private readonly CommandClient client; private readonly CommandExecutor executor; private readonly ILogger logger; @@ -46,6 +59,7 @@ public sealed class CommandWorker : BackgroundService while (!stoppingToken.IsCancellationRequested) { + var pollStarted = System.Diagnostics.Stopwatch.StartNew(); try { var claim = await client.ClaimNextAsync(hostname, stoppingToken); @@ -64,18 +78,32 @@ public sealed class CommandWorker : BackgroundService "Permanent command-channel error; backing off"); await SafeDelay(PermanentRetryDelay, stoppingToken); } - catch (Exception error) when (IsIdlePollClose(error)) + catch (Exception error) when (IsIdlePollClose(error) + && pollStarted.Elapsed >= MinIdlePollDuration) { - // The server (or a proxy) closed the held-open long poll without - // a command. That is the normal end of a hold window, not a - // failure: log quietly at debug and re-poll immediately. + // The server (or a proxy) held the poll open for most of the + // hold window and then closed it without a command. That is the + // normal end of a hold window, not a failure: log quietly at + // debug and re-poll immediately. logger.LogDebug(error, "Command poll connection closed; re-polling"); } catch (Exception error) { + // Anything else - including a connection/TLS failure that drops + // in under MinIdlePollDuration (e.g. an http:// BaseUrl hitting + // the TLS port) - is a real transient failure. Back off so a + // misconfiguration cannot spin into a request storm. logger.LogWarning(error, "Command poll failed; retrying shortly"); await SafeDelay(TransientRetryDelay, stoppingToken); } + + // Floor the loop rate. On the immediate re-poll paths (204 or a + // genuine idle close) the elapsed time already exceeds this, so it + // adds no latency; it only bites if an iteration returned unusually + // fast, capping the worst case at a safe request rate. + var remaining = MinPollInterval - pollStarted.Elapsed; + if (remaining > TimeSpan.Zero) + await SafeDelay(remaining, stoppingToken); } logger.LogInformation("Command channel stopped"); @@ -114,8 +142,10 @@ public sealed class CommandWorker : BackgroundService // A long poll that the server or an intermediary closes without sending a // command surfaces as an HttpRequestException wrapping an HttpIOException - // (ResponseEnded), or a bare IOException. That is the expected end of a hold - // window, so it should not be treated as a transient failure. + // (ResponseEnded), or a bare IOException. Whether that is a benign hold- + // window close or a fast connect/TLS failure is decided by the caller using + // how long the poll stayed open (see MinIdlePollDuration); this only matches + // the exception shape. private static bool IsIdlePollClose(Exception error) { for (Exception? current = error; current is not null; current = current.InnerException) diff --git a/agent/ShopDbEndpointAgent/Program.cs b/agent/ShopDbEndpointAgent/Program.cs index c97a8839..aa75f556 100644 --- a/agent/ShopDbEndpointAgent/Program.cs +++ b/agent/ShopDbEndpointAgent/Program.cs @@ -46,6 +46,13 @@ if (!config.IsUsable()) return 1; } +var baseUrlProblem = config.BaseUrlProblem(); +if (baseUrlProblem is not null) +{ + bootstrapLogger.LogError("Config rejected: {Problem}", baseUrlProblem); + return 1; +} + try { if (runOnce)