From 1ce077fd16bb2c0e7b3a9fbd7d957b8bfb0b9604 Mon Sep 17 00:00:00 2001 From: Salman Muin Kayser Chishti Date: Mon, 22 Sep 2025 15:18:41 +0100 Subject: [PATCH 1/6] path fix --- src/Runner.Worker/Handlers/ScriptHandler.cs | 4 +- .../L0/Worker/Handlers/ScriptHandlerL0.cs | 50 +++++++++++++++++++ 2 files changed, 52 insertions(+), 2 deletions(-) create mode 100644 src/Test/L0/Worker/Handlers/ScriptHandlerL0.cs diff --git a/src/Runner.Worker/Handlers/ScriptHandler.cs b/src/Runner.Worker/Handlers/ScriptHandler.cs index e6fa90a0a11..adc12791650 100644 --- a/src/Runner.Worker/Handlers/ScriptHandler.cs +++ b/src/Runner.Worker/Handlers/ScriptHandler.cs @@ -249,7 +249,7 @@ public async Task RunAsync(ActionRunStage stage) { // We do not not the full path until we know what shell is being used, so that we can determine the file extension scriptFilePath = Path.Combine(tempDirectory, $"{Guid.NewGuid()}{ScriptHandlerHelpers.GetScriptFileExtension(shellCommand)}"); - resolvedScriptPath = StepHost.ResolvePathForStepHost(ExecutionContext, scriptFilePath).Replace("\"", "\\\""); + resolvedScriptPath = $"\"{StepHost.ResolvePathForStepHost(ExecutionContext, scriptFilePath).Replace("\"", "\\\"")}\""; } else { @@ -260,7 +260,7 @@ public async Task RunAsync(ActionRunStage stage) } scriptFilePath = Inputs["path"]; ArgUtil.NotNullOrEmpty(scriptFilePath, "path"); - resolvedScriptPath = Inputs["path"].Replace("\"", "\\\""); + resolvedScriptPath = $"\"{Inputs["path"].Replace("\"", "\\\"")}\""; } // Format arg string with script path diff --git a/src/Test/L0/Worker/Handlers/ScriptHandlerL0.cs b/src/Test/L0/Worker/Handlers/ScriptHandlerL0.cs new file mode 100644 index 00000000000..6273fef9ce3 --- /dev/null +++ b/src/Test/L0/Worker/Handlers/ScriptHandlerL0.cs @@ -0,0 +1,50 @@ +using System; +using System.Collections.Generic; +using System.IO; +using System.Runtime.CompilerServices; +using System.Threading.Tasks; +using GitHub.DistributedTask.Pipelines.ContextData; +using GitHub.DistributedTask.WebApi; +using GitHub.Runner.Common; +using GitHub.Runner.Worker; +using GitHub.Runner.Worker.Handlers; +using Moq; +using Xunit; + +namespace GitHub.Runner.Common.Tests.Worker.Handlers +{ + public sealed class ScriptHandlerL0 + { + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void ScriptPath_WithSpaces_ShouldBeQuoted() + { + // Arrange + using (TestHostContext hc = CreateTestContext()) + { + var scriptHandler = new ScriptHandler(); + scriptHandler.Initialize(hc); + + // Create a mock temp directory path with spaces + var tempPathWithSpaces = "/path with spaces/_temp"; + var scriptPathWithSpaces = Path.Combine(tempPathWithSpaces, "test-script.sh"); + + // Test the logic that our fix addresses + var originalPath = scriptPathWithSpaces.Replace("\"", "\\\""); + var quotedPath = $"\"{scriptPathWithSpaces.Replace("\"", "\\\"")}\""; + + // Assert + Assert.False(originalPath.StartsWith("\""), "Original path should not be quoted"); + Assert.True(quotedPath.StartsWith("\"") && quotedPath.EndsWith("\""), "Fixed path should be properly quoted"); + Assert.Contains("path with spaces", quotedPath, StringComparison.Ordinal); + } + } + + private TestHostContext CreateTestContext([CallerMemberName] string testName = "") + { + var hc = new TestHostContext(this, testName); + return hc; + } + } +} \ No newline at end of file From 8c6bd3e3c1871ade5515fa26ddbb080f4b03722d Mon Sep 17 00:00:00 2001 From: Salman Muin Kayser Chishti Date: Mon, 22 Sep 2025 15:25:07 +0100 Subject: [PATCH 2/6] simplify logic to not use scripthandler --- .../L0/Worker/Handlers/ScriptHandlerL0.cs | 55 +++++++++++-------- 1 file changed, 32 insertions(+), 23 deletions(-) diff --git a/src/Test/L0/Worker/Handlers/ScriptHandlerL0.cs b/src/Test/L0/Worker/Handlers/ScriptHandlerL0.cs index 6273fef9ce3..fd3ce5cf5d0 100644 --- a/src/Test/L0/Worker/Handlers/ScriptHandlerL0.cs +++ b/src/Test/L0/Worker/Handlers/ScriptHandlerL0.cs @@ -20,31 +20,40 @@ public sealed class ScriptHandlerL0 [Trait("Category", "Worker")] public void ScriptPath_WithSpaces_ShouldBeQuoted() { - // Arrange - using (TestHostContext hc = CreateTestContext()) - { - var scriptHandler = new ScriptHandler(); - scriptHandler.Initialize(hc); - - // Create a mock temp directory path with spaces - var tempPathWithSpaces = "/path with spaces/_temp"; - var scriptPathWithSpaces = Path.Combine(tempPathWithSpaces, "test-script.sh"); - - // Test the logic that our fix addresses - var originalPath = scriptPathWithSpaces.Replace("\"", "\\\""); - var quotedPath = $"\"{scriptPathWithSpaces.Replace("\"", "\\\"")}\""; - - // Assert - Assert.False(originalPath.StartsWith("\""), "Original path should not be quoted"); - Assert.True(quotedPath.StartsWith("\"") && quotedPath.EndsWith("\""), "Fixed path should be properly quoted"); - Assert.Contains("path with spaces", quotedPath, StringComparison.Ordinal); - } + // Arrange - Test the path quoting logic that our fix addresses + var tempPathWithSpaces = "/path with spaces/_temp"; + var scriptPathWithSpaces = Path.Combine(tempPathWithSpaces, "test-script.sh"); + + // Test the original (broken) behavior + var originalPath = scriptPathWithSpaces.Replace("\"", "\\\""); + + // Test our fix - properly quoted path + var quotedPath = $"\"{scriptPathWithSpaces.Replace("\"", "\\\"")}\""; + + // Assert + Assert.False(originalPath.StartsWith("\""), "Original path should not be quoted"); + Assert.True(quotedPath.StartsWith("\"") && quotedPath.EndsWith("\""), "Fixed path should be properly quoted"); + Assert.Contains("path with spaces", quotedPath, StringComparison.Ordinal); + + // Verify the specific scenario that was failing + Assert.Equal("\"/path with spaces/_temp/test-script.sh\"", quotedPath); } - - private TestHostContext CreateTestContext([CallerMemberName] string testName = "") + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void ScriptPath_WithQuotes_ShouldEscapeQuotes() { - var hc = new TestHostContext(this, testName); - return hc; + // Arrange - Test paths that contain quotes + var pathWithQuotes = "/path/\"quoted folder\"/script.sh"; + + // Test our fix - properly escape quotes and wrap in quotes + var quotedPath = $"\"{pathWithQuotes.Replace("\"", "\\\"")}\""; + + // Assert + Assert.Equal("\"/path/\\\"quoted folder\\\"/script.sh\"", quotedPath); + Assert.True(quotedPath.StartsWith("\"") && quotedPath.EndsWith("\""), "Path should be wrapped in quotes"); + Assert.Contains("\\\"", quotedPath, StringComparison.Ordinal); } } } \ No newline at end of file From 2b472844d3bc6805df3f084db3191189ad15c8e4 Mon Sep 17 00:00:00 2001 From: Salman Muin Kayser Chishti Date: Mon, 22 Sep 2025 15:50:50 +0100 Subject: [PATCH 3/6] better platform handling --- .../L0/Worker/Handlers/ScriptHandlerL0.cs | 88 ++++++++++++++++++- 1 file changed, 85 insertions(+), 3 deletions(-) diff --git a/src/Test/L0/Worker/Handlers/ScriptHandlerL0.cs b/src/Test/L0/Worker/Handlers/ScriptHandlerL0.cs index fd3ce5cf5d0..7b2396c0cac 100644 --- a/src/Test/L0/Worker/Handlers/ScriptHandlerL0.cs +++ b/src/Test/L0/Worker/Handlers/ScriptHandlerL0.cs @@ -35,8 +35,9 @@ public void ScriptPath_WithSpaces_ShouldBeQuoted() Assert.True(quotedPath.StartsWith("\"") && quotedPath.EndsWith("\""), "Fixed path should be properly quoted"); Assert.Contains("path with spaces", quotedPath, StringComparison.Ordinal); - // Verify the specific scenario that was failing - Assert.Equal("\"/path with spaces/_temp/test-script.sh\"", quotedPath); + // Verify the path is properly quoted (platform-agnostic check) + Assert.True(quotedPath.StartsWith("\"/path with spaces/_temp"), "Path should start with quoted temp directory"); + Assert.True(quotedPath.EndsWith("test-script.sh\""), "Path should end with quoted script name"); } [Fact] @@ -51,9 +52,90 @@ public void ScriptPath_WithQuotes_ShouldEscapeQuotes() var quotedPath = $"\"{pathWithQuotes.Replace("\"", "\\\"")}\""; // Assert - Assert.Equal("\"/path/\\\"quoted folder\\\"/script.sh\"", quotedPath); Assert.True(quotedPath.StartsWith("\"") && quotedPath.EndsWith("\""), "Path should be wrapped in quotes"); Assert.Contains("\\\"", quotedPath, StringComparison.Ordinal); + Assert.Contains("quoted folder", quotedPath, StringComparison.Ordinal); + + // Verify quotes are properly escaped + Assert.Contains("\\\"quoted folder\\\"", quotedPath, StringComparison.Ordinal); + } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void ScriptPath_ActionsRunnerWithSpaces_ShouldBeQuoted() + { + // Arrange - Test the specific real-world scenario that was failing + var runnerPathWithSpaces = "/Users/user/Downloads/actions-runner-osx-arm64-2.328.0 2"; + var tempPath = Path.Combine(runnerPathWithSpaces, "_work", "_temp"); + var scriptPath = Path.Combine(tempPath, "script-guid.sh"); + + // Test our fix + var quotedPath = $"\"{scriptPath.Replace("\"", "\\\"")}\""; + + // Assert + Assert.True(quotedPath.StartsWith("\"") && quotedPath.EndsWith("\""), "Path should be wrapped in quotes"); + Assert.Contains("actions-runner-osx-arm64-2.328.0 2", quotedPath, StringComparison.Ordinal); + Assert.Contains("_work", quotedPath, StringComparison.Ordinal); + Assert.Contains("_temp", quotedPath, StringComparison.Ordinal); + } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void ScriptPath_MultipleSpaces_ShouldBeQuoted() + { + // Arrange - Test paths with multiple spaces + var pathWithMultipleSpaces = "/path/with multiple spaces/script.sh"; + + // Test our fix + var quotedPath = $"\"{pathWithMultipleSpaces.Replace("\"", "\\\"")}\""; + + // Assert + Assert.True(quotedPath.StartsWith("\"") && quotedPath.EndsWith("\""), "Path should be wrapped in quotes"); + Assert.Contains("multiple spaces", quotedPath, StringComparison.Ordinal); + } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void ScriptPath_WithoutSpaces_ShouldStillBeQuoted() + { + // Arrange - Test normal paths without spaces (regression test) + var normalPath = "/home/user/runner/_work/_temp/script.sh"; + + // Test our fix + var quotedPath = $"\"{normalPath.Replace("\"", "\\\"")}\""; + + // Assert + Assert.True(quotedPath.StartsWith("\"") && quotedPath.EndsWith("\""), "Path should be wrapped in quotes"); + Assert.Equal($"\"{normalPath}\"", quotedPath); + } + + [Theory] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + [InlineData("/path with spaces/script.sh")] + [InlineData("/Users/user/Downloads/actions-runner-osx-arm64-2.328.0 2/_work/_temp/guid.sh")] + [InlineData("C:\\Program Files\\GitHub Runner\\script.cmd")] + [InlineData("/path/\"with quotes\"/script.sh")] + [InlineData("/path/with'single'quotes/script.sh")] + public void ScriptPath_VariousScenarios_ShouldBeProperlyQuoted(string inputPath) + { + // Arrange & Act + var quotedPath = $"\"{inputPath.Replace("\"", "\\\"")}\""; + + // Assert + Assert.True(quotedPath.StartsWith("\""), "Path should start with quote"); + Assert.True(quotedPath.EndsWith("\""), "Path should end with quote"); + + // Ensure the original path content is preserved + var unquotedContent = quotedPath.Substring(1, quotedPath.Length - 2); + if (inputPath.Contains("\"")) + { + // If original had quotes, they should be escaped in the result + Assert.Contains("\\\"", unquotedContent); + } } } } \ No newline at end of file From ece418e8c4340dac7628e86e8f8c9e86143f3694 Mon Sep 17 00:00:00 2001 From: Salman Chishti - SalmanMKC <13schishti@gmail.com> Date: Mon, 22 Sep 2025 23:28:07 +0100 Subject: [PATCH 4/6] Refactor Docker command options to use escaped options and add tests for quoting environment variables in scripts --- .../Container/DockerCommandManager.cs | 12 +- .../Handlers/ScriptHandlerHelpers.cs | 38 +++++ src/Runner.Worker/Handlers/StepHost.cs | 3 +- .../L0/Worker/Handlers/ScriptHandlerL0.cs | 137 ++++++++++++++++++ 4 files changed, 183 insertions(+), 7 deletions(-) diff --git a/src/Runner.Worker/Container/DockerCommandManager.cs b/src/Runner.Worker/Container/DockerCommandManager.cs index 41b914a5ee0..f891a2a80da 100644 --- a/src/Runner.Worker/Container/DockerCommandManager.cs +++ b/src/Runner.Worker/Container/DockerCommandManager.cs @@ -111,19 +111,19 @@ public async Task DockerCreate(IExecutionContext context, ContainerInfo { IList dockerOptions = new List(); // OPTIONS - dockerOptions.Add($"--name {container.ContainerDisplayName}"); + dockerOptions.Add(DockerUtil.CreateEscapedOption("--name", container.ContainerDisplayName)); dockerOptions.Add($"--label {DockerInstanceLabel}"); if (!string.IsNullOrEmpty(container.ContainerWorkDirectory)) { - dockerOptions.Add($"--workdir {container.ContainerWorkDirectory}"); + dockerOptions.Add(DockerUtil.CreateEscapedOption("--workdir", container.ContainerWorkDirectory)); } if (!string.IsNullOrEmpty(container.ContainerNetwork)) { - dockerOptions.Add($"--network {container.ContainerNetwork}"); + dockerOptions.Add(DockerUtil.CreateEscapedOption("--network", container.ContainerNetwork)); } if (!string.IsNullOrEmpty(container.ContainerNetworkAlias)) { - dockerOptions.Add($"--network-alias {container.ContainerNetworkAlias}"); + dockerOptions.Add(DockerUtil.CreateEscapedOption("--network-alias", container.ContainerNetworkAlias)); } foreach (var port in container.UserPortMappings) { @@ -195,10 +195,10 @@ public async Task DockerRun(IExecutionContext context, ContainerInfo contai { IList dockerOptions = new List(); // OPTIONS - dockerOptions.Add($"--name {container.ContainerDisplayName}"); + dockerOptions.Add(DockerUtil.CreateEscapedOption("--name", container.ContainerDisplayName)); dockerOptions.Add($"--label {DockerInstanceLabel}"); - dockerOptions.Add($"--workdir {container.ContainerWorkDirectory}"); + dockerOptions.Add(DockerUtil.CreateEscapedOption("--workdir", container.ContainerWorkDirectory)); dockerOptions.Add($"--rm"); foreach (var env in container.ContainerEnvironmentVariables) diff --git a/src/Runner.Worker/Handlers/ScriptHandlerHelpers.cs b/src/Runner.Worker/Handlers/ScriptHandlerHelpers.cs index 6ec953b78d0..4c34936dea2 100644 --- a/src/Runner.Worker/Handlers/ScriptHandlerHelpers.cs +++ b/src/Runner.Worker/Handlers/ScriptHandlerHelpers.cs @@ -2,6 +2,7 @@ using System; using System.Collections.Generic; using System.IO; +using System.Text.RegularExpressions; using GitHub.Runner.Sdk; using GitHub.Runner.Common; using GitHub.Runner.Common.Util; @@ -63,10 +64,47 @@ internal static string FixUpScriptContents(string scriptType, string contents) var append = @"if ((Test-Path -LiteralPath variable:\LASTEXITCODE)) { exit $LASTEXITCODE }"; contents = $"{prepend}{Environment.NewLine}{contents}{Environment.NewLine}{append}"; break; + case "bash": + case "sh": + contents = FixBashEnvironmentVariables(contents); + break; } return contents; } + /// + /// Fixes unquoted environment variables in bash/sh scripts to prevent issues with paths containing spaces. + /// This method quotes environment variables used in shell redirects and command substitutions. + /// + /// The shell script content to fix + /// Fixed shell script content with properly quoted environment variables + private static string FixBashEnvironmentVariables(string contents) + { + if (string.IsNullOrEmpty(contents)) + { + return contents; + } + + // Pattern to match environment variables in shell redirects that aren't already quoted + // This targets patterns like: >> $GITHUB_STEP_SUMMARY, > $GITHUB_OUTPUT, etc. + // but avoids already quoted ones like: >> "$GITHUB_STEP_SUMMARY" or >> '$GITHUB_OUTPUT' + var redirectPattern = new Regex( + @"(\s+(?:>>|>|<|2>>|2>)\s+)(\$[A-Za-z_][A-Za-z0-9_]*)\b(?!\s*['""])", + RegexOptions.Compiled | RegexOptions.Multiline + ); + + // Replace unquoted environment variables in redirects with quoted versions + contents = redirectPattern.Replace(contents, match => + { + var redirectOperator = match.Groups[1].Value; // e.g., " >> " + var envVar = match.Groups[2].Value; // e.g., "$GITHUB_STEP_SUMMARY" + + return $"{redirectOperator}\"{envVar}\""; + }); + + return contents; + } + internal static (string shellCommand, string shellArgs) ParseShellOptionString(string shellOption) { var shellStringParts = shellOption.Split(" ", 2); diff --git a/src/Runner.Worker/Handlers/StepHost.cs b/src/Runner.Worker/Handlers/StepHost.cs index 211009658e4..6c7a77952ae 100644 --- a/src/Runner.Worker/Handlers/StepHost.cs +++ b/src/Runner.Worker/Handlers/StepHost.cs @@ -5,6 +5,7 @@ using System.Threading.Tasks; using GitHub.Runner.Worker.Container; using GitHub.Runner.Common; +using GitHub.Runner.Worker.Container; using GitHub.Runner.Sdk; using System.Linq; using GitHub.Runner.Worker.Container.ContainerHooks; @@ -220,7 +221,7 @@ await containerHookManager.RunScriptStepAsync(context, // [OPTIONS] dockerCommandArgs.Add($"-i"); - dockerCommandArgs.Add($"--workdir {workingDirectory}"); + dockerCommandArgs.Add(DockerUtil.CreateEscapedOption("--workdir", workingDirectory)); foreach (var env in environment) { // e.g. -e MY_SECRET maps the value into the exec'ed process without exposing diff --git a/src/Test/L0/Worker/Handlers/ScriptHandlerL0.cs b/src/Test/L0/Worker/Handlers/ScriptHandlerL0.cs index 7b2396c0cac..13e2b1a566e 100644 --- a/src/Test/L0/Worker/Handlers/ScriptHandlerL0.cs +++ b/src/Test/L0/Worker/Handlers/ScriptHandlerL0.cs @@ -137,5 +137,142 @@ public void ScriptPath_VariousScenarios_ShouldBeProperlyQuoted(string inputPath) Assert.Contains("\\\"", unquotedContent); } } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void FixUpScriptContents_BashEnvironmentVariables_ShouldQuoteRedirects() + { + // Arrange + var scriptContent = @"echo ""## Dependency Status Report"" >> $GITHUB_STEP_SUMMARY +echo ""Generated on: $(date)"" >> $GITHUB_STEP_SUMMARY +echo ""| Component | Status |"" > $GITHUB_OUTPUT +echo ""npm-status=ok"" >> $GITHUB_OUTPUT"; + + // Act + var fixedContent = ScriptHandlerHelpers.FixUpScriptContents("bash", scriptContent); + + // Assert + Assert.Contains(">> \"$GITHUB_STEP_SUMMARY\"", fixedContent); + Assert.Contains("> \"$GITHUB_OUTPUT\"", fixedContent); + Assert.DoesNotContain(">> $GITHUB_STEP_SUMMARY", fixedContent); + Assert.DoesNotContain("> $GITHUB_OUTPUT", fixedContent); + } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void FixUpScriptContents_AlreadyQuotedVariables_ShouldNotDoubleQuote() + { + // Arrange + var scriptContent = @"echo ""test"" >> ""$GITHUB_STEP_SUMMARY"" +echo ""test"" > '$GITHUB_OUTPUT' +echo ""test"" 2>> ""$GITHUB_ENV"""; + + // Act + var fixedContent = ScriptHandlerHelpers.FixUpScriptContents("bash", scriptContent); + + // Assert - Should remain unchanged + Assert.Equal(scriptContent, fixedContent); + Assert.Contains(">> \"$GITHUB_STEP_SUMMARY\"", fixedContent); + Assert.Contains("> '$GITHUB_OUTPUT'", fixedContent); + Assert.Contains("2>> \"$GITHUB_ENV\"", fixedContent); + } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void FixUpScriptContents_ShellRedirectOperators_ShouldHandleAllTypes() + { + // Arrange + var scriptContent = @"echo ""test"" >> $VAR1 +echo ""test"" > $VAR2 +cat < $VAR3 +echo ""test"" 2>> $VAR4 +echo ""test"" 2> $VAR5"; + + // Act + var fixedContent = ScriptHandlerHelpers.FixUpScriptContents("sh", scriptContent); + + // Assert + Assert.Contains(">> \"$VAR1\"", fixedContent); + Assert.Contains("> \"$VAR2\"", fixedContent); + Assert.Contains("< \"$VAR3\"", fixedContent); + Assert.Contains("2>> \"$VAR4\"", fixedContent); + Assert.Contains("2> \"$VAR5\"", fixedContent); + } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void FixUpScriptContents_NonShellTypes_ShouldNotModifyEnvironmentVariables() + { + // Arrange + var scriptContent = @"echo ""test"" >> $GITHUB_STEP_SUMMARY"; + + // Act + var powershellFixed = ScriptHandlerHelpers.FixUpScriptContents("powershell", scriptContent); + var cmdFixed = ScriptHandlerHelpers.FixUpScriptContents("cmd", scriptContent); + var pythonFixed = ScriptHandlerHelpers.FixUpScriptContents("python", scriptContent); + + // Assert - Should not modify environment variables for non-shell types + Assert.Contains(">> $GITHUB_STEP_SUMMARY", powershellFixed); + Assert.Contains(">> $GITHUB_STEP_SUMMARY", cmdFixed); + Assert.Contains(">> $GITHUB_STEP_SUMMARY", pythonFixed); + } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void FixUpScriptContents_ComplexScript_ShouldQuoteOnlyUnquotedRedirects() + { + // Arrange + var scriptContent = @"#!/bin/bash +# This is a test script +echo ""Starting workflow"" >> $GITHUB_STEP_SUMMARY +echo ""Already quoted"" >> ""$GITHUB_OUTPUT"" +export MY_VAR=""$HOME/path with spaces"" +curl -s https://api.github.com/rate_limit > $TEMP_FILE +echo ""Final status"" 2>> $ERROR_LOG +if [ -f ""$GITHUB_ENV"" ]; then + echo ""MY_VAR=test"" >> $GITHUB_ENV +fi"; + + // Act + var fixedContent = ScriptHandlerHelpers.FixUpScriptContents("bash", scriptContent); + + // Assert + Assert.Contains(">> \"$GITHUB_STEP_SUMMARY\"", fixedContent); + Assert.Contains(">> \"$GITHUB_OUTPUT\"", fixedContent); // Should remain quoted + Assert.Contains("> \"$TEMP_FILE\"", fixedContent); + Assert.Contains("2>> \"$ERROR_LOG\"", fixedContent); + Assert.Contains(">> \"$GITHUB_ENV\"", fixedContent); + + // Other parts should remain unchanged + Assert.Contains("#!/bin/bash", fixedContent); + Assert.Contains("# This is a test script", fixedContent); + Assert.Contains("export MY_VAR=\"$HOME/path with spaces\"", fixedContent); + Assert.Contains("if [ -f \"$GITHUB_ENV\" ]; then", fixedContent); + } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void FixUpScriptContents_EnvironmentVariablesInCommands_ShouldNotQuote() + { + // Arrange - Environment variables not in redirects should not be touched + var scriptContent = @"echo $GITHUB_STEP_SUMMARY +cd $HOME +ls -la $TEMP_DIR +if [ ""$MY_VAR"" == ""test"" ]; then + echo ""match"" +fi"; + + // Act + var fixedContent = ScriptHandlerHelpers.FixUpScriptContents("bash", scriptContent); + + // Assert - Should remain unchanged as these are not redirects + Assert.Equal(scriptContent, fixedContent); + } } } \ No newline at end of file From 2bb77fda5391ad6f9caa48eff1ea05d3b742d81c Mon Sep 17 00:00:00 2001 From: Salman Chishti - SalmanMKC <13schishti@gmail.com> Date: Tue, 23 Sep 2025 01:33:09 +0100 Subject: [PATCH 5/6] Fix duplicate using directive in StepHost.cs Remove duplicate 'using GitHub.Runner.Worker.Container;' statement that was causing compilation error CS0105. --- src/Runner.Worker/Handlers/StepHost.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Runner.Worker/Handlers/StepHost.cs b/src/Runner.Worker/Handlers/StepHost.cs index 6c7a77952ae..a487b809db5 100644 --- a/src/Runner.Worker/Handlers/StepHost.cs +++ b/src/Runner.Worker/Handlers/StepHost.cs @@ -5,7 +5,6 @@ using System.Threading.Tasks; using GitHub.Runner.Worker.Container; using GitHub.Runner.Common; -using GitHub.Runner.Worker.Container; using GitHub.Runner.Sdk; using System.Linq; using GitHub.Runner.Worker.Container.ContainerHooks; From f5d4de2c1e1f8b5fa38aafb201b3caf961e1b1d5 Mon Sep 17 00:00:00 2001 From: Salman Chishti - SalmanMKC <13schishti@gmail.com> Date: Tue, 23 Sep 2025 01:39:24 +0100 Subject: [PATCH 6/6] Add InternalsVisibleTo attribute for testing purposes --- src/Runner.Worker/Runner.Worker.csproj | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/Runner.Worker/Runner.Worker.csproj b/src/Runner.Worker/Runner.Worker.csproj index 4470920e10c..076a6e7f3e6 100644 --- a/src/Runner.Worker/Runner.Worker.csproj +++ b/src/Runner.Worker/Runner.Worker.csproj @@ -12,6 +12,12 @@ true + + + <_Parameter1>Test + + +