Revert "docker: escape key-value pair as -e KEY and VALUE being environment var (#2091)"

This reverts commit 95459dea5f.

docs/adrs/1891-container-hooks.md

@@ -16,7 +16,7 @@ We should give them that option, and publish examples how how they can create th
   - For example, the current runner overrides `HOME`, we can do that in the hook, but we shouldn't pass that hook as an ENV with the other env's the user has set, as that is not user input, it is how the runner invokes containers
 
 ## Interface
-- You will set the variable `ACTIONS_RUNNER_CONTAINER_HOOK=/Users/foo/runner/hooks.js` which is the entrypoint to your hook handler.
+- You will set the variable `ACTIONS_RUNNER_CONTAINER_HOOKS=/Users/foo/runner/hooks.js` which is the entrypoint to your hook handler.
   - There is no partial opt in, you must handle every hook 
 - We will pass a command and some args via `stdin`
 - An exit code of 0 is a success, every other exit code is a failure

releaseNote.md

@@ -1,5 +1,5 @@
 ## Bugs
-- Fixed an issue where self hosted environments had their docker env's overwritten (#2107)
+- Fixed an issue where job and service container envs were corrupted (#2091)
 ## Misc
 
 ## Windows x64

releaseVersion

@@ -1 +1 @@
-2.296.2
+<Update to ./src/runnerversion when creating release>

src/Runner.Worker/ActionCommandManager.cs

@@ -585,6 +585,8 @@ namespace GitHub.Runner.Worker
 
         public void ProcessCommand(IExecutionContext context, string inputLine, ActionCommand command, ContainerInfo container)
         {
+            ValidateLinesAndColumns(command, context);
+        
             command.Properties.TryGetValue(IssueCommandProperties.File, out string file);
             command.Properties.TryGetValue(IssueCommandProperties.Line, out string line);
             command.Properties.TryGetValue(IssueCommandProperties.Column, out string column);

src/Runner.Worker/Container/DockerUtil.cs

@@ -6,9 +6,6 @@ namespace GitHub.Runner.Worker.Container
 {
     public class DockerUtil
     {
-        private static readonly Regex QuoteEscape = new Regex(@"(\\*)" + "\"", RegexOptions.Compiled);
-        private static readonly Regex EndOfStringEscape = new Regex(@"(\\+)$", RegexOptions.Compiled);
-
         public static List<PortMapping> ParseDockerPort(IList<string> portMappingLines)
         {
             const string targetPort = "targetPort";
@@ -71,7 +68,7 @@ namespace GitHub.Runner.Worker.Container
             {
                 return "";
             }
-            return $"{flag} {EscapeString(key)}";
+            return $"{flag} \"{EscapeString(key)}\"";
         }
 
         public static string CreateEscapedOption(string flag, string key, string value)
@@ -80,28 +77,12 @@ namespace GitHub.Runner.Worker.Container
             {
                 return "";
             }
-            var escapedString = EscapeString($"{key}={value}");
+            return $"{flag} \"{EscapeString(key)}={EscapeString(value)}\"";
-            return $"{flag} {escapedString}";
         }
 
         private static string EscapeString(string value)
         {
-            if (String.IsNullOrEmpty(value))
+            return value.Replace("\\", "\\\\").Replace("\"", "\\\"");
-            {
-                return "";
-            }
-            // Dotnet escaping rules are weird here, we can only escape \ if it precedes a "
-            // If a double quotation mark follows two or an even number of backslashes, each proceeding backslash pair is replaced with one backslash and the double quotation mark is removed.
-            // If a double quotation mark follows an odd number of backslashes, including just one, each preceding pair is replaced with one backslash and the remaining backslash is removed; however, in this case the double quotation mark is not removed.
-            // https://docs.microsoft.com/en-us/dotnet/api/system.environment.getcommandlineargs?redirectedfrom=MSDN&view=net-6.0#remarks
-
-            // First, find any \ followed by a " and double the number of \ + 1.
-             value = QuoteEscape.Replace(value, @"$1$1\" + "\"");
-            // Next, what if it ends in `\`, it would escape the end quote. So, we need to detect that at the end of the string and perform the same escape
-            // Luckily, we can just use the $ character with detects the end of string in regex
-            value = EndOfStringEscape.Replace(value, @"$1$1");
-            // Finally, wrap it in quotes
-            return $"\"{value}\"";
         }
     }
 }

src/Test/L0/Container/DockerUtilL0.cs

@@ -149,12 +149,13 @@ namespace GitHub.Runner.Common.Tests.Worker.Container
         [Trait("Level", "L0")]
         [Trait("Category", "Worker")]
         [InlineData("", "")]
-        [InlineData("foo", "foo")]
+        [InlineData("HOME alpine:3.8 sh -c id #", "HOME alpine:3.8 sh -c id #")]
-        [InlineData("foo \\ bar", "foo \\ bar")]
+        [InlineData("HOME \"alpine:3.8 sh -c id #", "HOME \\\"alpine:3.8 sh -c id #")]
-        [InlineData("foo \\", "foo \\\\")]
+        [InlineData("HOME \\\"alpine:3.8 sh -c id #", "HOME \\\\\\\"alpine:3.8 sh -c id #")]
-        [InlineData("foo \\\\", "foo \\\\\\\\")]
+        [InlineData("HOME \\\\\"alpine:3.8 sh -c id #", "HOME \\\\\\\\\\\"alpine:3.8 sh -c id #")]
-        [InlineData("foo \\\" bar", "foo \\\\\\\" bar")]
+        [InlineData("HOME \"\"alpine:3.8 sh -c id #", "HOME \\\"\\\"alpine:3.8 sh -c id #")]
-        [InlineData("foo \\\\\" bar", "foo \\\\\\\\\\\" bar")]
+        [InlineData("HOME \\\"\"alpine:3.8 sh -c id #", "HOME \\\\\\\"\\\"alpine:3.8 sh -c id #")]
+        [InlineData("HOME \"\\\"alpine:3.8 sh -c id #", "HOME \\\"\\\\\\\"alpine:3.8 sh -c id #")]
         public void CreateEscapedOption_keyOnly(string input, string escaped)
         {
             var flag = "--example";
@@ -174,11 +175,15 @@ namespace GitHub.Runner.Common.Tests.Worker.Container
         [Theory]
         [Trait("Level", "L0")]
         [Trait("Category", "Worker")]
-        [InlineData("foo", "bar", "foo=bar")]
+        [InlineData("HOME", "", "HOME", "")]
-        [InlineData("foo\\", "bar", "foo\\=bar")]
+        [InlineData("HOME alpine:3.8 sh -c id #", "HOME alpine:3.8 sh -c id #", "HOME alpine:3.8 sh -c id #", "HOME alpine:3.8 sh -c id #")]
-        [InlineData("foo\\", "bar\\", "foo\\=bar\\\\")]
+        [InlineData("HOME \"alpine:3.8 sh -c id #", "HOME \"alpine:3.8 sh -c id #", "HOME \\\"alpine:3.8 sh -c id #", "HOME \\\"alpine:3.8 sh -c id #")]
-        [InlineData("foo \\","bar \\", "foo \\=bar \\\\")]
+        [InlineData("HOME \\\"alpine:3.8 sh -c id #", "HOME \\\"alpine:3.8 sh -c id #", "HOME \\\\\\\"alpine:3.8 sh -c id #", "HOME \\\\\\\"alpine:3.8 sh -c id #")]
-        public void CreateEscapedOption_keyValue(string keyInput, string valueInput, string escapedString)
+        [InlineData("HOME \\\\\"alpine:3.8 sh -c id #", "HOME \\\\\"alpine:3.8 sh -c id #", "HOME \\\\\\\\\\\"alpine:3.8 sh -c id #", "HOME \\\\\\\\\\\"alpine:3.8 sh -c id #")]
+        [InlineData("HOME \"\"alpine:3.8 sh -c id #", "HOME \"\"alpine:3.8 sh -c id #", "HOME \\\"\\\"alpine:3.8 sh -c id #", "HOME \\\"\\\"alpine:3.8 sh -c id #")]
+        [InlineData("HOME \\\"\"alpine:3.8 sh -c id #", "HOME \\\"\"alpine:3.8 sh -c id #", "HOME \\\\\\\"\\\"alpine:3.8 sh -c id #", "HOME \\\\\\\"\\\"alpine:3.8 sh -c id #")]
+        [InlineData("HOME \"\\\"alpine:3.8 sh -c id #", "HOME \"\\\"alpine:3.8 sh -c id #", "HOME \\\"\\\\\\\"alpine:3.8 sh -c id #", "HOME \\\"\\\\\\\"alpine:3.8 sh -c id #")]
+        public void CreateEscapedOption_keyValue(string keyInput, string valueInput, string escapedKey, string escapedValue)
         {
             var flag = "--example";
             var actual = DockerUtil.CreateEscapedOption(flag, keyInput, valueInput);
@@ -189,7 +194,7 @@ namespace GitHub.Runner.Common.Tests.Worker.Container
             }
             else
             {
-                expected = $"{flag} \"{escapedString}\"";
+                expected = $"{flag} \"{escapedKey}={escapedValue}\"";
             }
             Assert.Equal(expected, actual);
         }

src/runnerversion

@@ -1 +1 @@
-2.296.2
+2.296.1