From 34521ccdad0ae5d011a83fb3b33b7c65fce6e707 Mon Sep 17 00:00:00 2001 From: Scott Beddall <45376673+scbedd@users.noreply.github.com> Date: Wed, 15 May 2024 13:45:53 -0700 Subject: [PATCH] Feedback from `Java` Integration (#8272) * ensure we get the exact same version of the list. this is probably paranoia but nothing telling we can't do it * make it so that a crashing body key sanitizer is logged, but doesn't kill the sanitization session * handle when the targeted path actually exists before attempting to secret scan it * remove duplicate sanitizer --- .../Common/ModifiableRecordSession.cs | 10 ++++- .../Common/SanitizerDictionary.cs | 4 -- .../Common/SecretScanner.cs | 21 ++++++---- .../Sanitizers/BodyKeySanitizer.cs | 41 +++++++++++-------- 4 files changed, 45 insertions(+), 31 deletions(-) diff --git a/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Common/ModifiableRecordSession.cs b/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Common/ModifiableRecordSession.cs index dc7eff129af..cd7776cc53f 100644 --- a/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Common/ModifiableRecordSession.cs +++ b/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Common/ModifiableRecordSession.cs @@ -14,14 +14,20 @@ public class ModifiableRecordSession public ModifiableRecordSession(SanitizerDictionary sanitizerRegistry, string sessionId) { - this.AppliedSanitizers = sanitizerRegistry.SessionSanitizers.ToList(); + lock(sanitizerRegistry.SessionSanitizerLock) + { + this.AppliedSanitizers = sanitizerRegistry.SessionSanitizers.ToList(); + } this.SessionId = sessionId; } public ModifiableRecordSession(RecordSession session, SanitizerDictionary sanitizerRegistry, string sessionId) { Session = session; - this.AppliedSanitizers = sanitizerRegistry.SessionSanitizers.ToList(); + lock (sanitizerRegistry.SessionSanitizerLock) + { + this.AppliedSanitizers = sanitizerRegistry.SessionSanitizers.ToList(); + } this.SessionId = sessionId; } diff --git a/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Common/SanitizerDictionary.cs b/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Common/SanitizerDictionary.cs index b45aa74278a..ea6d124cc80 100644 --- a/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Common/SanitizerDictionary.cs +++ b/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Common/SanitizerDictionary.cs @@ -607,10 +607,6 @@ public SanitizerDictionary() { new BodyKeySanitizer("$..apiKey"), "AZSDK3480" ), - new RegisteredSanitizer( - new BodyKeySanitizer("$..connectionString"), - "AZSDK3481" - ), new RegisteredSanitizer( new BodyKeySanitizer("$..password"), "AZSDK3482" diff --git a/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Common/SecretScanner.cs b/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Common/SecretScanner.cs index 82bb90ef873..5a650661115 100644 --- a/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Common/SecretScanner.cs +++ b/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Common/SecretScanner.cs @@ -38,20 +38,25 @@ public List> DiscoverSecrets(string assetRepoRoot, IEnu Parallel.ForEach(relativePaths, options, (filePath) => { - var content = File.ReadAllText(Path.Combine(assetRepoRoot, filePath)); - var fileDetections = DetectSecrets(content); + var path = Path.Combine(assetRepoRoot, filePath); - if (fileDetections != null && fileDetections.Count > 0) + if (File.Exists(path)) { - foreach (Detection detection in fileDetections) + var content = File.ReadAllText(path); + var fileDetections = DetectSecrets(content); + + if (fileDetections != null && fileDetections.Count > 0) { - detectedSecrets.Add(Tuple.Create(filePath, detection)); + foreach (Detection detection in fileDetections) + { + detectedSecrets.Add(Tuple.Create(filePath, detection)); + } } - } - Interlocked.Increment(ref seen); + Interlocked.Increment(ref seen); - Console.Write($"\r\u001b[2KScanned {seen}/{total}."); + Console.Write($"\r\u001b[2KScanned {seen}/{total}."); + } }); Console.WriteLine(string.Empty); diff --git a/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Sanitizers/BodyKeySanitizer.cs b/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Sanitizers/BodyKeySanitizer.cs index 9ee1c28428c..ac638be0c13 100644 --- a/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Sanitizers/BodyKeySanitizer.cs +++ b/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Sanitizers/BodyKeySanitizer.cs @@ -64,33 +64,40 @@ public override string SanitizeTextBody(string contentType, string body) return body; } - if (jsonO != null) { - foreach (JToken token in jsonO.SelectTokens(_jsonPath)) + try { - // HasValues is false for tokens with children. We will not apply sanitization if that is the case. - if (!token.HasValues) + foreach (JToken token in jsonO.SelectTokens(_jsonPath)) { - var originalValue = token.Value(); - - // regex replacement does not support null - if (originalValue == null) + // HasValues is false for tokens with children. We will not apply sanitization if that is the case. + if (!token.HasValues) { - continue; - } + var originalValue = token.Value(); - var replacement = StringSanitizer.SanitizeValue(originalValue, _newValue, _regexValue, _groupForReplace); + // regex replacement does not support null + if (originalValue == null) + { + continue; + } - // this sanitizer should only apply to actual values - // if we attempt to apply a regex update to a jtoken that has a more complex type, throw - token.Replace(JToken.FromObject(replacement)); + var replacement = StringSanitizer.SanitizeValue(originalValue, _newValue, _regexValue, _groupForReplace); - if (originalValue != replacement) - { - sanitized = true; + // this sanitizer should only apply to actual values + // if we attempt to apply a regex update to a jtoken that has a more complex type, throw + token.Replace(JToken.FromObject(replacement)); + + if (originalValue != replacement) + { + sanitized = true; + } } } + } + catch(Exception e) + { + DebugLogger.LogError($"Ran into exception \"{e.Message}\" while attempting to run regex \"{_regexValue}\" against body value \"{body}\""); + return body; } }