Skip to content

fix(agent): reject non-UUID AgentUuid on override + persisted paths - #220

Draft
ottobolyos wants to merge 6 commits into
TrakHound:masterfrom
ottobolyos:fix/agent-uuid-lifecycle-hardening
Draft

fix(agent): reject non-UUID AgentUuid on override + persisted paths#220
ottobolyos wants to merge 6 commits into
TrakHound:masterfrom
ottobolyos:fix/agent-uuid-lifecycle-hardening

Conversation

@ottobolyos

@ottobolyos ottobolyos commented Aug 17, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

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 prior single-path validation left Path 2 open: a corrupted state file (or a pre-hardening agent version that wrote non-UUID content) would flow straight to the wire and fail every downstream MTConnect Part 1 XSD-validating consumer on typed enum/decimal DataItems.

Change

  • New AgentUuidResolver.Resolve (public static, MTConnect.NET-Common) implements the three-path algorithm:
    • Path 1 — validated operator override wins;
    • Path 2 — validated persisted state wins over derivation;
    • Path 3 — deterministic UUID v5 derivation from ServiceName.
      Malformed input on Path 1 OR Path 2 logs a warning via a delegate hook (so MTConnect.NET-Common takes no hard dependency on NLog) and falls through to the next path. Raw values echoed to the warn log are pre-sanitized (CRLF stripped, truncated to 64 chars) as guards 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 renamed normalised → normalized (American English spelling is used throughout committed code); XML-doc verbs updated to match. Positional / out var callers unaffected.
  • Test fixtures brought forward — AgentUuidConfigOverrideTests + AgentUuidLongitudinalInvariantsTests now route their boot simulation through AgentUuidResolver.Resolve instead of inline replay, and their fixture literals are canonical RFC 4122 UUIDs (previously non-UUID strings that survived only because the inline replay omitted TryValidate — fake-green). Every fixture marked [NonParallelizable] with orphan-.bak.* sweep.

Test plan

Commits (RED → GREEN):

  1. test(common-tests): pin AgentUuid RFC 4122 contract on both resolution paths (RED) — a compile-error RED (tests reference AgentUuidResolver which lands in commit 2).
  2. fix(agent): reject non-UUID AgentUuid on override AND persisted paths — introduces the resolver, wires RunAgent through it, renames normalised → normalized.

Supersedes #207.

Depends on

@ottobolyos
ottobolyos force-pushed the fix/agent-uuid-lifecycle-hardening branch from aaeaa8e to 8fd6d38 Compare August 19, 2026 12:12
ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 19, 2026
@ottobolyos
test(common-tests): pin AgentUuid TryValidate boundaries, port axis, …
c0d9f54 
…RFC 4122 variant bits

Coverage-FLOOR characterisation tests per CONVENTIONS
§1.0d-trigies-novodecies — closes the boundary / bit-layout / matrix
gaps left after PR TrakHound#220's initial pass. All tests characterise
already-correct behaviour, so they land GREEN; RED-first ordering
(§1.0d-trigies-octies) 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 normalised 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 characterisation.
  - 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.
ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 19, 2026
@ottobolyos
fix(agent): apply Ultrareview findings — XML docs, trim, length cap, …
6dc88d6 
…Derive guard

Applies the fix-worthy findings from the PR TrakHound#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 CONVENTIONS
  §1.0d-decies (AmE spelling universal in MTConnect.NET canonical
  surfaces).

Adds tests pinning the new behaviour:

- 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 c0d9f54 boundary/enum/variant pin.
ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 19, 2026
@ottobolyos
merge: PR TrakHound#220 — verified Completed
49afd1a 
# Conflicts:
#	agent/MTConnect.NET-Applications-Agents/MTConnectAgentApplication.cs
ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 19, 2026
@ottobolyos
merge: PR TrakHound#220 — topological position 5
f4a4aac
@ottobolyos
test(common-tests): pin AgentUuid RFC 4122 contract on both resolutio…
7c49320 
…n paths (RED)

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 CONVENTIONS §1.0d-vicies-semel's behavioral-RED default.

Claude-Session: https://claude.ai/code/session_015FLSuNX8hZPFiTGtE8GYqH
@ottobolyos
fix(agent): reject non-UUID AgentUuid on override AND persisted paths
95e3d91 
Hardens the Agent meta-device UUID resolution slice added by TrakHound#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 normalised
  renamed to normalized per CONVENTIONS §1.0d-decies (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.

Claude-Session: https://claude.ai/code/session_015FLSuNX8hZPFiTGtE8GYqH
@ottobolyos
test(common-tests): close AgentUuidResolver warn-delegate and Sanitis…
4235c61 
…eForLog coverage gaps

Extends AgentUuidValidationTests.cs with 15 test cases the initial fixture
did not cover, per coverage-FLOOR panel §1.0d-trigies-novodecies:

  - TryValidate accepts the hex-braced "X" Guid format (missing enum arm).
  - TryValidate normalises 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-parameterised).
  - 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 serialiser and is still
    rejected by the resolver (integration failure-path proof, not just
    an in-memory string).
@ottobolyos
fix(agent): harden AgentUuid resolver — reject Guid.Empty, redact war…
0cdc40d 
…n values

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 sanitised-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 sanitise, 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.
@ottobolyos
test(common-tests): pin AgentUuid TryValidate boundaries, port axis, …
33aad6e 
…RFC 4122 variant bits

Coverage-FLOOR characterisation tests per CONVENTIONS
§1.0d-trigies-novodecies — closes the boundary / bit-layout / matrix
gaps left after PR TrakHound#220's initial pass. All tests characterise
already-correct behaviour, so they land GREEN; RED-first ordering
(§1.0d-trigies-octies) 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 normalised 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 characterisation.
  - 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.
@ottobolyos
fix(agent): apply Ultrareview findings — XML docs, trim, length cap, …
187f069 
…Derive guard

Applies the fix-worthy findings from the PR TrakHound#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 CONVENTIONS
  §1.0d-decies (AmE spelling universal in MTConnect.NET canonical
  surfaces).

Adds tests pinning the new behaviour:

- 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 c0d9f54 boundary/enum/variant pin.
@ottobolyos
ottobolyos force-pushed the fix/agent-uuid-lifecycle-hardening branch from 6dc88d6 to 187f069 Compare August 19, 2026 22:05
ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 19, 2026
@ottobolyos
merge: PR TrakHound#220 — topological position 5
129eb2d
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@ottobolyos