fix(sql): stop leaking provider exception text; de-dup health classification; guard Faulted

C1: SqlDriver's Initialize/Read failure paths no longer interpolate the ADO.NET
provider's ex.Message into LastError, the thrown message, or host status — only
ex.GetType().Name reaches the operator surface; the full exception goes to the log
sink via the structured logger's exception parameter. The driver is dialect-agnostic
over an arbitrary DbProviderFactory, so it cannot rely on any provider's message
discipline (Microsoft.Data.SqlClient's parser echoes an unrecognised keyword
lower-cased, which a value-based redactor misses).

C1a: replace the vacuous credential test (SQLite's "unable to open" message never
contains the connection string, so it passed regardless) with one that fabricates a
DbException whose own .Message carries a credential token and drives it through the
Initialize liveness-failure path via the injectable factory seam — red against the
pre-fix ex.Message interpolation.

I1: HandlePollError now ignores the DbException class ReadAsync already classified,
so a subscribed-read outage is classified (and logged) once, not twice with the
worse message.

I2: the poll's Healthy verdict routes through a shared SetHealthUnlessFaulted guard
(the one Degrade already used), so a late in-flight poll cannot un-fault a driver a
concurrent ReinitializeAsync just faulted.

I3: DiscoverAsync warns when materializing an omitted-type tag as String, the only
operator signal for the declared-vs-published type mismatch on a numeric column.

M1: BuildTagTable wraps the per-entry parse so a stray non-JsonException throw skips
the tag instead of stranding health at Initializing (it runs before Initialize's
try/catch). M2 left as a documented TODO to avoid touching SqlPollReader.

Claude-Session: https://claude.ai/code/session_01GASWkNEi68FSCtvr6rLoEW

src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlDriver.cs

@@ -220,10 +220,15 @@ public sealed class SqlDriver
         }
         catch (Exception ex)
         {
-            // Never interpolate _connectionString — Endpoint is the credential-free rendering.
+            // Never interpolate _connectionString OR ex.Message onto the operator surface. The driver is
+            // dialect-agnostic over an arbitrary DbProviderFactory, so it cannot rely on any one provider's
+            // message discipline — Microsoft.Data.SqlClient's connection-string parser, for one, echoes an
+            // unrecognised keyword lower-cased, which a value-based password redactor misses entirely. Only
+            // the exception TYPE reaches LastError/host status; the full exception object goes to the log
+            // SINK via the structured logger's exception parameter (below), never into a rendered string.
             var message =
-                $"Sql driver '{_driverInstanceId}' could not reach {Endpoint}: {ex.Message} " +
+                $"Sql driver '{_driverInstanceId}' could not reach {Endpoint}: {ex.GetType().Name}. " +
-                $"Check the database is up, the network path is open, and the configured credentials are valid.";
+                $"Check the database is reachable and the connection string is valid.";
             _logger.LogError(ex, "Sql driver {DriverInstanceId} liveness check against {Endpoint} failed.",
                 _driverInstanceId, Endpoint);
             WriteHealth(new DriverHealth(DriverState.Faulted, null, message));
@@ -296,6 +301,20 @@ public sealed class SqlDriver
         var folder = builder.Folder(DriverTypeName, DriverTypeName);
         foreach (var tag in Tags.Values)
         {
+            if (tag.DeclaredType is null)
+            {
+                // Discovery cannot infer a column's real type — that needs live result-set metadata, which
+                // only a poll returns — so an omitted-type tag materialises as String (the same fallback
+                // ISqlDialect.MapColumnType uses) while the reader coerces and publishes the column's actual,
+                // possibly numeric, CLR value against that String node. Nothing else signals the operator to
+                // this declared-vs-published mismatch, so warn and point them at the fix.
+                _logger.LogWarning(
+                    "Sql tag '{Tag}' has no authored \"type\" and is discovered as String; a numeric column " +
+                    "will then publish numeric values against a String node. Author an explicit \"type\" on " +
+                    "numeric tags. Driver={DriverInstanceId}",
+                    tag.Name, _driverInstanceId);
+            }
+
             folder.Variable(tag.Name, tag.Name, new DriverAttributeInfo(
                 FullName: tag.Name,
                 DriverDataType: tag.DeclaredType ?? DriverDataType.String,
@@ -344,7 +363,9 @@ public sealed class SqlDriver
             // The reader throws for one reason only: opening a connection failed (IReadable's contract).
             _logger.LogWarning(ex, "Sql driver {DriverInstanceId} could not reach {Endpoint} during a read.",
                 _driverInstanceId, Endpoint);
-            Degrade($"Sql driver '{_driverInstanceId}' could not reach {Endpoint}: {ex.Message}");
+            // Type name only — never ex.Message (see InitializeAsync for why); the full exception is already
+            // on the log sink via the LogWarning exception parameter above.
+            Degrade($"Sql driver '{_driverInstanceId}' could not reach {Endpoint}: {ex.GetType().Name}.");
             TransitionHostTo(HostState.Stopped);
             throw;
         }
@@ -414,7 +435,7 @@ public sealed class SqlDriver
     ///     cadence rather than backed off from.</para>
     /// </summary>
     /// <param name="snapshots">The snapshots the reader returned for one poll.</param>
-    private void ObservePollOutcome(IReadOnlyList<DataValueSnapshot> snapshots)
+    internal void ObservePollOutcome(IReadOnlyList<DataValueSnapshot> snapshots)
     {
         if (snapshots.Count == 0) return;
 
@@ -447,7 +468,11 @@ public sealed class SqlDriver
 
         if (good == 0) return;   // authoring-class Bad only — not a statement about the database.
 
-        WriteHealth(new DriverHealth(DriverState.Healthy, DateTime.UtcNow, null));
+        // Route through the Faulted guard, not a raw WriteHealth: a poll still in flight when a concurrent
+        // ReinitializeAsync has recorded Faulted (the engine's ~5 s teardown wait is shorter than the 15 s
+        // default operationTimeout, so a stale poll can still complete afterwards) must not flip Faulted back
+        // to Healthy with no real recovery. Only a successful (re)initialize clears Faulted.
+        SetHealthUnlessFaulted(new DriverHealth(DriverState.Healthy, DateTime.UtcNow, null));
         TransitionHostTo(HostState.Running);
     }
 
@@ -464,6 +489,15 @@ public sealed class SqlDriver
     /// <param name="ex">The exception the poll engine caught.</param>
     internal void HandlePollError(Exception ex)
     {
+        // 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.
+        if (ex is DbException) return;
+
         _logger.LogWarning(ex, "Sql poll failed. Driver={DriverInstanceId} Endpoint={Endpoint}",
             _driverInstanceId, Endpoint);
         Degrade(ex.Message);
@@ -478,8 +512,21 @@ public sealed class SqlDriver
     private void Degrade(string reason)
     {
         var current = ReadHealth();
-        if (current.State == DriverState.Faulted) return;
+        SetHealthUnlessFaulted(new DriverHealth(DriverState.Degraded, current.LastSuccessfulRead, reason));
-        WriteHealth(new DriverHealth(DriverState.Degraded, current.LastSuccessfulRead, reason));
+    }
+
+    /// <summary>
+    ///     Publishes <paramref name="value"/> unless the driver is already <see cref="DriverState.Faulted"/>
+    ///     — a liveness/config verdict that only a successful (re)initialize may clear. Every non-Initialize
+    ///     health write routes through here: <see cref="Degrade"/> and the poll's Healthy verdict in
+    ///     <see cref="ObservePollOutcome"/>, so a late in-flight poll cannot un-fault a driver a concurrent
+    ///     <see cref="ReinitializeAsync"/> just faulted.
+    /// </summary>
+    /// <param name="value">The health snapshot to publish when the driver is not Faulted.</param>
+    private void SetHealthUnlessFaulted(DriverHealth value)
+    {
+        if (ReadHealth().State == DriverState.Faulted) return;
+        WriteHealth(value);
     }
 
     /// <summary>Advances <c>LastSuccessfulRead</c> without changing the current state or error.</summary>
@@ -523,6 +570,8 @@ public sealed class SqlDriver
     {
         var table = new Dictionary<string, SqlTagDefinition>(_options.RawTags.Count, StringComparer.Ordinal);
         foreach (var entry in _options.RawTags)
+        {
+            try
             {
                 if (SqlEquipmentTagParser.TryParse(entry.TagConfig, entry.RawPath, out var definition))
                     table[entry.RawPath] = definition;
@@ -531,6 +580,18 @@ public sealed class SqlDriver
                         "Sql tag config did not map to a definition; skipping. Driver={DriverInstanceId} RawPath={RawPath}",
                         _driverInstanceId, entry.RawPath);
             }
+            catch (Exception ex)
+            {
+                // TryParse is contracted never to throw, but BuildTagTable runs BEFORE InitializeAsync's
+                // try/catch, so a stray throw here (a parser bug on one blob) would escape Initialize and
+                // strand health at Initializing across every DriverInstanceActor retry. Skip the offending
+                // tag — the same "one bad tag must not take the whole driver down" rule the malformed-blob
+                // branch above follows — rather than fault the driver over a single entry.
+                _logger.LogError(
+                    ex, "Sql tag config threw while parsing; skipping. Driver={DriverInstanceId} RawPath={RawPath}",
+                    _driverInstanceId, entry.RawPath);
+            }
+        }
 
         Volatile.Write(ref _tagsByRawPath, table);
     }
@@ -618,6 +679,8 @@ public sealed class SqlDriver
     /// <summary>
     ///     Observes an abandoned liveness attempt's eventual fault so it never surfaces as an unobserved
     ///     task exception. The task still owns — and disposes — its own connection.
+    ///     <para>TODO(M2): a verbatim duplicate of <c>SqlPollReader.Detach</c>; both live in Driver.Sql and
+    ///     could hoist to one internal helper. Left in place here to keep this change off the reader.</para>
     /// </summary>
     private static void Detach(Task work)
         => _ = work.ContinueWith(

tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlDriverTests.cs

@@ -1,3 +1,4 @@
+using System.Data;
 using System.Data.Common;
 using System.Globalization;
 using Microsoft.Data.Sqlite;
@@ -342,6 +343,132 @@ public sealed class SqlDriverTests
         }
     }
 
+    // ---- C1a: the credential guarantee, proven load-bearing ----
+
+    [Fact]
+    public async Task Initialize_whenTheProviderExceptionCarriesCredentials_leaksThemIntoNoOperatorSurface()
+    {
+        // The vacuous sibling test drives SQLite's "unable to open database file" path, whose message
+        // structurally never contains the connection string — so it passes with or without any defensive
+        // code. This one fabricates a DbException whose OWN .Message carries a credential-like token and
+        // drives it through Initialize's liveness-failure path via the injectable factory seam. It is RED
+        // against the pre-fix code that interpolated ex.Message into LastError and the thrown message.
+        const string leakyMessage = "login failed for user; Password=" + SecretToken;
+        var driver = new SqlDriver(
+            new SqlDriverOptions
+            {
+                RawTags = [KvEntry("Speed", SqlitePollFixture.PresentKey)],
+                OperationTimeout = TimeSpan.FromSeconds(15),
+                CommandTimeout = TimeSpan.FromSeconds(10),
+            },
+            DriverInstanceId,
+            new SqliteDialect(),
+            // A credential-free connection string: the ONLY place the token can come from is the exception.
+            "Server=sqlsrv01;Initial Catalog=Mes",
+            factory: new ThrowingConnectionFactory(leakyMessage),
+            logger: CapturingLogger.Null);
+        await using var _ = driver;
+
+        var thrown = await Should.ThrowAsync<Exception>(
+            async () => await driver.InitializeAsync(ConfigJson, CancellationToken.None));
+
+        var health = driver.GetHealth();
+        health.State.ShouldBe(DriverState.Faulted);
+        health.LastError.ShouldNotBeNullOrWhiteSpace();
+        // The token appears in NEITHER the operator-facing LastError NOR the thrown message NOR host status.
+        health.LastError!.ShouldNotContain(SecretToken);
+        thrown.Message.ShouldNotContain(SecretToken);
+        driver.GetHostStatuses()[0].HostName.ShouldNotContain(SecretToken);
+        driver.GetHostStatuses()[0].State.ShouldBe(HostState.Stopped);
+        // …and it is still actionable: it names the endpoint the operator has to go look at.
+        health.LastError.ShouldContain("sqlsrv01");
+    }
+
+    // ---- I1: no double health-classification on a subscribed-read outage ----
+
+    [Fact]
+    public async Task HandlePollError_forADbException_defersToReadAsync_soHealthIsClassifiedOnce()
+    {
+        // On a subscribed-read outage the reader throws a DbException; ReadAsync classifies health with a
+        // good, endpoint-bearing message + host Stopped and rethrows the SAME exception, which the poll
+        // engine then routes to HandlePollError. HandlePollError must ignore the DbException class ReadAsync
+        // owns — pre-fix it re-degraded with the bare ex.Message (the WORSE of the two LastErrors, and a
+        // credential-leak path) and logged the outage a second time.
+        using var fixture = new SqlitePollFixture();
+        var logger = new CapturingLogger();
+        await using var driver = NewDriver(fixture, logger, KvEntry("Speed", SqlitePollFixture.PresentKey));
+        await driver.InitializeAsync(ConfigJson, CancellationToken.None);
+        driver.GetHealth().State.ShouldBe(DriverState.Healthy);
+
+        driver.HandlePollError(new FakeDbException("connection failed; Password=" + SecretToken));
+
+        var health = driver.GetHealth();
+        // Untouched: HandlePollError left the DbException to ReadAsync, so it neither degraded nor leaked…
+        health.State.ShouldBe(DriverState.Healthy);
+        health.LastError.ShouldBeNull();
+        // …and it logged nothing (ReadAsync owns the single outage warning).
+        logger.Entries.Count(e => e.Level == LogLevel.Warning).ShouldBe(0);
+    }
+
+    [Fact]
+    public void HandlePollError_forAnEngineContractViolation_stillDegrades()
+    {
+        // The falsifiability control for the guard above: a non-DbException (the poll engine's own
+        // reader-contract-violation throw) is NOT owned by ReadAsync, so it must still reach the health
+        // surface. The guard skips only the DbException connection class.
+        using var fixture = new SqlitePollFixture();
+        var driver = NewDriver(fixture, KvEntry("Speed", SqlitePollFixture.PresentKey));
+
+        driver.HandlePollError(new InvalidOperationException("Reader contract violation: expected 1 snapshots"));
+
+        driver.GetHealth().State.ShouldBe(DriverState.Degraded);
+    }
+
+    // ---- I2: a late poll cannot un-fault a Faulted driver ----
+
+    [Fact]
+    public async Task ObservePollOutcome_whenALateGoodPollLands_cannotUnFaultTheDriver()
+    {
+        // The engine's teardown waits ~5 s for loops to wind down, but a poll may still be in flight up to
+        // the 15 s default operationTimeout — so a poll can complete AFTER a concurrent ReinitializeAsync has
+        // recorded Faulted. If that stale poll reports all-Good it must NOT flip Faulted back to Healthy: only
+        // a successful (re)initialize clears Faulted. RED against the pre-fix unconditional Healthy write.
+        using var fixture = new SqlitePollFixture();
+        await using var driver = NewDriver(
+            fixture, Unreachable(), CapturingLogger.Null, KvEntry("Speed", SqlitePollFixture.PresentKey));
+        await Should.ThrowAsync<Exception>(
+            async () => await driver.InitializeAsync(ConfigJson, CancellationToken.None));
+        driver.GetHealth().State.ShouldBe(DriverState.Faulted);
+
+        // The stale in-flight poll completes with an all-Good batch.
+        driver.ObservePollOutcome(
+            [new DataValueSnapshot(42, SqlStatusCodes.Good, DateTime.UtcNow, DateTime.UtcNow)]);
+
+        driver.GetHealth().State.ShouldBe(DriverState.Faulted);   // stayed Faulted
+    }
+
+    // ---- I3: an omitted-type tag warns at discovery ----
+
+    [Fact]
+    public async Task Discover_warnsForAnOmittedTypeTag_butNotForATypedOne()
+    {
+        using var fixture = new SqlitePollFixture();
+        var logger = new CapturingLogger();
+        await using var driver = NewDriver(
+            fixture, logger,
+            KvEntry("Speed", SqlitePollFixture.PresentKey),                 // no "type" → discovered String
+            TypedKvEntry("Rpm", SqlitePollFixture.PresentKey, "Int32"));    // explicit "type"
+        await driver.InitializeAsync(ConfigJson, CancellationToken.None);
+
+        var capture = new CapturingBuilder();
+        await ((ITagDiscovery)driver).DiscoverAsync(capture, CancellationToken.None);
+
+        logger.Entries.ShouldContain(e =>
+            e.Level == LogLevel.Warning && e.Message.Contains("Speed") && e.Message.Contains("no authored"));
+        logger.Entries.ShouldNotContain(e =>
+            e.Level == LogLevel.Warning && e.Message.Contains("Rpm") && e.Message.Contains("no authored"));
+    }
+
     // ---- helpers ----
 
     /// <summary>A distinctive password token: any assertion that finds it has found a credential leak.</summary>
@@ -417,6 +544,21 @@ public sealed class SqlDriverTests
             }
             """), WriteIdempotent: false);
 
+    /// <summary>A key-value entry carrying an explicit <c>"type"</c> — so <c>DeclaredType</c> is non-null.</summary>
+    private static RawTagEntry TypedKvEntry(string rawPath, string keyValue, string type)
+        => new(rawPath, string.Create(CultureInfo.InvariantCulture, $$"""
+            {
+              "driver": "Sql",
+              "model": "KeyValue",
+              "table": "{{SqlitePollFixture.KeyValueTable}}",
+              "keyColumn": "{{SqlitePollFixture.KeyColumn}}",
+              "keyValue": "{{keyValue}}",
+              "valueColumn": "{{SqlitePollFixture.ValueColumn}}",
+              "timestampColumn": "{{SqlitePollFixture.TimestampColumn}}",
+              "type": "{{type}}"
+            }
+            """), WriteIdempotent: false);
+
     /// <summary>The same tag as <see cref="KvEntry"/>, already typed — for the reference reader.</summary>
     private static SqlTagDefinition KvDefinition(string rawPath, string keyValue)
         => new(rawPath, SqlTagModel.KeyValue, SqlitePollFixture.KeyValueTable,
@@ -487,4 +629,47 @@ public sealed class SqlDriverTests
             public void Dispose() { }
         }
     }
+
+    /// <summary>
+    ///     A <see cref="DbException"/> whose message is under the test's control — stands in for the
+    ///     credential-echoing exceptions a real ADO.NET provider can raise, so the credential-hygiene
+    ///     guarantee can be exercised without a live database.
+    /// </summary>
+    private sealed class FakeDbException(string message) : DbException(message);
+
+    /// <summary>
+    ///     A <see cref="DbProviderFactory"/> whose connections always fail to open, with a
+    ///     caller-supplied (credential-bearing) message — the seam that drives the driver's
+    ///     unreachable-database path with a fabricated provider exception.
+    /// </summary>
+    private sealed class ThrowingConnectionFactory(string openFailureMessage) : DbProviderFactory
+    {
+        public override DbConnection CreateConnection() => new ThrowingConnection(openFailureMessage);
+    }
+
+    /// <summary>A connection that throws <see cref="FakeDbException"/> on open; every other member is inert.</summary>
+    private sealed class ThrowingConnection(string openFailureMessage) : DbConnection
+    {
+        [System.Diagnostics.CodeAnalysis.AllowNull]
+        public override string ConnectionString { get; set; } = string.Empty;
+
+        public override string Database => string.Empty;
+
+        public override string DataSource => string.Empty;
+
+        public override string ServerVersion => string.Empty;
+
+        public override ConnectionState State => ConnectionState.Closed;
+
+        public override void Open() => throw new FakeDbException(openFailureMessage);
+
+        public override void ChangeDatabase(string databaseName) => throw new NotSupportedException();
+
+        public override void Close() { }
+
+        protected override DbTransaction BeginDbTransaction(IsolationLevel isolationLevel)
+            => throw new NotSupportedException();
+
+        protected override DbCommand CreateDbCommand() => throw new NotSupportedException();
+    }
 }