Joseph Doherty
0d874f91ee
Code-review follow-up on the CLI-40/41/44 branch. ISSUE 1 (all five, critical): the message-only scrub still leaked the server-echoed credential through the redacted error's structured reply accessor (.NET Reply/Statuses, Java reply()/protocolStatus(), Go MxAccessError.Reply via errors.As, Rust reply()/into_reply(), Python raw_reply). The redacted error now carries a scrubbed clone of the reply (protocol_status.message, diagnostic_message, statuses[].diagnostic_text), with per-language tests asserting the reply accessor no longer contains the credential. ISSUE 2 (Rust, critical): ensure_command_success routed MXACCESS_FAILURE to Error::Command (unlike the other four clients), bypassing attach_secrets and leaking via derived Debug/Display. MXACCESS_FAILURE now routes to Error::MxAccess, fixing the cross-client inconsistency. ISSUE 3 (Go, important): the CLI-44 terminal send was unconditionally non-blocking, dropping a genuine terminal error under a full buffer on the never-drop SubscribeEvents path. It is now reserved-slot-non-blocking only for the cancel-on-overflow path and blocking for the never-drop path. New shared fixture authenticate-user.echoed-credential-mxaccess-failure.reply.json wired into all five suites. Minors: whitespace-secret guard on .NET/Java redact helpers; Java preserves exception subtype on redaction; redaction-helper unit tests (Go/Java/.NET). Docs (ClientBehaviorFixtures.md, ClientLibrariesDesign.md) updated to make the structured-field claim true.
113 lines
4.0 KiB
Python
113 lines
4.0 KiB
Python
"""Tests for the uniform malformed-reply contract (CLI-41) and the CLI-40
|
|
credential-redaction regression, driven through the shared fixtures.
|
|
|
|
CLI-41: an OK reply that carries neither the expected typed payload nor a usable
|
|
``return_value`` int32 fallback raises :class:`MalformedReplyError`; a legacy
|
|
reply that populates only ``return_value`` falls back to that int32.
|
|
|
|
CLI-40: an OK reply whose diagnostics echo the caller's credential must never
|
|
surface that credential in the raised error message.
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
import json
|
|
from pathlib import Path
|
|
|
|
import pytest
|
|
from google.protobuf.json_format import ParseDict
|
|
|
|
from zb_mom_ww_mxgateway import MalformedReplyError, MxAccessError
|
|
from zb_mom_ww_mxgateway.generated import mxaccess_gateway_pb2 as pb
|
|
|
|
from test_typed_command_helpers import _session_with
|
|
|
|
FIXTURE_ROOT = Path(__file__).resolve().parents[2] / "proto" / "fixtures" / "behavior"
|
|
|
|
|
|
def _load_reply(relative: str) -> pb.MxCommandReply:
|
|
path = FIXTURE_ROOT / relative
|
|
return ParseDict(json.loads(path.read_text()), pb.MxCommandReply())
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_authenticate_user_missing_payload_raises_malformed_reply() -> None:
|
|
reply = _load_reply("command-replies/authenticate-user.missing-payload.reply.json")
|
|
session, _ = await _session_with([reply])
|
|
|
|
with pytest.raises(MalformedReplyError) as captured:
|
|
await session.authenticate_user(12, "operator", "any-password")
|
|
|
|
assert captured.value.raw_reply is reply
|
|
assert "malformed reply" in str(captured.value)
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_authenticate_user_return_value_only_falls_back_to_int32() -> None:
|
|
reply = _load_reply("command-replies/authenticate-user.return-value-only.reply.json")
|
|
session, _ = await _session_with([reply])
|
|
|
|
user_id = await session.authenticate_user(12, "operator", "any-password")
|
|
|
|
assert user_id == 7
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_add_buffered_item_falls_back_to_return_value_int32() -> None:
|
|
reply = pb.MxCommandReply(
|
|
session_id="session-1",
|
|
kind=pb.MX_COMMAND_KIND_ADD_BUFFERED_ITEM,
|
|
protocol_status=pb.ProtocolStatus(code=pb.PROTOCOL_STATUS_CODE_OK),
|
|
return_value=pb.MxValue(int32_value=99),
|
|
)
|
|
session, _ = await _session_with([reply])
|
|
|
|
item_handle = await session.add_buffered_item(12, "Object.Attribute", "ctx")
|
|
|
|
assert item_handle == 99
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_add_buffered_item_missing_payload_raises_malformed_reply() -> None:
|
|
reply = pb.MxCommandReply(
|
|
session_id="session-1",
|
|
kind=pb.MX_COMMAND_KIND_ADD_BUFFERED_ITEM,
|
|
protocol_status=pb.ProtocolStatus(code=pb.PROTOCOL_STATUS_CODE_OK),
|
|
)
|
|
session, _ = await _session_with([reply])
|
|
|
|
with pytest.raises(MalformedReplyError) as captured:
|
|
await session.add_buffered_item(12, "Object.Attribute", "ctx")
|
|
|
|
assert captured.value.raw_reply is reply
|
|
|
|
|
|
@pytest.mark.parametrize(
|
|
"fixture",
|
|
[
|
|
"command-replies/authenticate-user.echoed-credential.reply.json",
|
|
"command-replies/authenticate-user.echoed-credential-mxaccess-failure.reply.json",
|
|
],
|
|
)
|
|
@pytest.mark.asyncio
|
|
async def test_authenticate_user_echoed_credential_is_scrubbed(fixture: str) -> None:
|
|
credential = "sup3rSecretVerify9f3a2b"
|
|
reply = _load_reply(fixture)
|
|
session, _ = await _session_with([reply])
|
|
|
|
with pytest.raises(MxAccessError) as captured:
|
|
await session.authenticate_user(12, "operator", credential)
|
|
|
|
exc = captured.value
|
|
message = str(exc)
|
|
assert credential not in message
|
|
assert "[redacted]" in message
|
|
|
|
# The credential must not survive in the structured protobuf context either:
|
|
# a logger dumping raw_reply's fields would otherwise reintroduce the leak.
|
|
assert exc.raw_reply is not None
|
|
assert credential not in exc.raw_reply.protocol_status.message
|
|
assert credential not in exc.raw_reply.diagnostic_message
|
|
for status in exc.raw_reply.statuses:
|
|
assert credential not in status.diagnostic_text
|