diff --git a/docs/plans/2026-07-08-deferred-work-register.md b/docs/plans/2026-07-08-deferred-work-register.md index aaa0f5d7..1234e066 100644 --- a/docs/plans/2026-07-08-deferred-work-register.md +++ b/docs/plans/2026-07-08-deferred-work-register.md @@ -38,10 +38,12 @@ All 7 fix-now items landed via PLAN-04/05/06/07/08 (verified in review 08 round | 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 | +| 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 | ## Resolved (verified against the code 2026-07-10) Rows removed from the Deferred table above once confirmed shipped. Kept here for traceability.