Integrate Secret Scanning from Microsoft.Security.Utilities (#8140)

scbedd · web-flow · commit 6e56dc9a374b · 2024-05-13T11:20:44.000-07:00
* Integrate Microsoft.Security.Utilities into the test-proxy, preventing pushes that contain detected secrets.
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
@@ -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);
             }
         }
+
+        /// <summary>
+        /// 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
+        /// </summary>
+        /// <param name="inputJson"></param>
+        /// <returns></returns>
+        [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<Assets>(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
@@ -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");
         }
 
+        /// <summary>
+        /// Create a new file with custom text
+        /// </summary>
+        /// <param name="testFolder">The temporary test folder created by TestHelpers.DescribeTestFolder</param>
+        /// <param name="fileName">The file to be created</param>
+        public static void CreateOrUpdateFileWithContent(string testFolder, string fileName, string textContent)
+        {
+            string fullFileName = Path.Combine(testFolder, fileName);
+
+            File.WriteAllText(fullFileName, textContent);
+        }
+
         /// <summary>
         /// 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<T> EnumerateArray<T>(JsonElement element)
         {
             List<T> values = new List<T>();
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
@@ -22,6 +22,7 @@
   </ItemGroup>
   
   <ItemGroup>
+    <PackageReference Include="Microsoft.Security.Utilities.Core" Version="1.4.14" />
     <PackageReference Include="System.CommandLine" Version="2.0.0-beta4.22272.1" />
   </ItemGroup>
 </Project>
diff --git a/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Common/SecretScanner.cs 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<Tuple<string, Detection>> DiscoverSecrets(string assetRepoRoot, IEnumerable<string> relativePaths)
+        {
+            var detectedSecrets = new ConcurrentBag<Tuple<string, Detection>>();
+            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<string> ReadFile(string filePath)
+        {
+            using (StreamReader reader = new StreamReader(filePath))
+            {
+                return await reader.ReadToEndAsync();
+            }
+        }
+
+        private ICollection<Detection> 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
@@ -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
@@ -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
@@ -1,4 +1,4 @@
-﻿namespace Azure.Sdk.Tools.TestProxy.Console
+namespace Azure.Sdk.Tools.TestProxy.Console
 {
     /// <summary>
     /// 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
@@ -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<NormalizedString> GetPath(string pathToAssetsJson)
             return new NormalizedString(config.AssetsRepoLocation);
         }
 
+        /// <summary>
+        /// Scans the changed files, checking for possible secrets. Returns true if secrets are discovered.
+        /// </summary>
+        /// <param name="assetsConfiguration"></param>
+        /// <param name="pendingChanges"></param>
+        /// <returns></returns>
+        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;
+        }
+
         /// <summary>
         /// Pushes a set of changed files to the assets repo. Honors configuration of assets.json passed into it.
         /// </summary>
@@ -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;
             }
 

Original file line number	Diff line number	Diff line change
`@@ -1,5 +1,4 @@`
`1`		`-`
`2`		`-using System;`
	`1`	`+`
`3`	`2`
`4`	`3`	`namespace Azure.Sdk.Tools.TestProxy.Console`
`5`	`4`	`{`
Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,3 @@`
`1`		`-using System;`
`2`	`1`
`3`	`2`	`namespace Azure.Sdk.Tools.TestProxy.Console`
`4`	`3`	`{`
`@@ -28,14 +27,17 @@ public void SetReadLineResponse(string readLineResponse)`
`28`	`27`	`{`
`29`	`28`	`_readLineResponse = readLineResponse;`
`30`	`29`	`}`
	`30`	`+`
`31`	`31`	`public void Write(string message)`
`32`	`32`	`{`
`33`	`33`	`System.Console.Write(message);`
`34`	`34`	`}`
	`35`	`+`
`35`	`36`	`public void WriteLine(string message)`
`36`	`37`	`{`
`37`	`38`	`System.Console.WriteLine(message);`
`38`	`39`	`}`
	`40`	`+`
`39`	`41`	`public string ReadLine()`
`40`	`42`	`{`
`41`	`43`	`System.Console.WriteLine($"ReadLine response for test: '{_readLineResponse}'");`
Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,4 @@`
`1`		`-namespace Azure.Sdk.Tools.TestProxy.Console`
	`1`	`+namespace Azure.Sdk.Tools.TestProxy.Console`
`2`	`2`	`{`
`3`	`3`	`/// <summary>`
`4`	`4`	`/// IConsoleWrapper is just an interface around Console functions. This is necessary for testing`