From 4604f77f32277b7e1e6f06cac618c839fc09c8dc Mon Sep 17 00:00:00 2001 From: Q Date: Sun, 26 Oct 2025 03:38:51 +0000 Subject: [PATCH 1/2] Fix bash syntax error by escaping parentheses in shell commands Implements Option 1 from issue #2485: Escape parentheses in shellEscapeCommandString to prevent bash from interpreting them as subshell syntax when commands are wrapped in double quotes. The issue occurred when tool names like 'shell(cat)' were wrapped in single quotes, then the entire command was wrapped in double quotes. Bash treats single quotes as literal characters inside double quotes, leaving parentheses unprotected. Changes: - Added parentheses escaping to shellEscapeCommandString function - Updated tests to expect escaped parentheses in command strings Fixes #2485 --- pkg/workflow/shell.go | 5 ++++- pkg/workflow/shell_test.go | 6 +++--- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/pkg/workflow/shell.go b/pkg/workflow/shell.go index 53946a2db..64a7f5d75 100644 --- a/pkg/workflow/shell.go +++ b/pkg/workflow/shell.go @@ -37,7 +37,7 @@ func shellEscapeArg(arg string) string { // shellEscapeCommandString escapes a complete command string (which may already contain // quoted arguments) for passing as a single argument to another command. // It wraps the command in double quotes and escapes any double quotes, dollar signs, -// backticks, and backslashes within the command. +// backticks, backslashes, and parentheses within the command. // This is useful when passing a command to wrapper programs like awf that expect // the command as a single quoted argument. func shellEscapeCommandString(cmd string) string { @@ -49,6 +49,9 @@ func shellEscapeCommandString(cmd string) string { escaped = strings.ReplaceAll(escaped, "$", "\\$") // Escape backticks (to prevent command substitution) escaped = strings.ReplaceAll(escaped, "`", "\\`") + // Escape parentheses (to prevent subshell interpretation inside double quotes) + escaped = strings.ReplaceAll(escaped, "(", "\\(") + escaped = strings.ReplaceAll(escaped, ")", "\\)") // Wrap in double quotes return "\"" + escaped + "\"" diff --git a/pkg/workflow/shell_test.go b/pkg/workflow/shell_test.go index b578745f8..e8c898c61 100644 --- a/pkg/workflow/shell_test.go +++ b/pkg/workflow/shell_test.go @@ -132,7 +132,7 @@ func TestShellEscapeCommandString(t *testing.T) { { name: "command with single-quoted arguments", input: "npx --allow-tool 'shell(cat)' --allow-tool 'shell(ls)'", - expected: "\"npx --allow-tool 'shell(cat)' --allow-tool 'shell(ls)'\"", + expected: "\"npx --allow-tool 'shell\\(cat\\)' --allow-tool 'shell\\(ls\\)'\"", }, { name: "command with double quotes", @@ -142,7 +142,7 @@ func TestShellEscapeCommandString(t *testing.T) { { name: "command with dollar sign (command substitution)", input: "echo $(date)", - expected: "\"echo \\$(date)\"", + expected: "\"echo \\$\\(date\\)\"", }, { name: "command with backticks", @@ -157,7 +157,7 @@ func TestShellEscapeCommandString(t *testing.T) { { name: "complex copilot command", input: "npx -y @github/copilot@0.0.351 --allow-tool 'github(list_workflows)' --prompt \"$(cat /tmp/prompt.txt)\"", - expected: "\"npx -y @github/copilot@0.0.351 --allow-tool 'github(list_workflows)' --prompt \\\"\\$(cat /tmp/prompt.txt)\\\"\"", + expected: "\"npx -y @github/copilot@0.0.351 --allow-tool 'github\\(list_workflows\\)' --prompt \\\"\\$\\(cat /tmp/prompt.txt\\)\\\"\"", }, } From 6ce885383fe272def192588a1c40166721de49b1 Mon Sep 17 00:00:00 2001 From: Copilot <198982749+Copilot@users.noreply.github.com> Date: Sat, 25 Oct 2025 21:23:47 -0700 Subject: [PATCH 2/2] Fix bash syntax error by escaping parentheses in Copilot shell commands (#2494) --- .github/workflows/artifacts-summary.lock.yml | 2 +- .github/workflows/mcp-inspector.lock.yml | 2 +- .github/workflows/research.lock.yml | 2 +- .../workflows/smoke-copilot.firewall.lock.yml | 2 +- .github/workflows/smoke-copilot.lock.yml | 2 +- pkg/workflow/shell.go | 49 ++- pkg/workflow/shell_test.go | 358 +++++++++++++++++- 7 files changed, 407 insertions(+), 10 deletions(-) diff --git a/.github/workflows/artifacts-summary.lock.yml b/.github/workflows/artifacts-summary.lock.yml index fc8269cdf..18666a51a 100644 --- a/.github/workflows/artifacts-summary.lock.yml +++ b/.github/workflows/artifacts-summary.lock.yml @@ -1336,7 +1336,7 @@ jobs: sudo -E awf --env-all \ --allow-domains api.enterprise.githubcopilot.com,api.github.com,github.com,raw.githubusercontent.com,registry.npmjs.org \ --log-level info \ - "npx -y @github/copilot@0.0.351 --add-dir /tmp/gh-aw/ --log-level all --disable-builtin-mcps --allow-tool 'github(list_workflow_run_artifacts)' --allow-tool 'github(list_workflow_runs)' --allow-tool 'github(list_workflows)' --allow-tool safeoutputs --allow-tool 'shell(cat)' --allow-tool 'shell(date)' --allow-tool 'shell(echo)' --allow-tool 'shell(grep)' --allow-tool 'shell(head)' --allow-tool 'shell(ls)' --allow-tool 'shell(pwd)' --allow-tool 'shell(sort)' --allow-tool 'shell(tail)' --allow-tool 'shell(uniq)' --allow-tool 'shell(wc)' --allow-tool 'shell(yq)' --allow-tool write --allow-all-paths --prompt \"\$(cat /tmp/gh-aw/aw-prompts/prompt.txt)\"" \ + "npx -y @github/copilot@0.0.351 --add-dir /tmp/gh-aw/ --log-level all --disable-builtin-mcps --allow-tool 'github\(list_workflow_run_artifacts\)' --allow-tool 'github\(list_workflow_runs\)' --allow-tool 'github\(list_workflows\)' --allow-tool safeoutputs --allow-tool 'shell\(cat\)' --allow-tool 'shell\(date\)' --allow-tool 'shell\(echo\)' --allow-tool 'shell\(grep\)' --allow-tool 'shell\(head\)' --allow-tool 'shell\(ls\)' --allow-tool 'shell\(pwd\)' --allow-tool 'shell\(sort\)' --allow-tool 'shell\(tail\)' --allow-tool 'shell\(uniq\)' --allow-tool 'shell\(wc\)' --allow-tool 'shell\(yq\)' --allow-tool write --allow-all-paths --prompt \"\$(cat /tmp/gh-aw/aw-prompts/prompt.txt)\"" \ 2>&1 | tee /tmp/gh-aw/agent-stdio.log # Move preserved Copilot logs to expected location diff --git a/.github/workflows/mcp-inspector.lock.yml b/.github/workflows/mcp-inspector.lock.yml index 281661885..cd7417de6 100644 --- a/.github/workflows/mcp-inspector.lock.yml +++ b/.github/workflows/mcp-inspector.lock.yml @@ -1874,7 +1874,7 @@ jobs: sudo -E awf --env-all \ --allow-domains api.enterprise.githubcopilot.com,api.github.com,github.com,raw.githubusercontent.com,registry.npmjs.org \ --log-level info \ - "npx -y @github/copilot@0.0.351 --add-dir /tmp/gh-aw/ --log-level all --disable-builtin-mcps --allow-tool arxiv --allow-tool 'arxiv(get_paper_details)' --allow-tool 'arxiv(get_paper_pdf)' --allow-tool 'arxiv(search_arxiv)' --allow-tool ast-grep --allow-tool 'ast-grep(*)' --allow-tool brave-search --allow-tool 'brave-search(*)' --allow-tool context7 --allow-tool 'context7(get-library-docs)' --allow-tool 'context7(resolve-library-id)' --allow-tool datadog --allow-tool 'datadog(get_datadog_metric)' --allow-tool 'datadog(search_datadog_dashboards)' --allow-tool 'datadog(search_datadog_metrics)' --allow-tool 'datadog(search_datadog_slos)' --allow-tool deepwiki --allow-tool 'deepwiki(ask_question)' --allow-tool 'deepwiki(read_wiki_contents)' --allow-tool 'deepwiki(read_wiki_structure)' --allow-tool fabric-rti --allow-tool 'fabric-rti(get_eventstream)' --allow-tool 'fabric-rti(get_eventstream_definition)' --allow-tool 'fabric-rti(kusto_get_entities_schema)' --allow-tool 'fabric-rti(kusto_get_function_schema)' --allow-tool 'fabric-rti(kusto_get_shots)' --allow-tool 'fabric-rti(kusto_get_table_schema)' --allow-tool 'fabric-rti(kusto_known_services)' --allow-tool 'fabric-rti(kusto_list_databases)' --allow-tool 'fabric-rti(kusto_list_tables)' --allow-tool 'fabric-rti(kusto_query)' --allow-tool 'fabric-rti(kusto_sample_function_data)' --allow-tool 'fabric-rti(kusto_sample_table_data)' --allow-tool 'fabric-rti(list_eventstreams)' --allow-tool gh-aw --allow-tool github --allow-tool markitdown --allow-tool 'markitdown(*)' --allow-tool memory --allow-tool 'memory(delete_memory)' --allow-tool 'memory(list_memories)' --allow-tool 'memory(retrieve_memory)' --allow-tool 'memory(store_memory)' --allow-tool microsoftdocs --allow-tool 'microsoftdocs(*)' --allow-tool notion --allow-tool 'notion(get_database)' --allow-tool 'notion(get_page)' --allow-tool 'notion(query_database)' --allow-tool 'notion(search_pages)' --allow-tool safeoutputs --allow-tool sentry --allow-tool 'sentry(analyze_issue_with_seer)' --allow-tool 'sentry(find_dsns)' --allow-tool 'sentry(find_organizations)' --allow-tool 'sentry(find_projects)' --allow-tool 'sentry(find_releases)' --allow-tool 'sentry(find_teams)' --allow-tool 'sentry(get_doc)' --allow-tool 'sentry(get_event_attachment)' --allow-tool 'sentry(get_issue_details)' --allow-tool 'sentry(get_trace_details)' --allow-tool 'sentry(search_docs requires SENTRY_OPENAI_API_KEY)' --allow-tool 'sentry(search_events)' --allow-tool 'sentry(search_issues)' --allow-tool 'sentry(whoami)' --allow-tool serena --allow-tool 'serena(*)' --allow-tool 'shell(cat)' --allow-tool 'shell(date)' --allow-tool 'shell(echo)' --allow-tool 'shell(grep)' --allow-tool 'shell(head)' --allow-tool 'shell(ls)' --allow-tool 'shell(pwd)' --allow-tool 'shell(sort)' --allow-tool 'shell(tail)' --allow-tool 'shell(uniq)' --allow-tool 'shell(wc)' --allow-tool 'shell(yq)' --allow-tool tavily --allow-tool 'tavily(*)' --allow-tool write --add-dir /tmp/gh-aw/cache-memory/ --allow-all-paths --prompt \"\$(cat /tmp/gh-aw/aw-prompts/prompt.txt)\"" \ + "npx -y @github/copilot@0.0.351 --add-dir /tmp/gh-aw/ --log-level all --disable-builtin-mcps --allow-tool arxiv --allow-tool 'arxiv\(get_paper_details\)' --allow-tool 'arxiv\(get_paper_pdf\)' --allow-tool 'arxiv\(search_arxiv\)' --allow-tool ast-grep --allow-tool 'ast-grep\(*\)' --allow-tool brave-search --allow-tool 'brave-search\(*\)' --allow-tool context7 --allow-tool 'context7\(get-library-docs\)' --allow-tool 'context7\(resolve-library-id\)' --allow-tool datadog --allow-tool 'datadog\(get_datadog_metric\)' --allow-tool 'datadog\(search_datadog_dashboards\)' --allow-tool 'datadog\(search_datadog_metrics\)' --allow-tool 'datadog\(search_datadog_slos\)' --allow-tool deepwiki --allow-tool 'deepwiki\(ask_question\)' --allow-tool 'deepwiki\(read_wiki_contents\)' --allow-tool 'deepwiki\(read_wiki_structure\)' --allow-tool fabric-rti --allow-tool 'fabric-rti\(get_eventstream\)' --allow-tool 'fabric-rti\(get_eventstream_definition\)' --allow-tool 'fabric-rti\(kusto_get_entities_schema\)' --allow-tool 'fabric-rti\(kusto_get_function_schema\)' --allow-tool 'fabric-rti\(kusto_get_shots\)' --allow-tool 'fabric-rti\(kusto_get_table_schema\)' --allow-tool 'fabric-rti\(kusto_known_services\)' --allow-tool 'fabric-rti\(kusto_list_databases\)' --allow-tool 'fabric-rti\(kusto_list_tables\)' --allow-tool 'fabric-rti\(kusto_query\)' --allow-tool 'fabric-rti\(kusto_sample_function_data\)' --allow-tool 'fabric-rti\(kusto_sample_table_data\)' --allow-tool 'fabric-rti\(list_eventstreams\)' --allow-tool gh-aw --allow-tool github --allow-tool markitdown --allow-tool 'markitdown\(*\)' --allow-tool memory --allow-tool 'memory\(delete_memory\)' --allow-tool 'memory\(list_memories\)' --allow-tool 'memory\(retrieve_memory\)' --allow-tool 'memory\(store_memory\)' --allow-tool microsoftdocs --allow-tool 'microsoftdocs\(*\)' --allow-tool notion --allow-tool 'notion\(get_database\)' --allow-tool 'notion\(get_page\)' --allow-tool 'notion\(query_database\)' --allow-tool 'notion\(search_pages\)' --allow-tool safeoutputs --allow-tool sentry --allow-tool 'sentry\(analyze_issue_with_seer\)' --allow-tool 'sentry\(find_dsns\)' --allow-tool 'sentry\(find_organizations\)' --allow-tool 'sentry\(find_projects\)' --allow-tool 'sentry\(find_releases\)' --allow-tool 'sentry\(find_teams\)' --allow-tool 'sentry\(get_doc\)' --allow-tool 'sentry\(get_event_attachment\)' --allow-tool 'sentry\(get_issue_details\)' --allow-tool 'sentry\(get_trace_details\)' --allow-tool 'sentry\(search_docs requires SENTRY_OPENAI_API_KEY\)' --allow-tool 'sentry\(search_events\)' --allow-tool 'sentry\(search_issues\)' --allow-tool 'sentry\(whoami\)' --allow-tool serena --allow-tool 'serena\(*\)' --allow-tool 'shell\(cat\)' --allow-tool 'shell\(date\)' --allow-tool 'shell\(echo\)' --allow-tool 'shell\(grep\)' --allow-tool 'shell\(head\)' --allow-tool 'shell\(ls\)' --allow-tool 'shell\(pwd\)' --allow-tool 'shell\(sort\)' --allow-tool 'shell\(tail\)' --allow-tool 'shell\(uniq\)' --allow-tool 'shell\(wc\)' --allow-tool 'shell\(yq\)' --allow-tool tavily --allow-tool 'tavily\(*\)' --allow-tool write --add-dir /tmp/gh-aw/cache-memory/ --allow-all-paths --prompt \"\$(cat /tmp/gh-aw/aw-prompts/prompt.txt)\"" \ 2>&1 | tee /tmp/gh-aw/agent-stdio.log # Move preserved Copilot logs to expected location diff --git a/.github/workflows/research.lock.yml b/.github/workflows/research.lock.yml index 9cb015ed3..332470587 100644 --- a/.github/workflows/research.lock.yml +++ b/.github/workflows/research.lock.yml @@ -1257,7 +1257,7 @@ jobs: sudo -E awf --env-all \ --allow-domains api.enterprise.githubcopilot.com,api.github.com,github.com,raw.githubusercontent.com,registry.npmjs.org \ --log-level info \ - "npx -y @github/copilot@0.0.351 --add-dir /tmp/gh-aw/ --log-level all --disable-builtin-mcps --allow-tool github --allow-tool safeoutputs --allow-tool tavily --allow-tool 'tavily(*)' --prompt \"\$(cat /tmp/gh-aw/aw-prompts/prompt.txt)\"" \ + "npx -y @github/copilot@0.0.351 --add-dir /tmp/gh-aw/ --log-level all --disable-builtin-mcps --allow-tool github --allow-tool safeoutputs --allow-tool tavily --allow-tool 'tavily\(*\)' --prompt \"\$(cat /tmp/gh-aw/aw-prompts/prompt.txt)\"" \ 2>&1 | tee /tmp/gh-aw/agent-stdio.log # Move preserved Copilot logs to expected location diff --git a/.github/workflows/smoke-copilot.firewall.lock.yml b/.github/workflows/smoke-copilot.firewall.lock.yml index 66c9f4aca..4fea03fdc 100644 --- a/.github/workflows/smoke-copilot.firewall.lock.yml +++ b/.github/workflows/smoke-copilot.firewall.lock.yml @@ -1240,7 +1240,7 @@ jobs: sudo -E awf --env-all \ --allow-domains api.enterprise.githubcopilot.com,api.github.com,github.com,raw.githubusercontent.com,registry.npmjs.org \ --log-level info \ - "npx -y @github/copilot@0.0.351 --add-dir /tmp/gh-aw/ --log-level all --disable-builtin-mcps --allow-tool github --allow-tool safeoutputs --allow-tool 'shell(cat)' --allow-tool 'shell(date)' --allow-tool 'shell(echo)' --allow-tool 'shell(grep)' --allow-tool 'shell(head)' --allow-tool 'shell(ls)' --allow-tool 'shell(pwd)' --allow-tool 'shell(sort)' --allow-tool 'shell(tail)' --allow-tool 'shell(uniq)' --allow-tool 'shell(wc)' --allow-tool 'shell(yq)' --allow-tool write --allow-all-paths --prompt \"\$(cat /tmp/gh-aw/aw-prompts/prompt.txt)\"" \ + "npx -y @github/copilot@0.0.351 --add-dir /tmp/gh-aw/ --log-level all --disable-builtin-mcps --allow-tool github --allow-tool safeoutputs --allow-tool 'shell\(cat\)' --allow-tool 'shell\(date\)' --allow-tool 'shell\(echo\)' --allow-tool 'shell\(grep\)' --allow-tool 'shell\(head\)' --allow-tool 'shell\(ls\)' --allow-tool 'shell\(pwd\)' --allow-tool 'shell\(sort\)' --allow-tool 'shell\(tail\)' --allow-tool 'shell\(uniq\)' --allow-tool 'shell\(wc\)' --allow-tool 'shell\(yq\)' --allow-tool write --allow-all-paths --prompt \"\$(cat /tmp/gh-aw/aw-prompts/prompt.txt)\"" \ 2>&1 | tee /tmp/gh-aw/agent-stdio.log # Move preserved Copilot logs to expected location diff --git a/.github/workflows/smoke-copilot.lock.yml b/.github/workflows/smoke-copilot.lock.yml index eef9be114..7ee24500c 100644 --- a/.github/workflows/smoke-copilot.lock.yml +++ b/.github/workflows/smoke-copilot.lock.yml @@ -1240,7 +1240,7 @@ jobs: sudo -E awf --env-all \ --allow-domains api.enterprise.githubcopilot.com,api.github.com,github.com,raw.githubusercontent.com,registry.npmjs.org \ --log-level info \ - "npx -y @github/copilot@0.0.351 --add-dir /tmp/gh-aw/ --log-level all --disable-builtin-mcps --allow-tool github --allow-tool safeoutputs --allow-tool 'shell(cat)' --allow-tool 'shell(date)' --allow-tool 'shell(echo)' --allow-tool 'shell(grep)' --allow-tool 'shell(head)' --allow-tool 'shell(ls)' --allow-tool 'shell(pwd)' --allow-tool 'shell(sort)' --allow-tool 'shell(tail)' --allow-tool 'shell(uniq)' --allow-tool 'shell(wc)' --allow-tool 'shell(yq)' --allow-tool write --allow-all-paths --prompt \"\$(cat /tmp/gh-aw/aw-prompts/prompt.txt)\"" \ + "npx -y @github/copilot@0.0.351 --add-dir /tmp/gh-aw/ --log-level all --disable-builtin-mcps --allow-tool github --allow-tool safeoutputs --allow-tool 'shell\(cat\)' --allow-tool 'shell\(date\)' --allow-tool 'shell\(echo\)' --allow-tool 'shell\(grep\)' --allow-tool 'shell\(head\)' --allow-tool 'shell\(ls\)' --allow-tool 'shell\(pwd\)' --allow-tool 'shell\(sort\)' --allow-tool 'shell\(tail\)' --allow-tool 'shell\(uniq\)' --allow-tool 'shell\(wc\)' --allow-tool 'shell\(yq\)' --allow-tool write --allow-all-paths --prompt \"\$(cat /tmp/gh-aw/aw-prompts/prompt.txt)\"" \ 2>&1 | tee /tmp/gh-aw/agent-stdio.log # Move preserved Copilot logs to expected location diff --git a/pkg/workflow/shell.go b/pkg/workflow/shell.go index 64a7f5d75..2cab891a2 100644 --- a/pkg/workflow/shell.go +++ b/pkg/workflow/shell.go @@ -40,6 +40,9 @@ func shellEscapeArg(arg string) string { // backticks, backslashes, and parentheses within the command. // This is useful when passing a command to wrapper programs like awf that expect // the command as a single quoted argument. +// +// Special case: Parentheses immediately following $ (i.e., $(...)) are NOT escaped +// to preserve command substitution syntax. func shellEscapeCommandString(cmd string) string { // Escape backslashes first (must be done before other escapes) escaped := strings.ReplaceAll(cmd, "\\", "\\\\") @@ -49,10 +52,50 @@ func shellEscapeCommandString(cmd string) string { escaped = strings.ReplaceAll(escaped, "$", "\\$") // Escape backticks (to prevent command substitution) escaped = strings.ReplaceAll(escaped, "`", "\\`") + // Escape parentheses (to prevent subshell interpretation inside double quotes) - escaped = strings.ReplaceAll(escaped, "(", "\\(") - escaped = strings.ReplaceAll(escaped, ")", "\\)") + // BUT preserve command substitution syntax: \$(...) should remain as \$(...) + // We need to escape ( and ) except when they immediately follow \$ (which was $ before escaping) + result := make([]byte, 0, len(escaped)*2) + for i := 0; i < len(escaped); i++ { + ch := escaped[i] + if ch == '(' { + // Don't escape opening paren if it follows \$ + if i >= 2 && escaped[i-2] == '\\' && escaped[i-1] == '$' { + result = append(result, ch) + } else { + result = append(result, '\\', ch) + } + } else if ch == ')' { + // Don't escape closing paren if we're inside a \$(...) construct + // We need to track by looking backward for matching \$( + inCommandSubst := false + depth := 1 // We're currently at a ')', so depth starts at 1 + for j := i - 1; j >= 0; j-- { + if escaped[j] == ')' { + depth++ + } else if escaped[j] == '(' { + depth-- + if depth == 0 { + // Found the matching opening paren + // Check if it's a command substitution \$( + if j >= 2 && escaped[j-2] == '\\' && escaped[j-1] == '$' { + inCommandSubst = true + } + break + } + } + } + if inCommandSubst { + result = append(result, ch) + } else { + result = append(result, '\\', ch) + } + } else { + result = append(result, ch) + } + } // Wrap in double quotes - return "\"" + escaped + "\"" + return "\"" + string(result) + "\"" } diff --git a/pkg/workflow/shell_test.go b/pkg/workflow/shell_test.go index e8c898c61..56b80a7d3 100644 --- a/pkg/workflow/shell_test.go +++ b/pkg/workflow/shell_test.go @@ -68,6 +68,137 @@ func TestShellEscapeArg(t *testing.T) { input: "\"\"", expected: "\"\"", }, + // Additional edge cases + { + name: "argument with curly braces", + input: "file{1,2,3}.txt", + expected: "'file{1,2,3}.txt'", + }, + { + name: "argument with question mark", + input: "file?.txt", + expected: "'file?.txt'", + }, + { + name: "argument with pipe", + input: "cmd1|cmd2", + expected: "'cmd1|cmd2'", + }, + { + name: "argument with ampersand", + input: "cmd1&cmd2", + expected: "'cmd1&cmd2'", + }, + { + name: "argument with semicolon", + input: "cmd1;cmd2", + expected: "'cmd1;cmd2'", + }, + { + name: "argument with less than", + input: "inputfile", + expected: "'output>file'", + }, + { + name: "argument with backslash", + input: "path\\to\\file", + expected: "'path\\to\\file'", + }, + { + name: "argument with backtick", + input: "cmd`date`", + expected: "'cmd`date`'", + }, + { + name: "argument with tab", + input: "hello\tworld", + expected: "'hello\tworld'", + }, + { + name: "argument with newline", + input: "hello\nworld", + expected: "'hello\nworld'", + }, + { + name: "multiple single quotes", + input: "it's can't won't", + expected: "'it'\"'\"'s can'\"'\"'t won'\"'\"'t'", + }, + { + name: "empty string", + input: "", + expected: "", + }, + { + name: "single character flag", + input: "-v", + expected: "-v", + }, + { + name: "path without special characters", + input: "/usr/bin/env", + expected: "/usr/bin/env", + }, + { + name: "path with spaces", + input: "/path/with spaces/file", + expected: "'/path/with spaces/file'", + }, + { + name: "command substitution pattern", + input: "$(date)", + expected: "'$(date)'", + }, + { + name: "variable expansion pattern", + input: "${VAR}", + expected: "'${VAR}'", + }, + { + name: "already quoted with extra chars should be escaped", + input: "\"test\"extra", + expected: "'\"test\"extra'", + }, + { + name: "single quote at start only", + input: "'incomplete", + expected: "''\"'\"'incomplete'", + }, + { + name: "single quote at end only", + input: "incomplete'", + expected: "'incomplete'\"'\"''", + }, + { + name: "double quote at start only", + input: "\"incomplete", + expected: "'\"incomplete'", + }, + { + name: "double quote at end only", + input: "incomplete\"", + expected: "'incomplete\"'", + }, + { + name: "only single quotes", + input: "''", + expected: "''", + }, + { + name: "nested parentheses", + input: "func((arg))", + expected: "'func((arg))'", + }, + { + name: "mixed brackets and parentheses", + input: "pattern[a-z](test)", + expected: "'pattern[a-z](test)'", + }, } for _, tt := range tests { @@ -106,6 +237,92 @@ func TestShellJoinArgs(t *testing.T) { input: []string{"copilot", "--add-dir", "/tmp/gh-aw/", "--prompt", "\"$INSTRUCTION\""}, expected: "copilot --add-dir /tmp/gh-aw/ --prompt \"$INSTRUCTION\"", }, + // Additional edge cases + { + name: "empty array", + input: []string{}, + expected: "", + }, + { + name: "single argument", + input: []string{"command"}, + expected: "command", + }, + { + name: "single argument with spaces", + input: []string{"hello world"}, + expected: "'hello world'", + }, + { + name: "multiple arguments with spaces", + input: []string{"echo", "hello world", "foo bar"}, + expected: "echo 'hello world' 'foo bar'", + }, + { + name: "arguments with various quote types", + input: []string{"cmd", "\"quoted\"", "'single'", "mixed\"quote"}, + expected: "cmd \"quoted\" 'single' 'mixed\"quote'", + }, + { + name: "arguments with command substitution patterns", + input: []string{"echo", "$(date)", "`whoami`"}, + expected: "echo '$(date)' '`whoami`'", + }, + { + name: "long command with many arguments", + input: []string{"npx", "@github/copilot", "--allow-tool", "shell(cat)", "--allow-tool", "shell(grep)", "--allow-tool", "shell(sed)", "--log-level", "debug"}, + expected: "npx @github/copilot --allow-tool 'shell(cat)' --allow-tool 'shell(grep)' --allow-tool 'shell(sed)' --log-level debug", + }, + { + name: "arguments with special shell operators", + input: []string{"cmd", "arg1|arg2", "arg3&arg4", "arg5;arg6"}, + expected: "cmd 'arg1|arg2' 'arg3&arg4' 'arg5;arg6'", + }, + { + name: "arguments with wildcards", + input: []string{"ls", "*.txt", "file?.doc", "test[0-9].log"}, + expected: "ls '*.txt' 'file?.doc' 'test[0-9].log'", + }, + { + name: "mixed flags and values", + input: []string{"-v", "--verbose", "-f", "file name.txt", "--output", "result.log"}, + expected: "-v --verbose -f 'file name.txt' --output result.log", + }, + { + name: "arguments with dollar signs", + input: []string{"echo", "$HOME", "$USER", "$PATH"}, + expected: "echo '$HOME' '$USER' '$PATH'", + }, + { + name: "pre-quoted arguments mixed with unquoted", + input: []string{"cmd", "\"arg1\"", "arg2", "'arg3'", "arg 4"}, + expected: "cmd \"arg1\" arg2 'arg3' 'arg 4'", + }, + { + name: "arguments with backslashes", + input: []string{"echo", "\\n", "\\t", "path\\to\\file"}, + expected: "echo '\\n' '\\t' 'path\\to\\file'", + }, + { + name: "arguments with parentheses and brackets", + input: []string{"tool", "func(arg)", "pattern[a-z]", "test{1,2}"}, + expected: "tool 'func(arg)' 'pattern[a-z]' 'test{1,2}'", + }, + { + name: "empty string arguments", + input: []string{"cmd", "", "arg"}, + expected: "cmd arg", + }, + { + name: "arguments with newlines and tabs", + input: []string{"echo", "line1\nline2", "col1\tcol2"}, + expected: "echo 'line1\nline2' 'col1\tcol2'", + }, + { + name: "single quotes in multiple arguments", + input: []string{"echo", "it's", "can't", "won't"}, + expected: "echo 'it'\"'\"'s' 'can'\"'\"'t' 'won'\"'\"'t'", + }, } for _, tt := range tests { @@ -142,7 +359,7 @@ func TestShellEscapeCommandString(t *testing.T) { { name: "command with dollar sign (command substitution)", input: "echo $(date)", - expected: "\"echo \\$\\(date\\)\"", + expected: "\"echo \\$(date)\"", }, { name: "command with backticks", @@ -157,7 +374,144 @@ func TestShellEscapeCommandString(t *testing.T) { { name: "complex copilot command", input: "npx -y @github/copilot@0.0.351 --allow-tool 'github(list_workflows)' --prompt \"$(cat /tmp/prompt.txt)\"", - expected: "\"npx -y @github/copilot@0.0.351 --allow-tool 'github\\(list_workflows\\)' --prompt \\\"\\$\\(cat /tmp/prompt.txt\\)\\\"\"", + expected: "\"npx -y @github/copilot@0.0.351 --allow-tool 'github\\(list_workflows\\)' --prompt \\\"\\$(cat /tmp/prompt.txt)\\\"\"", + }, + // Command substitution edge cases + { + name: "nested command substitution", + input: "echo $(echo $(date))", + expected: "\"echo \\$(echo \\$(date))\"", + }, + { + name: "multiple command substitutions", + input: "echo $(date) and $(whoami)", + expected: "\"echo \\$(date) and \\$(whoami)\"", + }, + { + name: "command substitution with pipes", + input: "echo $(cat file | grep pattern)", + expected: "\"echo \\$(cat file | grep pattern)\"", + }, + { + name: "command substitution with parentheses in arguments", + input: "echo $(grep 'pattern(test)' file)", + expected: "\"echo \\$(grep 'pattern\\(test\\)' file)\"", + }, + { + name: "command substitution at the end", + input: "prompt \"$(cat /tmp/file)\"", + expected: "\"prompt \\\"\\$(cat /tmp/file)\\\"\"", + }, + // Regular parentheses (not command substitution) + { + name: "standalone parentheses should be escaped", + input: "echo (test)", + expected: "\"echo \\(test\\)\"", + }, + { + name: "multiple levels of parentheses", + input: "echo ((test))", + expected: "\"echo \\(\\(test\\)\\)\"", + }, + { + name: "parentheses in quoted strings", + input: "echo 'test(ing)' and \"more(stuff)\"", + expected: "\"echo 'test\\(ing\\)' and \\\"more\\(stuff\\)\\\"\"", + }, + // Mixed scenarios + { + name: "command substitution and regular parentheses", + input: "echo $(date) and func(arg) and $(cat file)", + expected: "\"echo \\$(date) and func\\(arg\\) and \\$(cat file)\"", + }, + { + name: "parentheses before dollar sign (not command substitution)", + input: "echo (prefix)$VAR", + expected: "\"echo \\(prefix\\)\\$VAR\"", + }, + // Backslash edge cases + { + name: "existing backslash before parenthesis", + input: "echo \\(already escaped\\)", + expected: "\"echo \\\\\\(already escaped\\\\\\)\"", + }, + { + name: "backslash before dollar sign", + input: "echo \\$HOME", + expected: "\"echo \\\\\\$HOME\"", + }, + // Dollar sign edge cases + { + name: "dollar sign without parentheses (variable)", + input: "echo $HOME", + expected: "\"echo \\$HOME\"", + }, + { + name: "dollar sign with braces", + input: "echo ${HOME}", + expected: "\"echo \\${HOME}\"", + }, + { + name: "multiple dollar signs", + input: "echo $VAR1 $VAR2 $(cmd)", + expected: "\"echo \\$VAR1 \\$VAR2 \\$(cmd)\"", + }, + // Quote edge cases + { + name: "mixed quotes", + input: "echo \"test\" 'more' $(cmd)", + expected: "\"echo \\\"test\\\" 'more' \\$(cmd)\"", + }, + { + name: "escaped quotes in input", + input: "echo \\\"already escaped\\\"", + expected: "\"echo \\\\\\\"already escaped\\\\\\\"\"", + }, + // Real-world copilot scenarios + { + name: "copilot with multiple shell tools and command substitution", + input: "npx @github/copilot --allow-tool 'shell(cat)' --allow-tool 'shell(grep)' --allow-tool 'shell(sed)' --prompt \"$(cat /tmp/prompt.txt)\"", + expected: "\"npx @github/copilot --allow-tool 'shell\\(cat\\)' --allow-tool 'shell\\(grep\\)' --allow-tool 'shell\\(sed\\)' --prompt \\\"\\$(cat /tmp/prompt.txt)\\\"\"", + }, + { + name: "copilot with environment variable and command substitution", + input: "npx @github/copilot --log-dir $LOG_DIR --prompt \"$(cat $PROMPT_FILE)\"", + expected: "\"npx @github/copilot --log-dir \\$LOG_DIR --prompt \\\"\\$(cat \\$PROMPT_FILE)\\\"\"", + }, + // Empty and whitespace + { + name: "empty string", + input: "", + expected: "\"\"", + }, + { + name: "only whitespace", + input: " ", + expected: "\" \"", + }, + // Special characters combination + { + name: "all special characters", + input: "cmd \"quoted\" 'single' $VAR $(subst) `backtick` \\backslash (paren)", + expected: "\"cmd \\\"quoted\\\" 'single' \\$VAR \\$(subst) \\`backtick\\` \\\\backslash \\(paren\\)\"", + }, + // Edge case: $ at end of string + { + name: "dollar sign at end", + input: "echo test$", + expected: "\"echo test\\$\"", + }, + // Edge case: Opening paren at start + { + name: "opening paren at start", + input: "(command)", + expected: "\"\\(command\\)\"", + }, + // Edge case: Command substitution with escaped characters inside + { + name: "command substitution with special chars inside", + input: "echo $(echo \"test\" | grep 'pattern')", + expected: "\"echo \\$(echo \\\"test\\\" | grep 'pattern')\"", }, }