From 77971853e2dd7614362d9e637cb9826c1e8c7420 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 2 Dec 2025 00:09:10 +0000 Subject: [PATCH 1/4] Initial plan From a03ae458beb19590980657cf883d72def4fe8417 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 2 Dec 2025 00:20:06 +0000 Subject: [PATCH 2/4] Add CommandArguments property to Exec task with shell escaping Co-authored-by: baronfel <573979+baronfel@users.noreply.github.com> --- src/Tasks.UnitTests/Exec_Tests.cs | 128 ++++++++++++++++++++++++ src/Tasks/Exec.cs | 158 ++++++++++++++++++++++++++++-- 2 files changed, 278 insertions(+), 8 deletions(-) diff --git a/src/Tasks.UnitTests/Exec_Tests.cs b/src/Tasks.UnitTests/Exec_Tests.cs index 2f0f9e1ace3..d2714c66cc5 100644 --- a/src/Tasks.UnitTests/Exec_Tests.cs +++ b/src/Tasks.UnitTests/Exec_Tests.cs @@ -1058,6 +1058,134 @@ public void ConsoleOutputDoesNotTrimLeadingWhitespace() exec.ConsoleOutput[0].ItemSpec.ShouldBe(lineWithLeadingWhitespace); } } + + [Fact] + public void CommandArgumentsBasic() + { + Exec exec = PrepareExec("echo"); + exec.CommandArguments = new[] { "Hello", "World" }; + bool result = exec.Execute(); + + result.ShouldBeTrue(); + ((MockEngine)exec.BuildEngine).AssertLogContains("Hello"); + ((MockEngine)exec.BuildEngine).AssertLogContains("World"); + } + + [Fact] + public void CommandArgumentsWithSpaces() + { + Exec exec = PrepareExec("echo"); + exec.CommandArguments = new[] { "Hello World", "Test Message" }; + bool result = exec.Execute(); + + result.ShouldBeTrue(); + ((MockEngine)exec.BuildEngine).AssertLogContains("Hello World"); + ((MockEngine)exec.BuildEngine).AssertLogContains("Test Message"); + } + + [Fact] + public void CommandArgumentsWithSpecialCharacters() + { + string arg = NativeMethodsShared.IsWindows ? "test&echo" : "test;echo"; + Exec exec = PrepareExec("echo"); + exec.CommandArguments = new[] { arg }; + bool result = exec.Execute(); + + result.ShouldBeTrue(); + ((MockEngine)exec.BuildEngine).AssertLogContains(arg); + } + + [Fact] + public void CommandArgumentsWithQuotes() + { + Exec exec = PrepareExec("echo"); + exec.CommandArguments = new[] { "test\"quote" }; + bool result = exec.Execute(); + + result.ShouldBeTrue(); + ((MockEngine)exec.BuildEngine).AssertLogContains("quote"); + } + + [Fact] + public void CommandArgumentsOnly() + { + Exec exec = PrepareExec(""); + exec.CommandArguments = new[] { "echo", "Hello" }; + bool result = exec.Execute(); + + result.ShouldBeTrue(); + ((MockEngine)exec.BuildEngine).AssertLogContains("Hello"); + } + + [Fact] + public void CommandArgumentsEmpty() + { + Exec exec = PrepareExec("echo"); + exec.CommandArguments = new[] { "" }; + bool result = exec.Execute(); + + result.ShouldBeTrue(); + } + + [Fact] + public void CommandArgumentsWithPercent() + { + if (NativeMethodsShared.IsWindows) + { + Exec exec = PrepareExec("echo"); + exec.CommandArguments = new[] { "test%PATH%" }; + bool result = exec.Execute(); + + result.ShouldBeTrue(); + ((MockEngine)exec.BuildEngine).AssertLogContains("test%PATH%"); + } + } + + [UnixOnlyFact] + public void CommandArgumentsWithSingleQuoteUnix() + { + Exec exec = PrepareExec("echo"); + exec.CommandArguments = new[] { "test'quote" }; + bool result = exec.Execute(); + + result.ShouldBeTrue(); + ((MockEngine)exec.BuildEngine).AssertLogContains("test'quote"); + } + + [Fact] + public void CommandArgumentsCombinedWithCommand() + { + Exec exec = PrepareExec("echo Start"); + exec.CommandArguments = new[] { "Middle", "End" }; + bool result = exec.Execute(); + + result.ShouldBeTrue(); + ((MockEngine)exec.BuildEngine).AssertLogContains("Start"); + ((MockEngine)exec.BuildEngine).AssertLogContains("Middle"); + ((MockEngine)exec.BuildEngine).AssertLogContains("End"); + } + + [WindowsOnlyFact] + public void CommandArgumentsWithParentheses() + { + Exec exec = PrepareExec("echo"); + exec.CommandArguments = new[] { "test(with)parens" }; + bool result = exec.Execute(); + + result.ShouldBeTrue(); + ((MockEngine)exec.BuildEngine).AssertLogContains("test(with)parens"); + } + + [Fact] + public void NoCommandNoArgumentsShouldFail() + { + Exec exec = PrepareExec(""); + exec.CommandArguments = Array.Empty(); + bool result = exec.Execute(); + + result.ShouldBeFalse(); + ((MockEngine)exec.BuildEngine).AssertLogContains("MSB3072"); + } } internal sealed class ExecWrapper : Exec diff --git a/src/Tasks/Exec.cs b/src/Tasks/Exec.cs index 4daa47cf647..aac348a2f89 100644 --- a/src/Tasks/Exec.cs +++ b/src/Tasks/Exec.cs @@ -58,6 +58,7 @@ public Exec() private Encoding _standardErrorEncoding; private Encoding _standardOutputEncoding; private string _command; + private string[] _commandArguments; // '^' before _any_ character escapes that character, don't escape it. private static readonly char[] _charactersToEscape = { '(', ')', '=', ';', '!', ',', '&', ' ' }; @@ -66,7 +67,6 @@ public Exec() #region Properties - [Required] public string Command { get => _command; @@ -82,6 +82,16 @@ public string Command public string WorkingDirectory { get; set; } + /// + /// Array of command-line arguments to append to the Command. + /// Each argument will be properly escaped for the target shell. + /// + public string[] CommandArguments + { + get => _commandArguments; + set => _commandArguments = value; + } + public bool IgnoreExitCode { get; set; } /// @@ -193,12 +203,110 @@ public ITaskItem[] Outputs #endregion #region Methods + + /// + /// Escapes an argument for Windows cmd.exe. + /// + private static string EscapeArgumentForWindows(string argument) + { + if (string.IsNullOrEmpty(argument)) + { + return "\"\""; + } + + // Check if the argument contains special characters that need quoting + bool needsQuoting = false; + foreach (char c in argument) + { + if (c == ' ' || c == '\t' || c == '"' || c == '&' || c == '|' || c == '<' || c == '>' || + c == '^' || c == '%' || c == '(' || c == ')' || c == '!' || c == '=' || c == ';' || c == ',') + { + needsQuoting = true; + break; + } + } + + if (!needsQuoting) + { + return argument; + } + + StringBuilder escaped = new StringBuilder(); + escaped.Append('"'); + + for (int i = 0; i < argument.Length; i++) + { + char c = argument[i]; + + if (c == '"') + { + // Escape double quotes by doubling them + escaped.Append("\"\""); + } + else if (c == '%') + { + // Escape percent signs + escaped.Append("%%"); + } + else + { + escaped.Append(c); + } + } + + escaped.Append('"'); + return escaped.ToString(); + } + + /// + /// Escapes an argument for Unix sh. + /// + private static string EscapeArgumentForUnix(string argument) + { + if (string.IsNullOrEmpty(argument)) + { + return "''"; + } + + // Use single quotes for Unix, which preserve everything literally except single quotes + // For single quotes within the string, we end the quoted string, add an escaped single quote, and start a new quoted string + if (argument.IndexOf('\'') == -1) + { + return "'" + argument + "'"; + } + + // If there are single quotes, we need to handle them specially + StringBuilder escaped = new StringBuilder(); + escaped.Append('\''); + + foreach (char c in argument) + { + if (c == '\'') + { + // End current quote, add escaped quote, start new quote + escaped.Append("'\\''"); + } + else + { + escaped.Append(c); + } + } + + escaped.Append('\''); + return escaped.ToString(); + } + /// /// Write out a temporary batch file with the user-specified command in it. /// private void CreateTemporaryBatchFile() { - var encoding = EncodingUtilities.BatchFileEncoding(Command + WorkingDirectory, UseUtf8Encoding); + string contentForEncoding = Command + WorkingDirectory; + if (CommandArguments != null) + { + contentForEncoding += string.Join(" ", CommandArguments); + } + var encoding = EncodingUtilities.BatchFileEncoding(contentForEncoding, UseUtf8Encoding); // Temporary file with the extension .Exec.bat _batchFile = FileUtilities.GetTemporaryFileName(".exec.cmd"); @@ -255,7 +363,30 @@ private void CreateTemporaryBatchFile() sw.WriteLine("#!/bin/sh"); } - sw.WriteLine(Command); + // Write the command if provided + if (!string.IsNullOrWhiteSpace(Command)) + { + sw.Write(Command); + } + + // Append command arguments if provided + if (CommandArguments != null && CommandArguments.Length > 0) + { + foreach (string arg in CommandArguments) + { + sw.Write(' '); + if (NativeMethodsShared.IsUnixLike) + { + sw.Write(EscapeArgumentForUnix(arg)); + } + else + { + sw.Write(EscapeArgumentForWindows(arg)); + } + } + } + + sw.WriteLine(); // End the command line if (!NativeMethodsShared.IsUnixLike) { @@ -311,18 +442,24 @@ protected override int ExecuteTool(string pathToTool, string responseFileCommand /// protected override bool HandleTaskExecutionErrors() { + string fullCommand = Command ?? string.Empty; + if (CommandArguments != null && CommandArguments.Length > 0) + { + fullCommand += " " + string.Join(" ", CommandArguments); + } + if (IgnoreExitCode) { // Don't log when EchoOff and IgnoreExitCode. if (!EchoOff) { - Log.LogMessageFromResources(MessageImportance.Normal, "Exec.CommandFailedNoErrorCode", Command, ExitCode); + Log.LogMessageFromResources(MessageImportance.Normal, "Exec.CommandFailedNoErrorCode", fullCommand, ExitCode); } return true; } // Don't emit expanded form of Command when EchoOff is set. - string commandForLog = EchoOff ? "..." : Command; + string commandForLog = EchoOff ? "..." : fullCommand; if (ExitCode == NativeMethods.SE_ERR_ACCESSDENIED) { Log.LogErrorWithCodeFromResources("Exec.CommandFailedAccessDenied", commandForLog, ExitCode); @@ -357,7 +494,12 @@ protected override void LogToolCommand(string message) // Dont print the command line if Echo is Off. if (!EchoOff) { - base.LogToolCommand(Command); + string commandToLog = Command ?? string.Empty; + if (CommandArguments != null && CommandArguments.Length > 0) + { + commandToLog += " " + string.Join(" ", CommandArguments); + } + base.LogToolCommand(commandToLog); } } @@ -447,8 +589,8 @@ protected override bool ValidateParameters() return false; } - // Make sure that at least the Command property was set - if (Command.Trim().Length == 0) + // Make sure that at least the Command property was set, unless CommandArguments is provided + if (string.IsNullOrWhiteSpace(Command) && (CommandArguments == null || CommandArguments.Length == 0)) { Log.LogErrorWithCodeFromResources("Exec.MissingCommandError"); return false; From aaa82461d385222ce3ff087eb7b1422f0ad2f878 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 2 Dec 2025 00:26:12 +0000 Subject: [PATCH 3/4] Handle CommandArguments-only case where first arg is the command Co-authored-by: baronfel <573979+baronfel@users.noreply.github.com> --- src/Tasks/Exec.cs | 31 +++++++++++++++++++++++++------ 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/src/Tasks/Exec.cs b/src/Tasks/Exec.cs index aac348a2f89..90d60254e65 100644 --- a/src/Tasks/Exec.cs +++ b/src/Tasks/Exec.cs @@ -367,21 +367,40 @@ private void CreateTemporaryBatchFile() if (!string.IsNullOrWhiteSpace(Command)) { sw.Write(Command); - } - // Append command arguments if provided - if (CommandArguments != null && CommandArguments.Length > 0) + // Append command arguments if provided + if (CommandArguments != null && CommandArguments.Length > 0) + { + foreach (string arg in CommandArguments) + { + sw.Write(' '); + if (NativeMethodsShared.IsUnixLike) + { + sw.Write(EscapeArgumentForUnix(arg)); + } + else + { + sw.Write(EscapeArgumentForWindows(arg)); + } + } + } + } + else if (CommandArguments != null && CommandArguments.Length > 0) { - foreach (string arg in CommandArguments) + // If no Command but we have CommandArguments, treat the first argument as the command + // and the rest as arguments + sw.Write(CommandArguments[0]); + + for (int i = 1; i < CommandArguments.Length; i++) { sw.Write(' '); if (NativeMethodsShared.IsUnixLike) { - sw.Write(EscapeArgumentForUnix(arg)); + sw.Write(EscapeArgumentForUnix(CommandArguments[i])); } else { - sw.Write(EscapeArgumentForWindows(arg)); + sw.Write(EscapeArgumentForWindows(CommandArguments[i])); } } } From 1b86a1d2864fa0432a1282f8589e0af4b720143f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 2 Dec 2025 00:32:25 +0000 Subject: [PATCH 4/4] Address code review feedback: improve escaping implementation Co-authored-by: baronfel <573979+baronfel@users.noreply.github.com> --- src/Tasks/Exec.cs | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/src/Tasks/Exec.cs b/src/Tasks/Exec.cs index 90d60254e65..6578a647288 100644 --- a/src/Tasks/Exec.cs +++ b/src/Tasks/Exec.cs @@ -214,12 +214,15 @@ private static string EscapeArgumentForWindows(string argument) return "\"\""; } + // Characters that require quoting in Windows cmd.exe + // Based on cmd.exe special characters + char[] specialChars = { ' ', '\t', '"', '&', '|', '<', '>', '^', '%', '(', ')', '!', '=', ';', ',' }; + // Check if the argument contains special characters that need quoting bool needsQuoting = false; foreach (char c in argument) { - if (c == ' ' || c == '\t' || c == '"' || c == '&' || c == '|' || c == '<' || c == '>' || - c == '^' || c == '%' || c == '(' || c == ')' || c == '!' || c == '=' || c == ';' || c == ',') + if (Array.IndexOf(specialChars, c) >= 0) { needsQuoting = true; break; @@ -388,8 +391,15 @@ private void CreateTemporaryBatchFile() else if (CommandArguments != null && CommandArguments.Length > 0) { // If no Command but we have CommandArguments, treat the first argument as the command - // and the rest as arguments - sw.Write(CommandArguments[0]); + // and the rest as arguments. Escape the command if it contains special characters. + if (NativeMethodsShared.IsUnixLike) + { + sw.Write(EscapeArgumentForUnix(CommandArguments[0])); + } + else + { + sw.Write(EscapeArgumentForWindows(CommandArguments[0])); + } for (int i = 1; i < CommandArguments.Length; i++) {