Merge branch 'fix/r3-01-mesh-grpc-allowanonymous' — R3-01: mesh gRPC + LocalDb sync endpoints opt out of the AdminUI cookie FallbackPolicy (PSK interceptors are the gate); real-auth-stack boundary tests + map-convention guard; rig DisableLogin parameterised (archreview 03/S14 Critical)

archreview/plans/R3-01-mesh-grpc-allowanonymous-plan.md

@@ -352,3 +352,87 @@ T10 last — and its step-0 control runs on master BEFORE the branch is merged
 ```
 
 Overall effort: **~half a dev-day code+tests, plus the two-pass rig session.** The Critical itself is five lines; everything else in this plan exists because the five lines were invisible to a seven-phase live-gate program.
+
+---
+
+## Execution notes (T1–T9, 2026-08-20)
+
+Branch `fix/r3-01-mesh-grpc-allowanonymous`, one commit per task as specified. T1–T9 all landed;
+T10 (live gate) left `pending` for the orchestrator per the run brief. Every code-touching task
+(T1–T7) was verified red-before-fix by hand — for T2–T5 by asserting against the un-fixed
+`MeshGrpcEndpoints.cs`/`Program.cs` (via a temporary `git show <parent-commit>:<path>` swap,
+rebuild, run, then restore — confirmed byte-identical restore via `git diff --stat` each time)
+and observing the expected failure before reapplying the fix. Two things surfaced during that
+process that are worth recording:
+
+1. **The census's endpoint filter could not stay a "route pattern starts with `/`" heuristic once
+   T3's empty-prefix `MapGroup` landed.** `RealMethodEndpoints` in `MeshGrpcAnonymousMetadataTests`
+   originally filtered on `RoutePattern.RawText?.StartsWith('/')` to exclude grpc-dotnet's own
+   per-app "Unimplemented service" catch-all and per-service "Unimplemented method for X"
+   fallback (neither carries a route pattern starting with `/` in the ungrouped case). Once
+   LocalDbSync moved behind `app.MapGroup(string.Empty).AllowAnonymous().MapZbLocalDbSync()`,
+   those SAME fallback patterns *also* gained a leading `/` inside the group, so the heuristic
+   started over-counting (`DriverOnly_WithASyncPort_AlsoMapsTheLocalDbSyncEndpoint` and
+   `LocalDbSync_AllowAnonymousGroup_LeavesTheRoutePatternByteIdentical` both went red for the
+   wrong reason — 3 endpoints found where 1 was expected). Fixed by matching against the exact
+   known set of five method route-pattern literals instead of a shape heuristic. Not a plan defect
+   — the plan didn't specify the filter mechanism — but a genuine gotcha for anyone extending this
+   census: **grpc-dotnet's route-pattern-shape for its own fallback endpoints is not stable across
+   a plain `MapGrpcService` vs. a grouped one**, so don't filter on shape once any mesh map moves
+   behind a group.
+
+2. **T4's fixture does not map or wire `LocalDbSyncService`'s real dependencies.** The plan's
+   fixture description says "maps the mesh services through `MapOtOpcUaMeshGrpcServices`"; T4's
+   `MeshGrpcAuthStackTests` calls it with `syncListenPort: 0` (LocalDbSync NOT mapped by the
+   helper) and instead hand-maps the library's `LocalDbSyncService` directly, with no
+   `.AllowAnonymous()`, purely as the negative control. This is a minimal, deliberate narrowing:
+   no assertion in T4 or T5 exercises LocalDbSync's real RPC (the plan's own positive/negative
+   lists never name it), and standing up its DI graph (`AddZbLocalDb`/`AddZbLocalDbReplication`,
+   a temp-file SQLite `ILocalDb`) would have added fixture weight with nothing to verify — the
+   authorization middleware refuses an unauthorized call before the service is ever constructed,
+   so the DI graph doesn't need to exist for the negative control to be meaningful. Recorded here
+   because it's a case where the fixture is deliberately narrower than a literal reading of the
+   plan's prose, even though every named assertion is still covered.
+
+3. **The positive/negative service lists T4 and T5 assert are asymmetric, and that asymmetry is
+   the plan's own, not introduced here.** T4's positive list is {`DeploymentArtifactService.Fetch`,
+   `CentralCommandService.ReportApply`, `NodeCommandService.Execute`} — Telemetry is not asserted
+   positively there. T5's no-PSK detail-string list is {Config-serve, Telemetry stream, Node
+   command} — `CentralCommandService`'s own detail string
+   (`"Central command authentication failed."`) is never asserted anywhere in T4/T5. Implemented
+   exactly as specified (no scope widening per the run brief); flagging the gap in case a
+   follow-up wants a `NoOrWrongPsk_CentralCommandReportApply_FailsWithTheInterceptorsDetail` case
+   for full symmetry — trivial to add given the fixture already maps and keys that service.
+
+4. **Observed cookie-challenge signature (T5 step 2), captured directly against this repo's
+   fixture stack (macOS, Kestrel h2c, `Grpc.Net.Client` over `HttpClient`/`SocketsHttpHandler`):**
+   every blocked positive-test call in the red state failed as
+   `RpcException(StatusCode.Internal, "Error starting gRPC call. HttpRequestException: The HTTP/2
+   server reset the stream. HTTP/2 error code 'PROTOCOL_ERROR' (0x1). (HttpProtocolError)
+   HttpProtocolException: The HTTP/2 server reset the stream. HTTP/2 error code 'PROTOCOL_ERROR'
+   (0x1). (HttpProtocolError)")` — never `Unauthenticated`/`PermissionDenied`. The cookie handler's
+   redirect/401 breaks the prior-knowledge h2c stream before any gRPC trailers are written, so the
+   client sees a transport-level reset rather than a clean gRPC status. **The live gate (T10)
+   should grep node/central logs and browser network traces for `PROTOCOL_ERROR`, `/login`, `302`
+   and `401` together** — not for `401` alone, and not for a gRPC `Unauthenticated`/
+   `PermissionDenied` status, which this environment never actually produces for the blocked case.
+   (A real reverse-proxy in front of a production deployment could still surface a clean HTTP 401
+   instead of a stream reset, depending on how it terminates HTTP/2 — the plan's own multi-signal
+   grep advice already covers that; this note is the empirical confirmation of one concrete shape.)
+
+5. **T6 passes identically on both the pre-fix and post-fix mapper (confirmed by the same
+   temporary-swap procedure).** Driver-only nodes never register `AddOtOpcUaAuth` at all, so
+   `.AllowAnonymous()` presence or absence on the mesh maps has zero effect on that shape — T6 is
+   a structural pin/regression-control test, not a red-then-green TDD test for a behaviour T1–T3
+   changed. This matches the plan's own "standard (test-only)" classification; recorded because
+   the run brief asked for red-before-fix confirmation to be checked (and reported) for every task,
+   and this is the one task where "red" never existed to begin with.
+
+6. **T8's `docker compose config` verification ran locally on macOS (OrbStack-backed Docker
+   Compose v5.1.2)** — no Windows-VM `lmxopcua-fix` helper needed, consistent with the Mac-path
+   guidance in CLAUDE.md. Confirmed both branches: unset `OTOPCUA_DISABLE_LOGIN` resolves
+   `Security__Auth__DisableLogin: "true"` on both central-1/central-2; `OTOPCUA_DISABLE_LOGIN=false`
+   resolves `"false"` on both.
+
+No other deviations from the plan's design or task breakdown. T10 was not run — left `pending`
+per the run brief, for the orchestrator.

archreview/plans/R3-01-mesh-grpc-allowanonymous-plan.md.tasks.json

@@ -5,13 +5,13 @@
     {
       "id": "T1",
       "subject": "Extract MapOtOpcUaMeshGrpcServices from Program.cs:743-784 (behaviour-preserving) + endpoint census scaffold (03/S14)",
-      "status": "pending",
+      "status": "done",
       "blockedBy": []
     },
     {
       "id": "T2",
       "subject": "Census asserts IAllowAnonymous + scope/count guard-of-the-guard; .AllowAnonymous() on the four MapGrpcService maps",
-      "status": "pending",
+      "status": "done",
       "blockedBy": [
         "T1"
       ]
@@ -19,7 +19,7 @@
     {
       "id": "T3",
       "subject": "LocalDb sync endpoint anonymised via an empty-prefix MapGroup (MapZbLocalDbSync returns IEndpointRouteBuilder, not chainable)",
-      "status": "pending",
+      "status": "done",
       "blockedBy": [
         "T2"
       ]
@@ -27,7 +27,7 @@
     {
       "id": "T4",
       "subject": "MeshGrpcAuthStackTests: PSK-bearing calls succeed through the REAL AddOtOpcUaAuth + UseAuthentication/UseAuthorization stack, with an un-anonymised negative control",
-      "status": "pending",
+      "status": "done",
       "blockedBy": [
         "T3"
       ]
@@ -35,7 +35,7 @@
     {
       "id": "T5",
       "subject": "Status fidelity: a no-PSK call reports the interceptor's DETAIL string, not a cookie 401/302 (ConfigServe Unauthenticated collision makes status-only assertions vacuous)",
-      "status": "pending",
+      "status": "done",
       "blockedBy": [
         "T4"
       ]
@@ -43,7 +43,7 @@
     {
       "id": "T6",
       "subject": "MeshGrpcDriverOnlyPipelineTests: driver-only nodes carry no authorization middleware — pins the 03/S14 exemption",
-      "status": "pending",
+      "status": "done",
       "blockedBy": [
         "T3"
       ]
@@ -51,7 +51,7 @@
     {
       "id": "T7",
       "subject": "MeshGrpcMapConventionTests: Program.cs source scan — zero direct MapGrpcService/MapZbLocalDbSync sites, exactly one mapper call",
-      "status": "pending",
+      "status": "done",
       "blockedBy": [
         "T1"
       ]
@@ -59,13 +59,13 @@
     {
       "id": "T8",
       "subject": "docker-dev: Security__Auth__DisableLogin parameterised as ${OTOPCUA_DISABLE_LOGIN:-true} on central-1/central-2 + header docs",
-      "status": "pending",
+      "status": "done",
       "blockedBy": []
     },
     {
       "id": "T9",
       "subject": "docs/security.md \"Mesh gRPC ingress is AllowAnonymous by design\" + cross-refs in docs/Configuration.md and docs/Telemetry.md",
-      "status": "pending",
+      "status": "done",
       "blockedBy": []
     },
     {