Skip to content

Commit 3118fcd

Browse files
authored
Stop logging stdio transport environment variables (#1538)
1 parent 3686a4a commit 3118fcd

2 files changed

Lines changed: 41 additions & 5 deletions

File tree

src/ModelContextProtocol.Core/Client/StdioClientTransport.cs

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -126,9 +126,8 @@ public async Task<ITransport> ConnectAsync(CancellationToken cancellationToken =
126126

127127
if (logger.IsEnabled(LogLevel.Trace))
128128
{
129-
LogCreateProcessForTransportSensitive(logger, endpointName, _options.Command,
129+
LogCreateProcessForTransportDetailed(logger, endpointName, _options.Command,
130130
startInfo.Arguments,
131-
string.Join(", ", startInfo.Environment.Select(kvp => $"{kvp.Key}={kvp.Value}")),
132131
startInfo.WorkingDirectory);
133132
}
134133
else
@@ -300,8 +299,8 @@ private static string EscapeArgumentString(string argument) =>
300299
[LoggerMessage(Level = LogLevel.Information, Message = "{EndpointName} starting server process. Command: '{Command}'.")]
301300
private static partial void LogCreateProcessForTransport(ILogger logger, string endpointName, string command);
302301

303-
[LoggerMessage(Level = LogLevel.Trace, Message = "{EndpointName} starting server process. Command: '{Command}', Arguments: {Arguments}, Environment: {Environment}, Working directory: {WorkingDirectory}.")]
304-
private static partial void LogCreateProcessForTransportSensitive(ILogger logger, string endpointName, string command, string? arguments, string environment, string workingDirectory);
302+
[LoggerMessage(Level = LogLevel.Trace, Message = "{EndpointName} starting server process. Command: '{Command}', Arguments: {Arguments}, Working directory: {WorkingDirectory}.")]
303+
private static partial void LogCreateProcessForTransportDetailed(ILogger logger, string endpointName, string command, string? arguments, string workingDirectory);
305304

306305
[LoggerMessage(Level = LogLevel.Warning, Message = "{EndpointName} failed to start server process.")]
307306
private static partial void LogTransportProcessStartFailed(ILogger logger, string endpointName);

tests/ModelContextProtocol.Tests/Transport/StdioClientTransportTests.cs

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
using ModelContextProtocol.Client;
1+
using Microsoft.Extensions.Logging;
2+
using ModelContextProtocol.Client;
23
using ModelContextProtocol.Protocol;
34
using ModelContextProtocol.Tests.Utils;
45
using System.IO.Pipelines;
@@ -12,6 +13,42 @@ public class StdioClientTransportTests(ITestOutputHelper testOutputHelper) : Log
1213
{
1314
public static bool IsStdErrCallbackSupported => !PlatformDetection.IsMonoRuntime;
1415

16+
[Fact]
17+
public async Task ConnectAsync_DoesNotLogEnvironmentVariablesAtTrace()
18+
{
19+
string secretName = $"MCP_TEST_SECRET_{Guid.NewGuid():N}";
20+
string secretValue = $"secret-{Guid.NewGuid():N}";
21+
22+
using var loggerFactory = Microsoft.Extensions.Logging.LoggerFactory.Create(builder =>
23+
{
24+
builder.AddProvider(MockLoggerProvider);
25+
builder.SetMinimumLevel(LogLevel.Trace);
26+
});
27+
28+
StdioClientTransport transport = RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ?
29+
new(new()
30+
{
31+
Command = "cmd.exe",
32+
Arguments = ["/c", "exit /b 0"],
33+
EnvironmentVariables = new Dictionary<string, string?> { [secretName] = secretValue },
34+
}, loggerFactory) :
35+
new(new()
36+
{
37+
Command = "sh",
38+
Arguments = ["-c", "exit 0"],
39+
EnvironmentVariables = new Dictionary<string, string?> { [secretName] = secretValue },
40+
}, loggerFactory);
41+
42+
await using var _ = await transport.ConnectAsync(TestContext.Current.CancellationToken);
43+
44+
Assert.Contains(MockLoggerProvider.LogMessages, log =>
45+
log.LogLevel == LogLevel.Trace &&
46+
log.Message.Contains("starting server process", StringComparison.Ordinal));
47+
Assert.DoesNotContain(MockLoggerProvider.LogMessages, log =>
48+
log.Message.Contains(secretName, StringComparison.Ordinal) ||
49+
log.Message.Contains(secretValue, StringComparison.Ordinal));
50+
}
51+
1552
[Fact]
1653
public async Task CreateAsync_ValidProcessInvalidServer_Throws()
1754
{

0 commit comments

Comments
 (0)