Job hook provider now sets shell name for script handler (actions#1826)

* Job hook provider now sets shell name for script handler * fixed script handler and job hook provider to work with the name without fail * returned used import by osx * fixed order of imports * added quotes around resolved script path allowing space in script path * added quotes around bash and sh _defaultArguments * Changed double quotes to single quotes in sh -e Co-authored-by: Ferenc Hammerl <31069338+fhammerl@users.noreply.github.com> * Changed double quotes to single quotes in bash Co-authored-by: Ferenc Hammerl <31069338+fhammerl@users.noreply.github.com> Co-authored-by: Ferenc Hammerl <31069338+fhammerl@users.noreply.github.com>
nikola-jokic · Jun 8, 2022 · c3d5449 · c3d5449
1 parent 9c5300b
commit c3d5449
Show file tree

Hide file tree

Showing 4 changed files with 42 additions and 20 deletions.
diff --git a/src/Runner.Sdk/Util/WhichUtil.cs b/src/Runner.Sdk/Util/WhichUtil.cs
@@ -1,7 +1,6 @@
 using System;
 using System.IO;
 using System.Linq;
-using GitHub.Runner.Sdk;
 
 namespace GitHub.Runner.Sdk
 {

diff --git a/src/Runner.Worker/Handlers/ScriptHandler.cs b/src/Runner.Worker/Handlers/ScriptHandler.cs
@@ -202,14 +202,32 @@ public async Task RunAsync(ActionRunStage stage)
             }
             else
             {
-                var parsed = ScriptHandlerHelpers.ParseShellOptionString(shell);
-                shellCommand = parsed.shellCommand;
-                // For non-ContainerStepHost, the command must be located on the host by Which
-                commandPath = WhichUtil.Which(parsed.shellCommand, !isContainerStepHost, Trace, prependPath);
-                argFormat = $"{parsed.shellArgs}".TrimStart();
-                if (string.IsNullOrEmpty(argFormat))
+                // For these shells, we want to use system binaries
+                var systemShells = new string[] { "bash", "sh", "powershell", "pwsh" };
+                if (!IsActionStep && systemShells.Contains(shell))
                 {
-                    argFormat = ScriptHandlerHelpers.GetScriptArgumentsFormat(shellCommand);
+                    shellCommand = shell;
+                    commandPath = WhichUtil.Which(shell, !isContainerStepHost, Trace, prependPath);
+                    if (shell == "bash")
+                    {
+                        argFormat = ScriptHandlerHelpers.GetScriptArgumentsFormat("sh");
+                    }
+                    else
+                    {
+                        argFormat = ScriptHandlerHelpers.GetScriptArgumentsFormat(shell);
+                    }
+                }
+                else
+                {
+                    var parsed = ScriptHandlerHelpers.ParseShellOptionString(shell);
+                    shellCommand = parsed.shellCommand;
+                    // For non-ContainerStepHost, the command must be located on the host by Which
+                    commandPath = WhichUtil.Which(parsed.shellCommand, !isContainerStepHost, Trace, prependPath);
+                    argFormat = $"{parsed.shellArgs}".TrimStart();
+                    if (string.IsNullOrEmpty(argFormat))
+                    {
+                        argFormat = ScriptHandlerHelpers.GetScriptArgumentsFormat(shellCommand);
+                    }
                 }
             }
 
@@ -229,7 +247,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(scriptFilePath).Replace("\"", "\\\"")}";
+                resolvedScriptPath = StepHost.ResolvePathForStepHost(scriptFilePath).Replace("\"", "\\\"");
             }
             else
             {

diff --git a/src/Runner.Worker/Handlers/ScriptHandlerHelpers.cs b/src/Runner.Worker/Handlers/ScriptHandlerHelpers.cs
@@ -13,8 +13,8 @@ internal class ScriptHandlerHelpers
             ["cmd"] = "/D /E:ON /V:OFF /S /C \"CALL \"{0}\"\"",
             ["pwsh"] = "-command \". '{0}'\"",
             ["powershell"] = "-command \". '{0}'\"",
-            ["bash"] = "--noprofile --norc -e -o pipefail {0}",
-            ["sh"] = "-e {0}",
+            ["bash"] = "--noprofile --norc -e -o pipefail '{0}'",
+            ["sh"] = "-e '{0}'",
             ["python"] = "{0}"
         };
 
@@ -82,18 +82,23 @@ internal static (string shellCommand, string shellArgs) ParseShellOptionString(s
             }
         }
 
-        internal static string GetDefaultShellForScript(string path, Common.Tracing trace, string prependPath)
+        internal static string GetDefaultShellNameForScript(string path, Common.Tracing trace, string prependPath)
         {
-            var format = "{0} {1}";
             switch (Path.GetExtension(path))
             {
                 case ".sh":
                     // use 'sh' args but prefer bash
-                    var pathToShell = WhichUtil.Which("bash", false, trace, prependPath) ?? WhichUtil.Which("sh", true, trace, prependPath);
-                    return string.Format(format, pathToShell, _defaultArguments["sh"]);
+                    if (WhichUtil.Which("bash", false, trace, prependPath) != null)
+                    {
+                        return "bash";
+                    }
+                    return "sh";
                 case ".ps1":
-                    var pathToPowershell = WhichUtil.Which("pwsh", false, trace, prependPath) ?? WhichUtil.Which("powershell", true, trace, prependPath);
-                    return string.Format(format, pathToPowershell, _defaultArguments["powershell"]);
+                    if (WhichUtil.Which("pwsh", false, trace, prependPath) != null)
+                    {
+                        return "pwsh";
+                    }
+                    return "powershell";
                 default:
                     throw new ArgumentException($"{path} is not a valid path to a script. Make sure it ends in '.sh' or '.ps1'.");
             }

diff --git a/src/Runner.Worker/JobHookProvider.cs b/src/Runner.Worker/JobHookProvider.cs
@@ -18,8 +18,8 @@ public interface IJobHookProvider : IRunnerService
 
     public class JobHookData
     {
-        public string Path {get; private set;}
-        public ActionRunStage Stage {get; private set;}
+        public string Path { get; private set; }
+        public ActionRunStage Stage { get; private set; }
 
         public JobHookData(ActionRunStage stage, string path)
         {
@@ -60,7 +60,7 @@ public async Task RunHook(IExecutionContext executionContext, object data)
             Dictionary<string, string> inputs = new()
             {
                 ["path"] = hookData.Path,
-                ["shell"] = ScriptHandlerHelpers.GetDefaultShellForScript(hookData.Path, Trace, prependPath)
+                ["shell"] = ScriptHandlerHelpers.GetDefaultShellNameForScript(hookData.Path, Trace, prependPath)
             };
 
             // Create the handler