fix(sql): close the residual credential leak in HandlePollError
The C1 credential-hygiene fix converted InitializeAsync and ReadAsync to
surface only ex.GetType().Name, but left HandlePollError building its
operator-facing Degrade() message from raw ex.Message, gated only by
'if (ex is DbException) return;'. That guard exists for the I1
double-classification concern, not for message safety, and it is incomplete
for the leak vector: a malformed connection string throws ArgumentException
from the keyword parser (the same unquoted-';'-in-password shape the sibling
SqlDriverBrowser.Sanitize special-cases), which is not a DbException and so
reaches Degrade(ex.Message) unredacted.
It is unreachable in today's control flow only because the connection string
is static and validated identically at Initialize first — an emergent
property, not an enforced invariant, and a live per-poll leak the moment a
future edit (refresh-on-reinit, a different provider) breaks that assumption.
Make the fallback type-only like the other two sites; the full exception still
reaches the log sink via the exception parameter. The I1 defer + control tests
are unaffected (they assert Degraded, not message text). Pinned by a new test
driving a credential-bearing ArgumentException through HandlePollError;
verified load-bearing (red against Degrade(ex.Message), green after).
Closes the C1 review finding on commit 37f444b9.
Claude-Session: https://claude.ai/code/session_01GASWkNEi68FSCtvr6rLoEW
This commit is contained in:
@@ -490,15 +490,17 @@ public sealed class SqlDriver
|
||||
// A subscribed-read outage surfaces as a DbException the reader threw, which ReadAsync has ALREADY
|
||||
// classified — good, endpoint-bearing LastError + host Stopped — before rethrowing. The poll engine
|
||||
// then routes that same exception here. Re-degrading would overwrite the good LastError with a worse,
|
||||
// guidance-free one (and, for a provider whose text echoes credentials, re-open the C1 leak) and log
|
||||
// the outage a second time. So skip the connection class ReadAsync owns; the only exceptions that
|
||||
// reach past this guard are the engine's own contract-violation throws — an InvalidOperationException
|
||||
// carrying no provider text — which nothing else classifies.
|
||||
// guidance-free one and log the outage a second time, so skip the connection class ReadAsync owns.
|
||||
if (ex is DbException) return;
|
||||
|
||||
// Whatever reaches here — an engine contract-violation, or a non-DbException the provider raised
|
||||
// (e.g. an ArgumentException from a malformed connection string, which the parser echoes with
|
||||
// credential fragments) — the operator surface carries only the exception TYPE, never ex.Message.
|
||||
// The full exception, with its text, still reaches the log SINK via the exception parameter. This is
|
||||
// the same unconditional rule InitializeAsync/ReadAsync follow; do not weaken it to a type check.
|
||||
_logger.LogWarning(ex, "Sql poll failed. Driver={DriverInstanceId} Endpoint={Endpoint}",
|
||||
_driverInstanceId, Endpoint);
|
||||
Degrade(ex.Message);
|
||||
Degrade($"Sql driver '{_driverInstanceId}' poll failed: {ex.GetType().Name}.");
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
|
||||
@@ -424,6 +424,27 @@ public sealed class SqlDriverTests
|
||||
driver.GetHealth().State.ShouldBe(DriverState.Degraded);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void HandlePollError_forANonDbExceptionCarryingCredentials_degradesWithoutLeakingThem()
|
||||
{
|
||||
// The residual C1 leg HandlePollError owns: a NON-DbException raised from provider interaction —
|
||||
// canonically the ArgumentException a malformed connection string's keyword parser throws, echoing
|
||||
// credential fragments (the exact unquoted-';'-in-password vector this whole fix exists for). It is
|
||||
// not the DbException class ReadAsync owns, so it reaches Degrade — and must arrive TYPE-only, never
|
||||
// ex.Message. Pre-fix this degraded with the raw message and leaked the token.
|
||||
using var fixture = new SqlitePollFixture();
|
||||
var driver = NewDriver(fixture, KvEntry("Speed", SqlitePollFixture.PresentKey));
|
||||
|
||||
driver.HandlePollError(new ArgumentException(
|
||||
"Keyword not supported: 'secrettail;connect timeout'. Password=" + SecretToken));
|
||||
|
||||
var health = driver.GetHealth();
|
||||
health.State.ShouldBe(DriverState.Degraded);
|
||||
health.LastError.ShouldNotBeNull();
|
||||
health.LastError.ShouldNotContain(SecretToken); // never the message text
|
||||
health.LastError.ShouldContain(nameof(ArgumentException)); // but still actionable (the type)
|
||||
}
|
||||
|
||||
// ---- I2: a late poll cannot un-fault a Faulted driver ----
|
||||
|
||||
[Fact]
|
||||
|
||||
Reference in New Issue
Block a user