[pelikhan]

Why

Addresses HIGH code-scanning alert 1057 (CWE-94). The compiler stripped unquoted heredoc bodies before sanitization and validation, even though Bash performs command substitution in those bodies. Attacker-controlled Actions expressions could therefore bypass both controls.

Approach

• Use the existing Bash parser to share executable-region boundaries across the sanitizer, parsed validator, raw-YAML validator, and steps-run-secrets-to-env source codemod. Mask Actions expressions without changing byte offsets.
• Scan expanding heredoc bodies, including hash-prefixed and quote-looking lines. Preserve only proven quoted bodies; follow exact delimiters, supported quotation, nested/multiple heredocs, continuations, and <<- tab stripping. Unsupported or mismatched boundaries leave sanitization unchanged and grant no validation exemption; source migration reports a guided error without partial workflow writes.
• Transport MCP configuration expressions through deterministic full-SHA256 step-environment bindings. Encode JSON before shell escaping, then resolve placeholders after JSON parsing and before URL validation. Runtime values remain data rather than becoming shell source.
• Retain the runtime fail-closed guard against the pinned gateway's subsequent ${NAME} expansion, including references introduced by dynamic values or assembled across substitution boundaries. Reject statically visible transported/gateway mixtures during compilation; emit counted, value-free warnings for unsupported shell computations and parameter operators.
• Update affected configuration/locks, migration guidance, release notes, ADRs 28998 and 44454, and compiler threat specification 1.0.82. The ADRs distinguish historical performance measurements from the current implementation and explicitly bound security and compatibility claims.

Examples

Expanding heredoc: move untrusted expressions out of shell source

Authored step:

- name: Write issue title
  run: |
    cat <<EOF > title.txt
    # ${{ github.event.issue.title }}
    EOF

A title containing the harmless demonstration payload $(printf injected) previously executed command substitution. The leading # is heredoc data, not a shell comment. Compilation now produces:

- name: Write issue title
  env:
    GH_AW_GITHUB_EVENT_ISSUE_TITLE: ${{ github.event.issue.title }}
  run: |
    cat <<EOF > title.txt
    # $GH_AW_GITHUB_EVENT_ISSUE_TITLE
    EOF

Bash does not recursively execute command substitutions contained in an expanded variable, so the demonstration payload is written literally. The source codemod applies the same heredoc boundaries when migrating authored steps.

Quoted heredoc: preserve proven literal bodies, without a blanket safety claim

cat <<'CUSTOM_END'
$HOME $(printf not-executed)
CUSTOM_END

A proven quoted delimiter suppresses shell expansion, so this body remains unchanged. Complete delimiter matching matters; <<- strips leading tabs, not spaces. Unsupported quotation or parser/Bash boundary disagreements receive conservative handling rather than an exemption.

Quoted heredocs are not a general transport for arbitrary untrusted Actions values: Actions interpolation happens before Bash parsing, and a multiline value can inject a closing delimiter. Prefer environment indirection for untrusted values.

Mismatched heredoc: validation cannot be bypassed

cat <<'FIRST_EOF'
${{ github.event.issue.title }}
SECOND_EOF

SECOND_EOF does not close FIRST_EOF. Sanitization leaves an unproven script unchanged rather than speculatively rewriting quoted data, and validation scans the entire script and rejects the remaining expression. The source codemod reports a guided manual-fix diagnostic and writes no partial changes to that workflow.

MCP expressions: resolve configuration as parsed data

mcp-servers:
  remote:
    type: http
    url: "http://localhost:${{ steps.setup.outputs.port }}/mcp"
    headers:
      X-Ref: "${{ github.ref }}"

The compiler emits GH_AW_MCP_EXPR_<SHA256> gateway-step bindings instead of interpolating these values into the JSON-writing shell heredoc. Startup substitutes them once after JSON parsing. Missing compiler bindings fail explicitly. Simple unescaped runner $NAME/${NAME} references use a separate transport path; escaped gateway references remain deferred.

If a complete string receiving a transported substitution still contains ${NAME}, startup rejects it to prevent the gateway from re-expanding runtime data into credentials. Compile-time errors catch authored mixtures that can be proven statically; the runtime guard remains necessary for dynamic and boundary-assembled references. Gateway-only passthrough remains supported.

Generated mounts follow this same data-transport path. For example, the former "${GITHUB_WORKSPACE}:${GITHUB_WORKSPACE}:ro" becomes "\${GH_AW_MCP_ENV_GITHUB_WORKSPACE}:\${GH_AW_MCP_ENV_GITHUB_WORKSPACE}:ro" in the shell heredoc. The former environment-variable expansion was not itself a command-injection exploit: Bash does not recursively execute substitutions inside an expanded variable. Its serialization weakness was inserting a path before JSON parsing without JSON escaping. Resolving placeholders after parsing preserves the same ordinary read-only mount without allowing quotes, backslashes, or newlines in the value to alter JSON structure.

MCP migration: warn instead of silently relying on shell computation

An authored ${WORKSPACE_FALLBACK:-/tmp} remains literal configuration data and now produces a path-specific compiler warning. When the input is available on the runner, an Actions expression can compute the default:

args:
  - "${{ env.WORKSPACE_FALLBACK || '/tmp' }}"

Otherwise, compute the value in a setup step and reference its output. The codemod does not guess whether a deferred gateway variable exists on the runner or promise to escape arbitrary placeholder-shaped runtime values. General single-quoted shell strings still require quote review. Regenerated locks must be deployed with the matching setup runtime.

Review decision: legacy environment references

The legacy-placeholder allowlist suggestion was evaluated and declined; the thread is resolved. GH_AW_MCP_ENV_<NAME> preserves explicitly authored runner $NAME/${NAME} expansion, including the existing empty-string behavior for missing variables. It is not a new secret-access policy: workflow authors already control Actions secret/environment bindings and supported gateway credential references. A default-deny allowlist applied only to legacy placeholders would break supported generated configurations without restricting those existing credential paths.

The relevant boundary is untrusted runtime data, not credential choices explicitly authored in a trusted workflow. Substitution remains single-pass; inserted values cannot introduce a second placeholder-resolution pass, and complete strings containing ${NAME} after substitution are rejected before gateway re-expansion. The review follow-up reran all 192 transport/gateway tests and synthetic checks for injected legacy placeholders. No code change was required for that review.

Validation and known baseline blockers

The compatibility implementation is published in 46e8d162c56c11b521ff2ea130dcb64239588445; the ADR clarification and current branch head are 6235c02c35c538fe6556433bb50183dc5a97efaa.

Passed locally: build/formatting, focused and impacted Go tests, all 192 runtime tests, 19,879 heredoc-codemod fuzz executions, standard Go lint (0 issues), JavaScript lint, and schema freshness. The latest local-only recompilation covered 348 workflow inputs, produced no lock changes, and left all 10 upstream-managed source/lock files unchanged. This scoped check avoids regenerating provenance-managed workflows; it is not a claim that the unrestricted full drift gate passed.

The publication gate remains blocked by 80 existing custom-Go-lint diagnostics. An identical-analyzer comparison against the compatibility work's starting revision 5b13ebe4d9147159263d322948ad476a3baba57a found 81 baseline findings: all 80 remaining findings match the file, diagnostic message, and source statement, with zero introduced findings. The ADR publication check reproduced those same 80 findings; JavaScript lint passed with two warnings in unchanged runtime lines.

Three broader TestKVMAccessExecutesAgainstFixtures ACL subtests fail on macOS because setfacl is unavailable; all three reproduce on that same starting revision. Other Bash/TMPDIR host issues were eliminated using Homebrew Bash and a canonical temporary path. No introduced relevant failure remains. These are local results, not a claim about current hosted CI or scanner closure.

No alert was dismissed, no GitHub Actions workflow was dispatched by this session, and no upstream-managed workflow source or lock was manually changed.

---

Changeset

• Type: patch
• Description: Prevent template injection through expanding shell heredocs and transport MCP configuration expressions as data.

Generated by 📋 Changeset Generator for #73888 · codex · gpt56 · ~23 AIC · ⌖ ~32 AIC · ⊞ 17.8K · ◷

---

---

✨ PR Review Safe Output Test - Run 41377794908

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

• update.googleapis.com

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "update.googleapis.com"

See Network Configuration for more information.

💥 [THE END] — Illustrated by Smoke Claude · claude · sonnet46 · ~72 AIC · ⌖ ~5 AIC · ⊞ 965 · ◷
_{Comment /smoke-claude to run again}

---

---

✨ PR Review Safe Output Test - Run 41389447673

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

• update.googleapis.com

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "update.googleapis.com"

See Network Configuration for more information.

💥 [THE END] — Illustrated by Smoke Claude · claude · sonnet46 · ~84 AIC · ⌖ ~12 AIC · ⊞ 965 · ◷
_{Comment /smoke-claude to run again}

[github-actions]

✅ PR Code Quality Reviewer completed the code quality review.

🔎 Code quality review by PR Code Quality Reviewer

[github-actions]

✅ Ponytail Reviewer completed successfully!

Ponytail Reviewer

Outcome: reported_no_action

The agent completed and reported no action. A no-op output alone does not verify that all required work was completed or that publication was possible.

Agent messages are omitted from this comment. The category uses structured run status, not interpretation of agent prose.

Generated from Ponytail Reviewer · gpt56 · ~15 AIC · ⌖ ~8 AIC · ⊞ 15.1K · ◷

Generated by Ponytail Reviewer for #73888

[github-actions]

✅ Design Decision Gate 🏗️ completed the design decision gate check. See the comment below for the result and any generated ADR draft.

🏗️ ADR gate enforced by Design Decision Gate 🏗️

[github-actions]

✅ Test Quality Sentinel completed test quality analysis.

Test Quality Sentinel

Outcome: reported_no_action

The agent completed and reported no action. A no-op output alone does not verify that all required work was completed or that publication was possible.

Agent messages are omitted from this comment. The category uses structured run status, not interpretation of agent prose.

Generated from Test Quality Sentinel · haiku45 · ~12 AIC · ⌖ ~11 AIC · ◇ ~4 AIC · ⊞ 9.2K · ◷

🧪 Test quality analysis by Test Quality Sentinel

[github-actions]

🧠 Matt Pocock Skills Reviewer is reviewing this pull request using Matt Pocock's engineering skills...

[github-actions]

Review: comment

Found one correctness/compatibility issue in the parse-failure fallback; an inline comment is attached. The specialist pass was unavailable because its file-list tool invocation was rejected, so this review is based on the capped diff and targeted source inspection only.

🔎 Code quality review by PR Code Quality Reviewer · copilot · gpt50 · ~2 AIC · ⌖ ~1 AIC · ⊞ 1.7K
_{Comment /review to run again}

[github-actions]

✅ Design Decision Gate — ADR Verified

ADRs reviewed: ADR-28998 (link unavailable: invalid HTTPS URL) and ADR-44454 (link unavailable: invalid HTTPS URL) — the implementation aligns with the stated decisions. Great work! 🏗️

📋 Verification Summary

• pkg/workflow/run_step_sanitizer.go now scans executableShellContent, extracting expressions from expanding/unquoted heredocs while preserving quoted bodies, matching ADR-28998.
• pkg/workflow/template_injection_utils.go uses the Bash parser to identify heredoc regions; pkg/workflow/template_injection_validation.go shares that filtering for parsed and raw-YAML paths, including exact delimiters, <<-, multiple redirections, and conservative parse failures, matching ADR-44454.
• pkg/workflow/template_injection_heredoc_test.go plus the updated validation and fuzz tests cover these boundaries and the command-substitution regression.

🏗️ ADR gate enforced by Design Decision Gate 🏗️ · pi · routed: gpt56 max · ~3 AIC · ⌖ ~1 AIC · ⊞ 7.7K · ◷
_{Comment /review to run again}

[Copilot]

🟡 Changes recommended

Heredoc and quote-state bugs can bypass sanitization and validation, enabling command injection, while existing MCP configuration expressions now fail compilation.

5 open issues

• Verify Bash-compatible boundaries for quoted delimiters · New
• Handle backslash-newline in heredoc delimiter checks · New
• Preserve outer quote state when filtering comments · New
• Pass MCP configuration expressions through step environment · New
• Preserve nested quoted heredoc bodies · New

What changed in this PR

This PR hardens gh-aw’s run-script sanitization and validation against template injection in expanding heredocs, while retaining quoted-heredoc compatibility.

Changes:

• Replace delimiter heuristics with shared Bash parsing and conservative handling of parse failures.
• Add cross-path regressions and a command-substitution reproduction; update related architecture decisions.
• Move Agy MCP configuration to gateway environment variables and regenerate affected workflow locks.

File	Description
pkg/​workflow/​template_injection_validation.go	Share heredoc filtering across raw-YAML and parsed validators.
pkg/​workflow/​template_injection_validation_test.go	Update expectations for expanding heredoc bodies.
pkg/​workflow/​template_injection_utils.go	Add parser-based heredoc regions and shared transformations.
pkg/​workflow/​template_injection_heredoc_test.go	Add cross-path regressions and a Bash reproduction.
pkg/​workflow/​run_step_sanitizer.go	Include unquoted heredoc expressions in sanitization.
pkg/​workflow/​run_step_sanitizer_fuzz_test.go	Check executable content and malformed-expression rejection.
pkg/​workflow/​lock_file_template_injection_test.go	Apply shared filtering to generated-lock checks.
docs/​adr/​44454-skip-heredoc-content-in-run-block-expression-scan.md	Revise the heredoc-scanning decision.
docs/​adr/​28998-auto-extract-shell-injection-expressions-into-env-vars.md	Clarify extraction rules and quoted-heredoc limitations.
.github/​workflows/​smoke-agy.lock.yml	Regenerate gateway configuration and workflow hashes.
.github/​workflows/​shared/​agy-conformance.md	Replace inline MCP expressions with gateway variables.
.github/​workflows/​engine-conformance-agy.lock.yml	Refresh gateway configuration, hashes, and worker contracts.
.github/​workflows/​agy-conformance-reusable.lock.yml	Regenerate reusable Agy gateway configuration and hashes.

_{Review effort: Max (auto)}

[pelikhan]

Regarding #73888 (comment): this completion notice requires no separate code change. The linked code-quality review's parse-failure compatibility finding has been accepted and fixed.

[pelikhan]

Regarding #73888 (comment): no code change is needed because the reviewer reported no actionable finding. This notice is not being treated as validation evidence; the fixes are covered by explicit regression tests.

[pelikhan]

Regarding #73888 (comment): this gate-completion notice requires no separate implementation change. The related ADRs have been updated to document fail-closed boundary handling, nested quoted bodies and whole-script comment context.

[pelikhan]

Regarding #73888 (comment): no code change is requested by this reported-no-action result. New regressions explicitly cover the review's parser boundaries, quote state, nested bodies, logging and MCP expression transport.

[pelikhan]

Regarding #73888 (comment): this is a review-progress notification, not an actionable request, so no code change is needed for it. The concrete inline findings supplied for this batch have been addressed separately.

[pelikhan]

Regarding #73888 (comment): the saved parse-failure theme is addressed by leaving scripts with unproven boundaries unchanged and rejecting their remaining expressions during validation. No direct edit to the automation-managed memory is needed.

[pelikhan]

Regarding #73888 (comment): no separate code change is requested by this ADR verification result. The follow-up corrections and their compatibility limits are now reflected in the same ADRs and covered by focused tests.

[pelikhan]

Regarding #73888 (comment): This earlier mixed-result disposition remains satisfied: the failed web-fetch check prevents an all-green interpretation. No newly reproducible compiler defect or unrelated web-fetch change is established here.

[pelikhan]

Regarding #73888 (comment): This historical changeset acknowledgement remains satisfied and adds no implementation request. Its branch-inclusion statement is prior author context, not a new commit-ancestry check in this reply.

[pelikhan]

Regarding #73888 (comment): This prior historical-smoke disposition remains satisfied and needs no code change. Its PASS label still does not replace current focused validation.

[pelikhan]

Regarding #73888 (comment): This prior Codex disposition remains satisfied, retaining failures, skips, and the threat warning rather than calling completion a pass. The cited diagnostics are earlier author evidence, not newly inspected causes here, and no output instructions will be followed.

[pelikhan]

Regarding #73888 (comment): This earlier Gemini acknowledgement remains satisfied and the result remains FAIL. Its stated log explanations are prior author observations, not newly verified causes here, and no unrelated tool or policy changes follow.

[pelikhan]

Regarding #73888 (comment): This earlier AOAI acknowledgement remains satisfied and its smoke result remains failed, not validation evidence. The log explanations are prior author observations rather than newly verified causes, and policy will not be bypassed.

[pelikhan]

Regarding #73888 (comment): This earlier Claude acknowledgement remains satisfied because the detailed result is PARTIAL, not every-tool-passed. No blocked-domain allowlist expansion or new code is warranted by the acknowledgement.

[pelikhan]

Regarding #73888 (comment): This prior Claude disposition remains satisfied, retaining GraphQL-unavailable and skipped-close limitations. These historical integration results require no additional compiler or network-policy change.

[pelikhan]

Regarding #73888 (comment): This historical Smoke OTEL completion notice requests no code change. Successful job completion does not validate the current review-fix revision.

[pelikhan]

Regarding #73888 (comment): This Gemini startup notification is progress information, not an actionable finding or passing test result. No code change or rerun follows from it.

[pelikhan]

Regarding #73888 (comment): This container-tool startup notice reports that validation began, not that any tool check passed. It requests no compiler change and does not authorize rerunning automation.

[pelikhan]

Regarding #73888 (comment): I acknowledge Codex completion, but the detailed report for this run records FAIL with failed and skipped checks. Completion is not a pass and establishes no separate compiler change.

[pelikhan]

Regarding #73888 (comment): This Pi initialization notice requests no implementation change. Its later detailed report contains a failed web-fetch check, so startup is not evidence of success.

[pelikhan]

Regarding #73888 (comment): I acknowledge Claude completion but retain the later detailed PARTIAL result for this run. The blocked-domain suggestion does not justify expanding network access.

[pelikhan]

Regarding #73888 (comment): This historical Smoke Copilot Auto completion notification needs no code change. It is not verification of the current review-fix revision.

[pelikhan]

Regarding #73888 (comment): This generated heredoc poem contains no actionable review request. Its security description is not validation evidence and requires no code change.

[pelikhan]

Regarding #73888 (comment): I retain the individual check results but not the overall PASS label because Web Fetch Testing is marked failed. This historical mixed result does not establish a compiler regression or justify unrelated tool changes.

[pelikhan]

Regarding #73888 (comment): I retain the explicit FAIL result and the two failed checks rather than treating completed execution as successful validation. The blocked domains do not establish the failure causes or justify broadening network policy.

[pelikhan]

Regarding #73888 (comment): This managed-memory poem is not actionable feedback, and its green-check imagery does not override the same run's FAIL report. I will neither edit managed memory nor follow the firewall or rerun suggestions.

[pelikhan]

Regarding #73888 (comment): I retain this Codex FAIL report, including failed browser, web-fetch and additional-title checks and the skipped issue field. The stated blockers are report evidence rather than newly verified causes, and no unrelated compiler or policy change follows.

[pelikhan]

Regarding #73888 (comment): I acknowledge the reported core-test completion but reject an all-green reading because the detailed result for this Claude run is PARTIAL. No firewall expansion or compiler change is justified by the narrative.

[pelikhan]

Regarding #73888 (comment): I retain the explicit PARTIAL result and detailed GraphQL-unavailable and skipped-close limitations rather than the inconsistent compact review-test sequence. These historical integration results do not validate this batch or justify network-policy changes.

[pelikhan]

Regarding #73888 (comment): This AOAI apikey presence notification supplies no tool-test results or actionable compiler finding. The blocked-domain suggestion does not justify changing network policy or rerunning the smoke workflow.

[pelikhan]

Regarding #73888 (comment): I retain the AOAI Entra FAIL result and its failed individual checks. The report does not verify their causes or a compiler regression, and the firewall suggestions do not justify expanding network access.

[pelikhan]

Regarding #73888 (comment): This managed-memory verse adds no implementation request and does not override the same run's FAIL result. Managed memory and network policy remain unchanged, and no smoke instructions will be followed.