dohertj2/lmxopcua
1
0
Fork 0
Code Issues 19 Pull Requests Actions Packages Projects Releases Wiki Activity

fix(mqtt): pin security-critical defaults with tests + redact password in ToString

Browse Source 
Code review of f22db5d8 (approved-with-findings): the existing round-trip
tests only exercised explicit JSON payloads, so nothing failed if UseTls /
AllowUntrustedServerCertificate or the §5.1 numeric defaults were flipped.
Adds a defaults-pinning test plus a partial-JSON test that omits the TLS
knobs entirely (the production case — an operator config that just doesn't
mention TLS must not silently land insecure).

Also overrides the record's PrintMembers so ToString() no longer prints
Password in plaintext (verified RED before the fix: the new test failed
with the raw password in the rendered string). OpcUaClientDriverOptions has
the same unredacted-ToString() shape but is out of scope for this task per
the coordinator's note; flagging as a possible follow-up.

Claude-Session: https://claude.ai/code/session_01GASWkNEi68FSCtvr6rLoEW
This commit is contained in:
Joseph Doherty
2026-07-24 14:01:01 -04:00
parent f22db5d801
commit a5a8710af5
2 changed files with 71 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files
tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/MqttDriverOptionsTests.cs
+34
View File
@@ -36,4 +36,38 @@ public sealed class MqttDriverOptionsTests
s.ShouldContain("\"Plain\"");
s.ShouldNotContain("\"mode\":0");
}
// Pins the plan's cross-cutting rule ("never ship anonymous/public-broker defaults") as an
// executable assertion — nothing else in this suite fails if UseTls / AllowUntrustedServerCertificate
// (or the §5.1 numeric defaults) get flipped by an unrelated edit.
[Fact]
public void Defaults_SecurityCriticalKnobs_MatchPlan()
{
var o = new MqttDriverOptions();
o.UseTls.ShouldBeTrue();
o.AllowUntrustedServerCertificate.ShouldBeFalse();
o.ConnectTimeoutSeconds.ShouldBe(15);
o.ReconnectMinBackoffSeconds.ShouldBe(1);
o.ReconnectMaxBackoffSeconds.ShouldBe(30);
}
// The production case: an operator-authored config blob that simply doesn't mention TLS must
// never silently deserialize into an insecure instance.
[Fact]
public void Deserialize_PartialJsonOmittingTlsKnobs_KeepsSecureDefaults()
{
const string json = """{ "host":"10.100.0.35","port":8883 }""";
var o = JsonSerializer.Deserialize<MqttDriverOptions>(json, J)!;
o.UseTls.ShouldBeTrue();
o.AllowUntrustedServerCertificate.ShouldBeFalse();
}
[Fact]
public void ToString_DoesNotLeakPassword()
{
var o = new MqttDriverOptions { Password = "hunter2-supersecret" };
var s = o.ToString();
s.ShouldNotContain("hunter2-supersecret");
s.ShouldContain("***");
}
}