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.
166 lines
7.3 KiB
C#
166 lines
7.3 KiB
C#
using Google.Protobuf;
|
|
using ZB.MOM.WW.MxGateway.Contracts.Proto;
|
|
|
|
namespace ZB.MOM.WW.MxGateway.Client.Tests;
|
|
|
|
/// <summary>
|
|
/// Tests for the credential-scrub (CLI-40) and malformed-reply (CLI-41) contracts on the
|
|
/// credential and id-returning session helpers, driven from shared behavior fixtures.
|
|
/// </summary>
|
|
public sealed class MxGatewaySessionReplyContractTests
|
|
{
|
|
/// <summary>
|
|
/// CLI-40: when MXAccess echoes the submitted credential back in its failure diagnostic,
|
|
/// the surfaced exception message must scrub it to the library redaction marker.
|
|
/// </summary>
|
|
[Fact]
|
|
public async Task AuthenticateUserAsync_RedactsEchoedCredentialInFailureMessage()
|
|
{
|
|
const string password = "sup3rSecretVerify9f3a2b";
|
|
FakeGatewayTransport transport = CreateTransport();
|
|
transport.AddInvokeReply(ReadReplyFixture("authenticate-user.echoed-credential.reply.json"));
|
|
await using MxGatewayClient client = CreateClient(transport);
|
|
MxGatewaySession session = await client.OpenSessionAsync();
|
|
|
|
MxAccessException exception = await Assert.ThrowsAsync<MxAccessException>(
|
|
async () => await session.AuthenticateUserAsync(12, "operator", password));
|
|
|
|
Assert.DoesNotContain(password, exception.Message, StringComparison.Ordinal);
|
|
Assert.Contains("<redacted>", exception.Message, StringComparison.Ordinal);
|
|
// ToString() is what logging frameworks emit; the secret-bearing original must not be
|
|
// chained as an inner exception where it would re-surface the credential verbatim.
|
|
Assert.DoesNotContain(password, exception.ToString(), StringComparison.Ordinal);
|
|
}
|
|
|
|
/// <summary>
|
|
/// CLI-40: the redacted exception must not leak the echoed credential through any structured
|
|
/// accessor either — <see cref="MxAccessException.Reply"/> (protocol message, diagnostic
|
|
/// message, and each MXSTATUS_PROXY diagnostic text) and <see cref="MxGatewayException.Statuses"/>
|
|
/// all carry the server-echoed credential verbatim before the fix. Both the OK+negative-HRESULT
|
|
/// and the MXACCESS_FAILURE reply route to <see cref="MxAccessException"/>, so both must scrub.
|
|
/// </summary>
|
|
/// <param name="fixture">The echoed-credential reply fixture to drive.</param>
|
|
[Theory]
|
|
[InlineData("authenticate-user.echoed-credential.reply.json")]
|
|
[InlineData("authenticate-user.echoed-credential-mxaccess-failure.reply.json")]
|
|
public async Task AuthenticateUserAsync_RedactsEchoedCredentialInStructuredAccessors(string fixture)
|
|
{
|
|
const string password = "sup3rSecretVerify9f3a2b";
|
|
FakeGatewayTransport transport = CreateTransport();
|
|
transport.AddInvokeReply(ReadReplyFixture(fixture));
|
|
await using MxGatewayClient client = CreateClient(transport);
|
|
MxGatewaySession session = await client.OpenSessionAsync();
|
|
|
|
MxAccessException exception = await Assert.ThrowsAsync<MxAccessException>(
|
|
async () => await session.AuthenticateUserAsync(12, "operator", password));
|
|
|
|
Assert.DoesNotContain(password, exception.Message, StringComparison.Ordinal);
|
|
Assert.Contains("<redacted>", exception.Message, StringComparison.Ordinal);
|
|
Assert.DoesNotContain(password, exception.ToString(), StringComparison.Ordinal);
|
|
Assert.DoesNotContain(password, exception.Reply.ProtocolStatus.Message, StringComparison.Ordinal);
|
|
Assert.DoesNotContain(password, exception.Reply.DiagnosticMessage, StringComparison.Ordinal);
|
|
foreach (MxStatusProxy status in exception.Reply.Statuses)
|
|
{
|
|
Assert.DoesNotContain(password, status.DiagnosticText, StringComparison.Ordinal);
|
|
}
|
|
|
|
foreach (MxStatusProxy status in exception.Statuses)
|
|
{
|
|
Assert.DoesNotContain(password, status.DiagnosticText, StringComparison.Ordinal);
|
|
}
|
|
}
|
|
|
|
/// <summary>
|
|
/// CLI-41: an OK reply that carries neither the typed AuthenticateUser payload nor an
|
|
/// int32 return_value is a malformed reply, surfaced as a typed exception rather than an NRE.
|
|
/// </summary>
|
|
[Fact]
|
|
public async Task AuthenticateUserAsync_MissingPayloadAndReturnValue_ThrowsMalformedReply()
|
|
{
|
|
FakeGatewayTransport transport = CreateTransport();
|
|
transport.AddInvokeReply(ReadReplyFixture("authenticate-user.missing-payload.reply.json"));
|
|
await using MxGatewayClient client = CreateClient(transport);
|
|
MxGatewaySession session = await client.OpenSessionAsync();
|
|
|
|
await Assert.ThrowsAsync<MxGatewayMalformedReplyException>(
|
|
async () => await session.AuthenticateUserAsync(12, "operator", "pw"));
|
|
}
|
|
|
|
/// <summary>
|
|
/// CLI-41: an OK reply that omits the typed payload but carries an int32 return_value
|
|
/// resolves to that return value.
|
|
/// </summary>
|
|
[Fact]
|
|
public async Task AuthenticateUserAsync_ReturnValueOnly_ResolvesReturnValue()
|
|
{
|
|
FakeGatewayTransport transport = CreateTransport();
|
|
transport.AddInvokeReply(ReadReplyFixture("authenticate-user.return-value-only.reply.json"));
|
|
await using MxGatewayClient client = CreateClient(transport);
|
|
MxGatewaySession session = await client.OpenSessionAsync();
|
|
|
|
int userId = await session.AuthenticateUserAsync(12, "operator", "pw");
|
|
|
|
Assert.Equal(7, userId);
|
|
}
|
|
|
|
/// <summary>
|
|
/// CLI-41: the AddBufferedItem fallback shares the malformed-reply contract — an OK reply
|
|
/// with neither a typed item handle nor an int32 return_value throws the typed exception.
|
|
/// </summary>
|
|
[Fact]
|
|
public async Task AddBufferedItemAsync_MissingPayloadAndReturnValue_ThrowsMalformedReply()
|
|
{
|
|
FakeGatewayTransport transport = CreateTransport();
|
|
transport.AddInvokeReply(new MxCommandReply
|
|
{
|
|
SessionId = "session-fixture",
|
|
Kind = MxCommandKind.AddBufferedItem,
|
|
ProtocolStatus = new ProtocolStatus { Code = ProtocolStatusCode.Ok },
|
|
});
|
|
await using MxGatewayClient client = CreateClient(transport);
|
|
MxGatewaySession session = await client.OpenSessionAsync();
|
|
|
|
await Assert.ThrowsAsync<MxGatewayMalformedReplyException>(
|
|
async () => await session.AddBufferedItemAsync(12, "Area001.Pump001.Speed", "runtime"));
|
|
}
|
|
|
|
private static MxGatewayClient CreateClient(FakeGatewayTransport transport)
|
|
{
|
|
return new MxGatewayClient(transport.Options, transport);
|
|
}
|
|
|
|
private static FakeGatewayTransport CreateTransport()
|
|
{
|
|
return new FakeGatewayTransport(new MxGatewayClientOptions
|
|
{
|
|
Endpoint = new Uri("http://localhost:5000"),
|
|
ApiKey = "test-api-key",
|
|
});
|
|
}
|
|
|
|
private static MxCommandReply ReadReplyFixture(string fileName)
|
|
{
|
|
DirectoryInfo directory = new(AppContext.BaseDirectory);
|
|
while (directory is not null)
|
|
{
|
|
string path = Path.Combine(
|
|
directory.FullName,
|
|
"clients",
|
|
"proto",
|
|
"fixtures",
|
|
"behavior",
|
|
"command-replies",
|
|
fileName);
|
|
|
|
if (File.Exists(path))
|
|
{
|
|
return JsonParser.Default.Parse<MxCommandReply>(File.ReadAllText(path));
|
|
}
|
|
|
|
directory = directory.Parent!;
|
|
}
|
|
|
|
throw new FileNotFoundException(fileName);
|
|
}
|
|
}
|