docs(code-reviews): regenerate index after batch 3 High fixes; fix regen-readme.py to parse the Won't Fix status

code-reviews/InboundAPI/findings.md

@@ -8,7 +8,7 @@
 | Last reviewed | 2026-05-16 |
 | Reviewer | claude-agent |
 | Commit reviewed | `9c60592` |
-| Open findings | 13 |
+| Open findings | 10 |
 
 ## Summary
 
@@ -53,7 +53,7 @@ are High severity and should be addressed before production use.
 |--|--|
 | Severity | High |
 | Category | Concurrency & thread safety |
-| Status | Open |
+| Status | Resolved |
 | Location | `src/ScadaLink.InboundAPI/InboundScriptExecutor.cs:17`, `:32`, `:40`, `:89`, `:123-128` |
 
 **Description**
@@ -76,7 +76,12 @@ compile at most once.
 
 **Resolution**
 
-_Unresolved._
+Resolved 2026-05-16 (commit `<pending>`): replaced the plain `Dictionary` handler
+cache with a `ConcurrentDictionary`; `RemoveHandler` now uses `TryRemove`; the
+lazy-compile path in `ExecuteAsync` compiles outside the cache and inserts atomically
+via `GetOrAdd` so concurrent first-callers share one handler. Regression tests
+`ConcurrentLazyCompile_SameMethod_DoesNotCorruptCache` and
+`ConcurrentRegisterAndExecute_DoesNotThrow` added.
 
 ### InboundAPI-002 — Lazy compilation is a check-then-act race with no atomicity
 
@@ -114,7 +119,7 @@ _Unresolved._
 |--|--|
 | Severity | High |
 | Category | Security |
-| Status | Open |
+| Status | Resolved |
 | Location | `src/ScadaLink.ConfigurationDatabase/Repositories/InboundApiRepository.cs:22-23`, consumed by `src/ScadaLink.InboundAPI/ApiKeyValidator.cs:33` |
 
 **Description**
@@ -138,7 +143,17 @@ match-position timing.
 
 **Resolution**
 
-_Unresolved._
+Resolved 2026-05-16 (commit `<pending>`): `ApiKeyValidator` no longer calls the
+secret-equality lookup `GetApiKeyByValueAsync` (the SQL `WHERE KeyValue = @secret`
+timing oracle). It now fetches all keys via `GetAllApiKeysAsync` and matches the
+secret in-process with `CryptographicOperations.FixedTimeEquals` over the UTF-8 bytes,
+scanning every candidate so neither match position nor secret length is observable.
+Regression tests `ValidateAsync_DoesNotUseSecretEqualityLookup`,
+`ValidateAsync_WrongKey_ConstantTimePath_Returns401`, and
+`ValidateAsync_KeyOfDifferentLength_Returns401` added. Note: the timing-oracle method
+`GetApiKeyByValueAsync` remains on `IInboundApiRepository` (it is outside this module);
+removing it from the repository is left as separate follow-up since the validator no
+longer depends on it.
 
 ### InboundAPI-004 — Client disconnect is misreported as a script timeout
 
@@ -178,7 +193,7 @@ _Unresolved._
 |--|--|
 | Severity | High |
 | Category | Security |
-| Status | Open |
+| Status | Resolved |
 | Location | `src/ScadaLink.InboundAPI/InboundScriptExecutor.cs:56-93` |
 
 **Description**
@@ -205,7 +220,16 @@ forbidden-API checker so the trust model is enforced consistently. Reject the me
 
 **Resolution**
 
-_Unresolved._
+Resolved 2026-05-16 (commit `<pending>`): added `ForbiddenApiChecker`, a Roslyn
+`CSharpSyntaxWalker` that statically rejects scripts referencing forbidden namespaces
+(`System.IO`, `System.Diagnostics`, `System.Threading` except `Tasks`,
+`System.Reflection`, `System.Net`, `System.Runtime.InteropServices`, `Microsoft.Win32`)
+whether reached via a `using` directive or a fully-qualified name. `CompileAndRegister`
+now runs the check before Roslyn compilation and refuses to register (and logs) a
+violating method; `ExecuteAsync`'s lazy-compile path is gated by the same check.
+Regression tests `CompileAndRegister_ForbiddenApi_RejectsScript` (theory),
+`ExecuteAsync_ForbiddenApiScript_DoesNotRunAndReturnsFailure`, and
+`CompileAndRegister_PermittedScript_StillRegisters` added.
 
 ### InboundAPI-006 — No request body size limit on the inbound endpoint
 

code-reviews/ManagementService/findings.md

@@ -8,7 +8,7 @@
 | Last reviewed | 2026-05-16 |
 | Reviewer | claude-agent |
 | Commit reviewed | `9c60592` |
-| Open findings | 13 |
+| Open findings | 10 |
 
 ## Summary
 
@@ -51,7 +51,7 @@ authorization bypass with no workaround.
 |--|--|
 | Severity | High |
 | Category | Security |
-| Status | Open |
+| Status | Resolved |
 | Location | `src/ScadaLink.ManagementService/ManagementActor.cs:1465`, `:1481`, `:1493`, `:641`, `:649` |
 
 **Description**
@@ -76,7 +76,16 @@ is already loaded — call `EnforceSiteScope(user, instance.SiteId)` (which requ
 
 **Resolution**
 
-_Unresolved._
+Resolved 2026-05-16 (commit `<pending>`). Threaded `AuthenticatedUser` into
+`HandleQueryEventLogs`, `HandleQueryParkedMessages`, `HandleRetryParkedMessage`,
+`HandleDiscardParkedMessage`, and `HandleDebugSnapshot`; added an
+`EnforceSiteScopeForIdentifier` helper that resolves the site by identifier and applies
+`EnforceSiteScope`. `HandleDebugSnapshot` enforces against the already-loaded instance's
+`SiteId`. Regression tests: `QueryEventLogs_OutOfScopeForSiteScopedUser_ReturnsUnauthorized`,
+`QueryParkedMessages_OutOfScopeForSiteScopedUser_ReturnsUnauthorized`,
+`RetryParkedMessage_OutOfScopeForSiteScopedUser_ReturnsUnauthorized`,
+`DiscardParkedMessage_OutOfScopeForSiteScopedUser_ReturnsUnauthorized`,
+`DebugSnapshot_OutOfScopeForSiteScopedUser_ReturnsUnauthorized`.
 
 ### ManagementService-002 — Single-entity query handlers leak data across site scope
 
@@ -84,7 +93,7 @@ _Unresolved._
 |--|--|
 | Severity | High |
 | Category | Security |
-| Status | Open |
+| Status | Resolved |
 | Location | `src/ScadaLink.ManagementService/ManagementActor.cs:510`, `:673`, `:733`, `:774`, `:631`, `:624` |
 
 **Description**
@@ -106,7 +115,16 @@ the resolved site ID in `HandleGetSite`, `HandleListAreas`, and `HandleGetDataCo
 
 **Resolution**
 
-_Unresolved._
+Resolved 2026-05-16 (commit `<pending>`). `HandleGetInstance`, `HandleGetSite`,
+`HandleGetDataConnection` now take `AuthenticatedUser` and call `EnforceSiteScope` against
+the resolved entity's site ID (instance `SiteId`, site `Id`, data-connection `SiteId`);
+`HandleListAreas` enforces against the requested `SiteId` before querying. Regression tests:
+`GetInstance_OutOfScopeForSiteScopedUser_ReturnsUnauthorized`,
+`GetInstance_InScopeForSiteScopedUser_ReturnsSuccess`,
+`GetSite_OutOfScopeForSiteScopedUser_ReturnsUnauthorized`,
+`GetSite_OutOfScopeForAdminUser_ReturnsSuccess`,
+`ListAreas_OutOfScopeForSiteScopedUser_ReturnsUnauthorized`,
+`GetDataConnection_OutOfScopeForSiteScopedUser_ReturnsUnauthorized`.
 
 ### ManagementService-003 — DebugStreamHub.SubscribeInstance performs no per-instance authorization
 
@@ -114,7 +132,7 @@ _Unresolved._
 |--|--|
 | Severity | High |
 | Category | Security |
-| Status | Open |
+| Status | Resolved |
 | Location | `src/ScadaLink.ManagementService/DebugStreamHub.cs:104` |
 
 **Description**
@@ -135,7 +153,17 @@ established in `OnConnectedAsync` must be persisted on the connection (e.g. in
 
 **Resolution**
 
-_Unresolved._
+Resolved 2026-05-16 (commit `<pending>`). `OnConnectedAsync` now persists the resolved
+roles and `PermittedSiteIds` in `Context.Items`. `SubscribeInstance` resolves the
+instance's site via `ITemplateEngineRepository` and rejects the subscription (sending
+`OnStreamTerminated`) when the new pure `DebugStreamHub.IsInstanceAccessAllowed` check
+fails. The check grants access for the Admin role or system-wide Deployment (empty
+permitted set) and otherwise requires the instance's site in the permitted set. Regression
+tests: `IsInstanceAccessAllowed_SiteScopedUser_OutOfScopeInstance_Denied`,
+`IsInstanceAccessAllowed_SiteScopedUser_InScopeInstance_Allowed`,
+`IsInstanceAccessAllowed_SystemWideDeployment_AnySiteAllowed`,
+`IsInstanceAccessAllowed_AdminRole_BypassesSiteScope`,
+`IsInstanceAccessAllowed_AdminRoleCheck_IsCaseInsensitive`.
 
 ### ManagementService-004 — Actor offloads work to Task.Run instead of using PipeTo
 

code-reviews/NotificationService/findings.md

@@ -8,7 +8,7 @@
 | Last reviewed | 2026-05-16 |
 | Reviewer | claude-agent |
 | Commit reviewed | `9c60592` |
-| Open findings | 11 |
+| Open findings | 8 |
 
 ## Summary
 
@@ -82,7 +82,7 @@ path. Fixed by the commit whose message references `NotificationService-001`.
 |--|--|
 | Severity | High |
 | Category | Error handling & resilience |
-| Status | Open |
+| Status | Resolved |
 | Location | `src/ScadaLink.NotificationService/NotificationDeliveryService.cs:157-167` |
 
 **Description**
@@ -95,7 +95,14 @@ Re-throw `OperationCanceledException`/`TaskCanceledException` when `cancellation
 
 **Resolution**
 
-_Unresolved._
+Resolved 2026-05-16 (commit `<pending>`). Classification was rewritten around a typed
+`ClassifySmtpError` helper: a caller-requested cancellation (`OperationCanceledException`/
+`TaskCanceledException` while `cancellationToken.IsCancellationRequested`) now propagates
+out of both `SendAsync` and `DeliverAsync` via dedicated `catch` filters instead of being
+buffered. The broad `IOException` catch-all was dropped — only MailKit's typed exceptions
+plus `SocketException`/`TimeoutException` are treated as transient. Regression tests
+`Send_CancellationRequested_PropagatesAndDoesNotBuffer` and
+`Send_TaskCanceledException_WithCancellation_Propagates`.
 
 ### NotificationService-003 — Error classification by substring matching on exception messages is fragile
 
@@ -103,7 +110,7 @@ _Unresolved._
 |--|--|
 | Severity | High |
 | Category | Error handling & resilience |
-| Status | Open |
+| Status | Resolved |
 | Location | `src/ScadaLink.NotificationService/NotificationDeliveryService.cs:144-147`, `src/ScadaLink.NotificationService/NotificationDeliveryService.cs:163-166` |
 
 **Description**
@@ -116,7 +123,16 @@ Classify on MailKit's typed exceptions and `SmtpCommandException.StatusCode` (4x
 
 **Resolution**
 
-_Unresolved._
+Resolved 2026-05-16 (commit `<pending>`). All `ex.Message.Contains(...)` checks were
+removed. The new `ClassifySmtpError` helper inspects `SmtpCommandException.StatusCode`
+(numeric SMTP code: 4xx → transient, 5xx → permanent) and treats `SmtpProtocolException`,
+`ServiceNotConnectedException`, `SocketException` and `TimeoutException` as transient;
+anything else is `Unknown` and propagates unclassified rather than being guessed. The
+permanent-promotion `catch` block in `DeliverAsync` now keys off this classification.
+Regression tests `Send_Smtp5xxCommandException_ClassifiedPermanent`,
+`Send_Smtp4xxCommandException_ClassifiedTransientAndBuffered`,
+`Send_SmtpProtocolException_ClassifiedTransient`, and
+`Send_NonSmtpExceptionWith5xxLookalikeText_NotPromotedToPermanent`.
 
 ### NotificationService-004 — `DeliverAsync` constructs two SMTP clients and leaks the used one
 
@@ -124,7 +140,7 @@ _Unresolved._
 |--|--|
 | Severity | High |
 | Category | Performance & resource management |
-| Status | Open |
+| Status | Resolved |
 | Location | `src/ScadaLink.NotificationService/NotificationDeliveryService.cs:118-119` |
 
 **Description**
@@ -143,7 +159,12 @@ Create exactly one client and dispose the one that is actually used:
 
 **Resolution**
 
-_Unresolved._
+Resolved 2026-05-16 (commit `<pending>`). `DeliverAsync` now invokes `_smtpClientFactory()`
+exactly once and disposes the client actually used via `using var disposable = smtp as
+IDisposable;`. The previous code created two `MailKitSmtpClientWrapper` instances per send
+and disposed the unused one while leaking the connected one. Regression test
+`Send_CreatesExactlyOneSmtpClient_AndDisposesIt` verifies the factory is invoked once and
+the resulting client is disposed.
 
 ### NotificationService-005 — Non-TLS path uses `SecureSocketOptions.Auto`, contradicting the requested mode
 

code-reviews/README.md

@@ -40,10 +40,10 @@ module file and counted in **Total**.
 | Severity | Open findings |
 |----------|---------------|
 | Critical | 0 |
-| High | 28 |
+| High | 12 |
 | Medium | 100 |
 | Low | 89 |
-| **Total** | **217** |
+| **Total** | **201** |
 
 ## Module Status
 
@@ -60,11 +60,11 @@ module file and counted in **Total**.
 | [ExternalSystemGateway](ExternalSystemGateway/findings.md) | 2026-05-16 | `9c60592` | 0/0/7/4 | 11 | 14 |
 | [HealthMonitoring](HealthMonitoring/findings.md) | 2026-05-16 | `9c60592` | 0/0/5/5 | 10 | 12 |
 | [Host](Host/findings.md) | 2026-05-16 | `9c60592` | 0/0/3/7 | 10 | 11 |
-| [InboundAPI](InboundAPI/findings.md) | 2026-05-16 | `9c60592` | 0/3/5/5 | 13 | 13 |
+| [InboundAPI](InboundAPI/findings.md) | 2026-05-16 | `9c60592` | 0/0/5/5 | 10 | 13 |
-| [ManagementService](ManagementService/findings.md) | 2026-05-16 | `9c60592` | 0/3/5/5 | 13 | 13 |
+| [ManagementService](ManagementService/findings.md) | 2026-05-16 | `9c60592` | 0/0/5/5 | 10 | 13 |
-| [NotificationService](NotificationService/findings.md) | 2026-05-16 | `9c60592` | 0/3/5/3 | 11 | 12 |
+| [NotificationService](NotificationService/findings.md) | 2026-05-16 | `9c60592` | 0/0/5/3 | 8 | 12 |
-| [Security](Security/findings.md) | 2026-05-16 | `9c60592` | 0/3/4/4 | 11 | 11 |
+| [Security](Security/findings.md) | 2026-05-16 | `9c60592` | 0/0/4/4 | 8 | 11 |
-| [SiteEventLogging](SiteEventLogging/findings.md) | 2026-05-16 | `9c60592` | 0/4/4/3 | 11 | 11 |
+| [SiteEventLogging](SiteEventLogging/findings.md) | 2026-05-16 | `9c60592` | 0/0/4/3 | 7 | 11 |
 | [SiteRuntime](SiteRuntime/findings.md) | 2026-05-16 | `9c60592` | 0/3/8/5 | 16 | 16 |
 | [StoreAndForward](StoreAndForward/findings.md) | 2026-05-16 | `9c60592` | 0/2/4/6 | 12 | 14 |
 | [TemplateEngine](TemplateEngine/findings.md) | 2026-05-16 | `9c60592` | 0/5/5/4 | 14 | 14 |
@@ -80,28 +80,12 @@ description, location, recommendation — lives in the module's `findings.md`.
 
 _None open._
 
-### High (28)
+### High (12)
 
 | ID | Module | Title |
 |----|--------|-------|
 | ClusterInfrastructure-001 | [ClusterInfrastructure](ClusterInfrastructure/findings.md) | Module implements none of its documented responsibilities |
 | DeploymentManager-006 | [DeploymentManager](DeploymentManager/findings.md) | Query-the-site-before-redeploy idempotency requirement not implemented |
-| InboundAPI-001 | [InboundAPI](InboundAPI/findings.md) | Singleton script handler cache mutated without synchronization |
-| InboundAPI-003 | [InboundAPI](InboundAPI/findings.md) | API key compared with non-constant-time string equality |
-| InboundAPI-005 | [InboundAPI](InboundAPI/findings.md) | Compiled API scripts run with no script-trust-model enforcement |
-| ManagementService-001 | [ManagementService](ManagementService/findings.md) | Remote-query and debug-snapshot handlers bypass site-scope enforcement |
-| ManagementService-002 | [ManagementService](ManagementService/findings.md) | Single-entity query handlers leak data across site scope |
-| ManagementService-003 | [ManagementService](ManagementService/findings.md) | DebugStreamHub.SubscribeInstance performs no per-instance authorization |
-| NotificationService-002 | [NotificationService](NotificationService/findings.md) | `TimeoutException`/`OperationCanceledException` misclassified as transient |
-| NotificationService-003 | [NotificationService](NotificationService/findings.md) | Error classification by substring matching on exception messages is fragile |
-| NotificationService-004 | [NotificationService](NotificationService/findings.md) | `DeliverAsync` constructs two SMTP clients and leaks the used one |
-| Security-001 | [Security](Security/findings.md) | StartTLS upgrade path is unreachable dead code |
-| Security-002 | [Security](Security/findings.md) | Authentication cookie is not marked `Secure` |
-| Security-003 | [Security](Security/findings.md) | JWT signing key length is never validated |
-| SiteEventLogging-001 | [SiteEventLogging](SiteEventLogging/findings.md) | `PRAGMA incremental_vacuum` is a no-op; storage cap cannot reclaim space |
-| SiteEventLogging-002 | [SiteEventLogging](SiteEventLogging/findings.md) | Storage-cap purge deletes the entire table when space is not reclaimed |
-| SiteEventLogging-003 | [SiteEventLogging](SiteEventLogging/findings.md) | Shared `SqliteConnection` used by purge and query without the write lock |
-| SiteEventLogging-004 | [SiteEventLogging](SiteEventLogging/findings.md) | Event-log handler runs as a cluster singleton that can land on the standby node |
 | SiteRuntime-001 | [SiteRuntime](SiteRuntime/findings.md) | `Instance.SetAttribute` never writes to the Data Connection Layer |
 | SiteRuntime-002 | [SiteRuntime](SiteRuntime/findings.md) | `RouteInboundApiSetAttributes` always treats writes as static overrides |
 | SiteRuntime-003 | [SiteRuntime](SiteRuntime/findings.md) | Redeployment relies on a fixed 500 ms reschedule and can collide on the child actor name |

code-reviews/Security/findings.md

@@ -8,7 +8,7 @@
 | Last reviewed | 2026-05-16 |
 | Reviewer | claude-agent |
 | Commit reviewed | `9c60592` |
-| Open findings | 11 |
+| Open findings | 8 |
 
 ## Summary
 
@@ -51,7 +51,7 @@ defects that should be fixed before any production deployment.
 |--|--|
 | Severity | High |
 | Category | Security |
-| Status | Open |
+| Status | Resolved |
 | Location | `src/ScadaLink.Security/LdapAuthService.cs:37-47` |
 
 **Description**
@@ -74,7 +74,12 @@ session is encrypted before binding. Remove the unreachable conditional.
 
 **Resolution**
 
-_Unresolved._
+Resolved 2026-05-16 (commit `<pending>`). Added an explicit `LdapTransport` enum
+(`Ldaps`/`StartTls`/`None`); `SecureSocketLayer` is set only for LDAPS, and the
+StartTLS branch now connects in plaintext, calls `StartTls()`, and verifies
+`connection.Tls` before binding. `LdapUseTls` is retained as a compatibility shim
+mapping onto the enum. Regression tests `AuthenticateAsync_StartTlsTransport_AttemptsConnection`
+and `AuthenticateAsync_NoTlsTransport_RejectedWithoutAllowInsecure`.
 
 ### Security-002 — Authentication cookie is not marked `Secure`
 
@@ -82,7 +87,7 @@ _Unresolved._
 |--|--|
 | Severity | High |
 | Category | Security |
-| Status | Open |
+| Status | Resolved |
 | Location | `src/ScadaLink.Security/ServiceCollectionExtensions.cs:16-23` |
 
 **Description**
@@ -102,7 +107,12 @@ the documented 15-minute JWT / 30-minute idle policy.
 
 **Resolution**
 
-_Unresolved._
+Resolved 2026-05-16 (commit `<pending>`). Confirmed the cookie is configured in this
+module (`ServiceCollectionExtensions.AddSecurity`), not CentralUI — no misattribution.
+Added `options.Cookie.SecurePolicy = CookieSecurePolicy.Always` so the JWT-bearing
+cookie is never sent over plain HTTP. Regression test
+`AddSecurity_AuthCookie_IsMarkedSecureAlways`. (`ExpireTimeSpan`/`SlidingExpiration`
+tuning left as a separate, lower-priority improvement.)
 
 ### Security-003 — JWT signing key length is never validated
 
@@ -110,7 +120,7 @@ _Unresolved._
 |--|--|
 | Severity | High |
 | Category | Security |
-| Status | Open |
+| Status | Resolved |
 | Location | `src/ScadaLink.Security/JwtTokenService.cs:33`, `src/ScadaLink.Security/SecurityOptions.cs:42` |
 
 **Description**
@@ -131,7 +141,14 @@ constructor so a weak key is rejected before any token is issued.
 
 **Resolution**
 
-_Unresolved._
+Resolved 2026-05-16 (commit `<pending>`). The `JwtTokenService` constructor now
+fails fast with an `InvalidOperationException` when `JwtSigningKey` is empty or
+shorter than 32 bytes (`SecurityOptions.MinJwtSigningKeyBytes`), so a weak key is
+rejected before any token is issued. The misleading `SecurityOptions` XML doc was
+corrected to state the requirement. Regression tests
+`JwtTokenService_EmptySigningKey_ThrowsAtConstruction`,
+`JwtTokenService_ShortSigningKey_ThrowsAtConstruction`, and
+`JwtTokenService_AdequateSigningKey_ConstructsSuccessfully`.
 
 ### Security-004 — Search filter uses `uid=` while fallback DN construction uses `cn=`
 

code-reviews/SiteEventLogging/findings.md

@@ -8,7 +8,7 @@
 | Last reviewed | 2026-05-16 |
 | Reviewer | claude-agent |
 | Commit reviewed | `9c60592` |
-| Open findings | 11 |
+| Open findings | 7 |
 
 ## Summary
 
@@ -51,7 +51,7 @@ the requirement strictly, and several documentation/maintainability issues.
 |--|--|
 | Severity | High |
 | Category | Correctness & logic bugs |
-| Status | Open |
+| Status | Resolved |
 | Location | `src/ScadaLink.SiteEventLogging/EventLogPurgeService.cs:100-102`, `src/ScadaLink.SiteEventLogging/SiteEventLogger.cs:36-55` |
 
 **Description**
@@ -78,7 +78,12 @@ deletes, or measure logical data size (e.g. `page_count - freelist_count` times
 
 **Resolution**
 
-_Unresolved._
+Resolved 2026-05-16 (commit `<pending>`): `InitializeSchema` now sets
+`PRAGMA auto_vacuum = INCREMENTAL` before any table is created, and
+`GetDatabaseSizeBytes` measures logical size as `(page_count - freelist_count) *
+page_size` so reclaimed pages no longer mask the size drop. The cap-purge loop now
+reliably observes the database shrinking. Regression test
+`PurgeByStorageCap_StopsWhenUnderCap_DoesNotEmptyTable`.
 
 ### SiteEventLogging-002 — Storage-cap purge deletes the entire table when space is not reclaimed
 
@@ -86,7 +91,7 @@ _Unresolved._
 |--|--|
 | Severity | High |
 | Category | Design-document adherence |
-| Status | Open |
+| Status | Resolved |
 | Location | `src/ScadaLink.SiteEventLogging/EventLogPurgeService.cs:87-105` |
 
 **Description**
@@ -111,7 +116,14 @@ removed.
 
 **Resolution**
 
-_Unresolved._
+Resolved 2026-05-16 (commit `<pending>`): with the size measurement fixed
+(SiteEventLogging-001) the cap loop terminates when the file is genuinely under the
+cap. An additional guard stops the loop if the on-disk size fails to decrease across
+an iteration, so a cap that can never be met no longer empties the whole table.
+Regression tests `PurgeByStorageCap_StopsWhenUnderCap_DoesNotEmptyTable` (asserts the
+table is not emptied and the file ends under a realistic non-zero cap) and
+`PurgeByStorageCap_RemovesOldestEventsFirst` (asserts only the oldest events are
+removed).
 
 ### SiteEventLogging-003 — Shared `SqliteConnection` used by purge and query without the write lock
 
@@ -119,7 +131,7 @@ _Unresolved._
 |--|--|
 | Severity | High |
 | Category | Concurrency & thread safety |
-| Status | Open |
+| Status | Resolved |
 | Location | `src/ScadaLink.SiteEventLogging/EventLogPurgeService.cs:64,90,100,110,114`, `src/ScadaLink.SiteEventLogging/EventLogQueryService.cs:36`, `src/ScadaLink.SiteEventLogging/SiteEventLogger.cs:34,72` |
 
 **Description**
@@ -145,15 +157,24 @@ makes this safer). Do not share one `SqliteConnection` across threads.
 
 **Resolution**
 
-_Unresolved._
+Resolved 2026-05-16 (commit `<pending>`): the raw `internal Connection` property was
+removed from `SiteEventLogger` and replaced with lock-guarded `WithConnection(...)`
+overloads that hold the existing `_writeLock` for the duration of the caller's
+delegate. `EventLogPurgeService`, `EventLogQueryService`, and `LogEventAsync` all now
+access the connection exclusively through `WithConnection`, so the purge thread,
+query thread, and recording threads are serialised on a single lock. `Dispose` was
+also brought under the lock to avoid a dispose/use race. Regression test
+`PurgeByStorageCap_ConcurrentWritesDoNotCorruptConnection` exercises purge running
+concurrently with multiple writer threads.
 
 ### SiteEventLogging-004 — Event-log handler runs as a cluster singleton that can land on the standby node
 
 | | |
 |--|--|
-| Severity | High |
+| Severity | Low |
+| Original severity | High (re-triaged down to Low on 2026-05-16 — see Re-triage note) |
 | Category | Design-document adherence |
-| Status | Open |
+| Status | Won't Fix |
 | Location | `src/ScadaLink.Host/Actors/AkkaHostedService.cs:313-336`, `src/ScadaLink.SiteEventLogging/EventLogHandlerActor.cs:21-25` |
 
 **Description**
@@ -179,9 +200,40 @@ the active node explicitly (the node owning the Deployment Manager singleton), o
 handler are guaranteed co-located. Reconcile the design doc and the inline comment
 with whichever model is chosen.
 
+**Re-triage note (2026-05-16)**
+
+The finding's central claim — that a remote query "can be served by the standby
+node and read that node's near-empty database" — is incorrect for the query path.
+In `AkkaHostedService.cs` the `event-log-handler` `ClusterSingletonManager` and the
+`deployment-manager` `ClusterSingletonManager` are created with the **same role**
+(`siteRole`) in the **same cluster**. Akka.NET pins every cluster singleton of a
+given role to the *oldest member of that role* — so all same-role singletons in a
+cluster co-locate on one node. The "active node" in this codebase is, by definition,
+the node hosting the `deployment-manager` singleton; the event-log query singleton
+is therefore *guaranteed* to run on that same node. A `ClusterClient` query cannot
+land on the standby. The inline comment in `AkkaHostedService.cs` is accurate, not
+"the opposite of what happens".
+
+A real but distinct concern exists: the *writer* (`SiteEventLogger`) is registered
+as a plain per-node DI singleton (`AddSiteEventLogging`), so it records to a local
+SQLite file on **every** node, including the standby. That wastes storage on the
+standby but does **not** cause the query-returns-nothing symptom the finding
+describes, because the query singleton always reads the *active* node's (populated)
+database. Gating the writer to the active node would be a `ScadaLink.Host` wiring
+change, outside this module's scope, and is a minor optimisation rather than a
+correctness defect.
+
+Re-triaged from High to Low and closed as **Won't Fix**: the High-severity
+correctness claim does not hold. Any residual cleanup (gate the standby-node writer;
+the comment needs no change) can be raised as a fresh Low finding against
+`ScadaLink.Host` if desired.
+
 **Resolution**
 
-_Unresolved._
+Won't Fix — 2026-05-16 (commit `<pending>`). Re-triaged: the asserted defect (query
+served by standby returning an empty log) cannot occur because the event-log query
+singleton and the deployment-manager singleton share a role and so always co-locate
+on the active node. No code change made; see the re-triage note above.
 
 ### SiteEventLogging-005 — `LogEventAsync` performs synchronous disk I/O on the caller's thread
 

code-reviews/regen-readme.py

@@ -45,7 +45,7 @@ def parse_findings(module):
         fid = m.group(1).strip()
         title = m.group(2).strip().lstrip("—–-").strip().replace("|", "\\|")
         sev = re.search(r"\|\s*Severity\s*\|\s*([A-Za-z]+)", block)
-        status = re.search(r"\|\s*Status\s*\|\s*([A-Za-z ]+?)\s*\|", block)
+        status = re.search(r"\|\s*Status\s*\|\s*([A-Za-z' ]+?)\s*\|", block)
         if not sev or not status:
             raise SystemExit(f"{module}/findings.md: {fid} is missing a Severity or Status field")
         findings.append((module, fid, sev.group(1), title, status.group(1).strip()))

src/ScadaLink.InboundAPI/ApiKeyValidator.cs

@@ -1,3 +1,5 @@
+using System.Security.Cryptography;
+using System.Text;
 using ScadaLink.Commons.Entities.InboundApi;
 using ScadaLink.Commons.Interfaces.Repositories;
 
@@ -30,7 +32,14 @@ public class ApiKeyValidator
             return ApiKeyValidationResult.Unauthorized("Missing X-API-Key header");
         }
 
-        var apiKey = await _repository.GetApiKeyByValueAsync(apiKeyValue, cancellationToken);
+        // InboundAPI-003: do NOT resolve the key with a secret-equality lookup
+        // (GetApiKeyByValueAsync translates to a SQL "WHERE KeyValue = @secret" early-exit
+        // comparison — a timing side-channel). Fetch all keys and match the secret
+        // in-process with a constant-time comparison so neither match position nor
+        // secret length is observable to a network attacker.
+        var apiKey = FindKeyConstantTime(
+            await _repository.GetAllApiKeysAsync(cancellationToken),
+            apiKeyValue);
         if (apiKey == null || !apiKey.IsEnabled)
         {
             return ApiKeyValidationResult.Unauthorized("Invalid or disabled API key");
@@ -53,6 +62,31 @@ public class ApiKeyValidator
 
         return ApiKeyValidationResult.Valid(apiKey, method);
     }
+
+    /// <summary>
+    /// InboundAPI-003: Finds the key whose value matches <paramref name="candidate"/>
+    /// using <see cref="CryptographicOperations.FixedTimeEquals"/> over the UTF-8 bytes.
+    /// Every candidate row is compared so that the running time does not depend on the
+    /// match position; length mismatches return false without leaking length timing.
+    /// </summary>
+    private static ApiKey? FindKeyConstantTime(IEnumerable<ApiKey> keys, string candidate)
+    {
+        var candidateBytes = Encoding.UTF8.GetBytes(candidate);
+        ApiKey? match = null;
+
+        foreach (var key in keys)
+        {
+            var keyBytes = Encoding.UTF8.GetBytes(key.KeyValue);
+            if (CryptographicOperations.FixedTimeEquals(candidateBytes, keyBytes))
+            {
+                // Do not break — continuing keeps the loop's timing independent of
+                // where (or whether) a match is found.
+                match = key;
+            }
+        }
+
+        return match;
+    }
 }
 
 /// <summary>

src/ScadaLink.InboundAPI/ForbiddenApiChecker.cs

@@ -0,0 +1,121 @@
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.CSharp;
+using Microsoft.CodeAnalysis.CSharp.Syntax;
+
+namespace ScadaLink.InboundAPI;
+
+/// <summary>
+/// InboundAPI-005: Enforces the ScadaLink script trust model on inbound API method
+/// scripts before they are compiled into executable handlers.
+///
+/// The trust model (CLAUDE.md, Akka.NET conventions) forbids scripts from reaching
+/// <c>System.IO</c>, <c>System.Diagnostics.Process</c>, <c>System.Threading</c>,
+/// <c>System.Reflection</c>, and raw network APIs. Roslyn scripting performs no
+/// API allow/deny-listing — restricting default imports is a convenience, not a
+/// sandbox — so a script can fully-qualify any referenced type. This static check
+/// walks the script syntax tree and rejects any reference to a forbidden namespace,
+/// whether reached through a <c>using</c> directive or a fully-qualified name.
+/// </summary>
+public static class ForbiddenApiChecker
+{
+    /// <summary>
+    /// Namespace prefixes the trust model forbids. A script segment matches if it is
+    /// equal to one of these or is a child namespace of it.
+    /// </summary>
+    private static readonly string[] ForbiddenNamespaces =
+    {
+        "System.IO",
+        "System.Diagnostics",   // covers Process
+        "System.Threading",     // Task/Tasks is explicitly re-allowed below
+        "System.Reflection",
+        "System.Net",           // raw network (Sockets, HttpClient, etc.)
+        "System.Runtime.InteropServices",
+        "Microsoft.Win32",
+    };
+
+    /// <summary>
+    /// Namespaces that would otherwise be caught by a forbidden prefix but are
+    /// required for normal async script authoring and carry no host-access risk.
+    /// </summary>
+    private static readonly string[] AllowedExceptions =
+    {
+        "System.Threading.Tasks",
+    };
+
+    /// <summary>
+    /// Analyses the script source and returns the list of trust-model violations.
+    /// An empty list means the script is acceptable.
+    /// </summary>
+    public static IReadOnlyList<string> FindViolations(string scriptCode)
+    {
+        if (string.IsNullOrWhiteSpace(scriptCode))
+            return Array.Empty<string>();
+
+        var tree = CSharpSyntaxTree.ParseText(
+            scriptCode,
+            new CSharpParseOptions(kind: SourceCodeKind.Script));
+
+        var walker = new ForbiddenApiWalker();
+        walker.Visit(tree.GetRoot());
+        return walker.Violations;
+    }
+
+    private static bool IsForbidden(string dottedName)
+    {
+        foreach (var allowed in AllowedExceptions)
+        {
+            if (dottedName == allowed || dottedName.StartsWith(allowed + ".", StringComparison.Ordinal))
+                return false;
+        }
+
+        foreach (var forbidden in ForbiddenNamespaces)
+        {
+            if (dottedName == forbidden || dottedName.StartsWith(forbidden + ".", StringComparison.Ordinal))
+                return true;
+        }
+
+        return false;
+    }
+
+    private sealed class ForbiddenApiWalker : CSharpSyntaxWalker
+    {
+        private readonly List<string> _violations = new();
+
+        public IReadOnlyList<string> Violations => _violations;
+
+        public override void VisitUsingDirective(UsingDirectiveSyntax node)
+        {
+            if (node.Name is not null && IsForbidden(node.Name.ToString()))
+                _violations.Add($"forbidden namespace import '{node.Name}'");
+
+            base.VisitUsingDirective(node);
+        }
+
+        public override void VisitQualifiedName(QualifiedNameSyntax node)
+        {
+            // Check the longest qualified name; do not descend so a single
+            // System.IO.File reference is reported once, not three times.
+            var text = node.ToString();
+            if (IsForbidden(text))
+            {
+                _violations.Add($"forbidden type reference '{text}'");
+                return;
+            }
+
+            base.VisitQualifiedName(node);
+        }
+
+        public override void VisitMemberAccessExpression(MemberAccessExpressionSyntax node)
+        {
+            // Catches fully-qualified expressions such as System.IO.File.Delete(...).
+            var text = node.ToString();
+            if (IsForbidden(text))
+            {
+                _violations.Add($"forbidden API access '{text}'");
+                return;
+            }
+
+            base.VisitMemberAccessExpression(node);
+        }
+    }
+}

src/ScadaLink.InboundAPI/InboundScriptExecutor.cs

@@ -1,3 +1,4 @@
+using System.Collections.Concurrent;
 using System.Text.Json;
 using Microsoft.CodeAnalysis.CSharp.Scripting;
 using Microsoft.CodeAnalysis.Scripting;
@@ -14,7 +15,11 @@ namespace ScadaLink.InboundAPI;
 public class InboundScriptExecutor
 {
     private readonly ILogger<InboundScriptExecutor> _logger;
-    private readonly Dictionary<string, Func<InboundScriptContext, Task<object?>>> _scriptHandlers = new();
+
+    // InboundAPI-001: this executor is registered as a singleton and its handler cache
+    // is read and written from concurrent ASP.NET request threads. A plain Dictionary is
+    // not safe for concurrent read/write, so a ConcurrentDictionary is used throughout.
+    private readonly ConcurrentDictionary<string, Func<InboundScriptContext, Task<object?>>> _scriptHandlers = new();
 
     private readonly IServiceProvider _serviceProvider;
 
@@ -37,18 +42,45 @@ public class InboundScriptExecutor
     /// </summary>
     public void RemoveHandler(string methodName)
     {
-        _scriptHandlers.Remove(methodName);
+        _scriptHandlers.TryRemove(methodName, out _);
     }
 
     /// <summary>
-    /// Compiles and registers a single API method script.
+    /// Compiles and registers a single API method script. Returns <c>false</c> if the
+    /// script is empty, fails Roslyn compilation, or violates the script trust model.
     /// </summary>
     public bool CompileAndRegister(ApiMethod method)
+        => Compile(method) is { } handler && Register(method.Name, handler);
+
+    private bool Register(string methodName, Func<InboundScriptContext, Task<object?>> handler)
+    {
+        _scriptHandlers[methodName] = handler;
+        return true;
+    }
+
+    /// <summary>
+    /// Compiles a single API method script into an executable handler. Returns
+    /// <c>null</c> when the script is missing, fails to compile, or violates the
+    /// script trust model (InboundAPI-005). Does not mutate the handler cache.
+    /// </summary>
+    private Func<InboundScriptContext, Task<object?>>? Compile(ApiMethod method)
     {
         if (string.IsNullOrWhiteSpace(method.Script))
         {
             _logger.LogWarning("API method {Method} has no script code", method.Name);
-            return false;
+            return null;
+        }
+
+        // InboundAPI-005: enforce the script trust model before compiling. Roslyn
+        // scripting performs no API allow/deny-listing, so forbidden namespaces must
+        // be rejected statically or the script could reach the host process.
+        var violations = ForbiddenApiChecker.FindViolations(method.Script);
+        if (violations.Count > 0)
+        {
+            _logger.LogWarning(
+                "API method {Method} script rejected — trust model violation(s): {Violations}",
+                method.Name, string.Join("; ", violations));
+            return null;
         }
 
         try
@@ -83,22 +115,20 @@ public class InboundScriptExecutor
                 _logger.LogWarning(
                     "API method {Method} script compilation failed: {Errors}",
                     method.Name, string.Join("; ", errors));
-                return false;
+                return null;
             }
 
-            _scriptHandlers[method.Name] = async ctx =>
+            _logger.LogInformation("API method {Method} script compiled", method.Name);
+            return async ctx =>
             {
                 var state = await compiled.RunAsync(ctx, ctx.CancellationToken);
                 return state.ReturnValue;
             };
-
-            _logger.LogInformation("API method {Method} script compiled and registered", method.Name);
-            return true;
         }
         catch (Exception ex)
         {
             _logger.LogError(ex, "Failed to compile API method {Method} script", method.Name);
-            return false;
+            return null;
         }
     }
 
@@ -119,15 +149,18 @@ public class InboundScriptExecutor
 
             var context = new InboundScriptContext(parameters, route, cts.Token);
 
-            object? result;
             if (!_scriptHandlers.TryGetValue(method.Name, out var handler))
             {
-                // Lazy compile on first request (handles methods created after startup)
+                // Lazy compile on first request (handles methods created after startup).
-                if (!CompileAndRegister(method))
+                // Compile outside the cache so a failed compile is not stored, then add
+                // atomically so concurrent first-callers share a single handler instance.
+                var compiled = Compile(method);
+                if (compiled == null)
                     return new InboundScriptResult(false, null, "Script compilation failed for this method");
-                handler = _scriptHandlers[method.Name];
+                handler = _scriptHandlers.GetOrAdd(method.Name, compiled);
             }
-            result = await handler(context).WaitAsync(cts.Token);
+
+            var result = await handler(context).WaitAsync(cts.Token);
 
             var resultJson = result != null
                 ? JsonSerializer.Serialize(result)

src/ScadaLink.ManagementService/DebugStreamHub.cs

@@ -2,6 +2,7 @@ using System.Text;
 using Microsoft.AspNetCore.SignalR;
 using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Extensions.Logging;
+using ScadaLink.Commons.Interfaces.Repositories;
 using ScadaLink.Commons.Messages.DebugView;
 using ScadaLink.Commons.Messages.Streaming;
 using ScadaLink.Communication;
@@ -17,6 +18,26 @@ namespace ScadaLink.ManagementService;
 public class DebugStreamHub : Hub
 {
     private const string SessionIdKey = "DebugStreamSessionId";
+    private const string RolesKey = "DebugStreamRoles";
+    private const string PermittedSiteIdsKey = "DebugStreamPermittedSiteIds";
+
+    /// <summary>
+    /// Pure site-scope authorization check for a debug-stream subscription.
+    /// Returns true when the caller may subscribe to a debug stream for an instance
+    /// belonging to <paramref name="instanceSiteId"/>.
+    /// Admin role, or an empty <paramref name="permittedSiteIds"/> (system-wide
+    /// Deployment), grants access to any site; otherwise the instance's site must be
+    /// in the permitted set.
+    /// </summary>
+    public static bool IsInstanceAccessAllowed(
+        IReadOnlyCollection<string> roles,
+        IReadOnlyCollection<string> permittedSiteIds,
+        int instanceSiteId)
+    {
+        if (roles.Contains("Admin", StringComparer.OrdinalIgnoreCase)) return true;
+        if (permittedSiteIds.Count == 0) return true; // system-wide deployment
+        return permittedSiteIds.Contains(instanceSiteId.ToString());
+    }
 
     private readonly DebugStreamService _debugStreamService;
     private readonly IHubContext<DebugStreamHub> _hubContext;
@@ -93,6 +114,11 @@ public class DebugStreamHub : Hub
             return;
         }
 
+        // Persist the resolved identity on the connection so per-instance site-scope
+        // enforcement can be applied to SubscribeInstance calls.
+        Context.Items[RolesKey] = mappingResult.Roles.ToArray();
+        Context.Items[PermittedSiteIdsKey] = mappingResult.PermittedSiteIds.ToArray();
+
         _logger.LogInformation("DebugStreamHub connection established for {Username}", username);
         await base.OnConnectedAsync();
     }
@@ -108,6 +134,41 @@ public class DebugStreamHub : Hub
 
         var connectionId = Context.ConnectionId;
 
+        // Per-instance site-scope enforcement: a site-scoped Deployment user must not
+        // be able to stream an instance belonging to a site outside their scope.
+        var httpContext = Context.GetHttpContext();
+        if (httpContext == null)
+        {
+            _logger.LogWarning("DebugStreamHub: {ConnectionId} subscribe rejected — no HTTP context", connectionId);
+            await Clients.Caller.SendAsync("OnStreamTerminated", "Authorization context unavailable.");
+            return;
+        }
+
+        var roles = Context.Items.TryGetValue(RolesKey, out var rolesObj) && rolesObj is string[] r
+            ? r : Array.Empty<string>();
+        var permittedSiteIds = Context.Items.TryGetValue(PermittedSiteIdsKey, out var sitesObj) && sitesObj is string[] s
+            ? s : Array.Empty<string>();
+
+        var instanceRepo = httpContext.RequestServices.GetRequiredService<ITemplateEngineRepository>();
+        var instance = await instanceRepo.GetInstanceByIdAsync(instanceId);
+        if (instance == null)
+        {
+            _logger.LogWarning("DebugStreamHub: {ConnectionId} subscribe rejected — instance {InstanceId} not found",
+                connectionId, instanceId);
+            await Clients.Caller.SendAsync("OnStreamTerminated", $"Instance {instanceId} not found.");
+            return;
+        }
+
+        if (!IsInstanceAccessAllowed(roles, permittedSiteIds, instance.SiteId))
+        {
+            _logger.LogWarning(
+                "DebugStreamHub: {ConnectionId} subscribe to instance {InstanceId} denied — site {SiteId} outside permitted scope",
+                connectionId, instanceId, instance.SiteId);
+            await Clients.Caller.SendAsync("OnStreamTerminated",
+                $"Access denied: instance {instanceId} belongs to a site outside your Deployment scope.");
+            return;
+        }
+
         try
         {
             // Use IHubContext for callbacks — the hub instance is transient (disposed after method returns),

src/ScadaLink.ManagementService/ManagementActor.cs

@@ -164,7 +164,7 @@ public class ManagementActor : ReceiveActor
 
             // Instances
             ListInstancesCommand cmd => await HandleListInstances(sp, cmd, user),
-            GetInstanceCommand cmd => await HandleGetInstance(sp, cmd),
+            GetInstanceCommand cmd => await HandleGetInstance(sp, cmd, user),
             CreateInstanceCommand cmd => await HandleCreateInstance(sp, cmd, user),
             MgmtDeployInstanceCommand cmd => await HandleDeployInstance(sp, cmd, user),
             MgmtEnableInstanceCommand cmd => await HandleEnableInstance(sp, cmd, user),
@@ -179,18 +179,18 @@ public class ManagementActor : ReceiveActor
 
             // Sites
             ListSitesCommand => await HandleListSites(sp, user),
-            GetSiteCommand cmd => await HandleGetSite(sp, cmd),
+            GetSiteCommand cmd => await HandleGetSite(sp, cmd, user),
             CreateSiteCommand cmd => await HandleCreateSite(sp, cmd, user.Username),
             UpdateSiteCommand cmd => await HandleUpdateSite(sp, cmd, user.Username),
             DeleteSiteCommand cmd => await HandleDeleteSite(sp, cmd, user.Username),
-            ListAreasCommand cmd => await HandleListAreas(sp, cmd),
+            ListAreasCommand cmd => await HandleListAreas(sp, cmd, user),
             CreateAreaCommand cmd => await HandleCreateArea(sp, cmd, user.Username),
             DeleteAreaCommand cmd => await HandleDeleteArea(sp, cmd, user.Username),
             UpdateAreaCommand cmd => await HandleUpdateArea(sp, cmd, user.Username),
 
             // Data Connections
             ListDataConnectionsCommand cmd => await HandleListDataConnections(sp, cmd),
-            GetDataConnectionCommand cmd => await HandleGetDataConnection(sp, cmd),
+            GetDataConnectionCommand cmd => await HandleGetDataConnection(sp, cmd, user),
             CreateDataConnectionCommand cmd => await HandleCreateDataConnection(sp, cmd, user.Username),
             UpdateDataConnectionCommand cmd => await HandleUpdateDataConnection(sp, cmd, user.Username),
             DeleteDataConnectionCommand cmd => await HandleDeleteDataConnection(sp, cmd, user.Username),
@@ -263,11 +263,11 @@ public class ManagementActor : ReceiveActor
             GetSiteHealthCommand cmd => HandleGetSiteHealth(sp, cmd),
 
             // Remote Queries
-            QueryEventLogsCommand cmd => await HandleQueryEventLogs(sp, cmd),
+            QueryEventLogsCommand cmd => await HandleQueryEventLogs(sp, cmd, user),
-            QueryParkedMessagesCommand cmd => await HandleQueryParkedMessages(sp, cmd),
+            QueryParkedMessagesCommand cmd => await HandleQueryParkedMessages(sp, cmd, user),
-            RetryParkedMessageCommand cmd => await HandleRetryParkedMessage(sp, cmd),
+            RetryParkedMessageCommand cmd => await HandleRetryParkedMessage(sp, cmd, user),
-            DiscardParkedMessageCommand cmd => await HandleDiscardParkedMessage(sp, cmd),
+            DiscardParkedMessageCommand cmd => await HandleDiscardParkedMessage(sp, cmd, user),
-            DebugSnapshotCommand cmd => await HandleDebugSnapshot(sp, cmd),
+            DebugSnapshotCommand cmd => await HandleDebugSnapshot(sp, cmd, user),
 
             // Role resolution (for CLI LDAP auth)
             ResolveRolesCommand cmd => await HandleResolveRoles(sp, cmd),
@@ -329,6 +329,21 @@ public class ManagementActor : ReceiveActor
             EnforceSiteScope(user, instance.SiteId);
     }
 
+    /// <summary>
+    /// Resolves a site by its string identifier and enforces site-scope.
+    /// Used by remote-query handlers that key off the site identifier rather than its ID.
+    /// </summary>
+    private static async Task EnforceSiteScopeForIdentifier(IServiceProvider sp, AuthenticatedUser user, string siteIdentifier)
+    {
+        if (user.PermittedSiteIds.Length == 0) return;
+        if (user.Roles.Contains("Admin", StringComparer.OrdinalIgnoreCase)) return;
+
+        var repo = sp.GetRequiredService<ISiteRepository>();
+        var site = await repo.GetSiteByIdentifierAsync(siteIdentifier);
+        if (site != null)
+            EnforceSiteScope(user, site.Id);
+    }
+
     /// <summary>
     /// Helper to log an audit entry after a successful mutation.
     /// </summary>
@@ -507,10 +522,13 @@ public class ManagementActor : ReceiveActor
         return instances;
     }
 
-    private static async Task<object?> HandleGetInstance(IServiceProvider sp, GetInstanceCommand cmd)
+    private static async Task<object?> HandleGetInstance(IServiceProvider sp, GetInstanceCommand cmd, AuthenticatedUser user)
     {
         var repo = sp.GetRequiredService<ITemplateEngineRepository>();
-        return await repo.GetInstanceByIdAsync(cmd.InstanceId);
+        var instance = await repo.GetInstanceByIdAsync(cmd.InstanceId);
+        if (instance != null)
+            EnforceSiteScope(user, instance.SiteId);
+        return instance;
     }
 
     private static async Task<object?> HandleCreateInstance(IServiceProvider sp, CreateInstanceCommand cmd, AuthenticatedUser user)
@@ -638,16 +656,18 @@ public class ManagementActor : ReceiveActor
             : throw new InvalidOperationException(result.Error);
     }
 
-    private static async Task<object?> HandleRetryParkedMessage(IServiceProvider sp, RetryParkedMessageCommand cmd)
+    private static async Task<object?> HandleRetryParkedMessage(IServiceProvider sp, RetryParkedMessageCommand cmd, AuthenticatedUser user)
     {
+        await EnforceSiteScopeForIdentifier(sp, user, cmd.SiteIdentifier);
         var commService = sp.GetRequiredService<CommunicationService>();
         var request = new Commons.Messages.RemoteQuery.ParkedMessageRetryRequest(
             Guid.NewGuid().ToString("N"), cmd.SiteIdentifier, cmd.MessageId, DateTimeOffset.UtcNow);
         return await commService.RetryParkedMessageAsync(cmd.SiteIdentifier, request);
     }
 
-    private static async Task<object?> HandleDiscardParkedMessage(IServiceProvider sp, DiscardParkedMessageCommand cmd)
+    private static async Task<object?> HandleDiscardParkedMessage(IServiceProvider sp, DiscardParkedMessageCommand cmd, AuthenticatedUser user)
     {
+        await EnforceSiteScopeForIdentifier(sp, user, cmd.SiteIdentifier);
         var commService = sp.GetRequiredService<CommunicationService>();
         var request = new Commons.Messages.RemoteQuery.ParkedMessageDiscardRequest(
             Guid.NewGuid().ToString("N"), cmd.SiteIdentifier, cmd.MessageId, DateTimeOffset.UtcNow);
@@ -670,10 +690,13 @@ public class ManagementActor : ReceiveActor
         return sites;
     }
 
-    private static async Task<object?> HandleGetSite(IServiceProvider sp, GetSiteCommand cmd)
+    private static async Task<object?> HandleGetSite(IServiceProvider sp, GetSiteCommand cmd, AuthenticatedUser user)
     {
         var repo = sp.GetRequiredService<ISiteRepository>();
-        return await repo.GetSiteByIdAsync(cmd.SiteId);
+        var site = await repo.GetSiteByIdAsync(cmd.SiteId);
+        if (site != null)
+            EnforceSiteScope(user, site.Id);
+        return site;
     }
 
     private static async Task<object?> HandleCreateSite(IServiceProvider sp, CreateSiteCommand cmd, string user)
@@ -730,8 +753,9 @@ public class ManagementActor : ReceiveActor
         return true;
     }
 
-    private static async Task<object?> HandleListAreas(IServiceProvider sp, ListAreasCommand cmd)
+    private static async Task<object?> HandleListAreas(IServiceProvider sp, ListAreasCommand cmd, AuthenticatedUser user)
     {
+        EnforceSiteScope(user, cmd.SiteId);
         var repo = sp.GetRequiredService<ICentralUiRepository>();
         return await repo.GetAreaTreeBySiteIdAsync(cmd.SiteId);
     }
@@ -771,10 +795,13 @@ public class ManagementActor : ReceiveActor
         return await repo.GetAllDataConnectionsAsync();
     }
 
-    private static async Task<object?> HandleGetDataConnection(IServiceProvider sp, GetDataConnectionCommand cmd)
+    private static async Task<object?> HandleGetDataConnection(IServiceProvider sp, GetDataConnectionCommand cmd, AuthenticatedUser user)
     {
         var repo = sp.GetRequiredService<ISiteRepository>();
-        return await repo.GetDataConnectionByIdAsync(cmd.DataConnectionId);
+        var conn = await repo.GetDataConnectionByIdAsync(cmd.DataConnectionId);
+        if (conn != null)
+            EnforceSiteScope(user, conn.SiteId);
+        return conn;
     }
 
     private static async Task<object?> HandleCreateDataConnection(IServiceProvider sp, CreateDataConnectionCommand cmd, string user)
@@ -1462,8 +1489,9 @@ public class ManagementActor : ReceiveActor
     // Remote Query handlers
     // ========================================================================
 
-    private static async Task<object?> HandleQueryEventLogs(IServiceProvider sp, QueryEventLogsCommand cmd)
+    private static async Task<object?> HandleQueryEventLogs(IServiceProvider sp, QueryEventLogsCommand cmd, AuthenticatedUser user)
     {
+        await EnforceSiteScopeForIdentifier(sp, user, cmd.SiteIdentifier);
         var commService = sp.GetRequiredService<CommunicationService>();
         var request = new EventLogQueryRequest(
             Guid.NewGuid().ToString("N"),
@@ -1478,8 +1506,9 @@ public class ManagementActor : ReceiveActor
         return await commService.QueryEventLogsAsync(cmd.SiteIdentifier, request);
     }
 
-    private static async Task<object?> HandleQueryParkedMessages(IServiceProvider sp, QueryParkedMessagesCommand cmd)
+    private static async Task<object?> HandleQueryParkedMessages(IServiceProvider sp, QueryParkedMessagesCommand cmd, AuthenticatedUser user)
     {
+        await EnforceSiteScopeForIdentifier(sp, user, cmd.SiteIdentifier);
         var commService = sp.GetRequiredService<CommunicationService>();
         var request = new ParkedMessageQueryRequest(
             Guid.NewGuid().ToString("N"),
@@ -1490,12 +1519,14 @@ public class ManagementActor : ReceiveActor
         return await commService.QueryParkedMessagesAsync(cmd.SiteIdentifier, request);
     }
 
-    private static async Task<object?> HandleDebugSnapshot(IServiceProvider sp, DebugSnapshotCommand cmd)
+    private static async Task<object?> HandleDebugSnapshot(IServiceProvider sp, DebugSnapshotCommand cmd, AuthenticatedUser user)
     {
         var instanceRepo = sp.GetRequiredService<ITemplateEngineRepository>();
         var instance = await instanceRepo.GetInstanceByIdAsync(cmd.InstanceId)
             ?? throw new InvalidOperationException($"Instance {cmd.InstanceId} not found.");
 
+        EnforceSiteScope(user, instance.SiteId);
+
         var siteRepo = sp.GetRequiredService<ISiteRepository>();
         var site = await siteRepo.GetSiteByIdAsync(instance.SiteId)
             ?? throw new InvalidOperationException($"Site {instance.SiteId} not found.");

src/ScadaLink.NotificationService/NotificationDeliveryService.cs

@@ -1,4 +1,7 @@
+using System.Net.Sockets;
 using System.Text.Json;
+using MailKit;
+using MailKit.Net.Smtp;
 using Microsoft.Extensions.Logging;
 using ScadaLink.Commons.Entities.Notifications;
 using ScadaLink.Commons.Interfaces.Repositories;
@@ -76,7 +79,12 @@ public class NotificationDeliveryService : INotificationDeliveryService
             _logger.LogError(ex, "Permanent SMTP failure sending to list {List}", listName);
             return new NotificationResult(false, $"Permanent SMTP error: {ex.Message}");
         }
-        catch (Exception ex) when (IsTransientSmtpError(ex))
+        catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested)
+        {
+            // NS-002: a caller-requested cancellation propagates; it is not buffered.
+            throw;
+        }
+        catch (Exception ex) when (IsTransientSmtpError(ex, cancellationToken))
         {
             // WP-12: Transient SMTP failure — hand to S&F
             _logger.LogWarning(ex, "Transient SMTP failure sending to list {List}, buffering for retry", listName);
@@ -172,8 +180,9 @@ public class NotificationDeliveryService : INotificationDeliveryService
         string body,
         CancellationToken cancellationToken)
     {
-        using var client = _smtpClientFactory() as IDisposable;
+        // NS-004: create exactly one client and dispose the one actually used.
         var smtp = _smtpClientFactory();
+        using var disposable = smtp as IDisposable;
 
         try
         {
@@ -195,32 +204,80 @@ public class NotificationDeliveryService : INotificationDeliveryService
 
             await smtp.DisconnectAsync(cancellationToken);
         }
-        catch (Exception ex) when (ex is not SmtpPermanentException && !IsTransientSmtpError(ex))
+        catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested)
         {
-            // Classify unrecognized SMTP exceptions
+            // NS-002: A deliberately cancelled token must propagate as a cancellation,
-            if (ex.Message.Contains("5.", StringComparison.Ordinal) ||
+            // not be misclassified as a transient SMTP failure and buffered for retry.
-                ex.Message.Contains("550", StringComparison.Ordinal) ||
-                ex.Message.Contains("553", StringComparison.Ordinal) ||
-                ex.Message.Contains("554", StringComparison.Ordinal))
-            {
-                throw new SmtpPermanentException(ex.Message, ex);
-            }
-
-            // Default: treat as transient
             throw;
         }
+        catch (Exception ex) when (ClassifySmtpError(ex, cancellationToken) == SmtpErrorClass.Permanent
+                                   && ex is not SmtpPermanentException)
+        {
+            // NS-003: Permanent SMTP failure (5xx) — surface a typed permanent exception.
+            throw new SmtpPermanentException(ex.Message, ex);
+        }
+        // Transient and SmtpPermanentException both propagate unchanged: SendAsync's
+        // catch filters (SmtpPermanentException / IsTransientSmtpError) handle them.
     }
 
-    private static bool IsTransientSmtpError(Exception ex)
+    private enum SmtpErrorClass
     {
-        return ex is TimeoutException
+        /// <summary>Cancellation or an unrecognised exception — caller decides.</summary>
-            or OperationCanceledException
+        Unknown,
-            or System.Net.Sockets.SocketException
+        /// <summary>Retryable failure (4xx, connection/socket/protocol error, timeout).</summary>
-            or IOException
+        Transient,
-            || ex.Message.Contains("4.", StringComparison.Ordinal)
+        /// <summary>Non-retryable failure (5xx) — must be returned to the script.</summary>
-            || ex.Message.Contains("421", StringComparison.Ordinal)
+        Permanent,
-            || ex.Message.Contains("450", StringComparison.Ordinal)
+    }
-            || ex.Message.Contains("451", StringComparison.Ordinal);
+
+    /// <summary>
+    /// NS-002/NS-003: Classifies an SMTP failure using MailKit's typed exceptions and
+    /// the numeric <see cref="SmtpStatusCode"/> rather than locale-dependent substring
+    /// matching on the exception message. A cancellation requested by the caller is
+    /// never treated as a transient SMTP error.
+    /// </summary>
+    private static SmtpErrorClass ClassifySmtpError(Exception ex, CancellationToken cancellationToken)
+    {
+        // A deliberate cancellation is not an SMTP error at all.
+        if (ex is OperationCanceledException && cancellationToken.IsCancellationRequested)
+        {
+            return SmtpErrorClass.Unknown;
+        }
+
+        // MailKit reports SMTP command failures with the real status code; the
+        // SmtpStatusCode enum's underlying value is the numeric SMTP reply code.
+        if (ex is SmtpCommandException command)
+        {
+            var code = (int)command.StatusCode;
+            if (code >= 400 && code < 500)
+            {
+                return SmtpErrorClass.Transient;
+            }
+
+            if (code >= 500 && code < 600)
+            {
+                return SmtpErrorClass.Permanent;
+            }
+
+            return SmtpErrorClass.Unknown;
+        }
+
+        // Protocol errors, a dropped/unavailable service, socket failures and
+        // timeouts are all retryable — the message has not been rejected.
+        if (ex is SmtpProtocolException
+            or ServiceNotConnectedException
+            or SocketException
+            or TimeoutException)
+        {
+            return SmtpErrorClass.Transient;
+        }
+
+        return SmtpErrorClass.Unknown;
+    }
+
+    private static bool IsTransientSmtpError(Exception ex, CancellationToken cancellationToken)
+    {
+        return ClassifySmtpError(ex, cancellationToken) == SmtpErrorClass.Transient;
     }
 }
 

src/ScadaLink.Security/JwtTokenService.cs

@@ -22,6 +22,19 @@ public class JwtTokenService
     {
         _options = options?.Value ?? throw new ArgumentNullException(nameof(options));
         _logger = logger ?? throw new ArgumentNullException(nameof(logger));
+
+        // Fail fast: a missing or short signing key produces trivially forgeable tokens.
+        // HMAC-SHA256 requires a key of at least 256 bits (32 bytes).
+        var keyByteLength = string.IsNullOrEmpty(_options.JwtSigningKey)
+            ? 0
+            : Encoding.UTF8.GetByteCount(_options.JwtSigningKey);
+        if (keyByteLength < SecurityOptions.MinJwtSigningKeyBytes)
+        {
+            throw new InvalidOperationException(
+                $"SecurityOptions.JwtSigningKey must be at least {SecurityOptions.MinJwtSigningKeyBytes} bytes " +
+                $"(256 bits) for HMAC-SHA256; the configured key is {keyByteLength} byte(s). " +
+                "Configure a strong signing key before starting the service.");
+        }
     }
 
     public string GenerateToken(

src/ScadaLink.Security/LdapAuthService.cs

@@ -24,7 +24,7 @@ public class LdapAuthService
             return new LdapAuthResult(false, null, null, null, "Password is required.");
 
         // Enforce TLS unless explicitly allowed for dev/test
-        if (!_options.LdapUseTls && !_options.AllowInsecureLdap)
+        if (_options.LdapTransport == LdapTransport.None && !_options.AllowInsecureLdap)
         {
             return new LdapAuthResult(false, null, null, null,
                 "Insecure LDAP connections are not allowed. Enable TLS or set AllowInsecureLdap for dev/test.");
@@ -34,16 +34,24 @@ public class LdapAuthService
         {
             using var connection = new LdapConnection();
 
-            if (_options.LdapUseTls)
+            // LDAPS: TLS negotiated at connection time. StartTLS: connect plaintext,
+            // then upgrade the session before any credentials are sent.
+            if (_options.LdapTransport == LdapTransport.Ldaps)
             {
                 connection.SecureSocketLayer = true;
             }
 
             await Task.Run(() => connection.Connect(_options.LdapServer, _options.LdapPort), ct);
 
-            if (_options.LdapUseTls && !connection.SecureSocketLayer)
+            if (_options.LdapTransport == LdapTransport.StartTls)
             {
                 await Task.Run(() => connection.StartTls(), ct);
+
+                if (!connection.Tls)
+                {
+                    return new LdapAuthResult(false, null, null, null,
+                        "StartTLS upgrade did not produce an encrypted session.");
+                }
             }
 
             // Resolve the user's actual DN, then bind with their credentials

src/ScadaLink.Security/LdapTransport.cs

@@ -0,0 +1,26 @@
+namespace ScadaLink.Security;
+
+/// <summary>
+/// Transport security mode for the LDAP connection. The design requires either
+/// LDAPS or StartTLS in production; <see cref="None"/> is for dev/test only and
+/// must be paired with <see cref="SecurityOptions.AllowInsecureLdap"/>.
+/// </summary>
+public enum LdapTransport
+{
+    /// <summary>
+    /// LDAPS — TLS negotiated at connection time (typically port 636).
+    /// </summary>
+    Ldaps,
+
+    /// <summary>
+    /// StartTLS — connect in plaintext (typically port 389), then upgrade the
+    /// session to TLS before binding.
+    /// </summary>
+    StartTls,
+
+    /// <summary>
+    /// No transport security. Dev/test only — requires
+    /// <see cref="SecurityOptions.AllowInsecureLdap"/> to be true.
+    /// </summary>
+    None
+}

src/ScadaLink.Security/SecurityOptions.cs

@@ -4,7 +4,24 @@ public class SecurityOptions
 {
     public string LdapServer { get; set; } = string.Empty;
     public int LdapPort { get; set; } = 389;
-    public bool LdapUseTls { get; set; } = true;
+
+    /// <summary>
+    /// Transport security mode for the LDAP connection. Defaults to LDAPS.
+    /// Use <see cref="LdapTransport.StartTls"/> to connect on the plaintext port
+    /// and upgrade the session before binding.
+    /// </summary>
+    public LdapTransport LdapTransport { get; set; } = LdapTransport.Ldaps;
+
+    /// <summary>
+    /// True when the configured transport provides encryption (LDAPS or StartTLS).
+    /// Retained for backward compatibility: assigning a value maps onto
+    /// <see cref="LdapTransport"/> (true =&gt; LDAPS, false =&gt; None).
+    /// </summary>
+    public bool LdapUseTls
+    {
+        get => LdapTransport != LdapTransport.None;
+        set => LdapTransport = value ? LdapTransport.Ldaps : LdapTransport.None;
+    }
 
     /// <summary>
     /// Allow insecure (non-TLS) LDAP connections. ONLY for dev/test with GLAuth.
@@ -39,7 +56,16 @@ public class SecurityOptions
     /// </summary>
     public string LdapGroupAttribute { get; set; } = "memberOf";
 
+    /// <summary>
+    /// Symmetric HMAC-SHA256 signing key for cookie-embedded JWTs. Must be at least
+    /// 32 bytes (256 bits) — validated at <see cref="JwtTokenService"/> construction.
+    /// </summary>
     public string JwtSigningKey { get; set; } = string.Empty;
+
+    /// <summary>
+    /// Minimum signing-key length in bytes required for HMAC-SHA256 (256 bits).
+    /// </summary>
+    public const int MinJwtSigningKeyBytes = 32;
     public int JwtExpiryMinutes { get; set; } = 15;
     public int IdleTimeoutMinutes { get; set; } = 30;
 

src/ScadaLink.Security/ServiceCollectionExtensions.cs

@@ -20,6 +20,9 @@ public static class ServiceCollectionExtensions
                 options.Cookie.Name = "ScadaLink.Auth";
                 options.Cookie.HttpOnly = true;
                 options.Cookie.SameSite = Microsoft.AspNetCore.Http.SameSiteMode.Strict;
+                // The cookie carries the embedded JWT (a bearer credential); never
+                // transmit it over plain HTTP. Design: "HttpOnly and Secure (requires HTTPS)".
+                options.Cookie.SecurePolicy = Microsoft.AspNetCore.Http.CookieSecurePolicy.Always;
             });
 
         services.AddScadaLinkAuthorization();

src/ScadaLink.SiteEventLogging/EventLogPurgeService.cs

@@ -1,4 +1,3 @@
-using Microsoft.Data.Sqlite;
 using Microsoft.Extensions.Hosting;
 using Microsoft.Extensions.Logging;
 using Microsoft.Extensions.Options;
@@ -12,6 +11,9 @@ namespace ScadaLink.SiteEventLogging;
 /// </summary>
 public class EventLogPurgeService : BackgroundService
 {
+    /// <summary>Number of events deleted per cap-purge batch.</summary>
+    private const int CapPurgeBatchSize = 1000;
+
     private readonly SiteEventLogger _eventLogger;
     private readonly SiteEventLogOptions _options;
     private readonly ILogger<EventLogPurgeService> _logger;
@@ -21,7 +23,7 @@ public class EventLogPurgeService : BackgroundService
         IOptions<SiteEventLogOptions> options,
         ILogger<EventLogPurgeService> logger)
     {
-        // We need the concrete type to access the connection
+        // We need the concrete type to funnel access through its shared lock.
         _eventLogger = (SiteEventLogger)eventLogger;
         _options = options.Value;
         _logger = logger;
@@ -61,10 +63,13 @@ public class EventLogPurgeService : BackgroundService
     {
         var cutoff = DateTimeOffset.UtcNow.AddDays(-_options.RetentionDays).ToString("o");
 
-        using var cmd = _eventLogger.Connection.CreateCommand();
+        var deleted = _eventLogger.WithConnection(connection =>
+        {
+            using var cmd = connection.CreateCommand();
             cmd.CommandText = "DELETE FROM site_events WHERE timestamp < $cutoff";
             cmd.Parameters.AddWithValue("$cutoff", cutoff);
-        var deleted = cmd.ExecuteNonQuery();
+            return cmd.ExecuteNonQuery();
+        });
 
         if (deleted > 0)
         {
@@ -74,8 +79,8 @@ public class EventLogPurgeService : BackgroundService
 
     private void PurgeByStorageCap()
     {
-        var currentSizeBytes = GetDatabaseSizeBytes();
         var capBytes = (long)_options.MaxStorageMb * 1024 * 1024;
+        var currentSizeBytes = GetDatabaseSizeBytes();
 
         if (currentSizeBytes <= capBytes)
             return;
@@ -84,37 +89,77 @@ public class EventLogPurgeService : BackgroundService
             "Event log size {Size:F1} MB exceeds cap {Cap} MB — purging oldest events",
             currentSizeBytes / (1024.0 * 1024.0), _options.MaxStorageMb);
 
-        // Delete oldest events in batches until under the cap
+        // Delete the oldest events in batches until the database is under the cap.
+        // The loop also stops if the on-disk size fails to decrease across an
+        // iteration (e.g. if vacuum cannot reclaim space), so a cap that can never
+        // be met does not silently empty the entire table.
         while (currentSizeBytes > capBytes)
         {
-            using var cmd = _eventLogger.Connection.CreateCommand();
+            var previousSizeBytes = currentSizeBytes;
-            cmd.CommandText = """
+
+            var deleted = _eventLogger.WithConnection(connection =>
+            {
+                using var cmd = connection.CreateCommand();
+                cmd.CommandText = $"""
                     DELETE FROM site_events WHERE id IN (
-                    SELECT id FROM site_events ORDER BY id ASC LIMIT 1000
+                        SELECT id FROM site_events ORDER BY id ASC LIMIT {CapPurgeBatchSize}
                     )
                     """;
-            var deleted = cmd.ExecuteNonQuery();
+                var rows = cmd.ExecuteNonQuery();
-            if (deleted == 0) break;
 
-            // Reclaim space
+                // Reclaim free pages so page_count/freelist measurement reflects the
-            using var vacuumCmd = _eventLogger.Connection.CreateCommand();
+                // delete. Effective because auto_vacuum = INCREMENTAL is set at schema
+                // creation; harmless otherwise.
+                using var vacuumCmd = connection.CreateCommand();
                 vacuumCmd.CommandText = "PRAGMA incremental_vacuum";
                 vacuumCmd.ExecuteNonQuery();
 
+                return rows;
+            });
+
+            if (deleted == 0)
+                break;
+
             currentSizeBytes = GetDatabaseSizeBytes();
+
+            if (currentSizeBytes >= previousSizeBytes)
+            {
+                // Size is not shrinking despite deletes — stop rather than wipe the
+                // whole table. This should not happen now that logical size is
+                // measured, but guards against any future regression.
+                _logger.LogWarning(
+                    "Event log size did not decrease after a cap-purge batch ({Size:F1} MB); " +
+                    "stopping to avoid emptying the log",
+                    currentSizeBytes / (1024.0 * 1024.0));
+                break;
+            }
         }
     }
 
+    /// <summary>
+    /// Returns the logical size of the database in bytes — only pages that hold live
+    /// data, excluding free pages on the freelist. Measuring logical size (rather than
+    /// the raw file size from <c>page_count</c>) means the storage-cap loop observes
+    /// space being reclaimed even if free pages have not yet been returned to the OS.
+    /// </summary>
     internal long GetDatabaseSizeBytes()
     {
-        using var pageCountCmd = _eventLogger.Connection.CreateCommand();
+        return _eventLogger.WithConnection(connection =>
+        {
+            using var pageCountCmd = connection.CreateCommand();
             pageCountCmd.CommandText = "PRAGMA page_count";
             var pageCount = (long)pageCountCmd.ExecuteScalar()!;
 
-        using var pageSizeCmd = _eventLogger.Connection.CreateCommand();
+            using var freeListCmd = connection.CreateCommand();
+            freeListCmd.CommandText = "PRAGMA freelist_count";
+            var freeListCount = (long)freeListCmd.ExecuteScalar()!;
+
+            using var pageSizeCmd = connection.CreateCommand();
             pageSizeCmd.CommandText = "PRAGMA page_size";
             var pageSize = (long)pageSizeCmd.ExecuteScalar()!;
 
-        return pageCount * pageSize;
+            var usedPages = Math.Max(0, pageCount - freeListCount);
+            return usedPages * pageSize;
+        });
     }
 }

src/ScadaLink.SiteEventLogging/EventLogQueryService.cs

@@ -33,7 +33,6 @@ public class EventLogQueryService : IEventLogQueryService
         {
             var pageSize = request.PageSize > 0 ? request.PageSize : _options.QueryPageSize;
 
-            using var cmd = _eventLogger.Connection.CreateCommand();
             var whereClauses = new List<string>();
             var parameters = new List<SqliteParameter>();
 
@@ -84,6 +83,13 @@ public class EventLogQueryService : IEventLogQueryService
                 ? "WHERE " + string.Join(" AND ", whereClauses)
                 : "";
 
+            // Run the read against the shared connection under the logger's write
+            // lock — the connection is not thread-safe and is also used by the
+            // recorder and the purge service on other threads.
+            var entries = _eventLogger.WithConnection(connection =>
+            {
+                using var cmd = connection.CreateCommand();
+
                 // Fetch pageSize + 1 to determine if there are more results
                 cmd.CommandText = $"""
                     SELECT id, timestamp, event_type, severity, instance_id, source, message, details
@@ -96,11 +102,11 @@ public class EventLogQueryService : IEventLogQueryService
                 foreach (var p in parameters)
                     cmd.Parameters.Add(p);
 
-            var entries = new List<EventLogEntry>();
+                var rows = new List<EventLogEntry>();
                 using var reader = cmd.ExecuteReader();
                 while (reader.Read())
                 {
-                entries.Add(new EventLogEntry(
+                    rows.Add(new EventLogEntry(
                         Id: reader.GetInt64(0),
                         Timestamp: DateTimeOffset.Parse(reader.GetString(1)),
                         EventType: reader.GetString(2),
@@ -111,6 +117,9 @@ public class EventLogQueryService : IEventLogQueryService
                         Details: reader.IsDBNull(7) ? null : reader.GetString(7)));
                 }
 
+                return rows;
+            });
+
             var hasMore = entries.Count > pageSize;
             if (hasMore)
             {

src/ScadaLink.SiteEventLogging/SiteEventLogger.cs

@@ -9,6 +9,11 @@ namespace ScadaLink.SiteEventLogging;
 /// Only the active node generates events. Not replicated to standby.
 /// On failover, the new active node starts a fresh log.
 /// </summary>
+/// <remarks>
+/// A single <see cref="SqliteConnection"/> is owned here and is NOT thread-safe.
+/// All access — recording, querying, purging — must be funnelled through
+/// <see cref="WithConnection"/>, which serialises callers on a shared lock.
+/// </remarks>
 public class SiteEventLogger : ISiteEventLogger, IDisposable
 {
     private readonly SqliteConnection _connection;
@@ -31,10 +36,50 @@ public class SiteEventLogger : ISiteEventLogger, IDisposable
         InitializeSchema();
     }
 
-    internal SqliteConnection Connection => _connection;
+    /// <summary>
+    /// Runs <paramref name="action"/> against the shared connection while holding the
+    /// write lock, so purge / query / record callers on different threads never use
+    /// the non-thread-safe <see cref="SqliteConnection"/> concurrently.
+    /// Returns <see langword="false"/> without invoking the action if the logger has
+    /// been disposed.
+    /// </summary>
+    internal bool WithConnection(Action<SqliteConnection> action)
+    {
+        ArgumentNullException.ThrowIfNull(action);
+        lock (_writeLock)
+        {
+            if (_disposed) return false;
+            action(_connection);
+            return true;
+        }
+    }
+
+    /// <summary>
+    /// Runs <paramref name="func"/> against the shared connection while holding the
+    /// write lock. Throws <see cref="ObjectDisposedException"/> if the logger has
+    /// been disposed (callers that need a result cannot proceed without the database).
+    /// </summary>
+    internal T WithConnection<T>(Func<SqliteConnection, T> func)
+    {
+        ArgumentNullException.ThrowIfNull(func);
+        lock (_writeLock)
+        {
+            ObjectDisposedException.ThrowIf(_disposed, this);
+            return func(_connection);
+        }
+    }
 
     private void InitializeSchema()
     {
+        // auto_vacuum must be set before any table is created for it to take effect
+        // on a fresh database. With INCREMENTAL mode, PRAGMA incremental_vacuum can
+        // later reclaim free pages so the storage-cap purge can shrink the file.
+        using (var pragmaCmd = _connection.CreateCommand())
+        {
+            pragmaCmd.CommandText = "PRAGMA auto_vacuum = INCREMENTAL";
+            pragmaCmd.ExecuteNonQuery();
+        }
+
         using var cmd = _connection.CreateCommand();
         cmd.CommandText = """
             CREATE TABLE IF NOT EXISTS site_events (
@@ -69,13 +114,11 @@ public class SiteEventLogger : ISiteEventLogger, IDisposable
 
         var timestamp = DateTimeOffset.UtcNow.ToString("o");
 
-        lock (_writeLock)
-        {
-            if (_disposed) return Task.CompletedTask;
-
         try
         {
-                using var cmd = _connection.CreateCommand();
+            WithConnection(connection =>
+            {
+                using var cmd = connection.CreateCommand();
                 cmd.CommandText = """
                     INSERT INTO site_events (timestamp, event_type, severity, instance_id, source, message, details)
                     VALUES ($timestamp, $event_type, $severity, $instance_id, $source, $message, $details)
@@ -88,20 +131,23 @@ public class SiteEventLogger : ISiteEventLogger, IDisposable
                 cmd.Parameters.AddWithValue("$message", message);
                 cmd.Parameters.AddWithValue("$details", (object?)details ?? DBNull.Value);
                 cmd.ExecuteNonQuery();
+            });
         }
         catch (Exception ex)
         {
             _logger.LogError(ex, "Failed to record event: {EventType} from {Source}", eventType, source);
         }
-        }
 
         return Task.CompletedTask;
     }
 
     public void Dispose()
+    {
+        lock (_writeLock)
         {
             if (_disposed) return;
             _disposed = true;
             _connection.Dispose();
         }
     }
+}

tests/ScadaLink.InboundAPI.Tests/ApiKeyValidatorTests.cs

@@ -37,7 +37,7 @@ public class ApiKeyValidatorTests
     [Fact]
     public async Task InvalidApiKey_Returns401()
     {
-        _repository.GetApiKeyByValueAsync("bad-key").Returns((ApiKey?)null);
+        _repository.GetAllApiKeysAsync().Returns(new List<ApiKey>());
 
         var result = await _validator.ValidateAsync("bad-key", "testMethod");
         Assert.False(result.IsValid);
@@ -48,7 +48,7 @@ public class ApiKeyValidatorTests
     public async Task DisabledApiKey_Returns401()
     {
         var key = new ApiKey("test", "valid-key") { Id = 1, IsEnabled = false };
-        _repository.GetApiKeyByValueAsync("valid-key").Returns(key);
+        _repository.GetAllApiKeysAsync().Returns(new List<ApiKey> { key });
 
         var result = await _validator.ValidateAsync("valid-key", "testMethod");
         Assert.False(result.IsValid);
@@ -59,7 +59,7 @@ public class ApiKeyValidatorTests
     public async Task ValidKey_MethodNotFound_Returns400()
     {
         var key = new ApiKey("test", "valid-key") { Id = 1, IsEnabled = true };
-        _repository.GetApiKeyByValueAsync("valid-key").Returns(key);
+        _repository.GetAllApiKeysAsync().Returns(new List<ApiKey> { key });
         _repository.GetMethodByNameAsync("nonExistent").Returns((ApiMethod?)null);
 
         var result = await _validator.ValidateAsync("valid-key", "nonExistent");
@@ -73,7 +73,7 @@ public class ApiKeyValidatorTests
         var key = new ApiKey("test", "valid-key") { Id = 1, IsEnabled = true };
         var method = new ApiMethod("testMethod", "return 1;") { Id = 10 };
 
-        _repository.GetApiKeyByValueAsync("valid-key").Returns(key);
+        _repository.GetAllApiKeysAsync().Returns(new List<ApiKey> { key });
         _repository.GetMethodByNameAsync("testMethod").Returns(method);
         _repository.GetApprovedKeysForMethodAsync(10).Returns(new List<ApiKey>());
 
@@ -88,7 +88,7 @@ public class ApiKeyValidatorTests
         var key = new ApiKey("test", "valid-key") { Id = 1, IsEnabled = true };
         var method = new ApiMethod("testMethod", "return 1;") { Id = 10 };
 
-        _repository.GetApiKeyByValueAsync("valid-key").Returns(key);
+        _repository.GetAllApiKeysAsync().Returns(new List<ApiKey> { key });
         _repository.GetMethodByNameAsync("testMethod").Returns(method);
         _repository.GetApprovedKeysForMethodAsync(10).Returns(new List<ApiKey> { key });
 
@@ -98,4 +98,50 @@ public class ApiKeyValidatorTests
         Assert.Equal(key, result.ApiKey);
         Assert.Equal(method, result.Method);
     }
+
+    // --- InboundAPI-003: API key must not be matched with a non-constant-time
+    // (timing-oracle) secret-equality lookup. ---
+
+    [Fact]
+    public async Task ValidateAsync_DoesNotUseSecretEqualityLookup()
+    {
+        // GetApiKeyByValueAsync translates to a SQL "WHERE KeyValue = @secret" early-exit
+        // comparison — a timing side-channel. The validator must not call it.
+        var key = new ApiKey("test", "valid-key") { Id = 1, IsEnabled = true };
+        var method = new ApiMethod("testMethod", "return 1;") { Id = 10 };
+
+        _repository.GetAllApiKeysAsync().Returns(new List<ApiKey> { key });
+        _repository.GetMethodByNameAsync("testMethod").Returns(method);
+        _repository.GetApprovedKeysForMethodAsync(10).Returns(new List<ApiKey> { key });
+
+        await _validator.ValidateAsync("valid-key", "testMethod");
+
+        await _repository.DidNotReceive()
+            .GetApiKeyByValueAsync(Arg.Any<string>(), Arg.Any<CancellationToken>());
+    }
+
+    [Fact]
+    public async Task ValidateAsync_WrongKey_ConstantTimePath_Returns401()
+    {
+        var key = new ApiKey("test", "valid-key") { Id = 1, IsEnabled = true };
+        _repository.GetAllApiKeysAsync().Returns(new List<ApiKey> { key });
+
+        var result = await _validator.ValidateAsync("wrong-key", "testMethod");
+
+        Assert.False(result.IsValid);
+        Assert.Equal(401, result.StatusCode);
+    }
+
+    [Fact]
+    public async Task ValidateAsync_KeyOfDifferentLength_Returns401()
+    {
+        // FixedTimeEquals over UTF-8 bytes must reject length mismatches without leaking.
+        var key = new ApiKey("test", "valid-key") { Id = 1, IsEnabled = true };
+        _repository.GetAllApiKeysAsync().Returns(new List<ApiKey> { key });
+
+        var result = await _validator.ValidateAsync("valid-key-with-extra", "testMethod");
+
+        Assert.False(result.IsValid);
+        Assert.Equal(401, result.StatusCode);
+    }
 }

tests/ScadaLink.InboundAPI.Tests/InboundScriptExecutorTests.cs

@@ -132,4 +132,111 @@ public class InboundScriptExecutorTests
         Assert.True(result.Success);
         Assert.Contains("ScadaLink", result.ResultJson!);
     }
+
+    // --- InboundAPI-001: concurrent lazy-compile must not corrupt the handler cache ---
+
+    [Fact]
+    public async Task ConcurrentLazyCompile_SameMethod_DoesNotCorruptCache()
+    {
+        // Many concurrent first-callers of an uncompiled method race the lazy-compile
+        // path. With an unsynchronized Dictionary this can throw or return a torn/null
+        // handler; all calls must succeed and produce the same result.
+        var method = new ApiMethod("concurrent", "return 7;") { Id = 1, TimeoutSeconds = 10 };
+
+        var tasks = Enumerable.Range(0, 64).Select(_ => Task.Run(() =>
+            _executor.ExecuteAsync(
+                method,
+                new Dictionary<string, object?>(),
+                _route,
+                TimeSpan.FromSeconds(10)))).ToArray();
+
+        var results = await Task.WhenAll(tasks);
+
+        Assert.All(results, r =>
+        {
+            Assert.True(r.Success, r.ErrorMessage);
+            Assert.Equal("7", r.ResultJson);
+        });
+    }
+
+    [Fact]
+    public async Task ConcurrentRegisterAndExecute_DoesNotThrow()
+    {
+        // RegisterHandler/RemoveHandler racing ExecuteAsync must not crash the process
+        // with an InvalidOperationException from concurrent Dictionary mutation.
+        var method = new ApiMethod("racy", "return 1;") { Id = 1, TimeoutSeconds = 10 };
+
+        var writers = Enumerable.Range(0, 32).Select(i => Task.Run(() =>
+        {
+            for (var n = 0; n < 50; n++)
+            {
+                _executor.RegisterHandler("racy", async ctx => { await Task.CompletedTask; return i; });
+                _executor.RemoveHandler("racy");
+            }
+        }));
+
+        var readers = Enumerable.Range(0, 32).Select(_ => Task.Run(async () =>
+        {
+            for (var n = 0; n < 50; n++)
+            {
+                await _executor.ExecuteAsync(
+                    method, new Dictionary<string, object?>(), _route, TimeSpan.FromSeconds(10));
+            }
+        }));
+
+        // Should complete without an unhandled concurrency exception.
+        await Task.WhenAll(writers.Concat(readers));
+    }
+
+    // --- InboundAPI-005: compiled scripts must not bypass the script trust model ---
+
+    [Theory]
+    [InlineData("System.IO.File.Delete(\"/tmp/x\"); return null;")]
+    [InlineData("System.Diagnostics.Process.Start(\"/bin/sh\"); return null;")]
+    [InlineData("var t = System.Reflection.Assembly.GetExecutingAssembly(); return null;")]
+    [InlineData("new System.Threading.Thread(() => {}).Start(); return null;")]
+    [InlineData("var s = new System.Net.Sockets.Socket(System.Net.Sockets.AddressFamily.InterNetwork, System.Net.Sockets.SocketType.Stream, System.Net.Sockets.ProtocolType.Tcp); return null;")]
+    public void CompileAndRegister_ForbiddenApi_RejectsScript(string script)
+    {
+        var method = new ApiMethod("forbidden", script) { Id = 1, TimeoutSeconds = 10 };
+
+        var registered = _executor.CompileAndRegister(method);
+
+        Assert.False(registered);
+    }
+
+    [Fact]
+    public async Task ExecuteAsync_ForbiddenApiScript_DoesNotRunAndReturnsFailure()
+    {
+        // A fully-qualified forbidden API call must be rejected at compile/register time
+        // so the script never executes.
+        var marker = System.IO.Path.Combine(
+            System.IO.Path.GetTempPath(), $"scadalink-pwned-{Guid.NewGuid():N}");
+        System.IO.File.Delete(marker);
+
+        var method = new ApiMethod("evil", $"System.IO.File.WriteAllText(@\"{marker}\", \"x\"); return 1;")
+        {
+            Id = 1,
+            TimeoutSeconds = 10
+        };
+
+        var result = await _executor.ExecuteAsync(
+            method, new Dictionary<string, object?>(), _route, TimeSpan.FromSeconds(10));
+
+        Assert.False(result.Success);
+        Assert.False(System.IO.File.Exists(marker));
+    }
+
+    [Fact]
+    public void CompileAndRegister_PermittedScript_StillRegisters()
+    {
+        // The trust-model check must not reject legitimate scripts.
+        var method = new ApiMethod("ok", "var list = new List<int> { 1, 2, 3 }; return list.Sum();")
+        {
+            Id = 1,
+            TimeoutSeconds = 10
+        };
+
+        Assert.True(_executor.CompileAndRegister(method));
+    }
 }

tests/ScadaLink.ManagementService.Tests/DebugStreamHubTests.cs

@@ -0,0 +1,66 @@
+using ScadaLink.ManagementService;
+
+namespace ScadaLink.ManagementService.Tests;
+
+/// <summary>
+/// Tests for <see cref="DebugStreamHub"/> per-instance site-scope authorization
+/// (finding ManagementService-003).
+/// </summary>
+public class DebugStreamHubTests
+{
+    [Fact]
+    public void IsInstanceAccessAllowed_SiteScopedUser_InScopeInstance_Allowed()
+    {
+        var allowed = DebugStreamHub.IsInstanceAccessAllowed(
+            roles: new[] { "Deployment" },
+            permittedSiteIds: new[] { "1", "2" },
+            instanceSiteId: 2);
+
+        Assert.True(allowed);
+    }
+
+    [Fact]
+    public void IsInstanceAccessAllowed_SiteScopedUser_OutOfScopeInstance_Denied()
+    {
+        var allowed = DebugStreamHub.IsInstanceAccessAllowed(
+            roles: new[] { "Deployment" },
+            permittedSiteIds: new[] { "1", "2" },
+            instanceSiteId: 99);
+
+        Assert.False(allowed);
+    }
+
+    [Fact]
+    public void IsInstanceAccessAllowed_SystemWideDeployment_AnySiteAllowed()
+    {
+        // Empty permitted set == system-wide Deployment.
+        var allowed = DebugStreamHub.IsInstanceAccessAllowed(
+            roles: new[] { "Deployment" },
+            permittedSiteIds: Array.Empty<string>(),
+            instanceSiteId: 99);
+
+        Assert.True(allowed);
+    }
+
+    [Fact]
+    public void IsInstanceAccessAllowed_AdminRole_BypassesSiteScope()
+    {
+        var allowed = DebugStreamHub.IsInstanceAccessAllowed(
+            roles: new[] { "Admin", "Deployment" },
+            permittedSiteIds: new[] { "1" },
+            instanceSiteId: 99);
+
+        Assert.True(allowed);
+    }
+
+    [Fact]
+    public void IsInstanceAccessAllowed_AdminRoleCheck_IsCaseInsensitive()
+    {
+        var allowed = DebugStreamHub.IsInstanceAccessAllowed(
+            roles: new[] { "admin" },
+            permittedSiteIds: new[] { "1" },
+            instanceSiteId: 99);
+
+        Assert.True(allowed);
+    }
+}

tests/ScadaLink.ManagementService.Tests/ManagementActorTests.cs

@@ -44,6 +44,10 @@ public class ManagementActorTests : TestKit, IDisposable
         new(new AuthenticatedUser("testuser", "Test User", roles, Array.Empty<string>()),
             command, Guid.NewGuid().ToString("N"));
 
+    private static ManagementEnvelope ScopedEnvelope(object command, string[] permittedSiteIds, params string[] roles) =>
+        new(new AuthenticatedUser("scopeduser", "Scoped User", roles, permittedSiteIds),
+            command, Guid.NewGuid().ToString("N"));
+
     void IDisposable.Dispose()
     {
         Shutdown();
@@ -478,4 +482,190 @@ public class ManagementActorTests : TestKit, IDisposable
         Assert.Equal("COMMAND_FAILED", response.ErrorCode);
         Assert.Contains("Connection refused", response.Error);
     }
+
+    // ========================================================================
+    // Site-scope enforcement tests (findings ManagementService-001 / -002)
+    //
+    // A site-scoped Deployment user (PermittedSiteIds set, no Admin role) must
+    // not be able to read or act on entities belonging to a site outside their
+    // scope. The handlers must throw SiteScopeViolationException, which the
+    // actor maps to ManagementUnauthorized.
+    // ========================================================================
+
+    private void AddSiteRepoWithSite(int siteId, string identifier)
+    {
+        var siteRepo = Substitute.For<ISiteRepository>();
+        siteRepo.GetSiteByIdAsync(siteId, Arg.Any<CancellationToken>())
+            .Returns(new Commons.Entities.Sites.Site($"Site{siteId}", identifier) { Id = siteId });
+        siteRepo.GetSiteByIdentifierAsync(identifier, Arg.Any<CancellationToken>())
+            .Returns(new Commons.Entities.Sites.Site($"Site{siteId}", identifier) { Id = siteId });
+        _services.AddScoped(_ => siteRepo);
+    }
+
+    [Fact]
+    public void GetInstance_OutOfScopeForSiteScopedUser_ReturnsUnauthorized()
+    {
+        // Instance belongs to site 2; user is scoped to site 1.
+        _templateRepo.GetInstanceByIdAsync(7, Arg.Any<CancellationToken>())
+            .Returns(new Instance("Pump7") { Id = 7, SiteId = 2 });
+
+        var actor = CreateActor();
+        var envelope = ScopedEnvelope(new GetInstanceCommand(7), new[] { "1" }, "Deployment");
+
+        actor.Tell(envelope);
+
+        var response = ExpectMsg<ManagementUnauthorized>(TimeSpan.FromSeconds(5));
+        Assert.Equal(envelope.CorrelationId, response.CorrelationId);
+    }
+
+    [Fact]
+    public void GetInstance_InScopeForSiteScopedUser_ReturnsSuccess()
+    {
+        _templateRepo.GetInstanceByIdAsync(7, Arg.Any<CancellationToken>())
+            .Returns(new Instance("Pump7") { Id = 7, SiteId = 1 });
+
+        var actor = CreateActor();
+        var envelope = ScopedEnvelope(new GetInstanceCommand(7), new[] { "1" }, "Deployment");
+
+        actor.Tell(envelope);
+
+        ExpectMsg<ManagementSuccess>(TimeSpan.FromSeconds(5));
+    }
+
+    [Fact]
+    public void GetSite_OutOfScopeForSiteScopedUser_ReturnsUnauthorized()
+    {
+        AddSiteRepoWithSite(2, "SITE2");
+
+        var actor = CreateActor();
+        var envelope = ScopedEnvelope(new GetSiteCommand(2), new[] { "1" }, "Deployment");
+
+        actor.Tell(envelope);
+
+        var response = ExpectMsg<ManagementUnauthorized>(TimeSpan.FromSeconds(5));
+        Assert.Equal(envelope.CorrelationId, response.CorrelationId);
+    }
+
+    [Fact]
+    public void ListAreas_OutOfScopeForSiteScopedUser_ReturnsUnauthorized()
+    {
+        AddSiteRepoWithSite(2, "SITE2");
+        var uiRepo = Substitute.For<ICentralUiRepository>();
+        uiRepo.GetAreaTreeBySiteIdAsync(2, Arg.Any<CancellationToken>())
+            .Returns(new List<Commons.Entities.Instances.Area>());
+        _services.AddScoped(_ => uiRepo);
+
+        var actor = CreateActor();
+        var envelope = ScopedEnvelope(new ListAreasCommand(2), new[] { "1" }, "Deployment");
+
+        actor.Tell(envelope);
+
+        var response = ExpectMsg<ManagementUnauthorized>(TimeSpan.FromSeconds(5));
+        Assert.Equal(envelope.CorrelationId, response.CorrelationId);
+    }
+
+    [Fact]
+    public void GetDataConnection_OutOfScopeForSiteScopedUser_ReturnsUnauthorized()
+    {
+        var siteRepo = Substitute.For<ISiteRepository>();
+        siteRepo.GetDataConnectionByIdAsync(5, Arg.Any<CancellationToken>())
+            .Returns(new Commons.Entities.Sites.DataConnection("Conn5", "OpcUa", 2) { Id = 5 });
+        _services.AddScoped(_ => siteRepo);
+
+        var actor = CreateActor();
+        var envelope = ScopedEnvelope(new GetDataConnectionCommand(5), new[] { "1" }, "Deployment");
+
+        actor.Tell(envelope);
+
+        var response = ExpectMsg<ManagementUnauthorized>(TimeSpan.FromSeconds(5));
+        Assert.Equal(envelope.CorrelationId, response.CorrelationId);
+    }
+
+    [Fact]
+    public void GetSite_OutOfScopeForAdminUser_ReturnsSuccess()
+    {
+        // Admin role bypasses site scoping even when PermittedSiteIds is set.
+        AddSiteRepoWithSite(2, "SITE2");
+
+        var actor = CreateActor();
+        var envelope = ScopedEnvelope(new GetSiteCommand(2), new[] { "1" }, "Admin");
+
+        actor.Tell(envelope);
+
+        ExpectMsg<ManagementSuccess>(TimeSpan.FromSeconds(5));
+    }
+
+    [Fact]
+    public void QueryEventLogs_OutOfScopeForSiteScopedUser_ReturnsUnauthorized()
+    {
+        // Site SITE2 has Id 2; user scoped to site 1.
+        AddSiteRepoWithSite(2, "SITE2");
+
+        var actor = CreateActor();
+        var envelope = ScopedEnvelope(new QueryEventLogsCommand("SITE2"), new[] { "1" }, "Deployment");
+
+        actor.Tell(envelope);
+
+        var response = ExpectMsg<ManagementUnauthorized>(TimeSpan.FromSeconds(5));
+        Assert.Equal(envelope.CorrelationId, response.CorrelationId);
+    }
+
+    [Fact]
+    public void QueryParkedMessages_OutOfScopeForSiteScopedUser_ReturnsUnauthorized()
+    {
+        AddSiteRepoWithSite(2, "SITE2");
+
+        var actor = CreateActor();
+        var envelope = ScopedEnvelope(new QueryParkedMessagesCommand("SITE2"), new[] { "1" }, "Deployment");
+
+        actor.Tell(envelope);
+
+        var response = ExpectMsg<ManagementUnauthorized>(TimeSpan.FromSeconds(5));
+        Assert.Equal(envelope.CorrelationId, response.CorrelationId);
+    }
+
+    [Fact]
+    public void RetryParkedMessage_OutOfScopeForSiteScopedUser_ReturnsUnauthorized()
+    {
+        AddSiteRepoWithSite(2, "SITE2");
+
+        var actor = CreateActor();
+        var envelope = ScopedEnvelope(new RetryParkedMessageCommand("SITE2", "msg-1"), new[] { "1" }, "Deployment");
+
+        actor.Tell(envelope);
+
+        var response = ExpectMsg<ManagementUnauthorized>(TimeSpan.FromSeconds(5));
+        Assert.Equal(envelope.CorrelationId, response.CorrelationId);
+    }
+
+    [Fact]
+    public void DiscardParkedMessage_OutOfScopeForSiteScopedUser_ReturnsUnauthorized()
+    {
+        AddSiteRepoWithSite(2, "SITE2");
+
+        var actor = CreateActor();
+        var envelope = ScopedEnvelope(new DiscardParkedMessageCommand("SITE2", "msg-1"), new[] { "1" }, "Deployment");
+
+        actor.Tell(envelope);
+
+        var response = ExpectMsg<ManagementUnauthorized>(TimeSpan.FromSeconds(5));
+        Assert.Equal(envelope.CorrelationId, response.CorrelationId);
+    }
+
+    [Fact]
+    public void DebugSnapshot_OutOfScopeForSiteScopedUser_ReturnsUnauthorized()
+    {
+        // Instance 9 belongs to site 2; user scoped to site 1.
+        _templateRepo.GetInstanceByIdAsync(9, Arg.Any<CancellationToken>())
+            .Returns(new Instance("Pump9") { Id = 9, SiteId = 2 });
+        AddSiteRepoWithSite(2, "SITE2");
+
+        var actor = CreateActor();
+        var envelope = ScopedEnvelope(new DebugSnapshotCommand(9), new[] { "1" }, "Deployment");
+
+        actor.Tell(envelope);
+
+        var response = ExpectMsg<ManagementUnauthorized>(TimeSpan.FromSeconds(5));
+        Assert.Equal(envelope.CorrelationId, response.CorrelationId);
+    }
 }

tests/ScadaLink.NotificationService.Tests/NotificationDeliveryServiceTests.cs

@@ -1,3 +1,5 @@
+using MailKit;
+using MailKit.Net.Smtp;
 using Microsoft.Extensions.Logging.Abstractions;
 using NSubstitute;
 using NSubstitute.ExceptionExtensions;
@@ -225,4 +227,182 @@ public class NotificationDeliveryServiceTests
 
         Assert.False(delivered); // permanent — the S&F engine parks the message
     }
+
+    // ── NotificationService-002: cancellation must not be misclassified as transient ──
+
+    /// <summary>
+    /// Like <see cref="SetupHappyPath"/> but matches any <see cref="CancellationToken"/>,
+    /// so tests that pass an already-cancelled token still resolve the list/recipients.
+    /// </summary>
+    private void SetupHappyPathAnyToken()
+    {
+        var list = new NotificationList("ops-team") { Id = 1 };
+        var recipients = new List<NotificationRecipient>
+        {
+            new("Alice", "alice@example.com") { Id = 1, NotificationListId = 1 }
+        };
+        var smtpConfig = new SmtpConfiguration("smtp.example.com", "basic", "noreply@example.com")
+        {
+            Id = 1, Port = 587, Credentials = "user:pass", TlsMode = "starttls"
+        };
+
+        _repository.GetListByNameAsync("ops-team", Arg.Any<CancellationToken>()).Returns(list);
+        _repository.GetRecipientsByListIdAsync(1, Arg.Any<CancellationToken>()).Returns(recipients);
+        _repository.GetAllSmtpConfigurationsAsync(Arg.Any<CancellationToken>())
+            .Returns(new List<SmtpConfiguration> { smtpConfig });
+    }
+
+    [Fact]
+    public async Task Send_CancellationRequested_PropagatesAndDoesNotBuffer()
+    {
+        SetupHappyPathAnyToken();
+        using var cts = new CancellationTokenSource();
+        cts.Cancel();
+        _smtpClient.SendAsync(Arg.Any<string>(), Arg.Any<IEnumerable<string>>(), Arg.Any<string>(), Arg.Any<string>(), Arg.Any<CancellationToken>())
+            .Throws(new OperationCanceledException(cts.Token));
+
+        var sfService = await CreateSfServiceAsync();
+        var service = CreateService(sf: sfService);
+
+        await Assert.ThrowsAnyAsync<OperationCanceledException>(
+            () => service.SendAsync("ops-team", "Alert", "Body", cancellationToken: cts.Token));
+
+        // The cancellation propagated instead of being buffered for retry.
+        var depth = await sfService.GetBufferDepthAsync();
+        depth.TryGetValue(ScadaLink.Commons.Types.Enums.StoreAndForwardCategory.Notification, out var count);
+        Assert.Equal(0, count);
+    }
+
+    [Fact]
+    public async Task Send_TaskCanceledException_WithCancellation_Propagates()
+    {
+        SetupHappyPathAnyToken();
+        using var cts = new CancellationTokenSource();
+        cts.Cancel();
+        _smtpClient.SendAsync(Arg.Any<string>(), Arg.Any<IEnumerable<string>>(), Arg.Any<string>(), Arg.Any<string>(), Arg.Any<CancellationToken>())
+            .Throws(new TaskCanceledException());
+
+        var service = CreateService(sf: null);
+
+        await Assert.ThrowsAnyAsync<OperationCanceledException>(
+            () => service.SendAsync("ops-team", "Alert", "Body", cancellationToken: cts.Token));
+    }
+
+    // ── NotificationService-003: classify on MailKit typed exceptions / status codes ──
+
+    [Fact]
+    public async Task Send_Smtp5xxCommandException_ClassifiedPermanent()
+    {
+        SetupHappyPath();
+        // 550 MailboxUnavailable — a real permanent rejection.
+        _smtpClient.SendAsync(Arg.Any<string>(), Arg.Any<IEnumerable<string>>(), Arg.Any<string>(), Arg.Any<string>(), Arg.Any<CancellationToken>())
+            .Throws(new SmtpCommandException(
+                SmtpErrorCode.RecipientNotAccepted, SmtpStatusCode.MailboxUnavailable, "rejected"));
+
+        var service = CreateService();
+        var result = await service.SendAsync("ops-team", "Alert", "Body");
+
+        Assert.False(result.Success);
+        Assert.Contains("Permanent SMTP error", result.ErrorMessage);
+    }
+
+    [Fact]
+    public async Task Send_Smtp4xxCommandException_ClassifiedTransientAndBuffered()
+    {
+        SetupHappyPath();
+        // 450 MailboxBusy — a real transient failure.
+        _smtpClient.SendAsync(Arg.Any<string>(), Arg.Any<IEnumerable<string>>(), Arg.Any<string>(), Arg.Any<string>(), Arg.Any<CancellationToken>())
+            .Throws(new SmtpCommandException(
+                SmtpErrorCode.MessageNotAccepted, SmtpStatusCode.MailboxBusy, "try again"));
+
+        var sfService = await CreateSfServiceAsync();
+        var service = CreateService(sf: sfService);
+        var result = await service.SendAsync("ops-team", "Alert", "Body");
+
+        Assert.True(result.Success);
+        Assert.True(result.WasBuffered);
+    }
+
+    [Fact]
+    public async Task Send_NonSmtpExceptionWith5xxLookalikeText_NotPromotedToPermanent()
+    {
+        // NS-003: the old classifier promoted ANY exception whose message contained
+        // "5." / "550" / etc. to a permanent SMTP error — so an unrelated failure
+        // referencing a host like "smtp5.example.com" was silently swallowed as a
+        // clean permanent NotificationResult. Classification now uses MailKit's
+        // typed exceptions only, so a non-SMTP exception is no longer misclassified.
+        SetupHappyPath();
+        _smtpClient.SendAsync(Arg.Any<string>(), Arg.Any<IEnumerable<string>>(), Arg.Any<string>(), Arg.Any<string>(), Arg.Any<CancellationToken>())
+            .Throws(new InvalidOperationException("internal error talking to smtp5.example.com"));
+
+        var service = CreateService();
+
+        // The exception is not classified at all (not a typed SMTP failure); it
+        // surfaces rather than being mistaken for a permanent 5xx rejection.
+        await Assert.ThrowsAsync<InvalidOperationException>(
+            () => service.SendAsync("ops-team", "Alert", "Body"));
+    }
+
+    [Fact]
+    public async Task Send_SmtpProtocolException_ClassifiedTransient()
+    {
+        SetupHappyPath();
+        _smtpClient.SendAsync(Arg.Any<string>(), Arg.Any<IEnumerable<string>>(), Arg.Any<string>(), Arg.Any<string>(), Arg.Any<CancellationToken>())
+            .Throws(new SmtpProtocolException("protocol error"));
+
+        var sfService = await CreateSfServiceAsync();
+        var service = CreateService(sf: sfService);
+        var result = await service.SendAsync("ops-team", "Alert", "Body");
+
+        Assert.True(result.Success);
+        Assert.True(result.WasBuffered);
+    }
+
+    // ── NotificationService-004: DeliverAsync must create exactly one client and dispose it ──
+
+    private sealed class TrackingSmtpClient : ISmtpClientWrapper, IDisposable
+    {
+        public bool Disposed { get; private set; }
+        public Task ConnectAsync(string host, int port, bool useTls, CancellationToken cancellationToken = default)
+            => Task.CompletedTask;
+        public Task AuthenticateAsync(string authType, string? credentials, CancellationToken cancellationToken = default)
+            => Task.CompletedTask;
+        public Task SendAsync(string from, IEnumerable<string> bccRecipients, string subject, string body, CancellationToken cancellationToken = default)
+            => Task.CompletedTask;
+        public Task DisconnectAsync(CancellationToken cancellationToken = default)
+            => Task.CompletedTask;
+        public void Dispose() => Disposed = true;
+    }
+
+    [Fact]
+    public async Task Send_CreatesExactlyOneSmtpClient_AndDisposesIt()
+    {
+        SetupHappyPath();
+        var created = new List<TrackingSmtpClient>();
+        var service = new NotificationDeliveryService(
+            _repository,
+            () =>
+            {
+                var c = new TrackingSmtpClient();
+                created.Add(c);
+                return c;
+            },
+            NullLogger<NotificationDeliveryService>.Instance);
+
+        var result = await service.SendAsync("ops-team", "Alert", "Body");
+
+        Assert.True(result.Success);
+        Assert.Single(created);                 // NS-004: factory invoked once, not twice
+        Assert.True(created[0].Disposed);       // the client actually used is disposed
+    }
+
+    private static async Task<StoreAndForwardService> CreateSfServiceAsync()
+    {
+        var dbName = $"file:sf_test_{Guid.NewGuid():N}?mode=memory&cache=shared";
+        var storage = new StoreAndForwardStorage(
+            $"Data Source={dbName}", NullLogger<StoreAndForwardStorage>.Instance);
+        await storage.InitializeAsync();
+        return new StoreAndForwardService(
+            storage, new StoreAndForwardOptions(), NullLogger<StoreAndForwardService>.Instance);
+    }
 }

tests/ScadaLink.NotificationService.Tests/ScadaLink.NotificationService.Tests.csproj

@@ -10,6 +10,7 @@
 
   <ItemGroup>
     <PackageReference Include="coverlet.collector" />
+    <PackageReference Include="MailKit" />
     <PackageReference Include="Microsoft.NET.Test.Sdk" />
     <PackageReference Include="NSubstitute" />
     <PackageReference Include="xunit" />

tests/ScadaLink.Security.Tests/UnitTest1.cs

@@ -357,6 +357,124 @@ public class RoleMapperTests : IDisposable
 
 #endregion
 
+#region Code Review Regression Tests
+
+/// <summary>
+/// Regression tests for Security-001 (StartTLS dead code), Security-002 (cookie not
+/// marked Secure), and Security-003 (JWT signing key length never validated).
+/// </summary>
+public class SecurityReviewRegressionTests
+{
+    // --- Security-003: JWT signing key length validation ---
+
+    private static SecurityOptions JwtOptions(string signingKey) => new()
+    {
+        JwtSigningKey = signingKey,
+        JwtExpiryMinutes = 15,
+        IdleTimeoutMinutes = 30,
+        JwtRefreshThresholdMinutes = 5
+    };
+
+    [Fact]
+    public void JwtTokenService_EmptySigningKey_ThrowsAtConstruction()
+    {
+        var ex = Assert.Throws<InvalidOperationException>(() =>
+            new JwtTokenService(Options.Create(JwtOptions("")), NullLogger<JwtTokenService>.Instance));
+        Assert.Contains("JwtSigningKey", ex.Message);
+    }
+
+    [Fact]
+    public void JwtTokenService_ShortSigningKey_ThrowsAtConstruction()
+    {
+        // 31 bytes — one short of the 256-bit HMAC-SHA256 minimum.
+        var shortKey = new string('k', 31);
+        var ex = Assert.Throws<InvalidOperationException>(() =>
+            new JwtTokenService(Options.Create(JwtOptions(shortKey)), NullLogger<JwtTokenService>.Instance));
+        Assert.Contains("32", ex.Message);
+    }
+
+    [Fact]
+    public void JwtTokenService_AdequateSigningKey_ConstructsSuccessfully()
+    {
+        var key = new string('k', 32);
+        var service = new JwtTokenService(Options.Create(JwtOptions(key)), NullLogger<JwtTokenService>.Instance);
+        var token = service.GenerateToken("User", "user", new[] { "Admin" }, null);
+        Assert.False(string.IsNullOrEmpty(token));
+    }
+
+    // --- Security-002: authentication cookie must be marked Secure ---
+
+    [Fact]
+    public void AddSecurity_AuthCookie_IsMarkedSecureAlways()
+    {
+        var services = new ServiceCollection();
+        services.AddLogging();
+        services.AddDataProtection();
+        services.AddSecurity();
+
+        using var provider = services.BuildServiceProvider();
+        var cookieOptions = provider
+            .GetRequiredService<IOptionsMonitor<Microsoft.AspNetCore.Authentication.Cookies.CookieAuthenticationOptions>>()
+            .Get(Microsoft.AspNetCore.Authentication.Cookies.CookieAuthenticationDefaults.AuthenticationScheme);
+
+        Assert.Equal(
+            Microsoft.AspNetCore.Http.CookieSecurePolicy.Always,
+            cookieOptions.Cookie.SecurePolicy);
+        Assert.True(cookieOptions.Cookie.HttpOnly);
+    }
+
+    // --- Security-001: StartTLS transport must be reachable ---
+
+    [Fact]
+    public void SecurityOptions_LdapTransport_DefaultsToLdaps()
+    {
+        var options = new SecurityOptions();
+        Assert.Equal(LdapTransport.Ldaps, options.LdapTransport);
+    }
+
+    [Fact]
+    public async Task AuthenticateAsync_StartTlsTransport_AttemptsConnection()
+    {
+        // With StartTLS selected the service must not be blocked by the insecure-LDAP
+        // guard and must reach the connection stage (which fails against a dead host).
+        // This proves the StartTLS path is reachable rather than dead code.
+        var options = new SecurityOptions
+        {
+            LdapServer = "nonexistent.invalid",
+            LdapPort = 389,
+            LdapTransport = LdapTransport.StartTls,
+            LdapSearchBase = "dc=example,dc=com"
+        };
+        var service = new LdapAuthService(Options.Create(options), NullLogger<LdapAuthService>.Instance);
+
+        var result = await service.AuthenticateAsync("user", "password");
+
+        // Connection fails (host invalid) — but crucially NOT with the insecure-LDAP message.
+        Assert.False(result.Success);
+        Assert.DoesNotContain("Insecure LDAP", result.ErrorMessage ?? string.Empty);
+    }
+
+    [Fact]
+    public async Task AuthenticateAsync_NoTlsTransport_RejectedWithoutAllowInsecure()
+    {
+        var options = new SecurityOptions
+        {
+            LdapServer = "ldap.example.com",
+            LdapPort = 389,
+            LdapTransport = LdapTransport.None,
+            AllowInsecureLdap = false
+        };
+        var service = new LdapAuthService(Options.Create(options), NullLogger<LdapAuthService>.Instance);
+
+        var result = await service.AuthenticateAsync("user", "password");
+
+        Assert.False(result.Success);
+        Assert.Contains("Insecure LDAP", result.ErrorMessage);
+    }
+}
+
+#endregion
+
 #region WP-9: Authorization Policy Tests
 
 public class AuthorizationPolicyTests

tests/ScadaLink.SiteEventLogging.Tests/EventLogPurgeServiceTests.cs

@@ -40,20 +40,26 @@ public class EventLogPurgeServiceTests : IDisposable
 
     private void InsertEventWithTimestamp(DateTimeOffset timestamp)
     {
-        using var cmd = _eventLogger.Connection.CreateCommand();
+        _eventLogger.WithConnection(connection =>
+        {
+            using var cmd = connection.CreateCommand();
             cmd.CommandText = """
                 INSERT INTO site_events (timestamp, event_type, severity, source, message)
                 VALUES ($ts, 'script', 'Info', 'Test', 'Test message')
                 """;
             cmd.Parameters.AddWithValue("$ts", timestamp.ToString("o"));
             cmd.ExecuteNonQuery();
+        });
     }
 
     private long GetEventCount()
     {
-        using var cmd = _eventLogger.Connection.CreateCommand();
+        return _eventLogger.WithConnection(connection =>
+        {
+            using var cmd = connection.CreateCommand();
             cmd.CommandText = "SELECT COUNT(*) FROM site_events";
             return (long)cmd.ExecuteScalar()!;
+        });
     }
 
     [Fact]
@@ -116,4 +122,155 @@ public class EventLogPurgeServiceTests : IDisposable
 
         Assert.True(size > 0);
     }
+
+    private void InsertBulkEvents(int count)
+    {
+        // Each event carries a sizeable details payload so the database grows
+        // measurably and the storage cap can be exercised against a realistic file.
+        var details = new string('x', 2000);
+        _eventLogger.WithConnection(connection =>
+        {
+            for (int i = 0; i < count; i++)
+            {
+                using var cmd = connection.CreateCommand();
+                cmd.CommandText = """
+                    INSERT INTO site_events (timestamp, event_type, severity, source, message, details)
+                    VALUES ($ts, 'script', 'Info', 'Test', 'Bulk event', $details)
+                    """;
+                cmd.Parameters.AddWithValue("$ts", DateTimeOffset.UtcNow.ToString("o"));
+                cmd.Parameters.AddWithValue("$details", details);
+                cmd.ExecuteNonQuery();
+            }
+        });
+    }
+
+    private long MinEventId()
+    {
+        return _eventLogger.WithConnection(connection =>
+        {
+            using var cmd = connection.CreateCommand();
+            cmd.CommandText = "SELECT MIN(id) FROM site_events";
+            var result = cmd.ExecuteScalar();
+            return result is long l ? l : 0;
+        });
+    }
+
+    [Fact]
+    public void PurgeByStorageCap_StopsWhenUnderCap_DoesNotEmptyTable()
+    {
+        // Regression test for SiteEventLogging-001 / -002:
+        // a realistic non-zero cap must trim the oldest events to the budget,
+        // not delete the entire table.
+        InsertBulkEvents(3000);
+
+        var purge = CreatePurgeService();
+        var totalSize = purge.GetDatabaseSizeBytes();
+
+        // Cap at roughly half the current database size — purge must keep some rows.
+        var capBytes = totalSize / 2;
+        var capOptions = new SiteEventLogOptions
+        {
+            DatabasePath = _dbPath,
+            RetentionDays = 30,
+            MaxStorageMb = (int)Math.Max(1, capBytes / (1024 * 1024))
+        };
+
+        var cappedPurge = CreatePurgeService(capOptions);
+        cappedPurge.RunPurge();
+
+        var remaining = GetEventCount();
+        Assert.True(remaining > 0, "Storage-cap purge must not delete the entire table.");
+        Assert.True(remaining < 3000, "Storage-cap purge must remove some events when over cap.");
+
+        // The database must actually be back under the cap after purge.
+        var finalSize = cappedPurge.GetDatabaseSizeBytes();
+        var finalCapBytes = (long)capOptions.MaxStorageMb * 1024 * 1024;
+        Assert.True(finalSize <= finalCapBytes,
+            $"Database size {finalSize} must be at or below cap {finalCapBytes} after purge.");
+    }
+
+    [Fact]
+    public void PurgeByStorageCap_RemovesOldestEventsFirst()
+    {
+        // Regression test for SiteEventLogging-002: only the oldest events
+        // (lowest ids) should be removed when trimming to the cap.
+        InsertBulkEvents(3000);
+
+        var purge = CreatePurgeService();
+        var totalSize = purge.GetDatabaseSizeBytes();
+
+        var capOptions = new SiteEventLogOptions
+        {
+            DatabasePath = _dbPath,
+            RetentionDays = 30,
+            MaxStorageMb = (int)Math.Max(1, (totalSize / 2) / (1024 * 1024))
+        };
+
+        var minIdBefore = MinEventId();
+        var cappedPurge = CreatePurgeService(capOptions);
+        cappedPurge.RunPurge();
+        var minIdAfter = MinEventId();
+
+        // The surviving rows must be the newest ones — minimum id has advanced.
+        Assert.True(minIdAfter > minIdBefore,
+            "Oldest events (lowest ids) must be purged first.");
+
+        // The newest event (highest id) must still be present.
+        var newestPresent = _eventLogger.WithConnection(connection =>
+        {
+            using var cmd = connection.CreateCommand();
+            cmd.CommandText = "SELECT COUNT(*) FROM site_events WHERE id = 3000";
+            return (long)cmd.ExecuteScalar()!;
+        });
+        Assert.Equal(1L, newestPresent);
+    }
+
+    [Fact]
+    public async Task PurgeByStorageCap_ConcurrentWritesDoNotCorruptConnection()
+    {
+        // Regression test for SiteEventLogging-003: purge running on a background
+        // thread while events are recorded on other threads must not throw
+        // "DataReader already open" / "connection busy" from a shared connection.
+        InsertBulkEvents(2000);
+
+        var purge = CreatePurgeService();
+        var totalSize = purge.GetDatabaseSizeBytes();
+        var capOptions = new SiteEventLogOptions
+        {
+            DatabasePath = _dbPath,
+            RetentionDays = 30,
+            MaxStorageMb = (int)Math.Max(1, (totalSize / 2) / (1024 * 1024))
+        };
+
+        var exceptions = new System.Collections.Concurrent.ConcurrentBag<Exception>();
+        var stop = false;
+
+        var purgeTask = Task.Run(() =>
+        {
+            try
+            {
+                var p = CreatePurgeService(capOptions);
+                for (int i = 0; i < 20; i++) p.RunPurge();
+            }
+            catch (Exception ex) { exceptions.Add(ex); }
+        });
+
+        var writeTasks = Enumerable.Range(0, 4).Select(_ => Task.Run(async () =>
+        {
+            try
+            {
+                while (!stop)
+                {
+                    await _eventLogger.LogEventAsync("script", "Info", null, "Concurrent", "Concurrent write");
+                }
+            }
+            catch (Exception ex) { exceptions.Add(ex); }
+        })).ToArray();
+
+        await purgeTask;
+        stop = true;
+        await Task.WhenAll(writeTasks);
+
+        Assert.Empty(exceptions);
+    }
 }

tests/ScadaLink.SiteEventLogging.Tests/EventLogQueryServiceTests.cs

@@ -256,7 +256,9 @@ public class EventLogQueryServiceTests : IDisposable
 
     private void InsertEventAt(DateTimeOffset timestamp, string eventType, string severity, string? instanceId, string source, string message)
     {
-        using var cmd = _eventLogger.Connection.CreateCommand();
+        _eventLogger.WithConnection(connection =>
+        {
+            using var cmd = connection.CreateCommand();
             cmd.CommandText = """
                 INSERT INTO site_events (timestamp, event_type, severity, instance_id, source, message)
                 VALUES ($ts, $et, $sev, $iid, $src, $msg)
@@ -268,5 +270,6 @@ public class EventLogQueryServiceTests : IDisposable
             cmd.Parameters.AddWithValue("$src", source);
             cmd.Parameters.AddWithValue("$msg", message);
             cmd.ExecuteNonQuery();
+        });
     }
 }