Component-AuditLog.md has always required "we over-redact, never under-redact,
on configuration faults", but the body / SQL-parameter redactors violated it.
AuditRegexCache rejects a pattern that is malformed OR whose compile exceeds a
100 ms budget, caching the rejection for the process lifetime.
ScadaBridgeAuditRedactor then simply dropped the rejected pattern from its
redactor set and emitted the payload anyway — publishing precisely the values
the operator configured it to suppress, onto a row that looks entirely normal
downstream. Recovery required a process restart and the only signal was one
Warning line. The SQL path was worse: TryGetSqlParamRedactor returned a bare
false for both "no redactor configured for this connection" and "the configured
one will not compile", and CLAUDE.md records SQL parameter capture as on by
default.
Two changes:
1. Fail closed. A pattern that is CONFIGURED but unavailable now over-redacts
the whole payload and increments AuditRedactionFailure, reusing the existing
safety-net path. "Not configured at all" stays permissive — conflating those
two states is the actual defect, so both are pinned by tests.
2. Precompile off the hot path. The audit-log roadmap specifies patterns are
"precompiled at startup; rejected if compile takes >100ms"; the implementation
had drifted to compiling lazily on first event, which put a wall-clock budget
on a hot path under production load. RegexOptions.Compiled emits IL during
construction, so a busy node could blow the budget on a perfectly valid
pattern. Warm-up now runs at construction and on every options reload. The
residual window between a reload and its warm-up is safe because that path
now fails closed.
Warm-up deliberately does not fail the boot — an unusable pattern degrades the
node to over-redaction (safe, loud) rather than refusing to start. Reading
CurrentValue happens inside the warm-up try so an options provider that throws
still surfaces via Apply's over-redact path, not the constructor
(OuterCatch_OptionsThrows_NeverLeaks_AllSensitiveFieldsOverRedacted).
Also de-flakes GrpcCentralTransportTests.DeadlineExceeded_IsNotRetriedOnThePeer,
which is how this was found. It black-holed node A behind a 300 ms deadline, but
on a saturated machine the call could fail to even START — a genuinely-unsent
failure that IsConnectFailure correctly fails over on, so node B's ack arrived
instead of the expected Status.Failure. The test read as a flake while actually
reporting that its own premise had not held. Split in two: the hard rule now
injects an explicit DeadlineExceeded via a trailers-only response (deterministic,
load-independent), and a new BlackHoledNode_DoesNotHang covers the
deadline-is-actually-applied half with both nodes black-holed so no ack can
arrive down any path.
Verified: both fixes were confirmed to fail before they pass — reverting the
fail-closed guard fails exactly the 5 fail-closed tests while the 4 controls
still pass, and adding DeadlineExceeded to IsConnectFailure fails the rewritten
transport test. AuditLog 367/367, Host.Tests GrpcCentralTransport 8/8, solution
build clean. The previously-intermittent
Filter_PicksUp_NewBodyRedactor_OnConfigReload is green in a full sweep for the
first time.
Not addressed here, and noted on #35: the 100 ms wall-clock budget remains a
weak proxy for catastrophic backtracking (RegexOptions.Compiled defers JIT to
first match, so construction time measures the wrong thing), and a rejection is
still cached permanently. Both are now safe rather than dangerous, so they are
hardening rather than a leak.
Introduce ICentralTransport as the site->central choke point inside
SiteCommunicationActor. The seven site->central sends (notification submit/
status, audit + cached-telemetry ingest, reconcile, health, heartbeat) now
delegate to an injected transport instead of owning ClusterClient.Send inline.
- AkkaCentralTransport: verbatim extraction of today's ClusterClient.Send path,
including the exact sender-forwarding that routes central's reply straight
back to the waiting Ask. Default when no transport is injected -> behaviour
unchanged, existing SiteCommunicationActorTests pass as-is.
- GrpcCentralTransport + CentralChannelProvider: dial CentralControlService with
sticky failover + background failback (1s-doubling-cap-60s), PSK +
x-scadabridge-site via ControlPlaneCredentials, per-call deadlines mirroring
today's Ask timeouts. Cross-node retry ONLY on provably-unsent
connect failures; never on DeadlineExceeded. Heartbeat stays fire-and-forget.
- StaticSitePskProvider: site's single own-key provider (fail-closed).
- CommunicationOptions: CentralTransport flag (default Akka) + CentralGrpcEndpoints
(validator: required when transport=Grpc). Host selects the impl; the
ClusterClient is created only on the Akka path.
Tests: actor-with-fake-transport (7 delegations + fault routing + heartbeat
no-fault), AkkaCentralTransport sender-forwarding, GrpcCentralTransport over
in-process TestServer (failover flip, sticky, failback, PSK+header, deadline,
no-retry-on-deadline), validator. Communication.Tests 371 green, Host.Tests 391
green; the three above-seam suites pass unmodified.
Joseph Doherty