New docs/security.md section "Mesh gRPC ingress is AllowAnonymous by design"
(archreview 03/S14): what the cookie FallbackPolicy covers, why it can block a
correctly-PSK'd call before the interceptor's verdict matters, why the
AllowAnonymous opt-out is safe (all five interceptors fail-closed), the
affected-endpoint table (fused-vs-driver-only distinction), and the two
standing guards (MeshGrpcAnonymousMetadataTests, MeshGrpcMapConventionTests)
plus the auth-stack/driver-only boundary tests.
One cross-reference line each in docs/Configuration.md (§MeshTransport,
§ConfigSource/ConfigServe) and docs/Telemetry.md, so an operator reading
about the listener ports meets the constraint where they'd look for it.
MeshGrpcMapConventionTests - source-scan of Program.cs (solution-root walk,
following FlushLoggingConventionTests.cs's idiom): zero direct
MapGrpcService</MapZbLocalDbSync( occurrences, exactly one
MapOtOpcUaMeshGrpcServices( call.
Complements the endpoint-metadata census (T1-T3): the census catches a map
added to MeshGrpcEndpoints without the AllowAnonymous convention, but cannot
see a map that bypasses the helper and goes straight into Program.cs - that
shape never reaches MapOtOpcUaMeshGrpcServices at all. Verified red against
the pre-T1 Program.cs (five inline map sites, zero mapper calls) and green
against the current extracted form.
Security__Auth__DisableLogin: "${OTOPCUA_DISABLE_LOGIN:-true}" on central-1
and central-2 - byte-identical default behaviour, verified via `docker
compose config` both unset (resolves "true") and with
OTOPCUA_DISABLE_LOGIN=false (resolves "false").
DisableLogin's always-succeeding handler, registered under the cookie scheme
name, masks the AdminUI cookie FallbackPolicy entirely - which is exactly how
03/S14 (the FallbackPolicy blocking every mesh gRPC call on a login-enabled
admin node) survived seven phases of prior live gates that all ran with the
default. The production-faithful mode is now a one-variable flip instead of
an edit-and-remember-to-revert; header comment names the flip and the LDAP
users it needs (archreview 03/S14, R3-01 T8).
MeshGrpcDriverOnlyPipelineTests - the driver-role shape (no AddOtOpcUaAuth, no
UseAuthentication/UseAuthorization at all) behaves identically to the fused
shape's T4 positive/negative tests for NodeCommandService and
TelemetryStreamService: a correctly-keyed call succeeds, a no-key call gets
the interceptor's PermissionDenied + detail string.
Also asserts the STRUCTURAL reason it is unaffected - no
IAuthorizationPolicyProvider registered at all - so a future "register auth
everywhere for consistency" change fails this test before it ever reaches a
live site outage. Confirmed (by temporarily swapping in the pre-T2/T3 mapper)
that this class passes identically on both builds: driver-only nodes were
never exposed to 03/S14 in the first place, with or without
.AllowAnonymous().
Extends MeshGrpcAuthStackTests (T5): a no-bearer or wrong-bearer call to
DeploymentArtifactService.Fetch, TelemetryStreamService.Subscribe and
NodeCommandService.Execute fails with the INTERCEPTOR's detail string
("Config-serve authentication failed.", "Telemetry stream authentication
failed.", "Node command authentication failed."), asserted alongside the
status code rather than in place of it - the vacuity trap the plan calls out
(ConfigServeAuthInterceptor throws Unauthenticated, and Grpc.Net.Client can
map an HTTP 401 to the same status, so status alone would not discriminate a
broken build).
Verified red-then-green against the T1 (pre-AllowAnonymous) mapper: all 6 new
theory cases fail there with StatusCode.Internal ("HTTP/2 server reset the
stream, PROTOCOL_ERROR") instead of Unauthenticated/PermissionDenied - the
cookie challenge breaking the h2c stream before any gRPC trailers are
written, never a clean auth denial. That is the exact signature the live
gate (T10) should grep for; recorded in the class remarks.
MeshGrpcAuthStackTests - the CI-shaped guard 03/S14 asked for. A real
AddOtOpcUaAuth(config) + UseAuthentication/UseAuthorization pipeline (cookie
scheme, DisableLogin=false, EnvironmentName=Production) with the production
interceptor set, driving DeploymentArtifactService.Fetch,
CentralCommandService.ReportApply and NodeCommandService.Execute through
MapOtOpcUaMeshGrpcServices.
Verified red-then-green by hand: with the T1 (pre-AllowAnonymous) mapper
swapped in, all three positive calls fail with
"HTTP/2 server reset the stream, PROTOCOL_ERROR" (RpcException/Internal) -
the cookie challenge breaking prior-knowledge h2c, not a clean auth denial.
With the current mapper (T2/T3) all three succeed. That asymmetry is the only
thing distinguishing the two builds; every other assertion here (cookie
handler identity, the falsifiability control) is unchanged by the fix and
proves nothing on its own.
Falsifiability control: LocalDbSyncService is mapped directly (bypassing
MapOtOpcUaMeshGrpcServices) with no .AllowAnonymous(), and a correctly-shaped
call to it is refused - proving the fixture's auth stack is genuinely live,
not accidentally permissive.
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.
Every round-2 finding re-verified against code (73 closed / 31 partial / 97 carried);
matrix 19/28 cells up, one down. New Critical 03/S14 (AdminUI FallbackPolicy 401s every
mesh gRPC map on a login-enabled central; masked on every rig gate by DisableLogin=true).
New Highs 03/S15, 05/STAB-19, 05/STAB-18, 03/S16 — the stale-Good family at four seams.
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.
CLAUDE.md's "Browse needs ancestor visibility" bullet and docs/security.md's
ancestor-visibility bullet now name the topmost ancestor: the single shared
root folder (s=OtOpcUa, raw namespace) that both realm trees hang under, its
mapping onto the trie's zero-segment realm-root scope, the either-realm transit
(it fronts both trees), and the realm qualification.
Also corrects the adjacent security.md bullet that claimed "the two namespace
root folders remain visible" — there is one anchor, not one per namespace, and
what an ungranted session sees is the ns=0 Objects listing of it rather than a
browse into it. Flagged rather than silently left, since it contradicts the
mechanism the new bullet documents.
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.
Live gate on the local OrbStack docker-dev rig (branch tip images, fresh
down -v + up -d --build).
404: did NOT reproduce through deploy.sh on a fresh boot — the readiness
gate passed on its first poll, and compose's depends_on: service_healthy
serialization means up -d returns ~30 s after Traefik starts, so the window
is already closed. Reproduced it directly instead, by rapid-polling :9200
across a traefik container restart: connection-refused -> 404 for ~65-160 ms
-> 200. Body is exactly Go's "404 page not found" (19 bytes) and the access
log records it with an EMPTY RouterName/ServiceName at 0ms, so no router
matched and no backend was dialled. Mechanism: the entrypoint binds before
the dynamic file-provider config is loaded. The competing candidate (health-
checked service with no healthy server) is excluded — that is a matched
router answering 503, and there are zero 503s in the log. Caveat recorded:
the earliest 404s predate the accesslog handler itself and are not logged,
so an empty access log does not prove absence.
No-body POST: verified, not 400. The minimal-API DeployRequest? binds both
a missing body and a Content-Length: 0 body as null; the handler falls back
to createdBy = "rest-api". A bare ./docker-dev/deploy.sh answered 202
Accepted and the Deployment row carried CreatedBy = rest-api.
deploy.sh's header NOTE goes from "unverified" to the observed truth.
The racing test encoded two timing assumptions and failed under full-solution
parallel load while passing in isolation. Both are removed; no production code
changed.
1. Assertion shape. failures.ShouldBeEmpty() assumed at most ONE apply-boundary
clear can land inside a single evaluation, so the evaluator's one-shot retry
always suffices. Under contention two can land and the retry legitimately
exhausts. The test now asserts the invariant the retry actually guarantees and
which is timing-independent: no raw disposed-script failure ESCAPES the retry.
Every failure must be the documented exhaustion outcome ("evaluator disposed
during evaluation") and never the pre-fix generic escape ("script threw:
Cannot access a disposed object..."), plus a deliberately loose bound on the
exhaustion rate so the check cannot go vacuous.
2. Workload shape. The fixed 12x400 evaluations against a fixed 45s WaitAsync
budget is CPU-dependent (every clear forces a recompile). Four concurrent
instances of this class reproduced a hard TimeoutException 4/4 - the likely
real shape of the reported load failure. The loops are now duration-bounded
(10s race window or 400 iterations, whichever first); the 45s WaitAsync stays
purely as a hang detector.
Renamed to Evaluate_racing_ClearCompiledScripts_never_leaks_a_raw_disposed_failure
so the name matches what it now asserts; the old name is cited in the doc-comment
for traceability.
Review round on 3e62f161. Two coverage gaps and three wording/comment nits.
IMPORTANT 1 — deregistration is now proven DATA-PRESERVING. The self-heal test
inserted a deployment_pointer row and rebooted unconfigured but never looked at
the row again, so a library regression that touched base-table data during
cleanup would have gone unnoticed — on the very tables that ARE the
boot-from-cache backstop, and on the default-OFF nodes least likely to notice.
The new DisablingReplication_PrunesBookkeepingButNeverTouchesTheCachedDataItself
asserts the row's CONTENT survives (not just its count — a count passes against
a cleanup that blanks columns) while __localdb_row_version and __localdb_oplog,
the bookkeeping deregistration does own, go to zero.
IMPORTANT 2 — mid-loop partial failure is documented and pinned. Both loops flip
four tables with no try/catch, so a throw on the third leaves two flipped and
propagates out of the AddZbLocalDb singleton factory, failing host start. That
is correct — a node that came up anyway would be silently capturing on an
arbitrary subset — and both library calls are idempotent, so the retried boot
completes the flip. Written into OnReady's remarks and pinned by
AFailureMidwayThroughRegistration_FailsBootLoudly_AndTheRetriedBootConverges,
which drives OnReady through a delegating ILocalDb that throws on the Nth
RegisterReplicated, asserts the throw propagates, asserts the wrapper actually
reached that call, then re-runs OnReady unwrapped and asserts all four converge.
MINOR 3 — the Boot helper now records that OnReady's null-logger fallback lands
on Serilog's static Log.Logger, which this assembly never assigns; assertions
about logging pass an explicit logger.
MINOR 5 — log wording: "are both unset" -> "are both absent — unset or blank",
matching IsNullOrWhiteSpace.
MINOR 4 — LocalDbDeploymentArtifactCacheTests said its setup "mirrors
LocalDbSetup.OnReady"; it reproduces the two tables only and registers them
unconditionally without baselining. Softened to say so and to name the
divergence as intentional and inert there.
Falsified both new pins. Wrapper set to failOnCall: 99 (never throws) => the
partial-flip pin is the SOLE failure, so it genuinely exercises the Nth-call
throw path rather than some incidental exception. Predicate forced constant-true
=> five pins red including the new data-preservation one. Both mutations
reverted.
LocalDbSetupTests 15/15. Host.IntegrationTests 226 passed / 0 failed / 5
skipped. Runtime.Tests LocalDbDeploymentArtifactCacheTests 12/12.
LocalDbSetup.OnReady registered all four replicated tables unconditionally on
every driver node, replicating or not. That was not merely wasteful (an
unreplicated node paid two INSERTs plus a json_object of the whole row inside
every write transaction, for an oplog nothing ever drained) — it made
baselineExistingRows unusable, so enabling replication on a long-running
default-OFF node converged on SUBSEQUENT WRITES ONLY and the rows already in the
file stayed invisible to the peer, silently and forever.
Registration now follows one predicate, LocalDbSetup.ReplicationIsConfigured:
LocalDb:Replication:PeerAddress OR :ApiKey non-blank. The OR is load-bearing —
only the dialling half of a pair sets PeerAddress, so keying on it alone would
strip capture from every passive node and the pair would converge in one
direction only. SyncListenPort deliberately does not join it: the passive
endpoint is fail-closed on the key, so a port with no ApiKey syncs with nobody.
configured => RegisterReplicated(table, baselineExistingRows: true), per
table, in the unchanged load-bearing DDL->register order
unconfigured => DeregisterReplicated(table), an idempotent boot self-heal that
drops the capture triggers an earlier build left behind and
prunes that table's oplog/row-version rows. One Information
line, only when it actually cleaned something.
PAIR SYMMETRY: the sync handshake compares registered-table digests fail-closed,
so flipping either key is a both-nodes-together change — set it on both and
restart both. Documented in CLAUDE.md, docs/Configuration.md and the replication
runbook (which claimed "this repo never calls DeregisterReplicated").
Verified against the pinned LocalDb 0.2.1: RegisterReplicated(string, bool) and
DeregisterReplicated(string) -> bool both exist; baselining seeds
__localdb_row_version at HLC 0 and the deregister return value distinguishes
"cleaned" from "nothing to clean". No pin bump needed.
Tests: LocalDbSetupTests gains pins for both branches (configured via
PeerAddress alone and via ApiKey alone; unconfigured => no tables, zero
triggers; late-enable => an HLC-0 baseline ledger entry; the log line fires once
and only when something was cleaned) — four went red against the old
unconditional code, and all four go red again under a constant-true predicate.
LocalDbWiringTests splits its exact-set pin into a replicating graph and a
default-OFF one. LocalDbPairHarness and AlarmSfLegacyMigratorTests built their
database from LocalDb:Path alone while configuring the replication engine from a
second config — production reads one configuration, so the two are now aligned.
Host.IntegrationTests: 224 passed, 0 failed, 5 skipped.
Joseph Doherty