Commit Graph

3 Commits

Author SHA1 Message Date
Joseph Doherty 9fb52153fd fix(test): order three more unsynchronized assertions behind the observables they follow
Deferred flake-pattern sweep of tests/ for the class fixed in c4caebe9 and
cfa6acbf — a bounded wait on observable A followed by a bare assert on an
observable B that the product only reaches strictly after A. Three clear
instances, each reproduced deterministically by delaying only the later step
and each re-verified green with that same delay still injected.

AlarmOnTriggerRuns_ShedAtTheSameCap_WithAnAlarmScopedSiteEvent gated on the
rate-limited shed site event and then asserted the shed COUNT bare.
AlarmActor.ShedAlarmRun increments the counter and only then emits the event,
and the event fires on the first shed only — so the gate observed Flap(4)'s
shed and ordered nothing with respect to Flap(5)'s, which is a separate
mailbox message with no observable of its own (an alarm on-trigger run has no
Ask caller to reply to, unlike ScriptActor.ShedRun, whose sibling test is
correctly ordered by its ScriptCallResult and is left alone). Deferring
Flap(5) by 2 s failed it with "Expected: 2 / Actual: 1". The count is now
ACCUMULATED across polls rather than re-read, because
SiteHealthCollector.CollectReport DRAINS the interval counters — a poll loop
that simply re-read it would consume the first shed and never reach 2.

EndToEnd_GrpcStubError_RowStays_Pending_NextTick_Succeeds gated on the
central row arriving and then asserted bare that the site SQLite row had left
Pending. SiteAuditTelemetryActor pushes via IngestAuditEventsAsync (which is
what writes the central row) and calls MarkForwardedAsync only after parsing
the ack. Delaying just that post-push step failed it with
"Assert.DoesNotContain() Failure: Filter matched in collection".

PreSnapshotBuffer_IsCapped_DropsOldest_AndCountsTheDrops gated on
"Count >= cap" and then asserted "Count == cap + 1" bare — a gate strictly
weaker than the assertion it guards, so it ordered nothing with respect to
the last event of a FlushBuffer loop that delivers one at a time. Parking
that loop after its 19,999th delivery failed it with
"Expected: 20001 / Actual: 20000".

Also hardens GrpcCentralTransportTests.WaitUntil, which returned silently on
timeout; today's single caller re-asserts immediately, so this only sharpens
the message rather than fixing a live flake.

Cleared with evidence, not guessed: SiteAlarmLiveCacheService's LingerStop
removes the site entry inside one lock, so IsLive and GetCurrentAlarms flip
atomically; and SiteReconciliationActor walks response.Gap with a sequential
foreach in which the asserted "Gone" log precedes the awaited "Good" row, the
inverse of this class.

Test-only; every ordering named above is correct as written.
2026-08-15 03:37:35 -04:00
Joseph Doherty 006202f3c7 fix(audit): fail closed when a configured redactor is unavailable (#35)
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.
2026-08-12 03:04:50 -04:00
Joseph Doherty 33b15f10a4 feat(grpc): site-side ICentralTransport seam + gRPC transport (T1A.3)
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.
2026-07-22 19:29:14 -04:00