diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/DriverCredentialPolicy.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/DriverCredentialPolicy.cs index 308f28bfd..33a238a0e 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/DriverCredentialPolicy.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/DriverCredentialPolicy.cs @@ -211,8 +211,8 @@ public static class DriverCredentialPolicy /// /// The instance-identifying segment of an in-scope secret: reference: the first path segment after the /// matched namespace (secret:galaxy/<X>/apikey ⇒ X; secret:otopcua/galaxy/<X> ⇒ - /// X). Null for anything else — env: references carry no instance convention, and literals and - /// out-of-scope references are the #815 rule's business. + /// X). Null for anything else — env: references use , and literals + /// and out-of-scope references are the #815 rule's business. /// /// The authored field value. /// The driver's scope. @@ -232,24 +232,81 @@ public static class DriverCredentialPolicy } /// - /// True when the config carries at least one in-scope secret: reference — i.e. the cross-instance rule - /// () has something to check, so a seam must load the instance list. - /// Lets seams skip the ConfigDb read otherwise. + /// lmxopcua#849: the part of an in-scope env: reference after the matched (longest) prefix — + /// env:OTOPCUA_GALAXY_<REST> ⇒ REST. The per-instance convention is + /// env:<PREFIX><INSTANCE>_…, INSTANCE being a driver id or name in + /// form; the part NAMES an instance when it equals that form or starts + /// with it followed by _ (see ). Null for anything but an in-scope env:. + /// + /// The authored field value. + /// The driver's scope. + /// The remainder, or null. + public static string? EnvironmentInstancePart(string? value, CredentialScope scope) + { + ArgumentNullException.ThrowIfNull(scope); + if (CredentialReferences.Parse(value, scope, out var name) != CredentialReferenceKind.Environment) return null; + var prefix = scope.EnvVariablePrefixes + .Where(p => name.StartsWith(p, StringComparison.Ordinal)) + .OrderByDescending(p => p.Length) + .FirstOrDefault(); + return prefix is null ? null : name[prefix.Length..]; + } + + /// + /// lmxopcua#849: a driver id or name in environment-variable form — ASCII letters upper-cased, digits kept, every + /// other character _ (DRV-1a2b ⇒ DRV_1A2B, Line 1 ⇒ LINE_1). Empty when the + /// identifier has no letter or digit (such an identifier names no variable). + /// + /// The id or name. + /// The normalised form. + public static string NormalizeEnvironmentIdentifier(string? identifier) + { + if (string.IsNullOrEmpty(identifier) || !identifier.Any(char.IsAsciiLetterOrDigit)) return ""; + return string.Create(identifier.Length, identifier, static (span, id) => + { + for (var i = 0; i < id.Length; i++) + span[i] = char.IsAsciiLetterOrDigit(id[i]) ? char.ToUpperInvariant(id[i]) : '_'; + }); + } + + /// + /// Whether an instance reference () names the driver id or name + /// : a secret: segment equal to it (case-insensitive), or an env: part + /// equal to its form or starting with that form plus _. + /// + /// The reference. + /// A driver id or name. + /// Whether it names it. + public static bool ReferenceNames(InstanceReference reference, string? identifier) + { + if (string.IsNullOrEmpty(identifier)) return false; + if (reference.Kind == CredentialReferenceKind.Secret) + return string.Equals(reference.Part, identifier, StringComparison.OrdinalIgnoreCase); + var normalized = NormalizeEnvironmentIdentifier(identifier); + return normalized.Length > 0 + && (string.Equals(reference.Part, normalized, StringComparison.Ordinal) + || reference.Part.StartsWith(normalized + "_", StringComparison.Ordinal)); + } + + /// + /// True when the config carries at least one in-scope secret: or env: reference — i.e. the + /// cross-instance rule () has something to check, so a seam must load + /// the instance list. Lets seams skip the ConfigDb read otherwise. /// /// The owning driver's type. /// The config JSON. - /// Whether any instance segment is present. + /// Whether any instance reference is present. public static bool HasInstanceSegments(string? driverType, string? json) => - CollectSegments(driverType, json).Count > 0; + CollectInstanceReferences(driverType, json).Count > 0; /// - /// lmxopcua#835: inside one namespace, a reference whose instance-identifying segment (see - /// ) names a DIFFERENT existing instance of the same driver type — by its - /// or its , - /// case-insensitively — is refused: otherwise a Designer could create (or Test Connect / browse) a driver that - /// points at their own endpoint with secret:galaxy/<other-instance>/apikey and capture that - /// instance's key. Shared names that are no other instance's id or name (secret:galaxy/gw/apikey, - /// secret:galaxy/apikey) stay valid, as does a reference naming the instance itself. + /// lmxopcua#835/#849: inside one namespace, a reference that names (see ) a DIFFERENT + /// existing instance of the same driver type — by its or its + /// — is refused: otherwise a Designer could create (or Test Connect / + /// browse) a driver that points at their own endpoint with secret:galaxy/<other-instance>/apikey (or + /// env:OTOPCUA_GALAXY_<OTHER_INSTANCE>_KEY) and capture that instance's key. Shared names that name no + /// other instance (secret:galaxy/gw/apikey, env:GALAXY_MXGW_API_KEY) stay valid, as does a reference + /// naming the instance itself. /// /// /// A match against ANOTHER instance wins over a match against the owner — names are not unique, and renaming @@ -267,24 +324,26 @@ public static class DriverCredentialPolicy ArgumentNullException.ThrowIfNull(instances); var problems = new List(); if (!TryGet(driverType, out var rules)) return problems; - var segments = CollectSegments(driverType, json); - if (segments.Count == 0) return problems; + var references = CollectInstanceReferences(driverType, json); + if (references.Count == 0) return problems; var others = instances .Where(i => string.Equals(i.DriverType, rules.DriverType, StringComparison.OrdinalIgnoreCase) && !string.Equals(i.DriverInstanceId, ownDriverInstanceId, StringComparison.Ordinal)) .ToList(); - foreach (var (fieldPath, segment) in segments) + foreach (var reference in references) { - var other = others.FirstOrDefault(o => - string.Equals(o.DriverInstanceId, segment, StringComparison.OrdinalIgnoreCase) - || string.Equals(o.Name, segment, StringComparison.OrdinalIgnoreCase)); + var other = others.FirstOrDefault(o => ReferenceNames(reference, o.DriverInstanceId) || ReferenceNames(reference, o.Name)); if (other is null) continue; - problems.Add( - $"{fieldPath} names a secret under '{segment}/', which is another {rules.Scope.Label} driver instance " - + $"('{other.Name}', {other.DriverInstanceId}) — a driver may reference only its own secrets " - + $"(secret:{rules.Scope.SecretNamespaces[0]}/…) or a shared name that is no other " - + "instance's id or name (lmxopcua#835)."); + problems.Add(reference.Kind == CredentialReferenceKind.Secret + ? $"{reference.FieldPath} names a secret under '{reference.Part}/', which is another {rules.Scope.Label} driver " + + $"instance ('{other.Name}', {other.DriverInstanceId}) — a driver may reference only its own secrets " + + $"(secret:{rules.Scope.SecretNamespaces[0]}/…) or a shared name that is no other " + + "instance's id or name (lmxopcua#835)." + : $"{reference.FieldPath} names an environment variable of another {rules.Scope.Label} driver instance " + + $"('{other.Name}', {other.DriverInstanceId}) — a driver may reference only its own variables " + + $"(env:{rules.Scope.EnvVariablePrefixes[0]}_…, the id upper-cased with every other " + + "character as '_') or a shared name that starts with no other instance's id or name (lmxopcua#849)."); } return problems; @@ -397,29 +456,36 @@ public static class DriverCredentialPolicy using (doc) use(doc.RootElement); } - private static List<(string FieldPath, string Segment)> CollectSegments(string? driverType, string? json) + /// + /// Every in-scope instance reference of a config (lmxopcua#835 secret: segments + #849 env: parts), at + /// any depth. Empty for an unreadable config or a type without rules. + /// + /// The owning driver's type. + /// The config JSON. + /// The references, in document order. + public static IReadOnlyList CollectInstanceReferences(string? driverType, string? json) { - var found = new List<(string, string)>(); + var found = new List(); if (!TryGet(driverType, out var rules) || string.IsNullOrWhiteSpace(json)) return found; - JsonDocument doc; - try + // Unreadable JSON is reported by DescribeProblems. + WithDocument(json, root => Visit(root, "", rules, 0, (fieldPath, value, field, _) => { - doc = JsonDocument.Parse(json, new JsonDocumentOptions { CommentHandling = JsonCommentHandling.Skip, AllowTrailingCommas = true }); - } - catch (JsonException) - { - return found; // DescribeProblems reports unreadable JSON. - } - - using (doc) - { - Visit(doc.RootElement, "", rules, 0, (fieldPath, value, field, _) => - { - if (field is null || value.ValueKind != JsonValueKind.String) return; - if (InstanceSegment(value.GetString(), rules.Scope) is { Length: > 0 } segment) found.Add((fieldPath, segment)); - }); - } - + if (field is null || value.ValueKind != JsonValueKind.String) return; + var text = value.GetString(); + if (InstanceSegment(text, rules.Scope) is { Length: > 0 } segment) + found.Add(new(fieldPath, CredentialReferenceKind.Secret, segment)); + else if (EnvironmentInstancePart(text, rules.Scope) is { Length: > 0 } part) + found.Add(new(fieldPath, CredentialReferenceKind.Environment, part)); + })); return found; } } + +/// The instance-identifying part of one in-scope credential reference (lmxopcua#835 / #849). +/// Where it sits in the config (never the value). +/// or . +/// +/// The secret: instance segment () or the env: remainder +/// after the prefix (). +/// +public readonly record struct InstanceReference(string FieldPath, CredentialReferenceKind Kind, string Part); diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Browsing/BrowserSessionService.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Browsing/BrowserSessionService.cs index 0054d0225..30b84f638 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Browsing/BrowserSessionService.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Browsing/BrowserSessionService.cs @@ -26,7 +26,7 @@ namespace ZB.MOM.WW.OtOpcUa.AdminUI.Browsing; /// Supplies the calling circuit's user. /// /// The config DB, for the cross-instance credential rule (lmxopcua#835). Null (test graphs) ⇒ a config carrying an -/// in-scope secret: reference cannot be checked and is refused (fail-closed). +/// in-scope secret:/env: reference cannot be checked and is refused (fail-closed). /// /// The credential-use audit sink (#835 review I-1); null ⇒ the structured log line only. public sealed class BrowserSessionService( @@ -73,7 +73,7 @@ public sealed class BrowserSessionService( if (DriverCredentialPolicy.HasInstanceSegments(driverType, configJson)) { if (dbFactory is null) - return new(false, "Browse refused: the driver-instance list needed to check this config's secret: " + return new(false, "Browse refused: the driver-instance list needed to check this config's secret:/env: " + "references is unavailable (lmxopcua#835, fail-closed).", Guid.Empty); try { @@ -88,7 +88,7 @@ public sealed class BrowserSessionService( // Review L-2: a ConfigDb fault must not become a circuit error — refuse (fail-closed) with a message. logger.LogWarning(ex, "Browse credential check failed for driverType={DriverType}", driverType); return new(false, "Browse refused: the driver-instance list could not be read to check this config's " - + "secret: references (lmxopcua#835, fail-closed). Try again.", Guid.Empty); + + "secret:/env: references (lmxopcua#835/#849, fail-closed). Try again.", Guid.Empty); } } diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/AdminOperations/AdminOperationsActor.cs b/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/AdminOperations/AdminOperationsActor.cs index 441810fb7..12e015bc6 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/AdminOperations/AdminOperationsActor.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/AdminOperations/AdminOperationsActor.cs @@ -611,9 +611,9 @@ public sealed class AdminOperationsActor : ReceiveActor var sw = System.Diagnostics.Stopwatch.StartNew(); try { - // lmxopcua#835: a reference naming ANOTHER driver instance's secret (secret:galaxy//…) is refused + // lmxopcua#835/#849: a reference naming ANOTHER driver instance's secret (secret:galaxy//…, env:OTOPCUA_GALAXY__…) is refused // before the probe resolves it — off the mailbox, since it reads the ConfigDb (only when the config - // carries an in-scope secret: reference at all). + // carries an in-scope secret:/env: reference at all). if (DriverCredentialPolicy.HasInstanceSegments(msg.DriverType, msg.ConfigJson)) { await using var db = await _dbFactory.CreateDbContextAsync(cts.Token); diff --git a/tests/Core/ZB.MOM.WW.OtOpcUa.Configuration.Tests/DraftValidatorCredentialInstanceAndPathTests.cs b/tests/Core/ZB.MOM.WW.OtOpcUa.Configuration.Tests/DraftValidatorCredentialInstanceAndPathTests.cs index 4e32aa8f3..e26637fd4 100644 --- a/tests/Core/ZB.MOM.WW.OtOpcUa.Configuration.Tests/DraftValidatorCredentialInstanceAndPathTests.cs +++ b/tests/Core/ZB.MOM.WW.OtOpcUa.Configuration.Tests/DraftValidatorCredentialInstanceAndPathTests.cs @@ -50,6 +50,27 @@ public sealed class DraftValidatorCredentialInstanceAndPathTests [new Device { DeviceId = "dev-b", DriverInstanceId = "DRV-b", Name = "d", DeviceConfig = """{"Password":"secret:opcuaclient/victim/pw"}""" }]) .ShouldHaveSingleItem().Context.ShouldBe("dev-b"); + /// lmxopcua#849: the env: form carries the same per-instance rule, under the same code. + [Fact] + public void A_galaxy_env_reference_naming_another_instance_is_refused() + { + var e = Validate( + Drv("DRV-victim", "Victim", "GalaxyMxGateway", """{"Gateway":{"ApiKeySecretRef":"env:OTOPCUA_GALAXY_VICTIM_KEY"}}"""), + Drv("DRV-evil", "Evil", "GalaxyMxGateway", """{"Gateway":{"Endpoint":"http://evil:5001","ApiKeySecretRef":"env:GALAXY_MXGW_DRV_VICTIM_KEY"}}""")) + .ShouldHaveSingleItem(); + e.Code.ShouldBe(CredentialReferenceDeployChecks.OtherInstanceCode); + e.Context.ShouldBe("DRV-evil"); + e.Message.ShouldContain("lmxopcua#849"); + } + + [Fact] + public void Shared_and_own_env_variables_pass() => + Validate( + Drv("DRV-a", "A", "GalaxyMxGateway", """{"Gateway":{"ApiKeySecretRef":"env:OTOPCUA_GALAXY_DRV_A_KEY"}}"""), + Drv("DRV-b", "B", "GalaxyMxGateway", """{"Gateway":{"ApiKeySecretRef":"env:GALAXY_MXGW_API_KEY"}}"""), + Drv("DRV-c", "C", "GalaxyMxGateway", """{"Gateway":{"ApiKeySecretRef":"env:GALAXY_MXGW_API_KEY"}}""")) + .ShouldBeEmpty(); + [Fact] public void Shared_and_own_keys_pass() => Validate( diff --git a/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions.Tests/CredentialInstanceScopeTests.cs b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions.Tests/CredentialInstanceScopeTests.cs index 22e032bca..6d0ab5541 100644 --- a/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions.Tests/CredentialInstanceScopeTests.cs +++ b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions.Tests/CredentialInstanceScopeTests.cs @@ -44,7 +44,10 @@ public sealed class CredentialInstanceScopeTests [InlineData("secret:galaxy/gw/apikey")] // a shared name no instance uses [InlineData("secret:galaxy/apikey")] [InlineData("secret:galaxy/UpstreamC/apikey")] // another TYPE's instance — not this namespace's owner - [InlineData("env:OTOPCUA_GALAXY_PLANTB_KEY")] // env carries no instance convention + [InlineData("env:OTOPCUA_GALAXY_PLANTA_KEY")] // its own name in env form (lmxopcua#849) + [InlineData("env:OTOPCUA_GALAXY_DRV_AAA_KEY")] // its own id in env form + [InlineData("env:GALAXY_MXGW_API_KEY")] // a shared variable that names no instance + [InlineData("env:OTOPCUA_GALAXY_PLANTBX_KEY")] // 'PLANTB' only at a '_' boundary [InlineData("dev:k")] [InlineData("")] public void Own_shared_and_non_secret_references_pass(string reference) => @@ -98,7 +101,7 @@ public sealed class CredentialInstanceScopeTests public void HasInstanceSegments_gates_the_ConfigDb_read() { DriverCredentialPolicy.HasInstanceSegments(DriverTypeNames.Galaxy, Galaxy("secret:galaxy/x/apikey")).ShouldBeTrue(); - DriverCredentialPolicy.HasInstanceSegments(DriverTypeNames.Galaxy, Galaxy("env:OTOPCUA_GALAXY_X")).ShouldBeFalse(); + DriverCredentialPolicy.HasInstanceSegments(DriverTypeNames.Galaxy, Galaxy("env:OTOPCUA_GALAXY_X")).ShouldBeTrue(); // #849 DriverCredentialPolicy.HasInstanceSegments(DriverTypeNames.Galaxy, Galaxy("env:ConnectionStrings__ConfigDb")).ShouldBeFalse(); DriverCredentialPolicy.HasInstanceSegments(DriverTypeNames.Modbus, """{"Password":"secret:galaxy/x"}""").ShouldBeFalse(); DriverCredentialPolicy.HasInstanceSegments(DriverTypeNames.Galaxy, "not json").ShouldBeFalse(); @@ -113,4 +116,73 @@ public sealed class CredentialInstanceScopeTests problems.ShouldContain(p => p.Contains("lmxopcua#815")); problems.ShouldContain(p => p.Contains("lmxopcua#835")); } + + // --------------------------------------------------------------------------------------------------------------- + // lmxopcua#849 — env: references follow the same per-instance rule (env:_…) + // --------------------------------------------------------------------------------------------------------------- + + [Theory] + [InlineData("env:OTOPCUA_GALAXY_PLANTB_KEY")] // another instance's name, upper-cased + [InlineData("env:OTOPCUA_GALAXY_PLANTB")] // the part alone + [InlineData("env:OTOPCUA_GALAXY_DRV_BBB_KEY")] // another instance's id, '-' as '_' + [InlineData("env:GALAXY_MXGW_PLANTB_API_KEY")] // the legacy prefix too + public void An_env_reference_naming_another_instance_is_refused(string reference) + { + var problem = DriverCredentialPolicy + .DescribeOtherInstanceProblems(DriverTypeNames.Galaxy, Galaxy(reference), "DRV-aaa", Instances) + .ShouldHaveSingleItem(); + problem.ShouldContain("lmxopcua#849"); + problem.ShouldContain("DRV-bbb"); + problem.ShouldContain("Gateway.ApiKeySecretRef"); + } + + [Fact] + public void An_env_match_against_another_instance_wins_over_the_owners() + { + // Owner "Line 1" (LINE_1) and another instance "Line" (LINE): OTOPCUA_GALAXY_LINE_1_KEY names both. + IEnumerable lines = + [new("DRV-1", "Line 1", DriverTypeNames.Galaxy), new("DRV-2", "Line", DriverTypeNames.Galaxy)]; + DriverCredentialPolicy.DescribeOtherInstanceProblems(DriverTypeNames.Galaxy, Galaxy("env:OTOPCUA_GALAXY_LINE_1_KEY"), "DRV-1", lines) + .ShouldHaveSingleItem(); + // …while the owner's id-scoped variable names only the owner. + DriverCredentialPolicy.DescribeOtherInstanceProblems(DriverTypeNames.Galaxy, Galaxy("env:OTOPCUA_GALAXY_DRV_1_KEY"), "DRV-1", lines) + .ShouldBeEmpty(); + } + + [Fact] + public void OpcUaClient_and_Heidenhain_env_forms_have_the_rule() + { + DriverCredentialPolicy.DescribeOtherInstanceProblems( + DriverTypeNames.OpcUaClient, """{"Password":"env:OTOPCUA_OPCUACLIENT_UPSTREAMD_PW"}""", "DRV-ccc", Instances) + .ShouldHaveSingleItem().ShouldContain("lmxopcua#849"); + IEnumerable hh = [new("H1", "line1", DriverTypeNames.Heidenhain), new("H2", "line2", DriverTypeNames.Heidenhain)]; + DriverCredentialPolicy.DescribeOtherInstanceProblems( + DriverTypeNames.Heidenhain, """{"Access":{"DncPassword":"env:OTOPCUA_HEIDENHAIN_LINE2_DNC"}}""", "H1", hh) + .ShouldHaveSingleItem(); + DriverCredentialPolicy.DescribeOtherInstanceProblems( + DriverTypeNames.Heidenhain, """{"Access":{"DncPassword":"env:OTOPCUA_HEIDENHAIN_DNC_PASSWORD"}}""", "H1", hh) + .ShouldBeEmpty(); + } + + [Theory] + [InlineData("DRV-1a2b", "DRV_1A2B")] + [InlineData("Line 1", "LINE_1")] + [InlineData("plant.b", "PLANT_B")] + [InlineData("---", "")] + [InlineData("", "")] + public void Identifiers_normalise_to_environment_form(string identifier, string expected) => + DriverCredentialPolicy.NormalizeEnvironmentIdentifier(identifier).ShouldBe(expected); + + [Fact] + public void Instance_references_are_collected_for_both_kinds() + { + var refs = DriverCredentialPolicy.CollectInstanceReferences(DriverTypeNames.Galaxy, + """{"Gateway":{"ApiKeySecretRef":"secret:galaxy/gw/k"},"AlarmAck":{"FallbackPasswordSecretRef":"env:OTOPCUA_GALAXY_ACK_PW"}}"""); + refs.Count.ShouldBe(2); + refs[0].ShouldBe(new InstanceReference("Gateway.ApiKeySecretRef", CredentialReferenceKind.Secret, "gw")); + refs[1].ShouldBe(new InstanceReference("AlarmAck.FallbackPasswordSecretRef", CredentialReferenceKind.Environment, "ACK_PW")); + DriverCredentialPolicy.ReferenceNames(refs[1], "ack").ShouldBeTrue(); + DriverCredentialPolicy.ReferenceNames(refs[1], "ac").ShouldBeFalse(); + DriverCredentialPolicy.ReferenceNames(refs[0], "GW").ShouldBeTrue(); + } } diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Browsing/BrowserSessionCredentialScopeTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Browsing/BrowserSessionCredentialScopeTests.cs index a5459573e..916ed9105 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Browsing/BrowserSessionCredentialScopeTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Browsing/BrowserSessionCredentialScopeTests.cs @@ -72,6 +72,41 @@ public sealed class BrowserSessionCredentialScopeTests universal.Opens.ShouldBe(0); } + /// lmxopcua#849: an env: reference named after ANOTHER instance never opens a browser either. + [Theory] + [InlineData("env:OTOPCUA_GALAXY_VICTIM_KEY")] + [InlineData("env:GALAXY_MXGW_DRV_VICTIM_KEY")] + public async Task An_env_reference_naming_another_instance_never_opens_a_browser(string reference) + { + var bespoke = new RecordingBrowser(DriverTypeNames.Galaxy); + var svc = new BrowserSessionService( + [bespoke], new BrowseSessionRegistry(), NullLogger.Instance, new RecordingUniversal(), null!, null!, + SeededFactory()); + + var result = await svc.OpenAsync(DriverTypeNames.Galaxy, + """{"Gateway":{"Endpoint":"http://evil:5001","ApiKeySecretRef":"REF"}}""".Replace("REF", reference), "DRV-evil", + TestContext.Current.CancellationToken); + + result.Ok.ShouldBeFalse(); + result.Message.ShouldNotBeNull().ShouldContain("lmxopcua#849"); + bespoke.Opens.ShouldBe(0); + } + + /// Positive control: a shared env variable that names no instance reaches the browser. + [Fact] + public async Task A_shared_env_variable_reaches_the_browser() + { + var bespoke = new RecordingBrowser(DriverTypeNames.Galaxy); + var svc = new BrowserSessionService( + [bespoke], new BrowseSessionRegistry(), NullLogger.Instance, new RecordingUniversal(), null!, null!, + SeededFactory()); + + await svc.OpenAsync(DriverTypeNames.Galaxy, """{"Gateway":{"ApiKeySecretRef":"env:GALAXY_MXGW_API_KEY"}}""", + TestContext.Current.CancellationToken); + + bespoke.Opens.ShouldBe(1); + } + /// Positive control: the owning driver's own id-scoped secret reaches the browser. [Fact] public async Task The_owners_own_secret_reaches_the_browser() diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/AdminOperationsActorCredentialScopeTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/AdminOperationsActorCredentialScopeTests.cs index 28ab21f4e..45157e005 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/AdminOperationsActorCredentialScopeTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/AdminOperationsActorCredentialScopeTests.cs @@ -76,6 +76,44 @@ public sealed class AdminOperationsActorCredentialScopeTests : ControlPlaneActor probe.Calls.ShouldBe(0); } + /// lmxopcua#849: an env: reference named after ANOTHER instance never reaches the probe either. + [Theory] + [InlineData("env:OTOPCUA_GALAXY_VICTIM_API_KEY")] + [InlineData("env:GALAXY_MXGW_DRV_VICTIM_KEY")] + public void An_env_reference_naming_another_instance_never_reaches_the_probe(string reference) + { + var factory = NewInMemoryDbFactory(); + SeedGalaxyDrivers(factory); + var probe = new RecordingProbe(DriverTypeNames.Galaxy); + var actor = Sys.ActorOf(AdminOperationsActor.Props(factory, CreateTestProbe("coord").Ref, new IDriverProbe[] { probe })); + + actor.Tell(new TestDriverConnect( + DriverTypeNames.Galaxy, """{"Gateway":{"Endpoint":"http://attacker:5001","ApiKeySecretRef":"REF"}}""".Replace("REF", reference), + 5, Guid.NewGuid(), "DRV-evil")); + + var reply = ExpectMsg(PresenceBudget, cancellationToken: TestContext.Current.CancellationToken); + reply.Ok.ShouldBeFalse(); + reply.Message.ShouldNotBeNull().ShouldContain("lmxopcua#849"); + probe.Calls.ShouldBe(0); + } + + /// Positive control: a shared env variable naming no instance reaches the probe. + [Fact] + public void A_shared_env_variable_reaches_the_probe() + { + var factory = NewInMemoryDbFactory(); + SeedGalaxyDrivers(factory); + var probe = new RecordingProbe(DriverTypeNames.Galaxy); + var actor = Sys.ActorOf(AdminOperationsActor.Props(factory, CreateTestProbe("coord").Ref, new IDriverProbe[] { probe })); + + actor.Tell(new TestDriverConnect( + DriverTypeNames.Galaxy, """{"Gateway":{"Endpoint":"http://attacker:5001","ApiKeySecretRef":"env:GALAXY_MXGW_API_KEY"}}""", + 5, Guid.NewGuid())); + + ExpectMsg(PresenceBudget, cancellationToken: TestContext.Current.CancellationToken).Ok.ShouldBeTrue(); + probe.Calls.ShouldBe(1); + } + /// Positive control: the owner's own secret (and a shared one) does reach the probe. [Theory] [InlineData("secret:galaxy/DRV-victim/apikey")]