From 5a235776b6ac2dcea3c3f52a3ba7fde47e625e36 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Fri, 6 Nov 2026 15:32:27 -0500 Subject: [PATCH] docs: point markdown backlogs at Gitea issues (status lives in Gitea) --- CLAUDE.md | 7 ++ .../2026-07-12/remediation/00-tracking.md | 5 + .../90-candidate-findings-next-cycle.md | 9 +- .../2026-08-19/remediation/00-tracking.md | 9 +- .../2026-10-21/remediation/00-tracking.md | 107 +++++++++--------- archreview/remediation/00-tracking.md | 5 + code-reviews/IntegrationTests/findings.md | 5 +- docs/plans/2026-06-14-deferred-followups.md | 7 +- stillpending.md | 19 ++-- 9 files changed, 106 insertions(+), 67 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index f376839..8b1a3a6 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -131,6 +131,13 @@ powershell -ExecutionPolicy Bypass -File scripts/run-client-e2e-tests.ps1 - **Documentation style** (`StyleGuide.md`): PascalCase filenames, no marketing language, present tense, explain *why* not *what*. - **Update docs in the same change as the source.** When public APIs, contracts, configuration, build steps, security behavior, event shapes, value conversion, status mapping, or lifecycle rules change, the affected docs (`gateway.md`, `docs/`, client READMEs, design docs) must change in the same commit. Don't leave stale prose describing old behavior. +## Backlog / open work + +- **Open work lives in Gitea issues** on `gitea.dohertylan.com/dohertj2/mxaccessgw` (`tea issues list --repo dohertj2/mxaccessgw`). The 2026-11-06 migration filed the open markdown items as issues labelled `migrated` (#169–#213). +- **Markdown backlogs are historical records with `→ #N` pointers**, not status registers: the archreview trackers (`archreview/remediation/00-tracking.md`, `archreview/2026-07-12/remediation/00-tracking.md` + `90-candidate-findings-next-cycle.md`, `archreview/2026-08-19/remediation/00-tracking.md`, `archreview/2026-10-21/remediation/00-tracking.md`), `stillpending.md`, `docs/plans/2026-06-14-deferred-followups.md`, and `code-reviews/*/findings.md`. Do not update status there. +- **File new follow-ups, deferrals and review findings as issues**, using the family labels `type:{bug,feature,followup,review-finding,deploy-gate,debt,docs,test,infra}`, `priority:p0`..`p3`, `blocked`, `cross-repo`. +- Reference issues from plans and docs as `#N` (cross-repo: `lmxopcua#N`, `ScadaBridge#N`). + ## Source Update Workflow When source code changes, build and test the affected component before reporting work done. If the change crosses component boundaries, build each affected component — don't rely on a single top-level build: diff --git a/archreview/2026-07-12/remediation/00-tracking.md b/archreview/2026-07-12/remediation/00-tracking.md index 64d7a9a..3007726 100644 --- a/archreview/2026-07-12/remediation/00-tracking.md +++ b/archreview/2026-07-12/remediation/00-tracking.md @@ -1,5 +1,10 @@ # MxAccessGateway — Remediation Tracking (2026-07-12 review) +> **Status lives in Gitea.** Open items from this file are tracked as issues on gitea.dohertylan.com/dohertj2/mxaccessgw +> (label `migrated`, 2026-11-06). This file is a historical/design record — do not track status here; file new work as an issue. +> +> Rows still open in this cycle's tracker were re-triaged and carried into the [2026-10-21 tracker](../../2026-10-21/remediation/00-tracking.md) (`R21-*` rows; cycle-1 rows promoted as `R21-O1`..`R21-O34`), whose `→ #N` pointers name the Gitea issues. + Master progress tracker for the 2026-07-12 follow-up architecture review. Generated 2026-07-13. Source review: [`../00-overall.md`](../00-overall.md) · Per-domain remediation designs are linked below and hold the full **Finding / Impact / Design / Implementation / Verification** for every entry here. The first-cycle tracker ([`../../remediation/00-tracking.md`](../../remediation/00-tracking.md)) remains the record for the original 153 findings; this document tracks only the 47 new IDs (GWC-24+, WRK-21+, IPC-23+, SEC-31+, CLI-35+, TST-25+) plus the old-tracker actions listed at the end. diff --git a/archreview/2026-07-12/remediation/90-candidate-findings-next-cycle.md b/archreview/2026-07-12/remediation/90-candidate-findings-next-cycle.md index 476412f..fc95fdc 100644 --- a/archreview/2026-07-12/remediation/90-candidate-findings-next-cycle.md +++ b/archreview/2026-07-12/remediation/90-candidate-findings-next-cycle.md @@ -1,5 +1,10 @@ # Candidate Findings for the Next Review Cycle (surfaced during 2026-07-12 remediation) +> **Status lives in Gitea.** Open items from this file are tracked as issues on gitea.dohertylan.com/dohertj2/mxaccessgw +> (label `migrated`, 2026-11-06). This file is a historical/design record — do not track status here; file new work as an issue. +> +> Rows still open in this cycle's tracker were re-triaged and carried into the [2026-10-21 tracker](../../2026-10-21/remediation/00-tracking.md) (`R21-*` rows; cycle-1 rows promoted as `R21-O1`..`R21-O34`), whose `→ #N` pointers name the Gitea issues. + These were discovered while remediating the 2026-07-12 backlog but were **out of scope** for it — each is either pre-existing, by-design residual, or a new observation. They are recorded here so the next review cycle can triage them. None blocks the 2026-07-12 cycle, which is complete. Rows struck through have since been fixed ahead of that cycle; the original finding text is kept so the triage record stays readable. | ID (proposed) | Area | Severity (est.) | Summary | @@ -11,7 +16,7 @@ These were discovered while remediating the 2026-07-12 backlog but were **out of | ~~NEXT-05~~ | Worker frame writer | Info | **Resolved 2026-08-10 as a documented decision** — the lazy `DequeueNext` purge stays: any subsequent write drains both queues to empty and the heartbeat loop bounds tombstone residency to one interval, while eager `Queue` rebuilds under `_gate` would add ordering-invariant surface next to the WRK-22 interlock for no real gain. Rationale recorded in `docs/WorkerFrameProtocol.md`. Original finding: A batch whose remaining frames are tombstoned by cancellation leaves dead `PendingFrame` entries in `_eventFrames`/`_controlFrames` until a future `DequeueNext` pops and skips them. Same pre-existing behavior as single-frame WRK-22, amplified per-batch; in practice heartbeats purge them promptly, so not a real leak. | | ~~NEXT-06~~ | Testing / live LDAP | Medium | **Resolved 2026-08-07** — fixtures realigned to the shared directory (`admin`/`password` for the GwAdmin success path, `gw-viewer`/`password` for the bind-succeeds-but-no-role path); verified `Failed: 0, Passed: 5` live against the shared GLAuth at `10.100.0.35:3893`, so the success-path assertion (GwAdmin group claim + Admin role claim) now fails if the service-account credential is wrong. Original finding: `DashboardLdapLiveTests` fixtures have drifted from the shared GLAuth directory, leaving the suite with **no positive-proof coverage of the service-account bind**. Its only success-path test, `AuthenticateAsync_AdminInGwAdminGroup_Succeeds`, binds `admin`/`admin123`, but the directory's `admin` user carries the standard dev password (`scadaproj/infra/glauth/config.toml`), so that assertion cannot pass. `AuthenticateAsync_ReadOnlyUserMissingGwAdminGroup_Fails` binds fixture user `readonly`, which **does not exist** in the GLAuth config at all — it passes for the wrong reason (user-not-found rather than the group-missing branch it names; the `readonly` name is in fact barred by the README's user/group case-collision rule). The three remaining tests are negative assertions that pass whether or not the service account can bind. Net effect: a green `DashboardLdapLiveTests` run proves nothing about the bind credential — surfaced during SEC-36, where the suite was considered as a substitute for the deferred dashboard-login check and rejected. Fix: realign the fixtures to real directory users (e.g. `multi-role`/`gw-viewer`) or add the missing users to the GLAuth config, and add one test that fails when the service-account credential is wrong. | | ~~NEXT-07~~ | Deployment / windev | High | **Resolved 2026-08-07** — a fresh portable framework-dependent publish of `origin/main` (`a346d51`) was built in a clean clone at `C:\build\mxgw-redeploy`, deployed to `C:\publish\mxaccessgw\Server-20260807`, and the `MxAccessGw` NSSM service repointed at it; the service now holds a stable PID with both `5120`/`5130` listening, a worker spawned, the Galaxy snapshot restored (129 objects / 56,731 attributes) and a clean event log. Root cause confirmed as the version skew this row predicted: the deployed 2026-06-25 build carried `ZB.MOM.WW.Auth.ApiKeys` 0.1.2.0, which supports auth-DB schema 2, against a `gateway-auth.db` stamped at schema 3 on 2026-07-15 by an ephemeral run of newer code — schema 3 is the current shared-lib version (`SqliteAuthSchema.CurrentVersion=3` in Auth 0.1.5), so the redeploy is the forward fix and the DB was left alone. Rollback artifacts kept: `C:\ProgramData\MxGateway\gateway-auth.db.bak-next07` (with `-wal`/`-shm`) and the previous `C:\publish\mxaccessgw\Server` directory. Two side effects worth recording: the old deploy's `appsettings.json` held the LDAP bind password in **plaintext on disk**, while the new one keeps the repo's `${secret:ldap/mxgateway/bind}` token with the NSSM environment supplying the value, so no plaintext LDAP secret remains on that host; and the redeploy tripped the SEC-06 `Ldap:Transport=None` production hard-stop (`GatewayOptionsValidator.cs:178`), resolved by relabelling the host — windev runs `Dashboard:DisableLogin=true`, which this repo's own docs mark dev/test-only, so its `Production` label contradicted its configuration and `DOTNET_ENVIRONMENT` was changed to `Staging` (that one NSSM environment entry only; the other nine preserved byte-identical). SEC-06 is untouched for genuinely production hosts — see NEXT-08 for the posture problem that relabelling defers. Original finding: The `10.100.0.48` (windev) gateway deployment is **stale and crash-looping**, and has been since at least 2026-08-06 (~10k Hosting-failed events/day). The deployed Server binary dates to 2026-06-25 and predates the auth-DB migration of 2026-07-15: it opens a schema-version-3 `gateway-auth.db` that it supports only at version 2 and aborts at startup, so the `MxAccessGw` service never reaches a listening state. Not a code defect in the current tree — a deploy-drift/operations gap — but it means the repo's only deployed host has been dark for over a day and any host-level verification (including SEC-36's dashboard-login check) is blocked until it is repaired. Fix: deploy a current Server build to windev, or restore/downgrade the auth DB to schema 2 if the old binary must stand. Worth asking separately why a service in a permanent restart loop raised no alert. Discovered during SEC-36. | -| NEXT-08 | Security / LDAP posture | Medium | **The shared GLAuth offers no TLS, so SEC-06 makes it undeployable from a `Production`-labelled host.** `GatewayOptionsValidator` (`src/ZB.MOM.WW.MxGateway.Server/Configuration/GatewayOptionsValidator.cs:178`) refuses to start when `Ldap:Transport=None` in the `Production` environment, and `docs/GatewayConfiguration.md`'s `Transport` row states "Deployed hosts must set `Ldaps` or `StartTls`" — but the shared instance at `10.100.0.35:3893` has `[ldaps] enabled=false`, port `3894` closed, and answers StartTLS with `protocolError`, so neither value can work against it. That instruction is currently unsatisfiable for every host that authenticates there. windev sidestepped it on 2026-08-07 by moving to the `Staging` environment name (NEXT-07), which is honest for a dev/test rig but is not available to a real production host. Resolution needs either LDAPS/StartTLS on the shared GLAuth (certificate plus a trust story on each gateway host) or an explicit written posture decision that production gateways bind a different, TLS-capable directory. Surfaced during the NEXT-07 redeploy. | +| NEXT-08 → #180 | Security / LDAP posture | Medium | **The shared GLAuth offers no TLS, so SEC-06 makes it undeployable from a `Production`-labelled host.** `GatewayOptionsValidator` (`src/ZB.MOM.WW.MxGateway.Server/Configuration/GatewayOptionsValidator.cs:178`) refuses to start when `Ldap:Transport=None` in the `Production` environment, and `docs/GatewayConfiguration.md`'s `Transport` row states "Deployed hosts must set `Ldaps` or `StartTls`" — but the shared instance at `10.100.0.35:3893` has `[ldaps] enabled=false`, port `3894` closed, and answers StartTLS with `protocolError`, so neither value can work against it. That instruction is currently unsatisfiable for every host that authenticates there. windev sidestepped it on 2026-08-07 by moving to the `Staging` environment name (NEXT-07), which is honest for a dev/test rig but is not available to a real production host. Resolution needs either LDAPS/StartTLS on the shared GLAuth (certificate plus a trust story on each gateway host) or an explicit written posture decision that production gateways bind a different, TLS-capable directory. Surfaced during the NEXT-07 redeploy. | | ~~NEXT-09~~ | Build / versioning | Low | **Resolved 2026-08-10** — the Exec path now appends `.` so the trailing backslash can no longer escape the closing quote, and `SourceRevisionId` is additionally gated on a short-SHA regex so no future git failure text can be stamped either. macOS stamp verified unchanged; Windows stamp verified on windev with this batch. Original finding: **Windows builds stamp git's error text into `InformationalVersion`.** `src/Directory.Build.props:29` runs `git -C "$(MSBuildThisFileDirectory)" …`; MSBuild's directory property ends in a backslash, which escapes the closing quote, so the command is malformed on Windows. The target carries `ContinueOnError`, so the failure is silent and git's stderr is captured as the revision — an observed stamp reads `0.1.2+fatal: cannot change to …`. Any Windows build without a preset `SourceRevisionId` therefore ships a binary that cannot be correlated back to a commit, defeating the point of TST-11. Not reproducible on macOS/Linux, where the separator is `/`. Fix sketch: append `.` to the path or trim the trailing separator before quoting. Surfaced while identifying the deployed binary during NEXT-07. | | NEXT-10 | Docs / glauth | Medium | **`glauth.md`'s "Pre-provisioned users" table contradicts both the directory and the rest of its own file.** It documents `readonly`/`readonly123` and `admin`/`admin123`, neither of which matches `scadaproj/infra/glauth/config.toml` (`readonly` does not exist there; `admin` carries the standard dev password), and lists the `ReadOnly` gid as `5501` against an actual `5601`. Its dashboard section, by contrast, is correct — so the file is internally inconsistent and a reader cannot tell which half to trust. This table was the **root cause of the NEXT-06 fixture drift**, and it has propagated further: `docs/GatewayTesting.md`'s `MXGATEWAY_LIVE_MXACCESS_WRITE_SECURED_PASSWORD` default and the matching literal in `WorkerLiveMxAccessSmokeTests` both take `admin123` from it. Deliberately **not** fixed in the 2026-08-07 pass: the table is entangled with the OPC-UA group taxonomy (gids, role mapping, and the sister-repo consumers of the same directory), so reconciling it means sweeping that taxonomy as one unit rather than patching two rows. | @@ -21,5 +26,5 @@ These are **live-infrastructure actions the operator must execute** — the repo - ~~**SEC-36** — rotate the dev LDAP service-account credential per `docs/runbooks/SEC-36-ldap-credential-rotation.md` (generate new secret in `scadaproj/infra/glauth`, pre-stage the NSSM env var on deployed hosts, rotate GLAuth on `10.100.0.35`, verify dashboard login). The committed literal is gone from the working tree but remains recoverable from git history until rotation completes — **rotation is the load-bearing half.**~~ **Executed 2026-08-07**: the `serviceaccount` `passsha256` was replaced in `scadaproj/infra/glauth/config.toml` (commit `aada53b`) and the shared GLAuth recreated on `10.100.0.35`, so the literal recoverable from this repo's history no longer binds `dc=zb,dc=local`. The new value lives only in the GLAuth hash, windev's NSSM environment, and dev user-secrets. `wonder-app-vd03` was out of scope (it binds a different, `dc=scadalink`/`dc=scadabridge` directory). **Caveat, since closed:** windev's dashboard `/login` verification was deferred while that host's gateway was crash-looping on the unrelated stale-deployment fault filed as NEXT-07, so the bind was verified directly by `ldapsearch` as `cn=serviceaccount,dc=zb,dc=local` instead. After the 2026-08-07 redeploy the real check ran on windev (login as `multi-role` → 302 + dashboard cookie, anonymous control → `/login`), so the rotated credential is now proven through the `DashboardAuthenticator` path itself; see `docs/runbooks/SEC-36-ldap-credential-rotation.md` Correction 3. SEC-36 is fully `Done`. - ~~**TST-30** — register a second Gitea `act_runner` on `10.100.0.35` per `docs/runbooks/TST-30-second-ci-runner.md` to relieve the single-shared-runner bottleneck.~~ **Executed 2026-08-07**: `gitea-runner-2` (id 5, capacity 2) is online on `10.100.0.35` via the `/opt/gitea` compose stack, same `container.network: traefik`, token from a `0600` file mount; the existing runner (id 1, capacity 4) was untouched. Concurrency verified — jobs from three runs ran simultaneously across both runners, and a `gitea-runner-2` job cloned successfully from `http://gitea:3000`. TST-30 is now fully `Done`. -- **TST-30 follow-up — reset the Gitea instance runner registration token.** Runner-1's compose block was moved to the same `0600` file-mount pattern as runner-2 on 2026-08-07 (compose and both backups now `0600 root:root`, runner-1 recreated with its identity intact), but hygiene alone does not retire the token: both runners share **one instance-scope registration token** that was world-readable for roughly five months and is still live — a probe registered runner id 6 with it, then deleted it. Gitea 1.26.4 exposes no rotation via CLI or API (both paths are get-or-create and hand back the same value), so the reset must be done in the admin web UI ("Reset registration token"). Afterwards, refresh `/opt/gitea/runner_token` on `10.100.0.35` and shred the two token-bearing compose backups — they are the last copies of the old value. See `docs/runbooks/TST-30-second-ci-runner.md`. +- **TST-30 follow-up — reset the Gitea instance runner registration token.** → #181 Runner-1's compose block was moved to the same `0600` file-mount pattern as runner-2 on 2026-08-07 (compose and both backups now `0600 root:root`, runner-1 recreated with its identity intact), but hygiene alone does not retire the token: both runners share **one instance-scope registration token** that was world-readable for roughly five months and is still live — a probe registered runner id 6 with it, then deleted it. Gitea 1.26.4 exposes no rotation via CLI or API (both paths are get-or-create and hand back the same value), so the reset must be done in the admin web UI ("Reset registration token"). Afterwards, refresh `/opt/gitea/runner_token` on `10.100.0.35` and shred the two token-bearing compose backups — they are the last copies of the old value. See `docs/runbooks/TST-30-second-ci-runner.md`. - **TST-25 follow-ups** — old **TST-05** (scheduled live-MXAccess smoke) is now covered by the `nightly-windev` job; old **TST-24** (client wire tests in CI) is unblocked by the working Windows tier. diff --git a/archreview/2026-08-19/remediation/00-tracking.md b/archreview/2026-08-19/remediation/00-tracking.md index af91874..bec2654 100644 --- a/archreview/2026-08-19/remediation/00-tracking.md +++ b/archreview/2026-08-19/remediation/00-tracking.md @@ -1,5 +1,10 @@ # MxAccessGateway — Remediation Plan & Tracking (2026-08-19 review) +> **Status lives in Gitea.** Open items from this file are tracked as issues on gitea.dohertylan.com/dohertj2/mxaccessgw +> (label `migrated`, 2026-11-06). This file is a historical/design record — do not track status here; file new work as an issue. +> +> Rows still open in this cycle's tracker were re-triaged and carried into the [2026-10-21 tracker](../../2026-10-21/remediation/00-tracking.md) (`R21-*` rows; cycle-1 rows promoted as `R21-O1`..`R21-O34`), whose `→ #N` pointers name the Gitea issues. + Source review: [`../00-overall.md`](../00-overall.md) (HEAD `8b3e3b3`, committed as `f4919ba`). Generated 2026-08-20. Tracks the 34 cycle-3 IDs (GWC-31+, WRK-32+, IPC-33+, SEC-37+, CLI-46+, TST-31+) plus the tracker-housekeeping actions the overall asked for. Cycle-1/2 trackers remain the record for their own IDs. @@ -25,8 +30,8 @@ This plan is executed as **seven parallel batches**, each by one subagent workin | ID | Status | Why | |---|---|---| -| TST-32 | Blocked (external) | Gitea runner registration-token reset is a web-UI action on the Gitea admin page; no repo change. Operator to-do. | -| SEC-37 | Blocked (external) | Family-wide: TLS on the shared GLAuth lives in `scadaproj/infra/glauth/` (separate repo + docker host). Plan separately; after it lands, move the deployed service to `Production`. | +| TST-32 | Blocked (external) → #181 | Gitea runner registration-token reset is a web-UI action on the Gitea admin page; no repo change. Operator to-do. | +| SEC-37 | Blocked (external) → #180 | Family-wide: TLS on the shared GLAuth lives in `scadaproj/infra/glauth/` (separate repo + docker host). Plan separately; after it lands, move the deployed service to `Production`. | | CLI-47/48/49/50, IPC-36/37, TST-35 | Done | Activated by the 2026-08-25 P2 continuation below and closed there: CLI-47/48/49/50 in batch K (`5cdb3da`), IPC-36/37 in batch J (`d6c5a11`), TST-35 in batch M (`cc143b5`). | | GWC-33/35, SEC-38/39 | Done | Closed by continuation batch I; regression counts and commits are recorded below. | | GWC-36, IPC-38, TST-33/34/36 | Done | Closed by continuation batch H; verification and commit are recorded below. | diff --git a/archreview/2026-10-21/remediation/00-tracking.md b/archreview/2026-10-21/remediation/00-tracking.md index e0d35ff..e1d81d4 100644 --- a/archreview/2026-10-21/remediation/00-tracking.md +++ b/archreview/2026-10-21/remediation/00-tracking.md @@ -1,5 +1,8 @@ # MxAccessGateway — Remediation Tracking (2026-10-21 system review) +> **Status lives in Gitea.** Open items from this file are tracked as issues on gitea.dohertylan.com/dohertj2/mxaccessgw +> (label `migrated`, 2026-11-06). This file is a historical/design record — do not track status here; file new work as an issue. + 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). @@ -8,7 +11,7 @@ Tracks the findings of the 2026-10-21 system review and the cycle-1 findings tha - 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. +- 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. *(Superseded 2026-11-06: status now lives in the Gitea issue named by each `→ #N` pointer; the Status column is frozen as of the migration.)* **Status legend:** `Done` · `Done (merge 2013573)` · `Done (merge 2fad124)` · `Deferred` · `Won't fix` · `Open` · `Blocked (external)`. @@ -31,22 +34,22 @@ The review was remediated by ten parallel streams on branches `fix/review-2026-1 | 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. +Worker changes (stream E) need the windev x86 build and `Worker.Tests` run after merge; stream E's own live facts passed on windev. → #169 (merge to origin `main` + windev Worker gate) ## 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-S2 | Medium | Sessions | Sweep-driven closes skipped `SessionClosed()`, so the open-sessions gauge drifted | Done → #168 | 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-S6 | Medium | Sessions | Sessions that never stream pinned a worker for the whole lease | Done → #167 | 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 | +| R21-S10 | Low | Sessions | The replay ring has a count cap but no byte cap | Open → #172 | — | Documented as bounded by count. Next step: add a byte cap if large-array events make the count cap insufficient | ## WorkerClient (stream B) @@ -70,7 +73,7 @@ Worker changes (stream E) need the windev x86 build and `Worker.Tests` run after | 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 | +| R21-G13 | Low | Security | The verifier generation guard depends on key-id case | Open → #173 | — | Latent only. Next step: normalize the key id (ordinal-ignore-case) at the guard and add a test | ## Dashboard (stream D) @@ -86,13 +89,13 @@ Worker changes (stream E) need the windev x86 build and `Worker.Tests` run after | 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` | +| R21-D11 | Low | Dashboard | `BrowsePage` search CTS cancel/dispose race | Open → #174 | — | 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 → #174 | — | Next step: refresh on first subscribe | +| R21-D13 | Low | Dashboard | Hub pumps are not awaited on broadcaster dispose | Open → #174 | — | 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`. +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`. → #169 | ID | Sev | Area | Description | Status | Commit / stream | Reason / note | |---|---|---|---|---|---|---| @@ -105,10 +108,10 @@ All rows are on `fix/review-2026-10-21-e` (`7618caf`, follow-up `38fbbde`), not | 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 | +| R21-K10 | Low | Worker | Unregister-first shutdown cleanup | Deferred → #175 | — | Needs a parity probe against the installed MXAccess before it can change cleanup order | +| R21-K11 | Medium | Security | A per-operator `AuthenticateUser` cache | Deferred → #176 | — | Security tradeoff (caching credentials' outcome); decide with the security owner before building | +| R21-K12 | Low | Worker | Batch task-completion-source allocation rework | Deferred → #177 | — | Low value | +| R21-K13 | Low | Worker | The alarm poll builds a full-BSTR `XmlDocument` every 500 ms | Deferred → #177 | — | Steady GC cost, not a correctness issue; revisit if profiling flags it | ## Clients (streams F to J) @@ -142,14 +145,14 @@ All rows are on `fix/review-2026-10-21-e` (`7618caf`, follow-up `38fbbde`), not | 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 | +| R21-L8 | Low | Tests | No tests for the `MxStatusDetailText` table | Deferred → #178 | — | 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-X1 | Medium | Security | TLS on the shared GLAuth (cycle-3 `SEC-37`) | Blocked (external) → #180 | — | 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) → #181 | — | 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) @@ -158,40 +161,40 @@ These are cycle-1 findings that no later tracker re-listed. Each was checked aga | 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) | +| 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 → #182 | 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 → #183 | 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 → #184 | 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 → #185 | 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 → #186 | Map `EndOfStream` to `PipeConnectionFailed`; add a `WorkerApplicationTests` case | +| R21-O6 | WRK-18 | Low | Worker | Event-queue overflow exits as generic `UnexpectedFailure` (1) | Open → #186 | 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 → #184 | 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 → #187 | 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 → #188 | 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 → #189 | Replace with a `TaskCompletionSource` signalled on state change | +| R21-O11 | GWC-17 | Low | Gateway | `SparseArrayExpander` throws `Grpc.Core.RpcException` from the Sessions layer | Open → #190 | 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 → #191 | 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 → #191 | 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 → #192 | 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 → #192 | 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 → #193 | 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 → #194 | 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 → #195 | 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 → #196 | 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 → #196 | 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 → #197 | 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 → #198 | 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 → #199 | 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 → #200 | Delete one (cosmetic) | +| R21-O25 | CLI-22 | Low | Go client | `newCorrelationID` returns an empty string if `crypto/rand` fails | Open → #201 | 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 → #202 | 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 → #203 | 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 → #204 | 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 → #205 | 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 → #206 | 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 → #207 | 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 → #205 | 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 → #208 | 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 → #209 | Add the keys to the block (docs-only) | ## Cycle-1 triage summary @@ -221,4 +224,4 @@ The seven rows singled out for assessment: `WRK-03` Open (`R21-O1`: the worker s | R21-M2 | Low | Metrics | One worker fault was counted 1+K times (K = number of `StreamEvents` subscribers) | Done | M: `a0b5e91` | | | R21-M3 | Low | Sessions | A cancelled open, or one drained at shutdown, was reported as `OpenFailed` | Done | M: `a178d68` | Now `Cancelled` and `GatewayStopping` respectively | | R21-M4 | Low | Docs | Unattached-grace exemptions, subscriber-limit status code, stale event-stream description in Grpc.md | Done | M: `3ffba83` | | -| R21-M5 | Low | Docs | Client docs described the ReadBulk timeout as per-tag | Done | M: `dd62335` | Worker-side comments (`MxAccessCommandExecutor.cs:15`, `MxAccessValueCache.cs:219`, `MxAccessValueCacheTests.cs:246`) still say per-tag: Open | +| R21-M5 | Low | Docs | Client docs described the ReadBulk timeout as per-tag | Done | M: `dd62335` | Worker-side comments (`MxAccessCommandExecutor.cs:15`, `MxAccessValueCache.cs:219`, `MxAccessValueCacheTests.cs:246`) still say per-tag: Open → #179 | diff --git a/archreview/remediation/00-tracking.md b/archreview/remediation/00-tracking.md index daae30a..972619f 100644 --- a/archreview/remediation/00-tracking.md +++ b/archreview/remediation/00-tracking.md @@ -1,5 +1,10 @@ # MxAccessGateway — Remediation Tracking +> **Status lives in Gitea.** Open items from this file are tracked as issues on gitea.dohertylan.com/dohertj2/mxaccessgw +> (label `migrated`, 2026-11-06). This file is a historical/design record — do not track status here; file new work as an issue. +> +> Rows still open in this cycle's tracker were re-triaged and carried into the [2026-10-21 tracker](../2026-10-21/remediation/00-tracking.md) (`R21-*` rows; cycle-1 rows promoted as `R21-O1`..`R21-O34`), whose `→ #N` pointers name the Gitea issues. + Master progress tracker for the 2026-07-08 architecture review. Generated 2026-07-09. Source review: [`../00-overall.md`](../00-overall.md) · Per-domain remediation designs are linked below and hold the full **Finding / Impact / Design / Implementation / Verification** for every entry here. diff --git a/code-reviews/IntegrationTests/findings.md b/code-reviews/IntegrationTests/findings.md index f53bb71..496cfa4 100644 --- a/code-reviews/IntegrationTests/findings.md +++ b/code-reviews/IntegrationTests/findings.md @@ -1,5 +1,8 @@ # Code Review — IntegrationTests +> **Status lives in Gitea.** Open items from this file are tracked as issues on gitea.dohertylan.com/dohertj2/mxaccessgw +> (label `migrated`, 2026-11-06). This file is a historical/design record — do not track status here; file new work as an issue. + | Field | Value | |---|---| | Module | `src/ZB.MOM.WW.MxGateway.IntegrationTests` | @@ -710,4 +713,4 @@ The prior `DashboardAuthenticator` ctor took `IOptions`, so the **Recommendation:** Add AddItem2/Write2 to the parity test (or a dedicated test) asserting each produces a real reply (not InvalidRequest) against a valid handle and item-definition split. -**Resolution:** 2026-06-16: requires a live MXAccess rig + provider state not available on this dev box; add the AddItem2/Write2 parity assertions when running on the MXAccess host. +**Resolution:** 2026-06-16: requires a live MXAccess rig + provider state not available on this dev box; add the AddItem2/Write2 parity assertions when running on the MXAccess host. → #212 diff --git a/docs/plans/2026-06-14-deferred-followups.md b/docs/plans/2026-06-14-deferred-followups.md index fb08c9a..6b1b424 100644 --- a/docs/plans/2026-06-14-deferred-followups.md +++ b/docs/plans/2026-06-14-deferred-followups.md @@ -1,5 +1,8 @@ # Deferred Follow-ups Implementation Plan +> **Status lives in Gitea.** Open items from this file are tracked as issues on gitea.dohertylan.com/dohertj2/mxaccessgw +> (label `migrated`, 2026-11-06). This file is a historical/design record — do not track status here; file new work as an issue. + **Date:** 2026-06-14 **Status:** D1 executed (commit 4af24b9 — `mxgateway.alarms.provider_switches` emitted in `DashboardSnapshotService.cs:198`). D2 resolved as no-op (see resolution section below). D3 (ForceSubtag validation) was exercised through the forced-subtag work (`2026-06-15-forced-subtag-mode-fix.md`); D4 (windev backup prune) has no completion record in the repo and is treated as pending; D5 (wonder redeploy) was superseded by later wonder deployments. D3–D5 are ops/validation with no code. **Context:** After the alarm-subtag-fallback cleanup (merged `5976770`) and its redeploy to @@ -281,7 +284,7 @@ remain explicitly open — none are code defects: emitted on the alarm failover/failback path and unit-tested, but the dev rig's object-driven alarms can't be made to fail a real alarmmgr→subtag switch from outside, so the `reason` tagging is unproven against a live failover. Re-verify when a rig (or capture) can drive an actual - alarmmgr fault. (stillpending §1.3.) + alarmmgr fault. (stillpending §1.3.) → #210 - **`DrainEvents` is a diagnostic snapshot, not an exhaustive drain.** The worker now answers `DrainEvents` (handled in `WorkerPipeSession`, off-STA), but it pulls from the same event queue @@ -294,7 +297,7 @@ remain explicitly open — none are code defects: / `SetBufferedUpdateInterval` are implemented and live-confirmed; the empty `NoData` bootstrap event converts cleanly live (`f7ada90`). A real parallel quality/timestamp sample batch (length > 1) is undrivable on the current rig, so the multi-sample path is exercised only by unit - tests against a fake `IMxAccessServer`. (stillpending §3.2.) + tests against a fake `IMxAccessServer`. (stillpending §3.2.) → #210 - **8-arg alarm ack operator `domain`/`full_name` — vendor-blocked.** The AVEVA `IwwAlarmConsumer2` 8-arg `AlarmAckByName` returns −55 (stub) and `AlarmAckByGUID` is `E_NOTIMPL` on this build, so the diff --git a/stillpending.md b/stillpending.md index 428566a..77fb1c3 100644 --- a/stillpending.md +++ b/stillpending.md @@ -1,5 +1,8 @@ # Still Pending — Deferred / Partial / Unfinished / Missing Functionality +> **Status lives in Gitea.** Open items from this file are tracked as issues on gitea.dohertylan.com/dohertj2/mxaccessgw +> (label `migrated`, 2026-11-06). This file is a historical/design record — do not track status here; file new work as an issue. + **Generated:** 2026-06-15 · **Commit:** `c7f754c` (main) · **Method:** six parallel read-only audits (Server, Worker, Contracts/proto, all five clients, docs/design/plans, tests + review backlog). Every item cites a verified `file:line`. > **Historical inventory:** This file preserves the June 2026 audit and its resolution annotations. It is not the live remediation register. Current finding status and verification evidence live under `archreview/`, including the active cycle tracker at `archreview/2026-08-19/remediation/00-tracking.md`. @@ -38,7 +41,7 @@ Items are graded by what they actually are, because most "pending" surface in th - **Resolved (`8415f35`, `55526d5`):** `request.ClientCorrelationId` is threaded from `Invoke` → `ApplyConstraintsAsync` → the six filter helpers → `IConstraintEnforcer.RecordDenialAsync` (new `string? correlationId` param); the `TODO(Task 2.3)` is gone. The audit `CorrelationId` column is `Guid?`, so a GUID-parseable id is stored typed; **and** the raw string is always preserved in the audit record's `DetailsJson["clientCorrelationId"]` — this matters because the Rust client sends non-GUID ids (`rust-client--`) on all traffic and Python/Java default to empty, which would otherwise have left the typed column null. An end-to-end test asserts the value propagates through `Invoke`. - **Original finding:** denied-operation audit records always wrote `CorrelationId = null` (`ConstraintEnforcer.cs:134-136,147`); threading needed an `IConstraintEnforcer` signature change. -### 🔴 1.3 — `provider_switches{from,to,reason}` counter never exercised live +### 🔴 1.3 — `provider_switches{from,to,reason}` counter never exercised live → #210 - **Location:** metric emitted in `Alarms/GatewayAlarmMonitor.cs` (failover/failback path); residual recorded in `docs/plans/2026-06-14-deferred-followups.md:124-125`. - **Evidence:** *"that counter's live exercise remains the one gap; record it explicitly rather than claiming coverage."* Unit-tested (Tests-032) but the dev rig can't drive a real alarmmgr→subtag failover (`project_rig_alarms_object_driven`), so the counter's `reason` tagging is unproven in production. @@ -72,13 +75,13 @@ These are documented, deliberate, and mostly enforced. Listed so the deferred su Intentionally incomplete, awaiting a live capture or an AVEVA fix; raw payload/metadata is preserved in the meantime (no synthesis). -- 🟡 **3.1 `OperationComplete` native trigger condition unknown** — modeled and emitted only from the real event (no synthesis), but the runtime condition that fires it isn't captured. `docs/DesignDecisions.md:280-289`, `gateway.md:1094`, `docs/MxAccessWorkerInstanceDesign.md:341,366`. -- 🟡 **3.2 `OnBufferedDataChange` multi-sample conversion unvalidated — STILL OPEN (residual after B8)** — `AddBufferedItem`/`SetBufferedUpdateInterval` are now implemented and live-confirmed (§1.1), and the live B8 test (`f7ada90`) confirms the worker receives and cleanly converts the empty `NoData` bootstrap `OnBufferedDataChange` (no crash, no dropped payload). But the rig's object logic does not drive a multi-sample buffered batch on demand (same limitation as the alarm rig), so a real parallel quality/timestamp sample array (length > 1) has never been observed live — it is exercised only by the B-bundle unit tests against a fake `IMxAccessServer`. Re-run `GatewaySession_WithLiveWorker_BufferedItem_*` against a fast-changing simulated tag to close this. `docs/DesignDecisions.md:291-297`. -- 🟡 **3.3 Completion-only status → `MXSTATUS_PROXY[]` mapping unproven** — completion-only operation-status bytes are kept as raw diagnostic metadata until the analysis proves an exact mapping. `docs/DesignDecisions.md:299-306`. +- 🟡 **3.1 `OperationComplete` native trigger condition unknown** → #211 — modeled and emitted only from the real event (no synthesis), but the runtime condition that fires it isn't captured. `docs/DesignDecisions.md:280-289`, `gateway.md:1094`, `docs/MxAccessWorkerInstanceDesign.md:341,366`. +- 🟡 **3.2 `OnBufferedDataChange` multi-sample conversion unvalidated — STILL OPEN (residual after B8)** → #210 — `AddBufferedItem`/`SetBufferedUpdateInterval` are now implemented and live-confirmed (§1.1), and the live B8 test (`f7ada90`) confirms the worker receives and cleanly converts the empty `NoData` bootstrap `OnBufferedDataChange` (no crash, no dropped payload). But the rig's object logic does not drive a multi-sample buffered batch on demand (same limitation as the alarm rig), so a real parallel quality/timestamp sample array (length > 1) has never been observed live — it is exercised only by the B-bundle unit tests against a fake `IMxAccessServer`. Re-run `GatewaySession_WithLiveWorker_BufferedItem_*` against a fast-changing simulated tag to close this. `docs/DesignDecisions.md:291-297`. +- 🟡 **3.3 Completion-only status → `MXSTATUS_PROXY[]` mapping unproven** → #211 — completion-only operation-status bytes are kept as raw diagnostic metadata until the analysis proves an exact mapping. `docs/DesignDecisions.md:299-306`. - 🟡 **3.4 `AlarmAckByGUID` is `E_NOTIMPL` on this AVEVA build (historical; note corrected 2026-10-22)** — it throws `NotImplementedException`, but the worker no longer calls it. `MxCommandKind.AcknowledgeAlarm` has live dispatch (`Worker/MxAccess/MxAccessCommandExecutor.cs` `ExecuteAcknowledgeAlarm`, ~895-936): it parses `alarm_guid` and calls `alarmCommandHandler.Acknowledge(guid, comment, AlarmAckOperator)`, which acknowledges by writing `AckMsg` as the authenticated operator. A GUID ack therefore works; only the vendor `AlarmAckByGUID` call is dead. `docs/AlarmClientDiscovery.md:750-763`. - 🟡 **3.5 8-arg `AlarmAckByName` v2 is a vendor stub (returns -55)** — no longer on the ack path (acks write `AckMsg`); `operator_full_name` is carried for forward-compat only and is not propagated (see 1.4). `docs/AlarmClientDiscovery.md:743-748`. - 🟡 **3.6 Subtag degraded-mode fidelity limits** — `category`, `description`, `alarm_type_name`, operator fields, and `retrigger` are not populated/synthesized in subtag fallback (no subtag exists for them). Documented, by design. `docs/AlarmClientDiscovery.md:913-931`, `docs/plans/2026-06-13-alarm-subtag-fallback-design.md:292-298`. -- 🟡 **3.7 Subtag `Clear` transition unvalidated live** — Raise/Ack/AckMsg are live-confirmed; Clear is externally undrivable on the rig (object logic owns alarm state). Environmental, not code. (`project_alarm_subtag_fallback`, `project_rig_alarms_object_driven`.) +- 🟡 **3.7 Subtag `Clear` transition unvalidated live** → #210 — Raise/Ack/AckMsg are live-confirmed; Clear is externally undrivable on the rig (object logic owns alarm state). Environmental, not code. (`project_alarm_subtag_fallback`, `project_rig_alarms_object_driven`.) --- @@ -159,10 +162,10 @@ No placeholder/empty/`Assert.True(true)` tests were found anywhere. **Status after `feat/stillpending-completion` (2026-06-15):** the net-new functionality is **done** — **§1.1** (all 11 worker command kinds, COM half live-verified on the rig), **§1.2** (audit CorrelationId, with a raw-string fallback for non-GUID clients), and the entire **§4** client CLI/helper parity surface (`Write2`, `ping`, `galaxy-*`, `galaxy-browse` 5/5, name aliases). Doc hygiene **§7** is done (`0032d2d`, `bd46ba1`). Zero `.proto` changes were required. **Still open (all deliberate or environment/vendor-gated):** -- **§1.3** — `provider_switches` counter still only unit-tested; the dev rig can't drive a real alarmmgr→subtag failover, so live `reason`-tag coverage remains a recorded residual. +- **§1.3** (→ #210) — `provider_switches` counter still only unit-tested; the dev rig can't drive a real alarmmgr→subtag failover, so live `reason`-tag coverage remains a recorded residual. - **§1.4 / §3.4 / §3.5** — the AVEVA 8-arg `AlarmAckByName` is a vendor stub (−55) and `AlarmAckByGUID` is `E_NOTIMPL`; `operator_domain` is consumed by the `AckMsg` path; `operator_full_name` stays forward-compat-only (not propagated). -- **§3.2** — buffered commands work and the empty bootstrap converts cleanly live, but a multi-sample buffered batch is undrivable on the rig (unit-tested only). -- **§3.1 / §3.3 / §3.6 / §3.7** — await live MXAccess captures. +- **§3.2** (→ #210) — buffered commands work and the empty bootstrap converts cleanly live, but a multi-sample buffered batch is undrivable on the rig (unit-tested only). +- **§3.1 / §3.3 / §3.6 / §3.7** — await live MXAccess captures. (§3.1/§3.3 → #211; §3.7 → #210) - **§2** — deliberate v1 scope. Multi-subscriber fan-out, reconnect, and the per-session dashboard ACL are resolved; orphan-worker reattach remains deliberately deferred and not planned. **§5** — opt-in verification gates. **§7.6** — accepted `Won't Fix` review findings. MXAccess **event/data/value/write** mapping, the **Galaxy** RPC surface, and now the **full command surface** are complete; no `NotImplementedException`s, stubbed RPC bodies, or empty tests remain in the production paths.