Skip to content

Commit 633e2c0

Browse files
Sanitize worker arguments before logging (#10260)
1 parent 7c9dc23 commit 633e2c0

File tree

2 files changed

+44
-1
lines changed

2 files changed

+44
-1
lines changed

src/WebJobs.Script/Workers/ProcessManagement/WorkerProcess.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,8 +81,9 @@ public Task StartProcessAsync()
8181
Process.OutputDataReceived += (sender, e) => OnOutputDataReceived(sender, e);
8282
Process.Exited += (sender, e) => OnProcessExited(sender, e);
8383
Process.EnableRaisingEvents = true;
84+
string sanitizedArguments = Sanitizer.Sanitize(Process.StartInfo.Arguments);
8485

85-
_workerProcessLogger?.LogDebug($"Starting worker process with FileName:{Process.StartInfo.FileName} WorkingDirectory:{Process.StartInfo.WorkingDirectory} Arguments:{Process.StartInfo.Arguments}");
86+
_workerProcessLogger?.LogDebug($"Starting worker process with FileName:{Process.StartInfo.FileName} WorkingDirectory:{Process.StartInfo.WorkingDirectory} Arguments:{sanitizedArguments}");
8687
Process.Start();
8788
_workerProcessLogger?.LogDebug($"{Process.StartInfo.FileName} process with Id={Process.Id} started");
8889

test/WebJobs.Script.Tests/Workers/Http/HttpWorkerProcessTests.cs

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,5 +119,47 @@ public async Task StartProcess_LinuxConsumption_TriesToAssignExecutePermissions_
119119
var testLogs = _testLogger.GetLogMessages();
120120
Assert.Contains("File path does not exist, not assigning permissions", testLogs[0].FormattedMessage);
121121
}
122+
123+
[Theory]
124+
[InlineData("AccountKey=abcde==", true)]
125+
[InlineData("teststring", false)]
126+
public async Task StartProcess_VerifySanitizedCredentialLogging(string input, bool isSecret)
127+
{
128+
try
129+
{
130+
_httpWorkerOptions.Arguments.WorkerArguments.Add(input);
131+
132+
File.Create(_executablePath).Dispose();
133+
TestEnvironment testEnvironment = new TestEnvironment();
134+
testEnvironment.SetEnvironmentVariable(EnvironmentSettingNames.ContainerName, "TestContainer");
135+
var mockHttpWorkerProcess = new HttpWorkerProcess(_testWorkerId, _rootScriptPath, _httpWorkerOptions, _mockEventManager.Object, _defaultWorkerProcessFactory, _processRegistry, _testLogger, _languageWorkerConsoleLogSource.Object, testEnvironment, new TestMetricsLogger(), _serviceProviderMock.Object, new LoggerFactory());
136+
137+
try
138+
{
139+
await mockHttpWorkerProcess.StartProcessAsync();
140+
}
141+
catch
142+
{
143+
// expected to throw. Just verifying a log statement occurred before then.
144+
}
145+
146+
// Verify
147+
var testLogs = _testLogger.GetLogMessages();
148+
149+
if (isSecret)
150+
{
151+
Assert.DoesNotContain(input, testLogs[1].FormattedMessage);
152+
Assert.Contains($"Arguments: [Hidden Credential]", testLogs[1].FormattedMessage);
153+
}
154+
else
155+
{
156+
Assert.Contains($"Arguments: {input}", testLogs[1].FormattedMessage);
157+
}
158+
}
159+
finally
160+
{
161+
File.Delete(_executablePath);
162+
}
163+
}
122164
}
123165
}

0 commit comments

Comments
 (0)