Simple sanitization in strings used in CLI before logging (#1155)

src/Microsoft.ComponentDetection.Common/CommandLineInvocationService.cs

@@ -1,4 +1,4 @@
-namespace Microsoft.ComponentDetection.Common;
+namespace Microsoft.ComponentDetection.Common;
 
 using System;
 using System.Collections.Concurrent;
@@ -71,15 +71,16 @@ public async Task<CommandLineExecutionResult> ExecuteCommandAsync(string command
 
         var pathToRun = this.commandLocatableCache[command];
         var joinedParameters = string.Join(" ", parameters);
+        var commandForLogging = joinedParameters.RemoveSensitiveInformation();
         try
         {
             var result = await RunProcessAsync(pathToRun, joinedParameters, workingDirectory);
-            record.Track(result, pathToRun, joinedParameters);
+            record.Track(result, pathToRun, commandForLogging);
             return result;
         }
         catch (Exception ex)
         {
-            record.Track(ex, pathToRun, joinedParameters);
+            record.Track(ex, pathToRun, commandForLogging);
             throw;
         }
     }

src/Microsoft.ComponentDetection.Common/Utilities/StringUtilities.cs

@@ -0,0 +1,32 @@
+namespace Microsoft.ComponentDetection.Common;
+
+using System;
+using System.Text.RegularExpressions;
+
+public static class StringUtilities
+{
+    private static readonly Regex SensitiveInfoRegex = new Regex(@"(?<=https://)(.+)(?=@)", RegexOptions.Compiled, TimeSpan.FromSeconds(5));
+
+    /// <summary>
+    /// Utility method to remove sensitive information from a string, currently focused on removing on the credentials placed within URL which can be part of CLI commands.
+    /// </summary>
+    /// <param name="inputString">String with possible credentials.</param>
+    /// <returns>New string identical to original string, except credentials in URL are replaced with placeholders.</returns>
+    public static string RemoveSensitiveInformation(this string inputString)
+    {
+        if (string.IsNullOrWhiteSpace(inputString))
+        {
+            return inputString;
+        }
+
+        try
+        {
+            return SensitiveInfoRegex.Replace(inputString, "******");
+        }
+        catch (Exception)
+        {
+            // No matter the exception, we should not break flow due to regex failure/timeout.
+            return inputString;
+        }
+    }
+}

src/Microsoft.ComponentDetection.Detectors/pip/PipCommandService.cs

@@ -4,6 +4,7 @@ namespace Microsoft.ComponentDetection.Detectors.Pip;
 using System.Collections.Generic;
 using System.IO;
 using System.Threading.Tasks;
+using Microsoft.ComponentDetection.Common;
 using Microsoft.ComponentDetection.Common.Telemetry.Records;
 using Microsoft.ComponentDetection.Contracts;
 using Microsoft.Extensions.Logging;
@@ -118,14 +119,14 @@ private async Task<bool> CanCommandBeLocatedAsync(string pipPath)
 
         // When PIP_INDEX_URL is set, we need to pass it as a parameter to pip install command.
         // This should be done before running detection by the build system, otherwise the detection
-        // will default to the public PyPI index if not configured in pip defaults.
+        // will default to the public PyPI index if not configured in pip defaults. Note this index URL may have credentials, we need to remove it when logging.
         pipReportCommand += $" --dry-run --ignore-installed --quiet --report {reportName}";
         if (this.environmentService.DoesEnvironmentVariableExist("PIP_INDEX_URL"))
         {
             pipReportCommand += $" --index-url {this.environmentService.GetEnvironmentVariable("PIP_INDEX_URL")}";
         }
 
-        this.logger.LogDebug("PipReport: Generating pip installation report for {Path} with command: {Command}", formattedPath, pipReportCommand);
+        this.logger.LogDebug("PipReport: Generating pip installation report for {Path} with command: {Command}", formattedPath, pipReportCommand.RemoveSensitiveInformation());
         command = await this.commandLineInvocationService.ExecuteCommandAsync(
             pipExecutable,
             null,

test/Microsoft.ComponentDetection.Common.Tests/StringUtilitiesTests.cs

@@ -0,0 +1,28 @@
+namespace Microsoft.ComponentDetection.Common.Tests;
+
+using FluentAssertions;
+using Microsoft.VisualStudio.TestTools.UnitTesting;
+
+[TestClass]
+[TestCategory("Governance/All")]
+[TestCategory("Governance/ComponentDetection")]
+[TestCategory("Governance/Utilities")]
+public class StringUtilitiesTests
+{
+    [TestMethod]
+    [DataRow("", "")]
+    [DataRow(null, null)]
+    [DataRow("  ", "  ")]
+    [DataRow(" https:// ", " https:// ")]
+    [DataRow("https://username:password@domain.me", "https://******@domain.me")]
+    [DataRow("https://domain.me", "https://domain.me")]
+    [DataRow("https://@domain.me", "https://@domain.me")]
+    [DataRow(
+        "install -r requirements.txt --dry-run --ignore-installed --quiet --report file.zvn --index-url https://user:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa@someregistry.localhost.com",
+        "install -r requirements.txt --dry-run --ignore-installed --quiet --report file.zvn --index-url https://******@someregistry.localhost.com")]
+    public void RemoveSensitiveInformation_ReturnsAsExpected(string input, string expected)
+    {
+        var actual = StringUtilities.RemoveSensitiveInformation(input);
+        actual.Should().Be(expected);
+    }
+}