Update releaseVersion

* Fix escaping of docker envs backport

* create as prerelease

* update release notes

Co-authored-by: Nikola Jokic <97525037+nikola-jokic@users.noreply.github.com>

.github/workflows/build.yml

@@ -37,7 +37,7 @@ jobs:
           devScript: ./dev.sh
 
         - runtime: win-x64
-          os: windows-latest
+          os: windows-2019
           devScript: ./dev
 
     runs-on: ${{ matrix.os }}

.github/workflows/release.yml

@@ -72,7 +72,7 @@ jobs:
           devScript: ./dev.sh
 
         - runtime: win-x64
-          os: windows-latest
+          os: windows-2019
           devScript: ./dev
 
     runs-on: ${{ matrix.os }}
@@ -164,6 +164,7 @@ jobs:
         release_name: "v${{ steps.releaseNote.outputs.version }}"
         body: |
           ${{ steps.releaseNote.outputs.note }}
+        prerelease: true
 
     # Upload release assets
     - name: Upload Release Asset (win-x64)

releaseNote.md

@@ -2,15 +2,10 @@
 
 ## Bugs
 
-- Fixed an issue where ephemeral runners deregistered themselves when jobs were not successful (#1384)
-- Fixed an issue where you were not able to un-configure a runner that changed groups (#1359)
-- Disable `stop-commands` command using well known keywords as a token (#1371)
+- Fixed an issue where container environment variables names or values could escape the docker command (#2108)
 
 ## Misc
 
-- Don't retry 422 error codes when downloading actions (#1352)
-- Handle upgrade more smoothly on OSX (#1381)
-
 ## Windows x64
 We recommend configuring the runner in a root folder of the Windows drive (e.g. "C:\actions-runner"). This will help avoid issues related to service identity folder permissions and long file path restrictions on Windows.
 

releaseVersion

@@ -1 +1 @@
-2.283.2
+2.283.4

src/Misc/layoutroot/run.sh

@@ -43,6 +43,21 @@ else
         else
             sleep 5
         fi
+    elif [[ $returnCode == 4 ]]; then
+        if [ ! -x "$(command -v sleep)" ]; then
+            if [ ! -x "$(command -v ping)" ]; then
+                COUNT="0"
+                while [[ $COUNT != 5000 ]]; do
+                    echo "SLEEP" > /dev/null
+                    COUNT=$[$COUNT+1]
+                done
+            else
+                ping -c 5 127.0.0.1 > /dev/null
+            fi
+        else
+            sleep 5
+        fi
+        "$DIR"/bin/Runner.Listener run $*
     else
         exit $returnCode
     fi

src/Runner.Worker/Container/DockerCommandManager.cs

@@ -131,11 +131,11 @@ namespace GitHub.Runner.Worker.Container
             {
                 if (String.IsNullOrEmpty(env.Value))
                 {
-                    dockerOptions.Add($"-e \"{env.Key}\"");
+                    dockerOptions.Add(DockerUtil.CreateEscapedOption("-e", env.Key));
                 }
                 else
                 {
-                    dockerOptions.Add($"-e \"{env.Key}={env.Value.Replace("\"", "\\\"")}\"");
+                    dockerOptions.Add(DockerUtil.CreateEscapedOption("-e", env.Key, env.Value));
                 }
             }
 
@@ -202,7 +202,7 @@ namespace GitHub.Runner.Worker.Container
             {
                 // e.g. -e MY_SECRET maps the value into the exec'ed process without exposing
                 // the value directly in the command
-                dockerOptions.Add($"-e {env.Key}");
+                dockerOptions.Add(DockerUtil.CreateEscapedOption("-e", env.Key));
             }
 
             // Watermark for GitHub Action environment

src/Runner.Worker/Container/DockerUtil.cs

@@ -6,6 +6,9 @@ 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";
@@ -17,7 +20,7 @@ namespace GitHub.Runner.Worker.Container
             string pattern = $"^(?<{targetPort}>\\d+)/(?<{proto}>\\w+) -> (?<{host}>.+):(?<{hostPort}>\\d+)$";
 
             List<PortMapping> portMappings = new List<PortMapping>();
-            foreach(var line in portMappingLines)
+            foreach (var line in portMappingLines)
             {
                 Match m = Regex.Match(line, pattern, RegexOptions.None, TimeSpan.FromSeconds(1));
                 if (m.Success)
@@ -61,5 +64,44 @@ namespace GitHub.Runner.Worker.Container
             }
             return "";
         }
+
+        public static string CreateEscapedOption(string flag, string key)
+        {
+            if (String.IsNullOrEmpty(key))
+            {
+                return "";
+            }
+            return $"{flag} {EscapeString(key)}";
+        }
+
+        public static string CreateEscapedOption(string flag, string key, string value)
+        {
+            if (String.IsNullOrEmpty(key))
+            {
+                return "";
+            }
+            var escapedString = EscapeString($"{key}={value}");
+            return $"{flag} {escapedString}";
+        }
+
+        private static string EscapeString(string value)
+        {
+            if (String.IsNullOrEmpty(value))
+            {
+                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/Runner.Worker/Handlers/StepHost.cs

@@ -188,7 +188,7 @@ namespace GitHub.Runner.Worker.Handlers
             {
                 // e.g. -e MY_SECRET maps the value into the exec'ed process without exposing
                 // the value directly in the command
-                dockerCommandArgs.Add($"-e {env.Key}");
+                dockerCommandArgs.Add(DockerUtil.CreateEscapedOption("-e", env.Key));
             }
             if (!string.IsNullOrEmpty(PrependPath))
             {

src/Test/L0/Container/DockerUtilL0.cs

@@ -144,5 +144,54 @@ namespace GitHub.Runner.Common.Tests.Worker.Container
             var actual = DockerUtil.ParseRegistryHostnameFromImageName(input);
             Assert.Equal(expected, actual);
         }
+
+        [Theory]
+        [Trait("Level", "L0")]
+        [Trait("Category", "Worker")]
+        [InlineData("", "")]
+        [InlineData("foo", "foo")]
+        [InlineData("foo \\ bar", "foo \\ bar")]
+        [InlineData("foo \\", "foo \\\\")]
+        [InlineData("foo \\\\", "foo \\\\\\\\")]
+        [InlineData("foo \\\" bar", "foo \\\\\\\" bar")]
+        [InlineData("foo \\\\\" bar", "foo \\\\\\\\\\\" bar")]
+        public void CreateEscapedOption_keyOnly(string input, string escaped)
+        {
+            var flag = "--example";
+            var actual = DockerUtil.CreateEscapedOption(flag, input);
+            string expected;
+            if (String.IsNullOrEmpty(input))
+            {
+                expected = "";
+            }
+            else
+            {
+                expected = $"{flag} \"{escaped}\"";
+            }
+            Assert.Equal(expected, actual);
+        }
+
+        [Theory]
+        [Trait("Level", "L0")]
+        [Trait("Category", "Worker")]
+        [InlineData("foo", "bar", "foo=bar")]
+        [InlineData("foo\\", "bar", "foo\\=bar")]
+        [InlineData("foo\\", "bar\\", "foo\\=bar\\\\")]
+        [InlineData("foo \\","bar \\", "foo \\=bar \\\\")]
+        public void CreateEscapedOption_keyValue(string keyInput, string valueInput, string escapedString)
+        {
+            var flag = "--example";
+            var actual = DockerUtil.CreateEscapedOption(flag, keyInput, valueInput);
+            string expected;
+            if (String.IsNullOrEmpty(keyInput))
+            {
+                expected = "";
+            }
+            else
+            {
+                expected = $"{flag} \"{escapedString}\"";
+            }
+            Assert.Equal(expected, actual);
+        }
     }
 }

src/runnerversion

@@ -1 +1 @@
-2.283.2
+2.283.4