Merge pull request #22 from mjcheetham/tracing-tweaks

Fix a few problems with tracing secrets and missing vital error codes

Author: mjcheetham

common/src/Microsoft.Git.CredentialManager/Application.cs

@@ -1,8 +1,10 @@
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT license.
 using System;
+using System.ComponentModel;
 using System.Threading.Tasks;
 using Microsoft.Git.CredentialManager.Commands;
+using Microsoft.Git.CredentialManager.SecureStorage;
 
 namespace Microsoft.Git.CredentialManager
 {
@@ -75,12 +77,23 @@ protected override void Dispose(bool disposing)
             base.Dispose(disposing);
         }
 
-        protected bool WriteException(Exception e)
+        protected bool WriteException(Exception ex)
         {
-            Context.StdError.WriteLine("fatal: {0}", e.Message);
-            if (e.InnerException != null)
+            // Try and use a nicer format for some well-known exception types
+            switch (ex)
             {
-                Context.StdError.WriteLine("fatal: {0}", e.InnerException.Message);
+                case Win32Exception w32Ex:
+                    Context.StdError.WriteLine("fatal: {0} [0x{1:x}]", w32Ex.Message, w32Ex.NativeErrorCode);
+                    break;
+                default:
+                    Context.StdError.WriteLine("fatal: {0}", ex.Message);
+                    break;
+            }
+
+            // Recurse to print all inner exceptions
+            if (!(ex.InnerException is null))
+            {
+                WriteException(ex.InnerException);
             }
 
             return true;

common/src/Microsoft.Git.CredentialManager/Commands/Command.cs

@@ -65,7 +65,7 @@ public override async Task ExecuteAsync(ICommandContext context, string[] args)
 
             // Determine the host provider
             context.Trace.WriteLine("Detecting host provider for input:");
-            context.Trace.WriteDictionary(inputDict);
+            context.Trace.WriteDictionarySecrets(inputDict, new []{ "password" }, StringComparer.OrdinalIgnoreCase);
             IHostProvider provider = _hostProviderRegistry.GetProvider(input);
             context.Trace.WriteLine($"Host provider '{provider.Name}' was selected.");
 

common/src/Microsoft.Git.CredentialManager/SecureStorage/NativeMethods.Windows.cs

@@ -41,7 +41,7 @@ public static void ThrowIfError(int error, string defaultErrorMessage = null)
                     default:
                         // The Win32Exception constructor will automatically get the human-readable
                         // message for the error code.
-                        throw new Win32Exception(error, defaultErrorMessage);
+                        throw new Exception(defaultErrorMessage, new Win32Exception(error));
                 }
             }
 

common/src/Microsoft.Git.CredentialManager/Trace.cs

@@ -65,6 +65,25 @@ void WriteDictionary<TKey, TValue>(
             [System.Runtime.CompilerServices.CallerLineNumber] int lineNumber = 0,
             [System.Runtime.CompilerServices.CallerMemberName] string memberName = "");
 
+        /// <summary>
+        /// Write the contents of a dictionary that contains sensitive information to the trace writer.
+        /// <para/>
+        /// Calls <see cref="object.ToString"/> on all keys and values, except keys specified as secret.
+        /// </summary>
+        /// <param name="dictionary">The dictionary to write.</param>
+        /// <param name="secretKeys">Dictionary keys that contain secrets/sensitive information.</param>
+        /// <param name="keyComparer">Comparer to use for <paramref name="secretKeys"/>.</param>
+        /// <param name="filePath">Path of the file this method is called from.</param>
+        /// <param name="lineNumber">Line number of file this method is called from.</param>
+        /// <param name="memberName">Name of the member in which this method is called.</param>
+        void WriteDictionarySecrets<TKey, TValue>(
+            IDictionary<TKey, TValue> dictionary,
+            TKey[] secretKeys,
+            IEqualityComparer<TKey> keyComparer = null,
+            [System.Runtime.CompilerServices.CallerFilePath] string filePath = "",
+            [System.Runtime.CompilerServices.CallerLineNumber] int lineNumber = 0,
+            [System.Runtime.CompilerServices.CallerMemberName] string memberName = "");
+
         /// <summary>
         /// Writes a message to the trace writer followed by a line terminator.
         /// </summary>
@@ -101,6 +120,8 @@ void WriteLineSecrets(
 
     public class Trace : ITrace, IDisposable
     {
+        private const string SecretMask = "********";
+
         private readonly object _writersLock = new object();
         private readonly List<TextWriter> _writers = new List<TextWriter>();
 
@@ -188,6 +209,30 @@ public void WriteDictionary<TKey, TValue>(
             }
         }
 
+        [System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Design", "CA1026:DefaultParametersShouldNotBeUsed")]
+        public void WriteDictionarySecrets<TKey, TValue>(
+            IDictionary<TKey, TValue> dictionary,
+            TKey[] secretKeys,
+            IEqualityComparer<TKey> keyComparer = null,
+            [System.Runtime.CompilerServices.CallerFilePath] string filePath = "",
+            [System.Runtime.CompilerServices.CallerLineNumber] int lineNumber = 0,
+            [System.Runtime.CompilerServices.CallerMemberName] string memberName = "")
+        {
+            foreach (KeyValuePair<TKey, TValue> entry in dictionary)
+            {
+                bool isSecretEntry = !(secretKeys is null) &&
+                                     secretKeys.Contains(entry.Key, keyComparer ?? EqualityComparer<TKey>.Default);
+                if (isSecretEntry && !this.IsSecretTracingEnabled)
+                {
+                    WriteLine($"\t{entry.Key}={SecretMask}", filePath, lineNumber, memberName);
+                }
+                else
+                {
+                    WriteLine($"\t{entry.Key}={entry.Value}", filePath, lineNumber, memberName);
+                }
+            }
+        }
+
         [System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Design", "CA1026:DefaultParametersShouldNotBeUsed")]
         public void WriteLine(
             string message,
@@ -227,7 +272,7 @@ public void WriteLineSecrets(
         {
             string message = this.IsSecretTracingEnabled
                            ? string.Format(format, secrets)
-                           : string.Format(format, secrets.Select(_ => (object)"********").ToArray());
+                           : string.Format(format, secrets.Select(_ => (object)SecretMask).ToArray());
 
             WriteLine(message, filePath, lineNumber, memberName);
         }

common/tests/TestInfrastructure/Objects/NullTrace.cs

@@ -32,6 +32,10 @@ void ITrace.WriteException(Exception exception, string filePath, int lineNumber,
         void ITrace.WriteDictionary<TKey, TValue>(
             IDictionary<TKey, TValue> dictionary, string filePath, int lineNumber, string memberName) { }
 
+        public void WriteDictionarySecrets<TKey, TValue>(IDictionary<TKey, TValue> dictionary, TKey[] secretKeys,
+            IEqualityComparer<TKey> keyComparer = null, string filePath = "", int lineNumber = 0,
+            string memberName = "") { }
+
         void ITrace.WriteLine(string message, string filePath, int lineNumber, string memberName) { }
 
         void ITrace.WriteLineSecrets(