From c6da64ef476495c957d85fdfae3916822e8cb59e Mon Sep 17 00:00:00 2001 From: "Jiaxiao (mossaka) Zhou" Date: Mon, 27 Oct 2025 18:06:48 +0000 Subject: [PATCH] feat: implement escapeBashCommand function to handle special character escaping for bash commands Signed-off-by: Jiaxiao (mossaka) Zhou --- scripts/ci/test-bash-escaping.sh | 149 +++++++++++++++++++++++++++++ src/docker-manager.test.ts | 157 ++++++++++++++++++++++++++++++- src/docker-manager.ts | 66 ++++++++++++- 3 files changed, 368 insertions(+), 4 deletions(-) create mode 100755 scripts/ci/test-bash-escaping.sh diff --git a/scripts/ci/test-bash-escaping.sh b/scripts/ci/test-bash-escaping.sh new file mode 100755 index 000000000..725443f20 --- /dev/null +++ b/scripts/ci/test-bash-escaping.sh @@ -0,0 +1,149 @@ +#!/bin/bash +# Integration test for bash escaping issue (gh-aw PR #2493) +# Tests that AWF properly handles parentheses in Copilot CLI tool names + +set -e + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +PROJECT_ROOT="$(cd "$SCRIPT_DIR/../.." && pwd)" + +echo "===========================================" +echo "Bash Escaping Integration Tests" +echo "Testing fix for gh-aw PR #2493" +echo "===========================================" + +# Ensure we're using the local awf build +cd "$PROJECT_ROOT" +if [ ! -f "dist/cli.js" ]; then + echo "Building awf..." + npm run build +fi + +# Export minimal environment for Copilot (even though we won't actually run it fully) +export GITHUB_TOKEN="${GITHUB_TOKEN:-dummy-token-for-testing}" + +# Test 1: Reproduce the exact failure from PR #2493 +echo "" +echo "Test 1: Reproduce bash syntax error with parentheses in tool names" +echo "-------------------------------------------------------------------" +echo "Running: awf with --allow-tool 'shell(cat)', 'shell(date)', 'shell(echo)'" +echo "" + +# This command should fail with the current code (bash syntax error) +# After the fix, it should either succeed or fail with a different error (not bash syntax) +set +e +sudo -E node "$PROJECT_ROOT/dist/cli.js" \ + --log-level debug \ + --allow-domains github.com,api.github.com,registry.npmjs.org,api.enterprise.githubcopilot.com \ + "npx @github/copilot@0.0.351 --allow-tool 'shell(cat)' --allow-tool 'shell(date)' --allow-tool 'shell(echo)' --help" \ + 2>&1 | tee /tmp/awf-test-1.log + +EXIT_CODE=$? +set -e + +echo "" +echo "Exit code: $EXIT_CODE" + +# Check for bash syntax error (the bug we're trying to fix) +if grep -q "syntax error near unexpected token" /tmp/awf-test-1.log; then + echo "❌ FAILED: Bash syntax error detected (bug not fixed)" + echo " This is expected BEFORE the fix is applied" + BASH_SYNTAX_ERROR=1 +else + echo "✅ PASSED: No bash syntax error detected" + BASH_SYNTAX_ERROR=0 +fi + +# Check for Copilot validation error (what we might see with incorrect escaping) +if grep -q "Invalid rule format.*shell\\\\(cat\\\\)" /tmp/awf-test-1.log; then + echo "❌ FAILED: Copilot validation error (incorrect escaping)" + echo " Copilot received escaped format instead of clean format" + VALIDATION_ERROR=1 +else + echo "✅ PASSED: No Copilot validation error detected" + VALIDATION_ERROR=0 +fi + +# Test 2: Verify with simple echo command (should always work) +echo "" +echo "Test 2: Baseline test with simple command (no special chars)" +echo "-------------------------------------------------------------------" +echo "Running: awf with simple echo command" +echo "" + +set +e +sudo -E node "$PROJECT_ROOT/dist/cli.js" \ + --log-level debug \ + --allow-domains github.com \ + "echo 'Hello World'" \ + 2>&1 | tee /tmp/awf-test-2.log + +EXIT_CODE_2=$? +set -e + +echo "" +echo "Exit code: $EXIT_CODE_2" + +if [ $EXIT_CODE_2 -eq 0 ]; then + echo "✅ PASSED: Simple command executed successfully" + SIMPLE_TEST=0 +else + echo "❌ FAILED: Simple command failed (baseline broken)" + SIMPLE_TEST=1 +fi + +# Test 3: Test with dollar signs (existing functionality) +echo "" +echo "Test 3: Test dollar sign escaping for Docker Compose" +echo "-------------------------------------------------------------------" +echo "Running: awf with command containing dollar sign" +echo "" + +set +e +sudo -E node "$PROJECT_ROOT/dist/cli.js" \ + --log-level debug \ + --allow-domains github.com \ + 'echo "Testing dollar sign: $HOME"' \ + 2>&1 | tee /tmp/awf-test-3.log + +EXIT_CODE_3=$? +set -e + +echo "" +echo "Exit code: $EXIT_CODE_3" + +if [ $EXIT_CODE_3 -eq 0 ] && grep -q "Testing dollar sign:" /tmp/awf-test-3.log; then + echo "✅ PASSED: Dollar sign handled correctly" + DOLLAR_TEST=0 +else + echo "❌ FAILED: Dollar sign escaping broken" + DOLLAR_TEST=1 +fi + +# Summary +echo "" +echo "===========================================" +echo "Test Summary" +echo "===========================================" +echo "Test 1 (Parentheses): Bash syntax error=$BASH_SYNTAX_ERROR, Validation error=$VALIDATION_ERROR" +echo "Test 2 (Simple command): Exit code=$SIMPLE_TEST" +echo "Test 3 (Dollar signs): Exit code=$DOLLAR_TEST" +echo "" + +# Determine overall status +if [ $BASH_SYNTAX_ERROR -eq 1 ]; then + echo "⚠️ ISSUE REPRODUCED: Bash syntax error with parentheses" + echo " This confirms the bug exists. Apply the fix and re-run this test." + exit 1 +elif [ $VALIDATION_ERROR -eq 1 ]; then + echo "⚠️ INCORRECT FIX: Copilot validation error" + echo " The escaping is too aggressive. Copilot should receive 'shell(cat)' not 'shell\\(cat\\)'" + exit 1 +elif [ $SIMPLE_TEST -ne 0 ] || [ $DOLLAR_TEST -ne 0 ]; then + echo "❌ REGRESSION: Basic functionality broken" + exit 1 +else + echo "✅ ALL TESTS PASSED" + echo " Bash escaping is working correctly!" + exit 0 +fi diff --git a/src/docker-manager.test.ts b/src/docker-manager.test.ts index fcb1570bf..fb84a30e3 100644 --- a/src/docker-manager.test.ts +++ b/src/docker-manager.test.ts @@ -1,4 +1,4 @@ -import { generateDockerCompose } from './docker-manager'; +import { generateDockerCompose, escapeBashCommand } from './docker-manager'; import { WrapperConfig } from './types'; describe('docker-manager', () => { @@ -181,7 +181,8 @@ describe('docker-manager', () => { const copilot = result.services.copilot; // Docker compose requires $$ to represent a literal $ - expect(copilot.command).toEqual(['/bin/bash', '-c', 'echo $$HOME && echo $${USER}']); + // Ampersands are also escaped to prevent unintended backgrounding + expect(copilot.command).toEqual(['/bin/bash', '-c', 'echo $$HOME \\&\\& echo $${USER}']); }); it('should pass through GITHUB_TOKEN when present in environment', () => { @@ -287,4 +288,156 @@ describe('docker-manager', () => { } }); }); + + describe('escapeBashCommand', () => { + describe('parentheses escaping (gh-aw PR #2493)', () => { + it('should escape parentheses in single shell tool name', () => { + const input = "npx @github/copilot --allow-tool 'shell(cat)' --prompt 'test'"; + const escaped = escapeBashCommand(input); + + // Parentheses should be escaped to prevent subshell interpretation + expect(escaped).toContain('shell\\(cat\\)'); + // Single quotes should be preserved + expect(escaped).toContain("'"); + }); + + it('should escape parentheses in multiple shell tool names', () => { + const input = "--allow-tool 'shell(cat)' --allow-tool 'shell(grep)' --allow-tool 'shell(date)'"; + const escaped = escapeBashCommand(input); + + // All parentheses should be escaped + expect(escaped).toContain('shell\\(cat\\)'); + expect(escaped).toContain('shell\\(grep\\)'); + expect(escaped).toContain('shell\\(date\\)'); + }); + + it('should escape parentheses outside of quotes', () => { + const input = "echo (test) value"; + const escaped = escapeBashCommand(input); + + expect(escaped).toContain('\\(test\\)'); + }); + }); + + describe('dollar sign escaping (Docker Compose)', () => { + it('should double dollar signs for Docker Compose variable interpolation', () => { + const input = 'echo $HOME'; + const escaped = escapeBashCommand(input); + + // Single $ should become $$ for docker-compose + expect(escaped).toBe('echo $$HOME'); + }); + + it('should handle multiple dollar signs', () => { + const input = 'echo $HOME $USER $PATH'; + const escaped = escapeBashCommand(input); + + expect(escaped).toBe('echo $$HOME $$USER $$PATH'); + }); + + it('should handle dollar signs in complex commands', () => { + const input = "echo \"What's in $(pwd)?\""; + const escaped = escapeBashCommand(input); + + expect(escaped).toContain('$$'); + }); + }); + + describe('other special characters', () => { + it('should escape backticks', () => { + const input = 'echo `date`'; + const escaped = escapeBashCommand(input); + + expect(escaped).toContain('\\`'); + }); + + it('should escape semicolons', () => { + const input = 'echo hello; echo world'; + const escaped = escapeBashCommand(input); + + expect(escaped).toContain('\\;'); + }); + + it('should escape ampersands', () => { + const input = 'echo hello & echo world'; + const escaped = escapeBashCommand(input); + + expect(escaped).toContain('\\&'); + }); + + it('should escape pipes', () => { + const input = 'echo hello | grep hello'; + const escaped = escapeBashCommand(input); + + expect(escaped).toContain('\\|'); + }); + + it('should escape redirects', () => { + const input = 'echo hello > file.txt'; + const escaped = escapeBashCommand(input); + + expect(escaped).toContain('\\>'); + }); + }); + + describe('real-world Copilot commands', () => { + it('should handle full Copilot CLI command from gh-aw PR #2493', () => { + const input = `npx @github/copilot@0.0.351 --allow-tool github --allow-tool safeoutputs --allow-tool 'shell(cat)' --allow-tool 'shell(date)' --allow-tool 'shell(echo)' --allow-tool 'shell(grep)' --prompt "test prompt"`; + const escaped = escapeBashCommand(input); + + // Should escape parentheses + expect(escaped).toContain('\\('); + expect(escaped).toContain('\\)'); + + // Should preserve overall command structure + expect(escaped).toContain('npx'); + expect(escaped).toContain('@github/copilot'); + expect(escaped).toContain('--allow-tool'); + }); + + it('should handle Copilot command with complex prompt', () => { + const input = `npx @github/copilot --allow-tool 'shell(cat)' --prompt "What's in $(pwd)?"`; + const escaped = escapeBashCommand(input); + + // Should escape parentheses in tool name + expect(escaped).toContain('shell\\(cat\\)'); + // Should escape dollar signs + expect(escaped).toContain('$$'); + }); + }); + + describe('edge cases', () => { + it('should handle empty string', () => { + const input = ''; + const escaped = escapeBashCommand(input); + + expect(escaped).toBe(''); + }); + + it('should handle command with no special characters', () => { + const input = 'echo hello world'; + const escaped = escapeBashCommand(input); + + // Should remain unchanged (no special chars to escape) + expect(escaped).toBe('echo hello world'); + }); + + it('should handle nested quotes', () => { + const input = `echo "He said 'hello'"`; + const escaped = escapeBashCommand(input); + + // Should preserve quote structure + expect(escaped).toContain('"'); + expect(escaped).toContain("'"); + }); + + it('should handle backslashes', () => { + const input = 'echo \\n newline'; + const escaped = escapeBashCommand(input); + + // Backslashes should be escaped to prevent interpretation + expect(escaped).toContain('\\\\'); + }); + }); + }); }); diff --git a/src/docker-manager.ts b/src/docker-manager.ts index ddb99e8da..bb5a6bb68 100644 --- a/src/docker-manager.ts +++ b/src/docker-manager.ts @@ -107,6 +107,68 @@ async function generateRandomSubnet(): Promise<{ subnet: string; squidIp: string ); } +/** + * Escapes special characters in a command string for safe execution in bash -c + * + * This function addresses gh-aw PR #2493 where parentheses in tool names like + * `--allow-tool 'shell(cat)'` were being interpreted as subshell syntax by bash. + * + * The issue occurs because when gh-aw's workflow compiler passes commands to AWF, + * it wraps them in double quotes. When bash receives these via `bash -c "..."`, + * single quotes inside become literal characters (not delimiters), exposing + * parentheses to be interpreted as subshell syntax. + * + * Special characters that need escaping: + * - ( ) : Subshell/command grouping + * - $ : Variable expansion (also needs doubling for Docker Compose) + * - ` : Command substitution + * - ; : Command separator + * - & : Background execution + * - | : Pipe + * - < > : Redirection + * - \ : Escape character itself + * + * @param command - The command string to escape + * @returns The escaped command string safe for bash -c execution + */ +export function escapeBashCommand(command: string): string { + if (!command) { + return command; + } + + let escaped = command; + + // Escape backslashes first (must be done before other escaping) + escaped = escaped.replace(/\\/g, '\\\\'); + + // Escape parentheses (to prevent subshell interpretation) + escaped = escaped.replace(/\(/g, '\\('); + escaped = escaped.replace(/\)/g, '\\)'); + + // Escape backticks (to prevent command substitution) + escaped = escaped.replace(/`/g, '\\`'); + + // Escape semicolons (to prevent command chaining) + escaped = escaped.replace(/;/g, '\\;'); + + // Escape ampersands (to prevent backgrounding) + escaped = escaped.replace(/&/g, '\\&'); + + // Escape pipes (to prevent piping) + escaped = escaped.replace(/\|/g, '\\|'); + + // Escape redirections + escaped = escaped.replace(//g, '\\>'); + + // Double dollar signs for Docker Compose variable interpolation + // Docker Compose requires $$ to represent a literal $ + // This must be done after other escaping to avoid double-escaping + escaped = escaped.replace(/\$/g, '$$$$'); + + return escaped; +} + /** * Generates Docker Compose configuration * Note: Uses external network 'awf-net' created by host-iptables setup @@ -239,8 +301,8 @@ export function generateDockerCompose( cap_add: ['NET_ADMIN'], // Required for iptables stdin_open: true, tty: false, // Disable TTY to prevent ANSI escape sequences in logs - // Escape $ with $$ for Docker Compose variable interpolation - command: ['/bin/bash', '-c', config.copilotCommand.replace(/\$/g, '$$$$')], + // Escape special bash characters for safe execution (fixes gh-aw PR #2493) + command: ['/bin/bash', '-c', escapeBashCommand(config.copilotCommand)], }; // Use GHCR image or build locally