Mask feed credentials for CLI commands + remove superfluous risky logs (#1627)

* Mask feed credentials for CLI commands + remove superfluous risky logs

Author: grvillic

src/Microsoft.ComponentDetection.Common/CommandLineInvocationService.cs

@@ -78,16 +78,15 @@ public async Task<CommandLineExecutionResult> ExecuteCommandAsync(
 
         var pathToRun = this.commandLocatableCache[command];
         var joinedParameters = string.Join(" ", parameters);
-        var commandForLogging = joinedParameters.RemoveSensitiveInformation();
         try
         {
             var result = await RunProcessAsync(pathToRun, joinedParameters, workingDirectory, cancellationToken);
-            record.Track(result, pathToRun, commandForLogging);
+            record.Track(result, pathToRun, joinedParameters);
             return result;
         }
         catch (Exception ex)
         {
-            record.Track(ex, pathToRun, commandForLogging);
+            record.Track(ex, pathToRun, joinedParameters);
             throw;
         }
     }

src/Microsoft.ComponentDetection.Common/Telemetry/CommandLineTelemetryService.cs

@@ -3,6 +3,7 @@ namespace Microsoft.ComponentDetection.Common.Telemetry;
 
 using System;
 using System.Collections.Concurrent;
+using System.Linq;
 using System.Text.Json;
 using System.Text.Json.Nodes;
 using Microsoft.ComponentDetection.Common.Telemetry.Records;
@@ -45,6 +46,9 @@ public void PostRecord(IDetectionTelemetryRecord record)
         jsonRecord["Timestamp"] = DateTime.UtcNow;
         jsonRecord["CorrelationId"] = TelemetryConstants.CorrelationId;
 
+        // Mask sensitive information in all string values before storing/logging
+        MaskSensitiveInformation(jsonRecord);
+
         this.records.Enqueue(jsonRecord);
 
         if (this.telemetryMode == TelemetryMode.Debug)
@@ -55,4 +59,51 @@ public void PostRecord(IDetectionTelemetryRecord record)
 
     /// <inheritdoc/>
     public void SetMode(TelemetryMode mode) => this.telemetryMode = mode;
+
+    /// <summary>
+    /// Recursively masks sensitive information in all string values within a JSON node.
+    /// </summary>
+    /// <param name="node">The JSON node to process.</param>
+    private static void MaskSensitiveInformation(JsonNode node)
+    {
+        if (node == null)
+        {
+            return;
+        }
+
+        if (node is JsonObject jsonObject)
+        {
+            // Get keys first to avoid collection modified during enumeration
+            var keys = jsonObject.Select(p => p.Key).ToList();
+            foreach (var key in keys)
+            {
+                var value = jsonObject[key];
+                if (value is JsonValue jsonValue && jsonValue.TryGetValue<string>(out var stringValue))
+                {
+                    // Mask sensitive info in string values
+                    jsonObject[key] = stringValue.RemoveSensitiveInformation();
+                }
+                else
+                {
+                    // Recurse into nested objects and arrays
+                    MaskSensitiveInformation(value);
+                }
+            }
+        }
+        else if (node is JsonArray jsonArray)
+        {
+            for (var index = 0; index < jsonArray.Count; index++)
+            {
+                var item = jsonArray[index];
+                if (item is JsonValue jsonValue && jsonValue.TryGetValue<string>(out var stringValue))
+                {
+                    jsonArray[index] = stringValue.RemoveSensitiveInformation();
+                }
+                else
+                {
+                    MaskSensitiveInformation(item);
+                }
+            }
+        }
+    }
 }

src/Microsoft.ComponentDetection.Common/Telemetry/Records/CommandLineInvocationTelemetryRecord.cs

@@ -20,21 +20,21 @@ public class CommandLineInvocationTelemetryRecord : BaseDetectionTelemetryRecord
     internal void Track(CommandLineExecutionResult result, string path, string parameters)
     {
         this.ExitCode = result.ExitCode;
-        this.StandardError = result.StdErr;
+        this.StandardError = result.StdErr?.RemoveSensitiveInformation();
         this.TrackCommon(path, parameters);
     }
 
     internal void Track(Exception ex, string path, string parameters)
     {
         this.ExitCode = -1;
-        this.UnhandledException = ex.ToString();
+        this.UnhandledException = ex.ToString().RemoveSensitiveInformation();
         this.TrackCommon(path, parameters);
     }
 
     private void TrackCommon(string path, string parameters)
     {
         this.PathThatWasRan = path;
-        this.Parameters = parameters;
+        this.Parameters = parameters?.RemoveSensitiveInformation();
         this.StopExecutionTimer();
     }
 }

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

@@ -6,14 +6,22 @@ namespace Microsoft.ComponentDetection.Common;
 
 public static class StringUtilities
 {
-    private static readonly Regex SensitiveInfoRegex = new Regex(@"(?<=https://)(.+)(?=@)", RegexOptions.Compiled | RegexOptions.IgnoreCase, TimeSpan.FromSeconds(5));
+    // Matches credentials in URLs like http://user:password@host or https://user:password@host
+    private static readonly Regex UrlCredentialsRegex = new(@"(?<=https?://)(.+?)(?=@)", RegexOptions.Compiled | RegexOptions.IgnoreCase, TimeSpan.FromSeconds(5));
+
+    // Matches SAS tokens in query strings (e.g., ?sv=...&sig=... or ?se=...&sig=...)
+    // SAS tokens contain a 'sig' parameter which is the signature - we mask the entire query string
+    private static readonly Regex SasTokenRegex = new(@"\?[^""'\s]*sig=[^""'\s]*", RegexOptions.Compiled | RegexOptions.IgnoreCase, TimeSpan.FromSeconds(5));
+
     public const string SensitivePlaceholder = "******";
 
     /// <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.
+    /// Utility method to remove sensitive information from a string, including:
+    /// - Credentials in URLs (e.g., https://user:password@host)
+    /// - SAS tokens in query strings (e.g., ?sv=...&amp;sig=...).
     /// </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>
+    /// <returns>New string identical to original string, except sensitive information is replaced with placeholders.</returns>
     public static string RemoveSensitiveInformation(this string inputString)
     {
         if (string.IsNullOrWhiteSpace(inputString))
@@ -23,7 +31,9 @@ public static string RemoveSensitiveInformation(this string inputString)
 
         try
         {
-            return SensitiveInfoRegex.Replace(inputString, SensitivePlaceholder);
+            var result = UrlCredentialsRegex.Replace(inputString, SensitivePlaceholder);
+            result = SasTokenRegex.Replace(result, $"?{SensitivePlaceholder}");
+            return result;
         }
         catch (Exception)
         {

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

@@ -192,11 +192,10 @@ private async Task<CommandLineExecutionResult> ExecuteCommandAsync(
 
             if (command.ExitCode == -1 && cancellationToken.IsCancellationRequested)
             {
-                var errorMessage = $"PipReport: Cancelled for file '{formattedPath}' with command '{pipReportCommand.RemoveSensitiveInformation()}'.";
+                var errorMessage = $"PipReport: Cancelled for file '{formattedPath}'.";
                 using var failureRecord = new PipReportFailureTelemetryRecord
                 {
                     ExitCode = command.ExitCode,
-                    StdErr = $"{errorMessage} {command.StdErr}",
                 };
 
                 this.logger.LogWarning("{Error}", errorMessage);
@@ -207,10 +206,9 @@ private async Task<CommandLineExecutionResult> ExecuteCommandAsync(
                 using var failureRecord = new PipReportFailureTelemetryRecord
                 {
                     ExitCode = command.ExitCode,
-                    StdErr = command.StdErr,
                 };
 
-                this.logger.LogDebug("PipReport: Pip installation report error: {StdErr}", command.StdErr);
+                this.logger.LogDebug("PipReport: Pip installation report failed with exit code {ExitCode} for {Path}", command.ExitCode, formattedPath);
                 throw new InvalidOperationException($"PipReport: Failed to generate pip installation report for file {path} with exit code {command.ExitCode}");
             }