fix(gateway): preserve raw client correlation id in denial audit DetailsJson + add wiring test (§1.2)

This commit is contained in:
Joseph Doherty
2026-06-15 09:56:24 -04:00
parent a59fc998e3
commit 55526d5e56
3 changed files with 49 additions and 5 deletions
@@ -121,8 +121,10 @@ public sealed class ConstraintEnforcer(
/// <param name="target">The target being accessed (tag address or handle).</param>
/// <param name="failure">The constraint failure details.</param>
/// <param name="correlationId">
/// The per-request client correlation id, if any. Persisted as the audit record's
/// <c>CorrelationId</c> when it parses as a GUID; a non-GUID value is dropped (left null).
/// The per-request client correlation id, if any. Persisted as the audit record's typed
/// <c>CorrelationId</c> when it parses as a GUID; a non-GUID value leaves that column null.
/// The raw string is always preserved in <c>DetailsJson["clientCorrelationId"]</c> so a
/// non-GUID id (e.g. from Rust/Python/Java clients) is never silently lost.
/// </param>
/// <param name="cancellationToken">Token to observe for cancellation.</param>
public async Task RecordDenialAsync(
@@ -153,6 +155,11 @@ public sealed class ConstraintEnforcer(
["message"] = failure.Message,
["commandKind"] = commandKind,
["target"] = target,
// Always preserve the raw client correlation id here so it is never silently
// lost: the typed CorrelationId column only retains GUID-parseable ids, but
// clients (Rust/Python/Java) commonly send non-GUID or empty trace ids. The
// raw id is a client trace id, not a secret, so storing it is fine.
["clientCorrelationId"] = correlationId ?? "",
}),
};
@@ -548,6 +548,33 @@ public sealed class MxAccessGatewayServiceConstraintTests
Assert.Equal("42", enforcer.RecordedDenials[0].Target);
}
/// <summary>
/// End-to-end wiring (M-2): the per-request <c>ClientCorrelationId</c> must propagate
/// all the way through <c>Invoke</c> -> <c>ApplyConstraintsAsync</c> -> the unary write
/// enforce helper -> <c>RecordDenialAsync</c>, so the recorded denial carries the exact
/// id the client sent (including non-GUID trace ids used by Rust/Python/Java clients).
/// </summary>
[Fact]
public async Task Invoke_Write_WithDeniedHandle_ThreadsClientCorrelationIdIntoRecordedDenial()
{
const string CorrelationId = "rust-client-Write-7";
PredicateConstraintEnforcer enforcer = new()
{
DenyWriteHandle = (serverHandle, itemHandle) => serverHandle == 7 && itemHandle == 42,
};
FakeSessionManager sessionManager = CreateSessionManagerWithSeed();
MxAccessGatewayService service = CreateService(sessionManager, enforcer);
MxCommandRequest request = CreateWriteRequest(serverHandle: 7, itemHandle: 42);
request.ClientCorrelationId = CorrelationId;
await Assert.ThrowsAsync<RpcException>(
async () => await service.Invoke(request, new TestServerCallContext()));
Assert.Single(enforcer.RecordedDenials);
Assert.Equal(CorrelationId, enforcer.RecordedDenials[0].CorrelationId);
}
/// <summary>
/// Unary <c>WriteSecured</c> against a denied handle takes the same enforce path
/// and rejects identically — proving the four-arm switch in
@@ -1,3 +1,4 @@
using System.Text.Json;
using ZB.MOM.WW.Audit;
using ZB.MOM.WW.MxGateway.Contracts.Proto.Galaxy;
using ZB.MOM.WW.MxGateway.Contracts.Proto;
@@ -102,9 +103,12 @@ public sealed class ConstraintEnforcerTests
Assert.Equal(correlationId, auditEvent.CorrelationId);
}
/// <summary>A denial with a non-GUID correlation id leaves the audit correlation id null.</summary>
/// <summary>
/// A denial with a non-GUID correlation id leaves the typed audit correlation id null but
/// still preserves the raw client correlation id in DetailsJson so it is not lost.
/// </summary>
[Fact]
public async Task RecordDenialAsync_WithNonGuidCorrelationId_LeavesCorrelationIdNull()
public async Task RecordDenialAsync_WithNonGuidCorrelationId_LeavesCorrelationIdNullButPreservesRawInDetails()
{
ConstraintEnforcer enforcer = CreateEnforcer(out FakeAuditWriter auditWriter);
@@ -113,11 +117,17 @@ public sealed class ConstraintEnforcerTests
"Read",
"Secret.Tag",
new ConstraintFailure("read_scope", "Tag is outside the API key read scope."),
"cli-xyz",
"rust-client-Write-7",
CancellationToken.None);
AuditEvent auditEvent = Assert.Single(auditWriter.Events);
Assert.Null(auditEvent.CorrelationId);
Assert.NotNull(auditEvent.DetailsJson);
Dictionary<string, string>? details =
JsonSerializer.Deserialize<Dictionary<string, string>>(auditEvent.DetailsJson);
Assert.NotNull(details);
Assert.Equal("rust-client-Write-7", details["clientCorrelationId"]);
}
/// <summary>A denial with no identity records the canonical "anonymous" actor.</summary>