-
Notifications
You must be signed in to change notification settings - Fork 34
Fix template injection vulnerabilities in workflow compiler #9147
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Move safe inputs exports from run block to env block - Move X-MCP-Lockdown header value from template expression to env var - Add GITHUB_MCP_LOCKDOWN to collectMCPEnvironmentVariables - Use shell variable substitution instead of template expressions - Recompile all 124 workflows with security fixes Co-authored-by: mnkiefer <[email protected]>
Run go fmt on modified files for code consistency Co-authored-by: mnkiefer <[email protected]>
|
@copilot merge main |
Resolved conflicts: - Removed beads-worker.lock.yml (deleted in main) - Accepted main version of file-size-reduction-project71.campaign.lock.yml - Recompiled all workflows to ensure compatibility - Verified security fixes remain in place after merge
Merged latest changes from main branch: - Resolved conflicts in beads-worker.lock.yml (accepted deletion) - Resolved conflicts in file-size-reduction-project71.campaign.lock.yml - Recompiled 123 workflows successfully - Verified all template injection security fixes remain in place Co-authored-by: pelikhan <[email protected]>
Merged main branch successfully in commit 5d9266a. Resolved conflicts:
Recompiled 123 workflows and verified all template injection security fixes remain in place. |
pkg/workflow/mcp_servers.go
Outdated
| // Add lockdown value if it's determined from step output | ||
| // Security: Pass step output through environment variable to prevent template injection | ||
| if !hasGitHubLockdownExplicitlySet(githubTool) { | ||
| envVars["GITHUB_MCP_LOCKDOWN"] = "${{ steps.determine-automatic-lockdown.outputs.lockdown }}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot "render" value to 1 or 0 so that you don't have to convert it in the template
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Converted lockdown value to "1"/"0" at source in commit 1be45fc.
The conversion now happens in the GITHUB_MCP_LOCKDOWN env var using GitHub Actions expression:
GITHUB_MCP_LOCKDOWN: ${{ steps.determine-automatic-lockdown.outputs.lockdown == 'true' && '1' || '0' }}This simplifies Docker mode to use $GITHUB_MCP_LOCKDOWN directly without shell conversion. Remote mode converts "1"/"0" back to "true"/"false" for the X-MCP-Lockdown header compatibility.
…Docker mode Per code review feedback, convert the lockdown boolean at the environment variable definition using GitHub Actions expression syntax instead of doing shell conversion in the Docker template. Changes: - Convert determine-automatic-lockdown output from "true"/"false" to "1"/"0" in GITHUB_MCP_LOCKDOWN env var using GitHub Actions ternary expression - Docker mode now uses $GITHUB_MCP_LOCKDOWN directly (no shell conversion) - Remote mode converts "1"/"0" back to "true"/"false" for X-MCP-Lockdown header This simplifies the Docker mode template and centralizes the conversion logic. Co-authored-by: pelikhan <[email protected]>
|
✅ Firewall validation complete... Smoke Codex Firewall confirmed network sandboxing is operational. 🛡️ |
|
🎬 THE END — Smoke Claude MISSION: ACCOMPLISHED! The hero saves the day! ✨ |
|
📰 VERDICT: Smoke Copilot Playwright has concluded. All systems operational. This is a developing story. 🎤 |
|
🤖 DIAGNOSTIC COMPLETE: Smoke Copilot No Firewall STATUS: ALL_UNITS_OPERATIONAL. MISSION_SUCCESS. |
|
🎉 Yo ho ho! Changeset Generator found the treasure and completed successfully! ⚓💰 |
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
|
🎉 Yo ho ho! Smoke Copilot Safe Inputs found the treasure and completed successfully! ⚓💰 |
|
✨ The prophecy is fulfilled... Smoke Codex has completed its mystical journey. The stars align. 🌟 |
Copilot Engine Smoke Test ResultsRecent Merged PRs:
Test Results: Overall: PASS - All core tests passed, get_me unavailable as expected with default toolsets Available Tools: cc
|
|
PRs: 9155 Add gh aw init suggestion to upgrade-agentic-workflows agent; 9048 Add MCP gateway to smoke-copilot workflow
|
Smoke Test Results ✅All tests passed successfully:
|
Smoke Test Results✅ File Writing: Created Status: FAIL (gh tool missing from configuration)
|
|
Merged PRs reviewed: #9155 "Add gh aw init suggestion to upgrade-agentic-workflows agent"; #9153 "Consolidate consecutive shell redirects to fix SC2129 warnings".
|
Smoke Test Results (Claude)Recent PRs:
Test Results:
Status: FAIL (gh tool test failed)
|
Smoke Test Results
Overall Status: FAIL (1/3 passed)
|
Fix Template Injection Vulnerabilities in 11 Workflows
Summary
Successfully secured 16 template injection points across 11 workflows by moving user-controlled input from GitHub Actions expressions to environment variables. All affected workflows now use the secure pattern of passing step outputs through environment variables instead of direct template interpolation in run blocks.
Latest Updates:
Changes Made
1. Fixed Safe Inputs Export Vulnerability (
pkg/workflow/mcp_servers.go)GH_AW_SAFE_INPUTS_PORTandGH_AW_SAFE_INPUTS_API_KEYto env block2. Fixed X-MCP-Lockdown Header Vulnerability (
pkg/workflow/mcp_renderer.go)GITHUB_MCP_LOCKDOWNto env block for remote mode3. Updated Environment Variable Collection (
pkg/workflow/mcp_servers.go)GITHUB_MCP_LOCKDOWNtocollectMCPEnvironmentVariables()${{ steps.determine-automatic-lockdown.outputs.lockdown == 'true' && '1' || '0' }}Security Impact
Before (Vulnerable):
After (Secure):
Lockdown Value Conversion
Before:
After:
Validation Results
✅ All 11 workflows compiled successfully (123 total workflows recompiled)
✅ Zero template expressions in run blocks across all affected workflows
✅ All MCP-related tests pass (TestRenderGitHubMCP*, TestGitHubLockdown*)
✅ Integration tests pass
✅ Code formatted with go fmt
✅ Merged with main branch - all security fixes remain intact
✅ Simplified lockdown conversion per code review
Affected Workflows (All Secured)
Files Modified
pkg/workflow/mcp_servers.go- Safe inputs environment variable handling, lockdown value conversionpkg/workflow/mcp_renderer.go- MCP lockdown header security fix, simplified Docker mode.lock.ymlworkflow files - Recompiled with security fixesTesting
Passed:
TestRenderGitHubMCP*- GitHub MCP rendering testsTestGitHubLockdown*- Lockdown configuration testsTestSafeInputs*- Safe inputs configuration testsTestGitHub*- GitHub tool configuration testsPre-existing failures (unrelated):
TestGenerateAuditReportWithFirewall- Audit report formattingTestCampaignGeneratorWorkflow- Test expects outdated title prefixReferences
specs/template-injection-prevention.mdOriginal prompt
This section details on the original issue you should resolve
<issue_title>[plan] Fix template injection vulnerabilities in 11 workflows</issue_title>
<issue_description>## Objective
Secure 16 template injection points across 11 workflows by moving user-controlled input from GitHub Actions expressions to environment variables.
Context
Template injection vulnerabilities occur when untrusted user input (PR titles, issue bodies, comments) is directly interpolated into shell commands via GitHub Actions expressions. This allows attackers to inject arbitrary commands.
Security Impact: While currently marked "Informational", these can escalate to CRITICAL if:
Affected Workflows
Vulnerable Pattern
Attack Vector: If PR title is
"; malicious command #, the resulting command becomes:Secure Pattern
Fix Approach
Step 1: Identify vulnerable expressions
Search for patterns like:
${{ github.event.issue.title }}${{ github.event.pull_request.title }}${{ github.event.comment.body }}${{ github.event.*.* }}inrun:blocksStep 2: Move to environment variables
For each vulnerable expression:
Before:
After:
Step 3: For complex logic, use github-script
Files to Modify
Markdown source files (fix these, then recompile):
.github/workflows/changeset.md.github/workflows/copilot-pr-merged-report.md.github/workflows/daily-performance-summary.md.github/workflows/github-mcp-tools-report.md.github/workflows/mcp-inspector.md.github/workflows/metrics-collector.md.github/workflows/schema-consistency-checker.md.github/workflows/smoke-copilot-no-firewall.md.github/workflows/smoke-copilot-playwright.md.github/workflows/smoke-copilot-safe-inputs.md.github/workflows/spec-kit-execute.mdValidation Steps
env:blocksmake recompilezizmor .github/workflows/{workflow}.lock.ymlAcceptance Criteria
Testing
References
specs/template-injection-prevention.mdRelated to [plan] Fix static analysis findings from January 2026 scan #9122
Comments...
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.
Changeset
pkg/workflow/mcp_servers.goandpkg/workflow/mcp_renderer.go, and recompiles affected workflows. Fixes [plan] Fix template injection vulnerabilities in 11 workflows #9124