From ba6ac0dadb7f6d9490aa6863c127990b46f7e4a6 Mon Sep 17 00:00:00 2001 From: TingluoHuang Date: Tue, 30 Nov 2021 10:42:19 -0500 Subject: [PATCH 1/2] Add mask for multiline secrets. --- src/Runner.Worker/ActionCommandManager.cs | 7 +++++++ src/Test/L0/Worker/ActionCommandManagerL0.cs | 19 +++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/src/Runner.Worker/ActionCommandManager.cs b/src/Runner.Worker/ActionCommandManager.cs index 45b2fefdb67..bfa1add045d 100644 --- a/src/Runner.Worker/ActionCommandManager.cs +++ b/src/Runner.Worker/ActionCommandManager.cs @@ -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); + foreach (var item in split) + { + HostContext.SecretMasker.AddValue(item.Trim()); + } } } } diff --git a/src/Test/L0/Worker/ActionCommandManagerL0.cs b/src/Test/L0/Worker/ActionCommandManagerL0.cs index ac5c4af9324..6a2ae67d340 100644 --- a/src/Test/L0/Worker/ActionCommandManagerL0.cs +++ b/src/Test/L0/Worker/ActionCommandManagerL0.cs @@ -418,6 +418,24 @@ 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", 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("abc\rdef\nghi")); + } + } + private TestHostContext CreateTestContext([CallerMemberName] string testName = "") { var hostContext = new TestHostContext(this, testName); @@ -431,6 +449,7 @@ private TestHostContext CreateTestContext([CallerMemberName] string testName = " new InternalPluginSetRepoPathCommandExtension(), new SetEnvCommandExtension(), new WarningCommandExtension(), + new AddMaskCommandExtension(), }; foreach (var command in commands) { From 14828bb4f4b8ef3b828d7b10ef3a378dd46836f2 Mon Sep 17 00:00:00 2001 From: TingluoHuang Date: Tue, 30 Nov 2021 22:30:28 -0500 Subject: [PATCH 2/2] . --- src/Runner.Worker/ActionCommandManager.cs | 4 ++-- src/Runner.Worker/Worker.cs | 11 ++++++----- src/Test/L0/Worker/ActionCommandManagerL0.cs | 19 +++++++++++++------ 3 files changed, 21 insertions(+), 13 deletions(-) diff --git a/src/Runner.Worker/ActionCommandManager.cs b/src/Runner.Worker/ActionCommandManager.cs index bfa1add045d..6375db2934a 100644 --- a/src/Runner.Worker/ActionCommandManager.cs +++ b/src/Runner.Worker/ActionCommandManager.cs @@ -383,10 +383,10 @@ public void ProcessCommand(IExecutionContext context, string line, ActionCommand 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); + var split = command.Data.Split(new[] { '\r', '\n' }, StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries); foreach (var item in split) { - HostContext.SecretMasker.AddValue(item.Trim()); + HostContext.SecretMasker.AddValue(item); } } } diff --git a/src/Runner.Worker/Worker.cs b/src/Runner.Worker/Worker.cs index 1c83c434292..115789c4fff 100644 --- a/src/Runner.Worker/Worker.cs +++ b/src/Runner.Worker/Worker.cs @@ -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 SecretVariableMaskWhitelist = new HashSet(StringComparer.OrdinalIgnoreCase){ + private static HashSet SecretVariableMaskWhitelist = new HashSet(StringComparer.OrdinalIgnoreCase) + { Constants.Variables.Actions.StepDebug, Constants.Variables.Actions.RunnerDebug - }; + }; public async Task RunAsync(string pipeIn, string pipeOut) { @@ -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); } } } diff --git a/src/Test/L0/Worker/ActionCommandManagerL0.cs b/src/Test/L0/Worker/ActionCommandManagerL0.cs index 6a2ae67d340..cb53a681673 100644 --- a/src/Test/L0/Worker/ActionCommandManagerL0.cs +++ b/src/Test/L0/Worker/ActionCommandManagerL0.cs @@ -122,14 +122,14 @@ public void StopProcessCommand__AllowsInvalidStopTokens__IfEnvVarIsSet(string in { _ec.Object.Global.EnvironmentVariables = new Dictionary(); 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()); @@ -426,13 +426,20 @@ public void AddMaskWithMultilineValue() using (TestHostContext hc = CreateTestContext()) { // Act - _commandManager.TryProcessCommand(_ec.Object, $"::add-mask::abc%0Ddef%0Aghi", null); + _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("abc\rdef\nghi")); + 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(" ")); } }