From c74243e185f6b828ef5164f5bc63cf2133db3daf Mon Sep 17 00:00:00 2001 From: "Scott Beddall (from Dev Box)" Date: Mon, 22 Apr 2024 17:31:31 -0700 Subject: [PATCH 01/18] add first simple cut at detecting secrets when calling push --- .../Azure.Sdk.Tools.TestProxy.csproj | 1 + .../Common/SecretScanner.cs | 47 +++++++++++++++++++ .../Store/GitStore.cs | 18 +++++-- 3 files changed, 63 insertions(+), 3 deletions(-) create mode 100644 tools/test-proxy/Azure.Sdk.Tools.TestProxy/Common/SecretScanner.cs diff --git a/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Azure.Sdk.Tools.TestProxy.csproj b/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Azure.Sdk.Tools.TestProxy.csproj index b6cc7713b91..20c5cb31ada 100644 --- a/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Azure.Sdk.Tools.TestProxy.csproj +++ b/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Azure.Sdk.Tools.TestProxy.csproj @@ -22,6 +22,7 @@ + diff --git a/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Common/SecretScanner.cs b/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Common/SecretScanner.cs new file mode 100644 index 00000000000..be3be4fb3a6 --- /dev/null +++ b/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Common/SecretScanner.cs @@ -0,0 +1,47 @@ +using System.Collections.Generic; +using System.IO; +using System.Threading.Tasks; +using Microsoft.Security.Utilities; +using static System.Net.WebRequestMethods; + +namespace Azure.Sdk.Tools.TestProxy.Common +{ + public static class SecretScanner + { + public static SecretMasker SecretMasker = new SecretMasker(WellKnownRegexPatterns.HighConfidenceMicrosoftSecurityModels, generateCorrelatingIds: true); + + public static async Task> DiscoverSecrets(string inputDirectory) + { + var files = Directory.GetFiles("", "*", SearchOption.AllDirectories); + List detectedSecrets = new List(); + + foreach (string filePath in files) + { + var content = await ReadFile(filePath); + var fileDetections = await DetectSecrets(content); + + if (!string.IsNullOrWhiteSpace(fileDetections)) + { + detectedSecrets.Add(fileDetections); + } + } + + return detectedSecrets; + } + + public static async Task ReadFile(string filePath) + { + using (StreamReader reader = new StreamReader(filePath)) + { + return await reader.ReadToEndAsync(); + } + } + + public static async Task DetectSecrets(string stringContent) + { + await Task.Delay(100); + return "hah!"; + } + + } +} diff --git a/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Store/GitStore.cs b/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Store/GitStore.cs index 69e0a5dd52f..8967f8974e9 100644 --- a/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Store/GitStore.cs +++ b/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Store/GitStore.cs @@ -3,18 +3,16 @@ using System; using System.Text.Json; using System.Threading.Tasks; -using System.Diagnostics; using System.Net.Http; using System.Net.Http.Headers; -using System.Text; using System.Linq; using Azure.Sdk.Tools.TestProxy.Common.Exceptions; using Azure.Sdk.Tools.TestProxy.Common; using Azure.Sdk.Tools.TestProxy.Console; using System.Collections.Concurrent; -using System.Reflection.Metadata; using System.Text.RegularExpressions; using Azure.Sdk.tools.TestProxy.Common; +using Microsoft.Security.Utilities; namespace Azure.Sdk.Tools.TestProxy.Store { @@ -94,6 +92,18 @@ public async Task GetPath(string pathToAssetsJson) return new NormalizedString(config.AssetsRepoLocation); } + public void CheckForSecrets(AssetsConfiguration assetsConfiguration) + { + var secretsDetected = false; + + + if (secretsDetected) + { + _consoleWrapper.WriteLine("A secret was detected in the pushed code. Please register a sanitizer, re-record, and attempt pushing again. Detailed errors follow: "); + Environment.Exit(-1); + } + } + /// /// Pushes a set of changed files to the assets repo. Honors configuration of assets.json passed into it. /// @@ -120,6 +130,8 @@ public async Task Push(string pathToAssetsJson) { if (pendingChanges.Length > 0) { + CheckForSecrets(config); + try { string branchGuid = Guid.NewGuid().ToString().Substring(0, 8); From a3aaebeea8160cff59207992b29b7de90309d18e Mon Sep 17 00:00:00 2001 From: "Scott Beddall (from Dev Box)" Date: Mon, 22 Apr 2024 17:36:41 -0700 Subject: [PATCH 02/18] we are calling into this thing now. time to add a test for it --- .../Azure.Sdk.Tools.TestProxy/Common/SecretScanner.cs | 5 ++--- .../Azure.Sdk.Tools.TestProxy/Store/GitStore.cs | 9 ++++----- 2 files changed, 6 insertions(+), 8 deletions(-) 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 be3be4fb3a6..3b61c7c400a 100644 --- a/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Common/SecretScanner.cs +++ b/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Common/SecretScanner.cs @@ -2,7 +2,6 @@ using System.IO; using System.Threading.Tasks; using Microsoft.Security.Utilities; -using static System.Net.WebRequestMethods; namespace Azure.Sdk.Tools.TestProxy.Common { @@ -29,7 +28,7 @@ public static async Task> DiscoverSecrets(string inputDirectory) return detectedSecrets; } - public static async Task ReadFile(string filePath) + private static async Task ReadFile(string filePath) { using (StreamReader reader = new StreamReader(filePath)) { @@ -37,7 +36,7 @@ public static async Task ReadFile(string filePath) } } - public static async Task DetectSecrets(string stringContent) + private static async Task DetectSecrets(string stringContent) { await Task.Delay(100); return "hah!"; diff --git a/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Store/GitStore.cs b/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Store/GitStore.cs index 8967f8974e9..aa0b0e289c2 100644 --- a/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Store/GitStore.cs +++ b/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Store/GitStore.cs @@ -92,12 +92,11 @@ public async Task GetPath(string pathToAssetsJson) return new NormalizedString(config.AssetsRepoLocation); } - public void CheckForSecrets(AssetsConfiguration assetsConfiguration) + public async Task CheckForSecrets(GitAssetsConfiguration assetsConfiguration) { - var secretsDetected = false; + var detectedSecrets = await SecretScanner.DiscoverSecrets(assetsConfiguration.AssetsRepoLocation); - - if (secretsDetected) + if (detectedSecrets.Count > 0) { _consoleWrapper.WriteLine("A secret was detected in the pushed code. Please register a sanitizer, re-record, and attempt pushing again. Detailed errors follow: "); Environment.Exit(-1); @@ -130,7 +129,7 @@ public async Task Push(string pathToAssetsJson) { if (pendingChanges.Length > 0) { - CheckForSecrets(config); + await CheckForSecrets(config); try { From b99ef923db98712f73e05830aa5a45b8b73d1463 Mon Sep 17 00:00:00 2001 From: "Scott Beddall (from Dev Box)" Date: Mon, 22 Apr 2024 18:02:40 -0700 Subject: [PATCH 03/18] close enough to actually cause this thing to error --- .../Common/SecretScanner.cs | 18 ++++++++++-------- .../Store/GitStore.cs | 10 ++++++++++ 2 files changed, 20 insertions(+), 8 deletions(-) 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 3b61c7c400a..c18bebf82fb 100644 --- a/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Common/SecretScanner.cs +++ b/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Common/SecretScanner.cs @@ -9,19 +9,22 @@ public static class SecretScanner { public static SecretMasker SecretMasker = new SecretMasker(WellKnownRegexPatterns.HighConfidenceMicrosoftSecurityModels, generateCorrelatingIds: true); - public static async Task> DiscoverSecrets(string inputDirectory) + public static async Task> DiscoverSecrets(string inputDirectory) { var files = Directory.GetFiles("", "*", SearchOption.AllDirectories); - List detectedSecrets = new List(); + List detectedSecrets = new List(); foreach (string filePath in files) { var content = await ReadFile(filePath); - var fileDetections = await DetectSecrets(content); + var fileDetections = DetectSecrets(content); - if (!string.IsNullOrWhiteSpace(fileDetections)) + if (fileDetections != null && fileDetections.Count > 0) { - detectedSecrets.Add(fileDetections); + foreach(Detection detection in fileDetections) + { + detectedSecrets.Add(detection); + } } } @@ -36,10 +39,9 @@ private static async Task ReadFile(string filePath) } } - private static async Task DetectSecrets(string stringContent) + private static ICollection DetectSecrets(string stringContent) { - await Task.Delay(100); - return "hah!"; + return SecretMasker.DetectSecrets(stringContent); } } diff --git a/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Store/GitStore.cs b/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Store/GitStore.cs index aa0b0e289c2..e9497ae261e 100644 --- a/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Store/GitStore.cs +++ b/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Store/GitStore.cs @@ -92,6 +92,12 @@ public async Task GetPath(string pathToAssetsJson) return new NormalizedString(config.AssetsRepoLocation); } + /// + /// Currently this is written to always scan every file within the repo before pushing. By passing the _results_ of DetectPendingChanges (the set of changed files) + /// as an argument to this function, we can easily short circuit the scan and only scan the CHANGED files. + /// + /// + /// public async Task CheckForSecrets(GitAssetsConfiguration assetsConfiguration) { var detectedSecrets = await SecretScanner.DiscoverSecrets(assetsConfiguration.AssetsRepoLocation); @@ -99,6 +105,10 @@ public async Task CheckForSecrets(GitAssetsConfiguration assetsConfiguration) if (detectedSecrets.Count > 0) { _consoleWrapper.WriteLine("A secret was detected in the pushed code. Please register a sanitizer, re-record, and attempt pushing again. Detailed errors follow: "); + foreach (var detection in detectedSecrets) + { + _consoleWrapper.WriteLine(detection.ToString()); + } Environment.Exit(-1); } } From e7ade6a3cb1fdda4b17de4293e62d55fc2343618 Mon Sep 17 00:00:00 2001 From: "Scott Beddall (from Dev Box)" Date: Wed, 24 Apr 2024 14:07:03 -0700 Subject: [PATCH 04/18] secret scanner not passing the proper directory --- .../Azure.Sdk.Tools.TestProxy/Common/SecretScanner.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) 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 c18bebf82fb..af4d3ca8107 100644 --- a/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Common/SecretScanner.cs +++ b/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Common/SecretScanner.cs @@ -11,9 +11,10 @@ public static class SecretScanner public static async Task> DiscoverSecrets(string inputDirectory) { - var files = Directory.GetFiles("", "*", SearchOption.AllDirectories); + var files = Directory.GetFiles(inputDirectory, "*", SearchOption.AllDirectories); List detectedSecrets = new List(); + var total = 0; foreach (string filePath in files) { var content = await ReadFile(filePath); @@ -26,6 +27,7 @@ public static async Task> DiscoverSecrets(string inputDirectory) detectedSecrets.Add(detection); } } + total++; } return detectedSecrets; From b1a552f6c196c9dbc757e7d2de730ac77686ce43 Mon Sep 17 00:00:00 2001 From: "Scott Beddall (from Dev Box)" Date: Wed, 24 Apr 2024 14:51:20 -0700 Subject: [PATCH 05/18] move away from static, handle lowconfidence matches too --- .../Common/SecretScanner.cs | 17 ++++++++++++----- .../Azure.Sdk.Tools.TestProxy/Store/GitStore.cs | 1 + 2 files changed, 13 insertions(+), 5 deletions(-) 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 af4d3ca8107..ee2d14fc1ba 100644 --- a/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Common/SecretScanner.cs +++ b/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Common/SecretScanner.cs @@ -1,15 +1,22 @@ using System.Collections.Generic; using System.IO; +using System.Linq; using System.Threading.Tasks; using Microsoft.Security.Utilities; namespace Azure.Sdk.Tools.TestProxy.Common { - public static class SecretScanner + public class SecretScanner { - public static SecretMasker SecretMasker = new SecretMasker(WellKnownRegexPatterns.HighConfidenceMicrosoftSecurityModels, generateCorrelatingIds: true); + public SecretMasker SecretMasker = new SecretMasker( + WellKnownRegexPatterns.HighConfidenceMicrosoftSecurityModels.Concat(WellKnownRegexPatterns.LowConfidencePotentialSecurityKeys), + generateCorrelatingIds: true); - public static async Task> DiscoverSecrets(string inputDirectory) + public SecretScanner() { + + } + + public async Task> DiscoverSecrets(string inputDirectory) { var files = Directory.GetFiles(inputDirectory, "*", SearchOption.AllDirectories); List detectedSecrets = new List(); @@ -33,7 +40,7 @@ public static async Task> DiscoverSecrets(string inputDirectory) return detectedSecrets; } - private static async Task ReadFile(string filePath) + private async Task ReadFile(string filePath) { using (StreamReader reader = new StreamReader(filePath)) { @@ -41,7 +48,7 @@ private static async Task ReadFile(string filePath) } } - private static ICollection DetectSecrets(string stringContent) + private ICollection DetectSecrets(string stringContent) { return SecretMasker.DetectSecrets(stringContent); } diff --git a/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Store/GitStore.cs b/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Store/GitStore.cs index e9497ae261e..353a8f8672b 100644 --- a/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Store/GitStore.cs +++ b/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Store/GitStore.cs @@ -39,6 +39,7 @@ public class GitStore : IAssetsStore public static readonly string GIT_COMMIT_OWNER_ENV_VAR = "GIT_COMMIT_OWNER"; public static readonly string GIT_COMMIT_EMAIL_ENV_VAR = "GIT_COMMIT_EMAIL"; private bool LocalCacheRefreshed = false; + public SecretScanner SecretScanner = new SecretScanner(); public GitStoreBreadcrumb BreadCrumb = new GitStoreBreadcrumb(); From 3aa8530381632fee0d51e6e3dc24198edfbe274d Mon Sep 17 00:00:00 2001 From: "Scott Beddall (from Dev Box)" Date: Wed, 1 May 2024 13:42:25 -0700 Subject: [PATCH 06/18] git status --- .../Azure.Sdk.Tools.TestProxy/Azure.Sdk.Tools.TestProxy.csproj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Azure.Sdk.Tools.TestProxy.csproj b/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Azure.Sdk.Tools.TestProxy.csproj index 20c5cb31ada..532a087ea9c 100644 --- a/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Azure.Sdk.Tools.TestProxy.csproj +++ b/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Azure.Sdk.Tools.TestProxy.csproj @@ -1,6 +1,6 @@ - net6.0 + net7.0,net8.0 $(NoWarn);1591;AZC0001;AZC0012;CA1724;CA1801;CA1812;CA1822;SA1028 true true From 55a8c1d320f3cb3e2db09eb8018a198c8d9b098e Mon Sep 17 00:00:00 2001 From: "Scott Beddall (from Dev Box)" Date: Mon, 6 May 2024 16:41:23 -0700 Subject: [PATCH 07/18] restore this to a working version --- .../Azure.Sdk.Tools.TestProxy/Azure.Sdk.Tools.TestProxy.csproj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Azure.Sdk.Tools.TestProxy.csproj b/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Azure.Sdk.Tools.TestProxy.csproj index 532a087ea9c..20c5cb31ada 100644 --- a/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Azure.Sdk.Tools.TestProxy.csproj +++ b/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Azure.Sdk.Tools.TestProxy.csproj @@ -1,6 +1,6 @@ - net7.0,net8.0 + net6.0 $(NoWarn);1591;AZC0001;AZC0012;CA1724;CA1801;CA1812;CA1822;SA1028 true true From 3aecbc8a329e8315ecda1df3d5cb7a5d1cea1029 Mon Sep 17 00:00:00 2001 From: "Scott Beddall (from Dev Box)" Date: Tue, 7 May 2024 12:31:49 -0700 Subject: [PATCH 08/18] clarify how we're going to represent a check and how it passes through --- .../GitStoreIntegrationPushTests.cs | 75 +++++++++++++++++++ .../Common/SecretScanner.cs | 34 ++++++++- .../Store/GitStore.cs | 27 ++++--- 3 files changed, 123 insertions(+), 13 deletions(-) diff --git a/tools/test-proxy/Azure.Sdk.Tools.TestProxy.Tests/IntegrationTests/GitStoreIntegrationPushTests.cs b/tools/test-proxy/Azure.Sdk.Tools.TestProxy.Tests/IntegrationTests/GitStoreIntegrationPushTests.cs index c829abf5e2f..1408ee5b291 100644 --- a/tools/test-proxy/Azure.Sdk.Tools.TestProxy.Tests/IntegrationTests/GitStoreIntegrationPushTests.cs +++ b/tools/test-proxy/Azure.Sdk.Tools.TestProxy.Tests/IntegrationTests/GitStoreIntegrationPushTests.cs @@ -5,6 +5,7 @@ using System.Text.Json; using System.Threading.Tasks; using Azure.Sdk.Tools.TestProxy.Common; +using Azure.Sdk.Tools.TestProxy.Console; using Azure.Sdk.Tools.TestProxy.Store; using Microsoft.Extensions.Logging; using Xunit; @@ -581,5 +582,79 @@ public async Task LargePushPerformance(int numberOfFiles, double fileSize) TestHelpers.CleanupIntegrationTestTag(updatedAssets); } } + + /// + /// 1. Restore from empty tag + /// 2. Add/Delete/Update files which will include a fake secret + /// 3. Attempt to push + /// 4. Assert that the expected exception is thrown, preventing a secret being pushed upstream + /// + /// + /// + [EnvironmentConditionalSkipTheory] + [InlineData( + @"{ + ""AssetsRepo"": ""Azure/azure-sdk-assets-integration"", + ""AssetsRepoPrefixPath"": ""pull/scenarios"", + ""AssetsRepoId"": """", + ""TagPrefix"": ""language/tables"", + ""Tag"": """" + }")] + [Trait("Category", "Integration")] + public async Task SecretProtectionPreventsPush(string inputJson) + { + var folderStructure = new string[] + { + GitStoretests.AssetsJson + }; + Assets assets = JsonSerializer.Deserialize(inputJson); + Assets updatedAssets = null; + string originalTagPrefix = assets.TagPrefix; + string originalTag = assets.Tag; + var testFolder = TestHelpers.DescribeTestFolder(assets, folderStructure, isPushTest: true); + try + { + ConsoleWrapper consoleWrapper = new ConsoleWrapper(); + GitStore store = new GitStore(consoleWrapper); + var recordingHandler = new RecordingHandler(testFolder, store); + + + // Ensure that the Tag was updated + Assert.NotEqual(originalTag, assets.Tag); + + var jsonFileLocation = Path.Join(testFolder, GitStoretests.AssetsJson); + + var parsedConfiguration = await _defaultStore.ParseConfigurationFile(jsonFileLocation); + await _defaultStore.Restore(jsonFileLocation); + + // Calling Path.GetFullPath of the Path.Combine will ensure any directory separators are normalized for + // the OS the test is running on. The reason being is that AssetsRepoPrefixPath, if there's a separator, + // will be a forward one as expected by git but on Windows this won't result in a usable path. + string localFilePath = Path.GetFullPath(Path.Combine(parsedConfiguration.AssetsRepoLocation.ToString(), parsedConfiguration.AssetsRepoPrefixPath.ToString())); + + // Create a new file with a "secret" present + TestHelpers.CreateFileWithInitialVersion(localFilePath, "file6.txt"); + + // Use the built in secretscanner + await store.Push(jsonFileLocation); + + // assert that are still in a pushable state + // + + // Ensure that the config was updated with the new Tag as part of the push + updatedAssets = TestHelpers.LoadAssetsFromFile(jsonFileLocation); + Assert.NotEqual(originalTag, updatedAssets.Tag); + + // Ensure that the targeted tag is present on the repo + TestHelpers.CheckExistenceOfTag(updatedAssets, localFilePath); + await TestHelpers.CheckBreadcrumbAgainstAssetsJsons(new string[] { jsonFileLocation }); + } + finally + { + DirectoryHelper.DeleteGitDirectory(testFolder); + TestHelpers.CleanupIntegrationTestTag(assets); + TestHelpers.CleanupIntegrationTestTag(updatedAssets); + } + } } } 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 ee2d14fc1ba..96e9eee9819 100644 --- a/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Common/SecretScanner.cs +++ b/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Common/SecretScanner.cs @@ -2,6 +2,7 @@ using System.IO; using System.Linq; using System.Threading.Tasks; +using Azure.Sdk.Tools.TestProxy.Console; using Microsoft.Security.Utilities; namespace Azure.Sdk.Tools.TestProxy.Common @@ -12,8 +13,36 @@ public class SecretScanner WellKnownRegexPatterns.HighConfidenceMicrosoftSecurityModels.Concat(WellKnownRegexPatterns.LowConfidencePotentialSecurityKeys), generateCorrelatingIds: true); - public SecretScanner() { - + private IConsoleWrapper Console; + + public SecretScanner(IConsoleWrapper consoleWrapper) + { + Console = consoleWrapper; + } + + + public async Task> DiscoverSecrets(IEnumerable paths) + { + List detectedSecrets = new List(); + var total = paths.Count(); + var seen = 0; + foreach (string filePath in paths) + { + var content = await ReadFile(filePath); + var fileDetections = DetectSecrets(content); + + if (fileDetections != null && fileDetections.Count > 0) + { + foreach (Detection detection in fileDetections) + { + detectedSecrets.Add(detection); + } + } + seen++; + Console.WriteLine($"Scanned {filePath}. {seen}/{total}."); + } + + return detectedSecrets; } public async Task> DiscoverSecrets(string inputDirectory) @@ -35,6 +64,7 @@ public async Task> DiscoverSecrets(string inputDirectory) } } total++; + Console.WriteLine($"Scanned {filePath}. {total}/{files.Length}."); } return detectedSecrets; diff --git a/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Store/GitStore.cs b/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Store/GitStore.cs index 353a8f8672b..f59204ad24f 100644 --- a/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Store/GitStore.cs +++ b/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Store/GitStore.cs @@ -39,7 +39,7 @@ public class GitStore : IAssetsStore public static readonly string GIT_COMMIT_OWNER_ENV_VAR = "GIT_COMMIT_OWNER"; public static readonly string GIT_COMMIT_EMAIL_ENV_VAR = "GIT_COMMIT_EMAIL"; private bool LocalCacheRefreshed = false; - public SecretScanner SecretScanner = new SecretScanner(); + public SecretScanner SecretScanner; public GitStoreBreadcrumb BreadCrumb = new GitStoreBreadcrumb(); @@ -64,15 +64,13 @@ public class GitStore : IAssetsStore public GitStore() { _consoleWrapper = new ConsoleWrapper(); + SecretScanner = new SecretScanner(_consoleWrapper); } public GitStore(IConsoleWrapper consoleWrapper) { _consoleWrapper = consoleWrapper; - } - - public GitStore(GitProcessHandler processHandler) { - GitHandler = processHandler; + SecretScanner = new SecretScanner(consoleWrapper); } #region push, reset, restore, and other asset repo implementations @@ -94,14 +92,16 @@ public async Task GetPath(string pathToAssetsJson) } /// - /// Currently this is written to always scan every file within the repo before pushing. By passing the _results_ of DetectPendingChanges (the set of changed files) - /// as an argument to this function, we can easily short circuit the scan and only scan the CHANGED files. + /// Scans the changed files, checking for possible secrets. Returns true if secrets are discovered. /// /// + /// /// - public async Task CheckForSecrets(GitAssetsConfiguration assetsConfiguration) + public async Task CheckForSecrets(GitAssetsConfiguration assetsConfiguration, string[] pendingChanges) { - var detectedSecrets = await SecretScanner.DiscoverSecrets(assetsConfiguration.AssetsRepoLocation); + var absolutePaths = pendingChanges.Select(x => Path.Combine(assetsConfiguration.AssetsRepoLocation, x)); + + var detectedSecrets = await SecretScanner.DiscoverSecrets(absolutePaths); if (detectedSecrets.Count > 0) { @@ -110,8 +110,9 @@ public async Task CheckForSecrets(GitAssetsConfiguration assetsConfiguration) { _consoleWrapper.WriteLine(detection.ToString()); } - Environment.Exit(-1); } + + return detectedSecrets.Count > 0; } /// @@ -140,7 +141,11 @@ public async Task Push(string pathToAssetsJson) { if (pendingChanges.Length > 0) { - await CheckForSecrets(config); + if (!(await CheckForSecrets(config, pendingChanges))) + { + Environment.ExitCode = -1; + return; + } try { From 41b7523e60942294ce02891c030cfc58246df231 Mon Sep 17 00:00:00 2001 From: "Scott Beddall (from Dev Box)" Date: Tue, 7 May 2024 12:46:08 -0700 Subject: [PATCH 09/18] Have an integration test defined. Now I need to finish generating fake secrets, checking the detected fake secrets are what we expect, and in-place update of the status of scanning. --- .../GitStoreIntegrationPushTests.cs | 16 ++++++++++++---- .../TestHelpers.cs | 18 ++++++++++++++++++ 2 files changed, 30 insertions(+), 4 deletions(-) diff --git a/tools/test-proxy/Azure.Sdk.Tools.TestProxy.Tests/IntegrationTests/GitStoreIntegrationPushTests.cs b/tools/test-proxy/Azure.Sdk.Tools.TestProxy.Tests/IntegrationTests/GitStoreIntegrationPushTests.cs index 1408ee5b291..670b59214fb 100644 --- a/tools/test-proxy/Azure.Sdk.Tools.TestProxy.Tests/IntegrationTests/GitStoreIntegrationPushTests.cs +++ b/tools/test-proxy/Azure.Sdk.Tools.TestProxy.Tests/IntegrationTests/GitStoreIntegrationPushTests.cs @@ -624,7 +624,7 @@ public async Task SecretProtectionPreventsPush(string inputJson) var jsonFileLocation = Path.Join(testFolder, GitStoretests.AssetsJson); - var parsedConfiguration = await _defaultStore.ParseConfigurationFile(jsonFileLocation); + var parsedConfiguration = await store.ParseConfigurationFile(jsonFileLocation); await _defaultStore.Restore(jsonFileLocation); // Calling Path.GetFullPath of the Path.Combine will ensure any directory separators are normalized for @@ -632,14 +632,22 @@ public async Task SecretProtectionPreventsPush(string inputJson) // will be a forward one as expected by git but on Windows this won't result in a usable path. string localFilePath = Path.GetFullPath(Path.Combine(parsedConfiguration.AssetsRepoLocation.ToString(), parsedConfiguration.AssetsRepoPrefixPath.ToString())); - // Create a new file with a "secret" present - TestHelpers.CreateFileWithInitialVersion(localFilePath, "file6.txt"); + // generate a couple strings that LOOKs like secrets to the secret scanner. + var secretType1 = ""; + var secretType2 = ""; + + // place these strings in files for discovery by the tool + TestHelpers.CreateFileWithContent(localFilePath, "secret_type_1.txt", secretType1); + TestHelpers.CreateFileWithContent(localFilePath, "secret_type_2.txt", secretType2); // Use the built in secretscanner await store.Push(jsonFileLocation); // assert that are still in a pushable state - // + var pendingChanges = store.DetectPendingChanges(parsedConfiguration); + + Assert.Equal(2, pendingChanges.Length); + // Ensure that the config was updated with the new Tag as part of the push updatedAssets = TestHelpers.LoadAssetsFromFile(jsonFileLocation); diff --git a/tools/test-proxy/Azure.Sdk.Tools.TestProxy.Tests/TestHelpers.cs b/tools/test-proxy/Azure.Sdk.Tools.TestProxy.Tests/TestHelpers.cs index 9a5465e3b0d..278472dd754 100644 --- a/tools/test-proxy/Azure.Sdk.Tools.TestProxy.Tests/TestHelpers.cs +++ b/tools/test-proxy/Azure.Sdk.Tools.TestProxy.Tests/TestHelpers.cs @@ -347,6 +347,24 @@ public static void CreateFileWithInitialVersion(string testFolder, string fileNa File.WriteAllText(fullFileName, "1"); } + /// + /// Create a new file with custom text + /// + /// The temporary test folder created by TestHelpers.DescribeTestFolder + /// The file to be created + public static void CreateFileWithContent(string testFolder, string fileName, string textContent) + { + string fullFileName = Path.Combine(testFolder, fileName); + + if (File.Exists(fullFileName)) + { + string errorString = String.Format("AssetsJsonFileName {0} already exists", fullFileName); + throw new ArgumentException(errorString); + } + + File.WriteAllText(fullFileName, textContent); + } + /// /// This function is used to confirm that the .breadcrumb file under the assets store contains the appropriate /// information. From 6b917bbca78979e34161f15c6ee869dffcf348d5 Mon Sep 17 00:00:00 2001 From: "Scott Beddall (from Dev Box)" Date: Tue, 7 May 2024 13:27:48 -0700 Subject: [PATCH 10/18] we have a test that is generating a fake secret, but that fake secret isn't being detected by the secretscanner --- .../GitStoreIntegrationPushTests.cs | 14 +++---------- .../TestHelpers.cs | 21 +++++++++++++++++++ .../Store/GitStore.cs | 5 ++--- 3 files changed, 26 insertions(+), 14 deletions(-) diff --git a/tools/test-proxy/Azure.Sdk.Tools.TestProxy.Tests/IntegrationTests/GitStoreIntegrationPushTests.cs b/tools/test-proxy/Azure.Sdk.Tools.TestProxy.Tests/IntegrationTests/GitStoreIntegrationPushTests.cs index 670b59214fb..72142451972 100644 --- a/tools/test-proxy/Azure.Sdk.Tools.TestProxy.Tests/IntegrationTests/GitStoreIntegrationPushTests.cs +++ b/tools/test-proxy/Azure.Sdk.Tools.TestProxy.Tests/IntegrationTests/GitStoreIntegrationPushTests.cs @@ -618,10 +618,6 @@ public async Task SecretProtectionPreventsPush(string inputJson) GitStore store = new GitStore(consoleWrapper); var recordingHandler = new RecordingHandler(testFolder, store); - - // Ensure that the Tag was updated - Assert.NotEqual(originalTag, assets.Tag); - var jsonFileLocation = Path.Join(testFolder, GitStoretests.AssetsJson); var parsedConfiguration = await store.ParseConfigurationFile(jsonFileLocation); @@ -633,22 +629,18 @@ public async Task SecretProtectionPreventsPush(string inputJson) string localFilePath = Path.GetFullPath(Path.Combine(parsedConfiguration.AssetsRepoLocation.ToString(), parsedConfiguration.AssetsRepoPrefixPath.ToString())); // generate a couple strings that LOOKs like secrets to the secret scanner. - var secretType1 = ""; - var secretType2 = ""; + var secretType1 = TestHelpers.GenerateString(3) + "7Q~" + TestHelpers.GenerateString(34); // place these strings in files for discovery by the tool TestHelpers.CreateFileWithContent(localFilePath, "secret_type_1.txt", secretType1); - TestHelpers.CreateFileWithContent(localFilePath, "secret_type_2.txt", secretType2); // Use the built in secretscanner await store.Push(jsonFileLocation); - // assert that are still in a pushable state + // no changes should be committed var pendingChanges = store.DetectPendingChanges(parsedConfiguration); - - Assert.Equal(2, pendingChanges.Length); + Assert.Equal(1, pendingChanges.Length); - // Ensure that the config was updated with the new Tag as part of the push updatedAssets = TestHelpers.LoadAssetsFromFile(jsonFileLocation); Assert.NotEqual(originalTag, updatedAssets.Tag); diff --git a/tools/test-proxy/Azure.Sdk.Tools.TestProxy.Tests/TestHelpers.cs b/tools/test-proxy/Azure.Sdk.Tools.TestProxy.Tests/TestHelpers.cs index 278472dd754..773a4334bc6 100644 --- a/tools/test-proxy/Azure.Sdk.Tools.TestProxy.Tests/TestHelpers.cs +++ b/tools/test-proxy/Azure.Sdk.Tools.TestProxy.Tests/TestHelpers.cs @@ -11,6 +11,7 @@ using System.Linq; using Xunit; using System.Threading.Tasks; +using System.Security.Cryptography; namespace Azure.Sdk.Tools.TestProxy.Tests { @@ -535,6 +536,26 @@ public static bool CheckExistenceOfTag(Assets assets, string workingDirectory) return result.StdOut.Trim().Length > 0; } + public static string GenerateString(int count) + { + char[] alphabet = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789".ToArray(); + + StringBuilder builder = new StringBuilder(); + + builder.Length = 0; + for (int i = 0; i < count; i++) + { + var bytes = RandomNumberGenerator.GetBytes(1); + int index = bytes[0] % alphabet.Length; + char ch = alphabet[index]; + _ = builder.Append(ch); + } + + string result = builder.ToString(); + + return result; + } + public static List EnumerateArray(JsonElement element) { List values = new List(); diff --git a/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Store/GitStore.cs b/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Store/GitStore.cs index f59204ad24f..0c2f4ced315 100644 --- a/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Store/GitStore.cs +++ b/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Store/GitStore.cs @@ -100,7 +100,6 @@ public async Task GetPath(string pathToAssetsJson) public async Task CheckForSecrets(GitAssetsConfiguration assetsConfiguration, string[] pendingChanges) { var absolutePaths = pendingChanges.Select(x => Path.Combine(assetsConfiguration.AssetsRepoLocation, x)); - var detectedSecrets = await SecretScanner.DiscoverSecrets(absolutePaths); if (detectedSecrets.Count > 0) @@ -141,7 +140,7 @@ public async Task Push(string pathToAssetsJson) { if (pendingChanges.Length > 0) { - if (!(await CheckForSecrets(config, pendingChanges))) + if (await CheckForSecrets(config, pendingChanges)) { Environment.ExitCode = -1; return; @@ -375,7 +374,7 @@ public string[] DetectPendingChanges(GitAssetsConfiguration config) { // Normally, we'd use Environment.NewLine here but this doesn't work on Windows since its NewLine is \r\n and // Git's NewLine is just \n - var individualResults = diffResult.StdOut.Split("\n").Select(x => x.Trim()).ToArray(); + var individualResults = diffResult.StdOut.Split("\n").Select(x => x.Trim().TrimStart('?').Trim()).Where(x => !string.IsNullOrWhiteSpace(x)).ToArray(); return individualResults; } From 76ba4524e1644ae3ab8c81dfec79b067d996b596 Mon Sep 17 00:00:00 2001 From: "Scott Beddall (from Dev Box)" Date: Tue, 7 May 2024 13:47:57 -0700 Subject: [PATCH 11/18] we are actually exercising the push protection now --- .../GitStoreIntegrationPushTests.cs | 23 +++++++------------ 1 file changed, 8 insertions(+), 15 deletions(-) diff --git a/tools/test-proxy/Azure.Sdk.Tools.TestProxy.Tests/IntegrationTests/GitStoreIntegrationPushTests.cs b/tools/test-proxy/Azure.Sdk.Tools.TestProxy.Tests/IntegrationTests/GitStoreIntegrationPushTests.cs index 72142451972..bcf24a75d17 100644 --- a/tools/test-proxy/Azure.Sdk.Tools.TestProxy.Tests/IntegrationTests/GitStoreIntegrationPushTests.cs +++ b/tools/test-proxy/Azure.Sdk.Tools.TestProxy.Tests/IntegrationTests/GitStoreIntegrationPushTests.cs @@ -608,9 +608,6 @@ public async Task SecretProtectionPreventsPush(string inputJson) GitStoretests.AssetsJson }; Assets assets = JsonSerializer.Deserialize(inputJson); - Assets updatedAssets = null; - string originalTagPrefix = assets.TagPrefix; - string originalTag = assets.Tag; var testFolder = TestHelpers.DescribeTestFolder(assets, folderStructure, isPushTest: true); try { @@ -629,7 +626,7 @@ public async Task SecretProtectionPreventsPush(string inputJson) string localFilePath = Path.GetFullPath(Path.Combine(parsedConfiguration.AssetsRepoLocation.ToString(), parsedConfiguration.AssetsRepoPrefixPath.ToString())); // generate a couple strings that LOOKs like secrets to the secret scanner. - var secretType1 = TestHelpers.GenerateString(3) + "7Q~" + TestHelpers.GenerateString(34); + var secretType1 = TestHelpers.GenerateString(3) + "8Q~" + TestHelpers.GenerateString(34); // place these strings in files for discovery by the tool TestHelpers.CreateFileWithContent(localFilePath, "secret_type_1.txt", secretType1); @@ -638,22 +635,18 @@ public async Task SecretProtectionPreventsPush(string inputJson) await store.Push(jsonFileLocation); // no changes should be committed - var pendingChanges = store.DetectPendingChanges(parsedConfiguration); - Assert.Equal(1, pendingChanges.Length); - - // Ensure that the config was updated with the new Tag as part of the push - updatedAssets = TestHelpers.LoadAssetsFromFile(jsonFileLocation); - Assert.NotEqual(originalTag, updatedAssets.Tag); + var pendingChanges = store.DetectPendingChanges(parsedConfiguration).Select(x => Path.Combine(parsedConfiguration.AssetsRepoLocation, x)); + Assert.Single(pendingChanges); - // Ensure that the targeted tag is present on the repo - TestHelpers.CheckExistenceOfTag(updatedAssets, localFilePath); - await TestHelpers.CheckBreadcrumbAgainstAssetsJsons(new string[] { jsonFileLocation }); + // now double check the actual scan results to ensure they are where we expect. + var detectedSecrets = await store.SecretScanner.DiscoverSecrets(pendingChanges); + + Assert.Single(detectedSecrets); + Assert.Equal("SEC101/156", detectedSecrets[0].Id); } finally { DirectoryHelper.DeleteGitDirectory(testFolder); - TestHelpers.CleanupIntegrationTestTag(assets); - TestHelpers.CleanupIntegrationTestTag(updatedAssets); } } } From d2d1a1054264aa66ad349644e6a17da0c041fb23 Mon Sep 17 00:00:00 2001 From: "Scott Beddall (from Dev Box)" Date: Tue, 7 May 2024 15:03:15 -0700 Subject: [PATCH 12/18] working through the 'scanned blah' issues --- .../GitStoreIntegrationPushTests.cs | 19 +++++++++++++------ .../TestHelpers.cs | 13 ++----------- .../Common/SecretScanner.cs | 1 + .../Console/ConsoleWrapper.cs | 6 +++++- .../Console/ConsoleWrapperTester.cs | 10 +++++++++- .../Console/IConsoleWrapper.cs | 3 ++- .../Store/GitStore.cs | 16 +++++++++++++--- 7 files changed, 45 insertions(+), 23 deletions(-) diff --git a/tools/test-proxy/Azure.Sdk.Tools.TestProxy.Tests/IntegrationTests/GitStoreIntegrationPushTests.cs b/tools/test-proxy/Azure.Sdk.Tools.TestProxy.Tests/IntegrationTests/GitStoreIntegrationPushTests.cs index bcf24a75d17..8428769fdbf 100644 --- a/tools/test-proxy/Azure.Sdk.Tools.TestProxy.Tests/IntegrationTests/GitStoreIntegrationPushTests.cs +++ b/tools/test-proxy/Azure.Sdk.Tools.TestProxy.Tests/IntegrationTests/GitStoreIntegrationPushTests.cs @@ -623,26 +623,33 @@ public async Task SecretProtectionPreventsPush(string inputJson) // Calling Path.GetFullPath of the Path.Combine will ensure any directory separators are normalized for // the OS the test is running on. The reason being is that AssetsRepoPrefixPath, if there's a separator, // will be a forward one as expected by git but on Windows this won't result in a usable path. - string localFilePath = Path.GetFullPath(Path.Combine(parsedConfiguration.AssetsRepoLocation.ToString(), parsedConfiguration.AssetsRepoPrefixPath.ToString())); + string localFilePath = Path.GetFullPath(Path.Combine(parsedConfiguration.AssetsRepoLocation, parsedConfiguration.AssetsRepoPrefixPath)); // generate a couple strings that LOOKs like secrets to the secret scanner. var secretType1 = TestHelpers.GenerateString(3) + "8Q~" + TestHelpers.GenerateString(34); - // place these strings in files for discovery by the tool - TestHelpers.CreateFileWithContent(localFilePath, "secret_type_1.txt", secretType1); + // place an entirely new file with the secret + TestHelpers.CreateOrUpdateFileWithContent(localFilePath, "secret_type_1.txt", secretType1); + + // modify an existing file with the secret + TestHelpers.CreateOrUpdateFileWithContent(localFilePath, "file2.txt", secretType1); + + // delete a file to ensure that we don't attempt to scan a file that no longer exists + File.Delete(Path.Combine(localFilePath, "file5.txt")); // Use the built in secretscanner await store.Push(jsonFileLocation); // no changes should be committed var pendingChanges = store.DetectPendingChanges(parsedConfiguration).Select(x => Path.Combine(parsedConfiguration.AssetsRepoLocation, x)); - Assert.Single(pendingChanges); + Assert.Equal(2, pendingChanges.Count()); - // now double check the actual scan results to ensure they are where we expect. + // now double check the actual scan results to ensure they are where we expect var detectedSecrets = await store.SecretScanner.DiscoverSecrets(pendingChanges); - Assert.Single(detectedSecrets); + Assert.Equal(2, detectedSecrets.Count); Assert.Equal("SEC101/156", detectedSecrets[0].Id); + Assert.Equal("SEC101/156", detectedSecrets[1].Id); } finally { diff --git a/tools/test-proxy/Azure.Sdk.Tools.TestProxy.Tests/TestHelpers.cs b/tools/test-proxy/Azure.Sdk.Tools.TestProxy.Tests/TestHelpers.cs index 773a4334bc6..6715ca07046 100644 --- a/tools/test-proxy/Azure.Sdk.Tools.TestProxy.Tests/TestHelpers.cs +++ b/tools/test-proxy/Azure.Sdk.Tools.TestProxy.Tests/TestHelpers.cs @@ -353,16 +353,10 @@ public static void CreateFileWithInitialVersion(string testFolder, string fileNa /// /// The temporary test folder created by TestHelpers.DescribeTestFolder /// The file to be created - public static void CreateFileWithContent(string testFolder, string fileName, string textContent) + public static void CreateOrUpdateFileWithContent(string testFolder, string fileName, string textContent) { string fullFileName = Path.Combine(testFolder, fileName); - if (File.Exists(fullFileName)) - { - string errorString = String.Format("AssetsJsonFileName {0} already exists", fullFileName); - throw new ArgumentException(errorString); - } - File.WriteAllText(fullFileName, textContent); } @@ -542,7 +536,6 @@ public static string GenerateString(int count) StringBuilder builder = new StringBuilder(); - builder.Length = 0; for (int i = 0; i < count; i++) { var bytes = RandomNumberGenerator.GetBytes(1); @@ -551,9 +544,7 @@ public static string GenerateString(int count) _ = builder.Append(ch); } - string result = builder.ToString(); - - return result; + return builder.ToString(); } public static List EnumerateArray(JsonElement element) 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 96e9eee9819..4777d1da960 100644 --- a/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Common/SecretScanner.cs +++ b/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Common/SecretScanner.cs @@ -40,6 +40,7 @@ public async Task> DiscoverSecrets(IEnumerable paths) } seen++; Console.WriteLine($"Scanned {filePath}. {seen}/{total}."); + Console.ResetCursor(); } return detectedSecrets; diff --git a/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Console/ConsoleWrapper.cs b/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Console/ConsoleWrapper.cs index ec1a9283348..3d3ca3ac016 100644 --- a/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Console/ConsoleWrapper.cs +++ b/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Console/ConsoleWrapper.cs @@ -1,4 +1,4 @@ - + using System; namespace Azure.Sdk.Tools.TestProxy.Console @@ -20,5 +20,9 @@ public string ReadLine() { return System.Console.ReadLine(); } + public void ResetCursor() + { + System.Console.SetCursorPosition(0, System.Console.CursorTop); + } } } diff --git a/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Console/ConsoleWrapperTester.cs b/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Console/ConsoleWrapperTester.cs index 42613f08d07..e84fde7a7af 100644 --- a/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Console/ConsoleWrapperTester.cs +++ b/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Console/ConsoleWrapperTester.cs @@ -1,4 +1,4 @@ -using System; +using System; namespace Azure.Sdk.Tools.TestProxy.Console { @@ -28,14 +28,22 @@ public void SetReadLineResponse(string readLineResponse) { _readLineResponse = readLineResponse; } + public void Write(string message) { System.Console.Write(message); } + public void WriteLine(string message) { System.Console.WriteLine(message); } + + public void ResetCursor() + { + // don't need this for testing + } + public string ReadLine() { System.Console.WriteLine($"ReadLine response for test: '{_readLineResponse}'"); diff --git a/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Console/IConsoleWrapper.cs b/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Console/IConsoleWrapper.cs index 04bcd2cbced..5d646dfb9a3 100644 --- a/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Console/IConsoleWrapper.cs +++ b/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Console/IConsoleWrapper.cs @@ -1,4 +1,4 @@ -namespace Azure.Sdk.Tools.TestProxy.Console +namespace Azure.Sdk.Tools.TestProxy.Console { /// /// IConsoleWrapper is just an interface around Console functions. This is necessary for testing @@ -9,5 +9,6 @@ public interface IConsoleWrapper void Write(string message); void WriteLine(string message); string ReadLine(); + public void ResetCursor(); } } diff --git a/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Store/GitStore.cs b/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Store/GitStore.cs index 0c2f4ced315..ad1b87a77be 100644 --- a/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Store/GitStore.cs +++ b/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Store/GitStore.cs @@ -372,9 +372,19 @@ public string[] DetectPendingChanges(GitAssetsConfiguration config) if (!string.IsNullOrWhiteSpace(diffResult.StdOut)) { - // Normally, we'd use Environment.NewLine here but this doesn't work on Windows since its NewLine is \r\n and - // Git's NewLine is just \n - var individualResults = diffResult.StdOut.Split("\n").Select(x => x.Trim().TrimStart('?').Trim()).Where(x => !string.IsNullOrWhiteSpace(x)).ToArray(); + /* + * Normally, we'd use Environment.NewLine here but this doesn't work on Windows since its NewLine is \r\n and Git's NewLine is just \n + * + * The output from git status porcelain will include two possible additional values + * " ?? path/to/file" -> File that is new + * " M path/to/file" -> File that is modified + * " D path/to/file" -> File that is deleted + */ + var individualResults = diffResult.StdOut.Split("\n") + // strim the leading space, the characters for ADDED or MODIFIED, and the space after them + .Select(x => x.Trim().TrimStart('?', 'M').Trim()) + // exclude empty paths or paths that have been DELETED + .Where(x => !string.IsNullOrWhiteSpace(x) && !x.StartsWith("D")).ToArray(); return individualResults; } From ce9b441828482b675a05e0f165b15a3d5569855e Mon Sep 17 00:00:00 2001 From: "Scott Beddall (from Dev Box)" Date: Tue, 7 May 2024 15:41:37 -0700 Subject: [PATCH 13/18] spacing adjustment --- .../test-proxy/Azure.Sdk.Tools.TestProxy/Common/SecretScanner.cs | 1 - 1 file changed, 1 deletion(-) 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 4777d1da960..2d65ff80bc8 100644 --- a/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Common/SecretScanner.cs +++ b/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Common/SecretScanner.cs @@ -20,7 +20,6 @@ public SecretScanner(IConsoleWrapper consoleWrapper) Console = consoleWrapper; } - public async Task> DiscoverSecrets(IEnumerable paths) { List detectedSecrets = new List(); From 1cb18abcfe47163773d1e401f3a2bd7cba1e998e Mon Sep 17 00:00:00 2001 From: "Scott Beddall (from Dev Box)" Date: Wed, 8 May 2024 19:12:25 -0700 Subject: [PATCH 14/18] update formatting of error output --- .../GitStoreIntegrationPushTests.cs | 8 ++--- .../Common/SecretScanner.cs | 34 +++++++++++-------- .../Store/GitStore.cs | 10 +++--- 3 files changed, 30 insertions(+), 22 deletions(-) diff --git a/tools/test-proxy/Azure.Sdk.Tools.TestProxy.Tests/IntegrationTests/GitStoreIntegrationPushTests.cs b/tools/test-proxy/Azure.Sdk.Tools.TestProxy.Tests/IntegrationTests/GitStoreIntegrationPushTests.cs index 8428769fdbf..97a1b7a0917 100644 --- a/tools/test-proxy/Azure.Sdk.Tools.TestProxy.Tests/IntegrationTests/GitStoreIntegrationPushTests.cs +++ b/tools/test-proxy/Azure.Sdk.Tools.TestProxy.Tests/IntegrationTests/GitStoreIntegrationPushTests.cs @@ -641,15 +641,15 @@ public async Task SecretProtectionPreventsPush(string inputJson) await store.Push(jsonFileLocation); // no changes should be committed - var pendingChanges = store.DetectPendingChanges(parsedConfiguration).Select(x => Path.Combine(parsedConfiguration.AssetsRepoLocation, x)); + var pendingChanges = store.DetectPendingChanges(parsedConfiguration); Assert.Equal(2, pendingChanges.Count()); // now double check the actual scan results to ensure they are where we expect - var detectedSecrets = await store.SecretScanner.DiscoverSecrets(pendingChanges); + var detectedSecrets = await store.SecretScanner.DiscoverSecrets(parsedConfiguration.AssetsRepoLocation, pendingChanges); Assert.Equal(2, detectedSecrets.Count); - Assert.Equal("SEC101/156", detectedSecrets[0].Id); - Assert.Equal("SEC101/156", detectedSecrets[1].Id); + Assert.Equal("SEC101/156", detectedSecrets[0].Item2.Id); + Assert.Equal("SEC101/156", detectedSecrets[1].Item2.Id); } finally { 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 2d65ff80bc8..1f75051e6e4 100644 --- a/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Common/SecretScanner.cs +++ b/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Common/SecretScanner.cs @@ -1,8 +1,10 @@ +using System; using System.Collections.Generic; using System.IO; using System.Linq; using System.Threading.Tasks; using Azure.Sdk.Tools.TestProxy.Console; +using Microsoft.Build.Tasks; using Microsoft.Security.Utilities; namespace Azure.Sdk.Tools.TestProxy.Common @@ -20,37 +22,39 @@ public SecretScanner(IConsoleWrapper consoleWrapper) Console = consoleWrapper; } - public async Task> DiscoverSecrets(IEnumerable paths) + public async Task>> DiscoverSecrets(string assetRepoRoot, IEnumerable relativePaths) { - List detectedSecrets = new List(); - var total = paths.Count(); + var detectedSecrets = new List>(); + var total = relativePaths.Count(); var seen = 0; - foreach (string filePath in paths) + Console.WriteLine(string.Empty); + foreach (string filePath in relativePaths) { - var content = await ReadFile(filePath); + var content = await ReadFile(Path.Combine(assetRepoRoot, filePath)); var fileDetections = DetectSecrets(content); if (fileDetections != null && fileDetections.Count > 0) { foreach (Detection detection in fileDetections) { - detectedSecrets.Add(detection); + detectedSecrets.Add(Tuple.Create(filePath, detection)); } } seen++; - Console.WriteLine($"Scanned {filePath}. {seen}/{total}."); - Console.ResetCursor(); + System.Console.Write($"\r\u001b[2KScanned {seen}/{total}."); } + Console.WriteLine(string.Empty); return detectedSecrets; } - public async Task> DiscoverSecrets(string inputDirectory) + public async Task>> DiscoverSecrets(string inputDirectory) { var files = Directory.GetFiles(inputDirectory, "*", SearchOption.AllDirectories); - List detectedSecrets = new List(); + var detectedSecrets = new List>(); - var total = 0; + var seen = 0; + Console.WriteLine(string.Empty); foreach (string filePath in files) { var content = await ReadFile(filePath); @@ -60,13 +64,15 @@ public async Task> DiscoverSecrets(string inputDirectory) { foreach(Detection detection in fileDetections) { - detectedSecrets.Add(detection); + detectedSecrets.Add(Tuple.Create(filePath, detection)); } } - total++; - Console.WriteLine($"Scanned {filePath}. {total}/{files.Length}."); + seen++; + System.Console.Write($"\r\u001b[2KScanned {seen}/{files.Length}."); } + Console.WriteLine(string.Empty); + return detectedSecrets; } diff --git a/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Store/GitStore.cs b/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Store/GitStore.cs index ad1b87a77be..8db8dcb7995 100644 --- a/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Store/GitStore.cs +++ b/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Store/GitStore.cs @@ -99,15 +99,17 @@ public async Task GetPath(string pathToAssetsJson) /// public async Task CheckForSecrets(GitAssetsConfiguration assetsConfiguration, string[] pendingChanges) { - var absolutePaths = pendingChanges.Select(x => Path.Combine(assetsConfiguration.AssetsRepoLocation, x)); - var detectedSecrets = await SecretScanner.DiscoverSecrets(absolutePaths); + _consoleWrapper.WriteLine($"Detected new recordings. Prior to pushing to destination repo, test-proxy will scan {pendingChanges.Length} files."); + var detectedSecrets = await SecretScanner.DiscoverSecrets(assetsConfiguration.AssetsRepoLocation, pendingChanges); if (detectedSecrets.Count > 0) { - _consoleWrapper.WriteLine("A secret was detected in the pushed code. Please register a sanitizer, re-record, and attempt pushing again. Detailed errors follow: "); + _consoleWrapper.WriteLine("At least one secret was detected in the pushed code. Please register a sanitizer, re-record, and attempt pushing again. Detailed errors follow: "); foreach (var detection in detectedSecrets) { - _consoleWrapper.WriteLine(detection.ToString()); + _consoleWrapper.WriteLine($"{detection.Item1}"); + _consoleWrapper.WriteLine($"\t{detection.Item2.Id}: {detection.Item2.Name}"); + _consoleWrapper.WriteLine($"\tStart: {detection.Item2.Start}, End: {detection.Item2.End}.\n"); } } From 1495872c58542ff6b3f2ffee0f1ee7469cc8b913 Mon Sep 17 00:00:00 2001 From: "Scott Beddall (from Dev Box)" Date: Thu, 9 May 2024 13:58:30 -0700 Subject: [PATCH 15/18] invoke secret scanning in parallel --- .../GitStoreIntegrationPushTests.cs | 2 +- .../Common/SecretScanner.cs | 49 ++++++------------- .../Store/GitStore.cs | 6 +-- 3 files changed, 20 insertions(+), 37 deletions(-) diff --git a/tools/test-proxy/Azure.Sdk.Tools.TestProxy.Tests/IntegrationTests/GitStoreIntegrationPushTests.cs b/tools/test-proxy/Azure.Sdk.Tools.TestProxy.Tests/IntegrationTests/GitStoreIntegrationPushTests.cs index 97a1b7a0917..9a14a7227d5 100644 --- a/tools/test-proxy/Azure.Sdk.Tools.TestProxy.Tests/IntegrationTests/GitStoreIntegrationPushTests.cs +++ b/tools/test-proxy/Azure.Sdk.Tools.TestProxy.Tests/IntegrationTests/GitStoreIntegrationPushTests.cs @@ -645,7 +645,7 @@ public async Task SecretProtectionPreventsPush(string inputJson) Assert.Equal(2, pendingChanges.Count()); // now double check the actual scan results to ensure they are where we expect - var detectedSecrets = await store.SecretScanner.DiscoverSecrets(parsedConfiguration.AssetsRepoLocation, pendingChanges); + var detectedSecrets = store.SecretScanner.DiscoverSecrets(parsedConfiguration.AssetsRepoLocation, pendingChanges); Assert.Equal(2, detectedSecrets.Count); Assert.Equal("SEC101/156", detectedSecrets[0].Item2.Id); 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 1f75051e6e4..3f0db57b9a2 100644 --- a/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Common/SecretScanner.cs +++ b/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Common/SecretScanner.cs @@ -1,7 +1,9 @@ using System; +using System.Collections.Concurrent; using System.Collections.Generic; using System.IO; using System.Linq; +using System.Threading; using System.Threading.Tasks; using Azure.Sdk.Tools.TestProxy.Console; using Microsoft.Build.Tasks; @@ -22,15 +24,21 @@ public SecretScanner(IConsoleWrapper consoleWrapper) Console = consoleWrapper; } - public async Task>> DiscoverSecrets(string assetRepoRoot, IEnumerable relativePaths) + public List> DiscoverSecrets(string assetRepoRoot, IEnumerable relativePaths) { - var detectedSecrets = new List>(); + var detectedSecrets = new ConcurrentBag>(); var total = relativePaths.Count(); var seen = 0; Console.WriteLine(string.Empty); - foreach (string filePath in relativePaths) + + var options = new ParallelOptions + { + MaxDegreeOfParallelism = Environment.ProcessorCount + }; + + Parallel.ForEach(relativePaths, options, (filePath) => { - var content = await ReadFile(Path.Combine(assetRepoRoot, filePath)); + var content = File.ReadAllText(Path.Combine(assetRepoRoot, filePath)); var fileDetections = DetectSecrets(content); if (fileDetections != null && fileDetections.Count > 0) @@ -40,40 +48,15 @@ public async Task>> DiscoverSecrets(string assetRe detectedSecrets.Add(Tuple.Create(filePath, detection)); } } - seen++; - System.Console.Write($"\r\u001b[2KScanned {seen}/{total}."); - } - Console.WriteLine(string.Empty); - return detectedSecrets; - } - - public async Task>> DiscoverSecrets(string inputDirectory) - { - var files = Directory.GetFiles(inputDirectory, "*", SearchOption.AllDirectories); - var detectedSecrets = new List>(); - - var seen = 0; - Console.WriteLine(string.Empty); - foreach (string filePath in files) - { - var content = await ReadFile(filePath); - var fileDetections = DetectSecrets(content); + Interlocked.Increment(ref seen); - if (fileDetections != null && fileDetections.Count > 0) - { - foreach(Detection detection in fileDetections) - { - detectedSecrets.Add(Tuple.Create(filePath, detection)); - } - } - seen++; - System.Console.Write($"\r\u001b[2KScanned {seen}/{files.Length}."); - } + System.Console.Write($"\r\u001b[2KScanned {seen}/{total}."); + }); Console.WriteLine(string.Empty); - return detectedSecrets; + return detectedSecrets.ToList(); } private async Task ReadFile(string filePath) diff --git a/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Store/GitStore.cs b/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Store/GitStore.cs index 8db8dcb7995..6a5b7c9fd3b 100644 --- a/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Store/GitStore.cs +++ b/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Store/GitStore.cs @@ -97,10 +97,10 @@ public async Task GetPath(string pathToAssetsJson) /// /// /// - public async Task CheckForSecrets(GitAssetsConfiguration assetsConfiguration, string[] pendingChanges) + public bool CheckForSecrets(GitAssetsConfiguration assetsConfiguration, string[] pendingChanges) { _consoleWrapper.WriteLine($"Detected new recordings. Prior to pushing to destination repo, test-proxy will scan {pendingChanges.Length} files."); - var detectedSecrets = await SecretScanner.DiscoverSecrets(assetsConfiguration.AssetsRepoLocation, pendingChanges); + var detectedSecrets = SecretScanner.DiscoverSecrets(assetsConfiguration.AssetsRepoLocation, pendingChanges); if (detectedSecrets.Count > 0) { @@ -142,7 +142,7 @@ public async Task Push(string pathToAssetsJson) { if (pendingChanges.Length > 0) { - if (await CheckForSecrets(config, pendingChanges)) + if (CheckForSecrets(config, pendingChanges)) { Environment.ExitCode = -1; return; From ed2006f52fb30285de645ba33604a5841aefaebc Mon Sep 17 00:00:00 2001 From: Scott Beddall Date: Mon, 13 May 2024 10:38:42 -0700 Subject: [PATCH 16/18] commit the fixes without testing --- .../Azure.Sdk.Tools.TestProxy/Common/SecretScanner.cs | 2 +- .../Azure.Sdk.Tools.TestProxy/Console/ConsoleWrapper.cs | 4 ---- .../Console/ConsoleWrapperTester.cs | 5 ----- .../Azure.Sdk.Tools.TestProxy/Console/IConsoleWrapper.cs | 1 - 4 files changed, 1 insertion(+), 11 deletions(-) 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 3f0db57b9a2..82bb90ef873 100644 --- a/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Common/SecretScanner.cs +++ b/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Common/SecretScanner.cs @@ -51,7 +51,7 @@ public List> DiscoverSecrets(string assetRepoRoot, IEnu Interlocked.Increment(ref seen); - System.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/Console/ConsoleWrapper.cs b/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Console/ConsoleWrapper.cs index 3d3ca3ac016..898fe3ca809 100644 --- a/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Console/ConsoleWrapper.cs +++ b/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Console/ConsoleWrapper.cs @@ -20,9 +20,5 @@ public string ReadLine() { return System.Console.ReadLine(); } - public void ResetCursor() - { - System.Console.SetCursorPosition(0, System.Console.CursorTop); - } } } diff --git a/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Console/ConsoleWrapperTester.cs b/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Console/ConsoleWrapperTester.cs index e84fde7a7af..6492b56c5b1 100644 --- a/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Console/ConsoleWrapperTester.cs +++ b/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Console/ConsoleWrapperTester.cs @@ -39,11 +39,6 @@ public void WriteLine(string message) System.Console.WriteLine(message); } - public void ResetCursor() - { - // don't need this for testing - } - public string ReadLine() { System.Console.WriteLine($"ReadLine response for test: '{_readLineResponse}'"); diff --git a/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Console/IConsoleWrapper.cs b/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Console/IConsoleWrapper.cs index 5d646dfb9a3..4b1e4c1b200 100644 --- a/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Console/IConsoleWrapper.cs +++ b/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Console/IConsoleWrapper.cs @@ -9,6 +9,5 @@ public interface IConsoleWrapper void Write(string message); void WriteLine(string message); string ReadLine(); - public void ResetCursor(); } } From 80f3a05a8be48b29847982befa6ed462bc971faa Mon Sep 17 00:00:00 2001 From: Scott Beddall <45376673+scbedd@users.noreply.github.com> Date: Mon, 13 May 2024 10:41:49 -0700 Subject: [PATCH 17/18] Update tools/test-proxy/Azure.Sdk.Tools.TestProxy/Console/ConsoleWrapperTester.cs Remove unneeded using. --- .../Azure.Sdk.Tools.TestProxy/Console/ConsoleWrapperTester.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Console/ConsoleWrapperTester.cs b/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Console/ConsoleWrapperTester.cs index 6492b56c5b1..3de57382e5f 100644 --- a/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Console/ConsoleWrapperTester.cs +++ b/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Console/ConsoleWrapperTester.cs @@ -1,4 +1,3 @@ -using System; namespace Azure.Sdk.Tools.TestProxy.Console { From d8cf94edc4b1655c928bcc476ff9e0cffdae445e Mon Sep 17 00:00:00 2001 From: Scott Beddall <45376673+scbedd@users.noreply.github.com> Date: Mon, 13 May 2024 10:42:16 -0700 Subject: [PATCH 18/18] Update tools/test-proxy/Azure.Sdk.Tools.TestProxy/Console/ConsoleWrapper.cs Remove another useless using. --- .../Azure.Sdk.Tools.TestProxy/Console/ConsoleWrapper.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Console/ConsoleWrapper.cs b/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Console/ConsoleWrapper.cs index 898fe3ca809..e7883f81940 100644 --- a/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Console/ConsoleWrapper.cs +++ b/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Console/ConsoleWrapper.cs @@ -1,5 +1,4 @@ -using System; namespace Azure.Sdk.Tools.TestProxy.Console {