feat(grpc): Phase 0 — PSK-authenticate the site gRPC control plane #25

2026-07-22T18:10:43-04:00

dohertj2 commented

2026-07-22 18:10:43 -04:00

Phase 0 of the ClusterClient→gRPC migration (docs/plans/2026-07-22-clusterclient-to-grpc-plan.md). Prerequisite work only — no traffic moves off ClusterClient in this PR.

What changed

PSK-authenticates the site gRPC control plane. SiteStreamService shipped unauthenticated: plaintext h2c, no interceptor, so anything that could reach a site node's :8083 could open a live data stream or call PullAuditEvents/PullSiteCalls and read audit rows back. New ControlPlaneAuthInterceptor gates it, fail-closed.
Per-site keys, not a fleet-wide one (SB-GRPC-PSK-{siteId}), so one leaked key is one site. Central resolves them at channel-build time via ISitePskProvider (its site set is dynamic); sites resolve their own key pre-host through the existing ${secret:} expander.
Separate from LocalDb replication's key, deliberately — that authenticates the pair partner, not central. Sharing one would mean a site's central-facing key also admits DB writes. Pinned by test.
Deletes the vestigial /user/management receptionist registration. Zero callers; in-process access goes through ManagementActorHolder.
Records the dead IntegrationCallRequest path in docs/known-issues/, with a wire-or-delete decision due before Phase 4.

Verification

Live gate PASS, 7 checks on the 9-container rig — docs/plans/2026-07-22-clusterclient-to-grpc-live-gate.md. Baseline captured first, proving an unauthenticated PullAuditEvents returned {} before the change. After: unauthenticated → PermissionDenied on all 3 sites; site-b's key against site-a → PermissionDenied (per-site scoping is real); correct key → success; 14 successful central→site pulls with 0 auth failures; LocalDb sync unaffected.
29 suites / 6,872 tests / 0 failures. Playwright 170/173 — the 2 failures are pre-existing on main and root-caused in the gate doc; this PR touches no EF/CentralUI/Transport/ManagementService file. One of them is a real production bug (Transport bundle import is broken against real MS SQL) worth its own issue.

Read this before Phase 1A/1B

The first cut of this gate was inert and the green suite did not notice. ControlPlaneAuthInterceptor had two public constructors; Grpc.AspNetCore's InterceptorRegistration.GetFactory() throws on that, per call, inside the pipeline. Nothing failed at startup — the node booted, joined and reported healthy — and every gated call died with Unknown / "Exception was thrown by handler", which reads as a handler bug. The tell was that correct key, wrong key and no key produced identical errors.

The E2E test missed it because it pre-registered the interceptor with AddSingleton, so DI handed back the instance and gRPC's activation path — the thing that threw — never ran.

Phases 1A and 1B both add services to this same interceptor: extend DefaultGatedPrefixes, do not add a second public constructor. The 3-arg ctor is internal now and a reflection test pins "exactly one public constructor".

Deploy note — breaking

A site node on this build will not boot without a key (StartupValidator, deliberate: "no key" must never degrade to "no auth"). The gitignored deploy/wonder-app-vd03/ overlay needs ScadaBridge:Communication:GrpcPsk added before its next deploy. docker-env2 has its key in config but was not redeployed or gated.

Phase 0 of the ClusterClient→gRPC migration (`docs/plans/2026-07-22-clusterclient-to-grpc-plan.md`). Prerequisite work only — **no traffic moves off ClusterClient in this PR.** ## What changed - **PSK-authenticates the site gRPC control plane.** `SiteStreamService` shipped unauthenticated: plaintext h2c, no interceptor, so anything that could reach a site node's :8083 could open a live data stream or call `PullAuditEvents`/`PullSiteCalls` and read audit rows back. New `ControlPlaneAuthInterceptor` gates it, fail-closed. - **Per-site keys, not a fleet-wide one** (`SB-GRPC-PSK-{siteId}`), so one leaked key is one site. Central resolves them at channel-build time via `ISitePskProvider` (its site set is dynamic); sites resolve their own key pre-host through the existing `${secret:}` expander. - **Separate from LocalDb replication's key**, deliberately — that authenticates the pair partner, not central. Sharing one would mean a site's central-facing key also admits DB writes. Pinned by test. - **Deletes the vestigial `/user/management` receptionist registration.** Zero callers; in-process access goes through `ManagementActorHolder`. - Records the dead `IntegrationCallRequest` path in `docs/known-issues/`, with a wire-or-delete decision due before Phase 4. ## Verification - **Live gate PASS**, 7 checks on the 9-container rig — `docs/plans/2026-07-22-clusterclient-to-grpc-live-gate.md`. Baseline captured first, proving an unauthenticated `PullAuditEvents` returned `{}` before the change. After: unauthenticated → `PermissionDenied` on all 3 sites; site-b's key against site-a → `PermissionDenied` (per-site scoping is real); correct key → success; 14 successful central→site pulls with 0 auth failures; LocalDb sync unaffected. - **29 suites / 6,872 tests / 0 failures.** Playwright 170/173 — the 2 failures are pre-existing on `main` and root-caused in the gate doc; this PR touches no EF/CentralUI/Transport/ManagementService file. **One of them is a real production bug** (Transport bundle import is broken against real MS SQL) worth its own issue. ## Read this before Phase 1A/1B The first cut of this gate was **inert and the green suite did not notice.** `ControlPlaneAuthInterceptor` had two public constructors; `Grpc.AspNetCore`'s `InterceptorRegistration.GetFactory()` throws on that, per call, inside the pipeline. Nothing failed at startup — the node booted, joined and reported healthy — and every gated call died with `Unknown / "Exception was thrown by handler"`, which reads as a handler bug. The tell was that correct key, wrong key and no key produced *identical* errors. The E2E test missed it because it pre-registered the interceptor with `AddSingleton`, so DI handed back the instance and gRPC's activation path — the thing that threw — never ran. **Phases 1A and 1B both add services to this same interceptor: extend `DefaultGatedPrefixes`, do not add a second public constructor.** The 3-arg ctor is `internal` now and a reflection test pins "exactly one public constructor". ## Deploy note — breaking A site node on this build **will not boot without a key** (`StartupValidator`, deliberate: "no key" must never degrade to "no auth"). The gitignored `deploy/wonder-app-vd03/` overlay needs `ScadaBridge:Communication:GrpcPsk` added before its next deploy. `docker-env2` has its key in config but was not redeployed or gated.

dohertj2 added 4 commits 2026-07-22 18:10:44 -04:00

feat(grpc): PSK-authenticate the site gRPC control plane; drop the vestigial management receptionist registration 2ee84af1c0

Phase 0 of the ClusterClient→gRPC migration
(docs/plans/2026-07-22-clusterclient-to-grpc-plan.md). Standalone hardening: it
closes a gap that exists today and is a precondition for moving command/control
onto gRPC in later phases.

T0.1 — delete the ManagementActor ClusterClientReceptionist registration.
It was built for an out-of-cluster CLI that was never written: the shipped CLI
speaks HTTP Basic to /management, which asks the actor in-process through
ManagementActorHolder. Nothing in the repo ever sent to /user/management. The
actor still runs there; only the cross-boundary advertisement is gone. Six
documents claimed the CLI used ClusterClient — including the CLI's own README
"Architecture Notes" — and are corrected here rather than left to rot.

T0.2 — record, do not port, the dead integration-routing path.
IntegrationCallRequest is unwired at BOTH ends: RouteIntegrationCallAsync has
zero callers anywhere, and RegisterLocalHandler(Integration, …) appears only in
a test, so production always answers "Integration handler not available". It is
excluded from the gRPC contract (28 of 29 commands migrate) rather than
enshrined on an additive-only wire format, and deleting it during a
transport migration would mix a behavioural change into a change whose whole
value is that behaviour is identical. See
docs/known-issues/2026-07-22-integration-call-routing-is-dead-code.md.

T0.3 — preshared-key authentication on SiteStreamService.
The service shipped with no auth at all: plaintext h2c, no interceptor, so
anything that could reach a site node's :8083 could open a live data stream or
read audit rows back via PullAuditEvents/PullSiteCalls. ControlPlaneAuthInterceptor
now gates /sitestream.SiteStreamService/ — modeled on LocalDbSyncAuthInterceptor
(constant-time compare, fail-closed, PermissionDenied) but gating a SET of
service prefixes so phases 1A/1B add services rather than interceptors. LocalDb
sync keeps its own separate key: it authenticates the pair partner, not central,
and collapsing the two would make a site's central-facing key also admit writes
into its database.

Keys are per site (SB-GRPC-PSK-<siteId>), never fleet-wide, so a compromised
site yields only its own. Central attaches them through ControlPlaneCredentials,
which binds CallCredentials to the channel — covering unary and streaming
uniformly, and letting the key resolve asynchronously, which a client
interceptor could not do without blocking. All three central→site channel
creation sites go through it (SiteStreamGrpcClient and both audit pull invokers);
the pull invokers' channel caches are re-keyed by (site, endpoint) because
credentials are per-site and bound to the channel.

Two decisions beyond the plan:

  * StartupValidator now requires GrpcPsk on Site nodes. The plan specified only
    the runtime gate, but fail-closed with no boot check produces a node that
    joins, answers heartbeats and reports healthy while refusing every stream,
    audit pull and telemetry ingest — silent and total. Same reasoning as the
    existing inbound API-key pepper rule.

  * Added Communication:SitePsks as a central-side key map. The plan assumed
    central would read the store, seeded via a dev KEK; the docker rig
    deliberately boots with no master key, so store-only resolution would leave
    it unable to dial its own sites. The store stays primary — it is the only
    source that can serve a site added at runtime — with the map covering
    key-less hosts and one-off pins. Neither source falling back to
    "unauthenticated" is the invariant.

T0.4 — dev keys on both rigs and tests.
34 tests. The seven that matter most exercise a real in-process gRPC stack over
TestServer: the unit tests on either side of the wire would both stay green if
the halves disagreed, and gRPC refuses call credentials on a plaintext channel
by default — the UnsafeUseInsecureChannelCallCredentials opt-in is only provable
by making a real call. They confirm correct key passes on unary AND streaming,
wrong key and no-credentials both get PermissionDenied, and an unresolvable key
fails the call with nothing reaching the service.

OPERATIONAL: a site node upgraded to this build without a key will not boot.
That includes the gitignored deploy/wonder-app-vd03/ overlay.

fix(grpc): one public constructor on ControlPlaneAuthInterceptor — two made the gate inert 228ff8b428

Caught by the Phase 0 live gate, not by the suite.

Grpc.AspNetCore registers the interceptor BY TYPE, and
InterceptorRegistration.GetFactory() throws "Multiple constructors accepting all
given argument types have been found" when more than one public constructor is
applicable. The interceptor had two: the DI one and a prefix-set overload added
for later phases.

The failure mode is nasty. The throw happens inside the interceptor pipeline on
every call, so nothing fails at startup — the site node boots, joins, reports
healthy. Every gated call then dies with Unknown / "Exception was thrown by
handler", which reads as a handler bug rather than an auth bug. And it fails
OPEN in the sense that matters least and closed in the sense that matters most:
no call is ever authorized, but no call is ever correctly REFUSED either, so the
rig showed identical errors for a correct key, a wrong key and no key at all.
Live evidence, site-a: three PullAuditEvents calls, three identical
InvalidOperationExceptions in the node log.

Fix: the prefix-set constructor is internal (Host.Tests already has
InternalsVisibleTo). Later phases extend DefaultGatedPrefixes rather than adding
a second public registration shape.

Why the tests missed it, and what changed: ControlPlaneAuthEndToEndTests
registered the interceptor with AddSingleton alongside AddGrpc, so DI handed
back the instance and Grpc.AspNetCore's activation path — the thing that throws
— never ran. The harness now registers exactly as Program.cs does, by type and
not in DI. Plus a direct reflection assertion that the type has exactly one
public constructor, since that is the real invariant and it is cheap to pin.

docs(grpc): Phase 0 live gate PASS — record results, the inert-gate defect, and the trap for phases 1A/1B 6ef8c7d70a

The gate's first run failed on a defect the green suite could not see: two public
constructors on ControlPlaneAuthInterceptor made Grpc.AspNetCore's activation
throw per call, so correct key, wrong key and no key all produced identical
errors. Recorded in full because the symptom (Unknown / "Exception was thrown by
handler") points at the handler, not at auth, and because phases 1A/1B both add
services to this same interceptor — they must extend DefaultGatedPrefixes rather
than add a second public constructor.

Also records what the gate does NOT cover: live streaming under load, key
rotation on a running pair, and docker-env2 (keyed but neither redeployed nor
gated).

docs(grpc): record the Playwright result and root-cause both failures 3fa955556d

Phase 0's gate doc now carries the full suite picture, not just the rig checks.

Playwright: 170 pass / 2 fail / 1 skip of 173. Both failures were run down to
root cause and both are pre-existing on main, unrelated to this branch (which
touches no EF, CentralUI, Transport or ManagementService file):

- TransportImportTests is a REAL production bug: BundleImporter.cs:1298 opens a
  user-initiated transaction while the central context has EnableRetryOnFailure,
  so SqlServerRetryingExecutionStrategy refuses the split query inside it and
  bundle import fails against real MS SQL. The unit/integration suite cannot see
  it -- the in-memory EF provider has no retrying strategy and BeginTransaction
  is a no-op there.

- SmsNotificationE2ETests is a stale fixture: SID 'ACtest123' (2026-06-19) vs the
  ^AC[0-9a-fA-F]{32}$ guard added 2026-07-10 (40088a21). Failing since then, which
  has also silenced everything after the toast assertion -- including the
  secret-non-leak check on the Auth Token.

Also records that the earlier 44-failure run is void: a concurrent deploy.sh was
recreating the cluster underneath it.

Neither is fixed here; both are out of scope for a PSK-auth branch.

dohertj2 commented

2026-07-22 18:11:09 -04:00

Landed on main by fast-forward at 3fa95555 (no merge commit), so Gitea did not flip this to merged automatically. Closing — the four commits 2ee84af1..3fa95555 are on main and pushed.

Landed on `main` by fast-forward at `3fa95555` (no merge commit), so Gitea did not flip this to merged automatically. Closing — the four commits `2ee84af1..3fa95555` are on `main` and pushed.

dohertj2 closed this pull request

2026-07-22 18:11:09 -04:00

dohertj2 referenced this issue from a commit

2026-07-22 18:12:04 -04:00

docs(plans): tick Phase 0 DoD — PR #25 merged to main @ 3fa95555

Pull request closed

Please reopen this pull request to perform a merge.

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: dohertj2/ScadaBridge#25