Compare commits

...

12 Commits

Author SHA1 Message Date
Joseph Doherty 61c00d59a9 feat(gateway): readiness check for the worker executable (#206)
ci / nightly-windev (push) Has been skipped
ci / windows-x86 (push) Successful in 1m27s
ci / java (push) Successful in 2m13s
ci / portable (push) Failing after 4m25s
Readiness covered only the auth store, so a gateway whose worker exe was
missing or of the wrong architecture reported ready while every OpenSession
failed. The new ready-tier check worker-executable applies the launcher's own
rule (path resolution, .exe, presence, PE architecture), now shared as
WorkerExecutableValidator.ResolveAndValidate, and reports only the launch
error code (the endpoint is anonymous). The session check stays on the
active tier, so zero sessions keeps readiness green.

The bounded Galaxy SQL check is not in this change: its tier, failure status
and probe cadence need a decision (see the issue).

Tests: WorkerExecutableHealthCheckTests (healthy, missing, wrong architecture,
and the ready-tier registration, which fails without the wiring).
2026-11-06 16:13:09 -05:00
Joseph Doherty 574369b666 feat(mxgw-go): add -require-certificate-validation for strict TLS (#199)
Over TLS without -ca-cert the CLI was always skip-verify, with no way to ask
for verification against the system roots even though the library supports
it. The new common flag is wired to Options.RequireCertificateValidation in
both the gateway and Galaxy dialers and echoed in the redacted JSON options.

Test: tls_flag_test.go starts a self-signed TLS gRPC gateway; the default
open-session succeeds (positive control) and the same call with the flag
fails to dial (fails on the old code with 'flag provided but not defined').
2026-11-06 16:10:39 -05:00
Joseph Doherty 9351ec1d2c feat(client-go): typed Unauthenticated vs PermissionDenied errors (#198)
Every RPC failure surfaced as a GatewayError with no way to tell a rejected
key from a missing scope, though docs/ClientLibrariesDesign.md requires the
two be treated distinctly. GatewayError now matches the new sentinels
ErrUnauthenticated and ErrPermissionDenied via errors.Is (by gRPC status code
only, so it works through the secret-redaction wrapper) and exposes Code().

Tests: errors_auth_test.go — sentinel matching per code, Code(), and a bufconn
AcknowledgeAlarm round trip proving the match survives the wire and the
password stays redacted (both behavioural tests fail without GatewayError.Is).
2026-11-06 16:08:51 -05:00
Joseph Doherty b9096f7c57 fix(client-dotnet): size the safe-unary retry budget for every attempt (#197)
ExecuteSafeUnaryAsync capped the whole retry pipeline at one
DefaultCallTimeout while each attempt carried the same deadline, so a first
attempt ending in DeadlineExceeded spent the entire budget and the retry
never ran. The outer cap is now MaxAttempts x DefaultCallTimeout plus MaxDelay
per backoff (MxGatewayClientRetryPolicy.SafeUnaryBudget), in both
MxGatewayClient and GalaxyRepositoryClient; a budget too large for a
cancellation timer leaves only the per-attempt deadlines.

Tests: MxGatewayClientWireTests.SafeUnaryRetry_AfterDeadlineExceededFirstAttempt_RunsTheRetry
(real gRPC loopback server stalls the first Invoke; fails with Cancelled on
the old code), plus two SafeUnaryBudget unit tests.
2026-11-06 16:07:35 -05:00
Joseph Doherty ff397b27be fix(gateway): remove the worker startup-probe retry options that could never retry (#188)
WorkerProcessStartedProbe only checks HasExited and throws a non-retryable
WorkerProcessLaunchException, so StartupProbeRetryAttempts and
StartupProbeRetryDelayMilliseconds configured a Polly retry that no shipped
probe could trigger. Real readiness is WorkerClient's WorkerReady handshake
under the same StartupTimeoutSeconds budget, so the probe now runs once and
the two options, their validation, the Polly pipeline (and the server's now
unused Polly.Core reference) and their docs are removed. A stale value in
configuration is ignored by the binder.

Tests: WorkerProcessLauncherTests.LaunchAsync_WhenStartupProbeFails_DoesNotRetryTheProbe,
WorkerOptions_HasNoStartupProbeRetryOptions (both fail on the old code).
2026-11-06 16:05:42 -05:00
Joseph Doherty 3a3d9e8e06 fix(worker): reply WorkerUnavailable to commands that race shutdown (#182)
TryStartCommandTask returned silently when command intake had stopped, so a
command refused during shutdown got no reply and no log and the gateway's
correlation wait ran out its own timeout. The refusal now writes a
WorkerUnavailable reply for the correlation and logs
WorkerCommandRejectedAfterShutdown. StopAcceptingCommands is internal so the
test can reach the guard (the serial read loop cannot race it today).

Test: WorkerPipeSessionTests.CommandAfterShutdown_RepliesWorkerUnavailable.
2026-11-06 16:03:47 -05:00
Joseph Doherty e5e7f1854f chore(clients/dotnet): bump Contracts + Client 0.2.1 -> 0.3.0 for publish (#214)
ci / nightly-windev (push) Has been skipped
ci / windows-x86 (push) Successful in 1m23s
ci / java (push) Successful in 2m19s
ci / portable (push) Failing after 4m35s
0.2.1 on the feed predates the 488649c AcknowledgeAlarmRequest
operator_password/operator_domain fields; 0.3.0 ships them so OtOpcUa can
adopt operator-authenticated alarm ack (lmxopcua#749).
2026-11-06 16:00:21 -05:00
Joseph Doherty 777fef9cad fix(sessions): drop the deny-NETWORK pipe ACE (#187)
ci / windows-x86 (push) Successful in 1m26s
ci / nightly-windev (push) Has been skipped
ci / java (push) Successful in 2m30s
ci / portable (push) Failing after 4m47s
NT AUTHORITY\NETWORK marks a network logon, not a remote caller. On windev
a gateway run from an SSH session carries it in its own token, so the deny
entry locked the gateway's same-machine workers out (UnauthorizedAccess on
connect; 15 fake-worker tests failed). The ACL now holds exactly one entry:
FullControl for the gateway's user.
2026-11-06 15:52:46 -05:00
Joseph Doherty 34a7453ac3 fix(sessions): restrict the worker named pipe to the gateway's account (#187)
ci / nightly-windev (push) Has been skipped
ci / windows-x86 (push) Successful in 1m23s
ci / java (push) Successful in 2m20s
ci / portable (push) Failing after 4m46s
On Windows each worker pipe is created via NamedPipeServerStreamAcl with a
protected DACL granting FullControl only to the gateway process user (the
worker is launched under the same account) and denying NETWORK logons. On
non-Windows hosts (test matrix) the pipe uses PipeOptions.CurrentUserOnly.
Tests assert the factory ACL and the ACL read back from the created pipe
(Windows), and that a same-user client still connects.
2026-11-06 15:45:50 -05:00
Joseph Doherty 9354d69bf9 fix(diagnostics): redact WriteSecuredBulk/WriteSecured2Bulk payloads (#193)
Adds both secured bulk kinds to GatewayLogRedactor's credential-bearing set,
plus a contract cross-check test that walks every MxCommand payload descriptor
and fails if a secured or password-bearing command kind is not redacted.
2026-11-06 15:45:50 -05:00
Joseph Doherty 5a235776b6 docs: point markdown backlogs at Gitea issues (status lives in Gitea)
ci / nightly-windev (push) Has been skipped
ci / windows-x86 (push) Successful in 1m28s
ci / java (push) Successful in 2m29s
ci / portable (push) Failing after 4m58s
2026-11-06 15:32:27 -05:00
Joseph Doherty 8c0a51dc90 Merge branch 'fix/review-2026-10-21' into main (2026-10-21 system review remediation) 2026-10-27 09:56:50 -04:00
49 changed files with 1229 additions and 241 deletions
+7
View File
@@ -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 |
+5
View File
@@ -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.
+5
View File
@@ -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.
+4 -2
View File
@@ -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
+4 -1
View File
@@ -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
View File
@@ -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 |
|---|---| |---|---|
+20 -12
View File
@@ -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
} }
+105
View File
@@ -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())
}
}
+42 -1
View File
@@ -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 {
+85
View File
@@ -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)
}
})
}
}
+4 -1
View File
@@ -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
+9
View File
@@ -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
View File
@@ -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
+4 -6
View File
@@ -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
-2
View File
@@ -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
View File
@@ -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
+5 -1
View File
@@ -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`,
+9 -11
View File
@@ -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.
+5 -2
View File
@@ -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
View File
@@ -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.