Update releaseVersion

* set env in ProcessInvoker sanitized (#2280)

* set env in ProcessInvoker sanitized

* Update release notes and runnerversion

* Update runnerversion

---------

Co-authored-by: Stefan Ruvceski <96768603+ruvceskistefan@users.noreply.github.com>

.github/workflows/release.yml

@@ -288,6 +288,7 @@ jobs:
         release_name: "v${{ steps.releaseNote.outputs.version }}"
         body: |
           ${{ steps.releaseNote.outputs.note }}
+        prerelease: true
 
     # Upload release assets (full runner packages)
     - name: Upload Release Asset (win-x64)

releaseNote.md

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

releaseVersion

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

src/Runner.Sdk/ProcessInvoker.cs

@@ -264,7 +264,17 @@ namespace GitHub.Runner.Sdk
             {
                 foreach (KeyValuePair<string, string> kvp in environment)
                 {
+#if OS_WINDOWS
+                    string tempKey = String.IsNullOrWhiteSpace(kvp.Key) ? kvp.Key : kvp.Key.Split('\0')[0];
+                    string tempValue = String.IsNullOrWhiteSpace(kvp.Value) ? kvp.Value : kvp.Value.Split('\0')[0];
+                    if(!String.IsNullOrWhiteSpace(tempKey))
+                    {
+                         _proc.StartInfo.Environment[tempKey] = tempValue;
+                    }
+#else
                     _proc.StartInfo.Environment[kvp.Key] = kvp.Value;
+
+#endif
                 }
             }
 

src/Runner.Worker/ActionCommandManager.cs

@@ -585,8 +585,6 @@ 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/DockerCommandManager.cs

@@ -107,7 +107,6 @@ namespace GitHub.Runner.Worker.Container
         public async Task<string> DockerCreate(IExecutionContext context, ContainerInfo container)
         {
             IList<string> dockerOptions = new List<string>();
-            IDictionary<string, string> environment = new Dictionary<string, string>();
             // OPTIONS
             dockerOptions.Add($"--name {container.ContainerDisplayName}");
             dockerOptions.Add($"--label {DockerInstanceLabel}");
@@ -136,8 +135,7 @@ namespace GitHub.Runner.Worker.Container
                 }
                 else
                 {
-                    environment.Add(env.Key, env.Value);
-                    dockerOptions.Add(DockerUtil.CreateEscapedOption("-e", env.Key));
+                    dockerOptions.Add(DockerUtil.CreateEscapedOption("-e", env.Key, env.Value));
                 }
             }
 
@@ -185,7 +183,7 @@ namespace GitHub.Runner.Worker.Container
             dockerOptions.Add($"{container.ContainerEntryPointArgs}");
 
             var optionsString = string.Join(" ", dockerOptions);
-            List<string> outputStrings = await ExecuteDockerCommandAsync(context, "create", optionsString, environment);
+            List<string> outputStrings = await ExecuteDockerCommandAsync(context, "create", optionsString);
 
             return outputStrings.FirstOrDefault();
         }
@@ -445,11 +443,6 @@ namespace GitHub.Runner.Worker.Container
         }
 
         private async Task<List<string>> ExecuteDockerCommandAsync(IExecutionContext context, string command, string options)
-        {
-            return await ExecuteDockerCommandAsync(context, command, options, null);
-        }
-
-        private async Task<List<string>> ExecuteDockerCommandAsync(IExecutionContext context, string command, string options, IDictionary<string, string> environment)
         {
             string arg = $"{command} {options}".Trim();
             context.Command($"{DockerPath} {arg}");
@@ -477,7 +470,7 @@ namespace GitHub.Runner.Worker.Container
                             workingDirectory: context.GetGitHubContext("workspace"),
                             fileName: DockerPath,
                             arguments: arg,
-                            environment: environment,
+                            environment: null,
                             requireExitCodeZero: true,
                             outputEncoding: null,
                             cancellationToken: CancellationToken.None);

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";
@@ -68,12 +71,37 @@ namespace GitHub.Runner.Worker.Container
             {
                 return "";
             }
-            return $"{flag} \"{EscapeString(key)}\"";
+            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)
         {
-            return value.Replace("\\", "\\\\").Replace("\"", "\\\"");
+            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/ContainerOperationProvider.cs

@@ -134,6 +134,7 @@ namespace GitHub.Runner.Worker
             ArgUtil.NotNull(executionContext, nameof(executionContext));
             ArgUtil.NotNull(container, nameof(container));
             ArgUtil.NotNullOrEmpty(container.ContainerImage, nameof(container.ContainerImage));
+
             Trace.Info($"Container name: {container.ContainerName}");
             Trace.Info($"Container image: {container.ContainerImage}");
             Trace.Info($"Container options: {container.ContainerCreateOptions}");
@@ -196,8 +197,6 @@ namespace GitHub.Runner.Worker
 
             container.ContainerId = await _dockerManager.DockerCreate(executionContext, container);
             ArgUtil.NotNullOrEmpty(container.ContainerId, nameof(container.ContainerId));
-            
-
 
             // Start container
             int startExitCode = await _dockerManager.DockerStart(executionContext, container.ContainerId);
@@ -208,37 +207,20 @@ namespace GitHub.Runner.Worker
 
             try
             {
-                
                 // Make sure container is up and running
                 var psOutputs = await _dockerManager.DockerPS(executionContext, $"--all --filter id={container.ContainerId} --filter status=running --no-trunc --format \"{{{{.ID}}}} {{{{.Status}}}}\"");
-                
                 if (psOutputs.FirstOrDefault(x => !string.IsNullOrEmpty(x))?.StartsWith(container.ContainerId) != true)
                 {
                     // container is not up and running, pull docker log for this container.
                     await _dockerManager.DockerPS(executionContext, $"--all --filter id={container.ContainerId} --no-trunc --format \"{{{{.ID}}}} {{{{.Status}}}}\"");
-                    
-                    
-                    //executionContext.Output("##[group]Getting docker logs..");
-
                     int logsExitCode = await _dockerManager.DockerLogs(executionContext, container.ContainerId);
-                    
                     if (logsExitCode != 0)
                     {
                         executionContext.Warning($"Docker logs fail with exit code {logsExitCode}");
                     }
-                    //executionContext.Output("##[endgroup]");
+
                     executionContext.Warning($"Docker container {container.ContainerId} is not in running state.");
                 }
-                else {
-                        executionContext.Output($"##[group]Container {container.ContainerId} is not running!");
-                        string opt = "--format=\'{{.State.ExitCode}}\'";
-                        await _dockerManager.DockerInspect(executionContext, container.ContainerId, opt);
-                        await _dockerManager.DockerLogs(executionContext, container.ContainerId);
-                        //if exit code is not 0 we can maybe print a message?
-                        //executionContext.Output("Container exit code: ");
-                
-                    executionContext.Output("##[endgroup]");
-                }
             }
             catch (Exception ex)
             {

src/Runner.Worker/JobExtension.cs

@@ -279,7 +279,7 @@ namespace GitHub.Runner.Worker
 
                         preJobSteps.Add(new JobExtensionRunner(runAsync: containerProvider.StartContainersAsync,
                                                                           condition: $"{PipelineTemplateConstants.Success}()",
-                                                                          displayName: "**Initialize containers**",
+                                                                          displayName: "Initialize containers",
                                                                           data: (object)containers));
                     }
 

src/Test/L0/Container/DockerUtilL0.cs

@@ -149,13 +149,12 @@ namespace GitHub.Runner.Common.Tests.Worker.Container
         [Trait("Level", "L0")]
         [Trait("Category", "Worker")]
         [InlineData("", "")]
-        [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 #")]
-        [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 #")]
-        [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 #")]
-        [InlineData("HOME \"\\\"alpine:3.8 sh -c id #", "HOME \\\"\\\\\\\"alpine:3.8 sh -c id #")]
+        [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";
@@ -171,5 +170,28 @@ namespace GitHub.Runner.Common.Tests.Worker.Container
             }
             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/Test/L0/ProcessInvokerL0.cs

@@ -129,7 +129,76 @@ namespace GitHub.Runner.Common.Tests
                 }
             }
         }
+#if OS_WINDOWS
+        [Fact]
+        [Trait("Level", "L0")]
+        [Trait("Category", "Common")]
+        public async Task SetTestEnvWithNullInKey()
+        {
+            using (TestHostContext hc = new(this))
+            {
+                Tracing trace = hc.GetTrace();
 
+                Int32 exitCode = -1;
+                var processInvoker = new ProcessInvokerWrapper();
+                processInvoker.Initialize(hc);
+                var stdout = new List<string>();
+                var stderr = new List<string>();
+                processInvoker.OutputDataReceived += (object sender, ProcessDataReceivedEventArgs e) =>
+                {
+                    trace.Info(e.Data);
+                    stdout.Add(e.Data);
+                };
+                processInvoker.ErrorDataReceived += (object sender, ProcessDataReceivedEventArgs e) =>
+                {
+                    trace.Info(e.Data);
+                    stderr.Add(e.Data);
+                };
+
+                exitCode = await processInvoker.ExecuteAsync("", "cmd.exe", "/c \"echo %TEST%\"",  new Dictionary<string, string>() { { "TEST\0second", "first" } }, CancellationToken.None);
+
+
+                trace.Info("Exit Code: {0}", exitCode);
+                Assert.Equal(0, exitCode);
+                Assert.Equal("first", stdout.First(x => !string.IsNullOrWhiteSpace(x)));
+
+            }
+        }
+
+        [Fact]
+        [Trait("Level", "L0")]
+        [Trait("Category", "Common")]
+        public async Task SetTestEnvWithNullInValue()
+        {
+            using (TestHostContext hc = new(this))
+            {
+                Tracing trace = hc.GetTrace();
+
+                Int32 exitCode = -1;
+                var processInvoker = new ProcessInvokerWrapper();
+                processInvoker.Initialize(hc);
+                var stdout = new List<string>();
+                var stderr = new List<string>();
+                processInvoker.OutputDataReceived += (object sender, ProcessDataReceivedEventArgs e) =>
+                {
+                    trace.Info(e.Data);
+                    stdout.Add(e.Data);
+                };
+                processInvoker.ErrorDataReceived += (object sender, ProcessDataReceivedEventArgs e) =>
+                {
+                    trace.Info(e.Data);
+                    stderr.Add(e.Data);
+                };
+
+                exitCode = await processInvoker.ExecuteAsync("", "cmd.exe", "/c \"echo %TEST%\"",  new Dictionary<string, string>() { { "TEST", "first\0second" } }, CancellationToken.None);
+
+                trace.Info("Exit Code: {0}", exitCode);
+                Assert.Equal(0, exitCode);
+                Assert.Equal("first", stdout.First(x => !string.IsNullOrWhiteSpace(x)));
+
+            }
+        }
+#endif
         [Fact]
         [Trait("Level", "L0")]
         [Trait("Category", "Common")]

src/runnerversion

@@ -1 +1 @@
-2.296.1
+2.296.3