fix(security): warn-only compatibility for deployed drivers + upgrade preflight (#815)

User decision: save, Test Connect, live browse and the deploy gate keep refusing out-of-namespace credential references, but an ALREADY-DEPLOYED driver must not fault at the upgrade restart or (Galaxy) at the next gateway reconnect, since ReopenAsync re-resolves Gateway.ApiKeySecretRef. - LegacyCredentialReferenceTracker (Core.Abstractions): the one, deprecated, call-site-chosen compatibility path. Running Galaxy (InitializeAsync, ReopenAsync, ack fallback) and OpcUaClient (session open) pass it and keep the pre-#815 semantics (case-insensitive prefixes, Galaxy file:, OpcUaClient literal pass-through) with a rate-limited Warning (driver/field/allowed form, never the value) and a credential-compat:<field> diagnostics entry (both drivers now implement IDriverDiagnostics). Browsers and probes pass none and stay hard-fail; Heidenhain has none. - Deploy-gate message names the migration. - Upgrade preflight: CredentialReferencePreflightService (admin nodes, once after ApplicationStarted, ConfigDb only, resolves nothing) logs a Warning per Draft row / Deployed-artifact finding; the /raw banner shows them plus Running drivers reporting warn-only use (driver-health diagnostics). - docs/security.md: exact replacement table (incl. the old form default env:MX_API_KEY, file:, un-namespaced secret:, casing changes), the ReopenAsync reconnect behaviour, the warn-only window and its removal. Co-Authored-By: Claude <noreply@anthropic.com>
2026-11-11 02:07:33 -05:00
parent 9742a66a9e
commit 4563940792
22 changed files with 980 additions and 34 deletions
@@ -32,6 +32,7 @@ public sealed class DraftValidatorCredentialReferenceTests
        e.Context.ShouldBe("drv");
        e.Message.ShouldContain("secret:");
        e.Message.ShouldContain("lmxopcua#815");
+        e.Message.ShouldContain("Migrate it before deploying");
        e.Message.ShouldNotContain("ConnectionStrings__ConfigDb");
    }

@@ -54,6 +54,23 @@ public sealed class GalaxyDriverBrowserTests
        ex.Message.ShouldContain("ClientName");
    }

+    /// <summary>
+    ///     lmxopcua#815: the browser runs on the admin node against an operator-chosen endpoint with no deploy, so it
+    ///     never gets the running driver's warn-only compatibility — a legacy ref (the old form default
+    ///     <c>env:MX_API_KEY</c>, a <c>file:</c> path, another subsystem's secret) is refused before any connect.
+    /// </summary>
+    [Theory]
+    [InlineData("env:MX_API_KEY")]
+    [InlineData("file:appsettings.json")]
+    [InlineData("secret:otopcua/historian/api-key")]
+    public async Task OpenAsync_refuses_an_out_of_namespace_api_key_ref_with_no_warn_only_fallback(string reference)
+    {
+        var json = "{\"Gateway\":{\"Endpoint\":\"http://127.0.0.1:1\",\"ApiKeySecretRef\":\"" + reference
+                   + "\"},\"MxAccess\":{\"ClientName\":\"X\"},\"Repository\":{},\"Reconnect\":{}}";
+        await Should.ThrowAsync<ZB.MOM.WW.OtOpcUa.Core.Abstractions.CredentialReferenceException>(
+            () => _sut.OpenAsync(json, TestContext.Current.CancellationToken));
+    }
+
    /// <summary>A JSON literal that deserializes to null must fail fast with a
    /// "deserialized to null" message — never a downstream NRE.</summary>
    [Fact]
@@ -0,0 +1,146 @@
+using Microsoft.Extensions.Logging;
+using Shouldly;
+using Xunit;
+using ZB.MOM.WW.OtOpcUa.Core.Abstractions;
+using ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Config;
+using ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Runtime;
+using ZB.MOM.WW.Secrets.Abstractions;
+
+namespace ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests;
+
+/// <summary>
+///     lmxopcua#815 warn-only compatibility (deprecated): an ALREADY-DEPLOYED Galaxy driver whose config predates the
+///     namespaces — e.g. the old AdminUI form default <c>env:MX_API_KEY</c> — keeps resolving it on init and on every
+///     gateway reconnect (<c>ReopenAsync</c> → <c>BuildClientOptionsAsync</c>), warning rate-limited and surfacing it
+///     on the diagnostics bag; while the admin-node paths (browser) stay hard-fail.
+/// </summary>
+public sealed class GalaxyDriverLegacyCredentialTests
+{
+    private const string LegacyVariable = "MX_API_KEY_815_TEST";
+
+    private static GalaxyDriverOptions Opts(string apiKeyRef, string? ackRef = null) => new(
+        new GalaxyGatewayOptions("https://mxgw.test:5001", apiKeyRef),
+        new GalaxyMxAccessOptions("Compat815"),
+        new GalaxyRepositoryOptions(WatchDeployEvents: false),
+        new GalaxyReconnectOptions())
+    {
+        AlarmAck = ackRef is null ? new GalaxyAlarmAckOptions() : new GalaxyAlarmAckOptions("ack-svc", null, ackRef),
+    };
+
+    [Fact]
+    public async Task A_running_driver_with_a_legacy_env_ref_resolves_on_init_and_every_reconnect_and_warns_once()
+    {
+        Environment.SetEnvironmentVariable(LegacyVariable, "the-real-key");
+        try
+        {
+            var logger = new CaptureLogger();
+            var driver = new GalaxyDriver("gx-legacy", Opts($"env:{LegacyVariable}"), new NullResolver(), logger);
+            var ct = TestContext.Current.CancellationToken;
+
+            // InitializeAsync and each ReopenAsync (gateway reconnect) build the client options through this one method.
+            (await driver.BuildClientOptionsAsync(Opts($"env:{LegacyVariable}").Gateway, ct)).ApiKey.ShouldBe("the-real-key");
+            (await driver.BuildClientOptionsAsync(Opts($"env:{LegacyVariable}").Gateway, ct)).ApiKey.ShouldBe("the-real-key");
+            (await driver.BuildClientOptionsAsync(Opts($"env:{LegacyVariable}").Gateway, ct)).ApiKey.ShouldBe("the-real-key");
+
+            var warnings = logger.Entries.Where(e => e.Level == LogLevel.Warning && e.Message.Contains("lmxopcua#815")).ToList();
+            warnings.Count.ShouldBe(1); // rate-limited: one per field per hour, not one per reconnect
+            warnings[0].Message.ShouldContain("Gateway.ApiKeySecretRef");
+            warnings[0].Message.ShouldContain("env:OTOPCUA_GALAXY_<NAME>");
+            warnings[0].Message.ShouldNotContain("the-real-key");
+
+            var diagnostics = ((IDriverDiagnostics)driver).GetDiagnostics();
+            diagnostics.Keys.ShouldContain(LegacyCredentialReferenceTracker.DiagnosticsKeyPrefix + "Gateway.ApiKeySecretRef");
+            diagnostics.Values.ShouldAllBe(v => !v.Contains("the-real-key"));
+            DriverDiagnostics.Bound(diagnostics).Values.ShouldAllBe(v => v.Length <= DriverDiagnostics.MaxValueLength);
+        }
+        finally
+        {
+            Environment.SetEnvironmentVariable(LegacyVariable, null);
+        }
+    }
+
+    [Fact]
+    public async Task A_running_driver_keeps_the_old_case_insensitive_and_file_semantics()
+    {
+        var path = Path.Combine(Path.GetTempPath(), $"gx-815-{Guid.NewGuid():N}.txt");
+        File.WriteAllText(path, "  key-from-file \n");
+        try
+        {
+            var driver = new GalaxyDriver("gx-legacy-file", Opts($"file:{path}"), new NullResolver());
+            (await driver.BuildClientOptionsAsync(Opts($"file:{path}").Gateway, TestContext.Current.CancellationToken))
+                .ApiKey.ShouldBe("key-from-file");
+            ((IDriverDiagnostics)driver).GetDiagnostics().ShouldNotBeEmpty();
+        }
+        finally
+        {
+            File.Delete(path);
+        }
+    }
+
+    [Fact]
+    public async Task A_namespaced_ref_resolves_without_a_compatibility_warning()
+    {
+        var logger = new CaptureLogger();
+        var driver = new GalaxyDriver("gx-ok", Opts("dev:k"), new NullResolver(), logger);
+        (await driver.BuildClientOptionsAsync(Opts("dev:k").Gateway, TestContext.Current.CancellationToken)).ApiKey.ShouldBe("k");
+        logger.Entries.ShouldNotContain(e => e.Message.Contains("lmxopcua#815"));
+        ((IDriverDiagnostics)driver).GetDiagnostics().ShouldBeEmpty();
+    }
+
+    /// <summary>The same legacy ref through the admin-node browser path (no tracker) is refused before any read.</summary>
+    [Fact]
+    public async Task The_same_ref_without_a_tracker_is_refused()
+    {
+        var reads = 0;
+        await Should.ThrowAsync<CredentialReferenceException>(() => GalaxySecretRef.ResolveApiKeyAsync(
+            "env:MX_API_KEY", new NullResolver(), ct: TestContext.Current.CancellationToken,
+            environment: _ => { reads++; return "the-real-key"; }));
+        reads.ShouldBe(0);
+    }
+
+    [Fact]
+    public async Task The_running_ack_fallback_keeps_a_legacy_ref_and_warns()
+    {
+        var warnings = new List<string>();
+        var tracker = new LegacyCredentialReferenceTracker(DriverCredentialPolicy.GalaxyScope, "gx", warnings.Add);
+        var resolver = new GalaxyAckCredentialResolver(
+            "gx", new GalaxyAlarmAckOptions("ack-svc", null, "env:LEGACY_ACK_PW"), new NullResolver(), new CaptureLogger(),
+            n => n == "LEGACY_ACK_PW" ? "pw" : null, tracker);
+
+        (await resolver.ResolveAsync(null, TestContext.Current.CancellationToken)).Credential.ShouldNotBeNull().Password.ShouldBe("pw");
+        warnings.ShouldHaveSingleItem().ShouldContain("AlarmAck.FallbackPasswordSecretRef");
+    }
+
+    [Fact]
+    public void The_tracker_re_warns_after_the_interval()
+    {
+        var now = new DateTime(2026, 11, 11, 0, 0, 0, DateTimeKind.Utc);
+        var warnings = new List<string>();
+        var tracker = new LegacyCredentialReferenceTracker(DriverCredentialPolicy.GalaxyScope, "gx", warnings.Add, () => now);
+        tracker.Report("F");
+        tracker.Report("F");
+        now += LegacyCredentialReferenceTracker.WarnInterval;
+        tracker.Report("F");
+        tracker.Report("G");
+        warnings.Count.ShouldBe(3);
+    }
+
+    private sealed class NullResolver : ISecretResolver
+    {
+        public Task<string?> GetAsync(SecretName name, CancellationToken ct) => Task.FromResult<string?>(null);
+    }
+
+    private sealed class CaptureLogger : ILogger<GalaxyDriver>
+    {
+        public List<(LogLevel Level, string Message)> Entries { get; } = new();
+
+        public IDisposable? BeginScope<TState>(TState state) where TState : notnull => null;
+
+        public bool IsEnabled(LogLevel logLevel) => true;
+
+        public void Log<TState>(LogLevel logLevel, EventId eventId, TState state, Exception? exception, Func<TState, Exception?, string> formatter)
+        {
+            lock (Entries) Entries.Add((logLevel, formatter(state, exception)));
+        }
+    }
+}
@@ -124,6 +124,35 @@ public sealed class OpcUaClientSecretResolutionTests
        resolved.Password.ShouldBe("pw");
    }

+    /// <summary>
+    ///     lmxopcua#815 warn-only compatibility (deprecated): the RUNNING driver's session open keeps the old semantics
+    ///     for an out-of-namespace value — a case-insensitive <c>secret:</c> resolves, anything else is a literal — and
+    ///     warns once; the browser (no tracker) refuses the same value (see the theory above).
+    /// </summary>
+    [Fact]
+    public async Task The_running_driver_keeps_old_semantics_for_a_legacy_value_and_warns()
+    {
+        var warnings = new List<string>();
+        var tracker = new LegacyCredentialReferenceTracker(DriverCredentialPolicy.OpcUaClientScope, "ouc", warnings.Add);
+        var legacyOptions = new OpcUaClientDriverOptions
+        {
+            Password = $"SECRET:{KnownPwName}",      // pre-#815: case-insensitive secret: resolved
+            UserCertificatePassword = "env:NOT_A_REF", // pre-#815: only secret: resolved — a literal password
+        };
+
+        for (var i = 0; i < 3; i++)
+        {
+            var resolved = await OpcUaClientSecretResolution.ResolveSecretRefsAsync(
+                legacyOptions, FakeResolver(), CancellationToken.None, tracker);
+            resolved.Password.ShouldBe(KnownPwValue);
+            resolved.UserCertificatePassword.ShouldBe("env:NOT_A_REF");
+        }
+
+        warnings.Count.ShouldBe(2); // one per field, rate-limited across the three session opens
+        warnings.ShouldAllBe(w => !w.Contains(KnownPwValue));
+        tracker.Diagnostics.Count.ShouldBe(2);
+    }
+
    private sealed class CountingResolver : ISecretResolver
    {
        public int Calls { get; private set; }
@@ -0,0 +1,177 @@
+using System.Text;
+using Microsoft.Extensions.DependencyInjection;
+using Microsoft.Extensions.Hosting;
+using Microsoft.Extensions.Logging;
+using Shouldly;
+using Xunit;
+using ZB.MOM.WW.OtOpcUa.AdminUI.Deployments;
+using ZB.MOM.WW.OtOpcUa.AdminUI.Tests.Uns;
+using ZB.MOM.WW.OtOpcUa.Commons.Messages.Drivers;
+using ZB.MOM.WW.OtOpcUa.Configuration;
+using ZB.MOM.WW.OtOpcUa.Configuration.Entities;
+using ZB.MOM.WW.OtOpcUa.Core.Abstractions;
+
+namespace ZB.MOM.WW.OtOpcUa.AdminUI.Tests.Deployments;
+
+/// <summary>
+///     lmxopcua#815 upgrade preflight: stored (draft) and deployed configs with out-of-namespace credential references
+///     are listed and logged — driver, device, field, allowed form — and never the value; nothing is resolved.
+/// </summary>
+[Trait("Category", "Unit")]
+public sealed class CredentialReferencePreflightTests
+{
+    private static CancellationToken Ct => TestContext.Current.CancellationToken;
+
+    private const string LegacyGalaxy = """{"Gateway":{"Endpoint":"http://gw:5001","ApiKeySecretRef":"env:MX_API_KEY"}}""";
+
+    private static string SeedLegacy()
+    {
+        var name = RawTreeTestDb.SeedFresh();
+        using var db = RawTreeTestDb.CreateNamed(name);
+        // Written straight to the DB, as a pre-#815 build would have — the save gate now refuses it.
+        db.DriverInstances.Add(new DriverInstance
+        {
+            DriverInstanceId = "gx-old", ClusterId = RawTreeTestDb.ClusterId, Name = "Galaxy main",
+            DriverType = DriverTypeNames.Galaxy, DriverConfig = LegacyGalaxy,
+        });
+        db.Devices.Add(new Device
+        {
+            DeviceId = "ouc-dev", DriverInstanceId = "gx-old", Name = "gw1",
+            DeviceConfig = """{"AlarmAck":{"FallbackPasswordSecretRef":"file:C:\\keys\\ack.txt"}}""",
+        });
+        db.DriverInstances.Add(new DriverInstance
+        {
+            DriverInstanceId = "mb", ClusterId = RawTreeTestDb.ClusterId, Name = "modbus",
+            DriverType = DriverTypeNames.Modbus, DriverConfig = """{"Password":"env:ConnectionStrings__ConfigDb"}""",
+        });
+        var artifact = """
+            {"DriverInstances":[{"DriverInstanceId":"ouc","Name":"Upstream","DriverType":"OpcUaClient",
+              "DriverConfig":"{\"Password\":\"secret:plant/opcua-pw\"}"}]}
+            """;
+        db.Deployments.Add(new Deployment { RevisionHash = "abc", CreatedBy = "t", ArtifactBlob = Encoding.UTF8.GetBytes(artifact) });
+        db.SaveChanges();
+        return name;
+    }
+
+    [Fact]
+    public async Task Scan_lists_draft_rows_devices_and_the_deployed_artifact_without_values()
+    {
+        var findings = await new CredentialReferencePreflight(RawTreeTestDb.Factory(SeedLegacy())).ScanAsync(Ct);
+
+        findings.ShouldContain(f => f.Source == CredentialFindingSource.Draft && f.DriverName == "Galaxy main"
+                                    && f.DeviceName == null && f.Problem.Contains("Gateway.ApiKeySecretRef")
+                                    && f.Problem.Contains("env:OTOPCUA_GALAXY_<NAME>"));
+        findings.ShouldContain(f => f.Source == CredentialFindingSource.Draft && f.DeviceName == "gw1"
+                                    && f.Problem.Contains("AlarmAck.FallbackPasswordSecretRef"));
+        findings.ShouldContain(f => f.Source == CredentialFindingSource.Deployed && f.DriverName == "Upstream"
+                                    && f.Problem.Contains("secret:opcuaclient/<name>"));
+        findings.ShouldNotContain(f => f.DriverInstanceId == "mb"); // not a credential-resolving driver
+        findings.ShouldAllBe(f => !f.Problem.Contains("MX_API_KEY") && !f.Problem.Contains("plant/opcua-pw")
+                                  && !f.Problem.Contains("ack.txt"));
+    }
+
+    [Fact]
+    public async Task A_clean_database_has_no_findings()
+        => (await new CredentialReferencePreflight(RawTreeTestDb.Factory(RawTreeTestDb.SeedFresh())).ScanAsync(Ct)).ShouldBeEmpty();
+
+    [Fact]
+    public async Task The_boot_pass_logs_one_warning_per_finding_and_publishes_the_state()
+    {
+        var services = new ServiceCollection()
+            .AddSingleton(RawTreeTestDb.Factory(SeedLegacy()))
+            .BuildServiceProvider();
+        var state = new CredentialReferencePreflightState();
+        var logger = new CaptureLogger();
+        var svc = new CredentialReferencePreflightService(services, new FakeLifetime(), state, logger) { StartDelay = TimeSpan.Zero };
+
+        var findings = await svc.RunOnceAsync(Ct);
+
+        findings.Count.ShouldBe(3);
+        state.Current.Findings.Count.ShouldBe(3);
+        state.Current.ScannedAtUtc.ShouldNotBeNull();
+        var warnings = logger.Entries.Where(e => e.Level == LogLevel.Warning).ToList();
+        warnings.Count.ShouldBe(3);
+        warnings.ShouldAllBe(w => w.Message.Contains("lmxopcua#815") && !w.Message.Contains("MX_API_KEY"));
+        warnings.ShouldContain(w => w.Message.Contains("Galaxy main") && w.Message.Contains("Gateway.ApiKeySecretRef"));
+    }
+
+    [Fact]
+    public async Task A_node_without_a_config_db_skips_quietly()
+    {
+        var svc = new CredentialReferencePreflightService(
+            new ServiceCollection().BuildServiceProvider(), new FakeLifetime(), new CredentialReferencePreflightState(), new CaptureLogger());
+        (await svc.RunOnceAsync(Ct)).ShouldBeEmpty();
+    }
+
+    [Fact]
+    public async Task The_hosted_service_waits_for_ApplicationStarted_before_scanning()
+    {
+        var services = new ServiceCollection().AddSingleton(RawTreeTestDb.Factory(SeedLegacy())).BuildServiceProvider();
+        var lifetime = new FakeLifetime();
+        var state = new CredentialReferencePreflightState();
+        var svc = new CredentialReferencePreflightService(services, lifetime, state, new CaptureLogger()) { StartDelay = TimeSpan.Zero };
+
+        await svc.StartAsync(Ct);
+        await Task.Delay(100, Ct);
+        state.Current.ScannedAtUtc.ShouldBeNull(); // nothing before the host has started
+
+        lifetime.Start();
+        var deadline = DateTime.UtcNow.AddSeconds(10);
+        while (state.Current.ScannedAtUtc is null && DateTime.UtcNow < deadline) await Task.Delay(20, Ct);
+        state.Current.Findings.Count.ShouldBe(3);
+        await svc.StopAsync(Ct);
+    }
+
+    [Fact]
+    public void Running_drivers_reporting_warn_only_resolution_are_surfaced_from_driver_health()
+    {
+        var snapshots = new[]
+        {
+            new DriverHealthChanged("c", "gx-old", "Healthy", null, null, 0, DateTime.UtcNow,
+                Diagnostics: new Dictionary<string, string>
+                {
+                    [LegacyCredentialReferenceTracker.DiagnosticsKeyPrefix + "Gateway.ApiKeySecretRef"] = "deprecated …",
+                    ["s7.pdu"] = "480",
+                }),
+            new DriverHealthChanged("c", "mb", "Healthy", null, null, 0, DateTime.UtcNow),
+        };
+
+        var running = CredentialReferencePreflight.RunningFindings(snapshots);
+
+        running.ShouldHaveSingleItem().ShouldSatisfyAllConditions(
+            f => f.Source.ShouldBe(CredentialFindingSource.Running),
+            f => f.DriverInstanceId.ShouldBe("gx-old"),
+            f => f.Problem.ShouldStartWith("Gateway.ApiKeySecretRef"));
+    }
+
+    private sealed class FakeLifetime : IHostApplicationLifetime
+    {
+        private readonly CancellationTokenSource _started = new();
+
+        public CancellationToken ApplicationStarted => _started.Token;
+
+        public CancellationToken ApplicationStopping => CancellationToken.None;
+
+        public CancellationToken ApplicationStopped => CancellationToken.None;
+
+        public void Start() => _started.Cancel();
+
+        public void StopApplication()
+        {
+        }
+    }
+
+    private sealed class CaptureLogger : ILogger<CredentialReferencePreflightService>
+    {
+        public List<(LogLevel Level, string Message)> Entries { get; } = new();
+
+        public IDisposable? BeginScope<TState>(TState state) where TState : notnull => null;
+
+        public bool IsEnabled(LogLevel logLevel) => true;
+
+        public void Log<TState>(LogLevel logLevel, EventId eventId, TState state, Exception? exception, Func<TState, Exception?, string> formatter)
+        {
+            lock (Entries) Entries.Add((logLevel, formatter(state, exception)));
+        }
+    }
+}
@@ -17,6 +17,7 @@ public sealed class AdminOperationsActorCredentialScopeTests : ControlPlaneActor
 {
    [Theory]
    [InlineData(DriverTypeNames.Galaxy, """{"Gateway":{"Endpoint":"http://attacker:5001","ApiKeySecretRef":"env:ConnectionStrings__ConfigDb"}}""")]
+    [InlineData(DriverTypeNames.Galaxy, """{"Gateway":{"Endpoint":"http://attacker:5001","ApiKeySecretRef":"env:MX_API_KEY"}}""")]
    [InlineData(DriverTypeNames.Galaxy, """{"Gateway":{"Endpoint":"http://attacker:5001","ApiKeySecretRef":"env:ServerHistorian__ApiKey"}}""")]
    [InlineData(DriverTypeNames.Galaxy, """{"Gateway":{"Endpoint":"http://attacker:5001","ApiKeySecretRef":"file:appsettings.json"}}""")]
    [InlineData(DriverTypeNames.OpcUaClient, """{"EndpointUrl":"opc.tcp://attacker:4840","Password":"secret:otopcua/historian/api-key"}""")]