Skip to content

Commit d70115b

Browse files
committed
Ensure we're not tracing secret info in dictionaries
1 parent f54a51b commit d70115b

File tree

3 files changed

+51
-2
lines changed

3 files changed

+51
-2
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ public override async Task ExecuteAsync(ICommandContext context, string[] args)
6565

6666
// Determine the host provider
6767
context.Trace.WriteLine("Detecting host provider for input:");
68-
context.Trace.WriteDictionary(inputDict);
68+
context.Trace.WriteDictionarySecrets(inputDict, new []{ "password" }, StringComparer.OrdinalIgnoreCase);
6969
IHostProvider provider = _hostProviderRegistry.GetProvider(input);
7070
context.Trace.WriteLine($"Host provider '{provider.Name}' was selected.");
7171

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

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,25 @@ void WriteDictionary<TKey, TValue>(
6565
[System.Runtime.CompilerServices.CallerLineNumber] int lineNumber = 0,
6666
[System.Runtime.CompilerServices.CallerMemberName] string memberName = "");
6767

68+
/// <summary>
69+
/// Write the contents of a dictionary that contains sensitive information to the trace writer.
70+
/// <para/>
71+
/// Calls <see cref="object.ToString"/> on all keys and values, except keys specified as secret.
72+
/// </summary>
73+
/// <param name="dictionary">The dictionary to write.</param>
74+
/// <param name="secretKeys">Dictionary keys that contain secrets/sensitive information.</param>
75+
/// <param name="keyComparer">Comparer to use for <paramref name="secretKeys"/>.</param>
76+
/// <param name="filePath">Path of the file this method is called from.</param>
77+
/// <param name="lineNumber">Line number of file this method is called from.</param>
78+
/// <param name="memberName">Name of the member in which this method is called.</param>
79+
void WriteDictionarySecrets<TKey, TValue>(
80+
IDictionary<TKey, TValue> dictionary,
81+
TKey[] secretKeys,
82+
IEqualityComparer<TKey> keyComparer = null,
83+
[System.Runtime.CompilerServices.CallerFilePath] string filePath = "",
84+
[System.Runtime.CompilerServices.CallerLineNumber] int lineNumber = 0,
85+
[System.Runtime.CompilerServices.CallerMemberName] string memberName = "");
86+
6887
/// <summary>
6988
/// Writes a message to the trace writer followed by a line terminator.
7089
/// </summary>
@@ -101,6 +120,8 @@ void WriteLineSecrets(
101120

102121
public class Trace : ITrace, IDisposable
103122
{
123+
private const string SecretMask = "********";
124+
104125
private readonly object _writersLock = new object();
105126
private readonly List<TextWriter> _writers = new List<TextWriter>();
106127

@@ -188,6 +209,30 @@ public void WriteDictionary<TKey, TValue>(
188209
}
189210
}
190211

212+
[System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Design", "CA1026:DefaultParametersShouldNotBeUsed")]
213+
public void WriteDictionarySecrets<TKey, TValue>(
214+
IDictionary<TKey, TValue> dictionary,
215+
TKey[] secretKeys,
216+
IEqualityComparer<TKey> keyComparer = null,
217+
[System.Runtime.CompilerServices.CallerFilePath] string filePath = "",
218+
[System.Runtime.CompilerServices.CallerLineNumber] int lineNumber = 0,
219+
[System.Runtime.CompilerServices.CallerMemberName] string memberName = "")
220+
{
221+
foreach (KeyValuePair<TKey, TValue> entry in dictionary)
222+
{
223+
bool isSecretEntry = !(secretKeys is null) &&
224+
secretKeys.Contains(entry.Key, keyComparer ?? EqualityComparer<TKey>.Default);
225+
if (isSecretEntry && !this.IsSecretTracingEnabled)
226+
{
227+
WriteLine($"\t{entry.Key}={SecretMask}", filePath, lineNumber, memberName);
228+
}
229+
else
230+
{
231+
WriteLine($"\t{entry.Key}={entry.Value}", filePath, lineNumber, memberName);
232+
}
233+
}
234+
}
235+
191236
[System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Design", "CA1026:DefaultParametersShouldNotBeUsed")]
192237
public void WriteLine(
193238
string message,
@@ -227,7 +272,7 @@ public void WriteLineSecrets(
227272
{
228273
string message = this.IsSecretTracingEnabled
229274
? string.Format(format, secrets)
230-
: string.Format(format, secrets.Select(_ => (object)"********").ToArray());
275+
: string.Format(format, secrets.Select(_ => (object)SecretMask).ToArray());
231276

232277
WriteLine(message, filePath, lineNumber, memberName);
233278
}

common/tests/TestInfrastructure/Objects/NullTrace.cs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,10 @@ void ITrace.WriteException(Exception exception, string filePath, int lineNumber,
3232
void ITrace.WriteDictionary<TKey, TValue>(
3333
IDictionary<TKey, TValue> dictionary, string filePath, int lineNumber, string memberName) { }
3434

35+
public void WriteDictionarySecrets<TKey, TValue>(IDictionary<TKey, TValue> dictionary, TKey[] secretKeys,
36+
IEqualityComparer<TKey> keyComparer = null, string filePath = "", int lineNumber = 0,
37+
string memberName = "") { }
38+
3539
void ITrace.WriteLine(string message, string filePath, int lineNumber, string memberName) { }
3640

3741
void ITrace.WriteLineSecrets(

0 commit comments

Comments
 (0)