shellEscapeArg pre-quoted bypass → shell injection via AgentFile path

[lpcox]

Summary

shellEscapeArg contains a fast-path that treats any string starting and ending with " as "already safely quoted" and returns it verbatim. The check inspects only arg[0] and arg[len(arg)-1] — the interior is never validated. A string like "a"; id; "b" passes this check and is emitted into generated bash unescaped.

ResolveAgentFilePath deliberately produces output in this shape to trigger the bypass, and its agentFile parameter is derived from a filesystem path under .github/agents/. POSIX filesystems permit ", $, `, ;, | in filenames, so a committed file with a crafted name reaches the Codex/Claude engine command lines as live shell.

Upstream tracking: github/agentic-workflows#182
Severity: High — shell injection primitive; concretely exploitable via agent-file import

Affected Code

What	File	Lines
shellEscapeArg pre-quoted bypass	pkg/workflow/shell.go	L26-36 (two if fast-paths)
ResolveAgentFilePath (no interior escaping)	pkg/workflow/engine_helpers.go	L110-134
Codex embeds path in awk command (AWF path)	pkg/workflow/codex_engine.go	~L240-242
Codex embeds path in awk command (non-AWF path)	pkg/workflow/codex_engine.go	~L264-267
Claude embeds path in awk command	pkg/workflow/claude_engine.go	~L245-251
Copilot prompt arg through shellJoinArgs	pkg/workflow/copilot_engine_execution.go	~L121-126
Gemini prompt arg through shellJoinArgs	pkg/workflow/gemini_engine.go	~L223-224

Root Cause

There are two interrelated problems:

Problem 1: shellEscapeArg pre-quoted bypass

// shell.go:26-36
func shellEscapeArg(arg string) string {
    // If the argument is already properly quoted with double quotes, leave it as-is
    if len(arg) >= 2 && arg[0] == ' && arg[len(arg)-1] == ' {
        return arg    // ← interior never inspected
    }
    // If the argument is already properly quoted with single quotes, leave it as-is
    if len(arg) >= 2 && arg[0] == ' && arg[len(arg)-1] == ' {
        return arg    // ← interior never inspected
    }
    ...
}

Problem 2: ResolveAgentFilePath intentionally triggers the bypass

// engine_helpers.go:132-134
func ResolveAgentFilePath(agentFile string) string {
    return fmt.Sprintf("\"${GITHUB_WORKSPACE}/%s\"", agentFile)  // no interior escaping
}

The comment at engine_helpers.go:129 documents the bypass as intended behaviour: "The shellEscapeArg function recognizes it as already-quoted and doesn't add single quotes."

Problem 3: Engine prompt args also rely on the bypass

All four engines (copilot, claude, codex, gemini) pass pre-quoted shell variable references like "$COPILOT_CLI_INSTRUCTION" through shellJoinArgs → shellEscapeArg, relying on the bypass to preserve variable expansion. When the bypass is removed, these must be separated from the escaped args.

Steps to Reproduce

1. Create an agent file with shell metacharacters in its name:

   mkdir -p '.github/agents'
   cat > '.github/agents/a";id>&2;"b.md' <<'EOF'
   ---
   name: test
   ---
   Agent body.
   EOF

2. Create .github/workflows/poc.md:

   ---
   on: workflow_dispatch
   engine: codex
   imports:
     - '.github/agents/a";id>&2;"b.md'
   ---
   Hello.

3. Compile: gh aw compile .github/workflows/poc.md

4. Inspect the .lock.yml — the id command executes as shell before the agent starts.

Recommended Solution

This was prototyped and validated in closed PR #22987 (commit c19a4340a0). The fix has three parts:

Part 1: Remove pre-quoted fast-paths from shellEscapeArg (pkg/workflow/shell.go)

Delete both if blocks that check for pre-quoted strings (lines ~26-36). The function should always inspect and escape content:

// shellEscapeArg escapes a single argument for safe use in shell commands.
// Arguments containing special characters are wrapped in single quotes.
// Interior content is always validated — no pre-quoted fast-paths.
func shellEscapeArg(arg string) string {
    if strings.ContainsAny(arg, "()[]{}*?$`\"'\\|&;<> \t\n") {
        escaped := strings.ReplaceAll(arg, "'", "'\\''")
        return "'" + escaped + "'"
    }
    return arg
}

Part 2: Validate agent file paths in ResolveAgentFilePath (pkg/workflow/engine_helpers.go)

Add a regex validation allowlist and change the return signature to (string, error):

var agentFilePathRE = regexp.MustCompile(`^[A-Za-z0-9._/ -]+$`)

func ResolveAgentFilePath(agentFile string) (string, error) {
    if !agentFilePathRE.MatchString(agentFile) {
        return "", fmt.Errorf("agent file path contains disallowed characters: %q", agentFile)
    }
    // The path is validated against agentFilePathRE which only allows [A-Za-z0-9._/ -].
    // These characters are all safe inside double quotes, so we can wrap directly
    // without shellDoubleQuoteArg (which would escape ${GITHUB_WORKSPACE}).
    return fmt.Sprintf("\"${GITHUB_WORKSPACE}/%s\"", agentFile), nil
}

Update all callers (claude_engine.go, codex_engine.go — 3 call sites total) to handle the error:

agentPath, err := ResolveAgentFilePath(workflowData.AgentFile)
if err != nil {
    engineLog.Printf("Error: %v", err)
    return []GitHubActionStep{}
}

Part 3: Separate prompt args from shellJoinArgs in all engines

Shell variable references like "$COPILOT_CLI_INSTRUCTION" and "$(cat ...)" must NOT go through shellEscapeArg (which would now single-quote them, preventing expansion). Instead, append them raw after shellJoinArgs:

copilot_engine_execution.go:

// Before: copilotArgs = append(copilotArgs, "--prompt", "\"$COPILOT_CLI_INSTRUCTION\"")
// After:
var promptArg string
if sandboxEnabled {
    promptArg = `--prompt "$(cat /tmp/gh-aw/aw-prompts/prompt.txt)"`
} else {
    promptArg = `--prompt "$COPILOT_CLI_INSTRUCTION"`
}
copilotCommand = fmt.Sprintf("%s %s %s", commandName, shellJoinArgs(copilotArgs), promptArg)

claude_engine.go:

// Before: commandParts = append(commandParts, promptCommand); claudeCommand := shellJoinArgs(commandParts)
// After:
claudeCommand := shellJoinArgs(commandParts) + " " + promptCommand

gemini_engine.go:

// Before: geminiArgs = append(geminiArgs, "--prompt", "\"$(cat ...)\"")
// After:
promptArg := `--prompt "$(cat /tmp/gh-aw/aw-prompts/prompt.txt)"`
geminiCommand := fmt.Sprintf("%s %s %s", commandName, shellJoinArgs(geminiArgs), promptArg)

codex_engine.go: No change needed — codex builds "$INSTRUCTION" directly in a format string, not through shellJoinArgs.

Test updates required

pkg/workflow/shell_test.go:

• Update test cases that expected pre-quoted bypass behavior:
  • "\"$INSTRUCTION\"" → now expects '\"$INSTRUCTION\"' (single-quoted)
  • "'hello world'" → now expects ''\\''hello world'\\''' (escaped)
  • "\"\"" → now expects '\"\"' (escaped)
• Update shellJoinArgs test similarly

pkg/workflow/engine_helpers_test.go:

• TestResolveAgentFilePath — change to test (string, error) return; add test cases for rejected paths ($, `, ", ;, |, \n)
• TestShellEscapeArgWithFullyQuotedAgentPath — remove (tested the bypass that no longer exists)
• TestShellVariableExpansionInAgentPath — update for new signature

Golden files need regeneration due to prompt arg format changes:

go test -v ./pkg/workflow -run='^TestWasmGolden_CompileFixtures' -update
make build && make recompile

Prior Work

• PR fix: harden shell injection surfaces (heredoc, shellEscapeArg, YAML env) #22987 (closed, not merged) implemented and validated this fix. Commit c19a4340a0 contains the complete diff for this finding.
• That PR combined three fixes (start to split apart compiler.go a little #181, "gh aw run" reports previous workflow run, not latest #182, Fix test-agentics workflow to use compiled gh-aw #183). This issue covers only "gh aw run" reports previous workflow run, not latest #182.
• Some test file changes in PR fix: harden shell injection surfaces (heredoc, shellEscapeArg, YAML env) #22987 overlap with start to split apart compiler.go a little #181 (heredoc delimiter randomization). If start to split apart compiler.go a little #181 is implemented first, some test updates here will already be done.

Checklist

• Remove pre-quoted fast-paths from shellEscapeArg() in shell.go
• Add path validation regex to ResolveAgentFilePath() in engine_helpers.go
• Change ResolveAgentFilePath return to (string, error) and update 3 callers
• Separate prompt args from shellJoinArgs in copilot, claude, gemini engines
• Update shell_test.go for new escaping behavior
• Update engine_helpers_test.go for new ResolveAgentFilePath signature
• Regenerate wasm golden files and recompile lock files
• Run make agent-finish to validate

[lpcox]

@pelikhan labeled high-severity