fix(mtconnect): redact the Agent URI everywhere it is shown (R06:T-8)

The driver logged and surfaced the raw AgentUri (which may carry user:pass@) as HostName (the /hosts key), in ~20 log lines and in exception messages that reach DriverHealth.LastError, and the client's request-URI messages (timeouts, stream end, not-a-stream, framing) embedded it too - while the Test-Connect probe deliberately redacted it. New MTConnectUriRedaction (user-info, and for the configured string its query, removed; a clean URI is unchanged so host keys do not move); the driver computes its display form once per options document. Refs #743 Claude-Session: https://claude.ai/code/session_01Ub9hGmYLQvn9XETzw7CMsT
2026-10-22 16:00:40 -04:00
parent d4b9fa1c29
commit d67634cc4f
7 changed files with 263 additions and 33 deletions
@@ -0,0 +1,125 @@
+using System.Net;
+using Shouldly;
+using Xunit;
+using ZB.MOM.WW.OtOpcUa.Core.Abstractions;
+
+namespace ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests;
+
+/// <summary>
+///     Review 2026-10-21 R06:T-8 — the driver logged and surfaced the raw <c>AgentUri</c> (which may carry
+///     <c>user:pass@</c>) as its <c>HostName</c>, in every log line and in exception messages that reach
+///     <c>DriverHealth.LastError</c> → <c>/hosts</c>, while its own Test-Connect probe deliberately redacted it.
+/// </summary>
+public sealed class MTConnectUriRedactionTests
+{
+    private const string Secret = "s3cretPw";
+    private const string CredentialedUri = "http://operator:" + Secret + "@fixture-agent:5000";
+
+    private static CancellationToken Ct => TestContext.Current.CancellationToken;
+
+    [Theory]
+    [InlineData("http://fixture-agent:5000", "http://fixture-agent:5000")]                 // unchanged when clean
+    [InlineData("http://agent:5000/prefix", "http://agent:5000/prefix")]
+    [InlineData("http://u:p@agent:5000", "http://agent:5000")]
+    [InlineData("https://u:p@agent/prefix?token=abc", "https://agent/prefix")]
+    [InlineData("not a uri with u:p@host", MTConnectUriRedaction.Unparseable)]
+    [InlineData("not-a-uri", "not-a-uri")]
+    public void The_configured_uri_loses_its_user_info(string raw, string expected) =>
+        MTConnectUriRedaction.Redact(raw).ShouldBe(expected);
+
+    [Fact]
+    public void A_request_uri_loses_its_user_info_and_keeps_its_query() =>
+        MTConnectUriRedaction.Redact(new Uri("http://u:p@agent:5000/sample?from=5&count=10"))
+            .ShouldBe("http://agent:5000/sample?from=5&count=10");
+
+    /// <summary>The host key, every log line and the health text of a credentialed driver carry no password.</summary>
+    [Fact]
+    public async Task A_credentialed_agent_uri_never_reaches_host_name_logs_or_health()
+    {
+        var logger = new RecordingDriverLogger();
+        var client = CannedAgentClient.FromFixtures();
+        var driver = new MTConnectDriver(
+            new MTConnectDriverOptions
+            {
+                AgentUri = CredentialedUri,
+                RequestTimeoutMs = 200,
+                Tags = [new MTConnectTagDefinition("dev1_pos", DriverDataType.Float64)],
+            },
+            "mt-t8",
+            _ => client,
+            logger);
+        await driver.InitializeAsync("{}", Ct);
+        try
+        {
+            // A /current that never answers: the read times out, degrades health and logs a Warning.
+            client.CurrentGate = new TaskCompletionSource();
+            await driver.ReadAsync(["dev1_pos"], Ct);
+
+            var health = driver.GetHealth();
+            health.State.ShouldBe(DriverState.Degraded, "positive control: the failure path that carries the URI ran");
+            health.LastError.ShouldNotBeNull().ShouldNotContain(Secret);
+            health.LastError.ShouldContain("fixture-agent:5000", Case.Sensitive, "the host is still named");
+
+            driver.HostName.ShouldBe("http://fixture-agent:5000");
+            ((IHostConnectivityProbe)driver).GetHostStatuses().ShouldAllBe(s => !s.HostName.Contains(Secret));
+
+            var lines = logger.AllSnapshot();
+            lines.ShouldNotBeEmpty("positive control: the initialize line and the read warning were logged");
+            lines.ShouldContain(l => l.Contains("fixture-agent:5000"));
+            lines.ShouldAllBe(l => !l.Contains(Secret));
+        }
+        finally
+        {
+            client.CurrentGate?.TrySetResult();
+            await driver.ShutdownAsync(CancellationToken.None);
+        }
+    }
+
+    /// <summary>The production client's own exception messages (they become health text) carry no password.</summary>
+    [Fact]
+    public async Task The_client_timeout_message_carries_no_password()
+    {
+        using var handler = new HangingHandler();
+        using var client = new MTConnectAgentClient(
+            new MTConnectDriverOptions { AgentUri = CredentialedUri, RequestTimeoutMs = 200 }, handler);
+
+        var ex = await Should.ThrowAsync<TimeoutException>(async () => await client.CurrentAsync(Ct));
+
+        ex.Message.ShouldContain("fixture-agent:5000");
+        ex.Message.ShouldNotContain(Secret);
+    }
+
+    /// <summary>The stream-ended exception (it becomes the pump's Degrade text) carries no password.</summary>
+    [Fact]
+    public async Task The_client_stream_messages_carry_no_password()
+    {
+        using var handler = new SingleDocumentHandler();
+        using var client = new MTConnectAgentClient(new MTConnectDriverOptions { AgentUri = CredentialedUri }, handler);
+
+        var ex = await Should.ThrowAsync<MTConnectStreamNotSupportedException>(async () =>
+        {
+            await foreach (var _ in client.SampleAsync(1, Ct)) { }
+        });
+
+        ex.Message.ShouldContain("fixture-agent:5000");
+        ex.Message.ShouldNotContain(Secret);
+    }
+
+    private sealed class HangingHandler : HttpMessageHandler
+    {
+        protected override async Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
+        {
+            await Task.Delay(Timeout.Infinite, cancellationToken);
+            return new HttpResponseMessage(HttpStatusCode.OK);
+        }
+    }
+
+    private sealed class SingleDocumentHandler : HttpMessageHandler
+    {
+        protected override Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) =>
+            Task.FromResult(new HttpResponseMessage(HttpStatusCode.OK)
+            {
+                Content = new StringContent(File.ReadAllText("Fixtures/current.xml"), System.Text.Encoding.UTF8, "text/xml"),
+            });
+    }
+}
@@ -18,6 +18,24 @@ internal sealed class RecordingDriverLogger : ILogger<MTConnectDriver>
    /// <summary>Formatted <see cref="LogLevel.Error"/> lines, in order.</summary>
    public List<string> Errors { get; } = [];

+    /// <summary>Formatted <see cref="LogLevel.Information"/> lines, in order.</summary>
+    public List<string> Infos { get; } = [];
+
+    /// <summary>Every recorded line at Information and above, with its exception's message appended.</summary>
+    public IReadOnlyList<string> AllSnapshot()
+    {
+        var all = new List<string>();
+        foreach (var sink in new[] { Infos, Warnings, Errors })
+        {
+            lock (sink)
+            {
+                all.AddRange(sink);
+            }
+        }
+
+        return all;
+    }
+
    /// <inheritdoc/>
    public IDisposable? BeginScope<TState>(TState state)
        where TState : notnull => null;
@@ -39,6 +57,7 @@ internal sealed class RecordingDriverLogger : ILogger<MTConnectDriver>
        {
            LogLevel.Warning => Warnings,
            LogLevel.Error => Errors,
+            LogLevel.Information => Infos,
            _ => null,
        };

@@ -50,7 +69,7 @@ internal sealed class RecordingDriverLogger : ILogger<MTConnectDriver>

        lock (sink)
        {
-            sink.Add(formatter(state, exception));
+            sink.Add(exception is null ? formatter(state, exception) : $"{formatter(state, exception)} | {exception.Message}");
        }
    }