Task 4 of the LocalDb Phase 1 adoption plan.
OperationTrackingStore took IOptions<OperationTrackingOptions> and opened its own
SqliteConnection from ConnectionString. It now takes ILocalDb and gets every
connection - writer and the two ad-hoc reader paths - from CreateConnection().
This is not cosmetic. OperationTracking is a RegisterReplicated table, so its
capture triggers call zb_hlc_next(). That UDF is registered per connection by
ILocalDb and by nothing else, so a raw SqliteConnection would fail closed on
every write. Connections from CreateConnection() also arrive already open -
calling Open()/OpenAsync() on one throws - hence the removed OpenAsync calls on
the reader paths.
OperationTrackingOptions.ConnectionString is now vestigial for this store; the
database location is LocalDb:Path. The options class stays (retention settings)
and the config key stays bound for the site config DB.
InitializeSchema is kept but is now always a no-op in the host: onReady runs
while ILocalDb is being constructed, strictly before this constructor can
receive it. It remains so a directly-constructed store (tests, tooling) is
self-sufficient.
Tests: the store fixture moves off mode=memory&cache=shared onto a real ILocalDb
over a temp file. There is no in-memory mode - LocalDbOptions.Path is a
filesystem path - and testing through a raw in-memory connection would no longer
resemble the host. Verifier connections stay raw on purpose: this fixture never
registers the table, so no trigger fires and the UDF is never reached.
Three DI fixtures needed LocalDb:Path added, because resolving
IOperationTrackingStore now forces ILocalDb construction:
SiteCompositionRootTests, SiteAuditWiringTests, and the AuditLog
CombinedTelemetryHarness.
Verified: build 0 warnings; Host 294/294, SiteRuntime 530/530, AuditLog 354/354.
Claude-Session: https://claude.ai/code/session_01BL2Vu1ESDQ9SCN4gVKkdts
The NuGetAuditSuppress in Directory.Packages.props was masking a LIVE high-severity
vulnerability, not documenting an accepted one. Only the Host project resolved a
patched SQLitePCLRaw.lib.e_sqlite3 2.1.12 (transitively, via ZB.MOM.WW.Auth.ApiKeys).
Every other SQLite consumer - AuditLog, SiteRuntime, StoreAndForward, SiteEventLogging
and 11 test projects - still resolved the vulnerable 2.1.11.
The suppression's rationale was factually wrong: it claimed 'the only patched native
lib is the SQLitePCLRaw 3.x line'. 2.1.12 patches this advisory within the 2.1.x line,
so the feared risky force-override of the whole family to 3.x is unnecessary.
Fix: explicit PackageReference to the patched 2.1.12 in each SQLite-consuming project,
plus the central PackageVersion row. Suppression removed, so the advisory is audited
again rather than silenced.
Rejected alternative: CentralPackageTransitivePinningEnabled. It clears the advisory in
one line but makes every central version a ceiling for transitive resolution, demanding
bumps to Google.Protobuf, Grpc.Net.Client, Microsoft.Data.SqlClient and Newtonsoft.Json.
That is a gRPC/data-access change to a production SCADA platform and deserves its own
reviewed commit.
Verified: forced full-solution restore reports no NU1903 (only the pre-existing,
unrelated AngleSharp NU1902 in CentralUI.Tests); all four previously-vulnerable src
projects now resolve 2.1.12; 55 projects build 0 warnings / 0 errors; 1108 tests pass
across the SQLite layer (AuditLog 354, ConfigurationDatabase 357, Security 181,
StoreAndForward 152, SiteEventLogging 64).
The host does not guarantee IHostedService.StopAsync is driven before the DI
container is disposed — WebApplicationFactory's teardown reaches Dispose first
— so cancelling the internal CTS from StopAsync threw ObjectDisposedException
and aborted the host's whole shutdown sequence. Four services shared the same
copy-pasted lifecycle and the same two races: StopAsync cancelling an already-
disposed CTS, and StartAsync reading _cts.Token lazily inside the Task.Run
lambda, which faults the loop task the host awaits when Dispose wins that race.
Each service now captures the token on the caller's thread, tolerates a
disposed CTS, and cancels-before-disposing so the loop is always signalled and
its pending Task.Delay sees a cancelled token rather than a dead source.
SiteAuditBacklogReporter also gains the outer OperationCanceledException guard
its sibling SiteAuditRetentionService already carried (arch-review 04 R2, R7),
without which a shutdown landing mid-probe threw TaskCanceledException out of
Host.StopAsync.
Surfaced while verifying the Gitea #15 test-harness fix: in Host.Tests the
aborted teardown skipped the fixture's env-var restore, contaminating every
later test in the run.
Refs: Gitea #15
Consolidates the reporter's poll cadence onto SqliteAuditWriterOptions instead of
the hard-coded 30 s constant, closing deferred-work register item #21 (the config-
shape cleanup its own XML doc flagged as a follow-up).
- SqliteAuditWriterOptions.BacklogPollIntervalSeconds (default 30).
- Reporter reads it via IOptions; precedence explicit-override (tests) > configured
> 30 s default; a non-positive value falls back to the default. Corrected the
stale "hard-code / tunable in a follow-up" class-doc.
- Cadence tests + register updated (row 21 -> Resolved).
Claude-Session: https://claude.ai/code/session_01MtdgwpEeCUn6cUA5f1LMPj
SiteEntry gains an additive FallbackGrpcEndpoint; SiteEnumerator now picks NodeA
else NodeB as primary (a NodeB-only site is no longer skipped) and carries a
distinct NodeB as the fallback. Both GrpcPull{AuditEvents,SiteCalls} clients dial
the primary first and, on a transport fault (Unavailable/DeadlineExceeded/
Cancelled/HttpRequestException/SocketException/OperationCanceled), fail over to
NodeB once before collapsing to empty — a mapping/unexpected fault does NOT fail
over. Keeps the reconciliation loss-recovery net available during a NodeA outage.
When a pulled AuditEvent fails to insert on every retry up to the permanent-
abandon threshold, the reconciliation actor now writes ONE synthetic
ReconciliationAbandoned row (new AuditKind) alongside the Critical log line —
fresh EventId, Status=Failed, channel preserved from the lost row, Extra carrying
the abandoned EventId + source site + final error — via the same
ScadaBridgeAuditEventFactory the ingest path uses. Best-effort in its own
try/catch so it never blocks the cursor twice. The permanent loss is now
queryable in the Audit Log, not only in a rotating log file.
Reconciliation cursor becomes composite (UpdatedAtUtc, TrackedOperationId);
IPullSiteCallsClient/GrpcPullSiteCallsClient forward the Task-15 after_id keyset
(additive param, null preserves the legacy inclusive >= contract). A burst
sharing one exact UpdatedAtUtc now drains via the id tiebreak instead of pinning
forever. A legacy site that ignores after_id is latched + published as
SiteCallReconciliationPinnedChanged on the EventStream (transition-only),
replacing the prior silent log line.
SiteAuditRetentionService (site-only IHostedService) ticks PurgeExpiredAsync on
InitialDelay(5m)/PurgeInterval(24h, clamped >=1m), cutoff = UtcNow - RetentionDays.
Registered site-only in AddAuditLogHealthMetricsBridge; options bound from
AuditLog:SiteRetention. Per-tick failures swallowed. Added a test-only
PurgeIntervalOverride (mirrors SiteCallAuditOptions) so the ms-cadence tick test
can bypass the 1-min production clamp. Doc updated. (PLAN-04 Task 3, S1/U2)
RetentionDays clamped [1,90]; PurgeInterval clamped to >= 1 min (Timer spin
footgun); 5-min InitialDelay so a daily-recycled node still purges. (PLAN-04 Task 2)
Retention purge deletes Forwarded/Reconciled rows older than the cutoff and
reclaims pages via incremental_vacuum; Pending rows are never purged on age
alone (would lose audit data that never reached central). Temp-table purge set
avoids the SQLite 999-param limit; runs under _writeLock. (PLAN-04 Task 1, S1/U2)
Fixes the 8 findings from the 2026-06-24 re-review (commit c42bb485), with a
regression test per Medium finding:
- DataConnectionLayer-029 (Med): HandleAlarmSubscribeCompleted now mirrors the
tag-path re-check — if a feed is already stored for the source, release the
redundant just-created subscription instead of overwriting + leaking the first
one (the double-subscribe window DCL-023 reopened). +regression test.
- InboundAPI-031 (Med): remove WaitForAttribute's local 5s grace backstop (tighter
than the CommunicationService Ask's timeout+IntegrationTimeout round-trip budget,
so a slow-but-valid timed-out 'false' got cancelled into a 500). Link only the
client-abort + explicit caller tokens; the lower layer owns the backstop. +test.
- SiteRuntime-032 (Med): derive the deployed count from an authoritative set of
deployed config names (HashSet) instead of a map-presence-gated int, so deleting
a DISABLED instance decrements correctly (SiteRuntime-029's gate leaked it).
+deploy->disable->delete regression test.
- StoreAndForward-028 (Med): reset _bufferedCount in StopAsync alongside the
register-guard so a same-instance Stop->Start re-seeds from a clean base (no ~2N
gauge double-count). +restart regression test.
- AuditLog-017 (Low): test the OnIngestAsync scope-resolution guard (actor survives,
replies empty, counts the failure) — no longer unpinned.
- CentralUI-037 / ScriptAnalysis-009 / SiteRuntime-033 (Low): doc-comment + spec
fixes (Database-throws in the inbound sandbox; baseReferences param wording;
native-alarm cap return-to-normal + per-condition NativeAlarmDropped eviction).
Targeted suites green: SiteRuntime 5, StoreAndForward 6, InboundAPI 31,
DataConnectionLayer 10, AuditLog 5, ScriptAnalysis 40, CentralUI ScriptAnalysis 52.
Remediation from the full per-module code review at 4307c381 (findings recorded
separately in code-reviews/).
Highs fixed:
- DeploymentManager-025/SiteRuntime-031: stop broadcasting notification lists + SMTP
configs (incl. credentials) to sites; site purges already-persisted rows on apply
(enforces the central-only delivery design; clears plaintext SMTP creds at rest).
- DataConnectionLayer-023: guard the native-alarm subscribe path against the
mid-flight-unsubscribe adapter-feed leak (mirrors the DCL-021 tag-path fix).
- SiteEventLogging-024: normalize From/To query bounds to UTC (the -016 fix the
audit trail claimed but never committed).
- KpiHistory-001: add an in-flight guard to the recorder sample tick.
- ScriptAnalysis-001: harden the trust analyzer's TPA-absent fallback (resolve
forbidden anchors in the minimal reference set; warn on degraded mode) — anchors
added to validation references only, never the compile gate.
(InboundAPI-026 left to the feat/ipsen-movein effort per owner decision.)
Medium/Low: DM-026 deterministic deploy-status tiebreaker; SR-027/028/029/030
native-alarm leak/phantom-active/delete-during-redeploy fixes; AL-013/014/016;
TE-024 (folder-mutation audit rows now persisted)/025; SF-025 gauge-provider
clear-on-stop; ESG-025/026; SEC-023/024/025; SCA-007/008/009; plus doc/test
accuracy COM-023/024, HOST-025/026, HM-024/025, NS-027/028.
Full-solution build 0 warnings; ~3560 tests across 18 touched suites green.
1. Request headers in Extra JSON (AuditWriteMiddleware): adds a `requestHeaders`
object to the existing Extra JSON alongside remoteIp/userAgent; headers whose
names appear in AuditLogOptions.HeaderRedactList (Authorization, X-Api-Key,
Cookie, Set-Cookie by default) are replaced with "<redacted>" using
OrdinalIgnoreCase matching — same policy as ScadaBridgeAuditRedactor.
2. AuditInboundCeilingHits counter: new IAuditInboundCeilingHitsCounter interface
+ NoOpAuditInboundCeilingHitsCounter default; AuditCentralHealthSnapshot
implements the interface (Interlocked field, thread-safe) and exposes
AuditInboundCeilingHits on IAuditCentralHealthSnapshot; AddAuditLog registers
the NoOp default, AddAuditLogCentralMaintenance forwards to the snapshot;
AuditWriteMiddleware accepts the counter as an optional ctor arg and increments
it once per request where either the request or response body hit the cap.
3. Per-method SkipBodyCapture opt-out: adds SkipBodyCapture bool to
PerTargetRedactionOverride; AuditWriteMiddleware consults the per-target
override map at the start of InvokeAsync (before EnableBuffering) and, when
set, skips body read + capture entirely — the audit row still emits with
headers/metadata but null RequestSummary/ResponseSummary; truncation flags
are also cleared so the ceiling-hits counter is not bumped for opted-out methods.
Site Call Audit (#22): build the documented periodic reconciliation PULL
self-heal path for the eventually-consistent central SiteCalls mirror, as a
dedicated PullSiteCalls gRPC RPC kept separate from the audit pull. This is the
pull PLUMBING only; the central reconciliation tick is a separate follow-up.
- IOperationTrackingStore.ReadChangedSinceAsync(sinceUtc, batchSize): inclusive
UpdatedAtUtc cursor, oldest-first, batch-capped; SQLite impl projects tracking
rows onto SiteCallOperational (Kind->Channel, TargetSummary->Target, SourceSite
left empty - the store has no site-id column).
- sitestream.proto: rpc PullSiteCalls + PullSiteCallsRequest/Response, mirroring
PullAuditEvents; regenerated checked-in SiteStreamGrpc/*.cs.
- SiteCallDtoMapper.ToDto(SiteCallOperational): inverse of FromDto for the handler.
- SiteStreamGrpcServer.PullSiteCalls handler + SetOperationTrackingStore seam;
Host wires the seam alongside SetSiteAuditQueue (site roles only).
- Central IPullSiteCallsClient + GrpcPullSiteCallsClient (home: AuditLog/Central to
reuse ISiteEnumerator; SiteCallAudit does not reference AuditLog). Re-stamps
SourceSite from the dialed siteId; no-throw on tolerable transport faults;
SpecifyKind (not ToUniversalTime) cursor handling. Central-only DI registration.
Tests: ReadChangedSinceAsync (4), PullSiteCalls handler (6), GrpcPullSiteCallsClient
(8). Full solution build 0 warnings/0 errors (TreatWarningsAsErrors).
Perf re-baseline (HotPathLatencyTests): empirical p95 on Apple M-series Release
build: 4KB DetailsJson slow path ≈14 µs, small-DetailsJson no-redactors ≈2 µs,
true no-op fast path ≈0 µs. Thresholds updated: 200 µs / 30 µs / 5 µs (≈15×
headroom for contested CI runners). Old thresholds (50 µs / 10 µs) were set for
the pre-C3 typed-field path; canonical JSON parse+rewrite is empirically faster.
Adds a third test (Filter_Apply_NoDetailsJson_FastPath) that asserts same-instance
return on the DetailsJson-null + within-cap fast path. Env-var overrides retained.
CollapseAuditLogToCanonicalMigrationTests (new): three MSSQL-gated [SkippableFact]
tests verifying Action/Category/Outcome projection, NULL Actor, DetailsJson codec
round-trip, and all six persisted computed columns (Kind/Status/SourceSiteId/
ExecutionId/ParentExecutionId) for ApiOutbound, InboundAuthFailure, and Failed-
status rows.
AddAuditLogTableMigrationTests: rename CreatesFiveNamedIndexes →
CreatesNineNamedIndexes; expand coverage from 5 original indexes to all 9 named
non-clustered indexes present after CollapseAuditLogToCanonical (adds
IX_AuditLog_Execution, IX_AuditLog_ParentExecution, IX_AuditLog_Node_Occurred,
UX_AuditLog_EventId).
Dead-cref cleanup: zero references to the deleted IAuditPayloadFilter /
DefaultAuditPayloadFilter / SafeDefaultAuditPayloadFilter types remain in any
.cs file (source or test). 26 occurrences across 13 files replaced with correct
references to IAuditRedactor / ScadaBridgeAuditRedactor / SafeDefaultAuditRedactor
or reworded as plain prose.
Residual sweep: no unused transitional code found beyond the acknowledged
"C3 transitional shim" comments on IngestedAtUtc stamping (active code, not dead).
I1 (security): OverRedact() in ScadaBridgeAuditRedactor now suppresses ErrorDetail,
ErrorMessage, and Extra (in addition to RequestSummary/ResponseSummary) to the
over-redacted marker in BOTH code paths (Deserialize+with path and the fallback
new-AuditDetails path). SafeDefaultAuditRedactor catch block aligned to match.
M3 (test): OuterCatch_OptionsThrows_NeverLeaks_AllSensitiveFieldsOverRedacted forces
the outer try/catch → OverRedact path via a ThrowingMonitor that throws from
CurrentValue (the first statement in the try block). Asserts (a) Apply does not
throw, and (b) all five sensitive free-text fields are suppressed to the
over-redacted marker with PayloadTruncated=true.
M1 (consistency): SafeDefaultAuditRedactor now uses AuditRedactionPrimitives
constants (RedactedMarker for line-format header values, OverRedactedEventMarker
for the catch block), eliminating the divergent [REDACTED]/[redacted by ...]
strings. AuditRedactionPrimitives gains OverRedactedEventMarker = RedactorErrorMarker.
SafeDefaultAuditRedactorTests updated from [REDACTED] → <redacted>.
M2 (comment): Added one-line note in TruncateField explaining why the char-count
(result.Length != value.Length) truncation check is sufficient given TruncateUtf8
only ever shortens.
Joseph Doherty