From c36224d95653201f69c368eda5d2a4c15e5ca05c Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Fri, 9 Oct 2026 06:21:55 -0400 Subject: [PATCH] =?UTF-8?q?test(abcip):=20hw5=20review=20fixes=20=E2=80=94?= =?UTF-8?q?=20HardwareGate=20guard,=20(host,=20tag)=20allow-list,=20differ?= =?UTF-8?q?ential-run=20refusal=20(#592)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 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 --- docs/drivers/AbCip.md | 4 +- .../HardwareGate/HardwareGateGuardTests.cs | 70 +++++++++++++++++++ .../HardwareGate/StringWriteGateTests.cs | 37 +++++++--- 3 files changed, 102 insertions(+), 9 deletions(-) create mode 100644 tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests/HardwareGate/HardwareGateGuardTests.cs diff --git a/docs/drivers/AbCip.md b/docs/drivers/AbCip.md index 5c7b8576..23c68e35 100644 --- a/docs/drivers/AbCip.md +++ b/docs/drivers/AbCip.md @@ -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 diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests/HardwareGate/HardwareGateGuardTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests/HardwareGate/HardwareGateGuardTests.cs new file mode 100644 index 00000000..ec8c0506 --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests/HardwareGate/HardwareGateGuardTests.cs @@ -0,0 +1,70 @@ +using System.Runtime.CompilerServices; +using Shouldly; +using Xunit; + +namespace ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests.HardwareGate; + +/// +/// Offline guard for the hardware-gate folder — deliberately NOT env-gated, so it runs in every build. The folder sits +/// outside the read-only Differential guard because one test in it writes; this pins how far that goes: +/// only 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. +/// +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; +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests/HardwareGate/StringWriteGateTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests/HardwareGate/StringWriteGateTests.cs index e6eaa4c7..05822f66 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests/HardwareGate/StringWriteGateTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests/HardwareGate/StringWriteGateTests.cs @@ -15,24 +15,43 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests.HardwareGate; /// restored byte-exact). /// /// AB_HW_WRITE_GATEWAY — ab://host/1,0; absent ⇒ skip. -/// AB_HW_WRITE_STRING_TAG — must be one of (the brief's allow-list). +/// AB_HW_WRITE_STRING_TAG — with the gateway's host, must be a pair on . /// AB_HW_WRITE_APPROVED=1 — the explicit opt-in. /// AB_HW_WRITE_GAP_SECONDS — snapshot gap, default 30, at least 30. /// AB_HW_WRITE_DRYRUN=1 — stop after the read-only preflight (no write). /// -/// Never set by any other run; (the simulator opt-in) is deliberately not +/// Refused (skipped) while AB_DIFF_ENDPOINT is set (a read-only differential run). Never set by any other +/// run; (the simulator opt-in) is deliberately not /// what enables this. /// [Trait("Category", "LiveIntegration")] public sealed class StringWriteGateTests { - /// The only tags this gate may write: logic-, alias-, trend- and Galaxy-unreferenced (L5X census + raw grep). - private static readonly string[] Approved = + /// + /// 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. + /// pins this list. + /// + 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"), ]; + /// The host of an ab://host[:port]/path gateway, or null when it is not one. + 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; + } + + /// True only for an exact (host, tag) pair on . + 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))