diff --git a/archreview/2026-07-12/remediation/00-tracking.md b/archreview/2026-07-12/remediation/00-tracking.md index c6ed0c2..6187f19 100644 --- a/archreview/2026-07-12/remediation/00-tracking.md +++ b/archreview/2026-07-12/remediation/00-tracking.md @@ -120,7 +120,7 @@ Full design + implementation for each row lives in the linked domain doc under i | CLI-42 | Low | P1 | S | — | Not started | Document the vendored Rust proto layout (CLI-02's missing doc half) | | CLI-43 | Low | — | S | — | Not started | Java style guide still prescribes "Java 21 preferred" | | CLI-44 | Low | — | S | — | Not started | Go event goroutine can mislabel a genuine terminal error as `ErrSlowConsumer` | -| CLI-45 | Low | P1 | M | — | Not started | Standardize CLI credential env-var names; fail fast on missing/empty passwords | +| CLI-45 | Low | P1 | M | — | Done | Standardize CLI credential env-var names; fail fast on missing/empty passwords | ### Testing, docs & gaps — [60-testing-docs-gaps.md](60-testing-docs-gaps.md) @@ -164,3 +164,4 @@ Sequence these together rather than piecemeal — several are one change set spa | 2026-08-07 | **TST-29 → `Done`:** migrated the Phase-5 (orphan-worker reattach) deferred-not-planned governance record and the settled Phase-4 Viewer-default decision from `oldtasks.md` into a new "Session-Resilience Epic Scope" entry in `docs/DesignDecisions.md`; repointed CLAUDE.md and `stillpending.md:7,165` from `oldtasks.md` to `docs/DesignDecisions.md` / `docs/plans/2026-06-15-session-resilience.md.tasks.json`; `git rm oldtasks.md`. The five untracked root docs-review artifacts (`MxAccessGateway-docs-{issues,fixed,final}.md`, `MxGatewayClient-docs-{issues,fixed}.md`) were absent from this worktree — delete from the main working tree separately. | | 2026-08-07 | **GWC-24 → `Done`** (branch `fix/gwc-24-staging-bound`). `WorkerClient._eventStaging` is now `Channel.CreateBounded` at `2 × EventChannelCapacity` (`Wait`, single reader/writer, no sync continuations); a rejected staging `TryWrite` faults the client `ProtocolViolation` with `QueueOverflow("worker-event-staging")` unless `IsTerminalState()` (shutdown stays a silent drop), so a consumer draining slower than its worker produces dies at a fixed ceiling instead of growing gateway memory. Queue-depth accounting moved from `EnqueueWorkerEventAsync` to `StageWorkerEvent`, so the single gauge reports staged + queued; the timed-write fault (`EventChannelFullModeTimeout` / `QueueOverflow("worker-events")`) is unchanged and still catches the full-stall case first. No new config key — total gateway-side buffering is `3 × MxGateway:Events:QueueCapacity`, derived; coordination with still-open old **GWC-21** (`EventChannelFullModeTimeout` configurability) remains open and was not blocked on. Docs same commit: `GatewayProcessDesign.md` (two overflow faults), `MxAccessWorkerInstanceDesign.md`, `GatewayConfiguration.md`, `Metrics.md`. Tests: `WorkerClientTests.StagingChannelOverflowFaultsWorkerWithoutWaitingForFullModeTimeout` and `.WorkerEventQueueDepthGaugeCountsStagedEvents`; `WorkerClientTests` 22/22 green, `NonWindows.slnx` builds with 0 warnings. | | 2026-08-07 | **ReplayGap end-to-end cluster (GWC-25 + CLI-35 + CLI-36) → `Done`** on `fix/gwc-25-replaygap-trio`. GWC-25: `SessionEventDistributor.RegisterWithReplay`'s empty-ring branch now reports `oldestAvailableSequence = _highestSequenceSeen + 1` when `gap == true` (still `0` when no gap), so the universal `oldest - 1` resume formula no longer wraps to `ulong.MaxValue` and dead-stream the subscriber; `docs/Sessions.md` documents the empty-ring value. CLI-35: the Python CLI renders a `ReplayGap` as a `{"replayGap": {...}}` row via a new `_event_row` helper instead of crashing in `MessageToDict`. CLI-36: the Go CLI branches on `result.IsReplayGap()` and prints the typed `REPLAY_GAP requested_after= oldest_available=` line / `replayGap` JSON row instead of formatting the library's cleared `Event`. `docs/CrossLanguageSmokeMatrix.md` gained a per-CLI gap-rendering table (one edit covering both client findings). Four new tests as designed (3 × `SessionEventDistributorTests`, `GatewayEndToEndReconnectReplayTests.ReconnectAfterFullAgeEvictionResumesWithSentinelFormula`) plus `test_stream_events_renders_replay_gap` (Python) and `TestRunStreamEventsPrintsReplayGap` (Go); all written red first and each reproducing its defect verbatim. **Deferred:** GWC-25's `ReplayGap.oldest_available_sequence` proto-comment amendment is **not** in this change — it is comment-only but triggers the full five-client regen fan-out, so it lands with the later codegen wave (alongside IPC-23's proto-comment edits) rather than forcing a regen for one sentence. Note for that wave: the fake-worker gateway e2e suite cannot run on the macOS worktree without `TMPDIR` shortened (macOS caps the Unix-domain-socket path backing .NET named pipes at 104 chars; `TMPDIR=/tmp dotnet test …` works and was used here). | +| 2026-08-07 | **CLI-45 → `Done`** on `fix/cli-45-credential-envvar`. All five CLIs now share one credential contract for `authenticate-user`: flags `--password` / `--password-env` (Go: `-password` / `-password-env`) defaulting to env `MXGATEWAY_VERIFY_PASSWORD`, resolution flag-then-env, and a resolved credential that is missing **or empty** is a usage error naming the flag and the variable — never the value, never sent to the wire. Go and Java previously authenticated with an empty password: Go now returns the guard error before dialing, Java throws a picocli `ParameterException` instead of falling back to `""`. Python's `--password-env` gained the canonical default (its `UsageError` was already conformant) and its message now names the resolved variable. Rust treats an empty `--password` or empty env value as missing (resolution extracted into a testable `resolve_verify_user_password`). .NET adopted the canonical flags and keeps its pre-existing names as **deprecated aliases for one release** — order: `--password`, `--verify-user-password`, the variable named by `--password-env` (or the deprecated `--verify-user-password-env`; default `MXGATEWAY_VERIFY_PASSWORD`), then `MXGATEWAY_VERIFY_USER_PASSWORD`. Tests: `TestRunAuthenticateUser{RejectsEmptyPassword,ReadsPasswordFromCanonicalEnv}` (Go), 3 picocli cases (Java), 3 click cases (Python), 2 clap/resolver cases (Rust), 4 xUnit cases covering the canonical flag, both env-name paths, the deprecated flag+env aliases, and the missing/empty failure (.NET). Docs same commit: `docs/CrossLanguageSmokeMatrix.md` gained a "Credential contract for `authenticate-user`" section **and** the per-CLI subcommand-coverage table — the half of this finding that is documented rather than fixed (.NET exposes all nine single-item session commands; Rust `unregister` + the credential pair; Go/Python/Java the credential pair only; verified against each dispatch table, and every gap is CLI surface only since all five *libraries* implement all nine helpers). All five client READMEs name the canonical variable and the fail-fast rule; the .NET README gained an `authenticate-user` credentials section carrying the deprecation note. **Deviation:** Java keeps `isBlank()` (per this design's "null or blank" wording for Java) where the other four test emptiness, so a whitespace-only credential is additionally rejected there. Verification (all five, on macOS): Go `gofmt -l .` clean, `go build ./...` clean, `go test ./...` ok; Java `JAVA_HOME=/opt/homebrew/opt/openjdk@17 gradle test` BUILD SUCCESSFUL, CLI suite 51 tests / 0 failures — **no generated-tree churn appeared this run**, `git status` for `clients/java/**/generated` clean with no revert needed (no `.proto` changed); Python `python -m pytest` 148 passed / 1 skipped (TLS opt-in); .NET `dotnet build …Client.slnx` 0 warnings / 0 errors and client tests 108 passed / 1 skipped (live-gateway opt-in); Rust `cargo fmt` (diff confined to the new code), `cargo check --workspace`, `cargo test --workspace` 100 tests across 6 targets all green, `cargo clippy --all-targets -- -D warnings` clean. | diff --git a/archreview/2026-07-12/remediation/50-clients.md b/archreview/2026-07-12/remediation/50-clients.md index 41d0d34..9ce06bb 100644 --- a/archreview/2026-07-12/remediation/50-clients.md +++ b/archreview/2026-07-12/remediation/50-clients.md @@ -26,7 +26,7 @@ Operating constraints carried from prior work: | CLI-42 | Low | P1 | S | — | Not started | Document the vendored Rust proto layout (CLI-02's missing doc half) | | CLI-43 | Low | — | S | — | Not started | Java style guide still prescribes "Java 21 preferred" | | CLI-44 | Low | — | S | — | Not started | Go event goroutine can mislabel a genuine terminal error as `ErrSlowConsumer` | -| CLI-45 | Low | P1 | M | — | Not started | Standardize CLI credential env-var name and fail fast on missing/empty passwords | +| CLI-45 | Low | P1 | M | — | Done | Standardize CLI credential env-var name and fail fast on missing/empty passwords | Cross-domain dependencies: **CLI-35/CLI-36 pair with GWC-25** (gateway emits `oldest_available_sequence = 0` on an empty replay ring — the server-side half of the same reconnect story; the CLI fixes here are independently landable but the end-to-end resume walk in the smoke matrix needs both). **CLI-39 pairs with the publishing process** (`scripts/pack-clients.ps1`, `scripts/tag-go-module.ps1`, Gitea package registry). diff --git a/clients/dotnet/README.md b/clients/dotnet/README.md index ebdd94c..4db57b4 100644 --- a/clients/dotnet/README.md +++ b/clients/dotnet/README.md @@ -258,6 +258,32 @@ optionally writes a value when `--type` and `--value` are supplied, reads a bounded event stream, and closes the session in a `finally` block. CLI error output redacts API keys supplied through `--api-key`. +### `authenticate-user` credentials + +```powershell +$env:MXGATEWAY_VERIFY_PASSWORD = "" +dotnet run --project clients/dotnet/ZB.MOM.WW.MxGateway.Client.Cli -- authenticate-user --session-id --server-handle 1 --verify-user operator --json +``` + +The credential comes from `--password` or, preferably, the environment variable +named by `--password-env` (default `MXGATEWAY_VERIFY_PASSWORD`) so it stays out +of shell history and the process table. It is never echoed to stdout or stderr, +and error output routes it through the same redaction seam as the API key. A +missing or empty resolved credential is a usage error naming the option and the +variable: the CLI fails before the invoke rather than authenticating with an +empty password. + +`MXGATEWAY_VERIFY_PASSWORD` is the canonical variable across all five client CLIs +— see [Cross-Language Smoke Matrix](../../docs/CrossLanguageSmokeMatrix.md). + +**Deprecated names.** This CLI previously used `--verify-user-password`, +`--verify-user-password-env`, and `MXGATEWAY_VERIFY_USER_PASSWORD`. All three +still resolve, for one release only, so existing scripts keep working; migrate to +the canonical names above. The full resolution order is `--password`, +`--verify-user-password`, the variable named by `--password-env` (or the +deprecated `--verify-user-password-env`, default `MXGATEWAY_VERIFY_PASSWORD`), +then `MXGATEWAY_VERIFY_USER_PASSWORD`. + ## Galaxy Repository Browse `GalaxyRepositoryClient` is a separate read-only wrapper around the diff --git a/clients/dotnet/ZB.MOM.WW.MxGateway.Client.Cli/MxGatewayClientCli.cs b/clients/dotnet/ZB.MOM.WW.MxGateway.Client.Cli/MxGatewayClientCli.cs index e6d0283..5320e3c 100644 --- a/clients/dotnet/ZB.MOM.WW.MxGateway.Client.Cli/MxGatewayClientCli.cs +++ b/clients/dotnet/ZB.MOM.WW.MxGateway.Client.Cli/MxGatewayClientCli.cs @@ -346,31 +346,78 @@ public static class MxGatewayClientCli } /// - /// Resolves the effective MXAccess verify-user credential from - /// --verify-user-password or, failing that, the - /// --verify-user-password-env-named environment variable (default - /// MXGATEWAY_VERIFY_USER_PASSWORD). The credential is never echoed; - /// this resolver exists so the error-redaction catch block can strip it - /// from any surfaced error (CLI-04), mirroring . + /// Canonical CLI credential environment variable, shared by every official + /// client CLI (CLI-45) so one exported variable drives the same operator + /// workflow in all five languages. + /// + private const string DefaultVerifyPasswordEnvironmentName = "MXGATEWAY_VERIFY_PASSWORD"; + + /// + /// Pre-CLI-45 environment variable, still honoured as a deprecated fallback + /// for one release so existing scripts keep working. + /// + private const string LegacyVerifyPasswordEnvironmentName = "MXGATEWAY_VERIFY_USER_PASSWORD"; + + /// + /// Resolves the name of the environment variable holding the verify-user + /// credential: --password-env, then the deprecated + /// --verify-user-password-env alias, then + /// MXGATEWAY_VERIFY_PASSWORD. + /// + private static string ResolveVerifyPasswordEnvironmentName(CliArguments arguments) + { + string? environmentName = arguments.GetOptional("password-env"); + if (!string.IsNullOrEmpty(environmentName)) + { + return environmentName; + } + + environmentName = arguments.GetOptional("verify-user-password-env"); + return string.IsNullOrEmpty(environmentName) + ? DefaultVerifyPasswordEnvironmentName + : environmentName; + } + + /// + /// Resolves the effective MXAccess verify-user credential in the CLI-45 + /// order: --password, the deprecated --verify-user-password + /// alias, the environment variable named by --password-env (default + /// MXGATEWAY_VERIFY_PASSWORD), then the deprecated + /// MXGATEWAY_VERIFY_USER_PASSWORD. An empty value from any source is + /// treated as absent. The credential is never echoed; this resolver exists so + /// the error-redaction catch block can strip it from any surfaced error + /// (CLI-04), mirroring . /// private static string? TryResolveVerifyUserPassword(CliArguments arguments) { - string? password = arguments.GetOptional("verify-user-password"); + string? password = arguments.GetOptional("password"); if (!string.IsNullOrEmpty(password)) { return password; } - string passwordEnvironmentName = arguments.GetOptional("verify-user-password-env") - ?? "MXGATEWAY_VERIFY_USER_PASSWORD"; + password = arguments.GetOptional("verify-user-password"); + if (!string.IsNullOrEmpty(password)) + { + return password; + } - return Environment.GetEnvironmentVariable(passwordEnvironmentName); + password = Environment.GetEnvironmentVariable(ResolveVerifyPasswordEnvironmentName(arguments)); + if (!string.IsNullOrEmpty(password)) + { + return password; + } + + password = Environment.GetEnvironmentVariable(LegacyVerifyPasswordEnvironmentName); + return string.IsNullOrEmpty(password) ? null : password; } /// /// Resolves the verify-user credential for authenticate-user, throwing - /// a redaction-safe error when neither the flag nor the env var is set. The - /// thrown message names only the option/env var, never the value. + /// a redaction-safe error when no source yields a non-empty value. Failing + /// fast keeps a misconfigured environment from becoming a real MXAccess + /// authentication attempt with an empty credential (CLI-45); the thrown + /// message names only the option/env var, never the value. /// private static string ResolveVerifyUserPassword(CliArguments arguments) { @@ -380,11 +427,10 @@ public static class MxGatewayClientCli return password; } - string passwordEnvironmentName = arguments.GetOptional("verify-user-password-env") - ?? "MXGATEWAY_VERIFY_USER_PASSWORD"; - throw new ArgumentException( - $"Verify-user password is required. Pass --verify-user-password or set {passwordEnvironmentName}."); + "Verify-user password is required. Pass --password or set " + + $"{ResolveVerifyPasswordEnvironmentName(arguments)} (deprecated aliases: " + + $"--verify-user-password, --verify-user-password-env, {LegacyVerifyPasswordEnvironmentName})."); } private static CancellationTokenSource CreateCancellation(CliArguments arguments, string command) @@ -710,8 +756,10 @@ public static class MxGatewayClientCli TextWriter output, CancellationToken cancellationToken) { - // The credential is resolved from --verify-user-password or its env var and - // is never echoed. On any surfaced error the RunCoreAsync catch block routes + // The credential is resolved from --password or its env var (default + // MXGATEWAY_VERIFY_PASSWORD) and is never echoed; a missing or empty value + // fails fast before the invoke rather than reaching the wire (CLI-45). + // On any surfaced error the RunCoreAsync catch block routes // it through MxGatewayCliSecretRedactor so it cannot reach stderr (CLI-04). return InvokeAndWriteAsync( arguments, @@ -2372,7 +2420,9 @@ public static class MxGatewayClientCli writer.WriteLine("mxgw-dotnet activate --session-id --server-handle --item-handle [--json]"); writer.WriteLine("mxgw-dotnet write-secured --session-id --server-handle --item-handle --type --value --current-user-id [--verifier-user-id ] [--json]"); writer.WriteLine("mxgw-dotnet write-secured2 --session-id --server-handle --item-handle --type --value --current-user-id [--verifier-user-id ] [--timestamp ] [--json]"); - writer.WriteLine("mxgw-dotnet authenticate-user --session-id --server-handle --verify-user (--verify-user-password | --verify-user-password-env ) [--json]"); + writer.WriteLine("mxgw-dotnet authenticate-user --session-id --server-handle --verify-user [--password ] [--password-env ] [--json]"); + writer.WriteLine(" credential: --password, else the --password-env variable (default MXGATEWAY_VERIFY_PASSWORD); required and never empty."); + writer.WriteLine(" deprecated aliases: --verify-user-password, --verify-user-password-env, MXGATEWAY_VERIFY_USER_PASSWORD."); writer.WriteLine("mxgw-dotnet archestra-user-to-id --session-id --server-handle --user-guid [--json]"); writer.WriteLine("mxgw-dotnet subscribe-bulk --session-id --server-handle --items [--json]"); writer.WriteLine("mxgw-dotnet unsubscribe-bulk --session-id --server-handle --item-handles [--json]"); diff --git a/clients/dotnet/ZB.MOM.WW.MxGateway.Client.Tests/MxGatewayClientCliTests.cs b/clients/dotnet/ZB.MOM.WW.MxGateway.Client.Tests/MxGatewayClientCliTests.cs index 8262caf..2a98d75 100644 --- a/clients/dotnet/ZB.MOM.WW.MxGateway.Client.Tests/MxGatewayClientCliTests.cs +++ b/clients/dotnet/ZB.MOM.WW.MxGateway.Client.Tests/MxGatewayClientCliTests.cs @@ -235,7 +235,7 @@ public sealed class MxGatewayClientCliTests "--session-id", "session-fixture", "--server-handle", "12", "--verify-user", "operator", - "--verify-user-password", password, + "--password", password, ], output, error, @@ -246,6 +246,235 @@ public sealed class MxGatewayClientCliTests Assert.Contains("[redacted]", error.ToString()); } + /// + /// CLI-45: --password is the primary credential flag, matching the other + /// four CLIs. The credential reaches the wire but never stdout/stderr. + /// + /// A task that represents the asynchronous operation. + [Fact] + public async Task RunAsync_AuthenticateUser_AcceptsCanonicalPasswordFlag() + { + const string password = "canonical-flag-credential"; + using var output = new StringWriter(); + using var error = new StringWriter(); + FakeCliClient fakeClient = new(); + fakeClient.InvokeReplies.Enqueue(new MxCommandReply + { + SessionId = "session-fixture", + Kind = MxCommandKind.AuthenticateUser, + ProtocolStatus = new ProtocolStatus { Code = ProtocolStatusCode.Ok }, + AuthenticateUser = new AuthenticateUserReply { UserId = 11 }, + }); + + int exitCode = await MxGatewayClientCli.RunAsync( + [ + "authenticate-user", + "--endpoint", "http://localhost:5000", + "--api-key", "test-api-key", + "--session-id", "session-fixture", + "--server-handle", "12", + "--verify-user", "operator", + "--password", password, + "--json", + ], + output, + error, + _ => fakeClient); + + Assert.Equal(0, exitCode); + MxCommandRequest request = Assert.Single(fakeClient.InvokeRequests); + Assert.Equal(password, request.Command.AuthenticateUser.VerifyUserPassword); + Assert.DoesNotContain(password, output.ToString(), StringComparison.Ordinal); + Assert.DoesNotContain(password, error.ToString(), StringComparison.Ordinal); + } + + /// + /// CLI-45: the credential is read from the environment variable named by + /// --password-env, whose default is the canonical + /// MXGATEWAY_VERIFY_PASSWORD shared by all five CLIs. + /// + /// A task that represents the asynchronous operation. + [Theory] + [InlineData(null, "MXGATEWAY_VERIFY_PASSWORD")] + [InlineData("MXGW_TEST_CLI45_ENV", "MXGW_TEST_CLI45_ENV")] + public async Task RunAsync_AuthenticateUser_ReadsCredentialFromNamedEnvironmentVariable( + string? passwordEnvArgument, + string environmentName) + { + const string password = "env-sourced-credential"; + using EnvironmentVariableScope scope = new(environmentName, password); + using var output = new StringWriter(); + using var error = new StringWriter(); + FakeCliClient fakeClient = new(); + fakeClient.InvokeReplies.Enqueue(new MxCommandReply + { + SessionId = "session-fixture", + Kind = MxCommandKind.AuthenticateUser, + ProtocolStatus = new ProtocolStatus { Code = ProtocolStatusCode.Ok }, + AuthenticateUser = new AuthenticateUserReply { UserId = 12 }, + }); + + List args = + [ + "authenticate-user", + "--endpoint", "http://localhost:5000", + "--api-key", "test-api-key", + "--session-id", "session-fixture", + "--server-handle", "12", + "--verify-user", "operator", + "--json", + ]; + if (passwordEnvArgument is not null) + { + args.Add("--password-env"); + args.Add(passwordEnvArgument); + } + + int exitCode = await MxGatewayClientCli.RunAsync([.. args], output, error, _ => fakeClient); + + Assert.Equal(0, exitCode); + MxCommandRequest request = Assert.Single(fakeClient.InvokeRequests); + Assert.Equal(password, request.Command.AuthenticateUser.VerifyUserPassword); + Assert.DoesNotContain(password, output.ToString(), StringComparison.Ordinal); + } + + /// + /// CLI-45: the pre-rename names stay usable for one release — the deprecated + /// --verify-user-password flag and the deprecated + /// MXGATEWAY_VERIFY_USER_PASSWORD environment variable both still resolve. + /// + /// A task that represents the asynchronous operation. + [Fact] + public async Task RunAsync_AuthenticateUser_HonoursDeprecatedAliases() + { + using var flagOutput = new StringWriter(); + using var flagError = new StringWriter(); + FakeCliClient flagClient = new(); + flagClient.InvokeReplies.Enqueue(new MxCommandReply + { + SessionId = "session-fixture", + Kind = MxCommandKind.AuthenticateUser, + ProtocolStatus = new ProtocolStatus { Code = ProtocolStatusCode.Ok }, + AuthenticateUser = new AuthenticateUserReply { UserId = 13 }, + }); + + int flagExitCode = await MxGatewayClientCli.RunAsync( + [ + "authenticate-user", + "--endpoint", "http://localhost:5000", + "--api-key", "test-api-key", + "--session-id", "session-fixture", + "--server-handle", "12", + "--verify-user", "operator", + "--verify-user-password", "legacy-flag-credential", + "--json", + ], + flagOutput, + flagError, + _ => flagClient); + + Assert.Equal(0, flagExitCode); + Assert.Equal( + "legacy-flag-credential", + Assert.Single(flagClient.InvokeRequests).Command.AuthenticateUser.VerifyUserPassword); + + using EnvironmentVariableScope canonical = new("MXGATEWAY_VERIFY_PASSWORD", null); + using EnvironmentVariableScope legacy = new("MXGATEWAY_VERIFY_USER_PASSWORD", "legacy-env-credential"); + using var envOutput = new StringWriter(); + using var envError = new StringWriter(); + FakeCliClient envClient = new(); + envClient.InvokeReplies.Enqueue(new MxCommandReply + { + SessionId = "session-fixture", + Kind = MxCommandKind.AuthenticateUser, + ProtocolStatus = new ProtocolStatus { Code = ProtocolStatusCode.Ok }, + AuthenticateUser = new AuthenticateUserReply { UserId = 14 }, + }); + + int envExitCode = await MxGatewayClientCli.RunAsync( + [ + "authenticate-user", + "--endpoint", "http://localhost:5000", + "--api-key", "test-api-key", + "--session-id", "session-fixture", + "--server-handle", "12", + "--verify-user", "operator", + "--json", + ], + envOutput, + envError, + _ => envClient); + + Assert.Equal(0, envExitCode); + Assert.Equal( + "legacy-env-credential", + Assert.Single(envClient.InvokeRequests).Command.AuthenticateUser.VerifyUserPassword); + } + + /// + /// CLI-45: a missing or empty credential fails fast before the invoke — the CLI + /// never sends a fabricated empty password to the wire. The error names the flag + /// and the environment variable, never a value. + /// + /// Whether to pass an explicit empty --password. + /// A task that represents the asynchronous operation. + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task RunAsync_AuthenticateUser_FailsFastOnMissingOrEmptyCredential(bool explicitEmptyFlag) + { + using EnvironmentVariableScope canonical = new("MXGATEWAY_VERIFY_PASSWORD", explicitEmptyFlag ? string.Empty : null); + using EnvironmentVariableScope legacy = new("MXGATEWAY_VERIFY_USER_PASSWORD", null); + using var output = new StringWriter(); + using var error = new StringWriter(); + FakeCliClient fakeClient = new(); + + List args = + [ + "authenticate-user", + "--endpoint", "http://localhost:5000", + "--api-key", "test-api-key", + "--session-id", "session-fixture", + "--server-handle", "12", + "--verify-user", "operator", + ]; + if (explicitEmptyFlag) + { + args.Add("--password"); + args.Add(string.Empty); + } + + int exitCode = await MxGatewayClientCli.RunAsync([.. args], output, error, _ => fakeClient); + + Assert.Equal(1, exitCode); + Assert.Empty(fakeClient.InvokeRequests); + Assert.Contains("--password", error.ToString(), StringComparison.Ordinal); + Assert.Contains("MXGATEWAY_VERIFY_PASSWORD", error.ToString(), StringComparison.Ordinal); + } + + /// + /// Sets an environment variable for the duration of a test and restores the + /// previous value on dispose, so credential-resolution tests do not depend on + /// (or leak into) the ambient environment. + /// + private sealed class EnvironmentVariableScope : IDisposable + { + private readonly string _name; + private readonly string? _original; + + public EnvironmentVariableScope(string name, string? value) + { + _name = name; + _original = Environment.GetEnvironmentVariable(name); + Environment.SetEnvironmentVariable(name, value); + } + + public void Dispose() + { + Environment.SetEnvironmentVariable(_name, _original); + } + } + /// Verifies that error output redacts sensitive API key values. /// A task that represents the asynchronous operation. [Fact] diff --git a/clients/go/README.md b/clients/go/README.md index c76cbbe..5653d32 100644 --- a/clients/go/README.md +++ b/clients/go/README.md @@ -177,7 +177,11 @@ parity holds: a `WriteSecured` issued without a matching prior `AuthenticateUser and supervisory advise fails natively, and that failure is surfaced unchanged rather than pre-empted. The CLI exposes `authenticate-user` (credential via `-password-env`, default `MXGATEWAY_VERIFY_PASSWORD`, or `-password`) and -`write-secured`. +`write-secured`. The credential is required: a missing or empty resolved value is +a usage error naming the flag and the variable, so the CLI fails before dialing +instead of authenticating with an empty password. `MXGATEWAY_VERIFY_PASSWORD` is +the canonical variable across all five client CLIs — see +[Cross-Language Smoke Matrix](../../docs/CrossLanguageSmokeMatrix.md). ### Array writes replace the whole array diff --git a/clients/go/cmd/mxgw-go/main.go b/clients/go/cmd/mxgw-go/main.go index 2695363..95898d1 100644 --- a/clients/go/cmd/mxgw-go/main.go +++ b/clients/go/cmd/mxgw-go/main.go @@ -446,6 +446,11 @@ func runWriteSecured(ctx context.Context, args []string, stdout, stderr io.Write return writeCommandOutput(stdout, *jsonOutput, "write-secured", options, reply, err) } +// defaultVerifyPasswordEnv is the canonical CLI credential environment variable, +// shared by every official client CLI (CLI-45) so one exported variable drives +// the same operator workflow in all five languages. +const defaultVerifyPasswordEnv = "MXGATEWAY_VERIFY_PASSWORD" + func runAuthenticateUser(ctx context.Context, args []string, stdout, stderr io.Writer) error { flags := flag.NewFlagSet("authenticate-user", flag.ContinueOnError) flags.SetOutput(stderr) @@ -458,7 +463,7 @@ func runAuthenticateUser(ctx context.Context, args []string, stdout, stderr io.W // prefer the environment variable so it stays out of shell history and the // process table. The -password flag remains for non-interactive scripting. password := flags.String("password", "", "verify-user password (prefer -password-env)") - passwordEnv := flags.String("password-env", "MXGATEWAY_VERIFY_PASSWORD", "environment variable containing the verify-user password") + passwordEnv := flags.String("password-env", defaultVerifyPasswordEnv, "environment variable containing the verify-user password") if err := flags.Parse(args); err != nil { return err @@ -471,8 +476,18 @@ func runAuthenticateUser(ctx context.Context, args []string, stdout, stderr io.W } resolvedPassword := *password - if resolvedPassword == "" && *passwordEnv != "" { - resolvedPassword = os.Getenv(*passwordEnv) + envName := *passwordEnv + if envName == "" { + envName = defaultVerifyPasswordEnv + } + if resolvedPassword == "" { + resolvedPassword = os.Getenv(envName) + } + // Fail fast rather than dialing: an unset or empty variable must not become a + // real MXAccess authentication attempt with an empty credential. The message + // names only the flag and the variable — never the resolved value. + if resolvedPassword == "" { + return fmt.Errorf("a password is required via -password or the %s environment variable", envName) } client, options, err := dialForCommand(ctx, common) diff --git a/clients/go/cmd/mxgw-go/main_test.go b/clients/go/cmd/mxgw-go/main_test.go index 3c5a4d2..f69c0b3 100644 --- a/clients/go/cmd/mxgw-go/main_test.go +++ b/clients/go/cmd/mxgw-go/main_test.go @@ -598,6 +598,69 @@ func TestRunAuthenticateUserRequiresVerifyUser(t *testing.T) { } } +// TestRunAuthenticateUserRejectsEmptyPassword pins the CLI-45 fail-fast contract: +// an unresolved credential must abort before dialing rather than authenticating +// with an empty password, and the usage error must name both -password and the +// canonical environment variable without echoing any value. +func TestRunAuthenticateUserRejectsEmptyPassword(t *testing.T) { + t.Setenv("MXGATEWAY_VERIFY_PASSWORD", "") + + var stdout, stderr bytes.Buffer + err := runWithIO(t.Context(), []string{ + "authenticate-user", + "-session-id", "s1", + "-verify-user", "operator", + "-plaintext", + "-api-key", "test", + }, &stdout, &stderr) + if err == nil { + t.Fatal("authenticate-user without a credential must fail before dialing") + } + if !strings.Contains(err.Error(), "-password") { + t.Fatalf("error must name the -password flag: %v", err) + } + if !strings.Contains(err.Error(), "MXGATEWAY_VERIFY_PASSWORD") { + t.Fatalf("error must name the canonical environment variable: %v", err) + } +} + +// TestRunAuthenticateUserReadsPasswordFromCanonicalEnv pins that the default +// -password-env is MXGATEWAY_VERIFY_PASSWORD: with it set the credential guard +// passes and the command proceeds past it to the dial, which fails against an +// unused port under a short context — proving the guard was cleared without +// needing a live gateway. +func TestRunAuthenticateUserReadsPasswordFromCanonicalEnv(t *testing.T) { + t.Setenv("MXGATEWAY_VERIFY_PASSWORD", "env-sourced-credential") + + ctx, cancel := context.WithTimeout(t.Context(), 200*time.Millisecond) + defer cancel() + + var stdout, stderr bytes.Buffer + err := runWithIO(ctx, []string{ + "authenticate-user", + "-session-id", "s1", + "-verify-user", "operator", + "-endpoint", "127.0.0.1:1", + "-plaintext", + "-api-key", "test", + "-call-timeout", "1s", + }, &stdout, &stderr) + if err == nil { + t.Fatal("expected the dial/RPC to fail against an unused port") + } + if strings.Contains(err.Error(), "flag provided but not defined") { + t.Fatalf("test invoked an unknown flag, so it never reached the guard: %v", err) + } + if strings.Contains(err.Error(), "a password is required") { + t.Fatalf("credential guard must be satisfied from %s: %v", "MXGATEWAY_VERIFY_PASSWORD", err) + } + if strings.Contains(err.Error(), "env-sourced-credential") || + strings.Contains(stdout.String(), "env-sourced-credential") || + strings.Contains(stderr.String(), "env-sourced-credential") { + t.Fatal("the resolved credential must never be echoed") + } +} + // TestRunWriteBulkVariantRejectsMismatchedHandlesAndValues pins the len-mismatch // guard so a write-bulk with unequal item-handles / values counts fails fast // before any dial. diff --git a/clients/java/README.md b/clients/java/README.md index 2e0b117..c10f2f4 100644 --- a/clients/java/README.md +++ b/clients/java/README.md @@ -167,8 +167,13 @@ session.write(serverHandle, itemHandle, value, userId); native failure is surfaced, not papered over. The CLI exposes `advise-supervisory`, `write-secured`, and `authenticate-user` -(credential via `--password` or `--password-env`, never echoed), and `write` / -`write2` take `--user-id`. +(credential via `--password` or the variable named by `--password-env`, default +`MXGATEWAY_VERIFY_PASSWORD`, never echoed), and `write` / `write2` take +`--user-id`. The credential is required: a missing or empty resolved value is a +picocli usage error naming the option and the variable, so the CLI fails before +connecting instead of authenticating with an empty password. +`MXGATEWAY_VERIFY_PASSWORD` is the canonical variable across all five client +CLIs — see [Cross-Language Smoke Matrix](../../docs/CrossLanguageSmokeMatrix.md). ### Array writes replace the whole array diff --git a/clients/java/zb-mom-ww-mxgateway-cli/src/main/java/com/zb/mom/ww/mxgateway/cli/MxGatewayCli.java b/clients/java/zb-mom-ww-mxgateway-cli/src/main/java/com/zb/mom/ww/mxgateway/cli/MxGatewayCli.java index 181f14b..9bb0f43 100644 --- a/clients/java/zb-mom-ww-mxgateway-cli/src/main/java/com/zb/mom/ww/mxgateway/cli/MxGatewayCli.java +++ b/clients/java/zb-mom-ww-mxgateway-cli/src/main/java/com/zb/mom/ww/mxgateway/cli/MxGatewayCli.java @@ -70,6 +70,7 @@ import picocli.CommandLine.Command; import picocli.CommandLine.Mixin; import picocli.CommandLine.Model.CommandSpec; import picocli.CommandLine.Option; +import picocli.CommandLine.ParameterException; import picocli.CommandLine.Spec; /** @@ -182,6 +183,13 @@ public final class MxGatewayCli implements Callable { /** Sentinel written to stdout after every command result in batch mode. */ static final String BATCH_EOR = "__MXGW_BATCH_EOR__"; + /** + * Canonical CLI credential environment variable, shared by every official + * client CLI (CLI-45) so one exported variable drives the same operator + * workflow in all five languages. + */ + static final String DEFAULT_VERIFY_PASSWORD_ENV = "MXGATEWAY_VERIFY_PASSWORD"; + /** Sentinel queued by {@code stream-alarms} to mark a clean end of the alarm feed. */ private static final Object ALARM_FEED_END = new Object(); @@ -1139,7 +1147,7 @@ public final class MxGatewayCli implements Callable { @Option( names = "--password-env", - defaultValue = "MXGATEWAY_VERIFY_PASSWORD", + defaultValue = DEFAULT_VERIFY_PASSWORD_ENV, description = "Environment variable holding the password when --password is omitted.") String passwordEnv; @@ -1151,11 +1159,20 @@ public final class MxGatewayCli implements Callable { public Integer call() { // Resolve the credential from the flag or environment. It flows only // into the request; it is never written to output, logs, or errors. + String environmentName = + passwordEnv == null || passwordEnv.isBlank() ? DEFAULT_VERIFY_PASSWORD_ENV : passwordEnv; String resolvedPassword = password == null || password.isBlank() - ? System.getenv(passwordEnv) + ? System.getenv(environmentName) : password; - if (resolvedPassword == null) { - resolvedPassword = ""; + if (resolvedPassword == null || resolvedPassword.isBlank()) { + // Fail fast instead of dialing: a misconfigured environment must not + // become a real MXAccess authentication attempt with an empty + // credential (CLI-45). The message names the option and the variable + // only — never the value. + throw new ParameterException( + common.spec.commandLine(), + "a password is required via --password or the " + environmentName + + " environment variable"); } try (MxGatewayCliClient client = clientFactory.connect(common.resolved())) { int userId = client.session(sessionId) diff --git a/clients/java/zb-mom-ww-mxgateway-cli/src/test/java/com/zb/mom/ww/mxgateway/cli/MxGatewayCliTests.java b/clients/java/zb-mom-ww-mxgateway-cli/src/test/java/com/zb/mom/ww/mxgateway/cli/MxGatewayCliTests.java index 9639cc2..587a108 100644 --- a/clients/java/zb-mom-ww-mxgateway-cli/src/test/java/com/zb/mom/ww/mxgateway/cli/MxGatewayCliTests.java +++ b/clients/java/zb-mom-ww-mxgateway-cli/src/test/java/com/zb/mom/ww/mxgateway/cli/MxGatewayCliTests.java @@ -2,7 +2,10 @@ package com.zb.mom.ww.mxgateway.cli; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assumptions.assumeTrue; import com.zb.mom.ww.mxgateway.client.MxGatewayAlarmFeedSubscription; import com.zb.mom.ww.mxgateway.client.MxGatewayClientOptions; @@ -211,6 +214,73 @@ final class MxGatewayCliTests { assertFalse(run.errors().contains("super-secret-pw"), "password must never be echoed to stderr"); } + /** + * CLI-45: an unresolved credential must abort with a picocli usage error + * before the CLI dials, instead of authenticating with an empty password. + * The message names the option and the variable, never a value. + */ + @Test + void authenticateUserRejectsMissingCredentialWithUsageError() { + FakeClientFactory factory = new FakeClientFactory(); + CliRun run = execute( + factory, + "authenticate-user", + "--session-id", "session-cli", + "--server-handle", "3", + "--verify-user", "operator", + "--password-env", "MXGW_CLI45_ABSENT_PASSWORD_VAR", + "--json"); + + assertNotEquals(0, run.exitCode(), "a missing credential must fail"); + assertTrue(run.errors().contains("--password"), run.errors()); + assertTrue(run.errors().contains("MXGW_CLI45_ABSENT_PASSWORD_VAR"), run.errors()); + assertNull(factory.client, "the CLI must not connect without a credential"); + } + + /** + * CLI-45: a blank {@code --password} is treated as missing — the CLI never + * sends a fabricated empty credential to the wire. + */ + @Test + void authenticateUserRejectsBlankPasswordValue() { + FakeClientFactory factory = new FakeClientFactory(); + CliRun run = execute( + factory, + "authenticate-user", + "--session-id", "session-cli", + "--server-handle", "3", + "--verify-user", "operator", + "--password", "", + "--password-env", "MXGW_CLI45_ABSENT_PASSWORD_VAR", + "--json"); + + assertNotEquals(0, run.exitCode(), "a blank credential must fail"); + assertNull(factory.client, "the CLI must not connect without a credential"); + } + + /** + * CLI-45: {@code --password-env} defaults to the canonical + * {@code MXGATEWAY_VERIFY_PASSWORD}, so the usage error names it when no + * explicit variable is given. Skipped if the canonical variable happens to be + * exported in the running environment (which would satisfy the credential). + */ + @Test + void authenticateUserDefaultsToCanonicalPasswordEnvName() { + assumeTrue(System.getenv(MxGatewayCli.DEFAULT_VERIFY_PASSWORD_ENV) == null); + + FakeClientFactory factory = new FakeClientFactory(); + CliRun run = execute( + factory, + "authenticate-user", + "--session-id", "session-cli", + "--server-handle", "3", + "--verify-user", "operator", + "--json"); + + assertNotEquals(0, run.exitCode()); + assertTrue(run.errors().contains("MXGATEWAY_VERIFY_PASSWORD"), run.errors()); + } + // ---- ping subcommand (D4) ---- @Test diff --git a/clients/python/README.md b/clients/python/README.md index 2e14f73..3b37ffe 100644 --- a/clients/python/README.md +++ b/clients/python/README.md @@ -187,7 +187,13 @@ await session.write_secured( ``` The CLI mirrors these as `authenticate-user` (credential via `--password` or, -preferably, `--password-env`) and `write-secured`. +preferably, the variable named by `--password-env`, default +`MXGATEWAY_VERIFY_PASSWORD`) and `write-secured`. The credential is required: a +missing or empty resolved value raises a `UsageError` naming the option and the +variable, so the CLI fails before connecting instead of authenticating with an +empty password. `MXGATEWAY_VERIFY_PASSWORD` is the canonical variable across all +five client CLIs — see +[Cross-Language Smoke Matrix](../../docs/CrossLanguageSmokeMatrix.md). ### Array writes replace the whole array diff --git a/clients/python/src/zb_mom_ww_mxgateway_cli/commands.py b/clients/python/src/zb_mom_ww_mxgateway_cli/commands.py index 65bf969..005949c 100644 --- a/clients/python/src/zb_mom_ww_mxgateway_cli/commands.py +++ b/clients/python/src/zb_mom_ww_mxgateway_cli/commands.py @@ -32,6 +32,11 @@ logger = logging.getLogger(__name__) MAX_AGGREGATE_EVENTS = 10_000 +#: Canonical CLI credential environment variable, shared by every official client +#: CLI (CLI-45) so one exported variable drives the same operator workflow in all +#: five languages. +DEFAULT_VERIFY_PASSWORD_ENV = "MXGATEWAY_VERIFY_PASSWORD" + _BATCH_EOR = "__MXGW_BATCH_EOR__" @@ -328,7 +333,8 @@ def write_secured(**kwargs: Any) -> None: ) @click.option( "--password-env", - default=None, + default=DEFAULT_VERIFY_PASSWORD_ENV, + show_default=True, help="Environment variable holding the user password.", ) @click.option("--correlation-id", default="", help="Client correlation id.") @@ -834,17 +840,23 @@ async def _authenticate_user(**kwargs: Any) -> dict[str, Any]: def _resolve_password(kwargs: dict[str, Any]) -> str: """Resolve the authenticate-user password from --password or --password-env. - Prefers the explicit flag, then falls back to the named environment - variable. The resolved secret is never echoed; callers pass it into the - ``secrets`` redaction list so it cannot leak through a surfaced error. + Prefers the explicit flag, then falls back to the environment variable named + by ``--password-env`` (default :data:`DEFAULT_VERIFY_PASSWORD_ENV`). A missing + *or empty* value from either source is a usage error (CLI-45): the CLI never + sends a fabricated empty credential to the wire. The error names the option + and the variable only — the resolved secret is never echoed, and callers pass + it into the ``secrets`` redaction list so it cannot leak through a surfaced + error either. """ + env_name = kwargs.get("password_env") or DEFAULT_VERIFY_PASSWORD_ENV password = kwargs.get("password") if not password: - env_name = kwargs.get("password_env") - password = os.environ.get(env_name) if env_name else None + password = os.environ.get(env_name) if not password: - raise click.UsageError("a password is required via --password or --password-env") + raise click.UsageError( + f"a password is required via --password or the {env_name} environment variable" + ) return password diff --git a/clients/python/tests/test_cli.py b/clients/python/tests/test_cli.py index 0da08a4..5b89587 100644 --- a/clients/python/tests/test_cli.py +++ b/clients/python/tests/test_cli.py @@ -752,7 +752,9 @@ def test_authenticate_user_reads_password_from_env(monkeypatch: pytest.MonkeyPat assert fake.last_request.command.authenticate_user.verify_user_password == "env-secret-pw" -def test_authenticate_user_requires_a_password() -> None: +def test_authenticate_user_requires_a_password(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.delenv("MXGATEWAY_VERIFY_PASSWORD", raising=False) + result = CliRunner().invoke( main, [ @@ -770,6 +772,81 @@ def test_authenticate_user_requires_a_password() -> None: assert result.exit_code != 0 assert "password is required" in result.output + # CLI-45: the usage error names the option and the canonical env var. + assert "--password" in result.output + assert "MXGATEWAY_VERIFY_PASSWORD" in result.output + + +def test_authenticate_user_reads_password_from_canonical_default_env( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """CLI-45: --password-env defaults to MXGATEWAY_VERIFY_PASSWORD. + + Exporting the canonical variable alone must satisfy the credential, with no + explicit --password-env flag — the same operator workflow as the other CLIs. + """ + from zb_mom_ww_mxgateway.generated import mxaccess_gateway_pb2 as pb + + reply = pb.MxCommandReply( + session_id="s1", + kind=pb.MX_COMMAND_KIND_AUTHENTICATE_USER, + protocol_status=pb.ProtocolStatus(code=pb.PROTOCOL_STATUS_CODE_OK), + authenticate_user=pb.AuthenticateUserReply(user_id=11), + ) + fake = _FakeInvokeClient(reply) + + async def fake_connect(options, **_kwargs): + return fake + + monkeypatch.setattr(commands_module.GatewayClient, "connect", fake_connect) + monkeypatch.setenv("MXGATEWAY_VERIFY_PASSWORD", "canonical-env-pw") + + result = CliRunner().invoke( + main, + [ + "authenticate-user", + "--plaintext", + "--session-id", + "s1", + "--server-handle", + "3", + "--verify-user", + "operator", + "--json", + ], + ) + + assert result.exit_code == 0, result.output + assert json.loads(result.output)["userId"] == 11 + assert "canonical-env-pw" not in result.output + assert fake.last_request.command.authenticate_user.verify_user_password == "canonical-env-pw" + + +def test_authenticate_user_rejects_empty_password_value( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """CLI-45: an empty resolved credential fails fast, never reaching the wire.""" + monkeypatch.setenv("MXGATEWAY_VERIFY_PASSWORD", "") + + result = CliRunner().invoke( + main, + [ + "authenticate-user", + "--plaintext", + "--session-id", + "s1", + "--server-handle", + "3", + "--verify-user", + "operator", + "--password", + "", + "--json", + ], + ) + + assert result.exit_code != 0 + assert "password is required" in result.output def test_write_secured_command_does_not_echo_value_on_failure( diff --git a/clients/rust/README.md b/clients/rust/README.md index 2f1b951..a4aa80a 100644 --- a/clients/rust/README.md +++ b/clients/rust/README.md @@ -216,7 +216,12 @@ the wire — the client never logs them and never embeds them in an `Error`'s `Display`/`Debug`; the only error text that can surface (from `tonic::Status` messages and reply diagnostics) is scrubbed by the credential-redaction seam. The CLI mirrors these as `authenticate-user` (password via `--password` or the -`--password-env` env var, never echoed) and `write-secured`. +variable named by `--password-env`, default `MXGATEWAY_VERIFY_PASSWORD`, never +echoed) and `write-secured`. The credential is required: a missing or empty +resolved value is a usage error naming the flag and the variable, so the CLI +fails before dialing instead of authenticating with an empty password. +`MXGATEWAY_VERIFY_PASSWORD` is the canonical variable across all five client +CLIs — see [Cross-Language Smoke Matrix](../../docs/CrossLanguageSmokeMatrix.md). The remaining single-item command helpers round out MXAccess parity: `unregister`, `suspend` / `activate` (each returns the operation's diff --git a/clients/rust/crates/mxgw-cli/src/main.rs b/clients/rust/crates/mxgw-cli/src/main.rs index 056d510..e28d0b5 100644 --- a/clients/rust/crates/mxgw-cli/src/main.rs +++ b/clients/rust/crates/mxgw-cli/src/main.rs @@ -752,17 +752,7 @@ async fn dispatch(command: Command) -> Result<(), Error> { password_env, json, } => { - // Resolve the credential from --password or the named env var. - // The password is passed straight to the typed helper and is never - // echoed to stdout/stderr or embedded in an error message. - let verify_user_password = password - .or_else(|| env::var(&password_env).ok()) - .ok_or_else(|| Error::InvalidArgument { - name: "password".to_owned(), - detail: format!( - "supply --password or set the environment variable `{password_env}`" - ), - })?; + let verify_user_password = resolve_verify_user_password(password, &password_env)?; let session = session_for(connection, session_id).await?; let user_id = session .authenticate_user(server_handle, &verify_user, &verify_user_password) @@ -1736,6 +1726,31 @@ fn print_ok(operation: &str, use_json: bool) { } } +/// Resolves the `authenticate-user` credential from `--password`, falling back to +/// the environment variable named by `--password-env` (default +/// `MXGATEWAY_VERIFY_PASSWORD`). +/// +/// An empty value from either source counts as missing (CLI-45): the CLI fails +/// fast with a usage error rather than sending a fabricated empty credential to +/// the wire. The error names the flag and the variable only — never the value, +/// which is never echoed to stdout/stderr or embedded in an error message. +fn resolve_verify_user_password( + password: Option, + password_env: &str, +) -> Result { + password + .filter(|value| !value.is_empty()) + .or_else(|| { + env::var(password_env) + .ok() + .filter(|value| !value.is_empty()) + }) + .ok_or_else(|| Error::InvalidArgument { + name: "password".to_owned(), + detail: format!("supply --password or set the environment variable `{password_env}`"), + }) +} + fn print_bulk_results( operation: &str, results: &[zb_mom_ww_mxgateway_client::generated::mxaccess_gateway::v1::SubscribeResult], @@ -2617,6 +2632,66 @@ mod tests { assert!(parsed.is_ok(), "parse failed: {parsed:?}"); } + /// CLI-45: `--password-env` must default to the canonical + /// `MXGATEWAY_VERIFY_PASSWORD` shared by every official client CLI. + #[test] + fn authenticate_user_password_env_defaults_to_canonical_name() { + let parsed = Cli::try_parse_from([ + "mxgw", + "authenticate-user", + "--session-id", + "session-1", + "--server-handle", + "7", + "--verify-user", + "verifier", + ]) + .expect("parse"); + match parsed.command { + Command::AuthenticateUser { password_env, .. } => { + assert_eq!(password_env, "MXGATEWAY_VERIFY_PASSWORD"); + } + other => panic!("expected authenticate-user, got {other:?}"), + } + } + + /// CLI-45: a credential that resolves to an empty string — whether from the + /// flag or from the named environment variable — is treated as missing, so + /// the CLI never sends a fabricated empty password to the wire. The usage + /// error names the flag and the variable, never a value. + #[test] + fn resolve_verify_user_password_rejects_missing_and_empty_values() { + const ABSENT: &str = "MXGW_CLI45_ABSENT_PASSWORD_VAR"; + const EMPTY: &str = "MXGW_CLI45_EMPTY_PASSWORD_VAR"; + const PRESENT: &str = "MXGW_CLI45_PRESENT_PASSWORD_VAR"; + std::env::remove_var(ABSENT); + std::env::set_var(EMPTY, ""); + std::env::set_var(PRESENT, "env-sourced-credential"); + + for (password, env_name) in [ + (None, ABSENT), + (Some(String::new()), ABSENT), + (None, EMPTY), + (Some(String::new()), EMPTY), + ] { + let error = super::resolve_verify_user_password(password, env_name) + .expect_err("empty or missing credential must be a usage error"); + let rendered = error.to_string(); + assert!(rendered.contains("--password"), "{rendered}"); + assert!(rendered.contains(env_name), "{rendered}"); + } + + assert_eq!( + super::resolve_verify_user_password(None, PRESENT).expect("env credential"), + "env-sourced-credential" + ); + assert_eq!( + super::resolve_verify_user_password(Some("flag-credential".to_owned()), EMPTY) + .expect("flag credential"), + "flag-credential" + ); + } + #[test] fn parses_write_secured_command() { let parsed = Cli::try_parse_from([ diff --git a/docs/CrossLanguageSmokeMatrix.md b/docs/CrossLanguageSmokeMatrix.md index e0547b3..acd1a37 100644 --- a/docs/CrossLanguageSmokeMatrix.md +++ b/docs/CrossLanguageSmokeMatrix.md @@ -90,6 +90,37 @@ The shared inputs are: The commands in the matrix use `MXGATEWAY_API_KEY` through each CLI's `api-key-env` flag. They must not embed bearer tokens or raw API keys. +### Credential contract for `authenticate-user` + +Every CLI resolves the MXAccess verify-user credential the same way, so one +exported variable drives the same operator workflow in all five languages: + +| Variable | Default | Purpose | +|----------|---------|---------| +| `MXGATEWAY_VERIFY_PASSWORD` | Empty | Verify-user credential read by `authenticate-user` when `--password` is omitted. | + +- Flags are `--password` (Go: `-password`) for an explicit value and + `--password-env` (Go: `-password-env`) for the *name* of the environment + variable, defaulting to `MXGATEWAY_VERIFY_PASSWORD`. +- Resolution order is flag, then environment variable. Prefer the variable: the + flag puts the secret in shell history and the process table. +- A resolved credential that is **missing or empty** is a usage error. The CLI + fails fast before dialing rather than authenticating with an empty password, + and the error names only the flag and the variable — never the value. Nothing + echoes the credential to stdout, stderr, or logs. + +This is CLI argument validation, not an MXAccess parity exception: the client +*libraries* still transmit whatever credential they are given. Only the operator +tools refuse to fabricate an empty one. + +The .NET CLI accepted `--verify-user-password`, `--verify-user-password-env`, and +`MXGATEWAY_VERIFY_USER_PASSWORD` before this contract was unified. Those names +remain as deprecated aliases for one release; new scripts must use the canonical +names above. The full .NET resolution order is `--password`, +`--verify-user-password`, the variable named by `--password-env` (or the +deprecated `--verify-user-password-env`, default `MXGATEWAY_VERIFY_PASSWORD`), +then `MXGATEWAY_VERIFY_USER_PASSWORD`. + ### TLS variant The matrix runs over plaintext (`h2c`) by default. A TLS variant exists but stays @@ -132,6 +163,31 @@ for quick local checks, but the full cross-language matrix uses explicit operation commands because not every bundled smoke command streams events yet. The explicit sequence remains the parity baseline for issue-level validation. +## Per-CLI Subcommand Coverage + +The matrix sequence itself is available everywhere, but the later single-item +session commands were not added to every CLI at the same time. A runner that +reaches beyond the required sequence must branch on language, so the current +deltas are specified here rather than left to be discovered: + +| Subcommand | .NET | Rust | Go | Python | Java | +|------------|------|------|----|--------|------| +| `unregister` | yes | yes | no | no | no | +| `add-buffered-item` | yes | no | no | no | no | +| `set-buffered-update-interval` | yes | no | no | no | no | +| `suspend` | yes | no | no | no | no | +| `activate` | yes | no | no | no | no | +| `write-secured` | yes | yes | yes | yes | yes | +| `write-secured2` | yes | no | no | no | no | +| `authenticate-user` | yes | yes | yes | yes | yes | +| `archestra-user-to-id` | yes | no | no | no | no | + +Only .NET exposes all nine. Rust adds `unregister` and the credential pair; Go, +Python, and Java expose the credential pair only. Every gap is CLI surface only — +all five *libraries* implement all nine typed helpers, so a gap is a missing +operator command, never a missing capability. Levelling the CLIs is separate +feature work and is not tracked as a defect here. + ## Validation Run the matrix shape tests after changing the smoke matrix: