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..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 @@ -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); + var testFolder = TestHelpers.DescribeTestFolder(assets, folderStructure, isPushTest: true); + try + { + ConsoleWrapper consoleWrapper = new ConsoleWrapper(); + GitStore store = new GitStore(consoleWrapper); + var recordingHandler = new RecordingHandler(testFolder, store); + + var jsonFileLocation = Path.Join(testFolder, GitStoretests.AssetsJson); + + 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 + // 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, parsedConfiguration.AssetsRepoPrefixPath)); + + // generate a couple strings that LOOKs like secrets to the secret scanner. + var secretType1 = TestHelpers.GenerateString(3) + "8Q~" + TestHelpers.GenerateString(34); + + // 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); + Assert.Equal(2, pendingChanges.Count()); + + // now double check the actual scan results to ensure they are where we expect + var detectedSecrets = store.SecretScanner.DiscoverSecrets(parsedConfiguration.AssetsRepoLocation, pendingChanges); + + Assert.Equal(2, detectedSecrets.Count); + Assert.Equal("SEC101/156", detectedSecrets[0].Item2.Id); + Assert.Equal("SEC101/156", detectedSecrets[1].Item2.Id); + } + finally + { + DirectoryHelper.DeleteGitDirectory(testFolder); + } + } } } 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..6715ca07046 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 { @@ -347,6 +348,18 @@ 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 CreateOrUpdateFileWithContent(string testFolder, string fileName, string textContent) + { + string fullFileName = Path.Combine(testFolder, fileName); + + File.WriteAllText(fullFileName, textContent); + } + /// /// This function is used to confirm that the .breadcrumb file under the assets store contains the appropriate /// information. @@ -517,6 +530,23 @@ 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(); + + 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); + } + + return builder.ToString(); + } + public static List EnumerateArray(JsonElement element) { List values = new List(); 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..82bb90ef873 --- /dev/null +++ b/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Common/SecretScanner.cs @@ -0,0 +1,76 @@ +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; +using Microsoft.Security.Utilities; + +namespace Azure.Sdk.Tools.TestProxy.Common +{ + public class SecretScanner + { + public SecretMasker SecretMasker = new SecretMasker( + WellKnownRegexPatterns.HighConfidenceMicrosoftSecurityModels.Concat(WellKnownRegexPatterns.LowConfidencePotentialSecurityKeys), + generateCorrelatingIds: true); + + private IConsoleWrapper Console; + + public SecretScanner(IConsoleWrapper consoleWrapper) + { + Console = consoleWrapper; + } + + public List> DiscoverSecrets(string assetRepoRoot, IEnumerable relativePaths) + { + var detectedSecrets = new ConcurrentBag>(); + var total = relativePaths.Count(); + var seen = 0; + Console.WriteLine(string.Empty); + + var options = new ParallelOptions + { + MaxDegreeOfParallelism = Environment.ProcessorCount + }; + + Parallel.ForEach(relativePaths, options, (filePath) => + { + var content = File.ReadAllText(Path.Combine(assetRepoRoot, filePath)); + var fileDetections = DetectSecrets(content); + + if (fileDetections != null && fileDetections.Count > 0) + { + foreach (Detection detection in fileDetections) + { + detectedSecrets.Add(Tuple.Create(filePath, detection)); + } + } + + Interlocked.Increment(ref seen); + + Console.Write($"\r\u001b[2KScanned {seen}/{total}."); + }); + + Console.WriteLine(string.Empty); + + return detectedSecrets.ToList(); + } + + private async Task ReadFile(string filePath) + { + using (StreamReader reader = new StreamReader(filePath)) + { + return await reader.ReadToEndAsync(); + } + } + + private ICollection DetectSecrets(string stringContent) + { + return SecretMasker.DetectSecrets(stringContent); + } + + } +} 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..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 { 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..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 { @@ -28,14 +27,17 @@ 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 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..4b1e4c1b200 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 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 69dbbfda174..0f3c5339131 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 { @@ -41,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; public readonly object LocalCacheLock = new object(); public GitStoreBreadcrumb BreadCrumb = new GitStoreBreadcrumb(); @@ -66,15 +65,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 @@ -95,6 +92,31 @@ public async Task GetPath(string pathToAssetsJson) return new NormalizedString(config.AssetsRepoLocation); } + /// + /// Scans the changed files, checking for possible secrets. Returns true if secrets are discovered. + /// + /// + /// + /// + 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 = SecretScanner.DiscoverSecrets(assetsConfiguration.AssetsRepoLocation, pendingChanges); + + if (detectedSecrets.Count > 0) + { + _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.Item1}"); + _consoleWrapper.WriteLine($"\t{detection.Item2.Id}: {detection.Item2.Name}"); + _consoleWrapper.WriteLine($"\tStart: {detection.Item2.Start}, End: {detection.Item2.End}.\n"); + } + } + + return detectedSecrets.Count > 0; + } + /// /// Pushes a set of changed files to the assets repo. Honors configuration of assets.json passed into it. /// @@ -121,6 +143,12 @@ public async Task Push(string pathToAssetsJson) { if (pendingChanges.Length > 0) { + if (CheckForSecrets(config, pendingChanges)) + { + Environment.ExitCode = -1; + return; + } + try { string branchGuid = Guid.NewGuid().ToString().Substring(0, 8); @@ -347,9 +375,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()).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; }