Fix template-injection in MCP config heredocs#15066
Conversation
…s in MCP config Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
…ions Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
✨ The prophecy is fulfilled... Smoke Codex has completed its mystical journey. The stars align. 🌟 |
|
📰 BREAKING: Smoke Copilot is now investigating this pull request. Sources say the story is developing... |
|
🧪 Smoke Project is now testing project operations... |
|
🎬 THE END — Smoke Claude MISSION: ACCOMPLISHED! The hero saves the day! ✨ |
|
✅ Changeset Generator completed successfully! |
Agent Container Tool Check ✅
Result: 12/12 tools available ✅ All required development tools are present and functional in the agent container environment.
|
|
✅ Smoke Project completed successfully. All project operations validated. |
Smoke Test ResultsPRs Reviewed:
Test Results:
Overall Status: PASS ✅ @pelikhan
|
|
Smoke test results
|
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
There was a problem hiding this comment.
Pull request overview
This PR mitigates a template-injection risk in generated MCP gateway heredocs by removing direct GitHub Actions template expressions from JSON/TOML config bodies and replacing them with environment-variable references.
Changes:
- Added helpers to detect
${{ env.* }}expressions and replace${{ secrets.* }},${{ env.* }}, and${{ github.workspace }}with env-var placeholders for heredoc-safe rendering. - Updated MCP config renderers/builders to emit workspace/secret references via
${GITHUB_WORKSPACE}/${VAR_NAME}patterns (and\${...}where needed). - Recompiled affected workflows and updated tests/fixtures accordingly.
Reviewed changes
Copilot reviewed 53 out of 53 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/secret_extraction.go | Adds env-expression extraction and template→env-var replacement helper. |
| pkg/workflow/mcp_renderer_test.go | Updates expectations for workspace arg escaping in generated JSON. |
| pkg/workflow/mcp_renderer.go | Switches Serena TOML rendering from ${{ github.workspace }} to ${GITHUB_WORKSPACE}. |
| pkg/workflow/mcp_config_serena_renderer.go | Escapes workspace in Serena JSON config rendering. |
| pkg/workflow/mcp_config_refactor_test.go | Updates expected mounts/args strings to new workspace placeholder patterns. |
| pkg/workflow/mcp_config_custom.go | Applies template→env-var replacement for args/mounts/env rendering (currently Copilot-gated) and adjusts TOML env rendering. |
| pkg/workflow/mcp_config_comprehensive_test.go | Updates expected Serena entrypoint args to new workspace placeholder pattern. |
| pkg/workflow/mcp_config_compilation_test.go | Updates expected compiled workflow content for workspace arg escaping. |
| pkg/workflow/mcp_config_builtin.go | Updates built-in MCP JSON/TOML renderers to use ${GITHUB_WORKSPACE} patterns. |
| pkg/workflow/importable_tools_test.go | Updates expected workspace arg escaping. |
| pkg/constants/constants.go | Updates default workspace mount constant to use an env-var placeholder form. |
| .github/workflows/workflow-normalizer.lock.yml | Recompiled: replaces workspace template usage in MCP configs with env-var placeholders. |
| .github/workflows/typist.lock.yml | Recompiled: replaces workspace template usage in MCP configs with env-var placeholders. |
| .github/workflows/terminal-stylist.lock.yml | Recompiled: replaces workspace template usage in MCP configs with env-var placeholders. |
| .github/workflows/static-analysis-report.lock.yml | Recompiled: replaces workspace template usage in MCP configs with env-var placeholders. |
| .github/workflows/smoke-copilot.lock.yml | Recompiled: replaces workspace template usage in MCP configs with env-var placeholders. |
| .github/workflows/smoke-codex.lock.yml | Recompiled: replaces workspace template usage in MCP configs with env-var placeholders. |
| .github/workflows/smoke-claude.lock.yml | Recompiled: replaces workspace template usage in MCP configs with env-var placeholders. |
| .github/workflows/sergo.lock.yml | Recompiled: replaces workspace template usage in MCP configs with env-var placeholders. |
| .github/workflows/semantic-function-refactor.lock.yml | Recompiled: replaces workspace template usage in MCP configs with env-var placeholders. |
| .github/workflows/security-review.lock.yml | Recompiled: replaces workspace template usage in MCP configs with env-var placeholders. |
| .github/workflows/safe-output-health.lock.yml | Recompiled: replaces workspace template usage in MCP configs with env-var placeholders. |
| .github/workflows/repository-quality-improver.lock.yml | Recompiled: replaces workspace template usage in MCP configs with env-var placeholders. |
| .github/workflows/q.lock.yml | Recompiled: replaces workspace template usage in MCP configs with env-var placeholders. |
| .github/workflows/python-data-charts.lock.yml | Recompiled: replaces workspace template usage in MCP configs with env-var placeholders. |
| .github/workflows/prompt-clustering-analysis.lock.yml | Recompiled: replaces workspace template usage in MCP configs with env-var placeholders. |
| .github/workflows/portfolio-analyst.lock.yml | Recompiled: replaces workspace template usage in MCP configs with env-var placeholders. |
| .github/workflows/notion-issue-summary.lock.yml | Recompiled: replaces secret template usage in MCP configs with env-var placeholders. |
| .github/workflows/metrics-collector.lock.yml | Recompiled: replaces workspace template usage in MCP configs with env-var placeholders. |
| .github/workflows/mcp-inspector.lock.yml | Recompiled: replaces workspace/secrets/env templates in MCP configs with env-var placeholders. |
| .github/workflows/jsweep.lock.yml | Recompiled: replaces workspace template usage in MCP configs with env-var placeholders. |
| .github/workflows/go-fan.lock.yml | Recompiled: replaces workspace template usage in MCP configs with env-var placeholders. |
| .github/workflows/glossary-maintainer.lock.yml | Recompiled: replaces workspace template usage in MCP configs with env-var placeholders. |
| .github/workflows/example-workflow-analyzer.lock.yml | Recompiled: replaces workspace template usage in MCP configs with env-var placeholders. |
| .github/workflows/duplicate-code-detector.lock.yml | Recompiled: replaces workspace template usage in MCP configs with env-var placeholders. |
| .github/workflows/developer-docs-consolidator.lock.yml | Recompiled: replaces workspace template usage in MCP configs with env-var placeholders. |
| .github/workflows/dev-hawk.lock.yml | Recompiled: replaces workspace template usage in MCP configs with env-var placeholders. |
| .github/workflows/deep-report.lock.yml | Recompiled: replaces workspace template usage in MCP configs with env-var placeholders. |
| .github/workflows/daily-testify-uber-super-expert.lock.yml | Recompiled: replaces workspace template usage in MCP configs with env-var placeholders. |
| .github/workflows/daily-safe-output-optimizer.lock.yml | Recompiled: replaces workspace template usage in MCP configs with env-var placeholders. |
| .github/workflows/daily-observability-report.lock.yml | Recompiled: replaces workspace template usage in MCP configs with env-var placeholders. |
| .github/workflows/daily-mcp-concurrency-analysis.lock.yml | Recompiled: replaces workspace template usage in MCP configs with env-var placeholders. |
| .github/workflows/daily-firewall-report.lock.yml | Recompiled: replaces workspace template usage in MCP configs with env-var placeholders. |
| .github/workflows/daily-file-diet.lock.yml | Recompiled: replaces workspace template usage in MCP configs with env-var placeholders. |
| .github/workflows/daily-compiler-quality.lock.yml | Recompiled: replaces workspace template usage in MCP configs with env-var placeholders. |
| .github/workflows/daily-cli-tools-tester.lock.yml | Recompiled: replaces workspace template usage in MCP configs with env-var placeholders. |
| .github/workflows/cloclo.lock.yml | Recompiled: replaces workspace template usage in MCP configs with env-var placeholders. |
| .github/workflows/brave.lock.yml | Recompiled: replaces secret template usage in MCP configs with env-var placeholders. |
| .github/workflows/audit-workflows.lock.yml | Recompiled: replaces workspace template usage in MCP configs with env-var placeholders. |
| .github/workflows/archie.lock.yml | Recompiled: replaces workspace template usage in MCP configs with env-var placeholders. |
| .github/workflows/agent-persona-explorer.lock.yml | Recompiled: replaces workspace template usage in MCP configs with env-var placeholders. |
| .github/workflows/agent-performance-analyzer.lock.yml | Recompiled: replaces workspace template usage in MCP configs with env-var placeholders. |
| .changeset/patch-escape-mcp-template-expressions.md | Adds a patch changeset describing the security fix. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Replace template expressions with environment variable references | ||
| argValue := arg | ||
| if renderer.RequiresCopilotFields { | ||
| argValue = ReplaceTemplateExpressionsWithEnvVars(argValue) | ||
| } | ||
| fmt.Fprintf(yaml, "%s \"%s\"%s\n", renderer.IndentLevel, argValue, argComma) |
There was a problem hiding this comment.
Template-expression escaping is currently gated on renderer.RequiresCopilotFields. The template-injection issue is about ${{ ... }} being embedded in heredoc content, which can also happen for non-Copilot JSON renderers (e.g., Claude/custom) since they also write JSON via cat << .... Consider applying ReplaceTemplateExpressionsWithEnvVars (or an equivalent) based on whether the output is going into a heredoc (or on renderer.Format == "json"), not on whether Copilot-specific fields are required.
| // DefaultWorkspaceMount is the mount path for the GitHub workspace directory in containerized MCP servers | ||
| // Uses GitHub Actions expression syntax which expands to the actual workspace path at runtime | ||
| // This is required when MCP servers need read-write access to repository files | ||
| const DefaultWorkspaceMount = "${{ github.workspace }}:${{ github.workspace }}:rw" | ||
| // Security: Uses GITHUB_WORKSPACE environment variable instead of template expansion to prevent template injection | ||
| // The GITHUB_WORKSPACE environment variable is automatically set by GitHub Actions and passed to the MCP gateway | ||
| const DefaultWorkspaceMount = "\\${GITHUB_WORKSPACE}:\\${GITHUB_WORKSPACE}:rw" |
There was a problem hiding this comment.
DefaultWorkspaceMount now embeds backslash-escaped shell syntax (\\${GITHUB_WORKSPACE}), which is a rendering-context concern (heredoc escaping) rather than an invariant mount value. Since this constant is reused by both JSON and TOML renderers, consider keeping the constant as the unescaped mount expression and applying the appropriate escaping at render time (or splitting into format-specific constants) to avoid accidental behavior changes across formats.
| // Security: Use GITHUB_WORKSPACE environment variable instead of template expansion to prevent template injection | ||
| yaml.WriteString(" \"${GITHUB_WORKSPACE}\"") | ||
|
|
There was a problem hiding this comment.
PR description says backslash escaping is used to prevent shell expansion inside heredocs (deferring substitution to the MCP gateway), but this TOML renderer writes ${GITHUB_WORKSPACE} without escaping. In the unquoted heredocs used by the compiled workflows, that will be expanded by the shell before the config is consumed. Either escape it consistently (e.g., \${GITHUB_WORKSPACE}) / quote the heredoc delimiter, or adjust the PR description to reflect that TOML uses shell-time expansion.
MCP server configurations embedded GitHub Actions template expressions directly in shell heredocs, allowing potential code injection if user-controlled values flow into these expressions.
Changes
Template Expression Replacement
${{ github.workspace }}→\${GITHUB_WORKSPACE}in JSON configs${{ secrets.* }}→\${VAR_NAME}for all secret references${{ env.* }}→\${VAR_NAME}for all env var referencesImplementation
ReplaceTemplateExpressionsWithEnvVars()insecret_extraction.goDefaultWorkspaceMountconstant from template to env var referenceBefore:
After:
Scope
Affects 40+ workflows using MCP servers. All 148 workflows recompiled successfully.
Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.
Changeset