Merge branch 'fix/cli-45-credential-envvar'
# Conflicts: # archreview/2026-07-12/remediation/00-tracking.md
This commit is contained in:
@@ -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
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
|
||||
@@ -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(
|
||||
|
||||
Reference in New Issue
Block a user