diff --git a/src/Microsoft.ComponentDetection.Common/CommandLineInvocationService.cs b/src/Microsoft.ComponentDetection.Common/CommandLineInvocationService.cs index 0c8640c91..8107a3dc7 100644 --- a/src/Microsoft.ComponentDetection.Common/CommandLineInvocationService.cs +++ b/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 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; } } diff --git a/src/Microsoft.ComponentDetection.Common/Utilities/StringUtilities.cs b/src/Microsoft.ComponentDetection.Common/Utilities/StringUtilities.cs new file mode 100644 index 000000000..b63ac7a8e --- /dev/null +++ b/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)); + + /// + /// 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. + /// + /// String with possible credentials. + /// New string identical to original string, except credentials in URL are replaced with placeholders. + 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; + } + } +} diff --git a/src/Microsoft.ComponentDetection.Detectors/pip/PipCommandService.cs b/src/Microsoft.ComponentDetection.Detectors/pip/PipCommandService.cs index 51791ac0a..13acae112 100644 --- a/src/Microsoft.ComponentDetection.Detectors/pip/PipCommandService.cs +++ b/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 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, diff --git a/test/Microsoft.ComponentDetection.Common.Tests/StringUtilitiesTests.cs b/test/Microsoft.ComponentDetection.Common.Tests/StringUtilitiesTests.cs new file mode 100644 index 000000000..96c347bdb --- /dev/null +++ b/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); + } +}