fix(logging): redact command values on the shared ILogRedactor seam
ci / windows-x86 (push) Failing after 18s
ci / nightly-windev (push) Has been skipped
ci / java (push) Successful in 2m13s
ci / portable (push) Successful in 7m6s

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.
This commit is contained in:
Joseph Doherty
2026-07-27 17:01:04 -04:00
parent aecc50a14b
commit 47c0b646a9
2 changed files with 100 additions and 1 deletions
@@ -10,8 +10,15 @@ public sealed class GatewayLogRedactorSeam : ILogRedactor
{ {
private static readonly string[] IdentityKeys = ["ClientIdentity", "authorization", "Authorization"]; private static readonly string[] IdentityKeys = ["ClientIdentity", "authorization", "Authorization"];
/// <summary>Property name carrying the MXAccess command method, which gates value redaction.</summary>
private const string CommandMethodProperty = "CommandMethod";
/// <summary>Property name carrying a command payload value that may bear credentials.</summary>
private const string CommandValueProperty = "CommandValue";
/// <summary> /// <summary>
/// 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.
/// </summary> /// </summary>
/// <param name="properties">The log event property dictionary to redact in place.</param> /// <param name="properties">The log event property dictionary to redact in place.</param>
public void Redact(IDictionary<string, object?> properties) public void Redact(IDictionary<string, object?> properties)
@@ -24,5 +31,26 @@ public sealed class GatewayLogRedactorSeam : ILogRedactor
properties[key] = GatewayLogRedactor.RedactClientIdentity(s); properties[key] = GatewayLogRedactor.RedactClientIdentity(s);
} }
} }
RedactCommandValue(properties);
}
/// <summary>
/// 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.
/// </summary>
/// <param name="properties">The log event property dictionary to redact in place.</param>
private static void RedactCommandValue(IDictionary<string, object?> 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);
} }
} }
@@ -13,4 +13,75 @@ public sealed class GatewayLogRedactorSeamTests
redactor.Redact(props); redactor.Redact(props);
Assert.Equal("Bearer mxgw_operator01_[redacted]", props["ClientIdentity"]); Assert.Equal("Bearer mxgw_operator01_[redacted]", props["ClientIdentity"]);
} }
/// <summary>A command value carried by a credential-bearing command never reaches a sink in the clear.</summary>
[Fact]
public void Redact_MasksCommandValueForCredentialBearingCommand()
{
GatewayLogRedactorSeam redactor = new();
Dictionary<string, object?> props = new()
{
["CommandMethod"] = "AuthenticateUser",
["CommandValue"] = "operator01:hunter2"
};
redactor.Redact(props);
Assert.Equal(GatewayLogRedactor.RedactedValue, props["CommandValue"]);
}
/// <summary>
/// Value logging is off by default, so an ordinary command value is masked too — the seam
/// exposes no opt-in, matching <see cref="GatewayLogRedactor.RedactCommandValue"/>'s default.
/// </summary>
[Fact]
public void Redact_MasksCommandValueForOrdinaryCommand()
{
GatewayLogRedactorSeam redactor = new();
Dictionary<string, object?> props = new()
{
["CommandMethod"] = "Read",
["CommandValue"] = "plaintext-tag-value"
};
redactor.Redact(props);
Assert.Equal(GatewayLogRedactor.RedactedValue, props["CommandValue"]);
}
/// <summary>A command value with no accompanying method is still masked — an unknown method cannot be cleared.</summary>
[Fact]
public void Redact_MasksCommandValueWhenCommandMethodAbsent()
{
GatewayLogRedactorSeam redactor = new();
Dictionary<string, object?> props = new() { ["CommandValue"] = "plaintext-tag-value" };
redactor.Redact(props);
Assert.Equal(GatewayLogRedactor.RedactedValue, props["CommandValue"]);
}
/// <summary>A null command value stays null rather than becoming the redaction placeholder.</summary>
[Fact]
public void Redact_LeavesNullCommandValueNull()
{
GatewayLogRedactorSeam redactor = new();
Dictionary<string, object?> props = new() { ["CommandValue"] = null };
redactor.Redact(props);
Assert.Null(props["CommandValue"]);
}
/// <summary>An event carrying no command value is untouched — the seam must not invent the property.</summary>
[Fact]
public void Redact_DoesNotAddCommandValueWhenAbsent()
{
GatewayLogRedactorSeam redactor = new();
Dictionary<string, object?> props = new() { ["ClientIdentity"] = "anonymous" };
redactor.Redact(props);
Assert.False(props.ContainsKey("CommandValue"));
}
} }