Skip to content
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

Add secret masking for the telemetry.publish command #4461

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/Agent.Sdk/Knob/AgentKnobs.cs
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,7 @@ public class AgentKnobs
public static readonly Knob MaskUsingCredScanRegexes = new Knob(
nameof(MaskUsingCredScanRegexes),
"Use the CredScan regexes for masking secrets. CredScan is an internal tool developed at Microsoft to keep passwords and authentication keys from being checked in. This defaults to disabled, as there are performance problems with some task outputs.",
new RuntimeKnobSource("AZP_USE_CREDSCAN_REGEXES"),
new EnvironmentKnobSource("AZP_USE_CREDSCAN_REGEXES"),
new BuiltInDefaultKnobSource("false"));

Expand Down
9 changes: 9 additions & 0 deletions src/Agent.Worker/ExecutionContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
using Microsoft.TeamFoundation.DistributedTask.WebApi;
using Pipelines = Microsoft.TeamFoundation.DistributedTask.Pipelines;
using Microsoft.VisualStudio.Services.Agent.Util;
using Agent.Sdk.Util;

namespace Microsoft.VisualStudio.Services.Agent.Worker
{
Expand Down Expand Up @@ -511,6 +512,14 @@ public void InitializeJob(Pipelines.AgentJobRequestMessage message, Cancellation
Variables = new Variables(HostContext, message.Variables, out warnings);
Variables.StringTranslator = TranslatePathForStepTarget;

if (AgentKnobs.MaskUsingCredScanRegexes.GetValue(this).AsBoolean())
{
foreach (var pattern in AdditionalMaskingRegexes.CredScanPatterns)
{
HostContext.SecretMasker.AddRegex(pattern, $"ExecutionContext_{WellKnownSecretAliases.CredScanPatterns}");
}
}

if (Variables.GetBoolean("agent.useWorkspaceId") == true)
{
try
Expand Down
1 change: 1 addition & 0 deletions src/Agent.Worker/Telemetry/TelemetryCommandExtension.cs
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ public void Execute(IExecutionContext context, Command command)
throw new ArgumentException(StringUtil.Loc("ArgumentNeeded", "EventTrackerData"));
}

data = context.GetHostContext().SecretMasker.MaskSecrets(data);
CustomerIntelligenceEvent ciEvent;
try
{
Expand Down
7 changes: 0 additions & 7 deletions src/Microsoft.VisualStudio.Services.Agent/HostContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -106,13 +106,6 @@ public HostContext(HostType hostType, string logFile = null)
this.SecretMasker.AddValueEncoder(ValueEncoders.UriDataEscape, $"HostContext_{WellKnownSecretAliases.UriDataEscape}");
this.SecretMasker.AddValueEncoder(ValueEncoders.BackslashEscape, $"HostContext_{WellKnownSecretAliases.UriDataEscape}");
this.SecretMasker.AddRegex(AdditionalMaskingRegexes.UrlSecretPattern, $"HostContext_{WellKnownSecretAliases.UrlSecretPattern}");
if (AgentKnobs.MaskUsingCredScanRegexes.GetValue(this).AsBoolean())
{
foreach (var pattern in AdditionalMaskingRegexes.CredScanPatterns)
{
this.SecretMasker.AddRegex(pattern, $"HostContext_{WellKnownSecretAliases.CredScanPatterns}");
}
}

// Create the trace manager.
if (string.IsNullOrEmpty(logFile))
Expand Down
32 changes: 0 additions & 32 deletions src/Test/L0/HostContextL0.cs
Original file line number Diff line number Diff line change
Expand Up @@ -86,38 +86,6 @@ public void UrlSecretsAreMasked(string input, string expected)
}
}

[Theory]
[Trait("Level", "L0")]
[Trait("Category", "Common")]
// some secrets that CredScan should suppress
[InlineData("xoxr-1xwlcyhsnfn9k69m4efzj3zkfhk", "***")] // Slack token
[InlineData("(+n97tcqhcpvu9zkhwwiwx4==)", "(***)")] // 128-bit symmetric key
[InlineData("<jwt>eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c</jwt>", "<jwt>***</jwt>")]
// some secrets that CredScan should NOT suppress
[InlineData("The password is knock knock knock", "The password is knock knock knock")]
[InlineData("SSdtIGEgY29tcGxldGVseSBpbm5vY3VvdXMgc3RyaW5nLg==", "SSdtIGEgY29tcGxldGVseSBpbm5vY3VvdXMgc3RyaW5nLg==")]
public void OtherSecretsAreMasked(string input, string expected)
{
// Arrange.
try
{
Environment.SetEnvironmentVariable("AZP_USE_CREDSCAN_REGEXES", "true");

using (var _hc = Setup())
{
// Act.
var result = _hc.SecretMasker.MaskSecrets(input);

// Assert.
Assert.Equal(expected, result);
}
}
finally
{
Environment.SetEnvironmentVariable("AZP_USE_CREDSCAN_REGEXES", null);
}
}

[Fact]
public void LogFileChangedAccordingToEnvVariable()
{
Expand Down
8 changes: 8 additions & 0 deletions src/Test/L0/TestHostContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,14 @@ public TestHostContext(object testClass, [CallerMemberName] string testName = ""
_secretMasker.AddValueEncoder(ValueEncoders.JsonStringEscape);
_secretMasker.AddValueEncoder(ValueEncoders.UriDataEscape);
_secretMasker.AddValueEncoder(ValueEncoders.BackslashEscape);
if (Boolean.TryParse(Environment.GetEnvironmentVariable("AZP_USE_CREDSCAN_REGEXES"), out bool credScanEnabled) && credScanEnabled)
{
foreach (var pattern in AdditionalMaskingRegexes.CredScanPatterns)
{
_secretMasker.AddRegex(pattern, $"ExecutionContext_{WellKnownSecretAliases.CredScanPatterns}");
}
}

_traceManager = new TraceManager(traceListener, _secretMasker);
_trace = GetTrace(nameof(TestHostContext));

Expand Down
35 changes: 35 additions & 0 deletions src/Test/L0/Worker/ExecutionContextL0.cs
Original file line number Diff line number Diff line change
Expand Up @@ -566,6 +566,41 @@ public void TranslatePathForStepTarget_should_convert_path_only_for_containers(b
}
}

[Theory]
[Trait("Level", "L0")]
[Trait("Category", "Worker")]
// some secrets that CredScan should suppress
[InlineData("xoxr-1xwlcyhsnfn9k69m4efzj3zkfhk", "***", true)] // Slack token
[InlineData("xoxr-1xwlcyhsnfn9k69m4efzj3zkfhk", "xoxr-1xwlcyhsnfn9k69m4efzj3zkfhk", false)]
[InlineData("(+n97tcqhcpvu9zkhwwiwx4==)", "(***)", true)] // 128-bit symmetric key
[InlineData("(+n97tcqhcpvu9zkhwwiwx4==)", "(+n97tcqhcpvu9zkhwwiwx4==)", false)]
[InlineData("<jwt>eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c</jwt>", "<jwt>***</jwt>", true)]
[InlineData("<jwt>eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c</jwt>", "<jwt>eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c</jwt>", false)]
// some secrets that CredScan should NOT suppress
[InlineData("The password is knock knock knock", "The password is knock knock knock", true)]
[InlineData("SSdtIGEgY29tcGxldGVseSBpbm5vY3VvdXMgc3RyaW5nLg==", "SSdtIGEgY29tcGxldGVseSBpbm5vY3VvdXMgc3RyaW5nLg==", true)]
public void UseCredScan(string input, string expected, bool enabled)
{
// Arrange.
try
{
Environment.SetEnvironmentVariable("AZP_USE_CREDSCAN_REGEXES", enabled.ToString());

using (var test_hc = CreateTestContext())
{
// Act.
var result = test_hc.SecretMasker.MaskSecrets(input);

// Assert.
Assert.Equal(expected, result);
}
}
finally
{
Environment.SetEnvironmentVariable("AZP_USE_CREDSCAN_REGEXES", null);
}
}


private TestHostContext CreateTestContext([CallerMemberName] String testName = "")
{
Expand Down
Loading