Commit Graph

11 Commits

Author SHA1 Message Date
Joseph Doherty 7a2980e2f1 fix(grpc): keep the RpcException off the failover warning so no sink can render Status.Detail
Review follow-ups: the warning now carries only the two endpoint URIs and the
status code — attaching the exception object would let any real sink render
its ToString(), which for RpcException includes server-authored Status.Detail.
The leak pin test now also asserts no exception rides on the warning, via a
new WarningExceptions capture on RecordingLogger. Plus two cosmetic nits:
unified ThrowIfNull idiom, documented that FallbackEndpoints mutation after
Validate is unsupported.

Claude-Session: https://claude.ai/code/session_014WNM4vjoVksyyBraTXSZE1
2026-08-07 09:49:45 -04:00
Joseph Doherty 0d4978953f feat(secrets-grpc): FallbackEndpoints + sticky failover hub reader (scadaproj#4)
SecretsGrpcHubClientOptions gains a getter-only FallbackEndpoints list:
additional hub addresses tried in order when a call fails with a
transport-level RpcException. Validation fails startup on blank/relative
entries and on any duplicate of the primary or another fallback (a
duplicated endpoint silently halves the configured failover).

New FailoverSecretsHubReader composes one GrpcSecretsHubClient per
endpoint: the answering endpoint is sticky (a sweep is GetManifest + N
GetSecrets batches — without stickiness every call after a primary
outage re-pays a full CallDeadline before reaching the healthy
fallback), rotation wraps so a recovered primary is retried when the
stuck-to endpoint fails, only RpcException triggers failover
(cancellation and defects propagate untouched), and when every endpoint
fails the last RpcException is rethrown so the sweep degrades to
last-known-good exactly as today. Failover warnings name only the two
endpoint URIs and the status code — never any credential material.

DI: zero fallbacks keeps the registration graph byte-identical to 0.4.x
(single keyed channel under ChannelServiceKey, plain
GrpcSecretsHubClient); with fallbacks, one container-owned keyed channel
per endpoint (fallback i under "{ChannelServiceKey}:fallback:{i}") wired
into the failover reader. GrpcSecretsHubClient itself is unchanged.

Safe only when every listed endpoint serves the SAME store — for
ScadaBridge, the central pair over one shared SQL Server store; the
options doc and CHANGELOG carry the warning.

Claude-Session: https://claude.ai/code/session_014WNM4vjoVksyyBraTXSZE1
2026-08-07 09:37:22 -04:00
Joseph Doherty c86cead5c2 fix(secrets): rate-limited denial warning on the Grpc hub (live-gate finding) — 0.4.1
The ScadaBridge live gate (check 4) found SecretsHubAuthInterceptor denying
correctly but logging nothing: the only trace of a refusal was a framework
Grpc.AspNetCore.Server Information line. A follower whose bearer token was
mis-rotated therefore stopped converging silently, with the hub showing it
only at INF.

Every denial is now counted and summarised in a Warning emitted on the first
denial after startup or after a 60-second quiet window, carrying the denials
since the previous summary plus a breakdown by cause. A single probe is
visible immediately; a sustained flood is bounded to one line per window,
which is what makes warning safe at all — the endpoint is unauthenticated by
construction, so a per-denial warning would be an unbounded log write driven
by an unauthenticated caller. Counters are Interlocked and the emit is
decided by a compare-exchange on the last-emit timestamp, so exactly one
thread emits per window and a denial counted after the drain carries into
the next summary rather than being lost.

The cause breakdown is server-side diagnostics only. Denied() is untouched:
every rejection still returns the identical uniform Unauthenticated status,
the comparison is still constant-time, and no token material appears in any
log line. The existing warn-once-on-unconfigured-token signal is unchanged
and still emitted exactly once; denials in that state count toward the new
summary's "unconfigured" category.

TimeProvider is an optional default-valued parameter rather than a second
overload — the type must keep exactly one public constructor or the gRPC
interceptor activator silently stops invoking the auth gate.

Two existing tests scoped their assertions to the configuration warning,
since both kinds now share a category: the new line is deliberate behavior,
not drift.

Claude-Session: https://claude.ai/code/session_014WNM4vjoVksyyBraTXSZE1
2026-08-07 08:26:49 -04:00
Joseph Doherty 5f81e65684 fix(secrets): route the unset-token harness through the real hub composition (review drift-risk)
An internal validateEagerly seam on AddZbSecretsGrpcHub replaces the harness's
hand-copied registration fragment, so the deny-all second-layer tests exercise
the production wiring and cannot drift from it.

Claude-Session: https://claude.ai/code/session_014WNM4vjoVksyyBraTXSZE1
2026-08-07 06:52:26 -04:00
Joseph Doherty 3e3748b985 test(secrets): Grpc hub end-to-end integration suite + pull-only contract pin
Adds the end-to-end layer the unit suites structurally cannot reach: a real
in-process ASP.NET Core host serving SecretsHubService through
AddZbSecretsGrpcHub/MapZbSecretsHub, a follower built by the shipped
AddZbSecretsGrpcHubClient, a real gRPC call between them, and real SQLite
stores on both sides. The class of defect this catches is pieces that are each
individually correct and are never connected to one another — for instance the
interceptor's warn-once, which only holds if the container really hands gRPC
the singleton it registered.

Covered: byte-identical convergence over the wire (every ciphertext/nonce/tag/
wrapped-DEK byte, kek_id, revision, both timestamps INCLUDING their offsets,
across a tombstone, a nulls-everywhere row and an empty-string-description row);
delete propagation; a hub unreachable at follower boot leaving the last-known-
good store intact and converging once it answers; the auth negatives fail-closed
across the wire with one uniform denial detail; the per-request name cap as
ResourceExhausted; and an RpcException from the call invoker escaping the hub
client unchanged, so a transport fault stays distinguishable from a parse fault.

The credential-hygiene assertions are made against logs captured at Trace on
BOTH hosts, so "no token material ever reaches a sink" is asserted at the most
verbose setting a deployment could switch on.

SecretsHubContractPinTests reads the generated descriptor — not the .proto text
— and pins the RPC set to exactly {GetManifest, GetSecrets}, plus unary-only and
the package-qualified service name. The hub is pull-only by construction:
updates originate at central only, and the absence of a write RPC is what
enforces that, not authorization a mis-wired follower may hold more of than
intended. A third RPC silently retires the guarantee, so the failure message
says so.

Microsoft.AspNetCore.TestHost pinned at 10.0.7 — the version ZB.MOM.WW.LocalDb,
OtOpcUa and ScadaBridge already pin, matching the Microsoft.Extensions.* line.

Full suite: 436 passed / 0 failed / 15 skipped, no new warnings.

Claude-Session: https://claude.ai/code/session_014WNM4vjoVksyyBraTXSZE1
2026-08-07 06:44:13 -04:00
Joseph Doherty 55e0178d5c merge(secrets): Task 3 client half — pull-only sweep + per-row parse isolation
Claude-Session: https://claude.ai/code/session_014WNM4vjoVksyyBraTXSZE1
2026-08-07 06:32:04 -04:00
Joseph Doherty 1c31e6bcb8 fix(secrets): Grpc hub client — per-row parse isolation (review Critical) + real-client CallInvoker tests
A single malformed row or manifest entry poisoned the whole sweep, permanently.
WireToDomain.ToRows/ToManifest materialized eagerly, so one unparseable element
threw out of the fetch callback, past SecretReplicationReconciler's per-batch
handling, into SweepSafelyAsync's whole-sweep catch. The follower's pull set is
derived from the hub's manifest, so the identical bad element arrived in the
identical batch on every interval and nothing ever converged — logged as "sync
failed", indistinguishable from a hub outage.

Isolation now matches the Akka sibling's split exactly: WireToDomain stays a
strict per-item parser and loses its two batch entry points (their absence is
pinned by a reflection test), while GrpcSecretsHubClient converts each item
inside its own try — the same shape as SecretReplicationActor.Materialize.
A dropped item costs one warning naming the secret and a reason category, and
every well-formed sibling still converges. Only parse-shaped exceptions are
caught (ArgumentException, FormatException); a defect still propagates. A row
whose own name is unparseable is logged under a marker rather than echoing the
untrusted string, and no ciphertext, DEK or credential reaches a log line.

GrpcSecretsHubClient had zero coverage — the existing fake implements the
ISecretsHubReader seam directly, bypassing the header, the deadline and the
conversion, which are the only things that class does. It is now driven for
real through a fake CallInvoker handed to the generated client.

The stale doc comment claiming the reconciler isolated the failure is replaced
by one describing the mechanism that actually exists, and Endpoint gains a note
that a plain-http hub sends the bearer token cleartext (doc only, no validation
change).

Tests 52 -> 67 in the package suite; solution green, no new warnings.

Claude-Session: https://claude.ai/code/session_014WNM4vjoVksyyBraTXSZE1
2026-08-07 06:29:51 -04:00
Joseph Doherty 353af72265 fix(secrets): Grpc hub server review follow-ups — GetSecrets batch cap, DI single-registration guard + tests
Five review items on the Task 2 server half.

1. GetSecrets now enforces a server-side cap on request.Names, refusing an
   over-cap batch with ResourceExhausted BEFORE any store read runs. The
   well-behaved follower chunks at 500, but client discipline is not a server
   defence — the family precedent (HistorianGateway's max_values) is that the
   server declines regardless. The new SecretsGrpcHubServerOptions.
   MaxNamesPerRequest defaults to 1000, comfortably above that chunk so a
   compliant sweep never meets it, and Validate() refuses a non-positive value
   at startup rather than letting it read at runtime as a broken follower. The
   refusal states the cap and the received count — neither is secret, and an
   operator cannot resize a batch without them — and never a name. The cap sits
   behind the interceptor, so it is defence in depth against a misbehaving
   token holder, not an anonymous-DoS control.

2. AddZbSecretsGrpcHub now refuses a second registration. The two calls do not
   compose: options binding MERGES (a key absent from the second section leaves
   the first section's value standing) while the SectionPath stamp is
   last-one-wins, so a hub registered twice could serve a token bound from one
   section while every failure message named the other. Detected via a private
   marker registration. The extension also gains real DI coverage — the intended
   single-call path asserted for token/SectionPath coherence through a built
   provider, a custom section path stamped end to end, a missing token throwing
   from the registration call itself, and the duplicate call throwing.

3. Client-streaming and duplex-streaming denial tests, so all four gated handler
   shapes are pinned rather than only unary and server-streaming, plus a
   valid-token test that reaches all three streaming continuations.

4. A cancellation test: GetSecrets hands the ServerCallContext token through to
   the store, pinned against a fake that records it.

5. The proto's service comment stated the ISecretReplicationHub mapping in prose
   that no longer matched the RPC name; it now says GetManifestAsync ->
   GetManifest and GetManyAsync -> GetSecrets explicitly. Comment-only — no
   message or rpc definition is touched. The GetSecrets rpc comment also notes
   the new RESOURCE_EXHAUSTED cap, which a client author has to know about.

Grpc server suite 50 -> 73 tests; full solution suite green, 0 new warnings.

Claude-Session: https://claude.ai/code/session_014WNM4vjoVksyyBraTXSZE1
2026-08-07 06:27:16 -04:00
Joseph Doherty c5ea3a7b59 feat(secrets): Grpc hub client — pull-only sweep service + options + DI
Client half of the central-hosted secrets hub: a site node follows central and
never pushes.

- `ISecretsHubReader` + `GrpcSecretsHubClient` — the read half of the hub as a
  follower sees it (manifest + bulk fetch, nothing else). Bearer credential and a
  per-call deadline are attached per call; responses convert through
  `WireToDomain`. Faults are deliberately not caught here, so an unreachable hub
  can never be mistaken for an empty one.
- `WireToDomain` — strict, per-row proto -> domain parse mirroring the Akka
  replicator's `SecretRowDto`: a name the domain would reject, a content type
  naming no enum member, or a non-round-trip timestamp is refused at the boundary.
  Explicit proto3 presence keeps null distinguishable from empty.
- `GrpcSecretSyncService` — `SqlServerSecretSyncService` with the push half
  removed. One `ReconcileAsync` per tick (pull-only by the reconciler's own
  contract, and batched, so a cold-start pull set cannot exceed gRPC's 4 MB cap),
  never throws out of `ExecuteAsync`, warns once and retries on the next interval.
  The local store is left completely undecorated — the sweep is its only hub-ward
  writer.
- `SecretsGrpcHubClientOptions` — fail-closed `Validate()` naming the exact
  configuration key, including the section a caller actually bound. The bearer
  token is never echoed by any message.
- `AddZbSecretsGrpcHubClient` — binds, eagerly validates, and registers the
  channel under a service KEY so it cannot lose a `TryAdd` race to an
  application's own `GrpcChannel` and dial the wrong endpoint with the hub
  credential attached.

Tests 52/52 (red-first), including a source-scan guard pinning that no client file
names `ComputePushSet`, `ReadLocalAsync`, or `ApplyReplicatedAsync`, and a
reflection pin that the reader seam exposes exactly two read operations.

Claude-Session: https://claude.ai/code/session_014WNM4vjoVksyyBraTXSZE1
2026-08-07 06:12:03 -04:00
Joseph Doherty 7fc77f2790 feat(secrets): Grpc hub server — SecretsHubService + fail-closed bearer interceptor + DI
The server half of the central-hosted pull-only secrets hub. SecretsHubService
adapts the node's own ISecretStore onto secrets_hub.v1 using exactly two read
members (GetManifestAsync, GetAsync) — the proto exposes no write RPC, so a
follower has no path to put a row into central whatever its credentials allow.

GetSecrets omits a name the hub does not hold and skips one that will not parse
as a SecretName; neither is an error, because both describe a follower acting on
a manifest that was true a moment ago. Failing the call would stall the sweep and
let one bad name deny the follower every other row in the batch. Names are
re-validated server-side (the wire type is a plain string) and de-duplicated on
the normalized value.

DomainToWire mirrors the Akka replicator's SecretRowDto discipline exactly:
round-trip "O" timestamps so a DateTimeOffset's offset and ticks survive the
last-writer-wins equality compare, the content type as its enum MEMBER NAME so an
unknown value is rejected rather than decoded to the zero member, and nullable
fields left absent rather than written as "" so null and empty stay distinct.

SecretsHubAuthInterceptor fails closed: with no token configured every call is
denied and one warning names the exact configuration key — never a token value,
presented or configured, at any level. Denial is uniform (one Status, one detail,
no "why") so a caller gets no oracle. Comparison is constant-time over UTF-8
bytes. All four handler shapes are gated, not just unary, so a future RPC cannot
arrive unauthenticated by omission; more than one Authorization header is refused
as a smuggling shape. The type keeps EXACTLY ONE public constructor — Grpc's
activator silently stops invoking an interceptor it cannot construct
unambiguously, and the failure mode is an auth gate that quietly does not run — so
a test pins the count.

Tests: 50, red-first (compile-red, then a fail-closed mutation verified 5 of them
bite). Full Secrets suite 328 passed / 0 failed / 15 live-gated skips, 0 new
warnings.

Claude-Session: https://claude.ai/code/session_014WNM4vjoVksyyBraTXSZE1
2026-08-07 06:09:23 -04:00
Joseph Doherty cc95590196 test(secrets): scaffold empty Grpc replicator test project (Tasks 2/3 drop files in without shared-file edits)
Claude-Session: https://claude.ai/code/session_014WNM4vjoVksyyBraTXSZE1
2026-08-07 05:57:42 -04:00