fix(gateway): resolve 2026-06-18 array-write review findings

- Server-057: extend []-suffix normalization to AddItemBulk/AddBufferedItem so bulk-added
  array tags bind write-capable handles (authz check, worker bind, and registration kept
  consistent); update gateway.md + client READMEs. Tests: AddItemBulk/AddBufferedItem wiring.
- Server-058: assert []-fallback-resolved bare array names are still denied when out of
  read/write scope and that MaxWriteClassification is enforced on suffixed array registrations.
- Contracts-023/024/025: round-trip + field-19 descriptor pin for MxSparseArray; document
  MxSparseArray in docs/Contracts.md; enumerate it in the protocol-version-3 test summary.
- Tests-040: add wiring tests for the six uncovered sparse-write arms (WriteSecured, Write2,
  WriteSecured2, Write2Bulk, WriteSecuredBulk, WriteSecured2Bulk).

dotnet build + targeted tests green (184 passed).
This commit is contained in:
Joseph Doherty
2026-06-18 10:58:42 -04:00
parent 6c853b43af
commit 2671639250
10 changed files with 718 additions and 30 deletions
+10 -4
View File
@@ -7,7 +7,7 @@
| Review date | 2026-06-18 |
| Commit reviewed | `88915c3` |
| Status | Re-reviewed |
| Open findings | 3 |
| Open findings | 0 |
## Checklist coverage
@@ -510,7 +510,7 @@ and `docs/WorkerConversion.md` (section "Sparse array expansion").
| Severity | Low |
| Category | Testing coverage |
| Location | `src/ZB.MOM.WW.MxGateway.Tests/Contracts/ProtobufContractRoundTripTests.cs` |
| Status | Open |
| Status | Resolved |
**Description:** No round-trip test or descriptor pin exists for the new `MxSparseArray` message, `MxSparseElement` message, or `MxValue.KindOneofCase.SparseArrayValue` (field number 19). A future renumber or type-narrowing of `sparse_array_value = 19`, or of `MxSparseArray`'s field numbers (1/2/3) or `MxSparseElement`'s field numbers (1/2), would not be caught at the contract level. This is the same gap class as Contracts-007 (`MxValue.raw_value`), Contracts-010 (bulk write/read), Contracts-018 (alarm-provider fallback), and Contracts-022 (`ReplayGap`) — all of which were resolved by adding focused round-trip tests.
@@ -518,6 +518,8 @@ Additionally, the `MxSparseElement.value` field is typed `MxValue` (the full val
**Recommendation:** Add round-trip / descriptor-pin tests to `ProtobufContractRoundTripTests`: (a) pin `MxValue.SparseArrayValueFieldNumber == 19` via the generated constant; (b) round-trip an `MxSparseArray` with `element_data_type`, `total_length`, and at least one `MxSparseElement` (covering `index` and a scalar `value`), embedded in an `MxValue` with `KindCase == SparseArrayValue`; (c) assert the `MxSparseArray` field numbers by name via `MxSparseArray.Descriptor.Fields` (1 = `element_data_type`, 2 = `total_length`, 3 = `elements`) and `MxSparseElement.Descriptor.Fields` (1 = `index`, 2 = `value`). Optionally add a second test with an empty `elements` list (valid all-defaults case) to pin that zero elements is not a proto-level error.
**Resolution:** _(2026-06-18)_ Confirmed all three gaps against the proto and generated constants. Added `ProtobufContractRoundTripTests.MxValue_RoundTripsSparseArrayValueAndPinsFieldNumbers` to `ProtobufContractRoundTripTests.cs`. The test: (a) pins `MxValue.SparseArrayValueFieldNumber == 19` via the generated constant; (b) pins all five field numbers by name + number via the descriptor (`MxSparseArray` fields 1/2/3 and `MxSparseElement` fields 1/2); (c) round-trips an `MxValue` with `KindCase == SparseArrayValue` carrying a populated `MxSparseArray` (one `MxSparseElement` with a scalar float value at index 2); (d) verifies an all-defaults `MxSparseArray` with no elements is not a proto-level error. The full `ProtobufContractRoundTrip|GatewayContractInfo` filter is 54/54 green.
### Contracts-024
| Field | Value |
@@ -525,12 +527,14 @@ Additionally, the `MxSparseElement.value` field is typed `MxValue` (the full val
| Severity | Low |
| Category | Documentation & comments |
| Location | `docs/Contracts.md:9-11` |
| Status | Open |
| Status | Resolved |
**Description:** `docs/Contracts.md` lists `MxValue`, `MxArray`, and `MxStatusProxy` as the types defined in `mxaccess_gateway.proto`, and documents both bulk subscription and bulk write/read command families in detail. The new `MxSparseArray` value arm (`sparse_array_value = 19`) — a public-facing addition to the `MxValue` oneof that changes the write API available to every command variant — is not mentioned anywhere in `docs/Contracts.md`. The CLAUDE.md rule "Update docs in the same change as the source. When public APIs, contracts, configuration … change, the affected docs … must change in the same commit" was not satisfied for this addition; `docs/Contracts.md` now undercounts the public `MxValue` surface. `gateway.md` and `docs/WorkerConversion.md` were updated, but `docs/Contracts.md` — the canonical contracts document linked from the client generation doc — was not.
**Recommendation:** Extend `docs/Contracts.md` to describe `MxSparseArray`: the write-only `sparse_array_value = 19` arm on `MxValue`, the two messages (`MxSparseArray` with `element_data_type`, `total_length`, `elements`; `MxSparseElement` with `index`, `value`), the default-fill-not-preserve semantics for unmentioned indices, and the fact that it is accepted by every write variant (`Write`, `Write2`, `WriteSecured`, `WriteSecured2`, and each `*BulkEntry` entry) but rejected on read/event paths. Cross-reference `gateway.md` for the validation rules and expansion details rather than restating them.
**Resolution:** _(2026-06-18)_ Confirmed `docs/Contracts.md` had no mention of `MxSparseArray` / `MxSparseElement` / `sparse_array_value = 19`. Added a new paragraph in the "Files" section immediately after the `mxaccess_gateway.proto` intro sentence (before the bulk-subscription commands section): names `MxSparseArray` alongside `MxValue`, `MxArray`, and `MxStatusProxy` in the intro line; explains that `sparse_array_value = 19` is the `MxValue.kind` oneof arm for write-only partial-array writes; documents both messages with their fields and field numbers; states the default-fill-not-preserve semantics; and enumerates every write variant that accepts it plus the read/event rejection. Cross-references `gateway.md` for expansion rules and validation constraints.
### Contracts-025
| Field | Value |
@@ -538,8 +542,10 @@ Additionally, the `MxSparseElement.value` field is typed `MxValue` (the full val
| Severity | Low |
| Category | Documentation & comments |
| Location | `src/ZB.MOM.WW.MxGateway.Tests/Contracts/GatewayContractInfoTests.cs:14-25` |
| Status | Open |
| Status | Resolved |
**Description:** The XML summary on `GatewayContractInfoTests.GatewayProtocolVersion_IsVersionThree` (updated under Contracts-013 resolution to enumerate the alarm and bulk write/read extensions shipped under version 3) does not mention the new `MxSparseArray` / `sparse_array_value = 19` addition, which is also a strictly additive contract change shipped under version 3 without a bump. A reader checking whether a new additive contract feature requires a `GatewayProtocolVersion` bump will look at this test for precedent; finding only the alarm and bulk write/read examples, they cannot tell whether the sparse array addition was also additive-under-3 or was simply omitted by mistake. This is the same class of stale-summary issue as Contracts-013 (which noted the bulk write/read extension was not mentioned after the alarm-only summary).
**Recommendation:** Extend the XML summary to list the `MxSparseArray` write ergonomics extension (`MxSparseArray` / `MxSparseElement` + `sparse_array_value = 19` on `MxValue`, plus the suffix-normalization behavior) alongside the alarm and bulk write/read extensions as a third example of a strictly additive change that shipped under version 3 without a bump. Comment-only change; no test logic or version constant changes.
**Resolution:** _(2026-06-18)_ Confirmed the XML summary on `GatewayProtocolVersion_IsVersionThree` enumerated only the alarm and bulk write/read extensions; the sparse-array addition was missing. Extended the summary to list all three additive-under-version-3 extensions as an ordered enumeration: (1) alarm proto extension; (2) bulk write/read command family; (3) sparse-array write ergonomics (`MxSparseArray` / `MxSparseElement` messages plus `sparse_array_value = 19` on the `MxValue` oneof). Comment-only change; test logic and version constant are unchanged. The full `ProtobufContractRoundTrip|GatewayContractInfo` filter is 54/54 green.
+5 -5
View File
@@ -7,7 +7,7 @@
| Review date | 2026-06-18 |
| Commit reviewed | `88915c3` |
| Status | Re-reviewed |
| Open findings | 2 |
| Open findings | 0 |
## Checklist coverage
@@ -1109,13 +1109,13 @@ Additionally, `GatewayAlarmMonitor.ApplyProviderModeChangeAsync` increments the
| Severity | Medium |
| Category | Correctness & logic bugs |
| Location | `src/ZB.MOM.WW.MxGateway.Server/Sessions/GatewaySession.cs:976-1000` (`NormalizeOutboundCommand`), `:1085-1095` (`MapCommand` tracking), `gateway.md` (array-write ergonomics section), `clients/*/README.md` |
| Status | Open |
| Status | Resolved |
**Description:** The array-suffix `[]` normalization runs only for the single-add commands `AddItem` and `AddItem2``NormalizeOutboundCommand` has no case for `AddItemBulk` (or `AddBufferedItem`), and the `MapCommand` tracking switch likewise normalizes only the `AddItem`/`AddItem2` arms (`AddItemBulk` flows through `TrackBulkItems`/`AddBufferedItem` with the raw address). MXAccess requires the `[]` suffix on the AddItem address for an array attribute to register a *write-capable* handle. A client that registers a bare array address via `AddItemBulk` therefore binds a non-write-capable handle, and a later `Write`/`WriteSecured`/sparse write against that handle silently lands on a read-only-ish handle — the exact failure mode this feature fixes for the single-add path. The behavior is inconsistent across the add family, and `gateway.md` / the client READMEs describe normalization as happening "at AddItem time" and explicitly carve out only `ReadBulk`, giving no signal that `AddItemBulk` is excluded. `AddBufferedItem` is lower-risk (buffered/historical read items are not normally written) but is the same gap.
**Recommendation:** Either (a) extend `NormalizeOutboundCommand` and the `MapCommand` tracking path to normalize each `AddItemBulk.TagAddresses` entry (and `AddBufferedItem.ItemDefinition`) the same `IsArray`-gated way, keeping the constraint check, the worker bind, and the stored `SessionItemRegistration.TagAddress` consistent; or (b) if bulk-add normalization is intentionally out of scope for this feature, state that explicitly in `gateway.md` and the client READMEs (alongside the existing `ReadBulk` carve-out) so clients know bulk-added array handles must carry the `[]` suffix themselves to be writable.
**Resolution:** _(empty until closed)_
**Resolution:** 2026-06-18 — Took option (a). Root cause confirmed: `NormalizeOutboundCommand` had no `AddItemBulk`/`AddBufferedItem` case, so the worker bound bare (non-write-capable) array handles for those paths while single-add was already fixed. Added `AddItemBulk` (normalizes each `TagAddresses` entry in place) and `AddBufferedItem` (normalizes `ItemDefinition`) cases to `NormalizeOutboundCommand`; added the matching `AddBufferedItem` normalization to the `TrackCommandReply`/`MapCommand` tracking path (its registration keys off the command's `ItemDefinition`). `AddItemBulk` tracking needs no change — the worker echoes the already-suffixed address back in each `SubscribeResult.TagAddress`, which `TrackBulkItems` stores. Authz is unchanged and consistent: `FilterTagBulkAsync` checks the bare address through `ConstraintEnforcer.ResolveTarget`'s `[]` fallback, mirroring single-add. Updated `gateway.md` and all five client READMEs (dotnet/go/python/rust/java) so the add-family normalization no longer reads as AddItem-only; the `ReadBulk` carve-out stays. Regression tests: `GatewayArrayWriteWiringTests.AddItemBulk_BareArrayAddress_NormalizedOnWireAndInRegistration`, `.AddBufferedItem_BareArrayAddress_NormalizedOnWireAndInRegistration`.
### Server-058
@@ -1124,10 +1124,10 @@ Additionally, `GatewayAlarmMonitor.ApplyProviderModeChangeAsync` increments the
| Severity | Low |
| Category | Testing coverage |
| Location | `src/ZB.MOM.WW.MxGateway.Tests/Security/Authorization/ConstraintEnforcerTests.cs` |
| Status | Open |
| Status | Resolved |
**Description:** The new `ConstraintEnforcer.ResolveTarget` `[]`-suffix fallback is security-sensitive: it turns a bare-array address that previously missed the Galaxy index (→ spurious `tag_metadata` deny) into a real scope/classification decision. The added tests cover the in-scope-allow case (`CheckReadTagAsync_WithBareArrayName_ResolvesViaArraySuffixFallback`) and the missing/non-array negative case (`CheckReadTagAsync_WithMissingNonArrayName_StillFailsToResolve`), but **no test asserts that a bare-array name which now resolves via the fallback is still denied when it is out of the key's read/write scope** — i.e. that the fallback widened resolution but not authorization. There is also no `CheckWriteHandleAsync` test exercising an array handle whose `SessionItemRegistration.TagAddress` is the suffixed form (`Obj.Arr[]`) resolving directly through `ResolveTarget` for the write-scope and `MaxWriteClassification` checks. These are the precise paths a regression could silently widen.
**Recommendation:** Add a `CheckReadTagAsync` (and a `CheckWriteHandleAsync`) case where the bare/suffixed array attribute resolves but the configured `ReadTagGlobs`/`WriteSubtrees` exclude it, asserting a `read_scope`/`write_scope` `ConstraintFailure` is still returned; and a `CheckWriteHandleAsync` case asserting `MaxWriteClassification` is enforced against the array attribute's `SecurityClassification` via the suffixed registration address.
**Resolution:** _(empty until closed)_
**Resolution:** 2026-06-18 — Added three `ConstraintEnforcerTests` cases (the test fixture gained a second array attribute `Pump_001.Setpoints[]` with `SecurityClassification = 2` to exercise the classification path): `CheckReadTagAsync_WithBareArrayName_OutOfScope_StillDeniedReadScope` (bare array resolves via the `[]` fallback but is denied `read_scope` when out of `ReadTagGlobs` — guards that the fallback widened resolution, not authorization), `CheckWriteHandleAsync_WithSuffixedArrayRegistration_OutOfScope_StillDeniedWriteScope` (an array handle whose registration `TagAddress` is the suffixed `Pump_001.Levels[]` resolves through `ResolveTarget` and is denied `write_scope`), and `CheckWriteHandleAsync_WithSuffixedArrayRegistration_ClassificationTooHigh_StillDenied` (in-scope suffixed array handle denied `max_write_classification` when the attribute's `SecurityClassification` exceeds `MaxWriteClassification`). Tests-only change; no production code touched.
+4 -2
View File
@@ -7,7 +7,7 @@
| Review date | 2026-06-18 |
| Commit reviewed | `88915c3` |
| Status | Re-reviewed |
| Open findings | 1 |
| Open findings | 0 |
## Checklist coverage
@@ -749,8 +749,10 @@ The cancellation tests for `WorkerClient` in `WorkerClientTests` *do* exercise t
| Severity | Low |
| Category | Testing coverage |
| Location | `src/ZB.MOM.WW.MxGateway.Tests/Gateway/Sessions/GatewayArrayWriteWiringTests.cs` |
| Status | Open |
| Status | Resolved |
**Description:** `GatewayArrayWriteWiringTests` covers two of eight sparse-write arms in `GatewaySession.NormalizeOutboundCommand`: `Write` (single) and `WriteBulk`. The remaining six arms — `WriteSecured`, `Write2`, `WriteSecured2`, `Write2Bulk`, `WriteSecuredBulk`, `WriteSecured2Bulk` — all call `ExpandValue(entry.Value)` through the same switch, but no wiring test exercises them through `GatewaySession.InvokeAsync`. The class summary says it covers "the single outbound choke point" and that "no sparse value is ever forwarded"; the claim is accurate for the two covered variants but unverified for the other six. A regression accidentally dropping one of the six remaining `case` arms, or shifting a brace so a case falls through to the default (no-op), would pass the entire `GatewayArrayWriteWiringTests` suite.
**Recommendation:** Add one wiring test per uncovered variant (or a single `[Theory]` over the six command kinds), constructing the matching command type with a `SparseArrayValue` and asserting `worker.LastCommand!.Command.<Variant>.Value.KindCase == MxValue.KindOneofCase.ArrayValue` after `session.InvokeAsync`. The `SparseArrayExpanderTests` already pin the expander logic exhaustively; the wiring tests need only check that the choke point invokes expansion for each variant, not the expansion semantics themselves. The four secured variants (`WriteSecured`, `Write2`, `WriteSecured2`, `WriteSecured2Bulk`) can reuse the same `CapturingWorkerClient` stub.
**Resolution:** 2026-06-18 — root cause confirmed: the six arms (`WriteSecured`, `Write2`, `WriteSecured2`, `Write2Bulk`, `WriteSecuredBulk`, `WriteSecured2Bulk`) each had a `case` in `NormalizeOutboundCommand` calling `ExpandValue` but no wiring test. Server-057's additions (`AddItemBulk`, `AddBufferedItem`) covered address-normalization tests only, not the missing write-expansion variants. Added six tests to `GatewayArrayWriteWiringTests.cs` — one per uncovered arm — each constructing the matching command with a 4-element `SparseArrayValue` (Integer, single element set), driving it through `GatewaySession.InvokeAsync`, and asserting `worker.LastCommand.Command.<Variant>.Value.KindCase == ArrayValue` and the expected element positions. Tests: `WriteSecured_SparseArrayValue_ExpandedBeforeReachingWorker`, `Write2_SparseArrayValue_ExpandedBeforeReachingWorker`, `WriteSecured2_SparseArrayValue_ExpandedBeforeReachingWorker`, `Write2Bulk_SparseArrayEntryValue_ExpandedBeforeReachingWorker`, `WriteSecuredBulk_SparseArrayEntryValue_ExpandedBeforeReachingWorker`, `WriteSecured2Bulk_SparseArrayEntryValue_ExpandedBeforeReachingWorker`. All 13 `GatewayArrayWriteWiring` tests pass (7 pre-existing + 6 new).
+18 -1
View File
@@ -8,7 +8,24 @@ recreated by the contracts project build.
`src/ZB.MOM.WW.MxGateway.Contracts/Protos/mxaccess_gateway.proto` defines the public
`MxAccessGateway` gRPC service, command payloads, command replies, event DTOs,
`MxValue`, `MxArray`, and `MxStatusProxy`.
`MxValue`, `MxArray`, `MxSparseArray`, and `MxStatusProxy`.
`MxValue` carries a `kind` oneof with arms for all scalar and array types. One
arm is `sparse_array_value = 19` (field number 19), which carries an
`MxSparseArray`. `MxSparseArray` is a write-only value type: the gateway accepts
it on every write variant (`Write`, `Write2`, `WriteSecured`, `WriteSecured2`,
and the corresponding `*BulkEntry` shapes), expands it into a full,
default-filled `MxArray` before forwarding to the worker, and rejects it on
read or event paths. The worker never receives or produces it.
`MxSparseArray` has three fields: `element_data_type` (1, the `MxDataType` of
every element), `total_length` (2, the length of the expanded full array), and
`elements` (3, `repeated MxSparseElement`). Each `MxSparseElement` has `index`
(1, zero-based position in the expanded array) and `value` (2, a scalar
`MxValue`). Indices not mentioned in `elements` take the element type's default
value — they are reset, not preserved. See `gateway.md` section
"MxSparseArray — default-fill partial array writes" for the expansion rules,
validation constraints, and the scope requirements per write variant.
The public command model includes bulk subscription command kinds for
`AddItemBulk`, `AdviseItemBulk`, `RemoveItemBulk`, `UnAdviseItemBulk`,
+11 -8
View File
@@ -1124,14 +1124,17 @@ Known important parity areas from existing captures:
representation (unmentioned indices → type default) before sending the
whole-array write to the worker.
- Array attribute addresses require the `[]` body suffix to be write-capable.
The gateway normalizes bare-name addresses at `AddItem` time: when Galaxy
metadata confirms `is_array`, the gateway appends `[]` before registering the
handle with the worker. When metadata is unavailable or the address is not
recognized as an array, the address is forwarded unchanged so existing
behavior is not regressed. The normalized address is stored in
`SessionItemRegistration.TagAddress` and applies consistently to all
subsequent writes on that handle. `ReadBulk` is unaffected — it uses raw
address strings with its own ephemeral registration.
The gateway normalizes bare-name addresses at add-item time across the whole
add family — single `AddItem`/`AddItem2`, the batched `AddItemBulk`, and
`AddBufferedItem`: when Galaxy metadata confirms `is_array`, the gateway
appends `[]` before registering the handle with the worker. When metadata is
unavailable or the address is not recognized as an array, the address is
forwarded unchanged so existing behavior is not regressed. The normalized
address is stored in `SessionItemRegistration.TagAddress` (for `AddItemBulk`
the worker echoes the suffixed address it bound back in each
`SubscribeResult.TagAddress`) and applies consistently to all subsequent
writes on that handle. `ReadBulk` is unaffected — it uses raw address strings
with its own ephemeral registration.
The gateway should not "fix" these behaviors unless the client explicitly opts
into a non-parity mode.
@@ -987,6 +987,19 @@ public sealed class GatewaySession
break;
case MxCommand.PayloadOneofCase.AddItem2:
command.AddItem2.ItemDefinition = NormalizeAddress(command.AddItem2.ItemDefinition);
break;
case MxCommand.PayloadOneofCase.AddBufferedItem:
command.AddBufferedItem.ItemDefinition = NormalizeAddress(command.AddBufferedItem.ItemDefinition);
break;
case MxCommand.PayloadOneofCase.AddItemBulk:
// Normalize each bare array address in place so the worker binds a write-capable handle
// for every array tag in the batch (the same IsArray-gated rewrite the single-add path
// applies). Scalar addresses pass through unchanged.
for (int i = 0; i < command.AddItemBulk.TagAddresses.Count; i++)
{
command.AddItemBulk.TagAddresses[i] = NormalizeAddress(command.AddItemBulk.TagAddresses[i]);
}
break;
case MxCommand.PayloadOneofCase.Write:
ExpandValue(command.Write.Value);
@@ -1089,9 +1102,16 @@ public sealed class GatewaySession
TrackItem(command.AddItem2.ServerHandle, reply.AddItem2.ItemHandle, NormalizeAddress(command.AddItem2.ItemDefinition));
break;
case MxCommandKind.AddBufferedItem when reply.AddBufferedItem is not null:
TrackItem(command.AddBufferedItem.ServerHandle, reply.AddBufferedItem.ItemHandle, command.AddBufferedItem.ItemDefinition);
// The reply carries no address, so tracking keys off the command's ItemDefinition;
// re-apply the array-suffix normalization (the tracking copy is a separate, un-mutated
// instance from the one forwarded at the InvokeAsync choke point) so the registration
// matches the write-capable handle the worker bound.
TrackItem(command.AddBufferedItem.ServerHandle, reply.AddBufferedItem.ItemHandle, NormalizeAddress(command.AddBufferedItem.ItemDefinition));
break;
case MxCommandKind.AddItemBulk when reply.AddItemBulk is not null:
// The worker echoes back the (already-normalized) address it bound in each
// SubscribeResult.TagAddress, so TrackBulkItems stores the suffixed array address
// without re-normalizing here.
TrackBulkItems(reply.AddItemBulk);
break;
case MxCommandKind.SubscribeBulk when reply.SubscribeBulk is not null:
@@ -13,15 +13,18 @@ public sealed class GatewayContractInfoTests
/// <summary>
/// Pins the current <see cref="GatewayContractInfo.GatewayProtocolVersion"/>
/// constant at 3. Both the alarm proto extension (`AcknowledgeAlarm` /
/// `QueryActiveAlarms` RPCs, the `OnAlarmTransitionEvent` body, and the
/// alarm command/reply payload cases) and the bulk write/read command
/// family extension (`WriteBulk` / `Write2Bulk` / `WriteSecuredBulk` /
/// `WriteSecured2Bulk` / `ReadBulk` plus their `BulkWriteReply` and
/// `BulkReadReply` payloads) shipped under version 3 — both were strictly
/// additive contract changes, so neither required a further bump. A
/// future breaking contract change should bump this constant and update
/// this test in lock-step.
/// constant at 3. All three of the following extensions shipped under
/// version 3 as strictly additive contract changes that did not require a
/// further bump: (1) the alarm proto extension (<c>AcknowledgeAlarm</c> /
/// <c>QueryActiveAlarms</c> RPCs, the <c>OnAlarmTransitionEvent</c> body,
/// and the alarm command/reply payload cases); (2) the bulk write/read
/// command family (<c>WriteBulk</c> / <c>Write2Bulk</c> /
/// <c>WriteSecuredBulk</c> / <c>WriteSecured2Bulk</c> / <c>ReadBulk</c>
/// plus their <c>BulkWriteReply</c> and <c>BulkReadReply</c> payloads);
/// (3) the sparse-array write ergonomics extension (<c>MxSparseArray</c> /
/// <c>MxSparseElement</c> messages plus the <c>sparse_array_value = 19</c>
/// arm on the <c>MxValue</c> oneof). A future breaking contract change
/// should bump this constant and update this test in lock-step.
/// </summary>
[Fact]
public void GatewayProtocolVersion_IsVersionThree()
@@ -1588,4 +1588,87 @@ public sealed class ProtobufContractRoundTripTests
Assert.Equal(150UL, parsed.ReplayGap.RequestedAfterSequence);
Assert.Equal(200UL, parsed.ReplayGap.OldestAvailableSequence);
}
/// <summary>
/// Pins the wire field number for <c>MxValue.sparse_array_value</c> (19)
/// via the generated constant, pins the <see cref="MxSparseArray"/> and
/// <see cref="MxSparseElement"/> field numbers by name + number against the
/// descriptor, and round-trips an <see cref="MxValue"/> carrying a
/// <see cref="MxSparseArray"/> with at least one <see cref="MxSparseElement"/>
/// so a future renumber or type-narrowing is caught at the contract level.
/// Also verifies that an all-defaults <see cref="MxSparseArray"/> (no elements)
/// is not a proto-level error. See Contracts-023.
/// </summary>
[Fact]
public void MxValue_RoundTripsSparseArrayValueAndPinsFieldNumbers()
{
// Pin MxValue.sparse_array_value wire field number == 19.
Assert.Equal(19, MxValue.SparseArrayValueFieldNumber);
// Pin MxSparseArray field numbers by name + number.
var sparseArrayFields = MxSparseArray.Descriptor.Fields;
Assert.Equal(1, sparseArrayFields[MxSparseArray.ElementDataTypeFieldNumber].FieldNumber);
Assert.Equal("element_data_type", sparseArrayFields[MxSparseArray.ElementDataTypeFieldNumber].Name);
Assert.Equal(2, sparseArrayFields[MxSparseArray.TotalLengthFieldNumber].FieldNumber);
Assert.Equal("total_length", sparseArrayFields[MxSparseArray.TotalLengthFieldNumber].Name);
Assert.Equal(3, sparseArrayFields[MxSparseArray.ElementsFieldNumber].FieldNumber);
Assert.Equal("elements", sparseArrayFields[MxSparseArray.ElementsFieldNumber].Name);
// Pin MxSparseElement field numbers by name + number.
var sparseElementFields = MxSparseElement.Descriptor.Fields;
Assert.Equal(1, sparseElementFields[MxSparseElement.IndexFieldNumber].FieldNumber);
Assert.Equal("index", sparseElementFields[MxSparseElement.IndexFieldNumber].Name);
Assert.Equal(2, sparseElementFields[MxSparseElement.ValueFieldNumber].FieldNumber);
Assert.Equal("value", sparseElementFields[MxSparseElement.ValueFieldNumber].Name);
// Round-trip an MxValue with a populated MxSparseArray (one scalar element).
var original = new MxValue
{
DataType = MxDataType.Float,
SparseArrayValue = new MxSparseArray
{
ElementDataType = MxDataType.Float,
TotalLength = 4,
Elements =
{
new MxSparseElement
{
Index = 2,
Value = new MxValue
{
DataType = MxDataType.Float,
FloatValue = 3.14f,
VariantType = "VT_R4",
},
},
},
},
};
var parsed = MxValue.Parser.ParseFrom(original.ToByteArray());
Assert.Equal(original, parsed);
Assert.Equal(MxValue.KindOneofCase.SparseArrayValue, parsed.KindCase);
Assert.NotNull(parsed.SparseArrayValue);
Assert.Equal(MxDataType.Float, parsed.SparseArrayValue.ElementDataType);
Assert.Equal(4u, parsed.SparseArrayValue.TotalLength);
var element = Assert.Single(parsed.SparseArrayValue.Elements);
Assert.Equal(2u, element.Index);
Assert.Equal(3.14f, element.Value.FloatValue);
// An all-defaults MxSparseArray (no elements) is not a proto-level error.
var empty = new MxValue
{
DataType = MxDataType.Float,
SparseArrayValue = new MxSparseArray
{
ElementDataType = MxDataType.Float,
TotalLength = 10,
},
};
var parsedEmpty = MxValue.Parser.ParseFrom(empty.ToByteArray());
Assert.Equal(empty, parsedEmpty);
Assert.Equal(MxValue.KindOneofCase.SparseArrayValue, parsedEmpty.KindCase);
Assert.Empty(parsedEmpty.SparseArrayValue.Elements);
}
}
@@ -247,6 +247,440 @@ public sealed class GatewayArrayWriteWiringTests
Assert.Equal(new[] { 0, 7, 0, 0 }, forwarded.ArrayValue.Int32Values.Values);
}
/// <summary>
/// A bare array address added via <c>AddItemBulk</c> is normalized to its writable array form
/// on the wire (so the worker registers a write-capable handle), while a scalar address in the
/// same batch is forwarded unchanged. Tracking the worker's echoed reply lands the normalized
/// address in the <see cref="SessionItemRegistration"/>.
/// </summary>
[Fact]
public async Task AddItemBulk_BareArrayAddress_NormalizedOnWireAndInRegistration()
{
CapturingWorkerClient worker = new();
GatewaySession session = CreateReadySession(worker);
WorkerCommand command = new()
{
Command = new MxCommand
{
Kind = MxCommandKind.AddItemBulk,
AddItemBulk = new AddItemBulkCommand
{
ServerHandle = 1,
TagAddresses = { "Obj.Arr", "Obj.Scalar" },
},
},
};
await session.InvokeAsync(command, CancellationToken.None);
// The array address gains the writable "[]" suffix; the scalar passes through unchanged.
Assert.NotNull(worker.LastCommand);
Assert.Equal(
new[] { "Obj.Arr[]", "Obj.Scalar" },
worker.LastCommand!.Command.AddItemBulk.TagAddresses);
// The real worker echoes back the (suffixed) address it bound; tracking the reply must land the
// normalized address in the registration so a later write resolves the write-capable handle.
MxCommand trackingCopy = new()
{
Kind = MxCommandKind.AddItemBulk,
AddItemBulk = new AddItemBulkCommand
{
ServerHandle = 1,
TagAddresses = { "Obj.Arr", "Obj.Scalar" },
},
};
MxCommandReply reply = new()
{
Kind = MxCommandKind.AddItemBulk,
ProtocolStatus = new ProtocolStatus { Code = ProtocolStatusCode.Ok },
AddItemBulk = new BulkSubscribeReply
{
Results =
{
new SubscribeResult
{
ServerHandle = 1,
ItemHandle = 50,
TagAddress = "Obj.Arr[]",
WasSuccessful = true,
},
new SubscribeResult
{
ServerHandle = 1,
ItemHandle = 51,
TagAddress = "Obj.Scalar",
WasSuccessful = true,
},
},
},
};
session.TrackCommandReply(trackingCopy, reply);
Assert.True(session.TryGetItemRegistration(1, 50, out SessionItemRegistration arrayRegistration));
Assert.Equal("Obj.Arr[]", arrayRegistration.TagAddress);
Assert.True(session.TryGetItemRegistration(1, 51, out SessionItemRegistration scalarRegistration));
Assert.Equal("Obj.Scalar", scalarRegistration.TagAddress);
}
/// <summary>
/// A sparse-array <see cref="WriteSecuredCommand"/> value is expanded to a full,
/// default-filled <see cref="MxArray"/> before reaching the worker; the secured variant's
/// <c>case</c> arm in <c>NormalizeOutboundCommand</c> is wired to the expander.
/// </summary>
[Fact]
public async Task WriteSecured_SparseArrayValue_ExpandedBeforeReachingWorker()
{
CapturingWorkerClient worker = new();
GatewaySession session = CreateReadySession(worker);
WorkerCommand command = new()
{
Command = new MxCommand
{
Kind = MxCommandKind.WriteSecured,
WriteSecured = new WriteSecuredCommand
{
ServerHandle = 1,
ItemHandle = 42,
Value = new MxValue
{
SparseArrayValue = new MxSparseArray
{
ElementDataType = MxDataType.Integer,
TotalLength = 4,
Elements =
{
new MxSparseElement
{
Index = 2,
Value = new MxValue { Int32Value = 9 },
},
},
},
},
},
},
};
await session.InvokeAsync(command, CancellationToken.None);
MxValue forwarded = worker.LastCommand!.Command.WriteSecured.Value;
Assert.Equal(MxValue.KindOneofCase.ArrayValue, forwarded.KindCase);
Assert.Equal(new[] { 0, 0, 9, 0 }, forwarded.ArrayValue.Int32Values.Values);
}
/// <summary>
/// A sparse-array <see cref="Write2Command"/> (timestamped) value is expanded to a full,
/// default-filled <see cref="MxArray"/> before reaching the worker; the Write2 variant's
/// <c>case</c> arm in <c>NormalizeOutboundCommand</c> is wired to the expander.
/// </summary>
[Fact]
public async Task Write2_SparseArrayValue_ExpandedBeforeReachingWorker()
{
CapturingWorkerClient worker = new();
GatewaySession session = CreateReadySession(worker);
WorkerCommand command = new()
{
Command = new MxCommand
{
Kind = MxCommandKind.Write2,
Write2 = new Write2Command
{
ServerHandle = 1,
ItemHandle = 42,
Value = new MxValue
{
SparseArrayValue = new MxSparseArray
{
ElementDataType = MxDataType.Integer,
TotalLength = 4,
Elements =
{
new MxSparseElement
{
Index = 3,
Value = new MxValue { Int32Value = 5 },
},
},
},
},
},
},
};
await session.InvokeAsync(command, CancellationToken.None);
MxValue forwarded = worker.LastCommand!.Command.Write2.Value;
Assert.Equal(MxValue.KindOneofCase.ArrayValue, forwarded.KindCase);
Assert.Equal(new[] { 0, 0, 0, 5 }, forwarded.ArrayValue.Int32Values.Values);
}
/// <summary>
/// A sparse-array <see cref="WriteSecured2Command"/> (timestamped secured) value is expanded
/// to a full, default-filled <see cref="MxArray"/> before reaching the worker; the
/// WriteSecured2 variant's <c>case</c> arm in <c>NormalizeOutboundCommand</c> is wired to
/// the expander.
/// </summary>
[Fact]
public async Task WriteSecured2_SparseArrayValue_ExpandedBeforeReachingWorker()
{
CapturingWorkerClient worker = new();
GatewaySession session = CreateReadySession(worker);
WorkerCommand command = new()
{
Command = new MxCommand
{
Kind = MxCommandKind.WriteSecured2,
WriteSecured2 = new WriteSecured2Command
{
ServerHandle = 1,
ItemHandle = 42,
Value = new MxValue
{
SparseArrayValue = new MxSparseArray
{
ElementDataType = MxDataType.Integer,
TotalLength = 4,
Elements =
{
new MxSparseElement
{
Index = 0,
Value = new MxValue { Int32Value = 3 },
},
},
},
},
},
},
};
await session.InvokeAsync(command, CancellationToken.None);
MxValue forwarded = worker.LastCommand!.Command.WriteSecured2.Value;
Assert.Equal(MxValue.KindOneofCase.ArrayValue, forwarded.KindCase);
Assert.Equal(new[] { 3, 0, 0, 0 }, forwarded.ArrayValue.Int32Values.Values);
}
/// <summary>
/// A sparse-array entry in a <see cref="Write2BulkCommand"/> (timestamped) is expanded to a
/// full, default-filled <see cref="MxArray"/> before reaching the worker; the Write2Bulk
/// variant's <c>case</c> arm in <c>NormalizeOutboundCommand</c> is wired to the expander.
/// </summary>
[Fact]
public async Task Write2Bulk_SparseArrayEntryValue_ExpandedBeforeReachingWorker()
{
CapturingWorkerClient worker = new();
GatewaySession session = CreateReadySession(worker);
WorkerCommand command = new()
{
Command = new MxCommand
{
Kind = MxCommandKind.Write2Bulk,
Write2Bulk = new Write2BulkCommand
{
ServerHandle = 1,
Entries =
{
new Write2BulkEntry
{
ItemHandle = 42,
Value = new MxValue
{
SparseArrayValue = new MxSparseArray
{
ElementDataType = MxDataType.Integer,
TotalLength = 4,
Elements =
{
new MxSparseElement
{
Index = 1,
Value = new MxValue { Int32Value = 11 },
},
},
},
},
},
},
},
},
};
await session.InvokeAsync(command, CancellationToken.None);
MxValue forwarded = worker.LastCommand!.Command.Write2Bulk.Entries[0].Value;
Assert.Equal(MxValue.KindOneofCase.ArrayValue, forwarded.KindCase);
Assert.Equal(new[] { 0, 11, 0, 0 }, forwarded.ArrayValue.Int32Values.Values);
}
/// <summary>
/// A sparse-array entry in a <see cref="WriteSecuredBulkCommand"/> is expanded to a full,
/// default-filled <see cref="MxArray"/> before reaching the worker; the WriteSecuredBulk
/// variant's <c>case</c> arm in <c>NormalizeOutboundCommand</c> is wired to the expander.
/// </summary>
[Fact]
public async Task WriteSecuredBulk_SparseArrayEntryValue_ExpandedBeforeReachingWorker()
{
CapturingWorkerClient worker = new();
GatewaySession session = CreateReadySession(worker);
WorkerCommand command = new()
{
Command = new MxCommand
{
Kind = MxCommandKind.WriteSecuredBulk,
WriteSecuredBulk = new WriteSecuredBulkCommand
{
ServerHandle = 1,
Entries =
{
new WriteSecuredBulkEntry
{
ItemHandle = 42,
Value = new MxValue
{
SparseArrayValue = new MxSparseArray
{
ElementDataType = MxDataType.Integer,
TotalLength = 4,
Elements =
{
new MxSparseElement
{
Index = 2,
Value = new MxValue { Int32Value = 13 },
},
},
},
},
},
},
},
},
};
await session.InvokeAsync(command, CancellationToken.None);
MxValue forwarded = worker.LastCommand!.Command.WriteSecuredBulk.Entries[0].Value;
Assert.Equal(MxValue.KindOneofCase.ArrayValue, forwarded.KindCase);
Assert.Equal(new[] { 0, 0, 13, 0 }, forwarded.ArrayValue.Int32Values.Values);
}
/// <summary>
/// A sparse-array entry in a <see cref="WriteSecured2BulkCommand"/> (timestamped secured) is
/// expanded to a full, default-filled <see cref="MxArray"/> before reaching the worker; the
/// WriteSecured2Bulk variant's <c>case</c> arm in <c>NormalizeOutboundCommand</c> is wired to
/// the expander.
/// </summary>
[Fact]
public async Task WriteSecured2Bulk_SparseArrayEntryValue_ExpandedBeforeReachingWorker()
{
CapturingWorkerClient worker = new();
GatewaySession session = CreateReadySession(worker);
WorkerCommand command = new()
{
Command = new MxCommand
{
Kind = MxCommandKind.WriteSecured2Bulk,
WriteSecured2Bulk = new WriteSecured2BulkCommand
{
ServerHandle = 1,
Entries =
{
new WriteSecured2BulkEntry
{
ItemHandle = 42,
Value = new MxValue
{
SparseArrayValue = new MxSparseArray
{
ElementDataType = MxDataType.Integer,
TotalLength = 4,
Elements =
{
new MxSparseElement
{
Index = 3,
Value = new MxValue { Int32Value = 17 },
},
},
},
},
},
},
},
},
};
await session.InvokeAsync(command, CancellationToken.None);
MxValue forwarded = worker.LastCommand!.Command.WriteSecured2Bulk.Entries[0].Value;
Assert.Equal(MxValue.KindOneofCase.ArrayValue, forwarded.KindCase);
Assert.Equal(new[] { 0, 0, 0, 17 }, forwarded.ArrayValue.Int32Values.Values);
}
/// <summary>
/// A bare array address added via <c>AddBufferedItem</c> is normalized to its writable array
/// form on the wire and in the tracked <see cref="SessionItemRegistration"/>.
/// </summary>
[Fact]
public async Task AddBufferedItem_BareArrayAddress_NormalizedOnWireAndInRegistration()
{
CapturingWorkerClient worker = new();
GatewaySession session = CreateReadySession(worker);
WorkerCommand command = new()
{
Command = new MxCommand
{
Kind = MxCommandKind.AddBufferedItem,
AddBufferedItem = new AddBufferedItemCommand
{
ServerHandle = 1,
ItemDefinition = "Obj.Arr",
},
},
};
worker.NextReply = new WorkerCommandReply
{
Reply = new MxCommandReply
{
Kind = MxCommandKind.AddBufferedItem,
ProtocolStatus = new ProtocolStatus { Code = ProtocolStatusCode.Ok },
AddBufferedItem = new AddBufferedItemReply { ItemHandle = 60 },
},
};
await session.InvokeAsync(command, CancellationToken.None);
Assert.NotNull(worker.LastCommand);
Assert.Equal("Obj.Arr[]", worker.LastCommand!.Command.AddBufferedItem.ItemDefinition);
// AddBufferedItem tracking keys off the command's ItemDefinition (the reply carries no address),
// so the tracking-path normalization must run for the registration to match the bound handle.
MxCommand trackingCopy = new()
{
Kind = MxCommandKind.AddBufferedItem,
AddBufferedItem = new AddBufferedItemCommand
{
ServerHandle = 1,
ItemDefinition = "Obj.Arr",
},
};
session.TrackCommandReply(trackingCopy, worker.NextReply.Reply);
Assert.True(session.TryGetItemRegistration(1, 60, out SessionItemRegistration registration));
Assert.Equal("Obj.Arr[]", registration.TagAddress);
}
private static GatewaySession CreateReadySession(IWorkerClient workerClient)
{
GatewaySession session = new(
@@ -263,6 +263,116 @@ public sealed class ConstraintEnforcerTests
Assert.Equal("tag_metadata", unknown.ConstraintName);
}
/// <summary>
/// The <c>[]</c>-suffix fallback widened *resolution* of a bare array name, not *authorization*:
/// a bare array attribute that resolves through the fallback but is outside the key's read scope
/// must still be denied with a <c>read_scope</c> failure (Server-058).
/// </summary>
[Fact]
public async Task CheckReadTagAsync_WithBareArrayName_OutOfScope_StillDeniedReadScope()
{
ConstraintEnforcer enforcer = CreateEnforcer(out _);
ApiKeyIdentity identity = CreateIdentity(ApiKeyConstraints.Empty with
{
// Scope covers a different object, so the resolved array attribute is out of scope.
ReadTagGlobs = ["Other_001.*"],
});
ConstraintFailure? failure = await enforcer.CheckReadTagAsync(
identity,
"Pump_001.Levels",
CancellationToken.None);
Assert.NotNull(failure);
Assert.Equal("read_scope", failure.ConstraintName);
}
/// <summary>
/// A write against an array handle whose registration carries the suffixed form ("Pump_001.Levels[]")
/// resolves through <c>ResolveTarget</c> and is denied with a <c>write_scope</c> failure when the
/// array attribute is outside the key's write scope (Server-058).
/// </summary>
[Fact]
public async Task CheckWriteHandleAsync_WithSuffixedArrayRegistration_OutOfScope_StillDeniedWriteScope()
{
ConstraintEnforcer enforcer = CreateEnforcer(out _);
ApiKeyIdentity identity = CreateIdentity(ApiKeyConstraints.Empty with
{
WriteSubtrees = ["Other/*"],
});
GatewaySession session = CreateSession();
session.TrackCommandReply(
new MxCommand
{
Kind = MxCommandKind.AddItem,
AddItem = new AddItemCommand
{
ServerHandle = 12,
// The worker-bound, suffixed array address is what lands in the registration.
ItemDefinition = "Pump_001.Levels[]",
},
},
new MxCommandReply
{
ProtocolStatus = ZB.MOM.WW.MxGateway.Server.Grpc.MxAccessGrpcMapper.Ok(),
AddItem = new AddItemReply { ItemHandle = 70 },
});
ConstraintFailure? failure = await enforcer.CheckWriteHandleAsync(
identity,
session,
serverHandle: 12,
itemHandle: 70,
CancellationToken.None);
Assert.NotNull(failure);
Assert.Equal("write_scope", failure.ConstraintName);
}
/// <summary>
/// A write against an in-scope array handle whose registration carries the suffixed form is still
/// denied when the array attribute's <c>SecurityClassification</c> exceeds the key's
/// <c>MaxWriteClassification</c>, resolved through <c>ResolveTarget</c> via the suffixed address
/// (Server-058).
/// </summary>
[Fact]
public async Task CheckWriteHandleAsync_WithSuffixedArrayRegistration_ClassificationTooHigh_StillDenied()
{
ConstraintEnforcer enforcer = CreateEnforcer(out _);
ApiKeyIdentity identity = CreateIdentity(ApiKeyConstraints.Empty with
{
// In scope for the array attribute, but the array's classification (2) exceeds the cap (1).
WriteTagGlobs = ["Pump_001.*"],
MaxWriteClassification = 1,
});
GatewaySession session = CreateSession();
session.TrackCommandReply(
new MxCommand
{
Kind = MxCommandKind.AddItem,
AddItem = new AddItemCommand
{
ServerHandle = 12,
ItemDefinition = "Pump_001.Setpoints[]",
},
},
new MxCommandReply
{
ProtocolStatus = ZB.MOM.WW.MxGateway.Server.Grpc.MxAccessGrpcMapper.Ok(),
AddItem = new AddItemReply { ItemHandle = 71 },
});
ConstraintFailure? failure = await enforcer.CheckWriteHandleAsync(
identity,
session,
serverHandle: 12,
itemHandle: 71,
CancellationToken.None);
Assert.NotNull(failure);
Assert.Equal("max_write_classification", failure.ConstraintName);
}
private static ConstraintEnforcer CreateEnforcer(out FakeAuditWriter auditWriter)
{
auditWriter = new FakeAuditWriter();
@@ -340,6 +450,16 @@ public sealed class ConstraintEnforcerTests
FullTagReference = "Pump_001.Levels[]",
IsArray = true,
},
new GalaxyAttribute
{
// A second array attribute carrying a non-zero security classification so
// the MaxWriteClassification check can be exercised via a suffixed
// registration address resolving through ResolveTarget.
AttributeName = "Setpoints",
FullTagReference = "Pump_001.Setpoints[]",
IsArray = true,
SecurityClassification = 2,
},
},
},
new GalaxyObject