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 masks for multiline secrets from ::add-mask:: #1521

Merged
merged 2 commits into from
Dec 1, 2021
Merged
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
7 changes: 7 additions & 0 deletions src/Runner.Worker/ActionCommandManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -381,6 +381,13 @@ public void ProcessCommand(IExecutionContext context, string line, ActionCommand

HostContext.SecretMasker.AddValue(command.Data);
Trace.Info($"Add new secret mask with length of {command.Data.Length}");

// Also add each individual line. Typically individual lines are processed from STDOUT of child processes.
var split = command.Data.Split(new[] { '\r', '\n' }, StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries);
foreach (var item in split)
{
HostContext.SecretMasker.AddValue(item);
}
}
}
}
Expand Down
11 changes: 6 additions & 5 deletions src/Runner.Worker/Worker.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,13 @@ public sealed class Worker : RunnerService, IWorker
{
private readonly TimeSpan _workerStartTimeout = TimeSpan.FromSeconds(30);
private ManualResetEvent _completedCommand = new ManualResetEvent(false);

// Do not mask the values of these secrets
private static HashSet<String> SecretVariableMaskWhitelist = new HashSet<String>(StringComparer.OrdinalIgnoreCase){
private static HashSet<String> SecretVariableMaskWhitelist = new HashSet<String>(StringComparer.OrdinalIgnoreCase)
{
Constants.Variables.Actions.StepDebug,
Constants.Variables.Actions.RunnerDebug
};
};

public async Task<int> RunAsync(string pipeIn, string pipeOut)
{
Expand Down Expand Up @@ -138,10 +139,10 @@ private void InitializeSecretMasker(Pipelines.AgentJobRequestMessage message)
HostContext.SecretMasker.AddValue(value);

// Also add each individual line. Typically individual lines are processed from STDOUT of child processes.
var split = value.Split(new[] { '\r', '\n' }, StringSplitOptions.RemoveEmptyEntries);
var split = value.Split(new[] { '\r', '\n' }, StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries);
foreach (var item in split)
{
HostContext.SecretMasker.AddValue(item.Trim());
HostContext.SecretMasker.AddValue(item);
}
}
}
Expand Down
34 changes: 30 additions & 4 deletions src/Test/L0/Worker/ActionCommandManagerL0.cs
Original file line number Diff line number Diff line change
Expand Up @@ -122,14 +122,14 @@ public void StopProcessCommand__AllowsInvalidStopTokens__IfEnvVarIsSet(string in
{
_ec.Object.Global.EnvironmentVariables = new Dictionary<string, string>();
var expressionValues = new DictionaryContextData
{
["env"] =
{
["env"] =
#if OS_WINDOWS
new DictionaryContextData{ { Constants.Variables.Actions.AllowUnsupportedStopCommandTokens, new StringContextData(allowUnsupportedStopCommandTokens) }}
#else
new CaseSensitiveDictionaryContextData{ { Constants.Variables.Actions.AllowUnsupportedStopCommandTokens, new StringContextData(allowUnsupportedStopCommandTokens) }}
new CaseSensitiveDictionaryContextData { { Constants.Variables.Actions.AllowUnsupportedStopCommandTokens, new StringContextData(allowUnsupportedStopCommandTokens) } }
#endif
};
};
_ec.Setup(x => x.ExpressionValues).Returns(expressionValues);
_ec.Setup(x => x.JobTelemetry).Returns(new List<JobTelemetry>());

Expand Down Expand Up @@ -418,6 +418,31 @@ public void AddMatcherTranslatesFilePath()
}
}

[Fact]
[Trait("Level", "L0")]
[Trait("Category", "Worker")]
public void AddMaskWithMultilineValue()
{
using (TestHostContext hc = CreateTestContext())
{
// Act
_commandManager.TryProcessCommand(_ec.Object, $"::add-mask::abc%0Ddef%0Aghi%0D%0Ajkl", null);
_commandManager.TryProcessCommand(_ec.Object, $"::add-mask:: %0D %0A %0D%0A %0D", null);

// Assert
Assert.Equal("***", hc.SecretMasker.MaskSecrets("abc"));
Assert.Equal("***", hc.SecretMasker.MaskSecrets("def"));
Assert.Equal("***", hc.SecretMasker.MaskSecrets("ghi"));
Assert.Equal("***", hc.SecretMasker.MaskSecrets("jkl"));
Assert.Equal("***", hc.SecretMasker.MaskSecrets("abc\rdef\nghi\r\njkl"));
Assert.Equal("", hc.SecretMasker.MaskSecrets(""));
Assert.Equal(" ", hc.SecretMasker.MaskSecrets(" "));
Assert.Equal(" ", hc.SecretMasker.MaskSecrets(" "));
Assert.Equal(" ", hc.SecretMasker.MaskSecrets(" "));
Assert.Equal(" ", hc.SecretMasker.MaskSecrets(" "));
}
}

private TestHostContext CreateTestContext([CallerMemberName] string testName = "")
{
var hostContext = new TestHostContext(this, testName);
Expand All @@ -431,6 +456,7 @@ private TestHostContext CreateTestContext([CallerMemberName] string testName = "
new InternalPluginSetRepoPathCommandExtension(),
new SetEnvCommandExtension(),
new WarningCommandExtension(),
new AddMaskCommandExtension(),
};
foreach (var command in commands)
{
Expand Down