From 585f827ef3cfb28a044d4dcd511039dc000ad865 Mon Sep 17 00:00:00 2001
From: Joseph Doherty <dohejw01@gmail.com>
Date: Fri, 24 Jul 2026 15:39:14 -0400
Subject: [PATCH] fix(sql): close the residual credential leak in
 HandlePollError
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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
---
 .../ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlDriver.cs | 12 ++++++-----
 .../SqlDriverTests.cs                         | 21 +++++++++++++++++++
 2 files changed, 28 insertions(+), 5 deletions(-)

diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlDriver.cs
index 7c566ebe..9d2396bf 100644
--- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlDriver.cs
+++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlDriver.cs
@@ -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>
diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlDriverTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlDriverTests.cs
index 9b7a5bf8..9533b8bf 100644
--- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlDriverTests.cs
+++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlDriverTests.cs
@@ -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]