From 8a1f037d5abfcbc263d2f2c5b5effa7f04d3d0c3 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 18 Jun 2026 03:25:48 -0400 Subject: [PATCH] fix(gateway): resolve array attribute constraints by bare name via [] fallback --- .../Authorization/ConstraintEnforcer.cs | 29 ++++- .../Sessions/GatewayArrayWriteWiringTests.cs | 106 ++++++++++++++++++ .../Authorization/ConstraintEnforcerTests.cs | 64 +++++++++++ 3 files changed, 194 insertions(+), 5 deletions(-) diff --git a/src/ZB.MOM.WW.MxGateway.Server/Security/Authorization/ConstraintEnforcer.cs b/src/ZB.MOM.WW.MxGateway.Server/Security/Authorization/ConstraintEnforcer.cs index abdd70d..812815a 100644 --- a/src/ZB.MOM.WW.MxGateway.Server/Security/Authorization/ConstraintEnforcer.cs +++ b/src/ZB.MOM.WW.MxGateway.Server/Security/Authorization/ConstraintEnforcer.cs @@ -196,11 +196,30 @@ public sealed class ConstraintEnforcer( private GalaxyTagLookup? ResolveTarget(string tagAddress) { - GalaxyHierarchyCacheEntry entry = cache.Current; - return !string.IsNullOrWhiteSpace(tagAddress) - && entry.Index.TagsByAddress.TryGetValue(tagAddress, out GalaxyTagLookup? lookup) - ? lookup - : null; + if (string.IsNullOrWhiteSpace(tagAddress)) + { + return null; + } + + IReadOnlyDictionary tagsByAddress = cache.Current.Index.TagsByAddress; + if (tagsByAddress.TryGetValue(tagAddress, out GalaxyTagLookup? lookup)) + { + return lookup; + } + + // Galaxy SQL keys array attributes by their suffixed FullTagReference (e.g. "Obj.Arr[]"), + // but callers pass the bare address ("Obj.Arr") before the worker-boundary normalization + // runs. Probe the suffixed form so a bare array name resolves to its array attribute, + // consistent with ArrayAddressNormalizer. Only build the suffixed string on a direct miss + // when the address is not already suffixed, and only accept it when it is truly an array. + if (!tagAddress.EndsWith("[]", StringComparison.Ordinal) + && tagsByAddress.TryGetValue(tagAddress + "[]", out GalaxyTagLookup? arrayLookup) + && arrayLookup.Attribute?.IsArray == true) + { + return arrayLookup; + } + + return null; } private static bool MatchesPathOrTag( diff --git a/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Sessions/GatewayArrayWriteWiringTests.cs b/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Sessions/GatewayArrayWriteWiringTests.cs index 204ed6e..36424da 100644 --- a/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Sessions/GatewayArrayWriteWiringTests.cs +++ b/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Sessions/GatewayArrayWriteWiringTests.cs @@ -141,6 +141,112 @@ public sealed class GatewayArrayWriteWiringTests Assert.Equal(new[] { 0, 7, 0, 0 }, forwarded.ArrayValue.Int32Values.Values); } + /// + /// A bare array AddItem2 address is normalized to its writable array form on the wire, + /// and the normalized address lands in the tracked . + /// + [Fact] + public async Task AddItem2_BareArrayAddress_NormalizedOnWireAndInRegistration() + { + CapturingWorkerClient worker = new(); + GatewaySession session = CreateReadySession(worker); + + WorkerCommand command = new() + { + Command = new MxCommand + { + Kind = MxCommandKind.AddItem2, + AddItem2 = new AddItem2Command + { + ServerHandle = 1, + ItemDefinition = "Obj.Arr", + }, + }, + }; + + worker.NextReply = new WorkerCommandReply + { + Reply = new MxCommandReply + { + Kind = MxCommandKind.AddItem2, + ProtocolStatus = new ProtocolStatus { Code = ProtocolStatusCode.Ok }, + AddItem2 = new AddItem2Reply { ItemHandle = 43 }, + }, + }; + + await session.InvokeAsync(command, CancellationToken.None); + + Assert.NotNull(worker.LastCommand); + Assert.Equal("Obj.Arr[]", worker.LastCommand!.Command.AddItem2.ItemDefinition); + + MxCommand trackingCopy = new() + { + Kind = MxCommandKind.AddItem2, + AddItem2 = new AddItem2Command + { + ServerHandle = 1, + ItemDefinition = "Obj.Arr", + }, + }; + session.TrackCommandReply(trackingCopy, worker.NextReply.Reply); + + Assert.True(session.TryGetItemRegistration(1, 43, out SessionItemRegistration registration)); + Assert.Equal("Obj.Arr[]", registration.TagAddress); + } + + /// + /// A sparse-array entry in a is expanded to a full, + /// default-filled before reaching the worker; no sparse value is ever + /// forwarded inside a bulk write. + /// + [Fact] + public async Task WriteBulk_SparseArrayEntryValue_ExpandedBeforeReachingWorker() + { + CapturingWorkerClient worker = new(); + GatewaySession session = CreateReadySession(worker); + + WorkerCommand command = new() + { + Command = new MxCommand + { + Kind = MxCommandKind.WriteBulk, + WriteBulk = new WriteBulkCommand + { + ServerHandle = 1, + Entries = + { + new WriteBulkEntry + { + ItemHandle = 42, + Value = new MxValue + { + SparseArrayValue = new MxSparseArray + { + ElementDataType = MxDataType.Integer, + TotalLength = 4, + Elements = + { + new MxSparseElement + { + Index = 1, + Value = new MxValue { Int32Value = 7 }, + }, + }, + }, + }, + }, + }, + }, + }, + }; + + await session.InvokeAsync(command, CancellationToken.None); + + MxValue forwarded = worker.LastCommand!.Command.WriteBulk.Entries[0].Value; + Assert.Equal(MxValue.KindOneofCase.ArrayValue, forwarded.KindCase); + Assert.Equal(new[] { 0, 7, 0, 0 }, forwarded.ArrayValue.Int32Values.Values); + } + private static GatewaySession CreateReadySession(IWorkerClient workerClient) { GatewaySession session = new( diff --git a/src/ZB.MOM.WW.MxGateway.Tests/Security/Authorization/ConstraintEnforcerTests.cs b/src/ZB.MOM.WW.MxGateway.Tests/Security/Authorization/ConstraintEnforcerTests.cs index 19a7917..69ee056 100644 --- a/src/ZB.MOM.WW.MxGateway.Tests/Security/Authorization/ConstraintEnforcerTests.cs +++ b/src/ZB.MOM.WW.MxGateway.Tests/Security/Authorization/ConstraintEnforcerTests.cs @@ -207,6 +207,62 @@ public sealed class ConstraintEnforcerTests Assert.Equal("read_historized_only", failure.ConstraintName); } + /// + /// A bare array attribute address (no trailing []) resolves through the Galaxy index + /// even though arrays are keyed by their suffixed FullTagReference (e.g. "Pump_001.Levels[]"). + /// Without the [] fallback in ResolveTarget the bare name misses the index and a + /// read-constrained key gets a spurious tag_metadata denial for an AddItem it should allow. + /// + [Fact] + public async Task CheckReadTagAsync_WithBareArrayName_ResolvesViaArraySuffixFallback() + { + ConstraintEnforcer enforcer = CreateEnforcer(out _); + ApiKeyIdentity identity = CreateIdentity(ApiKeyConstraints.Empty with + { + // A read constraint that covers the Pump_001 subtree; the array attribute is inside it. + ReadTagGlobs = ["Pump_001.*"], + }); + + ConstraintFailure? failure = await enforcer.CheckReadTagAsync( + identity, + "Pump_001.Levels", + CancellationToken.None); + + // Before the fix: bare "Pump_001.Levels" misses the index (keyed "Pump_001.Levels[]") and + // returns a tag_metadata failure. After the fix: it resolves and is within scope -> null. + Assert.Null(failure); + } + + /// + /// A bare non-array name that is genuinely absent from the index still resolves to null: + /// the [] probe must not manufacture a false positive for a scalar/missing tag. + /// + [Fact] + public async Task CheckReadTagAsync_WithMissingNonArrayName_StillFailsToResolve() + { + ConstraintEnforcer enforcer = CreateEnforcer(out _); + ApiKeyIdentity identity = CreateIdentity(ApiKeyConstraints.Empty with + { + ReadTagGlobs = ["Pump_001.*"], + }); + + // "Pump_001.Scalar" is not in the index, and "Pump_001.Scalar[]" is not either, so the + // suffix probe must not resolve it. A genuinely-unknown name behaves the same. + ConstraintFailure? missingScalar = await enforcer.CheckReadTagAsync( + identity, + "Pump_001.Scalar", + CancellationToken.None); + Assert.NotNull(missingScalar); + Assert.Equal("tag_metadata", missingScalar.ConstraintName); + + ConstraintFailure? unknown = await enforcer.CheckReadTagAsync( + identity, + "DoesNotExist_999.Whatever", + CancellationToken.None); + Assert.NotNull(unknown); + Assert.Equal("tag_metadata", unknown.ConstraintName); + } + private static ConstraintEnforcer CreateEnforcer(out FakeAuditWriter auditWriter) { auditWriter = new FakeAuditWriter(); @@ -276,6 +332,14 @@ public sealed class ConstraintEnforcerTests AttributeName = "NonHistorized", FullTagReference = "Pump_001.NonHistorized", }, + new GalaxyAttribute + { + // Galaxy SQL keys array attributes by their suffixed FullTagReference, + // so the index entry is "Pump_001.Levels[]", not the bare name. + AttributeName = "Levels", + FullTagReference = "Pump_001.Levels[]", + IsArray = true, + }, }, }, new GalaxyObject