From d4048534f2dd61b302002a4bcf6c7887b0b5e275 Mon Sep 17 00:00:00 2001 From: Tom Longhurst <30480171+thomhurst@users.noreply.github.com> Date: Tue, 4 Aug 2026 21:08:42 +0100 Subject: [PATCH] perf: obfuscate secrets in one scan --- .../Engine/SecretObfuscator.cs | 112 +++++++----------- .../Logging/SecretObfuscatorCachingTests.cs | 35 +++++- 2 files changed, 78 insertions(+), 69 deletions(-) diff --git a/src/ModularPipelines/Engine/SecretObfuscator.cs b/src/ModularPipelines/Engine/SecretObfuscator.cs index 991ae5a3b61..1092cb0c714 100644 --- a/src/ModularPipelines/Engine/SecretObfuscator.cs +++ b/src/ModularPipelines/Engine/SecretObfuscator.cs @@ -16,14 +16,10 @@ namespace ModularPipelines.Engine; /// called concurrently from multiple threads without external synchronization. /// /// -/// Performance: For optimal performance, secrets are sorted by length (longest first) -/// to ensure longer secrets are masked before shorter ones that might be substrings. -/// When case-insensitive matching is enabled, a single-pass algorithm using -/// is used. This approach uses .NET's -/// highly optimized string search which performs well for typical log message sizes. -/// For extremely large log outputs with many secrets, consider reducing the number of -/// registered secrets or using case-sensitive matching which uses the more efficient -/// . +/// Performance: Secrets are sorted by length (longest first) so longer secrets +/// win when multiple patterns match at the same position. A cached +/// finds matching positions in a single forward pass, +/// regardless of whether matching is case-sensitive. /// /// /// @@ -74,14 +70,12 @@ public string Obfuscate(string? input, object? optionsObject) return input; } - // For case-sensitive matching, StringBuilder.Replace is efficient - // For case-insensitive matching, we need a different approach - if (caseInsensitive) - { - return ObfuscateCaseInsensitive(input, secretCache.Secrets, maskValue); - } - - return ObfuscateCaseSensitive(input, secretCache.Secrets, maskValue); + return ObfuscateMatches( + input, + secretCache.Secrets, + secretCache.SearchValues, + maskValue, + caseInsensitive ? StringComparison.OrdinalIgnoreCase : StringComparison.Ordinal); } internal SecretCache GetSecretCache( @@ -198,66 +192,48 @@ private static SecretCache CreateSecretCache( searchValues); } - /// - /// Performs case-sensitive obfuscation using StringBuilder.Replace. - /// This is the most efficient approach for case-sensitive matching. - /// - private static string ObfuscateCaseSensitive(string input, IReadOnlyList secrets, string maskValue) + private static string ObfuscateMatches( + string input, + IReadOnlyList secrets, + SearchValues searchValues, + string maskValue, + StringComparison comparison) { - var stringBuilder = new StringBuilder(input); - - foreach (var secret in secrets) + var result = new StringBuilder(input.Length); + var inputOffset = 0; + while (inputOffset < input.Length) { - stringBuilder.Replace(secret, maskValue); - } - - return stringBuilder.ToString(); - } + var remainingInput = input.AsSpan(inputOffset); + var relativeMatchIndex = remainingInput.IndexOfAny(searchValues); + if (relativeMatchIndex < 0) + { + break; + } - /// - /// Performs case-insensitive obfuscation using IndexOf with OrdinalIgnoreCase. - /// - private static string ObfuscateCaseInsensitive(string input, IReadOnlyList secrets, string maskValue) - { - var result = input; + var matchIndex = inputOffset + relativeMatchIndex; + result.Append(input, inputOffset, matchIndex - inputOffset); - foreach (var secret in secrets) - { - result = ReplaceCaseInsensitive(result, secret, maskValue); - } - - return result; - } - - /// - /// Replaces all occurrences of a pattern in a string, case-insensitively. - /// - private static string ReplaceCaseInsensitive(string input, string pattern, string replacement) - { - if (pattern.Length == 0) - { - return input; - } + var matchingInput = input.AsSpan(matchIndex); + string? matchedSecret = null; + foreach (var secret in secrets) + { + if (matchingInput.StartsWith(secret, comparison)) + { + matchedSecret = secret; + break; + } + } - var firstIndex = input.IndexOf(pattern, StringComparison.OrdinalIgnoreCase); - if (firstIndex < 0) - { - return input; - } + if (matchedSecret is null) + { + throw new InvalidOperationException("SearchValues returned a position without a matching secret."); + } - var result = new StringBuilder(input.Length); - var lastIndex = 0; - var index = firstIndex; - while (index >= 0) - { - result.Append(input, lastIndex, index - lastIndex); - result.Append(replacement); - lastIndex = index + pattern.Length; - index = input.IndexOf(pattern, lastIndex, StringComparison.OrdinalIgnoreCase); + result.Append(maskValue); + inputOffset = matchIndex + matchedSecret.Length; } - // Append the remaining part after the last match - result.Append(input, lastIndex, input.Length - lastIndex); + result.Append(input, inputOffset, input.Length - inputOffset); return result.ToString(); } diff --git a/test/ModularPipelines.UnitTests/Logging/SecretObfuscatorCachingTests.cs b/test/ModularPipelines.UnitTests/Logging/SecretObfuscatorCachingTests.cs index fb0d745e3d5..c67a79e1d43 100644 --- a/test/ModularPipelines.UnitTests/Logging/SecretObfuscatorCachingTests.cs +++ b/test/ModularPipelines.UnitTests/Logging/SecretObfuscatorCachingTests.cs @@ -36,6 +36,37 @@ public async Task ReturnsOriginalStringWhenNoSecretMatches() await Assert.That(result).IsSameReferenceAs(input); } + [Test] + public async Task UsesLongestSecretWhenPatternsMatchAtSamePosition() + { + var secretProvider = new Mock(); + secretProvider.Setup(x => x.GetSnapshot()) + .Returns(new SecretSnapshot(0, ["secret", "secret-value"])); + var obfuscator = CreateObfuscator(secretProvider.Object); + + var result = obfuscator.Obfuscate("secret-value and secret", null); + + await Assert.That(result).IsEqualTo("********** and **********"); + } + + [Test] + [Arguments(false)] + [Arguments(true)] + public async Task DoesNotRescanMaskReplacement(bool caseInsensitive) + { + var secretProvider = new Mock(); + secretProvider.Setup(x => x.GetSnapshot()) + .Returns(new SecretSnapshot(0, ["secret", "*"])); + var obfuscator = CreateObfuscator( + secretProvider.Object, + caseInsensitive, + maskValue: "***"); + + var result = obfuscator.Obfuscate(caseInsensitive ? "SECRET" : "secret", null); + + await Assert.That(result).IsEqualTo("***"); + } + [Test] public async Task RebuildsSecretSnapshotWhenProviderVersionChanges() { @@ -219,13 +250,15 @@ public async Task InvalidatesOptionsCacheWhenOptionsObjectChanges() private static SecretObfuscator CreateObfuscator( ISecretProvider secretProvider, - bool caseInsensitive = false) + bool caseInsensitive = false, + string? maskValue = null) { return new SecretObfuscator( secretProvider, Microsoft.Extensions.Options.Options.Create(new SecretMaskingOptions { CaseInsensitive = caseInsensitive, + MaskValue = maskValue ?? "**********", })); } }