Merge branch 'transport-session-fixes' — abandoned-session release + per-IP unlock throttle enforced (register rows 43, 44)

docs/components/Transport.md

@@ -39,7 +39,15 @@ When a passphrase is supplied, `BundleSecretEncryptor` derives a 256-bit key via
 
 ### BundleSession and session lifecycle
 
-After `LoadAsync` validates and decrypts a bundle, the plaintext content is stored in a `BundleSession` held by the singleton `BundleSessionStore` (a `ConcurrentDictionary<Guid, BundleSession>`). Sessions have a 30-minute TTL (`BundleSessionTtlMinutes`). `BundleSessionEvictionService` sweeps the store every minute so abandoned sessions — and the secrets they carry — are released without waiting for the next `Get` call. `ApplyAsync` explicitly zeros and removes the session on both success and failure (T-007).
+After `LoadAsync` validates and decrypts a bundle, the plaintext content is stored in a `BundleSession` held by the singleton `BundleSessionStore` (a `ConcurrentDictionary<Guid, BundleSession>`). Sessions have a 30-minute TTL (`BundleSessionTtlMinutes`). `BundleSessionEvictionService` sweeps the store every minute, but that only reclaims sessions whose TTL has already **elapsed** — it is a backstop, not an abandonment path. `ApplyAsync` explicitly zeros and removes the session on both success and failure (T-007).
+
+`IBundleImporter.Abandon(sessionId)` is the explicit release for a session that will never be applied, applying the same zero-then-remove ordering as `ApplyAsync`. It is idempotent (double release, unknown id, and already-expired id are all silent no-ops) and, because session ids are random GUIDs handed only to their opener, can never release another caller's session. Every abandonment path calls it:
+
+- Central UI wizard — new upload, Back-to-Upload / `ResetSessionState`, the 3-strike lockout reset, and `Dispose` (navigation away or circuit teardown).
+- CLI `transport preview` — a preview never applies, so the handler releases in a `finally`.
+- CLI `transport import` — covers the aborts that happen before `ApplyAsync` (blocker rows, unresolved site/connection mappings).
+
+Without this, an abandoned wizard or a plain CLI preview held one of the eight `MaxConcurrentImportSessions` slots — and the decrypted bundle content — for the full TTL; eight of them locked out every import on the active node (deferred register row 43, observed live).
 
 ### Conflict resolution and `BundleImportId` correlation
 
@@ -110,7 +118,15 @@ Inbound API keys are explicitly excluded — per re-architecture C4, keys are en
 
 **Phase 1 — `LoadAsync`**: copies the upload stream to a seekable `MemoryStream`, enforces the bundle size cap (`MaxBundleSizeMb`), validates the ZIP envelope (entry count, per-entry decompressed size, compression ratio — all before decompression), reads and validates `manifest.json` (format version, SHA-256 content hash), and decrypts `content.enc` when `Encryption` is present. The decrypted bytes are stored in a new `BundleSession`.
 
-Passphrase lockout operates at two levels: per-bundle (3-strike counter in `BundleSessionStore`, keyed by `ContentHash` so a second browser tab sharing the same bundle bytes cannot reset the counter) and per-IP-per-hour (`BundleUnlockRateLimiter`, default 10 attempts). A successful decrypt clears the per-bundle counter.
+Passphrase lockout operates at two levels: per-bundle (3-strike counter in `BundleSessionStore`, keyed by `ContentHash` so a second browser tab sharing the same bundle bytes cannot reset the counter) and per-IP-per-hour (`BundleUnlockRateLimiter`, `MaxUnlockAttemptsPerIpPerHour`, default 10 attempts). A successful decrypt clears the per-bundle counter.
+
+The two levels interact in a specific order, and it is load-bearing (register row 44 — the limiter had been registered, injected and unit-tested but never called, so only the per-bundle lockout was real):
+
+1. **Per-bundle lockout first.** An already-locked bundle throws `BundleLockedException` without registering a rate-limiter attempt, so hammering a locked bundle cannot spend the client's hourly budget.
+2. **Per-IP throttle next, before the decrypt.** Over budget throws `BundleUnlockThrottledException` (Commons, so the management surface can catch it and return a curated message). Because it precedes the decrypt, a denial burns **no** per-bundle strike — otherwise the throttle would be a way to lock out a bundle someone is legitimately importing. Every unlock attempt is registered, successful ones included: the documented budget counts attempts, not failures.
+3. Only then the AES-GCM decrypt, whose failure increments the per-bundle counter as before.
+
+The client key is the caller's remote IP: the Central UI wizard reads it from `CircuitClientAddressProvider` (a scoped service that snapshots `Connection.RemoteIpAddress` at circuit establishment, since `HttpContext` is gone by the time a circuit submits a passphrase — the same pattern as `CookieAuthenticationStateProvider`), and the management endpoint stamps it on `ManagementEnvelope.ClientIp` for the CLI `transport preview` / `transport import` paths. A caller with no attributable address is counted against a single **shared** bucket rather than skipping the throttle — "unknown client" degrades to a stricter budget, never to none. As with the login throttle (`Component-Security.md`), per-client isolation is only real when ForwardedHeaders is configured for the fronting proxy; behind an untrusted proxy every caller collapses into the proxy's bucket.
 
 ```csharp
 // From BundleImporter.LoadAsync — decrypt path (simplified)
@@ -241,6 +257,10 @@ A template's scripts reference a `SharedScript` or `ExternalSystem` that exists
 
 After 3 wrong passphrase attempts against the same bundle (keyed by `ContentHash`), `BundleImporter.LoadAsync` throws `BundleLockedException`. The session is unusable. Re-upload the bundle file to get a new session with a fresh counter. A `BundleImportUnlockFailed` audit row is written on each failed attempt.
 
+### Unlock throttled ("Too many bundle unlock attempts")
+
+`BundleUnlockThrottledException` — this **client** (not this bundle) has used its `MaxUnlockAttemptsPerIpPerHour` budget within the trailing hour. Re-uploading does not help; the budget is keyed by IP and rolls forward on a sliding window, and the counters are process-local (a central node restart clears them, deliberately, so an outage doesn't strand legitimate operators). The denial burns no per-bundle strike, so the bundle itself is still importable once the window frees up. Raise `ScadaBridge:Transport:MaxUnlockAttemptsPerIpPerHour` if a legitimate workflow (many encrypted bundles in one sitting) trips it routinely.
+
 ### BundleImportAlarmScriptUnresolved / BundleImportCompositionUnresolved warnings
 
 These audit rows appear when the second-pass rewire (`ResolveAlarmScriptLinksAsync` / `ResolveCompositionEdgesAsync`) cannot match a name to a persisted row. The import commits — the FK is left null / the composition row is skipped — but the warning signals an incomplete import. Re-examine the bundle's dependency graph and re-export with the missing artifacts included.

docs/requirements/Component-Transport.md

@@ -112,7 +112,7 @@ The manifest is plaintext so the import wizard can preview bundle contents and s
 flowchart TD
     subgraph T["ZB.MOM.WW.ScadaBridge.Transport"]
         EXPORTER["IBundleExporter<br/>ExportAsync(ExportSelection, Passphrase?, ct) → Stream"]
-        IMPORTER["IBundleImporter<br/>LoadAsync(stream, Passphrase?, ct) → BundleSession<br/>PreviewAsync(sessionId, ct) → ImportPreview<br/>ApplyAsync(sessionId, resolutions, ct) → ImportResult"]
+        IMPORTER["IBundleImporter<br/>LoadAsync(stream, Passphrase?, ct, ClientIp?) → BundleSession<br/>PreviewAsync(sessionId, ct) → ImportPreview<br/>ApplyAsync(sessionId, resolutions, ct) → ImportResult<br/>Abandon(sessionId)"]
         RESOLVER["DependencyResolver"]
         SERIALIZER["BundleSerializer<br/>(manifest + content JSON; ZIP packer)"]
         ENCRYPTOR["BundleSecretEncryptor<br/>(AES-256-GCM + PBKDF2)"]
@@ -136,6 +136,8 @@ flowchart TD
 
 The component is central-hosted. It is registered in `ZB.MOM.WW.ScadaBridge.Host` for central roles only, never for site roles — even when a bundle carries site-scoped config (T18), Transport only writes the central configuration database, never the site clusters. All persistence flows through existing audited repository interfaces in `ZB.MOM.WW.ScadaBridge.ConfigurationDatabase` — the component does not call `DbContext.SaveChangesAsync` directly. `BundleSessionStore` is in-process on the active central node (matching Blazor Server circuit affinity): 30-minute TTL, eviction on expiry, 3-strike passphrase lockout per session, and a `MaxConcurrentImportSessions` cap (default 8; arch-review 05 Task 24 — startup-validated as `> 0` by `TransportOptionsValidator`, since a zero/negative cap would reject every import session forever) bounding the decrypted-content footprint since each open session pins a fully-decrypted bundle in memory.
 
+**Session release is explicit, not TTL-only (register row 43).** `ApplyAsync` releases the session it applied (zero-then-remove, T-007), and `IBundleImporter.Abandon(sessionId)` does the same for a session that will never be applied. Every abandonment path calls it: the Central UI wizard on new upload / Back-to-Upload / `ResetSessionState` / lockout reset / `Dispose` (navigation away or circuit teardown), the CLI `transport preview` handler (a preview never applies), and the CLI `transport import` handler for the aborts that precede apply (blocker rows, unresolved mappings). `Abandon` is idempotent — double release, an unknown id, and an already-expired id are all silent no-ops — and since session ids are random GUIDs handed only to their opener, abandoning the id you hold can never evict another operator's in-progress import. Relying on the TTL alone was an availability *and* secret-lifetime defect: eight abandoned wizards exhausted `MaxConcurrentImportSessions` and locked out every import on the active node for 30 minutes, each still holding its decrypted bundle content.
+
 **Preview→apply window (recorded decision).** The diff/preview an operator reviews is a point-in-time snapshot; there is **no optimistic concurrency** between preview and apply. If the target config changes in the window between `PreviewAsync` and `ApplyAsync` (another admin edits a template, say), the apply proceeds against the current state — the resolutions are keyed by `(EntityType, Name)`, so they still target the right rows, but the operator may not have seen the latest state in the diff. This is accepted for v1 (single-operator import is the expected workflow, and the terminal Confirm step already gates a typo-resistant environment-name entry). The bundle-format `version` fields are reserved for a future optimistic-concurrency check but are not yet enforced (Underdeveloped 5).
 
 ## Export Flow
@@ -334,7 +336,7 @@ Imports are all-or-nothing per bundle. A bundle either applies fully or not at a
 
 - **AES-256-GCM** for content encryption with **PBKDF2-SHA256 / 600 000 iterations** (OWASP 2023+ guidance), per-bundle random salt, random IV per encryption. The GCM auth tag is verified before decryption — a wrong passphrase fails cleanly.
 - **Passphrase never persisted.** It lives only inside the export/import service call path and is discarded after use. No environment variable, no log line.
-- **Failed-unlock rate limit:** per-session 3-strike lockout; per-IP-per-hour cap (default 10, configurable) to deter brute force against a stolen bundle. Each failed attempt produces a `BundleImportUnlockFailed` audit row.
+- **Failed-unlock rate limit:** per-bundle 3-strike lockout (keyed by `ContentHash`, server-side); per-IP-per-hour cap (`MaxUnlockAttemptsPerIpPerHour`, default 10, configurable) to deter brute force against a stolen bundle. Each failed attempt produces a `BundleImportUnlockFailed` audit row. The two are evaluated in order — bundle lockout, then per-IP throttle, then decrypt — so hammering an already-locked bundle spends no hourly budget, and a throttle denial (`BundleUnlockThrottledException`, fail-closed with a curated message on both the UI and CLI surfaces) burns **no** per-bundle strike, since it never reaches the decryptor. Successful unlocks count against the hourly budget too: it caps attempts, not failures. The client key is the caller's remote IP — from `CircuitClientAddressProvider` (snapshotted at Blazor circuit establishment, since `HttpContext` is unavailable mid-circuit) for the Central UI, and from `ManagementEnvelope.ClientIp` for the CLI; a caller with no attributable address shares a single stricter bucket rather than bypassing the throttle. Per-client isolation is only real behind a ForwardedHeaders-trusted proxy, the same caveat the login throttle carries in `Component-Security.md`. (Register row 44: the limiter existed, was configured and unit-tested, but was never invoked until this was wired.)
 - **Bundle size cap** on upload (default 100 MB, configurable) to bound memory.
 - **In-transit:** existing HTTPS to the Central UI; no new channel.
 - **Audit trail is the chain of custody.** Every export, every import (including aborted ones at validation), and every unlock failure is audit-logged with source env, content hash, encrypted yes/no, and artifact summary.