diff --git a/ZB.MOM.WW.Secrets/src/ZB.MOM.WW.Secrets.Replicator.Grpc/FailoverSecretsHubReader.cs b/ZB.MOM.WW.Secrets/src/ZB.MOM.WW.Secrets.Replicator.Grpc/FailoverSecretsHubReader.cs index 8ad8fe4..161fdc8 100644 --- a/ZB.MOM.WW.Secrets/src/ZB.MOM.WW.Secrets.Replicator.Grpc/FailoverSecretsHubReader.cs +++ b/ZB.MOM.WW.Secrets/src/ZB.MOM.WW.Secrets.Replicator.Grpc/FailoverSecretsHubReader.cs @@ -78,9 +78,11 @@ internal sealed partial class FailoverSecretsHubReader : ISecretsHubReader nameof(endpointLabels)); } + ArgumentNullException.ThrowIfNull(logger); + _readers = readers; _endpointLabels = endpointLabels; - _logger = logger ?? throw new ArgumentNullException(nameof(logger)); + _logger = logger; } /// @@ -115,11 +117,13 @@ internal sealed partial class FailoverSecretsHubReader : ISecretsHubReader if (attempt < _readers.Count - 1) { - // The message carries the two endpoint URIs and the status code — nothing from - // the exception detail, which is server-authored text no credential belongs in - // but this node cannot vouch for. + // The exception object is deliberately NOT attached to this warning: sinks + // render an attached exception's ToString(), which for RpcException includes + // Status.Detail — server-authored text no credential belongs in but this node + // cannot vouch for. The two endpoint URIs plus the status code are what make + // the warning actionable, and the terminal failure still reaches the sweep's + // catch-all with the full exception when every endpoint has failed. LogEndpointFailedOver( - ex, _endpointLabels[index], ex.StatusCode, _endpointLabels[(start + attempt + 1) % _readers.Count]); @@ -137,5 +141,5 @@ internal sealed partial class FailoverSecretsHubReader : ISecretsHubReader Level = LogLevel.Warning, Message = "Secrets hub endpoint {FailedEndpoint} did not answer ({StatusCode}); failing over to {NextEndpoint}.")] private partial void LogEndpointFailedOver( - Exception exception, string failedEndpoint, StatusCode statusCode, string nextEndpoint); + string failedEndpoint, StatusCode statusCode, string nextEndpoint); } diff --git a/ZB.MOM.WW.Secrets/src/ZB.MOM.WW.Secrets.Replicator.Grpc/SecretsGrpcHubClientOptions.cs b/ZB.MOM.WW.Secrets/src/ZB.MOM.WW.Secrets.Replicator.Grpc/SecretsGrpcHubClientOptions.cs index a832079..f5c8c04 100644 --- a/ZB.MOM.WW.Secrets/src/ZB.MOM.WW.Secrets.Replicator.Grpc/SecretsGrpcHubClientOptions.cs +++ b/ZB.MOM.WW.Secrets/src/ZB.MOM.WW.Secrets.Replicator.Grpc/SecretsGrpcHubClientOptions.cs @@ -38,8 +38,10 @@ public sealed class SecretsGrpcHubClientOptions /// /// /// Getter-only so the configuration binder adds entries to this instance rather than needing a - /// setter. Each entry carries the same http-cleartext caveat as : a plain - /// http fallback sends in cleartext on every failed-over call. + /// setter. Mutating the list after has run is unsupported — the DI + /// extension reads it exactly once, at registration, to shape the channel graph. Each entry + /// carries the same http-cleartext caveat as : a plain http + /// fallback sends in cleartext on every failed-over call. /// public IList FallbackEndpoints { get; } = []; diff --git a/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Replicator.Grpc.Tests/FailoverSecretsHubReaderTests.cs b/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Replicator.Grpc.Tests/FailoverSecretsHubReaderTests.cs index 6c4c1b6..e5c1cf1 100644 --- a/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Replicator.Grpc.Tests/FailoverSecretsHubReaderTests.cs +++ b/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Replicator.Grpc.Tests/FailoverSecretsHubReaderTests.cs @@ -175,6 +175,12 @@ public sealed class FailoverSecretsHubReaderTests } Assert.Equal(1, _logger.WarningCount); + + // The exception object must not ride along either: sinks render an attached exception's + // ToString(), which for RpcException includes Status.Detail — the very text the message + // template deliberately excludes. A clean message with the exception attached would still + // leak in production, so the warning must carry NO exception at all. + Assert.All(_logger.WarningExceptions, Assert.Null); } [Fact] diff --git a/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Replicator.Grpc.Tests/TestDoubles.cs b/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Replicator.Grpc.Tests/TestDoubles.cs index ed412a7..0b4e05d 100644 --- a/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Replicator.Grpc.Tests/TestDoubles.cs +++ b/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Replicator.Grpc.Tests/TestDoubles.cs @@ -130,6 +130,14 @@ internal sealed class RecordingLogger : ILogger /// Formatted messages logged at . public List Warnings { get; } = []; + /// + /// The exception object attached to each Warning entry, index-aligned with + /// . Captured separately from the formatted message because real sinks + /// ALSO render an attached exception's ToString() — for RpcException that includes + /// Status.Detail — so a leak test that only scans the message would miss that channel. + /// + public List WarningExceptions { get; } = []; + /// Count of information entries. public int InformationCount => Information.Count; @@ -161,6 +169,7 @@ internal sealed class RecordingLogger : ILogger break; case LogLevel.Warning: Warnings.Add(message); + WarningExceptions.Add(exception); break; default: break;