Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Worker termination path update #10367

Merged
merged 21 commits into from
Aug 13, 2024
Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions benchmarks/WebJobs.Script.Benchmarks/SanitizerBenchmarks.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the MIT License. See License.txt in the project root for license information.

using BenchmarkDotNet.Attributes;
using Microsoft.Azure.WebJobs.Logging;

namespace Microsoft.Azure.WebJobs.Script.Benchmarks
{
public class SanitizerBenchmarks
{
[Benchmark]
public void Sanitize()
{
Sanitizer.Sanitize("testprotocol://name:password@address:1111");
}
}
}
surgupta-msft marked this conversation as resolved.
Show resolved Hide resolved
1 change: 1 addition & 0 deletions release_notes.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,4 @@
- Updated dotnet-isolated worker to [1.0.11](https://github.com/Azure/azure-functions-dotnet-worker/pull/2653) (#10379)
- Update Java Worker Version to [2.15.0](https://github.com/Azure/azure-functions-java-worker/releases/tag/2.15.0)
- Update grpc-protobuf to 1.64.0 and application insights agent version to 3.5.2
- Worker termination path update (#10367)
24 changes: 23 additions & 1 deletion src/WebJobs.Script/Sanitizer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Licensed under the MIT License. See License.txt in the project root for license information.

using System;
using System.Text.RegularExpressions;
using Newtonsoft.Json.Linq;

namespace Microsoft.Azure.WebJobs.Logging
Expand All @@ -20,6 +21,21 @@ internal static class Sanitizer
internal static readonly string[] CredentialTokens = new string[] { "Token=", "DefaultEndpointsProtocol=http", "AccountKey=", "Data Source=", "Server=", "Password=", "pwd=", "&sig=", "&sig=", "?sig=", "SharedAccessKey=", "&code=", "&code=", "?code=" };
private static readonly string[] CredentialNameFragments = new[] { "password", "pwd", "key", "secret", "token", "sas" };

// Pattern of format : "<protocol>://<username>:<password>@<address>:<port>"
private static readonly string Pattern = @"
\b([a-zA-Z]+) # Capture protocol
:\/\/ # '://'
([^:/\s]+) # Capture username
: # ':'
([^@/\s]+) # Capture password
@ # '@'
([^:/\s]+) # Capture address
: # ':'
([0-9]+)\b # Capture port number
";

private static readonly Regex Regex = new Regex(Pattern, RegexOptions.Compiled | RegexOptions.IgnoreCase | RegexOptions.IgnorePatternWhitespace);

/// <summary>
/// Removes well-known credential strings from strings.
/// </summary>
Expand Down Expand Up @@ -73,6 +89,12 @@ internal static string Sanitize(string input)
}
}

// This check avoids unnecessary regex evaluation if the input does not contain any url
if (input.Contains(":"))
{
t = Regex.Replace(t, SecretReplacement);
}

return t;
}

Expand Down Expand Up @@ -153,6 +175,6 @@ static JToken Sanitize(JToken token)
/// Checks if a string even *possibly* contains one of our <see cref="CredentialTokens"/>.
/// Useful for short-circuiting more expensive checks and replacements if it's known we wouldn't do anything.
/// </summary>
internal static bool MayContainCredentials(string input) => input.Contains("=");
internal static bool MayContainCredentials(string input) => input.Contains("=") || input.Contains(":");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,8 @@ private void OnProcessExited(object sender, EventArgs e)
else
{
string exceptionMessage = string.Join(",", _processStdErrDataQueue.Where(s => !string.IsNullOrEmpty(s)));
var processExitEx = new WorkerProcessExitException($"{Process.StartInfo.FileName} exited with code {Process.ExitCode} (0x{Process.ExitCode.ToString("X")})", new Exception(exceptionMessage));
string sanitizedExceptionMessage = Sanitizer.Sanitize(exceptionMessage);
var processExitEx = new WorkerProcessExitException($"{Process.StartInfo.FileName} exited with code {Process.ExitCode} (0x{Process.ExitCode.ToString("X")})", new Exception(sanitizedExceptionMessage));
processExitEx.ExitCode = Process.ExitCode;
processExitEx.Pid = Process.Id;
HandleWorkerProcessExitError(processExitEx);
Expand Down
4 changes: 4 additions & 0 deletions test/WebJobs.Script.Tests/SanitizerTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@ public class SanitizerTests
[InlineData("test?code=XPAAAAAAAAAAAAAT-ag==", "test[Hidden Credential]")]
[InlineData("test?foo=bar&code=REAAAAAAAAAAAAAT-ag==", "test?foo=bar[Hidden Credential]")]
[InlineData("test&amp;code=MiAAAAAAAAAAAAAAAAT-ag==", "test[Hidden Credential]")]
[InlineData("aaa://aaa:aaaaaa1111aa@aaa.aaa.io:1111", "[Hidden Credential]")]
[InlineData("test,aaa://aaa:aaaaaa1111aa@aaa.aaa.io:1111,test", "test,[Hidden Credential],test")]
[InlineData(@"some text abc://abc:aaaaaa1111aa@aaa.abc.io:1111 some text abc://abc:aaaaaa1111aa@aaa.abc.io:1111 text", @"some text [Hidden Credential] some text [Hidden Credential] text")]
[InlineData(@"some text abc://abc:aaaaaa1111aa@aaa.abc.io:1111 some text AccountKey=heyyyyyyy text", @"some text [Hidden Credential] some text [Hidden Credential]")]
public void SanitizeString(string input, string expectedOutput)
{
var sanitized = Sanitizer.Sanitize(input);
Expand Down
13 changes: 13 additions & 0 deletions test/WebJobs.Script.Tests/Workers/Rpc/RpcWorkerProcessTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System.Diagnostics;
using System.Linq;
using Microsoft.Azure.WebJobs.Host.Scale;
using Microsoft.Azure.WebJobs.Logging;
using Microsoft.Azure.WebJobs.Script.Config;
using Microsoft.Azure.WebJobs.Script.Eventing;
using Microsoft.Azure.WebJobs.Script.Workers;
Expand Down Expand Up @@ -163,6 +164,18 @@ public void ErrorMessageQueue_Full_Enqueue_Success()
Assert.Equal("Error2,Error3,Error4", exceptionMessage);
}

[Fact]
public void VerifySanitizedErrorMessage_Success()
surgupta-msft marked this conversation as resolved.
Show resolved Hide resolved
{
WorkerProcessUtilities.AddStdErrMessage(_rpcWorkerProcess.ProcessStdErrDataQueue, "test aaa://aaa:aaaaaa1111aa@aaa.aaa.io:1111");
WorkerProcessUtilities.AddStdErrMessage(_rpcWorkerProcess.ProcessStdErrDataQueue, "Error2");
WorkerProcessUtilities.AddStdErrMessage(_rpcWorkerProcess.ProcessStdErrDataQueue, "Error3");
Assert.True(_rpcWorkerProcess.ProcessStdErrDataQueue.Count == 3);
string exceptionMessage = string.Join(",", _rpcWorkerProcess.ProcessStdErrDataQueue.Where(s => !string.IsNullOrEmpty(s)));
string sanitizedExceptionMessage = Sanitizer.Sanitize(exceptionMessage);
Assert.Equal("test [Hidden Credential],Error2,Error3", sanitizedExceptionMessage);
}

[Theory]
[InlineData("languageWorkerConsoleLog Connection established")]
[InlineData("LANGUAGEWORKERCONSOLELOG Connection established")]
Expand Down