options.Roles is unconditionally overwritten by AddOtOpcUaCluster's PostConfigure to equal
EffectiveRoles.Gossiped (T2's own structural-identity fix), so comparing options.Roles against
Gossiped could never observe a divergence — an overlay setting Cluster:Roles was silently
ignored, not fail-start. The validator now injects IConfiguration and re-reads the raw
Cluster:Roles section LIVE (reflecting any overlay applied after EffectiveRoles was first
resolved), normalized through EffectiveRoles.Resolve for reuse of its own validation.
AddOtOpcUaCluster now TryAddSingleton(configuration) so a bare ServiceCollection test harness
(not just WebApplicationBuilder) can supply IConfiguration to the validator.
Both guard tests rewritten to go through the real AddOtOpcUaCluster + PostConfigure pipeline
over a live ConfigurationManager, simulating an overlay as a second provider added AFTER
EffectiveRoles was resolved off the same object — exactly Program.cs's own sequencing. Confirmed
red beforehand: the "changes" test failed to throw against the pre-fix validator.
The default decider restarts a faulting child immediately and without limit, and
the hook makes every restart do ShutdownAsync+InitializeAsync+SubscribeAsync, so a
driver faulting on every value became a reconnect storm. Per-driver last-reprovision
stamp + DefaultReprovisionBackoff (10s, matched to DefaultReconnectInterval);
a held request is re-delivered by a single per-driver timer carrying the original
incarnation ref, so the F2 identity check still applies.
Seen RED with the backoff disabled: InitializeCount == 3 (second applied
immediately) where the test requires 2-then-3.
Also: the _children.Keys union-arm test (seeder gained taglessDrivers) and the
_desiredByDriver xmldoc note that it lingers on last-known-good after a failed apply.
A ReprovisionRequested in flight from an old incarnation could reach the NEW child
after HandleRestartDriver swapped it in. In Connecting that child DOES handle
InitializeRequested, so it started a second concurrent InitializeAsync, racing the
#527 dispose-before-overwrite.
Seen RED: InitializeCount == 2 on a child held in Connecting by a gated init
(an already-connected child swallows InitializeRequested and passes vacuously).
The alarm subscribe gated on 'a handle exists', which is correct for an idempotent
re-push and wrong for a deploy that widens/narrows the set. Combined with the
spawn-time desired set, a respawning deploy could latch the PREVIOUS revision's
alarm refs for the life of the connection; sourceNodeIds-filtering drivers
(OpcUaClient/AbCip/Focas) would never deliver the new alarm.
Seen RED: LastSubscribedAlarmRefs stayed [alarmX] after a deploy widening to
[alarmX, alarmY].
Seen RED against a T3-reverted build: all 4 failed, Created[1].SubscribeCount == 0
on the headline case. 20x isolation run green after the fix was restored.
MapZbLocalDbSync returns IEndpointRouteBuilder, not an IEndpointConventionBuilder,
so .AllowAnonymous() cannot be chained onto it the way the other four mesh maps
are (T2). app.MapGroup(string.Empty).AllowAnonymous().MapZbLocalDbSync() carries
the same convention while leaving the gRPC route pattern byte-identical -
verified directly (LocalDbSync_AllowAnonymousGroup_LeavesTheRoutePatternByteIdentical).
The census's RealMethodEndpoints filter had to move off a "route pattern starts
with /" heuristic to an exact-route-pattern-set match: inside the empty-prefix
group, grpc-dotnet's own "Unimplemented service"/"Unimplemented method" fallback
endpoints also gain a leading "/", so the heuristic let them back in and
over-counted (caught by DriverOnly_WithASyncPort_AlsoMapsTheLocalDbSyncEndpoint
and LocalDbSync_AllowAnonymousGroup_LeavesTheRoutePatternByteIdentical going red
against the naive filter before this fix).
All five mesh gRPC endpoints now carry IAllowAnonymous (archreview 03/S14, T3).
.AllowAnonymous() on DeploymentArtifactGrpcService, CentralCommandGrpcService,
TelemetryStreamGrpcService and NodeCommandGrpcService — the four MapGrpcService
maps that are chainable. On a login-enabled admin node the global cookie
FallbackPolicy (RequireAuthenticatedUser) applies to every endpoint without
explicit authorization metadata, gRPC included; the PSK bearer interceptors run
INSIDE the endpoint invocation, strictly after that middleware, so a
correctly-keyed node call was challenged before the interceptor's verdict could
matter. Safe because all four interceptors fail closed on an unset key.
The census now asserts IAllowAnonymous on all four (red before this change,
green after) plus a scope guard proving the opt-out never leaks onto an
unrelated endpoint. MapZbLocalDbSync (T3) is not chainable the same way and is
asserted absent here on purpose, so this test cannot start passing for the
wrong reason once T3 lands.
Behaviour-preserving: Program.cs:743-784 collapses to a single call, same gates,
same order, every existing per-map rationale comment carried over. A new
endpoint-metadata census (MeshGrpcAnonymousMetadataTests) drives the SAME
production mapper across all three role shapes (admin-only / driver-only /
fused), so the guard can never assert a shape production no longer has -
archreview 03/S14, T1.
The XML remark above OtOpcUaNodeManager.Browse still carried the exact claim the
security.md correction in 22ae1836 removed: there is ONE shared root folder
(V3NodeIds.SharedRootFolderId, raw namespace) fronting both realm trees, not one
per namespace. Rewritten to name the anchor, the ns=0 listing behaviour, and the
zero-segment realm-root mapping that now admits transit holders.
Also completes the BuildScope collision remark (code-review Minor): the degenerate
raw folder literally named OtOpcUa loses the accidental grant leak onto the
anchor's Read/Write that the old one-segment scope allowed — a narrowing,
fail-closed side effect, now recorded.
Comment-only change; addresses the code review's sole Important + Minor.
With Security:Acl:Enabled=true, a session holding any grant narrower than
Cluster could not browse the served address space at all — confirmed by the
live gate.
Mechanism. Both realm trees hang under ONE anchor folder minted in
CreateAddressSpace: NodeId("OtOpcUa", <raw ns>). ResolveParentFolder parents
every top-level raw folder AND every top-level UNS area under it, and the
Browse override throws BadUserAccessDenied for the WHOLE call when its source
node is denied. DataPlaneAuthorizer.BuildScope split that identifier like a
path, producing {Kind: Raw, Segments: ["OtOpcUa"]} — but trie grant paths are
["raw", <RawPath segments…>] and "OtOpcUa" is never a path segment, so
PermissionTrie.HasBrowseTransit failed at the realm-child lookup and
CollectMatches yielded only Cluster-scoped grants. Deny, at the front door.
The trie already handled a zero-segment scope as "the realm root, itself an
ancestor of everything granted in that realm" (PermissionTrie.cs:101-118) —
no production path had ever produced that scope. This wires it up.
- V3NodeIds.SharedRootFolderId names the anchor identity; CreateAddressSpace
now mints the NodeId/BrowseName/DisplayName from it (behaviour-identical,
makes the coupling explicit).
- BuildScope maps that identifier (realm-qualified to Raw — in the UNS
namespace s=OtOpcUa is an ordinary Area) onto the zero-segment Raw scope.
- Evaluate adds a second-realm OR-leg for Browse on the anchor only: it fronts
both trees, so a principal whose only Browse grant lives in the UNS tree
still enters through this raw node. Browse only; transit confers navigation,
never read/write/subscribe.
- No trie, evaluator or builder change. No reordering above the decision cache
— key revocation is still re-checked before the lookup — and the cache key
is unchanged (the combined verdict caches under the anchor's own
identifier/realm; a generation advance clears it).
Falsifications actually run (implement-then-revert-the-line, plus one
genuine red-first):
- DataPlaneAuthorizerTests: the 7 new facts were written BEFORE the fix and 4
went red (the 3 deny-only ones passed vacuously, as a deny-only test must).
- Deleting the UNS OR-leg reddens exactly one fact,
The_shared_root_folder_is_browsable_on_a_uns_only_browse_grant (26/27 pass).
- Dropping the realm qualification from IsSharedRootFolder reddens exactly
A_uns_area_named_like_the_anchor_gets_no_anchor_semantics. Its first draft
did NOT catch this — a raw-only grant is refused either way; the
discriminating grant had to be an unrelated UNS one, whose own transit marks
the zero-segment UNS scope the un-qualified mapping would land on.
- Disabling the BuildScope anchor branch reddens the new end-to-end fact
NodeManagerStage6EnforcementTests.The_shared_root_folder_is_browsable_on_a_narrow_grant_and_still_filters
(real booted server, real Browse dispatch: BadUserAccessDenied on the whole
call), which also pins that the reference filter still withholds the
non-granted sibling.
- Core PermissionTrieBrowseTransitTests gains 3 characterization facts pinning
the previously-dead realm-root branch (transit true for the granting realm's
root, false for the other realm's, false without the Browse bit). These were
green on arrival — the trie was always correct; nothing called it.
Tests: Core 351 -> 354 passed; OpcUaServer 482 -> 490 passed. Build clean.
Review follow-ups to 34829ea0.
IMPORTANT - the leak filter had its own flake vector. This test saturates the
thread pool by design, and TimedScriptEvaluator starts its 5s clock at WaitAsync
rather than when the inner Task.Run gets a thread, so a legitimately-enqueued
evaluation can blow the budget while still queued and surface "script timed out
after 5.0s". The reason != exhaustion filter classified that as a raw disposed
escape and hard-failed on it - handing the test back the load-fragility the
reshape removed. Timeouts are now their own bucket, tolerated and rate-bounded
exactly as exhaustions are; a raw disposed escape still fails on first occurrence.
Doc-comment names this second residual vector alongside the exhaustion calibration.
MINOR (scope extension authorised by review) - "evaluator disposed during
evaluation" existed as three unshared literals. Promoted to
ScriptEvalFailureReasons alongside VirtualTagEvalResult in Commons.Engines (the
assembly all three already reference) and referenced from both evaluators and the
test. The timeout prefix joins it, since the test now classifies on it too. String
values are byte-identical - no behaviour change.
Falsified: with a 12ms script budget the race produced real timeouts (3/2695).
Classified as leaks (the pre-review filter) they hard-failed the test 6/6 items;
bucketed as starvation they sit 0.11% under the 10% bound and the test is green.
Joseph Doherty