Compare commits
8 Commits
9cf24a4496
...
c6769ee95e
| Author | SHA1 | Date | |
|---|---|---|---|
| c6769ee95e | |||
| e59fa5ba4c | |||
| 5a1b4e30e6 | |||
| 5ab26a4233 | |||
| 23390a28ee | |||
| 300d49922e | |||
| 9e7697359e | |||
| 708389829b |
@@ -48,7 +48,7 @@ from `Component-NotificationOutbox.md`. No Critical findings; two High, six Medi
|
||||
| 3 | Concurrency & thread safety | Yes | Actor state mutated only on actor thread. Inherited CD-015 race on `InsertIfNotExistsAsync` (NotificationOutbox-005) is the only race; the dispatcher's in-flight guard correctly serializes sweeps. |
|
||||
| 4 | Error handling & resilience | Yes | Outer try/catch on `RunDispatchPass`/`RunPurgePass` keeps the in-flight guard sane; per-notification isolation is correct. CT not threaded into delivery (NotificationOutbox-003). |
|
||||
| 5 | Security | Yes | Inherited OAuth2 empty-user (NotificationOutbox-001) reachable through the adapter. No new credential or trust-boundary issues introduced by the outbox itself. |
|
||||
| 6 | Performance & resource management | Yes | Dispatch interval & batch size are simple polling; `ResolveAdapters` rebuilds the lookup per sweep (NotificationOutbox-006). No leaks. |
|
||||
| 6 | Performance & resource management | Yes | Dispatch interval & batch size are simple polling; `ResolveAdapters` rebuilds the lookup per sweep (NotificationOutbox-006 — its 2026-05-28 caching fix was later **superseded**, see the finding). No leaks. |
|
||||
| 7 | Design-document adherence | Yes | `NotificationOutboxOptions.DispatchBatchSize`, `DeliveredKpiWindow`, `PurgeInterval` are not in the design doc (NotificationOutbox-007). |
|
||||
| 8 | Code organization & conventions | Yes | Options class lives in the component project (correct); DI extension lives in the component (correct); adapter is `scoped`, actor singleton — interaction correctly documented in `ServiceCollectionExtensions`. No issues. |
|
||||
| 9 | Testing coverage | Yes | Solid actor-behaviour coverage. Missing tests for `FallbackMaxRetries` / empty-SMTP-config dispatch path (NotificationOutbox-008). |
|
||||
@@ -312,6 +312,8 @@ stateful intent (timeouts, circuit breakers) cannot accidentally lose state.
|
||||
|
||||
**Resolution (2026-05-28):** Cached the `NotificationType → adapter` dictionary on a new actor field `_adaptersCache`, built lazily on the first dispatch sweep that needs it. The cache is paired with an actor-lifetime `IServiceScope` (`_adaptersScope`, created on first use and disposed in `PostStop`) so the resolved scoped adapter instances live as long as the cache itself — respecting `EmailNotificationDeliveryAdapter`'s scoped lifetime without leaking captive-DbContext references across the actor's full lifetime. `RunDispatchPass` now calls `ResolveAdapters()` (instance method, no args), and the existing "resolved from the per-sweep scope" comment was rewritten to cross-reference the new cache rationale. Adapter set is static per process lifetime (confirmed against the DI registration in `AddNotificationOutbox`), so no invalidation hook is needed. Regression test `Dispatch_ResolvesAdaptersOnce_AcrossMultipleSweeps` registers a counting factory and pins the resolution count at exactly 1 across three end-to-end dispatch sweeps.
|
||||
|
||||
**SUPERSEDED 2026-08-15 (deferred register row 45).** The 2026-05-28 resolution above was **wrong and is reverted** — it was written before the dispatcher gained bounded parallel delivery (`MaxParallelDeliveries`, default 4). An actor-lifetime adapter instance is a *captive scoped service*: the adapter holds a scoped `INotificationRepository` over one EF `DbContext`, so all four concurrent deliveries ran on that single context and central-b logged `InvalidOperationException: A second operation was started on this context instance before a previous operation completed` continuously on the rig. The collided delivery's row is never written and the wasted attempt counts against its retry budget, so sustained contention could park a legitimate notification. `_adaptersCache`, `_adaptersScope` and `ResolveAdapters()` are deleted; `DeliverGatedAsync` now resolves the adapter for the notification's `Type` from the per-delivery DI scope it already opens (static `ResolveAdapter(IServiceProvider, NotificationType)`, last-wins preserved). The original NotificationOutbox-006 concern — "a future stateful adapter (rate limiter, circuit breaker) silently loses its state" — is answered by the design rule instead of by caching: an adapter must carry no cross-delivery state, and anything genuinely shared belongs in a singleton/factory it depends on (as the SMTP client factory and `IHttpClientFactory` already are). `Dispatch_ResolvesAdaptersOnce_AcrossMultipleSweeps` is replaced by `ParallelDeliveries_EachGetTheirOwnAdapterAndDbContext` (a simulated per-repository DbContext that throws EF's exact second-operation exception; fail-verified against the cached build) and `Dispatch_ResolvesAdapterPerDelivery_NotOncePerActorLifetime`. See [Component-NotificationOutbox.md](../../docs/requirements/Component-NotificationOutbox.md), Delivery Adapters.
|
||||
|
||||
### NotificationOutbox-007 — `NotificationOutboxOptions.DispatchBatchSize`, `DeliveredKpiWindow`, and `PurgeInterval` are not in the design document
|
||||
|
||||
| | |
|
||||
|
||||
@@ -25,13 +25,15 @@
|
||||
}
|
||||
},
|
||||
"Database": {
|
||||
"_comment": "ConfigurationDb/MachineDataDb (which carry the dev SQL password) are supplied as ScadaBridge__Database__* whole-key env overrides in docker/docker-compose.yml (dev-only-insecure, mirroring the ApiKeyPepper convention). The same dev password already ships committed for the sibling scadabridge-mssql container (infra/docker-compose.yml MSSQL_SA_PASSWORD) — this is consolidation, not new exposure. Env overrides layer over JSON before the ${secret:} expander runs, so the dev cluster boots with no KEK/secret store. Real/prod config uses ${secret:} tokens in src/.../appsettings.Central.json (T4)."
|
||||
"_comment": "ConfigurationDb/MachineDataDb (which carry the dev SQL password) are supplied as ScadaBridge__Database__* whole-key env overrides in docker/docker-compose.yml (dev-only-insecure, mirroring the ApiKeyPepper convention). The same dev password already ships committed for the sibling scadabridge-mssql container (infra/docker-compose.yml MSSQL_SA_PASSWORD) \u2014 this is consolidation, not new exposure. Env overrides layer over JSON before the ${secret:} expander runs, so the dev cluster boots with no KEK/secret store. Real/prod config uses ${secret:} tokens in src/.../appsettings.Central.json (T4)."
|
||||
},
|
||||
"Security": {
|
||||
"Ldap": {
|
||||
"Server": "host.docker.internal",
|
||||
"Port": 3893,
|
||||
"FallbackServers": [ "host.docker.internal:3894" ],
|
||||
"FallbackServers": [
|
||||
"host.docker.internal:3894"
|
||||
],
|
||||
"_comment_FallbackServers": "Redundant local GLAuth pair (scadaproj/infra/glauth-redundant): zb-ldap-primary :3893 + zb-ldap-backup :3894 on the Docker host. Replaces the shared 10.100.0.35 GLAuth, whose serviceaccount password was rotated (SEC-36) away from the rig's dev bind creds. Also live-exercises Auth 0.2.x FallbackServers sticky failover.",
|
||||
"Transport": "None",
|
||||
"AllowInsecure": true,
|
||||
@@ -50,7 +52,9 @@
|
||||
"ForwardedHeaders": {
|
||||
"_comment": "Traefik fronts this node on the external scadabridge-net docker network. Trust X-Forwarded-For from the docker bridge address pool so LoginThrottle keys on the real client IP (arch-review R2 N2). Narrow to the Traefik container IP if the network is pinned.",
|
||||
"Enabled": true,
|
||||
"KnownNetworks": [ "172.16.0.0/12" ]
|
||||
"KnownNetworks": [
|
||||
"172.16.0.0/12"
|
||||
]
|
||||
}
|
||||
},
|
||||
"Communication": {
|
||||
@@ -89,10 +93,12 @@
|
||||
"DefaultMaxSeriesPoints": 200
|
||||
},
|
||||
"Transport": {
|
||||
"SourceEnvironment": "docker-cluster"
|
||||
"SourceEnvironment": "docker-cluster",
|
||||
"MaxUnlockAttemptsPerIpPerHour": 500,
|
||||
"_comment_MaxUnlockAttemptsPerIpPerHour": "Test-rig override (row-44 throttle landed 2026-08-15): the production default of 10/hour/IP throttles the Playwright transport suite after ~2 back-to-back runs (all tests unlock from one host IP). 500 keeps the throttle path live while making repeated suite runs practical."
|
||||
},
|
||||
"Logging": {
|
||||
"MinimumLevel": "Information"
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -25,13 +25,15 @@
|
||||
}
|
||||
},
|
||||
"Database": {
|
||||
"_comment": "ConfigurationDb/MachineDataDb (which carry the dev SQL password) are supplied as ScadaBridge__Database__* whole-key env overrides in docker/docker-compose.yml (dev-only-insecure, mirroring the ApiKeyPepper convention). The same dev password already ships committed for the sibling scadabridge-mssql container (infra/docker-compose.yml MSSQL_SA_PASSWORD) — this is consolidation, not new exposure. Env overrides layer over JSON before the ${secret:} expander runs, so the dev cluster boots with no KEK/secret store. Real/prod config uses ${secret:} tokens in src/.../appsettings.Central.json (T4)."
|
||||
"_comment": "ConfigurationDb/MachineDataDb (which carry the dev SQL password) are supplied as ScadaBridge__Database__* whole-key env overrides in docker/docker-compose.yml (dev-only-insecure, mirroring the ApiKeyPepper convention). The same dev password already ships committed for the sibling scadabridge-mssql container (infra/docker-compose.yml MSSQL_SA_PASSWORD) \u2014 this is consolidation, not new exposure. Env overrides layer over JSON before the ${secret:} expander runs, so the dev cluster boots with no KEK/secret store. Real/prod config uses ${secret:} tokens in src/.../appsettings.Central.json (T4)."
|
||||
},
|
||||
"Security": {
|
||||
"Ldap": {
|
||||
"Server": "host.docker.internal",
|
||||
"Port": 3893,
|
||||
"FallbackServers": [ "host.docker.internal:3894" ],
|
||||
"FallbackServers": [
|
||||
"host.docker.internal:3894"
|
||||
],
|
||||
"_comment_FallbackServers": "Redundant local GLAuth pair (scadaproj/infra/glauth-redundant): zb-ldap-primary :3893 + zb-ldap-backup :3894 on the Docker host. Replaces the shared 10.100.0.35 GLAuth, whose serviceaccount password was rotated (SEC-36) away from the rig's dev bind creds. Also live-exercises Auth 0.2.x FallbackServers sticky failover.",
|
||||
"Transport": "None",
|
||||
"AllowInsecure": true,
|
||||
@@ -50,7 +52,9 @@
|
||||
"ForwardedHeaders": {
|
||||
"_comment": "Traefik fronts this node on the external scadabridge-net docker network. Trust X-Forwarded-For from the docker bridge address pool so LoginThrottle keys on the real client IP (arch-review R2 N2). Narrow to the Traefik container IP if the network is pinned.",
|
||||
"Enabled": true,
|
||||
"KnownNetworks": [ "172.16.0.0/12" ]
|
||||
"KnownNetworks": [
|
||||
"172.16.0.0/12"
|
||||
]
|
||||
}
|
||||
},
|
||||
"Communication": {
|
||||
@@ -89,10 +93,12 @@
|
||||
"DefaultMaxSeriesPoints": 200
|
||||
},
|
||||
"Transport": {
|
||||
"SourceEnvironment": "docker-cluster"
|
||||
"SourceEnvironment": "docker-cluster",
|
||||
"MaxUnlockAttemptsPerIpPerHour": 500,
|
||||
"_comment_MaxUnlockAttemptsPerIpPerHour": "Test-rig override (row-44 throttle landed 2026-08-15): the production default of 10/hour/IP throttles the Playwright transport suite after ~2 back-to-back runs (all tests unlock from one host IP). 500 keeps the throttle path live while making repeated suite runs practical."
|
||||
},
|
||||
"Logging": {
|
||||
"MinimumLevel": "Information"
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -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.
|
||||
|
||||
@@ -36,12 +36,17 @@ All 7 fix-now items landed via PLAN-04/05/06/07/08 (verified in review 08 round
|
||||
| 40 | **Wall-clock sleeps used as synchronization in tests (absence-assertion class)** | Found by the R5 sweep (`9fb52153`) — a different class from the one swept, deliberately left unfixed | Tests that sleep, then assert an *absence*: `ScriptDeadlineAtEnqueueTests.cs:105,128` (two `Thread.Sleep(2s)`, with `Assert.Equal(0, scheduler.DetachedThreadCount)` depending on the sleep out-racing the watchdog — worst offender), `DeploymentWarmThenGateTests.cs:117`, `SiteAlarmLiveCacheServiceTests.cs:195,216`, `DataConnectionActorTests.cs:435` (`Task.Delay(200)` to "settle" after an `AwaitCondition`). These fail toward false-pass rather than flake, so they don't trip CI — but each is a timing pin that weakens what the test proves. Fix pattern: replace the sleep with an explicit observable edge (or assert on the positive event with a bounded wait plus a negative control). | Next test-infra session, or the first time one of these areas regresses without its test catching it |
|
||||
| 41 | **LocalDb (scadaproj) sync-session tests flake under parallel load** (scadaproj repo, not this one) | Found by R1; confirmed pre-existing at base `f627cee` (1 failure in 3 full runs) | `SyncSessionTests.Handshake_DigestMismatch_FailsClosed` and `BatchBudgetTests.Pump_RowLargerThanTheBudget_IsSentAloneRatherThanStallingTheStream` intermittently time out. Root cause looks structural, not a timing pin: on a handshake fault, `RunAsync`'s catch cancels the writer loop, which can kill the session's OWN Handshake before it transmits — the healthy peer then waits out its full 15 s cap for a message that never comes. Product-adjacent (the same early-cancel could delay a real peer's fail-closed verdict), so worth a look beyond the tests. | Next LocalDb session |
|
||||
| 42 | **`build/push.sh`-class publish bug across the family repos** | Found by R1 the hard way (incident: 25 unrelated packages briefly published to the feed, deleted + verified same session; script fixed in scadaproj `0038d46`) | The bug shape: `dotnet pack -c Release -o ./artifacts` with no project argument and no `cd`, so the script packs the CALLER'S CWD. Fixed in LocalDb's script (cd to own repo root, name the solution, absolute output path). Peer repos' publish scripts (MxAccessGateway, HistorianGateway, OtOpcUa, others in scadaproj) should be checked for the same shape — the recovery only worked because the token happened to carry delete rights. | Before anyone runs a family publish script from outside its repo, or next umbrella-repo session |
|
||||
| 43 | **Central UI import wizard leaks a `BundleSession` slot on every abandoned import — imports lock out for 30 min** *(High — availability + secret lifetime)* | Found live by R6 (reproduced: 3 back-to-back suite runs → 6 of 8 slots held, rotating pair of `TransportImportTests` failures; central-b restart cleared it) | `TransportImport.razor.cs`: `ResetSessionState()` nulls the local field and `Dispose()` deletes only the staged temp file — neither calls `IBundleSessionStore.Remove(sessionId)`; only `BundleImporter.ApplyAsync` removes sessions. An operator who unlocks a bundle then navigates away burns one of `MaxConcurrentImportSessions` (8) for the full `BundleSessionTtlMinutes` (30); eight abandonments lock out ALL imports on the active node. The leaked session also holds the **decrypted** bundle content that T-007 deliberately zeroes on the Apply paths — so this is a secret-lifetime hole too. The class doc's "an abandoned wizard does not leak" claim is true of the plaintext file only. Fix: remove the session (and zero its content) from `Dispose`/`ResetSessionState`/back-navigation. | Fix-now candidate — next session touching Transport or Central UI |
|
||||
| 44 | **`BundleUnlockRateLimiter` is dead code — the documented per-IP unlock throttle is not enforced** *(Medium — security)* | Found by R6 | Registered, injected into `BundleImporter`, unit-tested, documented on `TransportOptions.MaxUnlockAttemptsPerIpPerHour = 10`, pinned in `appsettings.Central.json` — but `_unlockRateLimiter` is never invoked anywhere in `src/`. A brute-force passphrase attacker is bounded only by the per-bundle 3-strike lockout, not the advertised 10/hour/IP cap. Fix: call it on the unlock path (or delete it and the config surface honestly). | Fix-now candidate — next Transport/security session |
|
||||
| 45 | **Notification Outbox dispatcher shares one `DbContext` across up to 4 parallel deliveries** *(Medium — correctness under load)* | Found by R6; observed continuously on rig central-b: `InvalidOperationException: A second operation was started on this context instance` on dispatch | `NotificationOutboxActor.ResolveAdapters()` caches adapter instances in an actor-lifetime `IServiceScope` while the Email/Sms adapters are `AddScoped` and hold a scoped `INotificationRepository` (`ScadaBridgeDbContext`); `MaxParallelDeliveries` (4) then shares one context across concurrent deliveries. The per-delivery scope added in `DeliverGatedAsync` for exactly this hazard covers only `INotificationOutboxRepository` — the adapter escapes it. Effect today: failed attempts + retries + error noise; sustained contention could burn a row to `Parked`. Fix: resolve adapters inside the per-delivery scope. | Fix-now candidate — next Notification Outbox session, or first `Parked` row attributed to it |
|
||||
| 43 | ~~**Central UI import wizard leaks a `BundleSession` slot on every abandoned import — imports lock out for 30 min**~~ **RESOLVED 2026-08-15 (`70838982`).** New `IBundleImporter.Abandon(sessionId)` does the same zero-then-remove release `ApplyAsync` already did (T-007 discipline), idempotent (double/unknown/expired ids are no-ops) and only ever given the caller's own random-GUID id, so it cannot evict a concurrent circuit's session. Wired into EVERY abandonment path: wizard new-upload, `TryLoadAsync` pre-overwrite, Back-to-Upload/`ResetSessionState`, 3-strike lockout reset, `Dispose` — plus two CLI leaks the original row didn't know about: `transport preview` released nothing at all (every preview burned a slot for 30 min) and `transport import` stranded the session on pre-apply aborts; both now release in a `finally`. `BundleSessionEvictionService` is documented as the TTL backstop, not an abandonment path. Fail-before-fix stash-verified (3 of 4 new bUnit tests fail pre-fix). 10 new tests. | Found live by R6 | Closed 2026-08-15. | — |
|
||||
| 44 | ~~**`BundleUnlockRateLimiter` is dead code — the documented per-IP unlock throttle is not enforced**~~ **RESOLVED 2026-08-15 (`5ab26a42`).** `BundleImporter.LoadAsync` now registers every unlock attempt against `MaxUnlockAttemptsPerIpPerHour` (10). Ordering is load-bearing: AFTER the per-bundle 3-strike check (hammering a locked bundle spends no hourly budget) and BEFORE the decrypt (a throttle denial burns no per-bundle strike, so the throttle can't be abused to lock out a legitimate import); successful unlocks count too. Denial = `BundleUnlockThrottledException` (placed in Commons so the management surface returns a curated message). Client key: Central UI via a scoped `CircuitClientAddressProvider` snapshotting the IP at circuit establishment; CLI via additive `ManagementEnvelope.ClientIp`. No attributable IP falls into one shared `<unattributed>` bucket — stricter, never a bypass; per-client isolation is only real behind a ForwardedHeaders-trusted proxy (same caveat as the login throttle, documented). Fail-before-fix verified (4 of 6 new tests fail with the limiter neutralised; 2 negative controls). | Found by R6 | Closed 2026-08-15. | — |
|
||||
| 45 | ~~**Notification Outbox dispatcher shares one `DbContext` across up to 4 parallel deliveries**~~ **RESOLVED 2026-08-15 (`9e769735`).** `_adaptersCache`/`_adaptersScope`/`ResolveAdapters()` deleted; the typed adapter is resolved inside the per-delivery DI scope `DeliverGatedAsync` already opens (static `ResolveAdapter` preserving DI last-wins + the null-adapter park path), so each concurrent delivery gets its own adapter and `DbContext`. Nothing shared was lost: the Email adapter builds a fresh SMTP client per attempt from an injected factory, the SMS adapter draws from the singleton `IHttpClientFactory`, and CentralUI's adapter-gated Type selector (`NotificationChannelCatalog`) is an independent scoped consumer, untouched. Fail-before-fix stash-verified (simulated DbContext throwing EF's exact second-operation exception under a rendezvous-guaranteed overlap: 1 of 2 rows persisted pre-fix; 1-vs-4 distinct adapter instances). Root cause of note: the bug was INTRODUCED by code-review finding NotificationOutbox-006's caching "fix" (2026-05-28), whose own regression test pinned the broken behavior and predates bounded parallel delivery — that finding is marked SUPERSEDED. NotificationOutbox 153, AuditLog 383 green. | Found by R6; observed live on rig central-b | Closed 2026-08-15. | — |
|
||||
| 46 | **CLI `audit-config query` returns `{}` for every filter combination** *(Low)* | Found by R6 against the rig; underlying `AuditLogEntries` rows verified present | Possibly a CLI output-shape/serialization issue; not investigated (out of R6 scope). | Next CLI session |
|
||||
| 47 | ~~**`SiteStreamGrpcServer` duplicate-stream replacement disposed another handler's `CancellationTokenSource`**~~ **RESOLVED 2026-08-15 (found and fixed in-flight by R2, `e6842c10`).** `SubscribeInstance`'s duplicate-prevention path called `Cancel()` AND `Dispose()` on the replaced stream's CTS, which belongs to that still-running handler's `using var` and is still read — escaping as an unhandled `ObjectDisposedException` under scheduling pressure (confirmed pre-existing on `2b74851f`; load-only, passed 10/10 isolated). Fixed to cancel-only (the owner's `using` disposes exactly once), `Cancel` wrapped for the converse race, mirroring `CancelAllStreams()`. Deterministic regression test via a gate inside the setup window, fail-before/pass-after verified by reinstating the `Dispose()` as a negative control. Recorded here (though closed) because it is a product-bug instance of the disposal-race class, useful precedent for future stream-lifecycle work. | Closed 2026-08-15. |
|
||||
| 48 | **Docker rig has no running site instances — live stream probes have no traffic** | Found during the R2 close-out live probe (2026-08-15) | All 7 seeded instances are `soak-*` rows whose deploy fails pre-deployment validation (34 errors: ScriptCompilation ×4, ConnectionBinding ×30 — their bound data connections no longer exist on the sites), and `debug snapshot` returns `instanceNotFound`; `scadabridge_site_connection_up` is 0 on all six site nodes. Consequence: the R2 batching histogram (`scadabridge.site.stream.batch_size`) could not be observed live — batching verification rests on its integration/wire-skew/latency tests, which are thorough. Fix: re-seed a small working instance + data connection set (OPC UA sim is still up) so future live probes have traffic. | Next rig session, or before any live probe that needs event traffic |
|
||||
| 49 | **Sweep code-review "Resolved" entries that cached or lifetime-widened a DI-scoped service** *(Low, process)* | Found while fixing row 45: NotificationOutbox-006, a Low perf nit, introduced the row-45 Medium correctness bug, and its regression test actively pinned the broken behavior — written before `MaxParallelDeliveries` existed and never revisited when parallel delivery landed | The failure shape generalizes: a review resolution that widens a scoped service's lifetime is correct at the time and silently invalidated by a later concurrency feature. Sweep `code-reviews/*/findings.md` resolved entries for caching/lifetime-widening of scoped services and re-check each against features added after its date. | Next code-review or test-infra session |
|
||||
| 50 | **No guard against capturing a scoped service on a singleton actor (captive dependency)** *(Low)* | Found while fixing row 45 | The actors take root `IServiceProvider` rather than typed services, which defeats .NET's built-in scope validation — the row-45 class of bug is invisible at build time. Cheap mitigations: `ValidateScopes`/`ValidateOnBuild` on the central host's provider in Development, and/or a test asserting no `ReceiveActor` field holds an `IServiceScope`. | Next DI/host-hardening session |
|
||||
| 51 | **`BundleLockedException` reaches the CLI as "An internal error occurred"** *(Low, pre-existing)* | Found while fixing row 44 | `ManagementService` doesn't reference `Transport`, so `MapFault` can't curate the 3-strike lockout — a CLI operator gets a correlation id, not the reason. Row 44 solved this shape for the throttle exception by placing it in Commons; `BundleLockedException` needs the same move (public type relocation, low risk, deliberately kept out of the row-44 diff). | Next Transport/CLI session |
|
||||
| 52 | **The unlock throttle's counters are per-node and per-process** *(Low, by design but recorded)* | Row 44 implementation note; documented in `docs/components/Transport.md` | Each central node has its own limiter, so the effective fleet cap is 10/hour/IP per node, and a restart clears the window. Acceptable for the threat model today; a shared store is the hardening if it ever matters. | A hardening pass that wants a fleet-wide cap |
|
||||
| 53 | **`MaxConcurrentImportSessions` has no observability** *(Low)* | Found while fixing row 43 — R6 could only infer the leak from rotating test failures | Nothing counts open sessions or logs when `Open` rejects at the cap. A gauge (mind the `ObservedMeters` allowlist) or a Warning log on rejection would make a recurrence diagnosable in seconds. | Next Transport or observability session |
|
||||
|
||||
## Resolved (verified against the code 2026-07-10)
|
||||
Rows removed from the Deferred table above once confirmed shipped. Kept here for traceability.
|
||||
|
||||
@@ -117,7 +117,7 @@ The dispatcher loop runs on a fixed interval. On each tick the `NotificationOutb
|
||||
|
||||
1. Polls the `Notifications` table for **due rows** — `Pending` rows, and `Retrying` rows whose `NextAttemptAt` has passed.
|
||||
2. Resolves the target notification list to its recipients/targets at central, at delivery time.
|
||||
3. Hands the notification to the delivery adapter registered for its `Type`. Deliveries within the sweep run with bounded parallelism (`MaxParallelDeliveries`, default 4), each on its own DI scope/repository, off the actor thread.
|
||||
3. Hands the notification to the delivery adapter registered for its `Type`. Deliveries within the sweep run with bounded parallelism (`MaxParallelDeliveries`, default 4), each on its own DI scope, off the actor thread. **That per-delivery scope owns the channel adapter as well as the outbox repository** — see Delivery Adapters below.
|
||||
4. Applies the result:
|
||||
- **success** → `Delivered`, set `DeliveredAt`, snapshot `ResolvedTargets`.
|
||||
- **transient failure** → `Retrying`, increment `RetryCount`, set `NextAttemptAt`, record `LastError`; once retries are exhausted → `Parked`.
|
||||
@@ -136,7 +136,11 @@ A delivery adapter implementing `INotificationDeliveryAdapter` is registered per
|
||||
- **Email adapter.** The existing SMTP composition/send logic, relocated to the central cluster. Sends a single BCC email to all list recipients.
|
||||
- **SMS adapter (Twilio REST).** Added in T9/T10 (2026-06-19). Sends one Twilio REST request per recipient phone number. Per-recipient results are rolled up: all-accepted → `Success`; any-transient → retry/park; mix of accepted + permanent-bad → `Success` with bad numbers noted in `LastError`; all-permanent → `Permanent` (Park). See [Component-NotificationService.md](Component-NotificationService.md), SMS Delivery Adapter.
|
||||
|
||||
The outbox dispatches by looking up the adapter registered for the notification's `Type`. If no adapter is registered for a given `Type`, the notification is parked with a "no adapter" error — the seam is open for future delivery channels.
|
||||
The outbox dispatches by looking up the adapter registered for the notification's `Type`. If no adapter is registered for a given `Type`, the notification is parked with a "no adapter" error — the seam is open for future delivery channels. Where more than one adapter declares the same `Type`, the last registered wins, mirroring DI's own last-wins resolution.
|
||||
|
||||
**Adapters are scoped services, resolved once per delivery.** Both shipped adapters hold a scoped `INotificationRepository` over the EF `DbContext`, so an adapter instance is bound to exactly one DbContext. The dispatcher therefore resolves the matching adapter *inside the per-delivery DI scope* it already opens for the outbox repository — never once per sweep, and never cached on the singleton actor. Sharing one adapter across the `MaxParallelDeliveries` concurrent deliveries would put several deliveries on one DbContext, which fails with `InvalidOperationException: A second operation was started on this context instance before a previous operation completed`; the affected delivery's outcome is lost, its row is left unwritten, and the wasted attempt eats into the row's retry budget toward `Parked`. An adapter must accordingly carry **no state across deliveries**: anything genuinely shared (connection pools, token caches) belongs in a singleton or factory the adapter depends on — as the Email adapter's SMTP client factory and the SMS adapter's `IHttpClientFactory` already are — not in adapter fields.
|
||||
|
||||
The Central UI's channel catalog is a separate consumer of the same registrations: it enumerates the registered adapters' `Type` values to populate the notification-list Type selector, so the set of offerable channels can never drift from the set the central node can actually deliver.
|
||||
|
||||
Delivery adapters are provided by the Notification Service, which manages notification-list, SMTP, and SMS definitions and supplies the stateless per-type "deliver one notification" implementations. The SMS `SmsConfiguration` (including encrypted Auth Token) travels in Transport bundles alongside SMTP config.
|
||||
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -0,0 +1,53 @@
|
||||
using Microsoft.AspNetCore.Http;
|
||||
|
||||
namespace ZB.MOM.WW.ScadaBridge.CentralUI.Auth;
|
||||
|
||||
/// <summary>
|
||||
/// Supplies the remote IP of the client that established this Blazor circuit.
|
||||
/// <para>
|
||||
/// Same constraint — and the same solution — as
|
||||
/// <see cref="CookieAuthenticationStateProvider"/>: <see cref="IHttpContextAccessor.HttpContext"/>
|
||||
/// is valid only during the initial HTTP request that establishes the circuit and
|
||||
/// is null (or stale/foreign) for the life of the SignalR connection afterwards.
|
||||
/// Registered <c>Scoped</c>, this type is constructed inside that request's DI
|
||||
/// scope, so it snapshots <c>Connection.RemoteIpAddress</c> <b>once</b> in the
|
||||
/// constructor and serves that snapshot for the circuit's lifetime.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// The value is only as trustworthy as the proxy configuration: it is whatever
|
||||
/// Kestrel reports as the connection's remote address, which <c>ForwardedHeaders</c>
|
||||
/// (see <c>ForwardedHeadersSetup</c>, enabled for the shipped Traefik topologies)
|
||||
/// rewrites from <c>X-Forwarded-For</c> for CONFIGURED proxies only. Behind an
|
||||
/// untrusted proxy every circuit collapses onto the proxy's address — consumers
|
||||
/// must treat that as a shared bucket, not as per-client isolation.
|
||||
/// </para>
|
||||
/// </summary>
|
||||
public sealed class CircuitClientAddressProvider
|
||||
{
|
||||
/// <summary>
|
||||
/// Snapshots the client address from the current HTTP context for use throughout
|
||||
/// the circuit lifetime.
|
||||
/// </summary>
|
||||
/// <param name="httpContextAccessor">Accessor used to read the circuit-establishing request.</param>
|
||||
public CircuitClientAddressProvider(IHttpContextAccessor httpContextAccessor)
|
||||
{
|
||||
ArgumentNullException.ThrowIfNull(httpContextAccessor);
|
||||
ClientAddress = httpContextAccessor.HttpContext?.Connection.RemoteIpAddress?.ToString();
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Test seam: constructs the provider with an explicit address instead of reading
|
||||
/// an <see cref="IHttpContextAccessor"/>.
|
||||
/// </summary>
|
||||
/// <param name="clientAddress">The address to serve, or null for "unknown".</param>
|
||||
public CircuitClientAddressProvider(string? clientAddress)
|
||||
{
|
||||
ClientAddress = clientAddress;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// The remote IP of the circuit-establishing request, or null when it could not be
|
||||
/// determined (no HTTP context, or a transport that reports no remote address).
|
||||
/// </summary>
|
||||
public string? ClientAddress { get; }
|
||||
}
|
||||
+88
-6
@@ -51,6 +51,21 @@ namespace ZB.MOM.WW.ScadaBridge.CentralUI.Components.Pages.Design;
|
||||
/// retained on the component. The file is deleted on Back-to-Upload / Reset /
|
||||
/// successful Apply / component Dispose, so an abandoned wizard does not leak
|
||||
/// staged bundle plaintext beyond circuit teardown.
|
||||
///
|
||||
/// <para>
|
||||
/// Server-side session release (register row 43): the staged temp file is only
|
||||
/// half of an abandoned wizard's footprint — the server-side
|
||||
/// <see cref="BundleSession"/> opened by <see cref="IBundleImporter.LoadAsync"/>
|
||||
/// holds the DECRYPTED bundle content and occupies one of
|
||||
/// <c>TransportOptions.MaxConcurrentImportSessions</c> slots for the full
|
||||
/// <c>BundleSessionTtlMinutes</c>. Only <see cref="IBundleImporter.ApplyAsync"/>
|
||||
/// used to release it, so eight abandoned wizards locked out every import on the
|
||||
/// active node. Every path that drops <c>_session</c> — new upload, Back-to-Upload,
|
||||
/// <see cref="ResetSessionState"/>, lockout reset, circuit teardown via
|
||||
/// <see cref="Dispose"/> — now goes through <see cref="ReleaseSession"/>, which
|
||||
/// calls <see cref="IBundleImporter.Abandon"/> (zero + remove, idempotent and
|
||||
/// safe after a successful Apply has already released the same id).
|
||||
/// </para>
|
||||
/// </summary>
|
||||
public partial class TransportImport : ComponentBase, IDisposable
|
||||
{
|
||||
@@ -71,6 +86,11 @@ public partial class TransportImport : ComponentBase, IDisposable
|
||||
[Inject] private IAuditService AuditService { get; set; } = default!;
|
||||
[Inject] private ScadaBridgeDbContext DbContext { get; set; } = default!;
|
||||
|
||||
// Register row 44: the per-IP-per-hour unlock throttle needs the caller's
|
||||
// address, and HttpContext is long gone by the time a circuit submits a
|
||||
// passphrase — this provider snapshots it when the circuit was established.
|
||||
[Inject] private CircuitClientAddressProvider ClientAddress { get; set; } = default!;
|
||||
|
||||
// The Map step needs the destination environment's sites + each
|
||||
// site's connections to populate the "map to existing target" dropdowns.
|
||||
[Inject] private ISiteRepository SiteRepo { get; set; } = default!;
|
||||
@@ -149,7 +169,9 @@ public partial class TransportImport : ComponentBase, IDisposable
|
||||
{
|
||||
_errorMessage = null;
|
||||
_uploadInProgress = true;
|
||||
_session = null;
|
||||
// Release the previous upload's server-side session before we lose the
|
||||
// only reference to its id — otherwise re-selecting a file leaks a slot.
|
||||
ReleaseSession();
|
||||
DeleteBundleTempFile();
|
||||
try
|
||||
{
|
||||
@@ -207,6 +229,10 @@ public partial class TransportImport : ComponentBase, IDisposable
|
||||
_errorMessage = "No bundle staged — please re-select the file.";
|
||||
return;
|
||||
}
|
||||
// A successful LoadAsync overwrites _session; release whatever it is
|
||||
// replacing first so the id of the outgoing session is never dropped on
|
||||
// the floor (that would strand its slot for the full TTL).
|
||||
ReleaseSession();
|
||||
try
|
||||
{
|
||||
// Read the staged bundle straight off disk; the
|
||||
@@ -215,7 +241,8 @@ public partial class TransportImport : ComponentBase, IDisposable
|
||||
using var stream = new FileStream(
|
||||
_bundleTempPath, FileMode.Open, FileAccess.Read, FileShare.Read,
|
||||
bufferSize: 81920, useAsync: true);
|
||||
_session = await BundleImporter.LoadAsync(stream, passphrase, CancellationToken.None);
|
||||
_session = await BundleImporter.LoadAsync(
|
||||
stream, passphrase, CancellationToken.None, ClientAddress.ClientAddress);
|
||||
_errorMessage = null;
|
||||
}
|
||||
catch (ArgumentException)
|
||||
@@ -232,6 +259,14 @@ public partial class TransportImport : ComponentBase, IDisposable
|
||||
_session = null;
|
||||
throw;
|
||||
}
|
||||
catch (BundleUnlockThrottledException)
|
||||
{
|
||||
// Register row 44: per-IP-per-hour budget spent. Bubble so the caller
|
||||
// can surface it WITHOUT counting a passphrase failure — the attempt
|
||||
// never reached the decryptor.
|
||||
_session = null;
|
||||
throw;
|
||||
}
|
||||
catch (InvalidDataException ex)
|
||||
{
|
||||
_session = null;
|
||||
@@ -309,6 +344,11 @@ public partial class TransportImport : ComponentBase, IDisposable
|
||||
/// shared across tabs / CLI / circuits). The Razor counter is kept ONLY for
|
||||
/// display ("3 of N attempts used") — it is no longer the source of truth.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// Register row 44: a <see cref="BundleUnlockThrottledException"/> is a third,
|
||||
/// distinct outcome — this CLIENT has spent its per-IP-per-hour unlock budget.
|
||||
/// It burns no per-bundle strike and leaves the wizard on Step 2.
|
||||
/// </para>
|
||||
/// </summary>
|
||||
private async Task SubmitPassphraseAsync()
|
||||
{
|
||||
@@ -339,6 +379,19 @@ public partial class TransportImport : ComponentBase, IDisposable
|
||||
ResetSessionState();
|
||||
_step = ImportWizardStep.Upload;
|
||||
}
|
||||
catch (BundleUnlockThrottledException ex)
|
||||
{
|
||||
// Register row 44: fail closed on the per-IP-per-hour cap. Deliberately
|
||||
// does NOT touch _failedUnlockAttempts — a throttled attempt never
|
||||
// reached the decryptor, so it must not consume one of the bundle's
|
||||
// three strikes (otherwise the throttle could be used to lock a bundle
|
||||
// an operator is legitimately importing). The wizard stays on Step 2 so
|
||||
// the operator can retry once the hour window rolls forward.
|
||||
_passphrase = string.Empty;
|
||||
var throttledEntity = _session?.Manifest.ContentHash ?? "<no-session>";
|
||||
await EmitUnlockFailedAuditRowAsync(throttledEntity, _failedUnlockAttempts, ex.Message);
|
||||
_errorMessage = ex.Message;
|
||||
}
|
||||
catch (CryptographicException ex)
|
||||
{
|
||||
_failedUnlockAttempts++;
|
||||
@@ -808,7 +861,7 @@ public partial class TransportImport : ComponentBase, IDisposable
|
||||
|
||||
private void ResetSessionState()
|
||||
{
|
||||
_session = null;
|
||||
ReleaseSession();
|
||||
DeleteBundleTempFile();
|
||||
_preview = null;
|
||||
_resolutions = null;
|
||||
@@ -854,12 +907,41 @@ public partial class TransportImport : ComponentBase, IDisposable
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Ensures the staged temp file does not survive circuit
|
||||
/// teardown. Blazor invokes Dispose when the user navigates away or the
|
||||
/// circuit ends, so an abandoned wizard cleans up automatically.
|
||||
/// Releases the server-side <see cref="BundleSession"/> this wizard opened
|
||||
/// (zeroing its decrypted content) and clears the local reference. Idempotent:
|
||||
/// a null <c>_session</c> is a no-op, and <see cref="IBundleImporter.Abandon"/>
|
||||
/// tolerates an id that <see cref="IBundleImporter.ApplyAsync"/> already
|
||||
/// released. Only ever passes the id THIS circuit opened, so a concurrent
|
||||
/// operator's session is never touched.
|
||||
/// <para>
|
||||
/// Abandon failure must not block the wizard (or, from <see cref="Dispose"/>,
|
||||
/// tear down the circuit), so it is swallowed on the same best-effort footing
|
||||
/// as the staged-file delete below.
|
||||
/// </para>
|
||||
/// </summary>
|
||||
private void ReleaseSession()
|
||||
{
|
||||
var session = _session;
|
||||
_session = null;
|
||||
if (session is null) return;
|
||||
try
|
||||
{
|
||||
BundleImporter.Abandon(session.SessionId);
|
||||
}
|
||||
catch
|
||||
{
|
||||
// Best-effort: the session's own TTL is the backstop.
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Ensures neither the staged temp file nor the server-side bundle session
|
||||
/// survives circuit teardown. Blazor invokes Dispose when the user navigates
|
||||
/// away or the circuit ends, so an abandoned wizard cleans up automatically.
|
||||
/// </summary>
|
||||
public void Dispose()
|
||||
{
|
||||
ReleaseSession();
|
||||
DeleteBundleTempFile();
|
||||
}
|
||||
}
|
||||
|
||||
@@ -27,6 +27,13 @@ public static class ServiceCollectionExtensions
|
||||
services.AddScoped<AuthenticationStateProvider, CookieAuthenticationStateProvider>();
|
||||
services.AddCascadingAuthenticationState();
|
||||
|
||||
// Snapshots the circuit-establishing request's remote IP (HttpContext is only
|
||||
// valid then), so circuit-lifetime code can still attribute a client address —
|
||||
// today the Transport import wizard's per-IP unlock throttle. Explicit factory
|
||||
// because the type carries a second (test-seam) string ctor.
|
||||
services.AddScoped(sp => new CircuitClientAddressProvider(
|
||||
sp.GetRequiredService<Microsoft.AspNetCore.Http.IHttpContextAccessor>()));
|
||||
|
||||
// Resolves the current user's permitted site set from their SiteId claims
|
||||
// so Deployment/Monitoring pages can enforce site scoping.
|
||||
services.AddScoped<SiteScopeService>();
|
||||
|
||||
@@ -10,8 +10,20 @@ public interface IBundleImporter
|
||||
/// <param name="bundleStream">Stream containing the bundle zip archive.</param>
|
||||
/// <param name="passphrase">Optional passphrase for decrypting an encrypted bundle.</param>
|
||||
/// <param name="ct">Cancellation token.</param>
|
||||
/// <param name="clientKey">
|
||||
/// Identity of the calling client — the remote IP — used to enforce the
|
||||
/// per-IP-per-hour unlock throttle (<c>TransportOptions.MaxUnlockAttemptsPerIpPerHour</c>)
|
||||
/// on encrypted bundles. Trailing optional so unencrypted / in-process callers
|
||||
/// need not supply one; null or blank counts the attempt against a single shared
|
||||
/// "unattributed caller" bucket rather than skipping the throttle, so a caller
|
||||
/// that cannot be attributed can never spend an unbounded budget.
|
||||
/// </param>
|
||||
/// <returns>A task that resolves to session metadata for the loaded bundle.</returns>
|
||||
Task<BundleSession> LoadAsync(Stream bundleStream, string? passphrase, CancellationToken ct = default);
|
||||
Task<BundleSession> LoadAsync(
|
||||
Stream bundleStream,
|
||||
string? passphrase,
|
||||
CancellationToken ct = default,
|
||||
string? clientKey = null);
|
||||
|
||||
/// <summary>
|
||||
/// Diffs the loaded bundle against the target database and returns a per-artifact preview.
|
||||
@@ -46,4 +58,25 @@ public interface IBundleImporter
|
||||
string user,
|
||||
CancellationToken ct = default,
|
||||
BundleNameMap? nameMap = null);
|
||||
|
||||
/// <summary>
|
||||
/// Releases a session opened by <see cref="LoadAsync"/> that will never be
|
||||
/// applied — the operator navigated away, restarted the wizard, or a CLI
|
||||
/// preview/import gave up before <see cref="ApplyAsync"/> ran. Zeroes the
|
||||
/// session's decrypted plaintext and drops it from the session store, which
|
||||
/// frees one of the <c>MaxConcurrentImportSessions</c> slots immediately
|
||||
/// instead of at the end of <c>BundleSessionTtlMinutes</c>.
|
||||
/// <para>
|
||||
/// Idempotent and safe to call after <see cref="ApplyAsync"/> (which performs
|
||||
/// the same release on both its success and failure paths) or with an unknown
|
||||
/// / already-expired id — the call is then a no-op and never throws.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// Ownership: session ids are random GUIDs handed only to the caller that
|
||||
/// opened them, so abandoning the id you hold can never release a session
|
||||
/// another circuit / CLI caller owns.
|
||||
/// </para>
|
||||
/// </summary>
|
||||
/// <param name="sessionId">Session id returned by <see cref="LoadAsync"/>.</param>
|
||||
void Abandon(Guid sessionId);
|
||||
}
|
||||
|
||||
@@ -4,7 +4,22 @@ public record AuthenticatedUser(
|
||||
string Username, string DisplayName,
|
||||
string[] Roles, string[] PermittedSiteIds);
|
||||
|
||||
public record ManagementEnvelope(AuthenticatedUser User, object Command, string CorrelationId);
|
||||
/// <param name="User">The authenticated caller.</param>
|
||||
/// <param name="Command">The management command to run.</param>
|
||||
/// <param name="CorrelationId">Per-request correlation id echoed on the reply.</param>
|
||||
/// <param name="ClientIp">
|
||||
/// Remote IP of the HTTP caller, when the envelope came in over the management
|
||||
/// endpoint. Additive trailing-optional (message contracts evolve additively), so
|
||||
/// in-cluster senders that construct an envelope directly stay source-compatible and
|
||||
/// simply carry null. Consumed by the Transport bundle handlers to enforce the
|
||||
/// per-IP-per-hour unlock throttle; null degrades to a shared throttle bucket rather
|
||||
/// than to no throttling (register row 44).
|
||||
/// </param>
|
||||
public record ManagementEnvelope(
|
||||
AuthenticatedUser User,
|
||||
object Command,
|
||||
string CorrelationId,
|
||||
string? ClientIp = null);
|
||||
|
||||
public record ManagementSuccess(string CorrelationId, string JsonData);
|
||||
public record ManagementError(string CorrelationId, string Error, string ErrorCode);
|
||||
|
||||
@@ -0,0 +1,43 @@
|
||||
namespace ZB.MOM.WW.ScadaBridge.Commons.Types.Transport;
|
||||
|
||||
/// <summary>
|
||||
/// T-004 / register row 44: thrown by <c>IBundleImporter.LoadAsync</c> when a client has
|
||||
/// spent its trailing-hour bundle-unlock budget (<c>TransportOptions.MaxUnlockAttemptsPerIpPerHour</c>,
|
||||
/// default 10) as measured by <c>BundleUnlockRateLimiter</c>.
|
||||
/// <para>
|
||||
/// Distinct from <c>BundleLockedException</c> on purpose: that one means "this BUNDLE has
|
||||
/// burned its 3 strikes" and is keyed by the bundle's content hash; this one means "this
|
||||
/// CLIENT has attempted too many unlocks, across any bundles" and is keyed by the caller's
|
||||
/// IP. A throttled attempt never reaches the decryptor, so it deliberately burns no
|
||||
/// per-bundle strike — otherwise the throttle could be used to lock out a bundle an
|
||||
/// operator is legitimately importing.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// Lives in Commons rather than beside the importer because it is part of the
|
||||
/// <c>IBundleImporter.LoadAsync</c> contract, which is declared here: the management
|
||||
/// surface (which references Commons but not Transport) must be able to catch it to
|
||||
/// return a clear, curated error instead of a generic internal-failure message.
|
||||
/// </para>
|
||||
/// </summary>
|
||||
public sealed class BundleUnlockThrottledException : Exception
|
||||
{
|
||||
/// <summary>The rate-limiter key the attempt was counted against — the client IP, or the shared unattributed-caller bucket.</summary>
|
||||
public string ClientKey { get; }
|
||||
|
||||
/// <summary>The configured trailing-window cap that was reached.</summary>
|
||||
public int MaxAttemptsPerWindow { get; }
|
||||
|
||||
/// <summary>
|
||||
/// Initializes a new <see cref="BundleUnlockThrottledException"/>.
|
||||
/// </summary>
|
||||
/// <param name="clientKey">The rate-limiter key (client IP, or the shared unattributed bucket).</param>
|
||||
/// <param name="maxAttemptsPerWindow">The configured per-window cap that was reached.</param>
|
||||
public BundleUnlockThrottledException(string clientKey, int maxAttemptsPerWindow)
|
||||
: base(
|
||||
$"Too many bundle unlock attempts ({maxAttemptsPerWindow} per hour) from this client. "
|
||||
+ "Wait for the hour window to roll forward before trying again.")
|
||||
{
|
||||
ClientKey = clientKey ?? throw new ArgumentNullException(nameof(clientKey));
|
||||
MaxAttemptsPerWindow = maxAttemptsPerWindow;
|
||||
}
|
||||
}
|
||||
@@ -130,7 +130,7 @@ public class ManagementActor : ReceiveActor
|
||||
private async Task<object> ProcessCommand(ManagementEnvelope envelope, AuthenticatedUser user)
|
||||
{
|
||||
using var scope = _serviceProvider.CreateScope();
|
||||
var result = await DispatchCommand(scope.ServiceProvider, envelope.Command, user);
|
||||
var result = await DispatchCommand(scope.ServiceProvider, envelope.Command, user, envelope.ClientIp);
|
||||
return new ManagementSuccess(envelope.CorrelationId, SerializeResult(result));
|
||||
}
|
||||
|
||||
@@ -294,7 +294,8 @@ public class ManagementActor : ReceiveActor
|
||||
_ => null
|
||||
};
|
||||
|
||||
private async Task<object?> DispatchCommand(IServiceProvider sp, object command, AuthenticatedUser user)
|
||||
private async Task<object?> DispatchCommand(
|
||||
IServiceProvider sp, object command, AuthenticatedUser user, string? clientIp = null)
|
||||
{
|
||||
return command switch
|
||||
{
|
||||
@@ -474,8 +475,8 @@ public class ManagementActor : ReceiveActor
|
||||
|
||||
// Transport bundle operations
|
||||
ExportBundleCommand cmd => await HandleExportBundle(sp, cmd, user.Username),
|
||||
PreviewBundleCommand cmd => await HandlePreviewBundle(sp, cmd),
|
||||
ImportBundleCommand cmd => await HandleImportBundle(sp, cmd, user.Username),
|
||||
PreviewBundleCommand cmd => await HandlePreviewBundle(sp, cmd, clientIp),
|
||||
ImportBundleCommand cmd => await HandleImportBundle(sp, cmd, user.Username, clientIp),
|
||||
|
||||
// NOTE: ResolveRolesCommand is intentionally NOT dispatched. The two-step
|
||||
// "ResolveRoles + command" flow is retired — the HTTP endpoint performs LDAP
|
||||
@@ -3549,7 +3550,7 @@ public class ManagementActor : ReceiveActor
|
||||
/// not apply anything; callers can use this to gate Apply on the diff shape.
|
||||
/// </summary>
|
||||
private static async Task<object?> HandlePreviewBundle(
|
||||
IServiceProvider sp, PreviewBundleCommand cmd)
|
||||
IServiceProvider sp, PreviewBundleCommand cmd, string? clientIp)
|
||||
{
|
||||
await TransportGate.WaitAsync();
|
||||
try
|
||||
@@ -3560,7 +3561,7 @@ public class ManagementActor : ReceiveActor
|
||||
try
|
||||
{
|
||||
await using var stream = new MemoryStream(bytes);
|
||||
session = await importer.LoadAsync(stream, cmd.Passphrase);
|
||||
session = await importer.LoadAsync(stream, cmd.Passphrase, clientKey: clientIp);
|
||||
}
|
||||
catch (ArgumentException ex)
|
||||
{
|
||||
@@ -3570,17 +3571,35 @@ public class ManagementActor : ReceiveActor
|
||||
{
|
||||
throw new ManagementCommandException("Wrong passphrase or bundle was tampered.");
|
||||
}
|
||||
catch (BundleUnlockThrottledException ex)
|
||||
{
|
||||
// Register row 44: per-IP-per-hour unlock cap. Curated so the CLI
|
||||
// prints the reason instead of MapFault's generic internal-error
|
||||
// message; no per-bundle strike was burned.
|
||||
throw new ManagementCommandException(ex.Message);
|
||||
}
|
||||
|
||||
var preview = await importer.PreviewAsync(session.SessionId);
|
||||
var adds = preview.Items.Count(i => i.Kind == ConflictKind.New);
|
||||
var mods = preview.Items.Count(i => i.Kind == ConflictKind.Modified);
|
||||
var ids = preview.Items.Count(i => i.Kind == ConflictKind.Identical);
|
||||
var blocks = preview.Items.Count(i => i.Kind == ConflictKind.Blocker);
|
||||
// Surface the required site/connection mappings so an operator
|
||||
// (CLI or UI) sees which references need resolving before applying.
|
||||
return new PreviewBundleResult(
|
||||
preview.Items, adds, mods, ids, blocks,
|
||||
preview.RequiredSiteMappings, preview.RequiredConnectionMappings);
|
||||
// Register row 43: a preview NEVER applies, so nothing else would ever
|
||||
// release this session — every `transport preview` used to burn one of
|
||||
// MaxConcurrentImportSessions (and hold the decrypted bundle) for the
|
||||
// full BundleSessionTtlMinutes. Release it on the way out, always.
|
||||
try
|
||||
{
|
||||
var preview = await importer.PreviewAsync(session.SessionId);
|
||||
var adds = preview.Items.Count(i => i.Kind == ConflictKind.New);
|
||||
var mods = preview.Items.Count(i => i.Kind == ConflictKind.Modified);
|
||||
var ids = preview.Items.Count(i => i.Kind == ConflictKind.Identical);
|
||||
var blocks = preview.Items.Count(i => i.Kind == ConflictKind.Blocker);
|
||||
// Surface the required site/connection mappings so an operator
|
||||
// (CLI or UI) sees which references need resolving before applying.
|
||||
return new PreviewBundleResult(
|
||||
preview.Items, adds, mods, ids, blocks,
|
||||
preview.RequiredSiteMappings, preview.RequiredConnectionMappings);
|
||||
}
|
||||
finally
|
||||
{
|
||||
importer.Abandon(session.SessionId);
|
||||
}
|
||||
}
|
||||
finally
|
||||
{
|
||||
@@ -3594,7 +3613,7 @@ public class ManagementActor : ReceiveActor
|
||||
/// <see cref="ConflictKind.Blocker"/> rows fail the call before Apply.
|
||||
/// </summary>
|
||||
private static async Task<object?> HandleImportBundle(
|
||||
IServiceProvider sp, ImportBundleCommand cmd, string username)
|
||||
IServiceProvider sp, ImportBundleCommand cmd, string username, string? clientIp)
|
||||
{
|
||||
await TransportGate.WaitAsync();
|
||||
try
|
||||
@@ -3607,7 +3626,7 @@ public class ManagementActor : ReceiveActor
|
||||
try
|
||||
{
|
||||
await using var stream = new MemoryStream(bytes);
|
||||
session = await importer.LoadAsync(stream, cmd.Passphrase);
|
||||
session = await importer.LoadAsync(stream, cmd.Passphrase, clientKey: clientIp);
|
||||
}
|
||||
catch (ArgumentException ex)
|
||||
{
|
||||
@@ -3617,51 +3636,70 @@ public class ManagementActor : ReceiveActor
|
||||
{
|
||||
throw new ManagementCommandException("Wrong passphrase or bundle was tampered.");
|
||||
}
|
||||
|
||||
var preview = await importer.PreviewAsync(session.SessionId);
|
||||
|
||||
var blockers = preview.Items.Where(i => i.Kind == ConflictKind.Blocker).ToList();
|
||||
if (blockers.Count > 0)
|
||||
catch (BundleUnlockThrottledException ex)
|
||||
{
|
||||
var details = string.Join("; ",
|
||||
blockers.Select(b => $"{b.Name}: {b.BlockerReason}"));
|
||||
throw new ManagementCommandException(
|
||||
$"Bundle has {blockers.Count} blocker(s); import aborted. {details}");
|
||||
// Register row 44: per-IP-per-hour unlock cap. Curated so the CLI
|
||||
// prints the reason instead of MapFault's generic internal-error
|
||||
// message; no per-bundle strike was burned.
|
||||
throw new ManagementCommandException(ex.Message);
|
||||
}
|
||||
|
||||
// Resolve every required site/connection reference into a
|
||||
// BundleNameMap. Precedence: an explicit operator spec wins; otherwise
|
||||
// fall back to the preview's auto-match; otherwise (no match) create-new
|
||||
// ONLY if the create-missing flag is set, else fail with a clear message.
|
||||
var nameMap = BuildNameMap(cmd, preview);
|
||||
|
||||
// Dedupe by (EntityType, Name) -- the preview can emit multiple rows per
|
||||
// entity (e.g. one row per modified member of a template), but ApplyAsync
|
||||
// requires a unique resolution per key. Last-write-wins matches the
|
||||
// Central UI's TransportImport.BuildDefaultResolutions behavior. For
|
||||
// DataConnection rows the preview item Name is already site-qualified
|
||||
// ({site}/{name}), so keying generically by (EntityType, Name) is
|
||||
// automatically correct — no bare-connection-name special case needed.
|
||||
var renameStamp = DateTime.UtcNow.ToString("yyyyMMdd-HHmmss");
|
||||
var resolutionsMap = new Dictionary<(string, string), ImportResolution>();
|
||||
foreach (var item in preview.Items)
|
||||
// Register row 43: ApplyAsync releases the session on both its success
|
||||
// and failure paths, but every abort BEFORE it (blockers, unresolved
|
||||
// mappings) used to strand the session — and its decrypted content —
|
||||
// for the full TTL. Abandon is idempotent, so covering the whole block
|
||||
// is safe even when ApplyAsync already released the same id.
|
||||
try
|
||||
{
|
||||
var action = item.Kind switch
|
||||
var preview = await importer.PreviewAsync(session.SessionId);
|
||||
|
||||
var blockers = preview.Items.Where(i => i.Kind == ConflictKind.Blocker).ToList();
|
||||
if (blockers.Count > 0)
|
||||
{
|
||||
ConflictKind.New => ResolutionAction.Add,
|
||||
ConflictKind.Identical => ResolutionAction.Skip,
|
||||
ConflictKind.Modified => policy,
|
||||
_ => ResolutionAction.Skip,
|
||||
};
|
||||
var renameTo = (item.Kind == ConflictKind.Modified && policy == ResolutionAction.Rename)
|
||||
? $"{item.Name}-imported-{renameStamp}"
|
||||
: null;
|
||||
resolutionsMap[(item.EntityType, item.Name)] = new ImportResolution(
|
||||
item.EntityType, item.Name, action, renameTo);
|
||||
}
|
||||
var details = string.Join("; ",
|
||||
blockers.Select(b => $"{b.Name}: {b.BlockerReason}"));
|
||||
throw new ManagementCommandException(
|
||||
$"Bundle has {blockers.Count} blocker(s); import aborted. {details}");
|
||||
}
|
||||
|
||||
return await importer.ApplyAsync(
|
||||
session.SessionId, resolutionsMap.Values.ToList(), username, nameMap: nameMap);
|
||||
// Resolve every required site/connection reference into a
|
||||
// BundleNameMap. Precedence: an explicit operator spec wins; otherwise
|
||||
// fall back to the preview's auto-match; otherwise (no match) create-new
|
||||
// ONLY if the create-missing flag is set, else fail with a clear message.
|
||||
var nameMap = BuildNameMap(cmd, preview);
|
||||
|
||||
// Dedupe by (EntityType, Name) -- the preview can emit multiple rows per
|
||||
// entity (e.g. one row per modified member of a template), but ApplyAsync
|
||||
// requires a unique resolution per key. Last-write-wins matches the
|
||||
// Central UI's TransportImport.BuildDefaultResolutions behavior. For
|
||||
// DataConnection rows the preview item Name is already site-qualified
|
||||
// ({site}/{name}), so keying generically by (EntityType, Name) is
|
||||
// automatically correct — no bare-connection-name special case needed.
|
||||
var renameStamp = DateTime.UtcNow.ToString("yyyyMMdd-HHmmss");
|
||||
var resolutionsMap = new Dictionary<(string, string), ImportResolution>();
|
||||
foreach (var item in preview.Items)
|
||||
{
|
||||
var action = item.Kind switch
|
||||
{
|
||||
ConflictKind.New => ResolutionAction.Add,
|
||||
ConflictKind.Identical => ResolutionAction.Skip,
|
||||
ConflictKind.Modified => policy,
|
||||
_ => ResolutionAction.Skip,
|
||||
};
|
||||
var renameTo = (item.Kind == ConflictKind.Modified && policy == ResolutionAction.Rename)
|
||||
? $"{item.Name}-imported-{renameStamp}"
|
||||
: null;
|
||||
resolutionsMap[(item.EntityType, item.Name)] = new ImportResolution(
|
||||
item.EntityType, item.Name, action, renameTo);
|
||||
}
|
||||
|
||||
return await importer.ApplyAsync(
|
||||
session.SessionId, resolutionsMap.Values.ToList(), username, nameMap: nameMap);
|
||||
}
|
||||
finally
|
||||
{
|
||||
importer.Abandon(session.SessionId);
|
||||
}
|
||||
}
|
||||
finally
|
||||
{
|
||||
|
||||
@@ -134,7 +134,14 @@ public static class ManagementEndpoints
|
||||
}
|
||||
|
||||
var correlationId = Guid.NewGuid().ToString("N");
|
||||
var envelope = new ManagementEnvelope(authenticatedUser, command, correlationId);
|
||||
// Carry the caller's remote IP so the Transport bundle handlers can enforce
|
||||
// the per-IP-per-hour unlock throttle (register row 44). ForwardedHeaders has
|
||||
// already rewritten this from X-Forwarded-For for CONFIGURED proxies only
|
||||
// (Program.cs), so it is a real client address in the shipped topologies and
|
||||
// the proxy's address behind an untrusted one.
|
||||
var envelope = new ManagementEnvelope(
|
||||
authenticatedUser, command, correlationId,
|
||||
context.Connection.RemoteIpAddress?.ToString());
|
||||
|
||||
var askTimeout = ResolveAskTimeout(
|
||||
context.RequestServices.GetService<IOptions<ManagementServiceOptions>>()?.Value,
|
||||
|
||||
@@ -50,34 +50,6 @@ public class NotificationOutboxActor : ReceiveActor, IWithTimers
|
||||
/// </summary>
|
||||
private bool _dispatching;
|
||||
|
||||
/// <summary>
|
||||
/// Cached <see cref="NotificationType"/> → adapter lookup, built
|
||||
/// lazily on the first dispatch sweep and reused for the lifetime of the actor. The
|
||||
/// adapter registration is decided at startup by <c>AddNotificationOutbox</c> (the set is
|
||||
/// keyed by <see cref="NotificationType"/> and is static per process lifetime), so
|
||||
/// rebuilding this dictionary on every sweep was pure allocation waste.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// The cache is paired with <see cref="_adaptersScope"/>, an actor-lifetime
|
||||
/// <see cref="IServiceScope"/> created on first use so the cached scoped adapter
|
||||
/// instances and their dependencies live as long as the cache itself. The scope is
|
||||
/// disposed in <see cref="PostStop"/>. The adapters are stateless wrappers that
|
||||
/// resolve their per-call collaborators (e.g. <see cref="INotificationRepository"/>'s
|
||||
/// underlying DbContext) through their own injected dependencies; holding them for
|
||||
/// the actor's lifetime is consistent with the actor's own singleton lifetime on the
|
||||
/// active central node.
|
||||
/// </remarks>
|
||||
private IReadOnlyDictionary<NotificationType, INotificationDeliveryAdapter>? _adaptersCache;
|
||||
|
||||
/// <summary>
|
||||
/// Actor-lifetime DI scope that owns the cached
|
||||
/// <see cref="_adaptersCache"/> adapter instances. Created lazily on the first
|
||||
/// dispatch sweep that needs adapters; disposed in <see cref="PostStop"/> so the
|
||||
/// scoped adapter graph (and any disposable dependencies it transitively holds) is
|
||||
/// torn down with the actor.
|
||||
/// </summary>
|
||||
private IServiceScope? _adaptersScope;
|
||||
|
||||
/// <summary>
|
||||
/// Lifecycle-scoped cancellation source, cancelled in <see cref="PostStop"/> so
|
||||
/// any in-flight dispatch sweep — including a long-running SMTP send via the channel
|
||||
@@ -156,13 +128,9 @@ public class NotificationOutboxActor : ReceiveActor, IWithTimers
|
||||
_shutdownCts?.Dispose();
|
||||
_shutdownCts = null;
|
||||
|
||||
// Dispose the actor-lifetime adapter scope so the cached
|
||||
// scoped adapter instances and their disposable dependencies are torn down with
|
||||
// the actor (e.g. on a CoordinatedShutdown / failover that stops the singleton).
|
||||
_adaptersScope?.Dispose();
|
||||
_adaptersScope = null;
|
||||
_adaptersCache = null;
|
||||
|
||||
// No adapter state to tear down: delivery adapters are resolved from — and
|
||||
// disposed with — the per-delivery DI scope opened in DeliverGatedAsync, so the
|
||||
// actor holds no scoped service beyond the lifetime of a single delivery.
|
||||
base.PostStop();
|
||||
}
|
||||
|
||||
@@ -278,12 +246,13 @@ public class NotificationOutboxActor : ReceiveActor, IWithTimers
|
||||
/// and retry-policy resolution can all throw, and a faulted task would otherwise leave
|
||||
/// the dispatcher's in-flight guard stuck and wedge the loop permanently.
|
||||
///
|
||||
/// The per-sweep DI scope still owns the repository graph
|
||||
/// (<see cref="INotificationOutboxRepository"/> + <see cref="INotificationRepository"/>),
|
||||
/// which is correct because those services back a fresh DbContext per sweep. The
|
||||
/// channel delivery adapters, however, are cached for the actor's lifetime via
|
||||
/// <see cref="ResolveAdapters"/> — see <see cref="_adaptersCache"/> for the
|
||||
/// rationale.
|
||||
/// The per-sweep DI scope owns only the sweep-level repository graph
|
||||
/// (<see cref="INotificationOutboxRepository"/> + <see cref="INotificationRepository"/>)
|
||||
/// used to claim the due batch and resolve the retry policies — both of which run
|
||||
/// before any delivery starts, on the sweep's own DbContext. The channel delivery
|
||||
/// adapters are NOT resolved here: each is resolved inside its own per-delivery scope
|
||||
/// by <see cref="DeliverGatedAsync"/>, so parallel deliveries never share an adapter
|
||||
/// or the DbContext behind it.
|
||||
/// </summary>
|
||||
private async Task RunDispatchPass(DateTimeOffset now, CancellationToken cancellationToken)
|
||||
{
|
||||
@@ -292,7 +261,6 @@ public class NotificationOutboxActor : ReceiveActor, IWithTimers
|
||||
using var scope = _serviceProvider.CreateScope();
|
||||
var outboxRepository = scope.ServiceProvider.GetRequiredService<INotificationOutboxRepository>();
|
||||
var notificationRepository = scope.ServiceProvider.GetRequiredService<INotificationRepository>();
|
||||
var adapters = ResolveAdapters();
|
||||
|
||||
IReadOnlyList<Notification> due;
|
||||
try
|
||||
@@ -322,18 +290,18 @@ public class NotificationOutboxActor : ReceiveActor, IWithTimers
|
||||
// (multi-second SMTP/Twilio round trips), so a strictly sequential sweep caps
|
||||
// throughput at ~1/attempt-latency — an alarm-storm backlog behind one slow
|
||||
// host would drain far too slowly (arch-review 04). A SemaphoreSlim caps the
|
||||
// number of in-flight deliveries; each delivery runs on its OWN DI scope/
|
||||
// repository (the EF DbContext is not thread-safe and rows are distinct per
|
||||
// notification, so concurrent deliveries never contend). The cached adapters
|
||||
// and the lifetime-scoped audit writer are already safe for concurrent use.
|
||||
// MaxParallelDeliveries = 1 preserves the strictly sequential behaviour.
|
||||
// number of in-flight deliveries; each delivery runs on its OWN DI scope —
|
||||
// repository AND channel adapter alike (the EF DbContext is not thread-safe
|
||||
// and rows are distinct per notification, so concurrent deliveries never
|
||||
// contend). The lifetime-scoped audit writer is already safe for concurrent
|
||||
// use. MaxParallelDeliveries = 1 preserves the strictly sequential behaviour.
|
||||
using var deliveryGate = new SemaphoreSlim(
|
||||
_options.ResolvedMaxParallelDeliveries, _options.ResolvedMaxParallelDeliveries);
|
||||
var deliveries = new List<Task>(due.Count);
|
||||
foreach (var notification in due)
|
||||
{
|
||||
deliveries.Add(DeliverGatedAsync(
|
||||
notification, now, policies, adapters, deliveryGate, cancellationToken));
|
||||
notification, now, policies, deliveryGate, cancellationToken));
|
||||
}
|
||||
|
||||
// Each DeliverGatedAsync isolates its own faults, so WhenAll never throws.
|
||||
@@ -355,11 +323,24 @@ public class NotificationOutboxActor : ReceiveActor, IWithTimers
|
||||
/// pre-attempt state for the next active sweep) and any other exception is logged — so
|
||||
/// one bad delivery never aborts the rest of the batch and <c>Task.WhenAll</c> never faults.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// The per-delivery scope covers the channel delivery adapter as well as the outbox
|
||||
/// repository. Both <see cref="EmailNotificationDeliveryAdapter"/> and
|
||||
/// <see cref="SmsNotificationDeliveryAdapter"/> are registered <em>scoped</em> and hold
|
||||
/// a scoped <see cref="INotificationRepository"/> backed by the EF DbContext, so an
|
||||
/// adapter resolved outside this scope and reused across concurrent deliveries put
|
||||
/// several deliveries on ONE DbContext — which fails with
|
||||
/// <c>InvalidOperationException: A second operation was started on this context
|
||||
/// instance…</c> and burns the row's retries. Resolving the adapter here binds its
|
||||
/// lifetime to exactly one delivery. Neither adapter carries state worth sharing: the
|
||||
/// Email adapter builds a fresh SMTP client per attempt from an injected factory and
|
||||
/// the SMS adapter takes its pooled connections from <c>IHttpClientFactory</c>, which
|
||||
/// is a singleton and shares the handler pool across scopes regardless.
|
||||
/// </remarks>
|
||||
private async Task DeliverGatedAsync(
|
||||
Notification notification,
|
||||
DateTimeOffset now,
|
||||
RetryPolicies policies,
|
||||
IReadOnlyDictionary<NotificationType, INotificationDeliveryAdapter> adapters,
|
||||
SemaphoreSlim gate,
|
||||
CancellationToken cancellationToken)
|
||||
{
|
||||
@@ -381,13 +362,14 @@ public class NotificationOutboxActor : ReceiveActor, IWithTimers
|
||||
return;
|
||||
}
|
||||
|
||||
// Each delivery owns its DI scope + repository so parallel UpdateAsync calls
|
||||
// never share a DbContext. The adapters map + audit writer are already safe
|
||||
// for concurrent use.
|
||||
// Each delivery owns its DI scope, repository AND channel adapter so parallel
|
||||
// deliveries never share a DbContext. The audit writer is already safe for
|
||||
// concurrent use.
|
||||
using var scope = _serviceProvider.CreateScope();
|
||||
var outboxRepository = scope.ServiceProvider.GetRequiredService<INotificationOutboxRepository>();
|
||||
var adapter = ResolveAdapter(scope.ServiceProvider, notification.Type);
|
||||
await DeliverOneAsync(
|
||||
notification, now, policies, outboxRepository, adapters, cancellationToken)
|
||||
notification, now, policies, outboxRepository, adapter, cancellationToken)
|
||||
.ConfigureAwait(false);
|
||||
}
|
||||
catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested)
|
||||
@@ -522,37 +504,36 @@ public class NotificationOutboxActor : ReceiveActor, IWithTimers
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Returns the <see cref="NotificationType"/> → adapter lookup, building it lazily on
|
||||
/// the first call and caching it on <see cref="_adaptersCache"/> for the actor's
|
||||
/// lifetime. The last adapter registered for a given type wins, mirroring DI's
|
||||
/// last-wins resolution semantics.
|
||||
/// Resolves the channel delivery adapter registered for <paramref name="type"/> from
|
||||
/// <paramref name="scopedServices"/> — the caller's per-delivery DI scope — or null
|
||||
/// when no adapter handles that type (the caller then parks the notification). The
|
||||
/// LAST adapter registered for a given type wins, mirroring DI's last-wins resolution
|
||||
/// semantics, which is why the enumeration runs to completion rather than short-
|
||||
/// circuiting on the first match.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// The lookup used to be rebuilt on every dispatch sweep
|
||||
/// from the per-sweep DI scope. Adapter registration is static per process
|
||||
/// lifetime, so the dict is now built ONCE — on the first sweep that needs it —
|
||||
/// and reused. To respect each adapter's scoped lifetime
|
||||
/// (<see cref="EmailNotificationDeliveryAdapter"/> takes a scoped
|
||||
/// <see cref="INotificationRepository"/>), the cache is paired with
|
||||
/// <see cref="_adaptersScope"/>, an actor-lifetime <see cref="IServiceScope"/> that
|
||||
/// owns the cached adapter instances and is disposed in <see cref="PostStop"/>.
|
||||
/// This deliberately resolves per delivery rather than caching an actor-lifetime
|
||||
/// lookup: the adapters are scoped services holding a scoped
|
||||
/// <see cref="INotificationRepository"/>/DbContext, so a cached instance shared by up
|
||||
/// to <c>MaxParallelDeliveries</c> concurrent deliveries drove a second-operation-on-
|
||||
/// this-context failure. See <see cref="DeliverGatedAsync"/>. Enumerating
|
||||
/// <c>GetServices</c> constructs every registered adapter (two today), which is a
|
||||
/// negligible cost next to the SMTP/HTTPS round trip that follows and keeps the
|
||||
/// last-wins contract identical to the previous lookup.
|
||||
/// </remarks>
|
||||
private IReadOnlyDictionary<NotificationType, INotificationDeliveryAdapter> ResolveAdapters()
|
||||
private static INotificationDeliveryAdapter? ResolveAdapter(
|
||||
IServiceProvider scopedServices, NotificationType type)
|
||||
{
|
||||
if (_adaptersCache is not null)
|
||||
INotificationDeliveryAdapter? match = null;
|
||||
foreach (var adapter in scopedServices.GetServices<INotificationDeliveryAdapter>())
|
||||
{
|
||||
return _adaptersCache;
|
||||
if (adapter.Type == type)
|
||||
{
|
||||
match = adapter;
|
||||
}
|
||||
}
|
||||
|
||||
_adaptersScope = _serviceProvider.CreateScope();
|
||||
var adapters = new Dictionary<NotificationType, INotificationDeliveryAdapter>();
|
||||
foreach (var adapter in _adaptersScope.ServiceProvider.GetServices<INotificationDeliveryAdapter>())
|
||||
{
|
||||
adapters[adapter.Type] = adapter;
|
||||
}
|
||||
|
||||
_adaptersCache = adapters;
|
||||
return _adaptersCache;
|
||||
return match;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
@@ -585,7 +566,7 @@ public class NotificationOutboxActor : ReceiveActor, IWithTimers
|
||||
DateTimeOffset now,
|
||||
RetryPolicies policies,
|
||||
INotificationOutboxRepository outboxRepository,
|
||||
IReadOnlyDictionary<NotificationType, INotificationDeliveryAdapter> adapters,
|
||||
INotificationDeliveryAdapter? adapter,
|
||||
CancellationToken cancellationToken)
|
||||
{
|
||||
// Select the retry policy by delivery channel: SMS honors SmsConfiguration's own
|
||||
@@ -594,7 +575,7 @@ public class NotificationOutboxActor : ReceiveActor, IWithTimers
|
||||
? policies.SmsPolicy
|
||||
: policies.SmtpPolicy;
|
||||
|
||||
if (!adapters.TryGetValue(notification.Type, out var adapter))
|
||||
if (adapter is null)
|
||||
{
|
||||
// Missing-adapter park: from the dispatcher's perspective this is an
|
||||
// attempt that resolved to a terminal park. Emit Attempted then the
|
||||
|
||||
@@ -37,9 +37,11 @@ public static class ServiceCollectionExtensions
|
||||
///
|
||||
/// <see cref="EmailNotificationDeliveryAdapter"/> is registered <em>scoped</em> because it
|
||||
/// takes a scoped <see cref="ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Repositories.INotificationRepository"/>
|
||||
/// directly. The <see cref="NotificationOutboxActor"/> resolves the adapters from a fresh
|
||||
/// scope per dispatch sweep rather than holding them, so no scoped adapter is captured by
|
||||
/// the singleton actor.
|
||||
/// directly. The <see cref="NotificationOutboxActor"/> resolves the matching adapter from
|
||||
/// the fresh DI scope it opens <em>per delivery</em> — not per sweep and never held on the
|
||||
/// actor — so no scoped adapter is captured by the singleton actor and no two of the
|
||||
/// dispatcher's concurrent deliveries (<c>MaxParallelDeliveries</c>) can land on one
|
||||
/// DbContext.
|
||||
///
|
||||
/// The <see cref="SmsNotificationDeliveryAdapter"/> (Twilio-REST) is registered the same
|
||||
/// way alongside the Email adapter, so both resolve from
|
||||
|
||||
@@ -165,8 +165,22 @@ public sealed class BundleImporter : IBundleImporter
|
||||
_logger = logger;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// T-004 / register row 44: the rate-limiter bucket used when a caller supplies no
|
||||
/// client key. Deliberately a single SHARED key rather than a per-call bypass — an
|
||||
/// unattributable caller is throttled together with every other unattributable
|
||||
/// caller, so "no IP available" degrades to a stricter budget, never to none. A
|
||||
/// production unlock always carries an IP (Central UI circuit, management endpoint),
|
||||
/// so in practice this bucket only serves in-process callers and tests.
|
||||
/// </summary>
|
||||
internal const string UnattributedClientKey = "<unattributed>";
|
||||
|
||||
/// <inheritdoc />
|
||||
public async Task<BundleSession> LoadAsync(Stream bundleStream, string? passphrase, CancellationToken ct = default)
|
||||
public async Task<BundleSession> LoadAsync(
|
||||
Stream bundleStream,
|
||||
string? passphrase,
|
||||
CancellationToken ct = default,
|
||||
string? clientKey = null)
|
||||
{
|
||||
ArgumentNullException.ThrowIfNull(bundleStream);
|
||||
|
||||
@@ -264,6 +278,33 @@ public sealed class BundleImporter : IBundleImporter
|
||||
throw new BundleLockedException(manifest.ContentHash, priorFailures);
|
||||
}
|
||||
|
||||
// T-004 / register row 44: the documented per-IP-per-hour cap. The limiter
|
||||
// was registered, injected and unit-tested but never invoked, so a
|
||||
// brute-force attacker against a stolen bundle was bounded only by the
|
||||
// per-bundle 3-strike lockout (side-steppable by re-exporting the bundle
|
||||
// for a fresh ContentHash).
|
||||
//
|
||||
// Ordering is load-bearing:
|
||||
// * AFTER the per-bundle lockout check, so hammering an already-locked
|
||||
// bundle does not spend the client's hourly budget on rejected calls.
|
||||
// * BEFORE the decrypt, so a THROTTLED attempt never reaches
|
||||
// IncrementUnlockFailureCount — a denial must not burn a per-bundle
|
||||
// strike, or the throttle would become a way to lock a bundle that an
|
||||
// operator is legitimately importing.
|
||||
// Registered on every unlock attempt (successful ones included) because the
|
||||
// documented budget counts attempts, not failures.
|
||||
var throttleKey = string.IsNullOrWhiteSpace(clientKey)
|
||||
? UnattributedClientKey
|
||||
: clientKey;
|
||||
var perIpCap = _options.Value.MaxUnlockAttemptsPerIpPerHour;
|
||||
if (!_unlockRateLimiter.TryRegisterAttempt(throttleKey, perIpCap))
|
||||
{
|
||||
_logger?.LogWarning(
|
||||
"Bundle unlock throttled: client {ClientKey} exceeded {Cap} attempts in the trailing hour.",
|
||||
throttleKey, perIpCap);
|
||||
throw new BundleUnlockThrottledException(throttleKey, perIpCap);
|
||||
}
|
||||
|
||||
var aad = Encryption.BundleManifestAad.Compute(manifest);
|
||||
try
|
||||
{
|
||||
@@ -1715,6 +1756,32 @@ public sealed class BundleImporter : IBundleImporter
|
||||
}
|
||||
}
|
||||
|
||||
/// <inheritdoc />
|
||||
/// <remarks>
|
||||
/// Register row 43: every abandonment path (Central UI wizard reset /
|
||||
/// back-to-upload / circuit teardown, CLI preview, CLI import that aborts
|
||||
/// before Apply) funnels here so a session that will never be applied stops
|
||||
/// holding one of the <c>MaxConcurrentImportSessions</c> slots — and stops
|
||||
/// holding its decrypted plaintext — for the rest of its TTL. Applies the same
|
||||
/// zero-then-remove ordering as the T-007 release inside
|
||||
/// <see cref="ApplyAsync"/>: zeroing first means a caller still holding the
|
||||
/// session reference (the Razor page keeps one for the Result step) observes
|
||||
/// the cleared buffer too.
|
||||
/// </remarks>
|
||||
public void Abandon(Guid sessionId)
|
||||
{
|
||||
// Get() returns null for an unknown or already-expired id, which makes the
|
||||
// double-release after ApplyAsync (which already removed it) a no-op rather
|
||||
// than a throw. Remove is called unconditionally so an expired-but-still-
|
||||
// resident entry is dropped as well.
|
||||
var session = _sessionStore.Get(sessionId);
|
||||
if (session is not null)
|
||||
{
|
||||
ZeroDecryptedContent(session);
|
||||
}
|
||||
_sessionStore.Remove(sessionId);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Enumerates the deployed instances stale-ed by this import.
|
||||
/// Overwriting a template changes the flattened-config hash of every instance
|
||||
|
||||
+198
-2
@@ -9,6 +9,7 @@ using Microsoft.Extensions.DependencyInjection;
|
||||
using Microsoft.Extensions.Options;
|
||||
using NSubstitute;
|
||||
using NSubstitute.ExceptionExtensions;
|
||||
using ZB.MOM.WW.ScadaBridge.CentralUI.Auth;
|
||||
using ZB.MOM.WW.ScadaBridge.Commons.Entities.Sites;
|
||||
using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Repositories;
|
||||
using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Services;
|
||||
@@ -17,6 +18,7 @@ using ZB.MOM.WW.ScadaBridge.Commons.Types.Transport;
|
||||
using ZB.MOM.WW.ScadaBridge.ConfigurationDatabase;
|
||||
using ZB.MOM.WW.ScadaBridge.Security;
|
||||
using ZB.MOM.WW.ScadaBridge.Transport;
|
||||
using ZB.MOM.WW.ScadaBridge.Transport.Import;
|
||||
using TransportImportPage = ZB.MOM.WW.ScadaBridge.CentralUI.Components.Pages.Design.TransportImport;
|
||||
|
||||
namespace ZB.MOM.WW.ScadaBridge.CentralUI.Tests.Pages.Design;
|
||||
@@ -44,6 +46,9 @@ public class TransportImportPageTests : BunitContext
|
||||
// the Map tests) can render the wizard without a missing-service failure.
|
||||
private readonly ISiteRepository _siteRepo = Substitute.For<ISiteRepository>();
|
||||
|
||||
/// <summary>Client address the wizard reports for the per-IP unlock throttle (register row 44).</summary>
|
||||
private const string TestClientIp = "203.0.113.7";
|
||||
|
||||
public TransportImportPageTests()
|
||||
{
|
||||
JSInterop.Mode = JSRuntimeMode.Loose;
|
||||
@@ -51,6 +56,11 @@ public class TransportImportPageTests : BunitContext
|
||||
Services.AddSingleton(_importer);
|
||||
Services.AddSingleton(_auditService);
|
||||
|
||||
// Register row 44: the wizard passes the circuit's client IP to LoadAsync so
|
||||
// the per-IP unlock throttle can attribute the attempt. bUnit has no HTTP
|
||||
// context, so use the provider's explicit-address test seam.
|
||||
Services.AddSingleton(new CircuitClientAddressProvider(TestClientIp));
|
||||
|
||||
_siteRepo.GetAllSitesAsync(Arg.Any<CancellationToken>())
|
||||
.Returns(Array.Empty<Site>());
|
||||
_siteRepo.GetDataConnectionsBySiteIdAsync(Arg.Any<int>(), Arg.Any<CancellationToken>())
|
||||
@@ -131,7 +141,7 @@ public class TransportImportPageTests : BunitContext
|
||||
public async Task Decryption_failure_increments_attempt_counter()
|
||||
{
|
||||
// Set up the importer to throw CryptographicException for wrong passphrases.
|
||||
_importer.LoadAsync(Arg.Any<Stream>(), Arg.Any<string>(), Arg.Any<CancellationToken>())
|
||||
_importer.LoadAsync(Arg.Any<Stream>(), Arg.Any<string>(), Arg.Any<CancellationToken>(), Arg.Any<string?>())
|
||||
.Throws(new CryptographicException("authentication tag mismatch"));
|
||||
|
||||
var cut = Render<TransportImportPage>();
|
||||
@@ -161,7 +171,7 @@ public class TransportImportPageTests : BunitContext
|
||||
[Fact]
|
||||
public async Task Three_failed_unlocks_force_reupload()
|
||||
{
|
||||
_importer.LoadAsync(Arg.Any<Stream>(), Arg.Any<string>(), Arg.Any<CancellationToken>())
|
||||
_importer.LoadAsync(Arg.Any<Stream>(), Arg.Any<string>(), Arg.Any<CancellationToken>(), Arg.Any<string?>())
|
||||
.Throws(new CryptographicException("authentication tag mismatch"));
|
||||
|
||||
var cut = Render<TransportImportPage>();
|
||||
@@ -688,6 +698,192 @@ public class TransportImportPageTests : BunitContext
|
||||
Assert.Empty(connChoices);
|
||||
}
|
||||
|
||||
// ─────────────────────────────────────────────────────────────────────
|
||||
// Register row 43: an abandoned wizard must not strand its server-side
|
||||
// BundleSession. These tests swap the NSubstitute importer for one backed
|
||||
// by a REAL BundleSessionStore so the assertion is "the slot is actually
|
||||
// released", not "a mock was called".
|
||||
// ─────────────────────────────────────────────────────────────────────
|
||||
|
||||
/// <summary>
|
||||
/// Minimal <see cref="IBundleImporter"/> over a real <see cref="BundleSessionStore"/>.
|
||||
/// <see cref="Abandon"/> and <see cref="ApplyAsync"/> mirror the production
|
||||
/// importer's zero-then-remove release; every other member is unused by these
|
||||
/// tests. Sessions are seeded directly with <see cref="OpenSession"/> because
|
||||
/// driving <c>InputFile</c> from bUnit is impractical (see the class remarks).
|
||||
/// </summary>
|
||||
private sealed class StoreBackedImporter : IBundleImporter
|
||||
{
|
||||
public BundleSessionStore Store { get; } = new(
|
||||
Microsoft.Extensions.Options.Options.Create(new TransportOptions()),
|
||||
TimeProvider.System);
|
||||
|
||||
public int AbandonCalls { get; private set; }
|
||||
|
||||
public BundleSession OpenSession(string sourceEnv = "prod-cluster")
|
||||
{
|
||||
var seed = BuildEncryptedSession(sourceEnv);
|
||||
var session = new BundleSession
|
||||
{
|
||||
SessionId = seed.SessionId,
|
||||
Manifest = seed.Manifest,
|
||||
// Non-zero bytes so the zeroing assertion is meaningful.
|
||||
DecryptedContent = new byte[] { 0x11, 0x22, 0x33, 0x44 },
|
||||
ExpiresAt = DateTimeOffset.UtcNow.AddMinutes(30),
|
||||
};
|
||||
return Store.Open(session);
|
||||
}
|
||||
|
||||
public Task<BundleSession> LoadAsync(
|
||||
Stream bundleStream, string? passphrase, CancellationToken ct = default, string? clientKey = null)
|
||||
=> throw new NotSupportedException("These tests seed sessions via OpenSession().");
|
||||
|
||||
public Task<ImportPreview> PreviewAsync(Guid sessionId, CancellationToken ct = default)
|
||||
=> Task.FromResult(new ImportPreview(sessionId, new List<ImportPreviewItem>()));
|
||||
|
||||
public Task<ImportResult> ApplyAsync(
|
||||
Guid sessionId,
|
||||
IReadOnlyList<ImportResolution> resolutions,
|
||||
string user,
|
||||
CancellationToken ct = default,
|
||||
BundleNameMap? nameMap = null)
|
||||
{
|
||||
Release(sessionId);
|
||||
return Task.FromResult(new ImportResult(
|
||||
BundleImportId: Guid.NewGuid(),
|
||||
Added: 1,
|
||||
Overwritten: 0,
|
||||
Skipped: 0,
|
||||
Renamed: 0,
|
||||
StaleInstanceIds: Array.Empty<int>(),
|
||||
AuditEventCorrelation: Guid.NewGuid().ToString()));
|
||||
}
|
||||
|
||||
public void Abandon(Guid sessionId)
|
||||
{
|
||||
AbandonCalls++;
|
||||
Release(sessionId);
|
||||
}
|
||||
|
||||
private void Release(Guid sessionId)
|
||||
{
|
||||
var session = Store.Get(sessionId);
|
||||
if (session is { DecryptedContent.Length: > 0 })
|
||||
{
|
||||
Array.Clear(session.DecryptedContent, 0, session.DecryptedContent.Length);
|
||||
}
|
||||
Store.Remove(sessionId);
|
||||
}
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task Disposing_the_wizard_releases_its_bundle_session()
|
||||
{
|
||||
// Circuit teardown / navigation away. Before the fix, Dispose deleted only
|
||||
// the staged temp file, so the session (and its decrypted content) held one
|
||||
// of MaxConcurrentImportSessions for the full 30-minute TTL.
|
||||
var importer = new StoreBackedImporter();
|
||||
Services.AddSingleton<IBundleImporter>(importer);
|
||||
|
||||
var cut = Render<TransportImportPage>();
|
||||
var session = importer.OpenSession();
|
||||
var content = session.DecryptedContent;
|
||||
await cut.InvokeAsync(() => SetField(cut.Instance, "_session", session));
|
||||
|
||||
cut.Instance.Dispose();
|
||||
|
||||
Assert.Null(importer.Store.Get(session.SessionId));
|
||||
Assert.All(content, b => Assert.Equal(0, b));
|
||||
Assert.Null(GetField<object?>(cut.Instance, "_session"));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task BackToUpload_releases_the_bundle_session()
|
||||
{
|
||||
// The wizard's only cancel affordance ("Back" on steps 2/3/5) routes through
|
||||
// BackToUpload → ResetSessionState, which used to null the local field only.
|
||||
var importer = new StoreBackedImporter();
|
||||
Services.AddSingleton<IBundleImporter>(importer);
|
||||
|
||||
var cut = Render<TransportImportPage>();
|
||||
var session = importer.OpenSession();
|
||||
var content = session.DecryptedContent;
|
||||
await cut.InvokeAsync(() =>
|
||||
{
|
||||
SetField(cut.Instance, "_session", session);
|
||||
SetField(cut.Instance, "_step", TransportImportPage.ImportWizardStep.Diff);
|
||||
});
|
||||
|
||||
await cut.InvokeAsync(() =>
|
||||
{
|
||||
var method = cut.Instance.GetType().GetMethod(
|
||||
"BackToUpload",
|
||||
System.Reflection.BindingFlags.Instance | System.Reflection.BindingFlags.NonPublic)!;
|
||||
method.Invoke(cut.Instance, Array.Empty<object?>());
|
||||
});
|
||||
|
||||
Assert.Null(importer.Store.Get(session.SessionId));
|
||||
Assert.All(content, b => Assert.Equal(0, b));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task Apply_then_dispose_releases_the_session_once_without_throwing()
|
||||
{
|
||||
// ApplyAsync already removes the session; the wizard's own release must be
|
||||
// idempotent so circuit teardown after a completed import is a silent no-op.
|
||||
var importer = new StoreBackedImporter();
|
||||
Services.AddSingleton<IBundleImporter>(importer);
|
||||
|
||||
var cut = Render<TransportImportPage>();
|
||||
var session = importer.OpenSession(sourceEnv: "prod-cluster");
|
||||
await cut.InvokeAsync(() =>
|
||||
{
|
||||
SetField(cut.Instance, "_session", session);
|
||||
SetField(cut.Instance, "_resolutions",
|
||||
new Dictionary<(string EntityType, string Name), ImportResolution>
|
||||
{
|
||||
[("Template", "Pump")] = new("Template", "Pump", ResolutionAction.Add, null),
|
||||
});
|
||||
SetField(cut.Instance, "_step", TransportImportPage.ImportWizardStep.Confirm);
|
||||
SetField(cut.Instance, "_confirmEnvironmentText", "prod-cluster");
|
||||
});
|
||||
|
||||
await cut.InvokeAsync(() => InvokeAsyncMethod(cut.Instance, "ApplyAsync"));
|
||||
Assert.Equal(
|
||||
TransportImportPage.ImportWizardStep.Result,
|
||||
GetField<TransportImportPage.ImportWizardStep>(cut.Instance, "_step"));
|
||||
Assert.Null(importer.Store.Get(session.SessionId));
|
||||
|
||||
// Must not throw, and must not resurrect or double-release anything.
|
||||
cut.Instance.Dispose();
|
||||
cut.Instance.Dispose();
|
||||
|
||||
Assert.Null(importer.Store.Get(session.SessionId));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task Disposing_one_wizard_leaves_a_concurrent_circuits_session_open()
|
||||
{
|
||||
// Sessions are keyed by a GUID held only by the circuit that opened them,
|
||||
// so one operator abandoning their import must not evict another's.
|
||||
var importer = new StoreBackedImporter();
|
||||
Services.AddSingleton<IBundleImporter>(importer);
|
||||
|
||||
var first = Render<TransportImportPage>();
|
||||
var second = Render<TransportImportPage>();
|
||||
var mine = importer.OpenSession();
|
||||
var theirs = importer.OpenSession(sourceEnv: "other-cluster");
|
||||
await first.InvokeAsync(() => SetField(first.Instance, "_session", mine));
|
||||
await second.InvokeAsync(() => SetField(second.Instance, "_session", theirs));
|
||||
|
||||
first.Instance.Dispose();
|
||||
|
||||
Assert.Null(importer.Store.Get(mine.SessionId));
|
||||
var survivor = importer.Store.Get(theirs.SessionId);
|
||||
Assert.NotNull(survivor);
|
||||
Assert.Contains(survivor!.DecryptedContent, b => b != 0);
|
||||
}
|
||||
|
||||
// ─────────────────────────────────────────────────────────────────────
|
||||
// Reflection helpers — the wizard's per-instance state is private (the
|
||||
// razor partial pattern). We poke at it via reflection rather than
|
||||
|
||||
@@ -234,6 +234,10 @@ public sealed class QueryStringDrillInTests
|
||||
Services.AddSingleton(Substitute.For<IAuditService>());
|
||||
// M8 E2: TransportImport's Map step injects ISiteRepository.
|
||||
Services.AddSingleton(Substitute.For<ISiteRepository>());
|
||||
// Register row 44: the wizard reads the circuit's client IP for the
|
||||
// per-IP unlock throttle. No HTTP context under bUnit — use the seam.
|
||||
Services.AddSingleton(new ZB.MOM.WW.ScadaBridge.CentralUI.Auth.CircuitClientAddressProvider(
|
||||
clientAddress: null));
|
||||
Services.AddSingleton<IOptions<TransportOptions>>(
|
||||
Microsoft.Extensions.Options.Options.Create(new TransportOptions
|
||||
{
|
||||
|
||||
@@ -2734,7 +2734,7 @@ public class ManagementActorTests : TestKit, IDisposable
|
||||
var (_, importer) = AddBundleSubstitutes();
|
||||
|
||||
var sessionId = Guid.NewGuid();
|
||||
importer.LoadAsync(Arg.Any<Stream>(), Arg.Any<string?>(), Arg.Any<CancellationToken>())
|
||||
importer.LoadAsync(Arg.Any<Stream>(), Arg.Any<string?>(), Arg.Any<CancellationToken>(), Arg.Any<string?>())
|
||||
.Returns(new Commons.Types.Transport.BundleSession
|
||||
{
|
||||
SessionId = sessionId,
|
||||
@@ -2787,7 +2787,7 @@ public class ManagementActorTests : TestKit, IDisposable
|
||||
var (_, importer) = AddBundleSubstitutes();
|
||||
|
||||
var sessionId = Guid.NewGuid();
|
||||
importer.LoadAsync(Arg.Any<Stream>(), Arg.Any<string?>(), Arg.Any<CancellationToken>())
|
||||
importer.LoadAsync(Arg.Any<Stream>(), Arg.Any<string?>(), Arg.Any<CancellationToken>(), Arg.Any<string?>())
|
||||
.Returns(new Commons.Types.Transport.BundleSession
|
||||
{
|
||||
SessionId = sessionId,
|
||||
@@ -2865,7 +2865,7 @@ public class ManagementActorTests : TestKit, IDisposable
|
||||
IReadOnlyList<Commons.Types.Transport.RequiredConnectionMapping> connectionMappings)
|
||||
{
|
||||
var sessionId = Guid.NewGuid();
|
||||
importer.LoadAsync(Arg.Any<Stream>(), Arg.Any<string?>(), Arg.Any<CancellationToken>())
|
||||
importer.LoadAsync(Arg.Any<Stream>(), Arg.Any<string?>(), Arg.Any<CancellationToken>(), Arg.Any<string?>())
|
||||
.Returns(new Commons.Types.Transport.BundleSession
|
||||
{
|
||||
SessionId = sessionId,
|
||||
@@ -3089,7 +3089,7 @@ public class ManagementActorTests : TestKit, IDisposable
|
||||
var (_, importer) = AddBundleSubstitutes();
|
||||
|
||||
var sessionId = Guid.NewGuid();
|
||||
importer.LoadAsync(Arg.Any<Stream>(), Arg.Any<string?>(), Arg.Any<CancellationToken>())
|
||||
importer.LoadAsync(Arg.Any<Stream>(), Arg.Any<string?>(), Arg.Any<CancellationToken>(), Arg.Any<string?>())
|
||||
.Returns(new Commons.Types.Transport.BundleSession
|
||||
{
|
||||
SessionId = sessionId,
|
||||
@@ -3126,6 +3126,99 @@ public class ManagementActorTests : TestKit, IDisposable
|
||||
Assert.Contains("OpcA", success.JsonData);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void PreviewBundleCommand_ReleasesTheSession_AndForwardsTheCallersIp()
|
||||
{
|
||||
// Register row 43: a preview never applies, so nothing else releases the
|
||||
// session — it used to hold one of MaxConcurrentImportSessions for the full
|
||||
// TTL. Register row 44: the envelope's client IP must reach LoadAsync so the
|
||||
// per-IP-per-hour unlock throttle can attribute the attempt.
|
||||
var (_, importer) = AddBundleSubstitutes();
|
||||
|
||||
var sessionId = Guid.NewGuid();
|
||||
importer.LoadAsync(Arg.Any<Stream>(), Arg.Any<string?>(), Arg.Any<CancellationToken>(), Arg.Any<string?>())
|
||||
.Returns(new Commons.Types.Transport.BundleSession
|
||||
{
|
||||
SessionId = sessionId,
|
||||
Manifest = null!,
|
||||
ExpiresAt = DateTimeOffset.UtcNow.AddMinutes(5),
|
||||
});
|
||||
importer.PreviewAsync(sessionId, Arg.Any<CancellationToken>())
|
||||
.Returns(new Commons.Types.Transport.ImportPreview(
|
||||
sessionId,
|
||||
Array.Empty<Commons.Types.Transport.ImportPreviewItem>()));
|
||||
|
||||
var payload = Convert.ToBase64String(new byte[] { 0x01, 0x02, 0x03 });
|
||||
var actor = CreateActor();
|
||||
var envelope = Envelope(new PreviewBundleCommand(payload, null), "Administrator")
|
||||
with { ClientIp = "198.51.100.9" };
|
||||
|
||||
actor.Tell(envelope);
|
||||
|
||||
ExpectMsg<ManagementSuccess>(TimeSpan.FromSeconds(5));
|
||||
importer.Received(1).LoadAsync(
|
||||
Arg.Any<Stream>(), Arg.Any<string?>(), Arg.Any<CancellationToken>(), "198.51.100.9");
|
||||
importer.Received(1).Abandon(sessionId);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void ImportBundleCommand_WithBlockerRow_StillReleasesTheSession()
|
||||
{
|
||||
// Register row 43: ApplyAsync releases the session, but this import aborts
|
||||
// BEFORE apply — the handler must release it anyway.
|
||||
var (_, importer) = AddBundleSubstitutes();
|
||||
|
||||
var sessionId = Guid.NewGuid();
|
||||
importer.LoadAsync(Arg.Any<Stream>(), Arg.Any<string?>(), Arg.Any<CancellationToken>(), Arg.Any<string?>())
|
||||
.Returns(new Commons.Types.Transport.BundleSession
|
||||
{
|
||||
SessionId = sessionId,
|
||||
Manifest = null!,
|
||||
ExpiresAt = DateTimeOffset.UtcNow.AddMinutes(5),
|
||||
});
|
||||
importer.PreviewAsync(sessionId, Arg.Any<CancellationToken>())
|
||||
.Returns(new Commons.Types.Transport.ImportPreview(
|
||||
sessionId,
|
||||
new[]
|
||||
{
|
||||
new Commons.Types.Transport.ImportPreviewItem(
|
||||
EntityType: "Template",
|
||||
Name: "BlockedTemplate",
|
||||
ExistingVersion: null,
|
||||
IncomingVersion: 1,
|
||||
Kind: Commons.Types.Transport.ConflictKind.Blocker,
|
||||
FieldDiffJson: null,
|
||||
BlockerReason: "FK to missing site"),
|
||||
}));
|
||||
|
||||
var payload = Convert.ToBase64String(new byte[] { 0x01, 0x02, 0x03 });
|
||||
var actor = CreateActor();
|
||||
actor.Tell(Envelope(new ImportBundleCommand(payload, null, "skip"), "Administrator"));
|
||||
|
||||
ExpectMsg<ManagementError>(TimeSpan.FromSeconds(5));
|
||||
importer.Received(1).Abandon(sessionId);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void ImportBundleCommand_ThrottledUnlock_SurfacesTheCuratedReason()
|
||||
{
|
||||
// Register row 44: a per-IP throttle denial must fail closed with a clear
|
||||
// message, not MapFault's generic "an internal error occurred".
|
||||
var (_, importer) = AddBundleSubstitutes();
|
||||
|
||||
importer.LoadAsync(Arg.Any<Stream>(), Arg.Any<string?>(), Arg.Any<CancellationToken>(), Arg.Any<string?>())
|
||||
.Returns<Commons.Types.Transport.BundleSession>(_ =>
|
||||
throw new Commons.Types.Transport.BundleUnlockThrottledException("198.51.100.9", 10));
|
||||
|
||||
var payload = Convert.ToBase64String(new byte[] { 0x01, 0x02, 0x03 });
|
||||
var actor = CreateActor();
|
||||
actor.Tell(Envelope(new ImportBundleCommand(payload, "pass", "skip"), "Administrator"));
|
||||
|
||||
var response = ExpectMsg<ManagementError>(TimeSpan.FromSeconds(5));
|
||||
Assert.Equal("COMMAND_FAILED", response.ErrorCode);
|
||||
Assert.Contains("Too many bundle unlock attempts", response.Error);
|
||||
}
|
||||
|
||||
// ========================================================================
|
||||
// Native alarm source CRUD (Task 21)
|
||||
// ========================================================================
|
||||
|
||||
@@ -53,7 +53,7 @@ public class TransportGateTests : TestKit, IDisposable
|
||||
var loadCalls = 0;
|
||||
|
||||
var session = new BundleSession { SessionId = Guid.NewGuid() };
|
||||
_importer.LoadAsync(Arg.Any<Stream>(), Arg.Any<string?>(), Arg.Any<CancellationToken>())
|
||||
_importer.LoadAsync(Arg.Any<Stream>(), Arg.Any<string?>(), Arg.Any<CancellationToken>(), Arg.Any<string?>())
|
||||
.Returns(_ =>
|
||||
{
|
||||
Interlocked.Increment(ref loadCalls);
|
||||
|
||||
+246
-75
@@ -110,29 +110,6 @@ public class NotificationOutboxActorDispatchTests : TestKit
|
||||
.Returns(new[] { config });
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Drives a sweep to completion by re-telling <see cref="InternalMessages.DispatchTick"/>
|
||||
/// on every poll iteration until <paramref name="assertion"/> holds. This is the durable
|
||||
/// barrier for chained-sweep tests: the in-flight guard (<c>_dispatching</c>) is only
|
||||
/// lowered when a sweep's <see cref="InternalMessages.DispatchComplete"/> round-trips back
|
||||
/// to the actor — AFTER the side-effect the assertion observes — so a single one-shot tick
|
||||
/// can race the still-raised guard and be silently dropped (the dispatcher's intended
|
||||
/// overlap protection), permanently stalling the count. A dropped tick is never retried, so
|
||||
/// merely widening the wait cannot recover it. Re-telling each poll is idempotent (a tick
|
||||
/// landing while the guard is up is harmlessly dropped) and the assertion's exact-count
|
||||
/// strength is preserved unchanged.
|
||||
/// </summary>
|
||||
private void DrivePollingTick(IActorRef actor, Action assertion)
|
||||
{
|
||||
AwaitAssert(
|
||||
() =>
|
||||
{
|
||||
actor.Tell(InternalMessages.DispatchTick.Instance);
|
||||
assertion();
|
||||
},
|
||||
duration: TimeSpan.FromSeconds(10));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void DispatchTick_ClaimsDueNotifications_AndInvokesAdapter()
|
||||
{
|
||||
@@ -477,72 +454,266 @@ public class NotificationOutboxActorDispatchTests : TestKit
|
||||
"PostStop did not cancel the in-flight delivery promptly.");
|
||||
}
|
||||
|
||||
// ── NotificationOutbox-006: adapter dictionary cached for the actor's lifetime ──
|
||||
// ── Register row 45: the delivery adapter is resolved PER DELIVERY ────────
|
||||
//
|
||||
// Supersedes NotificationOutbox-006 ("resolve the adapter dictionary once for the
|
||||
// actor's lifetime"). That optimisation cached scoped adapter instances — which hold
|
||||
// a scoped INotificationRepository over an EF DbContext — in an actor-lifetime DI
|
||||
// scope, so MaxParallelDeliveries concurrent deliveries all ran on ONE DbContext and
|
||||
// the rig logged `InvalidOperationException: A second operation was started on this
|
||||
// context instance before a previous operation completed` continuously. The adapter
|
||||
// now resolves inside the per-delivery scope that DeliverGatedAsync already opens.
|
||||
|
||||
/// <summary>
|
||||
/// Aggregates what the parallel deliveries observed: how many times two deliveries
|
||||
/// were simultaneously inside ONE simulated DbContext, and a rendezvous that only
|
||||
/// completes once every delivery is concurrently in flight.
|
||||
/// </summary>
|
||||
private sealed class DeliveryConcurrencyLedger
|
||||
{
|
||||
private readonly int _expectedArrivals;
|
||||
private readonly TaskCompletionSource _allArrived =
|
||||
new(TaskCreationOptions.RunContinuationsAsynchronously);
|
||||
|
||||
private int _arrived;
|
||||
private int _overlaps;
|
||||
|
||||
public DeliveryConcurrencyLedger(int expectedArrivals) => _expectedArrivals = expectedArrivals;
|
||||
|
||||
/// <summary>Number of second-operation-on-one-context collisions observed.</summary>
|
||||
public int Overlaps => Volatile.Read(ref _overlaps);
|
||||
|
||||
public void RecordOverlap() => Interlocked.Increment(ref _overlaps);
|
||||
|
||||
/// <summary>
|
||||
/// Holds the caller's simulated DbContext open until every delivery has reached
|
||||
/// this point — guaranteeing real overlap — or a safety timeout elapses. The
|
||||
/// timeout matters for the shared-context (pre-fix) run, where the colliding
|
||||
/// delivery throws instead of arriving and the rendezvous can never complete.
|
||||
/// </summary>
|
||||
public async Task ArriveAndWaitAsync(CancellationToken cancellationToken)
|
||||
{
|
||||
if (Interlocked.Increment(ref _arrived) >= _expectedArrivals)
|
||||
{
|
||||
_allArrived.TrySetResult();
|
||||
}
|
||||
|
||||
await Task.WhenAny(
|
||||
_allArrived.Task,
|
||||
Task.Delay(TimeSpan.FromSeconds(2), cancellationToken))
|
||||
.ConfigureAwait(false);
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Stands in for the EF <c>DbContext</c> behind a scoped
|
||||
/// <see cref="INotificationRepository"/> — one probe per resolved repository. A second
|
||||
/// concurrent operation throws the exact <see cref="InvalidOperationException"/> EF
|
||||
/// raises, so a test that shares one repository across parallel deliveries fails the
|
||||
/// same way the rig did.
|
||||
/// </summary>
|
||||
private sealed class SimulatedDbContextProbe
|
||||
{
|
||||
private readonly DeliveryConcurrencyLedger _ledger;
|
||||
private int _inFlight;
|
||||
|
||||
public SimulatedDbContextProbe(DeliveryConcurrencyLedger ledger) => _ledger = ledger;
|
||||
|
||||
public async Task EnterAsync(CancellationToken cancellationToken)
|
||||
{
|
||||
if (Interlocked.Increment(ref _inFlight) > 1)
|
||||
{
|
||||
Interlocked.Decrement(ref _inFlight);
|
||||
_ledger.RecordOverlap();
|
||||
throw new InvalidOperationException(
|
||||
"A second operation was started on this context instance before a "
|
||||
+ "previous operation completed.");
|
||||
}
|
||||
|
||||
try
|
||||
{
|
||||
await _ledger.ArriveAndWaitAsync(cancellationToken);
|
||||
}
|
||||
finally
|
||||
{
|
||||
Interlocked.Decrement(ref _inFlight);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Adapter shaped like the real Email/SMS adapters: it does its work THROUGH the
|
||||
/// scoped <see cref="INotificationRepository"/> it was constructed with, so sharing
|
||||
/// the adapter across deliveries also shares that repository's DbContext.
|
||||
/// </summary>
|
||||
private sealed class RepositoryBackedAdapter : INotificationDeliveryAdapter
|
||||
{
|
||||
private readonly INotificationRepository _repository;
|
||||
|
||||
public RepositoryBackedAdapter(INotificationRepository repository) => _repository = repository;
|
||||
|
||||
public NotificationType Type => NotificationType.Email;
|
||||
|
||||
public async Task<DeliveryOutcome> DeliverAsync(
|
||||
Notification notification, CancellationToken cancellationToken = default)
|
||||
{
|
||||
// Mirrors the real adapters' first act: resolve the target list off the
|
||||
// scoped repository (and therefore the scoped DbContext).
|
||||
await _repository.GetListByNameAsync(notification.ListName, cancellationToken);
|
||||
return DeliveryOutcome.Success("ops@example.com");
|
||||
}
|
||||
}
|
||||
|
||||
private static Task<NotificationList?> EnterProbeAsync(
|
||||
SimulatedDbContextProbe probe, CancellationToken cancellationToken)
|
||||
{
|
||||
return EnterProbeCoreAsync(probe, cancellationToken);
|
||||
|
||||
static async Task<NotificationList?> EnterProbeCoreAsync(
|
||||
SimulatedDbContextProbe probe, CancellationToken cancellationToken)
|
||||
{
|
||||
await probe.EnterAsync(cancellationToken);
|
||||
// A null list is enough: the stub adapter ignores the result, and the
|
||||
// dispatcher never sees it.
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Dispatch_ResolvesAdaptersOnce_AcrossMultipleSweeps()
|
||||
public void ParallelDeliveries_EachGetTheirOwnAdapterAndDbContext()
|
||||
{
|
||||
// NotificationOutbox-006: adapter registration is static per process lifetime,
|
||||
// so the NotificationType -> adapter lookup must be built ONCE for the actor's
|
||||
// lifetime, not per dispatch sweep. The cache is paired with an actor-lifetime
|
||||
// DI scope (see _adaptersScope) so scoped adapter instances are reused safely.
|
||||
SetupSmtpRetryPolicy(maxRetries: 5, retryDelay: TimeSpan.FromMinutes(1));
|
||||
// Isolated substitutes for this test — we replace the dispatcher's per-sweep
|
||||
// INotificationOutboxRepository registration with a private counting factory,
|
||||
// so we don't mutate the shared _outboxRepository field that other tests in
|
||||
// this class configure differently.
|
||||
var outboxRepository = OutboxRepositorySubstitute.Healthy();
|
||||
// De-race (S11): hand out a fresh due notification for the FIRST THREE claims, then
|
||||
// an empty batch forever. This caps the deliverable work — and therefore the
|
||||
// UpdateAsync count — at exactly three, no matter how many dispatch ticks the
|
||||
// driver below fires. Without this ceiling the exact-count barrier is doubly
|
||||
// brittle: a single one-shot tick that races the in-flight guard is silently
|
||||
// dropped (stalling the count below three), while a tick-per-poll driver lets
|
||||
// queued ticks pile up under CPU starvation and overshoot three. Capping the
|
||||
// claimable batch lets the driver re-tell ticks idempotently — surplus sweeps
|
||||
// claim nothing and never call UpdateAsync — so Received(3) is a stable target
|
||||
// that, once reached, cannot be exceeded.
|
||||
var dueClaims = 0;
|
||||
outboxRepository.GetDueAsync(Arg.Any<DateTimeOffset>(), Arg.Any<int>(), Arg.Any<CancellationToken>())
|
||||
.Returns(_ => Interlocked.Increment(ref dueClaims) <= 3
|
||||
? new[] { MakeNotification() }
|
||||
: Array.Empty<Notification>());
|
||||
// Register row 45 (fail-before-fix). Two due notifications dispatched with
|
||||
// MaxParallelDeliveries = 4. Each delivery's adapter reaches its scoped
|
||||
// INotificationRepository, whose simulated DbContext throws EF's
|
||||
// second-operation exception if a second delivery enters it concurrently.
|
||||
//
|
||||
// Pre-fix: the actor-lifetime _adaptersScope hands BOTH deliveries the same
|
||||
// adapter -> the same repository -> one probe. The rendezvous holds the first
|
||||
// delivery inside the context while the second collides, so Overlaps == 1 and
|
||||
// the collided row is never persisted (DeliverGatedAsync logs and moves on).
|
||||
// Post-fix: two per-delivery scopes -> two adapters -> two probes, both arrive
|
||||
// at the rendezvous concurrently, nothing collides, both rows persist.
|
||||
const int dueCount = 2;
|
||||
var ledger = new DeliveryConcurrencyLedger(expectedArrivals: dueCount);
|
||||
|
||||
var outboxRepository = OutboxRepositorySubstitute.Healthy();
|
||||
var batch = Enumerable.Range(0, dueCount).Select(_ => MakeNotification()).ToArray();
|
||||
var claimed = 0;
|
||||
outboxRepository.GetDueAsync(Arg.Any<DateTimeOffset>(), Arg.Any<int>(), Arg.Any<CancellationToken>())
|
||||
.Returns(_ => Interlocked.Increment(ref claimed) == 1 ? batch : Array.Empty<Notification>());
|
||||
|
||||
var smtpConfiguration = new SmtpConfiguration("smtp.example.com", "Basic", "noreply@example.com")
|
||||
{
|
||||
MaxRetries = 5,
|
||||
RetryDelay = TimeSpan.FromMinutes(1),
|
||||
};
|
||||
|
||||
// Counting factory: increments each time the DI container resolves an
|
||||
// INotificationDeliveryAdapter. Pre-fix this would have ticked once per
|
||||
// sweep; post-fix it ticks exactly once for the actor's lifetime.
|
||||
var resolutionCount = 0;
|
||||
var services = new ServiceCollection();
|
||||
services.AddScoped(_ => outboxRepository);
|
||||
services.AddScoped(_ => _notificationRepository);
|
||||
services.AddScoped<INotificationDeliveryAdapter>(_ =>
|
||||
// Scoped, with its OWN simulated DbContext per resolution — exactly the
|
||||
// production lifetime of INotificationRepository.
|
||||
services.AddScoped<INotificationRepository>(_ =>
|
||||
{
|
||||
Interlocked.Increment(ref resolutionCount);
|
||||
return new StubAdapter(() => DeliveryOutcome.Success("ops@example.com"));
|
||||
var probe = new SimulatedDbContextProbe(ledger);
|
||||
var repository = Substitute.For<INotificationRepository>();
|
||||
repository.GetAllSmtpConfigurationsAsync(Arg.Any<CancellationToken>())
|
||||
.Returns(new[] { smtpConfiguration });
|
||||
repository.GetListByNameAsync(Arg.Any<string>(), Arg.Any<CancellationToken>())
|
||||
.Returns(call => EnterProbeAsync(probe, call.Arg<CancellationToken>()));
|
||||
return repository;
|
||||
});
|
||||
var sp = services.BuildServiceProvider();
|
||||
// Scoped adapter holding the scoped repository — the production shape of both
|
||||
// EmailNotificationDeliveryAdapter and SmsNotificationDeliveryAdapter.
|
||||
services.AddScoped<INotificationDeliveryAdapter>(sp =>
|
||||
new RepositoryBackedAdapter(sp.GetRequiredService<INotificationRepository>()));
|
||||
var serviceProvider = services.BuildServiceProvider();
|
||||
|
||||
var actor = Sys.ActorOf(Props.Create(() => new NotificationOutboxActor(
|
||||
sp,
|
||||
new NotificationOutboxOptions { DispatchInterval = TimeSpan.FromHours(1) },
|
||||
serviceProvider,
|
||||
new NotificationOutboxOptions
|
||||
{
|
||||
DispatchInterval = TimeSpan.FromHours(1),
|
||||
MaxParallelDeliveries = 4,
|
||||
},
|
||||
new NoOpCentralAuditWriter(),
|
||||
NullLogger<NotificationOutboxActor>.Instance)));
|
||||
|
||||
// Drive dispatch sweeps until all three claimable notifications have been
|
||||
// delivered (Received(3).UpdateAsync). DrivePollingTick re-tells the tick on
|
||||
// every poll iteration; the GetDueAsync cap above makes that idempotent (a
|
||||
// surplus sweep claims an empty batch and calls UpdateAsync zero times), so
|
||||
// the count climbs to exactly three and then holds there — Received(3) is a
|
||||
// stable ceiling rather than a fragile one-shot target. This proves multiple
|
||||
// dispatch sweeps ran across the actor's lifetime, which is the precondition
|
||||
// for the resolve-adapters-once assertion that follows.
|
||||
DrivePollingTick(actor, () => outboxRepository.Received(3).UpdateAsync(
|
||||
Arg.Any<Notification>(), Arg.Any<CancellationToken>()));
|
||||
actor.Tell(InternalMessages.DispatchTick.Instance);
|
||||
|
||||
// The adapter resolution must have happened EXACTLY ONCE despite the multiple
|
||||
// dispatch sweeps just driven. Pre-fix this would have been once per sweep.
|
||||
// This is the real invariant under test (NotificationOutbox-006).
|
||||
Assert.Equal(1, resolutionCount);
|
||||
AwaitAssert(
|
||||
() => outboxRepository.Received(dueCount).UpdateAsync(
|
||||
Arg.Is<Notification>(n => n.Status == NotificationStatus.Delivered),
|
||||
Arg.Any<CancellationToken>()),
|
||||
duration: TimeSpan.FromSeconds(10));
|
||||
|
||||
Assert.Equal(0, ledger.Overlaps);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Dispatch_ResolvesAdapterPerDelivery_NotOncePerActorLifetime()
|
||||
{
|
||||
// Register row 45 (isolation, direct form): every delivery must resolve its own
|
||||
// adapter instance from its own DI scope. Four due rows dispatched in one sweep
|
||||
// must therefore yield four DISTINCT adapter instances. Pre-fix the actor cached
|
||||
// a single instance for its whole lifetime and this collapses to 1.
|
||||
const int dueCount = 4;
|
||||
SetupSmtpRetryPolicy(maxRetries: 5, retryDelay: TimeSpan.FromMinutes(1));
|
||||
|
||||
var outboxRepository = OutboxRepositorySubstitute.Healthy();
|
||||
var batch = Enumerable.Range(0, dueCount).Select(_ => MakeNotification()).ToArray();
|
||||
var claimed = 0;
|
||||
outboxRepository.GetDueAsync(Arg.Any<DateTimeOffset>(), Arg.Any<int>(), Arg.Any<CancellationToken>())
|
||||
.Returns(_ => Interlocked.Increment(ref claimed) == 1 ? batch : Array.Empty<Notification>());
|
||||
|
||||
var deliveringInstances = new System.Collections.Concurrent.ConcurrentBag<INotificationDeliveryAdapter>();
|
||||
var services = new ServiceCollection();
|
||||
services.AddScoped(_ => outboxRepository);
|
||||
services.AddScoped(_ => _notificationRepository);
|
||||
services.AddScoped<INotificationDeliveryAdapter>(_ => new RecordingStubAdapter(deliveringInstances));
|
||||
var serviceProvider = services.BuildServiceProvider();
|
||||
|
||||
var actor = Sys.ActorOf(Props.Create(() => new NotificationOutboxActor(
|
||||
serviceProvider,
|
||||
new NotificationOutboxOptions
|
||||
{
|
||||
DispatchInterval = TimeSpan.FromHours(1),
|
||||
MaxParallelDeliveries = 4,
|
||||
},
|
||||
new NoOpCentralAuditWriter(),
|
||||
NullLogger<NotificationOutboxActor>.Instance)));
|
||||
|
||||
actor.Tell(InternalMessages.DispatchTick.Instance);
|
||||
|
||||
AwaitAssert(
|
||||
() => outboxRepository.Received(dueCount).UpdateAsync(
|
||||
Arg.Is<Notification>(n => n.Status == NotificationStatus.Delivered),
|
||||
Arg.Any<CancellationToken>()),
|
||||
duration: TimeSpan.FromSeconds(10));
|
||||
|
||||
// RecordingStubAdapter does not override Equals, so Distinct() is reference
|
||||
// identity — exactly the "one instance per delivery" question being asked.
|
||||
Assert.Equal(dueCount, deliveringInstances.Distinct().Count());
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Stub adapter that records which instance actually served each delivery, so a
|
||||
/// test can prove one adapter instance per delivery.
|
||||
/// </summary>
|
||||
private sealed class RecordingStubAdapter : INotificationDeliveryAdapter
|
||||
{
|
||||
private readonly System.Collections.Concurrent.ConcurrentBag<INotificationDeliveryAdapter> _seen;
|
||||
|
||||
public RecordingStubAdapter(
|
||||
System.Collections.Concurrent.ConcurrentBag<INotificationDeliveryAdapter> seen) => _seen = seen;
|
||||
|
||||
public NotificationType Type => NotificationType.Email;
|
||||
|
||||
public Task<DeliveryOutcome> DeliverAsync(
|
||||
Notification notification, CancellationToken cancellationToken = default)
|
||||
{
|
||||
_seen.Add(this);
|
||||
return Task.FromResult(DeliveryOutcome.Success("ops@example.com"));
|
||||
}
|
||||
}
|
||||
|
||||
[Fact]
|
||||
|
||||
@@ -502,6 +502,269 @@ public sealed class BundleImporterLoadTests
|
||||
() => rig.Importer.LoadAsync(stream, passphrase: null));
|
||||
}
|
||||
|
||||
// ─────────────────────────────────────────────────────────────────────
|
||||
// Register row 44 — the per-IP-per-hour unlock throttle is actually enforced.
|
||||
// BundleUnlockRateLimiter was registered, injected and unit-tested but never
|
||||
// invoked, so these all passed-by-doing-nothing before the wiring.
|
||||
// ─────────────────────────────────────────────────────────────────────
|
||||
|
||||
[Fact]
|
||||
public async Task LoadAsync_rejects_unlock_attempts_past_the_per_ip_hourly_cap()
|
||||
{
|
||||
// Per-bundle strikes are effectively disabled here (100) so the throttle is
|
||||
// unambiguously what stops the run.
|
||||
var rig = BuildRig(opts =>
|
||||
{
|
||||
opts.MaxUnlockAttemptsPerIpPerHour = 3;
|
||||
opts.MaxUnlockAttemptsPerSession = 100;
|
||||
});
|
||||
using var packed = PackEncryptedBundle(
|
||||
rig.Serializer, rig.ManifestBuilder, rig.Encryptor, EmptyContent(), "correct");
|
||||
var bundleBytes = ((MemoryStream)packed).ToArray();
|
||||
|
||||
for (var attempt = 1; attempt <= 3; attempt++)
|
||||
{
|
||||
await Assert.ThrowsAnyAsync<CryptographicException>(
|
||||
() => rig.Importer.LoadAsync(
|
||||
new MemoryStream(bundleBytes), passphrase: "wrong", clientKey: "10.1.1.5"));
|
||||
}
|
||||
|
||||
var throttled = await Assert.ThrowsAsync<BundleUnlockThrottledException>(
|
||||
() => rig.Importer.LoadAsync(
|
||||
new MemoryStream(bundleBytes), passphrase: "wrong", clientKey: "10.1.1.5"));
|
||||
Assert.Equal(3, throttled.MaxAttemptsPerWindow);
|
||||
Assert.Equal("10.1.1.5", throttled.ClientKey);
|
||||
|
||||
// Fail closed: even the CORRECT passphrase is refused while the client is
|
||||
// over budget.
|
||||
await Assert.ThrowsAsync<BundleUnlockThrottledException>(
|
||||
() => rig.Importer.LoadAsync(
|
||||
new MemoryStream(bundleBytes), passphrase: "correct", clientKey: "10.1.1.5"));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task Throttled_unlock_is_not_a_passphrase_failure_and_burns_no_strike()
|
||||
{
|
||||
// A throttled attempt never reaches the decryptor, so it must not consume one
|
||||
// of the bundle's three strikes — otherwise the per-IP throttle would become a
|
||||
// way to lock a bundle that an operator is legitimately importing.
|
||||
var rig = BuildRig(opts =>
|
||||
{
|
||||
opts.MaxUnlockAttemptsPerIpPerHour = 1;
|
||||
opts.MaxUnlockAttemptsPerSession = 3;
|
||||
});
|
||||
using var packed = PackEncryptedBundle(
|
||||
rig.Serializer, rig.ManifestBuilder, rig.Encryptor, EmptyContent(), "correct");
|
||||
var bundleBytes = ((MemoryStream)packed).ToArray();
|
||||
var contentHash = rig.Serializer.ReadManifest(new MemoryStream(bundleBytes)).ContentHash;
|
||||
|
||||
await Assert.ThrowsAnyAsync<CryptographicException>(
|
||||
() => rig.Importer.LoadAsync(
|
||||
new MemoryStream(bundleBytes), passphrase: "wrong", clientKey: "10.1.1.6"));
|
||||
Assert.Equal(1, rig.SessionStore.GetUnlockFailureCount(contentHash));
|
||||
|
||||
await Assert.ThrowsAsync<BundleUnlockThrottledException>(
|
||||
() => rig.Importer.LoadAsync(
|
||||
new MemoryStream(bundleBytes), passphrase: "wrong", clientKey: "10.1.1.6"));
|
||||
|
||||
// Still 1 — the denial burned no strike, so the bundle is not locked.
|
||||
Assert.Equal(1, rig.SessionStore.GetUnlockFailureCount(contentHash));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task Unlock_throttle_is_isolated_per_client()
|
||||
{
|
||||
// One noisy client must not spend another operator's budget.
|
||||
var rig = BuildRig(opts =>
|
||||
{
|
||||
opts.MaxUnlockAttemptsPerIpPerHour = 1;
|
||||
opts.MaxUnlockAttemptsPerSession = 100;
|
||||
});
|
||||
using var packed = PackEncryptedBundle(
|
||||
rig.Serializer, rig.ManifestBuilder, rig.Encryptor, EmptyContent(), "correct");
|
||||
var bundleBytes = ((MemoryStream)packed).ToArray();
|
||||
|
||||
await Assert.ThrowsAnyAsync<CryptographicException>(
|
||||
() => rig.Importer.LoadAsync(
|
||||
new MemoryStream(bundleBytes), passphrase: "wrong", clientKey: "10.1.1.7"));
|
||||
await Assert.ThrowsAsync<BundleUnlockThrottledException>(
|
||||
() => rig.Importer.LoadAsync(
|
||||
new MemoryStream(bundleBytes), passphrase: "wrong", clientKey: "10.1.1.7"));
|
||||
|
||||
// A different client still has its full budget — reaches the decryptor and
|
||||
// fails on the passphrase, not on the throttle.
|
||||
await Assert.ThrowsAnyAsync<CryptographicException>(
|
||||
() => rig.Importer.LoadAsync(
|
||||
new MemoryStream(bundleBytes), passphrase: "wrong", clientKey: "10.1.1.8"));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task Unattributed_callers_share_one_throttle_bucket_rather_than_bypassing_it()
|
||||
{
|
||||
// Conservative fallback: a caller with no IP is counted against a single
|
||||
// shared bucket, so "unknown client" is a stricter budget, never none.
|
||||
var rig = BuildRig(opts =>
|
||||
{
|
||||
opts.MaxUnlockAttemptsPerIpPerHour = 1;
|
||||
opts.MaxUnlockAttemptsPerSession = 100;
|
||||
});
|
||||
using var packed = PackEncryptedBundle(
|
||||
rig.Serializer, rig.ManifestBuilder, rig.Encryptor, EmptyContent(), "correct");
|
||||
var bundleBytes = ((MemoryStream)packed).ToArray();
|
||||
|
||||
await Assert.ThrowsAnyAsync<CryptographicException>(
|
||||
() => rig.Importer.LoadAsync(new MemoryStream(bundleBytes), passphrase: "wrong"));
|
||||
|
||||
// Blank is treated the same as null — both land in the shared bucket, which
|
||||
// the first attempt already emptied.
|
||||
var throttled = await Assert.ThrowsAsync<BundleUnlockThrottledException>(
|
||||
() => rig.Importer.LoadAsync(
|
||||
new MemoryStream(bundleBytes), passphrase: "wrong", clientKey: " "));
|
||||
Assert.Equal(BundleImporter.UnattributedClientKey, throttled.ClientKey);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task Unencrypted_bundles_do_not_consume_the_unlock_budget()
|
||||
{
|
||||
// The throttle guards passphrase brute-forcing; a plain bundle has no unlock
|
||||
// step, so repeated loads must not be able to exhaust an operator's budget.
|
||||
var rig = BuildRig(opts => opts.MaxUnlockAttemptsPerIpPerHour = 1);
|
||||
using var packed = PackPlainBundle(rig.Serializer, rig.ManifestBuilder, SmallContent());
|
||||
var bundleBytes = ((MemoryStream)packed).ToArray();
|
||||
|
||||
for (var i = 0; i < 3; i++)
|
||||
{
|
||||
var session = await rig.Importer.LoadAsync(
|
||||
new MemoryStream(bundleBytes), passphrase: null, clientKey: "10.1.1.9");
|
||||
Assert.NotNull(session);
|
||||
rig.Importer.Abandon(session.SessionId);
|
||||
}
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task An_already_locked_bundle_does_not_spend_the_clients_hourly_budget()
|
||||
{
|
||||
// Ordering pin: the per-bundle lockout is checked BEFORE the throttle, so
|
||||
// hammering a locked bundle cannot burn through the client's hourly budget
|
||||
// and spill over into denying its next legitimate import.
|
||||
var rig = BuildRig(opts =>
|
||||
{
|
||||
opts.MaxUnlockAttemptsPerSession = 1;
|
||||
opts.MaxUnlockAttemptsPerIpPerHour = 2;
|
||||
});
|
||||
using var packed = PackEncryptedBundle(
|
||||
rig.Serializer, rig.ManifestBuilder, rig.Encryptor, EmptyContent(), "correct");
|
||||
var bundleBytes = ((MemoryStream)packed).ToArray();
|
||||
|
||||
// One wrong attempt locks the bundle (cap 1) and spends 1 of 2 IP budget.
|
||||
await Assert.ThrowsAsync<BundleLockedException>(
|
||||
() => rig.Importer.LoadAsync(
|
||||
new MemoryStream(bundleBytes), passphrase: "wrong", clientKey: "10.1.1.10"));
|
||||
|
||||
// Three more attempts on the locked bundle stay BundleLocked — none of them
|
||||
// register against the rate limiter.
|
||||
for (var i = 0; i < 3; i++)
|
||||
{
|
||||
await Assert.ThrowsAsync<BundleLockedException>(
|
||||
() => rig.Importer.LoadAsync(
|
||||
new MemoryStream(bundleBytes), passphrase: "wrong", clientKey: "10.1.1.10"));
|
||||
}
|
||||
|
||||
// A DIFFERENT bundle from the same client still has budget left.
|
||||
using var other = PackEncryptedBundle(
|
||||
rig.Serializer, rig.ManifestBuilder, rig.Encryptor, SmallContent(), "correct2");
|
||||
var otherSession = await rig.Importer.LoadAsync(
|
||||
new MemoryStream(((MemoryStream)other).ToArray()),
|
||||
passphrase: "correct2", clientKey: "10.1.1.10");
|
||||
Assert.NotNull(otherSession);
|
||||
}
|
||||
|
||||
// ─────────────────────────────────────────────────────────────────────
|
||||
// Register row 43 — abandoned-session release.
|
||||
// ─────────────────────────────────────────────────────────────────────
|
||||
|
||||
[Fact]
|
||||
public async Task Abandon_releases_the_session_slot_and_zeroes_its_decrypted_content()
|
||||
{
|
||||
// Row 43: only ApplyAsync used to release a session, so any caller that
|
||||
// loaded a bundle and walked away held one of MaxConcurrentImportSessions
|
||||
// (plus the decrypted plaintext) for the full TTL.
|
||||
var rig = BuildRig();
|
||||
using var packed = PackEncryptedBundle(
|
||||
rig.Serializer, rig.ManifestBuilder, rig.Encryptor, SmallContent(), "correct");
|
||||
var session = await rig.Importer.LoadAsync(
|
||||
new MemoryStream(((MemoryStream)packed).ToArray()), passphrase: "correct");
|
||||
|
||||
var decrypted = session.DecryptedContent;
|
||||
Assert.NotEmpty(decrypted);
|
||||
Assert.Contains(decrypted, b => b != 0);
|
||||
Assert.NotNull(rig.SessionStore.Get(session.SessionId));
|
||||
|
||||
rig.Importer.Abandon(session.SessionId);
|
||||
|
||||
Assert.Null(rig.SessionStore.Get(session.SessionId));
|
||||
// Zeroed in place, so even a caller still holding the reference (the Razor
|
||||
// page keeps one for the Result step) can no longer read the plaintext.
|
||||
Assert.All(decrypted, b => Assert.Equal(0, b));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task Abandon_is_idempotent_and_tolerates_unknown_ids()
|
||||
{
|
||||
// The Razor page calls Abandon from Dispose even after a successful Apply
|
||||
// already released the same id; a double release must be a silent no-op.
|
||||
var rig = BuildRig();
|
||||
using var packed = PackPlainBundle(rig.Serializer, rig.ManifestBuilder, SmallContent());
|
||||
var session = await rig.Importer.LoadAsync(packed, passphrase: null);
|
||||
|
||||
rig.Importer.Abandon(session.SessionId);
|
||||
rig.Importer.Abandon(session.SessionId);
|
||||
rig.Importer.Abandon(Guid.NewGuid());
|
||||
|
||||
Assert.Null(rig.SessionStore.Get(session.SessionId));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task Abandon_does_not_release_another_callers_session()
|
||||
{
|
||||
// Sessions are keyed by a random GUID handed only to their opener, so one
|
||||
// operator abandoning their wizard must not evict a concurrent import.
|
||||
var rig = BuildRig();
|
||||
using var packedA = PackPlainBundle(rig.Serializer, rig.ManifestBuilder, SmallContent());
|
||||
using var packedB = PackPlainBundle(rig.Serializer, rig.ManifestBuilder, EmptyContent());
|
||||
var a = await rig.Importer.LoadAsync(packedA, passphrase: null);
|
||||
var b = await rig.Importer.LoadAsync(packedB, passphrase: null);
|
||||
|
||||
rig.Importer.Abandon(a.SessionId);
|
||||
|
||||
Assert.Null(rig.SessionStore.Get(a.SessionId));
|
||||
var survivor = rig.SessionStore.Get(b.SessionId);
|
||||
Assert.NotNull(survivor);
|
||||
Assert.NotEmpty(survivor!.DecryptedContent);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task Abandon_frees_a_concurrent_import_session_slot()
|
||||
{
|
||||
// The availability half of row 43: abandoned sessions used to exhaust the
|
||||
// cap and lock out every import on the node until the TTL expired.
|
||||
var rig = BuildRig(opts => opts.MaxConcurrentImportSessions = 2);
|
||||
using var packed = PackPlainBundle(rig.Serializer, rig.ManifestBuilder, SmallContent());
|
||||
var bundleBytes = ((MemoryStream)packed).ToArray();
|
||||
|
||||
var first = await rig.Importer.LoadAsync(new MemoryStream(bundleBytes), passphrase: null);
|
||||
await rig.Importer.LoadAsync(new MemoryStream(bundleBytes), passphrase: null);
|
||||
|
||||
var full = await Assert.ThrowsAsync<InvalidOperationException>(
|
||||
() => rig.Importer.LoadAsync(new MemoryStream(bundleBytes), passphrase: null));
|
||||
Assert.Contains("Too many concurrent import sessions", full.Message);
|
||||
|
||||
rig.Importer.Abandon(first.SessionId);
|
||||
|
||||
var third = await rig.Importer.LoadAsync(new MemoryStream(bundleBytes), passphrase: null);
|
||||
Assert.NotNull(rig.SessionStore.Get(third.SessionId));
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Builds a zip directly so the test can write a manifest whose
|
||||
/// <c>ContentHash</c> or <c>BundleFormatVersion</c> intentionally
|
||||
|
||||
Reference in New Issue
Block a user