From 3dbfe6ec0a0971b7a2f665b7e59627d50728cf4a Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Wed, 11 Nov 2026 06:24:59 -0500 Subject: [PATCH] fix(security): preflight and /raw banner check the merged driver+device config (#850) The deploy gate's merged-config path check is now one helper, CredentialReferenceDeployChecks.MergedOnlyProblems (problems the runtime merge reveals that the per-config checks do not), shared by the gate and CredentialReferencePreflight.Scan - so an AuthType switch in one config and a UserCertificatePath in the other is listed as a Draft finding '(merged with its devices)' on the banner before a deploy refuses it. Co-Authored-By: Claude --- .../CredentialReferenceDeployChecks.cs | 51 +++++++++++-------- .../CredentialReferencePreflight.cs | 18 ++++++- ...lReferencePreflightInstanceAndPathTests.cs | 42 +++++++++++++++ 3 files changed, 89 insertions(+), 22 deletions(-) diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Validation/CredentialReferenceDeployChecks.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Validation/CredentialReferenceDeployChecks.cs index 7c3f18864..9346a7343 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Validation/CredentialReferenceDeployChecks.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Validation/CredentialReferenceDeployChecks.cs @@ -74,35 +74,44 @@ public static class CredentialReferenceDeployChecks foreach (var problem in DriverCredentialPolicy.DescribeProblems(driver.DriverType, driver.DriverConfig)) errors.Add(new(Code, $"{driver.DriverType} driver '{driver.Name}': {problem}{MigrationHint}", driver.DriverInstanceId)); - var reported = new HashSet(StringComparer.Ordinal); - foreach (var problem in DriverCredentialPolicy.DescribeProblems(driver.DriverType, driver.DriverConfig)) - reported.Add(problem); foreach (var device in devices) { foreach (var problem in DriverCredentialPolicy.DescribeProblems(driver.DriverType, device.DeviceConfig)) - { - reported.Add(problem); errors.Add(new(Code, $"{driver.DriverType} device '{device.Name}': {problem}{MigrationHint}", device.DeviceId)); - } } - // #836 review L-1: a path field is exempt only in an object that explicitly switches it off (OpcUaClient - // AuthType ≠ Certificate), and the switch and the path may sit in different configs (driver vs device). Check - // the config exactly as the runtime merges it, so a split can never deploy a path the driver will load. - if (rules.PathFieldList.Count > 0) - { - var merged = DriverDeviceConfigMerger.Merge( - driver.DriverConfig, - devices.Select(d => new DriverDeviceConfigMerger.DeviceRow(d.Name, d.DeviceConfig)).ToList(), - []); - foreach (var problem in DriverCredentialPolicy.DescribeProblems(driver.DriverType, merged)) - { - if (reported.Add(problem)) - errors.Add(new(Code, $"{driver.DriverType} driver '{driver.Name}' (merged with its devices): {problem}{MigrationHint}", driver.DriverInstanceId)); - } - } + foreach (var problem in MergedOnlyProblems( + driver.DriverType, driver.DriverConfig, devices.Select(d => (d.Name, (string?)d.DeviceConfig)).ToList())) + errors.Add(new(Code, $"{driver.DriverType} driver '{driver.Name}' (merged with its devices): {problem}{MigrationHint}", driver.DriverInstanceId)); } return errors; } + + /// + /// #836 review L-1 / lmxopcua#850: a path field is exempt only in an object that explicitly switches it off + /// (OpcUaClient AuthType ≠ Certificate), and the switch and the path may sit in different configs (driver vs + /// device). This checks the config exactly as the runtime merges it () and + /// returns the problems the merge reveals that the driver and device configs, checked one by one, do not — so a + /// split can never hide a path the driver will load. The ONE implementation the deploy gate and the upgrade + /// preflight / /raw banner share. Empty for a type without path fields. + /// + /// The driver type. + /// The stored DriverConfig. + /// Its devices (name, DeviceConfig). + /// The merged-only problems. + public static IReadOnlyList MergedOnlyProblems( + string? driverType, string? driverConfig, IReadOnlyList<(string Name, string? DeviceConfig)> devices) + { + ArgumentNullException.ThrowIfNull(devices); + if (!DriverCredentialPolicy.TryGet(driverType, out var rules) || rules.PathFieldList.Count == 0) return []; + var reported = new HashSet(DriverCredentialPolicy.DescribeProblems(driverType, driverConfig), StringComparer.Ordinal); + foreach (var (_, deviceConfig) in devices) + reported.UnionWith(DriverCredentialPolicy.DescribeProblems(driverType, deviceConfig)); + var merged = DriverDeviceConfigMerger.Merge( + driverConfig ?? "{}", + devices.Select(d => new DriverDeviceConfigMerger.DeviceRow(d.Name, d.DeviceConfig ?? "{}")).ToList(), + []); + return DriverCredentialPolicy.DescribeProblems(driverType, merged).Where(reported.Add).ToList(); + } } diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Deployments/CredentialReferencePreflight.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Deployments/CredentialReferencePreflight.cs index 45d896c15..ac9cdd2ff 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Deployments/CredentialReferencePreflight.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Deployments/CredentialReferencePreflight.cs @@ -5,6 +5,7 @@ using Microsoft.Extensions.Logging; using ZB.MOM.WW.OtOpcUa.Configuration; using ZB.MOM.WW.OtOpcUa.Configuration.Entities; using ZB.MOM.WW.OtOpcUa.Configuration.Enums; +using ZB.MOM.WW.OtOpcUa.Configuration.Validation; using ZB.MOM.WW.OtOpcUa.Core.Abstractions; using ZB.MOM.WW.OtOpcUa.Runtime.Drivers; @@ -85,13 +86,28 @@ public sealed class CredentialReferencePreflight(IDbContextFactory string.Equals(dev.DriverInstanceId, d.DriverInstanceId, StringComparison.Ordinal)) + .OrderBy(dev => dev.DeviceId, StringComparer.Ordinal) + .Select(dev => (dev.Name, (string?)dev.DeviceConfig)) + .ToList(); + foreach (var p in CredentialReferenceDeployChecks.MergedOnlyProblems(d.DriverType, d.DriverConfig, own)) + findings.Add(new(CredentialFindingSource.Draft, d.DriverType, d.Name, d.DriverInstanceId, null, $"(merged with its devices) {p}")); + } + var deployedList = deployed.ToList(); var deployedInstances = deployedList.Select(s => new DriverInstanceIdentity(s.DriverInstanceId, s.Name, s.DriverType)).ToList(); foreach (var s in deployedList) diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Deployments/CredentialReferencePreflightInstanceAndPathTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Deployments/CredentialReferencePreflightInstanceAndPathTests.cs index d47573f2c..452648a9e 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Deployments/CredentialReferencePreflightInstanceAndPathTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Deployments/CredentialReferencePreflightInstanceAndPathTests.cs @@ -52,4 +52,46 @@ public sealed class CredentialReferencePreflightInstanceAndPathTests f.Problem.ShouldContain("lmxopcua#836"); f.Problem.ShouldNotContain("plant.pfx"); } + + /// + /// lmxopcua#850: the AuthType switch in one config and the path in the other — each config passes alone, the + /// runtime-merged config does not. The deploy gate catches it; the banner must show it before a deploy does. + /// + [Fact] + public void A_path_split_from_its_certificate_switch_is_listed_from_the_merged_config() + { + var findings = CredentialReferencePreflight.Scan( + [Drv("o", "Upstream", DriverTypeNames.OpcUaClient, """{"AuthType":"Anonymous","UserCertificatePath":"C:\\stolen\\host.pfx"}""")], + [new Device { DeviceId = "dev-o", DriverInstanceId = "o", Name = "d", DeviceConfig = """{"AuthType":"Certificate"}""" }], + []); + + var f = findings.ShouldHaveSingleItem(); + f.Source.ShouldBe(CredentialFindingSource.Draft); + f.DriverInstanceId.ShouldBe("o"); + f.DeviceName.ShouldBeNull(); + f.Problem.ShouldContain("merged with its devices"); + f.Problem.ShouldContain("lmxopcua#836"); + f.Problem.ShouldNotContain("host.pfx"); + } + + /// Control: a problem already reported on one config is not listed a second time from the merge. + [Fact] + public void A_path_already_reported_on_its_own_config_is_not_duplicated_by_the_merge() => + CredentialReferencePreflight.Scan( + [Drv("o", "Upstream", DriverTypeNames.OpcUaClient, """{"AuthType":"Certificate","UserCertificatePath":"C:\\certs\\plant.pfx"}""")], + [new Device { DeviceId = "dev-o", DriverInstanceId = "o", Name = "d", DeviceConfig = "{}" }], + []) + .ShouldHaveSingleItem().Problem.ShouldNotContain("merged"); + + /// lmxopcua#849: an env: reference naming another instance is listed like a secret: one. + [Fact] + public void A_cross_instance_env_reference_is_listed() => + CredentialReferencePreflight.Scan( + [ + Drv("DRV-a", "A", DriverTypeNames.Galaxy, "{}"), + Drv("DRV-b", "B", DriverTypeNames.Galaxy, """{"Gateway":{"ApiKeySecretRef":"env:OTOPCUA_GALAXY_DRV_A_KEY"}}"""), + ], + [], + []) + .ShouldHaveSingleItem().Problem.ShouldContain("lmxopcua#849"); }