Stop-Commands: stopToken restrictions (#1371)

* Prevent stopTokens that are workflow commands

Co-authored-by: Thomas Boop <52323235+thboop@users.noreply.github.com>

* Check context for env var too

* Accept true, 1 and $true instead of just "true"

* Setup ExpressionValues in tests

* Update src/Runner.Common/Constants.cs

Co-authored-by: Thomas Boop <52323235+thboop@users.noreply.github.com>

* Separate success and fail tests for invalid token

* Fix envcontext for tests

Co-authored-by: Thomas Boop <52323235+thboop@users.noreply.github.com>

src/Runner.Common/Constants.cs

@@ -154,6 +154,7 @@ namespace GitHub.Runner.Common
             public static readonly string LowDiskSpace = "LOW_DISK_SPACE";
             public static readonly string UnsupportedCommand = "UNSUPPORTED_COMMAND";
             public static readonly string UnsupportedCommandMessageDisabled = "The `{0}` command is disabled. Please upgrade to using Environment Files or opt into unsecure command execution by setting the `ACTIONS_ALLOW_UNSECURE_COMMANDS` environment variable to `true`. For more information see: https://github.blog/changelog/2020-10-01-github-actions-deprecating-set-env-and-add-path-commands/";
+            public static readonly string UnsupportedStopCommandTokenDisabled = "You cannot use a endToken that is an empty string, the string 'pause-logging', or another workflow command. For more information see:  https://docs.github.com/en/actions/learn-github-actions/workflow-commands-for-github-actions#example-stopping-and-starting-workflow-commands or opt into insecure command execution by setting the `ACTIONS_ALLOW_UNSECURE_STOPCOMMAND_TOKENS` environment variable to `true`.";
         }
 
         public static class RunnerEvent
@@ -213,6 +214,7 @@ namespace GitHub.Runner.Common
                 // Keep alphabetical
                 //
                 public static readonly string AllowUnsupportedCommands = "ACTIONS_ALLOW_UNSECURE_COMMANDS";
+                public static readonly string AllowUnsupportedStopCommandTokens = "ACTIONS_ALLOW_UNSECURE_STOPCOMMAND_TOKENS";
                 public static readonly string RunnerDebug = "ACTIONS_RUNNER_DEBUG";
                 public static readonly string StepDebug = "ACTIONS_STEP_DEBUG";
             }

src/Runner.Worker/ActionCommandManager.cs

@@ -108,22 +108,18 @@ namespace GitHub.Runner.Worker
                     // Stop command
                     if (string.Equals(actionCommand.Command, _stopCommand, StringComparison.OrdinalIgnoreCase))
                     {
-                        context.Output(input);
+                        ValidateStopToken(context, actionCommand.Data);
-                        context.Debug("Paused processing commands until '##[{actionCommand.Data}]' is received");
+
                         _stopToken = actionCommand.Data;
-                        if (_registeredCommands.Contains(actionCommand.Data)
-                            || string.IsNullOrEmpty(actionCommand.Data)
-                            || string.Equals(actionCommand.Data, "pause-logging", StringComparison.OrdinalIgnoreCase))
-                        {
-                            var telemetry = new JobTelemetry
-                            {
-                                Message = $"Invoked ::stopCommand:: with token: [{actionCommand.Data}]",
-                                Type = JobTelemetryType.ActionCommand
-                            };
-                            context.JobTelemetry.Add(telemetry);
-                        }
                         _stopProcessCommand = true;
                         _registeredCommands.Add(_stopToken);
+                        if (_stopToken.Length > 6)
+                        {
+                            HostContext.SecretMasker.AddValue(_stopToken);
+                        }
+
+                        context.Output(input);
+                        context.Debug("Paused processing commands until the token you called ::stopCommands:: with is received");
                         return true;
                     }
                     // Found command
@@ -157,6 +153,40 @@ namespace GitHub.Runner.Worker
             return true;
         }
 
+        private void ValidateStopToken(IExecutionContext context, string stopToken)
+        {
+#if OS_WINDOWS
+            var envContext = context.ExpressionValues["env"] as DictionaryContextData;
+#else
+            var envContext = context.ExpressionValues["env"] as CaseSensitiveDictionaryContextData;
+#endif
+            var allowUnsecureStopCommandTokens = false;
+            allowUnsecureStopCommandTokens = StringUtil.ConvertToBoolean(Environment.GetEnvironmentVariable(Constants.Variables.Actions.AllowUnsupportedStopCommandTokens));
+            if (!allowUnsecureStopCommandTokens && envContext.ContainsKey(Constants.Variables.Actions.AllowUnsupportedStopCommandTokens))
+            {
+                allowUnsecureStopCommandTokens = StringUtil.ConvertToBoolean(envContext[Constants.Variables.Actions.AllowUnsupportedStopCommandTokens].ToString());
+            }
+
+            bool isTokenInvalid = _registeredCommands.Contains(stopToken)
+                || string.IsNullOrEmpty(stopToken)
+                || string.Equals(stopToken, "pause-logging", StringComparison.OrdinalIgnoreCase);
+
+            if (isTokenInvalid)
+            {
+                var telemetry = new JobTelemetry
+                {
+                    Message = $"Invoked ::stopCommand:: with token: [{stopToken}]",
+                    Type = JobTelemetryType.ActionCommand
+                };
+                context.JobTelemetry.Add(telemetry);
+            }
+
+            if (isTokenInvalid && !allowUnsecureStopCommandTokens)
+            {
+                throw new Exception(Constants.Runner.UnsupportedStopCommandTokenDisabled);
+            }
+        }
+
         internal static bool EnhancedAnnotationsEnabled(IExecutionContext context)
         {
             return context.Global.Variables.GetBoolean("DistributedTask.EnhancedAnnotations") ?? false;

src/Test/L0/Worker/ActionCommandManagerL0.cs

@@ -2,6 +2,7 @@
 using System.Collections.Generic;
 using System.IO;
 using System.Runtime.CompilerServices;
+using GitHub.DistributedTask.Pipelines.ContextData;
 using GitHub.DistributedTask.WebApi;
 using GitHub.Runner.Worker;
 using GitHub.Runner.Worker.Container;
@@ -83,6 +84,7 @@ namespace GitHub.Runner.Common.Tests.Worker
         {
             using (TestHostContext hc = CreateTestContext())
             {
+                _ec.Setup(x => x.ExpressionValues).Returns(GetExpressionValues());
                 _ec.Setup(x => x.Write(It.IsAny<string>(), It.IsAny<string>()))
                    .Returns((string tag, string line) =>
                             {
@@ -105,6 +107,88 @@ namespace GitHub.Runner.Common.Tests.Worker
             }
         }
 
+        [Theory]
+        [InlineData("stop-commands", "1")]
+        [InlineData("", "1")]
+        [InlineData("set-env", "1")]
+        [InlineData("stop-commands", "true")]
+        [InlineData("", "true")]
+        [InlineData("set-env", "true")]
+        [Trait("Level", "L0")]
+        [Trait("Category", "Worker")]
+        public void StopProcessCommand__AllowsInvalidStopTokens__IfEnvVarIsSet(string invalidToken, string allowUnsupportedStopCommandTokens)
+        {
+            using (TestHostContext hc = CreateTestContext())
+            {
+                _ec.Object.Global.EnvironmentVariables = new Dictionary<string, string>();
+                var expressionValues = new DictionaryContextData
+            {
+                ["env"] =
+#if OS_WINDOWS
+                        new DictionaryContextData{ { Constants.Variables.Actions.AllowUnsupportedStopCommandTokens, new StringContextData(allowUnsupportedStopCommandTokens) }}
+#else
+                        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>());
+
+                Assert.True(_commandManager.TryProcessCommand(_ec.Object, $"::stop-commands::{invalidToken}", null));
+            }
+        }
+
+        [Theory]
+        [InlineData("stop-commands")]
+        [InlineData("")]
+        [InlineData("set-env")]
+        [Trait("Level", "L0")]
+        [Trait("Category", "Worker")]
+        public void StopProcessCommand__FailOnInvalidStopTokens(string invalidToken)
+        {
+            using (TestHostContext hc = CreateTestContext())
+            {
+                _ec.Object.Global.EnvironmentVariables = new Dictionary<string, string>();
+                _ec.Setup(x => x.ExpressionValues).Returns(GetExpressionValues());
+                _ec.Setup(x => x.JobTelemetry).Returns(new List<JobTelemetry>());
+                Assert.Throws<Exception>(() => _commandManager.TryProcessCommand(_ec.Object, $"::stop-commands::{invalidToken}", null));
+            }
+        }
+
+        [Fact]
+        [Trait("Level", "L0")]
+        [Trait("Category", "Worker")]
+        public void StopProcessCommandAcceptsValidToken()
+        {
+            var validToken = "randomToken";
+            using (TestHostContext hc = CreateTestContext())
+            {
+                _ec.Setup(x => x.ExpressionValues).Returns(GetExpressionValues());
+                Assert.True(_commandManager.TryProcessCommand(_ec.Object, $"::stop-commands::{validToken}", null));
+                Assert.False(_commandManager.TryProcessCommand(_ec.Object, "##[set-env name=foo]bar", null));
+                Assert.True(_commandManager.TryProcessCommand(_ec.Object, $"::{validToken}::", null));
+                Assert.True(_commandManager.TryProcessCommand(_ec.Object, "##[set-env name=foo]bar", null));
+            }
+        }
+
+        [Fact]
+        [Trait("Level", "L0")]
+        [Trait("Category", "Worker")]
+        public void StopProcessCommandMasksValidTokenForEntireRun()
+        {
+            var validToken = "randomToken";
+            using (TestHostContext hc = CreateTestContext())
+            {
+                _ec.Setup(x => x.ExpressionValues).Returns(GetExpressionValues());
+                Assert.True(_commandManager.TryProcessCommand(_ec.Object, $"::stop-commands::{validToken}", null));
+                Assert.False(_commandManager.TryProcessCommand(_ec.Object, "##[set-env name=foo]bar", null));
+                Assert.Equal("***", hc.SecretMasker.MaskSecrets(validToken));
+
+                Assert.True(_commandManager.TryProcessCommand(_ec.Object, $"::{validToken}::", null));
+                Assert.True(_commandManager.TryProcessCommand(_ec.Object, "##[set-env name=foo]bar", null));
+                Assert.Equal("***", hc.SecretMasker.MaskSecrets(validToken));
+            }
+        }
+
         [Fact]
         [Trait("Level", "L0")]
         [Trait("Category", "Worker")]
@@ -202,15 +286,15 @@ namespace GitHub.Runner.Common.Tests.Worker
                                 return 1;
                             });
 
-                var registeredCommands = new HashSet<string>(new string[1]{ "warning" });
+                var registeredCommands = new HashSet<string>(new string[1] { "warning" });
                 ActionCommand command;
-                
+
                 // Columns when lines are different
                 ActionCommand.TryParseV2("::warning line=1,endLine=2,col=1,endColumn=2::this is a warning", registeredCommands, out command);
                 Assert.Equal("1", command.Properties["col"]);
                 IssueCommandExtension.ValidateLinesAndColumns(command, _ec.Object);
                 Assert.False(command.Properties.ContainsKey("col"));
-            
+
                 // No lines with columns
                 ActionCommand.TryParseV2("::warning col=1,endColumn=2::this is a warning", registeredCommands, out command);
                 Assert.Equal("1", command.Properties["col"]);
@@ -375,5 +459,19 @@ namespace GitHub.Runner.Common.Tests.Worker
 
             return hostContext;
         }
+
+        private DictionaryContextData GetExpressionValues()
+        {
+            return new DictionaryContextData
+            {
+                ["env"] =
+#if OS_WINDOWS
+                        new DictionaryContextData()
+#else
+                        new CaseSensitiveDictionaryContextData()
+#endif
+            };
+        }
+
     }
 }