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

test(abcip): hw5 review fixes — HardwareGate guard, (host, tag) allow-list, differential-run refusal (#592)

Browse Source 
- HardwareGateGuardTests: only StringWriteGateTests may name a write path in HardwareGate/; the
  allow-list is exactly the two Z-35162 (10.209.70.160) STRING tags; the opt-in, the (host, tag)
  check and the AB_DIFF_ENDPOINT refusal are present. Proven red by a WriteAsync mutation.
- StringWriteGateTests: allow-list entries are (host, tag); any other endpoint is refused; skips
  while AB_DIFF_ENDPOINT is set, like IntegrationWriteGate.
- AbCip.md #566: the close happened somewhere in the 16-60 s window; RST vs FIN not captured.

Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe
This commit is contained in:
Joseph Doherty
2026-10-09 06:21:55 -04:00
parent 1f983d610f
commit c36224d956
3 changed files with 102 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files
docs/drivers/AbCip.md
+3 -1
View File
@@ -180,7 +180,9 @@ The driver has two interchangeable wire transports, selected by one `DriverConfi
`BadCommunicationError` (transport `BadConnection`); a keep-alive that gets it recycles the connection
before any request fails. Every other status — tag-level errors, other `0x01` extended codes — is left to
the request. **Observed on a real CompactLogix (Z-35162, hw5, 2026-10-09): neither code.** A connection left
idle past its 16 s timeout (keep-alive disabled for the test) was closed at the TCP level; the next request
idle 60 s, past its 16 s timeout (keep-alive disabled for the test), was closed at the TCP level somewhere in
that 16–60 s window — the exact moment, and whether the controller sent RST or FIN, were not captured (no
packet capture; EPIPE on the first write only says the close was complete by then); the next request
failed on its write in 9 ms (`Broken pipe` → `BadConnection` → `BadCommunicationError`, no CIP or
encapsulation status) and the request after it reopened with a fresh Forward Open. That path needs no CIP
check — a faulted session always reopens on the next send — and is pinned offline by
tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests/HardwareGate/HardwareGateGuardTests.cs
+70
View File
@@ -0,0 +1,70 @@
using System.Runtime.CompilerServices;
using Shouldly;
using Xunit;
namespace ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests.HardwareGate;
/// <summary>
/// Offline guard for the hardware-gate folder — deliberately NOT env-gated, so it runs in every build. The folder sits
/// outside the read-only <c>Differential</c> guard because one test in it writes; this pins how far that goes:
/// only <see cref="StringWriteGateTests"/> may name a write path, its allow-list is exactly the two approved
/// (Z-35162, tag) pairs, and its explicit opt-in and differential-run refusal are present.
/// </summary>
public sealed class HardwareGateGuardTests
{
[Fact]
public void Only_the_string_write_gate_names_a_write_path()
{
// Built by concatenation so this file's own text never matches itself.
string[] forbidden =
[
"Write" + "Async", "Encode" + "Value", "WriteBit" + "InDInt", "Write" + "Tag", "Write" + "Request", "AbCip" + "Driver",
];
var sources = Directory.GetFiles(Path.GetDirectoryName(ThisFile())!, "*.cs");
sources.ShouldContain(f => Path.GetFileName(f) == nameof(StringWriteGateTests) + ".cs");
sources.ShouldContain(f => Path.GetFileName(f) == nameof(IdleConnectionEvidenceTests) + ".cs");
var hits = (from f in sources
where Path.GetFileName(f) != nameof(StringWriteGateTests) + ".cs"
let text = File.ReadAllText(f)
from token in forbidden
where text.Contains(token, StringComparison.Ordinal)
select $"{Path.GetFileName(f)} names {token}").ToList();
hits.ShouldBeEmpty();
// Positive control: the scan does see the one file that is allowed to write.
var writer = File.ReadAllText(sources.Single(f => Path.GetFileName(f) == nameof(StringWriteGateTests) + ".cs"));
writer.ShouldContain(forbidden[0]);
}
[Fact]
public void The_write_allow_list_is_exactly_the_two_approved_pairs_on_Z35162()
{
StringWriteGateTests.Approved.ShouldBe(
[
("10.209.70.160", "Program:ES03_TemperatureControl._StrTmp"),
("10.209.70.160", "Program:ES03_TemperatureControl._Prefix"),
]);
StringWriteGateTests.IsApproved("ab://10.209.70.160/1,0", "Program:ES03_TemperatureControl._StrTmp").ShouldBeTrue();
StringWriteGateTests.IsApproved("ab://10.209.70.160:44818/1,0", "Program:ES03_TemperatureControl._Prefix").ShouldBeTrue();
// Same tag on any other controller, another tag on Z-35162, or a malformed gateway: refused.
StringWriteGateTests.IsApproved("ab://10.209.70.161/1,0", "Program:ES03_TemperatureControl._StrTmp").ShouldBeFalse();
StringWriteGateTests.IsApproved("ab://10.100.0.35/1,0", "Program:ES03_TemperatureControl._StrTmp").ShouldBeFalse();
StringWriteGateTests.IsApproved("ab://10.209.70.160/1,0", "test2").ShouldBeFalse();
StringWriteGateTests.IsApproved("ab://10.209.70.160/1,0", "Program:UN01_Press._Prefix").ShouldBeFalse();
StringWriteGateTests.IsApproved("10.209.70.160", "Program:ES03_TemperatureControl._StrTmp").ShouldBeFalse();
}
[Fact]
public void The_string_write_gate_keeps_its_opt_in_and_refuses_during_a_differential_run()
{
var text = File.ReadAllText(Path.Combine(Path.GetDirectoryName(ThisFile())!, nameof(StringWriteGateTests) + ".cs"));
text.ShouldContain("""GetEnvironmentVariable("AB_HW_WRITE_APPROVED") != "1") Assert.Skip(""");
text.ShouldContain("GetEnvironmentVariable(IntegrationWriteGate.DifferentialEndpointVar) is { Length: > 0 })");
text.ShouldContain("if (!IsApproved(gateway, tagPath))");
IntegrationWriteGate.DifferentialEndpointVar.ShouldBe("AB_DIFF_ENDPOINT");
}
private static string ThisFile([CallerFilePath] string path = "") => path;
}
tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests/HardwareGate/StringWriteGateTests.cs
+29 -8
View File
@@ -15,24 +15,43 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests.HardwareGate;
/// restored byte-exact).
/// <list type="bullet">
/// <item><c>AB_HW_WRITE_GATEWAY</c> — <c>ab://host/1,0</c>; absent ⇒ skip.</item>
/// <item><c>AB_HW_WRITE_STRING_TAG</c> — must be one of <see cref="Approved"/> (the brief's allow-list).</item>
/// <item><c>AB_HW_WRITE_STRING_TAG</c> — with the gateway's host, must be a pair on <see cref="Approved"/>.</item>
/// <item><c>AB_HW_WRITE_APPROVED=1</c> — the explicit opt-in.</item>
/// <item><c>AB_HW_WRITE_GAP_SECONDS</c> — snapshot gap, default 30, at least 30.</item>
/// <item><c>AB_HW_WRITE_DRYRUN=1</c> — stop after the read-only preflight (no write).</item>
/// </list>
/// Never set by any other run; <see cref="IntegrationWriteGate"/> (the simulator opt-in) is deliberately not
/// Refused (skipped) while <c>AB_DIFF_ENDPOINT</c> is set (a read-only differential run). Never set by any other
/// run; <see cref="IntegrationWriteGate"/> (the simulator opt-in) is deliberately not
/// what enables this.
/// </summary>
[Trait("Category", "LiveIntegration")]
public sealed class StringWriteGateTests
{
/// <summary>The only tags this gate may write: logic-, alias-, trend- and Galaxy-unreferenced (L5X census + raw grep).</summary>
private static readonly string[] Approved =
/// <summary>
/// The only (controller, tag) pairs this gate may write: Z-35162's logic-, alias-, trend- and Galaxy-unreferenced
/// STRINGs (L5X census + raw grep). Pinned to the host, so the same tag name on another controller is refused.
/// <see cref="HardwareGateGuardTests"/> pins this list.
/// </summary>
internal static readonly (string Host, string Tag)[] Approved =
[
"Program:ES03_TemperatureControl._StrTmp",
"Program:ES03_TemperatureControl._Prefix",
("10.209.70.160", "Program:ES03_TemperatureControl._StrTmp"),
("10.209.70.160", "Program:ES03_TemperatureControl._Prefix"),
];
/// <summary>The host of an <c>ab://host[:port]/path</c> gateway, or null when it is not one.</summary>
internal static string? GatewayHost(string gateway)
{
if (!gateway.StartsWith("ab://", StringComparison.OrdinalIgnoreCase)) return null;
var authority = gateway[5..].Split('/', 2)[0];
var host = authority.Split(':', 2)[0];
return host.Length == 0 ? null : host;
}
/// <summary>True only for an exact (host, tag) pair on <see cref="Approved"/>.</summary>
internal static bool IsApproved(string gateway, string tag) =>
GatewayHost(gateway) is { } host
&& Approved.Any(a => string.Equals(a.Host, host, StringComparison.Ordinal) && string.Equals(a.Tag, tag, StringComparison.Ordinal));
private const string TestValue = "OTOPCUA-TEST";
private const int BuiltInStringBytes = 88;
private const int StringCapacity = 82;
@@ -42,10 +61,12 @@ public sealed class StringWriteGateTests
{
var gateway = Environment.GetEnvironmentVariable("AB_HW_WRITE_GATEWAY")?.Trim();
if (string.IsNullOrEmpty(gateway)) Assert.Skip("AB_HW_WRITE_GATEWAY not set.");
if (Environment.GetEnvironmentVariable(IntegrationWriteGate.DifferentialEndpointVar) is { Length: > 0 })
Assert.Skip($"Write gate refused: {IntegrationWriteGate.DifferentialEndpointVar} is set, so a read-only differential run is in progress.");
if (Environment.GetEnvironmentVariable("AB_HW_WRITE_APPROVED") != "1") Assert.Skip("AB_HW_WRITE_APPROVED=1 not set.");
var tagPath = Environment.GetEnvironmentVariable("AB_HW_WRITE_STRING_TAG")?.Trim() ?? "";
if (!Approved.Contains(tagPath, StringComparer.Ordinal))
throw new InvalidOperationException($"AB_HW_WRITE_STRING_TAG='{tagPath}' is not on the approved list.");
if (!IsApproved(gateway, tagPath))
throw new InvalidOperationException($"('{gateway}', '{tagPath}') is not an approved (controller, tag) pair.");
var gap = 30;
if (Environment.GetEnvironmentVariable("AB_HW_WRITE_GAP_SECONDS") is { Length: > 0 } rawGap
&& (!int.TryParse(rawGap, out gap) || gap < 30 || gap > 300))