Skip to content

Fix: Remove env_vars from HTTP MCP server config for safe-inputs #8244

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.

Sign up for GitHub

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

Merged
pelikhan merged 2 commits into from
Dec 31, 2025
Merged

Fix: Remove env_vars from HTTP MCP server config for safe-inputs #8244

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1048eb0
Initial plan
Copilot Dec 31, 2025
d78fab4
Fix: Remove env_vars from HTTP MCP server config for safe-inputs
Copilot Dec 31, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion .github/workflows/daily-performance-summary.lock.yml
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

15 changes: 2 additions & 13 deletions pkg/workflow/mcp_renderer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -236,24 +236,13 @@ func (r *MCPConfigRendererUnified) RenderSafeInputsMCP(yaml *strings.Builder, sa
// renderSafeInputsTOML generates Safe Inputs MCP configuration in TOML format
// Uses HTTP transport exclusively
func (r *MCPConfigRendererUnified) renderSafeInputsTOML(yaml *strings.Builder, safeInputs *SafeInputsConfig) {
envVars := getSafeInputsEnvVars(safeInputs)

yaml.WriteString(" \n")
yaml.WriteString(" [mcp_servers." + constants.SafeInputsMCPServerID + "]\n")
yaml.WriteString(" type = \"http\"\n")
yaml.WriteString(" url = \"http://host.docker.internal:$GH_AW_SAFE_INPUTS_PORT\"\n")
yaml.WriteString(" headers = { Authorization = \"Bearer $GH_AW_SAFE_INPUTS_API_KEY\" }\n")

// Add environment variables: server config + tool-specific vars
envVarsWithServerConfig := append([]string{"GH_AW_SAFE_INPUTS_PORT", "GH_AW_SAFE_INPUTS_API_KEY"}, envVars...)
yaml.WriteString(" env_vars = [")
for i, envVar := range envVarsWithServerConfig {
if i > 0 {
yaml.WriteString(", ")
}
yaml.WriteString("\"" + envVar + "\"")
}
yaml.WriteString("]\n")
// Note: env_vars is not supported for HTTP transport in MCP configuration
// Environment variables are passed via the workflow job's env: section instead
}

// RenderAgenticWorkflowsMCP generates the Agentic Workflows MCP server configuration
Expand Down
26 changes: 10 additions & 16 deletions pkg/workflow/safe_inputs_http_codex_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,13 +90,10 @@ Test safe-inputs HTTP transport for Codex
t.Error("Codex config should not use stdio transport with mcp-server.cjs args, should use HTTP")
}

// Verify environment variables are included
if !strings.Contains(codexConfigSection, "GH_AW_SAFE_INPUTS_PORT") {
t.Error("Expected GH_AW_SAFE_INPUTS_PORT env var in config")
}

if !strings.Contains(codexConfigSection, "GH_AW_SAFE_INPUTS_API_KEY") {
t.Error("Expected GH_AW_SAFE_INPUTS_API_KEY env var in config")
// Verify environment variables are NOT in the MCP config (env_vars not supported for HTTP transport)
// They should be in the job's env section instead
if strings.Contains(codexConfigSection, "env_vars") {
t.Error("HTTP MCP servers should not have env_vars in config (not supported for HTTP transport)")
}

t.Logf("✓ Codex engine correctly uses HTTP transport for safe-inputs")
Expand Down Expand Up @@ -161,16 +158,13 @@ Test safe-inputs with secrets
yamlStr := string(lockContent)
codexConfigSection := extractCodexConfigSection(yamlStr)

// Verify tool-specific env vars are included in HTTP transport config
if !strings.Contains(codexConfigSection, "API_KEY") {
t.Error("Expected API_KEY env var in safe-inputs config")
}

if !strings.Contains(codexConfigSection, "GH_TOKEN") {
t.Error("Expected GH_TOKEN env var in safe-inputs config")
// Verify tool-specific env vars are NOT in the MCP config (env_vars not supported for HTTP)
// They should be passed via the job's env section instead
if strings.Contains(codexConfigSection, "env_vars") {
t.Error("HTTP MCP servers should not have env_vars in config (not supported for HTTP transport)")
}

// Verify env vars are set in Setup MCPs step
// Verify env vars are set in Setup MCPs step (this is the correct location for HTTP transport)
if !strings.Contains(yamlStr, "API_KEY: ${{ secrets.API_KEY }}") {
t.Error("Expected API_KEY secret in Setup MCPs env section")
}
Expand All @@ -179,5 +173,5 @@ Test safe-inputs with secrets
t.Error("Expected GH_TOKEN in Setup MCPs env section")
}

t.Logf("✓ Codex engine correctly passes secrets through HTTP transport")
t.Logf("✓ Codex engine correctly passes secrets through HTTP transport (via job env, not MCP config)")
}