Compare commits

..

6 Commits

Author SHA1 Message Date
Salman Chishti - SalmanMKC
f5d4de2c1e Add InternalsVisibleTo attribute for testing purposes 2025-09-23 01:39:24 +01:00
Salman Chishti - SalmanMKC
2bb77fda53 Fix duplicate using directive in StepHost.cs
Remove duplicate 'using GitHub.Runner.Worker.Container;' statement that was causing compilation error CS0105.
2025-09-23 01:33:09 +01:00
Salman Chishti - SalmanMKC
ece418e8c4 Refactor Docker command options to use escaped options and add tests for quoting environment variables in scripts 2025-09-22 23:28:07 +01:00
Salman Muin Kayser Chishti
2b472844d3 better platform handling 2025-09-22 15:50:50 +01:00
Salman Muin Kayser Chishti
8c6bd3e3c1 simplify logic to not use scripthandler 2025-09-22 15:25:07 +01:00
Salman Muin Kayser Chishti
1ce077fd16 path fix 2025-09-22 15:18:41 +01:00
8 changed files with 347 additions and 90 deletions

View File

@@ -32,47 +32,20 @@ jobs:
echo "Verifying availability in alpine_nodejs..."
ALPINE_RELEASES=$(curl -s https://api.github.com/repos/actions/alpine_nodejs/releases | jq -r '.[].tag_name')
if ! echo "$ALPINE_RELEASES" | grep -q "^v$LATEST_NODE20$"; then
if ! echo "$ALPINE_RELEASES" | grep -q "^node20-$LATEST_NODE20$"; then
echo "::warning title=Node 20 Fallback::Node 20 version $LATEST_NODE20 not found in alpine_nodejs releases, using fallback"
# Fall back to latest available alpine_nodejs v20 release
LATEST_NODE20=$(echo "$ALPINE_RELEASES" | grep "^v20\." | head -1 | sed 's/^v//')
LATEST_NODE20=$(echo "$ALPINE_RELEASES" | grep "^node20-" | head -1 | sed 's/^node20-//')
echo "Using latest available alpine_nodejs Node 20: $LATEST_NODE20"
fi
if ! echo "$ALPINE_RELEASES" | grep -q "^v$LATEST_NODE24$"; then
if ! echo "$ALPINE_RELEASES" | grep -q "^node24-$LATEST_NODE24$"; then
echo "::warning title=Node 24 Fallback::Node 24 version $LATEST_NODE24 not found in alpine_nodejs releases, using fallback"
# Fall back to latest available alpine_nodejs v24 release
LATEST_NODE24=$(echo "$ALPINE_RELEASES" | grep "^v24\." | head -1 | sed 's/^v//')
LATEST_NODE24=$(echo "$ALPINE_RELEASES" | grep "^node24-" | head -1 | sed 's/^node24-//')
echo "Using latest available alpine_nodejs Node 24: $LATEST_NODE24"
fi
# Validate that we have non-empty version numbers
if [ -z "$LATEST_NODE20" ] || [ "$LATEST_NODE20" = "" ]; then
echo "::error title=Invalid Node 20 Version::Failed to determine valid Node 20 version. Got: '$LATEST_NODE20'"
echo "Available alpine_nodejs releases:"
echo "$ALPINE_RELEASES" | head -10
exit 1
fi
if [ -z "$LATEST_NODE24" ] || [ "$LATEST_NODE24" = "" ]; then
echo "::error title=Invalid Node 24 Version::Failed to determine valid Node 24 version. Got: '$LATEST_NODE24'"
echo "Available alpine_nodejs releases:"
echo "$ALPINE_RELEASES" | head -10
exit 1
fi
# Additional validation: ensure versions match expected format (x.y.z)
if ! echo "$LATEST_NODE20" | grep -E '^[0-9]+\.[0-9]+\.[0-9]+$'; then
echo "::error title=Invalid Node 20 Format::Node 20 version '$LATEST_NODE20' does not match expected format (x.y.z)"
exit 1
fi
if ! echo "$LATEST_NODE24" | grep -E '^[0-9]+\.[0-9]+\.[0-9]+$'; then
echo "::error title=Invalid Node 24 Format::Node 24 version '$LATEST_NODE24' does not match expected format (x.y.z)"
exit 1
fi
echo "✅ Validated Node versions: 20=$LATEST_NODE20, 24=$LATEST_NODE24"
echo "latest_node20=$LATEST_NODE20" >> $GITHUB_OUTPUT
echo "latest_node24=$LATEST_NODE24" >> $GITHUB_OUTPUT
@@ -109,50 +82,13 @@ jobs:
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
run: |
# Final validation before making changes
NODE20_VERSION="${{ steps.node-versions.outputs.latest_node20 }}"
NODE24_VERSION="${{ steps.node-versions.outputs.latest_node24 }}"
echo "Final validation of versions before PR creation:"
echo "Node 20: '$NODE20_VERSION'"
echo "Node 24: '$NODE24_VERSION'"
# Validate versions are not empty and match expected format
if [ -z "$NODE20_VERSION" ] || ! echo "$NODE20_VERSION" | grep -E '^[0-9]+\.[0-9]+\.[0-9]+$'; then
echo "::error title=Invalid Node 20 Version::Refusing to create PR with invalid Node 20 version: '$NODE20_VERSION'"
exit 1
fi
if [ -z "$NODE24_VERSION" ] || ! echo "$NODE24_VERSION" | grep -E '^[0-9]+\.[0-9]+\.[0-9]+$'; then
echo "::error title=Invalid Node 24 Version::Refusing to create PR with invalid Node 24 version: '$NODE24_VERSION'"
exit 1
fi
echo "✅ All versions validated successfully"
# Update the files
if [ "${{ steps.node-versions.outputs.needs_update20 }}" == "true" ]; then
sed -i 's/NODE20_VERSION="[^"]*"/NODE20_VERSION="'"$NODE20_VERSION"'"/' src/Misc/externals.sh
sed -i 's/NODE20_VERSION="[^"]*"/NODE20_VERSION="${{ steps.node-versions.outputs.latest_node20 }}"/' src/Misc/externals.sh
fi
if [ "${{ steps.node-versions.outputs.needs_update24 }}" == "true" ]; then
sed -i 's/NODE24_VERSION="[^"]*"/NODE24_VERSION="'"$NODE24_VERSION"'"/' src/Misc/externals.sh
fi
# Verify the changes were applied correctly
echo "Verifying changes in externals.sh:"
grep "NODE20_VERSION=" src/Misc/externals.sh
grep "NODE24_VERSION=" src/Misc/externals.sh
# Ensure we actually have valid versions in the file
UPDATED_NODE20=$(grep "NODE20_VERSION=" src/Misc/externals.sh | cut -d'"' -f2)
UPDATED_NODE24=$(grep "NODE24_VERSION=" src/Misc/externals.sh | cut -d'"' -f2)
if [ -z "$UPDATED_NODE20" ] || [ -z "$UPDATED_NODE24" ]; then
echo "::error title=Update Failed::Failed to properly update externals.sh"
echo "Updated Node 20: '$UPDATED_NODE20'"
echo "Updated Node 24: '$UPDATED_NODE24'"
exit 1
sed -i 's/NODE24_VERSION="[^"]*"/NODE24_VERSION="${{ steps.node-versions.outputs.latest_node24 }}"/' src/Misc/externals.sh
fi
# Configure git
@@ -162,15 +98,15 @@ jobs:
# Create branch and commit changes
branch_name="chore/update-node"
git checkout -b "$branch_name"
git commit -a -m "chore: update Node versions (20: $NODE20_VERSION, 24: $NODE24_VERSION)"
git commit -a -m "chore: update Node versions (20: ${{ steps.node-versions.outputs.latest_node20 }}, 24: ${{ steps.node-versions.outputs.latest_node24 }})"
git push --force origin "$branch_name"
# Create PR body using here-doc for proper formatting
cat > pr_body.txt << EOF
cat > pr_body.txt << 'EOF'
Automated Node.js version update:
- Node 20: ${{ steps.node-versions.outputs.current_node20 }} → $NODE20_VERSION
- Node 24: ${{ steps.node-versions.outputs.current_node24 }} → $NODE24_VERSION
- Node 20: ${{ steps.node-versions.outputs.current_node20 }} → ${{ steps.node-versions.outputs.latest_node20 }}
- Node 24: ${{ steps.node-versions.outputs.current_node24 }} → ${{ steps.node-versions.outputs.latest_node24 }}
This update ensures we're using the latest stable Node.js versions for security and performance improvements.

View File

@@ -1815,11 +1815,10 @@
}
},
"node_modules/eslint-plugin-github/node_modules/brace-expansion": {
"version": "2.0.2",
"resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-2.0.2.tgz",
"integrity": "sha512-Jt0vHyM+jmUBqojB7E1NIYadt0vI0Qxjxd2TErW94wDz+E2LAm5vKMXXwg6ZZBTHPuUlDgQHKXvjGBdfcF1ZDQ==",
"version": "2.0.1",
"resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-2.0.1.tgz",
"integrity": "sha512-XnAIvQ8eM+kC6aULx6wuQiwVsnzsi9d3WxzV3FpWTGA19F621kwdbsAcFKXgKUHZWsy+mY6iL1sHTxWEFCytDA==",
"dev": true,
"license": "MIT",
"dependencies": {
"balanced-match": "^1.0.0"
}
@@ -5905,9 +5904,9 @@
}
},
"brace-expansion": {
"version": "2.0.2",
"resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-2.0.2.tgz",
"integrity": "sha512-Jt0vHyM+jmUBqojB7E1NIYadt0vI0Qxjxd2TErW94wDz+E2LAm5vKMXXwg6ZZBTHPuUlDgQHKXvjGBdfcF1ZDQ==",
"version": "2.0.1",
"resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-2.0.1.tgz",
"integrity": "sha512-XnAIvQ8eM+kC6aULx6wuQiwVsnzsi9d3WxzV3FpWTGA19F621kwdbsAcFKXgKUHZWsy+mY6iL1sHTxWEFCytDA==",
"dev": true,
"requires": {
"balanced-match": "^1.0.0"

View File

@@ -111,19 +111,19 @@ namespace GitHub.Runner.Worker.Container
{
IList<string> dockerOptions = new List<string>();
// 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 @@ namespace GitHub.Runner.Worker.Container
{
IList<string> dockerOptions = new List<string>();
// 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)

View File

@@ -249,7 +249,7 @@ namespace GitHub.Runner.Worker.Handlers
{
// 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 @@ namespace GitHub.Runner.Worker.Handlers
}
scriptFilePath = Inputs["path"];
ArgUtil.NotNullOrEmpty(scriptFilePath, "path");
resolvedScriptPath = Inputs["path"].Replace("\"", "\\\"");
resolvedScriptPath = $"\"{Inputs["path"].Replace("\"", "\\\"")}\"";
}
// Format arg string with script path

View File

@@ -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 @@ namespace GitHub.Runner.Worker.Handlers
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;
}
/// <summary>
/// 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.
/// </summary>
/// <param name="contents">The shell script content to fix</param>
/// <returns>Fixed shell script content with properly quoted environment variables</returns>
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);

View File

@@ -220,7 +220,7 @@ namespace GitHub.Runner.Worker.Handlers
// [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

View File

@@ -12,6 +12,12 @@
<PublishReadyToRunComposite>true</PublishReadyToRunComposite>
</PropertyGroup>
<ItemGroup>
<AssemblyAttribute Include="System.Runtime.CompilerServices.InternalsVisibleTo">
<_Parameter1>Test</_Parameter1>
</AssemblyAttribute>
</ItemGroup>
<ItemGroup>
<ProjectReference Include="..\Sdk\Sdk.csproj" />
<ProjectReference Include="..\Runner.Common\Runner.Common.csproj" />

View File

@@ -0,0 +1,278 @@
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 - 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 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]
[Trait("Level", "L0")]
[Trait("Category", "Worker")]
public void ScriptPath_WithQuotes_ShouldEscapeQuotes()
{
// 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.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);
}
}
[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);
}
}
}