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.
This commit is contained in:
benjamin.baker
2026-08-30 14:30:16 -04:00
committed by cproudlock
parent aa702fb81e
commit 50987c96ad
3 changed files with 76 additions and 6 deletions

View File

@@ -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

View File

@@ -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<CommandWorker> 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)

View File

@@ -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)