From 7de0ffb92fec12b5d1d13ea53d2ae7d81fd4826f Mon Sep 17 00:00:00 2001 From: Surgupta Date: Wed, 31 Jul 2024 16:24:21 -0700 Subject: [PATCH 01/19] Sanitizing exception message --- src/WebJobs.Script/Workers/ProcessManagement/WorkerProcess.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/WebJobs.Script/Workers/ProcessManagement/WorkerProcess.cs b/src/WebJobs.Script/Workers/ProcessManagement/WorkerProcess.cs index 79d048a824..1bee107af1 100644 --- a/src/WebJobs.Script/Workers/ProcessManagement/WorkerProcess.cs +++ b/src/WebJobs.Script/Workers/ProcessManagement/WorkerProcess.cs @@ -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); From 8e2f094dca18c6d20ca8e3aba39359569a266a8d Mon Sep 17 00:00:00 2001 From: Surgupta Date: Mon, 5 Aug 2024 12:09:13 -0700 Subject: [PATCH 02/19] Sanitizer update for websocket scenario --- src/WebJobs.Script/Sanitizer.cs | 6 ++++++ test/WebJobs.Script.Tests/SanitizerTests.cs | 2 ++ .../Workers/Rpc/RpcWorkerProcessTests.cs | 13 +++++++++++++ 3 files changed, 21 insertions(+) diff --git a/src/WebJobs.Script/Sanitizer.cs b/src/WebJobs.Script/Sanitizer.cs index 5166e0b6c1..48a64ee21a 100644 --- a/src/WebJobs.Script/Sanitizer.cs +++ b/src/WebJobs.Script/Sanitizer.cs @@ -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 @@ -20,6 +21,9 @@ 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" }; + private static readonly string WebSocketPattern = "(?i)((amqp|ssh|(ht|f)tps?)://[^%:\\s\"'/][^:\\s\"'/\\$]+[^:\\s\"'/\\$%]:([^%\\s\"'/][^@\\s\"'/]{0,100}[^%\\s\"'/])@[\\$a-z0-9:\\._%\\?=/]+|[a-z0-9]{3,5}://[^%:\\s\"'/][^:\\s\"'/\\$]+[^:\\s\"'/\\$%]:([^%\\s\"'/][^@\\s\"'/]{0,100}[^%\\s\"'/])@[\\$a-z0-9:\\._%\\?=/\\-]+)"; + private static readonly Regex WebSocketRegex = new Regex(WebSocketPattern, RegexOptions.Compiled | RegexOptions.IgnoreCase); + /// /// Removes well-known credential strings from strings. /// @@ -32,6 +36,8 @@ internal static string Sanitize(string input) return string.Empty; } + input = WebSocketRegex.Replace(input, SecretReplacement); + // Everything we *might* replace contains an equal, so if we don't have that short circuit out. // This can be likely be more efficient with a Regex, but that's best done with a large test suite and this is // a quick/simple win for the high traffic case. diff --git a/test/WebJobs.Script.Tests/SanitizerTests.cs b/test/WebJobs.Script.Tests/SanitizerTests.cs index f94f2f7bdf..9f5322b03b 100644 --- a/test/WebJobs.Script.Tests/SanitizerTests.cs +++ b/test/WebJobs.Script.Tests/SanitizerTests.cs @@ -37,6 +37,8 @@ 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&code=MiAAAAAAAAAAAAAAAAT-ag==", "test[Hidden Credential]")] + [InlineData(@"aaa://aaa:aaaaaa1111aa@aaa.aaa.io:1111/", @"[Hidden Credential]")] + [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")] public void SanitizeString(string input, string expectedOutput) { var sanitized = Sanitizer.Sanitize(input); diff --git a/test/WebJobs.Script.Tests/Workers/Rpc/RpcWorkerProcessTests.cs b/test/WebJobs.Script.Tests/Workers/Rpc/RpcWorkerProcessTests.cs index 9496305f1e..504df51e3b 100644 --- a/test/WebJobs.Script.Tests/Workers/Rpc/RpcWorkerProcessTests.cs +++ b/test/WebJobs.Script.Tests/Workers/Rpc/RpcWorkerProcessTests.cs @@ -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; @@ -163,6 +164,18 @@ public void ErrorMessageQueue_Full_Enqueue_Success() Assert.Equal("Error2,Error3,Error4", exceptionMessage); } + [Fact] + public void VerifySanitizedErrorMessage_Success() + { + 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")] From 062e992e6925607e2c68d878cc1b11f02936fcf3 Mon Sep 17 00:00:00 2001 From: Surgupta Date: Wed, 7 Aug 2024 08:52:38 -0700 Subject: [PATCH 03/19] Naming update --- src/WebJobs.Script/Sanitizer.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/WebJobs.Script/Sanitizer.cs b/src/WebJobs.Script/Sanitizer.cs index 48a64ee21a..a5c4d253fa 100644 --- a/src/WebJobs.Script/Sanitizer.cs +++ b/src/WebJobs.Script/Sanitizer.cs @@ -21,8 +21,8 @@ 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" }; - private static readonly string WebSocketPattern = "(?i)((amqp|ssh|(ht|f)tps?)://[^%:\\s\"'/][^:\\s\"'/\\$]+[^:\\s\"'/\\$%]:([^%\\s\"'/][^@\\s\"'/]{0,100}[^%\\s\"'/])@[\\$a-z0-9:\\._%\\?=/]+|[a-z0-9]{3,5}://[^%:\\s\"'/][^:\\s\"'/\\$]+[^:\\s\"'/\\$%]:([^%\\s\"'/][^@\\s\"'/]{0,100}[^%\\s\"'/])@[\\$a-z0-9:\\._%\\?=/\\-]+)"; - private static readonly Regex WebSocketRegex = new Regex(WebSocketPattern, RegexOptions.Compiled | RegexOptions.IgnoreCase); + private static readonly string Pattern = "(?i)((amqp|ssh|(ht|f)tps?)://[^%:\\s\"'/][^:\\s\"'/\\$]+[^:\\s\"'/\\$%]:([^%\\s\"'/][^@\\s\"'/]{0,100}[^%\\s\"'/])@[\\$a-z0-9:\\._%\\?=/]+|[a-z0-9]{3,5}://[^%:\\s\"'/][^:\\s\"'/\\$]+[^:\\s\"'/\\$%]:([^%\\s\"'/][^@\\s\"'/]{0,100}[^%\\s\"'/])@[\\$a-z0-9:\\._%\\?=/\\-]+)"; + private static readonly Regex Regex = new Regex(Pattern, RegexOptions.Compiled | RegexOptions.IgnoreCase); /// /// Removes well-known credential strings from strings. @@ -36,7 +36,7 @@ internal static string Sanitize(string input) return string.Empty; } - input = WebSocketRegex.Replace(input, SecretReplacement); + input = Regex.Replace(input, SecretReplacement); // Everything we *might* replace contains an equal, so if we don't have that short circuit out. // This can be likely be more efficient with a Regex, but that's best done with a large test suite and this is From 58f58c6a32b7091177d68953f421fb6b3a2050f9 Mon Sep 17 00:00:00 2001 From: Surgupta Date: Wed, 7 Aug 2024 20:09:45 -0700 Subject: [PATCH 04/19] Simplifying pattern --- src/WebJobs.Script/Sanitizer.cs | 3 ++- test/WebJobs.Script.Tests/SanitizerTests.cs | 7 +++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/WebJobs.Script/Sanitizer.cs b/src/WebJobs.Script/Sanitizer.cs index a5c4d253fa..907f1b4280 100644 --- a/src/WebJobs.Script/Sanitizer.cs +++ b/src/WebJobs.Script/Sanitizer.cs @@ -21,7 +21,8 @@ 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" }; - private static readonly string Pattern = "(?i)((amqp|ssh|(ht|f)tps?)://[^%:\\s\"'/][^:\\s\"'/\\$]+[^:\\s\"'/\\$%]:([^%\\s\"'/][^@\\s\"'/]{0,100}[^%\\s\"'/])@[\\$a-z0-9:\\._%\\?=/]+|[a-z0-9]{3,5}://[^%:\\s\"'/][^:\\s\"'/\\$]+[^:\\s\"'/\\$%]:([^%\\s\"'/][^@\\s\"'/]{0,100}[^%\\s\"'/])@[\\$a-z0-9:\\._%\\?=/\\-]+)"; + // Pattern of format : "://:@
:" + private static readonly string Pattern = "\\b(([^:\\/\\s]+):\\/\\/)?([^:\\/\\s]+):([^@\\/\\s]+)@([^:\\/\\s]+)(:[0-9]+)?(\\/\\S*)?\\b"; private static readonly Regex Regex = new Regex(Pattern, RegexOptions.Compiled | RegexOptions.IgnoreCase); /// diff --git a/test/WebJobs.Script.Tests/SanitizerTests.cs b/test/WebJobs.Script.Tests/SanitizerTests.cs index 9f5322b03b..46368b92bf 100644 --- a/test/WebJobs.Script.Tests/SanitizerTests.cs +++ b/test/WebJobs.Script.Tests/SanitizerTests.cs @@ -9,6 +9,7 @@ namespace Microsoft.Azure.WebJobs.Script.Tests public class SanitizerTests { [Theory] + /* [InlineData("", "")] [InlineData(null, "")] [InlineData("Foo", "Foo")] @@ -37,8 +38,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&code=MiAAAAAAAAAAAAAAAAT-ag==", "test[Hidden Credential]")] - [InlineData(@"aaa://aaa:aaaaaa1111aa@aaa.aaa.io:1111/", @"[Hidden Credential]")] - [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("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")] public void SanitizeString(string input, string expectedOutput) { var sanitized = Sanitizer.Sanitize(input); From d7bece73add4eb535656a2c48ef78a89735bc083 Mon Sep 17 00:00:00 2001 From: Surgupta Date: Thu, 8 Aug 2024 11:03:41 -0700 Subject: [PATCH 05/19] Cleaning up regex --- src/WebJobs.Script/Sanitizer.cs | 15 +++++++++++++-- .../Workers/Rpc/RpcWorkerProcessTests.cs | 2 +- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/src/WebJobs.Script/Sanitizer.cs b/src/WebJobs.Script/Sanitizer.cs index 907f1b4280..9950e474b4 100644 --- a/src/WebJobs.Script/Sanitizer.cs +++ b/src/WebJobs.Script/Sanitizer.cs @@ -22,8 +22,19 @@ internal static class Sanitizer private static readonly string[] CredentialNameFragments = new[] { "password", "pwd", "key", "secret", "token", "sas" }; // Pattern of format : "://:@
:" - private static readonly string Pattern = "\\b(([^:\\/\\s]+):\\/\\/)?([^:\\/\\s]+):([^@\\/\\s]+)@([^:\\/\\s]+)(:[0-9]+)?(\\/\\S*)?\\b"; - private static readonly Regex Regex = new Regex(Pattern, RegexOptions.Compiled | RegexOptions.IgnoreCase); + 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); /// /// Removes well-known credential strings from strings. diff --git a/test/WebJobs.Script.Tests/Workers/Rpc/RpcWorkerProcessTests.cs b/test/WebJobs.Script.Tests/Workers/Rpc/RpcWorkerProcessTests.cs index 504df51e3b..a0ca368617 100644 --- a/test/WebJobs.Script.Tests/Workers/Rpc/RpcWorkerProcessTests.cs +++ b/test/WebJobs.Script.Tests/Workers/Rpc/RpcWorkerProcessTests.cs @@ -167,7 +167,7 @@ public void ErrorMessageQueue_Full_Enqueue_Success() [Fact] public void VerifySanitizedErrorMessage_Success() { - WorkerProcessUtilities.AddStdErrMessage(_rpcWorkerProcess.ProcessStdErrDataQueue, "test aaa://aaa:aaaaaa1111aa@aaa.aaa.io:1111/"); + 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); From 56797fc454b0d8fc7017985bf4837fc676c06798 Mon Sep 17 00:00:00 2001 From: Surgupta Date: Thu, 8 Aug 2024 11:12:10 -0700 Subject: [PATCH 06/19] Updating tests --- test/WebJobs.Script.Tests/SanitizerTests.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/WebJobs.Script.Tests/SanitizerTests.cs b/test/WebJobs.Script.Tests/SanitizerTests.cs index 46368b92bf..d160ca0176 100644 --- a/test/WebJobs.Script.Tests/SanitizerTests.cs +++ b/test/WebJobs.Script.Tests/SanitizerTests.cs @@ -9,7 +9,6 @@ namespace Microsoft.Azure.WebJobs.Script.Tests public class SanitizerTests { [Theory] - /* [InlineData("", "")] [InlineData(null, "")] [InlineData("Foo", "Foo")] @@ -38,7 +37,6 @@ 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&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")] From 4c920927fcc2be7b5e49499e47b0f2d47c683cbb Mon Sep 17 00:00:00 2001 From: Surgupta Date: Thu, 8 Aug 2024 11:34:18 -0700 Subject: [PATCH 07/19] Using generatedRegex feature --- src/WebJobs.Script/Sanitizer.cs | 17 +-------------- src/WebJobs.Script/SanitizerRegex.cs | 32 ++++++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 16 deletions(-) create mode 100644 src/WebJobs.Script/SanitizerRegex.cs diff --git a/src/WebJobs.Script/Sanitizer.cs b/src/WebJobs.Script/Sanitizer.cs index 9950e474b4..1877e8bac2 100644 --- a/src/WebJobs.Script/Sanitizer.cs +++ b/src/WebJobs.Script/Sanitizer.cs @@ -21,21 +21,6 @@ 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 : "://:@
:" - 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); - /// /// Removes well-known credential strings from strings. /// @@ -48,7 +33,7 @@ internal static string Sanitize(string input) return string.Empty; } - input = Regex.Replace(input, SecretReplacement); + input = SanitizerRegex.Regex().Replace(input, SecretReplacement); // Everything we *might* replace contains an equal, so if we don't have that short circuit out. // This can be likely be more efficient with a Regex, but that's best done with a large test suite and this is diff --git a/src/WebJobs.Script/SanitizerRegex.cs b/src/WebJobs.Script/SanitizerRegex.cs new file mode 100644 index 0000000000..fa93801ea4 --- /dev/null +++ b/src/WebJobs.Script/SanitizerRegex.cs @@ -0,0 +1,32 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the MIT License. See License.txt in the project root for license information. + +using System.Text.RegularExpressions; + +namespace Microsoft.Azure.WebJobs.Logging +{ + internal partial class SanitizerRegex + { + // Pattern of format : "://:@
:" + 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 + "; + +#if NET8_0_OR_GREATER + [GeneratedRegex(Pattern, RegexOptions.IgnoreCase | RegexOptions.IgnorePatternWhitespace, "en-US")] + internal static partial Regex Regex(); +#else + private static readonly Regex _urlRegex = new Regex(Pattern, RegexOptions.Compiled | RegexOptions.IgnoreCase | RegexOptions.IgnorePatternWhitespace); + + internal static Regex Regex() => _urlRegex; +#endif + } +} From c1ce9c4978a395dbd6e4ae24614ce935d806f9d0 Mon Sep 17 00:00:00 2001 From: Surgupta Date: Thu, 8 Aug 2024 14:18:52 -0700 Subject: [PATCH 08/19] Addressing PR feedback --- src/WebJobs.Script/Sanitizer.cs | 4 ++-- test/WebJobs.Script.Tests/SanitizerTests.cs | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/WebJobs.Script/Sanitizer.cs b/src/WebJobs.Script/Sanitizer.cs index 1877e8bac2..12a17414d9 100644 --- a/src/WebJobs.Script/Sanitizer.cs +++ b/src/WebJobs.Script/Sanitizer.cs @@ -33,8 +33,6 @@ internal static string Sanitize(string input) return string.Empty; } - input = SanitizerRegex.Regex().Replace(input, SecretReplacement); - // Everything we *might* replace contains an equal, so if we don't have that short circuit out. // This can be likely be more efficient with a Regex, but that's best done with a large test suite and this is // a quick/simple win for the high traffic case. @@ -43,6 +41,8 @@ internal static string Sanitize(string input) return input; } + input = SanitizerRegex.Regex().Replace(input, SecretReplacement); + string t = input; string inputWithAllowedTokensHidden = input; diff --git a/test/WebJobs.Script.Tests/SanitizerTests.cs b/test/WebJobs.Script.Tests/SanitizerTests.cs index d160ca0176..0a2b13c975 100644 --- a/test/WebJobs.Script.Tests/SanitizerTests.cs +++ b/test/WebJobs.Script.Tests/SanitizerTests.cs @@ -40,6 +40,7 @@ public class SanitizerTests [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] text")] public void SanitizeString(string input, string expectedOutput) { var sanitized = Sanitizer.Sanitize(input); From 74b2f3e2f1845bb4f55a997fb55f053cc2356a60 Mon Sep 17 00:00:00 2001 From: Surgupta Date: Thu, 8 Aug 2024 14:23:37 -0700 Subject: [PATCH 09/19] Updating condition --- src/WebJobs.Script/Sanitizer.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/WebJobs.Script/Sanitizer.cs b/src/WebJobs.Script/Sanitizer.cs index 12a17414d9..c73fd79808 100644 --- a/src/WebJobs.Script/Sanitizer.cs +++ b/src/WebJobs.Script/Sanitizer.cs @@ -156,6 +156,6 @@ static JToken Sanitize(JToken token) /// Checks if a string even *possibly* contains one of our . /// Useful for short-circuiting more expensive checks and replacements if it's known we wouldn't do anything. ///
- internal static bool MayContainCredentials(string input) => input.Contains("="); + internal static bool MayContainCredentials(string input) => input.Contains("=") || input.Contains(":"); } } \ No newline at end of file From 73b87a612dd7879315f75fe779dc5df46fb9ac2e Mon Sep 17 00:00:00 2001 From: Surgupta Date: Thu, 8 Aug 2024 14:28:52 -0700 Subject: [PATCH 10/19] Updating release notes --- release_notes.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/release_notes.md b/release_notes.md index da4111d4d9..f1c23bb22f 100644 --- a/release_notes.md +++ b/release_notes.md @@ -10,4 +10,5 @@ - Update Python Worker Version to [4.30.3](https://github.com/Azure/azure-functions-python-worker/releases/tag/4.30.3) - Update PowerShell 7.2 worker to [4.0.4020](https://github.com/Azure/azure-functions-powershell-worker/releases/tag/v4.0.4020) - Update PowerShell 7.4 worker to [4.0.4021](https://github.com/Azure/azure-functions-powershell-worker/releases/tag/v4.0.4021) -- Updated dotnet-isolated worker to [1.0.10](https://github.com/Azure/azure-functions-dotnet-worker/pull/2629) (#10340) \ No newline at end of file +- Updated dotnet-isolated worker to [1.0.10](https://github.com/Azure/azure-functions-dotnet-worker/pull/2629) (#10340) +- Worker termination path update (#10367) \ No newline at end of file From 4855e55e55bb8a5c2e2406b741e0fdbb028faee9 Mon Sep 17 00:00:00 2001 From: Surgupta Date: Thu, 8 Aug 2024 14:30:15 -0700 Subject: [PATCH 11/19] Updating test --- test/WebJobs.Script.Tests/SanitizerTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/WebJobs.Script.Tests/SanitizerTests.cs b/test/WebJobs.Script.Tests/SanitizerTests.cs index 0a2b13c975..031a86dbb4 100644 --- a/test/WebJobs.Script.Tests/SanitizerTests.cs +++ b/test/WebJobs.Script.Tests/SanitizerTests.cs @@ -40,7 +40,7 @@ public class SanitizerTests [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] 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); From 5161dd4bc7c3652b68306a1572a620648bb28c4c Mon Sep 17 00:00:00 2001 From: Surgupta Date: Thu, 8 Aug 2024 15:05:24 -0700 Subject: [PATCH 12/19] Adding Perf benchmarks --- src/WebJobs.Script/Sanitizer.cs | 9 +++++++-- src/WebJobs.Script/WebJobs.Script.csproj | 1 + 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/WebJobs.Script/Sanitizer.cs b/src/WebJobs.Script/Sanitizer.cs index c73fd79808..d360d861f6 100644 --- a/src/WebJobs.Script/Sanitizer.cs +++ b/src/WebJobs.Script/Sanitizer.cs @@ -2,7 +2,7 @@ // Licensed under the MIT License. See License.txt in the project root for license information. using System; -using System.Text.RegularExpressions; +using BenchmarkDotNet.Attributes; using Newtonsoft.Json.Linq; namespace Microsoft.Azure.WebJobs.Logging @@ -26,6 +26,7 @@ internal static class Sanitizer ///
/// The string to sanitize. /// The sanitized string. + [Benchmark] internal static string Sanitize(string input) { if (string.IsNullOrEmpty(input)) @@ -41,7 +42,11 @@ internal static string Sanitize(string input) return input; } - input = SanitizerRegex.Regex().Replace(input, SecretReplacement); + // This check avoids unnecessary regex evaluation if the input does not contain any url + if (input.Contains(":")) + { + input = SanitizerRegex.Regex().Replace(input, SecretReplacement); + } string t = input; string inputWithAllowedTokensHidden = input; diff --git a/src/WebJobs.Script/WebJobs.Script.csproj b/src/WebJobs.Script/WebJobs.Script.csproj index c33c332c0b..589a9851bd 100644 --- a/src/WebJobs.Script/WebJobs.Script.csproj +++ b/src/WebJobs.Script/WebJobs.Script.csproj @@ -63,6 +63,7 @@ + From 9afbde2a9e2617b85b9f47d2a103f673f86df0bd Mon Sep 17 00:00:00 2001 From: Surgupta Date: Thu, 8 Aug 2024 15:09:27 -0700 Subject: [PATCH 13/19] Undo incorrect benchmark --- src/WebJobs.Script/Sanitizer.cs | 1 - src/WebJobs.Script/WebJobs.Script.csproj | 1 - 2 files changed, 2 deletions(-) diff --git a/src/WebJobs.Script/Sanitizer.cs b/src/WebJobs.Script/Sanitizer.cs index d360d861f6..2378d7c7d5 100644 --- a/src/WebJobs.Script/Sanitizer.cs +++ b/src/WebJobs.Script/Sanitizer.cs @@ -26,7 +26,6 @@ internal static class Sanitizer ///
/// The string to sanitize. /// The sanitized string. - [Benchmark] internal static string Sanitize(string input) { if (string.IsNullOrEmpty(input)) diff --git a/src/WebJobs.Script/WebJobs.Script.csproj b/src/WebJobs.Script/WebJobs.Script.csproj index 589a9851bd..c33c332c0b 100644 --- a/src/WebJobs.Script/WebJobs.Script.csproj +++ b/src/WebJobs.Script/WebJobs.Script.csproj @@ -63,7 +63,6 @@ - From 28334e897c9563e56e4897366f68545331c76d32 Mon Sep 17 00:00:00 2001 From: Surgupta Date: Thu, 8 Aug 2024 15:10:29 -0700 Subject: [PATCH 14/19] Remove ununsed usings --- src/WebJobs.Script/Sanitizer.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/WebJobs.Script/Sanitizer.cs b/src/WebJobs.Script/Sanitizer.cs index 2378d7c7d5..77a7154872 100644 --- a/src/WebJobs.Script/Sanitizer.cs +++ b/src/WebJobs.Script/Sanitizer.cs @@ -2,7 +2,6 @@ // Licensed under the MIT License. See License.txt in the project root for license information. using System; -using BenchmarkDotNet.Attributes; using Newtonsoft.Json.Linq; namespace Microsoft.Azure.WebJobs.Logging From b04b47afefc4a6992d962a6a6beab4b3a8bc0126 Mon Sep 17 00:00:00 2001 From: Surgupta Date: Thu, 8 Aug 2024 15:14:59 -0700 Subject: [PATCH 15/19] Adding perf benchmarks --- .../SanitizerBenchmarks.cs | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) create mode 100644 benchmarks/WebJobs.Script.Benchmarks/SanitizerBenchmarks.cs diff --git a/benchmarks/WebJobs.Script.Benchmarks/SanitizerBenchmarks.cs b/benchmarks/WebJobs.Script.Benchmarks/SanitizerBenchmarks.cs new file mode 100644 index 0000000000..b8a04fedea --- /dev/null +++ b/benchmarks/WebJobs.Script.Benchmarks/SanitizerBenchmarks.cs @@ -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"); + } + } +} From cc3e9c50426029fcd7dda6360562f373cdbbe131 Mon Sep 17 00:00:00 2001 From: Surgupta Date: Thu, 8 Aug 2024 16:14:41 -0700 Subject: [PATCH 16/19] Cleaning up --- src/WebJobs.Script/Sanitizer.cs | 18 +++++++++++++++- src/WebJobs.Script/SanitizerRegex.cs | 32 ---------------------------- 2 files changed, 17 insertions(+), 33 deletions(-) delete mode 100644 src/WebJobs.Script/SanitizerRegex.cs diff --git a/src/WebJobs.Script/Sanitizer.cs b/src/WebJobs.Script/Sanitizer.cs index 77a7154872..be9de546d5 100644 --- a/src/WebJobs.Script/Sanitizer.cs +++ b/src/WebJobs.Script/Sanitizer.cs @@ -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 @@ -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 : "://:@
:" + 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); + /// /// Removes well-known credential strings from strings. /// @@ -43,7 +59,7 @@ internal static string Sanitize(string input) // This check avoids unnecessary regex evaluation if the input does not contain any url if (input.Contains(":")) { - input = SanitizerRegex.Regex().Replace(input, SecretReplacement); + input = Regex.Replace(input, SecretReplacement); } string t = input; diff --git a/src/WebJobs.Script/SanitizerRegex.cs b/src/WebJobs.Script/SanitizerRegex.cs deleted file mode 100644 index fa93801ea4..0000000000 --- a/src/WebJobs.Script/SanitizerRegex.cs +++ /dev/null @@ -1,32 +0,0 @@ -// Copyright (c) .NET Foundation. All rights reserved. -// Licensed under the MIT License. See License.txt in the project root for license information. - -using System.Text.RegularExpressions; - -namespace Microsoft.Azure.WebJobs.Logging -{ - internal partial class SanitizerRegex - { - // Pattern of format : "://:@
:" - 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 - "; - -#if NET8_0_OR_GREATER - [GeneratedRegex(Pattern, RegexOptions.IgnoreCase | RegexOptions.IgnorePatternWhitespace, "en-US")] - internal static partial Regex Regex(); -#else - private static readonly Regex _urlRegex = new Regex(Pattern, RegexOptions.Compiled | RegexOptions.IgnoreCase | RegexOptions.IgnorePatternWhitespace); - - internal static Regex Regex() => _urlRegex; -#endif - } -} From 3f66040e1903206dc289a956fefcdbff6ffd2f70 Mon Sep 17 00:00:00 2001 From: Surgupta Date: Fri, 9 Aug 2024 10:31:14 -0700 Subject: [PATCH 17/19] Updating location of check --- src/WebJobs.Script/Sanitizer.cs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/WebJobs.Script/Sanitizer.cs b/src/WebJobs.Script/Sanitizer.cs index be9de546d5..ffff40c9bd 100644 --- a/src/WebJobs.Script/Sanitizer.cs +++ b/src/WebJobs.Script/Sanitizer.cs @@ -56,12 +56,6 @@ internal static string Sanitize(string input) return input; } - // This check avoids unnecessary regex evaluation if the input does not contain any url - if (input.Contains(":")) - { - input = Regex.Replace(input, SecretReplacement); - } - string t = input; string inputWithAllowedTokensHidden = input; @@ -95,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; } From 6eafdcabbb703d307bd2d94366d32ec69a0a8aeb Mon Sep 17 00:00:00 2001 From: Surgupta Date: Mon, 12 Aug 2024 16:23:55 -0700 Subject: [PATCH 18/19] Remvoing redundant test --- .../Workers/Rpc/RpcWorkerProcessTests.cs | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/test/WebJobs.Script.Tests/Workers/Rpc/RpcWorkerProcessTests.cs b/test/WebJobs.Script.Tests/Workers/Rpc/RpcWorkerProcessTests.cs index a0ca368617..c8ba6f4e01 100644 --- a/test/WebJobs.Script.Tests/Workers/Rpc/RpcWorkerProcessTests.cs +++ b/test/WebJobs.Script.Tests/Workers/Rpc/RpcWorkerProcessTests.cs @@ -164,18 +164,6 @@ public void ErrorMessageQueue_Full_Enqueue_Success() Assert.Equal("Error2,Error3,Error4", exceptionMessage); } - [Fact] - public void VerifySanitizedErrorMessage_Success() - { - 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")] From 0d6e7b85e908d38bcb740b4e3e29c218223e0e82 Mon Sep 17 00:00:00 2001 From: Surgupta Date: Mon, 12 Aug 2024 16:29:31 -0700 Subject: [PATCH 19/19] Updated release notes --- release_notes.md | 2 +- test/WebJobs.Script.Tests/Workers/Rpc/RpcWorkerProcessTests.cs | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/release_notes.md b/release_notes.md index 68586b79ba..6e83f42803 100644 --- a/release_notes.md +++ b/release_notes.md @@ -15,4 +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) \ No newline at end of file +- Worker termination path updated with sanitized logging (#10367) \ No newline at end of file diff --git a/test/WebJobs.Script.Tests/Workers/Rpc/RpcWorkerProcessTests.cs b/test/WebJobs.Script.Tests/Workers/Rpc/RpcWorkerProcessTests.cs index c8ba6f4e01..9496305f1e 100644 --- a/test/WebJobs.Script.Tests/Workers/Rpc/RpcWorkerProcessTests.cs +++ b/test/WebJobs.Script.Tests/Workers/Rpc/RpcWorkerProcessTests.cs @@ -5,7 +5,6 @@ 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;