Compare commits
12 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| 61c00d59a9 | |||
| 574369b666 | |||
| 9351ec1d2c | |||
| b9096f7c57 | |||
| ff397b27be | |||
| 3a3d9e8e06 | |||
| e5e7f1854f | |||
| 777fef9cad | |||
| 34a7453ac3 | |||
| 9354d69bf9 | |||
| 5a235776b6 | |||
| 8c0a51dc90 |
@@ -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*.
|
- **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.
|
- **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
|
## 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:
|
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:
|
||||||
|
|||||||
@@ -1,5 +1,10 @@
|
|||||||
# MxAccessGateway — Remediation Tracking (2026-07-12 review)
|
# 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.
|
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.
|
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.
|
||||||
|
|||||||
@@ -1,5 +1,10 @@
|
|||||||
# Candidate Findings for the Next Review Cycle (surfaced during 2026-07-12 remediation)
|
# 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.
|
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 |
|
| 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<T>` 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-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<T>` 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-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-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-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. |
|
| 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`.
|
- ~~**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** — 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.
|
- **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.
|
||||||
|
|||||||
@@ -1,5 +1,10 @@
|
|||||||
# MxAccessGateway — Remediation Plan & Tracking (2026-08-19 review)
|
# 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.
|
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.
|
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 |
|
| 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. |
|
| 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) | 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`. |
|
| 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`). |
|
| 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-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. |
|
| GWC-36, IPC-38, TST-33/34/36 | Done | Closed by continuation batch H; verification and commit are recorded below. |
|
||||||
|
|||||||
@@ -1,5 +1,8 @@
|
|||||||
# MxAccessGateway — Remediation Tracking (2026-10-21 system review)
|
# 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`.
|
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).
|
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.
|
- 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`).
|
- 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 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)`.
|
**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 |
|
| K | `fix/review-2026-10-21-k` | Tracker triage (this document) | Done |
|
||||||
| L | (stream L branch) | Proto comments, docs, test gaps | Done (merge 2fad124) |
|
| 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)
|
## Sessions (stream A)
|
||||||
|
|
||||||
| ID | Sev | Area | Description | Status | Commit / stream | Reason / note |
|
| 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-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-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-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-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-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-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-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)
|
## 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-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-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-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)
|
## 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-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-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-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-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 | — | Next step: refresh on first subscribe |
|
| 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 | — | Next step: await the pumps with a bounded timeout in `DisposeAsync` |
|
| 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)
|
## 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 |
|
| 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-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-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-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-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 | — | Security tradeoff (caching credentials' outcome); decide with the security owner before building |
|
| 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 | — | Low value |
|
| 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 | — | Steady GC cost, not a correctness issue; revisit if profiling flags it |
|
| 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)
|
## 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-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-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-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
|
## External / blocked
|
||||||
|
|
||||||
| ID | Sev | Area | Description | Status | Commit / stream | Reason / note |
|
| 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-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) | — | Web-UI action on the Gitea admin page |
|
| 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 |
|
| 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)
|
## 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 |
|
| 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-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 | Document the discard as v1 behavior (docs-only) |
|
| 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 | Add `exception_message` through `WorkerLogRedactor` in the three catch blocks; add a `WorkerApplicationTests` assertion |
|
| 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 | Refactor when `WorkerPipeSession` is next touched; existing IPC tests are the guard |
|
| 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 | Map `EndOfStream` to `PipeConnectionFailed`; add a `WorkerApplicationTests` case |
|
| 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 | 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-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 | Add level-gated `WorkerCommandStarted`/`WorkerCommandCompleted` in `StaCommandDispatcher.ExecuteQueuedCommandAsync` |
|
| 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 | 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-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 | 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-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 | Replace with a `TaskCompletionSource` signalled on state change |
|
| 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 | Throw a domain exception and map it to `InvalidArgument` in the gRPC layer |
|
| 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 | Declare the interface; check `await using` call sites |
|
| 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 | Delete it and fix the comments, or document it as the synchronous kill primitive |
|
| 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 | Split the option or rename it, and update `docs/GatewayConfiguration.md` |
|
| 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 | Expose both under `MxGateway:Worker:*` with validation, or record them as intentionally fixed |
|
| 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 | Add both bulk kinds plus a redactor test; cross-check every command kind that carries a password |
|
| 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 | Prefer an env var or stdin prompt; keep the flag only with a warning |
|
| 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 | Catch a typed exception from `ZB.MOM.WW.Auth.ApiKeys`, or check the pepper before the call |
|
| 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 | Add the cross-field check to `ValidateDashboard` |
|
| 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 | Project the four fields in `GatewayConfigurationProvider` |
|
| 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 | Make the outer budget `MaxAttempts x DefaultCallTimeout` (or drop the outer cap); add a wire test with a slow first attempt |
|
| 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 | Add sentinel or typed errors usable with `errors.Is`; keep credential redaction |
|
| 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 | Add `--require-certificate-validation` wired to `Options.RequireCertificateValidation` |
|
| 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 | Delete one (cosmetic) |
|
| 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 | Fall back to a counter-based id |
|
| 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 | Guard with an `asyncio.Lock` and cache the real reply, as .NET and Go do |
|
| 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 | Split by command group when next touched |
|
| 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 | Add one comparison table now that this cycle changed the Go and Java buffers |
|
| 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 | Convert to a fake time provider where one exists; otherwise leave |
|
| 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 | 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-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 | Write a deployment doc from the windev/wonder deploy notes; respect the current ban on remote execution on the wonder boxes |
|
| 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 | Add timer and startup tests with a fake time provider |
|
| 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 | Set `retainedFileCountLimit` and `fileSizeLimitBytes` in `appsettings.json`; document |
|
| 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 | Add the keys to the block (docs-only) |
|
| 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
|
## 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-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-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-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 |
|
||||||
|
|||||||
@@ -1,5 +1,10 @@
|
|||||||
# MxAccessGateway — Remediation Tracking
|
# 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.
|
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.
|
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.
|
||||||
|
|||||||
@@ -122,6 +122,11 @@ The .NET client applies a bounded Polly retry policy only to idempotent calls:
|
|||||||
streams, writes, secured writes, authentication, registration, item management,
|
streams, writes, secured writes, authentication, registration, item management,
|
||||||
or subscription changes because those calls can partially succeed in MXAccess.
|
or subscription changes because those calls can partially succeed in MXAccess.
|
||||||
|
|
||||||
|
Each attempt carries its own `DefaultCallTimeout` deadline. The overall budget
|
||||||
|
for a retried call is `MaxAttempts × DefaultCallTimeout` plus `MaxDelay` for each
|
||||||
|
backoff, so an attempt that ends in `DeadlineExceeded` leaves the retry its own
|
||||||
|
full deadline instead of consuming the whole budget.
|
||||||
|
|
||||||
API key may be loaded from `MXGATEWAY_API_KEY` by the CLI, not implicitly by the
|
API key may be loaded from `MXGATEWAY_API_KEY` by the CLI, not implicitly by the
|
||||||
library constructor unless a helper explicitly says it does that.
|
library constructor unless a helper explicitly says it does that.
|
||||||
|
|
||||||
|
|||||||
@@ -157,7 +157,9 @@ worker; call until empty). `DisposeAsync` closes the session best-effort and doe
|
|||||||
not throw when the gateway is unreachable or the client is already disposed.
|
not throw when the gateway is unreachable or the client is already disposed.
|
||||||
|
|
||||||
Retry applies only to idempotent calls (`CloseSessionRawAsync`, ping,
|
Retry applies only to idempotent calls (`CloseSessionRawAsync`, ping,
|
||||||
session-state and worker-info commands, Galaxy reads). `AcknowledgeAlarmAsync`
|
session-state and worker-info commands, Galaxy reads). Each attempt gets its own
|
||||||
|
`DefaultCallTimeout`; a retried call can take up to `Retry.MaxAttempts ×
|
||||||
|
DefaultCallTimeout` plus the backoff between attempts. `AcknowledgeAlarmAsync`
|
||||||
is never retried because it is a non-idempotent operator write that re-sends the
|
is never retried because it is a non-idempotent operator write that re-sends the
|
||||||
operator password and can double-acknowledge in the Historian.
|
operator password and can double-acknowledge in the Historian.
|
||||||
|
|
||||||
@@ -554,7 +556,7 @@ dotnet nuget add source https://gitea.dohertylan.com/api/packages/dohertj2/nuget
|
|||||||
Then add the package to your project:
|
Then add the package to your project:
|
||||||
|
|
||||||
````bash
|
````bash
|
||||||
dotnet add package ZB.MOM.WW.MxGateway.Client --version 0.2.0
|
dotnet add package ZB.MOM.WW.MxGateway.Client --version 0.3.0
|
||||||
````
|
````
|
||||||
|
|
||||||
The `ZB.MOM.WW.MxGateway.Contracts` package is pulled in transitively.
|
The `ZB.MOM.WW.MxGateway.Contracts` package is pulled in transitively.
|
||||||
|
|||||||
@@ -2,6 +2,39 @@ namespace ZB.MOM.WW.MxGateway.Client.Tests;
|
|||||||
|
|
||||||
public sealed class MxGatewayClientOptionsTests
|
public sealed class MxGatewayClientOptionsTests
|
||||||
{
|
{
|
||||||
|
/// <summary>
|
||||||
|
/// The safe-unary budget covers every attempt's deadline plus the backoff between them, so a
|
||||||
|
/// first attempt that hits its deadline does not consume the retry's time (#197).
|
||||||
|
/// </summary>
|
||||||
|
[Fact]
|
||||||
|
public void SafeUnaryBudget_CoversEveryAttemptAndBackoff()
|
||||||
|
{
|
||||||
|
var options = new MxGatewayClientOptions
|
||||||
|
{
|
||||||
|
Endpoint = new Uri("http://localhost:5000"),
|
||||||
|
ApiKey = "test-api-key",
|
||||||
|
DefaultCallTimeout = TimeSpan.FromSeconds(30),
|
||||||
|
Retry = new MxGatewayClientRetryOptions { MaxAttempts = 3, MaxDelay = TimeSpan.FromSeconds(2) },
|
||||||
|
};
|
||||||
|
|
||||||
|
Assert.Equal(TimeSpan.FromSeconds(94), MxGatewayClientRetryPolicy.SafeUnaryBudget(options));
|
||||||
|
}
|
||||||
|
|
||||||
|
/// <summary>A budget beyond what a cancellation timer accepts means no outer cap rather than a throw.</summary>
|
||||||
|
[Fact]
|
||||||
|
public void SafeUnaryBudget_WhenTooLargeForATimer_ReturnsNull()
|
||||||
|
{
|
||||||
|
var options = new MxGatewayClientOptions
|
||||||
|
{
|
||||||
|
Endpoint = new Uri("http://localhost:5000"),
|
||||||
|
ApiKey = "test-api-key",
|
||||||
|
DefaultCallTimeout = TimeSpan.FromDays(40),
|
||||||
|
Retry = new MxGatewayClientRetryOptions { MaxAttempts = 2 },
|
||||||
|
};
|
||||||
|
|
||||||
|
Assert.Null(MxGatewayClientRetryPolicy.SafeUnaryBudget(options));
|
||||||
|
}
|
||||||
|
|
||||||
/// <summary>Verifies that options with valid endpoint and API key pass validation.</summary>
|
/// <summary>Verifies that options with valid endpoint and API key pass validation.</summary>
|
||||||
[Fact]
|
[Fact]
|
||||||
public void Validate_WithAbsoluteEndpointAndApiKey_Succeeds()
|
public void Validate_WithAbsoluteEndpointAndApiKey_Succeeds()
|
||||||
|
|||||||
@@ -90,6 +90,41 @@ public sealed class MxGatewayClientWireTests
|
|||||||
server.Service.AuthorizationByMethod);
|
server.Service.AuthorizationByMethod);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// A safe unary call whose first attempt runs into its deadline must still get its retry:
|
||||||
|
/// the outer budget covers every attempt, not just one DefaultCallTimeout (#197).
|
||||||
|
/// </summary>
|
||||||
|
/// <returns>A task that represents the asynchronous operation.</returns>
|
||||||
|
[Fact]
|
||||||
|
public async Task SafeUnaryRetry_AfterDeadlineExceededFirstAttempt_RunsTheRetry()
|
||||||
|
{
|
||||||
|
await using WireFakeGatewayServer server = await WireFakeGatewayServer.StartAsync(service =>
|
||||||
|
service.StallFirstInvokes = 1);
|
||||||
|
await using MxGatewayClient client = server.CreateClient(
|
||||||
|
ApiKey,
|
||||||
|
TimeSpan.FromMilliseconds(500),
|
||||||
|
new MxGatewayClientRetryOptions
|
||||||
|
{
|
||||||
|
MaxAttempts = 2,
|
||||||
|
Delay = TimeSpan.FromMilliseconds(10),
|
||||||
|
MaxDelay = TimeSpan.FromMilliseconds(10),
|
||||||
|
UseJitter = false,
|
||||||
|
});
|
||||||
|
|
||||||
|
MxCommandReply reply = await client.InvokeAsync(new MxCommandRequest
|
||||||
|
{
|
||||||
|
SessionId = WireFakeGatewayServer.FakeGatewayService.SessionId,
|
||||||
|
Command = new MxCommand
|
||||||
|
{
|
||||||
|
Kind = MxCommandKind.Ping,
|
||||||
|
Ping = new PingCommand(),
|
||||||
|
},
|
||||||
|
});
|
||||||
|
|
||||||
|
Assert.Equal(ProtocolStatusCode.Ok, reply.ProtocolStatus.Code);
|
||||||
|
Assert.Equal(2, server.Service.InvokeCount);
|
||||||
|
}
|
||||||
|
|
||||||
/// <summary>
|
/// <summary>
|
||||||
/// Verifies the gateway's replay-gap sentinel survives serialization and is
|
/// Verifies the gateway's replay-gap sentinel survives serialization and is
|
||||||
/// surfaced as a typed, non-terminal stream item.
|
/// surfaced as a typed, non-terminal stream item.
|
||||||
|
|||||||
@@ -88,12 +88,26 @@ internal sealed class WireFakeGatewayServer : IAsyncDisposable
|
|||||||
/// <param name="apiKey">API key the client should present.</param>
|
/// <param name="apiKey">API key the client should present.</param>
|
||||||
/// <returns>A client that talks to this server over h2c.</returns>
|
/// <returns>A client that talks to this server over h2c.</returns>
|
||||||
public MxGatewayClient CreateClient(string apiKey) =>
|
public MxGatewayClient CreateClient(string apiKey) =>
|
||||||
|
CreateClient(apiKey, TimeSpan.FromSeconds(30), new MxGatewayClientRetryOptions());
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Creates a client bound to this server's endpoint with an explicit call timeout and retry policy.
|
||||||
|
/// </summary>
|
||||||
|
/// <param name="apiKey">API key the client should present.</param>
|
||||||
|
/// <param name="defaultCallTimeout">Per-attempt unary deadline.</param>
|
||||||
|
/// <param name="retry">Retry options for safe unary calls.</param>
|
||||||
|
/// <returns>A client that talks to this server over h2c.</returns>
|
||||||
|
public MxGatewayClient CreateClient(
|
||||||
|
string apiKey,
|
||||||
|
TimeSpan defaultCallTimeout,
|
||||||
|
MxGatewayClientRetryOptions retry) =>
|
||||||
MxGatewayClient.Create(new MxGatewayClientOptions
|
MxGatewayClient.Create(new MxGatewayClientOptions
|
||||||
{
|
{
|
||||||
Endpoint = Endpoint,
|
Endpoint = Endpoint,
|
||||||
ApiKey = apiKey,
|
ApiKey = apiKey,
|
||||||
UseTls = false,
|
UseTls = false,
|
||||||
DefaultCallTimeout = TimeSpan.FromSeconds(30),
|
DefaultCallTimeout = defaultCallTimeout,
|
||||||
|
Retry = retry,
|
||||||
});
|
});
|
||||||
|
|
||||||
/// <summary>Stops and disposes the underlying web application.</summary>
|
/// <summary>Stops and disposes the underlying web application.</summary>
|
||||||
@@ -145,6 +159,17 @@ internal sealed class WireFakeGatewayServer : IAsyncDisposable
|
|||||||
/// </summary>
|
/// </summary>
|
||||||
public ReplayGap? ReplayGap { get; set; }
|
public ReplayGap? ReplayGap { get; set; }
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Gets or sets how many leading <c>Invoke</c> calls stall until the client's deadline
|
||||||
|
/// expires, so the client sees <see cref="StatusCode.DeadlineExceeded"/> for them.
|
||||||
|
/// </summary>
|
||||||
|
public int StallFirstInvokes { get; set; }
|
||||||
|
|
||||||
|
/// <summary>Gets the number of <c>Invoke</c> calls the server received.</summary>
|
||||||
|
public int InvokeCount => Volatile.Read(ref _invokeCount);
|
||||||
|
|
||||||
|
private int _invokeCount;
|
||||||
|
|
||||||
/// <summary>
|
/// <summary>
|
||||||
/// Gets the last <c>Invoke</c> request the client sent, as decoded from the wire.
|
/// Gets the last <c>Invoke</c> request the client sent, as decoded from the wire.
|
||||||
/// </summary>
|
/// </summary>
|
||||||
@@ -179,17 +204,32 @@ internal sealed class WireFakeGatewayServer : IAsyncDisposable
|
|||||||
}
|
}
|
||||||
|
|
||||||
/// <inheritdoc />
|
/// <inheritdoc />
|
||||||
public override Task<MxCommandReply> Invoke(MxCommandRequest request, ServerCallContext context)
|
public override async Task<MxCommandReply> Invoke(MxCommandRequest request, ServerCallContext context)
|
||||||
{
|
{
|
||||||
Record(context);
|
Record(context);
|
||||||
InvokeRequest = request;
|
InvokeRequest = request;
|
||||||
|
|
||||||
|
if (Interlocked.Increment(ref _invokeCount) <= StallFirstInvokes)
|
||||||
|
{
|
||||||
|
// Hold the call past the client's deadline; the cancellation is the deadline
|
||||||
|
// firing (or the client giving up), either way the caller has stopped waiting.
|
||||||
|
try
|
||||||
|
{
|
||||||
|
await Task.Delay(Timeout.InfiniteTimeSpan, context.CancellationToken).ConfigureAwait(false);
|
||||||
|
}
|
||||||
|
catch (OperationCanceledException)
|
||||||
|
{
|
||||||
|
}
|
||||||
|
|
||||||
|
throw new RpcException(new Status(StatusCode.DeadlineExceeded, "stalled attempt"));
|
||||||
|
}
|
||||||
|
|
||||||
if (DenyInvoke)
|
if (DenyInvoke)
|
||||||
{
|
{
|
||||||
throw new RpcException(new Status(StatusCode.PermissionDenied, "invoke scope required"));
|
throw new RpcException(new Status(StatusCode.PermissionDenied, "invoke scope required"));
|
||||||
}
|
}
|
||||||
|
|
||||||
return Task.FromResult(new MxCommandReply
|
return new MxCommandReply
|
||||||
{
|
{
|
||||||
SessionId = request.SessionId,
|
SessionId = request.SessionId,
|
||||||
CorrelationId = request.ClientCorrelationId,
|
CorrelationId = request.ClientCorrelationId,
|
||||||
@@ -197,7 +237,7 @@ internal sealed class WireFakeGatewayServer : IAsyncDisposable
|
|||||||
ProtocolStatus = Ok(),
|
ProtocolStatus = Ok(),
|
||||||
Hresult = 0,
|
Hresult = 0,
|
||||||
Register = new RegisterReply { ServerHandle = ServerHandle },
|
Register = new RegisterReply { ServerHandle = ServerHandle },
|
||||||
});
|
};
|
||||||
}
|
}
|
||||||
|
|
||||||
/// <inheritdoc />
|
/// <inheritdoc />
|
||||||
|
|||||||
@@ -517,8 +517,15 @@ public sealed class GalaxyRepositoryClient : IAsyncDisposable
|
|||||||
Func<CancellationToken, Task<T>> call,
|
Func<CancellationToken, Task<T>> call,
|
||||||
CancellationToken cancellationToken)
|
CancellationToken cancellationToken)
|
||||||
{
|
{
|
||||||
|
// Each attempt carries its own DefaultCallTimeout deadline, so the outer cap must cover
|
||||||
|
// every attempt plus the backoff between them; capping it at one DefaultCallTimeout let a
|
||||||
|
// DeadlineExceeded first attempt spend the whole budget and the retry never ran (#197).
|
||||||
using CancellationTokenSource timeout = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken);
|
using CancellationTokenSource timeout = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken);
|
||||||
timeout.CancelAfter(Options.DefaultCallTimeout);
|
TimeSpan? budget = MxGatewayClientRetryPolicy.SafeUnaryBudget(Options);
|
||||||
|
if (budget is not null)
|
||||||
|
{
|
||||||
|
timeout.CancelAfter(budget.Value);
|
||||||
|
}
|
||||||
|
|
||||||
return await _safeUnaryRetryPipeline.ExecuteAsync(
|
return await _safeUnaryRetryPipeline.ExecuteAsync(
|
||||||
async token => await call(token).ConfigureAwait(false),
|
async token => await call(token).ConfigureAwait(false),
|
||||||
|
|||||||
@@ -375,8 +375,15 @@ public sealed class MxGatewayClient : IAsyncDisposable
|
|||||||
Func<CancellationToken, Task<T>> call,
|
Func<CancellationToken, Task<T>> call,
|
||||||
CancellationToken cancellationToken)
|
CancellationToken cancellationToken)
|
||||||
{
|
{
|
||||||
|
// Each attempt carries its own DefaultCallTimeout deadline, so the outer cap must cover
|
||||||
|
// every attempt plus the backoff between them; capping it at one DefaultCallTimeout let a
|
||||||
|
// DeadlineExceeded first attempt spend the whole budget and the retry never ran (#197).
|
||||||
using CancellationTokenSource timeout = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken);
|
using CancellationTokenSource timeout = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken);
|
||||||
timeout.CancelAfter(Options.DefaultCallTimeout);
|
TimeSpan? budget = MxGatewayClientRetryPolicy.SafeUnaryBudget(Options);
|
||||||
|
if (budget is not null)
|
||||||
|
{
|
||||||
|
timeout.CancelAfter(budget.Value);
|
||||||
|
}
|
||||||
|
|
||||||
return await _safeUnaryRetryPipeline.ExecuteAsync(
|
return await _safeUnaryRetryPipeline.ExecuteAsync(
|
||||||
async token => await call(token).ConfigureAwait(false),
|
async token => await call(token).ConfigureAwait(false),
|
||||||
|
|||||||
@@ -41,6 +41,29 @@ internal static class MxGatewayClientRetryPolicy
|
|||||||
.Build();
|
.Build();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Returns the overall budget for one retried safe unary call: every attempt's
|
||||||
|
/// <see cref="MxGatewayClientOptions.DefaultCallTimeout"/> deadline plus the longest backoff
|
||||||
|
/// between attempts. Returns <see langword="null"/> when the budget is too large for a
|
||||||
|
/// cancellation timer, in which case the per-attempt deadlines and the bounded attempt count
|
||||||
|
/// already bound the call.
|
||||||
|
/// </summary>
|
||||||
|
/// <param name="options">The client options supplying the call timeout and retry settings.</param>
|
||||||
|
/// <returns>The overall budget, or <see langword="null"/> for no outer cap.</returns>
|
||||||
|
public static TimeSpan? SafeUnaryBudget(MxGatewayClientOptions options)
|
||||||
|
{
|
||||||
|
ArgumentNullException.ThrowIfNull(options);
|
||||||
|
|
||||||
|
int attempts = Math.Max(1, options.Retry.MaxAttempts);
|
||||||
|
double milliseconds = (options.DefaultCallTimeout.TotalMilliseconds * attempts)
|
||||||
|
+ (options.Retry.MaxDelay.TotalMilliseconds * (attempts - 1));
|
||||||
|
|
||||||
|
// CancellationTokenSource.CancelAfter rejects anything above uint.MaxValue - 1 ms.
|
||||||
|
return milliseconds >= uint.MaxValue - 1
|
||||||
|
? null
|
||||||
|
: TimeSpan.FromMilliseconds(milliseconds);
|
||||||
|
}
|
||||||
|
|
||||||
/// <summary>Returns whether a command kind is eligible for automatic retry on transient failures.</summary>
|
/// <summary>Returns whether a command kind is eligible for automatic retry on transient failures.</summary>
|
||||||
/// <param name="kind">The command kind to check.</param>
|
/// <param name="kind">The command kind to check.</param>
|
||||||
/// <returns><see langword="true"/> if the command kind may be automatically retried; otherwise, <see langword="false"/>.</returns>
|
/// <returns><see langword="true"/> if the command kind may be automatically retried; otherwise, <see langword="false"/>.</returns>
|
||||||
|
|||||||
@@ -19,7 +19,7 @@
|
|||||||
<PropertyGroup>
|
<PropertyGroup>
|
||||||
<IsPackable>true</IsPackable>
|
<IsPackable>true</IsPackable>
|
||||||
<PackageId>ZB.MOM.WW.MxGateway.Client</PackageId>
|
<PackageId>ZB.MOM.WW.MxGateway.Client</PackageId>
|
||||||
<Version>0.2.1</Version>
|
<Version>0.3.0</Version>
|
||||||
<Description>.NET 10 gRPC client for the MxAccessGateway service. Provides typed wrappers, retry, and a lazy-browse walker over the Galaxy Repository hierarchy.</Description>
|
<Description>.NET 10 gRPC client for the MxAccessGateway service. Provides typed wrappers, retry, and a lazy-browse walker over the Galaxy Repository hierarchy.</Description>
|
||||||
<PackageReadmeFile>README.md</PackageReadmeFile>
|
<PackageReadmeFile>README.md</PackageReadmeFile>
|
||||||
<!-- Only the shipped library generates XML docs (matching src/Contracts). The Cli and
|
<!-- Only the shipped library generates XML docs (matching src/Contracts). The Cli and
|
||||||
|
|||||||
@@ -146,7 +146,10 @@ type MxAccessError struct { ... }
|
|||||||
```
|
```
|
||||||
|
|
||||||
Use `errors.Is` / `errors.As` support. Preserve raw protobuf replies on command
|
Use `errors.Is` / `errors.As` support. Preserve raw protobuf replies on command
|
||||||
errors.
|
errors. `GatewayError` matches the sentinels `ErrUnauthenticated` and
|
||||||
|
`ErrPermissionDenied` through `errors.Is` by gRPC status code, so callers treat
|
||||||
|
the two authorization failures distinctly (`docs/ClientLibrariesDesign.md`)
|
||||||
|
without parsing messages.
|
||||||
|
|
||||||
## Test CLI
|
## Test CLI
|
||||||
|
|
||||||
|
|||||||
+12
-2
@@ -92,7 +92,13 @@ returned subscription owns cancellation and exposes `Close` for deterministic
|
|||||||
goroutine cleanup. Raw protobuf messages remain available through the
|
goroutine cleanup. Raw protobuf messages remain available through the
|
||||||
`mxgateway` package aliases and the `Raw` helper methods. Typed errors support
|
`mxgateway` package aliases and the `Raw` helper methods. Typed errors support
|
||||||
`errors.As` for `GatewayError`, `CommandError`, and `MxAccessError`; command
|
`errors.As` for `GatewayError`, `CommandError`, and `MxAccessError`; command
|
||||||
errors preserve the raw reply.
|
errors preserve the raw reply. Authorization failures are distinct:
|
||||||
|
`errors.Is(err, mxgateway.ErrUnauthenticated)` matches a rejected API key
|
||||||
|
(gRPC `Unauthenticated` — fix the key) and `errors.Is(err,
|
||||||
|
mxgateway.ErrPermissionDenied)` matches a valid key missing a scope or another
|
||||||
|
key's session (gRPC `PermissionDenied` — grant the scope). `GatewayError.Code()`
|
||||||
|
returns the raw gRPC status code. The match reads only the status code, so it
|
||||||
|
works through the credential-redaction wrapper.
|
||||||
|
|
||||||
`EnsureMxAccessSuccess` follows COM semantics: only a **negative** HRESULT is a
|
`EnsureMxAccessSuccess` follows COM semantics: only a **negative** HRESULT is a
|
||||||
failure, so positive success codes such as `S_FALSE` (1) pass. `StatusSucceeded`
|
failure, so positive success codes such as `S_FALSE` (1) pass. `StatusSucceeded`
|
||||||
@@ -426,7 +432,11 @@ the gateway:
|
|||||||
|
|
||||||
Every subcommand wired into the CLI. All accept the common flags
|
Every subcommand wired into the CLI. All accept the common flags
|
||||||
(`-endpoint`, `-plaintext`, `-api-key` / `-api-key-env`, `-ca-cert`,
|
(`-endpoint`, `-plaintext`, `-api-key` / `-api-key-env`, `-ca-cert`,
|
||||||
`-server-name-override`, `-call-timeout`) and most accept `-json`.
|
`-server-name-override`, `-call-timeout`, `-require-certificate-validation`) and
|
||||||
|
most accept `-json`. Over TLS without `-ca-cert` the CLI accepts any gateway
|
||||||
|
certificate (the self-signed default); `-require-certificate-validation` sets
|
||||||
|
`Options.RequireCertificateValidation` so the certificate must chain to the
|
||||||
|
system trust roots instead.
|
||||||
|
|
||||||
| Command | Purpose |
|
| Command | Purpose |
|
||||||
|---|---|
|
|---|---|
|
||||||
|
|||||||
@@ -40,6 +40,10 @@ type commonOptions struct {
|
|||||||
CACertFile string `json:"caCertFile,omitempty"`
|
CACertFile string `json:"caCertFile,omitempty"`
|
||||||
ServerName string `json:"serverNameOverride,omitempty"`
|
ServerName string `json:"serverNameOverride,omitempty"`
|
||||||
CallTimeout string `json:"callTimeout,omitempty"`
|
CallTimeout string `json:"callTimeout,omitempty"`
|
||||||
|
// RequireCertificateValidation verifies the gateway certificate against the
|
||||||
|
// system trust roots when no -ca-cert is pinned, instead of the lenient
|
||||||
|
// self-signed default.
|
||||||
|
RequireCertificateValidation bool `json:"requireCertificateValidation,omitempty"`
|
||||||
|
|
||||||
apiKeyValue string
|
apiKeyValue string
|
||||||
timeout time.Duration
|
timeout time.Duration
|
||||||
@@ -1297,6 +1301,8 @@ func bindCommonFlags(flags *flag.FlagSet) *commonOptions {
|
|||||||
flags.StringVar(&common.CACertFile, "ca-cert", "", "CA certificate file")
|
flags.StringVar(&common.CACertFile, "ca-cert", "", "CA certificate file")
|
||||||
flags.StringVar(&common.ServerName, "server-name-override", "", "TLS server name override")
|
flags.StringVar(&common.ServerName, "server-name-override", "", "TLS server name override")
|
||||||
flags.StringVar(&common.CallTimeout, "call-timeout", "30s", "per-call timeout")
|
flags.StringVar(&common.CallTimeout, "call-timeout", "30s", "per-call timeout")
|
||||||
|
flags.BoolVar(&common.RequireCertificateValidation, "require-certificate-validation", false,
|
||||||
|
"verify the gateway TLS certificate against the system trust roots when no -ca-cert is set (default: accept any certificate)")
|
||||||
return common
|
return common
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -1307,12 +1313,13 @@ func dialForCommand(ctx context.Context, common *commonOptions) (*mxgateway.Clie
|
|||||||
}
|
}
|
||||||
|
|
||||||
client, err := mxgateway.Dial(ctx, mxgateway.Options{
|
client, err := mxgateway.Dial(ctx, mxgateway.Options{
|
||||||
Endpoint: options.Endpoint,
|
Endpoint: options.Endpoint,
|
||||||
APIKey: options.apiKeyValue,
|
APIKey: options.apiKeyValue,
|
||||||
Plaintext: options.Plaintext,
|
Plaintext: options.Plaintext,
|
||||||
CACertFile: options.CACertFile,
|
CACertFile: options.CACertFile,
|
||||||
ServerNameOverride: options.ServerName,
|
ServerNameOverride: options.ServerName,
|
||||||
CallTimeout: options.timeout,
|
CallTimeout: options.timeout,
|
||||||
|
RequireCertificateValidation: options.RequireCertificateValidation,
|
||||||
})
|
})
|
||||||
return client, options, err
|
return client, options, err
|
||||||
}
|
}
|
||||||
@@ -1523,12 +1530,13 @@ func dialGalaxyForCommand(ctx context.Context, common *commonOptions) (*mxgatewa
|
|||||||
}
|
}
|
||||||
|
|
||||||
client, err := mxgateway.DialGalaxy(ctx, mxgateway.Options{
|
client, err := mxgateway.DialGalaxy(ctx, mxgateway.Options{
|
||||||
Endpoint: options.Endpoint,
|
Endpoint: options.Endpoint,
|
||||||
APIKey: options.apiKeyValue,
|
APIKey: options.apiKeyValue,
|
||||||
Plaintext: options.Plaintext,
|
Plaintext: options.Plaintext,
|
||||||
CACertFile: options.CACertFile,
|
CACertFile: options.CACertFile,
|
||||||
ServerNameOverride: options.ServerName,
|
ServerNameOverride: options.ServerName,
|
||||||
CallTimeout: options.timeout,
|
CallTimeout: options.timeout,
|
||||||
|
RequireCertificateValidation: options.RequireCertificateValidation,
|
||||||
})
|
})
|
||||||
return client, options, err
|
return client, options, err
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -0,0 +1,105 @@
|
|||||||
|
package main
|
||||||
|
|
||||||
|
import (
|
||||||
|
"bytes"
|
||||||
|
"context"
|
||||||
|
"crypto/ecdsa"
|
||||||
|
"crypto/elliptic"
|
||||||
|
"crypto/rand"
|
||||||
|
"crypto/tls"
|
||||||
|
"crypto/x509"
|
||||||
|
"crypto/x509/pkix"
|
||||||
|
"math/big"
|
||||||
|
"net"
|
||||||
|
"strings"
|
||||||
|
"testing"
|
||||||
|
"time"
|
||||||
|
|
||||||
|
pb "gitea.dohertylan.com/dohertj2/mxaccessgw/clients/go/internal/generated"
|
||||||
|
"google.golang.org/grpc"
|
||||||
|
"google.golang.org/grpc/credentials"
|
||||||
|
)
|
||||||
|
|
||||||
|
// #199 — `--tls` without a CA used to be skip-verify with no way out from the
|
||||||
|
// CLI. -require-certificate-validation must make the CLI reject a certificate
|
||||||
|
// the system roots do not trust (the gateway's own self-signed one).
|
||||||
|
|
||||||
|
type openSessionFakeGateway struct {
|
||||||
|
pb.UnimplementedMxAccessGatewayServer
|
||||||
|
}
|
||||||
|
|
||||||
|
func (g *openSessionFakeGateway) OpenSession(context.Context, *pb.OpenSessionRequest) (*pb.OpenSessionReply, error) {
|
||||||
|
return &pb.OpenSessionReply{
|
||||||
|
SessionId: "tls-session",
|
||||||
|
ProtocolStatus: &pb.ProtocolStatus{Code: pb.ProtocolStatusCode_PROTOCOL_STATUS_CODE_OK},
|
||||||
|
}, nil
|
||||||
|
}
|
||||||
|
|
||||||
|
func startSelfSignedTLSGateway(t *testing.T) string {
|
||||||
|
t.Helper()
|
||||||
|
key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("generate key: %v", err)
|
||||||
|
}
|
||||||
|
template := &x509.Certificate{
|
||||||
|
SerialNumber: big.NewInt(1),
|
||||||
|
Subject: pkix.Name{CommonName: "localhost"},
|
||||||
|
DNSNames: []string{"localhost"},
|
||||||
|
IPAddresses: []net.IP{net.ParseIP("127.0.0.1")},
|
||||||
|
NotBefore: time.Now().Add(-time.Hour),
|
||||||
|
NotAfter: time.Now().Add(time.Hour),
|
||||||
|
KeyUsage: x509.KeyUsageDigitalSignature,
|
||||||
|
ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth},
|
||||||
|
}
|
||||||
|
der, err := x509.CreateCertificate(rand.Reader, template, template, &key.PublicKey, key)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("create certificate: %v", err)
|
||||||
|
}
|
||||||
|
cert := tls.Certificate{Certificate: [][]byte{der}, PrivateKey: key}
|
||||||
|
|
||||||
|
listener, err := net.Listen("tcp", "127.0.0.1:0")
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("listen: %v", err)
|
||||||
|
}
|
||||||
|
server := grpc.NewServer(grpc.Creds(credentials.NewTLS(&tls.Config{
|
||||||
|
Certificates: []tls.Certificate{cert},
|
||||||
|
MinVersion: tls.VersionTLS12,
|
||||||
|
})))
|
||||||
|
pb.RegisterMxAccessGatewayServer(server, &openSessionFakeGateway{})
|
||||||
|
go func() { _ = server.Serve(listener) }()
|
||||||
|
t.Cleanup(func() {
|
||||||
|
server.Stop()
|
||||||
|
_ = listener.Close()
|
||||||
|
})
|
||||||
|
return listener.Addr().String()
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestRequireCertificateValidationFlagRejectsSelfSignedGateway(t *testing.T) {
|
||||||
|
endpoint := startSelfSignedTLSGateway(t)
|
||||||
|
ctx, cancel := context.WithTimeout(t.Context(), 10*time.Second)
|
||||||
|
defer cancel()
|
||||||
|
base := []string{"open-session", "-endpoint", endpoint, "-api-key", "test", "-call-timeout", "5s"}
|
||||||
|
|
||||||
|
// Positive control: the lenient default accepts the self-signed certificate.
|
||||||
|
var okOut, okErr bytes.Buffer
|
||||||
|
if err := runWithIO(ctx, base, &okOut, &okErr); err != nil {
|
||||||
|
t.Fatalf("default TLS open-session error = %v; stderr = %s", err, okErr.String())
|
||||||
|
}
|
||||||
|
if got := strings.TrimSpace(okOut.String()); got != "tls-session" {
|
||||||
|
t.Fatalf("default TLS open-session output = %q", got)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Strict: the handshake never verifies, so the connected-dial wait runs out.
|
||||||
|
// The dial error (not a flag-parse error) is what proves the flag reached the
|
||||||
|
// TLS configuration; the positive control above proves the endpoint is live.
|
||||||
|
strictCtx, strictCancel := context.WithTimeout(t.Context(), 2*time.Second)
|
||||||
|
defer strictCancel()
|
||||||
|
var strictOut, strictErr bytes.Buffer
|
||||||
|
err := runWithIO(strictCtx, append(append([]string{}, base...), "-require-certificate-validation"), &strictOut, &strictErr)
|
||||||
|
if err == nil {
|
||||||
|
t.Fatalf("strict TLS open-session succeeded against a self-signed gateway; stdout = %s", strictOut.String())
|
||||||
|
}
|
||||||
|
if !strings.Contains(err.Error(), "dial failed") {
|
||||||
|
t.Fatalf("strict TLS open-session error = %v; want a dial failure (stderr = %s)", err, strictErr.String())
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -6,6 +6,8 @@ import (
|
|||||||
"strings"
|
"strings"
|
||||||
|
|
||||||
pb "gitea.dohertylan.com/dohertj2/mxaccessgw/clients/go/internal/generated"
|
pb "gitea.dohertylan.com/dohertj2/mxaccessgw/clients/go/internal/generated"
|
||||||
|
"google.golang.org/grpc/codes"
|
||||||
|
"google.golang.org/grpc/status"
|
||||||
"google.golang.org/protobuf/proto"
|
"google.golang.org/protobuf/proto"
|
||||||
)
|
)
|
||||||
|
|
||||||
@@ -180,7 +182,20 @@ func (e *MalformedReplyError) Error() string {
|
|||||||
return fmt.Sprintf("mxgateway: %s returned a malformed reply: %s", e.Op, e.Detail)
|
return fmt.Sprintf("mxgateway: %s returned a malformed reply: %s", e.Op, e.Detail)
|
||||||
}
|
}
|
||||||
|
|
||||||
// GatewayError wraps transport-level gRPC failures.
|
// ErrUnauthenticated matches, via errors.Is, a GatewayError whose gRPC status is
|
||||||
|
// Unauthenticated: the gateway did not accept the API key (missing, malformed,
|
||||||
|
// unknown, or revoked). Retrying with the same key will not help.
|
||||||
|
var ErrUnauthenticated = errors.New("mxgateway: unauthenticated")
|
||||||
|
|
||||||
|
// ErrPermissionDenied matches, via errors.Is, a GatewayError whose gRPC status
|
||||||
|
// is PermissionDenied: the API key is valid but lacks the scope the RPC needs, or
|
||||||
|
// the session belongs to another key. It is distinct from ErrUnauthenticated so a
|
||||||
|
// caller can tell "fix the key" from "grant the key a scope".
|
||||||
|
var ErrPermissionDenied = errors.New("mxgateway: permission denied")
|
||||||
|
|
||||||
|
// GatewayError wraps transport-level gRPC failures. errors.Is(err,
|
||||||
|
// ErrUnauthenticated) and errors.Is(err, ErrPermissionDenied) classify the two
|
||||||
|
// authorization failures; Code returns the raw gRPC status code.
|
||||||
type GatewayError struct {
|
type GatewayError struct {
|
||||||
// Op names the operation that failed (for example "dial" or "invoke").
|
// Op names the operation that failed (for example "dial" or "invoke").
|
||||||
Op string
|
Op string
|
||||||
@@ -207,6 +222,32 @@ func (e *GatewayError) Unwrap() error {
|
|||||||
return e.Err
|
return e.Err
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Code returns the gRPC status code of the wrapped error, codes.Unknown for a
|
||||||
|
// non-gRPC error, and codes.OK for a nil receiver or nil wrapped error.
|
||||||
|
func (e *GatewayError) Code() codes.Code {
|
||||||
|
if e == nil || e.Err == nil {
|
||||||
|
return codes.OK
|
||||||
|
}
|
||||||
|
return status.Code(e.Err)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Is reports whether target is the sentinel matching this error's gRPC status
|
||||||
|
// code (ErrUnauthenticated or ErrPermissionDenied). The check reads only the
|
||||||
|
// status code, never the message, so credential redaction is unaffected.
|
||||||
|
func (e *GatewayError) Is(target error) bool {
|
||||||
|
if e == nil {
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
switch target {
|
||||||
|
case ErrUnauthenticated:
|
||||||
|
return e.Code() == codes.Unauthenticated
|
||||||
|
case ErrPermissionDenied:
|
||||||
|
return e.Code() == codes.PermissionDenied
|
||||||
|
default:
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// CommandError reports a non-OK gateway protocol status and keeps the raw
|
// CommandError reports a non-OK gateway protocol status and keeps the raw
|
||||||
// command reply when one exists.
|
// command reply when one exists.
|
||||||
type CommandError struct {
|
type CommandError struct {
|
||||||
|
|||||||
@@ -0,0 +1,85 @@
|
|||||||
|
package mxgateway
|
||||||
|
|
||||||
|
import (
|
||||||
|
"context"
|
||||||
|
"errors"
|
||||||
|
"fmt"
|
||||||
|
"strings"
|
||||||
|
"testing"
|
||||||
|
|
||||||
|
pb "gitea.dohertylan.com/dohertj2/mxaccessgw/clients/go/internal/generated"
|
||||||
|
"google.golang.org/grpc/codes"
|
||||||
|
"google.golang.org/grpc/status"
|
||||||
|
)
|
||||||
|
|
||||||
|
// #198 — the gateway answers a bad key with Unauthenticated and a missing scope
|
||||||
|
// with PermissionDenied; callers must be able to tell them apart with errors.Is.
|
||||||
|
|
||||||
|
func TestGatewayErrorMatchesAuthSentinelsByStatusCode(t *testing.T) {
|
||||||
|
cases := []struct {
|
||||||
|
name string
|
||||||
|
err error
|
||||||
|
unauthenticated bool
|
||||||
|
permissionDenied bool
|
||||||
|
}{
|
||||||
|
{"unauthenticated", status.Error(codes.Unauthenticated, "bad key"), true, false},
|
||||||
|
{"permission denied", status.Error(codes.PermissionDenied, "scope required"), false, true},
|
||||||
|
{"unavailable", status.Error(codes.Unavailable, "down"), false, false},
|
||||||
|
{"non-grpc", errors.New("plain"), false, false},
|
||||||
|
}
|
||||||
|
for _, tc := range cases {
|
||||||
|
t.Run(tc.name, func(t *testing.T) {
|
||||||
|
err := fmt.Errorf("outer: %w", &GatewayError{Op: "invoke", Err: tc.err})
|
||||||
|
if got := errors.Is(err, ErrUnauthenticated); got != tc.unauthenticated {
|
||||||
|
t.Fatalf("errors.Is(ErrUnauthenticated) = %v, want %v", got, tc.unauthenticated)
|
||||||
|
}
|
||||||
|
if got := errors.Is(err, ErrPermissionDenied); got != tc.permissionDenied {
|
||||||
|
t.Fatalf("errors.Is(ErrPermissionDenied) = %v, want %v", got, tc.permissionDenied)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestGatewayErrorCode(t *testing.T) {
|
||||||
|
if got := (&GatewayError{Err: status.Error(codes.PermissionDenied, "x")}).Code(); got != codes.PermissionDenied {
|
||||||
|
t.Fatalf("Code() = %v", got)
|
||||||
|
}
|
||||||
|
if got := (*GatewayError)(nil).Code(); got != codes.OK {
|
||||||
|
t.Fatalf("nil Code() = %v", got)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestAuthSentinelsSurviveTheWireAndRedaction(t *testing.T) {
|
||||||
|
for _, tc := range []struct {
|
||||||
|
code codes.Code
|
||||||
|
sentinel error
|
||||||
|
other error
|
||||||
|
}{
|
||||||
|
{codes.Unauthenticated, ErrUnauthenticated, ErrPermissionDenied},
|
||||||
|
{codes.PermissionDenied, ErrPermissionDenied, ErrUnauthenticated},
|
||||||
|
} {
|
||||||
|
t.Run(tc.code.String(), func(t *testing.T) {
|
||||||
|
fake := &fakeGatewayWithAlarms{acknowledgeError: status.Error(tc.code, "denied pw-secret")}
|
||||||
|
client, cleanup := newBufconnClientWithAlarms(t, fake)
|
||||||
|
defer cleanup()
|
||||||
|
|
||||||
|
_, err := client.AcknowledgeAlarm(context.Background(), &pb.AcknowledgeAlarmRequest{
|
||||||
|
AlarmFullReference: "Tank01.Level.HiHi",
|
||||||
|
OperatorUser: "alice",
|
||||||
|
OperatorPassword: "pw-secret",
|
||||||
|
})
|
||||||
|
if err == nil {
|
||||||
|
t.Fatal("AcknowledgeAlarm() returned no error")
|
||||||
|
}
|
||||||
|
if !errors.Is(err, tc.sentinel) {
|
||||||
|
t.Fatalf("errors.Is(%v) = false for %v", tc.sentinel, err)
|
||||||
|
}
|
||||||
|
if errors.Is(err, tc.other) {
|
||||||
|
t.Fatalf("errors.Is(%v) = true for %v", tc.other, err)
|
||||||
|
}
|
||||||
|
if strings.Contains(err.Error(), "pw-secret") {
|
||||||
|
t.Fatalf("error leaked the operator password: %v", err)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -1,5 +1,8 @@
|
|||||||
# Code Review — IntegrationTests
|
# 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 |
|
| Field | Value |
|
||||||
|---|---|
|
|---|---|
|
||||||
| Module | `src/ZB.MOM.WW.MxGateway.IntegrationTests` |
|
| Module | `src/ZB.MOM.WW.MxGateway.IntegrationTests` |
|
||||||
@@ -710,4 +713,4 @@ The prior `DashboardAuthenticator` ctor took `IOptions<GatewayOptions>`, 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.
|
**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
|
||||||
|
|||||||
@@ -128,6 +128,15 @@ confirmed present through the Gitea package API after the push, and
|
|||||||
`go list -m` resolves the Go tag. These are the numbers a future release
|
`go list -m` resolves the Go tag. These are the numbers a future release
|
||||||
bumps off.
|
bumps off.
|
||||||
|
|
||||||
|
On 2026-11-06 (#214) the .NET pair alone moved to **0.3.0**: `nuget`
|
||||||
|
`ZB.MOM.WW.MxGateway.Contracts` **0.3.0** and `ZB.MOM.WW.MxGateway.Client`
|
||||||
|
**0.3.0**. The release carries the additive `AcknowledgeAlarmRequest`
|
||||||
|
`operator_password` / `operator_domain` fields from `488649c`. The feed's
|
||||||
|
0.2.1 predates them, so OtOpcUa could not consume operator-authenticated
|
||||||
|
alarm ack. The minor bump marks a new wire field. The Python, Rust, Java and
|
||||||
|
Go clients were not republished in that release, and their manifests were
|
||||||
|
left alone.
|
||||||
|
|
||||||
**The published pypi `0.2.0` artifact has a broken `connect()` (CLI-46).**
|
**The published pypi `0.2.0` artifact has a broken `connect()` (CLI-46).**
|
||||||
Its `connect()` built the `grpc.aio` channel inside `asyncio.to_thread`, and
|
Its `connect()` built the `grpc.aio` channel inside `asyncio.to_thread`, and
|
||||||
a `grpc.aio` channel binds to the event loop current on the *constructing*
|
a `grpc.aio` channel binds to the event loop current on the *constructing*
|
||||||
|
|||||||
+5
-2
@@ -71,6 +71,8 @@ private static readonly HashSet<string> SensitiveCommandMethods = new(StringComp
|
|||||||
"AuthenticateUser",
|
"AuthenticateUser",
|
||||||
"WriteSecured",
|
"WriteSecured",
|
||||||
"WriteSecured2",
|
"WriteSecured2",
|
||||||
|
"WriteSecuredBulk",
|
||||||
|
"WriteSecured2Bulk",
|
||||||
"AcknowledgeAlarm",
|
"AcknowledgeAlarm",
|
||||||
"AcknowledgeAlarmByName"
|
"AcknowledgeAlarmByName"
|
||||||
};
|
};
|
||||||
@@ -82,7 +84,7 @@ public static bool IsCredentialBearingCommand(string? commandMethod)
|
|||||||
}
|
}
|
||||||
```
|
```
|
||||||
|
|
||||||
The names match the MXAccess command list in `gateway.md` exactly. `Write` and `Write2` are not in the set because their payloads are tag values, not credentials, and are governed by the `valueLoggingEnabled` flag described below. `AcknowledgeAlarm` and `AcknowledgeAlarmByName` are in the set because their commands carry the operator's `operator_password`. Independently of the method set, the name-based seam redacts any property whose name contains `password`, so a credential field added to a payload later is redacted without a code change.
|
The names match the MXAccess command list in `gateway.md` exactly. `Write` and `Write2` are not in the set because their payloads are tag values, not credentials, and are governed by the `valueLoggingEnabled` flag described below. `WriteSecured`, `WriteSecured2`, `WriteSecuredBulk` and `WriteSecured2Bulk` are in the set because every secured write — single-item or bulk — carries a credential-sensitive value. `AcknowledgeAlarm` and `AcknowledgeAlarmByName` are in the set because their commands carry the operator's `operator_password`. `GatewayLogRedactorTests` walks the `MxCommand` payload descriptors and fails if a secured or password-bearing command kind is missing from the set. Independently of the method set, the name-based seam redacts any property whose name contains `password`, so a credential field added to a payload later is redacted without a code change.
|
||||||
|
|
||||||
### API key redaction
|
### API key redaction
|
||||||
|
|
||||||
@@ -216,11 +218,12 @@ The order matters: putting the logging scope first ensures that authentication f
|
|||||||
## Health Checks
|
## Health Checks
|
||||||
|
|
||||||
The shared `ZB.MOM.WW.Health` package maps three endpoints — `/healthz` (live), `/health/ready`, and
|
The shared `ZB.MOM.WW.Health` package maps three endpoints — `/healthz` (live), `/health/ready`, and
|
||||||
`/health/active` — and each registered check opts into a tier by tag. The gateway registers two:
|
`/health/active` — and each registered check opts into a tier by tag. The gateway registers three:
|
||||||
|
|
||||||
| Check | Endpoint tier | Fails when |
|
| Check | Endpoint tier | Fails when |
|
||||||
|---|---|---|
|
|---|---|---|
|
||||||
| `auth-store` | `ready` | The SQLite auth store cannot be opened. Every gRPC call authenticates against it, so its reachability genuinely gates whether the process should receive traffic. |
|
| `auth-store` | `ready` | The SQLite auth store cannot be opened. Every gRPC call authenticates against it, so its reachability genuinely gates whether the process should receive traffic. |
|
||||||
|
| `worker-executable` | `ready` | The configured `MxGateway:Worker:ExecutablePath` is not launchable — invalid path, not a `.exe`, missing, or not a PE image of `RequiredArchitecture`. It applies the launcher's own rule (`WorkerExecutableValidator.ResolveAndValidate`), so the probe and `OpenSession` cannot disagree. Every session launches the worker, so a gateway without one cannot serve MXAccess. It reports only the launch error code as entry `data` (`error_code`), never the path, because `/health/ready` is anonymous. It does not depend on session count. |
|
||||||
| `mxaccess-sessions` | `active` | Sessions exist and their workers have faulted. Reports `total` / `ready` / `faulted` / `starting` / `closing` as entry `data`. |
|
| `mxaccess-sessions` | `active` | Sessions exist and their workers have faulted. Reports `total` / `ready` / `faulted` / `starting` / `closing` as entry `data`. |
|
||||||
|
|
||||||
**Zero sessions is Healthy, and the tier choice follows from that.** The gateway opens an MXAccess
|
**Zero sessions is Healthy, and the tier choice follows from that.** The gateway opens an MXAccess
|
||||||
|
|||||||
@@ -22,8 +22,6 @@ paths, timeouts, queue sizes, enum values, or protocol values are invalid.
|
|||||||
"WorkingDirectory": null,
|
"WorkingDirectory": null,
|
||||||
"RequiredArchitecture": "X86",
|
"RequiredArchitecture": "X86",
|
||||||
"StartupTimeoutSeconds": 30,
|
"StartupTimeoutSeconds": 30,
|
||||||
"StartupProbeRetryAttempts": 3,
|
|
||||||
"StartupProbeRetryDelayMilliseconds": 250,
|
|
||||||
"PipeConnectAttemptTimeoutMilliseconds": 2000,
|
"PipeConnectAttemptTimeoutMilliseconds": 2000,
|
||||||
"ShutdownTimeoutSeconds": 10,
|
"ShutdownTimeoutSeconds": 10,
|
||||||
"HeartbeatIntervalSeconds": 5,
|
"HeartbeatIntervalSeconds": 5,
|
||||||
@@ -116,8 +114,6 @@ launch CWD (SEC-01, SEC-33).
|
|||||||
| `MxGateway:Worker:WorkingDirectory` | `null` | Optional working directory for the worker process. When set, it must be a valid filesystem path. |
|
| `MxGateway:Worker:WorkingDirectory` | `null` | Optional working directory for the worker process. When set, it must be a valid filesystem path. |
|
||||||
| `MxGateway:Worker:RequiredArchitecture` | `X86` | Required Portable Executable architecture for the worker. Supported values are `X86` and `X64`; MXAccess parity uses `X86`. |
|
| `MxGateway:Worker:RequiredArchitecture` | `X86` | Required Portable Executable architecture for the worker. Supported values are `X86` and `X64`; MXAccess parity uses `X86`. |
|
||||||
| `MxGateway:Worker:StartupTimeoutSeconds` | `30` | Total startup budget for process launch, startup probe, pipe connect, handshake, and worker readiness. |
|
| `MxGateway:Worker:StartupTimeoutSeconds` | `30` | Total startup budget for process launch, startup probe, pipe connect, handshake, and worker readiness. |
|
||||||
| `MxGateway:Worker:StartupProbeRetryAttempts` | `3` | Number of retry attempts for transient worker startup probe failures before pipe connection and handshake continue. |
|
|
||||||
| `MxGateway:Worker:StartupProbeRetryDelayMilliseconds` | `250` | Delay between transient startup probe retry attempts. |
|
|
||||||
| `MxGateway:Worker:PipeConnectAttemptTimeoutMilliseconds` | `2000` | Per-attempt timeout used by the worker named-pipe connect retry path. The overall pipe connection still stays under the startup budget. |
|
| `MxGateway:Worker:PipeConnectAttemptTimeoutMilliseconds` | `2000` | Per-attempt timeout used by the worker named-pipe connect retry path. The overall pipe connection still stays under the startup budget. |
|
||||||
| `MxGateway:Worker:WriteCompletionWaitMilliseconds` | `1500` | Bounded wait the worker holds a unary write reply (`Write`/`Write2`/`WriteSecured`/`WriteSecured2`; bulk writes excluded) for the matching MXAccess `OnWriteComplete` callback, so the reply's `statuses` carry the real commit outcome. `0` disables the wait (pure fire-and-forget replies). Must be `>= 0`. The gateway conveys the value to the worker via the `MXGATEWAY_WORKER_WRITE_COMPLETION_WAIT_MS` environment variable. Consumers that time their own writes must budget above this wait: OtOpcUa's GalaxyDriver wraps gateway writes in a 2 s Tier A resilience timeout, so a deployment raising this option past ~2000 must raise that driver `ResilienceConfig` write timeout in step or slow-but-successful commits surface as consumer-side failures. |
|
| `MxGateway:Worker:WriteCompletionWaitMilliseconds` | `1500` | Bounded wait the worker holds a unary write reply (`Write`/`Write2`/`WriteSecured`/`WriteSecured2`; bulk writes excluded) for the matching MXAccess `OnWriteComplete` callback, so the reply's `statuses` carry the real commit outcome. `0` disables the wait (pure fire-and-forget replies). Must be `>= 0`. The gateway conveys the value to the worker via the `MXGATEWAY_WORKER_WRITE_COMPLETION_WAIT_MS` environment variable. Consumers that time their own writes must budget above this wait: OtOpcUa's GalaxyDriver wraps gateway writes in a 2 s Tier A resilience timeout, so a deployment raising this option past ~2000 must raise that driver `ResilienceConfig` write timeout in step or slow-but-successful commits surface as consumer-side failures. |
|
||||||
| `MxGateway:Worker:EventQueueCapacity` | `10000` | Capacity, in events, of the worker's outbound MXAccess event queue. Must be between `1000` and `1000000`. This is burst headroom, not a throttle: the queue has no drop policy, so filling it records a `QueueOverflow` worker fault and faults the session. Raise it for sessions whose subscription set can outrun the drain loop (large advise sets, slow event consumers); the backing queue pre-allocates its slots, so the ceiling keeps a mistyped value from committing the 32-bit worker to an outsized allocation. The gateway conveys the value to the worker via the `MXGATEWAY_EVENT_QUEUE_CAPACITY` environment variable; a missing or unusable value leaves the worker on the 10000 default rather than failing the session. |
|
| `MxGateway:Worker:EventQueueCapacity` | `10000` | Capacity, in events, of the worker's outbound MXAccess event queue. Must be between `1000` and `1000000`. This is burst headroom, not a throttle: the queue has no drop policy, so filling it records a `QueueOverflow` worker fault and faults the session. Raise it for sessions whose subscription set can outrun the drain loop (large advise sets, slow event consumers); the backing queue pre-allocates its slots, so the ceiling keeps a mistyped value from committing the 32-bit worker to an outsized allocation. The gateway conveys the value to the worker via the `MXGATEWAY_EVENT_QUEUE_CAPACITY` environment variable; a missing or unusable value leaves the worker on the 10000 default rather than failing the session. |
|
||||||
@@ -126,7 +122,6 @@ launch CWD (SEC-01, SEC-33).
|
|||||||
| `MxGateway:Worker:HeartbeatGraceSeconds` | `15` | Maximum age of the last worker heartbeat before the gateway faults the worker. This must be greater than or equal to `HeartbeatIntervalSeconds`. |
|
| `MxGateway:Worker:HeartbeatGraceSeconds` | `15` | Maximum age of the last worker heartbeat before the gateway faults the worker. This must be greater than or equal to `HeartbeatIntervalSeconds`. |
|
||||||
| `MxGateway:Worker:MaxMessageBytes` | `16842752` | Maximum worker IPC frame payload size in bytes. The validator allows values from `1024` through `268435456`, and additionally requires this to be at least `MxGateway:Protocol:MaxGrpcMessageBytes` plus a 65536-byte (64 KiB) envelope-overhead reserve **and at most that cap plus twice the reserve**. The default is the 16 MiB public gRPC cap plus one reserve. The gateway conveys the negotiated value to the worker in the handshake (`GatewayHello.max_frame_bytes`), so the worker frames to the same limit rather than a hard-coded default. The band is narrow by design, so changing either message-size knob alone no longer validates — move `MxGateway:Protocol:MaxGrpcMessageBytes` and this one together, keeping this one one to two reserves above it. |
|
| `MxGateway:Worker:MaxMessageBytes` | `16842752` | Maximum worker IPC frame payload size in bytes. The validator allows values from `1024` through `268435456`, and additionally requires this to be at least `MxGateway:Protocol:MaxGrpcMessageBytes` plus a 65536-byte (64 KiB) envelope-overhead reserve **and at most that cap plus twice the reserve**. The default is the 16 MiB public gRPC cap plus one reserve. The gateway conveys the negotiated value to the worker in the handshake (`GatewayHello.max_frame_bytes`), so the worker frames to the same limit rather than a hard-coded default. The band is narrow by design, so changing either message-size knob alone no longer validates — move `MxGateway:Protocol:MaxGrpcMessageBytes` and this one together, keeping this one one to two reserves above it. |
|
||||||
|
|
||||||
`StartupProbeRetryAttempts`, `StartupProbeRetryDelayMilliseconds`,
|
|
||||||
`PipeConnectAttemptTimeoutMilliseconds`, timeout values, heartbeat values, and
|
`PipeConnectAttemptTimeoutMilliseconds`, timeout values, heartbeat values, and
|
||||||
`MaxMessageBytes` must be positive. `MaxMessageBytes` is intentionally bounded
|
`MaxMessageBytes` must be positive. `MaxMessageBytes` is intentionally bounded
|
||||||
to avoid accidental large allocations from malformed or oversized frames. It
|
to avoid accidental large allocations from malformed or oversized frames. It
|
||||||
@@ -742,7 +737,10 @@ Transport security here applies only to the public gRPC channel. The
|
|||||||
gateway↔worker link is a per-session **named pipe**
|
gateway↔worker link is a per-session **named pipe**
|
||||||
(`mxgw-{gatewayPid}-{sessionUid}`), not a network socket. It is not
|
(`mxgw-{gatewayPid}-{sessionUid}`), not a network socket. It is not
|
||||||
TLS-encrypted and does not need to be: it never leaves the local Windows host and
|
TLS-encrypted and does not need to be: it never leaves the local Windows host and
|
||||||
is secured by the OS pipe ACL. See [Worker Frame Protocol](./WorkerFrameProtocol.md).
|
is secured by the OS pipe ACL: on Windows the gateway creates each pipe with a
|
||||||
|
protected DACL whose only entry grants its own account (the account the worker
|
||||||
|
is launched as); on the non-Windows test matrix it uses
|
||||||
|
`PipeOptions.CurrentUserOnly`. See `WorkerPipeSecurity` and [Worker Frame Protocol](./WorkerFrameProtocol.md).
|
||||||
|
|
||||||
## Related Documentation
|
## Related Documentation
|
||||||
|
|
||||||
|
|||||||
@@ -940,8 +940,6 @@ Suggested configuration shape:
|
|||||||
"WorkingDirectory": null,
|
"WorkingDirectory": null,
|
||||||
"RequiredArchitecture": "X86",
|
"RequiredArchitecture": "X86",
|
||||||
"StartupTimeoutSeconds": 30,
|
"StartupTimeoutSeconds": 30,
|
||||||
"StartupProbeRetryAttempts": 3,
|
|
||||||
"StartupProbeRetryDelayMilliseconds": 250,
|
|
||||||
"PipeConnectAttemptTimeoutMilliseconds": 2000,
|
"PipeConnectAttemptTimeoutMilliseconds": 2000,
|
||||||
"ShutdownTimeoutSeconds": 10,
|
"ShutdownTimeoutSeconds": 10,
|
||||||
"HeartbeatIntervalSeconds": 5,
|
"HeartbeatIntervalSeconds": 5,
|
||||||
|
|||||||
+3
-5
@@ -109,7 +109,7 @@ The scalar fields mirror the counters and gauges. The four dictionaries provide
|
|||||||
- `CommandFailuresByMethod` keys by gRPC method name.
|
- `CommandFailuresByMethod` keys by gRPC method name.
|
||||||
- `EventsByFamily` keys by event family (the `Family` enum on a worker event).
|
- `EventsByFamily` keys by event family (the `Family` enum on a worker event).
|
||||||
- `EventsBySession` keys by `sessionId`; entries are removed via `RemoveSessionEvents` when a session closes so the map does not grow without bound.
|
- `EventsBySession` keys by `sessionId`; entries are removed via `RemoveSessionEvents` when a session closes so the map does not grow without bound.
|
||||||
- `RetryAttemptsByArea` keys by the resilience `area` tag, e.g. `worker_startup`.
|
- `RetryAttemptsByArea` keys by the resilience `area` tag passed to `RetryAttempted(area)`. No gateway code path records a retry at present (the worker launcher's startup-probe retry was removed in #188), so it is empty unless a future resilience path records one.
|
||||||
|
|
||||||
`EventsReceived` is read with `Interlocked.Read(ref _eventsReceived)` because `EventReceived` increments it via `Interlocked.Increment` outside the lock to keep the event-ingestion path non-blocking. `CommandsStarted`, `CommandsSucceeded`, `CommandsFailed`, and `CommandFailuresByMethod` are read the same way: the command counters run two-to-three times per gRPC call, so they are recorded with `Interlocked` and a `ConcurrentDictionary` rather than under `_syncRoot` (PERF-02). The two queue depths are pulled from their registered sources before the lock is taken, since those delegates reach into subscriber channels and worker clients.
|
`EventsReceived` is read with `Interlocked.Read(ref _eventsReceived)` because `EventReceived` increments it via `Interlocked.Increment` outside the lock to keep the event-ingestion path non-blocking. `CommandsStarted`, `CommandsSucceeded`, `CommandsFailed`, and `CommandFailuresByMethod` are read the same way: the command counters run two-to-three times per gRPC call, so they are recorded with `Interlocked` and a `ConcurrentDictionary` rather than under `_syncRoot` (PERF-02). The two queue depths are pulled from their registered sources before the lock is taken, since those delegates reach into subscriber channels and worker clients.
|
||||||
|
|
||||||
@@ -152,15 +152,13 @@ _metrics.RemoveSessionEvents(session.SessionId);
|
|||||||
|
|
||||||
### Worker process launcher
|
### Worker process launcher
|
||||||
|
|
||||||
`Workers/WorkerProcessLauncher.cs` records process-level kills and startup retries:
|
`Workers/WorkerProcessLauncher.cs` records process-level kills:
|
||||||
|
|
||||||
```csharp
|
```csharp
|
||||||
_metrics.WorkerKilled(reason);
|
_metrics.WorkerKilled(reason);
|
||||||
...
|
|
||||||
_metrics.RetryAttempted("worker_startup");
|
|
||||||
```
|
```
|
||||||
|
|
||||||
The `worker_startup` tag is hard-coded so the `RetryAttemptsByArea` snapshot reports launcher retries distinctly from other resilience areas.
|
The launcher no longer records retries: its startup probe runs once (#188).
|
||||||
|
|
||||||
### Session worker client factory
|
### Session worker client factory
|
||||||
|
|
||||||
|
|||||||
@@ -1011,7 +1011,11 @@ Graceful shutdown sequence:
|
|||||||
|
|
||||||
1. Pipe reader receives `WorkerShutdown`.
|
1. Pipe reader receives `WorkerShutdown`.
|
||||||
2. Worker host marks shutdown requested.
|
2. Worker host marks shutdown requested.
|
||||||
3. Reject new commands.
|
3. Reject new commands. A command that reaches the dispatcher after intake has
|
||||||
|
stopped is answered with a `WorkerUnavailable` reply and logged as
|
||||||
|
`WorkerCommandRejectedAfterShutdown`, so the gateway's correlation wait ends
|
||||||
|
at once instead of running out its own timeout. The read loop is serial and
|
||||||
|
stops after the shutdown frame, so this guard is defensive today.
|
||||||
4. Let current STA command finish if within timeout.
|
4. Let current STA command finish if within timeout.
|
||||||
5. Optionally run MXAccess cleanup:
|
5. Optionally run MXAccess cleanup:
|
||||||
- `UnAdvise`,
|
- `UnAdvise`,
|
||||||
|
|||||||
@@ -49,17 +49,15 @@ Before starting the process, the launcher validates that the configured worker
|
|||||||
path exists, has a `.exe` extension, contains a valid Windows Portable
|
path exists, has a `.exe` extension, contains a valid Windows Portable
|
||||||
Executable header, and matches the configured `RequiredArchitecture`.
|
Executable header, and matches the configured `RequiredArchitecture`.
|
||||||
|
|
||||||
After the process starts, `IWorkerStartupProbe` waits for startup readiness.
|
After the process starts, `IWorkerStartupProbe` runs once. The default probe
|
||||||
The default probe only verifies that the worker did not exit immediately. The
|
only verifies that the worker did not exit immediately; a worker that has
|
||||||
worker client replaces this probe when pipe connection, hello, and
|
already exited cannot be recovered by waiting, so the probe is not retried.
|
||||||
`WorkerReady` handling are implemented.
|
Real readiness is the pipe connection, hello, and `WorkerReady` handshake that
|
||||||
|
`WorkerClient` performs after the launch, under the same
|
||||||
Startup probing uses a bounded Polly retry policy. The gateway starts the worker
|
`WorkerOptions.StartupTimeoutSeconds` budget. (The former
|
||||||
process once, then retries only transient startup-probe failures while the
|
`StartupProbeRetryAttempts` / `StartupProbeRetryDelayMilliseconds` options were
|
||||||
process remains alive. The policy is configured by
|
removed because no shipped probe could ever produce a retryable failure; a
|
||||||
`WorkerOptions.StartupProbeRetryAttempts` and
|
stale value in configuration is ignored by the binder.)
|
||||||
`WorkerOptions.StartupProbeRetryDelayMilliseconds`; the retry counter is
|
|
||||||
recorded as `mxgateway.retries.attempted` with `area=worker_startup`.
|
|
||||||
|
|
||||||
The worker uses the exported pipe-attempt value inside its own bounded Polly
|
The worker uses the exported pipe-attempt value inside its own bounded Polly
|
||||||
retry loop.
|
retry loop.
|
||||||
|
|||||||
@@ -1,5 +1,8 @@
|
|||||||
# Deferred Follow-ups Implementation Plan
|
# 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
|
**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.
|
**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
|
**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
|
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
|
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
|
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` 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
|
`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
|
/ `SetBufferedUpdateInterval` are implemented and live-confirmed; the empty `NoData` bootstrap
|
||||||
event converts cleanly live (`f7ada90`). A real parallel quality/timestamp sample batch
|
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
|
(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 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
|
8-arg `AlarmAckByName` returns −55 (stub) and `AlarmAckByGUID` is `E_NOTIMPL` on this build, so the
|
||||||
|
|||||||
@@ -7,7 +7,7 @@
|
|||||||
<PropertyGroup>
|
<PropertyGroup>
|
||||||
<IsPackable>true</IsPackable>
|
<IsPackable>true</IsPackable>
|
||||||
<PackageId>ZB.MOM.WW.MxGateway.Contracts</PackageId>
|
<PackageId>ZB.MOM.WW.MxGateway.Contracts</PackageId>
|
||||||
<Version>0.2.1</Version>
|
<Version>0.3.0</Version>
|
||||||
<Authors>Joseph Doherty</Authors>
|
<Authors>Joseph Doherty</Authors>
|
||||||
<Company>ZB MOM WW</Company>
|
<Company>ZB MOM WW</Company>
|
||||||
<Copyright>Copyright (c) ZB MOM WW. All rights reserved.</Copyright>
|
<Copyright>Copyright (c) ZB MOM WW. All rights reserved.</Copyright>
|
||||||
|
|||||||
@@ -246,14 +246,6 @@ public sealed class GatewayOptionsValidator : OptionsValidatorBase<GatewayOption
|
|||||||
options.StartupTimeoutSeconds,
|
options.StartupTimeoutSeconds,
|
||||||
"MxGateway:Worker:StartupTimeoutSeconds must be greater than zero.",
|
"MxGateway:Worker:StartupTimeoutSeconds must be greater than zero.",
|
||||||
builder);
|
builder);
|
||||||
AddIfNotPositive(
|
|
||||||
options.StartupProbeRetryAttempts,
|
|
||||||
"MxGateway:Worker:StartupProbeRetryAttempts must be greater than zero.",
|
|
||||||
builder);
|
|
||||||
AddIfNotPositive(
|
|
||||||
options.StartupProbeRetryDelayMilliseconds,
|
|
||||||
"MxGateway:Worker:StartupProbeRetryDelayMilliseconds must be greater than zero.",
|
|
||||||
builder);
|
|
||||||
AddIfNotPositive(
|
AddIfNotPositive(
|
||||||
options.PipeConnectAttemptTimeoutMilliseconds,
|
options.PipeConnectAttemptTimeoutMilliseconds,
|
||||||
"MxGateway:Worker:PipeConnectAttemptTimeoutMilliseconds must be greater than zero.",
|
"MxGateway:Worker:PipeConnectAttemptTimeoutMilliseconds must be greater than zero.",
|
||||||
|
|||||||
@@ -15,12 +15,6 @@ public sealed class WorkerOptions
|
|||||||
/// <summary>The maximum time in seconds for the worker to start.</summary>
|
/// <summary>The maximum time in seconds for the worker to start.</summary>
|
||||||
public int StartupTimeoutSeconds { get; init; } = 30;
|
public int StartupTimeoutSeconds { get; init; } = 30;
|
||||||
|
|
||||||
/// <summary>The number of retry attempts for the startup probe.</summary>
|
|
||||||
public int StartupProbeRetryAttempts { get; init; } = 3;
|
|
||||||
|
|
||||||
/// <summary>The delay in milliseconds between startup probe retries.</summary>
|
|
||||||
public int StartupProbeRetryDelayMilliseconds { get; init; } = 250;
|
|
||||||
|
|
||||||
/// <summary>The timeout in milliseconds for connecting to the worker pipe.</summary>
|
/// <summary>The timeout in milliseconds for connecting to the worker pipe.</summary>
|
||||||
public int PipeConnectAttemptTimeoutMilliseconds { get; init; } = 2000;
|
public int PipeConnectAttemptTimeoutMilliseconds { get; init; } = 2000;
|
||||||
|
|
||||||
|
|||||||
@@ -11,8 +11,11 @@ public static class GatewayLogRedactor
|
|||||||
private static readonly HashSet<string> SensitiveCommandMethods = new(StringComparer.OrdinalIgnoreCase)
|
private static readonly HashSet<string> SensitiveCommandMethods = new(StringComparer.OrdinalIgnoreCase)
|
||||||
{
|
{
|
||||||
"AuthenticateUser",
|
"AuthenticateUser",
|
||||||
|
// Carry a credential-sensitive write value (single item and bulk).
|
||||||
"WriteSecured",
|
"WriteSecured",
|
||||||
"WriteSecured2",
|
"WriteSecured2",
|
||||||
|
"WriteSecuredBulk",
|
||||||
|
"WriteSecured2Bulk",
|
||||||
// Carry operator_password.
|
// Carry operator_password.
|
||||||
"AcknowledgeAlarm",
|
"AcknowledgeAlarm",
|
||||||
"AcknowledgeAlarmByName"
|
"AcknowledgeAlarmByName"
|
||||||
|
|||||||
@@ -0,0 +1,55 @@
|
|||||||
|
using Microsoft.Extensions.Diagnostics.HealthChecks;
|
||||||
|
using Microsoft.Extensions.Options;
|
||||||
|
using ZB.MOM.WW.MxGateway.Server.Configuration;
|
||||||
|
using ZB.MOM.WW.MxGateway.Server.Workers;
|
||||||
|
|
||||||
|
namespace ZB.MOM.WW.MxGateway.Server.Diagnostics;
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Readiness probe: verifies the configured worker executable is present and launchable (valid
|
||||||
|
/// path, <c>.exe</c>, on disk, PE image of the required architecture). Every <c>OpenSession</c>
|
||||||
|
/// launches it, so a gateway whose worker is missing cannot serve MXAccess at all and should not
|
||||||
|
/// receive traffic. This applies the same rule the launcher applies
|
||||||
|
/// (<see cref="WorkerExecutableValidator.ResolveAndValidate"/>), so the probe and the launch cannot
|
||||||
|
/// disagree.
|
||||||
|
/// </summary>
|
||||||
|
/// <remarks>
|
||||||
|
/// The result never carries the executable path: <c>/health/ready</c> is anonymous, so only the
|
||||||
|
/// launch error code is reported.
|
||||||
|
/// </remarks>
|
||||||
|
public sealed class WorkerExecutableHealthCheck : IHealthCheck
|
||||||
|
{
|
||||||
|
private readonly IOptions<GatewayOptions> _options;
|
||||||
|
|
||||||
|
/// <summary>Initializes a new instance of the <see cref="WorkerExecutableHealthCheck"/> class.</summary>
|
||||||
|
/// <param name="options">Gateway options carrying the worker executable settings.</param>
|
||||||
|
public WorkerExecutableHealthCheck(IOptions<GatewayOptions> options) =>
|
||||||
|
_options = options ?? throw new ArgumentNullException(nameof(options));
|
||||||
|
|
||||||
|
/// <summary>Checks the worker executable the next session launch would use.</summary>
|
||||||
|
/// <param name="context">The health check context.</param>
|
||||||
|
/// <param name="cancellationToken">Token to cancel the asynchronous operation.</param>
|
||||||
|
/// <returns>Healthy when the executable is launchable; otherwise Unhealthy with the launch error code.</returns>
|
||||||
|
public Task<HealthCheckResult> CheckHealthAsync(
|
||||||
|
HealthCheckContext context,
|
||||||
|
CancellationToken cancellationToken = default)
|
||||||
|
{
|
||||||
|
try
|
||||||
|
{
|
||||||
|
WorkerExecutableValidator.ResolveAndValidate(_options.Value.Worker);
|
||||||
|
return Task.FromResult(HealthCheckResult.Healthy("Worker executable is present."));
|
||||||
|
}
|
||||||
|
catch (WorkerProcessLaunchException exception)
|
||||||
|
{
|
||||||
|
return Task.FromResult(HealthCheckResult.Unhealthy(
|
||||||
|
$"Worker executable is not launchable ({exception.ErrorCode}).",
|
||||||
|
data: new Dictionary<string, object> { ["error_code"] = exception.ErrorCode.ToString() }));
|
||||||
|
}
|
||||||
|
catch (Exception exception) when (exception is IOException or UnauthorizedAccessException)
|
||||||
|
{
|
||||||
|
return Task.FromResult(HealthCheckResult.Unhealthy(
|
||||||
|
"Worker executable could not be read.",
|
||||||
|
data: new Dictionary<string, object> { ["error_code"] = exception.GetType().Name }));
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -120,6 +120,12 @@ public static class GatewayApplication
|
|||||||
"auth-store",
|
"auth-store",
|
||||||
failureStatus: null,
|
failureStatus: null,
|
||||||
tags: new[] { ZbHealthTags.Ready })
|
tags: new[] { ZbHealthTags.Ready })
|
||||||
|
// Ready: every OpenSession launches the worker, so a missing worker exe means the
|
||||||
|
// gateway cannot serve MXAccess. It never depends on session count.
|
||||||
|
.AddTypeActivatedCheck<WorkerExecutableHealthCheck>(
|
||||||
|
"worker-executable",
|
||||||
|
failureStatus: null,
|
||||||
|
tags: new[] { ZbHealthTags.Ready })
|
||||||
// Active, not Ready: a gateway holding no sessions is legitimately ready to serve.
|
// Active, not Ready: a gateway holding no sessions is legitimately ready to serve.
|
||||||
// See SessionHealthCheck for why zero sessions is healthy.
|
// See SessionHealthCheck for why zero sessions is healthy.
|
||||||
.AddTypeActivatedCheck<SessionHealthCheck>(
|
.AddTypeActivatedCheck<SessionHealthCheck>(
|
||||||
|
|||||||
@@ -171,15 +171,30 @@ public sealed class SessionWorkerClientFactory : ISessionWorkerClientFactory
|
|||||||
/// The buffer sizes are explicit so the pipe is not created with a zero quota; see
|
/// The buffer sizes are explicit so the pipe is not created with a zero quota; see
|
||||||
/// <see cref="PipeBufferSizeBytes"/>. On Unix hosts (the macOS test matrix, where named pipes
|
/// <see cref="PipeBufferSizeBytes"/>. On Unix hosts (the macOS test matrix, where named pipes
|
||||||
/// are Unix domain sockets) the sizes are advisory — the fix targets Windows production.
|
/// are Unix domain sockets) the sizes are advisory — the fix targets Windows production.
|
||||||
|
/// Access is restricted to the gateway's own account (#187): on Windows by the explicit ACL
|
||||||
|
/// from <see cref="WorkerPipeSecurity"/>, elsewhere by <see cref="PipeOptions.CurrentUserOnly"/>.
|
||||||
/// </remarks>
|
/// </remarks>
|
||||||
private static NamedPipeServerStream CreatePipe(string pipeName)
|
internal static NamedPipeServerStream CreatePipe(string pipeName)
|
||||||
{
|
{
|
||||||
|
if (OperatingSystem.IsWindows())
|
||||||
|
{
|
||||||
|
return NamedPipeServerStreamAcl.Create(
|
||||||
|
pipeName,
|
||||||
|
PipeDirection.InOut,
|
||||||
|
maxNumberOfServerInstances: 1,
|
||||||
|
PipeTransmissionMode.Byte,
|
||||||
|
PipeOptions.Asynchronous,
|
||||||
|
inBufferSize: PipeBufferSizeBytes,
|
||||||
|
outBufferSize: PipeBufferSizeBytes,
|
||||||
|
WorkerPipeSecurity.CreateForCurrentUser());
|
||||||
|
}
|
||||||
|
|
||||||
return new NamedPipeServerStream(
|
return new NamedPipeServerStream(
|
||||||
pipeName,
|
pipeName,
|
||||||
PipeDirection.InOut,
|
PipeDirection.InOut,
|
||||||
maxNumberOfServerInstances: 1,
|
maxNumberOfServerInstances: 1,
|
||||||
PipeTransmissionMode.Byte,
|
PipeTransmissionMode.Byte,
|
||||||
PipeOptions.Asynchronous,
|
WorkerPipeSecurity.UnixPipeOptions,
|
||||||
inBufferSize: PipeBufferSizeBytes,
|
inBufferSize: PipeBufferSizeBytes,
|
||||||
outBufferSize: PipeBufferSizeBytes);
|
outBufferSize: PipeBufferSizeBytes);
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -0,0 +1,62 @@
|
|||||||
|
using System.IO.Pipes;
|
||||||
|
using System.Runtime.Versioning;
|
||||||
|
using System.Security.AccessControl;
|
||||||
|
using System.Security.Principal;
|
||||||
|
|
||||||
|
namespace ZB.MOM.WW.MxGateway.Server.Sessions;
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Builds the access control applied to each gateway↔worker named pipe. The pipe carries
|
||||||
|
/// credential-bearing commands (secured writes, <c>AuthenticateUser</c>, alarm-ack passwords), so
|
||||||
|
/// its unguessable name is not enough: only the account the gateway runs as — which is also the
|
||||||
|
/// account the worker is launched as, because the launcher sets no alternate credentials — may
|
||||||
|
/// open it.
|
||||||
|
/// </summary>
|
||||||
|
/// <remarks>
|
||||||
|
/// There is deliberately no deny entry for <c>NT AUTHORITY\NETWORK</c>. That SID marks a network
|
||||||
|
/// <em>logon</em>, not a remote caller: a gateway started from an SSH or WinRM session carries it in
|
||||||
|
/// its own token, so a deny entry locked its own same-machine workers out (found on windev, #187).
|
||||||
|
/// </remarks>
|
||||||
|
internal static class WorkerPipeSecurity
|
||||||
|
{
|
||||||
|
/// <summary>Gets the options every worker pipe is created with on non-Windows hosts.</summary>
|
||||||
|
/// <remarks>
|
||||||
|
/// On Unix the pipe is a Unix domain socket; <see cref="PipeOptions.CurrentUserOnly"/> makes it
|
||||||
|
/// owner-only and rejects peers running as a different user. Windows uses the explicit ACL from
|
||||||
|
/// <see cref="Create(SecurityIdentifier)"/> instead.
|
||||||
|
/// </remarks>
|
||||||
|
public const PipeOptions UnixPipeOptions = PipeOptions.Asynchronous | PipeOptions.CurrentUserOnly;
|
||||||
|
|
||||||
|
/// <summary>Creates the pipe ACL for the account the gateway process runs as.</summary>
|
||||||
|
/// <returns>The pipe security descriptor.</returns>
|
||||||
|
[SupportedOSPlatform("windows")]
|
||||||
|
public static PipeSecurity CreateForCurrentUser()
|
||||||
|
{
|
||||||
|
using WindowsIdentity identity = WindowsIdentity.GetCurrent();
|
||||||
|
SecurityIdentifier user = identity.User
|
||||||
|
?? throw new InvalidOperationException("The gateway process identity has no user SID.");
|
||||||
|
return Create(user);
|
||||||
|
}
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Creates a pipe ACL that grants full control to <paramref name="user"/> only and does not
|
||||||
|
/// inherit any other entries.
|
||||||
|
/// </summary>
|
||||||
|
/// <param name="user">The account the gateway and its workers run as.</param>
|
||||||
|
/// <returns>The pipe security descriptor.</returns>
|
||||||
|
[SupportedOSPlatform("windows")]
|
||||||
|
public static PipeSecurity Create(SecurityIdentifier user)
|
||||||
|
{
|
||||||
|
ArgumentNullException.ThrowIfNull(user);
|
||||||
|
|
||||||
|
PipeSecurity security = new();
|
||||||
|
|
||||||
|
// Drop inherited entries so the default pipe DACL (Everyone/Anonymous read) cannot creep back in.
|
||||||
|
security.SetAccessRuleProtection(isProtected: true, preserveInheritance: false);
|
||||||
|
security.SetOwner(user);
|
||||||
|
security.AddAccessRule(new PipeAccessRule(user, PipeAccessRights.FullControl, AccessControlType.Allow));
|
||||||
|
|
||||||
|
|
||||||
|
return security;
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -13,6 +13,50 @@ internal static class WorkerExecutableValidator
|
|||||||
private const int MachineOffsetFromPeHeader = PeSignatureSize;
|
private const int MachineOffsetFromPeHeader = PeSignatureSize;
|
||||||
private const int MinimumHeaderSize = 0x40;
|
private const int MinimumHeaderSize = 0x40;
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Resolves the configured worker executable to a full path and checks it can be launched:
|
||||||
|
/// a valid path, a <c>.exe</c> extension, present on disk, and a PE image of the required
|
||||||
|
/// architecture. Shared by the launcher and the readiness check so both apply one rule.
|
||||||
|
/// </summary>
|
||||||
|
/// <param name="options">Worker options carrying the path and required architecture.</param>
|
||||||
|
/// <returns>The full path of the validated worker executable.</returns>
|
||||||
|
/// <exception cref="WorkerProcessLaunchException">Thrown when the executable is not launchable.</exception>
|
||||||
|
public static string ResolveAndValidate(WorkerOptions options)
|
||||||
|
{
|
||||||
|
ArgumentNullException.ThrowIfNull(options);
|
||||||
|
|
||||||
|
string executablePath;
|
||||||
|
try
|
||||||
|
{
|
||||||
|
executablePath = Path.GetFullPath(options.ExecutablePath);
|
||||||
|
}
|
||||||
|
catch (Exception exception) when (exception is ArgumentException or NotSupportedException or PathTooLongException)
|
||||||
|
{
|
||||||
|
throw new WorkerProcessLaunchException(
|
||||||
|
WorkerProcessLaunchErrorCode.InvalidExecutable,
|
||||||
|
"Worker executable path is not a valid filesystem path.",
|
||||||
|
exception);
|
||||||
|
}
|
||||||
|
|
||||||
|
if (!string.Equals(Path.GetExtension(executablePath), ".exe", StringComparison.OrdinalIgnoreCase))
|
||||||
|
{
|
||||||
|
throw new WorkerProcessLaunchException(
|
||||||
|
WorkerProcessLaunchErrorCode.InvalidExecutable,
|
||||||
|
"Worker executable path must point to a .exe file.");
|
||||||
|
}
|
||||||
|
|
||||||
|
if (!File.Exists(executablePath))
|
||||||
|
{
|
||||||
|
throw new WorkerProcessLaunchException(
|
||||||
|
WorkerProcessLaunchErrorCode.ExecutableNotFound,
|
||||||
|
"Worker executable does not exist.");
|
||||||
|
}
|
||||||
|
|
||||||
|
Validate(executablePath, options.RequiredArchitecture);
|
||||||
|
|
||||||
|
return executablePath;
|
||||||
|
}
|
||||||
|
|
||||||
/// <summary>Validates that a worker executable file has the required architecture.</summary>
|
/// <summary>Validates that a worker executable file has the required architecture.</summary>
|
||||||
/// <param name="executablePath">Full path to the worker executable file.</param>
|
/// <param name="executablePath">Full path to the worker executable file.</param>
|
||||||
/// <param name="requiredArchitecture">Required CPU architecture (x86 or x64).</param>
|
/// <param name="requiredArchitecture">Required CPU architecture (x86 or x64).</param>
|
||||||
|
|||||||
@@ -5,8 +5,6 @@ using Microsoft.Extensions.Options;
|
|||||||
using ZB.MOM.WW.MxGateway.Contracts;
|
using ZB.MOM.WW.MxGateway.Contracts;
|
||||||
using ZB.MOM.WW.MxGateway.Server.Configuration;
|
using ZB.MOM.WW.MxGateway.Server.Configuration;
|
||||||
using ZB.MOM.WW.MxGateway.Server.Metrics;
|
using ZB.MOM.WW.MxGateway.Server.Metrics;
|
||||||
using Polly;
|
|
||||||
using Polly.Retry;
|
|
||||||
|
|
||||||
namespace ZB.MOM.WW.MxGateway.Server.Workers;
|
namespace ZB.MOM.WW.MxGateway.Server.Workers;
|
||||||
|
|
||||||
@@ -140,15 +138,11 @@ public sealed class WorkerProcessLauncher : IWorkerProcessLauncher
|
|||||||
using CancellationTokenSource startupTimeout = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken);
|
using CancellationTokenSource startupTimeout = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken);
|
||||||
startupTimeout.CancelAfter(TimeSpan.FromSeconds(_workerOptions.StartupTimeoutSeconds));
|
startupTimeout.CancelAfter(TimeSpan.FromSeconds(_workerOptions.StartupTimeoutSeconds));
|
||||||
|
|
||||||
await CreateStartupProbePipeline(process)
|
// The probe runs once. Real readiness is the WorkerReady handshake WorkerClient performs
|
||||||
.ExecuteAsync(
|
// after launch, under the same StartupTimeoutSeconds budget; the probe only catches a
|
||||||
async token =>
|
// worker that exited immediately, which no retry can recover (#188).
|
||||||
{
|
await _startupProbe
|
||||||
await _startupProbe
|
.WaitUntilReadyAsync(process, request, startupTimeout.Token)
|
||||||
.WaitUntilReadyAsync(process, request, token)
|
|
||||||
.ConfigureAwait(false);
|
|
||||||
},
|
|
||||||
startupTimeout.Token)
|
|
||||||
.ConfigureAwait(false);
|
.ConfigureAwait(false);
|
||||||
|
|
||||||
return new WorkerProcessHandle(process, commandLine, startedAt);
|
return new WorkerProcessHandle(process, commandLine, startedAt);
|
||||||
@@ -230,39 +224,8 @@ public sealed class WorkerProcessLauncher : IWorkerProcessLauncher
|
|||||||
return startInfo;
|
return startInfo;
|
||||||
}
|
}
|
||||||
|
|
||||||
private string ResolveExecutablePath()
|
private string ResolveExecutablePath() =>
|
||||||
{
|
WorkerExecutableValidator.ResolveAndValidate(_workerOptions);
|
||||||
string executablePath;
|
|
||||||
try
|
|
||||||
{
|
|
||||||
executablePath = Path.GetFullPath(_workerOptions.ExecutablePath);
|
|
||||||
}
|
|
||||||
catch (Exception exception) when (exception is ArgumentException or NotSupportedException or PathTooLongException)
|
|
||||||
{
|
|
||||||
throw new WorkerProcessLaunchException(
|
|
||||||
WorkerProcessLaunchErrorCode.InvalidExecutable,
|
|
||||||
"Worker executable path is not a valid filesystem path.",
|
|
||||||
exception);
|
|
||||||
}
|
|
||||||
|
|
||||||
if (!string.Equals(Path.GetExtension(executablePath), ".exe", StringComparison.OrdinalIgnoreCase))
|
|
||||||
{
|
|
||||||
throw new WorkerProcessLaunchException(
|
|
||||||
WorkerProcessLaunchErrorCode.InvalidExecutable,
|
|
||||||
"Worker executable path must point to a .exe file.");
|
|
||||||
}
|
|
||||||
|
|
||||||
if (!File.Exists(executablePath))
|
|
||||||
{
|
|
||||||
throw new WorkerProcessLaunchException(
|
|
||||||
WorkerProcessLaunchErrorCode.ExecutableNotFound,
|
|
||||||
"Worker executable does not exist.");
|
|
||||||
}
|
|
||||||
|
|
||||||
WorkerExecutableValidator.Validate(executablePath, _workerOptions.RequiredArchitecture);
|
|
||||||
|
|
||||||
return executablePath;
|
|
||||||
}
|
|
||||||
|
|
||||||
private string ResolveWorkingDirectory(string executablePath)
|
private string ResolveWorkingDirectory(string executablePath)
|
||||||
{
|
{
|
||||||
@@ -310,43 +273,6 @@ public sealed class WorkerProcessLauncher : IWorkerProcessLauncher
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
private ResiliencePipeline CreateStartupProbePipeline(IWorkerProcess process)
|
|
||||||
{
|
|
||||||
RetryStrategyOptions retryOptions = new()
|
|
||||||
{
|
|
||||||
MaxRetryAttempts = Math.Max(0, _workerOptions.StartupProbeRetryAttempts - 1),
|
|
||||||
BackoffType = DelayBackoffType.Exponential,
|
|
||||||
UseJitter = true,
|
|
||||||
Delay = TimeSpan.FromMilliseconds(_workerOptions.StartupProbeRetryDelayMilliseconds),
|
|
||||||
MaxDelay = TimeSpan.FromSeconds(2),
|
|
||||||
ShouldHandle = new PredicateBuilder().Handle<Exception>(exception =>
|
|
||||||
ShouldRetryStartupProbe(exception, process)),
|
|
||||||
OnRetry = args =>
|
|
||||||
{
|
|
||||||
_metrics.RetryAttempted("worker_startup");
|
|
||||||
_logger.LogDebug(
|
|
||||||
args.Outcome.Exception,
|
|
||||||
"Retrying worker startup probe after transient failure. Attempt {Attempt}.",
|
|
||||||
args.AttemptNumber + 1);
|
|
||||||
return default;
|
|
||||||
},
|
|
||||||
};
|
|
||||||
|
|
||||||
return new ResiliencePipelineBuilder()
|
|
||||||
.AddRetry(retryOptions)
|
|
||||||
.Build();
|
|
||||||
}
|
|
||||||
|
|
||||||
private static bool ShouldRetryStartupProbe(Exception exception, IWorkerProcess process)
|
|
||||||
{
|
|
||||||
if (exception is OperationCanceledException or WorkerProcessLaunchException)
|
|
||||||
{
|
|
||||||
return false;
|
|
||||||
}
|
|
||||||
|
|
||||||
return !process.HasExited;
|
|
||||||
}
|
|
||||||
|
|
||||||
private static void ValidateRequest(WorkerProcessLaunchRequest request)
|
private static void ValidateRequest(WorkerProcessLaunchRequest request)
|
||||||
{
|
{
|
||||||
if (string.IsNullOrWhiteSpace(request.SessionId))
|
if (string.IsNullOrWhiteSpace(request.SessionId))
|
||||||
|
|||||||
@@ -35,7 +35,6 @@
|
|||||||
<!-- Security pin: GHSA-846f-f764-76mx / GHSA-2fmv-rv2c-59x7. Microsoft.Data.SqlClient 6.0.2
|
<!-- Security pin: GHSA-846f-f764-76mx / GHSA-2fmv-rv2c-59x7. Microsoft.Data.SqlClient 6.0.2
|
||||||
pulls the vulnerable System.Security.Cryptography.Pkcs 9.0.4; 10.0.13 patches it. -->
|
pulls the vulnerable System.Security.Cryptography.Pkcs 9.0.4; 10.0.13 patches it. -->
|
||||||
<PackageReference Include="System.Security.Cryptography.Pkcs" Version="10.0.13" />
|
<PackageReference Include="System.Security.Cryptography.Pkcs" Version="10.0.13" />
|
||||||
<PackageReference Include="Polly.Core" Version="8.6.6" />
|
|
||||||
</ItemGroup>
|
</ItemGroup>
|
||||||
|
|
||||||
<ItemGroup>
|
<ItemGroup>
|
||||||
|
|||||||
@@ -38,8 +38,6 @@ public sealed class GatewayOptionsTests
|
|||||||
Assert.Equal(@"src\ZB.MOM.WW.MxGateway.Worker\bin\x86\Release\ZB.MOM.WW.MxGateway.Worker.exe", options.Worker.ExecutablePath);
|
Assert.Equal(@"src\ZB.MOM.WW.MxGateway.Worker\bin\x86\Release\ZB.MOM.WW.MxGateway.Worker.exe", options.Worker.ExecutablePath);
|
||||||
Assert.Equal(WorkerArchitecture.X86, options.Worker.RequiredArchitecture);
|
Assert.Equal(WorkerArchitecture.X86, options.Worker.RequiredArchitecture);
|
||||||
Assert.Equal(30, options.Worker.StartupTimeoutSeconds);
|
Assert.Equal(30, options.Worker.StartupTimeoutSeconds);
|
||||||
Assert.Equal(3, options.Worker.StartupProbeRetryAttempts);
|
|
||||||
Assert.Equal(250, options.Worker.StartupProbeRetryDelayMilliseconds);
|
|
||||||
Assert.Equal(2000, options.Worker.PipeConnectAttemptTimeoutMilliseconds);
|
Assert.Equal(2000, options.Worker.PipeConnectAttemptTimeoutMilliseconds);
|
||||||
Assert.Equal(1500, options.Worker.WriteCompletionWaitMilliseconds);
|
Assert.Equal(1500, options.Worker.WriteCompletionWaitMilliseconds);
|
||||||
Assert.Equal(10, options.Worker.ShutdownTimeoutSeconds);
|
Assert.Equal(10, options.Worker.ShutdownTimeoutSeconds);
|
||||||
@@ -110,7 +108,6 @@ public sealed class GatewayOptionsTests
|
|||||||
/// <param name="expectedFailure">Expected validation error message.</param>
|
/// <param name="expectedFailure">Expected validation error message.</param>
|
||||||
[Theory]
|
[Theory]
|
||||||
[InlineData("MxGateway:Worker:ExecutablePath", "worker.dll", "MxGateway:Worker:ExecutablePath must point to a .exe file.")]
|
[InlineData("MxGateway:Worker:ExecutablePath", "worker.dll", "MxGateway:Worker:ExecutablePath must point to a .exe file.")]
|
||||||
[InlineData("MxGateway:Worker:StartupProbeRetryAttempts", "0", "MxGateway:Worker:StartupProbeRetryAttempts must be greater than zero.")]
|
|
||||||
[InlineData("MxGateway:Worker:PipeConnectAttemptTimeoutMilliseconds", "0", "MxGateway:Worker:PipeConnectAttemptTimeoutMilliseconds must be greater than zero.")]
|
[InlineData("MxGateway:Worker:PipeConnectAttemptTimeoutMilliseconds", "0", "MxGateway:Worker:PipeConnectAttemptTimeoutMilliseconds must be greater than zero.")]
|
||||||
[InlineData("MxGateway:Worker:WriteCompletionWaitMilliseconds", "-1", "MxGateway:Worker:WriteCompletionWaitMilliseconds must be greater than or equal to zero.")]
|
[InlineData("MxGateway:Worker:WriteCompletionWaitMilliseconds", "-1", "MxGateway:Worker:WriteCompletionWaitMilliseconds must be greater than or equal to zero.")]
|
||||||
[InlineData("MxGateway:Sessions:DefaultLeaseSeconds", "0", "MxGateway:Sessions:DefaultLeaseSeconds must be greater than zero.")]
|
[InlineData("MxGateway:Sessions:DefaultLeaseSeconds", "0", "MxGateway:Sessions:DefaultLeaseSeconds must be greater than zero.")]
|
||||||
|
|||||||
@@ -1,3 +1,5 @@
|
|||||||
|
using Google.Protobuf.Reflection;
|
||||||
|
using ZB.MOM.WW.MxGateway.Contracts.Proto;
|
||||||
using ZB.MOM.WW.MxGateway.Server.Diagnostics;
|
using ZB.MOM.WW.MxGateway.Server.Diagnostics;
|
||||||
|
|
||||||
namespace ZB.MOM.WW.MxGateway.Tests.Diagnostics;
|
namespace ZB.MOM.WW.MxGateway.Tests.Diagnostics;
|
||||||
@@ -118,6 +120,8 @@ public sealed class GatewayLogRedactorTests
|
|||||||
[InlineData("AuthenticateUser")]
|
[InlineData("AuthenticateUser")]
|
||||||
[InlineData("WriteSecured")]
|
[InlineData("WriteSecured")]
|
||||||
[InlineData("WriteSecured2")]
|
[InlineData("WriteSecured2")]
|
||||||
|
[InlineData("WriteSecuredBulk")]
|
||||||
|
[InlineData("WriteSecured2Bulk")]
|
||||||
[InlineData("AcknowledgeAlarm")]
|
[InlineData("AcknowledgeAlarm")]
|
||||||
[InlineData("AcknowledgeAlarmByName")]
|
[InlineData("AcknowledgeAlarmByName")]
|
||||||
public void IsCredentialBearingCommand_IdentifiesSensitiveMxAccessCommands(string commandMethod)
|
public void IsCredentialBearingCommand_IdentifiesSensitiveMxAccessCommands(string commandMethod)
|
||||||
@@ -146,6 +150,87 @@ public sealed class GatewayLogRedactorTests
|
|||||||
Assert.Equal("[redacted]", redacted);
|
Assert.Equal("[redacted]", redacted);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Verifies that the secured bulk writes are redacted even with value logging enabled. These
|
||||||
|
/// carry the same credential-sensitive value as the single-item secured writes (#193).
|
||||||
|
/// </summary>
|
||||||
|
/// <param name="kind">The secured bulk command kind.</param>
|
||||||
|
[Theory]
|
||||||
|
[InlineData(MxCommandKind.WriteSecuredBulk)]
|
||||||
|
[InlineData(MxCommandKind.WriteSecured2Bulk)]
|
||||||
|
public void RedactCommandValue_RedactsSecuredBulkWriteEvenWhenValueLoggingIsEnabled(MxCommandKind kind)
|
||||||
|
{
|
||||||
|
// WorkerClient logs the method as MxCommandKind.ToString(); exercise that exact spelling.
|
||||||
|
object? redacted = GatewayLogRedactor.RedactCommandValue(
|
||||||
|
kind.ToString(),
|
||||||
|
"credential-bearing-bulk-value",
|
||||||
|
valueLoggingEnabled: true);
|
||||||
|
|
||||||
|
Assert.Equal("[redacted]", redacted);
|
||||||
|
}
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Cross-checks the redactor against the contract: every <see cref="MxCommand"/> payload that
|
||||||
|
/// carries a password field, or is a secured write, must map to a credential-bearing command
|
||||||
|
/// kind. A new credential-bearing command added to the proto without a redactor entry fails here.
|
||||||
|
/// </summary>
|
||||||
|
[Fact]
|
||||||
|
public void IsCredentialBearingCommand_CoversEveryCredentialBearingPayloadInTheContract()
|
||||||
|
{
|
||||||
|
List<string> sensitivePayloads = [];
|
||||||
|
foreach (FieldDescriptor field in MxCommand.Descriptor.Oneofs.Single().Fields)
|
||||||
|
{
|
||||||
|
if (field.FieldType != FieldType.Message)
|
||||||
|
{
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
|
||||||
|
MessageDescriptor payload = field.MessageType;
|
||||||
|
if (payload.Name.Contains("Secured", StringComparison.Ordinal)
|
||||||
|
|| CarriesPassword(payload, []))
|
||||||
|
{
|
||||||
|
sensitivePayloads.Add(payload.Name);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Positive control: the walk must find the known credential carriers, or it proves nothing.
|
||||||
|
Assert.Contains("AuthenticateUserCommand", sensitivePayloads);
|
||||||
|
Assert.Contains("WriteSecuredBulkCommand", sensitivePayloads);
|
||||||
|
Assert.Contains("AcknowledgeAlarmByNameCommand", sensitivePayloads);
|
||||||
|
|
||||||
|
foreach (string payloadName in sensitivePayloads)
|
||||||
|
{
|
||||||
|
string kindName = payloadName[..^"Command".Length];
|
||||||
|
Assert.True(Enum.TryParse(kindName, out MxCommandKind _), $"No MxCommandKind named {kindName}.");
|
||||||
|
Assert.True(
|
||||||
|
GatewayLogRedactor.IsCredentialBearingCommand(kindName),
|
||||||
|
$"{kindName} carries credential material but is not redacted.");
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
private static bool CarriesPassword(MessageDescriptor message, HashSet<string> visited)
|
||||||
|
{
|
||||||
|
if (!visited.Add(message.FullName))
|
||||||
|
{
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
|
foreach (FieldDescriptor field in message.Fields.InDeclarationOrder())
|
||||||
|
{
|
||||||
|
if (field.Name.Contains("password", StringComparison.OrdinalIgnoreCase))
|
||||||
|
{
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
|
||||||
|
if (field.FieldType == FieldType.Message && CarriesPassword(field.MessageType, visited))
|
||||||
|
{
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
/// <summary>Verifies that RedactCommandValue allows non-sensitive values only when value logging is enabled.</summary>
|
/// <summary>Verifies that RedactCommandValue allows non-sensitive values only when value logging is enabled.</summary>
|
||||||
[Fact]
|
[Fact]
|
||||||
public void RedactCommandValue_AllowsNonSensitiveValueOnlyWhenValueLoggingIsEnabled()
|
public void RedactCommandValue_AllowsNonSensitiveValueOnlyWhenValueLoggingIsEnabled()
|
||||||
|
|||||||
@@ -0,0 +1,106 @@
|
|||||||
|
using Microsoft.AspNetCore.Builder;
|
||||||
|
using Microsoft.Extensions.DependencyInjection;
|
||||||
|
using Microsoft.Extensions.Diagnostics.HealthChecks;
|
||||||
|
using Microsoft.Extensions.Options;
|
||||||
|
using ZB.MOM.WW.Health;
|
||||||
|
using ZB.MOM.WW.MxGateway.Server;
|
||||||
|
using ZB.MOM.WW.MxGateway.Server.Configuration;
|
||||||
|
using ZB.MOM.WW.MxGateway.Server.Diagnostics;
|
||||||
|
|
||||||
|
namespace ZB.MOM.WW.MxGateway.Tests.Diagnostics;
|
||||||
|
|
||||||
|
/// <summary>Readiness coverage for the worker executable (#206).</summary>
|
||||||
|
public sealed class WorkerExecutableHealthCheckTests : IDisposable
|
||||||
|
{
|
||||||
|
private readonly string _directory = Path.Combine(Path.GetTempPath(), $"mxgw-workercheck-{Guid.NewGuid():N}");
|
||||||
|
|
||||||
|
/// <summary>Initializes a new instance of the <see cref="WorkerExecutableHealthCheckTests"/> class.</summary>
|
||||||
|
public WorkerExecutableHealthCheckTests() => Directory.CreateDirectory(_directory);
|
||||||
|
|
||||||
|
/// <inheritdoc />
|
||||||
|
public void Dispose() => Directory.Delete(_directory, recursive: true);
|
||||||
|
|
||||||
|
/// <summary>A present x86 worker image is healthy.</summary>
|
||||||
|
/// <returns>A task that represents the asynchronous operation.</returns>
|
||||||
|
[Fact]
|
||||||
|
public async Task Healthy_WhenWorkerExecutableIsLaunchable()
|
||||||
|
{
|
||||||
|
string path = WritePeStub(machine: 0x014c);
|
||||||
|
|
||||||
|
HealthCheckResult result = await CheckAsync(path, WorkerArchitecture.X86);
|
||||||
|
|
||||||
|
Assert.Equal(HealthStatus.Healthy, result.Status);
|
||||||
|
}
|
||||||
|
|
||||||
|
/// <summary>A missing worker executable is unhealthy and reports only the error code, never the path.</summary>
|
||||||
|
/// <returns>A task that represents the asynchronous operation.</returns>
|
||||||
|
[Fact]
|
||||||
|
public async Task Unhealthy_WhenWorkerExecutableIsMissing()
|
||||||
|
{
|
||||||
|
string path = Path.Combine(_directory, "missing", "ZB.MOM.WW.MxGateway.Worker.exe");
|
||||||
|
|
||||||
|
HealthCheckResult result = await CheckAsync(path, WorkerArchitecture.X86);
|
||||||
|
|
||||||
|
Assert.Equal(HealthStatus.Unhealthy, result.Status);
|
||||||
|
Assert.Equal("ExecutableNotFound", result.Data["error_code"]);
|
||||||
|
Assert.DoesNotContain(_directory, result.Description, StringComparison.OrdinalIgnoreCase);
|
||||||
|
}
|
||||||
|
|
||||||
|
/// <summary>A worker image of the wrong architecture is unhealthy, as the launcher would reject it.</summary>
|
||||||
|
/// <returns>A task that represents the asynchronous operation.</returns>
|
||||||
|
[Fact]
|
||||||
|
public async Task Unhealthy_WhenWorkerExecutableHasWrongArchitecture()
|
||||||
|
{
|
||||||
|
string path = WritePeStub(machine: 0x8664);
|
||||||
|
|
||||||
|
HealthCheckResult result = await CheckAsync(path, WorkerArchitecture.X86);
|
||||||
|
|
||||||
|
Assert.Equal(HealthStatus.Unhealthy, result.Status);
|
||||||
|
Assert.Equal("InvalidExecutable", result.Data["error_code"]);
|
||||||
|
}
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// The gateway registers the check on the ready tier, and the session check stays on the active
|
||||||
|
/// tier so zero sessions keeps readiness green.
|
||||||
|
/// </summary>
|
||||||
|
/// <returns>A task that represents the asynchronous operation.</returns>
|
||||||
|
[Fact]
|
||||||
|
public async Task Build_RegistersWorkerExecutableCheckOnReadyTier()
|
||||||
|
{
|
||||||
|
await using WebApplication app = GatewayApplication.Build([]);
|
||||||
|
HealthCheckServiceOptions options =
|
||||||
|
app.Services.GetRequiredService<IOptions<HealthCheckServiceOptions>>().Value;
|
||||||
|
|
||||||
|
HealthCheckRegistration worker = Assert.Single(options.Registrations, r => r.Name == "worker-executable");
|
||||||
|
Assert.Contains(ZbHealthTags.Ready, worker.Tags);
|
||||||
|
HealthCheckRegistration sessions = Assert.Single(options.Registrations, r => r.Name == "mxaccess-sessions");
|
||||||
|
Assert.DoesNotContain(ZbHealthTags.Ready, sessions.Tags);
|
||||||
|
}
|
||||||
|
|
||||||
|
private static Task<HealthCheckResult> CheckAsync(string path, WorkerArchitecture architecture)
|
||||||
|
{
|
||||||
|
WorkerExecutableHealthCheck check = new(Options.Create(new GatewayOptions
|
||||||
|
{
|
||||||
|
Worker = new WorkerOptions
|
||||||
|
{
|
||||||
|
ExecutablePath = path,
|
||||||
|
RequiredArchitecture = architecture,
|
||||||
|
},
|
||||||
|
}));
|
||||||
|
return check.CheckHealthAsync(new HealthCheckContext());
|
||||||
|
}
|
||||||
|
|
||||||
|
private string WritePeStub(ushort machine)
|
||||||
|
{
|
||||||
|
string path = Path.Combine(_directory, "ZB.MOM.WW.MxGateway.Worker.exe");
|
||||||
|
byte[] bytes = new byte[0x100];
|
||||||
|
bytes[0] = (byte)'M';
|
||||||
|
bytes[1] = (byte)'Z';
|
||||||
|
BitConverter.GetBytes(0x80).CopyTo(bytes, 0x3c);
|
||||||
|
bytes[0x80] = (byte)'P';
|
||||||
|
bytes[0x81] = (byte)'E';
|
||||||
|
BitConverter.GetBytes(machine).CopyTo(bytes, 0x84);
|
||||||
|
File.WriteAllBytes(path, bytes);
|
||||||
|
return path;
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -0,0 +1,116 @@
|
|||||||
|
using System.IO.Pipes;
|
||||||
|
using System.Runtime.Versioning;
|
||||||
|
using System.Security.AccessControl;
|
||||||
|
using System.Security.Principal;
|
||||||
|
using ZB.MOM.WW.MxGateway.Server.Sessions;
|
||||||
|
|
||||||
|
namespace ZB.MOM.WW.MxGateway.Tests.Gateway.Sessions;
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Verifies that the gateway↔worker pipe is restricted to the gateway's own account (#187).
|
||||||
|
/// The ACL assertions need Windows; the Unix assertion covers the macOS/Linux test matrix.
|
||||||
|
/// </summary>
|
||||||
|
public sealed class WorkerPipeSecurityTests
|
||||||
|
{
|
||||||
|
|
||||||
|
/// <summary>Verifies the factory grants only the given user.</summary>
|
||||||
|
[WindowsFact]
|
||||||
|
[SupportedOSPlatform("windows")]
|
||||||
|
public void Create_GrantsOnlyTheUser()
|
||||||
|
{
|
||||||
|
SecurityIdentifier user = new("S-1-5-21-1000-2000-3000-1001");
|
||||||
|
|
||||||
|
PipeSecurity security = WorkerPipeSecurity.Create(user);
|
||||||
|
|
||||||
|
Assert.True(security.AreAccessRulesProtected);
|
||||||
|
Assert.Equal(user, security.GetOwner(typeof(SecurityIdentifier)));
|
||||||
|
AssertOnlyUserAllowed(security, user);
|
||||||
|
}
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Verifies the pipe the factory actually creates carries the restricted ACL — read back from
|
||||||
|
/// the kernel object, not from the descriptor handed to it.
|
||||||
|
/// </summary>
|
||||||
|
[WindowsFact]
|
||||||
|
[SupportedOSPlatform("windows")]
|
||||||
|
public void CreatePipe_OnWindows_AppliesCurrentUserOnlyAcl()
|
||||||
|
{
|
||||||
|
using WindowsIdentity identity = WindowsIdentity.GetCurrent();
|
||||||
|
SecurityIdentifier user = identity.User!;
|
||||||
|
|
||||||
|
using NamedPipeServerStream pipe = SessionWorkerClientFactory.CreatePipe(NewPipeName());
|
||||||
|
PipeSecurity applied = pipe.GetAccessControl();
|
||||||
|
|
||||||
|
AssertOnlyUserAllowed(applied, user);
|
||||||
|
}
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Verifies the pipe still accepts a client running as the same user — the worker is launched
|
||||||
|
/// under the gateway's own account, so the restriction must not break the session handshake.
|
||||||
|
/// On windev this runs from an SSH (network-logon) session, which is what caught the earlier
|
||||||
|
/// deny-NETWORK entry.
|
||||||
|
/// </summary>
|
||||||
|
[Fact]
|
||||||
|
public async Task CreatePipe_AcceptsClientRunningAsTheSameUser()
|
||||||
|
{
|
||||||
|
string pipeName = NewPipeName();
|
||||||
|
using NamedPipeServerStream server = SessionWorkerClientFactory.CreatePipe(pipeName);
|
||||||
|
using NamedPipeClientStream client = new(".", pipeName, PipeDirection.InOut, PipeOptions.Asynchronous);
|
||||||
|
using CancellationTokenSource timeout = new(TimeSpan.FromSeconds(10));
|
||||||
|
|
||||||
|
Task accept = server.WaitForConnectionAsync(timeout.Token);
|
||||||
|
await client.ConnectAsync(timeout.Token);
|
||||||
|
await accept;
|
||||||
|
|
||||||
|
Assert.True(server.IsConnected);
|
||||||
|
Assert.True(client.IsConnected);
|
||||||
|
}
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Verifies non-Windows pipes are created current-user-only. On Unix .NET enforces this by
|
||||||
|
/// checking the connecting peer's credentials at accept time (the socket file mode is not
|
||||||
|
/// changed), so the observable contract is the option itself.
|
||||||
|
/// </summary>
|
||||||
|
[Fact]
|
||||||
|
public void UnixPipeOptions_AreAsynchronousAndCurrentUserOnly()
|
||||||
|
{
|
||||||
|
Assert.True(WorkerPipeSecurity.UnixPipeOptions.HasFlag(PipeOptions.CurrentUserOnly));
|
||||||
|
Assert.True(WorkerPipeSecurity.UnixPipeOptions.HasFlag(PipeOptions.Asynchronous));
|
||||||
|
}
|
||||||
|
|
||||||
|
[SupportedOSPlatform("windows")]
|
||||||
|
private static void AssertOnlyUserAllowed(PipeSecurity security, SecurityIdentifier user)
|
||||||
|
{
|
||||||
|
SecurityIdentifier everyoneSid = new(WellKnownSidType.WorldSid, null);
|
||||||
|
SecurityIdentifier anonymousSid = new(WellKnownSidType.AnonymousSid, null);
|
||||||
|
List<PipeAccessRule> rules = security
|
||||||
|
.GetAccessRules(includeExplicit: true, includeInherited: true, typeof(SecurityIdentifier))
|
||||||
|
.Cast<PipeAccessRule>()
|
||||||
|
.ToList();
|
||||||
|
|
||||||
|
// Exactly one entry: an allow for the gateway's user. In particular no deny entry — a deny
|
||||||
|
// for NETWORK locked out same-machine workers when the gateway ran from an SSH logon.
|
||||||
|
PipeAccessRule allow = Assert.Single(rules);
|
||||||
|
Assert.Equal(AccessControlType.Allow, allow.AccessControlType);
|
||||||
|
Assert.Equal(user, allow.IdentityReference);
|
||||||
|
Assert.Equal(PipeAccessRights.FullControl, allow.PipeAccessRights & PipeAccessRights.FullControl);
|
||||||
|
|
||||||
|
Assert.DoesNotContain(rules, r => r.IdentityReference == everyoneSid && r.AccessControlType == AccessControlType.Allow);
|
||||||
|
Assert.DoesNotContain(rules, r => r.IdentityReference == anonymousSid && r.AccessControlType == AccessControlType.Allow);
|
||||||
|
}
|
||||||
|
|
||||||
|
private static string NewPipeName() => $"mxgw-acl-{Guid.NewGuid():N}";
|
||||||
|
}
|
||||||
|
|
||||||
|
/// <summary>A fact that runs only on Windows.</summary>
|
||||||
|
public sealed class WindowsFactAttribute : FactAttribute
|
||||||
|
{
|
||||||
|
/// <summary>Initializes a new instance of the <see cref="WindowsFactAttribute"/> class.</summary>
|
||||||
|
public WindowsFactAttribute()
|
||||||
|
{
|
||||||
|
if (!OperatingSystem.IsWindows())
|
||||||
|
{
|
||||||
|
Skip = "Requires Windows.";
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -167,32 +167,48 @@ public sealed class WorkerProcessLauncherTests
|
|||||||
Assert.Equal(1, metrics.GetSnapshot().WorkerKills);
|
Assert.Equal(1, metrics.GetSnapshot().WorkerKills);
|
||||||
}
|
}
|
||||||
|
|
||||||
/// <summary>Verifies that transient startup probe failures are retried without respawning the worker process.</summary>
|
/// <summary>
|
||||||
|
/// The startup probe runs once. A probe failure fails the launch — there is no retry to
|
||||||
|
/// configure, because real readiness is the WorkerReady handshake that follows the launch
|
||||||
|
/// (#188).
|
||||||
|
/// </summary>
|
||||||
/// <returns>A task that represents the asynchronous operation.</returns>
|
/// <returns>A task that represents the asynchronous operation.</returns>
|
||||||
[Fact]
|
[Fact]
|
||||||
public async Task LaunchAsync_WhenStartupProbeFailsTransiently_RetriesWithoutRespawningWorker()
|
public async Task LaunchAsync_WhenStartupProbeFails_DoesNotRetryTheProbe()
|
||||||
{
|
{
|
||||||
using TestDirectory directory = TestDirectory.Create();
|
using TestDirectory directory = TestDirectory.Create();
|
||||||
string executablePath = directory.CreateWorkerExecutable(machine: 0x014c);
|
string executablePath = directory.CreateWorkerExecutable(machine: 0x014c);
|
||||||
FakeWorkerProcess process = new(processId: 1234);
|
FakeWorkerProcess process = new(processId: 1234);
|
||||||
FakeWorkerProcessFactory processFactory = new(process);
|
FakeWorkerProcessFactory processFactory = new(process);
|
||||||
GatewayMetrics metrics = new();
|
GatewayMetrics metrics = new();
|
||||||
|
TransientStartupProbe probe = new(failuresBeforeSuccess: 1);
|
||||||
WorkerProcessLauncher launcher = CreateLauncher(
|
WorkerProcessLauncher launcher = CreateLauncher(
|
||||||
executablePath,
|
executablePath,
|
||||||
processFactory,
|
processFactory,
|
||||||
new TransientStartupProbe(failuresBeforeSuccess: 1),
|
probe,
|
||||||
metrics,
|
metrics);
|
||||||
startupProbeRetryAttempts: 2,
|
|
||||||
startupProbeRetryDelayMilliseconds: 1);
|
|
||||||
|
|
||||||
using WorkerProcessHandle handle = await launcher.LaunchAsync(CreateRequest());
|
WorkerProcessLaunchException exception =
|
||||||
|
await Assert.ThrowsAsync<WorkerProcessLaunchException>(
|
||||||
|
async () => await launcher.LaunchAsync(CreateRequest()));
|
||||||
|
|
||||||
Assert.Same(process, handle.Process);
|
Assert.Equal(WorkerProcessLaunchErrorCode.StartupFailed, exception.ErrorCode);
|
||||||
|
Assert.Equal(1, probe.Attempts);
|
||||||
Assert.Equal(1, processFactory.StartCount);
|
Assert.Equal(1, processFactory.StartCount);
|
||||||
Assert.False(process.KillCalled);
|
Assert.True(process.KillCalled);
|
||||||
GatewayMetricsSnapshot snapshot = metrics.GetSnapshot();
|
Assert.Equal(0, metrics.GetSnapshot().RetryAttempts);
|
||||||
Assert.Equal(1, snapshot.RetryAttempts);
|
}
|
||||||
Assert.Equal(1, snapshot.RetryAttemptsByArea["worker_startup"]);
|
|
||||||
|
/// <summary>
|
||||||
|
/// The retry options are gone from <see cref="WorkerOptions"/> so configuration cannot
|
||||||
|
/// promise a startup-probe retry that never happens (#188).
|
||||||
|
/// </summary>
|
||||||
|
[Fact]
|
||||||
|
public void WorkerOptions_HasNoStartupProbeRetryOptions()
|
||||||
|
{
|
||||||
|
Assert.DoesNotContain(
|
||||||
|
typeof(WorkerOptions).GetProperties(),
|
||||||
|
property => property.Name.StartsWith("StartupProbeRetry", StringComparison.Ordinal));
|
||||||
}
|
}
|
||||||
|
|
||||||
/// <summary>Verifies that a startup probe timeout kills and disposes the worker process.</summary>
|
/// <summary>Verifies that a startup probe timeout kills and disposes the worker process.</summary>
|
||||||
@@ -290,8 +306,6 @@ public sealed class WorkerProcessLauncherTests
|
|||||||
IWorkerStartupProbe startupProbe,
|
IWorkerStartupProbe startupProbe,
|
||||||
GatewayMetrics? metrics = null,
|
GatewayMetrics? metrics = null,
|
||||||
int startupTimeoutSeconds = 30,
|
int startupTimeoutSeconds = 30,
|
||||||
int startupProbeRetryAttempts = 3,
|
|
||||||
int startupProbeRetryDelayMilliseconds = 250,
|
|
||||||
int eventQueueCapacity = 10000,
|
int eventQueueCapacity = 10000,
|
||||||
string ackClientName = "MxGateway")
|
string ackClientName = "MxGateway")
|
||||||
{
|
{
|
||||||
@@ -303,8 +317,6 @@ public sealed class WorkerProcessLauncherTests
|
|||||||
ExecutablePath = executablePath,
|
ExecutablePath = executablePath,
|
||||||
RequiredArchitecture = WorkerArchitecture.X86,
|
RequiredArchitecture = WorkerArchitecture.X86,
|
||||||
StartupTimeoutSeconds = startupTimeoutSeconds,
|
StartupTimeoutSeconds = startupTimeoutSeconds,
|
||||||
StartupProbeRetryAttempts = startupProbeRetryAttempts,
|
|
||||||
StartupProbeRetryDelayMilliseconds = startupProbeRetryDelayMilliseconds,
|
|
||||||
EventQueueCapacity = eventQueueCapacity,
|
EventQueueCapacity = eventQueueCapacity,
|
||||||
},
|
},
|
||||||
};
|
};
|
||||||
@@ -388,6 +400,9 @@ public sealed class WorkerProcessLauncherTests
|
|||||||
{
|
{
|
||||||
private int _attempts;
|
private int _attempts;
|
||||||
|
|
||||||
|
/// <summary>Gets the number of times the probe was invoked.</summary>
|
||||||
|
public int Attempts => Volatile.Read(ref _attempts);
|
||||||
|
|
||||||
/// <inheritdoc />
|
/// <inheritdoc />
|
||||||
public Task WaitUntilReadyAsync(
|
public Task WaitUntilReadyAsync(
|
||||||
IWorkerProcess process,
|
IWorkerProcess process,
|
||||||
|
|||||||
@@ -1943,6 +1943,60 @@ public sealed class WorkerPipeSessionTests
|
|||||||
&& (string?)correlationId == "command-dropped-after-shutdown");
|
&& (string?)correlationId == "command-dropped-after-shutdown");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// A command that reaches the dispatcher after shutdown has stopped command intake must be
|
||||||
|
/// answered with <see cref="ProtocolStatusCode.WorkerUnavailable"/> (and logged) rather than
|
||||||
|
/// dropped silently, which would leave the gateway's correlation wait to run out its own
|
||||||
|
/// timeout with no trace (#182).
|
||||||
|
/// </summary>
|
||||||
|
/// <returns>A task that represents the asynchronous operation.</returns>
|
||||||
|
[Fact]
|
||||||
|
public async Task CommandAfterShutdown_RepliesWorkerUnavailable()
|
||||||
|
{
|
||||||
|
using CancellationTokenSource cancellation = new(TimeSpan.FromSeconds(10));
|
||||||
|
using PipePair pipePair = await PipePair.CreateAsync(cancellation.Token);
|
||||||
|
FakeRuntimeSession runtime = new();
|
||||||
|
RecordingWorkerLogger logger = new();
|
||||||
|
WorkerFrameProtocolOptions options = CreateOptions();
|
||||||
|
WorkerPipeSession session = new(
|
||||||
|
new WorkerFrameReader(pipePair.WorkerStream, options),
|
||||||
|
new WorkerFrameWriter(pipePair.WorkerStream, options),
|
||||||
|
options,
|
||||||
|
() => 1234,
|
||||||
|
new WorkerPipeSessionOptions
|
||||||
|
{
|
||||||
|
HeartbeatInterval = TimeSpan.FromSeconds(1),
|
||||||
|
HeartbeatGrace = TimeSpan.FromSeconds(5),
|
||||||
|
},
|
||||||
|
() => runtime,
|
||||||
|
logger);
|
||||||
|
Task runTask = session.RunAsync(cancellation.Token);
|
||||||
|
await CompleteGatewayHandshakeAsync(pipePair, cancellation.Token);
|
||||||
|
|
||||||
|
session.StopAcceptingCommands();
|
||||||
|
await pipePair.GatewayWriter.WriteAsync(
|
||||||
|
CreateCommandEnvelope("command-after-shutdown"),
|
||||||
|
cancellation.Token);
|
||||||
|
|
||||||
|
WorkerEnvelope reply = await ReadUntilAsync(
|
||||||
|
pipePair.GatewayReader,
|
||||||
|
WorkerEnvelope.BodyOneofCase.WorkerCommandReply,
|
||||||
|
envelope => envelope.CorrelationId == "command-after-shutdown",
|
||||||
|
cancellation.Token);
|
||||||
|
|
||||||
|
Assert.Equal(ProtocolStatusCode.WorkerUnavailable, reply.WorkerCommandReply.Reply.ProtocolStatus.Code);
|
||||||
|
Assert.Equal(MxCommandKind.Register, reply.WorkerCommandReply.Reply.Kind);
|
||||||
|
Assert.Equal(SessionId, reply.WorkerCommandReply.Reply.SessionId);
|
||||||
|
Assert.False(runtime.DispatchStarted.IsSet);
|
||||||
|
Assert.Contains(
|
||||||
|
logger.Events,
|
||||||
|
entry => entry.EventName == "WorkerCommandRejectedAfterShutdown"
|
||||||
|
&& entry.Fields.TryGetValue("correlation_id", out object? correlationId)
|
||||||
|
&& (string?)correlationId == "command-after-shutdown");
|
||||||
|
|
||||||
|
await SendShutdownAndWaitAsync(pipePair, runTask, cancellation.Token);
|
||||||
|
}
|
||||||
|
|
||||||
private static WorkerPipeSession CreateSession(
|
private static WorkerPipeSession CreateSession(
|
||||||
Stream inbound,
|
Stream inbound,
|
||||||
Stream outbound,
|
Stream outbound,
|
||||||
|
|||||||
@@ -748,7 +748,11 @@ public sealed class WorkerPipeSession
|
|||||||
return await HandleControlCommandAsync(envelope, cancellationToken).ConfigureAwait(false);
|
return await HandleControlCommandAsync(envelope, cancellationToken).ConfigureAwait(false);
|
||||||
}
|
}
|
||||||
|
|
||||||
TryStartCommandTask(envelope, cancellationToken);
|
if (!TryStartCommandTask(envelope, cancellationToken))
|
||||||
|
{
|
||||||
|
await RejectCommandAfterShutdownAsync(envelope, cancellationToken).ConfigureAwait(false);
|
||||||
|
}
|
||||||
|
|
||||||
return true;
|
return true;
|
||||||
case WorkerEnvelope.BodyOneofCase.WorkerShutdown:
|
case WorkerEnvelope.BodyOneofCase.WorkerShutdown:
|
||||||
await ShutdownAsync(envelope.WorkerShutdown, cancellationToken).ConfigureAwait(false);
|
await ShutdownAsync(envelope.WorkerShutdown, cancellationToken).ConfigureAwait(false);
|
||||||
@@ -1230,7 +1234,12 @@ public sealed class WorkerPipeSession
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
private void TryStartCommandTask(
|
/// <summary>
|
||||||
|
/// Starts the command on the runtime session unless shutdown has already stopped command
|
||||||
|
/// intake. Returns <c>false</c> when the command was refused, so the caller can answer the
|
||||||
|
/// correlation instead of leaving the gateway's wait to run out its own timeout.
|
||||||
|
/// </summary>
|
||||||
|
private bool TryStartCommandTask(
|
||||||
WorkerEnvelope envelope,
|
WorkerEnvelope envelope,
|
||||||
CancellationToken cancellationToken)
|
CancellationToken cancellationToken)
|
||||||
{
|
{
|
||||||
@@ -1239,7 +1248,7 @@ public sealed class WorkerPipeSession
|
|||||||
{
|
{
|
||||||
if (!_acceptingCommands)
|
if (!_acceptingCommands)
|
||||||
{
|
{
|
||||||
return;
|
return false;
|
||||||
}
|
}
|
||||||
|
|
||||||
commandTask = ProcessCommandAsync(envelope, cancellationToken);
|
commandTask = ProcessCommandAsync(envelope, cancellationToken);
|
||||||
@@ -1254,6 +1263,60 @@ public sealed class WorkerPipeSession
|
|||||||
// would fault the discarded task with no observer. Close the gap the same way the rest of
|
// would fault the discarded task with no observer. Close the gap the same way the rest of
|
||||||
// the file does.
|
// the file does.
|
||||||
ObserveFaultWhenever(ObserveCommandTaskAsync(commandTask));
|
ObserveFaultWhenever(ObserveCommandTaskAsync(commandTask));
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Answers a command that arrived after shutdown stopped command intake with a
|
||||||
|
/// <see cref="ProtocolStatusCode.WorkerUnavailable"/> reply, and logs it. The worker is
|
||||||
|
/// going away, so the reply is best effort: if the pipe is already unusable the failure is
|
||||||
|
/// logged and the gateway falls back to its own correlation timeout, as it did before.
|
||||||
|
/// </summary>
|
||||||
|
private async Task RejectCommandAfterShutdownAsync(
|
||||||
|
WorkerEnvelope envelope,
|
||||||
|
CancellationToken cancellationToken)
|
||||||
|
{
|
||||||
|
MxCommandKind kind = envelope.WorkerCommand?.Command?.Kind ?? MxCommandKind.Unspecified;
|
||||||
|
const string message = "The worker is shutting down and no longer accepts commands.";
|
||||||
|
_logger?.Information(
|
||||||
|
"WorkerCommandRejectedAfterShutdown",
|
||||||
|
new Dictionary<string, object?>
|
||||||
|
{
|
||||||
|
["correlation_id"] = envelope.CorrelationId,
|
||||||
|
["command_kind"] = kind.ToString(),
|
||||||
|
["worker_state"] = _state.ToString(),
|
||||||
|
});
|
||||||
|
|
||||||
|
MxCommandReply reply = new()
|
||||||
|
{
|
||||||
|
SessionId = _options.SessionId,
|
||||||
|
CorrelationId = envelope.CorrelationId,
|
||||||
|
Kind = kind,
|
||||||
|
DiagnosticMessage = message,
|
||||||
|
ProtocolStatus = new ProtocolStatus
|
||||||
|
{
|
||||||
|
Code = ProtocolStatusCode.WorkerUnavailable,
|
||||||
|
Message = message,
|
||||||
|
},
|
||||||
|
};
|
||||||
|
|
||||||
|
try
|
||||||
|
{
|
||||||
|
await WriteControlReplyAsync(reply, cancellationToken).ConfigureAwait(false);
|
||||||
|
}
|
||||||
|
catch (Exception exception) when (exception is IOException
|
||||||
|
|| exception is ObjectDisposedException
|
||||||
|
|| exception is WorkerFrameProtocolException)
|
||||||
|
{
|
||||||
|
_logger?.Error(
|
||||||
|
"WorkerCommandRejectionReplyFailed",
|
||||||
|
new Dictionary<string, object?>
|
||||||
|
{
|
||||||
|
["correlation_id"] = envelope.CorrelationId,
|
||||||
|
["command_kind"] = kind.ToString(),
|
||||||
|
["reason"] = exception.Message,
|
||||||
|
});
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
private async Task ObserveCommandTaskAsync(Task commandTask)
|
private async Task ObserveCommandTaskAsync(Task commandTask)
|
||||||
@@ -1274,7 +1337,12 @@ public sealed class WorkerPipeSession
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
private void StopAcceptingCommands()
|
/// <summary>
|
||||||
|
/// Stops command intake. Internal so tests can reach the refusal path directly: the read
|
||||||
|
/// loop is serial and ends after a shutdown, so a command cannot race it over the pipe today,
|
||||||
|
/// but the guard must still answer if a future change lets one through.
|
||||||
|
/// </summary>
|
||||||
|
internal void StopAcceptingCommands()
|
||||||
{
|
{
|
||||||
lock (_commandTaskGate)
|
lock (_commandTaskGate)
|
||||||
{
|
{
|
||||||
|
|||||||
+11
-8
@@ -1,5 +1,8 @@
|
|||||||
# Still Pending — Deferred / Partial / Unfinished / Missing Functionality
|
# 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`.
|
**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`.
|
> **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-<op>-<n>`) 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`.
|
- **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-<op>-<n>`) 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.
|
- **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`.
|
- **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.
|
- **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).
|
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.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)** — `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.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** — completion-only operation-status bytes are kept as raw diagnostic metadata until the analysis proves an exact mapping. `docs/DesignDecisions.md:299-306`.
|
- 🟡 **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.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.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.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.
|
**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):**
|
**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).
|
- **§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.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 / §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.
|
- **§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.
|
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.
|
||||||
|
|||||||
Reference in New Issue
Block a user