From c3d544914658ed091f8459191afd66cff6f5ea33 Mon Sep 17 00:00:00 2001 From: Nikola Jokic <97525037+nikola-jokic@users.noreply.github.com> Date: Wed, 8 Jun 2022 13:54:23 +0200 Subject: [PATCH] Job hook provider now sets shell name for script handler (#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> --- src/Runner.Sdk/Util/WhichUtil.cs | 1 - src/Runner.Worker/Handlers/ScriptHandler.cs | 34 ++++++++++++++----- .../Handlers/ScriptHandlerHelpers.cs | 21 +++++++----- src/Runner.Worker/JobHookProvider.cs | 6 ++-- 4 files changed, 42 insertions(+), 20 deletions(-) diff --git a/src/Runner.Sdk/Util/WhichUtil.cs b/src/Runner.Sdk/Util/WhichUtil.cs index 598ad0980..1a12a889a 100644 --- 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 index 9844f0525..5530b15c7 100644 --- a/src/Runner.Worker/Handlers/ScriptHandler.cs +++ b/src/Runner.Worker/Handlers/ScriptHandler.cs @@ -202,14 +202,32 @@ namespace GitHub.Runner.Worker.Handlers } 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 @@ 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(scriptFilePath).Replace("\"", "\\\"")}"; + resolvedScriptPath = StepHost.ResolvePathForStepHost(scriptFilePath).Replace("\"", "\\\""); } else { diff --git a/src/Runner.Worker/Handlers/ScriptHandlerHelpers.cs b/src/Runner.Worker/Handlers/ScriptHandlerHelpers.cs index 7d16692a0..e6211d806 100644 --- a/src/Runner.Worker/Handlers/ScriptHandlerHelpers.cs +++ b/src/Runner.Worker/Handlers/ScriptHandlerHelpers.cs @@ -13,8 +13,8 @@ namespace GitHub.Runner.Worker.Handlers ["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 @@ namespace GitHub.Runner.Worker.Handlers } } - 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 index fbca9d0db..11d2c09dc 100644 --- a/src/Runner.Worker/JobHookProvider.cs +++ b/src/Runner.Worker/JobHookProvider.cs @@ -18,8 +18,8 @@ namespace GitHub.Runner.Worker 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 @@ namespace GitHub.Runner.Worker Dictionary inputs = new() { ["path"] = hookData.Path, - ["shell"] = ScriptHandlerHelpers.GetDefaultShellForScript(hookData.Path, Trace, prependPath) + ["shell"] = ScriptHandlerHelpers.GetDefaultShellNameForScript(hookData.Path, Trace, prependPath) }; // Create the handler