215 lines
26 KiB
Markdown
215 lines
26 KiB
Markdown
# MxAccessGateway — Remediation Tracking (2026-10-21 system review)
|
|
|
|
Source review: [`../00-overall.md`](../00-overall.md) (summary of the six review dimensions). Generated 2026-10-22 from the review outcomes and the integration branch `fix/review-2026-10-21`.
|
|
Tracks the findings of the 2026-10-21 system review and the cycle-1 findings that no later tracker had re-listed. The earlier trackers remain the record for their own IDs: [cycle 1](../../remediation/00-tracking.md), [cycle 2](../../2026-07-12/remediation/00-tracking.md), [cycle 3](../../2026-08-19/remediation/00-tracking.md).
|
|
|
|
## How to use this document
|
|
|
|
- IDs in this cycle use the prefix `R21-` so they cannot collide with the `GWC`/`WRK`/`IPC`/`SEC`/`CLI`/`TST` registers of earlier cycles. `R21-S` is sessions, `R21-W` is the worker client, `R21-G` is the gRPC surface, `R21-D` is the dashboard, `R21-K` is the Windows worker, `R21-C*` are the client libraries, `R21-L` is docs and test gaps, `R21-X` is external, and `R21-O` is a cycle-1 finding re-triaged as still open.
|
|
- Where a finding is the same as an earlier-cycle ID, the description names it (for example `R21-G11` is `SEC-14`).
|
|
- The review's own severity table is not in the repository. Only `R21-S1` carries a severity from the review (High); every other severity here is the triage estimate from the finding's impact and is open to correction.
|
|
- The **Status** column is the single source of truth. Update it in the same change that lands the fix, and cite the finding ID in commit messages.
|
|
|
|
**Status legend:** `Done` · `Done (merge 2013573)` · `Done (merge 2fad124)` · `Deferred` · `Won't fix` · `Open` · `Blocked (external)`.
|
|
|
|
## Execution model
|
|
|
|
The review was remediated by ten parallel streams on branches `fix/review-2026-10-21-{a..j}`, merged into the integration branch `fix/review-2026-10-21`, plus stream K (this tracker triage) and stream L (docs and test gaps).
|
|
|
|
| Stream | Branch | Scope | Status |
|
|
|---|---|---|---|
|
|
| A | `fix/review-2026-10-21-a` | Sessions (`R21-S*`) | Done, merged (`7f0720f`) |
|
|
| B | `fix/review-2026-10-21-b` | WorkerClient (`R21-W1`) | Done, merged (`81d4cd8`) |
|
|
| C | `fix/review-2026-10-21-c` | gRPC surface (`R21-G*`) | Done, merged (`3f9448f`) |
|
|
| D | `fix/review-2026-10-21-d` | Dashboard (`R21-D*`) | Done, merged (`8212b0e`) |
|
|
| E | `fix/review-2026-10-21-e` | Worker (`R21-K*`) | Done (merge 2013573) — `7618caf`, follow-up `38fbbde` |
|
|
| F | `fix/review-2026-10-21-f` | .NET client | Done, merged (`3a1ee28`) |
|
|
| G | `fix/review-2026-10-21-g` | Java client | Done, merged (`c18566b`) |
|
|
| H | `fix/review-2026-10-21-h` | Go client | Done, merged (`7636573`) |
|
|
| I | `fix/review-2026-10-21-i` | Rust client | Done, merged (`b2a7fc5`) |
|
|
| J | `fix/review-2026-10-21-j` | Python client | Done, merged (`29d4e51`) |
|
|
| K | `fix/review-2026-10-21-k` | Tracker triage (this document) | Done |
|
|
| L | (stream L branch) | Proto comments, docs, test gaps | Done (merge 2fad124) |
|
|
|
|
Worker changes (stream E) need the windev x86 build and `Worker.Tests` run after merge; stream E's own live facts passed on windev.
|
|
|
|
## Sessions (stream A)
|
|
|
|
| ID | Sev | Area | Description | Status | Commit / stream | Reason / note |
|
|
|---|---|---|---|---|---|---|
|
|
| R21-S1 | High | Sessions | A kill or shutdown during worker startup orphaned the worker | Done | A: `87f23d9`, `ec9f264` | Gated attach, discard on any exception, `OpenRecorded`; a late worker is never orphaned |
|
|
| R21-S2 | Medium | Sessions | Sweep-driven closes skipped `SessionClosed()`, so the open-sessions gauge drifted | Done | A: `09666cc` | |
|
|
| R21-S3 | Medium | Sessions | `OpenSession` was still accepted while the host was stopping | Done | A: `37ab629` | `GatewayStopping` returns `Unavailable`; `ShutdownAsync` re-snapshots until the registry is empty |
|
|
| R21-S4 | Low | Sessions | A shutdown timeout whose kill succeeded was reported as `CloseFailed` | Done | A: `2b165cd` | Now `Closed`, with `WorkerShutdownFailure` recorded |
|
|
| R21-S5 | Low | Sessions | A failed open left a metrics entry behind | Done | A: `87f23d9` | Open rollback goes through the removal path |
|
|
| R21-S6 | Medium | Sessions | Sessions that never stream pinned a worker for the whole lease | Done | A: `285ef64`, `ec9f264` | Config-gated `UnattachedSessionGraceSeconds` (default 0 = off); `ExtendLease` deleted; internal subscribers count as attached; idle clock starts at Ready |
|
|
| R21-S7 | Low | Sessions | A kill racing a close double-decremented the gauge | Done | A: `ec9f264` | Found in review of stream A |
|
|
| R21-S8 | Low | Sessions | A sweep that lost the race to a close was reported as a failure | Done | A: `ec9f264` | Found in review of stream A |
|
|
| R21-S9 | Medium | Sessions | After `ShutdownWorker` the session stayed `Ready` | Done | A: `cc1e895` | The session faults and the sweep reaps it. Found live |
|
|
| R21-S10 | Low | Sessions | The replay ring has a count cap but no byte cap | Open | — | Documented as bounded by count. Next step: add a byte cap if large-array events make the count cap insufficient |
|
|
|
|
## WorkerClient (stream B)
|
|
|
|
| ID | Sev | Area | Description | Status | Commit / stream | Reason / note |
|
|
|---|---|---|---|---|---|---|
|
|
| R21-W1 | Medium | WorkerClient | `ShutdownAsync` could block past `ShutdownTimeout` | Done | B: `b0100e7` | The timeout CTS now covers the envelope enqueue |
|
|
|
|
## gRPC surface (stream C)
|
|
|
|
| ID | Sev | Area | Description | Status | Commit / stream | Reason / note |
|
|
|---|---|---|---|---|---|---|
|
|
| R21-G1 | Medium | gRPC | Denied bulk commands flooded the audit channel | Done | C: `ca0cf19` | One aggregated event per bulk call |
|
|
| R21-G2 | Medium | gRPC | The authorization interceptor did not cover client-streaming or duplex calls (cycle-1 `SEC-17`) | Done | C: `19cc74f` | Both fail closed to the admin scope |
|
|
| R21-G3 | Low | gRPC | `StreamAlarms` overflow returned `Unavailable` | Done | C: `0b4a168` | Now `ResourceExhausted` |
|
|
| R21-G4 | Low | gRPC | `StreamAlarms` cloned the snapshot under the lock on every attach | Done | C: `0b4a168` | Memoized projection |
|
|
| R21-G5 | Low | gRPC | `WriteAsync` was not given the call's cancellation token | Done | C: `0b4a168` | |
|
|
| R21-G6 | Low | gRPC | `correlation_id` handling was inconsistent across replies | Done | C: `0b4a168`, `82a856b` | `client_correlation_id` is echoed; a Debug log records the client/envelope pair |
|
|
| R21-G7 | Low | gRPC | The `StreamDisconnected` reason was a constant (cycle-1 `GWC-22`) | Done | C: `6ae5ed9` | |
|
|
| R21-G8 | Low | Security | `CachingApiKeyVerifier` index pruning and single-flight misses | Done | C: `ed3a2c9` | |
|
|
| R21-G9 | Low | gRPC | `ConstraintEnforcer` allocated a `Task` per item | Done | C: `ca0cf19` | `ValueTask` |
|
|
| R21-G10 | Low | Metrics | `Enum.ToString` was called per event | Done | C: `68d6b5f` | `MxEventFamilyNames` cache |
|
|
| R21-G11 | Medium | Security | `/metrics` was unauthenticated (cycle-1 `SEC-14`) | Done | C: `65bca80`, `82a856b` | Opt-in `Metrics:RequireAuthentication` (admin key, or dashboard Admin/Viewer cookie); `DisableLogin` does not bypass it; `/health` stays open by design |
|
|
| R21-G12 | Low | Metrics | The `/metrics` per-peer limiter is keyed by IP while the gRPC limiter is keyed by connection | Won't fix | — | Documented difference; the two surfaces have different peer identities |
|
|
| R21-G13 | Low | Security | The verifier generation guard depends on key-id case | Open | — | Latent only. Next step: normalize the key id (ordinal-ignore-case) at the guard and add a test |
|
|
|
|
## Dashboard (stream D)
|
|
|
|
| ID | Sev | Area | Description | Status | Commit / stream | Reason / note |
|
|
|---|---|---|---|---|---|---|
|
|
| R21-D1 | Medium | Dashboard | Unbounded events-hub sends | Done | D: `f1db002`, `bd7abf4` | One drop-oldest pump per session; a lock re-check race was fixed in review |
|
|
| R21-D2 | Medium | Dashboard | One slow client stalled the alarms hub | Done | D: `3688fa1` | 2 s send timeout plus an `AlarmFeedReset` marker. Tradeoff documented: an abandoned send is not followed by a reset |
|
|
| R21-D3 | Low | Dashboard | Browse search ran on every keystroke | Done | D: `db78336` | Debounce, off-dispatcher, indexed |
|
|
| R21-D4 | Medium | Dashboard | The full snapshot plus Configuration went out every second | Done | D: `6733519` | Configuration is sent once per connection; sends have a timeout |
|
|
| R21-D5 | Low | Dashboard | `AlarmsPage` re-sorted twice per render | Done | D: `2a7d6fa` | `DashboardAlarmView` memo |
|
|
| R21-D6 | Low | Dashboard | `BrowsePage` disposal was unbounded | Done | D: `db78336` | |
|
|
| R21-D7 | Low | Dashboard | Two viewers thrashed the advise LRU | Done | D: `5d4aefc` | Per-viewer share |
|
|
| R21-D8 | Low | Dashboard | Disconnected circuits kept polling | Done | D: `db78336` | 30 s retention plus a connection tracker |
|
|
| R21-D9 | Low | Dashboard | `GatewayStatus` was hardcoded (cycle-1 `SEC-27`) | Done | D: `6733519` | Derived from health checks |
|
|
| R21-D10 | Low | Docs | `docs/GatewayDashboardDesign.md` described counters that do not exist | Done | D: `2c963b7` | Doc rewritten to match the shipped pages |
|
|
| R21-D11 | Low | Dashboard | `BrowsePage` search CTS cancel/dispose race | Open | — | Only a caught `ObjectDisposedException`. Next step: swap-and-dispose under a lock when the page is next touched |
|
|
| R21-D12 | Low | Dashboard | Cached health status is stale while the publisher is idle | Open | — | Next step: refresh on first subscribe |
|
|
| R21-D13 | Low | Dashboard | Hub pumps are not awaited on broadcaster dispose | Open | — | Next step: await the pumps with a bounded timeout in `DisposeAsync` |
|
|
|
|
## Worker (stream E)
|
|
|
|
All rows are on `fix/review-2026-10-21-e` (`7618caf`, follow-up `38fbbde`), not yet merged. After merge, run `windev-worker-ci.ps1 -Mode test` and update these rows to plain `Done`.
|
|
|
|
| ID | Sev | Area | Description | Status | Commit / stream | Reason / note |
|
|
|---|---|---|---|---|---|---|
|
|
| R21-K1 | Medium | Worker | The write-completion cache and value cache were never evicted | Done (merge 2013573) | E: `7618caf`, `38fbbde` | Ghost-entry eviction on `AddItem` added in the follow-up |
|
|
| R21-K2 | Medium | Worker | `ReadBulk` was serial (N x timeout) | Done (merge 2013573) | E: `7618caf`, `38fbbde` | Shared deadline; dedupe and periodic message pumping in the follow-up |
|
|
| R21-K3 | Medium | Worker | One alarm-poll failure faulted the session (cycle-1 `WRK-05`) | Done (merge 2013573) | E: `7618caf` | Consecutive-failure threshold of 3 |
|
|
| R21-K4 | Low | Worker | The primary alarm consumer leaked when the standby build failed | Done (merge 2013573) | E: `7618caf` | |
|
|
| R21-K5 | Low | Worker | `StaRuntime` drained the whole queue before pumping messages | Done (merge 2013573) | E: `7618caf` | |
|
|
| R21-K6 | Medium | Worker | STA thread death after start was silent (cycle-1 `WRK-02`) | Done (merge 2013573) | E: `7618caf` | `LoopFault` becomes a `StaHung` WorkerFault |
|
|
| R21-K7 | Low | Worker | No unhandled-exception hook (cycle-1 `WRK-09`) | Done (merge 2013573) | E: `7618caf` | Exit code 7 |
|
|
| R21-K8 | Low | Worker | `RETRIGGER` handling was undocumented | Done (merge 2fad124) | stream L | Documented as reserved, never synthesized; the proto comment is stream L's |
|
|
| R21-K9 | Medium | Tests | Live control-command facts for the worker (cycle-1 `TST-05` residual) | Done (merge 2013573) | E: `069972a`, `4ea3672` | Facts added and passing on windev |
|
|
| R21-K10 | Low | Worker | Unregister-first shutdown cleanup | Deferred | — | Needs a parity probe against the installed MXAccess before it can change cleanup order |
|
|
| R21-K11 | Medium | Security | A per-operator `AuthenticateUser` cache | Deferred | — | Security tradeoff (caching credentials' outcome); decide with the security owner before building |
|
|
| R21-K12 | Low | Worker | Batch task-completion-source allocation rework | Deferred | — | Low value |
|
|
| R21-K13 | Low | Worker | The alarm poll builds a full-BSTR `XmlDocument` every 500 ms | Deferred | — | Steady GC cost, not a correctness issue; revisit if profiling flags it |
|
|
|
|
## Clients (streams F to J)
|
|
|
|
| ID | Sev | Area | Description | Status | Commit / stream | Reason / note |
|
|
|---|---|---|---|---|---|---|
|
|
| R21-CN1 | Medium | .NET client | Acknowledge retried on transient failure | Done | F: `6f0b408` | No ack retry |
|
|
| R21-CN2 | Medium | .NET client | Client always owned its channel | Done | F: `6f0b408` | Caller-owned channel constructors |
|
|
| R21-CN3 | Medium | .NET client | `DisposeAsync` could throw or block (cycle-1 `CLI-06`) | Done | F: `6f0b408` | Safe, best-effort dispose |
|
|
| R21-CN4 | Low | .NET client | No typed Ping/GetSessionState/GetWorkerInfo/DrainEvents | Done | F: `6f0b408` | |
|
|
| R21-CN5 | Medium | .NET client | A session could not be re-attached by id (cycle-1 `CLI-05`) | Done | F: `6f0b408` | `AttachSession` |
|
|
| R21-CJ1 | Medium | Java client | Event queue was fixed at 16 | Done | G: `48c9323` | Configurable, default 1024 |
|
|
| R21-CJ2 | Medium | Java client | Overflow dropped buffered events | Done | G: `48c9323` | Overflow keeps buffered events |
|
|
| R21-CJ3 | Low | Java client | Interrupt did not cancel the call | Done | G: `48c9323` | |
|
|
| R21-CJ4 | Low | Java client | No typed diagnostics | Done | G: `48c9323` | |
|
|
| R21-CG1 | Medium | Go client | Event buffer fixed at 16 | Done | H: `a3a7bd2` | `EventBufferSize` option, default 1024 |
|
|
| R21-CG2 | Medium | Go client | Deprecated `DialContext` + `WithBlock` (cycle-1 `CLI-10`) | Done | H: `a3a7bd2` | `grpc.NewClient` |
|
|
| R21-CG3 | Low | Go client | No typed diagnostics | Done | H: `a3a7bd2` | |
|
|
| R21-CG4 | Low | Go client | `go vet` copylocks error in `types_alias_test.go` | Open | — | Pre-existing, not fixed by stream H. Next step: pass the type by pointer in the test; check `go vet` runs in CI |
|
|
| R21-CR1 | Medium | Rust client | Blocking file read on an async path (CA file) | Done | I: `0345726` | Async read |
|
|
| R21-CR2 | Low | Rust client | No typed diagnostics | Done | I: `0345726` | |
|
|
| R21-CP1 | Low | Python client | No typed diagnostics | Done | J: `99347d3`, `d716dec` | The client already had no ack retry, no event buffer, and cancels on exit |
|
|
|
|
## Docs and test gaps (stream L)
|
|
|
|
| ID | Sev | Area | Description | Status | Commit / stream | Reason / note |
|
|
|---|---|---|---|---|---|---|
|
|
| R21-L1 | Low | Docs | `operator_full_name` is silently dropped; proto comment and docs do not say so | Done (merge 2fad124) | stream L | |
|
|
| R21-L2 | Low | Contracts | `RETRIGGER` and the `WorkerFaultCategory` values `INVALID_ARGUMENTS` and `GATEWAY_AUTHENTICATION_FAILED` are reserved but not labeled so | Done (merge 2fad124) | stream L | Proto comments |
|
|
| R21-L3 | Low | Docs | Stale notes in `stillpending.md` (sections 1.4, 3.4, 7.1) and the deferred-followups header | Done (merge 2fad124) | stream L | |
|
|
| R21-L4 | Low | Docs | The 2026-08-19 tracker header says CLI-47..50, IPC-36/37, TST-35 are `Not started` while the rows say `Done` | Done | K | Header row corrected in this stream |
|
|
| R21-L5 | Low | Tests | No tests for `GatewayConfigPathRules` | Done (merge 2fad124) | stream L | |
|
|
| R21-L6 | Low | Tests | No tests for `DashboardRedactor` | Done (merge 2fad124) | stream L | |
|
|
| R21-L7 | Low | Tests | No tests for `EventSubscriberReplayAttachment` | Done (merge 2fad124) | stream L | |
|
|
| R21-L8 | Low | Tests | No tests for the `MxStatusDetailText` table | Deferred | — | Worker-side test; needs a windev run, not done in stream L |
|
|
|
|
## External / blocked
|
|
|
|
| ID | Sev | Area | Description | Status | Commit / stream | Reason / note |
|
|
|---|---|---|---|---|---|---|
|
|
| R21-X1 | Medium | Security | TLS on the shared GLAuth (cycle-3 `SEC-37`) | Blocked (external) | — | Lives in `scadaproj/infra/glauth/` (separate repo and docker host); after it lands, move the deployed service to `Production` |
|
|
| R21-X2 | Low | CI | Gitea runner registration-token reset (cycle-3 `TST-32`) | Blocked (external) | — | Web-UI action on the Gitea admin page |
|
|
| R21-X3 | Medium | OtOpcUa | `GalaxyDriver.ShutdownAsync` leaks sessions | Blocked (external) | — | Defect is in the OtOpcUa repo; the gateway's lease expiry and detach grace eventually reap a leaked session |
|
|
|
|
## Cycle-1 rows re-triaged as still open (promoted)
|
|
|
|
These are cycle-1 findings that no later tracker re-listed. Each was checked against the integration branch, and the cycle-1 row now points here. The `Medium` rows carry correctness, security or operability weight (`R21-O1`, `R21-O8`, `R21-O9`, `R21-O16`, `R21-O21` to `R21-O23`, `R21-O30`, `R21-O31`); the `Low` rows are hygiene to batch with the next change that touches the file.
|
|
|
|
| ID | Cycle-1 ID | Sev | Area | Description | Status | Next step |
|
|
|---|---|---|---|---|---|---|
|
|
| R21-O1 | WRK-03 | Medium | Worker | `TryStartCommandTask` returns silently when `_acceptingCommands` is false, so a command racing shutdown gets no reply and no log (`Ipc/WorkerPipeSession.cs:1240`) | Open | Reply `WorkerUnavailable` (at minimum log `WorkerCommandResultDropped`) and add `CommandAfterShutdown_RepliesWorkerUnavailable`; verify on windev |
|
|
| R21-O2 | WRK-08 | Low | Worker | Residual events queued after `WorkerShutdown` are discarded, and the shutdown section of `docs/MxAccessWorkerInstanceDesign.md` does not say so | Open | Document the discard as v1 behavior (docs-only) |
|
|
| R21-O3 | WRK-10 | Low | Worker | `WorkerApplication.Run` catch blocks log the exception type but never the message | Open | Add `exception_message` through `WorkerLogRedactor` in the three catch blocks; add a `WorkerApplicationTests` assertion |
|
|
| R21-O4 | WRK-16 | Low | Worker | Seven near-identical `CreateEnvelope` overload pairs and eight `WorkerPipeClient` constructors | Open | Refactor when `WorkerPipeSession` is next touched; existing IPC tests are the guard |
|
|
| R21-O5 | WRK-17 | Low | Worker | Gateway death surfaces as `EndOfStream` and exits 6 (`ProtocolViolation`) instead of 5 | Open | Map `EndOfStream` to `PipeConnectionFailed`; add a `WorkerApplicationTests` case |
|
|
| R21-O6 | WRK-18 | Low | Worker | Event-queue overflow exits as generic `UnexpectedFailure` (1) | Open | Add a dedicated exit code (stream E already uses 7 for unhandled exceptions, so use 8) and update the overflow paragraph of `docs/MxAccessWorkerInstanceDesign.md` |
|
|
| R21-O7 | WRK-19 | Low | Worker | The documented per-command start/end log with correlation id and latency does not exist | Open | Add level-gated `WorkerCommandStarted`/`WorkerCommandCompleted` in `StaCommandDispatcher.ExecuteQueuedCommandAsync` |
|
|
| R21-O8 | GWC-05 | Medium | Gateway | Worker pipe is created with no ACL and no `CurrentUserOnly` (`SessionWorkerClientFactory.CreatePipe`); only the unguessable name and `maxNumberOfServerInstances: 1` limit access | Open | Add `PipeOptions.CurrentUserOnly` (or a `PipeSecurity` for the worker account); confirm the worker runs as the same user on windev; add a launch test |
|
|
| R21-O9 | GWC-09 | Medium | Gateway | `WorkerProcessStartedProbe` only checks `HasExited`, so `StartupProbeRetryAttempts` and `StartupProbeRetryDelayMilliseconds` can never retry anything | Open | Either make the probe wait for a real readiness signal, or remove the two options and their docs so config does not promise a retry that cannot happen |
|
|
| R21-O10 | GWC-13 | Low | Gateway | Worker-ready wait is a 25 ms poll loop (`GatewaySession`, `pollIntervalMs = 25`) | Open | Replace with a `TaskCompletionSource` signalled on state change |
|
|
| R21-O11 | GWC-17 | Low | Gateway | `SparseArrayExpander` throws `Grpc.Core.RpcException` from the Sessions layer | Open | Throw a domain exception and map it to `InvalidArgument` in the gRPC layer |
|
|
| R21-O12 | GWC-18 | Low | Gateway | `GatewaySession` exposes `DisposeAsync` without implementing `IAsyncDisposable` | Open | Declare the interface; check `await using` call sites |
|
|
| R21-O13 | GWC-19 | Low | Gateway | `GatewaySession.KillWorker(string)` has no production callers (only test comments mention it) | Open | Delete it and fix the comments, or document it as the synchronous kill primitive |
|
|
| R21-O14 | GWC-20 | Low | Gateway | `HeartbeatCheckInterval` is fed from `HeartbeatIntervalSeconds`, conflating the send and check intervals | Open | Split the option or rename it, and update `docs/GatewayConfiguration.md` |
|
|
| R21-O15 | GWC-21 | Low | Gateway | `EventChannelFullModeTimeout` and `HeartbeatStuckCeiling` are compile-time constants (`WorkerClientOptions`) | Open | Expose both under `MxGateway:Worker:*` with validation, or record them as intentionally fixed |
|
|
| R21-O16 | SEC-13 | Medium | Security | `GatewayLogRedactor.SensitiveCommandMethods` lacks `WriteSecuredBulk` and `WriteSecured2Bulk`. Latent in cycle 1, but `GatewayLogRedactorSeam` now calls `RedactCommandValue`, so a secured-bulk payload would be logged unredacted if value logging is enabled | Open | Add both bulk kinds plus a redactor test; cross-check every command kind that carries a password |
|
|
| R21-O17 | SEC-16 | Low | Security | `apikey --pepper` is accepted on the command line (`ApiKeyAdminCommandLineParser`) and lands in shell history and the process list | Open | Prefer an env var or stdin prompt; keep the flag only with a warning |
|
|
| R21-O18 | SEC-18 | Low | Security | `DashboardApiKeyManagementService` detects a missing pepper by matching `"pepper unavailable"` in an exception message | Open | Catch a typed exception from `ZB.MOM.WW.Auth.ApiKeys`, or check the pepper before the call |
|
|
| R21-O19 | SEC-23 | Low | Security | The options validator does not reject a `__Host-` `CookieName` combined with `RequireHttpsCookie=false`, a cookie browsers silently drop | Open | Add the cross-field check to `ValidateDashboard` |
|
|
| R21-O20 | SEC-24 | Low | Security | `EffectiveDashboardConfiguration` omits `DisableLogin`, `AutoLoginUser`, `RequireHttpsCookie` and `CookieName`, so the Settings page cannot show that login is disabled | Open | Project the four fields in `GatewayConfigurationProvider` |
|
|
| R21-O21 | CLI-07 | Medium | .NET client | `ExecuteSafeUnaryAsync` wraps the retry pipeline in `CancelAfter(DefaultCallTimeout)` while each attempt has the same deadline, so a `DeadlineExceeded` first attempt consumes the whole budget and retries never run | Open | Make the outer budget `MaxAttempts x DefaultCallTimeout` (or drop the outer cap); add a wire test with a slow first attempt |
|
|
| R21-O22 | CLI-09 | Medium | Go client | Go wraps every RPC failure in `GatewayError`; there is no typed `Unauthenticated` vs `PermissionDenied` mapping, though `ClientLibrariesDesign.md` requires one | Open | Add sentinel or typed errors usable with `errors.Is`; keep credential redaction |
|
|
| R21-O23 | CLI-11 | Medium | Go client | `mxgw-go` has no flag for strict TLS validation, so `--tls` without a CA is always skip-verify | Open | Add `--require-certificate-validation` wired to `Options.RequireCertificateValidation` |
|
|
| R21-O24 | CLI-19 | Low | .NET client | `InternalsVisibleTo` is declared in both `Properties/AssemblyInfo.cs` and the csproj | Open | Delete one (cosmetic) |
|
|
| R21-O25 | CLI-22 | Low | Go client | `newCorrelationID` returns an empty string if `crypto/rand` fails | Open | Fall back to a counter-based id |
|
|
| R21-O26 | CLI-27 | Low | Python client | `Session.close()` has no lock around `_closed`, and a repeat close returns a synthesized reply | Open | Guard with an `asyncio.Lock` and cache the real reply, as .NET and Go do |
|
|
| R21-O27 | CLI-31 | Low | Rust client | The Rust CLI `main.rs` is now 3,023 lines | Open | Split by command group when next touched |
|
|
| R21-O28 | CLI-33 | Low | Clients | Slow-consumer behavior is documented in the Go README but not in the Java README or the cross-language `docs/ClientBehaviorFixtures.md` | Open | Add one comparison table now that this cycle changed the Go and Java buffers |
|
|
| R21-O29 | TST-07 | Low | Tests | Real-clock sleeps with negative assertions remain (for example `WorkerClientTests.cs:975`, 150 ms) | Open | Convert to a fake time provider where one exists; otherwise leave |
|
|
| R21-O30 | TST-09 | Medium | Ops | Health checks cover the auth store and sessions only; Galaxy SQL, LDAP and worker-executable presence are unchecked (`GatewayApplication.cs:118`) | Open | Add a Ready-tag worker-exe presence check first, then a bounded Galaxy SQL ping; zero sessions must stay Healthy because the Overview dashboard polls the Active tier |
|
|
| R21-O31 | TST-10 | Medium | Ops | No deploy or upgrade script or runbook for the NSSM-wrapped service | Open | Write a deployment doc from the windev/wonder deploy notes; respect the current ban on remote execution on the wonder boxes |
|
|
| R21-O32 | TST-18 | Low | Tests | `SessionLeaseMonitorHostedService` and `OrphanWorkerCleanupHostedService` timer/startup wiring has no direct tests | Open | Add timer and startup tests with a fake time provider |
|
|
| R21-O33 | TST-21 | Low | Ops | Serilog file sink rolls daily with no `retainedFileCountLimit` or size cap | Open | Set `retainedFileCountLimit` and `fileSizeLimitBytes` in `appsettings.json`; document |
|
|
| R21-O34 | TST-22 | Low | Docs | The Configuration Shape JSON block in `docs/GatewayConfiguration.md` omits `DisableLogin`, `AutoLoginUser`, `CookieName`, `Tls` and `Alarms:Fallback` | Open | Add the keys to the block (docs-only) |
|
|
|
|
## Cycle-1 triage summary
|
|
|
|
The 68 cycle-1 rows that were `Not started` with no later re-listing resolved as follows.
|
|
|
|
| Verdict | Count | Rows |
|
|
|---|:-:|---|
|
|
| Done | 21 | WRK-02, WRK-05, WRK-09, WRK-13, WRK-20, GWC-10, GWC-12, GWC-22, IPC-10, IPC-12, SEC-14, SEC-17, SEC-19, SEC-21, SEC-26, SEC-27, CLI-05, CLI-06, CLI-10, CLI-13, TST-14 |
|
|
| Won't fix | 13 | GWC-11, IPC-11, SEC-15, CLI-14, CLI-17, CLI-20, CLI-23, CLI-25, CLI-28, CLI-32, TST-19, TST-20, WRK-14 |
|
|
| Open (promoted) | 34 | `R21-O1` through `R21-O34` above |
|
|
|
|
`WRK-02`, `WRK-05` and `WRK-09` are closed by stream E and become plain `Done` when `fix/review-2026-10-21-e` merges.
|
|
|
|
The seven rows singled out for assessment: `WRK-03` Open (`R21-O1`: the worker still returns silently from `TryStartCommandTask` after shutdown starts); `WRK-05` Done (stream E); `CLI-06` Done (stream F); `CLI-07` Open (`R21-O21`: the outer `CancelAfter` still equals the per-attempt deadline); `CLI-09` Open (`R21-O22`: no typed auth errors in Go); `CLI-11` Open (`R21-O23`: no strict-TLS flag on `mxgw-go`); `CLI-13` Done (stream G).
|
|
|
|
## Change log
|
|
|
|
| Date | Change |
|
|
|---|---|
|
|
| 2026-10-22 | Initial tracker from the 2026-10-21 review outcomes. Stream E is recorded as done but unmerged; stream L items are in progress. 68 cycle-1 rows re-triaged and the 34 still-open ones promoted. |
|