From 89d1076dcb4b5b58473076aa67707f0edace7920 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Otto=20Boly=C3=B3s?= Date: Wed, 22 Jul 2026 13:13:41 +0200 Subject: [PATCH 1/7] test(common-tests): pin AgentUuid RFC 4122 contract on both resolution paths (RED) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds AgentUuidValidationTests + updates the existing AgentUuidConfigOverrideTests / AgentUuidLongitudinalInvariantsTests fixtures to pin the contract that every source of the Agent meta-device UUID (operator-supplied override AND persisted agent.information.json state) MUST parse as an RFC 4122 UUID before it reaches the wire. New coverage in AgentUuidValidationTests: * DeterministicAgentUuid.TryValidate low-level rejection (null, empty, whitespace, unparseable) and acceptance / normalization (canonical D-form unchanged; braced B, parenthesized P, bare-hex N normalized to D). * Malformed operator override on a fresh boot falls through to derived. * Malformed operator override with a valid persisted UUID preserves the persisted UUID. * Malformed persisted state with no override falls through to derived (Path 2 hardening — the missing symmetric guard). * Valid non-canonical operator override is normalized to hyphenated. * Persisted UUID is adopted on the second boot when no override is set. * First boot persists the derived UUID to agent.information.json so subsequent boots hit Path 2. Fixture updates: * AgentUuidConfigOverrideTests + AgentUuidLongitudinalInvariantsTests now route their boot simulation through AgentUuidResolver.Resolve (the shared production helper) instead of inline replay, so the tests cannot silently drift from StartAgent semantics. * Fixture literals converted from non-UUID strings (fixture-stable-uuid-A, from-config-uuid, etc.) to canonical RFC 4122 UUIDs so production validation would accept them (previously these fixtures passed only because their inline replay omitted TryValidate — fake-green). * Every fixture marked [NonParallelizable] and each SetUp sweeps orphan .bak.* / .valbak.* / .longinv.bak.* files from a prior crashed test run so successive TearDowns cannot restore stale state. This commit is compile-error RED — the tests reference AgentUuidResolver.Resolve and the AmE parameter name normalized which land in the following fix commit. The compile error proves the API absence, per the project's behavioral-RED default. --- .../Agents/AgentUuidConfigOverrideTests.cs | 89 ++-- .../AgentUuidLongitudinalInvariantsTests.cs | 111 ++--- .../Agents/AgentUuidValidationTests.cs | 379 ++++++++++++++++++ 3 files changed, 499 insertions(+), 80 deletions(-) create mode 100644 tests/MTConnect.NET-Common-Tests/Agents/AgentUuidValidationTests.cs diff --git a/tests/MTConnect.NET-Common-Tests/Agents/AgentUuidConfigOverrideTests.cs b/tests/MTConnect.NET-Common-Tests/Agents/AgentUuidConfigOverrideTests.cs index f46444b56..b8951378b 100644 --- a/tests/MTConnect.NET-Common-Tests/Agents/AgentUuidConfigOverrideTests.cs +++ b/tests/MTConnect.NET-Common-Tests/Agents/AgentUuidConfigOverrideTests.cs @@ -27,6 +27,7 @@ namespace MTConnect.Tests.Common.Agents /// Mirrors cppagent's AgentDeviceUUID configuration knob. /// [TestFixture] + [NonParallelizable] public class AgentUuidConfigOverrideTests { private string? _stateFilePath; @@ -37,6 +38,18 @@ public class AgentUuidConfigOverrideTests public void SetUp() { _stateFilePath = Path.Combine(AppDomain.CurrentDomain.BaseDirectory, MTConnectAgentInformation.Filename); + _backupStateFile = null; + + // Sweep orphan .bak.* files from a prior crashed test run so + // successive TearDowns cannot restore stale state. + var directory = Path.GetDirectoryName(_stateFilePath); + if (directory != null && Directory.Exists(directory)) + { + foreach (var stale in Directory.EnumerateFiles(directory, MTConnectAgentInformation.Filename + ".bak.*")) + { + try { File.Delete(stale); } catch { /* best-effort */ } + } + } // Back up any pre-existing state file so we do not perturb other // tests or the developer environment. @@ -65,38 +78,30 @@ public void TearDown() /// /// Test (a) — pre-condition: no agent.information.json on disk. - /// Setting configuration.AgentUuid pins the agent UUID to that - /// exact value (overriding the Guid.NewGuid() in + /// Setting configuration.AgentUuid to a valid RFC 4122 UUID + /// pins the agent UUID to that exact value (overriding the + /// Guid.NewGuid() in /// 's parameterless ctor). /// [Test] public void AgentUuid_set_in_config_flows_through_to_Agent_uuid() { - const string PinnedUuid = "fixture-stable-uuid-001"; + const string PinnedUuid = "11111111-1111-4111-8111-111111111111"; var configuration = new AgentApplicationConfiguration { AgentUuid = PinnedUuid, }; - // Mirror the exact StartAgent threading slice under test: - // var agentInformation = MTConnectAgentInformation.Read(); - // if (agentInformation == null) agentInformation = new MTConnectAgentInformation(); - // if (!string.IsNullOrEmpty(configuration.AgentUuid)) - // agentInformation.Uuid = configuration.AgentUuid; - var agentInformation = MTConnectAgentInformation.Read(); - if (agentInformation == null) - { - agentInformation = new MTConnectAgentInformation(); - } - if (!string.IsNullOrEmpty(configuration.AgentUuid)) - { - agentInformation.Uuid = configuration.AgentUuid; - } + // Route through the production resolver so the test exercises the + // same code path StartAgent uses (see + // agent/MTConnect.NET-Applications-Agents/MTConnectAgentApplication.cs + // RunAgent — resolves via AgentUuidResolver.Resolve). + var agentInformation = ResolveViaProduction(configuration); agentInformation.Save(); // The override must hold both in-memory and after the file - // round-trip that StartAgent performs at line 393. + // round-trip that StartAgent performs. Assert.That(agentInformation.Uuid, Is.EqualTo(PinnedUuid)); var reloaded = MTConnectAgentInformation.Read(); @@ -106,15 +111,16 @@ public void AgentUuid_set_in_config_flows_through_to_Agent_uuid() /// /// Test (b) — pre-condition: agent.information.json already - /// stores a different UUID. The config-level AgentUuid wins. + /// stores a different (valid) UUID. The config-level AgentUuid + /// wins. /// [Test] public void AgentUuid_set_in_config_takes_precedence_over_state_file() { - const string FromStateFileUuid = "from-state-file-uuid"; - const string FromConfigUuid = "from-config-uuid"; + const string FromStateFileUuid = "22222222-2222-4222-8222-222222222222"; + const string FromConfigUuid = "33333333-3333-4333-8333-333333333333"; - // Pre-write the state file with a stale UUID. + // Pre-write the state file with a stale (but valid) UUID. var preexisting = new MTConnectAgentInformation(FromStateFileUuid); preexisting.Save(); @@ -123,15 +129,12 @@ public void AgentUuid_set_in_config_takes_precedence_over_state_file() AgentUuid = FromConfigUuid, }; - var agentInformation = MTConnectAgentInformation.Read(); - Assert.That(agentInformation, Is.Not.Null); - Assert.That(agentInformation!.Uuid, Is.EqualTo(FromStateFileUuid), + var initial = MTConnectAgentInformation.Read(); + Assert.That(initial, Is.Not.Null); + Assert.That(initial!.Uuid, Is.EqualTo(FromStateFileUuid), "Pre-condition: the state file should be read first."); - if (!string.IsNullOrEmpty(configuration.AgentUuid)) - { - agentInformation.Uuid = configuration.AgentUuid; - } + var agentInformation = ResolveViaProduction(configuration); agentInformation.Save(); Assert.That(agentInformation.Uuid, Is.EqualTo(FromConfigUuid)); @@ -157,12 +160,14 @@ public void AgentUuid_set_in_config_takes_precedence_over_state_file() [Test] public void AgentUuid_is_exposed_on_interface_with_camelCase_wire_name() { + const string InterfaceProbe = "44444444-4444-4444-8444-444444444444"; + IAgentApplicationConfiguration configuration = new AgentApplicationConfiguration { - AgentUuid = "interface-surface-test", + AgentUuid = InterfaceProbe, }; - Assert.That(configuration.AgentUuid, Is.EqualTo("interface-surface-test")); + Assert.That(configuration.AgentUuid, Is.EqualTo(InterfaceProbe)); var property = typeof(AgentApplicationConfiguration).GetProperty( nameof(AgentApplicationConfiguration.AgentUuid), @@ -178,5 +183,27 @@ public void AgentUuid_is_exposed_on_interface_with_camelCase_wire_name() "AgentUuid must carry [JsonPropertyName(...)] to match the other config fields."); Assert.That(jsonNameAttribute!.Name, Is.EqualTo("agentUuid")); } + + /// + /// Replays the RunAgent UUID resolution slice via the shared + /// production helper so this fixture cannot silently drift from + /// StartAgent semantics. Returns the populated + /// ready for + /// . + /// + private static MTConnectAgentInformation ResolveViaProduction(AgentApplicationConfiguration configuration) + { + var existing = MTConnectAgentInformation.Read(); + var freshlyConstructed = existing == null; + var info = existing ?? new MTConnectAgentInformation(); + + info.Uuid = AgentUuidResolver.Resolve( + operatorSuppliedUuid: configuration.AgentUuid, + persistedUuid: freshlyConstructed ? null : info.Uuid, + agentName: configuration.ServiceName, + hostname: Environment.MachineName); + + return info; + } } } diff --git a/tests/MTConnect.NET-Common-Tests/Agents/AgentUuidLongitudinalInvariantsTests.cs b/tests/MTConnect.NET-Common-Tests/Agents/AgentUuidLongitudinalInvariantsTests.cs index dfc745fdf..21369e56d 100644 --- a/tests/MTConnect.NET-Common-Tests/Agents/AgentUuidLongitudinalInvariantsTests.cs +++ b/tests/MTConnect.NET-Common-Tests/Agents/AgentUuidLongitudinalInvariantsTests.cs @@ -27,16 +27,28 @@ namespace MTConnect.Tests.Common.Agents /// this file pins the longitudinal invariant. /// [TestFixture] + [NonParallelizable] public class AgentUuidLongitudinalInvariantsTests { private string? _stateFilePath; private string? _backupStateFile; - /// Sets up the fixture before each test. + /// Sets up the fixture before all tests in the class. [OneTimeSetUp] public void OneTimeSetUp() { _stateFilePath = Path.Combine(AppDomain.CurrentDomain.BaseDirectory, MTConnectAgentInformation.Filename); + _backupStateFile = null; + + // Sweep orphan .longinv.bak.* files from a prior crashed test run. + var directory = Path.GetDirectoryName(_stateFilePath); + if (directory != null && Directory.Exists(directory)) + { + foreach (var stale in Directory.EnumerateFiles(directory, MTConnectAgentInformation.Filename + ".longinv.bak.*")) + { + try { File.Delete(stale); } catch { /* best-effort */ } + } + } // Back up any pre-existing state file so we do not perturb other // tests or the developer environment. @@ -81,33 +93,30 @@ public void OneTimeTearDown() /// call to MTConnectAgentApplication.StartAgent, followed by the /// broker's post-device-add persist. /// - /// Production sources replayed (verify against live code if either drifts): - /// - /// - /// agent/MTConnect.NET-Applications-Agents/MTConnectAgentApplication.cs - /// lines 351–404 — Read, AgentUuid override, InstanceId zeroing, Save. - /// - /// - /// libraries/MTConnect.NET-Common/Agents/MTConnectAgent.cs - /// lines 207–234 — broker ctor: _instanceId = instanceId > 0 ? instanceId : CreateInstanceId() - /// where CreateInstanceId() returns (ulong)(UnixDateTime.Now / 1000 / 10000) - /// (line 2351–2353), and lines 2321–2345 — UpdateAgentInformation timer-driven - /// persist after device-add. - /// - /// + /// The UUID resolution slice routes through + /// — the same call production + /// makes — so this fixture cannot silently drift from StartAgent + /// semantics (branch order, guard shape, validation). + /// + /// InstanceId handling remains inline because it has no shared + /// helper: it depends on configuration.Durable + + /// durableBufferLoadSucceeds and mirrors the broker ctor's + /// _instanceId = instanceId > 0 ? instanceId : CreateInstanceId() + /// contract (libraries/MTConnect.NET-Common/Agents/MTConnectAgent.cs). /// private static (string uuid, ulong instanceId) SimulateBoot( AgentApplicationConfiguration configuration, bool durableBufferLoadSucceeds) { - // Mirrors MTConnectAgentApplication.StartAgent lines 351–404: - var info = MTConnectAgentInformation.Read(); - if (info == null) info = new MTConnectAgentInformation(); + var existing = MTConnectAgentInformation.Read(); + var freshlyConstructed = existing == null; + var info = existing ?? new MTConnectAgentInformation(); - if (!string.IsNullOrEmpty(configuration.AgentUuid)) - { - info.Uuid = configuration.AgentUuid; - } + info.Uuid = AgentUuidResolver.Resolve( + operatorSuppliedUuid: configuration.AgentUuid, + persistedUuid: freshlyConstructed ? null : info.Uuid, + agentName: configuration.ServiceName, + hostname: Environment.MachineName); var initializeDataItems = !durableBufferLoadSucceeds; if (!configuration.Durable || initializeDataItems) @@ -117,17 +126,16 @@ private static (string uuid, ulong instanceId) SimulateBoot( info.Save(); - // Mirrors MTConnectAgent ctor (MTConnectAgent.cs lines 207–234): + // Mirrors MTConnectAgent ctor: // _instanceId = instanceId > 0 ? instanceId : CreateInstanceId(); - // CreateInstanceId() returns (ulong)(UnixDateTime.Now / 1000 / 10000) — Unix epoch seconds. + // CreateInstanceId() = (ulong)(UnixDateTime.Now / 1000 / 10000) — Unix epoch seconds. var brokerInstanceId = info.InstanceId > 0 ? info.InstanceId : (ulong)(UnixDateTime.Now / 1000 / 10000); - // Mirrors MTConnectAgent.UpdateAgentInformation (MTConnectAgent.cs lines 2321–2345): - // The broker writes its chosen _instanceId back to the file via a timer-driven - // persist once a device is added. Simulate that here so the next boot's Read() - // sees the broker's resolved InstanceId, not the zeroed value from the Save() above. + // Broker writes _instanceId back via UpdateAgentInformation once a + // device is added; simulate that so the next boot's Read() sees + // the broker's resolved InstanceId, not the zeroed value. info.InstanceId = brokerInstanceId; info.Save(); @@ -143,9 +151,11 @@ private static (string uuid, ulong instanceId) SimulateBoot( [Test] public void Uuid_pinned_via_config_survives_two_non_durable_boots() { + const string PinnedUuid = "aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa"; + var configuration = new AgentApplicationConfiguration { - AgentUuid = "fixture-stable-uuid-A", + AgentUuid = PinnedUuid, Durable = false, }; @@ -156,9 +166,9 @@ public void Uuid_pinned_via_config_survives_two_non_durable_boots() var (uuid2, instanceId2) = SimulateBoot(configuration, durableBufferLoadSucceeds: false); - Assert.That(uuid1, Is.EqualTo("fixture-stable-uuid-A"), + Assert.That(uuid1, Is.EqualTo(PinnedUuid), "Boot 1: config-level AgentUuid must be applied."); - Assert.That(uuid2, Is.EqualTo("fixture-stable-uuid-A"), + Assert.That(uuid2, Is.EqualTo(PinnedUuid), "Boot 2: config-level AgentUuid must survive a non-durable restart."); Assert.That(instanceId1, Is.Not.EqualTo(instanceId2), "Non-durable buffer means the InstanceId resets each boot — the UUIDs are equal but InstanceIds differ."); @@ -178,9 +188,11 @@ public void Uuid_pinned_via_config_survives_two_non_durable_boots() [Test] public void Uuid_pinned_via_config_survives_durable_boot_with_buffer_load_success() { + const string PinnedUuid = "bbbbbbbb-bbbb-4bbb-8bbb-bbbbbbbbbbbb"; + var configuration = new AgentApplicationConfiguration { - AgentUuid = "fixture-stable-uuid-B", + AgentUuid = PinnedUuid, Durable = true, }; @@ -192,28 +204,26 @@ public void Uuid_pinned_via_config_survives_durable_boot_with_buffer_load_succes // Boot 2: warm restart — durable buffer loaded successfully. var (uuid2, instanceId2) = SimulateBoot(configuration, durableBufferLoadSucceeds: true); - Assert.That(uuid1, Is.EqualTo("fixture-stable-uuid-B"), + Assert.That(uuid1, Is.EqualTo(PinnedUuid), "Boot 1: config-level AgentUuid must be applied."); - Assert.That(uuid2, Is.EqualTo("fixture-stable-uuid-B"), + Assert.That(uuid2, Is.EqualTo(PinnedUuid), "Boot 2: config-level AgentUuid must survive a durable restart."); Assert.That(instanceId1, Is.EqualTo(instanceId2), "Durable buffer load success means InstanceId is preserved across boots (spec requirement)."); } /// - /// Documents the pre-fix bug from the consumer's perspective: - /// when AgentApplicationConfiguration.AgentUuid is null - /// and no state file persists across boots (e.g., an ephemeral container), - /// both UUID and InstanceId regenerate on every boot. - /// - /// This is not a regression check on the new feature — it is a - /// regression check on the bug itself still being a bug when the knob - /// is absent. Deleting the state file between boots simulates the - /// "no persistent storage" scenario that the new config knob lets - /// consumers escape. + /// Post-fix longitudinal invariant: when + /// AgentApplicationConfiguration.AgentUuid is + /// and no state file persists across boots (e.g. an ephemeral container), + /// the meta-device UUID is nevertheless stable across boots because + /// Path 3 derives it + /// deterministically from (agentName ?? hostname, hostname, port: 0). + /// The InstanceId still resets each boot because the durable + /// buffer did not load (spec-correct behaviour for Header.instanceId). /// [Test] - public void Uuid_not_pinned_and_no_state_file_regenerates_per_boot() + public void Uuid_not_pinned_and_no_state_file_is_stable_via_deterministic_derivation() { var configuration = new AgentApplicationConfiguration { @@ -234,11 +244,14 @@ public void Uuid_not_pinned_and_no_state_file_regenerates_per_boot() var (uuid2, instanceId2) = SimulateBoot(configuration, durableBufferLoadSucceeds: false); - Assert.That(uuid1, Is.Not.EqualTo(uuid2), - "No AgentUuid override + no state file = a fresh Guid is generated every boot. " + - "This is the pre-fix problem the new knob lets consumers avoid."); + Assert.That(uuid1, Is.EqualTo(uuid2), + "No override + no state file, but Path 3 derives deterministically " + + "from (agentName ?? hostname, hostname, port) — the meta-device UUID is stable " + + "across boots. This is the whole point of the AgentUuidResolver + " + + "DeterministicAgentUuid.Derive stack introduced by #168."); Assert.That(instanceId1, Is.Not.EqualTo(instanceId2), - "No state file = InstanceId is also regenerated each boot."); + "No state file = InstanceId is still regenerated each boot " + + "(Header.instanceId is per-boot by spec; separate concern from UUID stability)."); } } } diff --git a/tests/MTConnect.NET-Common-Tests/Agents/AgentUuidValidationTests.cs b/tests/MTConnect.NET-Common-Tests/Agents/AgentUuidValidationTests.cs new file mode 100644 index 000000000..c404831df --- /dev/null +++ b/tests/MTConnect.NET-Common-Tests/Agents/AgentUuidValidationTests.cs @@ -0,0 +1,379 @@ +// Copyright (c) 2026 TrakHound Inc., All Rights Reserved. +// TrakHound Inc. licenses this file to you under the MIT license. + +using System; +using System.IO; +using MTConnect.Agents; +using MTConnect.Configurations; +using NUnit.Framework; + +namespace MTConnect.Tests.Common.Agents +{ + /// + /// Pins the contract that + /// gates every source of the Agent meta-device UUID (operator-supplied + /// override AND persisted agent.information.json state) so that + /// malformed values — anything that does not parse as an RFC 4122 UUID — + /// are rejected on the way in and the resolution falls through to the + /// next path (persisted → derived). + /// + /// + /// Silently forwarding a non-UUID string violates MTConnect Part 1, which + /// types the uuid attribute as the UUID DataType (RFC 4122 + /// enumerated string token). Any downstream XSD-validating consumer + /// (cppagent parity, MQTT/JSON-cppagent transport) rejects the resulting + /// wire content on typed enum/decimal DataItems. + /// + /// + /// + /// Fixture drives directly — the + /// same method MTConnectAgentApplication.StartAgent calls — so + /// production and tests cannot silently diverge on branch order, guard + /// semantics, or normalisation output. + /// + /// + [TestFixture] + [NonParallelizable] + public class AgentUuidValidationTests + { + private string _stateFilePath = null!; + private string? _backupStateFile; + + /// Sets up the fixture before each test. + [SetUp] + public void SetUp() + { + _stateFilePath = Path.Combine(AppDomain.CurrentDomain.BaseDirectory, MTConnectAgentInformation.Filename); + _backupStateFile = null; + + // Sweep orphan .valbak.* files from a prior crashed test run so + // successive TearDowns cannot restore stale state. + var directory = Path.GetDirectoryName(_stateFilePath); + if (directory != null && Directory.Exists(directory)) + { + foreach (var stale in Directory.EnumerateFiles(directory, MTConnectAgentInformation.Filename + ".valbak.*")) + { + try { File.Delete(stale); } catch { /* best-effort — do not fail the test */ } + } + } + + // Back up any pre-existing state file so we do not perturb other + // tests or the developer environment. + if (File.Exists(_stateFilePath)) + { + _backupStateFile = _stateFilePath + ".valbak." + Guid.NewGuid().ToString("N"); + File.Move(_stateFilePath, _backupStateFile); + } + } + + /// Tears down the fixture after each test. + [TearDown] + public void TearDown() + { + // Remove any state file we left behind, then restore the backup. + if (File.Exists(_stateFilePath)) + { + File.Delete(_stateFilePath); + } + if (_backupStateFile != null && File.Exists(_backupStateFile)) + { + File.Move(_backupStateFile, _stateFilePath); + _backupStateFile = null; + } + } + + // ------------------------------------------------------------------ + // Unit tests — DeterministicAgentUuid.TryValidate low-level contract + // ------------------------------------------------------------------ + + /// + /// rejects + /// , empty, and whitespace-only inputs. + /// + [TestCase(null)] + [TestCase("")] + [TestCase(" ")] + [TestCase("\t")] + [TestCase("\n")] + public void TryValidate_null_or_whitespace_returns_false(string input) + { + var ok = DeterministicAgentUuid.TryValidate(input, out var normalized); + + Assert.That(ok, Is.False); + Assert.That(normalized, Is.Null); + } + + /// + /// rejects strings + /// that do not parse as RFC 4122 UUIDs. + /// + [TestCase("not-a-uuid")] + [TestCase("fixture-stable-uuid-001")] + [TestCase("agent_1234567890abcdef")] + [TestCase("123")] + [TestCase("6ba7b810-9dad-11d1-80b4-00c04fd430c8-extra")] + public void TryValidate_unparseable_returns_false(string input) + { + var ok = DeterministicAgentUuid.TryValidate(input, out var normalized); + + Assert.That(ok, Is.False); + Assert.That(normalized, Is.Null); + } + + /// + /// accepts the + /// canonical hyphenated "D" form and returns it unchanged. + /// + [Test] + public void TryValidate_canonical_hyphenated_form_returns_true_unchanged() + { + const string Canonical = "6ba7b810-9dad-11d1-80b4-00c04fd430c8"; + + var ok = DeterministicAgentUuid.TryValidate(Canonical, out var normalized); + + Assert.That(ok, Is.True); + Assert.That(normalized, Is.EqualTo(Canonical)); + } + + /// + /// normalizes the + /// braced "B", parenthesised "P", and bare-hex "N" forms to the + /// canonical hyphenated "D" form so the wire representation stays + /// stable regardless of the input format. + /// + [TestCase("{6ba7b810-9dad-11d1-80b4-00c04fd430c8}", "6ba7b810-9dad-11d1-80b4-00c04fd430c8")] + [TestCase("(6ba7b810-9dad-11d1-80b4-00c04fd430c8)", "6ba7b810-9dad-11d1-80b4-00c04fd430c8")] + [TestCase("6ba7b8109dad11d180b400c04fd430c8", "6ba7b810-9dad-11d1-80b4-00c04fd430c8")] + public void TryValidate_non_canonical_format_normalizes_to_hyphenated(string input, string expected) + { + var ok = DeterministicAgentUuid.TryValidate(input, out var normalized); + + Assert.That(ok, Is.True); + Assert.That(normalized, Is.EqualTo(expected)); + } + + // ------------------------------------------------------------------ + // Integration tests — AgentUuidResolver.Resolve three-path algorithm + // ------------------------------------------------------------------ + + /// + /// Boot-simulation regression — malformed + /// AgentApplicationConfiguration.AgentUuid on a fresh boot + /// (no agent.information.json) must fall through to the + /// derived UUID rather than being silently stored verbatim. + /// + [Test] + public void Malformed_AgentUuid_on_fresh_boot_falls_through_to_derived() + { + const string MalformedInput = "not-a-uuid"; + const string ServiceName = "test-agent-malformed-fresh"; + var hostname = Environment.MachineName; + + var configuration = new AgentApplicationConfiguration + { + AgentUuid = MalformedInput, + ServiceName = ServiceName, + }; + + var resolved = SimulateFreshBoot(configuration, hostname); + + var expectedDerived = DeterministicAgentUuid.Derive(ServiceName, hostname, port: 0); + Assert.That(resolved, Is.EqualTo(expectedDerived), + "Malformed operator override on a fresh boot must fall through to the derived UUID."); + Assert.That(resolved, Is.Not.EqualTo(MalformedInput), + "The malformed input must NOT be stored verbatim (Part-1 UuidType violation)."); + } + + /// + /// Boot-simulation regression — malformed + /// AgentApplicationConfiguration.AgentUuid when a valid + /// agent.information.json exists must preserve the persisted + /// UUID rather than being silently stored verbatim. + /// + [Test] + public void Malformed_AgentUuid_with_valid_persisted_state_preserves_persisted() + { + const string MalformedInput = "not-a-uuid"; + const string PersistedUuid = "cfbff0d1-9375-5685-968a-48ce8b50a653"; + + // Pre-write the state file with a valid UUID. + var preexisting = new MTConnectAgentInformation(PersistedUuid); + preexisting.Save(); + + var configuration = new AgentApplicationConfiguration + { + AgentUuid = MalformedInput, + ServiceName = "test-agent-malformed-persisted", + }; + + var resolved = SimulateWarmBoot(configuration, Environment.MachineName); + + Assert.That(resolved, Is.EqualTo(PersistedUuid), + "Malformed operator override with valid persisted state must keep the persisted UUID."); + Assert.That(resolved, Is.Not.EqualTo(MalformedInput), + "The malformed input must NOT overwrite the persisted UUID."); + } + + /// + /// Path-2 hardening — malformed persisted state (e.g. a pre-hardening + /// agent version wrote a non-UUID string, or the file was hand-edited) + /// must ALSO fall through to the derived UUID rather than flowing on + /// to the wire. Prevents the exact XSD-validation failure the PR was + /// written to prevent, closed on the persisted-state axis too. + /// + [Test] + public void Malformed_persisted_state_with_no_override_falls_through_to_derived() + { + const string MalformedPersisted = "agent_1234567890abcdef"; + const string ServiceName = "test-agent-malformed-persisted-path2"; + var hostname = Environment.MachineName; + + var preexisting = new MTConnectAgentInformation(MalformedPersisted); + preexisting.Save(); + + var configuration = new AgentApplicationConfiguration + { + AgentUuid = null, + ServiceName = ServiceName, + }; + + var resolved = SimulateWarmBoot(configuration, hostname); + + var expectedDerived = DeterministicAgentUuid.Derive(ServiceName, hostname, port: 0); + Assert.That(resolved, Is.EqualTo(expectedDerived), + "Malformed persisted UUID with no override must fall through to derived — Path 2 must validate."); + Assert.That(resolved, Is.Not.EqualTo(MalformedPersisted), + "The malformed persisted value must NOT reach the wire."); + } + + /// + /// Boot-simulation regression — valid non-canonical + /// AgentApplicationConfiguration.AgentUuid (e.g. braced form) + /// is accepted and normalized to the canonical hyphenated form so the + /// wire representation stays stable across boots. + /// + [Test] + public void Valid_non_canonical_AgentUuid_is_normalized_to_hyphenated() + { + const string BracedInput = "{6ba7b810-9dad-11d1-80b4-00c04fd430c8}"; + const string ExpectedCanonical = "6ba7b810-9dad-11d1-80b4-00c04fd430c8"; + + var configuration = new AgentApplicationConfiguration + { + AgentUuid = BracedInput, + ServiceName = "test-agent-canonicalization", + }; + + var resolved = SimulateFreshBoot(configuration, Environment.MachineName); + + Assert.That(resolved, Is.EqualTo(ExpectedCanonical), + "Non-canonical valid UUID inputs must be normalized to the hyphenated D-form."); + } + + /// + /// Two-boot regression — first boot with no override and no state file + /// derives + persists a UUID; second boot with no override reads the + /// persisted UUID from agent.information.json and adopts it + /// verbatim (Path 2). Bit-identical across the two boots. + /// + [Test] + public void Persisted_UUID_is_adopted_on_second_boot_when_no_override() + { + const string ServiceName = "test-agent-persisted-second-boot"; + var hostname = Environment.MachineName; + + var configuration = new AgentApplicationConfiguration + { + AgentUuid = null, + ServiceName = ServiceName, + }; + + // Boot 1 — no override, no state file → derive + persist. + var boot1Uuid = SimulateFreshBoot(configuration, hostname); + Assert.That(File.Exists(_stateFilePath), Is.True, + "Boot 1 must persist agent.information.json with the resolved UUID."); + var persistedAfterBoot1 = MTConnectAgentInformation.Read(); + Assert.That(persistedAfterBoot1, Is.Not.Null); + Assert.That(persistedAfterBoot1!.Uuid, Is.EqualTo(boot1Uuid), + "Boot 1's on-disk UUID must equal the resolved value."); + + // Boot 2 — no override, but persisted state exists → adopt persisted. + var boot2Uuid = SimulateWarmBoot(configuration, hostname); + Assert.That(boot2Uuid, Is.EqualTo(boot1Uuid), + "Boot 2 must adopt the persisted UUID bit-identically (Path 2)."); + } + + /// + /// First-boot persistence regression — no override, no state file → + /// Path 3 derives the UUID AND + /// writes it to agent.information.json so subsequent boots hit + /// Path 2. Guards against a regression where the resolve step returns + /// the derived value but the persist step is dropped. + /// + [Test] + public void First_boot_persists_derived_UUID_to_agent_information_json() + { + const string ServiceName = "test-agent-first-boot-persist"; + var hostname = Environment.MachineName; + + var configuration = new AgentApplicationConfiguration + { + AgentUuid = null, + ServiceName = ServiceName, + }; + + var resolved = SimulateFreshBoot(configuration, hostname); + + Assert.That(File.Exists(_stateFilePath), Is.True, + "agent.information.json must exist after the first boot."); + var persisted = MTConnectAgentInformation.Read(); + Assert.That(persisted, Is.Not.Null); + Assert.That(persisted!.Uuid, Is.EqualTo(resolved), + "Persisted UUID on disk must equal the resolved value."); + var expectedDerived = DeterministicAgentUuid.Derive(ServiceName, hostname, port: 0); + Assert.That(persisted.Uuid, Is.EqualTo(expectedDerived), + "Persisted UUID must equal the deterministic derivation for the given ServiceName."); + } + + // ------------------------------------------------------------------ + // Boot-simulation helpers — thin wrappers around AgentUuidResolver. + // ------------------------------------------------------------------ + + /// + /// Replays the fresh-boot UUID resolution slice of + /// MTConnectAgentApplication.StartAgent — no + /// agent.information.json on disk — via + /// (the exact call production + /// makes) followed by so + /// on-disk state assertions can verify persistence. + /// + private string SimulateFreshBoot( + AgentApplicationConfiguration configuration, + string hostname) + { + var existing = MTConnectAgentInformation.Read(); + var freshlyConstructed = existing == null; + var info = existing ?? new MTConnectAgentInformation(); + + info.Uuid = AgentUuidResolver.Resolve( + operatorSuppliedUuid: configuration.AgentUuid, + persistedUuid: freshlyConstructed ? null : info.Uuid, + agentName: configuration.ServiceName, + hostname: hostname); + + info.Save(); + return info.Uuid; + } + + /// + /// Replays the warm-boot UUID resolution slice — pre-existing + /// agent.information.json is read first — via + /// . Semantically identical to + /// ; two named helpers make the + /// per-test intent (fresh vs warm) unambiguous at the call site. + /// + private string SimulateWarmBoot( + AgentApplicationConfiguration configuration, + string hostname) => SimulateFreshBoot(configuration, hostname); + } +} From cefdf389982666a55d936ee456f4a58a42ded1e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Otto=20Boly=C3=B3s?= Date: Wed, 22 Jul 2026 13:14:01 +0200 Subject: [PATCH 2/7] fix(agent): reject non-UUID AgentUuid on override AND persisted paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Hardens the Agent meta-device UUID resolution slice added by #168 so malformed input never reaches the wire, closing the gap on both the operator-supplied override AND the persisted agent.information.json paths — the previous single-path validation left Path 2 open. Change: * New AgentUuidResolver.Resolve (public static, MTConnect.NET-Common) implements the three-path algorithm — Path 1 validated override, Path 2 validated persisted state, Path 3 deterministic derivation — with a delegate-based warn hook so MTConnect.NET-Common takes no hard dependency on NLog. The delegate emits the raw AgentUuid value pre-sanitized (CRLF stripped, truncated to 64 chars) to guard against log-injection and paste-in-wrong-field secret leakage. * MTConnectAgentApplication.RunAgent routes through the shared resolver instead of hand-rolling the dual-if resolution. Collapses the two-branch shape that re-tested string.IsNullOrEmpty twice. * DeterministicAgentUuid.TryValidate parameter name renamed to normalized per project convention (AmE in committed code); XML-doc verbs (normalizes, parenthesized) updated to match. Positional callers and out-var callers are unaffected. The RED tests from the preceding commit turn GREEN because AgentUuidResolver.Resolve now exists and the AmE parameter name lands. The three test fixtures share the same resolver so drift between production and tests is impossible. --- .../MTConnectAgentApplication.cs | 42 ++--- .../Agents/AgentUuidResolver.cs | 161 ++++++++++++++++++ .../Agents/DeterministicAgentUuid.cs | 44 +++++ 3 files changed, 221 insertions(+), 26 deletions(-) create mode 100644 libraries/MTConnect.NET-Common/Agents/AgentUuidResolver.cs diff --git a/agent/MTConnect.NET-Applications-Agents/MTConnectAgentApplication.cs b/agent/MTConnect.NET-Applications-Agents/MTConnectAgentApplication.cs index 117d12393..c56cf4ff5 100644 --- a/agent/MTConnect.NET-Applications-Agents/MTConnectAgentApplication.cs +++ b/agent/MTConnect.NET-Applications-Agents/MTConnectAgentApplication.cs @@ -433,32 +433,22 @@ public void StartAgent(IAgentApplicationConfiguration configuration, bool verbos var freshlyConstructed = (existingAgentInformation == null); var agentInformation = existingAgentInformation ?? new MTConnectAgentInformation(); - // Apply explicit AgentUuid config override (if set). This - // pins the Agent meta-device UUID across restarts without - // requiring agent.information.json to be present on disk, - // and takes precedence over any UUID previously stored in - // that file. See MTConnect v2.7 XSD UuidType ("for its - // entire life") vs Header.instanceId (per-boot). - if (!string.IsNullOrEmpty(configuration.AgentUuid)) - { - agentInformation.Uuid = configuration.AgentUuid; - } - - // When no operator config override and no persisted state file, - // derive a deterministic UUID v5 (RFC 4122 §4.3, DNS namespace, - // SHA-1) from ServiceName:port so the Agent meta-device satisfies - // UuidType's "for it's entire life" annotation across every restart - // in the ephemeral-container deployment path. Mirrors cppagent's - // name_generator prior art. Port is 0 (sentinel) because - // IAgentApplicationConfiguration does not surface a listener-port - // property; the seed is still unique per ServiceName. - if (freshlyConstructed && string.IsNullOrEmpty(configuration.AgentUuid)) - { - agentInformation.Uuid = DeterministicAgentUuid.Derive( - configuration.ServiceName, - System.Environment.MachineName, - port: 0); - } + // Resolve the Agent meta-device UUID via the shared three-path + // algorithm in AgentUuidResolver so this application and the + // test fixtures exercise the same code and cannot silently + // drift. Path 1 (validated operator override) wins, else Path 2 + // (validated persisted state), else Path 3 (deterministic + // UUID v5 derivation from ServiceName). Malformed input on + // either the override or the persisted path logs a warning + // and falls through — silently forwarding non-UUID content + // would break MTConnect Part 1's wire-XSD validation on every + // typed enum/decimal DataItem. + agentInformation.Uuid = AgentUuidResolver.Resolve( + operatorSuppliedUuid: configuration.AgentUuid, + persistedUuid: freshlyConstructed ? null : agentInformation.Uuid, + agentName: configuration.ServiceName, + hostname: System.Environment.MachineName, + warn: message => _applicationLogger?.Warn(message)); // Create Observation File Buffer if (configuration.Durable) diff --git a/libraries/MTConnect.NET-Common/Agents/AgentUuidResolver.cs b/libraries/MTConnect.NET-Common/Agents/AgentUuidResolver.cs new file mode 100644 index 000000000..2bb865452 --- /dev/null +++ b/libraries/MTConnect.NET-Common/Agents/AgentUuidResolver.cs @@ -0,0 +1,161 @@ +// Copyright (c) 2026 TrakHound Inc., All Rights Reserved. +// TrakHound Inc. licenses this file to you under the MIT license. + +using System; + +namespace MTConnect.Agents +{ + /// + /// Resolves the Agent meta-device UUID from the three canonical sources — + /// operator-supplied config override, persisted agent.information.json + /// state, and a deterministic UUID v5 derivation — with RFC 4122 validation + /// applied uniformly on both the override and the persisted paths. + /// + /// + /// Shared between MTConnectAgentApplication.StartAgent and the test + /// fixtures so the boot-time resolution is exercised by the same code in + /// both places; the tests cannot silently drift from production semantics. + /// + /// + /// + /// Resolution order: + /// + /// + /// + /// Path 1 — operator-supplied override. If + /// parses as an RFC 4122 UUID + /// (via ), the canonical + /// hyphenated form wins. Malformed input logs a warning via + /// and falls through to Path 2 / Path 3. + /// + /// + /// Path 2 — persisted state. If + /// parses as an RFC 4122 UUID, the + /// canonical form wins. Malformed persisted state (e.g. a pre-hardening + /// agent version wrote a non-UUID string, or the file was hand-edited) + /// logs a warning and falls through to Path 3 so a spec-conformant UUID + /// always reaches the wire. + /// + /// + /// Path 3 — deterministic derivation. + /// over + /// (agentName ?? hostname, hostname, port: 0). Port is 0 + /// because IAgentApplicationConfiguration does not surface a + /// listener-port property; the seed is still unique per agent name. + /// + /// + /// + /// + /// Spec rationale — MTConnect Part 1 types the uuid attribute as the + /// UUID DataType (RFC 4122). Silently forwarding a non-UUID string + /// (from any source) breaks XSD validation on every typed enum/decimal + /// DataItem and diverges from the cppagent reference implementation, which + /// rejects malformed input at ingress. + /// + /// + public static class AgentUuidResolver + { + /// + /// Maximum length of an AgentUuid value echoed to the warning + /// log. Valid UUIDs are 32–38 characters; anything longer is almost + /// certainly a paste-in-wrong-field mistake (e.g. an API key) whose + /// full value should not persist in log archives. + /// + private const int LogValueMaxLength = 64; + + /// + /// Resolves the Agent meta-device UUID per the three-path algorithm. + /// + /// + /// Raw value from AgentApplicationConfiguration.AgentUuid; may + /// be , empty, or malformed. + /// + /// + /// Raw value from MTConnectAgentInformation.Read().Uuid, or + /// when no agent.information.json exists + /// (freshly constructed lifecycle). May itself be malformed if a prior + /// agent boot wrote non-UUID content. + /// + /// + /// The logical agent name (typically configuration.ServiceName). + /// Passed verbatim to , + /// which falls back to when this is + /// or empty. + /// + /// + /// The machine host name (typically + /// ). Used by + /// as both the fallback + /// seed component and the deterministic derivation input. + /// + /// + /// Optional delegate invoked with a human-readable message when Path 1 + /// or Path 2 rejects malformed input. Kept as a plain + /// so MTConnect.NET-Common does not + /// take a hard dependency on any logging framework; the caller adapts + /// it to NLog, Serilog, or Microsoft.Extensions.Logging. Raw + /// values echoed via this delegate are pre-sanitised (CRLF stripped, + /// truncated to characters) to guard + /// against log-injection and secret leakage. + /// + /// + /// The canonical hyphenated RFC 4122 UUID string that the agent must + /// adopt for its meta-device. + /// + public static string Resolve( + string operatorSuppliedUuid, + string persistedUuid, + string agentName, + string hostname, + Action warn = null) + { + // Path 1 — validated operator override wins. + if (DeterministicAgentUuid.TryValidate(operatorSuppliedUuid, out var normalizedOverride)) + { + return normalizedOverride; + } + + // Path 1 rejected but operator supplied something → warn. + if (!string.IsNullOrEmpty(operatorSuppliedUuid)) + { + var fallbackKind = DeterministicAgentUuid.TryValidate(persistedUuid, out _) + ? "persisted" + : "derived"; + warn?.Invoke(string.Format( + "AgentUuid override '{0}' is not a valid RFC 4122 UUID; falling back to {1} UUID.", + SanitiseForLog(operatorSuppliedUuid), + fallbackKind)); + } + + // Path 2 — validated persisted state wins over derivation. + if (DeterministicAgentUuid.TryValidate(persistedUuid, out var normalizedPersisted)) + { + return normalizedPersisted; + } + + // Path 2 rejected but persisted state carried something → warn. + if (!string.IsNullOrEmpty(persistedUuid)) + { + warn?.Invoke( + "Persisted AgentUuid in agent.information.json is not a valid RFC 4122 UUID; falling back to derived UUID."); + } + + // Path 3 — deterministic derivation. + return DeterministicAgentUuid.Derive(agentName, hostname, port: 0); + } + + /// + /// Strips CR/LF (log-injection guard) and truncates to + /// characters (secret-leakage guard) + /// before echoing an operator-supplied value to the warning log. + /// + private static string SanitiseForLog(string value) + { + if (string.IsNullOrEmpty(value)) return value; + var stripped = value.Replace("\r", string.Empty).Replace("\n", string.Empty); + return stripped.Length > LogValueMaxLength + ? stripped.Substring(0, LogValueMaxLength) + "…" + : stripped; + } + } +} diff --git a/libraries/MTConnect.NET-Common/Agents/DeterministicAgentUuid.cs b/libraries/MTConnect.NET-Common/Agents/DeterministicAgentUuid.cs index 518fd8c44..a2cac9c9b 100644 --- a/libraries/MTConnect.NET-Common/Agents/DeterministicAgentUuid.cs +++ b/libraries/MTConnect.NET-Common/Agents/DeterministicAgentUuid.cs @@ -129,5 +129,49 @@ private static byte[] BigEndianToGuidBytes(byte[] beBytes) Buffer.BlockCopy(beBytes, 8, result, 8, 8); return result; } + + /// + /// Validates and normalizes an operator-supplied Agent UUID string. + /// + /// + /// Accepts any format that + /// recognizes — hyphenated "D" (xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx), + /// braced "B", parenthesized "P", bare-hex "N", or hex-braced "X" — and + /// returns the canonical hyphenated "D" form so the wire representation + /// is stable regardless of input format. Rejects , + /// empty, whitespace-only, and unparseable inputs. + /// + /// Motivation: MTConnect Part 1 types the uuid attribute as the + /// UUID DataType (RFC 4122). Silently forwarding a non-UUID + /// string emits wire content that fails XSD validation on any typed + /// enum/decimal DataItem and breaks parity with the cppagent reference + /// implementation. Callers that supply malformed input should log a + /// warning and fall through to persisted or derived UUIDs — the + /// three-path resolution in . + /// + /// + /// + /// The raw operator-supplied value from + /// AgentApplicationConfiguration.AgentUuid; may be + /// or empty. + /// + /// + /// On success, the canonical hyphenated "D" form of the parsed UUID + /// (e.g. cfbff0d1-9375-5685-968a-48ce8b50a653); + /// on failure. + /// + /// + /// if parses as an + /// RFC 4122 UUID; for , + /// empty, whitespace-only, or unparseable inputs. + /// + public static bool TryValidate(string input, out string normalized) + { + normalized = null; + if (string.IsNullOrWhiteSpace(input)) return false; + if (!Guid.TryParse(input, out var parsed)) return false; + normalized = parsed.ToString(); + return true; + } } } From dfb6907cec433ab4829e4ad3990f1bdd2aae9111 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Otto=20Boly=C3=B3s?= Date: Wed, 19 Aug 2026 08:29:04 +0200 Subject: [PATCH 3/7] test(common-tests): close AgentUuidResolver warn-delegate and SanitiseForLog coverage gaps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extends AgentUuidValidationTests.cs with 15 test cases the initial fixture did not cover, per the coverage-FLOOR panel: - TryValidate accepts the hex-braced "X" Guid format (missing enum arm). - TryValidate normalizes uppercase hex to lowercase (case-insensitivity). - Warn delegate is NOT invoked on the Path-1-wins and Path-2-wins happy paths, nor when both override and persisted are null/empty (four combinations, TestCase-parameterized). - Warn message names "persisted" when Path 1 rejected + Path 2 valid, "derived" when Path 1 rejected + Path 2 absent — both arms of the two-arm fallback-kind ternary in AgentUuidResolver.Resolve. - Warn delegate is invoked TWICE when both sources are malformed (guard against the second warn being swallowed). - Persisted-only rejection emits the persisted warn without the override warn. - Null warn delegate and default-omitted warn parameter do not throw NullReferenceException on either rejection path. - SanitiseForLog strips CRLF, lone CR, and lone LF (log-injection guard). - SanitiseForLog does NOT truncate at exactly 64 chars; DOES truncate and appends ellipsis at 65 chars (secret-leakage guard boundary). - SanitiseForLog measures length AFTER CRLF stripping (compose order). - Path 3 hostname fallback exercised when agentName is null / empty. - Malformed persisted UUID round-trips through the real MTConnectAgentInformation.Save/Read JSON serializer and is still rejected by the resolver (integration failure-path proof, not just an in-memory string). --- .../Agents/AgentUuidValidationTests.cs | 506 ++++++++++++++++++ 1 file changed, 506 insertions(+) diff --git a/tests/MTConnect.NET-Common-Tests/Agents/AgentUuidValidationTests.cs b/tests/MTConnect.NET-Common-Tests/Agents/AgentUuidValidationTests.cs index c404831df..482dd23f3 100644 --- a/tests/MTConnect.NET-Common-Tests/Agents/AgentUuidValidationTests.cs +++ b/tests/MTConnect.NET-Common-Tests/Agents/AgentUuidValidationTests.cs @@ -2,6 +2,7 @@ // TrakHound Inc. licenses this file to you under the MIT license. using System; +using System.Collections.Generic; using System.IO; using MTConnect.Agents; using MTConnect.Configurations; @@ -152,6 +153,43 @@ public void TryValidate_non_canonical_format_normalizes_to_hyphenated(string inp Assert.That(normalized, Is.EqualTo(expected)); } + /// + /// also accepts the + /// hex-braced "X" format ({0xhh,0xhh,0xhh,{...}}) that + /// recognises, and + /// normalises it to the canonical hyphenated "D" form. Closes the + /// last Guid-format enum arm not covered by the sibling test. + /// + [Test] + public void TryValidate_hex_braced_X_format_normalizes_to_hyphenated() + { + const string XForm = "{0x6ba7b810,0x9dad,0x11d1,{0x80,0xb4,0x00,0xc0,0x4f,0xd4,0x30,0xc8}}"; + const string Expected = "6ba7b810-9dad-11d1-80b4-00c04fd430c8"; + + var ok = DeterministicAgentUuid.TryValidate(XForm, out var normalized); + + Assert.That(ok, Is.True); + Assert.That(normalized, Is.EqualTo(Expected)); + } + + /// + /// accepts uppercase + /// hex characters (case-insensitive per RFC 4122) and normalises the + /// output to lowercase so the wire representation is stable regardless + /// of the operator's typing. + /// + [Test] + public void TryValidate_uppercase_hex_is_normalised_to_lowercase() + { + const string Uppercased = "6BA7B810-9DAD-11D1-80B4-00C04FD430C8"; + const string Expected = "6ba7b810-9dad-11d1-80b4-00c04fd430c8"; + + var ok = DeterministicAgentUuid.TryValidate(Uppercased, out var normalized); + + Assert.That(ok, Is.True); + Assert.That(normalized, Is.EqualTo(Expected)); + } + // ------------------------------------------------------------------ // Integration tests — AgentUuidResolver.Resolve three-path algorithm // ------------------------------------------------------------------ @@ -335,6 +373,474 @@ public void First_boot_persists_derived_UUID_to_agent_information_json() "Persisted UUID must equal the deterministic derivation for the given ServiceName."); } + // ------------------------------------------------------------------ + // Warn-delegate contract — invocation count, message shape, both + // arms of the "persisted" vs "derived" fallback-kind switch, and + // the null-delegate no-op guard. + // ------------------------------------------------------------------ + + /// + /// When Path 1 (operator override) wins, the warn delegate must NOT + /// be invoked — the happy path is silent by design so a valid + /// operator configuration does not pollute the log with warnings. + /// + [Test] + public void Warn_delegate_not_invoked_when_operator_override_is_valid() + { + var messages = new List(); + + var resolved = AgentUuidResolver.Resolve( + operatorSuppliedUuid: "6ba7b810-9dad-11d1-80b4-00c04fd430c8", + persistedUuid: null, + agentName: "test-agent", + hostname: "test-host", + warn: messages.Add); + + Assert.That(resolved, Is.EqualTo("6ba7b810-9dad-11d1-80b4-00c04fd430c8")); + Assert.That(messages, Is.Empty, + "Path 1 (valid override) must be silent — no warn."); + } + + /// + /// When Path 1 is null and Path 2 (persisted) wins, the warn delegate + /// must NOT be invoked — an empty override + valid persisted is the + /// normal warm-boot happy path. + /// + [Test] + public void Warn_delegate_not_invoked_when_override_null_and_persisted_valid() + { + var messages = new List(); + + var resolved = AgentUuidResolver.Resolve( + operatorSuppliedUuid: null, + persistedUuid: "cfbff0d1-9375-5685-968a-48ce8b50a653", + agentName: "test-agent", + hostname: "test-host", + warn: messages.Add); + + Assert.That(resolved, Is.EqualTo("cfbff0d1-9375-5685-968a-48ce8b50a653")); + Assert.That(messages, Is.Empty, + "Path 2 (valid persisted, null override) must be silent — no warn."); + } + + /// + /// When Path 1 is null AND Path 2 is null (both truly empty — the + /// fresh-boot case), the warn delegate must NOT be invoked — both + /// early-return guards are gated on !IsNullOrEmpty(...). + /// + [TestCase(null, null)] + [TestCase("", null)] + [TestCase(null, "")] + [TestCase("", "")] + public void Warn_delegate_not_invoked_when_both_override_and_persisted_are_empty( + string? overrideValue, string? persistedValue) + { + var messages = new List(); + + var resolved = AgentUuidResolver.Resolve( + operatorSuppliedUuid: overrideValue, + persistedUuid: persistedValue, + agentName: "test-agent", + hostname: "test-host", + warn: messages.Add); + + Assert.That(resolved, Is.EqualTo(DeterministicAgentUuid.Derive("test-agent", "test-host", 0))); + Assert.That(messages, Is.Empty, + "Both empty ≠ malformed; fresh boot must be silent."); + } + + /// + /// When Path 1 is rejected and Path 2 (persisted) is a valid UUID, + /// the warn message must name "persisted" as the fallback kind — the + /// operator learns which alternative source overrode their supplied + /// value. Pins the "persisted" arm of the two-arm fallback-kind + /// ternary in . + /// + [Test] + public void Warn_message_names_persisted_when_override_bad_and_persisted_valid() + { + var messages = new List(); + const string Malformed = "not-a-uuid"; + const string ValidPersisted = "cfbff0d1-9375-5685-968a-48ce8b50a653"; + + var resolved = AgentUuidResolver.Resolve( + operatorSuppliedUuid: Malformed, + persistedUuid: ValidPersisted, + agentName: "test-agent", + hostname: "test-host", + warn: messages.Add); + + Assert.That(resolved, Is.EqualTo(ValidPersisted)); + Assert.That(messages, Has.Count.EqualTo(1), + "One warn — for the rejected operator override; the valid persisted UUID needs no warn."); + Assert.That(messages[0], Does.Contain("AgentUuid override")); + Assert.That(messages[0], Does.Contain(Malformed)); + Assert.That(messages[0], Does.Contain("falling back to persisted UUID")); + } + + /// + /// When Path 1 is rejected and Path 2 (persisted) is also rejected / + /// null, the warn message for the operator override must name + /// "derived" as the fallback kind. Pins the "derived" arm of the + /// two-arm fallback-kind ternary in + /// . + /// + [Test] + public void Warn_message_names_derived_when_override_bad_and_persisted_absent() + { + var messages = new List(); + const string Malformed = "not-a-uuid"; + + var resolved = AgentUuidResolver.Resolve( + operatorSuppliedUuid: Malformed, + persistedUuid: null, + agentName: "test-agent", + hostname: "test-host", + warn: messages.Add); + + Assert.That(resolved, Is.EqualTo(DeterministicAgentUuid.Derive("test-agent", "test-host", 0))); + Assert.That(messages, Has.Count.EqualTo(1)); + Assert.That(messages[0], Does.Contain("AgentUuid override")); + Assert.That(messages[0], Does.Contain("falling back to derived UUID")); + } + + /// + /// When Path 1 (operator override) is rejected AND Path 2 (persisted + /// state) is rejected — the failing-both case a mid-life container + /// upgrade produces — the warn delegate must be invoked TWICE, once + /// per rejected source. Guards against a regression that swallows + /// the second warn under the first. + /// + [Test] + public void Warn_delegate_invoked_twice_when_both_override_and_persisted_are_malformed() + { + var messages = new List(); + const string BadOverride = "not-a-uuid"; + const string BadPersisted = "also-not-a-uuid"; + + var resolved = AgentUuidResolver.Resolve( + operatorSuppliedUuid: BadOverride, + persistedUuid: BadPersisted, + agentName: "test-agent", + hostname: "test-host", + warn: messages.Add); + + Assert.That(resolved, Is.EqualTo(DeterministicAgentUuid.Derive("test-agent", "test-host", 0))); + Assert.That(messages, Has.Count.EqualTo(2), + "Two rejected sources → two warns; the second must not be swallowed."); + Assert.That(messages[0], Does.Contain("AgentUuid override")); + Assert.That(messages[0], Does.Contain(BadOverride)); + Assert.That(messages[0], Does.Contain("falling back to derived UUID")); + Assert.That(messages[1], Does.Contain("Persisted AgentUuid")); + Assert.That(messages[1], Does.Contain("falling back to derived UUID")); + } + + /// + /// When Path 1 (operator override) is absent and Path 2 (persisted + /// state) is rejected, only the persisted-state warn is emitted — + /// the operator did not supply anything to warn about. + /// + [Test] + public void Warn_delegate_emits_persisted_warn_only_when_override_null_and_persisted_bad() + { + var messages = new List(); + const string BadPersisted = "also-not-a-uuid"; + + var resolved = AgentUuidResolver.Resolve( + operatorSuppliedUuid: null, + persistedUuid: BadPersisted, + agentName: "test-agent", + hostname: "test-host", + warn: messages.Add); + + Assert.That(resolved, Is.EqualTo(DeterministicAgentUuid.Derive("test-agent", "test-host", 0))); + Assert.That(messages, Has.Count.EqualTo(1)); + Assert.That(messages[0], Does.Contain("Persisted AgentUuid")); + Assert.That(messages[0], Does.Not.Contain("override"), + "No override supplied → the override-warn must NOT be emitted."); + } + + /// + /// A null warn delegate is valid — the resolver documents it + /// as "optional" and callers that do not want warnings must not + /// crash. Guards against a null-conditional invocation regression. + /// + [Test] + public void Null_warn_delegate_does_not_throw_on_either_rejection_path() + { + Assert.DoesNotThrow(() => + { + _ = AgentUuidResolver.Resolve( + operatorSuppliedUuid: "not-a-uuid", + persistedUuid: "also-not-a-uuid", + agentName: "test-agent", + hostname: "test-host", + warn: null); + }); + } + + /// + /// The warn parameter defaults to — + /// callers that omit it entirely must get the same no-throw + /// behaviour as an explicitly-null delegate. + /// + [Test] + public void Default_warn_argument_omitted_does_not_throw() + { + Assert.DoesNotThrow(() => + { + _ = AgentUuidResolver.Resolve( + operatorSuppliedUuid: "not-a-uuid", + persistedUuid: "also-not-a-uuid", + agentName: "test-agent", + hostname: "test-host"); + }); + } + + // ------------------------------------------------------------------ + // SanitiseForLog — CRLF stripping (log-injection guard) and + // truncation-at-boundary behaviour (secret-leakage guard). Exercised + // indirectly via the operator-override warn message content, which + // is the only route that pipes user input through the sanitiser. + // ------------------------------------------------------------------ + + /// + /// SanitiseForLog strips CR and LF characters from operator + /// input before it appears in the warn message — defends against + /// log-injection (forged log lines) where a hostile operator embeds + /// \r\n in the config file. + /// + [Test] + public void Warn_message_strips_CR_LF_from_operator_supplied_value() + { + var messages = new List(); + const string Injected = "bad-uuid\r\nFAKE-LOG-LINE-INJECTED"; + + _ = AgentUuidResolver.Resolve( + operatorSuppliedUuid: Injected, + persistedUuid: null, + agentName: "test-agent", + hostname: "test-host", + warn: messages.Add); + + Assert.That(messages, Has.Count.EqualTo(1)); + Assert.That(messages[0], Does.Not.Contain("\r")); + Assert.That(messages[0], Does.Not.Contain("\n")); + Assert.That(messages[0], Does.Contain("bad-uuidFAKE-LOG-LINE-INJECTED"), + "CR and LF must be stripped inline — surrounding characters remain."); + } + + /// + /// SanitiseForLog strips a lone CR (Mac Classic line-ending) + /// as well as CRLF pairs. Pins the guard against callers that split + /// on either terminator. + /// + [Test] + public void Warn_message_strips_lone_CR_from_operator_supplied_value() + { + var messages = new List(); + const string Injected = "bad\rvalue"; + + _ = AgentUuidResolver.Resolve( + operatorSuppliedUuid: Injected, + persistedUuid: null, + agentName: "test-agent", + hostname: "test-host", + warn: messages.Add); + + Assert.That(messages, Has.Count.EqualTo(1)); + Assert.That(messages[0], Does.Not.Contain("\r")); + Assert.That(messages[0], Does.Contain("badvalue")); + } + + /// + /// SanitiseForLog strips a lone LF as well, matching the + /// symmetric CR treatment. + /// + [Test] + public void Warn_message_strips_lone_LF_from_operator_supplied_value() + { + var messages = new List(); + const string Injected = "bad\nvalue"; + + _ = AgentUuidResolver.Resolve( + operatorSuppliedUuid: Injected, + persistedUuid: null, + agentName: "test-agent", + hostname: "test-host", + warn: messages.Add); + + Assert.That(messages, Has.Count.EqualTo(1)); + Assert.That(messages[0], Does.Not.Contain("\n")); + Assert.That(messages[0], Does.Contain("badvalue")); + } + + /// + /// SanitiseForLog does NOT truncate values at or below the + /// 64-character LogValueMaxLength boundary — pins the exact + /// boundary against off-by-one regressions on the length check. + /// + [Test] + public void Warn_message_does_not_truncate_at_exactly_max_length() + { + var messages = new List(); + // Exactly 64 chars, none of which is a valid UUID character + // pattern → guaranteed rejection by TryValidate. + var input = new string('z', 64); + + _ = AgentUuidResolver.Resolve( + operatorSuppliedUuid: input, + persistedUuid: null, + agentName: "test-agent", + hostname: "test-host", + warn: messages.Add); + + Assert.That(messages, Has.Count.EqualTo(1)); + Assert.That(messages[0], Does.Contain(input), + "A value at exactly the boundary must appear verbatim."); + Assert.That(messages[0], Does.Not.Contain("…"), + "No ellipsis must be appended at exactly the boundary."); + } + + /// + /// SanitiseForLog truncates values longer than 64 characters + /// and appends a single ellipsis (…) so the operator sees the + /// prefix but a leaked API-key-length string is not archived in + /// full. Pins the 65-char over-boundary case. + /// + [Test] + public void Warn_message_truncates_over_max_length_with_ellipsis() + { + var messages = new List(); + // 65 chars, one over the boundary. + var input = new string('z', 65); + + _ = AgentUuidResolver.Resolve( + operatorSuppliedUuid: input, + persistedUuid: null, + agentName: "test-agent", + hostname: "test-host", + warn: messages.Add); + + Assert.That(messages, Has.Count.EqualTo(1)); + Assert.That(messages[0], Does.Contain(new string('z', 64) + "…"), + "The first 64 characters must be preserved and an ellipsis appended."); + Assert.That(messages[0], Does.Not.Contain(new string('z', 65)), + "The full 65-character input must NOT appear verbatim."); + } + + /// + /// SanitiseForLog's truncation applies AFTER CRLF stripping — + /// a value whose raw length is over the boundary but whose stripped + /// length falls within it must NOT be truncated. Pins the compose + /// order of the two sanitiser steps. + /// + [Test] + public void Warn_message_measures_length_after_CRLF_stripping() + { + var messages = new List(); + // 64 z's interleaved with 10 CRLFs (raw length 84, stripped length 64). + var input = new string('z', 64) + "\r\n\r\n\r\n\r\n\r\n"; + + _ = AgentUuidResolver.Resolve( + operatorSuppliedUuid: input, + persistedUuid: null, + agentName: "test-agent", + hostname: "test-host", + warn: messages.Add); + + Assert.That(messages, Has.Count.EqualTo(1)); + Assert.That(messages[0], Does.Contain(new string('z', 64))); + Assert.That(messages[0], Does.Not.Contain("…"), + "Length is measured after CRLF stripping — no ellipsis needed here."); + } + + // ------------------------------------------------------------------ + // Path-3 hostname fallback — the agentName-is-null / empty case + // routes through DeterministicAgentUuid.Derive's own fallback. + // ------------------------------------------------------------------ + + /// + /// When Path 3 is reached and agentName is , + /// the hostname stands in as the seed component per + /// 's documented fallback. + /// The output is stable, deterministic, and identical to + /// Derive(hostname, hostname, 0). + /// + [TestCase(null)] + [TestCase("")] + public void Resolve_falls_back_to_hostname_when_agentName_is_null_or_empty(string? agentName) + { + var resolved = AgentUuidResolver.Resolve( + operatorSuppliedUuid: null, + persistedUuid: null, + agentName: agentName, + hostname: "canonical-host", + warn: null); + + var expected = DeterministicAgentUuid.Derive(agentName, "canonical-host", 0); + Assert.That(resolved, Is.EqualTo(expected)); + // Cross-check: Derive(null, host, 0) == Derive(host, host, 0). + var expectedViaHost = DeterministicAgentUuid.Derive("canonical-host", "canonical-host", 0); + Assert.That(resolved, Is.EqualTo(expectedViaHost), + "agentName null/empty ⇒ Derive seeds with hostname; outputs must match."); + } + + // ------------------------------------------------------------------ + // Persisted-path failure integration — the malformed persisted UUID + // is not just a synthetic string parameter, it MUST round-trip + // through the real MTConnectAgentInformation.Save/Read (JSON file + // on disk) and still be rejected by the resolver. Guards against a + // regression that only tests the in-memory path. + // ------------------------------------------------------------------ + + /// + /// End-to-end integration: a malformed UUID string is written to + /// agent.information.json via + /// , read back via the + /// real , and rejected + /// by which falls through + /// to the derived UUID. Confirms the failure path is exercised + /// through the real serialiser, not just an in-memory string. + /// + [Test] + public void Malformed_persisted_state_survives_JSON_round_trip_and_is_rejected() + { + const string MalformedPersisted = "definitely-not-a-uuid-42"; + const string ServiceName = "test-agent-json-round-trip"; + var hostname = Environment.MachineName; + + // Write malformed value to disk via the production serialiser. + var toPersist = new MTConnectAgentInformation(MalformedPersisted); + toPersist.Save(); + + // Verify the JSON on disk actually contains the malformed value — + // guards against a silent Save-side validator that never existed + // but might be added later without failing the harness. + var raw = File.ReadAllText(_stateFilePath); + Assert.That(raw, Does.Contain(MalformedPersisted), + "Precondition: the malformed value must actually be on disk."); + + // Read back via the production reader and hand its Uuid to Resolve. + var reread = MTConnectAgentInformation.Read(); + Assert.That(reread, Is.Not.Null); + Assert.That(reread!.Uuid, Is.EqualTo(MalformedPersisted), + "Precondition: the reader must surface the malformed value verbatim."); + + var messages = new List(); + var resolved = AgentUuidResolver.Resolve( + operatorSuppliedUuid: null, + persistedUuid: reread.Uuid, + agentName: ServiceName, + hostname: hostname, + warn: messages.Add); + + var expectedDerived = DeterministicAgentUuid.Derive(ServiceName, hostname, port: 0); + Assert.That(resolved, Is.EqualTo(expectedDerived)); + Assert.That(messages, Has.Count.EqualTo(1)); + Assert.That(messages[0], Does.Contain("Persisted AgentUuid")); + } + // ------------------------------------------------------------------ // Boot-simulation helpers — thin wrappers around AgentUuidResolver. // ------------------------------------------------------------------ From 4dc17eb9a3876a8af8b2b8d93c344fa8db696d91 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Otto=20Boly=C3=B3s?= Date: Wed, 19 Aug 2026 08:40:07 +0200 Subject: [PATCH 4/7] =?UTF-8?q?fix(agent):=20harden=20AgentUuid=20resolver?= =?UTF-8?q?=20=E2=80=94=20reject=20Guid.Empty,=20redact=20warn=20values?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adopts the Ultrareview cycle 1 findings on the AgentUuid three-path resolver introduced earlier in this PR: * Reject the all-zero `Guid.Empty` value in `TryValidate`. Guid.TryParse is happy to accept "00000000-0000-0000-0000-000000000000" in every supported format, but adopting it as an agent's meta-device UUID would collide every agent in a fleet on the same identifier — RFC 4122 requires uniqueness "for the resource's entire lifetime". * Redact operator-supplied and persisted values from the fallback warning messages. The prior sanitized-echo approach still leaked up to 64 characters of a mispasted API key, bearer token, or password in the AgentUuid config slot, and only stripped CR/LF for log-injection defense (NEL, Unicode LINE / PARAGRAPH SEPARATOR, ANSI CSI, and other C0 control bytes passed through). Emitting only `length=N` closes both surfaces without a partial guard. * Delete the `SanitiseForLog` helper — there is no longer any raw value to sanitize, so its rename target, the truncation ellipsis, and every log-injection edge case are moot. * Hoist the duplicate `TryValidate(persistedUuid, …)` call so the fallback-kind label ("persisted"/"derived") and the Path 2 branch share one parse. * Rewrite the `AgentUuidResolver` and `TryValidate` XML-doc rationale. The prior "fails XSD validation on every typed enum/decimal DataItem" claim is inaccurate — every `UuidType` in the shipped v1.5, v1.8, and v2.7 device schemas is `xs:restriction base="xs:string"` with no pattern, so the schema silently accepts a non-UUID value. The normative anchors are the Part 1 prose "for its entire life" contract and cppagent parity; the doc now says so. * Update `docs/reference/configuration.md` — both `agentUuid` rows now describe the accepted formats, the validation and length-only warn contract, and the three-path fallback order. * Update `AgentUuidValidationTests` to match the redacted warn format (length assertion + never-echoes-raw-value + control-character log-safety guard, over an expanded parametric input set covering CR/LF/NEL/LINE-SEP/PARA-SEP/NUL/ANSI/TAB and paste-in-wrong-field secret shapes). Add a parametric case pinning `Guid.Empty` rejection across every Guid.TryParse format. * Format-fix trailing whitespace introduced on the two PR-touched source files that dotnet-format flagged. --- .../MTConnectAgentApplication.cs | 1 - docs/reference/configuration.md | 4 +- .../Agents/AgentUuidResolver.cs | 73 +++--- .../Agents/DeterministicAgentUuid.cs | 26 +- .../Agents/AgentUuidConfigOverrideTests.cs | 2 +- .../Agents/AgentUuidValidationTests.cs | 234 ++++++++---------- 6 files changed, 156 insertions(+), 184 deletions(-) diff --git a/agent/MTConnect.NET-Applications-Agents/MTConnectAgentApplication.cs b/agent/MTConnect.NET-Applications-Agents/MTConnectAgentApplication.cs index c56cf4ff5..9464a71db 100644 --- a/agent/MTConnect.NET-Applications-Agents/MTConnectAgentApplication.cs +++ b/agent/MTConnect.NET-Applications-Agents/MTConnectAgentApplication.cs @@ -573,7 +573,6 @@ public void StartAgent(IAgentApplicationConfiguration configuration, bool verbos } } - // Initilialize Processors _processors = new MTConnectAgentProcessors(configuration); _processors.ProcessorLoaded += ProcessorLoaded; diff --git a/docs/reference/configuration.md b/docs/reference/configuration.md index b76a2be12..0dbf7a2e6 100644 --- a/docs/reference/configuration.md +++ b/docs/reference/configuration.md @@ -56,7 +56,7 @@ Configuration for an MTConnect Http Agent Application | Key | C# property | Type | Description | | --- | --- | --- | --- | -| `agentUuid` | `AgentUuid` | `string` | Optional static UUID to assign to the Agent meta-device. When set, this value overrides the per-boot Guid.NewGuid() default applied by 's parameterless constructor and survives restarts without relying on agent.information.json being present on disk. Corresponds to AgentDeviceUUID in the cppagent reference implementation. Per MTConnect v2.7 XSD UuidType, the uuid identifies the element "for its entire life" — Header.instanceId is the per-boot discriminator. | +| `agentUuid` | `AgentUuid` | `string` | Optional static UUID to assign to the Agent meta-device. Must parse as an RFC 4122 UUID — any format accepts (hyphenated xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx, braced, parenthesised, bare-hex, hex-braced) is normalised to the canonical hyphenated form. Malformed or all-zero (Guid.Empty) values are rejected on startup with a warning that reports only the value's length (never the raw content), and the resolver falls through to the next path: (1) validated override, else (2) validated agent.information.json persisted state, else (3) a deterministic UUID v5 derived from ServiceName + machine name so the meta-device UUID survives restarts even in ephemeral-container deployments. Corresponds to AgentDeviceUUID in the cppagent reference implementation. Per MTConnect v2.7 XSD UuidType, the uuid identifies the element "for its entire life" — Header.instanceId is the per-boot discriminator. | | `configurationFileRestartInterval` | `ConfigurationFileRestartInterval` | `int` | Gets or Sets the minimum time (in seconds) between Agent restarts when MonitorConfigurationFiles is enabled | | `devices` | `Devices` | `string` | The Path to look for the file(s) that represent the Device Information Models to load into the Agent. The path can either be a single file or a directory. The path can be absolute or relative to the executable's directory | | `durable` | `Durable` | `bool` | Gets or Sets whether the Agent buffers are durable and retain state after restart | @@ -169,7 +169,7 @@ Configuration for an MTConnect Shdr > Http Agent | Key | C# property | Type | Description | | --- | --- | --- | --- | -| `agentUuid` | `AgentUuid` | `string` | Optional static UUID to assign to the Agent meta-device. When set, this value overrides the per-boot Guid.NewGuid() default and survives restarts without relying on agent.information.json being present on disk. Corresponds to AgentDeviceUUID in the cppagent reference implementation. | +| `agentUuid` | `AgentUuid` | `string` | Optional static UUID to assign to the Agent meta-device. Must parse as an RFC 4122 UUID (any -accepted format, normalised to the canonical hyphenated form). Malformed or all-zero values log a length-only warning and the resolver falls through to persisted agent.information.json state or a deterministic UUID v5 derived from ServiceName + machine name. Corresponds to AgentDeviceUUID in the cppagent reference implementation. | | `configurationFileRestartInterval` | `ConfigurationFileRestartInterval` | `int` | Gets or Sets the minimum time (in seconds) between Agent restarts when MonitorConfigurationFiles is enabled | | `devices` | `Devices` | `string` | The Path to look for the file(s) that represent the Device Information Models to load into the Agent. The path can either be a single file or a directory. The path can be absolute or relative to the executable's directory | | `durable` | `Durable` | `bool` | Gets or Sets whether the Agent buffers are durable and retain state after restart | diff --git a/libraries/MTConnect.NET-Common/Agents/AgentUuidResolver.cs b/libraries/MTConnect.NET-Common/Agents/AgentUuidResolver.cs index 2bb865452..769750b5e 100644 --- a/libraries/MTConnect.NET-Common/Agents/AgentUuidResolver.cs +++ b/libraries/MTConnect.NET-Common/Agents/AgentUuidResolver.cs @@ -2,6 +2,7 @@ // TrakHound Inc. licenses this file to you under the MIT license. using System; +using System.Globalization; namespace MTConnect.Agents { @@ -46,23 +47,19 @@ namespace MTConnect.Agents /// /// /// - /// Spec rationale — MTConnect Part 1 types the uuid attribute as the - /// UUID DataType (RFC 4122). Silently forwarding a non-UUID string - /// (from any source) breaks XSD validation on every typed enum/decimal - /// DataItem and diverges from the cppagent reference implementation, which - /// rejects malformed input at ingress. + /// Spec rationale — MTConnect Part 1 types the uuid attribute as + /// the UUID DataType (RFC 4122) and mandates that value remain + /// stable and unique for the agent's entire lifetime. Silently forwarding + /// a non-UUID string from any source diverges from that prose contract and + /// from the cppagent reference implementation, which rejects malformed + /// input at ingress; the current wire XSD types uuid only as + /// xs:string, so validation would silently pass a non-UUID value, + /// but downstream consumers that trust the DataType annotation would then + /// mis-key their aggregation and history stores. /// /// public static class AgentUuidResolver { - /// - /// Maximum length of an AgentUuid value echoed to the warning - /// log. Valid UUIDs are 32–38 characters; anything longer is almost - /// certainly a paste-in-wrong-field mistake (e.g. an API key) whose - /// full value should not persist in log archives. - /// - private const int LogValueMaxLength = 64; - /// /// Resolves the Agent meta-device UUID per the three-path algorithm. /// @@ -93,10 +90,12 @@ public static class AgentUuidResolver /// or Path 2 rejects malformed input. Kept as a plain /// so MTConnect.NET-Common does not /// take a hard dependency on any logging framework; the caller adapts - /// it to NLog, Serilog, or Microsoft.Extensions.Logging. Raw - /// values echoed via this delegate are pre-sanitised (CRLF stripped, - /// truncated to characters) to guard - /// against log-injection and secret leakage. + /// it to NLog, Serilog, or Microsoft.Extensions.Logging. The + /// message reports only the length of the rejected value — the + /// raw string is never echoed, so a mis-pasted API key, bearer token, + /// or other secret in the AgentUuid config slot cannot leak + /// into the log archive, and CR/LF or other control characters in the + /// rejected value cannot forge additional log lines. /// /// /// The canonical hyphenated RFC 4122 UUID string that the agent must @@ -115,47 +114,39 @@ public static string Resolve( return normalizedOverride; } - // Path 1 rejected but operator supplied something → warn. + // Hoist Path 2 validity + normalisation so Path 1's rejection + // warning can label the fallback kind without a second parse of + // the persisted value. + var persistedIsValid = DeterministicAgentUuid.TryValidate(persistedUuid, out var normalizedPersisted); + + // Path 1 rejected but operator supplied something → warn (length only). if (!string.IsNullOrEmpty(operatorSuppliedUuid)) { - var fallbackKind = DeterministicAgentUuid.TryValidate(persistedUuid, out _) - ? "persisted" - : "derived"; + var fallbackKind = persistedIsValid ? "persisted" : "derived"; warn?.Invoke(string.Format( - "AgentUuid override '{0}' is not a valid RFC 4122 UUID; falling back to {1} UUID.", - SanitiseForLog(operatorSuppliedUuid), + CultureInfo.InvariantCulture, + "AgentUuid override (length={0}) is not a valid RFC 4122 UUID; falling back to {1} UUID.", + operatorSuppliedUuid.Length, fallbackKind)); } // Path 2 — validated persisted state wins over derivation. - if (DeterministicAgentUuid.TryValidate(persistedUuid, out var normalizedPersisted)) + if (persistedIsValid) { return normalizedPersisted; } - // Path 2 rejected but persisted state carried something → warn. + // Path 2 rejected but persisted state carried something → warn (length only). if (!string.IsNullOrEmpty(persistedUuid)) { - warn?.Invoke( - "Persisted AgentUuid in agent.information.json is not a valid RFC 4122 UUID; falling back to derived UUID."); + warn?.Invoke(string.Format( + CultureInfo.InvariantCulture, + "Persisted AgentUuid in agent.information.json (length={0}) is not a valid RFC 4122 UUID; falling back to derived UUID.", + persistedUuid.Length)); } // Path 3 — deterministic derivation. return DeterministicAgentUuid.Derive(agentName, hostname, port: 0); } - - /// - /// Strips CR/LF (log-injection guard) and truncates to - /// characters (secret-leakage guard) - /// before echoing an operator-supplied value to the warning log. - /// - private static string SanitiseForLog(string value) - { - if (string.IsNullOrEmpty(value)) return value; - var stripped = value.Replace("\r", string.Empty).Replace("\n", string.Empty); - return stripped.Length > LogValueMaxLength - ? stripped.Substring(0, LogValueMaxLength) + "…" - : stripped; - } } } diff --git a/libraries/MTConnect.NET-Common/Agents/DeterministicAgentUuid.cs b/libraries/MTConnect.NET-Common/Agents/DeterministicAgentUuid.cs index a2cac9c9b..95fb87f03 100644 --- a/libraries/MTConnect.NET-Common/Agents/DeterministicAgentUuid.cs +++ b/libraries/MTConnect.NET-Common/Agents/DeterministicAgentUuid.cs @@ -139,15 +139,19 @@ private static byte[] BigEndianToGuidBytes(byte[] beBytes) /// braced "B", parenthesized "P", bare-hex "N", or hex-braced "X" — and /// returns the canonical hyphenated "D" form so the wire representation /// is stable regardless of input format. Rejects , - /// empty, whitespace-only, and unparseable inputs. + /// empty, whitespace-only, unparseable inputs, and the all-zero + /// value (a fleet-wide UUID collision hazard + /// if adopted by more than one agent). /// /// Motivation: MTConnect Part 1 types the uuid attribute as the - /// UUID DataType (RFC 4122). Silently forwarding a non-UUID - /// string emits wire content that fails XSD validation on any typed - /// enum/decimal DataItem and breaks parity with the cppagent reference - /// implementation. Callers that supply malformed input should log a - /// warning and fall through to persisted or derived UUIDs — the - /// three-path resolution in . + /// UUID DataType (RFC 4122), and requires that value to remain + /// stable and unique for the agent's entire lifetime. Silently + /// forwarding a non-UUID string diverges from the Part 1 prose + /// contract and from the cppagent reference implementation, which + /// rejects malformed input at ingress. Callers that supply malformed + /// input should log a warning and fall through to persisted or + /// derived UUIDs — the three-path resolution in + /// . /// /// /// @@ -161,15 +165,17 @@ private static byte[] BigEndianToGuidBytes(byte[] beBytes) /// on failure. /// /// - /// if parses as an - /// RFC 4122 UUID; for , - /// empty, whitespace-only, or unparseable inputs. + /// if parses as a + /// non-empty RFC 4122 UUID; for + /// , empty, whitespace-only, unparseable, or + /// all-zero () inputs. /// public static bool TryValidate(string input, out string normalized) { normalized = null; if (string.IsNullOrWhiteSpace(input)) return false; if (!Guid.TryParse(input, out var parsed)) return false; + if (parsed == Guid.Empty) return false; normalized = parsed.ToString(); return true; } diff --git a/tests/MTConnect.NET-Common-Tests/Agents/AgentUuidConfigOverrideTests.cs b/tests/MTConnect.NET-Common-Tests/Agents/AgentUuidConfigOverrideTests.cs index b8951378b..e3f4a12f3 100644 --- a/tests/MTConnect.NET-Common-Tests/Agents/AgentUuidConfigOverrideTests.cs +++ b/tests/MTConnect.NET-Common-Tests/Agents/AgentUuidConfigOverrideTests.cs @@ -118,7 +118,7 @@ public void AgentUuid_set_in_config_flows_through_to_Agent_uuid() public void AgentUuid_set_in_config_takes_precedence_over_state_file() { const string FromStateFileUuid = "22222222-2222-4222-8222-222222222222"; - const string FromConfigUuid = "33333333-3333-4333-8333-333333333333"; + const string FromConfigUuid = "33333333-3333-4333-8333-333333333333"; // Pre-write the state file with a stale (but valid) UUID. var preexisting = new MTConnectAgentInformation(FromStateFileUuid); diff --git a/tests/MTConnect.NET-Common-Tests/Agents/AgentUuidValidationTests.cs b/tests/MTConnect.NET-Common-Tests/Agents/AgentUuidValidationTests.cs index 482dd23f3..2c8dc5c75 100644 --- a/tests/MTConnect.NET-Common-Tests/Agents/AgentUuidValidationTests.cs +++ b/tests/MTConnect.NET-Common-Tests/Agents/AgentUuidValidationTests.cs @@ -20,17 +20,20 @@ namespace MTConnect.Tests.Common.Agents /// /// /// Silently forwarding a non-UUID string violates MTConnect Part 1, which - /// types the uuid attribute as the UUID DataType (RFC 4122 - /// enumerated string token). Any downstream XSD-validating consumer - /// (cppagent parity, MQTT/JSON-cppagent transport) rejects the resulting - /// wire content on typed enum/decimal DataItems. + /// types the uuid attribute as the UUID DataType (RFC 4122) + /// and mandates that value remain stable and unique for the agent's entire + /// lifetime. The wire XSD currently types uuid only as + /// xs:string, so schema validation would silently accept a + /// malformed value; the cppagent reference implementation and every + /// downstream consumer that trusts the DataType annotation for aggregation + /// or historical keying reject the resulting wire content. /// /// /// /// Fixture drives directly — the /// same method MTConnectAgentApplication.StartAgent calls — so /// production and tests cannot silently diverge on branch order, guard - /// semantics, or normalisation output. + /// semantics, or normalization output. /// /// [TestFixture] @@ -121,6 +124,28 @@ public void TryValidate_unparseable_returns_false(string input) Assert.That(normalized, Is.Null); } + /// + /// rejects the + /// all-zero value across every accepted + /// input format — is + /// happy to parse "00000000-…", but adopting it as an agent's meta + /// UUID would collide every agent in a fleet on the same identifier, + /// which the RFC 4122 "unique for the resource's entire lifetime" + /// contract disallows. + /// + [TestCase("00000000-0000-0000-0000-000000000000")] + [TestCase("{00000000-0000-0000-0000-000000000000}")] + [TestCase("(00000000-0000-0000-0000-000000000000)")] + [TestCase("00000000000000000000000000000000")] + public void TryValidate_all_zero_guid_returns_false(string input) + { + var ok = DeterministicAgentUuid.TryValidate(input, out var normalized); + + Assert.That(ok, Is.False, + "Guid.Empty is a fleet-collision hazard — TryValidate must reject it on every accepted format."); + Assert.That(normalized, Is.Null); + } + /// /// accepts the /// canonical hyphenated "D" form and returns it unchanged. @@ -474,7 +499,10 @@ public void Warn_message_names_persisted_when_override_bad_and_persisted_valid() Assert.That(messages, Has.Count.EqualTo(1), "One warn — for the rejected operator override; the valid persisted UUID needs no warn."); Assert.That(messages[0], Does.Contain("AgentUuid override")); - Assert.That(messages[0], Does.Contain(Malformed)); + Assert.That(messages[0], Does.Contain($"length={Malformed.Length}"), + "Warn reports length only — the raw value must never appear (secret-leakage guard)."); + Assert.That(messages[0], Does.Not.Contain(Malformed), + "Warn must not echo the raw operator-supplied value."); Assert.That(messages[0], Does.Contain("falling back to persisted UUID")); } @@ -529,9 +557,14 @@ public void Warn_delegate_invoked_twice_when_both_override_and_persisted_are_mal Assert.That(messages, Has.Count.EqualTo(2), "Two rejected sources → two warns; the second must not be swallowed."); Assert.That(messages[0], Does.Contain("AgentUuid override")); - Assert.That(messages[0], Does.Contain(BadOverride)); + Assert.That(messages[0], Does.Contain($"length={BadOverride.Length}")); + Assert.That(messages[0], Does.Not.Contain(BadOverride), + "Raw operator value must never appear in the warn message."); Assert.That(messages[0], Does.Contain("falling back to derived UUID")); Assert.That(messages[1], Does.Contain("Persisted AgentUuid")); + Assert.That(messages[1], Does.Contain($"length={BadPersisted.Length}")); + Assert.That(messages[1], Does.Not.Contain(BadPersisted), + "Raw persisted-state value must never appear in the warn message."); Assert.That(messages[1], Does.Contain("falling back to derived UUID")); } @@ -598,95 +631,38 @@ public void Default_warn_argument_omitted_does_not_throw() } // ------------------------------------------------------------------ - // SanitiseForLog — CRLF stripping (log-injection guard) and - // truncation-at-boundary behaviour (secret-leakage guard). Exercised - // indirectly via the operator-override warn message content, which - // is the only route that pipes user input through the sanitiser. + // Redaction — the warn message reports the LENGTH of the rejected + // value and never echoes its raw content. This is the single guard + // that closes both log-injection (embedded CR/LF/U+2028/ANSI escapes + // cannot forge a new log line if the value never appears) and + // secret-leakage (a mis-pasted API key, bearer token, or password + // in the AgentUuid config slot cannot land in the log archive). // ------------------------------------------------------------------ /// - /// SanitiseForLog strips CR and LF characters from operator - /// input before it appears in the warn message — defends against - /// log-injection (forged log lines) where a hostile operator embeds - /// \r\n in the config file. + /// The Path 1 warn message must report the length of the rejected + /// operator override, must not echo any character of the raw value, + /// and must not contain any control character regardless of what the + /// operator supplied — a single redaction guard that closes both + /// log-injection (no CR/LF/U+2028/ANSI/control byte can appear if the + /// value itself is never echoed) and secret leakage (a mis-pasted + /// API key or bearer token in the AgentUuid slot cannot land in + /// archives). /// - [Test] - public void Warn_message_strips_CR_LF_from_operator_supplied_value() + [TestCase("bad-uuid\r\nFAKE-LOG-LINE-INJECTED")] + [TestCase("bad\rvalue")] + [TestCase("bad\nvalue")] + [TestCase("bad\u2028value")] // Unicode LINE SEPARATOR + [TestCase("bad\u2029value")] // Unicode PARAGRAPH SEPARATOR + [TestCase("bad\u0085value")] // NEL + [TestCase("bad\x1b[2Jvalue")] // ANSI CSI clear-screen + [TestCase("bad\x00value")] // NUL + [TestCase("bad\tvalue")] // TAB + [TestCase("ghp_abcdef0123456789abcdef0123456789abcd")] // paste-in-wrong-field secret shape + [TestCase("Bearer eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.payload.sig")] + public void Warn_message_never_echoes_raw_operator_value_and_reports_length(string input) { var messages = new List(); - const string Injected = "bad-uuid\r\nFAKE-LOG-LINE-INJECTED"; - - _ = AgentUuidResolver.Resolve( - operatorSuppliedUuid: Injected, - persistedUuid: null, - agentName: "test-agent", - hostname: "test-host", - warn: messages.Add); - - Assert.That(messages, Has.Count.EqualTo(1)); - Assert.That(messages[0], Does.Not.Contain("\r")); - Assert.That(messages[0], Does.Not.Contain("\n")); - Assert.That(messages[0], Does.Contain("bad-uuidFAKE-LOG-LINE-INJECTED"), - "CR and LF must be stripped inline — surrounding characters remain."); - } - - /// - /// SanitiseForLog strips a lone CR (Mac Classic line-ending) - /// as well as CRLF pairs. Pins the guard against callers that split - /// on either terminator. - /// - [Test] - public void Warn_message_strips_lone_CR_from_operator_supplied_value() - { - var messages = new List(); - const string Injected = "bad\rvalue"; - - _ = AgentUuidResolver.Resolve( - operatorSuppliedUuid: Injected, - persistedUuid: null, - agentName: "test-agent", - hostname: "test-host", - warn: messages.Add); - - Assert.That(messages, Has.Count.EqualTo(1)); - Assert.That(messages[0], Does.Not.Contain("\r")); - Assert.That(messages[0], Does.Contain("badvalue")); - } - - /// - /// SanitiseForLog strips a lone LF as well, matching the - /// symmetric CR treatment. - /// - [Test] - public void Warn_message_strips_lone_LF_from_operator_supplied_value() - { - var messages = new List(); - const string Injected = "bad\nvalue"; - - _ = AgentUuidResolver.Resolve( - operatorSuppliedUuid: Injected, - persistedUuid: null, - agentName: "test-agent", - hostname: "test-host", - warn: messages.Add); - - Assert.That(messages, Has.Count.EqualTo(1)); - Assert.That(messages[0], Does.Not.Contain("\n")); - Assert.That(messages[0], Does.Contain("badvalue")); - } - - /// - /// SanitiseForLog does NOT truncate values at or below the - /// 64-character LogValueMaxLength boundary — pins the exact - /// boundary against off-by-one regressions on the length check. - /// - [Test] - public void Warn_message_does_not_truncate_at_exactly_max_length() - { - var messages = new List(); - // Exactly 64 chars, none of which is a valid UUID character - // pattern → guaranteed rejection by TryValidate. - var input = new string('z', 64); _ = AgentUuidResolver.Resolve( operatorSuppliedUuid: input, @@ -696,63 +672,63 @@ public void Warn_message_does_not_truncate_at_exactly_max_length() warn: messages.Add); Assert.That(messages, Has.Count.EqualTo(1)); - Assert.That(messages[0], Does.Contain(input), - "A value at exactly the boundary must appear verbatim."); - Assert.That(messages[0], Does.Not.Contain("…"), - "No ellipsis must be appended at exactly the boundary."); + Assert.That(messages[0], Does.Contain("AgentUuid override")); + Assert.That(messages[0], Does.Contain($"length={input.Length}"), + "Warn must report the input length so operators can spot a length-mismatch mispaste."); + Assert.That(messages[0], Does.Not.Contain(input), + "Warn must never echo the raw operator value — closes secret-leakage."); + AssertLogSafe(messages[0]); } /// - /// SanitiseForLog truncates values longer than 64 characters - /// and appends a single ellipsis (…) so the operator sees the - /// prefix but a leaked API-key-length string is not archived in - /// full. Pins the 65-char over-boundary case. + /// The Path 2 (persisted state) warn message mirrors the Path 1 + /// guarantees — reports length, never echoes the raw persisted + /// value, and contains no control characters. The persisted-state + /// file may carry any garbage a prior agent version wrote, so the + /// same redaction discipline applies. /// - [Test] - public void Warn_message_truncates_over_max_length_with_ellipsis() + [TestCase("garbage-persisted-value")] + [TestCase("truncated-uuid-write\rfromacrashedboot")] + [TestCase("ghk_secret_that_leaked_into_state_file_somehow")] + public void Warn_message_for_persisted_state_never_echoes_raw_value_and_reports_length(string input) { var messages = new List(); - // 65 chars, one over the boundary. - var input = new string('z', 65); _ = AgentUuidResolver.Resolve( - operatorSuppliedUuid: input, - persistedUuid: null, + operatorSuppliedUuid: null, + persistedUuid: input, agentName: "test-agent", hostname: "test-host", warn: messages.Add); Assert.That(messages, Has.Count.EqualTo(1)); - Assert.That(messages[0], Does.Contain(new string('z', 64) + "…"), - "The first 64 characters must be preserved and an ellipsis appended."); - Assert.That(messages[0], Does.Not.Contain(new string('z', 65)), - "The full 65-character input must NOT appear verbatim."); + Assert.That(messages[0], Does.Contain("Persisted AgentUuid")); + Assert.That(messages[0], Does.Contain($"length={input.Length}")); + Assert.That(messages[0], Does.Not.Contain(input), + "Persisted-state warn must not echo the raw file content."); + AssertLogSafe(messages[0]); } /// - /// SanitiseForLog's truncation applies AFTER CRLF stripping — - /// a value whose raw length is over the boundary but whose stripped - /// length falls within it must NOT be truncated. Pins the compose - /// order of the two sanitiser steps. + /// Log-injection guard shared between the operator-override and + /// persisted-state warn assertions — every character in the message + /// must be either printable (>= U+0020) or a plain TAB (U+0009). + /// Rejects CR, LF, NEL, LINE / PARAGRAPH SEPARATOR, ANSI CSI, NUL + /// and every other C0 control byte, guarding against a regression + /// that funnels raw input back into the message. /// - [Test] - public void Warn_message_measures_length_after_CRLF_stripping() + private static void AssertLogSafe(string message) { - var messages = new List(); - // 64 z's interleaved with 10 CRLFs (raw length 84, stripped length 64). - var input = new string('z', 64) + "\r\n\r\n\r\n\r\n\r\n"; - - _ = AgentUuidResolver.Resolve( - operatorSuppliedUuid: input, - persistedUuid: null, - agentName: "test-agent", - hostname: "test-host", - warn: messages.Add); - - Assert.That(messages, Has.Count.EqualTo(1)); - Assert.That(messages[0], Does.Contain(new string('z', 64))); - Assert.That(messages[0], Does.Not.Contain("…"), - "Length is measured after CRLF stripping — no ellipsis needed here."); + foreach (var c in message) + { + Assert.That( + c == '\t' || c >= ' ', + Is.True, + $"Warn message contains disallowed control character U+{((int)c):X4}."); + Assert.That(c, Is.Not.EqualTo('\u0085'), "NEL leaked into warn."); + Assert.That(c, Is.Not.EqualTo('\u2028'), "LINE SEPARATOR leaked into warn."); + Assert.That(c, Is.Not.EqualTo('\u2029'), "PARAGRAPH SEPARATOR leaked into warn."); + } } // ------------------------------------------------------------------ From 998dcc974b45395901deac664256c0c3a4a95d0d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Otto=20Boly=C3=B3s?= Date: Wed, 19 Aug 2026 16:08:53 +0200 Subject: [PATCH 5/7] test(common-tests): pin AgentUuid TryValidate boundaries, port axis, RFC 4122 variant bits MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Coverage-FLOOR characterization tests — closes the boundary / bit-layout / matrix gaps left after PR #220's initial pass. All tests characterise already-correct behavior, so they land GREEN; RED-first ordering does not apply to pure coverage-pin commits. AgentUuidValidationTests.cs (+144 LOC): - TryValidate leading/trailing whitespace (space, tab, CRLF) is trimmed by Guid.TryParse — pin the delegated contract so a swap to Guid.TryParseExact would fail the harness. - TryValidate mixed-case hex accepted and normalized to lowercase (companion to the uppercase test). - TryValidate interior control / whitespace (NUL, CRLF, space, tab embedded in the middle) is rejected — outer-only trim contract. - TryValidate overlong (valid prefix + 200-char tail) is rejected — full-string match required. - TryValidate trailing CRLF+injected-text is rejected — parse layer refuses the classic log-injection payload shape even though the warn redaction guard already closes the log-line-forgery risk. - Resolve valid override + malformed persisted → Path 1 short-circuits; no warn is emitted for the malformed persisted value. Closes the 3x3 override/persisted matrix cell not covered by the existing warn-count assertions. AgentUuidDeterministicDefaultTests.cs (+79 LOC): - DeriveFromSeed output has the RFC 4122 variant high bits set to 0b10 in octet 9 (clock_seq_hi_and_reserved). Extends the sibling version-digit test to a full RFC 4122 §4.3 bit-layout characterization. - Derive port change (5000 vs 8080) produces a different UUID for the same agent name — pins the "agent:name:port" seed contract. - Derive port: 0 sentinel does NOT collide with port: 1 — pins the sentinel's uniqueness so a regression that treats 0 as "omit" cannot silently collide with a real port-1 deployment. --- .../AgentUuidDeterministicDefaultTests.cs | 79 ++++++++++ .../Agents/AgentUuidValidationTests.cs | 144 ++++++++++++++++++ 2 files changed, 223 insertions(+) diff --git a/tests/MTConnect.NET-Common-Tests/Agents/AgentUuidDeterministicDefaultTests.cs b/tests/MTConnect.NET-Common-Tests/Agents/AgentUuidDeterministicDefaultTests.cs index a38c16877..6cca6c9d2 100644 --- a/tests/MTConnect.NET-Common-Tests/Agents/AgentUuidDeterministicDefaultTests.cs +++ b/tests/MTConnect.NET-Common-Tests/Agents/AgentUuidDeterministicDefaultTests.cs @@ -195,5 +195,84 @@ public void Default_agent_uuid_changes_when_agentName_changes() Assert.That(uuidA, Is.Not.EqualTo(uuidB), "Different agentName values must produce distinct UUID v5 values."); } + + // --------------------------------------------------------------- + // RFC 4122 §4.3 bit-layout invariants — the sibling test pins + // only the version digit; these extend the pin to the variant + // bits (byte 8) that + // sets to 0b10xx_xxxx. Together the two tests characterise + // every masked bit in the RFC 4122 §4.3 layout. + // --------------------------------------------------------------- + + /// + /// The 9th octet of a derived UUID v5 must have its top two bits + /// set to 10 — the "RFC 4122 variant" that + /// stamps into + /// clock_seq_hi_and_reserved. Decodes the third hex byte of + /// the 4th hyphen group and asserts the top two bits directly, so + /// a regression that swaps the mask (e.g. 0x40 for the + /// deprecated NCS variant) is caught by an obvious contradiction. + /// + [Test] + public void DeriveFromSeed_output_has_RFC_4122_variant_high_bits_10() + { + var uuid = DeterministicAgentUuid.DeriveFromSeed("example.com"); + + var parts = uuid.Split('-'); + Assert.That(parts.Length, Is.EqualTo(5)); + + // Group 4 is clock_seq_hi_and_reserved (1 byte) + clock_seq_low (1 byte). + // The variant bits sit in the top 2 bits of clock_seq_hi_and_reserved, + // i.e. the first hex byte of parts[3]. + var clockSeqHi = Convert.ToByte(parts[3].Substring(0, 2), 16); + var variantBits = (clockSeqHi & 0xC0) >> 6; + + Assert.That(variantBits, Is.EqualTo(0b10), + $"RFC 4122 variant requires top two bits of octet 9 = '10'; got 0x{clockSeqHi:X2} " + + $"(variant bits = 0b{Convert.ToString(variantBits, 2).PadLeft(2, '0')})."); + } + + /// + /// Deterministic derivation must be sensitive to the port + /// component of the seed: same agentName and + /// hostname but different port ⇒ different UUID. + /// Pins the port participation the class doc-comment promises + /// ("agent:name:port") — a regression that drops the port + /// from the seed would collide co-located agents on the same + /// host + name that only differ by listener port. + /// + [Test] + public void Derive_port_change_produces_different_uuid_for_same_agent_name() + { + const string AgentName = "fixture-det-agent-port"; + const string Hostname = "canonical-host"; + + var derivedAt5000 = DeterministicAgentUuid.Derive(AgentName, Hostname, port: 5000); + var derivedAt8080 = DeterministicAgentUuid.Derive(AgentName, Hostname, port: 8080); + + Assert.That(derivedAt5000, Is.Not.EqualTo(derivedAt8080), + "Changing the port MUST change the derived UUID — port is part of the seed contract."); + } + + /// + /// port: 0 — the documented sentinel used when the listener + /// port is not available at the call site — must not collide with + /// any positive port for the same agentName / hostname. + /// Pins the sentinel's uniqueness in the port axis so a regression + /// that treats 0 as "omit" cannot silently collide with a + /// real port-1 deployment. + /// + [Test] + public void Derive_port_zero_sentinel_does_not_collide_with_any_positive_port() + { + const string AgentName = "fixture-det-agent-port-zero"; + const string Hostname = "canonical-host"; + + var derivedAtZero = DeterministicAgentUuid.Derive(AgentName, Hostname, port: 0); + var derivedAtOne = DeterministicAgentUuid.Derive(AgentName, Hostname, port: 1); + + Assert.That(derivedAtZero, Is.Not.EqualTo(derivedAtOne), + "port: 0 sentinel must be distinct from port: 1 — 0 is not silently 'omit'."); + } } } diff --git a/tests/MTConnect.NET-Common-Tests/Agents/AgentUuidValidationTests.cs b/tests/MTConnect.NET-Common-Tests/Agents/AgentUuidValidationTests.cs index 2c8dc5c75..797d30164 100644 --- a/tests/MTConnect.NET-Common-Tests/Agents/AgentUuidValidationTests.cs +++ b/tests/MTConnect.NET-Common-Tests/Agents/AgentUuidValidationTests.cs @@ -817,6 +817,150 @@ public void Malformed_persisted_state_survives_JSON_round_trip_and_is_rejected() Assert.That(messages[0], Does.Contain("Persisted AgentUuid")); } + // ------------------------------------------------------------------ + // TryValidate boundary characterisation — pins the delegated + // Guid.TryParse contract on the input classes the resolver may + // receive from operators or a hand-edited state file. Closes the + // §1.0d-trigies-novodecies coverage FLOOR on boundary classes. + // ------------------------------------------------------------------ + + /// + /// delegates to + /// , which trims leading + /// and trailing ASCII whitespace (space, tab, CR, LF). A copy-pasted + /// value with stray whitespace is accepted and normalised to the + /// canonical hyphenated "D" form. Pins that delegated contract so a + /// future .NET runtime tightening (or a swap to + /// Guid.TryParseExact) does not silently reject the input + /// class operators most commonly produce. + /// + [TestCase(" 6ba7b810-9dad-11d1-80b4-00c04fd430c8")] + [TestCase("6ba7b810-9dad-11d1-80b4-00c04fd430c8 ")] + [TestCase(" 6ba7b810-9dad-11d1-80b4-00c04fd430c8 ")] + [TestCase("\t6ba7b810-9dad-11d1-80b4-00c04fd430c8\r\n")] + public void TryValidate_leading_and_trailing_whitespace_is_trimmed_and_normalised(string input) + { + const string Expected = "6ba7b810-9dad-11d1-80b4-00c04fd430c8"; + + var ok = DeterministicAgentUuid.TryValidate(input, out var normalized); + + Assert.That(ok, Is.True, + "Guid.TryParse trims leading/trailing whitespace — TryValidate must forward that acceptance."); + Assert.That(normalized, Is.EqualTo(Expected), + "The trimmed value must round-trip to the canonical D form."); + } + + /// + /// Mixed-case hex is accepted (RFC 4122 case-insensitive) and the + /// output is normalised to lowercase so the wire representation is + /// stable regardless of how the operator typed the value. Companion + /// to . + /// + [Test] + public void TryValidate_mixed_case_hex_is_accepted_and_normalised_to_lowercase() + { + const string MixedCase = "6Ba7B810-9dAd-11D1-80B4-00c04Fd430c8"; + const string Expected = "6ba7b810-9dad-11d1-80b4-00c04fd430c8"; + + var ok = DeterministicAgentUuid.TryValidate(MixedCase, out var normalized); + + Assert.That(ok, Is.True); + Assert.That(normalized, Is.EqualTo(Expected)); + } + + /// + /// Inputs with an embedded control character in the middle of the + /// UUID text (NUL, CRLF, space) must be rejected — trimming only + /// removes outer whitespace, so any interior control byte breaks + /// the parse. Closes the "malformed-in-the-middle" boundary that + /// the redaction guard already handles at the warn layer. + /// + [TestCase("6ba7b810-9dad-11d1-80b4-00c04fd430c8\0")] // trailing NUL — Guid.TryParse rejects + [TestCase("6ba7b810\09dad-11d1-80b4-00c04fd430c8")] // embedded NUL + [TestCase("6ba7b810\r\n9dad-11d1-80b4-00c04fd430c8")] // embedded CRLF + [TestCase("6ba7b810 9dad-11d1-80b4-00c04fd430c8")] // embedded space + [TestCase("6ba7b810\t9dad-11d1-80b4-00c04fd430c8")] // embedded tab + public void TryValidate_interior_control_or_whitespace_is_rejected(string input) + { + var ok = DeterministicAgentUuid.TryValidate(input, out var normalized); + + Assert.That(ok, Is.False, + "Guid.TryParse only trims outer whitespace; interior control bytes must be rejected."); + Assert.That(normalized, Is.Null); + } + + /// + /// Overlong inputs — a valid UUID prefix followed by trailing + /// garbage — must be rejected. Closes the boundary class the + /// existing "…-extra" case sketches but with a much longer + /// tail more typical of a mis-pasted secret / bearer token. + /// + [Test] + public void TryValidate_overlong_input_with_valid_prefix_is_rejected() + { + var overlong = "6ba7b810-9dad-11d1-80b4-00c04fd430c8" + new string('x', 200); + + var ok = DeterministicAgentUuid.TryValidate(overlong, out var normalized); + + Assert.That(ok, Is.False, + "A valid UUID prefix + trailing garbage must not parse — full-string match required."); + Assert.That(normalized, Is.Null); + } + + /// + /// Trailing CR/LF followed by extra text (the classic log-injection + /// payload shape) must be rejected at the parse layer — the redaction + /// guard closes the log-line-forgery risk downstream, but the parse + /// layer must also refuse to canonicalise the value so it never + /// reaches the wire. + /// + [Test] + public void TryValidate_trailing_crlf_with_injected_text_is_rejected() + { + const string Injected = "6ba7b810-9dad-11d1-80b4-00c04fd430c8\r\nFAKE-LOG-INJECTED"; + + var ok = DeterministicAgentUuid.TryValidate(Injected, out var normalized); + + Assert.That(ok, Is.False); + Assert.That(normalized, Is.Null); + } + + // ------------------------------------------------------------------ + // Resolve Path-1-wins-with-malformed-Path-2 — covers the branch + // where a valid operator override short-circuits BEFORE the + // persisted-state validation runs, so no warn is emitted for the + // malformed persisted value. Closes the 3x3 override/persisted + // matrix cell not covered by the existing warn-count tests. + // ------------------------------------------------------------------ + + /// + /// When Path 1 (operator override) is valid AND Path 2 (persisted + /// state) is malformed, the resolver returns the override + /// immediately without touching the persisted-state validation — + /// so the warn delegate MUST NOT fire for the malformed persisted + /// value. Pins the early-return short-circuit at + /// line 114. + /// + [Test] + public void Resolve_valid_override_short_circuits_and_ignores_malformed_persisted_without_warn() + { + var messages = new List(); + const string ValidOverride = "6ba7b810-9dad-11d1-80b4-00c04fd430c8"; + const string MalformedPersist = "definitely-not-a-uuid"; + + var resolved = AgentUuidResolver.Resolve( + operatorSuppliedUuid: ValidOverride, + persistedUuid: MalformedPersist, + agentName: "test-agent", + hostname: "test-host", + warn: messages.Add); + + Assert.That(resolved, Is.EqualTo(ValidOverride), + "Valid override wins Path 1; persisted state is never consulted."); + Assert.That(messages, Is.Empty, + "Path 1 short-circuits before the persisted-state warn arm — malformed persisted must be silent."); + } + // ------------------------------------------------------------------ // Boot-simulation helpers — thin wrappers around AgentUuidResolver. // ------------------------------------------------------------------ From 09def57bbbcce7c964e4735214fa58f08f27583c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Otto=20Boly=C3=B3s?= Date: Wed, 19 Aug 2026 16:20:07 +0200 Subject: [PATCH 6/7] =?UTF-8?q?fix(agent):=20apply=20Ultrareview=20finding?= =?UTF-8?q?s=20=E2=80=94=20XML=20docs,=20trim,=20length=20cap,=20Derive=20?= =?UTF-8?q?guard?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Applies the fix-worthy findings from the PR #220 Ultrareview cycle: - F-DOC-001 / F-DOC-002 (HIGH): update stale XML doc summaries on AgentApplicationConfiguration.AgentUuid + IAgentApplicationConfiguration.AgentUuid to mirror the PR220 three-path resolution semantics. Regenerating docs/reference/configuration.md via MTConnect.NET-DocsGen picks up the new prose and closes the Configuration_Page_Is_In_Sync_With_Source docs-sync test failure that the pre-hardening XML docs caused. - F-IMP-001 (MEDIUM): DeterministicAgentUuid.TryValidate trims surrounding whitespace before parsing so a trailing newline / YAML indent / copy-paste padding does not silently reject an otherwise-valid UUID; the local trim also keeps the length cap defense meaningful against padded input and isolates the resolver from any future .NET runtime tightening around Guid.TryParse's implicit trim. - F-SEC-002 (LOW): TryValidate rejects inputs longer than 72 characters (longest RFC 4122 textual form + slack) before invoking Guid.TryParse so a pasted-in mega-payload cannot cost megabytes of parse state. - F-IMP-002 (MEDIUM): MTConnectAgentApplication.StartAgent emits an Info log line with the resolved meta-device UUID so operators no longer have to reproduce the resolver's three-path decision from configuration state during field-support triage. - F-IMP-004 (LOW): DeterministicAgentUuid.Derive throws ArgumentException when both agentName and hostname are null / empty so a misconfigured caller cannot silently produce a fleet-wide constant UUID. - F-CR-004 (LOW): AgentUuidResolver.Resolve warn message broadened to "not an acceptable RFC 4122 UUID (must be non-empty, parseable, and not the all-zero nil UUID)" so operators debugging a Guid.Empty rejection do not chase a parse failure that never happened — Guid.Empty IS RFC 4122 §4.1.7-defined but rejected as a fleet-wide collision hazard. - F-CR-002 / F-CR-003 (LOW): AmE typography drift closed in AgentUuidResolver.cs comment + three AgentUuidValidationTests.cs test method identifiers (normalised → normalized) and one XML-doc cross-ref (behaviour → behavior, parenthesised → parenthesized) per project convention (AmE spelling is universal in MTConnect.NET canonical surfaces). Adds tests pinning the new behavior: - TryValidate_input_one_past_length_cap_is_rejected — 73-char input boundary. - TryValidate_mega_payload_is_rejected_before_parse — 10 KB DoS defense. - Resolve_warn_on_nil_uuid_uses_broad_acceptable_wording — Guid.Empty warn wording pin. - Derive_throws_when_both_agent_name_and_hostname_are_empty — 4 cases covering the [null,null] / [null,""] / ["",null] / ["",""] matrix. - Derive_accepts_null_or_empty_hostname_when_agent_name_supplied — 2 cases confirming the non-empty-name path still succeeds. Runs on top of test-coverage-audit's c0d9f546 boundary/enum/variant pin. --- .../MTConnectAgentApplication.cs | 9 ++ docs/reference/configuration.md | 4 +- .../Agents/AgentUuidResolver.cs | 10 +- .../Agents/DeterministicAgentUuid.cs | 34 +++- .../AgentApplicationConfiguration.cs | 10 +- .../IAgentApplicationConfiguration.cs | 6 +- .../Agents/AgentUuidValidationTests.cs | 146 ++++++++++++++++-- 7 files changed, 180 insertions(+), 39 deletions(-) diff --git a/agent/MTConnect.NET-Applications-Agents/MTConnectAgentApplication.cs b/agent/MTConnect.NET-Applications-Agents/MTConnectAgentApplication.cs index 9464a71db..1c5d71a4d 100644 --- a/agent/MTConnect.NET-Applications-Agents/MTConnectAgentApplication.cs +++ b/agent/MTConnect.NET-Applications-Agents/MTConnectAgentApplication.cs @@ -450,6 +450,15 @@ public void StartAgent(IAgentApplicationConfiguration configuration, bool verbos hostname: System.Environment.MachineName, warn: message => _applicationLogger?.Warn(message)); + // Happy-path visibility: operators reading the startup log after + // a UUID-related field-support ticket should see which UUID the + // agent adopted without having to reproduce the resolver's + // three-path decision from configuration state. + _applicationLogger?.Info(string.Format( + System.Globalization.CultureInfo.InvariantCulture, + "Agent meta-device UUID resolved: {0}", + agentInformation.Uuid)); + // Create Observation File Buffer if (configuration.Durable) { diff --git a/docs/reference/configuration.md b/docs/reference/configuration.md index 0dbf7a2e6..d64ec4911 100644 --- a/docs/reference/configuration.md +++ b/docs/reference/configuration.md @@ -56,7 +56,7 @@ Configuration for an MTConnect Http Agent Application | Key | C# property | Type | Description | | --- | --- | --- | --- | -| `agentUuid` | `AgentUuid` | `string` | Optional static UUID to assign to the Agent meta-device. Must parse as an RFC 4122 UUID — any format accepts (hyphenated xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx, braced, parenthesised, bare-hex, hex-braced) is normalised to the canonical hyphenated form. Malformed or all-zero (Guid.Empty) values are rejected on startup with a warning that reports only the value's length (never the raw content), and the resolver falls through to the next path: (1) validated override, else (2) validated agent.information.json persisted state, else (3) a deterministic UUID v5 derived from ServiceName + machine name so the meta-device UUID survives restarts even in ephemeral-container deployments. Corresponds to AgentDeviceUUID in the cppagent reference implementation. Per MTConnect v2.7 XSD UuidType, the uuid identifies the element "for its entire life" — Header.instanceId is the per-boot discriminator. | +| `agentUuid` | `AgentUuid` | `string` | Optional static UUID to assign to the Agent meta-device. Must parse as an RFC 4122 UUID — any format Guid.TryParse accepts (hyphenated xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx, braced, parenthesized, bare-hex, hex-braced) is normalized to the canonical hyphenated form. Surrounding whitespace is trimmed. Malformed, all-zero (Guid.Empty / RFC 4122 nil), or unparseable values are rejected on startup with a warning that reports only the value's length (never the raw content), and the resolver falls through to the next path: (1) validated override, else (2) validated agent.information.json persisted state, else (3) a deterministic UUID v5 derived from ServiceName + machine name so the meta-device UUID survives restarts even in ephemeral-container deployments. Corresponds to AgentDeviceUUID in the cppagent reference implementation. Per MTConnect v2.7 XSD UuidType, the uuid identifies the element "for its entire life" — Header.instanceId is the per-boot discriminator. | | `configurationFileRestartInterval` | `ConfigurationFileRestartInterval` | `int` | Gets or Sets the minimum time (in seconds) between Agent restarts when MonitorConfigurationFiles is enabled | | `devices` | `Devices` | `string` | The Path to look for the file(s) that represent the Device Information Models to load into the Agent. The path can either be a single file or a directory. The path can be absolute or relative to the executable's directory | | `durable` | `Durable` | `bool` | Gets or Sets whether the Agent buffers are durable and retain state after restart | @@ -169,7 +169,7 @@ Configuration for an MTConnect Shdr > Http Agent | Key | C# property | Type | Description | | --- | --- | --- | --- | -| `agentUuid` | `AgentUuid` | `string` | Optional static UUID to assign to the Agent meta-device. Must parse as an RFC 4122 UUID (any -accepted format, normalised to the canonical hyphenated form). Malformed or all-zero values log a length-only warning and the resolver falls through to persisted agent.information.json state or a deterministic UUID v5 derived from ServiceName + machine name. Corresponds to AgentDeviceUUID in the cppagent reference implementation. | +| `agentUuid` | `AgentUuid` | `string` | Optional static UUID to assign to the Agent meta-device. Must parse as an RFC 4122 UUID (any Guid.TryParse-accepted format, normalized to the canonical hyphenated form after trimming surrounding whitespace). Malformed, all-zero (Guid.Empty / RFC 4122 nil), or unparseable values log a length-only warning and the resolver falls through to persisted agent.information.json state or a deterministic UUID v5 derived from ServiceName + machine name. Corresponds to AgentDeviceUUID in the cppagent reference implementation. | | `configurationFileRestartInterval` | `ConfigurationFileRestartInterval` | `int` | Gets or Sets the minimum time (in seconds) between Agent restarts when MonitorConfigurationFiles is enabled | | `devices` | `Devices` | `string` | The Path to look for the file(s) that represent the Device Information Models to load into the Agent. The path can either be a single file or a directory. The path can be absolute or relative to the executable's directory | | `durable` | `Durable` | `bool` | Gets or Sets whether the Agent buffers are durable and retain state after restart | diff --git a/libraries/MTConnect.NET-Common/Agents/AgentUuidResolver.cs b/libraries/MTConnect.NET-Common/Agents/AgentUuidResolver.cs index 769750b5e..e1a337bb0 100644 --- a/libraries/MTConnect.NET-Common/Agents/AgentUuidResolver.cs +++ b/libraries/MTConnect.NET-Common/Agents/AgentUuidResolver.cs @@ -114,18 +114,22 @@ public static string Resolve( return normalizedOverride; } - // Hoist Path 2 validity + normalisation so Path 1's rejection + // Hoist Path 2 validity + normalization so Path 1's rejection // warning can label the fallback kind without a second parse of // the persisted value. var persistedIsValid = DeterministicAgentUuid.TryValidate(persistedUuid, out var normalizedPersisted); // Path 1 rejected but operator supplied something → warn (length only). + // Message wording is intentionally broad: TryValidate rejects unparseable + // input AND the RFC 4122 nil UUID (Guid.Empty), which does parse but + // would collide across every misconfigured agent — so "not acceptable" + // covers both causes without leaking which one the operator hit. if (!string.IsNullOrEmpty(operatorSuppliedUuid)) { var fallbackKind = persistedIsValid ? "persisted" : "derived"; warn?.Invoke(string.Format( CultureInfo.InvariantCulture, - "AgentUuid override (length={0}) is not a valid RFC 4122 UUID; falling back to {1} UUID.", + "AgentUuid override (length={0}) is not an acceptable RFC 4122 UUID (must be non-empty, parseable, and not the all-zero nil UUID); falling back to {1} UUID.", operatorSuppliedUuid.Length, fallbackKind)); } @@ -141,7 +145,7 @@ public static string Resolve( { warn?.Invoke(string.Format( CultureInfo.InvariantCulture, - "Persisted AgentUuid in agent.information.json (length={0}) is not a valid RFC 4122 UUID; falling back to derived UUID.", + "Persisted AgentUuid in agent.information.json (length={0}) is not an acceptable RFC 4122 UUID (must be non-empty, parseable, and not the all-zero nil UUID); falling back to derived UUID.", persistedUuid.Length)); } diff --git a/libraries/MTConnect.NET-Common/Agents/DeterministicAgentUuid.cs b/libraries/MTConnect.NET-Common/Agents/DeterministicAgentUuid.cs index 95fb87f03..fcad353c7 100644 --- a/libraries/MTConnect.NET-Common/Agents/DeterministicAgentUuid.cs +++ b/libraries/MTConnect.NET-Common/Agents/DeterministicAgentUuid.cs @@ -67,6 +67,17 @@ public static class DeterministicAgentUuid public static string Derive(string agentName, string hostname, int port) { var nameComponent = !string.IsNullOrEmpty(agentName) ? agentName : hostname; + if (string.IsNullOrEmpty(nameComponent)) + { + // Refuse to derive from an empty seed. If both agentName and hostname + // are null / empty the resulting UUID would be a fleet-wide constant + // ("agent::0" hashes to a single UUID for every misconfigured agent), + // defeating the entire lifetime-unique guarantee. Callers must + // supply at least one non-empty seed component. + throw new ArgumentException( + "Both agentName and hostname were null or empty; cannot derive a deterministic Agent UUID.", + nameof(hostname)); + } var seed = "agent:" + nameComponent + ":" + port.ToString(CultureInfo.InvariantCulture); return DeriveFromSeed(seed); } @@ -138,10 +149,16 @@ private static byte[] BigEndianToGuidBytes(byte[] beBytes) /// recognizes — hyphenated "D" (xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx), /// braced "B", parenthesized "P", bare-hex "N", or hex-braced "X" — and /// returns the canonical hyphenated "D" form so the wire representation - /// is stable regardless of input format. Rejects , - /// empty, whitespace-only, unparseable inputs, and the all-zero - /// value (a fleet-wide UUID collision hazard - /// if adopted by more than one agent). + /// is stable regardless of input format. Surrounding whitespace is + /// trimmed before parsing so a trailing newline, YAML indent, or + /// copy-paste padding does not silently reject an otherwise-valid UUID. + /// Inputs longer than 72 characters (the longest RFC 4122 textual form + /// plus slack) are rejected without invoking Guid.TryParse so a + /// pasted-in mega-payload cannot cost megabytes of parse state. + /// Rejects , empty, whitespace-only, unparseable + /// inputs, over-length inputs, and the all-zero + /// value (the RFC 4122 nil UUID — a + /// fleet-wide collision hazard if adopted by more than one agent). /// /// Motivation: MTConnect Part 1 types the uuid attribute as the /// UUID DataType (RFC 4122), and requires that value to remain @@ -174,6 +191,15 @@ public static bool TryValidate(string input, out string normalized) { normalized = null; if (string.IsNullOrWhiteSpace(input)) return false; + // Trim surrounding whitespace so trailing newlines / YAML indent / + // copy-paste padding don't silently reject an otherwise-valid UUID. + // Guid.TryParse does not trim on its own. + input = input.Trim(); + // Length cap keeps `Guid.TryParse` from allocating megabytes of + // parse state when the config slot receives a pasted-in payload. + // The longest RFC 4122 format ("X" with braces around 11 hex + // segments plus surrounding braces) fits in 68 chars; +4 slack. + if (input.Length > 72) return false; if (!Guid.TryParse(input, out var parsed)) return false; if (parsed == Guid.Empty) return false; normalized = parsed.ToString(); diff --git a/libraries/MTConnect.NET-Common/Configurations/AgentApplicationConfiguration.cs b/libraries/MTConnect.NET-Common/Configurations/AgentApplicationConfiguration.cs index a57667d4b..238fe3ff6 100644 --- a/libraries/MTConnect.NET-Common/Configurations/AgentApplicationConfiguration.cs +++ b/libraries/MTConnect.NET-Common/Configurations/AgentApplicationConfiguration.cs @@ -16,15 +16,7 @@ namespace MTConnect.Configurations public class AgentApplicationConfiguration : AgentConfiguration, IAgentApplicationConfiguration { /// - /// Optional static UUID to assign to the Agent meta-device. When set, - /// this value overrides the per-boot Guid.NewGuid() default - /// applied by 's - /// parameterless constructor and survives restarts without relying on - /// agent.information.json being present on disk. Corresponds to - /// AgentDeviceUUID in the cppagent reference implementation. - /// Per MTConnect v2.7 XSD UuidType, the uuid identifies the - /// element "for its entire life" — Header.instanceId is the - /// per-boot discriminator. + /// Optional static UUID to assign to the Agent meta-device. Must parse as an RFC 4122 UUID — any format Guid.TryParse accepts (hyphenated xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx, braced, parenthesized, bare-hex, hex-braced) is normalized to the canonical hyphenated form. Surrounding whitespace is trimmed. Malformed, all-zero (Guid.Empty / RFC 4122 nil), or unparseable values are rejected on startup with a warning that reports only the value's length (never the raw content), and the resolver falls through to the next path: (1) validated override, else (2) validated agent.information.json persisted state, else (3) a deterministic UUID v5 derived from ServiceName + machine name so the meta-device UUID survives restarts even in ephemeral-container deployments. Corresponds to AgentDeviceUUID in the cppagent reference implementation. Per MTConnect v2.7 XSD UuidType, the uuid identifies the element "for its entire life" — Header.instanceId is the per-boot discriminator. /// [JsonPropertyName("agentUuid")] public string AgentUuid { get; set; } diff --git a/libraries/MTConnect.NET-Common/Configurations/IAgentApplicationConfiguration.cs b/libraries/MTConnect.NET-Common/Configurations/IAgentApplicationConfiguration.cs index cf91a4990..dbe39a035 100644 --- a/libraries/MTConnect.NET-Common/Configurations/IAgentApplicationConfiguration.cs +++ b/libraries/MTConnect.NET-Common/Configurations/IAgentApplicationConfiguration.cs @@ -12,11 +12,7 @@ namespace MTConnect.Configurations public interface IAgentApplicationConfiguration : IAgentConfiguration { /// - /// Optional static UUID to assign to the Agent meta-device. When set, - /// this value overrides the per-boot Guid.NewGuid() default and - /// survives restarts without relying on agent.information.json - /// being present on disk. Corresponds to AgentDeviceUUID in the - /// cppagent reference implementation. + /// Optional static UUID to assign to the Agent meta-device. Must parse as an RFC 4122 UUID (any Guid.TryParse-accepted format, normalized to the canonical hyphenated form after trimming surrounding whitespace). Malformed, all-zero (Guid.Empty / RFC 4122 nil), or unparseable values log a length-only warning and the resolver falls through to persisted agent.information.json state or a deterministic UUID v5 derived from ServiceName + machine name. Corresponds to AgentDeviceUUID in the cppagent reference implementation. /// string AgentUuid { get; set; } diff --git a/tests/MTConnect.NET-Common-Tests/Agents/AgentUuidValidationTests.cs b/tests/MTConnect.NET-Common-Tests/Agents/AgentUuidValidationTests.cs index 797d30164..3893373bb 100644 --- a/tests/MTConnect.NET-Common-Tests/Agents/AgentUuidValidationTests.cs +++ b/tests/MTConnect.NET-Common-Tests/Agents/AgentUuidValidationTests.cs @@ -163,7 +163,7 @@ public void TryValidate_canonical_hyphenated_form_returns_true_unchanged() /// /// normalizes the - /// braced "B", parenthesised "P", and bare-hex "N" forms to the + /// braced "B", parenthesized "P", and bare-hex "N" forms to the /// canonical hyphenated "D" form so the wire representation stays /// stable regardless of the input format. /// @@ -199,12 +199,12 @@ public void TryValidate_hex_braced_X_format_normalizes_to_hyphenated() /// /// accepts uppercase - /// hex characters (case-insensitive per RFC 4122) and normalises the + /// hex characters (case-insensitive per RFC 4122) and normalizes the /// output to lowercase so the wire representation is stable regardless /// of the operator's typing. /// [Test] - public void TryValidate_uppercase_hex_is_normalised_to_lowercase() + public void TryValidate_uppercase_hex_is_normalized_to_lowercase() { const string Uppercased = "6BA7B810-9DAD-11D1-80B4-00C04FD430C8"; const string Expected = "6ba7b810-9dad-11d1-80b4-00c04fd430c8"; @@ -615,7 +615,7 @@ public void Null_warn_delegate_does_not_throw_on_either_rejection_path() /// /// The warn parameter defaults to — /// callers that omit it entirely must get the same no-throw - /// behaviour as an explicitly-null delegate. + /// behavior as an explicitly-null delegate. /// [Test] public void Default_warn_argument_omitted_does_not_throw() @@ -825,20 +825,20 @@ public void Malformed_persisted_state_survives_JSON_round_trip_and_is_rejected() // ------------------------------------------------------------------ /// - /// delegates to - /// , which trims leading - /// and trailing ASCII whitespace (space, tab, CR, LF). A copy-pasted - /// value with stray whitespace is accepted and normalised to the - /// canonical hyphenated "D" form. Pins that delegated contract so a - /// future .NET runtime tightening (or a swap to - /// Guid.TryParseExact) does not silently reject the input - /// class operators most commonly produce. + /// trims leading and + /// trailing whitespace before delegating to + /// , so a copy-pasted + /// value with stray whitespace is accepted and normalized to the + /// canonical hyphenated "D" form. Pinning the trim locally (rather + /// than relying on Guid.TryParse's implicit trim behaviour) + /// keeps the length-cap defense meaningful against padded input and + /// isolates the resolver from any future runtime tightening. /// [TestCase(" 6ba7b810-9dad-11d1-80b4-00c04fd430c8")] [TestCase("6ba7b810-9dad-11d1-80b4-00c04fd430c8 ")] [TestCase(" 6ba7b810-9dad-11d1-80b4-00c04fd430c8 ")] [TestCase("\t6ba7b810-9dad-11d1-80b4-00c04fd430c8\r\n")] - public void TryValidate_leading_and_trailing_whitespace_is_trimmed_and_normalised(string input) + public void TryValidate_leading_and_trailing_whitespace_is_trimmed_and_normalized(string input) { const string Expected = "6ba7b810-9dad-11d1-80b4-00c04fd430c8"; @@ -852,12 +852,12 @@ public void TryValidate_leading_and_trailing_whitespace_is_trimmed_and_normalise /// /// Mixed-case hex is accepted (RFC 4122 case-insensitive) and the - /// output is normalised to lowercase so the wire representation is + /// output is normalized to lowercase so the wire representation is /// stable regardless of how the operator typed the value. Companion - /// to . + /// to . /// [Test] - public void TryValidate_mixed_case_hex_is_accepted_and_normalised_to_lowercase() + public void TryValidate_mixed_case_hex_is_accepted_and_normalized_to_lowercase() { const string MixedCase = "6Ba7B810-9dAd-11D1-80B4-00c04Fd430c8"; const string Expected = "6ba7b810-9dad-11d1-80b4-00c04fd430c8"; @@ -961,6 +961,120 @@ public void Resolve_valid_override_short_circuits_and_ignores_malformed_persiste "Path 1 short-circuits before the persisted-state warn arm — malformed persisted must be silent."); } + // ------------------------------------------------------------------ + // Length-cap defense — TryValidate rejects mega-payloads before + // invoking Guid.TryParse so a pasted-in blob can never allocate + // megabytes of parse state. 72 chars is the longest legal RFC 4122 + // textual form ("X" with braces around 11 hex segments) plus slack. + // ------------------------------------------------------------------ + + /// + /// A 73-character input (one past the length cap) must be rejected + /// even when it contains a syntactically valid UUID prefix — the + /// length gate short-circuits before Guid.TryParse runs. + /// + [Test] + public void TryValidate_input_one_past_length_cap_is_rejected() + { + var overCap = new string('a', 73); + + var ok = DeterministicAgentUuid.TryValidate(overCap, out var normalized); + + Assert.That(ok, Is.False, + "73 characters exceeds the 72-char length cap — must be rejected."); + Assert.That(normalized, Is.Null); + } + + /// + /// A mega-payload (10 KB) must be rejected instantly without + /// invoking Guid.TryParse on the whole string; the length + /// cap is the DoS defense that keeps the resolver O(1) on hostile + /// input. + /// + [Test] + public void TryValidate_mega_payload_is_rejected_before_parse() + { + var mega = new string('x', 10_000); + + var ok = DeterministicAgentUuid.TryValidate(mega, out var normalized); + + Assert.That(ok, Is.False); + Assert.That(normalized, Is.Null); + } + + // ------------------------------------------------------------------ + // Guid.Empty warn wording — the resolver's warn message must be + // broad enough to cover BOTH parse failure AND the RFC 4122 nil + // UUID (which parses fine but is rejected as a collision hazard). + // ------------------------------------------------------------------ + + /// + /// When the operator supplies the all-zero nil UUID, the resolver + /// must warn with the broad "not an acceptable RFC 4122 UUID" + /// wording — not the narrower "unparseable" wording — because the + /// nil UUID DOES parse per RFC 4122 §4.1.7. Operators debugging + /// why their all-zero UUID was rejected should not look for a + /// parse failure that never happened. + /// + [Test] + public void Resolve_warn_on_nil_uuid_uses_broad_acceptable_wording() + { + var messages = new List(); + const string Nil = "00000000-0000-0000-0000-000000000000"; + + _ = AgentUuidResolver.Resolve( + operatorSuppliedUuid: Nil, + persistedUuid: null, + agentName: "test-agent", + hostname: "test-host", + warn: messages.Add); + + Assert.That(messages, Has.Count.EqualTo(1)); + Assert.That(messages[0], Does.Contain("not an acceptable RFC 4122 UUID"), + "Warn must use broad 'not acceptable' wording so operators recognise nil-UUID rejection."); + Assert.That(messages[0], Does.Contain("nil UUID"), + "Warn should mention the nil-UUID rejection cause explicitly for diagnostic clarity."); + AssertLogSafe(messages[0]); + } + + // ------------------------------------------------------------------ + // Derive defensive guard — refuses to derive from an empty seed so + // a misconfigured caller cannot produce a fleet-wide collision UUID. + // ------------------------------------------------------------------ + + /// + /// throws + /// when both agentName and + /// hostname are null / empty — the derived seed would be + /// "agent::0", hashing to a single UUID for every misconfigured + /// agent and defeating the entire lifetime-unique guarantee. + /// + [TestCase(null, null)] + [TestCase(null, "")] + [TestCase("", null)] + [TestCase("", "")] + public void Derive_throws_when_both_agent_name_and_hostname_are_empty(string agentName, string hostname) + { + Assert.Throws( + () => DeterministicAgentUuid.Derive(agentName, hostname, port: 0), + "Both seed components empty must not silently derive a fleet-wide collision UUID."); + } + + /// + /// Non-empty agentName is sufficient — the null / empty + /// hostname is tolerated because Environment.MachineName + /// is never null in practice but the API surface accepts either. + /// + [TestCase("agent-name", null)] + [TestCase("agent-name", "")] + public void Derive_accepts_null_or_empty_hostname_when_agent_name_supplied(string agentName, string hostname) + { + var uuid = DeterministicAgentUuid.Derive(agentName, hostname, port: 0); + + Assert.That(Guid.TryParse(uuid, out _), Is.True, + "Derive must succeed when agentName is non-empty even if hostname is null / empty."); + } + // ------------------------------------------------------------------ // Boot-simulation helpers — thin wrappers around AgentUuidResolver. // ------------------------------------------------------------------ From d3f841073b92ad25d5b76d7220b9137c5eef4b8f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Otto=20Boly=C3=B3s?= Date: Sat, 22 Aug 2026 11:20:28 +0200 Subject: [PATCH 7/7] chore: dotnet format drift baseline compliance --- .../Agents/AgentUuidDeterministicDefaultTests.cs | 6 +++--- .../Agents/AgentUuidValidationTests.cs | 8 ++++---- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/MTConnect.NET-Common-Tests/Agents/AgentUuidDeterministicDefaultTests.cs b/tests/MTConnect.NET-Common-Tests/Agents/AgentUuidDeterministicDefaultTests.cs index 6cca6c9d2..693cefab5 100644 --- a/tests/MTConnect.NET-Common-Tests/Agents/AgentUuidDeterministicDefaultTests.cs +++ b/tests/MTConnect.NET-Common-Tests/Agents/AgentUuidDeterministicDefaultTests.cs @@ -245,7 +245,7 @@ public void DeriveFromSeed_output_has_RFC_4122_variant_high_bits_10() public void Derive_port_change_produces_different_uuid_for_same_agent_name() { const string AgentName = "fixture-det-agent-port"; - const string Hostname = "canonical-host"; + const string Hostname = "canonical-host"; var derivedAt5000 = DeterministicAgentUuid.Derive(AgentName, Hostname, port: 5000); var derivedAt8080 = DeterministicAgentUuid.Derive(AgentName, Hostname, port: 8080); @@ -266,10 +266,10 @@ public void Derive_port_change_produces_different_uuid_for_same_agent_name() public void Derive_port_zero_sentinel_does_not_collide_with_any_positive_port() { const string AgentName = "fixture-det-agent-port-zero"; - const string Hostname = "canonical-host"; + const string Hostname = "canonical-host"; var derivedAtZero = DeterministicAgentUuid.Derive(AgentName, Hostname, port: 0); - var derivedAtOne = DeterministicAgentUuid.Derive(AgentName, Hostname, port: 1); + var derivedAtOne = DeterministicAgentUuid.Derive(AgentName, Hostname, port: 1); Assert.That(derivedAtZero, Is.Not.EqualTo(derivedAtOne), "port: 0 sentinel must be distinct from port: 1 — 0 is not silently 'omit'."); diff --git a/tests/MTConnect.NET-Common-Tests/Agents/AgentUuidValidationTests.cs b/tests/MTConnect.NET-Common-Tests/Agents/AgentUuidValidationTests.cs index 3893373bb..b4f81a434 100644 --- a/tests/MTConnect.NET-Common-Tests/Agents/AgentUuidValidationTests.cs +++ b/tests/MTConnect.NET-Common-Tests/Agents/AgentUuidValidationTests.cs @@ -207,7 +207,7 @@ public void TryValidate_hex_braced_X_format_normalizes_to_hyphenated() public void TryValidate_uppercase_hex_is_normalized_to_lowercase() { const string Uppercased = "6BA7B810-9DAD-11D1-80B4-00C04FD430C8"; - const string Expected = "6ba7b810-9dad-11d1-80b4-00c04fd430c8"; + const string Expected = "6ba7b810-9dad-11d1-80b4-00c04fd430c8"; var ok = DeterministicAgentUuid.TryValidate(Uppercased, out var normalized); @@ -543,7 +543,7 @@ public void Warn_message_names_derived_when_override_bad_and_persisted_absent() public void Warn_delegate_invoked_twice_when_both_override_and_persisted_are_malformed() { var messages = new List(); - const string BadOverride = "not-a-uuid"; + const string BadOverride = "not-a-uuid"; const string BadPersisted = "also-not-a-uuid"; var resolved = AgentUuidResolver.Resolve( @@ -860,7 +860,7 @@ public void TryValidate_leading_and_trailing_whitespace_is_trimmed_and_normalize public void TryValidate_mixed_case_hex_is_accepted_and_normalized_to_lowercase() { const string MixedCase = "6Ba7B810-9dAd-11D1-80B4-00c04Fd430c8"; - const string Expected = "6ba7b810-9dad-11d1-80b4-00c04fd430c8"; + const string Expected = "6ba7b810-9dad-11d1-80b4-00c04fd430c8"; var ok = DeterministicAgentUuid.TryValidate(MixedCase, out var normalized); @@ -945,7 +945,7 @@ public void TryValidate_trailing_crlf_with_injected_text_is_rejected() public void Resolve_valid_override_short_circuits_and_ignores_malformed_persisted_without_warn() { var messages = new List(); - const string ValidOverride = "6ba7b810-9dad-11d1-80b4-00c04fd430c8"; + const string ValidOverride = "6ba7b810-9dad-11d1-80b4-00c04fd430c8"; const string MalformedPersist = "definitely-not-a-uuid"; var resolved = AgentUuidResolver.Resolve(