fix: harden shell injection surfaces (heredoc, shellEscapeArg, YAML env) by lpcox · Pull Request #22987 · github/gh-aw

lpcox · 2026-03-25T21:45:53Z

Security Fixes

Addresses three findings from the security review (github/security-reviews#2067):

#181 — Heredoc delimiter injection (High)

Problem: Static GH_AW_PROMPT_EOF delimiter was predictable; a crafted prompt containing it could close the heredoc and inject shell commands.
Fix: Use crypto/rand to generate 8 random bytes (16 hex chars) per delimiter, format: GH_AW_<NAME>_<HEX>_EOF.

#182 — shellEscapeArg pre-quoted bypass (High)

Problem: shellEscapeArg() had fast-paths that returned pre-quoted strings verbatim without inspecting their interior. ResolveAgentFilePath() produced pre-quoted paths from user input, enabling shell injection via crafted filenames.
Fix:

Remove pre-quoted fast-paths from shellEscapeArg()
Add path validation regex to ResolveAgentFilePath() rejecting shell metacharacters
Separate shell-variable prompt args from shellJoinArgs in all engines

#183 — YAML env value injection (Medium)

Problem: Environment variable values emitted with bare %s or "%s" format verbs; crafted values with newlines or quotes could inject additional YAML keys.
Fix: Use Go %q format verb which produces properly escaped double-quoted strings valid as YAML scalars.

Additional changes

normalizeHeredocDelimiters() in compiler preserves skip-write optimization despite randomized delimiters
Updated tests to use regex matching for randomized delimiter patterns
Regenerated golden test files and 178 lock files

Testing

All unit tests pass (only 3 pre-existing failures unrelated to this PR)
make fmt && make build && make lint pass (only pre-existing 3rd-party lint issues)

Use crypto/rand to generate unique heredoc delimiters for each compilation, preventing workflow authors from injecting matching delimiter strings in prompt content to break out of heredocs and execute arbitrary shell commands. The static delimiter GH_AW_PROMPT_EOF was predictable and could be embedded in a .md prompt to close the heredoc early, with subsequent lines executing as shell with workflow-token privileges. Since lock files are collapsed in PR review (linguist-generated), the payload was effectively invisible to reviewers. Fix: GH_AW_<NAME>_<16_HEX_CHARS>_EOF format makes collision cryptographically infeasible. Addresses github/agentic-workflows#181 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…paths Remove pre-quoted fast-paths from shellEscapeArg() that blindly trusted strings already wrapped in quotes. An attacker could craft a filename containing shell metacharacters (e.g., "; rm -rf /;") that would pass through the bypass unchanged, enabling shell injection. Changes: - Remove pre-quoted fast-paths from shellEscapeArg() (shell.go) - Add path validation regex to ResolveAgentFilePath() that rejects shell metacharacters ($, `, ", ;, |, etc.) - Return (string, error) from ResolveAgentFilePath for validation - Separate shell-variable prompt args from shellJoinArgs in copilot, claude, codex, and gemini engines to avoid single-quoting them - Add normalizeHeredocDelimiters to compiler skip-write comparison to preserve timestamp optimization with randomized delimiters - Update tests for new escaping behavior and randomized delimiters Security: github/agentic-workflows#182 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Use Go's %q format verb for YAML environment variable values to prevent injection via crafted values containing newlines, quotes, or YAML metacharacters. The %q verb produces properly escaped double-quoted strings that are valid YAML scalars. Affected env vars: - GH_AW_ALLOWED_BOTS (compiler_activation_job.go) - GH_AW_REPOSITORY_IMPORTS (compiler_yaml_main_job.go) - GH_AW_AGENT_FILE (compiler_yaml_main_job.go) - GH_AW_AGENT_IMPORT_SPEC (compiler_yaml_main_job.go) Security: github/agentic-workflows#183 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Recompile all 178 workflow lock files to reflect: - Randomized heredoc delimiters (#181) - shellEscapeArg bypass removal (#182) - YAML env %q quoting (#183) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Security hardening changes to reduce shell/YAML injection risk in generated workflows by randomizing heredoc delimiters, tightening shell argument escaping behavior, validating agent file paths, and ensuring YAML env values are properly escaped.

Changes:

Randomize heredoc delimiters per compilation and update tests/golden files to normalize/regex-match delimiters.
Remove shellEscapeArg() pre-quoted fast-paths and adjust engines to append prompt args separately to preserve intended expansions.
Emit YAML env values using %q to prevent multiline/quote injection into generated YAML.

Reviewed changes

Copilot reviewed 76 out of 203 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
pkg/workflow/strings.go	Generates randomized heredoc delimiters using `crypto/rand` + hex.
pkg/workflow/strings_test.go	Updates delimiter tests to validate format/prefix/suffix and uniqueness.
pkg/workflow/compiler.go	Normalizes randomized delimiters for lockfile “skip write if unchanged” optimization.
pkg/workflow/shell.go	Removes pre-quoted bypasses in `shellEscapeArg`; affects escaping/expansion behavior.
pkg/workflow/shell_test.go	Updates expectations to ensure pre-quoted inputs no longer bypass escaping.
pkg/workflow/engine_helpers.go	Adds regex allowlist validation for agent file paths; returns `(string, error)`.
pkg/workflow/engine_helpers_test.go	Updates tests for new `(string, error)` signature and rejection cases.
pkg/workflow/copilot_engine_execution.go	Builds prompt arg separately (raw) to preserve runtime expansion; appends it outside `shellJoinArgs`.
pkg/workflow/gemini_engine.go	Builds prompt arg separately (raw) to preserve command-substitution expansion.
pkg/workflow/claude_engine.go	Uses `ResolveAgentFilePath` with error handling; appends prompt raw outside `shellJoinArgs`.
pkg/workflow/codex_engine.go	Uses `ResolveAgentFilePath` with error handling in both sandbox/non-sandbox paths.
pkg/workflow/codex_engine_test.go	Normalizes randomized heredoc delimiters before comparing expected lines.
pkg/workflow/compiler_yaml_main_job.go	Uses `%q` for env values to prevent YAML injection via unescaped strings.
pkg/workflow/compiler_activation_job.go	Uses `%q` for env values to prevent YAML injection via unescaped strings.
pkg/workflow/xml_comments_test.go	Uses regex matching to count randomized heredoc blocks.
pkg/workflow/wasm_golden_test.go	Normalizes randomized heredoc delimiters prior to golden/determinism comparisons.
pkg/workflow/unified_prompt_creation_test.go	Uses regex matching for randomized heredoc delimiters in assertions.
pkg/workflow/secure_markdown_rendering_test.go	Switches delimiter detection to regex matching.
pkg/workflow/safe_scripts_test.go	Adjusts assertions to accept randomized delimiter prefixes/suffixes.
pkg/workflow/mcp_scripts_mode_test.go	Extracts randomized delimiter via regex rather than re-generating it.
pkg/workflow/heredoc_interpolation_test.go	Updates checks to match randomized quoted/unquoted delimiter patterns.
pkg/workflow/testdata/wasm_golden/TestWasmGolden_CompileFixtures/basic-copilot.golden	Regenerated golden output with randomized delimiters/quoting changes.
pkg/workflow/testdata/wasm_golden/TestWasmGolden_CompileFixtures/smoke-copilot.golden	Regenerated golden output with randomized delimiters/quoting changes.
pkg/workflow/testdata/wasm_golden/TestWasmGolden_CompileFixtures/with-imports.golden	Regenerated golden output with randomized delimiters/quoting changes.
.github/workflows/test-workflow.lock.yml	Regenerated lockfile reflecting randomized delimiters and updated quoting.
.github/workflows/test-dispatcher.lock.yml	Regenerated lockfile reflecting randomized delimiters and updated quoting.
.github/workflows/smoke-call-workflow.lock.yml	Regenerated lockfile reflecting randomized delimiters and updated quoting.
.github/workflows/smoke-agent-scoped-approved.lock.yml	Regenerated lockfile reflecting randomized delimiters and updated quoting.
.github/workflows/smoke-agent-public-none.lock.yml	Regenerated lockfile reflecting randomized delimiters and updated quoting.
.github/workflows/smoke-agent-public-approved.lock.yml	Regenerated lockfile reflecting randomized delimiters and updated quoting.
.github/workflows/smoke-agent-all-none.lock.yml	Regenerated lockfile reflecting randomized delimiters and updated quoting.
.github/workflows/smoke-agent-all-merged.lock.yml	Regenerated lockfile reflecting randomized delimiters and updated quoting.
.github/workflows/notion-issue-summary.lock.yml	Regenerated lockfile reflecting randomized delimiters and updated quoting.
.github/workflows/metrics-collector.lock.yml	Regenerated lockfile reflecting randomized delimiters and updated quoting.
.github/workflows/gpclean.lock.yml	Regenerated lockfile reflecting randomized delimiters and updated quoting.
.github/workflows/firewall.lock.yml	Regenerated lockfile reflecting randomized delimiters and updated quoting.
.github/workflows/example-permissions-warning.lock.yml	Regenerated lockfile reflecting randomized delimiters and updated quoting.
.github/workflows/dev.lock.yml	Regenerated lockfile reflecting randomized delimiters and updated quoting.
.github/workflows/daily-observability-report.lock.yml	Regenerated lockfile reflecting randomized delimiters and updated quoting.
.github/workflows/daily-malicious-code-scan.lock.yml	Regenerated lockfile reflecting randomized delimiters and updated quoting.
.github/workflows/daily-fact.lock.yml	Regenerated lockfile reflecting randomized delimiters and updated quoting.
.github/workflows/daily-assign-issue-to-user.lock.yml	Regenerated lockfile reflecting randomized delimiters and updated quoting.
.github/workflows/codex-github-remote-mcp-test.lock.yml	Regenerated lockfile reflecting randomized delimiters and updated quoting.
.github/workflows/bot-detection.lock.yml	Regenerated lockfile reflecting randomized delimiters and updated quoting.
.github/workflows/ai-moderator.lock.yml	Regenerated lockfile reflecting randomized delimiters and updated quoting.
.github/workflows/ace-editor.lock.yml	Regenerated lockfile reflecting randomized delimiters and updated quoting.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-25T21:50:54Z

pkg/workflow/claude_engine.go

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


On invalid AgentFile values, the engine logs an error and returns an empty step list. That can lead to generating a workflow that silently omits the execution step(s), which is hard to diagnose and may produce an invalid/partial lock file. Prefer surfacing this as a compile-time error (propagate an error up), or emit an explicit failing step with a clear message so the workflow fails fast.

Copilot · 2026-03-25T21:50:55Z

pkg/workflow/codex_engine.go

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


On invalid AgentFile values, this returns an empty step list after logging. That risks producing workflows that compile but omit execution entirely. Prefer propagating an error to the compiler or generating an explicit failing step so users get a clear failure instead of a silent no-op.

Copilot · 2026-03-25T21:50:55Z

pkg/workflow/shell.go

+// 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 the argument is already properly quoted with double quotes, leave it as-is
-	if len(arg) >= 2 && arg[0] == '"' && arg[len(arg)-1] == '"' {
-		shellLog.Print("Argument already double-quoted, leaving as-is")
-		return arg
-	}
-
-	// If the argument is already properly quoted with single quotes, leave it as-is
-	if len(arg) >= 2 && arg[0] == '\'' && arg[len(arg)-1] == '\'' {
-		shellLog.Print("Argument already single-quoted, leaving as-is")
-		return arg
-	}
-
 	// Check if the argument contains special shell characters that need escaping
 	if strings.ContainsAny(arg, "()[]{}*?$`\"'\\|&;<> \t\n") {
 		shellLog.Print("Argument contains special characters, applying escaping")
 		// Handle single quotes in the argument by escaping them
-		// Use '\'' instead of '\"'\"' to avoid creating double-quoted contexts
-		// that would interpret backslash escape sequences
 		escaped := strings.ReplaceAll(arg, "'", "'\\''")
 		return "'" + escaped + "'"
 	}


shellEscapeArg now single-quotes any argument containing $ or ", which prevents intended runtime expansion for strings like "${GITHUB_WORKSPACE}" / "${RUNNER_TEMP}" when they’re passed through shellJoinArgs (e.g., AWF args in BuildAWFArgs and Copilot’s --add-dir "${GITHUB_WORKSPACE}"). This results in commands receiving literal ${GITHUB_WORKSPACE} (often with embedded quotes) instead of the expanded path. Consider introducing an explicit “raw shell fragment / needs-expansion” pathway (similar to buildDockerCommandWithExpandableVars) for trusted variable expansions, or refactoring callers to avoid routing expansion-dependent args through shellEscapeArg.

Copilot · 2026-03-25T21:50:55Z

pkg/workflow/copilot_engine_execution.go

@@ -163,7 +166,8 @@ func (e *CopilotEngine) GetExecutionSteps(workflowData *WorkflowData, logFile st
 	}

 	// Build the command - model is always passed via COPILOT_MODEL env var (see env block below)
-	copilotCommand = fmt.Sprintf("%s %s", commandName, shellJoinArgs(copilotArgs))
+	// promptArg is appended raw (not through shellJoinArgs) to preserve variable expansion
+	copilotCommand = fmt.Sprintf("%s %s %s", commandName, shellJoinArgs(copilotArgs), promptArg)


The prompt arg is now appended outside shellJoinArgs (good), but other args in copilotArgs still rely on shell variable expansion (notably --add-dir "${GITHUB_WORKSPACE}" in sandbox mode). With the updated shellEscapeArg behavior, those args will be single-quoted and ${GITHUB_WORKSPACE} won’t expand, breaking Copilot’s workspace access. These expansion-dependent args likely need the same “append raw” treatment (or another safe expansion-preserving mechanism) as promptArg.

Copilot · 2026-03-25T21:50:55Z

.github/workflows/test-workflow.lock.yml

+          sudo -E awf --env-all --container-workdir '"${GITHUB_WORKSPACE}"' --mount '"${RUNNER_TEMP}/gh-aw:${RUNNER_TEMP}/gh-aw:ro"' --mount '"${RUNNER_TEMP}/gh-aw:/host${RUNNER_TEMP}/gh-aw:ro"' --allow-domains '"api.business.githubcopilot.com,api.enterprise.githubcopilot.com,api.github.com,api.githubcopilot.com,api.individual.githubcopilot.com,api.snapcraft.io,archive.ubuntu.com,azure.archive.ubuntu.com,crl.geotrust.com,crl.globalsign.com,crl.identrust.com,crl.sectigo.com,crl.thawte.com,crl.usertrust.com,crl.verisign.com,crl3.digicert.com,crl4.digicert.com,crls.ssl.com,github.com,host.docker.internal,json-schema.org,json.schemastore.org,keyserver.ubuntu.com,ocsp.digicert.com,ocsp.geotrust.com,ocsp.globalsign.com,ocsp.identrust.com,ocsp.sectigo.com,ocsp.ssl.com,ocsp.thawte.com,ocsp.usertrust.com,ocsp.verisign.com,packagecloud.io,packages.cloud.google.com,packages.microsoft.com,ppa.launchpad.net,raw.githubusercontent.com,registry.npmjs.org,s.symcb.com,s.symcd.com,security.ubuntu.com,telemetry.enterprise.githubcopilot.com,ts-crl.ws.symantec.com,ts-ocsp.ws.symantec.com,www.googleapis.com"' --log-level info --proxy-logs-dir /tmp/gh-aw/sandbox/firewall/logs --audit-dir /tmp/gh-aw/sandbox/firewall/audit --enable-host-access --image-tag 0.25.0 --skip-pull --enable-api-proxy \
+            -- /bin/bash -c '/usr/local/bin/copilot --add-dir /tmp/gh-aw/ --log-level all --log-dir /tmp/gh-aw/sandbox/agent/logs/ --add-dir '\''"${GITHUB_WORKSPACE}"'\'' --disable-builtin-mcps --allow-all-tools --allow-all-paths --prompt "$(cat /tmp/gh-aw/aw-prompts/prompt.txt)"' 2>&1 | tee -a /tmp/gh-aw/agent-stdio.log
        env:


In this generated workflow, AWF arguments like --container-workdir '"${GITHUB_WORKSPACE}"' and --mount '"${RUNNER_TEMP}/..."' are single-quoted, so ${GITHUB_WORKSPACE} / ${RUNNER_TEMP} will not expand at runtime and AWF will receive literal strings (including quote characters). This will likely break sandbox execution; the generator should emit args in a form that preserves variable expansion (e.g., plain double quotes, or the quote-breaking pattern used in buildDockerCommandWithExpandableVars).

pelikhan · 2026-03-25T21:59:44Z

You have to be super careful with those because we have multiple layers of templating at play. github actions renderer, then shell, etc...

pelikhan · 2026-03-25T22:13:32Z

@lpcox always split findings in separate issues. Giving multiple goals to the agent leads to worse performance and it is easier to review.

lpcox and others added 4 commits March 25, 2026 14:18

chore: recompile all workflows with security fixes

5868cf6

Recompile all 178 workflow lock files to reflect: - Randomized heredoc delimiters (#181) - shellEscapeArg bypass removal (#182) - YAML env %q quoting (#183) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI review requested due to automatic review settings March 25, 2026 21:45

Copilot started reviewing on behalf of lpcox March 25, 2026 21:46 View session

Copilot AI reviewed Mar 25, 2026

View reviewed changes

github-actions bot mentioned this pull request Mar 25, 2026

Smoke Test: Claude - 23568997760 #22999

Open

lpcox closed this Mar 25, 2026

lpcox mentioned this pull request Mar 26, 2026

Heredoc delimiter injection enables code-review bypass → RCE #23003

Open

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: harden shell injection surfaces (heredoc, shellEscapeArg, YAML env)#22987

fix: harden shell injection surfaces (heredoc, shellEscapeArg, YAML env)#22987
lpcox wants to merge 4 commits intomainfrom
security/fix-heredoc-delimiter-injection

lpcox commented Mar 25, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 25, 2026

Uh oh!

Copilot AI Mar 25, 2026

Uh oh!

Copilot AI Mar 25, 2026

Uh oh!

Copilot AI Mar 25, 2026

Uh oh!

Copilot AI Mar 25, 2026

Uh oh!

pelikhan commented Mar 25, 2026

Uh oh!

pelikhan commented Mar 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

lpcox commented Mar 25, 2026

Security Fixes

#181 — Heredoc delimiter injection (High)

#182 — shellEscapeArg pre-quoted bypass (High)

#183 — YAML env value injection (Medium)

Additional changes

Testing

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

pelikhan commented Mar 25, 2026

Uh oh!

pelikhan commented Mar 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants