-
Notifications
You must be signed in to change notification settings - Fork 864
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Configurable masking token, perf improvements, refactoring. #4586
base: master
Are you sure you want to change the base?
Changes from 5 commits
ec1a87d
de5824a
8de1ab2
d4b65df
a95cc17
a24381f
ba5aa24
8a772aa
f9e3a63
d911e20
1a87083
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,5 +8,5 @@ internal interface ISecret | |
/// <summary> | ||
/// Returns one item (start, length) for each match found in the input string. | ||
/// </summary> | ||
IEnumerable<ReplacementPosition> GetPositions(String input); | ||
IEnumerable<Replacement> GetReplacements(String input); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,12 @@ | ||
using Agent.Sdk.SecretMasking; | ||
using Microsoft.TeamFoundation.DistributedTask.Logging; | ||
|
||
using System; | ||
using System.Collections.Generic; | ||
using System.Diagnostics; | ||
using System.Text.RegularExpressions; | ||
using System.Threading; | ||
|
||
using ISecretMasker = Microsoft.TeamFoundation.DistributedTask.Logging.ISecretMasker; | ||
|
||
namespace Agent.Sdk.Util.SecretMasking; | ||
|
@@ -60,7 +67,7 @@ public void AddRegex(string pattern) | |
/// </summary> | ||
/// <param name="pattern"></param> | ||
/// <param name="origin"></param> | ||
public void AddRegex(string pattern, string origin) | ||
public void AddRegex(string pattern, string origin, string moniker = null, ISet<string> sniffLiterals = null, RegexOptions regexOptions = 0) | ||
{ | ||
this.Trace($"Setting up regex for origin: {origin}."); | ||
if (pattern == null) | ||
|
@@ -129,13 +136,34 @@ public ISecretMasker Clone() | |
return new LegacyLoggedSecretMasker(this._secretMasker.Clone()); | ||
} | ||
|
||
public double elapsedMaskingTime; | ||
|
||
|
||
public string MaskSecrets(string input) | ||
{ | ||
return this._secretMasker.MaskSecrets(input); | ||
var stopwatch = Stopwatch.StartNew(); | ||
string result = this._secretMasker.MaskSecrets(input); | ||
Interlocked.Exchange(ref elapsedMaskingTime, stopwatch.ElapsedTicks); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
return result; | ||
} | ||
|
||
Sdk.SecretMasking.ISecretMasker Sdk.SecretMasking.ISecretMasker.Clone() | ||
{ | ||
return new LegacyLoggedSecretMasker(this._secretMasker.Clone()); | ||
} | ||
|
||
public IDictionary<string, string> GetTelemetry() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
{ | ||
var result = new Dictionary<string, string> | ||
{ | ||
{ "ElapsedMaskingTime", elapsedMaskingTime.ToString() }, | ||
}; | ||
|
||
return result; | ||
} | ||
|
||
public void AddRegex(string pattern, ISet<string> sniffLiterals, RegexOptions regexOptions, string origin) | ||
{ | ||
throw new System.NotImplementedException(); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,38 +4,28 @@ | |
using System; | ||
using ValueEncoder = Microsoft.TeamFoundation.DistributedTask.Logging.ValueEncoder; | ||
using ISecretMaskerVSO = Microsoft.TeamFoundation.DistributedTask.Logging.ISecretMasker; | ||
using System.Collections.Generic; | ||
using System.Text.RegularExpressions; | ||
|
||
namespace Agent.Sdk.SecretMasking | ||
{ | ||
/// <summary> | ||
/// Extended secret masker service, that allows to log origins of secrets | ||
/// </summary> | ||
public class LoggedSecretMasker : ILoggedSecretMasker | ||
public class LoggedSecretMasker : SecretMasker, ILoggedSecretMasker | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The logged secret masker (as opposed to the legacy logged secret masker) can extend |
||
{ | ||
private ISecretMasker _secretMasker; | ||
private ITraceWriter _trace; | ||
|
||
|
||
private void Trace(string msg) | ||
{ | ||
this._trace?.Info(msg); | ||
} | ||
|
||
public LoggedSecretMasker(ISecretMasker secretMasker) | ||
{ | ||
this._secretMasker = secretMasker; | ||
} | ||
|
||
public void SetTrace(ITraceWriter trace) | ||
{ | ||
this._trace = trace; | ||
} | ||
|
||
public void AddValue(string pattern) | ||
{ | ||
this._secretMasker.AddValue(pattern); | ||
} | ||
|
||
/// <summary> | ||
/// Overloading of AddValue method with additional logic for logging origin of provided secret | ||
/// </summary> | ||
|
@@ -52,17 +42,13 @@ public void AddValue(string value, string origin) | |
|
||
AddValue(value); | ||
} | ||
public void AddRegex(string pattern) | ||
{ | ||
this._secretMasker.AddRegex(pattern); | ||
} | ||
|
||
/// <summary> | ||
/// Overloading of AddRegex method with additional logic for logging origin of provided secret | ||
/// </summary> | ||
/// <param name="pattern"></param> | ||
/// <param name="origin"></param> | ||
public void AddRegex(string pattern, string origin) | ||
public void AddRegex(string pattern, string origin, string moniker, ISet<string> sniffLiterals, RegexOptions regexOptions) | ||
{ | ||
this.Trace($"Setting up regex for origin: {origin}."); | ||
if (pattern == null) | ||
|
@@ -71,44 +57,38 @@ public void AddRegex(string pattern, string origin) | |
return; | ||
} | ||
|
||
AddRegex(pattern); | ||
AddRegex(pattern, moniker, sniffLiterals, regexOptions); | ||
} | ||
|
||
// We don't allow to skip secrets longer than 5 characters. | ||
// Note: the secret that will be ignored is of length n-1. | ||
public static int MinSecretLengthLimit => 6; | ||
|
||
public int MinSecretLength | ||
public override int MinSecretLength | ||
{ | ||
get | ||
{ | ||
return _secretMasker.MinSecretLength; | ||
return base.MinSecretLength; | ||
} | ||
set | ||
{ | ||
if (value > MinSecretLengthLimit) | ||
{ | ||
_secretMasker.MinSecretLength = MinSecretLengthLimit; | ||
base.MinSecretLength = MinSecretLengthLimit; | ||
} | ||
else | ||
{ | ||
_secretMasker.MinSecretLength = value; | ||
base.MinSecretLength = value; | ||
} | ||
} | ||
} | ||
|
||
public void RemoveShortSecretsFromDictionary() | ||
{ | ||
this._trace?.Info("Removing short secrets from masking dictionary"); | ||
_secretMasker.RemoveShortSecretsFromDictionary(); | ||
} | ||
|
||
public void AddValueEncoder(ValueEncoder encoder) | ||
{ | ||
this._secretMasker.AddValueEncoder(encoder); | ||
base.RemoveShortSecretsFromDictionary(); | ||
} | ||
|
||
|
||
/// <summary> | ||
/// Overloading of AddValueEncoder method with additional logic for logging origin of provided secret | ||
/// </summary> | ||
|
@@ -127,16 +107,17 @@ public void AddValueEncoder(ValueEncoder encoder, string origin) | |
AddValueEncoder(encoder); | ||
} | ||
|
||
public ISecretMasker Clone() | ||
{ | ||
return new LoggedSecretMasker(this._secretMasker.Clone()); | ||
} | ||
ISecretMaskerVSO ISecretMaskerVSO.Clone() => this.Clone(); | ||
|
||
public string MaskSecrets(string input) | ||
public IDictionary<string, string> GetTelemetry() | ||
{ | ||
return this._secretMasker.MaskSecrets(input); | ||
} | ||
var result = new Dictionary<string, string> | ||
{ | ||
{ nameof(ElapsedMaskingTime), ElapsedMaskingTime.ToString() }, | ||
}; | ||
|
||
ISecretMaskerVSO ISecretMaskerVSO.Clone() => this.Clone(); | ||
result["Redactions"] = string.Join(',', ReplacementTokens); | ||
return result; | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
// Copyright (c) Microsoft Corporation. | ||
// Licensed under the MIT License. | ||
|
||
|
||
using System.Collections.Generic; | ||
using System.Text.RegularExpressions; | ||
|
||
namespace Microsoft.VisualStudio.Services.Agent | ||
{ | ||
public class PatternDescriptor | ||
{ | ||
public string Regex { get; set; } | ||
|
||
public string Moniker { get; set; } | ||
|
||
public ISet<string> SniffLiterals { get; set; } | ||
|
||
public RegexOptions RegexOptions { get; set; } | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added this on ILoggedSecretMasker because I thought it made most sense given that we will produce this data for both the legacy and current secret maskers. This interface currently holds all extensions to the original ADO masker interface.