fix(host,deployment-manager,communication): repair cross-module DI regressions from batch 1-2

- DeploymentManager-008: revert IConfiguration overload (violated OptionsTests component-convention); Host now binds the ScadaLink:DeploymentManager section - SiteStreamGrpcServer: make test-only int ctor internal so DI sees one public ctor (resolves ambiguous-constructor failure in SiteCompositionRootTests) - Host site composition-root test config: supply Cluster:SeedNodes for the new ClusterOptionsValidator
docs(code-reviews): regenerate index after batch 3 medium fixes
2026-05-16 21:28:50 -04:00 · 2026-05-16 21:22:01 -04:00 · 2026-05-16 21:22:01 -04:00 · 2026-05-16 21:22:01 -04:00 · 2026-05-16 21:22:01 -04:00 · 2026-05-16 21:22:01 -04:00
49 changed files with 2345 additions and 266 deletions
@@ -402,18 +402,20 @@ Add an `IConfiguration` parameter (or a configure callback) to

 **Resolution**

-Resolved 2026-05-16 (commit pending): added an
-`AddDeploymentManager(IServiceCollection, IConfiguration)` overload that binds
-`DeploymentManagerOptions` to the `ScadaLink:DeploymentManager` configuration
-section (exposed as `ServiceCollectionExtensions.OptionsSection`). The
-`Microsoft.Extensions.Options.ConfigurationExtensions` package was added to the
-project. The original parameterless overload is retained for callers/tests that
-do not bind configuration. Regression tests:
-`AddDeploymentManager_WithConfiguration_BindsDeploymentManagerOptions`,
-`AddDeploymentManager_WithConfiguration_MissingSection_UsesDefaults`. Note: a
-one-line follow-up in `Host/Program.cs` (call the new overload with
-`builder.Configuration`) is required to take effect at runtime — that file is
-outside this module's edit scope and is surfaced for the Host owner.
+Resolved 2026-05-16 (commit pending): `AddDeploymentManager()` now calls
+`services.AddOptions<DeploymentManagerOptions>()` so `IOptions<DeploymentManagerOptions>`
+is always resolvable, and `Host/Program.cs` binds the
+`ScadaLink:DeploymentManager` section (exposed as
+`ServiceCollectionExtensions.OptionsSection`) via
+`services.Configure<DeploymentManagerOptions>(...)` — the same pattern the Host
+uses for `SecurityOptions`/`InboundApiOptions`. An earlier attempt added an
+`AddDeploymentManager(IConfiguration)` overload; that was reverted because the
+project convention (enforced by `Host.Tests.OptionsTests`) forbids component
+`Add*` methods from depending on `IConfiguration` — the Host owns
+configuration binding. Regression tests:
+`AddDeploymentManager_RegistersResolvableOptions_WithDefaults`,
+`AddDeploymentManager_OptionsBindToConfigurationSection_AsTheHostWires`,
+`OptionsSection_MatchesTheConventionalComponentSectionPath`.

 ### DeploymentManager-009 — Misleading timeout comment on `DeleteInstanceAsync`

@@ -8,7 +8,7 @@
 | Last reviewed | 2026-05-16 |
 | Reviewer | claude-agent |
 | Commit reviewed | `9c60592` |
-| Open findings | 10 |
+| Open findings | 8 |

 ## Summary

@@ -98,10 +98,10 @@ response body; it failed before the fix and passes after. Full Host suite green

 | | |
 |--|--|
-| Severity | Medium |
+| Severity | Medium — re-triaged: stale design doc, Host code is correct |
 | Category | Design-document adherence |
 | Status | Open |
-| Location | `src/ScadaLink.Host/Actors/AkkaHostedService.cs:70-108` |
+| Location | `src/ScadaLink.Host/Actors/AkkaHostedService.cs:70-108`, `docs/requirements/Component-Host.md` REQ-HOST-6 |

 **Description**

@@ -126,7 +126,21 @@ add the plugin packages and HOCON. Either way, code and doc must agree.

 **Resolution**

-_Unresolved._
+_Verified 2026-05-16, left Open — re-triaged._ The finding is accurate: a repo-wide
+search confirms there are **no** `PersistentActor` / `ReceivePersistentActor`
+subclasses anywhere in `src/`, no `akka.persistence` section in the HOCON built by
+`AkkaHostedService.StartAsync`, and `ScadaLink.Host.csproj` references no persistence
+plugin packages. The system deliberately uses component-owned SQLite storage
+services instead. The **Host code is therefore correct and internally consistent** —
+the only defect is that `docs/requirements/Component-Host.md` REQ-HOST-6 and its
+Dependencies list still mandate Akka.Persistence, a subsystem that does not (and is
+not intended to) exist. The sole fix is editing that design document, which lies
+outside this resolution task's permitted edit scope (`src/ScadaLink.Host`,
+`tests/ScadaLink.Host.Tests`, `code-reviews/Host/findings.md`). No code or test
+change can resolve a stale-doc finding. Left **Open** and surfaced for the design-doc
+owner: REQ-HOST-6 must drop the Akka.Persistence requirement (and the
+`Akka.Persistence.Hosting` Dependencies entry), stating that node-local persistence
+is provided by component-owned SQLite storage services.

 ### Host-003 — Secrets committed in plaintext in `appsettings.Central.json`

@@ -134,7 +148,7 @@ _Unresolved._
 |--|--|
 | Severity | Medium |
 | Category | Security |
-| Status | Open |
+| Status | Resolved |
 | Location | `src/ScadaLink.Host/appsettings.Central.json:20-31` |

 **Description**
@@ -159,7 +173,29 @@ environment.

 **Resolution**

-_Unresolved._
+Resolved 2026-05-16 (commit `pending`). Root cause confirmed against
+`appsettings.Central.json`: the committed file carried real-looking secrets in
+plaintext — SQL Server passwords (`Password=ScadaLink_Dev1#`) in both connection
+strings, an LDAP service-account password, and a JWT signing key. Fix: all four
+secrets were removed from the committed file and replaced with non-functional
+`${...}` placeholder references (`ConfigurationDb` / `MachineDataDb`,
+`LdapServiceAccountPassword`, `JwtSigningKey`). A new top-level `_secrets` note
+documents that the Host's configuration builder (`AddEnvironmentVariables()`)
+overlays the real values supplied via environment variables
+(`ScadaLink__Database__ConfigurationDb`, `ScadaLink__Database__MachineDataDb`,
+`ScadaLink__Security__LdapServiceAccountPassword`,
+`ScadaLink__Security__JwtSigningKey`); the placeholders are intentionally invalid so
+a misconfigured deployment fails loudly rather than silently using a committed key.
+Regression test class `ConfigSecretsTests` asserts the committed file carries no
+plaintext `Password=` value, no committed LDAP service-account password, and no
+committed JWT signing key; all three tests failed before the fix and pass after.
+Tests that drive the full `Program` startup pipeline against the real SQL provider
+(`HealthCheckTests`, `HostStartupTests.CentralRole_StartsWithoutError`) were adapted
+to supply the local dev connection strings themselves via the new
+`CentralDbTestEnvironment` test helper (environment variables) — they must no longer
+depend on committed secrets. Note: the `docker/central-node-*/appsettings.Central.json`
+files still contain the same dev secrets but lie outside this task's permitted edit
+scope; they should receive the same treatment in a follow-up.

 ### Host-004 — Site seed-node list points at the gRPC port, not a remoting port

@@ -167,7 +203,7 @@ _Unresolved._
 |--|--|
 | Severity | Medium |
 | Category | Correctness & logic bugs |
-| Status | Open |
+| Status | Resolved |
 | Location | `src/ScadaLink.Host/appsettings.Site.json:10-19` |

 **Description**
@@ -190,7 +226,23 @@ to reject a seed node whose port equals this node's `GrpcPort`.

 **Resolution**

-_Unresolved._
+Resolved 2026-05-16 (commit `pending`). Root cause confirmed against
+`appsettings.Site.json`: with `Node:RemotingPort = 8082` and `Node:GrpcPort = 8083`,
+the second `Cluster:SeedNodes` entry was `akka.tcp://scadalink@localhost:8083` — the
+Kestrel HTTP/2 gRPC port, not an Akka.Remote endpoint. `StartupValidator` only
+checked seed-node *count* (≥2), so the misconfiguration passed silently. Fix, two
+parts: (1) the shipped `appsettings.Site.json` second seed entry was corrected to a
+remoting port (`localhost:8084`); (2) `StartupValidator.Validate` was extended — for
+`Site` nodes it now parses each seed node's TCP port (via a new `SeedNodePort`
+helper) and rejects any entry whose port equals the node's `GrpcPort`, using the
+resolved GrpcPort including the `8083` `NodeOptions` default when the key is absent.
+The seed-node-count check was hoisted above the Site block so the new rule can reuse
+the parsed list. Regression tests in `StartupValidatorTests`:
+`Site_SeedNodeOnGrpcPort_FailsValidation`,
+`Site_SeedNodeOnDefaultGrpcPort_FailsValidation` (default-8083 path),
+`Site_SeedNodesOnRemotingPort_PassesValidation`, and
+`Central_SeedNodeOnPort8083_PassesValidation` (rule is Site-only) — all failed
+appropriately before the fix and pass after.

 ### Host-005 — Blocking sync-over-async (`GetAwaiter().GetResult()`) inside `StartAsync`

@@ -8,7 +8,7 @@
 | Last reviewed | 2026-05-16 |
 | Reviewer | claude-agent |
 | Commit reviewed | `9c60592` |
-| Open findings | 10 |
+| Open findings | 6 |

 ## Summary

@@ -87,10 +87,10 @@ via `GetOrAdd` so concurrent first-callers share one handler. Regression tests

 | | |
 |--|--|
-| Severity | Medium |
+| Severity | Medium — re-triaged: already fixed by the InboundAPI-001 fix; verified and closed |
 | Category | Concurrency & thread safety |
-| Status | Open |
-| Location | `src/ScadaLink.InboundAPI/InboundScriptExecutor.cs:123-129` |
+| Status | Resolved |
+| Location | `src/ScadaLink.InboundAPI/InboundScriptExecutor.cs:152-161` |

 **Description**

@@ -111,7 +111,17 @@ return the handler it produced rather than requiring a separate dictionary read.

 **Resolution**

-_Unresolved._
+Resolved 2026-05-16 (commit `pending`): re-triage — verified against the current
+source, this finding was **already fixed** by the InboundAPI-001 fix. The
+`InboundScriptExecutor.cs:152-161` lazy-compile path no longer does check-then-act
+re-read: `Compile(method)` runs unconditionally (it never reads the cache) and the
+result is published via the atomic `_scriptHandlers.GetOrAdd(method.Name, compiled)`.
+There is no separate dictionary indexer read, so the `KeyNotFoundException` race the
+finding describes cannot occur, and concurrent first-callers all share the single
+handler that `GetOrAdd` keeps. Regression test
+`LazyCompile_RacingRemoveHandler_NeverThrowsKeyNotFound` added (asserts a concurrent
+`RemoveHandler` storm against lazy-compiling callers never yields the catch-all
+"Internal script error"); it passes against the current code, confirming the fix.

 ### InboundAPI-003 — API key compared with non-constant-time string equality

@@ -161,7 +171,7 @@ longer depends on it.
 |--|--|
 | Severity | Medium |
 | Category | Error handling & resilience |
-| Status | Open |
+| Status | Resolved |
 | Location | `src/ScadaLink.InboundAPI/InboundScriptExecutor.cs:117-141` |

 **Description**
@@ -185,7 +195,17 @@ or use a dedicated timeout `CancellationTokenSource` so the two are separable.

 **Resolution**

-_Unresolved._
+Resolved 2026-05-16 (commit `pending`): `ExecuteAsync` now uses a dedicated timeout
+`CancellationTokenSource` (`new CancellationTokenSource(timeout)`) linked with the
+request-abort token, so the two cancellation sources are separable. The
+`OperationCanceledException` handler reports "Script execution timed out" (and logs a
+warning) **only** when the timeout CTS fired and the request token did not; a client
+abort instead returns "Request cancelled by client" and logs at Debug — the failure
+log stays reserved for genuine script-execution timeouts. `HandleInboundApiRequest`
+additionally short-circuits with `Results.Empty` (no warning log, no 500 body write)
+when `RequestAborted` is cancelled, since the connection is already gone. Regression
+tests `ClientDisconnect_IsNotReportedAsTimeout` and `GenuineTimeout_StillReportedAsTimeout`
+added.

 ### InboundAPI-005 — Compiled API scripts run with no script-trust-model enforcement

@@ -237,7 +257,7 @@ Regression tests `CompileAndRegister_ForbiddenApi_RejectsScript` (theory),
 |--|--|
 | Severity | Medium |
 | Category | Security |
-| Status | Open |
+| Status | Resolved |
 | Location | `src/ScadaLink.InboundAPI/EndpointExtensions.cs:54-62` |

 **Description**
@@ -260,16 +280,24 @@ Reject oversized bodies with 413 before buffering.

 **Resolution**

-_Unresolved._
+Resolved 2026-05-16 (commit `pending`): added `InboundApiEndpointFilter`, an
+`IEndpointFilter` applied to `POST /api/{methodName}` via `.AddEndpointFilter<>()`.
+It rejects requests whose declared `Content-Length` exceeds `InboundApiOptions.
+MaxRequestBodyBytes` (default 1 MiB) with HTTP 413 *before* the handler buffers the
+body into a `JsonDocument`, and also lowers the per-request `IHttpMaxRequestBodySizeFeature`
+cap so a chunked/unknown-length stream is cut off by Kestrel while being read. The
+limit is configurable via the bound `ScadaLink:InboundApi` options section. Regression
+tests `OversizedBody_ShortCircuitsWith413_AndDoesNotRunHandler`, `BodyAtLimit_RunsHandler`,
+and `FilterCapsMaxRequestBodySizeFeature` added.

 ### InboundAPI-007 — `Database.Connection()` script API from the design doc is not implemented

 | | |
 |--|--|
-| Severity | Medium |
+| Severity | Medium — verified real drift; left Open pending a design decision (see Resolution) |
 | Category | Design-document adherence |
 | Status | Open |
-| Location | `src/ScadaLink.InboundAPI/InboundScriptExecutor.cs:155-170` |
+| Location | `src/ScadaLink.InboundAPI/InboundScriptExecutor.cs:188-203` |

 **Description**

@@ -289,7 +317,27 @@ API.

 **Resolution**

-_Unresolved._
+_Unresolved — left Open; needs a design decision the resolving agent cannot make._
+Re-triage 2026-05-16: confirmed against the current source — the drift is **real**.
+`InboundScriptContext` (`InboundScriptExecutor.cs:188-203`) exposes only `Parameters`,
+`Route`, and `CancellationToken`; there is no `Database` member, so a method script
+following the documented `Database.Connection("name")` API fails to compile.
+
+This finding cannot be closed by the InboundAPI module agent for two reasons:
+1. **Scope** — the alternative resolution (deleting the "Database Access" section)
+   edits `docs/requirements/Component-InboundAPI.md`, which is outside the editable
+   scope (`src/ScadaLink.InboundAPI`, `tests/`, this file only).
+2. **It is a genuine design decision.** Implementing `Database.Connection()` is not a
+   mechanical fix: it hands inbound API scripts a *raw* MS SQL client. The ScadaLink
+   script trust model (CLAUDE.md, Akka.NET conventions) forbids scripts from `System.IO`
+   and raw network access, and `ForbiddenApiChecker` (added for InboundAPI-005) now
+   statically blocks `System.Net`/`System.IO`. A raw `SqlConnection` is in clear
+   tension with that trust model, and the set of connection names a script may open,
+   read-only vs. read-write access, and connection lifetime/pooling all require a
+   design call. **Surface to the design owner:** decide whether `Database.Connection()`
+   is in scope — if yes, write a design note covering the trust-model carve-out and
+   then implement a `Database` member backed by a connection-factory service; if no,
+   delete the "Database Access" section from `Component-InboundAPI.md`.

 ### InboundAPI-008 — Inbound API endpoint not restricted to the active central node

@@ -297,7 +345,7 @@ _Unresolved._
 |--|--|
 | Severity | Medium |
 | Category | Design-document adherence |
-| Status | Open |
+| Status | Resolved |
 | Location | `src/ScadaLink.InboundAPI/EndpointExtensions.cs:19-23`, `src/ScadaLink.Host/Program.cs:149` |

 **Description**
@@ -318,7 +366,19 @@ treated.

 **Resolution**

-_Unresolved._
+Resolved 2026-05-16 (commit `pending`): introduced `IActiveNodeGate`, an abstraction
+the inbound API uses to ask whether this node is the active (cluster-leader) central
+node. The new `InboundApiEndpointFilter` (applied to `POST /api/{methodName}`)
+consults the gate and short-circuits a standby node with HTTP 503 before any
+auth/script work, so Traefik/clients only reach the live node — consistent with
+`/health/active`. The gate is resolved optionally: when no implementation is
+registered (non-clustered host / tests) the endpoint defaults to "allow", preserving
+prior behaviour. Regression tests `StandbyNode_ShortCircuitsWith503_AndDoesNotRunHandler`,
+`ActiveNode_PassesGate_RunsHandler`, and `NoGateRegistered_PassesGate_RunsHandler`
+added. **Follow-up (outside this module's scope):** `ScadaLink.Host` should register
+an `IActiveNodeGate` implementation backed by `ActiveNodeHealthCheck` /
+`Cluster.State.Leader` in the central-role branch of `Program.cs` so the gate is
+actually enforced in production; until then the endpoint defaults to "allow".

 ### InboundAPI-009 — Failed compilation is retried on every subsequent request

@@ -8,7 +8,7 @@
 | Last reviewed | 2026-05-16 |
 | Reviewer | claude-agent |
 | Commit reviewed | `9c60592` |
-| Open findings | 10 |
+| Open findings | 5 |

 ## Summary

@@ -171,7 +171,7 @@ tests: `IsInstanceAccessAllowed_SiteScopedUser_OutOfScopeInstance_Denied`,
 |--|--|
 | Severity | Medium |
 | Category | Akka.NET conventions |
-| Status | Open |
+| Status | Resolved |
 | Location | `src/ScadaLink.ManagementService/ManagementActor.cs:61` |

 **Description**
@@ -196,7 +196,14 @@ that explicit with a router/dispatcher rather than ad-hoc `Task.Run`.

 **Resolution**

-_Unresolved._
+Resolved 2026-05-16 (commit pending). Confirmed: `HandleEnvelope` ran every command via
+`Task.Run` and replied from inside the continuation, contrary to the project's PipeTo
+convention. Replaced it with a `ProcessCommand` method returning a `Task<object>` and
+`PipeTo(sender, success, failure)`; faults are now mapped uniformly in a `MapFault` failure
+continuation (`SiteScopeViolationException` -> `ManagementUnauthorized`, otherwise
+`ManagementError`), which also unwraps `AggregateException`. Regression test:
+`UnknownCommandType_FaultMappedToManagementError`. Existing success/error/unauthorized
+mapping tests confirm behaviour is preserved.

 ### ManagementService-005 — ManagementActor declares no supervision strategy

@@ -231,7 +238,7 @@ _Unresolved._
 |--|--|
 | Severity | Medium |
 | Category | Performance & resource management |
-| Status | Open |
+| Status | Resolved |
 | Location | `src/ScadaLink.ManagementService/ManagementEndpoints.cs:83`, `:112` |

 **Description**
@@ -250,7 +257,16 @@ object, or restructure so the fallback path does not parse a throwaway document.

 **Resolution**

-_Unresolved._
+Resolved 2026-05-16 (commit pending). Confirmed: the request `JsonDocument` was never
+disposed and the empty-payload path allocated a second throwaway `JsonDocument`. Extracted
+request parsing into a testable `ManagementEndpoints.ParseCommand` helper that wraps the
+document in `using`; the missing-payload case now deserializes from the `"{}"` literal
+string rather than parsing a throwaway document. Regression tests:
+`ParseCommand_WithExplicitPayload_DeserializesIntoCommandType`,
+`ParseCommand_WithMissingPayload_DeserializesParameterlessCommand`,
+`ParseCommand_WithInvalidJson_ReturnsFailure`,
+`ParseCommand_WithMissingCommandField_ReturnsFailure`,
+`ParseCommand_WithUnknownCommand_ReturnsFailure`.

 ### ManagementService-007 — Inconsistent and cycle-prone serialization of repository entities

@@ -258,7 +274,7 @@ _Unresolved._
 |--|--|
 | Severity | Medium |
 | Category | Code organization & conventions |
-| Status | Open |
+| Status | Resolved |
 | Location | `src/ScadaLink.ManagementService/ManagementActor.cs:67`; `src/ScadaLink.ManagementService/ManagementEndpoints.cs:113` |

 **Description**
@@ -281,7 +297,14 @@ correctly.

 **Resolution**

-_Unresolved._
+Resolved 2026-05-16 (commit pending). Confirmed: the actor serialized results with
+`Newtonsoft.Json` (not even a direct package reference) while the HTTP endpoint uses
+`System.Text.Json`. Standardised the actor on `System.Text.Json` via a new
+`ManagementActor.SerializeResult` helper using a shared `JsonSerializerOptions` with
+`ReferenceHandler.IgnoreCycles` (cycle-safe for EF entity graphs) and camelCase naming
+(matches the CLI's case-insensitive deserializer). Removed the `Newtonsoft.Json` import.
+Regression tests: `SerializeResult_WithCyclicGraph_DoesNotThrow`,
+`SerializeResult_UsesCamelCasePropertyNames`.

 ### ManagementService-008 — HandleResolveRoles constructs RoleMapper manually instead of via DI

@@ -312,9 +335,9 @@ _Unresolved._

 | | |
 |--|--|
-| Severity | Medium |
-| Category | Error handling & resilience |
-| Status | Open |
+| Severity | Low — re-triaged from Medium; the claimed audit gap does not exist (see Description), leaving only an undocumented-convention issue. |
+| Category | Code organization & conventions |
+| Status | Resolved |
 | Location | `src/ScadaLink.ManagementService/ManagementActor.cs:357`, `:1134`, `:1085`, `:526`, `:1275` |

 **Description**
@@ -325,21 +348,36 @@ external-system/notification/security/area mutations), but the handlers that del
 domain service do **not** — `HandleCreateTemplate`/`HandleUpdateTemplate`/`HandleDeleteTemplate`,
 all template-member handlers (`HandleAddAttribute` ... `HandleDeleteComposition`), template-folder
 handlers, shared-script handlers, `HandleDeployArtifacts`, `HandleDeployInstance`,
-`HandleEnableInstance`/`Disable`/`Delete`, and the instance-binding/override handlers. This is
-correct only if every one of those services performs its own audit logging internally; the
-mixed pattern makes that impossible to verify by reading this module and creates a real risk
-of silent audit gaps for template authoring and deployment operations.
+`HandleEnableInstance`/`Disable`/`Delete`, and the instance-binding/override handlers.
+
+**Re-triage (2026-05-16):** the original finding claimed this "creates a real risk of silent
+audit gaps for template authoring and deployment operations." That claim was verified against
+the actual sources and is **false**. Every domain service the delegating handlers call —
+`TemplateService`, `SharedScriptService`, `InstanceService`, `AreaService`, `SiteService`,
+`TemplateFolderService`, `DeploymentService`, `ArtifactDeploymentService` — injects
+`IAuditService` and calls `LogAsync` on every mutation (`grep` confirms an `_auditService.LogAsync`
+call after each `Create`/`Update`/`Delete` in `TemplateService.cs`, `DeploymentService.cs`,
+`ArtifactDeploymentService.cs`, etc.). There is therefore no audit gap; if anything, adding
+explicit `AuditAsync` to a delegating handler would *double-log*. The genuine issue is purely
+organizational: the two-layer split (actor audits repo-direct mutations, services audit their
+own) was undocumented, which is what made it look risky. This is a Low-severity
+code-organization issue, not a Medium error-handling/resilience defect.

 **Recommendation**

-Decide on one layer that owns auditing. Either route all mutations through services that audit
-internally (and remove the explicit `AuditAsync` calls here), or audit uniformly in the actor
-after every successful mutation. Document the chosen contract so the inconsistency cannot
-recur, and confirm template/deployment services actually audit.
+Document the chosen contract so the split cannot be misread as a gap. (The original
+alternative — moving all auditing into the actor — would require un-auditing eight services
+and is not warranted given they already audit correctly.)

 **Resolution**

-_Unresolved._
+Resolved 2026-05-16 (commit pending). Re-triaged to Low / Code organization after verifying
+all eight delegated-to services audit internally — no audit gap exists. Documented the
+two-layer audit contract in an XML `<remarks>` block on `ManagementActor.AuditAsync`:
+repository-direct mutations call `AuditAsync`; service-delegating handlers must not, because
+the services own auditing and a duplicate call would double-log. No behavioural change, so
+no new regression test; existing `CreateInstanceCommand_WithDeploymentRole_ReturnsSuccess`
+covers the explicit-audit path.

 ### ManagementService-010 — ManagementServiceOptions.CommandTimeout is defined but never used

@@ -433,7 +471,7 @@ _Unresolved._
 |--|--|
 | Severity | Medium |
 | Category | Testing coverage |
-| Status | Open |
+| Status | Resolved |
 | Location | `tests/ScadaLink.ManagementService.Tests/ManagementActorTests.cs:1` |

 **Description**
@@ -457,4 +495,14 @@ malformed bodies, unknown commands, and the 200/400/403/401/504 mappings.

 **Resolution**

-_Unresolved._
+Resolved 2026-05-16 (commit pending). The site-scope and `DebugStreamHub` coverage gaps
+were closed by the resolution of findings 001/002/003 (the `ScopedEnvelope` helper plus the
+`*_OutOfScopeForSiteScopedUser_ReturnsUnauthorized` tests and `DebugStreamHubTests`). The
+remaining HTTP-endpoint gap is now covered by a new `ManagementEndpointsTests.cs` exercising
+`ManagementEndpoints.ParseCommand` — command deserialization, malformed JSON, missing
+`command` field, and unknown commands. Full `WebApplicationFactory` auth-flow tests were
+deliberately not added: `ManagementEndpoints` depends on `LdapAuthService` and live LDAP
+infrastructure, so the testable command-parsing/dispatch logic was extracted into the pure
+`ParseCommand` helper and covered instead. Tests: `ParseCommand_*` (5),
+`SerializeResult_*` (2), `UnknownCommandType_FaultMappedToManagementError`, plus the
+pre-existing site-scope and DebugStreamHub suites. `dotnet test` -> 48 passed.
@@ -8,7 +8,7 @@
 | Last reviewed | 2026-05-16 |
 | Reviewer | claude-agent |
 | Commit reviewed | `9c60592` |
-| Open findings | 8 |
+| Open findings | 3 |

 ## Summary

@@ -172,7 +172,7 @@ the resulting client is disposed.
 |--|--|
 | Severity | Medium |
 | Category | Correctness & logic bugs |
-| Status | Open |
+| Status | Resolved |
 | Location | `src/ScadaLink.NotificationService/MailKitSmtpClientWrapper.cs:18`, `src/ScadaLink.NotificationService/NotificationDeliveryService.cs:123` |

 **Description**
@@ -185,7 +185,16 @@ Pass the `TlsMode` string (or a `TlsMode` enum) through to the wrapper and map e

 **Resolution**

-_Unresolved._
+Resolved 2026-05-16 (commit pending). Root cause confirmed against source: the
+`bool useTls` parameter cannot represent three states, and the non-StartTLS branch used
+`SecureSocketOptions.Auto`. A new `SmtpTlsMode` enum (`None`/`StartTls`/`Ssl`) and
+`SmtpTlsModeParser` were added; `ISmtpClientWrapper.ConnectAsync` now takes `SmtpTlsMode`
+and `MailKitSmtpClientWrapper` maps it explicitly to `SecureSocketOptions.None`/
+`StartTls`/`SslOnConnect`. `SendAsync`/`DeliverBufferedAsync` validate the configured
+`TlsMode` up front — an unknown value returns a clean `NotificationResult` failure (or
+parks a buffered message) instead of silently negotiating TLS. Regression tests:
+`Send_TlsModeNone_DoesNotNegotiateTls`, `Send_TlsModeSsl_UsesImplicitSsl`,
+`Send_UnknownTlsMode_ReturnsErrorNotSilentFallback`, and the `SmtpTlsModeParserTests` set.

 ### NotificationService-006 — OAuth2 token cache is keyed to nothing; wrong token returned when multiple SMTP configs exist

@@ -193,7 +202,7 @@ _Unresolved._
 |--|--|
 | Severity | Medium |
 | Category | Concurrency & thread safety |
-| Status | Open |
+| Status | Resolved |
 | Location | `src/ScadaLink.NotificationService/OAuth2TokenService.cs:14-15`, `src/ScadaLink.NotificationService/OAuth2TokenService.cs:30-35` |

 **Description**
@@ -206,7 +215,14 @@ Key the cache by the credential identity (e.g. a dictionary keyed by `tenantId:c

 **Resolution**

-_Unresolved._
+Resolved 2026-05-16 (commit pending). Root cause confirmed: the singleton held a single
+`_cachedToken`/`_tokenExpiry` pair and `GetTokenAsync` ignored the `credentials` argument
+when validating the cache, so a second SMTP config got the first config's token.
+`OAuth2TokenService` now stores a `ConcurrentDictionary<string, CacheEntry>` keyed by the
+SHA-256 hash of the credential string; each distinct tenant/client/secret gets its own
+cached token, expiry, and per-credential `SemaphoreSlim` (double-checked locking
+preserved). Regression tests: `GetTokenAsync_DifferentCredentials_ReturnPerCredentialTokens`
+and `GetTokenAsync_SameCredentials_CachedPerCredential`.

 ### NotificationService-007 — Connection timeout and max-concurrent-connections from the design doc are not implemented

@@ -214,7 +230,7 @@ _Unresolved._
 |--|--|
 | Severity | Medium |
 | Category | Design-document adherence |
-| Status | Open |
+| Status | Resolved |
 | Location | `src/ScadaLink.NotificationService/NotificationOptions.cs:11-14`, `src/ScadaLink.NotificationService/MailKitSmtpClientWrapper.cs:16-20`, `src/ScadaLink.NotificationService/NotificationDeliveryService.cs:111-140` |

 **Description**
@@ -227,7 +243,17 @@ Set `SmtpClient.Timeout` from `ConnectionTimeoutSeconds` in `ConnectAsync` (and/

 **Resolution**

-_Unresolved._
+Resolved 2026-05-16 (commit pending). Root cause confirmed: `ConnectAsync` never set
+`SmtpClient.Timeout` and no throttle gated `DeliverAsync`. `ISmtpClientWrapper.ConnectAsync`
+now takes a `connectionTimeoutSeconds` argument; `MailKitSmtpClientWrapper` sets
+`SmtpClient.Timeout` from `SmtpConfiguration.ConnectionTimeoutSeconds`. `DeliverAsync`
+acquires a lazily-created `SemaphoreSlim` sized to `SmtpConfiguration.MaxConcurrentConnections`
+(default 5 when non-positive) and releases it in a `finally`, so concurrent SMTP
+deliveries per site are bounded. The timeout is sourced from the deployed
+`SmtpConfiguration` rather than `NotificationOptions`; the `NotificationOptions` fields
+remain as operational fallback defaults. Regression tests:
+`Send_PassesConfiguredConnectionTimeoutToClient` and
+`Send_MaxConcurrentConnections_LimitsConcurrentDeliveries`.

 ### NotificationService-008 — Recipient email addresses are not validated before send

@@ -235,7 +261,7 @@ _Unresolved._
 |--|--|
 | Severity | Medium |
 | Category | Correctness & logic bugs |
-| Status | Open |
+| Status | Resolved |
 | Location | `src/ScadaLink.NotificationService/NotificationDeliveryService.cs:136-137`, `src/ScadaLink.NotificationService/MailKitSmtpClientWrapper.cs:50-53` |

 **Description**
@@ -248,15 +274,24 @@ Validate addresses up front (e.g. `MailboxAddress.TryParse`) and return a `Notif

 **Resolution**

-_Unresolved._
+Resolved 2026-05-16 (commit pending). Root cause confirmed: `MailboxAddress.Parse` of a
+malformed `FromAddress`/recipient threw `ParseException`, which is unclassified and
+escaped `SendAsync` as an unhandled exception. A new `ValidateAddresses` helper uses
+`MailboxAddress.TryParse` for the sender and every recipient; `SendAsync` now returns a
+clean `NotificationResult(false, ...)` listing the invalid address(es) before any SMTP
+attempt, and `DeliverBufferedAsync` parks a buffered message with a bad address (a fault
+retrying cannot fix). Regression tests:
+`Send_MalformedRecipientAddress_ReturnsCleanError_DoesNotThrow` and
+`Send_MalformedFromAddress_ReturnsCleanError_DoesNotThrow`. Definition-time validation in
+the Central UI is a separate component and out of this module's scope.

 ### NotificationService-009 — Credentials handled as plaintext strings; OAuth2 client secret logged risk

 | | |
 |--|--|
-| Severity | Medium |
+| Severity | Medium — re-triaged: split into an in-scope log-leak fix (resolved) and a Commons-scoped at-rest-encryption / structured-credential follow-up (NotificationService-013, Deferred). |
 | Category | Security |
-| Status | Open |
+| Status | Resolved |
 | Location | `src/ScadaLink.NotificationService/NotificationDeliveryService.cs:127-134`, `src/ScadaLink.NotificationService/OAuth2TokenService.cs:30-65`, `src/ScadaLink.Commons/Entities/Notifications/SmtpConfiguration.cs:9` |

 **Description**
@@ -269,7 +304,55 @@ Store credentials encrypted at rest (DPAPI/Data Protection or a secret store) an

 **Resolution**

-_Unresolved._
+Resolved 2026-05-16 (commit pending). Root cause re-triaged against source: the finding
+conflates two concerns with different ownership.
+
+1. **Log-leak risk (in scope — fixed).** The original code logged whole exception objects
+   (`_logger.LogWarning(ex, ...)` / `LogError(ex, ...)`); MailKit auth exceptions can echo
+   server responses quoting the supplied credentials. A new internal `CredentialRedactor`
+   masks every colon-delimited credential component out of any text. `SendAsync` and
+   `DeliverBufferedAsync` now log a scrubbed message string (not the raw exception) and the
+   permanent-failure `NotificationResult` is scrubbed before it returns to the caller.
+   `OAuth2TokenService` logs the tenant id only — never the client secret or access token.
+   Regression tests: `CredentialRedactorTests` and
+   `Send_PermanentError_RedactsCredentialFromResultMessage`.
+2. **At-rest encryption + structured-credential modelling (out of scope — Deferred).**
+   Encrypting `SmtpConfiguration.Credentials` at rest and replacing the brittle
+   colon-packed `string` with structured fields requires editing
+   `src/ScadaLink.Commons/Entities/Notifications/SmtpConfiguration.cs` and the
+   ConfigurationDatabase EF layer — both outside this module. Tracked separately as
+   **NotificationService-013** (Deferred) so it is not lost.
+
+### NotificationService-013 — Encrypt SMTP credentials at rest; replace colon-packed string with structured fields
+
+| | |
+|--|--|
+| Severity | Medium |
+| Category | Security |
+| Status | Deferred |
+| Location | `src/ScadaLink.Commons/Entities/Notifications/SmtpConfiguration.cs:9`, ConfigurationDatabase EF mapping |
+
+**Description**
+
+Split out of NotificationService-009. `SmtpConfiguration.Credentials` packs Basic Auth
+`user:pass` and OAuth2 `tenantId:clientId:clientSecret` into a single plaintext
+colon-delimited `string`: (a) there is no encryption at rest in SQLite or the central
+config DB; (b) a password or client secret containing a `:` is split incorrectly by
+`Split(':', 2)` / `Split(':', 3)`, silently corrupting the secret.
+
+**Recommendation**
+
+Model credentials as structured fields (or an encrypted blob) on the Commons entity and
+encrypt at rest via Data Protection / a secret store. The colon-delimited parsing in
+`MailKitSmtpClientWrapper` and `OAuth2TokenService` would then consume the structured
+fields directly.
+
+**Resolution**
+
+Deferred — requires changes to `src/ScadaLink.Commons` and the ConfigurationDatabase
+component, which are outside the NotificationService module. To be addressed in a
+Commons/ConfigurationDatabase-scoped change. The associated log-leak risk is resolved
+under NotificationService-009.

 ### NotificationService-010 — `DeliverAsync` does not disconnect the SMTP client on failure

@@ -41,9 +41,9 @@ module file and counted in **Total**.
 |----------|---------------|
 | Critical | 0 |
 | High | 0 |
-| Medium | 45 |
+| Medium | 25 |
 | Low | 90 |
-| **Total** | **135** |
+| **Total** | **115** |

 ## Module Status

@@ -59,11 +59,11 @@ module file and counted in **Total**.
 | [DeploymentManager](DeploymentManager/findings.md) | 2026-05-16 | `9c60592` | 0/0/0/5 | 5 | 14 |
 | [ExternalSystemGateway](ExternalSystemGateway/findings.md) | 2026-05-16 | `9c60592` | 0/0/0/4 | 4 | 14 |
 | [HealthMonitoring](HealthMonitoring/findings.md) | 2026-05-16 | `9c60592` | 0/0/0/5 | 5 | 12 |
-| [Host](Host/findings.md) | 2026-05-16 | `9c60592` | 0/0/3/7 | 10 | 11 |
-| [InboundAPI](InboundAPI/findings.md) | 2026-05-16 | `9c60592` | 0/0/5/5 | 10 | 13 |
-| [ManagementService](ManagementService/findings.md) | 2026-05-16 | `9c60592` | 0/0/5/5 | 10 | 13 |
-| [NotificationService](NotificationService/findings.md) | 2026-05-16 | `9c60592` | 0/0/5/3 | 8 | 12 |
-| [Security](Security/findings.md) | 2026-05-16 | `9c60592` | 0/0/4/4 | 8 | 11 |
+| [Host](Host/findings.md) | 2026-05-16 | `9c60592` | 0/0/1/7 | 8 | 11 |
+| [InboundAPI](InboundAPI/findings.md) | 2026-05-16 | `9c60592` | 0/0/1/5 | 6 | 13 |
+| [ManagementService](ManagementService/findings.md) | 2026-05-16 | `9c60592` | 0/0/0/5 | 5 | 13 |
+| [NotificationService](NotificationService/findings.md) | 2026-05-16 | `9c60592` | 0/0/0/3 | 3 | 13 |
+| [Security](Security/findings.md) | 2026-05-16 | `9c60592` | 0/0/0/4 | 4 | 11 |
 | [SiteEventLogging](SiteEventLogging/findings.md) | 2026-05-16 | `9c60592` | 0/0/4/3 | 7 | 11 |
 | [SiteRuntime](SiteRuntime/findings.md) | 2026-05-16 | `9c60592` | 0/0/8/5 | 13 | 16 |
 | [StoreAndForward](StoreAndForward/findings.md) | 2026-05-16 | `9c60592` | 0/0/4/7 | 11 | 14 |
@@ -84,34 +84,14 @@ _None open._

 _None open._

-### Medium (45)
+### Medium (25)

 | ID | Module | Title |
 |----|--------|-------|
 | CentralUI-005 | [CentralUI](CentralUI/findings.md) | Session expiry implementation diverges from the documented policy |
 | CentralUI-006 | [CentralUI](CentralUI/findings.md) | Deployment status page polls every 10s despite the documented SignalR-push design |
 | Host-002 | [Host](Host/findings.md) | Akka.Persistence required by REQ-HOST-6 is not configured and not used |
-| Host-003 | [Host](Host/findings.md) | Secrets committed in plaintext in `appsettings.Central.json` |
-| Host-004 | [Host](Host/findings.md) | Site seed-node list points at the gRPC port, not a remoting port |
-| InboundAPI-002 | [InboundAPI](InboundAPI/findings.md) | Lazy compilation is a check-then-act race with no atomicity |
-| InboundAPI-004 | [InboundAPI](InboundAPI/findings.md) | Client disconnect is misreported as a script timeout |
-| InboundAPI-006 | [InboundAPI](InboundAPI/findings.md) | No request body size limit on the inbound endpoint |
 | InboundAPI-007 | [InboundAPI](InboundAPI/findings.md) | `Database.Connection()` script API from the design doc is not implemented |
-| InboundAPI-008 | [InboundAPI](InboundAPI/findings.md) | Inbound API endpoint not restricted to the active central node |
-| ManagementService-004 | [ManagementService](ManagementService/findings.md) | Actor offloads work to Task.Run instead of using PipeTo |
-| ManagementService-006 | [ManagementService](ManagementService/findings.md) | JsonDocument instances never disposed in the HTTP endpoint |
-| ManagementService-007 | [ManagementService](ManagementService/findings.md) | Inconsistent and cycle-prone serialization of repository entities |
-| ManagementService-009 | [ManagementService](ManagementService/findings.md) | Audit logging applied inconsistently across mutating handlers |
-| ManagementService-013 | [ManagementService](ManagementService/findings.md) | No tests for site-scope enforcement, the HTTP endpoint, or DebugStreamHub |
-| NotificationService-005 | [NotificationService](NotificationService/findings.md) | Non-TLS path uses `SecureSocketOptions.Auto`, contradicting the requested mode |
-| NotificationService-006 | [NotificationService](NotificationService/findings.md) | OAuth2 token cache is keyed to nothing; wrong token returned when multiple SMTP configs exist |
-| NotificationService-007 | [NotificationService](NotificationService/findings.md) | Connection timeout and max-concurrent-connections from the design doc are not implemented |
-| NotificationService-008 | [NotificationService](NotificationService/findings.md) | Recipient email addresses are not validated before send |
-| NotificationService-009 | [NotificationService](NotificationService/findings.md) | Credentials handled as plaintext strings; OAuth2 client secret logged risk |
-| Security-004 | [Security](Security/findings.md) | Search filter uses `uid=` while fallback DN construction uses `cn=` |
-| Security-005 | [Security](Security/findings.md) | DN injection in the no-service-account bind fallback |
-| Security-006 | [Security](Security/findings.md) | JWT validation disables issuer and audience checks |
-| Security-007 | [Security](Security/findings.md) | Idle-timeout claim is reset on every token refresh |
 | SiteEventLogging-005 | [SiteEventLogging](SiteEventLogging/findings.md) | `LogEventAsync` performs synchronous disk I/O on the caller's thread |
 | SiteEventLogging-007 | [SiteEventLogging](SiteEventLogging/findings.md) | `ISiteEventLogger` consumers downcast to the concrete type and reach into the DB connection |
 | SiteEventLogging-008 | [SiteEventLogging](SiteEventLogging/findings.md) | Event-recording write failures are silently swallowed |
@@ -8,7 +8,7 @@
 | Last reviewed | 2026-05-16 |
 | Reviewer | claude-agent |
 | Commit reviewed | `9c60592` |
-| Open findings | 8 |
+| Open findings | 4 |

 ## Summary

@@ -156,7 +156,7 @@ corrected to state the requirement. Regression tests
 |--|--|
 | Severity | Medium |
 | Category | Correctness & logic bugs |
-| Status | Open |
+| Status | Resolved |
 | Location | `src/ScadaLink.Security/LdapAuthService.cs:66`, `:138`, `:157-159` |

 **Description**
@@ -176,7 +176,15 @@ consistently in both the search filter and the fallback DN. Update the XML doc t

 **Resolution**

-_Unresolved._
+Resolved 2026-05-16 (commit `pending`). Confirmed: the search filter was hard-coded
+`(uid={username})` (both in `AuthenticateAsync` and `ResolveUserDnAsync`) while the
+fallback DN used `cn={username}` — the two auth modes were not interchangeable. Added
+a configurable `SecurityOptions.LdapUserIdAttribute` (default `uid`) used for both the
+search filter and the fallback DN via the new `BuildFallbackUserDn` helper, and
+corrected the `LdapServiceAccountDn` XML doc to reference `{LdapUserIdAttribute}`.
+Regression tests `BuildFallbackUserDn_UsesConfiguredUserIdAttribute`,
+`BuildFallbackUserDn_HonoursNonDefaultUserIdAttribute`,
+`SecurityOptions_LdapUserIdAttribute_DefaultsToUid`.

 ### Security-005 — DN injection in the no-service-account bind fallback

@@ -184,7 +192,7 @@ _Unresolved._
 |--|--|
 | Severity | Medium |
 | Category | Security |
-| Status | Open |
+| Status | Resolved |
 | Location | `src/ScadaLink.Security/LdapAuthService.cs:157-159` |

 **Description**
@@ -207,7 +215,15 @@ raw DN from untrusted input is risky; restrict it or remove it.

 **Resolution**

-_Unresolved._
+Resolved 2026-05-16 (commit `pending`). Confirmed: the fallback path interpolated
+the raw `username` straight into `cn={username},{LdapSearchBase}` with no DN escaping,
+and the `username.Contains('=')` shortcut let a caller supply an arbitrary bind DN.
+Added an RFC 4514 `EscapeLdapDn` helper (escapes `, + " \ < > ;`, leading/trailing
+space, leading `#`, NUL) applied in `BuildFallbackUserDn`, so a username such as
+`victim,ou=admins` can no longer alter the DN structure. The `Contains('=')` raw-DN
+shortcut was removed entirely — untrusted input no longer controls the bind identity.
+Regression tests `BuildFallbackUserDn_EscapesDnMetacharacters`,
+`EscapeLdapDn_EscapesAllRfc4514Specials`, `EscapeLdapDn_EscapesLeadingAndTrailingSpaces`.

 ### Security-006 — JWT validation disables issuer and audience checks

@@ -215,7 +231,7 @@ _Unresolved._
 |--|--|
 | Severity | Medium |
 | Category | Security |
-| Status | Open |
+| Status | Resolved |
 | Location | `src/ScadaLink.Security/JwtTokenService.cs:67-75`, `:56-59` |

 **Description**
@@ -236,7 +252,14 @@ validation.

 **Resolution**

-_Unresolved._
+Resolved 2026-05-16 (commit `pending`). Confirmed: `GenerateToken` set neither `iss`
+nor `aud` and `ValidateToken` had `ValidateIssuer = false`/`ValidateAudience = false`.
+`GenerateToken` now binds `JwtTokenService.TokenIssuer`/`TokenAudience`
+(both `"scadalink-central"`) into every token, and `ValidateToken` enables
+`ValidateIssuer`/`ValidateAudience` against those fixed values — a token signed with
+the shared key but a foreign issuer is now rejected. Regression tests
+`GenerateToken_SetsIssuerAndAudience`, `ValidateToken_RejectsTokenWithWrongIssuer`,
+`ValidateToken_AcceptsOwnIssuerAndAudience`.

 ### Security-007 — Idle-timeout claim is reset on every token refresh

@@ -244,7 +267,7 @@ _Unresolved._
 |--|--|
 | Severity | Medium |
 | Category | Correctness & logic bugs |
-| Status | Open |
+| Status | Resolved |
 | Location | `src/ScadaLink.Security/JwtTokenService.cs:40`, `:111-123` |

 **Description**
@@ -269,7 +292,22 @@ path agree on the semantics.

 **Resolution**

-_Unresolved._
+Resolved 2026-05-16 (commit `pending`). Confirmed: `RefreshToken` → `GenerateToken`
+unconditionally wrote `LastActivity = UtcNow`, so the idle clock reset on every
+refresh and the documented 30-minute idle timeout could never fire for a client that
+polls in the background. Implemented option (a) — the Security-side half of the
+documented "15-min sliding + 30-min idle" policy (the cross-module partner of
+CentralUI-005): `GenerateToken` now takes an optional `lastActivity` anchor;
+`RefreshToken` carries the **existing** `LastActivity` claim forward unchanged, and a
+new explicit `RecordActivity` method advances the anchor to now — to be called by the
+CentralUI request pipeline on genuine user interaction (not on a background refresh).
+`IsIdleTimedOut` is unchanged and now agrees with the refresh path. The remaining
+CentralUI-side wiring (calling `RecordActivity` from the middleware, setting
+`SlidingExpiration`/`ExpireTimeSpan`) stays tracked under CentralUI-005; this finding's
+Security-side defect — the reset-on-refresh bug — is fully fixed here. Regression tests
+`RefreshToken_PreservesOriginalLastActivityClaim`,
+`RefreshToken_DoesNotResetIdleTimeoutWhenUserIsActuallyIdle`,
+`RecordActivity_UpdatesLastActivityToNow`.

 ### Security-008 — N+1 query loading site-scope rules in `RoleMapper`

@@ -24,7 +24,11 @@ public class SiteStreamGrpcServer : SiteStreamService.SiteStreamServiceBase
    private volatile bool _ready;
    private long _actorCounter;

-    public SiteStreamGrpcServer(
+    /// <summary>
+    /// Test-only constructor — kept <c>internal</c> so the DI container sees a
+    /// single public constructor and is not faced with an ambiguous choice.
+    /// </summary>
+    internal SiteStreamGrpcServer(
        ISiteStreamSubscriber streamSubscriber,
        ILogger<SiteStreamGrpcServer> logger,
        int maxConcurrentStreams = 100)
@@ -9,6 +9,7 @@

  <ItemGroup>
    <InternalsVisibleTo Include="ScadaLink.Communication.Tests" />
+    <InternalsVisibleTo Include="ScadaLink.IntegrationTests" />
  </ItemGroup>

  <ItemGroup>
@@ -11,7 +11,6 @@
    <PackageReference Include="Microsoft.Extensions.DependencyInjection.Abstractions" />
    <PackageReference Include="Microsoft.Extensions.Logging.Abstractions" />
    <PackageReference Include="Microsoft.Extensions.Options" />
-    <PackageReference Include="Microsoft.Extensions.Options.ConfigurationExtensions" />
  </ItemGroup>

  <ItemGroup>
@@ -1,4 +1,3 @@
-using Microsoft.Extensions.Configuration;
 using Microsoft.Extensions.DependencyInjection;

 namespace ScadaLink.DeploymentManager;
@@ -7,37 +6,26 @@ public static class ServiceCollectionExtensions
 {
    /// <summary>
    /// Configuration section that <see cref="DeploymentManagerOptions"/> is bound to.
+    /// The Host binds this section to <c>appsettings.json</c> (see
+    /// <c>Program.cs</c>); component libraries do not depend on
+    /// <c>IConfiguration</c> directly, consistent with the Options-pattern
+    /// convention enforced by <c>OptionsTests</c>.
    /// </summary>
    public const string OptionsSection = "ScadaLink:DeploymentManager";

    /// <summary>
-    /// Registers the Deployment Manager services and binds
-    /// <see cref="DeploymentManagerOptions"/> to the
-    /// <see cref="OptionsSection"/> configuration section, consistent with the
-    /// Options-pattern convention ("Per-component configuration via
-    /// appsettings.json sections bound to options classes").
-    /// </summary>
-    public static IServiceCollection AddDeploymentManager(
-        this IServiceCollection services,
-        IConfiguration configuration)
-    {
-        ArgumentNullException.ThrowIfNull(configuration);
-
-        // DeploymentManager-008: bind the options class so the operation-lock
-        // and artifact-deployment timeouts are tunable via appsettings.json.
-        services.Configure<DeploymentManagerOptions>(configuration.GetSection(OptionsSection));
-        return services.AddDeploymentManager();
-    }
-
-    /// <summary>
-    /// Registers the Deployment Manager services without binding options to
-    /// configuration. <see cref="DeploymentManagerOptions"/> falls back to its
-    /// declared defaults unless configured elsewhere. Prefer the
-    /// <see cref="AddDeploymentManager(IServiceCollection, IConfiguration)"/>
-    /// overload so the options are bound to <c>appsettings.json</c>.
+    /// Registers the Deployment Manager services. <see cref="DeploymentManagerOptions"/>
+    /// is registered via <see cref="OptionsServiceCollectionExtensions.AddOptions"/> so
+    /// <c>IOptions&lt;DeploymentManagerOptions&gt;</c> is always resolvable; the Host
+    /// binds <see cref="OptionsSection"/> to configuration so the operation-lock and
+    /// artifact-deployment timeouts are tunable via <c>appsettings.json</c>.
    /// </summary>
    public static IServiceCollection AddDeploymentManager(this IServiceCollection services)
    {
+        // DeploymentManager-008: ensure the options class is always resolvable.
+        // The Host binds OptionsSection to appsettings.json; absent that binding
+        // the declared option-class defaults apply.
+        services.AddOptions<DeploymentManagerOptions>();
        services.AddSingleton<OperationLockManager>();
        services.AddScoped<IFlatteningPipeline, FlatteningPipeline>();
        services.AddScoped<DeploymentService>();
@@ -106,6 +106,8 @@ try
        SiteServiceRegistration.BindSharedOptions(builder.Services, builder.Configuration);
        builder.Services.Configure<SecurityOptions>(builder.Configuration.GetSection("ScadaLink:Security"));
        builder.Services.Configure<InboundApiOptions>(builder.Configuration.GetSection("ScadaLink:InboundApi"));
+        builder.Services.Configure<DeploymentManagerOptions>(
+            builder.Configuration.GetSection(ScadaLink.DeploymentManager.ServiceCollectionExtensions.OptionsSection));

        var app = builder.Build();

@@ -40,23 +40,55 @@ public static class StartupValidator
                errors.Add("ScadaLink:Security:JwtSigningKey required for Central");
        }

+        var seedNodes = configuration.GetSection("ScadaLink:Cluster:SeedNodes").Get<List<string>>();
+        if (seedNodes == null || seedNodes.Count < 2)
+            errors.Add("ScadaLink:Cluster:SeedNodes must have at least 2 entries");
+
        if (role == "Site")
        {
            var grpcPortStr = nodeSection["GrpcPort"];
-            if (grpcPortStr != null && (!int.TryParse(grpcPortStr, out var gp) || gp < 1 || gp > 65535))
+            int grpcPort = 8083; // NodeOptions default when the key is absent
+            if (grpcPortStr != null && (!int.TryParse(grpcPortStr, out grpcPort) || grpcPort < 1 || grpcPort > 65535))
                errors.Add("ScadaLink:Node:GrpcPort must be 1-65535");

            var dbSection = configuration.GetSection("ScadaLink:Database");
            if (string.IsNullOrEmpty(dbSection["SiteDbPath"]))
                errors.Add("ScadaLink:Database:SiteDbPath required for Site nodes");
-        }

-        var seedNodes = configuration.GetSection("ScadaLink:Cluster:SeedNodes").Get<List<string>>();
-        if (seedNodes == null || seedNodes.Count < 2)
-            errors.Add("ScadaLink:Cluster:SeedNodes must have at least 2 entries");
+            // Host-004: a seed node must reference an Akka.Remote endpoint, never the
+            // Kestrel HTTP/2 gRPC port. A seed entry whose port equals this node's
+            // GrpcPort would make a joining node attempt an Akka.Remote TCP
+            // association against the gRPC listener and fail.
+            if (seedNodes != null)
+            {
+                foreach (var seed in seedNodes)
+                {
+                    if (SeedNodePort(seed) == grpcPort)
+                        errors.Add(
+                            $"ScadaLink:Cluster:SeedNodes entry '{seed}' must not target the gRPC port " +
+                            $"({grpcPort}); seed nodes must reference Akka remoting ports");
+                }
+            }
+        }

        if (errors.Count > 0)
            throw new InvalidOperationException(
                $"Configuration validation failed:\n{string.Join("\n", errors.Select(e => $"  - {e}"))}");
    }
+
+    /// <summary>
+    /// Extracts the TCP port from an Akka seed-node address of the form
+    /// <c>akka.tcp://system@host:port</c>. Returns <c>-1</c> when no port can be parsed.
+    /// </summary>
+    private static int SeedNodePort(string seedNode)
+    {
+        if (string.IsNullOrWhiteSpace(seedNode))
+            return -1;
+
+        var lastColon = seedNode.LastIndexOf(':');
+        if (lastColon < 0 || lastColon == seedNode.Length - 1)
+            return -1;
+
+        return int.TryParse(seedNode[(lastColon + 1)..], out var port) ? port : -1;
+    }
 }
@@ -16,9 +16,10 @@
      "FailureDetectionThreshold": "00:00:10",
      "MinNrOfMembers": 1
    },
+    "_secrets": "Host-003: Secrets are NOT committed in this file. Supply them via environment variables, which the Host's configuration builder (AddEnvironmentVariables) overlays over this file. Required: ScadaLink__Database__ConfigurationDb, ScadaLink__Database__MachineDataDb, ScadaLink__Security__LdapServiceAccountPassword, ScadaLink__Security__JwtSigningKey. The ${...} placeholders below are intentionally non-functional and must be overridden per environment.",
    "Database": {
-      "ConfigurationDb": "Server=localhost,1433;Database=ScadaLinkConfig;User Id=scadalink_app;Password=ScadaLink_Dev1#;TrustServerCertificate=true",
-      "MachineDataDb": "Server=localhost,1433;Database=ScadaLinkMachineData;User Id=scadalink_app;Password=ScadaLink_Dev1#;TrustServerCertificate=true"
+      "ConfigurationDb": "${SCADALINK_CONFIGURATIONDB_CONNECTION_STRING}",
+      "MachineDataDb": "${SCADALINK_MACHINEDATADB_CONNECTION_STRING}"
    },
    "Security": {
      "LdapServer": "localhost",
@@ -27,8 +28,8 @@
      "AllowInsecureLdap": true,
      "LdapSearchBase": "dc=scadalink,dc=local",
      "LdapServiceAccountDn": "cn=admin,dc=scadalink,dc=local",
-      "LdapServiceAccountPassword": "password",
-      "JwtSigningKey": "scadalink-dev-jwt-signing-key-must-be-at-least-32-characters-long",
+      "LdapServiceAccountPassword": "${SCADALINK_LDAP_SERVICE_ACCOUNT_PASSWORD}",
+      "JwtSigningKey": "${SCADALINK_JWT_SIGNING_KEY}",
      "JwtExpiryMinutes": 15,
      "IdleTimeoutMinutes": 30
    },
@@ -10,7 +10,7 @@
    "Cluster": {
      "SeedNodes": [
        "akka.tcp://scadalink@localhost:8082",
-        "akka.tcp://scadalink@localhost:8083"
+        "akka.tcp://scadalink@localhost:8084"
      ],
      "SplitBrainResolverStrategy": "keep-oldest",
      "StableAfter": "00:00:15",
@@ -18,7 +18,10 @@ public static class EndpointExtensions
 {
    public static IEndpointRouteBuilder MapInboundAPI(this IEndpointRouteBuilder endpoints)
    {
-        endpoints.MapPost("/api/{methodName}", HandleInboundApiRequest);
+        endpoints.MapPost("/api/{methodName}", HandleInboundApiRequest)
+            // InboundAPI-006 / InboundAPI-008: active-node gating + request body
+            // size cap are enforced by the endpoint filter before the handler runs.
+            .AddEndpointFilter<InboundApiEndpointFilter>();
        return endpoints;
    }

@@ -86,6 +89,14 @@ public static class EndpointExtensions

        if (!scriptResult.Success)
        {
+            // InboundAPI-004: a client-aborted request is not a script failure.
+            // Do not pollute the failure log (reserved for genuine script errors)
+            // and do not attempt to write a 500 body to an already-gone connection.
+            if (httpContext.RequestAborted.IsCancellationRequested)
+            {
+                return Results.Empty;
+            }
+
            // WP-5: 500 for script failures, safe error message
            logger.LogWarning(
                "Inbound API script failure for method {Method}: {Error}",
@@ -0,0 +1,24 @@
+namespace ScadaLink.InboundAPI;
+
+/// <summary>
+/// InboundAPI-008: abstraction the inbound API endpoint uses to determine whether
+/// this node is the active (cluster-leader) central node.
+///
+/// The design states the inbound API is "Central cluster only (active node)" and
+/// "fails over with it". A standby central node must not execute method scripts or
+/// <c>Route.To()</c> calls — that can race the active node or run against stale
+/// singleton state. <see cref="InboundApiEndpointFilter"/> consults this gate and
+/// returns HTTP 503 from a standby so Traefik/clients only reach the live node.
+///
+/// The implementation lives in the Host (it needs Akka cluster state); when no
+/// implementation is registered, the endpoint defaults to "allow" so non-clustered
+/// hosts and tests are unaffected.
+/// </summary>
+public interface IActiveNodeGate
+{
+    /// <summary>
+    /// <c>true</c> when this node is the active central node and may serve the
+    /// inbound API; <c>false</c> on a standby node.
+    /// </summary>
+    bool IsActiveNode { get; }
+}
@@ -0,0 +1,78 @@
+using Microsoft.AspNetCore.Http;
+using Microsoft.AspNetCore.Http.Features;
+using Microsoft.Extensions.DependencyInjection;
+using Microsoft.Extensions.Logging;
+using Microsoft.Extensions.Options;
+
+namespace ScadaLink.InboundAPI;
+
+/// <summary>
+/// Endpoint filter applied to <c>POST /api/{methodName}</c> that enforces two
+/// cross-cutting guards before the request handler runs:
+///
+/// <list type="bullet">
+/// <item><description>
+/// InboundAPI-008 — active-node gating. The inbound API is central-active-node-only;
+/// a standby node returns HTTP 503 so it never executes method scripts.
+/// </description></item>
+/// <item><description>
+/// InboundAPI-006 — request body size cap. Oversized bodies are rejected with HTTP
+/// 413 before being buffered into a <c>JsonDocument</c>.
+/// </description></item>
+/// </list>
+/// </summary>
+public sealed class InboundApiEndpointFilter : IEndpointFilter
+{
+    private readonly ILogger<InboundApiEndpointFilter> _logger;
+    private readonly InboundApiOptions _options;
+
+    public InboundApiEndpointFilter(
+        ILogger<InboundApiEndpointFilter> logger,
+        IOptions<InboundApiOptions> options)
+    {
+        _logger = logger;
+        _options = options.Value;
+    }
+
+    public async ValueTask<object?> InvokeAsync(
+        EndpointFilterInvocationContext context,
+        EndpointFilterDelegate next)
+    {
+        var httpContext = context.HttpContext;
+
+        // InboundAPI-008: refuse to serve the inbound API on a standby central node.
+        // The gate is optional — when no IActiveNodeGate is registered (non-clustered
+        // host / tests) the API is served, preserving prior behaviour.
+        var gate = httpContext.RequestServices.GetService<IActiveNodeGate>();
+        if (gate is { IsActiveNode: false })
+        {
+            _logger.LogWarning(
+                "Inbound API request rejected — this node is a standby (not the active central node)");
+            return Results.Json(
+                new { error = "Inbound API is only available on the active central node" },
+                statusCode: StatusCodes.Status503ServiceUnavailable);
+        }
+
+        // InboundAPI-006: cap the request body size. Reject an over-limit body up
+        // front via Content-Length; also lower the per-request max body size so a
+        // chunked/unknown-length stream is cut off by Kestrel as it is read.
+        var maxBytes = _options.MaxRequestBodyBytes;
+        if (httpContext.Request.ContentLength is { } declaredLength && declaredLength > maxBytes)
+        {
+            _logger.LogWarning(
+                "Inbound API request rejected — body length {Length} exceeds limit {Limit}",
+                declaredLength, maxBytes);
+            return Results.Json(
+                new { error = "Request body too large" },
+                statusCode: StatusCodes.Status413PayloadTooLarge);
+        }
+
+        var sizeFeature = httpContext.Features.Get<IHttpMaxRequestBodySizeFeature>();
+        if (sizeFeature is { IsReadOnly: false })
+        {
+            sizeFeature.MaxRequestBodySize = maxBytes;
+        }
+
+        return await next(context);
+    }
+}
@@ -2,5 +2,19 @@ namespace ScadaLink.InboundAPI;

 public class InboundApiOptions
 {
+    /// <summary>
+    /// Default cap on the inbound API request body, in bytes (InboundAPI-006).
+    /// </summary>
+    public const long DefaultMaxRequestBodyBytes = 1L * 1024 * 1024; // 1 MiB
+
    public TimeSpan DefaultMethodTimeout { get; set; } = TimeSpan.FromSeconds(30);
+
+    /// <summary>
+    /// InboundAPI-006: maximum accepted request body size for <c>POST /api/{methodName}</c>.
+    /// Requests whose body exceeds this are rejected with HTTP 413 before being
+    /// buffered into a <see cref="System.Text.Json.JsonDocument"/>. The inbound API
+    /// has no rate limiting (a deliberate design choice), so an explicit, modest cap
+    /// bounds per-request allocations.
+    /// </summary>
+    public long MaxRequestBodyBytes { get; set; } = DefaultMaxRequestBodyBytes;
 }
@@ -142,11 +142,17 @@ public class InboundScriptExecutor
        TimeSpan timeout,
        CancellationToken cancellationToken = default)
    {
+        // InboundAPI-004: keep the timeout source and the request-abort source
+        // separable. A single linked CTS makes a genuine client disconnect
+        // indistinguishable from a method timeout, so a normal disconnect would be
+        // logged and reported as "Script execution timed out". Use a dedicated
+        // timeout CTS, linked with the request token, so the two can be told apart.
+        using var timeoutCts = new CancellationTokenSource(timeout);
+        using var cts = CancellationTokenSource.CreateLinkedTokenSource(
+            cancellationToken, timeoutCts.Token);
+
        try
        {
-            using var cts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken);
-            cts.CancelAfter(timeout);
-
            var context = new InboundScriptContext(parameters, route, cts.Token);

            if (!_scriptHandlers.TryGetValue(method.Name, out var handler))
@@ -169,10 +175,20 @@ public class InboundScriptExecutor
            return new InboundScriptResult(true, resultJson, null);
        }
        catch (OperationCanceledException)
+        {
+            // InboundAPI-004: distinguish a genuine method timeout from a client
+            // abort. Only the timeout CTS firing is a real timeout; if the caller's
+            // request token fired, the client disconnected — do not pollute the
+            // timeout log (reserved for genuine script-execution timeouts).
+            if (timeoutCts.IsCancellationRequested && !cancellationToken.IsCancellationRequested)
            {
                _logger.LogWarning("Script execution timed out for method {Method}", method.Name);
                return new InboundScriptResult(false, null, "Script execution timed out");
            }
+
+            _logger.LogDebug("Inbound API request for method {Method} cancelled by client", method.Name);
+            return new InboundScriptResult(false, null, "Request cancelled by client");
+        }
        catch (Exception ex)
        {
            _logger.LogError(ex, "Script execution failed for method {Method}", method.Name);
@@ -10,6 +10,10 @@ public static class ServiceCollectionExtensions
        services.AddSingleton<InboundScriptExecutor>();
        services.AddScoped<RouteHelper>();

+        // InboundAPI-006 / InboundAPI-008: endpoint filter enforcing the request
+        // body size cap and active-node gating for POST /api/{methodName}.
+        services.AddSingleton<InboundApiEndpointFilter>();
+
        return services;
    }
 }
@@ -1,6 +1,7 @@
 using System.Security.Cryptography;
+using System.Text.Json;
+using System.Text.Json.Serialization;
 using Akka.Actor;
-using Newtonsoft.Json;
 using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Extensions.Logging;
 using ScadaLink.Commons.Entities.ExternalSystems;
@@ -42,6 +43,25 @@ public class ManagementActor : ReceiveActor
        Receive<ManagementEnvelope>(HandleEnvelope);
    }

+    /// <summary>
+    /// Serializer settings for command results. <see cref="ReferenceHandler.IgnoreCycles"/>
+    /// keeps EF-backed entity graphs with bidirectional navigation properties from throwing;
+    /// camelCase matches what the CLI / HTTP layer expect (finding ManagementService-007).
+    /// </summary>
+    private static readonly JsonSerializerOptions ResultSerializerOptions = new()
+    {
+        ReferenceHandler = ReferenceHandler.IgnoreCycles,
+        PropertyNamingPolicy = JsonNamingPolicy.CamelCase,
+        DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull
+    };
+
+    /// <summary>
+    /// Serializes a command result to JSON using <see cref="System.Text.Json"/> — the same
+    /// serializer the HTTP endpoint uses — with cycle-safe settings.
+    /// </summary>
+    public static string SerializeResult(object? result) =>
+        JsonSerializer.Serialize(result, ResultSerializerOptions);
+
    private void HandleEnvelope(ManagementEnvelope envelope)
    {
        var sender = Sender;
@@ -57,27 +77,42 @@ public class ManagementActor : ReceiveActor
            return;
        }

-        // Process command asynchronously with scoped DI
-        Task.Run(async () =>
+        // Process the command and pipe the mapped result back to the captured sender.
+        // PipeTo (rather than Task.Run + Tell-from-continuation) is the project's Akka.NET
+        // convention: faults are mapped in the failure continuation, no actor state is
+        // captured in the closure, and synchronous faults in command setup are still mapped.
+        ProcessCommand(envelope, user)
+            .PipeTo(sender,
+                success: result => result,
+                failure: ex => MapFault(ex, correlationId, envelope.Command));
+    }
+
+    /// <summary>
+    /// Runs a command on a scoped service provider and maps the result to a management
+    /// response message. Returns a faulted task on error so the PipeTo failure
+    /// continuation maps it uniformly.
+    /// </summary>
+    private async Task<object> ProcessCommand(ManagementEnvelope envelope, AuthenticatedUser user)
    {
        using var scope = _serviceProvider.CreateScope();
-            try
-            {
        var result = await DispatchCommand(scope.ServiceProvider, envelope.Command, user);
-                var json = JsonConvert.SerializeObject(result, Formatting.None);
-                sender.Tell(new ManagementSuccess(correlationId, json));
+        return new ManagementSuccess(envelope.CorrelationId, SerializeResult(result));
    }
-            catch (SiteScopeViolationException ex)
+
+    /// <summary>
+    /// Maps an exception from command processing to the appropriate management response.
+    /// </summary>
+    private object MapFault(Exception ex, string correlationId, object command)
    {
-                sender.Tell(new ManagementUnauthorized(correlationId, ex.Message));
-            }
-            catch (Exception ex)
-            {
-                _logger.LogError(ex, "Management command {Command} failed (CorrelationId={CorrelationId})",
-                    envelope.Command.GetType().Name, correlationId);
-                sender.Tell(new ManagementError(correlationId, ex.Message, "COMMAND_FAILED"));
-            }
-        });
+        // PipeTo wraps continuation exceptions; unwrap to find the real cause.
+        var cause = ex is AggregateException agg ? agg.Flatten().InnerException ?? ex : ex;
+
+        if (cause is SiteScopeViolationException scope)
+            return new ManagementUnauthorized(correlationId, scope.Message);
+
+        _logger.LogError(cause, "Management command {Command} failed (CorrelationId={CorrelationId})",
+            command.GetType().Name, correlationId);
+        return new ManagementError(correlationId, cause.Message, "COMMAND_FAILED");
    }

    private static string? GetRequiredRole(object command) => command switch
@@ -345,8 +380,23 @@ public class ManagementActor : ReceiveActor
    }

    /// <summary>
-    /// Helper to log an audit entry after a successful mutation.
+    /// Logs an audit entry after a successful mutation.
    /// </summary>
+    /// <remarks>
+    /// Audit-logging contract (finding ManagementService-009). Every mutating operation is
+    /// audited exactly once, by whichever layer owns the write:
+    /// <list type="bullet">
+    /// <item>Handlers that mutate a repository directly (site, area, data-connection,
+    /// external-system, notification, security, API-key, scope-rule) call this helper
+    /// explicitly after the successful change.</item>
+    /// <item>Handlers that delegate to a domain service (<c>TemplateService</c>,
+    /// <c>SharedScriptService</c>, <c>InstanceService</c>, <c>AreaService</c>,
+    /// <c>SiteService</c>, <c>TemplateFolderService</c>, <c>DeploymentService</c>,
+    /// <c>ArtifactDeploymentService</c>) do NOT call this helper — those services own their
+    /// own <see cref="IAuditService"/> dependency and audit internally. Adding an explicit
+    /// <see cref="AuditAsync"/> call in such a handler would double-log.</item>
+    /// </list>
+    /// </remarks>
    private static async Task AuditAsync(IServiceProvider sp, string user, string action, string entityType, string entityId, string entityName, object? afterState)
    {
        var auditService = sp.GetRequiredService<IAuditService>();
@@ -77,46 +77,19 @@ public static class ManagementEndpoints
            permittedSiteIds);

        // 4. Parse command from request body
-        JsonDocument doc;
-        try
+        string body;
+        using (var reader = new StreamReader(context.Request.Body, Encoding.UTF8))
        {
-            doc = await JsonDocument.ParseAsync(context.Request.Body);
-        }
-        catch
-        {
-            return Results.Json(new { error = "Invalid JSON body.", code = "BAD_REQUEST" }, statusCode: 400);
+            body = await reader.ReadToEndAsync();
        }

-        if (!doc.RootElement.TryGetProperty("command", out var commandNameElement))
+        var parse = ParseCommand(body);
+        if (!parse.Success)
        {
-            return Results.Json(new { error = "Missing 'command' field.", code = "BAD_REQUEST" }, statusCode: 400);
+            return Results.Json(new { error = parse.ErrorMessage, code = parse.ErrorCode }, statusCode: 400);
        }

-        var commandName = commandNameElement.GetString();
-        if (string.IsNullOrWhiteSpace(commandName))
-        {
-            return Results.Json(new { error = "Empty 'command' field.", code = "BAD_REQUEST" }, statusCode: 400);
-        }
-
-        var commandType = ManagementCommandRegistry.Resolve(commandName);
-        if (commandType == null)
-        {
-            return Results.Json(new { error = $"Unknown command: '{commandName}'.", code = "BAD_REQUEST" }, statusCode: 400);
-        }
-
-        object command;
-        try
-        {
-            var payloadElement = doc.RootElement.TryGetProperty("payload", out var p)
-                ? p
-                : JsonDocument.Parse("{}").RootElement;
-            command = JsonSerializer.Deserialize(payloadElement.GetRawText(), commandType,
-                new JsonSerializerOptions { PropertyNameCaseInsensitive = true })!;
-        }
-        catch (Exception ex)
-        {
-            return Results.Json(new { error = $"Failed to deserialize payload: {ex.Message}", code = "BAD_REQUEST" }, statusCode: 400);
-        }
+        var command = parse.Command!;

        // 5. Dispatch to ManagementActor
        var holder = context.RequestServices.GetRequiredService<ManagementActorHolder>();
@@ -148,4 +121,68 @@ public static class ManagementEndpoints
            _ => Results.Json(new { error = "Unexpected response.", code = "INTERNAL_ERROR" }, statusCode: 500)
        };
    }
+
+    /// <summary>
+    /// Result of parsing a management request body into a strongly-typed command.
+    /// </summary>
+    public readonly record struct CommandParseResult(
+        bool Success, object? Command, string? ErrorMessage, string? ErrorCode)
+    {
+        public static CommandParseResult Ok(object command) => new(true, command, null, null);
+        public static CommandParseResult Fail(string message) => new(false, null, message, "BAD_REQUEST");
+    }
+
+    /// <summary>
+    /// Parses a management request body — a JSON object with a <c>command</c> name and an
+    /// optional <c>payload</c> — into the strongly-typed command record. The parsed
+    /// <see cref="JsonDocument"/> is disposed deterministically and the missing-payload
+    /// case does not allocate a throwaway document (finding ManagementService-006).
+    /// </summary>
+    public static CommandParseResult ParseCommand(string body)
+    {
+        using JsonDocument doc = ParseDocument(body, out var parseError);
+        if (parseError != null)
+            return CommandParseResult.Fail(parseError);
+
+        if (!doc.RootElement.TryGetProperty("command", out var commandNameElement))
+            return CommandParseResult.Fail("Missing 'command' field.");
+
+        var commandName = commandNameElement.GetString();
+        if (string.IsNullOrWhiteSpace(commandName))
+            return CommandParseResult.Fail("Empty 'command' field.");
+
+        var commandType = ManagementCommandRegistry.Resolve(commandName);
+        if (commandType == null)
+            return CommandParseResult.Fail($"Unknown command: '{commandName}'.");
+
+        try
+        {
+            // Missing payload: deserialize from the empty-object literal rather than
+            // allocating (and leaking) a throwaway JsonDocument.
+            var payloadJson = doc.RootElement.TryGetProperty("payload", out var p)
+                ? p.GetRawText()
+                : "{}";
+            var command = JsonSerializer.Deserialize(payloadJson, commandType,
+                new JsonSerializerOptions { PropertyNameCaseInsensitive = true })!;
+            return CommandParseResult.Ok(command);
+        }
+        catch (Exception ex)
+        {
+            return CommandParseResult.Fail($"Failed to deserialize payload: {ex.Message}");
+        }
+    }
+
+    private static JsonDocument ParseDocument(string body, out string? error)
+    {
+        try
+        {
+            error = null;
+            return JsonDocument.Parse(body);
+        }
+        catch
+        {
+            error = "Invalid JSON body.";
+            return JsonDocument.Parse("{}");
+        }
+    }
 }
@@ -0,0 +1,48 @@
+namespace ScadaLink.NotificationService;
+
+/// <summary>
+/// NS-009: Scrubs SMTP credential secrets out of free text (typically exception
+/// messages echoed back by an SMTP server) before that text is written to a log.
+/// MailKit authentication exceptions can contain server responses that quote the
+/// supplied credentials; this prevents a password, client secret, or OAuth2 token
+/// from leaking into the operational logs.
+/// </summary>
+internal static class CredentialRedactor
+{
+    private const string Mask = "***REDACTED***";
+
+    /// <summary>
+    /// Returns <paramref name="text"/> with every secret component of the supplied
+    /// colon-delimited credential string masked.
+    /// </summary>
+    /// <param name="text">The text to scrub (e.g. an exception message).</param>
+    /// <param name="credentials">
+    /// The credential string in use — Basic Auth <c>user:pass</c> or OAuth2
+    /// <c>tenantId:clientId:clientSecret</c>. May be null.
+    /// </param>
+    public static string Scrub(string? text, string? credentials)
+    {
+        if (string.IsNullOrEmpty(text) || string.IsNullOrEmpty(credentials))
+        {
+            return text ?? string.Empty;
+        }
+
+        var result = text;
+
+        // Mask each individual colon-delimited component (covers user, password,
+        // tenant, clientId, clientSecret) and the whole packed string. Order longest
+        // first so a component that is a substring of another is still fully masked.
+        var parts = credentials.Split(':')
+            .Where(p => p.Length >= 4)
+            .Append(credentials)
+            .Distinct()
+            .OrderByDescending(p => p.Length);
+
+        foreach (var part in parts)
+        {
+            result = result.Replace(part, Mask, StringComparison.Ordinal);
+        }
+
+        return result;
+    }
+}
@@ -5,7 +5,24 @@ namespace ScadaLink.NotificationService;
 /// </summary>
 public interface ISmtpClientWrapper
 {
-    Task ConnectAsync(string host, int port, bool useTls, CancellationToken cancellationToken = default);
+    /// <summary>
+    /// Connects to the SMTP server.
+    /// </summary>
+    /// <param name="tlsMode">
+    /// NS-005: explicit three-state TLS mode (None/StartTls/Ssl) — replaces the old
+    /// <c>bool useTls</c> which could not represent implicit-SSL and silently fell
+    /// back to opportunistic negotiation for non-StartTLS configurations.
+    /// </param>
+    /// <param name="connectionTimeoutSeconds">
+    /// NS-007: SMTP connection/operation timeout in seconds. A non-positive value
+    /// leaves the client's default timeout in place.
+    /// </param>
+    Task ConnectAsync(
+        string host,
+        int port,
+        SmtpTlsMode tlsMode,
+        int connectionTimeoutSeconds,
+        CancellationToken cancellationToken = default);
    Task AuthenticateAsync(string authType, string? credentials, CancellationToken cancellationToken = default);
    Task SendAsync(string from, IEnumerable<string> bccRecipients, string subject, string body, CancellationToken cancellationToken = default);
    Task DisconnectAsync(CancellationToken cancellationToken = default);
@@ -13,9 +13,33 @@ public class MailKitSmtpClientWrapper : ISmtpClientWrapper, IDisposable
 {
    private readonly SmtpClient _client = new();

-    public async Task ConnectAsync(string host, int port, bool useTls, CancellationToken cancellationToken = default)
+    public async Task ConnectAsync(
+        string host,
+        int port,
+        SmtpTlsMode tlsMode,
+        int connectionTimeoutSeconds,
+        CancellationToken cancellationToken = default)
    {
-        var secureSocket = useTls ? SecureSocketOptions.StartTls : SecureSocketOptions.Auto;
+        // NS-005: map the explicit three-state TLS mode onto MailKit's socket
+        // options. The old code collapsed everything to a boolean and used
+        // SecureSocketOptions.Auto for the non-StartTLS case, which let MailKit
+        // opportunistically negotiate TLS even when "None" was configured and
+        // gave SSL-on-connect no representation at all.
+        var secureSocket = tlsMode switch
+        {
+            SmtpTlsMode.None => SecureSocketOptions.None,
+            SmtpTlsMode.StartTls => SecureSocketOptions.StartTls,
+            SmtpTlsMode.Ssl => SecureSocketOptions.SslOnConnect,
+            _ => throw new ArgumentOutOfRangeException(nameof(tlsMode), tlsMode, "Unknown TLS mode."),
+        };
+
+        // NS-007: honour the configured connection timeout. SmtpClient.Timeout is
+        // in milliseconds and applies to connect/auth/send operations.
+        if (connectionTimeoutSeconds > 0)
+        {
+            _client.Timeout = connectionTimeoutSeconds * 1000;
+        }
+
        await _client.ConnectAsync(host, port, secureSocket, cancellationToken);
    }

@@ -3,6 +3,7 @@ using System.Text.Json;
 using MailKit;
 using MailKit.Net.Smtp;
 using Microsoft.Extensions.Logging;
+using MimeKit;
 using ScadaLink.Commons.Entities.Notifications;
 using ScadaLink.Commons.Interfaces.Repositories;
 using ScadaLink.Commons.Interfaces.Services;
@@ -68,6 +69,30 @@ public class NotificationDeliveryService : INotificationDeliveryService
            return new NotificationResult(false, "No SMTP configuration available");
        }

+        // NS-005: validate the configured TLS mode up front — an unknown value is a
+        // configuration error and must surface as a clean result, not a silent
+        // fallback to opportunistic TLS negotiation.
+        try
+        {
+            SmtpTlsModeParser.Parse(smtpConfig.TlsMode);
+        }
+        catch (ArgumentException ex)
+        {
+            _logger.LogError("Invalid SMTP TLS mode for list {List}: {Reason}", listName, ex.Message);
+            return new NotificationResult(false, ex.Message);
+        }
+
+        // NS-008: validate every email address before attempting delivery. A single
+        // malformed address previously caused MailboxAddress.Parse to throw a
+        // ParseException that escaped SendAsync unhandled; it must instead produce a
+        // clean NotificationResult the calling script can handle.
+        var addressError = ValidateAddresses(smtpConfig.FromAddress, recipients);
+        if (addressError != null)
+        {
+            _logger.LogWarning("Notification to list {List} has invalid addresses: {Reason}", listName, addressError);
+            return new NotificationResult(false, addressError);
+        }
+
        try
        {
            await DeliverAsync(smtpConfig, recipients, subject, message, cancellationToken);
@@ -75,9 +100,13 @@ public class NotificationDeliveryService : INotificationDeliveryService
        }
        catch (SmtpPermanentException ex)
        {
-            // WP-12: Permanent SMTP failure — returned to script
-            _logger.LogError(ex, "Permanent SMTP failure sending to list {List}", listName);
-            return new NotificationResult(false, $"Permanent SMTP error: {ex.Message}");
+            // WP-12: Permanent SMTP failure — returned to script.
+            // NS-009: scrub credential fragments out of the server-supplied message
+            // before logging or returning it.
+            var detail = CredentialRedactor.Scrub(ex.Message, smtpConfig.Credentials);
+            _logger.LogError(
+                "Permanent SMTP failure sending to list {List}: {Detail}", listName, detail);
+            return new NotificationResult(false, $"Permanent SMTP error: {detail}");
        }
        catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested)
        {
@@ -86,8 +115,11 @@ public class NotificationDeliveryService : INotificationDeliveryService
        }
        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);
+            // WP-12: Transient SMTP failure — hand to S&F.
+            // NS-009: scrub credential fragments before logging.
+            _logger.LogWarning(
+                "Transient SMTP failure sending to list {List} ({ExceptionType}): {Detail}; buffering for retry",
+                listName, ex.GetType().Name, CredentialRedactor.Scrub(ex.Message, smtpConfig.Credentials));

            if (_storeAndForward == null)
            {
@@ -155,6 +187,30 @@ public class NotificationDeliveryService : INotificationDeliveryService
            return false;
        }

+        // NS-005: an unknown TLS mode is a configuration error that retrying cannot
+        // fix — park the buffered message rather than throwing on every sweep.
+        try
+        {
+            SmtpTlsModeParser.Parse(smtpConfig.TlsMode);
+        }
+        catch (ArgumentException ex)
+        {
+            _logger.LogError(
+                "Buffered notification to list '{List}' cannot be delivered — {Reason}; parking.",
+                payload.ListName, ex.Message);
+            return false;
+        }
+
+        // NS-008: a malformed address cannot be fixed by retrying — park it.
+        var addressError = ValidateAddresses(smtpConfig.FromAddress, recipients);
+        if (addressError != null)
+        {
+            _logger.LogError(
+                "Buffered notification to list '{List}' has invalid addresses ({Reason}); parking.",
+                payload.ListName, addressError);
+            return false;
+        }
+
        try
        {
            await DeliverAsync(smtpConfig, recipients, payload.Subject, payload.Message, cancellationToken);
@@ -162,7 +218,10 @@ public class NotificationDeliveryService : INotificationDeliveryService
        }
        catch (SmtpPermanentException ex)
        {
-            _logger.LogError(ex, "Buffered notification to list '{List}' failed permanently; parking.", payload.ListName);
+            // NS-009: scrub credential fragments out of the message before logging.
+            _logger.LogError(
+                "Buffered notification to list '{List}' failed permanently ({Detail}); parking.",
+                payload.ListName, CredentialRedactor.Scrub(ex.Message, smtpConfig.Credentials));
            return false;
        }
        // Transient SMTP errors propagate out of DeliverAsync — the S&F engine retries.
@@ -171,7 +230,55 @@ public class NotificationDeliveryService : INotificationDeliveryService
    private sealed record BufferedNotification(string ListName, string Subject, string Message);

    /// <summary>
-    /// Delivers an email via SMTP. Throws on failure.
+    /// NS-007: throttles concurrent SMTP deliveries to the configured
+    /// <c>MaxConcurrentConnections</c>. Created lazily from the first SMTP config
+    /// seen (one SMTP config is deployed per site, so the limit is stable).
+    /// </summary>
+    private SemaphoreSlim? _concurrencyLimiter;
+    private readonly object _limiterLock = new();
+
+    private SemaphoreSlim GetConcurrencyLimiter(SmtpConfiguration config)
+    {
+        if (_concurrencyLimiter != null)
+        {
+            return _concurrencyLimiter;
+        }
+
+        lock (_limiterLock)
+        {
+            // NS-007: a non-positive configured value would make SemaphoreSlim
+            // throw; fall back to the design-doc default of 5.
+            var max = config.MaxConcurrentConnections > 0 ? config.MaxConcurrentConnections : 5;
+            _concurrencyLimiter ??= new SemaphoreSlim(max, max);
+            return _concurrencyLimiter;
+        }
+    }
+
+    /// <summary>
+    /// NS-008: Validates the sender and recipient email addresses, returning a
+    /// human-readable error string if any is malformed, or null if all parse.
+    /// </summary>
+    internal static string? ValidateAddresses(
+        string fromAddress, IReadOnlyList<NotificationRecipient> recipients)
+    {
+        if (!MailboxAddress.TryParse(fromAddress, out _))
+        {
+            return $"Invalid sender (from) email address: '{fromAddress}'";
+        }
+
+        var invalid = recipients
+            .Where(r => !MailboxAddress.TryParse(r.EmailAddress, out _))
+            .Select(r => r.EmailAddress)
+            .ToList();
+
+        return invalid.Count > 0
+            ? $"Invalid recipient email address(es): {string.Join(", ", invalid)}"
+            : null;
+    }
+
+    /// <summary>
+    /// Delivers an email via SMTP. Throws on failure (transient errors and
+    /// <see cref="SmtpPermanentException"/> propagate; the caller classifies them).
    /// </summary>
    internal async Task DeliverAsync(
        SmtpConfiguration config,
@@ -180,16 +287,23 @@ public class NotificationDeliveryService : INotificationDeliveryService
        string body,
        CancellationToken cancellationToken)
    {
+        var tlsMode = SmtpTlsModeParser.Parse(config.TlsMode);
+
+        // NS-007: bound the number of concurrent SMTP connections per site.
+        var limiter = GetConcurrencyLimiter(config);
+        await limiter.WaitAsync(cancellationToken);
+
        // NS-004: create exactly one client and dispose the one actually used.
        var smtp = _smtpClientFactory();
        using var disposable = smtp as IDisposable;

        try
        {
-            var useTls = config.TlsMode?.Equals("starttls", StringComparison.OrdinalIgnoreCase) == true;
-            await smtp.ConnectAsync(config.Host, config.Port, useTls, cancellationToken);
+            // NS-005/NS-007: explicit TLS mode and the configured connection timeout.
+            await smtp.ConnectAsync(
+                config.Host, config.Port, tlsMode, config.ConnectionTimeoutSeconds, cancellationToken);

-            // Resolve credentials (OAuth2 token refresh if needed)
+            // Resolve credentials (OAuth2 token fetched/cached by the token service).
            var credentials = config.Credentials;
            if (config.AuthType.Equals("oauth2", StringComparison.OrdinalIgnoreCase) && _tokenService != null && credentials != null)
            {
@@ -218,6 +332,11 @@ public class NotificationDeliveryService : INotificationDeliveryService
        }
        // Transient and SmtpPermanentException both propagate unchanged: SendAsync's
        // catch filters (SmtpPermanentException / IsTransientSmtpError) handle them.
+        finally
+        {
+            // NS-007: always release the concurrency slot, even on failure.
+            limiter.Release();
+        }
    }

    private enum SmtpErrorClass
@@ -1,3 +1,6 @@
+using System.Collections.Concurrent;
+using System.Security.Cryptography;
+using System.Text;
 using System.Text.Json;
 using Microsoft.Extensions.Logging;

@@ -6,14 +9,18 @@ namespace ScadaLink.NotificationService;
 /// <summary>
 /// WP-11: OAuth2 Client Credentials token lifecycle — fetch, cache, refresh on expiry.
 /// Used for Microsoft 365 SMTP authentication.
+/// NS-006: tokens are cached per credential identity (tenant/client/secret), so a
+/// second SMTP configuration with different credentials never receives the first
+/// configuration's token.
 /// </summary>
 public class OAuth2TokenService
 {
    private readonly IHttpClientFactory _httpClientFactory;
    private readonly ILogger<OAuth2TokenService> _logger;
-    private string? _cachedToken;
-    private DateTimeOffset _tokenExpiry = DateTimeOffset.MinValue;
-    private readonly SemaphoreSlim _lock = new(1, 1);
+
+    // NS-006: cache keyed by a hash of the credential string. Each distinct
+    // tenant/client/secret triple gets its own cached token and its own lock.
+    private readonly ConcurrentDictionary<string, CacheEntry> _cache = new();

    public OAuth2TokenService(
        IHttpClientFactory httpClientFactory,
@@ -29,18 +36,21 @@ public class OAuth2TokenService
    /// </summary>
    public async Task<string> GetTokenAsync(string credentials, CancellationToken cancellationToken = default)
    {
-        if (_cachedToken != null && DateTimeOffset.UtcNow < _tokenExpiry)
+        var key = CredentialKey(credentials);
+        var entry = _cache.GetOrAdd(key, _ => new CacheEntry());
+
+        if (entry.Token != null && DateTimeOffset.UtcNow < entry.Expiry)
        {
-            return _cachedToken;
+            return entry.Token;
        }

-        await _lock.WaitAsync(cancellationToken);
+        await entry.Lock.WaitAsync(cancellationToken);
        try
        {
-            // Double-check after acquiring lock
-            if (_cachedToken != null && DateTimeOffset.UtcNow < _tokenExpiry)
+            // Double-check after acquiring the per-credential lock.
+            if (entry.Token != null && DateTimeOffset.UtcNow < entry.Expiry)
            {
-                return _cachedToken;
+                return entry.Token;
            }

            var parts = credentials.Split(':', 3);
@@ -70,18 +80,39 @@ public class OAuth2TokenService
            var json = await response.Content.ReadAsStringAsync(cancellationToken);
            using var doc = JsonDocument.Parse(json);

-            _cachedToken = doc.RootElement.GetProperty("access_token").GetString()
+            var token = doc.RootElement.GetProperty("access_token").GetString()
                ?? throw new InvalidOperationException("No access_token in OAuth2 response");

            var expiresIn = doc.RootElement.GetProperty("expires_in").GetInt32();
-            _tokenExpiry = DateTimeOffset.UtcNow.AddSeconds(expiresIn - 60); // Refresh 60s before expiry
+            entry.Token = token;
+            entry.Expiry = DateTimeOffset.UtcNow.AddSeconds(expiresIn - 60); // Refresh 60s before expiry

-            _logger.LogInformation("OAuth2 token refreshed, expires in {ExpiresIn}s", expiresIn);
-            return _cachedToken;
+            // NS-009: the token endpoint identity is logged by tenant only — never
+            // the client secret or the access token itself.
+            _logger.LogInformation(
+                "OAuth2 token refreshed for tenant {Tenant}, expires in {ExpiresIn}s", tenantId, expiresIn);
+            return token;
        }
        finally
        {
-            _lock.Release();
+            entry.Lock.Release();
        }
    }
+
+    /// <summary>
+    /// NS-006: a stable, non-reversible key for the credential string so the cache
+    /// is partitioned by credential identity without holding the secret as a key.
+    /// </summary>
+    private static string CredentialKey(string credentials)
+    {
+        var hash = SHA256.HashData(Encoding.UTF8.GetBytes(credentials));
+        return Convert.ToHexString(hash);
+    }
+
+    private sealed class CacheEntry
+    {
+        public string? Token;
+        public DateTimeOffset Expiry = DateTimeOffset.MinValue;
+        public readonly SemaphoreSlim Lock = new(1, 1);
+    }
 }
@@ -0,0 +1,49 @@
+namespace ScadaLink.NotificationService;
+
+/// <summary>
+/// NS-005: The three TLS modes the design doc defines for SMTP connections.
+/// A single boolean cannot represent the requirement, so the configured
+/// <c>SmtpConfiguration.TlsMode</c> string is parsed into this three-state enum.
+/// </summary>
+public enum SmtpTlsMode
+{
+    /// <summary>No transport security — plain SMTP. Maps to <c>SecureSocketOptions.None</c>.</summary>
+    None,
+
+    /// <summary>Opportunistic STARTTLS upgrade (typically port 587). Maps to <c>SecureSocketOptions.StartTls</c>.</summary>
+    StartTls,
+
+    /// <summary>Implicit TLS / SSL-on-connect (typically port 465). Maps to <c>SecureSocketOptions.SslOnConnect</c>.</summary>
+    Ssl,
+}
+
+/// <summary>
+/// NS-005: Parses the free-text <c>SmtpConfiguration.TlsMode</c> value into a
+/// <see cref="SmtpTlsMode"/>, rejecting unknown values rather than silently
+/// falling back to opportunistic negotiation.
+/// </summary>
+public static class SmtpTlsModeParser
+{
+    /// <summary>
+    /// Parses a configured TLS mode string. A null or empty value defaults to
+    /// <see cref="SmtpTlsMode.StartTls"/> (the design-doc default for port 587).
+    /// </summary>
+    /// <exception cref="ArgumentException">The value is not one of None/StartTLS/SSL.</exception>
+    public static SmtpTlsMode Parse(string? tlsMode)
+    {
+        if (string.IsNullOrWhiteSpace(tlsMode))
+        {
+            return SmtpTlsMode.StartTls;
+        }
+
+        return tlsMode.Trim().ToLowerInvariant() switch
+        {
+            "none" => SmtpTlsMode.None,
+            "starttls" => SmtpTlsMode.StartTls,
+            "ssl" => SmtpTlsMode.Ssl,
+            _ => throw new ArgumentException(
+                $"Unknown SMTP TLS mode '{tlsMode}'. Expected one of: None, StartTLS, SSL.",
+                nameof(tlsMode)),
+        };
+    }
+}
@@ -18,6 +18,17 @@ public class JwtTokenService
    public const string SiteIdClaimType = "SiteId";
    public const string LastActivityClaimType = "LastActivity";

+    /// <summary>
+    /// Fixed issuer bound into every token and required on validation. Binding
+    /// issuer/audience is defence-in-depth: even though the HMAC key is shared only
+    /// between the two central nodes, accidental reuse of the same secret for an
+    /// unrelated internal token would otherwise be silently exploitable.
+    /// </summary>
+    public const string TokenIssuer = "scadalink-central";
+
+    /// <summary>Fixed audience bound into every token and required on validation.</summary>
+    public const string TokenAudience = "scadalink-central";
+
    public JwtTokenService(IOptions<SecurityOptions> options, ILogger<JwtTokenService> logger)
    {
        _options = options?.Value ?? throw new ArgumentNullException(nameof(options));
@@ -37,11 +48,19 @@ public class JwtTokenService
        }
    }

+    /// <summary>
+    /// Issues a fresh JWT. <paramref name="lastActivity"/> sets the idle-timeout
+    /// anchor; when omitted (a brand-new login) it defaults to now. On a token
+    /// refresh the caller MUST pass the existing anchor forward so the idle window
+    /// continues to be measured from the user's last genuine activity rather than
+    /// from token issuance time.
+    /// </summary>
    public string GenerateToken(
        string displayName,
        string username,
        IReadOnlyList<string> roles,
-        IReadOnlyList<string>? permittedSiteIds)
+        IReadOnlyList<string>? permittedSiteIds,
+        DateTimeOffset? lastActivity = null)
    {
        var key = new SymmetricSecurityKey(Encoding.UTF8.GetBytes(_options.JwtSigningKey));
        var credentials = new SigningCredentials(key, SecurityAlgorithms.HmacSha256);
@@ -50,7 +69,7 @@ public class JwtTokenService
        {
            new(DisplayNameClaimType, displayName),
            new(UsernameClaimType, username),
-            new(LastActivityClaimType, DateTimeOffset.UtcNow.ToString("o"))
+            new(LastActivityClaimType, (lastActivity ?? DateTimeOffset.UtcNow).ToString("o"))
        };

        foreach (var role in roles)
@@ -67,6 +86,8 @@ public class JwtTokenService
        }

        var token = new JwtSecurityToken(
+            issuer: TokenIssuer,
+            audience: TokenAudience,
            claims: claims,
            expires: DateTime.UtcNow.AddMinutes(_options.JwtExpiryMinutes),
            signingCredentials: credentials);
@@ -79,8 +100,10 @@ public class JwtTokenService
        var key = new SymmetricSecurityKey(Encoding.UTF8.GetBytes(_options.JwtSigningKey));
        var validationParameters = new TokenValidationParameters
        {
-            ValidateIssuer = false,
-            ValidateAudience = false,
+            ValidateIssuer = true,
+            ValidIssuer = TokenIssuer,
+            ValidateAudience = true,
+            ValidAudience = TokenAudience,
            ValidateLifetime = true,
            ValidateIssuerSigningKey = true,
            IssuerSigningKey = key,
@@ -121,6 +144,15 @@ public class JwtTokenService
        return (DateTimeOffset.UtcNow - lastActivity).TotalMinutes > _options.IdleTimeoutMinutes;
    }

+    /// <summary>
+    /// Issues a fresh token (new expiry, re-queried roles) while <b>preserving</b> the
+    /// existing <see cref="LastActivityClaimType"/> anchor. A refresh is itself
+    /// triggered by a request, but it must not be treated as user activity — the
+    /// idle window must keep being measured from the user's last genuine interaction,
+    /// otherwise the documented 30-minute idle timeout could never fire for a client
+    /// that polls in the background. Call <see cref="RecordActivity"/> to advance the
+    /// anchor when handling a genuine user request.
+    /// </summary>
    public string? RefreshToken(ClaimsPrincipal currentPrincipal, IReadOnlyList<string> currentRoles, IReadOnlyList<string>? permittedSiteIds)
    {
        var displayName = currentPrincipal.FindFirst(DisplayNameClaimType)?.Value;
@@ -132,6 +164,36 @@ public class JwtTokenService
            return null;
        }

-        return GenerateToken(displayName, username, currentRoles, permittedSiteIds);
+        return GenerateToken(displayName, username, currentRoles, permittedSiteIds,
+            ReadLastActivity(currentPrincipal));
+    }
+
+    /// <summary>
+    /// Issues a fresh token whose <see cref="LastActivityClaimType"/> anchor is
+    /// advanced to now. This is the explicit "user did something" path — distinct
+    /// from <see cref="RefreshToken"/> — to be called by the request pipeline when
+    /// handling a genuine user interaction.
+    /// </summary>
+    public string? RecordActivity(ClaimsPrincipal currentPrincipal, IReadOnlyList<string> currentRoles, IReadOnlyList<string>? permittedSiteIds)
+    {
+        var displayName = currentPrincipal.FindFirst(DisplayNameClaimType)?.Value;
+        var username = currentPrincipal.FindFirst(UsernameClaimType)?.Value;
+
+        if (displayName == null || username == null)
+        {
+            _logger.LogWarning("Cannot record activity: missing DisplayName or Username claims");
+            return null;
+        }
+
+        return GenerateToken(displayName, username, currentRoles, permittedSiteIds,
+            DateTimeOffset.UtcNow);
+    }
+
+    private static DateTimeOffset? ReadLastActivity(ClaimsPrincipal principal)
+    {
+        var claim = principal.FindFirst(LastActivityClaimType);
+        return claim != null && DateTimeOffset.TryParse(claim.Value, out var value)
+            ? value
+            : null;
    }
 }
@@ -71,7 +71,7 @@ public class LdapAuthService

            try
            {
-                var searchFilter = $"(uid={EscapeLdapFilter(username)})";
+                var searchFilter = $"({_options.LdapUserIdAttribute}={EscapeLdapFilter(username)})";
                var searchResults = await Task.Run(() =>
                    connection.Search(
                        _options.LdapSearchBase,
@@ -133,17 +133,13 @@ public class LdapAuthService
    /// </summary>
    private async Task<string> ResolveUserDnAsync(LdapConnection connection, string username, CancellationToken ct)
    {
-        // If username already looks like a DN, use it as-is
-        if (username.Contains('='))
-            return username;
-
        // If a service account is configured, search for the user's actual DN
        if (!string.IsNullOrWhiteSpace(_options.LdapServiceAccountDn))
        {
            await Task.Run(() =>
                connection.Bind(_options.LdapServiceAccountDn, _options.LdapServiceAccountPassword), ct);

-            var searchFilter = $"(uid={EscapeLdapFilter(username)})";
+            var searchFilter = $"({_options.LdapUserIdAttribute}={EscapeLdapFilter(username)})";
            var searchResults = await Task.Run(() =>
                connection.Search(
                    _options.LdapSearchBase,
@@ -158,13 +154,68 @@ public class LdapAuthService
                return entry.Dn;
            }

-            throw new LdapException("User not found", LdapException.NoSuchObject, $"No entry found for uid={username}");
+            throw new LdapException("User not found", LdapException.NoSuchObject,
+                $"No entry found for {_options.LdapUserIdAttribute}={username}");
        }

-        // Fallback: construct DN directly
-        return string.IsNullOrWhiteSpace(_options.LdapSearchBase)
-            ? $"cn={username}"
-            : $"cn={username},{_options.LdapSearchBase}";
+        // Fallback: construct the bind DN directly from the configured user-id
+        // attribute. The username is RFC 4514 DN-escaped so it cannot alter the
+        // DN structure (Security-005). The previous Contains('=') shortcut that
+        // accepted a raw caller-supplied DN has been removed — accepting an
+        // arbitrary DN from untrusted input let a client choose the bind identity.
+        return BuildFallbackUserDn(username, _options.LdapSearchBase, _options.LdapUserIdAttribute);
+    }
+
+    /// <summary>
+    /// Builds the no-service-account fallback bind DN as
+    /// <c>{userIdAttribute}={escaped-username}[,{searchBase}]</c>. The username is
+    /// escaped per RFC 4514 so DN metacharacters in untrusted input cannot inject
+    /// additional RDN components or change the bind identity.
+    /// </summary>
+    public static string BuildFallbackUserDn(string username, string searchBase, string userIdAttribute)
+    {
+        var rdn = $"{userIdAttribute}={EscapeLdapDn(username)}";
+        return string.IsNullOrWhiteSpace(searchBase) ? rdn : $"{rdn},{searchBase}";
+    }
+
+    /// <summary>
+    /// Escapes a string for use as an RFC 4514 DN attribute value: the special
+    /// characters <c>, + " \ &lt; &gt; ;</c> are backslash-escaped, as are a leading
+    /// or trailing space and a leading <c>#</c>.
+    /// </summary>
+    public static string EscapeLdapDn(string input)
+    {
+        if (string.IsNullOrEmpty(input))
+            return input;
+
+        var sb = new System.Text.StringBuilder(input.Length + 8);
+        for (var i = 0; i < input.Length; i++)
+        {
+            var c = input[i];
+            var isEdgeSpace = c == ' ' && (i == 0 || i == input.Length - 1);
+            var isLeadingHash = c == '#' && i == 0;
+            switch (c)
+            {
+                case ',':
+                case '+':
+                case '"':
+                case '\\':
+                case '<':
+                case '>':
+                case ';':
+                    sb.Append('\\').Append(c);
+                    break;
+                case '\0':
+                    sb.Append("\\00");
+                    break;
+                default:
+                    if (isEdgeSpace || isLeadingHash)
+                        sb.Append('\\');
+                    sb.Append(c);
+                    break;
+            }
+        }
+        return sb.ToString();
    }

    private static string EscapeLdapFilter(string input)
@@ -37,10 +37,19 @@ public class SecurityOptions
    /// <summary>
    /// Service account DN for LDAP user searches (e.g., "cn=admin,dc=example,dc=com").
    /// Required for search-then-bind authentication. If empty, direct bind with
-    /// cn={username},{LdapSearchBase} is attempted instead.
+    /// {LdapUserIdAttribute}={username},{LdapSearchBase} is attempted instead.
    /// </summary>
    public string LdapServiceAccountDn { get; set; } = string.Empty;

+    /// <summary>
+    /// LDAP attribute that identifies a user. Used both for the search-then-bind
+    /// filter (<c>({LdapUserIdAttribute}={username})</c>) and for constructing the
+    /// fallback bind DN when no service account is configured, so the two
+    /// authentication modes are interchangeable. Common values: <c>uid</c> (OpenLDAP),
+    /// <c>sAMAccountName</c> (Active Directory).
+    /// </summary>
+    public string LdapUserIdAttribute { get; set; } = "uid";
+
    /// <summary>
    /// Service account password for LDAP user searches.
    /// </summary>
@@ -5,15 +5,32 @@ using Microsoft.Extensions.Options;
 namespace ScadaLink.DeploymentManager.Tests;

 /// <summary>
-/// DeploymentManager-008: DeploymentManagerOptions must be bound to the
-/// "ScadaLink:DeploymentManager" configuration section, consistent with the
-/// Options-pattern convention used by the other components.
+/// DeploymentManager-008: DeploymentManagerOptions must be resolvable via the
+/// Options pattern and bindable to the "ScadaLink:DeploymentManager"
+/// configuration section. The component itself does not depend on
+/// IConfiguration (enforced by Host's OptionsTests) — the Host binds the
+/// section; AddDeploymentManager only guarantees IOptions resolvability.
 /// </summary>
 public class ServiceCollectionExtensionsTests
 {
    [Fact]
-    public void AddDeploymentManager_WithConfiguration_BindsDeploymentManagerOptions()
+    public void AddDeploymentManager_RegistersResolvableOptions_WithDefaults()
    {
+        var services = new ServiceCollection();
+        services.AddDeploymentManager();
+
+        using var provider = services.BuildServiceProvider();
+        var options = provider.GetRequiredService<IOptions<DeploymentManagerOptions>>().Value;
+
+        // No section bound -> the option-class defaults are retained.
+        Assert.Equal(TimeSpan.FromSeconds(5), options.OperationLockTimeout);
+    }
+
+    [Fact]
+    public void AddDeploymentManager_OptionsBindToConfigurationSection_AsTheHostWires()
+    {
+        // Mirrors the Host wiring: the Host calls Configure<DeploymentManagerOptions>
+        // against OptionsSection, then AddDeploymentManager().
        var configuration = new ConfigurationBuilder()
            .AddInMemoryCollection(new Dictionary<string, string?>
            {
@@ -23,7 +40,9 @@ public class ServiceCollectionExtensionsTests
            .Build();

        var services = new ServiceCollection();
-        services.AddDeploymentManager(configuration);
+        services.Configure<DeploymentManagerOptions>(
+            configuration.GetSection(ServiceCollectionExtensions.OptionsSection));
+        services.AddDeploymentManager();

        using var provider = services.BuildServiceProvider();
        var options = provider.GetRequiredService<IOptions<DeploymentManagerOptions>>().Value;
@@ -33,17 +52,8 @@ public class ServiceCollectionExtensionsTests
    }

    [Fact]
-    public void AddDeploymentManager_WithConfiguration_MissingSection_UsesDefaults()
+    public void OptionsSection_MatchesTheConventionalComponentSectionPath()
    {
-        var configuration = new ConfigurationBuilder().Build();
-
-        var services = new ServiceCollection();
-        services.AddDeploymentManager(configuration);
-
-        using var provider = services.BuildServiceProvider();
-        var options = provider.GetRequiredService<IOptions<DeploymentManagerOptions>>().Value;
-
-        // No section present -> the option-class defaults are retained.
-        Assert.Equal(TimeSpan.FromSeconds(5), options.OperationLockTimeout);
+        Assert.Equal("ScadaLink:DeploymentManager", ServiceCollectionExtensions.OptionsSection);
    }
 }
@@ -0,0 +1,41 @@
+namespace ScadaLink.Host.Tests;
+
+/// <summary>
+/// Host-003: <c>appsettings.Central.json</c> no longer commits database connection
+/// strings — they are externalised to environment variables. Tests that exercise the
+/// full <c>Program</c> startup pipeline against the real SQL provider must therefore
+/// supply the local dev connection strings the way a deployment would: via
+/// environment variables (<c>Program</c>'s configuration builder calls
+/// <c>AddEnvironmentVariables()</c>).
+///
+/// Dispose restores the previous values so tests stay isolated.
+/// </summary>
+internal sealed class CentralDbTestEnvironment : IDisposable
+{
+    // Local dev/test database — same credentials the infra docker-compose stack uses.
+    // This is a test fixture value, not a committed production secret.
+    private const string ConfigurationDb =
+        "Server=localhost,1433;Database=ScadaLinkConfig;User Id=scadalink_app;Password=ScadaLink_Dev1#;TrustServerCertificate=true";
+    private const string MachineDataDb =
+        "Server=localhost,1433;Database=ScadaLinkMachineData;User Id=scadalink_app;Password=ScadaLink_Dev1#;TrustServerCertificate=true";
+
+    private const string ConfigKey = "ScadaLink__Database__ConfigurationDb";
+    private const string MachineKey = "ScadaLink__Database__MachineDataDb";
+
+    private readonly string? _previousConfig;
+    private readonly string? _previousMachine;
+
+    public CentralDbTestEnvironment()
+    {
+        _previousConfig = Environment.GetEnvironmentVariable(ConfigKey);
+        _previousMachine = Environment.GetEnvironmentVariable(MachineKey);
+        Environment.SetEnvironmentVariable(ConfigKey, ConfigurationDb);
+        Environment.SetEnvironmentVariable(MachineKey, MachineDataDb);
+    }
+
+    public void Dispose()
+    {
+        Environment.SetEnvironmentVariable(ConfigKey, _previousConfig);
+        Environment.SetEnvironmentVariable(MachineKey, _previousMachine);
+    }
+}
@@ -298,6 +298,9 @@ public class SiteCompositionRootTests : IDisposable
            ["ScadaLink:Node:RemotingPort"] = "0",
            ["ScadaLink:Node:GrpcPort"] = "0",
            ["ScadaLink:Database:SiteDbPath"] = _tempDbPath,
+            // ClusterOptions requires at least one seed node (ClusterOptionsValidator).
+            ["ScadaLink:Cluster:SeedNodes:0"] = "akka.tcp://scadalink@localhost:2551",
+            ["ScadaLink:Cluster:SeedNodes:1"] = "akka.tcp://scadalink@localhost:2552",
        });

        // gRPC server registration (mirrors Program.cs site section)
@@ -0,0 +1,89 @@
+using System.Reflection;
+using System.Text.Json;
+
+namespace ScadaLink.Host.Tests;
+
+/// <summary>
+/// Host-003 regression: secrets must not be committed in plaintext in the
+/// shipped <c>appsettings.Central.json</c>. Connection-string passwords, the LDAP
+/// service-account password and the JWT signing key must be supplied via
+/// environment variables (or another secret store) at deployment time — the
+/// committed file may only carry non-sensitive structural defaults or
+/// placeholder values.
+/// </summary>
+public class ConfigSecretsTests
+{
+    private static string FindHostProjectDirectory()
+    {
+        var assemblyDir = Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location)!;
+        var dir = new DirectoryInfo(assemblyDir);
+        while (dir != null)
+        {
+            var hostPath = Path.Combine(dir.FullName, "src", "ScadaLink.Host");
+            if (Directory.Exists(hostPath))
+                return hostPath;
+            dir = dir.Parent;
+        }
+        throw new DirectoryNotFoundException("Could not locate src/ScadaLink.Host");
+    }
+
+    private static JsonElement ScadaLinkSection()
+    {
+        var path = Path.Combine(FindHostProjectDirectory(), "appsettings.Central.json");
+        var json = File.ReadAllText(path);
+        using var doc = JsonDocument.Parse(json);
+        return doc.RootElement.GetProperty("ScadaLink").Clone();
+    }
+
+    [Fact]
+    public void CentralConfig_ConnectionStrings_ContainNoPlaintextPassword()
+    {
+        var db = ScadaLinkSection().GetProperty("Database");
+        foreach (var prop in db.EnumerateObject())
+        {
+            var value = prop.Value.GetString() ?? string.Empty;
+            // A committed connection string must not carry a literal Password= value.
+            // Either the password is delivered via an environment variable or the
+            // whole connection string is. A placeholder reference is acceptable.
+            var idx = value.IndexOf("Password=", StringComparison.OrdinalIgnoreCase);
+            if (idx >= 0)
+            {
+                var after = value[(idx + "Password=".Length)..];
+                var literal = after.Split(';')[0];
+                Assert.True(
+                    literal.Length == 0 || literal.Contains('{') || literal.Contains('$'),
+                    $"appsettings.Central.json '{prop.Name}' contains a plaintext Password value '{literal}'. " +
+                    "Move the secret to an environment variable.");
+            }
+        }
+    }
+
+    [Fact]
+    public void CentralConfig_LdapServiceAccountPassword_IsNotCommitted()
+    {
+        var security = ScadaLinkSection().GetProperty("Security");
+        if (security.TryGetProperty("LdapServiceAccountPassword", out var pw))
+        {
+            var value = pw.GetString() ?? string.Empty;
+            Assert.True(
+                value.Length == 0 || value.Contains('{') || value.Contains('$'),
+                $"appsettings.Central.json carries a plaintext LdapServiceAccountPassword '{value}'. " +
+                "Move it to an environment variable.");
+        }
+    }
+
+    [Fact]
+    public void CentralConfig_JwtSigningKey_IsNotCommitted()
+    {
+        var security = ScadaLinkSection().GetProperty("Security");
+        if (security.TryGetProperty("JwtSigningKey", out var key))
+        {
+            var value = key.GetString() ?? string.Empty;
+            Assert.True(
+                value.Length == 0 || value.Contains('{') || value.Contains('$'),
+                $"appsettings.Central.json carries a committed JwtSigningKey '{value}'. " +
+                "A committed signing key lets anyone with repo access forge session tokens. " +
+                "Move it to an environment variable.");
+        }
+    }
+}
@@ -11,6 +11,12 @@ public class HealthCheckTests : IDisposable
 {
    private readonly List<IDisposable> _disposables = new();

+    public HealthCheckTests()
+    {
+        // Host-003: connection strings are externalised; supply them via env vars.
+        _disposables.Add(new CentralDbTestEnvironment());
+    }
+
    public void Dispose()
    {
        foreach (var d in _disposables)
@@ -28,6 +28,8 @@ public class HostStartupTests : IDisposable
        // Set the environment to Central so appsettings.Central.json is loaded,
        // and set DOTNET_ENVIRONMENT before the factory creates the host.
        var previousEnv = Environment.GetEnvironmentVariable("DOTNET_ENVIRONMENT");
+        // Host-003: connection strings are externalised; supply them via env vars.
+        using var dbEnv = new CentralDbTestEnvironment();
        try
        {
            Environment.SetEnvironmentVariable("DOTNET_ENVIRONMENT", "Central");
@@ -254,6 +254,62 @@ public class StartupValidatorTests
        Assert.Null(ex);
    }

+    [Fact]
+    public void Site_SeedNodeOnGrpcPort_FailsValidation()
+    {
+        // Host-004 regression: a site seed node must reference an Akka remoting
+        // endpoint, never the Kestrel HTTP/2 gRPC port. A seed node whose port
+        // equals this node's GrpcPort would make a joining node attempt an
+        // Akka.Remote TCP association against the gRPC listener and fail.
+        var values = ValidSiteConfig();
+        values["ScadaLink:Node:GrpcPort"] = "8083";
+        values["ScadaLink:Cluster:SeedNodes:1"] = "akka.tcp://scadalink@site-a-node1:8083";
+        var config = BuildConfig(values);
+
+        var ex = Assert.Throws<InvalidOperationException>(() => StartupValidator.Validate(config));
+        Assert.Contains("must not target the gRPC port", ex.Message);
+    }
+
+    [Fact]
+    public void Site_SeedNodeOnDefaultGrpcPort_FailsValidation()
+    {
+        // GrpcPort is absent here, so the NodeOptions default of 8083 applies.
+        // A seed node on 8083 must still be rejected.
+        var values = ValidSiteConfig();
+        values["ScadaLink:Cluster:SeedNodes:1"] = "akka.tcp://scadalink@site-a-node2:8083";
+        var config = BuildConfig(values);
+
+        var ex = Assert.Throws<InvalidOperationException>(() => StartupValidator.Validate(config));
+        Assert.Contains("must not target the gRPC port", ex.Message);
+    }
+
+    [Fact]
+    public void Site_SeedNodesOnRemotingPort_PassesValidation()
+    {
+        // Two distinct site nodes, both seed entries on the remoting port (8082).
+        var values = ValidSiteConfig();
+        values["ScadaLink:Node:GrpcPort"] = "8083";
+        values["ScadaLink:Cluster:SeedNodes:0"] = "akka.tcp://scadalink@site-a-node1:8082";
+        values["ScadaLink:Cluster:SeedNodes:1"] = "akka.tcp://scadalink@site-a-node2:8082";
+        var config = BuildConfig(values);
+
+        var ex = Record.Exception(() => StartupValidator.Validate(config));
+        Assert.Null(ex);
+    }
+
+    [Fact]
+    public void Central_SeedNodeOnPort8083_PassesValidation()
+    {
+        // The gRPC-port rule applies to Site nodes only. A Central node has no
+        // GrpcPort, so a seed node on 8083 must not be rejected.
+        var values = ValidCentralConfig();
+        values["ScadaLink:Cluster:SeedNodes:1"] = "akka.tcp://scadalink@central-node2:8083";
+        var config = BuildConfig(values);
+
+        var ex = Record.Exception(() => StartupValidator.Validate(config));
+        Assert.Null(ex);
+    }
+
    [Fact]
    public void MultipleErrors_AllReported()
    {
@@ -0,0 +1,147 @@
+using Microsoft.AspNetCore.Http;
+using Microsoft.AspNetCore.Http.Features;
+using Microsoft.Extensions.DependencyInjection;
+using Microsoft.Extensions.Logging.Abstractions;
+using Microsoft.Extensions.Options;
+
+namespace ScadaLink.InboundAPI.Tests;
+
+/// <summary>
+/// InboundAPI-006 / InboundAPI-008: the POST /api/{methodName} endpoint must be
+/// gated to the active central node and must cap the request body size. These
+/// behaviours are enforced by <see cref="InboundApiEndpointFilter"/>.
+/// </summary>
+public class EndpointGatingTests
+{
+    private static InboundApiEndpointFilter CreateFilter(InboundApiOptions? options = null) =>
+        new(NullLogger<InboundApiEndpointFilter>.Instance,
+            Options.Create(options ?? new InboundApiOptions()));
+
+    private static (DefaultHttpContext ctx, DefaultEndpointFilterInvocationContext invocation)
+        BuildInvocation(bool? activeNode, long? contentLength)
+    {
+        var services = new ServiceCollection();
+        if (activeNode.HasValue)
+            services.AddSingleton<IActiveNodeGate>(new StubActiveNodeGate(activeNode.Value));
+
+        var ctx = new DefaultHttpContext
+        {
+            RequestServices = services.BuildServiceProvider()
+        };
+        ctx.Request.Method = "POST";
+        if (contentLength.HasValue)
+            ctx.Request.ContentLength = contentLength.Value;
+
+        return (ctx, new DefaultEndpointFilterInvocationContext(ctx));
+    }
+
+    private static EndpointFilterDelegate NextSentinel(out Func<bool> wasCalled)
+    {
+        var called = false;
+        wasCalled = () => called;
+        return _ =>
+        {
+            called = true;
+            return ValueTask.FromResult<object?>(Results.Ok("handler-ran"));
+        };
+    }
+
+    // --- InboundAPI-008: standby node must not serve inbound API calls ---
+
+    [Fact]
+    public async Task StandbyNode_ShortCircuitsWith503_AndDoesNotRunHandler()
+    {
+        var (_, invocation) = BuildInvocation(activeNode: false, contentLength: 2);
+        var next = NextSentinel(out var handlerRan);
+
+        var result = await CreateFilter().InvokeAsync(invocation, next);
+
+        Assert.False(handlerRan());
+        var status = Assert.IsAssignableFrom<IStatusCodeHttpResult>(result);
+        Assert.Equal(StatusCodes.Status503ServiceUnavailable, status.StatusCode);
+    }
+
+    [Fact]
+    public async Task ActiveNode_PassesGate_RunsHandler()
+    {
+        var (_, invocation) = BuildInvocation(activeNode: true, contentLength: 2);
+        var next = NextSentinel(out var handlerRan);
+
+        await CreateFilter().InvokeAsync(invocation, next);
+
+        Assert.True(handlerRan());
+    }
+
+    [Fact]
+    public async Task NoGateRegistered_PassesGate_RunsHandler()
+    {
+        // When no IActiveNodeGate is registered (non-clustered host), gating is
+        // opt-in and defaults to "allow" so the endpoint is still served.
+        var (_, invocation) = BuildInvocation(activeNode: null, contentLength: 2);
+        var next = NextSentinel(out var handlerRan);
+
+        await CreateFilter().InvokeAsync(invocation, next);
+
+        Assert.True(handlerRan());
+    }
+
+    // --- InboundAPI-006: request body size must be capped ---
+
+    [Fact]
+    public async Task OversizedBody_ShortCircuitsWith413_AndDoesNotRunHandler()
+    {
+        var options = new InboundApiOptions();
+        var (_, invocation) = BuildInvocation(
+            activeNode: true, contentLength: options.MaxRequestBodyBytes + 1);
+        var next = NextSentinel(out var handlerRan);
+
+        var result = await CreateFilter(options).InvokeAsync(invocation, next);
+
+        Assert.False(handlerRan());
+        var status = Assert.IsAssignableFrom<IStatusCodeHttpResult>(result);
+        Assert.Equal(StatusCodes.Status413PayloadTooLarge, status.StatusCode);
+    }
+
+    [Fact]
+    public async Task BodyAtLimit_RunsHandler()
+    {
+        var options = new InboundApiOptions();
+        var (_, invocation) = BuildInvocation(
+            activeNode: true, contentLength: options.MaxRequestBodyBytes);
+        var next = NextSentinel(out var handlerRan);
+
+        await CreateFilter(options).InvokeAsync(invocation, next);
+
+        Assert.True(handlerRan());
+    }
+
+    [Fact]
+    public async Task FilterCapsMaxRequestBodySizeFeature()
+    {
+        // For chunked/unknown-length requests there is no Content-Length, so the
+        // filter must also cap the per-request body size feature so Kestrel rejects
+        // an oversized stream while it is being read.
+        var options = new InboundApiOptions();
+        var (ctx, invocation) = BuildInvocation(activeNode: true, contentLength: null);
+        ctx.Features.Set<IHttpMaxRequestBodySizeFeature>(new StubMaxBodySizeFeature());
+        var next = NextSentinel(out _);
+
+        await CreateFilter(options).InvokeAsync(invocation, next);
+
+        var feature = ctx.Features.Get<IHttpMaxRequestBodySizeFeature>();
+        Assert.Equal(options.MaxRequestBodyBytes, feature!.MaxRequestBodySize);
+    }
+
+    private sealed class StubActiveNodeGate : IActiveNodeGate
+    {
+        private readonly bool _isActive;
+        public StubActiveNodeGate(bool isActive) => _isActive = isActive;
+        public bool IsActiveNode => _isActive;
+    }
+
+    private sealed class StubMaxBodySizeFeature : IHttpMaxRequestBodySizeFeature
+    {
+        public bool IsReadOnly => false;
+        public long? MaxRequestBodySize { get; set; }
+    }
+}
@@ -239,4 +239,88 @@ public class InboundScriptExecutorTests

        Assert.True(_executor.CompileAndRegister(method));
    }
+
+    // --- InboundAPI-002: lazy compile-and-fetch must be atomic, never KeyNotFoundException ---
+
+    [Fact]
+    public async Task LazyCompile_RacingRemoveHandler_NeverThrowsKeyNotFound()
+    {
+        // The lazy-compile path must compile-and-fetch atomically: a concurrent
+        // RemoveHandler must not be able to turn a first-call into an "Internal
+        // script error" (the old check-then-act re-read could throw KeyNotFoundException).
+        var method = new ApiMethod("atomic", "return 5;") { Id = 1, TimeoutSeconds = 10 };
+
+        var removers = Enumerable.Range(0, 16).Select(_ => Task.Run(() =>
+        {
+            for (var n = 0; n < 200; n++)
+                _executor.RemoveHandler("atomic");
+        }));
+
+        var callers = Enumerable.Range(0, 16).Select(_ => Task.Run(async () =>
+        {
+            for (var n = 0; n < 50; n++)
+            {
+                var r = await _executor.ExecuteAsync(
+                    method, new Dictionary<string, object?>(), _route, TimeSpan.FromSeconds(10));
+                // Result must always be a clean success or a clean compilation
+                // failure — never the catch-all "Internal script error".
+                Assert.NotEqual("Internal script error", r.ErrorMessage);
+            }
+        }));
+
+        await Task.WhenAll(removers.Concat(callers));
+    }
+
+    // --- InboundAPI-004: a client disconnect must NOT be reported as a script timeout ---
+
+    [Fact]
+    public async Task ClientDisconnect_IsNotReportedAsTimeout()
+    {
+        // When the caller's request token is cancelled (client aborted the request),
+        // ExecuteAsync must report a client-cancelled failure, not "Script execution
+        // timed out" — that log line is reserved for genuine timeouts.
+        var method = new ApiMethod("aborted", "return 1;") { Id = 1, TimeoutSeconds = 30 };
+        _executor.RegisterHandler("aborted", async ctx =>
+        {
+            await Task.Delay(TimeSpan.FromSeconds(60), ctx.CancellationToken);
+            return "never";
+        });
+
+        using var clientAborted = new CancellationTokenSource();
+        clientAborted.CancelAfter(TimeSpan.FromMilliseconds(100));
+
+        var result = await _executor.ExecuteAsync(
+            method,
+            new Dictionary<string, object?>(),
+            _route,
+            // Generous method timeout so the timeout CTS is NOT the cause.
+            TimeSpan.FromSeconds(30),
+            clientAborted.Token);
+
+        Assert.False(result.Success);
+        Assert.DoesNotContain("timed out", result.ErrorMessage ?? string.Empty);
+    }
+
+    [Fact]
+    public async Task GenuineTimeout_StillReportedAsTimeout()
+    {
+        // A method that exceeds its timeout with no client abort must still be
+        // reported as "timed out" (regression guard for the InboundAPI-004 fix).
+        var method = new ApiMethod("genuine", "return 1;") { Id = 1, TimeoutSeconds = 1 };
+        _executor.RegisterHandler("genuine", async ctx =>
+        {
+            await Task.Delay(TimeSpan.FromSeconds(60), ctx.CancellationToken);
+            return "never";
+        });
+
+        var result = await _executor.ExecuteAsync(
+            method,
+            new Dictionary<string, object?>(),
+            _route,
+            TimeSpan.FromMilliseconds(100),
+            CancellationToken.None);
+
+        Assert.False(result.Success);
+        Assert.Contains("timed out", result.ErrorMessage);
+    }
 }
@@ -8,6 +8,10 @@
    <IsPackable>false</IsPackable>
  </PropertyGroup>

+  <ItemGroup>
+    <FrameworkReference Include="Microsoft.AspNetCore.App" />
+  </ItemGroup>
+
  <ItemGroup>
    <PackageReference Include="coverlet.collector" />
    <PackageReference Include="Microsoft.NET.Test.Sdk" />
@@ -668,4 +668,66 @@ public class ManagementActorTests : TestKit, IDisposable
        var response = ExpectMsg<ManagementUnauthorized>(TimeSpan.FromSeconds(5));
        Assert.Equal(envelope.CorrelationId, response.CorrelationId);
    }
+
+    // ========================================================================
+    // Serialization (finding ManagementService-007)
+    //
+    // Command results are serialized with System.Text.Json configured with
+    // ReferenceHandler.IgnoreCycles, so an entity graph with a bidirectional
+    // navigation property does not throw. Property names are camelCase, which
+    // the CLI's case-insensitive deserializer accepts.
+    // ========================================================================
+
+    private sealed class CyclicNode
+    {
+        public string Name { get; set; } = "";
+        public CyclicNode? Parent { get; set; }
+        public List<CyclicNode> Children { get; set; } = new();
+    }
+
+    [Fact]
+    public void SerializeResult_WithCyclicGraph_DoesNotThrow()
+    {
+        var parent = new CyclicNode { Name = "Parent" };
+        var child = new CyclicNode { Name = "Child", Parent = parent };
+        parent.Children.Add(child); // parent <-> child cycle
+
+        var json = ManagementActor.SerializeResult(parent);
+
+        Assert.Contains("Parent", json);
+        Assert.Contains("Child", json);
+    }
+
+    [Fact]
+    public void SerializeResult_UsesCamelCasePropertyNames()
+    {
+        var json = ManagementActor.SerializeResult(new CyclicNode { Name = "X" });
+
+        Assert.Contains("\"name\"", json);
+        Assert.DoesNotContain("\"Name\"", json);
+    }
+
+    // ========================================================================
+    // PipeTo fault mapping (finding ManagementService-004)
+    //
+    // Command processing is piped back via PipeTo; a fault raised inside
+    // DispatchCommand must be mapped to ManagementError by the failure
+    // continuation rather than escaping or being silently dropped.
+    // ========================================================================
+
+    [Fact]
+    public void UnknownCommandType_FaultMappedToManagementError()
+    {
+        // ManagementEnvelope.Command is typed object; an unrecognised payload
+        // makes DispatchCommand throw NotSupportedException. The PipeTo failure
+        // continuation must map it to ManagementError.
+        var actor = CreateActor();
+        var envelope = Envelope("not-a-command");
+
+        actor.Tell(envelope);
+
+        var response = ExpectMsg<ManagementError>(TimeSpan.FromSeconds(5));
+        Assert.Equal(envelope.CorrelationId, response.CorrelationId);
+        Assert.Equal("COMMAND_FAILED", response.ErrorCode);
+    }
 }
@@ -0,0 +1,65 @@
+using ScadaLink.Commons.Messages.Management;
+using ScadaLink.ManagementService;
+
+namespace ScadaLink.ManagementService.Tests;
+
+/// <summary>
+/// Tests for <see cref="ManagementEndpoints"/> request-body parsing
+/// (findings ManagementService-006 / -013).
+/// </summary>
+public class ManagementEndpointsTests
+{
+    [Fact]
+    public void ParseCommand_WithExplicitPayload_DeserializesIntoCommandType()
+    {
+        var json = """{ "command": "CreateSite", "payload": { "name": "Site1", "siteIdentifier": "SITE1", "description": "Desc" } }""";
+
+        var result = ManagementEndpoints.ParseCommand(json);
+
+        Assert.True(result.Success);
+        var command = Assert.IsType<CreateSiteCommand>(result.Command);
+        Assert.Equal("Site1", command.Name);
+        Assert.Equal("SITE1", command.SiteIdentifier);
+        Assert.Equal("Desc", command.Description);
+    }
+
+    [Fact]
+    public void ParseCommand_WithMissingPayload_DeserializesParameterlessCommand()
+    {
+        // No "payload" field at all -- the fallback must not allocate a throwaway
+        // JsonDocument and must still produce a valid parameterless command.
+        var json = """{ "command": "ListTemplates" }""";
+
+        var result = ManagementEndpoints.ParseCommand(json);
+
+        Assert.True(result.Success);
+        Assert.IsType<ListTemplatesCommand>(result.Command);
+    }
+
+    [Fact]
+    public void ParseCommand_WithInvalidJson_ReturnsFailure()
+    {
+        var result = ManagementEndpoints.ParseCommand("{ not json");
+
+        Assert.False(result.Success);
+        Assert.Equal("BAD_REQUEST", result.ErrorCode);
+    }
+
+    [Fact]
+    public void ParseCommand_WithMissingCommandField_ReturnsFailure()
+    {
+        var result = ManagementEndpoints.ParseCommand("""{ "payload": {} }""");
+
+        Assert.False(result.Success);
+        Assert.Equal("BAD_REQUEST", result.ErrorCode);
+    }
+
+    [Fact]
+    public void ParseCommand_WithUnknownCommand_ReturnsFailure()
+    {
+        var result = ManagementEndpoints.ParseCommand("""{ "command": "NoSuchCommand" }""");
+
+        Assert.False(result.Success);
+        Assert.Equal("BAD_REQUEST", result.ErrorCode);
+    }
+}
@@ -0,0 +1,38 @@
+namespace ScadaLink.NotificationService.Tests;
+
+/// <summary>
+/// NS-009: Tests for scrubbing SMTP credential secrets out of log/result text.
+/// </summary>
+public class CredentialRedactorTests
+{
+    [Fact]
+    public void Scrub_BasicAuthPassword_IsMasked()
+    {
+        var text = "535 5.7.8 Authentication failed for user 'svc' with password 'Hunter2pw!'";
+        var result = CredentialRedactor.Scrub(text, "svc:Hunter2pw!");
+
+        Assert.DoesNotContain("Hunter2pw!", result);
+        Assert.DoesNotContain("svc:Hunter2pw!", result);
+    }
+
+    [Fact]
+    public void Scrub_OAuth2ClientSecret_IsMasked()
+    {
+        var text = "Token request failed: client_secret=Sup3rSecretValue rejected by tenant";
+        var result = CredentialRedactor.Scrub(text, "tenant-guid:client-guid:Sup3rSecretValue");
+
+        Assert.DoesNotContain("Sup3rSecretValue", result);
+    }
+
+    [Fact]
+    public void Scrub_NullCredentials_ReturnsTextUnchanged()
+    {
+        Assert.Equal("plain text", CredentialRedactor.Scrub("plain text", null));
+    }
+
+    [Fact]
+    public void Scrub_NullText_ReturnsEmpty()
+    {
+        Assert.Equal(string.Empty, CredentialRedactor.Scrub(null, "user:pass"));
+    }
+}
@@ -111,7 +111,8 @@ public class NotificationDeliveryServiceTests

        await service.SendAsync("ops-team", "Alert", "Body");

-        await _smtpClient.Received().ConnectAsync("smtp.example.com", 587, true, Arg.Any<CancellationToken>());
+        await _smtpClient.Received().ConnectAsync(
+            "smtp.example.com", 587, SmtpTlsMode.StartTls, Arg.Any<int>(), Arg.Any<CancellationToken>());
        await _smtpClient.Received().AuthenticateAsync("basic", "user:pass", Arg.Any<CancellationToken>());
        await _smtpClient.Received().SendAsync(
            "noreply@example.com",
@@ -363,7 +364,7 @@ public class NotificationDeliveryServiceTests
    private sealed class TrackingSmtpClient : ISmtpClientWrapper, IDisposable
    {
        public bool Disposed { get; private set; }
-        public Task ConnectAsync(string host, int port, bool useTls, CancellationToken cancellationToken = default)
+        public Task ConnectAsync(string host, int port, SmtpTlsMode tlsMode, int connectionTimeoutSeconds, CancellationToken cancellationToken = default)
            => Task.CompletedTask;
        public Task AuthenticateAsync(string authType, string? credentials, CancellationToken cancellationToken = default)
            => Task.CompletedTask;
@@ -405,4 +406,240 @@ public class NotificationDeliveryServiceTests
        return new StoreAndForwardService(
            storage, new StoreAndForwardOptions(), NullLogger<StoreAndForwardService>.Instance);
    }
+
+    // ── NotificationService-005: explicit TLS mode passed through to the wrapper ──
+
+    /// <summary>An SMTP wrapper that records the TLS mode and timeout it was connected with.</summary>
+    private sealed class RecordingTlsClient : ISmtpClientWrapper
+    {
+        public SmtpTlsMode? TlsMode { get; private set; }
+        public int ConnectionTimeoutSeconds { get; private set; }
+        public Task ConnectAsync(string host, int port, SmtpTlsMode tlsMode, int connectionTimeoutSeconds, CancellationToken cancellationToken = default)
+        {
+            TlsMode = tlsMode;
+            ConnectionTimeoutSeconds = connectionTimeoutSeconds;
+            return 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;
+    }
+
+    private void SetupHappyPathWithSmtp(SmtpConfiguration smtpConfig)
+    {
+        var list = new NotificationList("ops-team") { Id = 1 };
+        var recipients = new List<NotificationRecipient>
+        {
+            new("Alice", "alice@example.com") { Id = 1, NotificationListId = 1 }
+        };
+        _repository.GetListByNameAsync("ops-team").Returns(list);
+        _repository.GetRecipientsByListIdAsync(1).Returns(recipients);
+        _repository.GetAllSmtpConfigurationsAsync().Returns(new List<SmtpConfiguration> { smtpConfig });
+    }
+
+    [Fact]
+    public async Task Send_TlsModeNone_DoesNotNegotiateTls()
+    {
+        // NS-005: TlsMode "none" must connect with SmtpTlsMode.None, not the old
+        // SecureSocketOptions.Auto (which let MailKit opportunistically negotiate TLS).
+        var cfg = new SmtpConfiguration("smtp.example.com", "basic", "noreply@example.com")
+        {
+            Id = 1, Port = 25, Credentials = "user:pass", TlsMode = "none"
+        };
+        SetupHappyPathWithSmtp(cfg);
+        var recording = new RecordingTlsClient();
+        var service = new NotificationDeliveryService(
+            _repository, () => recording, NullLogger<NotificationDeliveryService>.Instance);
+
+        var result = await service.SendAsync("ops-team", "Alert", "Body");
+
+        Assert.True(result.Success);
+        Assert.Equal(SmtpTlsMode.None, recording.TlsMode);
+    }
+
+    [Fact]
+    public async Task Send_TlsModeSsl_UsesImplicitSsl()
+    {
+        // NS-005: TlsMode "ssl" (port 465 implicit TLS) must be honoured, not
+        // collapsed into the same path as "none".
+        var cfg = new SmtpConfiguration("smtp.example.com", "basic", "noreply@example.com")
+        {
+            Id = 1, Port = 465, Credentials = "user:pass", TlsMode = "ssl"
+        };
+        SetupHappyPathWithSmtp(cfg);
+        var recording = new RecordingTlsClient();
+        var service = new NotificationDeliveryService(
+            _repository, () => recording, NullLogger<NotificationDeliveryService>.Instance);
+
+        var result = await service.SendAsync("ops-team", "Alert", "Body");
+
+        Assert.True(result.Success);
+        Assert.Equal(SmtpTlsMode.Ssl, recording.TlsMode);
+    }
+
+    [Fact]
+    public async Task Send_UnknownTlsMode_ReturnsErrorNotSilentFallback()
+    {
+        var cfg = new SmtpConfiguration("smtp.example.com", "basic", "noreply@example.com")
+        {
+            Id = 1, Port = 587, Credentials = "user:pass", TlsMode = "bogus"
+        };
+        SetupHappyPathWithSmtp(cfg);
+        var service = new NotificationDeliveryService(
+            _repository, () => new RecordingTlsClient(), NullLogger<NotificationDeliveryService>.Instance);
+
+        var result = await service.SendAsync("ops-team", "Alert", "Body");
+
+        Assert.False(result.Success);
+        Assert.Contains("TLS mode", result.ErrorMessage);
+    }
+
+    // ── NotificationService-007: connection timeout passed through to the wrapper ──
+
+    [Fact]
+    public async Task Send_PassesConfiguredConnectionTimeoutToClient()
+    {
+        // NS-007: SmtpConfiguration.ConnectionTimeoutSeconds must reach the wrapper
+        // so SmtpClient.Timeout is set; it was previously dead configuration.
+        var cfg = new SmtpConfiguration("smtp.example.com", "basic", "noreply@example.com")
+        {
+            Id = 1, Port = 587, Credentials = "user:pass", TlsMode = "starttls",
+            ConnectionTimeoutSeconds = 17
+        };
+        SetupHappyPathWithSmtp(cfg);
+        var recording = new RecordingTlsClient();
+        var service = new NotificationDeliveryService(
+            _repository, () => recording, NullLogger<NotificationDeliveryService>.Instance);
+
+        await service.SendAsync("ops-team", "Alert", "Body");
+
+        Assert.Equal(17, recording.ConnectionTimeoutSeconds);
+    }
+
+    [Fact]
+    public async Task Send_MaxConcurrentConnections_LimitsConcurrentDeliveries()
+    {
+        // NS-007: MaxConcurrentConnections must throttle concurrent SMTP deliveries.
+        var cfg = new SmtpConfiguration("smtp.example.com", "basic", "noreply@example.com")
+        {
+            Id = 1, Port = 587, Credentials = "user:pass", TlsMode = "starttls",
+            MaxConcurrentConnections = 2
+        };
+        SetupHappyPathWithSmtp(cfg);
+
+        var inFlight = 0;
+        var maxObserved = 0;
+        var gate = new SemaphoreSlim(0);
+        var sync = new object();
+
+        var service = new NotificationDeliveryService(
+            _repository,
+            () => new BlockingSmtpClient(
+                onSend: async () =>
+                {
+                    lock (sync)
+                    {
+                        inFlight++;
+                        if (inFlight > maxObserved) maxObserved = inFlight;
+                    }
+                    await gate.WaitAsync();
+                    lock (sync) { inFlight--; }
+                }),
+            NullLogger<NotificationDeliveryService>.Instance);
+
+        var sends = Enumerable.Range(0, 6)
+            .Select(_ => service.SendAsync("ops-team", "Alert", "Body"))
+            .ToList();
+
+        // Give the throttled sends time to reach the SMTP send call.
+        await Task.Delay(200);
+        gate.Release(6);
+        await Task.WhenAll(sends);
+
+        Assert.True(maxObserved <= 2, $"Expected at most 2 concurrent deliveries, observed {maxObserved}");
+    }
+
+    private sealed class BlockingSmtpClient : ISmtpClientWrapper, IDisposable
+    {
+        private readonly Func<Task> _onSend;
+        public BlockingSmtpClient(Func<Task> onSend) => _onSend = onSend;
+        public Task ConnectAsync(string host, int port, SmtpTlsMode tlsMode, int connectionTimeoutSeconds, 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)
+            => _onSend();
+        public Task DisconnectAsync(CancellationToken cancellationToken = default) => Task.CompletedTask;
+        public void Dispose() { }
+    }
+
+    // ── NotificationService-008: recipient address validation ──
+
+    [Fact]
+    public async Task Send_MalformedRecipientAddress_ReturnsCleanError_DoesNotThrow()
+    {
+        // NS-008: a malformed recipient address previously caused MailboxAddress.Parse
+        // to throw ParseException, which escaped SendAsync unhandled. It must now
+        // produce a clean NotificationResult failure.
+        var list = new NotificationList("ops-team") { Id = 1 };
+        var recipients = new List<NotificationRecipient>
+        {
+            new("Alice", "alice@example.com") { Id = 1, NotificationListId = 1 },
+            new("Bad", "not a valid address @@") { Id = 2, NotificationListId = 1 }
+        };
+        var cfg = new SmtpConfiguration("smtp.example.com", "basic", "noreply@example.com")
+        {
+            Id = 1, Port = 587, Credentials = "user:pass", TlsMode = "starttls"
+        };
+        _repository.GetListByNameAsync("ops-team").Returns(list);
+        _repository.GetRecipientsByListIdAsync(1).Returns(recipients);
+        _repository.GetAllSmtpConfigurationsAsync().Returns(new List<SmtpConfiguration> { cfg });
+
+        var service = CreateService();
+        var result = await service.SendAsync("ops-team", "Alert", "Body");
+
+        Assert.False(result.Success);
+        Assert.Contains("address", result.ErrorMessage, StringComparison.OrdinalIgnoreCase);
+        Assert.Contains("not a valid address @@", result.ErrorMessage);
+    }
+
+    // ── NotificationService-009: credential secrets scrubbed from logs/results ──
+
+    [Fact]
+    public async Task Send_PermanentError_RedactsCredentialFromResultMessage()
+    {
+        // NS-009: a permanent-failure message echoing a credential fragment must be
+        // scrubbed before it reaches the caller-facing NotificationResult.
+        var cfg = new SmtpConfiguration("smtp.example.com", "basic", "noreply@example.com")
+        {
+            Id = 1, Port = 587, Credentials = "svcuser:Hunter2Secret", TlsMode = "starttls"
+        };
+        SetupHappyPathWithSmtp(cfg);
+        _smtpClient.SendAsync(Arg.Any<string>(), Arg.Any<IEnumerable<string>>(), Arg.Any<string>(), Arg.Any<string>(), Arg.Any<CancellationToken>())
+            .Throws(new SmtpPermanentException("550 rejected — password Hunter2Secret is invalid"));
+
+        var service = CreateService();
+        var result = await service.SendAsync("ops-team", "Alert", "Body");
+
+        Assert.False(result.Success);
+        Assert.DoesNotContain("Hunter2Secret", result.ErrorMessage);
+    }
+
+    [Fact]
+    public async Task Send_MalformedFromAddress_ReturnsCleanError_DoesNotThrow()
+    {
+        var cfg = new SmtpConfiguration("smtp.example.com", "basic", "@@bad-from@@")
+        {
+            Id = 1, Port = 587, Credentials = "user:pass", TlsMode = "starttls"
+        };
+        SetupHappyPathWithSmtp(cfg);
+
+        var service = CreateService();
+        var result = await service.SendAsync("ops-team", "Alert", "Body");
+
+        Assert.False(result.Success);
+        Assert.Contains("address", result.ErrorMessage, StringComparison.OrdinalIgnoreCase);
+    }
 }
@@ -87,6 +87,70 @@ public class OAuth2TokenServiceTests
            () => service.GetTokenAsync("tenant:client:secret"));
    }

+    // ── NotificationService-006: token cache must be keyed to credential identity ──
+
+    [Fact]
+    public async Task GetTokenAsync_DifferentCredentials_ReturnPerCredentialTokens()
+    {
+        // NS-006: the singleton cached a single token ignoring the credentials
+        // argument, so a second SMTP config with a different tenant/client got the
+        // first config's token. Each distinct credential must get its own token.
+        var handler = new PerTenantHttpMessageHandler();
+        var client = new HttpClient(handler);
+        var factory = CreateMockFactory(client);
+        var service = new OAuth2TokenService(factory, NullLogger<OAuth2TokenService>.Instance);
+
+        var tokenA = await service.GetTokenAsync("tenantA:clientA:secretA");
+        var tokenB = await service.GetTokenAsync("tenantB:clientB:secretB");
+
+        Assert.Equal("token-for-tenantA", tokenA);
+        Assert.Equal("token-for-tenantB", tokenB);
+    }
+
+    [Fact]
+    public async Task GetTokenAsync_SameCredentials_CachedPerCredential()
+    {
+        // NS-006: caching still works — repeated calls with the same credential
+        // identity make exactly one HTTP call.
+        var handler = new PerTenantHttpMessageHandler();
+        var client = new HttpClient(handler);
+        var factory = CreateMockFactory(client);
+        var service = new OAuth2TokenService(factory, NullLogger<OAuth2TokenService>.Instance);
+
+        await service.GetTokenAsync("tenantA:clientA:secretA");
+        await service.GetTokenAsync("tenantA:clientA:secretA");
+        await service.GetTokenAsync("tenantB:clientB:secretB");
+
+        Assert.Equal(2, handler.CallCount); // one per distinct credential, not per call
+    }
+
+    /// <summary>
+    /// HTTP handler that returns a distinct access token per tenant id, parsed from
+    /// the request URL (<c>https://login.microsoftonline.com/{tenantId}/...</c>).
+    /// </summary>
+    private class PerTenantHttpMessageHandler : HttpMessageHandler
+    {
+        public int CallCount { get; private set; }
+
+        protected override Task<HttpResponseMessage> SendAsync(
+            HttpRequestMessage request, CancellationToken cancellationToken)
+        {
+            CallCount++;
+            var segments = request.RequestUri!.AbsolutePath.Trim('/').Split('/');
+            var tenantId = segments[0];
+            var json = JsonSerializer.Serialize(new
+            {
+                access_token = $"token-for-{tenantId}",
+                expires_in = 3600,
+                token_type = "Bearer"
+            });
+            return Task.FromResult(new HttpResponseMessage(HttpStatusCode.OK)
+            {
+                Content = new StringContent(json)
+            });
+        }
+    }
+
    /// <summary>
    /// Simple mock HTTP handler that returns a fixed response.
    /// </summary>
@@ -0,0 +1,40 @@
+namespace ScadaLink.NotificationService.Tests;
+
+/// <summary>
+/// NS-005: Tests for parsing the configured SMTP TLS mode into the three-state enum.
+/// </summary>
+public class SmtpTlsModeParserTests
+{
+    [Theory]
+    [InlineData("none", SmtpTlsMode.None)]
+    [InlineData("None", SmtpTlsMode.None)]
+    [InlineData("NONE", SmtpTlsMode.None)]
+    [InlineData("starttls", SmtpTlsMode.StartTls)]
+    [InlineData("StartTLS", SmtpTlsMode.StartTls)]
+    [InlineData("ssl", SmtpTlsMode.Ssl)]
+    [InlineData("SSL", SmtpTlsMode.Ssl)]
+    [InlineData("  starttls  ", SmtpTlsMode.StartTls)]
+    public void Parse_KnownModes_ReturnsExpected(string input, SmtpTlsMode expected)
+    {
+        Assert.Equal(expected, SmtpTlsModeParser.Parse(input));
+    }
+
+    [Theory]
+    [InlineData(null)]
+    [InlineData("")]
+    [InlineData("   ")]
+    public void Parse_NullOrEmpty_DefaultsToStartTls(string? input)
+    {
+        Assert.Equal(SmtpTlsMode.StartTls, SmtpTlsModeParser.Parse(input));
+    }
+
+    [Theory]
+    [InlineData("auto")]
+    [InlineData("tls")]
+    [InlineData("implicit")]
+    public void Parse_UnknownMode_Throws(string input)
+    {
+        // NS-005: an unknown mode must be rejected, not silently treated as Auto.
+        Assert.Throws<ArgumentException>(() => SmtpTlsModeParser.Parse(input));
+    }
+}
@@ -475,6 +475,203 @@ public class SecurityReviewRegressionTests

 #endregion

+#region Code Review Regression Tests — Security-004/005/006/007
+
+/// <summary>
+/// Regression tests for Security-004 (uid/cn attribute mismatch between search filter
+/// and fallback DN), Security-005 (DN injection in the no-service-account fallback),
+/// Security-006 (JWT issuer/audience checks disabled), and Security-007 (idle-timeout
+/// claim reset on every token refresh).
+/// </summary>
+public class SecurityReviewRegressionTests2
+{
+    private static SecurityOptions JwtOptions() => new()
+    {
+        JwtSigningKey = "this-is-a-test-signing-key-for-hmac-sha256-must-be-long-enough",
+        JwtExpiryMinutes = 15,
+        IdleTimeoutMinutes = 30,
+        JwtRefreshThresholdMinutes = 5
+    };
+
+    private static JwtTokenService CreateJwtService(SecurityOptions? options = null) =>
+        new(Options.Create(options ?? JwtOptions()), NullLogger<JwtTokenService>.Instance);
+
+    // --- Security-004: search filter and fallback DN must use the same attribute ---
+
+    [Fact]
+    public void BuildFallbackUserDn_UsesConfiguredUserIdAttribute()
+    {
+        // The default user-id attribute is "uid"; the fallback DN must use it,
+        // not a hard-coded "cn", so search-then-bind and direct-bind are interchangeable.
+        var dn = LdapAuthService.BuildFallbackUserDn("alice", "dc=example,dc=com", "uid");
+        Assert.Equal("uid=alice,dc=example,dc=com", dn);
+    }
+
+    [Fact]
+    public void BuildFallbackUserDn_HonoursNonDefaultUserIdAttribute()
+    {
+        var dn = LdapAuthService.BuildFallbackUserDn("alice", "dc=example,dc=com", "sAMAccountName");
+        Assert.Equal("sAMAccountName=alice,dc=example,dc=com", dn);
+    }
+
+    [Fact]
+    public void SecurityOptions_LdapUserIdAttribute_DefaultsToUid()
+    {
+        Assert.Equal("uid", new SecurityOptions().LdapUserIdAttribute);
+    }
+
+    // --- Security-005: DN-component escaping must be applied to the username ---
+
+    [Fact]
+    public void BuildFallbackUserDn_EscapesDnMetacharacters()
+    {
+        // A hostile username must not be able to alter the DN structure: the comma
+        // that would otherwise start a new RDN ("ou=admins") must be escaped so the
+        // whole string remains a single RDN value.
+        var dn = LdapAuthService.BuildFallbackUserDn("victim,ou=admins", "dc=example,dc=com", "uid");
+        Assert.Equal(@"uid=victim\,ou=admins,dc=example,dc=com", dn);
+        // The comma from the username is backslash-escaped, so it does not act as an
+        // RDN separator: the only unescaped comma is the one joining RDN and base DN.
+        Assert.Contains(@"victim\,ou=admins", dn);
+    }
+
+    [Fact]
+    public void EscapeLdapDn_EscapesAllRfc4514Specials()
+    {
+        var escaped = LdapAuthService.EscapeLdapDn("a,b+c\"d\\e<f>g;h");
+        Assert.Equal(@"a\,b\+c\""d\\e\<f\>g\;h", escaped);
+    }
+
+    [Fact]
+    public void EscapeLdapDn_EscapesLeadingAndTrailingSpaces()
+    {
+        Assert.Equal(@"\  x \ ", LdapAuthService.EscapeLdapDn("  x  "));
+    }
+
+    // --- Security-006: JWT issuer/audience must be bound and validated ---
+
+    [Fact]
+    public void GenerateToken_SetsIssuerAndAudience()
+    {
+        var service = CreateJwtService();
+        var token = service.GenerateToken("User", "user", new[] { "Admin" }, null);
+
+        var jwt = new System.IdentityModel.Tokens.Jwt.JwtSecurityTokenHandler().ReadJwtToken(token);
+        Assert.Equal(JwtTokenService.TokenIssuer, jwt.Issuer);
+        Assert.Contains(JwtTokenService.TokenAudience, jwt.Audiences);
+    }
+
+    [Fact]
+    public void ValidateToken_RejectsTokenWithWrongIssuer()
+    {
+        // A token signed with the same key but a foreign issuer must be rejected.
+        var options = JwtOptions();
+        var key = new Microsoft.IdentityModel.Tokens.SymmetricSecurityKey(
+            System.Text.Encoding.UTF8.GetBytes(options.JwtSigningKey));
+        var creds = new Microsoft.IdentityModel.Tokens.SigningCredentials(
+            key, Microsoft.IdentityModel.Tokens.SecurityAlgorithms.HmacSha256);
+        var foreign = new System.IdentityModel.Tokens.Jwt.JwtSecurityToken(
+            issuer: "some-other-system",
+            audience: JwtTokenService.TokenAudience,
+            claims: new[] { new Claim(JwtTokenService.UsernameClaimType, "user") },
+            expires: DateTime.UtcNow.AddMinutes(10),
+            signingCredentials: creds);
+        var foreignToken = new System.IdentityModel.Tokens.Jwt.JwtSecurityTokenHandler().WriteToken(foreign);
+
+        var service = CreateJwtService(options);
+        Assert.Null(service.ValidateToken(foreignToken));
+    }
+
+    [Fact]
+    public void ValidateToken_AcceptsOwnIssuerAndAudience()
+    {
+        var service = CreateJwtService();
+        var token = service.GenerateToken("User", "user", new[] { "Admin" }, null);
+        Assert.NotNull(service.ValidateToken(token));
+    }
+
+    // --- Security-007: refresh must preserve the original LastActivity timestamp ---
+
+    [Fact]
+    public void RefreshToken_PreservesOriginalLastActivityClaim()
+    {
+        var service = CreateJwtService();
+
+        // Mint a token whose LastActivity is 20 minutes in the past (still inside the
+        // 30-minute idle window). Refresh must NOT move it forward to "now".
+        var staleActivity = DateTimeOffset.UtcNow.AddMinutes(-20);
+        var principal = new ClaimsPrincipal(new ClaimsIdentity(new[]
+        {
+            new Claim(JwtTokenService.DisplayNameClaimType, "User"),
+            new Claim(JwtTokenService.UsernameClaimType, "user"),
+            new Claim(JwtTokenService.LastActivityClaimType, staleActivity.ToString("o"))
+        }, "test"));
+
+        var refreshed = service.RefreshToken(principal, new[] { "Admin" }, null);
+        Assert.NotNull(refreshed);
+
+        var refreshedPrincipal = service.ValidateToken(refreshed!);
+        Assert.NotNull(refreshedPrincipal);
+        var claim = refreshedPrincipal!.FindFirst(JwtTokenService.LastActivityClaimType);
+        Assert.NotNull(claim);
+        Assert.True(DateTimeOffset.TryParse(claim!.Value, out var carried));
+        // The carried timestamp must equal the original, not "now".
+        Assert.True(Math.Abs((carried - staleActivity).TotalSeconds) < 2,
+            $"LastActivity was reset on refresh: expected ~{staleActivity:o}, got {carried:o}");
+    }
+
+    [Fact]
+    public void RefreshToken_DoesNotResetIdleTimeoutWhenUserIsActuallyIdle()
+    {
+        // A user idle for 25 of the 30-minute window: a refresh fired by some background
+        // request must not make IsIdleTimedOut flip back to false forever.
+        var service = CreateJwtService();
+        var staleActivity = DateTimeOffset.UtcNow.AddMinutes(-25);
+        var principal = new ClaimsPrincipal(new ClaimsIdentity(new[]
+        {
+            new Claim(JwtTokenService.DisplayNameClaimType, "User"),
+            new Claim(JwtTokenService.UsernameClaimType, "user"),
+            new Claim(JwtTokenService.LastActivityClaimType, staleActivity.ToString("o"))
+        }, "test"));
+
+        var refreshed = service.RefreshToken(principal, new[] { "Admin" }, null);
+        var refreshedPrincipal = service.ValidateToken(refreshed!);
+
+        // Still 25 min idle after refresh — not reset to 0.
+        Assert.False(service.IsIdleTimedOut(refreshedPrincipal!)); // 25 < 30, still valid
+        var claim = refreshedPrincipal!.FindFirst(JwtTokenService.LastActivityClaimType);
+        Assert.True(DateTimeOffset.TryParse(claim!.Value, out var carried));
+        Assert.True((DateTimeOffset.UtcNow - carried).TotalMinutes > 20,
+            "Refresh wrongly reset the idle clock to ~now");
+    }
+
+    [Fact]
+    public void RecordActivity_UpdatesLastActivityToNow()
+    {
+        // Genuine user activity (a real request) — distinct from a token refresh —
+        // updates LastActivity to the current time.
+        var service = CreateJwtService();
+        var staleActivity = DateTimeOffset.UtcNow.AddMinutes(-20);
+        var principal = new ClaimsPrincipal(new ClaimsIdentity(new[]
+        {
+            new Claim(JwtTokenService.DisplayNameClaimType, "User"),
+            new Claim(JwtTokenService.UsernameClaimType, "user"),
+            new Claim(JwtTokenService.LastActivityClaimType, staleActivity.ToString("o"))
+        }, "test"));
+
+        var touched = service.RecordActivity(principal, new[] { "Admin" }, null);
+        Assert.NotNull(touched);
+
+        var touchedPrincipal = service.ValidateToken(touched!);
+        var claim = touchedPrincipal!.FindFirst(JwtTokenService.LastActivityClaimType);
+        Assert.True(DateTimeOffset.TryParse(claim!.Value, out var updated));
+        Assert.True((DateTimeOffset.UtcNow - updated).TotalSeconds < 5,
+            "RecordActivity should set LastActivity to ~now");
+    }
+}
+
+#endregion
+
 #region WP-9: Authorization Policy Tests

 public class AuthorizationPolicyTests
Author	SHA1	Message	Date
Joseph Doherty	632d44f38c	fix(host,deployment-manager,communication): repair cross-module DI regressions from batch 1-2 - DeploymentManager-008: revert IConfiguration overload (violated OptionsTests component-convention); Host now binds the ScadaLink:DeploymentManager section - SiteStreamGrpcServer: make test-only int ctor internal so DI sees one public ctor (resolves ambiguous-constructor failure in SiteCompositionRootTests) - Host site composition-root test config: supply Cluster:SeedNodes for the new ClusterOptionsValidator	2026-05-16 21:28:50 -04:00
Joseph Doherty	49fb85e92e	docs(code-reviews): regenerate index after batch 3 medium fixes	2026-05-16 21:22:01 -04:00
Joseph Doherty	30ebbdd183	fix(security): resolve Security-004..007 — configurable user-id attribute, DN escaping, JWT issuer/audience validation, idle-timeout preservation	2026-05-16 21:22:01 -04:00
Joseph Doherty	a702cb96a8	fix(notification-service): resolve NotificationService-005..009 — explicit TLS modes, per-credential token cache, timeout/throttle, address validation, credential redaction	2026-05-16 21:22:01 -04:00
Joseph Doherty	57679d49f2	fix(management-service): resolve ManagementService-004,006,007,013 — PipeTo dispatch, JsonDocument disposal, unified serialization, endpoint tests; re-triage MS-009	2026-05-16 21:22:01 -04:00
Joseph Doherty	da955042aa	fix(inbound-api): resolve InboundAPI-002,004,006,008 — disconnect vs timeout, body size limit, active-node gate; surface InboundAPI-007	2026-05-16 21:22:01 -04:00
Joseph Doherty	6563511b5f	fix(host): resolve Host-003,004 — replace plaintext secrets with env placeholders, validate site seed-node ports; re-triage Host-002	2026-05-16 21:22:01 -04:00