Merge branch 'fix/cli-40-41-44'
This commit is contained in:
Joseph Doherty
File diff suppressed because one or more lines are too long
@@ -21,11 +21,11 @@ Operating constraints carried from prior work:
|
||||
| CLI-37 | Medium | P1 | M | CLI-38 | Done | Status-array validation must branch on `category` per the proto contract (4-vs-1 divergence) |
|
||||
| CLI-38 | Medium | P1 | S | — | Done | Align .NET/Go/Java on `hresult < 0` — lands prior CLI-08 and cures the design-doc drift |
|
||||
| CLI-39 | Medium | P1 | S | CLI-35..38, CLI-45 | Not started | Bump client versions off the already-published 0.1.2 before the next publish; add registry-collision guard |
|
||||
| CLI-40 | Low | — | M | — | Not started | Port the exact-secret credential scrub to Rust/Java/.NET |
|
||||
| CLI-41 | Low | — | M | — | Not started | Uniform malformed-reply contract for AuthenticateUser/ArchestrAUserToId/AddBufferedItem |
|
||||
| CLI-40 | Low | — | M | — | Done | Port the exact-secret credential scrub to Rust/Java/.NET |
|
||||
| CLI-41 | Low | — | M | — | Done | Uniform malformed-reply contract for AuthenticateUser/ArchestrAUserToId/AddBufferedItem |
|
||||
| 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-44 | Low | — | S | — | Done | Go event goroutine can mislabel a genuine terminal error as `ErrSlowConsumer` |
|
||||
| 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).
|
||||
|
||||
@@ -0,0 +1,96 @@
|
||||
using ZB.MOM.WW.MxGateway.Contracts.Proto;
|
||||
|
||||
namespace ZB.MOM.WW.MxGateway.Client.Tests;
|
||||
|
||||
/// <summary>
|
||||
/// Unit tests for <see cref="MxGatewaySecretRedaction"/> — the exact-substring scrub applied to
|
||||
/// diagnostic text and rebuilt exceptions before they leave the client on a failure path.
|
||||
/// </summary>
|
||||
public sealed class MxGatewaySecretRedactionTests
|
||||
{
|
||||
[Fact]
|
||||
public void Redact_ReplacesEveryOccurrenceOfSecret()
|
||||
{
|
||||
string result = MxGatewaySecretRedaction.Redact(
|
||||
"pw=hunter2 retry pw=hunter2 again hunter2",
|
||||
"hunter2");
|
||||
|
||||
Assert.DoesNotContain("hunter2", result, StringComparison.Ordinal);
|
||||
Assert.Equal("pw=<redacted> retry pw=<redacted> again <redacted>", result);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Redact_ScrubsBothSecretsWhenOneIsSubstringOfTheOther()
|
||||
{
|
||||
// "secret" is a substring of "secretPassword"; both must be fully scrubbed regardless of
|
||||
// supplied order — no residual leak of either verbatim value.
|
||||
string result = MxGatewaySecretRedaction.Redact(
|
||||
"a=secretPassword b=secret",
|
||||
"secret",
|
||||
"secretPassword");
|
||||
|
||||
Assert.DoesNotContain("secretPassword", result, StringComparison.Ordinal);
|
||||
Assert.DoesNotContain("secret", result, StringComparison.Ordinal);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Redact_WithNullSecretsArray_ReturnsMessageUnchanged()
|
||||
{
|
||||
const string message = "nothing to scrub here";
|
||||
|
||||
string result = MxGatewaySecretRedaction.Redact(message, null!);
|
||||
|
||||
Assert.Equal(message, result);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Redact_WithEmptySecretsArray_ReturnsMessageUnchanged()
|
||||
{
|
||||
const string message = "nothing to scrub here";
|
||||
|
||||
string result = MxGatewaySecretRedaction.Redact(message);
|
||||
|
||||
Assert.Equal(message, result);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Redact_IgnoresWhitespaceOnlySecret()
|
||||
{
|
||||
// A whitespace-only secret must not over-redact the internal spaces of the message.
|
||||
const string message = "user operator logged in";
|
||||
|
||||
string result = MxGatewaySecretRedaction.Redact(message, " ");
|
||||
|
||||
Assert.Equal(message, result);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Redacted_PreservesConcreteSubtypeAndDoesNotChainSecretBearingOriginal()
|
||||
{
|
||||
const string secret = "hunter2";
|
||||
Exception transportCause = new InvalidOperationException("transport reset");
|
||||
MxGatewaySessionException original = new(
|
||||
$"session rejected credential '{secret}'",
|
||||
"session-1",
|
||||
"correlation-1",
|
||||
new ProtocolStatus { Code = ProtocolStatusCode.SessionNotReady, Message = $"echoed '{secret}'" },
|
||||
hResult: -1,
|
||||
statuses: [new MxStatusProxy { DiagnosticText = $"denied '{secret}'" }],
|
||||
innerException: transportCause);
|
||||
|
||||
MxGatewayException redacted = MxGatewaySecretRedaction.Redacted(original, secret);
|
||||
|
||||
// Concrete runtime type is preserved.
|
||||
Assert.IsType<MxGatewaySessionException>(redacted);
|
||||
// The secret is gone from the message and every structured accessor.
|
||||
Assert.DoesNotContain(secret, redacted.Message, StringComparison.Ordinal);
|
||||
Assert.DoesNotContain(secret, redacted.ToString(), StringComparison.Ordinal);
|
||||
Assert.DoesNotContain(secret, redacted.ProtocolStatus!.Message, StringComparison.Ordinal);
|
||||
Assert.All(redacted.Statuses, status =>
|
||||
Assert.DoesNotContain(secret, status.DiagnosticText, StringComparison.Ordinal));
|
||||
Assert.Contains("<redacted>", redacted.Message, StringComparison.Ordinal);
|
||||
// The secret-bearing original is NOT chained; the original's transport cause is carried.
|
||||
Assert.NotSame(original, redacted.InnerException);
|
||||
Assert.Same(transportCause, redacted.InnerException);
|
||||
}
|
||||
}
|
||||
+165
@@ -0,0 +1,165 @@
|
||||
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);
|
||||
}
|
||||
}
|
||||
@@ -20,7 +20,8 @@ public sealed class MxStatusProxyExtensionsTests
|
||||
MxStatusProxy status = JsonParser.Default.Parse<MxStatusProxy>(
|
||||
testCase.GetProperty("status").GetRawText());
|
||||
|
||||
Assert.Equal(status.Category is MxStatusCategory.Ok, status.IsSuccess());
|
||||
bool wantSuccess = testCase.GetProperty("wantSuccess").GetBoolean();
|
||||
Assert.Equal(wantSuccess, status.IsSuccess());
|
||||
Assert.Equal(
|
||||
testCase.GetProperty("status").GetProperty("rawCategory").GetInt32(),
|
||||
status.RawCategory);
|
||||
|
||||
@@ -0,0 +1,46 @@
|
||||
using ZB.MOM.WW.MxGateway.Contracts.Proto;
|
||||
|
||||
namespace ZB.MOM.WW.MxGateway.Client;
|
||||
|
||||
/// <summary>
|
||||
/// Exception thrown when the gateway returns a protocol-OK reply that carries neither the
|
||||
/// expected typed payload nor an int32 <c>return_value</c>, so the client cannot resolve the
|
||||
/// operation result. This replaces the historical <see cref="NullReferenceException"/> that a
|
||||
/// blind <c>reply.ReturnValue.Int32Value</c> fallback would throw.
|
||||
/// </summary>
|
||||
public sealed class MxGatewayMalformedReplyException : MxGatewayException
|
||||
{
|
||||
/// <summary>Initializes a new instance with the given message.</summary>
|
||||
/// <param name="message">The error message describing the malformed reply.</param>
|
||||
public MxGatewayMalformedReplyException(string message)
|
||||
: base(message)
|
||||
{
|
||||
}
|
||||
|
||||
/// <summary>Initializes a new instance with full diagnostic context.</summary>
|
||||
/// <param name="message">The error message describing the malformed reply.</param>
|
||||
/// <param name="sessionId">The session ID, if available.</param>
|
||||
/// <param name="correlationId">The correlation ID for tracing, if available.</param>
|
||||
/// <param name="protocolStatus">The protocol status details, if available.</param>
|
||||
/// <param name="hResult">The HResult code, if available.</param>
|
||||
/// <param name="statuses">The MXAccess statuses, if available.</param>
|
||||
/// <param name="innerException">The underlying exception, if any.</param>
|
||||
public MxGatewayMalformedReplyException(
|
||||
string message,
|
||||
string? sessionId = null,
|
||||
string? correlationId = null,
|
||||
ProtocolStatus? protocolStatus = null,
|
||||
int? hResult = null,
|
||||
IReadOnlyList<MxStatusProxy>? statuses = null,
|
||||
Exception? innerException = null)
|
||||
: base(
|
||||
message,
|
||||
sessionId,
|
||||
correlationId,
|
||||
protocolStatus,
|
||||
hResult,
|
||||
statuses ?? [],
|
||||
innerException)
|
||||
{
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,229 @@
|
||||
using ZB.MOM.WW.MxGateway.Contracts.Proto;
|
||||
|
||||
namespace ZB.MOM.WW.MxGateway.Client;
|
||||
|
||||
/// <summary>
|
||||
/// Scrubs exact secret substrings out of diagnostic text before it leaves the client on an
|
||||
/// exception path. MXAccess can echo a submitted credential or secured value back inside a
|
||||
/// failure diagnostic (protocol message, MXSTATUS_PROXY diagnostic text, HRESULT description);
|
||||
/// this helper replaces any such verbatim occurrence with <c><redacted></c> so the raw
|
||||
/// request payload never reaches a caught exception's message. The marker matches the Go, Rust,
|
||||
/// and Java clients.
|
||||
/// </summary>
|
||||
internal static class MxGatewaySecretRedaction
|
||||
{
|
||||
private const string Marker = "<redacted>";
|
||||
|
||||
/// <summary>
|
||||
/// Replaces every usable secret in <paramref name="secrets"/> with the redaction marker
|
||||
/// (ordinal comparison). Returns the message unchanged when it is null or empty, or when no
|
||||
/// usable secret is supplied. A secret that is null, empty, or whitespace-only is ignored so
|
||||
/// it cannot over-redact ordinary separator characters in the message.
|
||||
/// </summary>
|
||||
/// <param name="message">The diagnostic message to scrub.</param>
|
||||
/// <param name="secrets">The secret values to remove from the message.</param>
|
||||
/// <returns>The scrubbed message.</returns>
|
||||
internal static string Redact(string message, params string?[] secrets)
|
||||
{
|
||||
if (string.IsNullOrEmpty(message) || secrets is null)
|
||||
{
|
||||
return message;
|
||||
}
|
||||
|
||||
string result = message;
|
||||
foreach (string? secret in secrets)
|
||||
{
|
||||
if (!string.IsNullOrWhiteSpace(secret))
|
||||
{
|
||||
result = result.Replace(secret, Marker, StringComparison.Ordinal);
|
||||
}
|
||||
}
|
||||
|
||||
return result;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Returns a scrubbed clone of <paramref name="reply"/>: the protocol-status message, the
|
||||
/// reply-level diagnostic message, and each MXSTATUS_PROXY diagnostic text have every verbatim
|
||||
/// secret replaced with the redaction marker. The original is left untouched. MXAccess can echo
|
||||
/// a submitted credential into any of these fields, so a redacted exception must carry the
|
||||
/// scrubbed reply rather than the secret-bearing original.
|
||||
/// </summary>
|
||||
/// <param name="reply">The reply to clone and scrub.</param>
|
||||
/// <param name="secrets">The secret values to remove.</param>
|
||||
/// <returns>A scrubbed clone of the reply.</returns>
|
||||
internal static MxCommandReply RedactReply(MxCommandReply reply, params string?[] secrets)
|
||||
{
|
||||
ArgumentNullException.ThrowIfNull(reply);
|
||||
|
||||
MxCommandReply clone = reply.Clone();
|
||||
if (clone.ProtocolStatus is not null)
|
||||
{
|
||||
clone.ProtocolStatus.Message = Redact(clone.ProtocolStatus.Message, secrets);
|
||||
}
|
||||
|
||||
clone.DiagnosticMessage = Redact(clone.DiagnosticMessage, secrets);
|
||||
foreach (MxStatusProxy status in clone.Statuses)
|
||||
{
|
||||
status.DiagnosticText = Redact(status.DiagnosticText, secrets);
|
||||
}
|
||||
|
||||
return clone;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Returns a scrubbed clone of <paramref name="status"/> (its message with every verbatim
|
||||
/// secret removed), or <see langword="null"/> when the input is null.
|
||||
/// </summary>
|
||||
/// <param name="status">The protocol status to clone and scrub.</param>
|
||||
/// <param name="secrets">The secret values to remove.</param>
|
||||
/// <returns>A scrubbed clone, or <see langword="null"/>.</returns>
|
||||
internal static ProtocolStatus? RedactStatus(ProtocolStatus? status, params string?[] secrets)
|
||||
{
|
||||
if (status is null)
|
||||
{
|
||||
return null;
|
||||
}
|
||||
|
||||
ProtocolStatus clone = status.Clone();
|
||||
clone.Message = Redact(clone.Message, secrets);
|
||||
return clone;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Returns a list of scrubbed clones of <paramref name="statuses"/> — each MXSTATUS_PROXY's
|
||||
/// diagnostic text has every verbatim secret removed. The originals are left untouched.
|
||||
/// </summary>
|
||||
/// <param name="statuses">The statuses to clone and scrub.</param>
|
||||
/// <param name="secrets">The secret values to remove.</param>
|
||||
/// <returns>A list of scrubbed clones.</returns>
|
||||
internal static IReadOnlyList<MxStatusProxy> RedactStatuses(
|
||||
IReadOnlyList<MxStatusProxy> statuses,
|
||||
params string?[] secrets)
|
||||
{
|
||||
if (statuses is null || statuses.Count is 0)
|
||||
{
|
||||
return statuses ?? [];
|
||||
}
|
||||
|
||||
MxStatusProxy[] result = new MxStatusProxy[statuses.Count];
|
||||
for (int i = 0; i < statuses.Count; i++)
|
||||
{
|
||||
MxStatusProxy clone = statuses[i].Clone();
|
||||
clone.DiagnosticText = Redact(clone.DiagnosticText, secrets);
|
||||
result[i] = clone;
|
||||
}
|
||||
|
||||
return result;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Returns an exception equivalent to <paramref name="ex"/> but with any verbatim secret
|
||||
/// scrubbed from its message. When nothing changes, the original exception is returned
|
||||
/// unchanged; otherwise a new exception of the same concrete runtime type is built and the
|
||||
/// original reply/status context is preserved. The secret-bearing original is deliberately
|
||||
/// <b>not</b> chained as the inner exception — doing so would let its unredacted message
|
||||
/// re-surface through <see cref="Exception.ToString"/> (which logging frameworks call). The
|
||||
/// original's own inner cause (a transport error, never the request payload) is carried
|
||||
/// forward instead.
|
||||
/// </summary>
|
||||
/// <param name="ex">The exception to redact.</param>
|
||||
/// <param name="secrets">The secret values to remove from the message.</param>
|
||||
/// <returns>The redacted exception, or the original when no change was needed.</returns>
|
||||
internal static MxGatewayException Redacted(MxGatewayException ex, params string?[] secrets)
|
||||
{
|
||||
ArgumentNullException.ThrowIfNull(ex);
|
||||
|
||||
string redacted = Redact(ex.Message, secrets);
|
||||
bool messageChanged = !string.Equals(redacted, ex.Message, StringComparison.Ordinal);
|
||||
Exception? cause = ex.InnerException;
|
||||
|
||||
// MxAccessException derives its structured fields from the raw reply, so scrubbing must
|
||||
// clone and redact that reply — the message alone changing is not enough, because the reply
|
||||
// can carry the echoed secret even when the message does not.
|
||||
if (ex is MxAccessException access)
|
||||
{
|
||||
if (!messageChanged && !ReplyContainsSecret(access.Reply, secrets))
|
||||
{
|
||||
return ex;
|
||||
}
|
||||
|
||||
return new MxAccessException(redacted, RedactReply(access.Reply, secrets), cause);
|
||||
}
|
||||
|
||||
// Other subtypes carry the secret through ProtocolStatus.Message and Statuses[].DiagnosticText.
|
||||
if (!messageChanged
|
||||
&& !ContainsSecret(ex.ProtocolStatus?.Message, secrets)
|
||||
&& !StatusesContainSecret(ex.Statuses, secrets))
|
||||
{
|
||||
return ex;
|
||||
}
|
||||
|
||||
ProtocolStatus? status = RedactStatus(ex.ProtocolStatus, secrets);
|
||||
IReadOnlyList<MxStatusProxy> statuses = RedactStatuses(ex.Statuses, secrets);
|
||||
return ex switch
|
||||
{
|
||||
MxGatewaySessionException => new MxGatewaySessionException(
|
||||
redacted, ex.SessionId, ex.CorrelationId, status, ex.HResultCode, statuses, cause),
|
||||
MxGatewayWorkerException => new MxGatewayWorkerException(
|
||||
redacted, ex.SessionId, ex.CorrelationId, status, ex.HResultCode, statuses, cause),
|
||||
MxGatewayAuthenticationException => new MxGatewayAuthenticationException(
|
||||
redacted, ex.SessionId, ex.CorrelationId, status, ex.HResultCode, statuses, cause),
|
||||
MxGatewayAuthorizationException => new MxGatewayAuthorizationException(
|
||||
redacted, ex.SessionId, ex.CorrelationId, status, ex.HResultCode, statuses, cause),
|
||||
MxGatewayMalformedReplyException => new MxGatewayMalformedReplyException(
|
||||
redacted, ex.SessionId, ex.CorrelationId, status, ex.HResultCode, statuses, cause),
|
||||
MxGatewayCommandException => new MxGatewayCommandException(
|
||||
redacted, ex.SessionId, ex.CorrelationId, status, ex.HResultCode, statuses, cause),
|
||||
_ => new MxGatewayException(redacted, cause),
|
||||
};
|
||||
}
|
||||
|
||||
private static bool ContainsSecret(string? text, string?[] secrets)
|
||||
{
|
||||
if (string.IsNullOrEmpty(text) || secrets is null)
|
||||
{
|
||||
return false;
|
||||
}
|
||||
|
||||
foreach (string? secret in secrets)
|
||||
{
|
||||
if (!string.IsNullOrWhiteSpace(secret) && text.Contains(secret, StringComparison.Ordinal))
|
||||
{
|
||||
return true;
|
||||
}
|
||||
}
|
||||
|
||||
return false;
|
||||
}
|
||||
|
||||
private static bool StatusesContainSecret(IReadOnlyList<MxStatusProxy> statuses, string?[] secrets)
|
||||
{
|
||||
if (statuses is null)
|
||||
{
|
||||
return false;
|
||||
}
|
||||
|
||||
foreach (MxStatusProxy status in statuses)
|
||||
{
|
||||
if (ContainsSecret(status.DiagnosticText, secrets))
|
||||
{
|
||||
return true;
|
||||
}
|
||||
}
|
||||
|
||||
return false;
|
||||
}
|
||||
|
||||
private static bool ReplyContainsSecret(MxCommandReply reply, string?[] secrets)
|
||||
{
|
||||
if (reply is null)
|
||||
{
|
||||
return false;
|
||||
}
|
||||
|
||||
return ContainsSecret(reply.ProtocolStatus?.Message, secrets)
|
||||
|| ContainsSecret(reply.DiagnosticMessage, secrets)
|
||||
|| StatusesContainSecret(reply.Statuses, secrets);
|
||||
}
|
||||
}
|
||||
@@ -945,7 +945,7 @@ public sealed class MxGatewaySession : IAsyncDisposable
|
||||
cancellationToken)
|
||||
.ConfigureAwait(false);
|
||||
reply.EnsureProtocolSuccess().EnsureMxAccessSuccess();
|
||||
return reply.AddBufferedItem?.ItemHandle ?? reply.ReturnValue.Int32Value;
|
||||
return ResolveInt32Result(reply.AddBufferedItem?.ItemHandle, reply, "AddBufferedItem");
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
@@ -1141,8 +1141,15 @@ public sealed class MxGatewaySession : IAsyncDisposable
|
||||
verifierUserId,
|
||||
cancellationToken)
|
||||
.ConfigureAwait(false);
|
||||
try
|
||||
{
|
||||
reply.EnsureProtocolSuccess().EnsureMxAccessSuccess();
|
||||
}
|
||||
catch (MxGatewayException ex)
|
||||
{
|
||||
throw MxGatewaySecretRedaction.Redacted(ex, ExtractSecretString(value));
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Writes a secured value to an item without error checking. See
|
||||
@@ -1215,8 +1222,15 @@ public sealed class MxGatewaySession : IAsyncDisposable
|
||||
verifierUserId,
|
||||
cancellationToken)
|
||||
.ConfigureAwait(false);
|
||||
try
|
||||
{
|
||||
reply.EnsureProtocolSuccess().EnsureMxAccessSuccess();
|
||||
}
|
||||
catch (MxGatewayException ex)
|
||||
{
|
||||
throw MxGatewaySecretRedaction.Redacted(ex, ExtractSecretString(value));
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Writes a secured value and timestamp to an item without error checking. See
|
||||
@@ -1285,8 +1299,15 @@ public sealed class MxGatewaySession : IAsyncDisposable
|
||||
verifyUserPassword,
|
||||
cancellationToken)
|
||||
.ConfigureAwait(false);
|
||||
try
|
||||
{
|
||||
reply.EnsureProtocolSuccess().EnsureMxAccessSuccess();
|
||||
return reply.AuthenticateUser?.UserId ?? reply.ReturnValue.Int32Value;
|
||||
return ResolveInt32Result(reply.AuthenticateUser?.UserId, reply, "AuthenticateUser");
|
||||
}
|
||||
catch (MxGatewayException ex)
|
||||
{
|
||||
throw MxGatewaySecretRedaction.Redacted(ex, verifyUserPassword);
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
@@ -1337,7 +1358,7 @@ public sealed class MxGatewaySession : IAsyncDisposable
|
||||
MxCommandReply reply = await ArchestraUserToIdRawAsync(serverHandle, userIdGuid, cancellationToken)
|
||||
.ConfigureAwait(false);
|
||||
reply.EnsureProtocolSuccess().EnsureMxAccessSuccess();
|
||||
return reply.ArchestraUserToId?.UserId ?? reply.ReturnValue.Int32Value;
|
||||
return ResolveInt32Result(reply.ArchestraUserToId?.UserId, reply, "ArchestrAUserToId");
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
@@ -1367,6 +1388,51 @@ public sealed class MxGatewaySession : IAsyncDisposable
|
||||
cancellationToken);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Resolves the int32 result of an OK command reply: the typed payload value when present,
|
||||
/// otherwise an int32 <c>return_value</c> when the reply carries one. A reply that provides
|
||||
/// neither is malformed and surfaces as <see cref="MxGatewayMalformedReplyException"/>
|
||||
/// rather than the historical <see cref="NullReferenceException"/>.
|
||||
/// </summary>
|
||||
/// <param name="typedValue">The typed payload value, or <see langword="null"/> when absent.</param>
|
||||
/// <param name="reply">The OK command reply.</param>
|
||||
/// <param name="operation">The MXAccess operation name, for the diagnostic message.</param>
|
||||
/// <returns>The resolved int32 result.</returns>
|
||||
private static int ResolveInt32Result(int? typedValue, MxCommandReply reply, string operation)
|
||||
{
|
||||
if (typedValue.HasValue)
|
||||
{
|
||||
return typedValue.Value;
|
||||
}
|
||||
|
||||
if (reply.ReturnValue is not null
|
||||
&& reply.ReturnValue.KindCase == MxValue.KindOneofCase.Int32Value)
|
||||
{
|
||||
return reply.ReturnValue.Int32Value;
|
||||
}
|
||||
|
||||
throw new MxGatewayMalformedReplyException(
|
||||
$"{operation} returned a malformed reply: OK reply carried neither the typed payload nor an int32 return_value",
|
||||
reply.SessionId,
|
||||
reply.CorrelationId,
|
||||
reply.ProtocolStatus,
|
||||
reply.HasHresult ? reply.Hresult : null,
|
||||
reply.Statuses.ToArray());
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Extracts the raw string form of a credential-bearing <see cref="MxValue"/> for redaction,
|
||||
/// or <see langword="null"/> when the value does not carry a string.
|
||||
/// </summary>
|
||||
/// <param name="value">The value written by a secured write.</param>
|
||||
/// <returns>The string payload, or <see langword="null"/>.</returns>
|
||||
private static string? ExtractSecretString(MxValue value)
|
||||
{
|
||||
return value.KindCase == MxValue.KindOneofCase.StringValue
|
||||
? value.StringValue
|
||||
: null;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Invokes an MXAccess command on this session.
|
||||
/// </summary>
|
||||
|
||||
@@ -30,6 +30,6 @@ public static class MxStatusProxyExtensions
|
||||
? "no diagnostic text"
|
||||
: status.DiagnosticText;
|
||||
|
||||
return $"{status.Category} by {status.DetectedBy}; detail={status.Detail}; {diagnosticText}";
|
||||
return $"success={status.Success}; {status.Category} by {status.DetectedBy}; detail={status.Detail}; {diagnosticText}";
|
||||
}
|
||||
}
|
||||
|
||||
@@ -10,7 +10,9 @@ import (
|
||||
|
||||
pb "gitea.dohertylan.com/dohertj2/mxaccessgw/clients/go/internal/generated"
|
||||
"google.golang.org/grpc"
|
||||
"google.golang.org/grpc/codes"
|
||||
"google.golang.org/grpc/metadata"
|
||||
"google.golang.org/grpc/status"
|
||||
"google.golang.org/grpc/test/bufconn"
|
||||
)
|
||||
|
||||
@@ -200,6 +202,136 @@ func TestEventsSlowConsumerYieldsErrSlowConsumerBeforeClose(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestEventsFullBufferTerminalErrorKeepsRootCause(t *testing.T) {
|
||||
fake := &fakeGatewayServer{
|
||||
streamStarted: make(chan struct{}),
|
||||
streamDone: make(chan struct{}),
|
||||
streamEventCount: eventBufferSize,
|
||||
streamTerminalErr: status.Error(codes.Internal, "boom"),
|
||||
}
|
||||
client, cleanup := newBufconnClient(t, fake)
|
||||
defer cleanup()
|
||||
session := NewSessionForID(client, "session-1")
|
||||
|
||||
events, err := session.EventsAfter(context.Background(), 0)
|
||||
if err != nil {
|
||||
t.Fatalf("EventsAfter() error = %v", err)
|
||||
}
|
||||
<-fake.streamStarted
|
||||
|
||||
// Do not drain until the stream has fully ended: the server sends exactly
|
||||
// eventBufferSize events (filling the data slots) and then returns a genuine
|
||||
// terminal gRPC error. The client must report that error as itself, using the
|
||||
// reserved slot, rather than mislabeling it as ErrSlowConsumer.
|
||||
select {
|
||||
case <-fake.streamDone:
|
||||
case <-time.After(2 * time.Second):
|
||||
t.Fatal("event stream did not stop after terminal error")
|
||||
}
|
||||
// streamDone fires when the server returns; the client's producer goroutine
|
||||
// still needs a moment to drain the gRPC stream, fill all data slots, and
|
||||
// enqueue the terminal result. Let it settle before draining so the buffer is
|
||||
// genuinely full when the terminal error is processed (which is what makes the
|
||||
// mislabel bug observable).
|
||||
time.Sleep(250 * time.Millisecond)
|
||||
|
||||
var last EventResult
|
||||
gotResult := false
|
||||
for {
|
||||
select {
|
||||
case res, ok := <-events:
|
||||
if !ok {
|
||||
if !gotResult {
|
||||
t.Fatal("events channel closed without yielding any result")
|
||||
}
|
||||
var gwErr *GatewayError
|
||||
if !errors.As(last.Err, &gwErr) {
|
||||
t.Fatalf("final event result err is %T, want *GatewayError", last.Err)
|
||||
}
|
||||
if code := status.Code(last.Err); code != codes.Internal {
|
||||
t.Fatalf("final event result gRPC code = %s, want %s", code, codes.Internal)
|
||||
}
|
||||
if errors.Is(last.Err, ErrSlowConsumer) {
|
||||
t.Fatalf("final event result err = %v, must not be mislabeled as ErrSlowConsumer", last.Err)
|
||||
}
|
||||
return
|
||||
}
|
||||
last = res
|
||||
gotResult = true
|
||||
case <-time.After(2 * time.Second):
|
||||
t.Fatal("events channel did not close after terminal error")
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TestSubscribeEventsFullBufferDeliversTerminalError is the CLI-44 regression for
|
||||
// the never-drop Subscribe path. SubscribeEvents/SubscribeEventsAfter use
|
||||
// cancelWhenResultBufferFull=false, so ordinary sends are blocking and uncapped and
|
||||
// can fill every slot in the results channel — including the reserved terminal slot.
|
||||
// A genuine terminal Recv error must still be delivered as the final result, never
|
||||
// silently dropped. The server sends eventBufferSize+eventBufferReservedSlots events
|
||||
// (filling every slot) and then returns a genuine gRPC error; with an unconditional
|
||||
// non-blocking terminal send the error is dropped, so this fails red until the send
|
||||
// path blocks for the never-drop mode.
|
||||
func TestSubscribeEventsFullBufferDeliversTerminalError(t *testing.T) {
|
||||
fake := &fakeGatewayServer{
|
||||
streamStarted: make(chan struct{}),
|
||||
streamDone: make(chan struct{}),
|
||||
streamEventCount: eventBufferSize + eventBufferReservedSlots,
|
||||
streamTerminalErr: status.Error(codes.Internal, "boom"),
|
||||
}
|
||||
client, cleanup := newBufconnClient(t, fake)
|
||||
defer cleanup()
|
||||
session := NewSessionForID(client, "session-1")
|
||||
|
||||
subscription, err := session.SubscribeEvents(context.Background())
|
||||
if err != nil {
|
||||
t.Fatalf("SubscribeEvents() error = %v", err)
|
||||
}
|
||||
defer subscription.Close()
|
||||
<-fake.streamStarted
|
||||
|
||||
// Wait for the server to finish sending every event and return the terminal
|
||||
// error, so the producer goroutine has filled every buffered slot before the
|
||||
// terminal result is processed. That is what makes the dropped-terminal bug
|
||||
// observable: with the buffer full, an unconditional non-blocking send discards
|
||||
// the terminal error.
|
||||
select {
|
||||
case <-fake.streamDone:
|
||||
case <-time.After(2 * time.Second):
|
||||
t.Fatal("event stream did not stop after terminal error")
|
||||
}
|
||||
time.Sleep(250 * time.Millisecond)
|
||||
|
||||
// Drain fully. Every data event, then the terminal gRPC error as the final
|
||||
// result, must arrive; the channel must not close without yielding it.
|
||||
events := subscription.Events()
|
||||
var last EventResult
|
||||
gotResult := false
|
||||
for {
|
||||
select {
|
||||
case res, ok := <-events:
|
||||
if !ok {
|
||||
if !gotResult {
|
||||
t.Fatal("events channel closed without yielding any result")
|
||||
}
|
||||
var gwErr *GatewayError
|
||||
if !errors.As(last.Err, &gwErr) {
|
||||
t.Fatalf("final event result err is %T (%v), want the terminal *GatewayError; it was dropped", last.Err, last.Err)
|
||||
}
|
||||
if code := status.Code(last.Err); code != codes.Internal {
|
||||
t.Fatalf("final event result gRPC code = %s, want %s", code, codes.Internal)
|
||||
}
|
||||
return
|
||||
}
|
||||
last = res
|
||||
gotResult = true
|
||||
case <-time.After(2 * time.Second):
|
||||
t.Fatal("events channel did not close after terminal error")
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestEventsSurfacesReplayGapSentinelAsTypedSignal(t *testing.T) {
|
||||
fake := &fakeGatewayServer{
|
||||
streamStarted: make(chan struct{}),
|
||||
@@ -701,6 +833,7 @@ type fakeGatewayServer struct {
|
||||
streamDone chan struct{}
|
||||
streamEventCount int
|
||||
streamReplayGap *pb.ReplayGap
|
||||
streamTerminalErr error
|
||||
invokeReply *pb.MxCommandReply
|
||||
invokeRequest *pb.MxCommandRequest
|
||||
}
|
||||
@@ -772,6 +905,12 @@ func (s *fakeGatewayServer) StreamEvents(req *pb.StreamEventsRequest, stream grp
|
||||
return err
|
||||
}
|
||||
}
|
||||
if s.streamTerminalErr != nil {
|
||||
// Return a genuine terminal stream error immediately after sending the
|
||||
// events, without waiting on the client to cancel. This exercises the
|
||||
// Recv-error path while the client's result buffer is still full.
|
||||
return s.streamTerminalErr
|
||||
}
|
||||
<-stream.Context().Done()
|
||||
return io.EOF
|
||||
}
|
||||
|
||||
@@ -0,0 +1,197 @@
|
||||
package mxgateway
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
pb "gitea.dohertylan.com/dohertj2/mxaccessgw/clients/go/internal/generated"
|
||||
"google.golang.org/protobuf/encoding/protojson"
|
||||
)
|
||||
|
||||
// loadCommandReplyFixture parses a shared command-reply fixture into an
|
||||
// MxCommandReply so the Go client can be driven through the same wire shapes the
|
||||
// other language clients exercise.
|
||||
func loadCommandReplyFixture(t *testing.T, name string) *pb.MxCommandReply {
|
||||
t.Helper()
|
||||
path := filepath.Join("..", "..", "proto", "fixtures", "behavior", "command-replies", name)
|
||||
data, err := os.ReadFile(path)
|
||||
if err != nil {
|
||||
t.Fatalf("read fixture %s: %v", name, err)
|
||||
}
|
||||
var reply pb.MxCommandReply
|
||||
if err := protojson.Unmarshal(data, &reply); err != nil {
|
||||
t.Fatalf("parse fixture %s: %v", name, err)
|
||||
}
|
||||
return &reply
|
||||
}
|
||||
|
||||
func TestAuthenticateUserMissingPayloadReturnsMalformedReplyError(t *testing.T) {
|
||||
fake := &fakeGatewayServer{
|
||||
invokeReply: loadCommandReplyFixture(t, "authenticate-user.missing-payload.reply.json"),
|
||||
}
|
||||
client, cleanup := newBufconnClient(t, fake)
|
||||
defer cleanup()
|
||||
session := NewSessionForID(client, "session-1")
|
||||
|
||||
_, err := session.AuthenticateUser(context.Background(), 12, "operator", "secret")
|
||||
var malformed *MalformedReplyError
|
||||
if !errors.As(err, &malformed) {
|
||||
t.Fatalf("AuthenticateUser() error = %v (%T), want *MalformedReplyError", err, err)
|
||||
}
|
||||
if malformed.Op != "authenticate user" {
|
||||
t.Fatalf("MalformedReplyError.Op = %q, want %q", malformed.Op, "authenticate user")
|
||||
}
|
||||
}
|
||||
|
||||
func TestAuthenticateUserReturnValueOnlyUsesInt32ReturnValue(t *testing.T) {
|
||||
fake := &fakeGatewayServer{
|
||||
invokeReply: loadCommandReplyFixture(t, "authenticate-user.return-value-only.reply.json"),
|
||||
}
|
||||
client, cleanup := newBufconnClient(t, fake)
|
||||
defer cleanup()
|
||||
session := NewSessionForID(client, "session-1")
|
||||
|
||||
userID, err := session.AuthenticateUser(context.Background(), 12, "operator", "secret")
|
||||
if err != nil {
|
||||
t.Fatalf("AuthenticateUser() error = %v", err)
|
||||
}
|
||||
if userID != 7 {
|
||||
t.Fatalf("AuthenticateUser() = %d, want 7", userID)
|
||||
}
|
||||
}
|
||||
|
||||
// AddBufferedItem shares the prefer-payload / int32-return-value / malformed
|
||||
// fallback code path; cover both branches for one of the siblings.
|
||||
func TestAddBufferedItemFallbackHonoursReturnValueAndReportsMalformed(t *testing.T) {
|
||||
t.Run("return-value-only", func(t *testing.T) {
|
||||
fake := &fakeGatewayServer{
|
||||
invokeReply: loadCommandReplyFixture(t, "authenticate-user.return-value-only.reply.json"),
|
||||
}
|
||||
client, cleanup := newBufconnClient(t, fake)
|
||||
defer cleanup()
|
||||
session := NewSessionForID(client, "session-1")
|
||||
|
||||
itemHandle, err := session.AddBufferedItem(context.Background(), 12, "Area001.Pump001.Speed", "runtime")
|
||||
if err != nil {
|
||||
t.Fatalf("AddBufferedItem() error = %v", err)
|
||||
}
|
||||
if itemHandle != 7 {
|
||||
t.Fatalf("AddBufferedItem() = %d, want 7", itemHandle)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("missing-payload", func(t *testing.T) {
|
||||
fake := &fakeGatewayServer{
|
||||
invokeReply: loadCommandReplyFixture(t, "authenticate-user.missing-payload.reply.json"),
|
||||
}
|
||||
client, cleanup := newBufconnClient(t, fake)
|
||||
defer cleanup()
|
||||
session := NewSessionForID(client, "session-1")
|
||||
|
||||
_, err := session.AddBufferedItem(context.Background(), 12, "Area001.Pump001.Speed", "runtime")
|
||||
var malformed *MalformedReplyError
|
||||
if !errors.As(err, &malformed) {
|
||||
t.Fatalf("AddBufferedItem() error = %v (%T), want *MalformedReplyError", err, err)
|
||||
}
|
||||
if malformed.Op != "add buffered item" {
|
||||
t.Fatalf("MalformedReplyError.Op = %q, want %q", malformed.Op, "add buffered item")
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
// TestAuthenticateUserScrubsEchoedCredentialFromError is the CLI-40 regression:
|
||||
// a gateway diagnostic that echoes the raw credential back must never reach the
|
||||
// caller's surfaced error text.
|
||||
func TestAuthenticateUserScrubsEchoedCredentialFromError(t *testing.T) {
|
||||
const credential = "sup3rSecretVerify9f3a2b"
|
||||
fake := &fakeGatewayServer{
|
||||
invokeReply: loadCommandReplyFixture(t, "authenticate-user.echoed-credential.reply.json"),
|
||||
}
|
||||
client, cleanup := newBufconnClient(t, fake)
|
||||
defer cleanup()
|
||||
session := NewSessionForID(client, "session-1")
|
||||
|
||||
_, err := session.AuthenticateUser(context.Background(), 12, "operator", credential)
|
||||
if err == nil {
|
||||
t.Fatal("AuthenticateUser() error = nil, want an MXAccess failure")
|
||||
}
|
||||
message := err.Error()
|
||||
if strings.Contains(message, credential) {
|
||||
t.Fatalf("surfaced error leaked the credential: %q", message)
|
||||
}
|
||||
if !strings.Contains(message, "<redacted>") {
|
||||
t.Fatalf("surfaced error missing redaction marker: %q", message)
|
||||
}
|
||||
}
|
||||
|
||||
// TestAuthenticateUserScrubsEchoedCredentialFromStructuredReply is the CLI-40
|
||||
// follow-up: redacting only the rendered Error() string is not enough. The typed
|
||||
// *MxAccessError still carries the raw command reply, whose ProtocolStatus.Message,
|
||||
// DiagnosticMessage, and Statuses[].DiagnosticText echo the credential verbatim. A
|
||||
// logger dumping structured fields would reintroduce the leak, so the reply the
|
||||
// typed error carries must be a scrubbed clone. Both the OK+negative-HRESULT and the
|
||||
// MXACCESS_FAILURE fixtures route to *MxAccessError (via EnsureProtocolSuccess), so
|
||||
// both must be scrubbed identically.
|
||||
func TestAuthenticateUserScrubsEchoedCredentialFromStructuredReply(t *testing.T) {
|
||||
const credential = "sup3rSecretVerify9f3a2b"
|
||||
fixtures := []string{
|
||||
"authenticate-user.echoed-credential.reply.json",
|
||||
"authenticate-user.echoed-credential-mxaccess-failure.reply.json",
|
||||
}
|
||||
for _, fixture := range fixtures {
|
||||
t.Run(fixture, func(t *testing.T) {
|
||||
fake := &fakeGatewayServer{
|
||||
invokeReply: loadCommandReplyFixture(t, fixture),
|
||||
}
|
||||
client, cleanup := newBufconnClient(t, fake)
|
||||
defer cleanup()
|
||||
session := NewSessionForID(client, "session-1")
|
||||
|
||||
_, err := session.AuthenticateUser(context.Background(), 12, "operator", credential)
|
||||
if err == nil {
|
||||
t.Fatal("AuthenticateUser() error = nil, want an MXAccess failure")
|
||||
}
|
||||
|
||||
var mxErr *MxAccessError
|
||||
if !errors.As(err, &mxErr) {
|
||||
t.Fatalf("AuthenticateUser() error = %v (%T), want *MxAccessError", err, err)
|
||||
}
|
||||
|
||||
reply := mxErr.Reply
|
||||
if reply == nil {
|
||||
t.Fatal("MxAccessError.Reply is nil, want the scrubbed command reply")
|
||||
}
|
||||
if got := reply.GetProtocolStatus().GetMessage(); strings.Contains(got, credential) {
|
||||
t.Fatalf("MxAccessError.Reply.ProtocolStatus.Message leaked the credential: %q", got)
|
||||
}
|
||||
if got := reply.GetDiagnosticMessage(); strings.Contains(got, credential) {
|
||||
t.Fatalf("MxAccessError.Reply.DiagnosticMessage leaked the credential: %q", got)
|
||||
}
|
||||
for i, status := range reply.GetStatuses() {
|
||||
if got := status.GetDiagnosticText(); strings.Contains(got, credential) {
|
||||
t.Fatalf("MxAccessError.Reply.Statuses[%d].DiagnosticText leaked the credential: %q", i, got)
|
||||
}
|
||||
}
|
||||
|
||||
// The wrapped CommandError's status/reply must be scrubbed too.
|
||||
if mxErr.Command != nil {
|
||||
if got := mxErr.Command.Status.GetMessage(); strings.Contains(got, credential) {
|
||||
t.Fatalf("MxAccessError.Command.Status.Message leaked the credential: %q", got)
|
||||
}
|
||||
if cmdReply := mxErr.Command.Reply; cmdReply != nil {
|
||||
if got := cmdReply.GetDiagnosticMessage(); strings.Contains(got, credential) {
|
||||
t.Fatalf("MxAccessError.Command.Reply.DiagnosticMessage leaked the credential: %q", got)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if got := err.Error(); strings.Contains(got, credential) {
|
||||
t.Fatalf("rendered error leaked the credential: %q", got)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -52,6 +52,7 @@ func TestStatusConversionFixtures(t *testing.T) {
|
||||
var fixture struct {
|
||||
Cases []struct {
|
||||
ID string `json:"id"`
|
||||
WantSuccess bool `json:"wantSuccess"`
|
||||
Status json.RawMessage `json:"status"`
|
||||
} `json:"cases"`
|
||||
}
|
||||
@@ -65,9 +66,8 @@ func TestStatusConversionFixtures(t *testing.T) {
|
||||
if err := protojson.Unmarshal(tc.Status, &status); err != nil {
|
||||
t.Fatalf("parse status: %v", err)
|
||||
}
|
||||
want := status.GetCategory() == pb.MxStatusCategory_MX_STATUS_CATEGORY_OK
|
||||
if got := StatusSucceeded(&status); got != want {
|
||||
t.Fatalf("StatusSucceeded() = %v, want %v", got, want)
|
||||
if got := StatusSucceeded(&status); got != tc.WantSuccess {
|
||||
t.Fatalf("StatusSucceeded() = %v, want %v", got, tc.WantSuccess)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
@@ -6,6 +6,7 @@ import (
|
||||
"strings"
|
||||
|
||||
pb "gitea.dohertylan.com/dohertj2/mxaccessgw/clients/go/internal/generated"
|
||||
"google.golang.org/protobuf/proto"
|
||||
)
|
||||
|
||||
// redactedSecretMarker is the placeholder substituted for credential material in
|
||||
@@ -49,22 +50,110 @@ func (e *secretRedactingError) Unwrap() error {
|
||||
return e.err
|
||||
}
|
||||
|
||||
// redactSecrets wraps err so any occurrence of a non-empty secret in the surfaced
|
||||
// message is redacted, while errors.As / errors.Is still reach the wrapped typed
|
||||
// error. It returns nil unchanged and skips wrapping when no non-empty secret is
|
||||
// supplied, so non-secret-bearing calls keep their original error verbatim.
|
||||
// scrubReplyStrings returns a clone of reply with every non-empty secret replaced
|
||||
// by redactedSecretMarker in the free-text fields a gateway diagnostic could echo a
|
||||
// credential into: ProtocolStatus.Message, DiagnosticMessage, and each
|
||||
// Statuses[].DiagnosticText. It clones with proto.Clone so the caller's original
|
||||
// reply is never mutated. A nil reply, or an empty/whitespace-only secret set, is a
|
||||
// no-op (nil in, nil out; a clone otherwise).
|
||||
func scrubReplyStrings(reply *pb.MxCommandReply, secrets []string) *pb.MxCommandReply {
|
||||
if reply == nil {
|
||||
return nil
|
||||
}
|
||||
clone, ok := proto.Clone(reply).(*pb.MxCommandReply)
|
||||
if !ok {
|
||||
return reply
|
||||
}
|
||||
for _, secret := range secrets {
|
||||
if secret == "" {
|
||||
continue
|
||||
}
|
||||
if clone.GetProtocolStatus() != nil {
|
||||
clone.ProtocolStatus.Message = strings.ReplaceAll(clone.GetProtocolStatus().GetMessage(), secret, redactedSecretMarker)
|
||||
}
|
||||
clone.DiagnosticMessage = strings.ReplaceAll(clone.GetDiagnosticMessage(), secret, redactedSecretMarker)
|
||||
for _, status := range clone.GetStatuses() {
|
||||
status.DiagnosticText = strings.ReplaceAll(status.GetDiagnosticText(), secret, redactedSecretMarker)
|
||||
}
|
||||
}
|
||||
return clone
|
||||
}
|
||||
|
||||
// scrubProtocolStatusMessage returns a clone of status with every non-empty secret
|
||||
// redacted from its Message, leaving the original untouched.
|
||||
func scrubProtocolStatusMessage(status *ProtocolStatus, secrets []string) *ProtocolStatus {
|
||||
if status == nil {
|
||||
return nil
|
||||
}
|
||||
clone, ok := proto.Clone(status).(*ProtocolStatus)
|
||||
if !ok {
|
||||
return status
|
||||
}
|
||||
for _, secret := range secrets {
|
||||
if secret != "" {
|
||||
clone.Message = strings.ReplaceAll(clone.GetMessage(), secret, redactedSecretMarker)
|
||||
}
|
||||
}
|
||||
return clone
|
||||
}
|
||||
|
||||
// redactSecrets scrubs a non-empty secret set from the error it surfaces. When the
|
||||
// wrapped error is a typed *MxAccessError or *CommandError it is rebuilt carrying
|
||||
// scrubbed clones of its reply and protocol status, so a caller logging the typed
|
||||
// error's structured fields cannot reintroduce the credential the rendered message
|
||||
// hides. The rebuilt (or original, for other error types) value is then wrapped in
|
||||
// secretRedactingError as a belt-and-suspenders scrub of any remaining rendered
|
||||
// text. errors.As / errors.Is still reach the typed error through the wrapper. It
|
||||
// returns nil unchanged and skips all work when no non-empty secret is supplied, so
|
||||
// non-secret-bearing calls keep their original error verbatim.
|
||||
func redactSecrets(err error, secrets ...string) error {
|
||||
if err == nil {
|
||||
return nil
|
||||
}
|
||||
hasSecret := false
|
||||
for _, secret := range secrets {
|
||||
if secret != "" {
|
||||
return &secretRedactingError{err: err, secrets: secrets}
|
||||
hasSecret = true
|
||||
break
|
||||
}
|
||||
}
|
||||
if !hasSecret {
|
||||
return err
|
||||
}
|
||||
|
||||
rebuilt := rebuildScrubbedError(err, secrets)
|
||||
return &secretRedactingError{err: rebuilt, secrets: secrets}
|
||||
}
|
||||
|
||||
// rebuildScrubbedError rebuilds the typed error carrying scrubbed clones of any
|
||||
// command reply / protocol status it holds, so credential text never survives in the
|
||||
// error's structured fields. Non-reply-bearing error types are returned unchanged.
|
||||
func rebuildScrubbedError(err error, secrets []string) error {
|
||||
switch typed := err.(type) {
|
||||
case *MxAccessError:
|
||||
return &MxAccessError{
|
||||
Command: scrubCommandError(typed.Command, secrets),
|
||||
Reply: scrubReplyStrings(typed.Reply, secrets),
|
||||
}
|
||||
case *CommandError:
|
||||
return scrubCommandError(typed, secrets)
|
||||
default:
|
||||
return err
|
||||
}
|
||||
}
|
||||
|
||||
// scrubCommandError rebuilds a CommandError with a scrubbed Status and Reply.
|
||||
func scrubCommandError(cmd *CommandError, secrets []string) *CommandError {
|
||||
if cmd == nil {
|
||||
return nil
|
||||
}
|
||||
return &CommandError{
|
||||
Op: cmd.Op,
|
||||
Status: scrubProtocolStatusMessage(cmd.Status, secrets),
|
||||
Reply: scrubReplyStrings(cmd.Reply, secrets),
|
||||
}
|
||||
}
|
||||
|
||||
// ErrSlowConsumer is the terminal error sent on the Events/EventsAfter
|
||||
// (cancel-when-full) path when the buffered results channel overflows because
|
||||
// the consumer fell behind. It is delivered as the final EventResult.Err before
|
||||
@@ -72,6 +161,25 @@ func redactSecrets(err error, secrets ...string) error {
|
||||
// dropping events. Match it with errors.Is.
|
||||
var ErrSlowConsumer = errors.New("mxgateway: event consumer fell behind; stream terminated")
|
||||
|
||||
// MalformedReplyError reports an OK command reply that carried neither the
|
||||
// typed payload the operation expected nor a usable int32 return_value, so the
|
||||
// client cannot produce a result. It gives every affected helper one uniform,
|
||||
// inspectable failure instead of silently returning a zero value.
|
||||
type MalformedReplyError struct {
|
||||
// Op names the operation whose reply was malformed.
|
||||
Op string
|
||||
// Detail explains what the reply was missing.
|
||||
Detail string
|
||||
}
|
||||
|
||||
// Error returns the formatted malformed-reply message.
|
||||
func (e *MalformedReplyError) Error() string {
|
||||
if e == nil {
|
||||
return ""
|
||||
}
|
||||
return fmt.Sprintf("mxgateway: %s returned a malformed reply: %s", e.Op, e.Detail)
|
||||
}
|
||||
|
||||
// GatewayError wraps transport-level gRPC failures.
|
||||
type GatewayError struct {
|
||||
// Op names the operation that failed (for example "dial" or "invoke").
|
||||
|
||||
@@ -0,0 +1,115 @@
|
||||
package mxgateway
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
pb "gitea.dohertylan.com/dohertj2/mxaccessgw/clients/go/internal/generated"
|
||||
)
|
||||
|
||||
// TestScrubReplyStringsRedactsEveryOccurrence covers the multi-occurrence case:
|
||||
// one secret appearing across ProtocolStatus.Message, DiagnosticMessage, and every
|
||||
// Statuses[].DiagnosticText must be fully redacted with no residue.
|
||||
func TestScrubReplyStringsRedactsEveryOccurrence(t *testing.T) {
|
||||
const secret = "hunter2"
|
||||
reply := &pb.MxCommandReply{
|
||||
ProtocolStatus: &pb.ProtocolStatus{Message: "rejected hunter2 then hunter2 again"},
|
||||
DiagnosticMessage: "echoed hunter2 back",
|
||||
Statuses: []*pb.MxStatusProxy{
|
||||
{DiagnosticText: "first hunter2"},
|
||||
{DiagnosticText: "second hunter2 and hunter2"},
|
||||
},
|
||||
}
|
||||
|
||||
scrubbed := scrubReplyStrings(reply, []string{secret})
|
||||
|
||||
if strings.Contains(scrubbed.GetProtocolStatus().GetMessage(), secret) {
|
||||
t.Fatalf("ProtocolStatus.Message still contains the secret: %q", scrubbed.GetProtocolStatus().GetMessage())
|
||||
}
|
||||
if strings.Contains(scrubbed.GetDiagnosticMessage(), secret) {
|
||||
t.Fatalf("DiagnosticMessage still contains the secret: %q", scrubbed.GetDiagnosticMessage())
|
||||
}
|
||||
for i, status := range scrubbed.GetStatuses() {
|
||||
if strings.Contains(status.GetDiagnosticText(), secret) {
|
||||
t.Fatalf("Statuses[%d].DiagnosticText still contains the secret: %q", i, status.GetDiagnosticText())
|
||||
}
|
||||
}
|
||||
if !strings.Contains(scrubbed.GetProtocolStatus().GetMessage(), redactedSecretMarker) {
|
||||
t.Fatalf("ProtocolStatus.Message missing redaction marker: %q", scrubbed.GetProtocolStatus().GetMessage())
|
||||
}
|
||||
|
||||
// The original reply must be untouched (scrubReplyStrings clones).
|
||||
if !strings.Contains(reply.GetDiagnosticMessage(), secret) {
|
||||
t.Fatal("scrubReplyStrings mutated the original reply instead of cloning it")
|
||||
}
|
||||
}
|
||||
|
||||
// TestScrubReplyStringsRedactsOverlappingSecrets covers two secrets where one is a
|
||||
// substring of the other: both must be fully redacted, with no partial leak of the
|
||||
// longer secret's non-shared remainder.
|
||||
func TestScrubReplyStringsRedactsOverlappingSecrets(t *testing.T) {
|
||||
const shortSecret = "pass"
|
||||
const longSecret = "password123"
|
||||
reply := &pb.MxCommandReply{
|
||||
DiagnosticMessage: "value was password123 and also pass",
|
||||
}
|
||||
|
||||
scrubbed := scrubReplyStrings(reply, []string{longSecret, shortSecret})
|
||||
|
||||
got := scrubbed.GetDiagnosticMessage()
|
||||
if strings.Contains(got, shortSecret) {
|
||||
t.Fatalf("scrubbed message still contains a secret substring %q: %q", shortSecret, got)
|
||||
}
|
||||
if strings.Contains(got, longSecret) {
|
||||
t.Fatalf("scrubbed message still contains %q: %q", longSecret, got)
|
||||
}
|
||||
// "123" is the longer secret's remainder past the shared "pass" prefix; it must
|
||||
// not survive as a partial leak.
|
||||
if strings.Contains(got, "123") {
|
||||
t.Fatalf("scrubbed message leaked the longer secret's remainder: %q", got)
|
||||
}
|
||||
}
|
||||
|
||||
// TestRedactSecretsEmptyOrNilLeavesErrorUnchanged confirms the no-secret paths keep
|
||||
// the original typed error verbatim (no wrapping, no scrubbed clone).
|
||||
func TestRedactSecretsEmptyOrNilLeavesErrorUnchanged(t *testing.T) {
|
||||
base := &MxAccessError{Reply: &pb.MxCommandReply{DiagnosticMessage: "boom"}}
|
||||
|
||||
if got := redactSecrets(base); got != error(base) {
|
||||
t.Fatalf("redactSecrets with no secrets = %v, want the original error unchanged", got)
|
||||
}
|
||||
if got := redactSecrets(base, ""); got != error(base) {
|
||||
t.Fatalf("redactSecrets with only an empty secret = %v, want the original error unchanged", got)
|
||||
}
|
||||
if got := redactSecrets(nil, "secret"); got != nil {
|
||||
t.Fatalf("redactSecrets(nil, ...) = %v, want nil", got)
|
||||
}
|
||||
}
|
||||
|
||||
// TestRedactSecretsRebuildsTypedCommandError confirms a *CommandError (non-MXAccess
|
||||
// path) is rebuilt with a scrubbed Status and Reply, and errors.As still reaches it.
|
||||
func TestRedactSecretsRebuildsTypedCommandError(t *testing.T) {
|
||||
const secret = "topSecretValue"
|
||||
base := &CommandError{
|
||||
Op: "write secured",
|
||||
Status: &pb.ProtocolStatus{Message: "rejected topSecretValue"},
|
||||
Reply: &pb.MxCommandReply{DiagnosticMessage: "echoed topSecretValue"},
|
||||
}
|
||||
|
||||
redacted := redactSecrets(base, secret)
|
||||
|
||||
var cmdErr *CommandError
|
||||
if !errors.As(redacted, &cmdErr) {
|
||||
t.Fatalf("redactSecrets result %T does not unwrap to *CommandError", redacted)
|
||||
}
|
||||
if strings.Contains(cmdErr.Status.GetMessage(), secret) {
|
||||
t.Fatalf("CommandError.Status.Message leaked the secret: %q", cmdErr.Status.GetMessage())
|
||||
}
|
||||
if strings.Contains(cmdErr.Reply.GetDiagnosticMessage(), secret) {
|
||||
t.Fatalf("CommandError.Reply.DiagnosticMessage leaked the secret: %q", cmdErr.Reply.GetDiagnosticMessage())
|
||||
}
|
||||
if strings.Contains(redacted.Error(), secret) {
|
||||
t.Fatalf("rendered error leaked the secret: %q", redacted.Error())
|
||||
}
|
||||
}
|
||||
@@ -812,7 +812,13 @@ func (s *Session) AuthenticateUser(ctx context.Context, serverHandle int32, veri
|
||||
if reply.GetAuthenticateUser() != nil {
|
||||
return reply.GetAuthenticateUser().GetUserId(), nil
|
||||
}
|
||||
return reply.GetReturnValue().GetInt32Value(), nil
|
||||
if x, ok := reply.GetReturnValue().GetKind().(*pb.MxValue_Int32Value); ok {
|
||||
return x.Int32Value, nil
|
||||
}
|
||||
return 0, &MalformedReplyError{
|
||||
Op: "authenticate user",
|
||||
Detail: "reply carried neither an AuthenticateUser payload nor an int32 return_value",
|
||||
}
|
||||
}
|
||||
|
||||
// AuthenticateUserRaw invokes MXAccess AuthenticateUser and returns the raw
|
||||
@@ -847,7 +853,13 @@ func (s *Session) ArchestrAUserToId(ctx context.Context, serverHandle int32, use
|
||||
if reply.GetArchestraUserToId() != nil {
|
||||
return reply.GetArchestraUserToId().GetUserId(), nil
|
||||
}
|
||||
return reply.GetReturnValue().GetInt32Value(), nil
|
||||
if x, ok := reply.GetReturnValue().GetKind().(*pb.MxValue_Int32Value); ok {
|
||||
return x.Int32Value, nil
|
||||
}
|
||||
return 0, &MalformedReplyError{
|
||||
Op: "archestra user to id",
|
||||
Detail: "reply carried neither an ArchestrAUserToId payload nor an int32 return_value",
|
||||
}
|
||||
}
|
||||
|
||||
// ArchestrAUserToIdRaw invokes MXAccess ArchestrAUserToId and returns the raw reply.
|
||||
@@ -876,7 +888,13 @@ func (s *Session) AddBufferedItem(ctx context.Context, serverHandle int32, itemD
|
||||
if reply.GetAddBufferedItem() != nil {
|
||||
return reply.GetAddBufferedItem().GetItemHandle(), nil
|
||||
}
|
||||
return reply.GetReturnValue().GetInt32Value(), nil
|
||||
if x, ok := reply.GetReturnValue().GetKind().(*pb.MxValue_Int32Value); ok {
|
||||
return x.Int32Value, nil
|
||||
}
|
||||
return 0, &MalformedReplyError{
|
||||
Op: "add buffered item",
|
||||
Detail: "reply carried neither an AddBufferedItem payload nor an int32 return_value",
|
||||
}
|
||||
}
|
||||
|
||||
// AddBufferedItemRaw invokes MXAccess AddBufferedItem and returns the raw reply.
|
||||
@@ -981,10 +999,12 @@ func stringSecrets(values ...*MxValue) []string {
|
||||
// context cancellation stops Recv, or a terminal error is sent.
|
||||
//
|
||||
// The returned channel is buffered. If the consumer falls behind and the buffer
|
||||
// overflows, the stream is terminated and a final EventResult carrying a
|
||||
// GatewayError that wraps ErrSlowConsumer is delivered before the channel
|
||||
// closes. Callers must match it with errors.Is(res.Err, ErrSlowConsumer) to
|
||||
// distinguish a slow-consumer drop from a graceful server end. Use
|
||||
// overflows with data, the stream is terminated and a final EventResult carrying
|
||||
// a GatewayError that wraps ErrSlowConsumer is delivered before the channel
|
||||
// closes; match it with errors.Is(res.Err, ErrSlowConsumer) to distinguish a
|
||||
// slow-consumer drop from a graceful server end. A genuine stream error is
|
||||
// reported as itself even under overflow — it is never relabeled as
|
||||
// ErrSlowConsumer, so the underlying gRPC status stays inspectable. Use
|
||||
// SubscribeEvents for a blocking, backpressured stream that never drops.
|
||||
func (s *Session) Events(ctx context.Context) (<-chan EventResult, error) {
|
||||
return s.EventsAfter(ctx, 0)
|
||||
@@ -994,7 +1014,9 @@ func (s *Session) Events(ctx context.Context) (<-chan EventResult, error) {
|
||||
//
|
||||
// Like Events, the returned channel is buffered and terminates with a final
|
||||
// EventResult wrapping ErrSlowConsumer (matchable via errors.Is) if the consumer
|
||||
// falls behind and the buffer overflows, rather than silently closing.
|
||||
// falls behind and the buffer overflows with data, rather than silently closing.
|
||||
// A genuine stream error is reported as itself even under overflow, never
|
||||
// relabeled as ErrSlowConsumer.
|
||||
func (s *Session) EventsAfter(ctx context.Context, afterWorkerSequence uint64) (<-chan EventResult, error) {
|
||||
subscription, err := s.subscribeEventsAfter(ctx, afterWorkerSequence, true)
|
||||
if err != nil {
|
||||
@@ -1048,12 +1070,11 @@ func (s *Session) subscribeEventsAfter(ctx context.Context, afterWorkerSequence
|
||||
if err == io.EOF || status.Code(err) == codes.Canceled || streamCtx.Err() != nil {
|
||||
return
|
||||
}
|
||||
sendEventResult(
|
||||
streamCtx,
|
||||
results,
|
||||
EventResult{Err: &GatewayError{Op: "stream events", Err: err}},
|
||||
cancelWhenResultBufferFull,
|
||||
cancel)
|
||||
// A genuine terminal stream error must be reported as itself, even
|
||||
// when the data slots are full. Routing it through sendEventResult
|
||||
// would let the overflow branch substitute ErrSlowConsumer and lose
|
||||
// the real gRPC status, so send it directly, bypassing that branch.
|
||||
sendTerminalEventResult(streamCtx, results, EventResult{Err: &GatewayError{Op: "stream events", Err: err}}, cancelWhenResultBufferFull)
|
||||
return
|
||||
}
|
||||
}()
|
||||
@@ -1072,6 +1093,35 @@ func ensureBulkSize(name string, length int) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
// sendTerminalEventResult enqueues a terminal EventResult, bypassing
|
||||
// sendEventResult's overflow branch so a genuine stream error is reported verbatim
|
||||
// rather than relabeled as ErrSlowConsumer. How it sends depends on the mode:
|
||||
//
|
||||
// - cancelWhenBufferFull=true (Events/EventsAfter): ordinary data sends are capped
|
||||
// at eventBufferSize, leaving eventBufferReservedSlots free, so a non-blocking
|
||||
// send always lands the terminal result. Because this goroutine is the sole
|
||||
// producer, at most one terminal send ever races for the reserved slot, so the
|
||||
// select default only fires when the reserve is already spent — never dropping a
|
||||
// first terminal error.
|
||||
// - cancelWhenBufferFull=false (SubscribeEvents/SubscribeEventsAfter, never-drop):
|
||||
// ordinary data sends are uncapped and blocking, so every slot including the
|
||||
// reserve can hold data. A non-blocking send would then hit the full buffer and
|
||||
// silently drop the terminal error, breaking the never-drop contract; instead
|
||||
// block until the consumer drains a slot (or the stream context is cancelled).
|
||||
func sendTerminalEventResult(ctx context.Context, results chan<- EventResult, result EventResult, cancelWhenBufferFull bool) {
|
||||
if cancelWhenBufferFull {
|
||||
select {
|
||||
case results <- result:
|
||||
default:
|
||||
}
|
||||
return
|
||||
}
|
||||
select {
|
||||
case results <- result:
|
||||
case <-ctx.Done():
|
||||
}
|
||||
}
|
||||
|
||||
func sendEventResult(
|
||||
ctx context.Context,
|
||||
results chan<- EventResult,
|
||||
|
||||
+14
@@ -29,4 +29,18 @@ public final class MxAccessException extends MxGatewayCommandException {
|
||||
public MxAccessException(String operation, MxCommandReply reply) {
|
||||
super(operation, reply == null ? null : reply.getProtocolStatus(), reply);
|
||||
}
|
||||
|
||||
/**
|
||||
* Creates a new MXAccess exception with an already-built, verbatim message.
|
||||
* Used to re-surface an MXAccess failure with a redacted message while
|
||||
* preserving the original protocol status and reply.
|
||||
*
|
||||
* @param message the exact message to surface (already formatted/redacted)
|
||||
* @param protocolStatus protocol status reported by the gateway
|
||||
* @param reply raw command reply containing the MXAccess failure detail
|
||||
* @param cause underlying error, or {@code null}
|
||||
*/
|
||||
public MxAccessException(String message, ProtocolStatus protocolStatus, MxCommandReply reply, Throwable cause) {
|
||||
super(message, protocolStatus, reply, cause);
|
||||
}
|
||||
}
|
||||
|
||||
+17
@@ -25,6 +25,23 @@ public class MxGatewayCommandException extends MxGatewayException {
|
||||
this.reply = reply;
|
||||
}
|
||||
|
||||
/**
|
||||
* Creates a new command exception with an already-built, verbatim message.
|
||||
* Used to re-surface a failure with a redacted message while preserving the
|
||||
* original protocol status and reply.
|
||||
*
|
||||
* @param message the exact message to surface (already formatted/redacted)
|
||||
* @param protocolStatus protocol status returned by the gateway
|
||||
* @param reply raw command reply, or {@code null} when none was produced
|
||||
* @param cause underlying error, or {@code null}
|
||||
*/
|
||||
protected MxGatewayCommandException(
|
||||
String message, ProtocolStatus protocolStatus, MxCommandReply reply, Throwable cause) {
|
||||
super(message, cause);
|
||||
this.protocolStatus = protocolStatus;
|
||||
this.reply = reply;
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns the gateway protocol status that triggered this exception.
|
||||
*
|
||||
|
||||
+32
@@ -0,0 +1,32 @@
|
||||
package com.zb.mom.ww.mxgateway.client;
|
||||
|
||||
/**
|
||||
* Thrown when the gateway returns a protocol-OK command reply that carries
|
||||
* neither the expected typed payload nor a usable {@code return_value}.
|
||||
*
|
||||
* <p>A successful reply for a value-returning command (for example
|
||||
* {@code AuthenticateUser}, {@code ArchestrAUserToId}, or {@code AddBufferedItem})
|
||||
* must supply either the command's typed payload or an int32 {@code return_value}.
|
||||
* A reply that satisfies neither is malformed, and the client surfaces this
|
||||
* distinct failure rather than silently returning a default {@code 0}.
|
||||
*/
|
||||
public final class MxGatewayMalformedReplyException extends MxGatewayException {
|
||||
/**
|
||||
* Creates a new malformed-reply exception with the supplied message.
|
||||
*
|
||||
* @param message human-readable description of the malformed reply
|
||||
*/
|
||||
public MxGatewayMalformedReplyException(String message) {
|
||||
super(message);
|
||||
}
|
||||
|
||||
/**
|
||||
* Creates a new malformed-reply exception with the supplied message and cause.
|
||||
*
|
||||
* @param message human-readable description of the malformed reply
|
||||
* @param cause underlying error that triggered the failure
|
||||
*/
|
||||
public MxGatewayMalformedReplyException(String message, Throwable cause) {
|
||||
super(message, cause);
|
||||
}
|
||||
}
|
||||
+30
@@ -54,4 +54,34 @@ public final class MxGatewaySecrets {
|
||||
}
|
||||
return String.join(" ", parts);
|
||||
}
|
||||
|
||||
/**
|
||||
* Replaces every occurrence of each supplied secret with the redaction
|
||||
* marker {@code "<redacted>"}. Unlike {@link #redactCredentials(String)},
|
||||
* which scrubs by pattern, this performs an exact-substring scrub of the
|
||||
* caller-known secrets — used to strip a credential the gateway echoed back
|
||||
* into a free-form failure message.
|
||||
*
|
||||
* @param message the message to scrub, may be {@code null}
|
||||
* @param secrets the exact secret substrings to remove; {@code null}, empty,
|
||||
* and blank (whitespace-only) entries and a {@code null} array are ignored
|
||||
* @return {@code message} unchanged when it is {@code null} or no non-blank
|
||||
* secret is supplied, otherwise the message with every secret occurrence
|
||||
* replaced by {@code "<redacted>"}
|
||||
*/
|
||||
public static String redactExact(String message, String... secrets) {
|
||||
if (message == null || secrets == null) {
|
||||
return message;
|
||||
}
|
||||
|
||||
String result = message;
|
||||
for (String secret : secrets) {
|
||||
if (secret == null || secret.isBlank()) {
|
||||
// A blank "secret" would over-redact real whitespace; skip it.
|
||||
continue;
|
||||
}
|
||||
result = result.replace(secret, "<redacted>");
|
||||
}
|
||||
return result;
|
||||
}
|
||||
}
|
||||
|
||||
+184
-6
@@ -31,6 +31,7 @@ import mxaccess_gateway.v1.MxaccessGateway.MxSparseElement;
|
||||
import mxaccess_gateway.v1.MxaccessGateway.MxStatusProxy;
|
||||
import mxaccess_gateway.v1.MxaccessGateway.MxValue;
|
||||
import mxaccess_gateway.v1.MxaccessGateway.OpenSessionReply;
|
||||
import mxaccess_gateway.v1.MxaccessGateway.ProtocolStatus;
|
||||
import mxaccess_gateway.v1.MxaccessGateway.ReadBulkCommand;
|
||||
import mxaccess_gateway.v1.MxaccessGateway.RegisterCommand;
|
||||
import mxaccess_gateway.v1.MxaccessGateway.RemoveItemBulkCommand;
|
||||
@@ -782,7 +783,8 @@ public final class MxGatewaySession implements AutoCloseable {
|
||||
*/
|
||||
public MxCommandReply writeSecuredRaw(
|
||||
int serverHandle, int itemHandle, int currentUserId, int verifierUserId, MxValue value) {
|
||||
return invokeCommand(MxCommand.newBuilder()
|
||||
return invokeCommandRedacted(
|
||||
MxCommand.newBuilder()
|
||||
.setKind(MxCommandKind.MX_COMMAND_KIND_WRITE_SECURED)
|
||||
.setWriteSecured(WriteSecuredCommand.newBuilder()
|
||||
.setServerHandle(serverHandle)
|
||||
@@ -790,7 +792,8 @@ public final class MxGatewaySession implements AutoCloseable {
|
||||
.setCurrentUserId(currentUserId)
|
||||
.setVerifierUserId(verifierUserId)
|
||||
.setValue(value))
|
||||
.build());
|
||||
.build(),
|
||||
secretStringOf(value));
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -837,7 +840,8 @@ public final class MxGatewaySession implements AutoCloseable {
|
||||
int verifierUserId,
|
||||
MxValue value,
|
||||
MxValue timestampValue) {
|
||||
return invokeCommand(MxCommand.newBuilder()
|
||||
return invokeCommandRedacted(
|
||||
MxCommand.newBuilder()
|
||||
.setKind(MxCommandKind.MX_COMMAND_KIND_WRITE_SECURED2)
|
||||
.setWriteSecured2(WriteSecured2Command.newBuilder()
|
||||
.setServerHandle(serverHandle)
|
||||
@@ -846,7 +850,8 @@ public final class MxGatewaySession implements AutoCloseable {
|
||||
.setVerifierUserId(verifierUserId)
|
||||
.setValue(value)
|
||||
.setTimestampValue(timestampValue))
|
||||
.build());
|
||||
.build(),
|
||||
secretStringOf(value));
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -866,18 +871,25 @@ public final class MxGatewaySession implements AutoCloseable {
|
||||
* @throws MxAccessException when MXAccess rejects the credential
|
||||
*/
|
||||
public int authenticateUser(int serverHandle, String verifyUser, String verifyUserPassword) {
|
||||
MxCommandReply reply = invokeCommand(MxCommand.newBuilder()
|
||||
MxCommandReply reply = invokeCommandRedacted(
|
||||
MxCommand.newBuilder()
|
||||
.setKind(MxCommandKind.MX_COMMAND_KIND_AUTHENTICATE_USER)
|
||||
.setAuthenticateUser(AuthenticateUserCommand.newBuilder()
|
||||
.setServerHandle(serverHandle)
|
||||
.setVerifyUser(verifyUser)
|
||||
.setVerifyUserPassword(verifyUserPassword))
|
||||
.build());
|
||||
.build(),
|
||||
verifyUserPassword);
|
||||
if (reply.hasAuthenticateUser()) {
|
||||
return reply.getAuthenticateUser().getUserId();
|
||||
}
|
||||
if (reply.hasReturnValue() && reply.getReturnValue().getKindCase() == MxValue.KindCase.INT32_VALUE) {
|
||||
return reply.getReturnValue().getInt32Value();
|
||||
}
|
||||
throw new MxGatewayMalformedReplyException(
|
||||
"AuthenticateUser returned a malformed reply: OK reply carried neither "
|
||||
+ "the typed payload nor an int32 return_value");
|
||||
}
|
||||
|
||||
/**
|
||||
* Invokes MXAccess {@code ArchestrAUserToId}, resolving a Galaxy user GUID
|
||||
@@ -899,8 +911,13 @@ public final class MxGatewaySession implements AutoCloseable {
|
||||
if (reply.hasArchestraUserToId()) {
|
||||
return reply.getArchestraUserToId().getUserId();
|
||||
}
|
||||
if (reply.hasReturnValue() && reply.getReturnValue().getKindCase() == MxValue.KindCase.INT32_VALUE) {
|
||||
return reply.getReturnValue().getInt32Value();
|
||||
}
|
||||
throw new MxGatewayMalformedReplyException(
|
||||
"ArchestrAUserToId returned a malformed reply: OK reply carried neither "
|
||||
+ "the typed payload nor an int32 return_value");
|
||||
}
|
||||
|
||||
/**
|
||||
* Invokes MXAccess {@code AddBufferedItem} and returns the new item handle.
|
||||
@@ -925,8 +942,13 @@ public final class MxGatewaySession implements AutoCloseable {
|
||||
if (reply.hasAddBufferedItem()) {
|
||||
return reply.getAddBufferedItem().getItemHandle();
|
||||
}
|
||||
if (reply.hasReturnValue() && reply.getReturnValue().getKindCase() == MxValue.KindCase.INT32_VALUE) {
|
||||
return reply.getReturnValue().getInt32Value();
|
||||
}
|
||||
throw new MxGatewayMalformedReplyException(
|
||||
"AddBufferedItem returned a malformed reply: OK reply carried neither "
|
||||
+ "the typed payload nor an int32 return_value");
|
||||
}
|
||||
|
||||
/**
|
||||
* Invokes MXAccess {@code SetBufferedUpdateInterval}, controlling how often
|
||||
@@ -1027,6 +1049,162 @@ public final class MxGatewaySession implements AutoCloseable {
|
||||
.build());
|
||||
}
|
||||
|
||||
/**
|
||||
* Invokes a credential-bearing command, scrubbing any exact secret the
|
||||
* gateway may have echoed back into a surfaced failure message. The secret
|
||||
* lives only in the request, but a non-parity gateway or provider can copy
|
||||
* it into a diagnostic; this guarantees it never survives in the exception
|
||||
* text a caller might log.
|
||||
*
|
||||
* <p>On failure both the exception's message <em>and</em> its structured
|
||||
* context (the {@link ProtocolStatus} and {@link MxCommandReply} a caller can
|
||||
* inspect and log) are scrubbed with {@link MxGatewaySecrets#redactExact}: the
|
||||
* gateway echoes the credential into {@code protocolStatus.message},
|
||||
* {@code reply.diagnosticMessage}, and each {@code statuses[i].diagnosticText}.
|
||||
* If nothing carried the secret (the common case) the original exception is
|
||||
* rethrown untouched. Otherwise it is re-thrown as the same concrete type
|
||||
* carrying the redacted message and scrubbed context; the secret-bearing
|
||||
* original is not chained as a cause, so it cannot leak through a printed
|
||||
* stack trace.
|
||||
*/
|
||||
private MxCommandReply invokeCommandRedacted(MxCommand command, String... secrets) {
|
||||
try {
|
||||
return invokeCommand(command);
|
||||
} catch (MxGatewayException ex) {
|
||||
String original = ex.getMessage();
|
||||
String redactedMessage = MxGatewaySecrets.redactExact(original, secrets);
|
||||
boolean messageChanged = redactedMessage != null && !redactedMessage.equals(original);
|
||||
|
||||
ProtocolStatus status = protocolStatusOf(ex);
|
||||
ProtocolStatus scrubbedStatus = scrubProtocolStatus(status, secrets);
|
||||
boolean statusChanged = status != null && !status.equals(scrubbedStatus);
|
||||
|
||||
MxCommandReply reply = replyOf(ex);
|
||||
MxCommandReply scrubbedReply = scrubReply(reply, secrets);
|
||||
boolean replyChanged = reply != null && !reply.equals(scrubbedReply);
|
||||
|
||||
if (!messageChanged && !statusChanged && !replyChanged) {
|
||||
throw ex;
|
||||
}
|
||||
|
||||
String message = messageChanged ? redactedMessage : original;
|
||||
throw rebuildRedacted(ex, message, scrubbedStatus, scrubbedReply);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Extracts the {@link ProtocolStatus} an exception carries, if any, so it can
|
||||
* be scrubbed and re-attached to the rebuilt exception.
|
||||
*/
|
||||
private static ProtocolStatus protocolStatusOf(MxGatewayException ex) {
|
||||
if (ex instanceof MxGatewayCommandException command) {
|
||||
return command.protocolStatus();
|
||||
}
|
||||
if (ex instanceof MxGatewaySessionException session) {
|
||||
return session.protocolStatus();
|
||||
}
|
||||
if (ex instanceof MxGatewayWorkerException worker) {
|
||||
return worker.protocolStatus();
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Extracts the raw {@link MxCommandReply} an exception carries, if any.
|
||||
*/
|
||||
private static MxCommandReply replyOf(MxGatewayException ex) {
|
||||
if (ex instanceof MxGatewayCommandException command) {
|
||||
return command.reply();
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Rebuilds a gateway exception of the same concrete type with a redacted
|
||||
* message and already-scrubbed context, mirroring the .NET client's
|
||||
* type-switch. Only a truly-unknown subtype collapses to the base
|
||||
* {@link MxGatewayException}. The original (secret-bearing) exception is
|
||||
* deliberately not chained as a cause.
|
||||
*/
|
||||
private static MxGatewayException rebuildRedacted(
|
||||
MxGatewayException ex, String message, ProtocolStatus status, MxCommandReply reply) {
|
||||
if (ex instanceof MxAccessException) {
|
||||
return new MxAccessException(message, status, reply, null);
|
||||
}
|
||||
if (ex instanceof MxGatewayCommandException) {
|
||||
return new MxGatewayCommandException(message, status, reply, null);
|
||||
}
|
||||
if (ex instanceof MxGatewaySessionException) {
|
||||
return new MxGatewaySessionException(message, status, null);
|
||||
}
|
||||
if (ex instanceof MxGatewayWorkerException) {
|
||||
return new MxGatewayWorkerException(message, status, null);
|
||||
}
|
||||
if (ex instanceof MxGatewayMalformedReplyException) {
|
||||
return new MxGatewayMalformedReplyException(message);
|
||||
}
|
||||
if (ex instanceof MxGatewayAuthenticationException) {
|
||||
return new MxGatewayAuthenticationException(message, null);
|
||||
}
|
||||
if (ex instanceof MxGatewayAuthorizationException) {
|
||||
return new MxGatewayAuthorizationException(message, null);
|
||||
}
|
||||
return new MxGatewayException(message);
|
||||
}
|
||||
|
||||
/**
|
||||
* Produces a scrubbed clone of a command reply, removing any exact secret the
|
||||
* gateway echoed into {@code protocolStatus.message},
|
||||
* {@code diagnosticMessage}, or a status's {@code diagnosticText}.
|
||||
*
|
||||
* @param reply the reply to scrub, or {@code null}
|
||||
* @param secrets the exact secrets to strip
|
||||
* @return {@code null} when {@code reply} is {@code null}, otherwise a clone
|
||||
* with every echoed secret replaced by the redaction marker
|
||||
*/
|
||||
private static MxCommandReply scrubReply(MxCommandReply reply, String... secrets) {
|
||||
if (reply == null) {
|
||||
return null;
|
||||
}
|
||||
MxCommandReply.Builder builder = reply.toBuilder();
|
||||
if (builder.hasProtocolStatus()) {
|
||||
builder.setProtocolStatus(scrubProtocolStatus(builder.getProtocolStatus(), secrets));
|
||||
}
|
||||
builder.setDiagnosticMessage(MxGatewaySecrets.redactExact(builder.getDiagnosticMessage(), secrets));
|
||||
for (int index = 0; index < builder.getStatusesCount(); index++) {
|
||||
MxStatusProxy.Builder status = builder.getStatuses(index).toBuilder();
|
||||
status.setDiagnosticText(MxGatewaySecrets.redactExact(status.getDiagnosticText(), secrets));
|
||||
builder.setStatuses(index, status);
|
||||
}
|
||||
return builder.build();
|
||||
}
|
||||
|
||||
/**
|
||||
* Produces a scrubbed clone of a protocol status, removing any exact secret
|
||||
* the gateway echoed into its free-form {@code message}.
|
||||
*/
|
||||
private static ProtocolStatus scrubProtocolStatus(ProtocolStatus status, String... secrets) {
|
||||
if (status == null) {
|
||||
return null;
|
||||
}
|
||||
return status.toBuilder()
|
||||
.setMessage(MxGatewaySecrets.redactExact(status.getMessage(), secrets))
|
||||
.build();
|
||||
}
|
||||
|
||||
/**
|
||||
* Extracts the string payload of a secured-write value so it can be scrubbed
|
||||
* from an echoed failure message. Only string-kind values carry a
|
||||
* credential-shaped secret worth redacting; other kinds return {@code null}
|
||||
* (ignored by {@link MxGatewaySecrets#redactExact}).
|
||||
*/
|
||||
private static String secretStringOf(MxValue value) {
|
||||
if (value != null && value.getKindCase() == MxValue.KindCase.STRING_VALUE) {
|
||||
return value.getStringValue();
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
private static String newCorrelationId() {
|
||||
byte[] bytes = new byte[16];
|
||||
RANDOM.nextBytes(bytes);
|
||||
|
||||
+14
@@ -20,6 +20,20 @@ public final class MxGatewaySessionException extends MxGatewayException {
|
||||
this.protocolStatus = protocolStatus;
|
||||
}
|
||||
|
||||
/**
|
||||
* Creates a new session exception with an already-built, verbatim message.
|
||||
* Used to re-surface a failure with a redacted message while preserving the
|
||||
* (already scrubbed) protocol status.
|
||||
*
|
||||
* @param message the exact message to surface (already formatted/redacted)
|
||||
* @param protocolStatus protocol status returned by the gateway
|
||||
* @param cause underlying error, or {@code null}
|
||||
*/
|
||||
protected MxGatewaySessionException(String message, ProtocolStatus protocolStatus, Throwable cause) {
|
||||
super(message, cause);
|
||||
this.protocolStatus = protocolStatus;
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns the gateway protocol status that triggered this exception.
|
||||
*
|
||||
|
||||
+14
@@ -20,6 +20,20 @@ public final class MxGatewayWorkerException extends MxGatewayException {
|
||||
this.protocolStatus = protocolStatus;
|
||||
}
|
||||
|
||||
/**
|
||||
* Creates a new worker exception with an already-built, verbatim message.
|
||||
* Used to re-surface a failure with a redacted message while preserving the
|
||||
* (already scrubbed) protocol status.
|
||||
*
|
||||
* @param message the exact message to surface (already formatted/redacted)
|
||||
* @param protocolStatus protocol status returned by the gateway
|
||||
* @param cause underlying error, or {@code null}
|
||||
*/
|
||||
protected MxGatewayWorkerException(String message, ProtocolStatus protocolStatus, Throwable cause) {
|
||||
super(message, cause);
|
||||
this.protocolStatus = protocolStatus;
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns the gateway protocol status that triggered this exception.
|
||||
*
|
||||
|
||||
+201
@@ -0,0 +1,201 @@
|
||||
package com.zb.mom.ww.mxgateway.client;
|
||||
|
||||
import static org.junit.jupiter.api.Assertions.assertEquals;
|
||||
import static org.junit.jupiter.api.Assertions.assertFalse;
|
||||
import static org.junit.jupiter.api.Assertions.assertNotNull;
|
||||
import static org.junit.jupiter.api.Assertions.assertThrows;
|
||||
import static org.junit.jupiter.api.Assertions.assertTrue;
|
||||
|
||||
import com.google.protobuf.util.JsonFormat;
|
||||
import io.grpc.ManagedChannel;
|
||||
import io.grpc.Server;
|
||||
import io.grpc.inprocess.InProcessChannelBuilder;
|
||||
import io.grpc.inprocess.InProcessServerBuilder;
|
||||
import io.grpc.stub.StreamObserver;
|
||||
import java.nio.file.Files;
|
||||
import java.nio.file.Path;
|
||||
import java.time.Duration;
|
||||
import java.util.UUID;
|
||||
import mxaccess_gateway.v1.MxAccessGatewayGrpc;
|
||||
import mxaccess_gateway.v1.MxaccessGateway.MxCommandReply;
|
||||
import mxaccess_gateway.v1.MxaccessGateway.MxCommandRequest;
|
||||
import mxaccess_gateway.v1.MxaccessGateway.MxValue;
|
||||
import mxaccess_gateway.v1.MxaccessGateway.ProtocolStatus;
|
||||
import mxaccess_gateway.v1.MxaccessGateway.ProtocolStatusCode;
|
||||
import org.junit.jupiter.api.Test;
|
||||
|
||||
final class MxGatewayCredentialReplyTests {
|
||||
private static final String CREDENTIAL = "sup3rSecretVerify9f3a2b";
|
||||
|
||||
@Test
|
||||
void authenticateUserRedactsEchoedCredentialFromReplyDrivenError() throws Exception {
|
||||
assertCredentialFullyRedacted(
|
||||
"authenticate-user.echoed-credential.reply.json", "auth-echo-session");
|
||||
}
|
||||
|
||||
@Test
|
||||
void authenticateUserRedactsEchoedCredentialFromMxAccessFailureReply() throws Exception {
|
||||
assertCredentialFullyRedacted(
|
||||
"authenticate-user.echoed-credential-mxaccess-failure.reply.json",
|
||||
"auth-echo-failure-session");
|
||||
}
|
||||
|
||||
private static void assertCredentialFullyRedacted(String fixture, String sessionId) throws Exception {
|
||||
MxCommandReply reply = loadReply(fixture);
|
||||
|
||||
try (InProcessGateway gateway = InProcessGateway.startReturning(reply);
|
||||
MxGatewayClient client = gateway.client()) {
|
||||
MxGatewaySession session = MxGatewaySession.forSessionId(client, sessionId);
|
||||
|
||||
MxAccessException error = assertThrows(
|
||||
MxAccessException.class,
|
||||
() -> session.authenticateUser(12, "operator", CREDENTIAL));
|
||||
|
||||
assertFalse(error.getMessage().contains(CREDENTIAL),
|
||||
"credential echoed by the gateway must not survive in the surfaced message");
|
||||
assertTrue(error.getMessage().contains("<redacted>"),
|
||||
"the echoed credential must be replaced with the redaction marker");
|
||||
|
||||
// The rebuilt exception must not re-expose the credential through the
|
||||
// structured reply/protocolStatus a caller can inspect and log.
|
||||
MxCommandReply surfaced = error.reply();
|
||||
assertNotNull(surfaced, "the redacted exception must preserve a reply for inspection");
|
||||
assertFalse(surfaced.getProtocolStatus().getMessage().contains(CREDENTIAL),
|
||||
"reply protocol status message must not leak the echoed credential");
|
||||
assertFalse(surfaced.getDiagnosticMessage().contains(CREDENTIAL),
|
||||
"reply diagnostic message must not leak the echoed credential");
|
||||
for (int index = 0; index < surfaced.getStatusesCount(); index++) {
|
||||
assertFalse(surfaced.getStatusesList().get(index).getDiagnosticText().contains(CREDENTIAL),
|
||||
"reply status diagnostic text must not leak the echoed credential");
|
||||
}
|
||||
assertNotNull(error.protocolStatus(), "the redacted exception must preserve a protocol status");
|
||||
assertFalse(error.protocolStatus().getMessage().contains(CREDENTIAL),
|
||||
"exception protocol status must not leak the echoed credential");
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
void authenticateUserMissingPayloadThrowsMalformedReply() throws Exception {
|
||||
MxCommandReply reply = loadReply("authenticate-user.missing-payload.reply.json");
|
||||
|
||||
try (InProcessGateway gateway = InProcessGateway.startReturning(reply);
|
||||
MxGatewayClient client = gateway.client()) {
|
||||
MxGatewaySession session = MxGatewaySession.forSessionId(client, "auth-missing-session");
|
||||
|
||||
assertThrows(
|
||||
MxGatewayMalformedReplyException.class,
|
||||
() -> session.authenticateUser(3, "operator", "pw"));
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
void authenticateUserReturnValueOnlyReplyReturnsInt32Fallback() throws Exception {
|
||||
MxCommandReply reply = loadReply("authenticate-user.return-value-only.reply.json");
|
||||
|
||||
try (InProcessGateway gateway = InProcessGateway.startReturning(reply);
|
||||
MxGatewayClient client = gateway.client()) {
|
||||
MxGatewaySession session = MxGatewaySession.forSessionId(client, "auth-return-session");
|
||||
|
||||
assertEquals(7, session.authenticateUser(3, "operator", "pw"));
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
void addBufferedItemReturnValueOnlyReplyReturnsInt32Fallback() throws Exception {
|
||||
MxCommandReply reply = MxCommandReply.newBuilder()
|
||||
.setProtocolStatus(ok())
|
||||
.setReturnValue(MxValue.newBuilder().setInt32Value(55))
|
||||
.build();
|
||||
|
||||
try (InProcessGateway gateway = InProcessGateway.startReturning(reply);
|
||||
MxGatewayClient client = gateway.client()) {
|
||||
MxGatewaySession session = MxGatewaySession.forSessionId(client, "buffered-return-session");
|
||||
|
||||
assertEquals(55, session.addBufferedItem(3, "Tank01.Level", "galaxy"));
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
void addBufferedItemMissingPayloadThrowsMalformedReply() throws Exception {
|
||||
MxCommandReply reply = MxCommandReply.newBuilder().setProtocolStatus(ok()).build();
|
||||
|
||||
try (InProcessGateway gateway = InProcessGateway.startReturning(reply);
|
||||
MxGatewayClient client = gateway.client()) {
|
||||
MxGatewaySession session = MxGatewaySession.forSessionId(client, "buffered-malformed-session");
|
||||
|
||||
assertThrows(
|
||||
MxGatewayMalformedReplyException.class,
|
||||
() -> session.addBufferedItem(3, "Tank01.Level", "galaxy"));
|
||||
}
|
||||
}
|
||||
|
||||
private static ProtocolStatus ok() {
|
||||
return ProtocolStatus.newBuilder()
|
||||
.setCode(ProtocolStatusCode.PROTOCOL_STATUS_CODE_OK)
|
||||
.build();
|
||||
}
|
||||
|
||||
private static MxCommandReply loadReply(String fixture) throws Exception {
|
||||
MxCommandReply.Builder builder = MxCommandReply.newBuilder();
|
||||
JsonFormat.parser().merge(
|
||||
Files.readString(fixtureRoot().resolve("command-replies/" + fixture)),
|
||||
builder);
|
||||
return builder.build();
|
||||
}
|
||||
|
||||
private static Path fixtureRoot() {
|
||||
Path current = Path.of(System.getProperty("user.dir")).toAbsolutePath();
|
||||
for (Path path = current; path != null; path = path.getParent()) {
|
||||
Path candidate = path.resolve("clients/proto/fixtures/behavior");
|
||||
if (Files.exists(candidate)) {
|
||||
return candidate;
|
||||
}
|
||||
candidate = path.resolve("../proto/fixtures/behavior").normalize();
|
||||
if (Files.exists(candidate)) {
|
||||
return candidate;
|
||||
}
|
||||
}
|
||||
throw new IllegalStateException("could not locate behavior fixtures from " + current);
|
||||
}
|
||||
|
||||
private record InProcessGateway(Server server, ManagedChannel channel) implements AutoCloseable {
|
||||
static InProcessGateway startReturning(MxCommandReply reply) throws Exception {
|
||||
String serverName = "mxgw-java-cred-" + UUID.randomUUID();
|
||||
MxAccessGatewayGrpc.MxAccessGatewayImplBase service =
|
||||
new MxAccessGatewayGrpc.MxAccessGatewayImplBase() {
|
||||
@Override
|
||||
public void invoke(
|
||||
MxCommandRequest request, StreamObserver<MxCommandReply> responseObserver) {
|
||||
responseObserver.onNext(reply);
|
||||
responseObserver.onCompleted();
|
||||
}
|
||||
};
|
||||
Server server = InProcessServerBuilder.forName(serverName)
|
||||
.directExecutor()
|
||||
.addService(service)
|
||||
.build()
|
||||
.start();
|
||||
ManagedChannel channel = InProcessChannelBuilder.forName(serverName)
|
||||
.directExecutor()
|
||||
.build();
|
||||
return new InProcessGateway(server, channel);
|
||||
}
|
||||
|
||||
MxGatewayClient client() {
|
||||
return new MxGatewayClient(
|
||||
channel,
|
||||
MxGatewayClientOptions.builder()
|
||||
.endpoint("in-process")
|
||||
.apiKey("")
|
||||
.plaintext(true)
|
||||
.callTimeout(Duration.ofSeconds(5))
|
||||
.build());
|
||||
}
|
||||
|
||||
@Override
|
||||
public void close() {
|
||||
channel.shutdownNow();
|
||||
server.shutdownNow();
|
||||
}
|
||||
}
|
||||
}
|
||||
+50
@@ -0,0 +1,50 @@
|
||||
package com.zb.mom.ww.mxgateway.client;
|
||||
|
||||
import static org.junit.jupiter.api.Assertions.assertEquals;
|
||||
import static org.junit.jupiter.api.Assertions.assertFalse;
|
||||
import static org.junit.jupiter.api.Assertions.assertNull;
|
||||
|
||||
import org.junit.jupiter.api.Test;
|
||||
|
||||
final class MxGatewaySecretsTests {
|
||||
@Test
|
||||
void redactExactReplacesEveryOccurrenceOfASecret() {
|
||||
String message = "verify s3cr3t, retry s3cr3t, done s3cr3t";
|
||||
|
||||
String result = MxGatewaySecrets.redactExact(message, "s3cr3t");
|
||||
|
||||
assertFalse(result.contains("s3cr3t"), "no occurrence of the secret may survive");
|
||||
assertEquals("verify <redacted>, retry <redacted>, done <redacted>", result);
|
||||
}
|
||||
|
||||
@Test
|
||||
void redactExactFullyRedactsOverlappingSecretsWhenOneIsASubstringOfTheOther() {
|
||||
String message = "password=hunter2 token=hunter2extra";
|
||||
|
||||
String result = MxGatewaySecrets.redactExact(message, "hunter2extra", "hunter2");
|
||||
|
||||
assertFalse(result.contains("hunter2"), "both the secret and its superstring must be fully redacted");
|
||||
assertEquals("password=<redacted> token=<redacted>", result);
|
||||
}
|
||||
|
||||
@Test
|
||||
void redactExactWithNoSecretsReturnsMessageUnchanged() {
|
||||
String message = "nothing to scrub here";
|
||||
|
||||
assertEquals(message, MxGatewaySecrets.redactExact(message));
|
||||
}
|
||||
|
||||
@Test
|
||||
void redactExactToleratesNullMessage() {
|
||||
assertNull(MxGatewaySecrets.redactExact(null, "secret"));
|
||||
}
|
||||
|
||||
@Test
|
||||
void redactExactIgnoresBlankSecretSoRealSpacesAreNotOverRedacted() {
|
||||
String message = "keep these spaces intact";
|
||||
|
||||
String result = MxGatewaySecrets.redactExact(message, " ", "");
|
||||
|
||||
assertEquals(message, result);
|
||||
}
|
||||
}
|
||||
+22
@@ -0,0 +1,22 @@
|
||||
{
|
||||
"sessionId": "session-fixture",
|
||||
"correlationId": "gateway-correlation-authenticate-echoed-mxaccess-failure",
|
||||
"kind": "MX_COMMAND_KIND_AUTHENTICATE_USER",
|
||||
"protocolStatus": {
|
||||
"code": "PROTOCOL_STATUS_CODE_MXACCESS_FAILURE",
|
||||
"message": "MXAccess AuthenticateUser rejected credential 'sup3rSecretVerify9f3a2b'."
|
||||
},
|
||||
"hresult": -2147024891,
|
||||
"statuses": [
|
||||
{
|
||||
"success": 0,
|
||||
"category": "MX_STATUS_CATEGORY_SECURITY_ERROR",
|
||||
"detectedBy": "MX_STATUS_SOURCE_RESPONDING_NMX",
|
||||
"detail": 5,
|
||||
"rawCategory": 8,
|
||||
"rawDetectedBy": 5,
|
||||
"diagnosticText": "Authentication failed for password 'sup3rSecretVerify9f3a2b'."
|
||||
}
|
||||
],
|
||||
"diagnosticMessage": "MXAccess echoed the credential 'sup3rSecretVerify9f3a2b' back in its failure diagnostic."
|
||||
}
|
||||
+22
@@ -0,0 +1,22 @@
|
||||
{
|
||||
"sessionId": "session-fixture",
|
||||
"correlationId": "gateway-correlation-authenticate-echoed",
|
||||
"kind": "MX_COMMAND_KIND_AUTHENTICATE_USER",
|
||||
"protocolStatus": {
|
||||
"code": "PROTOCOL_STATUS_CODE_OK",
|
||||
"message": "MXAccess AuthenticateUser rejected credential 'sup3rSecretVerify9f3a2b'."
|
||||
},
|
||||
"hresult": -2147024891,
|
||||
"statuses": [
|
||||
{
|
||||
"success": 0,
|
||||
"category": "MX_STATUS_CATEGORY_SECURITY_ERROR",
|
||||
"detectedBy": "MX_STATUS_SOURCE_RESPONDING_NMX",
|
||||
"detail": 5,
|
||||
"rawCategory": 8,
|
||||
"rawDetectedBy": 5,
|
||||
"diagnosticText": "Authentication failed for password 'sup3rSecretVerify9f3a2b'."
|
||||
}
|
||||
],
|
||||
"diagnosticMessage": "MXAccess echoed the credential 'sup3rSecretVerify9f3a2b' back in its failure diagnostic."
|
||||
}
|
||||
+10
@@ -0,0 +1,10 @@
|
||||
{
|
||||
"sessionId": "session-fixture",
|
||||
"correlationId": "gateway-correlation-authenticate-missing-payload",
|
||||
"kind": "MX_COMMAND_KIND_AUTHENTICATE_USER",
|
||||
"protocolStatus": {
|
||||
"code": "PROTOCOL_STATUS_CODE_OK",
|
||||
"message": "AuthenticateUser reached MXAccess."
|
||||
},
|
||||
"diagnosticMessage": "Malformed: the OK reply carried neither an AuthenticateUser payload nor a return_value."
|
||||
}
|
||||
+15
@@ -0,0 +1,15 @@
|
||||
{
|
||||
"sessionId": "session-fixture",
|
||||
"correlationId": "gateway-correlation-authenticate-return-value-only",
|
||||
"kind": "MX_COMMAND_KIND_AUTHENTICATE_USER",
|
||||
"protocolStatus": {
|
||||
"code": "PROTOCOL_STATUS_CODE_OK",
|
||||
"message": "AuthenticateUser reached MXAccess."
|
||||
},
|
||||
"returnValue": {
|
||||
"dataType": "MX_DATA_TYPE_INTEGER",
|
||||
"variantType": "VT_I4",
|
||||
"int32Value": 7
|
||||
},
|
||||
"diagnosticMessage": "Legacy worker populated only return_value; the typed AuthenticateUser payload is absent."
|
||||
}
|
||||
@@ -48,6 +48,34 @@
|
||||
"path": "command-replies/write.hresult-e-fail.reply.json",
|
||||
"expectation": "A negative HRESULT fails the reply even when every status entry reports MX_STATUS_CATEGORY_OK."
|
||||
},
|
||||
{
|
||||
"id": "command-reply.authenticate-user.echoed-credential",
|
||||
"category": "command_replies",
|
||||
"messageType": "mxaccess_gateway.v1.MxCommandReply",
|
||||
"path": "command-replies/authenticate-user.echoed-credential.reply.json",
|
||||
"expectation": "When a gateway/MXAccess diagnostic echoes the caller's credential back (OK envelope, negative HRESULT), the surfaced error redacts the exact secret from both the rendered message and the structured reply accessors."
|
||||
},
|
||||
{
|
||||
"id": "command-reply.authenticate-user.echoed-credential-mxaccess-failure",
|
||||
"category": "command_replies",
|
||||
"messageType": "mxaccess_gateway.v1.MxCommandReply",
|
||||
"path": "command-replies/authenticate-user.echoed-credential-mxaccess-failure.reply.json",
|
||||
"expectation": "The same echoed-credential redaction holds when the reply is coded PROTOCOL_STATUS_CODE_MXACCESS_FAILURE, which every client routes to its MXAccess error type."
|
||||
},
|
||||
{
|
||||
"id": "command-reply.authenticate-user.missing-payload",
|
||||
"category": "command_replies",
|
||||
"messageType": "mxaccess_gateway.v1.MxCommandReply",
|
||||
"path": "command-replies/authenticate-user.missing-payload.reply.json",
|
||||
"expectation": "An OK reply with neither the typed AuthenticateUser payload nor a return_value raises a typed malformed-reply error, never a proto3 default 0 and never an NRE."
|
||||
},
|
||||
{
|
||||
"id": "command-reply.authenticate-user.return-value-only",
|
||||
"category": "command_replies",
|
||||
"messageType": "mxaccess_gateway.v1.MxCommandReply",
|
||||
"path": "command-replies/authenticate-user.return-value-only.reply.json",
|
||||
"expectation": "An OK reply missing the typed AuthenticateUser payload but carrying an int32 return_value falls back to the return_value (legacy-worker compatibility)."
|
||||
},
|
||||
{
|
||||
"id": "event-stream.session-ordered",
|
||||
"category": "event_streams",
|
||||
|
||||
@@ -3,6 +3,7 @@
|
||||
"cases": [
|
||||
{
|
||||
"id": "ok.responding-lmx",
|
||||
"wantSuccess": true,
|
||||
"status": {
|
||||
"success": 1,
|
||||
"category": "MX_STATUS_CATEGORY_OK",
|
||||
@@ -15,6 +16,7 @@
|
||||
},
|
||||
{
|
||||
"id": "security-error.requesting-lmx",
|
||||
"wantSuccess": false,
|
||||
"status": {
|
||||
"success": 0,
|
||||
"category": "MX_STATUS_CATEGORY_SECURITY_ERROR",
|
||||
@@ -27,6 +29,7 @@
|
||||
},
|
||||
{
|
||||
"id": "raw-unknown-category",
|
||||
"wantSuccess": false,
|
||||
"status": {
|
||||
"success": 0,
|
||||
"category": "MX_STATUS_CATEGORY_UNKNOWN",
|
||||
|
||||
@@ -11,6 +11,7 @@ from .generated.galaxy_repository_pb2 import (
|
||||
)
|
||||
from .events import ReplayGap
|
||||
from .errors import (
|
||||
MalformedReplyError,
|
||||
MxAccessError,
|
||||
MxGatewayAuthenticationError,
|
||||
MxGatewayAuthorizationError,
|
||||
@@ -35,6 +36,7 @@ __all__ = [
|
||||
"GalaxyRepositoryClient",
|
||||
"GatewayClient",
|
||||
"LazyBrowseNode",
|
||||
"MalformedReplyError",
|
||||
"MxAccessError",
|
||||
"MxGatewayAuthenticationError",
|
||||
"MxGatewayAuthorizationError",
|
||||
|
||||
@@ -53,6 +53,10 @@ class MxAccessError(MxGatewayCommandError):
|
||||
"""MXAccess HRESULT or status failure."""
|
||||
|
||||
|
||||
class MalformedReplyError(MxGatewayError):
|
||||
"""Raised when an OK reply lacks the expected typed payload and any usable return_value fallback."""
|
||||
|
||||
|
||||
def map_rpc_error(operation: str, error: grpc.RpcError) -> MxGatewayTransportError:
|
||||
"""Map a generated gRPC exception to the client exception hierarchy."""
|
||||
|
||||
@@ -153,8 +157,18 @@ def ensure_mxaccess_success(operation: str, reply: pb.MxCommandReply) -> pb.MxCo
|
||||
def _mxaccess_message(operation: str, reply: pb.MxCommandReply) -> str:
|
||||
status_text = reply.protocol_status.message or "MXAccess command failed"
|
||||
hresult = reply.hresult if reply.HasField("hresult") else None
|
||||
return (
|
||||
message = (
|
||||
f"{operation} failed: {status_text}; "
|
||||
f"session={reply.session_id}; correlation={reply.correlation_id}; "
|
||||
f"hresult={hresult}; statuses={len(reply.statuses)}"
|
||||
)
|
||||
# Append a per-status breakdown that carries the raw `success` COM member
|
||||
# verbatim for diagnostic parity with the other clients. `category` remains
|
||||
# the authoritative verdict; `success` is diagnostics only.
|
||||
for status in reply.statuses:
|
||||
category = pb.MxStatusCategory.Name(status.category)
|
||||
message += (
|
||||
f" [success={status.success}, category={category}, "
|
||||
f"detail={status.detail}, {status.diagnostic_text}]"
|
||||
)
|
||||
return message
|
||||
|
||||
@@ -5,7 +5,7 @@ from __future__ import annotations
|
||||
from collections.abc import AsyncIterator, Sequence
|
||||
|
||||
from .auth import redact_secret
|
||||
from .errors import MxGatewayError, ensure_mxaccess_success
|
||||
from .errors import MalformedReplyError, MxGatewayError, ensure_mxaccess_success
|
||||
from .events import ReplayGap
|
||||
from .generated import mxaccess_gateway_pb2 as pb
|
||||
from .values import MxValueInput, to_mx_value
|
||||
@@ -710,7 +710,15 @@ class Session:
|
||||
correlation_id=correlation_id,
|
||||
secrets=[verify_user_password],
|
||||
)
|
||||
if reply.HasField("authenticate_user"):
|
||||
return reply.authenticate_user.user_id
|
||||
if reply.HasField("return_value") and reply.return_value.WhichOneof("kind") == "int32_value":
|
||||
return reply.return_value.int32_value
|
||||
raise MalformedReplyError(
|
||||
"authenticate_user returned a malformed reply: OK reply carried "
|
||||
"neither the typed payload nor an int32 return_value",
|
||||
raw_reply=reply,
|
||||
)
|
||||
|
||||
async def archestra_user_to_id(
|
||||
self,
|
||||
@@ -730,7 +738,15 @@ class Session:
|
||||
),
|
||||
correlation_id=correlation_id,
|
||||
)
|
||||
if reply.HasField("archestra_user_to_id"):
|
||||
return reply.archestra_user_to_id.user_id
|
||||
if reply.HasField("return_value") and reply.return_value.WhichOneof("kind") == "int32_value":
|
||||
return reply.return_value.int32_value
|
||||
raise MalformedReplyError(
|
||||
"archestra_user_to_id returned a malformed reply: OK reply carried "
|
||||
"neither the typed payload nor an int32 return_value",
|
||||
raw_reply=reply,
|
||||
)
|
||||
|
||||
async def add_buffered_item(
|
||||
self,
|
||||
@@ -752,7 +768,15 @@ class Session:
|
||||
),
|
||||
correlation_id=correlation_id,
|
||||
)
|
||||
if reply.HasField("add_buffered_item"):
|
||||
return reply.add_buffered_item.item_handle
|
||||
if reply.HasField("return_value") and reply.return_value.WhichOneof("kind") == "int32_value":
|
||||
return reply.return_value.int32_value
|
||||
raise MalformedReplyError(
|
||||
"add_buffered_item returned a malformed reply: OK reply carried "
|
||||
"neither the typed payload nor an int32 return_value",
|
||||
raw_reply=reply,
|
||||
)
|
||||
|
||||
async def set_buffered_update_interval(
|
||||
self,
|
||||
@@ -895,19 +919,47 @@ def _value_secrets(value: MxValueInput) -> list[str]:
|
||||
|
||||
|
||||
def _redact_error(error: MxGatewayError, secrets: Sequence[str | None]) -> None:
|
||||
"""Scrub secret substrings from a raised error's message in place.
|
||||
"""Scrub secret substrings from a raised error's message and reply in place.
|
||||
|
||||
Rewrites ``error.args[0]`` (the message returned by ``str(error)``) through
|
||||
the shared :func:`~zb_mom_ww_mxgateway.auth.redact_secret` seam so credential
|
||||
text can never reach logs or be re-raised to a caller. The
|
||||
``protocol_status`` / ``raw_reply`` context is left untouched — those hold the
|
||||
gateway's own fields, which never echo the client-supplied secret.
|
||||
text can never reach logs or be re-raised to a caller.
|
||||
|
||||
A misbehaving MXAccess provider can echo the client-supplied credential back
|
||||
verbatim in its failure diagnostics, so ``error.raw_reply`` (the protobuf
|
||||
reply) can carry the secret in ``protocol_status.message``,
|
||||
``diagnostic_message``, and each ``statuses[].diagnostic_text``. A logger
|
||||
dumping those structured fields would reintroduce the leak the message scrub
|
||||
closes. When there is a secret to scrub and a reply is attached, this rebinds
|
||||
``error.raw_reply`` to a scrubbed deep copy so the raised exception carries no
|
||||
credential text on any surface. The clone leaves the original reply untouched.
|
||||
"""
|
||||
scrubbed = [secret for secret in secrets if secret]
|
||||
if not scrubbed:
|
||||
return
|
||||
if error.args and isinstance(error.args[0], str):
|
||||
error.args = (redact_secret(error.args[0], scrubbed), *error.args[1:])
|
||||
if error.raw_reply is not None:
|
||||
error.raw_reply = _redact_reply(error.raw_reply, scrubbed)
|
||||
|
||||
|
||||
def _redact_reply(reply: pb.MxCommandReply, secrets: Sequence[str]) -> pb.MxCommandReply:
|
||||
"""Return a deep copy of *reply* with credential text scrubbed from diagnostics.
|
||||
|
||||
Operates on a clone so the caller's original reply object is never mutated.
|
||||
Only the free-text diagnostic fields that can echo a client-supplied secret
|
||||
are scrubbed; the structured/enum fields the gateway itself sets are left as-is.
|
||||
"""
|
||||
clone = type(reply)()
|
||||
clone.CopyFrom(reply)
|
||||
if clone.protocol_status.message:
|
||||
clone.protocol_status.message = redact_secret(clone.protocol_status.message, secrets)
|
||||
if clone.diagnostic_message:
|
||||
clone.diagnostic_message = redact_secret(clone.diagnostic_message, secrets)
|
||||
for status in clone.statuses:
|
||||
if status.diagnostic_text:
|
||||
status.diagnostic_text = redact_secret(status.diagnostic_text, secrets)
|
||||
return clone
|
||||
|
||||
|
||||
from .client import GatewayClient # noqa: E402
|
||||
|
||||
@@ -0,0 +1,112 @@
|
||||
"""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
|
||||
@@ -140,10 +140,19 @@ async def test_write_secured_surfaces_native_failure_without_prior_authenticate(
|
||||
with pytest.raises(MxAccessError) as captured:
|
||||
await session.write_secured(12, 34, secret_value, current_user_id=5, verifier_user_id=6)
|
||||
|
||||
# Native failure is surfaced (not "fixed") and the raw reply is preserved...
|
||||
assert captured.value.raw_reply is failure
|
||||
# ...but the credential-sensitive value is scrubbed from the surfaced message.
|
||||
# Native failure is surfaced (not "fixed"): the raw reply's structure is
|
||||
# preserved so callers still see the native verdict...
|
||||
raw = captured.value.raw_reply
|
||||
assert raw is not None
|
||||
assert raw.kind == pb.MX_COMMAND_KIND_WRITE_SECURED
|
||||
assert raw.hresult == -2147217407
|
||||
assert raw.protocol_status.code == pb.PROTOCOL_STATUS_CODE_MXACCESS_FAILURE
|
||||
# ...but the credential-sensitive value is scrubbed from the surfaced message
|
||||
# AND from the reply's echoed diagnostics, so a logger dumping raw_reply's
|
||||
# structured fields cannot reintroduce the leak.
|
||||
assert secret_value not in str(captured.value)
|
||||
assert secret_value not in raw.protocol_status.message
|
||||
assert "[redacted]" in raw.protocol_status.message
|
||||
command = stub.invoke.requests[0].command
|
||||
assert command.kind == pb.MX_COMMAND_KIND_WRITE_SECURED
|
||||
assert command.write_secured.current_user_id == 5
|
||||
|
||||
+95
-16
@@ -193,17 +193,43 @@ impl std::error::Error for CommandError {}
|
||||
/// The wrapper is heap-allocated inside [`Error::MxAccess`] to keep the
|
||||
/// containing enum small. Callers can recover the reply with
|
||||
/// [`MxAccessError::reply`] or [`MxAccessError::into_reply`]. Its `Display`
|
||||
/// summarizes the `hresult` and status entries and scrubs any credential-like
|
||||
/// tokens from diagnostic text before it reaches a caller.
|
||||
#[derive(Clone, Debug)]
|
||||
/// summarizes the `hresult` and status entries and scrubs credentials from the
|
||||
/// rendered text before it reaches a caller: credential-*shaped* tokens
|
||||
/// (`mxgw_...`, `bearer`) via a pattern scrub, plus any exact caller-supplied
|
||||
/// secrets registered with [`MxAccessError::with_secrets`] — the latter catches
|
||||
/// a password MXAccess echoed back verbatim even though it has no token shape.
|
||||
///
|
||||
/// `Debug` is hand-written (not derived) so the attached exact secrets never
|
||||
/// reach `{:?}` output either: it scrubs them from the reply rendering and
|
||||
/// prints only the count of attached secrets, never their values.
|
||||
#[derive(Clone)]
|
||||
pub struct MxAccessError {
|
||||
reply: MxCommandReply,
|
||||
/// Exact caller-supplied secrets (e.g. an `AuthenticateUser` password or a
|
||||
/// `WriteSecured` string value) scrubbed from the rendered message. Empty
|
||||
/// unless a helper attaches them via [`Self::with_secrets`].
|
||||
secrets: Vec<String>,
|
||||
}
|
||||
|
||||
impl MxAccessError {
|
||||
/// Wrap a reply whose MXAccess-level result reported a failure.
|
||||
pub fn new(reply: MxCommandReply) -> Self {
|
||||
Self { reply }
|
||||
Self {
|
||||
reply,
|
||||
secrets: Vec::new(),
|
||||
}
|
||||
}
|
||||
|
||||
/// Register exact caller-supplied secrets to scrub from the rendered
|
||||
/// message, returning the updated error.
|
||||
///
|
||||
/// A credential MXAccess echoes back into its diagnostic text has no
|
||||
/// `mxgw_`/`bearer` shape, so the pattern scrub cannot catch it. Attaching
|
||||
/// the exact secret lets `Display` replace every occurrence with
|
||||
/// `<redacted>`.
|
||||
pub fn with_secrets(mut self, secrets: Vec<String>) -> Self {
|
||||
self.secrets = secrets;
|
||||
self
|
||||
}
|
||||
|
||||
/// Borrow the underlying reply (correlation id, hresult, statuses).
|
||||
@@ -217,15 +243,43 @@ impl MxAccessError {
|
||||
}
|
||||
}
|
||||
|
||||
impl std::fmt::Debug for MxAccessError {
|
||||
fn fmt(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
|
||||
// Render the reply, scrub any exact caller secret from it, and never
|
||||
// print the raw secrets themselves — only how many are attached.
|
||||
let mut reply = format!("{:?}", self.reply);
|
||||
for secret in &self.secrets {
|
||||
if !secret.is_empty() {
|
||||
reply = reply.replace(secret.as_str(), "<redacted>");
|
||||
}
|
||||
}
|
||||
|
||||
formatter
|
||||
.debug_struct("MxAccessError")
|
||||
.field("reply", &format_args!("{reply}"))
|
||||
.field(
|
||||
"secrets",
|
||||
&format_args!("[{} redacted]", self.secrets.len()),
|
||||
)
|
||||
.finish()
|
||||
}
|
||||
}
|
||||
|
||||
impl std::fmt::Display for MxAccessError {
|
||||
fn fmt(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
|
||||
use std::fmt::Write as _;
|
||||
|
||||
let hresult = match self.reply.hresult {
|
||||
Some(value) => value.to_string(),
|
||||
None => "none".to_owned(),
|
||||
};
|
||||
|
||||
// Render the whole body first so the exact-secret scrub can sweep every
|
||||
// field — including diagnostic text that already went through the
|
||||
// credential-shape scrub — before any of it reaches the caller.
|
||||
let mut body = String::new();
|
||||
write!(
|
||||
formatter,
|
||||
body,
|
||||
"hresult={hresult}, {} status entr{}",
|
||||
self.reply.statuses.len(),
|
||||
if self.reply.statuses.len() == 1 {
|
||||
@@ -233,20 +287,28 @@ impl std::fmt::Display for MxAccessError {
|
||||
} else {
|
||||
"ies"
|
||||
}
|
||||
)?;
|
||||
)
|
||||
.expect("writing to a String is infallible");
|
||||
|
||||
for status in &self.reply.statuses {
|
||||
let category = MxStatusCategory::try_from(status.category)
|
||||
.unwrap_or(MxStatusCategory::Unspecified);
|
||||
let diagnostic = redact_credentials(&status.diagnostic_text);
|
||||
write!(
|
||||
formatter,
|
||||
body,
|
||||
"; [success={}, category={category:?}, detail={}, {}]",
|
||||
status.success, status.detail, diagnostic
|
||||
)?;
|
||||
)
|
||||
.expect("writing to a String is infallible");
|
||||
}
|
||||
|
||||
Ok(())
|
||||
for secret in &self.secrets {
|
||||
if !secret.is_empty() {
|
||||
body = body.replace(secret.as_str(), "<redacted>");
|
||||
}
|
||||
}
|
||||
|
||||
formatter.write_str(&body)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -284,10 +346,17 @@ impl From<tonic::Status> for Error {
|
||||
/// Promote a non-OK protocol status carried inside an [`MxCommandReply`]
|
||||
/// to an [`Error::Command`].
|
||||
///
|
||||
/// [`ProtocolStatusCode::MxaccessFailure`] is deliberately **not** a
|
||||
/// command-level failure here: it signals an MXAccess-level rejection, so it
|
||||
/// falls through to [`ensure_mxaccess_success`] and surfaces as
|
||||
/// [`Error::MxAccess`] — matching the .NET, Java, Go, and Python clients. Every
|
||||
/// other non-`Ok` code stays [`Error::Command`].
|
||||
///
|
||||
/// # Errors
|
||||
///
|
||||
/// Returns [`Error::Command`] when `reply.protocol_status` is missing or
|
||||
/// reports any code other than [`ProtocolStatusCode::Ok`].
|
||||
/// reports any code other than [`ProtocolStatusCode::Ok`] or
|
||||
/// [`ProtocolStatusCode::MxaccessFailure`].
|
||||
pub fn ensure_command_success(reply: MxCommandReply) -> Result<MxCommandReply, Error> {
|
||||
let code = reply
|
||||
.protocol_status
|
||||
@@ -295,7 +364,7 @@ pub fn ensure_command_success(reply: MxCommandReply) -> Result<MxCommandReply, E
|
||||
.and_then(|status| ProtocolStatusCode::try_from(status.code).ok())
|
||||
.unwrap_or(ProtocolStatusCode::Unspecified);
|
||||
|
||||
if code == ProtocolStatusCode::Ok {
|
||||
if code == ProtocolStatusCode::Ok || code == ProtocolStatusCode::MxaccessFailure {
|
||||
Ok(reply)
|
||||
} else {
|
||||
Err(Box::new(CommandError::new(reply)).into())
|
||||
@@ -306,9 +375,12 @@ pub fn ensure_command_success(reply: MxCommandReply) -> Result<MxCommandReply, E
|
||||
/// [`MxCommandReply`] to an [`Error::MxAccess`].
|
||||
///
|
||||
/// This is the second reply check applied to the typed command path, after
|
||||
/// [`ensure_command_success`] confirms the protocol envelope is `Ok`. It
|
||||
/// enforces MXAccess parity: a reply can carry an `Ok` protocol envelope while
|
||||
/// MXAccess itself rejected the operation. Following COM semantics, only a
|
||||
/// [`ensure_command_success`] confirms the protocol envelope is `Ok` (or a
|
||||
/// [`ProtocolStatusCode::MxaccessFailure`] the first check lets fall through).
|
||||
/// It enforces MXAccess parity: a reply can carry an `Ok` protocol envelope
|
||||
/// while MXAccess itself rejected the operation, and a
|
||||
/// [`ProtocolStatusCode::MxaccessFailure`] envelope is itself an MXAccess-level
|
||||
/// failure regardless of `hresult`. Following COM semantics, only a
|
||||
/// **negative** `hresult` is a failure — positive codes such as `S_FALSE = 1`
|
||||
/// are success. A `MXSTATUS_PROXY` entry is treated as a failure when its
|
||||
/// `category` is not [`MxStatusCategory::Ok`]; the `success` member mirrors the
|
||||
@@ -321,17 +393,24 @@ pub fn ensure_command_success(reply: MxCommandReply) -> Result<MxCommandReply, E
|
||||
///
|
||||
/// # Errors
|
||||
///
|
||||
/// Returns [`Error::MxAccess`] when `reply.hresult` is negative or any
|
||||
/// Returns [`Error::MxAccess`] when the reply's protocol code is
|
||||
/// [`ProtocolStatusCode::MxaccessFailure`], `reply.hresult` is negative, or any
|
||||
/// `reply.statuses` entry reports a category other than
|
||||
/// [`MxStatusCategory::Ok`].
|
||||
pub fn ensure_mxaccess_success(reply: MxCommandReply) -> Result<MxCommandReply, Error> {
|
||||
let protocol_code = reply
|
||||
.protocol_status
|
||||
.as_ref()
|
||||
.and_then(|status| ProtocolStatusCode::try_from(status.code).ok())
|
||||
.unwrap_or(ProtocolStatusCode::Unspecified);
|
||||
let mxaccess_failure = protocol_code == ProtocolStatusCode::MxaccessFailure;
|
||||
let hresult_failure = reply.hresult.is_some_and(|hresult| hresult < 0);
|
||||
let status_failure = reply
|
||||
.statuses
|
||||
.iter()
|
||||
.any(|status| status.category != MxStatusCategory::Ok as i32);
|
||||
|
||||
if hresult_failure || status_failure {
|
||||
if mxaccess_failure || hresult_failure || status_failure {
|
||||
Err(Box::new(MxAccessError::new(reply)).into())
|
||||
} else {
|
||||
Ok(reply)
|
||||
|
||||
@@ -11,7 +11,7 @@
|
||||
use std::sync::atomic::{AtomicU64, Ordering};
|
||||
|
||||
use crate::client::{EventStream, GatewayClient};
|
||||
use crate::error::{ensure_protocol_success, Error};
|
||||
use crate::error::{ensure_protocol_success, Error, MxAccessError};
|
||||
use crate::generated::mxaccess_gateway::v1::mx_command::Payload;
|
||||
use crate::generated::mxaccess_gateway::v1::mx_command_reply;
|
||||
use crate::generated::mxaccess_gateway::v1::{
|
||||
@@ -27,7 +27,7 @@ use crate::generated::mxaccess_gateway::v1::{
|
||||
WriteSecured2BulkCommand, WriteSecured2BulkEntry, WriteSecured2Command,
|
||||
WriteSecuredBulkCommand, WriteSecuredBulkEntry, WriteSecuredCommand,
|
||||
};
|
||||
use crate::value::{MxStatus, MxValue};
|
||||
use crate::value::{MxStatus, MxValue, MxValueProjection};
|
||||
|
||||
const MAX_BULK_ITEMS: usize = 1_000;
|
||||
|
||||
@@ -801,6 +801,7 @@ impl Session {
|
||||
verifier_user_id: i32,
|
||||
value: MxValue,
|
||||
) -> Result<(), Error> {
|
||||
let secrets = string_secret(&value);
|
||||
self.invoke(
|
||||
MxCommandKind::WriteSecured,
|
||||
Payload::WriteSecured(WriteSecuredCommand {
|
||||
@@ -811,7 +812,8 @@ impl Session {
|
||||
value: Some(value.into_proto()),
|
||||
}),
|
||||
)
|
||||
.await?;
|
||||
.await
|
||||
.map_err(|error| attach_secrets(error, secrets))?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -831,6 +833,7 @@ impl Session {
|
||||
value: MxValue,
|
||||
timestamp_value: MxValue,
|
||||
) -> Result<(), Error> {
|
||||
let secrets = string_secret(&value);
|
||||
self.invoke(
|
||||
MxCommandKind::WriteSecured2,
|
||||
Payload::WriteSecured2(WriteSecured2Command {
|
||||
@@ -842,7 +845,8 @@ impl Session {
|
||||
timestamp_value: Some(timestamp_value.into_proto()),
|
||||
}),
|
||||
)
|
||||
.await?;
|
||||
.await
|
||||
.map_err(|error| attach_secrets(error, secrets))?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -882,7 +886,8 @@ impl Session {
|
||||
verify_user_password: verify_user_password.to_owned(),
|
||||
}),
|
||||
)
|
||||
.await?;
|
||||
.await
|
||||
.map_err(|error| attach_secrets(error, vec![verify_user_password.to_owned()]))?;
|
||||
|
||||
authenticate_user_id(&reply)
|
||||
}
|
||||
@@ -1074,8 +1079,14 @@ fn add_buffered_item_handle(reply: &MxCommandReply) -> Result<i32, Error> {
|
||||
fn authenticate_user_id(reply: &MxCommandReply) -> Result<i32, Error> {
|
||||
match reply.payload.as_ref() {
|
||||
Some(mx_command_reply::Payload::AuthenticateUser(authenticate)) => Ok(authenticate.user_id),
|
||||
_ => Err(Error::MalformedReply {
|
||||
detail: "authenticate_user reply lacked an AuthenticateUser payload".to_owned(),
|
||||
_ => reply
|
||||
.return_value
|
||||
.as_ref()
|
||||
.and_then(int32_reply_value)
|
||||
.ok_or_else(|| Error::MalformedReply {
|
||||
detail:
|
||||
"authenticate_user reply lacked an AuthenticateUser payload or int32 return_value"
|
||||
.to_owned(),
|
||||
}),
|
||||
}
|
||||
}
|
||||
@@ -1083,12 +1094,69 @@ fn authenticate_user_id(reply: &MxCommandReply) -> Result<i32, Error> {
|
||||
fn archestra_user_id(reply: &MxCommandReply) -> Result<i32, Error> {
|
||||
match reply.payload.as_ref() {
|
||||
Some(mx_command_reply::Payload::ArchestraUserToId(archestra)) => Ok(archestra.user_id),
|
||||
_ => Err(Error::MalformedReply {
|
||||
detail: "archestra_user_to_id reply lacked an ArchestraUserToId payload".to_owned(),
|
||||
_ => reply
|
||||
.return_value
|
||||
.as_ref()
|
||||
.and_then(int32_reply_value)
|
||||
.ok_or_else(|| Error::MalformedReply {
|
||||
detail:
|
||||
"archestra_user_to_id reply lacked an ArchestraUserToId payload or int32 return_value"
|
||||
.to_owned(),
|
||||
}),
|
||||
}
|
||||
}
|
||||
|
||||
/// Extract an exact string secret from a credential-sensitive [`MxValue`] so a
|
||||
/// failing `WriteSecured`/`WriteSecured2` can scrub it from the surfaced error.
|
||||
/// Non-string values carry no scrubbable secret and yield an empty vector.
|
||||
fn string_secret(value: &MxValue) -> Vec<String> {
|
||||
match value.projection() {
|
||||
MxValueProjection::String(text) if !text.is_empty() => vec![text.clone()],
|
||||
_ => Vec::new(),
|
||||
}
|
||||
}
|
||||
|
||||
/// Attach caller-supplied exact secrets to an [`Error::MxAccess`] before it
|
||||
/// propagates. This both scrubs the stored reply's caller-readable string
|
||||
/// fields (so `reply()`/`into_reply()` cannot recover a credential MXAccess
|
||||
/// echoed back verbatim) and keeps the secrets on the error as a
|
||||
/// belt-and-suspenders for `Display`/`Debug`. Any other error variant is
|
||||
/// returned unchanged.
|
||||
fn attach_secrets(error: Error, secrets: Vec<String>) -> Error {
|
||||
match error {
|
||||
Error::MxAccess(boxed) => {
|
||||
let mut reply = boxed.into_reply();
|
||||
scrub_reply_strings(&mut reply, &secrets);
|
||||
Error::MxAccess(Box::new(MxAccessError::new(reply).with_secrets(secrets)))
|
||||
}
|
||||
other => other,
|
||||
}
|
||||
}
|
||||
|
||||
/// Replace every non-empty secret occurrence with `<redacted>` in the reply's
|
||||
/// caller-readable string fields — `protocol_status.message`,
|
||||
/// `diagnostic_message`, and each `statuses[i].diagnostic_text`. A caller
|
||||
/// reading the structured reply back off an [`Error::MxAccess`] would otherwise
|
||||
/// reintroduce the leak that `Display`/`Debug` already close.
|
||||
fn scrub_reply_strings(reply: &mut MxCommandReply, secrets: &[String]) {
|
||||
for secret in secrets {
|
||||
if secret.is_empty() {
|
||||
continue;
|
||||
}
|
||||
if let Some(status) = reply.protocol_status.as_mut() {
|
||||
status.message = status.message.replace(secret.as_str(), "<redacted>");
|
||||
}
|
||||
reply.diagnostic_message = reply
|
||||
.diagnostic_message
|
||||
.replace(secret.as_str(), "<redacted>");
|
||||
for status in &mut reply.statuses {
|
||||
status.diagnostic_text = status
|
||||
.diagnostic_text
|
||||
.replace(secret.as_str(), "<redacted>");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn suspend_status(reply: MxCommandReply) -> Result<MxStatus, Error> {
|
||||
match reply.payload {
|
||||
Some(mx_command_reply::Payload::Suspend(suspend)) => suspend
|
||||
|
||||
@@ -84,8 +84,10 @@ async fn session_helpers_build_commands_and_preserve_command_errors() {
|
||||
.write(12, 34, ClientMxValue::int32(123), 0)
|
||||
.await
|
||||
.unwrap_err();
|
||||
let Error::Command(error) = error else {
|
||||
panic!("write failure should preserve the raw command reply: {error:?}");
|
||||
// A MXACCESS_FAILURE-coded reply is an MXAccess-level failure, routed to
|
||||
// Error::MxAccess (matching .NET/Java/Go/Python) rather than Error::Command.
|
||||
let Error::MxAccess(error) = error else {
|
||||
panic!("MXACCESS_FAILURE reply should route to Error::MxAccess: {error:?}");
|
||||
};
|
||||
assert_eq!(
|
||||
error.reply().protocol_status.as_ref().unwrap().code,
|
||||
@@ -804,6 +806,179 @@ async fn authenticate_user_keeps_credentials_out_of_surfaced_errors() {
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn authenticate_user_scrubs_exact_caller_credential_echoed_in_diagnostic() {
|
||||
// CLI-40: MXAccess can echo the supplied credential back inside its failure
|
||||
// diagnostic (here in statuses[0].diagnostic_text). The token has no
|
||||
// mxgw_/bearer shape, so the pattern scrub alone cannot catch it — the
|
||||
// exact-secret scrub must replace the caller's password with <redacted>.
|
||||
let credential = "sup3rSecretVerify9f3a2b";
|
||||
let state = Arc::new(FakeState::default());
|
||||
*state.invoke_override.lock().await = Some(InvokeOverride::CannedReply(Box::new(
|
||||
command_reply_fixture("authenticate-user.echoed-credential.reply.json"),
|
||||
)));
|
||||
let endpoint = spawn_fake_gateway(state.clone()).await;
|
||||
let client = GatewayClient::connect(ClientOptions::new(endpoint))
|
||||
.await
|
||||
.unwrap();
|
||||
let session = client.session("session-fixture");
|
||||
|
||||
let error = session
|
||||
.authenticate_user(7, "verifier", credential)
|
||||
.await
|
||||
.unwrap_err();
|
||||
|
||||
assert!(
|
||||
matches!(error, Error::MxAccess(_)),
|
||||
"OK protocol + negative hresult must route to Error::MxAccess: {error:?}"
|
||||
);
|
||||
let rendered = error.to_string();
|
||||
assert!(
|
||||
!rendered.contains(credential),
|
||||
"exact caller credential leaked into the surfaced error: {rendered}"
|
||||
);
|
||||
assert!(
|
||||
rendered.contains("<redacted>"),
|
||||
"credential occurrence must be replaced with <redacted>: {rendered}"
|
||||
);
|
||||
}
|
||||
|
||||
/// Drive `authenticate_user` against a canned reply that echoes the caller's
|
||||
/// credential in every string field, then assert the surfaced
|
||||
/// [`Error::MxAccess`] leaks it nowhere — neither through the structured reply a
|
||||
/// caller can read back (`reply().protocol_status.message`,
|
||||
/// `reply().diagnostic_message`, `reply().statuses[i].diagnostic_text`) nor
|
||||
/// through `Display`/`Debug`.
|
||||
async fn assert_authenticate_user_scrubs_structured_reply(fixture: &str) {
|
||||
let credential = "sup3rSecretVerify9f3a2b";
|
||||
let state = Arc::new(FakeState::default());
|
||||
*state.invoke_override.lock().await = Some(InvokeOverride::CannedReply(Box::new(
|
||||
command_reply_fixture(fixture),
|
||||
)));
|
||||
let endpoint = spawn_fake_gateway(state.clone()).await;
|
||||
let client = GatewayClient::connect(ClientOptions::new(endpoint))
|
||||
.await
|
||||
.unwrap();
|
||||
let session = client.session("session-fixture");
|
||||
|
||||
let error = session
|
||||
.authenticate_user(7, "verifier", credential)
|
||||
.await
|
||||
.unwrap_err();
|
||||
|
||||
let Error::MxAccess(mx_access) = &error else {
|
||||
panic!("{fixture}: credential-echoed reply must route to Error::MxAccess, got {error:?}");
|
||||
};
|
||||
|
||||
// The structured reply a caller can read back must be scrubbed too — the raw
|
||||
// MxCommandReply otherwise reintroduces the leak Display/Debug already close.
|
||||
let reply = mx_access.reply();
|
||||
if let Some(status) = reply.protocol_status.as_ref() {
|
||||
assert!(
|
||||
!status.message.contains(credential),
|
||||
"{fixture}: credential leaked via reply().protocol_status.message: {}",
|
||||
status.message
|
||||
);
|
||||
}
|
||||
assert!(
|
||||
!reply.diagnostic_message.contains(credential),
|
||||
"{fixture}: credential leaked via reply().diagnostic_message: {}",
|
||||
reply.diagnostic_message
|
||||
);
|
||||
for (index, status) in reply.statuses.iter().enumerate() {
|
||||
assert!(
|
||||
!status.diagnostic_text.contains(credential),
|
||||
"{fixture}: credential leaked via reply().statuses[{index}].diagnostic_text: {}",
|
||||
status.diagnostic_text
|
||||
);
|
||||
}
|
||||
|
||||
let display = error.to_string();
|
||||
let debug = format!("{error:?}");
|
||||
assert!(
|
||||
!display.contains(credential),
|
||||
"{fixture}: credential leaked into Display: {display}"
|
||||
);
|
||||
assert!(
|
||||
!debug.contains(credential),
|
||||
"{fixture}: credential leaked into Debug: {debug}"
|
||||
);
|
||||
assert!(
|
||||
display.contains("<redacted>"),
|
||||
"{fixture}: Display must mark the redaction: {display}"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn authenticate_user_scrubs_credential_from_structured_reply_ok_protocol_variant() {
|
||||
// OK protocol envelope + negative hresult: already Error::MxAccess before
|
||||
// ISSUE 2, but the stored reply's string fields still leaked the credential.
|
||||
assert_authenticate_user_scrubs_structured_reply(
|
||||
"authenticate-user.echoed-credential.reply.json",
|
||||
)
|
||||
.await;
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn authenticate_user_scrubs_credential_from_structured_reply_mxaccess_failure_variant() {
|
||||
// PROTOCOL_STATUS_CODE_MXACCESS_FAILURE: before ISSUE 2 this landed in
|
||||
// Error::Command (unscrubbed, raw Display/Debug) — the red-first case.
|
||||
assert_authenticate_user_scrubs_structured_reply(
|
||||
"authenticate-user.echoed-credential-mxaccess-failure.reply.json",
|
||||
)
|
||||
.await;
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn authenticate_user_maps_missing_payload_reply_to_malformed_reply() {
|
||||
// CLI-41: an OK reply with neither a typed AuthenticateUser payload nor a
|
||||
// return_value is malformed.
|
||||
let state = Arc::new(FakeState::default());
|
||||
*state.invoke_override.lock().await = Some(InvokeOverride::CannedReply(Box::new(
|
||||
command_reply_fixture("authenticate-user.missing-payload.reply.json"),
|
||||
)));
|
||||
let endpoint = spawn_fake_gateway(state.clone()).await;
|
||||
let client = GatewayClient::connect(ClientOptions::new(endpoint))
|
||||
.await
|
||||
.unwrap();
|
||||
let session = client.session("session-fixture");
|
||||
|
||||
let error = session
|
||||
.authenticate_user(7, "verifier", "pw")
|
||||
.await
|
||||
.unwrap_err();
|
||||
|
||||
assert!(
|
||||
matches!(error, Error::MalformedReply { .. }),
|
||||
"missing payload + missing return_value must be MalformedReply, got {error:?}"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn authenticate_user_falls_back_to_return_value_when_typed_payload_absent() {
|
||||
// CLI-41: an OK reply that carries only a return_value (legacy worker) must
|
||||
// resolve the user id from it, mirroring add_buffered_item's fallback.
|
||||
let state = Arc::new(FakeState::default());
|
||||
*state.invoke_override.lock().await = Some(InvokeOverride::CannedReply(Box::new(
|
||||
command_reply_fixture("authenticate-user.return-value-only.reply.json"),
|
||||
)));
|
||||
let endpoint = spawn_fake_gateway(state.clone()).await;
|
||||
let client = GatewayClient::connect(ClientOptions::new(endpoint))
|
||||
.await
|
||||
.unwrap();
|
||||
let session = client.session("session-fixture");
|
||||
|
||||
let user_id = session
|
||||
.authenticate_user(7, "verifier", "pw")
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
assert_eq!(
|
||||
user_id, 7,
|
||||
"user id must resolve from the int32 return_value"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn stream_alarms_emits_snapshot_then_complete_then_transition_in_order() {
|
||||
let state = Arc::new(FakeState::default());
|
||||
@@ -955,6 +1130,11 @@ enum InvokeOverride {
|
||||
/// `AuthenticateUser` rejected by MXAccess) so the client's
|
||||
/// `ensure_mxaccess_success` check is exercised on the typed helper path.
|
||||
MxAccessFailure,
|
||||
/// Reply with a caller-supplied canned [`MxCommandReply`]. Lets a test
|
||||
/// drive a helper with a shared behavior fixture (e.g. the
|
||||
/// echoed-credential / missing-payload / return-value-only
|
||||
/// authenticate-user replies). Boxed to keep the enum small.
|
||||
CannedReply(Box<MxCommandReply>),
|
||||
}
|
||||
|
||||
#[derive(Clone)]
|
||||
@@ -1057,6 +1237,7 @@ impl MxAccessGateway for FakeGateway {
|
||||
payload: None,
|
||||
..MxCommandReply::default()
|
||||
})),
|
||||
InvokeOverride::CannedReply(reply) => Ok(Response::new(*reply)),
|
||||
InvokeOverride::WriteOk => {
|
||||
// Extract and capture the WriteCommand payload so the test
|
||||
// can assert on server_handle, item_handle, user_id, and value.
|
||||
@@ -1443,15 +1624,48 @@ fn command_reply_fixture(file_name: &str) -> MxCommandReply {
|
||||
})
|
||||
.collect();
|
||||
|
||||
// The fixtures that exercise the return_value fallback path carry a typed
|
||||
// `returnValue` (VT_I4). Project it so a canned reply can drive the
|
||||
// helper's payload -> return_value -> MalformedReply precedence.
|
||||
let return_value = fixture.get("returnValue").and_then(|value| {
|
||||
value["int32Value"].as_i64().map(|int32| MxValue {
|
||||
data_type: MxDataType::Integer as i32,
|
||||
variant_type: value["variantType"].as_str().unwrap_or("VT_I4").to_owned(),
|
||||
kind: Some(Kind::Int32Value(int32 as i32)),
|
||||
..MxValue::default()
|
||||
})
|
||||
});
|
||||
|
||||
// Honor the fixture's real protocol status (code + message) so a canned
|
||||
// reply can drive the MXACCESS_FAILURE routing path, not just an OK
|
||||
// envelope. Falls back to an OK envelope when the fixture omits it.
|
||||
let protocol_status = fixture.get("protocolStatus").map_or_else(
|
||||
|| ok_status("command ok"),
|
||||
|status| {
|
||||
let code_name = status["code"].as_str().unwrap_or("PROTOCOL_STATUS_CODE_OK");
|
||||
ProtocolStatus {
|
||||
code: ProtocolStatusCode::from_str_name(code_name)
|
||||
.unwrap_or_else(|| panic!("unknown protocol status code {code_name}"))
|
||||
as i32,
|
||||
message: status["message"].as_str().unwrap_or_default().to_owned(),
|
||||
}
|
||||
},
|
||||
);
|
||||
|
||||
MxCommandReply {
|
||||
session_id: fixture["sessionId"].as_str().unwrap_or_default().to_owned(),
|
||||
correlation_id: fixture["correlationId"]
|
||||
.as_str()
|
||||
.unwrap_or_default()
|
||||
.to_owned(),
|
||||
protocol_status: Some(ok_status("command ok")),
|
||||
protocol_status: Some(protocol_status),
|
||||
hresult: fixture["hresult"].as_i64().map(|hresult| hresult as i32),
|
||||
statuses,
|
||||
diagnostic_message: fixture["diagnosticMessage"]
|
||||
.as_str()
|
||||
.unwrap_or_default()
|
||||
.to_owned(),
|
||||
return_value,
|
||||
..MxCommandReply::default()
|
||||
}
|
||||
}
|
||||
|
||||
@@ -70,6 +70,40 @@ The rules those fixtures lock in are:
|
||||
negative. Positive COM success codes such as `S_FALSE` pass, matching COM
|
||||
semantics.
|
||||
|
||||
### Malformed-Reply And Credential-Redaction Conformance
|
||||
|
||||
Three further command reply fixtures pin the id/handle-extraction and
|
||||
credential-redaction contracts for the credential-bearing helpers:
|
||||
|
||||
| Fixture | Reply | Expected behavior |
|
||||
|---|---|---|
|
||||
| `authenticate-user.echoed-credential.reply.json` | OK envelope, negative `hresult`, and the caller's credential echoed into `protocolStatus.message`, `statuses[0].diagnosticText`, and `diagnosticMessage` | the surfaced error redacts the exact secret from **both** the rendered message and the structured reply accessors (never leaks the verbatim value) |
|
||||
| `authenticate-user.echoed-credential-mxaccess-failure.reply.json` | the same echo, but coded `PROTOCOL_STATUS_CODE_MXACCESS_FAILURE` | identical redaction; confirms every client routes the MXAccess-failure protocol code to its MXAccess error type and scrubs it |
|
||||
| `authenticate-user.missing-payload.reply.json` | OK envelope, no `AuthenticateUser` payload, no `return_value` | a typed malformed-reply error, never a proto3 default `0` and never an NRE |
|
||||
| `authenticate-user.return-value-only.reply.json` | OK envelope, `return_value.int32_value = 7`, no typed payload | the id resolves to `7` via the legacy `return_value` compatibility path |
|
||||
|
||||
The rules those fixtures lock in are:
|
||||
|
||||
- **Malformed-reply extraction (CLI-41).** Every helper that extracts a scalar
|
||||
id/handle (`AuthenticateUser`, `ArchestrAUserToId`, `AddBufferedItem`, and the
|
||||
handle extractors) prefers the typed payload; when it is absent it falls back
|
||||
to `return_value` **only** when `return_value` is present with the expected
|
||||
int32 variant; when neither is present it raises a typed malformed-reply error.
|
||||
It never surfaces a proto3 default `0` and never throws a null-reference.
|
||||
- **Credential redaction (CLI-40).** The credential-bearing helpers
|
||||
(`AuthenticateUser`, `WriteSecured`/`WriteSecured2`) scrub the exact secret
|
||||
values they were called with from any surfaced error — both the rendered
|
||||
message text **and** the structured reply the error still exposes (a
|
||||
server-echoed credential lives in `protocolStatus.message` and
|
||||
`statuses[].diagnosticText`, which the error's raw-reply accessor would
|
||||
otherwise re-expose to a logger dumping structured fields). The redacted error
|
||||
therefore carries a scrubbed clone of the reply. This is defense-in-depth on
|
||||
top of the by-construction guarantee that exceptions carry reply-derived text,
|
||||
not the request. The marker is `<redacted>` in the Go, Rust, and Java clients
|
||||
and `[redacted]` in the Python client and the .NET CLI; each suite asserts that
|
||||
neither the surfaced message nor the exposed reply still contains the
|
||||
credential, and that the message contains the client's marker.
|
||||
|
||||
## Event Streams
|
||||
|
||||
Event stream fixtures live in
|
||||
@@ -100,6 +134,12 @@ behavior. A language helper may expose native booleans, integers, strings,
|
||||
arrays, and timestamps, but it must keep `rawDiagnostic`, raw data type fields,
|
||||
and raw byte payloads accessible when conversion is incomplete.
|
||||
|
||||
Each status case also carries an independent `wantSuccess` boolean alongside its
|
||||
`status` object. The success/failure conformance tests assert the helper's
|
||||
verdict against this fixture-declared expectation rather than recomputing it from
|
||||
`category` (the same formula under test), so a regression in the verdict rule
|
||||
cannot hide behind a self-consistent computation.
|
||||
|
||||
## Auth, Timeout, And Cancel Behavior
|
||||
|
||||
Authentication fixtures live in `clients/proto/fixtures/behavior/auth/`. They
|
||||
|
||||
@@ -126,11 +126,28 @@ rules across all five clients (see
|
||||
failing before a prior `AuthenticateUser` + `AdviseSupervisory` surfaces the
|
||||
native failure unchanged — the helper does not pre-validate or reorder it.
|
||||
|
||||
**Malformed-reply extraction:** the id/handle-returning helpers
|
||||
(`AuthenticateUser`, `ArchestrAUserToId`, `AddBufferedItem`, and the handle
|
||||
extractors) follow one contract across all five clients — prefer the typed
|
||||
payload; fall back to `return_value` only when it is present with the expected
|
||||
int32 variant; when neither is present, raise a typed malformed-reply error.
|
||||
They never surface a proto3 default `0` and never throw a null-reference (CLI-41).
|
||||
|
||||
**Credential handling:** `AuthenticateUser` credentials and `WriteSecured`
|
||||
secured payloads route through each client's secret-redaction seam so they never
|
||||
reach logs, exception text, or `ToString`/`Debug`/`Display` — the value is carried
|
||||
only on the wire. Each client's test suite asserts a distinctive credential is
|
||||
absent from any surfaced error.
|
||||
only on the wire. In addition to that by-construction guarantee (exceptions carry
|
||||
reply-derived text, not the request), every client scrubs the **exact** secret
|
||||
values it was called with from any surfaced error as defense-in-depth, so a
|
||||
gateway or MXAccess diagnostic that echoes a credential back cannot leak it
|
||||
(CLI-40). The scrub covers **both** the rendered message and the structured reply
|
||||
the error still exposes (`protocolStatus.message`, `statuses[].diagnosticText`,
|
||||
`diagnosticMessage`): the redacted error carries a scrubbed clone of the reply so
|
||||
a logger dumping the exception's structured fields cannot reintroduce the leak.
|
||||
This holds regardless of whether the reply is coded `OK` (with a failing HRESULT)
|
||||
or `MXACCESS_FAILURE` — every client routes both to its MXAccess error type. Each
|
||||
client's test suite asserts the distinctive credential is absent from both the
|
||||
surfaced message and the exposed reply, and that the redaction marker is present.
|
||||
|
||||
Shipped in all five clients (.NET / Go / Rust / Python / Java).
|
||||
|
||||
|
||||
Reference in New Issue
Block a user