From 47c0b646a923fc1efeb9558a387aeac2962865be Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 27 Jul 2026 17:01:04 -0400 Subject: [PATCH] fix(logging): redact command values on the shared ILogRedactor seam MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GatewayLogRedactor.RedactCommandValue had no production caller. Its only one was GatewayLogRedactorAdapter on the abandoned feat/adopt-zb-telemetry-serilog branch; when that work was re-implemented on main as GatewayLogRedactorSeam the identity half was carried over and the command-value half was not. Four unit tests kept the policy green, so it read as wired while masking nothing. No live leak today — no log statement currently emits a CommandValue property — but the next one to do so would have written credential-bearing MXAccess payloads (AuthenticateUser, WriteSecured, WriteSecured2) to every sink in the clear, with passing tests suggesting otherwise. Ports the missing half onto the seam: a non-null CommandValue is masked via the existing policy, gated on CommandMethod. Value logging stays off — the seam exposes no opt-in — so ordinary values are masked too, matching RedactCommandValue's default. A null value stays null rather than becoming the placeholder, and the property is never invented when absent. Five tests added, three of which were red first on the leak itself (operator01:hunter2 reaching the assertion unmasked). The other two pin the null and absent guards. Identity redaction is untouched. Full NonWindows suite: 785 pass, 45 pre-existing macOS NamedPipe-harness failures unchanged from baseline (verified by stashing this change). Build 0 warnings. --- .../Diagnostics/GatewayLogRedactorSeam.cs | 30 +++++++- .../GatewayLogRedactorSeamTests.cs | 71 +++++++++++++++++++ 2 files changed, 100 insertions(+), 1 deletion(-) diff --git a/src/ZB.MOM.WW.MxGateway.Server/Diagnostics/GatewayLogRedactorSeam.cs b/src/ZB.MOM.WW.MxGateway.Server/Diagnostics/GatewayLogRedactorSeam.cs index 0beab89..7976a61 100644 --- a/src/ZB.MOM.WW.MxGateway.Server/Diagnostics/GatewayLogRedactorSeam.cs +++ b/src/ZB.MOM.WW.MxGateway.Server/Diagnostics/GatewayLogRedactorSeam.cs @@ -10,8 +10,15 @@ public sealed class GatewayLogRedactorSeam : ILogRedactor { private static readonly string[] IdentityKeys = ["ClientIdentity", "authorization", "Authorization"]; + /// Property name carrying the MXAccess command method, which gates value redaction. + private const string CommandMethodProperty = "CommandMethod"; + + /// Property name carrying a command payload value that may bear credentials. + private const string CommandValueProperty = "CommandValue"; + /// - /// Masks API-key/credential material in known identity-bearing log properties. + /// Masks API-key/credential material in known identity-bearing log properties, and any + /// command payload value. /// /// The log event property dictionary to redact in place. public void Redact(IDictionary properties) @@ -24,5 +31,26 @@ public sealed class GatewayLogRedactorSeam : ILogRedactor properties[key] = GatewayLogRedactor.RedactClientIdentity(s); } } + + RedactCommandValue(properties); + } + + /// + /// Masks a command payload value. Value logging is off here — the seam exposes no opt-in — so + /// every non-null value is masked, credential-bearing command or not. + /// + /// The log event property dictionary to redact in place. + private static void RedactCommandValue(IDictionary properties) + { + if (!properties.TryGetValue(CommandValueProperty, out object? value) || value is null) + { + return; + } + + string? commandMethod = properties.TryGetValue(CommandMethodProperty, out object? method) + ? method as string + : null; + + properties[CommandValueProperty] = GatewayLogRedactor.RedactCommandValue(commandMethod, value); } } diff --git a/src/ZB.MOM.WW.MxGateway.Tests/Diagnostics/GatewayLogRedactorSeamTests.cs b/src/ZB.MOM.WW.MxGateway.Tests/Diagnostics/GatewayLogRedactorSeamTests.cs index 16f52c0..31e4071 100644 --- a/src/ZB.MOM.WW.MxGateway.Tests/Diagnostics/GatewayLogRedactorSeamTests.cs +++ b/src/ZB.MOM.WW.MxGateway.Tests/Diagnostics/GatewayLogRedactorSeamTests.cs @@ -13,4 +13,75 @@ public sealed class GatewayLogRedactorSeamTests redactor.Redact(props); Assert.Equal("Bearer mxgw_operator01_[redacted]", props["ClientIdentity"]); } + + /// A command value carried by a credential-bearing command never reaches a sink in the clear. + [Fact] + public void Redact_MasksCommandValueForCredentialBearingCommand() + { + GatewayLogRedactorSeam redactor = new(); + Dictionary props = new() + { + ["CommandMethod"] = "AuthenticateUser", + ["CommandValue"] = "operator01:hunter2" + }; + + redactor.Redact(props); + + Assert.Equal(GatewayLogRedactor.RedactedValue, props["CommandValue"]); + } + + /// + /// Value logging is off by default, so an ordinary command value is masked too — the seam + /// exposes no opt-in, matching 's default. + /// + [Fact] + public void Redact_MasksCommandValueForOrdinaryCommand() + { + GatewayLogRedactorSeam redactor = new(); + Dictionary props = new() + { + ["CommandMethod"] = "Read", + ["CommandValue"] = "plaintext-tag-value" + }; + + redactor.Redact(props); + + Assert.Equal(GatewayLogRedactor.RedactedValue, props["CommandValue"]); + } + + /// A command value with no accompanying method is still masked — an unknown method cannot be cleared. + [Fact] + public void Redact_MasksCommandValueWhenCommandMethodAbsent() + { + GatewayLogRedactorSeam redactor = new(); + Dictionary props = new() { ["CommandValue"] = "plaintext-tag-value" }; + + redactor.Redact(props); + + Assert.Equal(GatewayLogRedactor.RedactedValue, props["CommandValue"]); + } + + /// A null command value stays null rather than becoming the redaction placeholder. + [Fact] + public void Redact_LeavesNullCommandValueNull() + { + GatewayLogRedactorSeam redactor = new(); + Dictionary props = new() { ["CommandValue"] = null }; + + redactor.Redact(props); + + Assert.Null(props["CommandValue"]); + } + + /// An event carrying no command value is untouched — the seam must not invent the property. + [Fact] + public void Redact_DoesNotAddCommandValueWhenAbsent() + { + GatewayLogRedactorSeam redactor = new(); + Dictionary props = new() { ["ClientIdentity"] = "anonymous" }; + + redactor.Redact(props); + + Assert.False(props.ContainsKey("CommandValue")); + } }