Skip to content

Forward OIDC env vars to MCP Gateway docker command#25773

Merged
pelikhan merged 1 commit intomainfrom
copilot/recreate-pr-25729
Apr 11, 2026
Merged

Forward OIDC env vars to MCP Gateway docker command#25773
pelikhan merged 1 commit intomainfrom
copilot/recreate-pr-25729

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 11, 2026

The compiler omits ACTIONS_ID_TOKEN_REQUEST_URL and ACTIONS_ID_TOKEN_REQUEST_TOKEN from the MCP Gateway docker run -e flags. The firewall forwards these into the agent container, but the second hop (agent → gateway container) was never wired up, so HTTP MCP servers with auth.type: "github-oidc" cannot mint tokens.

Changes

  • pkg/workflow/mcp_environment.go — Add hasGitHubOIDCAuthInTools() helper that scans the tools map for any HTTP MCP server using auth.type: "github-oidc", skipping built-in tools that don't support auth config
  • pkg/workflow/mcp_setup_generator.go — Conditionally append -e ACTIONS_ID_TOKEN_REQUEST_URL -e ACTIONS_ID_TOKEN_REQUEST_TOKEN to the gateway docker command and register both in the dedup map, following the existing OTEL env var pattern
  • pkg/workflow/mcp_environment_test.go — Unit tests for detection helper (empty tools, standard-only, HTTP without auth, HTTP with OIDC, mixed, stdio)
  • pkg/workflow/mcp_setup_generator_test.go — Integration tests compiling real workflows to verify env vars appear/don't appear in generated YAML
  • docs/adr/0001-conditional-oidc-env-var-forwarding-to-mcp-gateway.md — ADR documenting the least-privilege approach over unconditional forwarding

Example workflow that triggers forwarding

tools:
  github:
    toolsets: [repos]
mcp-servers:
  my-server:
    type: http
    url: "https://my-server.example.com/mcp"
    auth:
      type: github-oidc
      audience: "https://my-server.example.com"

Add hasGitHubOIDCAuthInTools() to detect HTTP MCP servers using
auth.type: github-oidc, and conditionally append
-e ACTIONS_ID_TOKEN_REQUEST_URL and -e ACTIONS_ID_TOKEN_REQUEST_TOKEN
to the MCP gateway docker run command.

Includes unit tests for the detection helper, integration tests for
the compiled workflow output, and an ADR documenting the decision.

Agent-Logs-Url: https://github.com/github/gh-aw/sessions/2ccca16b-146d-4364-be62-f3d38ccede86

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
@pelikhan pelikhan marked this pull request as ready for review April 11, 2026 09:43
Copilot AI review requested due to automatic review settings April 11, 2026 09:44
@pelikhan pelikhan merged commit 6b5cdc6 into main Apr 11, 2026
65 of 68 checks passed
@pelikhan pelikhan deleted the copilot/recreate-pr-25729 branch April 11, 2026 09:44
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes missing forwarding of GitHub Actions OIDC environment variables into the MCP Gateway container so HTTP MCP servers configured with auth.type: "github-oidc" can mint tokens.

Changes:

  • Add hasGitHubOIDCAuthInTools() helper to detect github-oidc auth usage in configured HTTP MCP servers.
  • Conditionally append -e ACTIONS_ID_TOKEN_REQUEST_URL and -e ACTIONS_ID_TOKEN_REQUEST_TOKEN to the gateway docker run command (and dedup tracking) when OIDC auth is detected.
  • Add unit + integration tests covering detection and generated YAML behavior; add an ADR documenting the least-privilege rationale.
Show a summary per file
File Description
pkg/workflow/mcp_environment.go Adds helper to detect OIDC-auth HTTP MCP servers.
pkg/workflow/mcp_setup_generator.go Conditionally forwards OIDC env vars into the gateway container docker run command.
pkg/workflow/mcp_environment_test.go Unit tests for OIDC auth detection helper.
pkg/workflow/mcp_setup_generator_test.go Integration tests asserting OIDC env vars appear (or not) in compiled YAML.
docs/adr/0001-conditional-oidc-env-var-forwarding-to-mcp-gateway.md ADR documenting conditional forwarding approach.

Copilot's findings

Tip

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

  • Files reviewed: 5/5 changed files
  • Comments generated: 2

Comment on lines +199 to +206
func hasGitHubOIDCAuthInTools(tools map[string]any) bool {
for toolName, toolValue := range tools {
// Skip standard tools that don't support auth config
if toolName == "github" || toolName == "playwright" ||
toolName == "cache-memory" || toolName == "agentic-workflows" ||
toolName == "safe-outputs" || toolName == "mcp-scripts" {
continue
}
Copy link

Copilot AI Apr 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The built-in tool skip list is duplicated (it already exists in this file’s earlier HTTP MCP env-var scan). To avoid the two lists drifting over time, consider extracting a shared helper/const (e.g., isStandardMCPTool(toolName)) and using it in both places.

Copilot uses AI. Check for mistakes.
}

mcpConfig, err := getMCPConfig(toolConfig, toolName)
if err != nil {
Copy link

Copilot AI Apr 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When getMCPConfig returns an error, the function silently skips the tool. Since this decision controls whether OIDC env vars get forwarded, it would be helpful to log the parse error (similar to collectMCPEnvironmentVariables above) so misconfigurations or future schema changes don’t fail silently.

Suggested change
if err != nil {
if err != nil {
mcpEnvironmentLog.Printf("Error parsing MCP config for tool '%s' while checking github-oidc auth: %v", toolName, err)

Copilot uses AI. Check for mistakes.
@github-actions
Copy link
Copy Markdown
Contributor

✅ Design Decision Gate — ADR Verified

The implementation in this PR aligns with the stated Architecture Decision Record.

ADR reviewed: ADR-0001: Conditional OIDC Environment Variable Forwarding to MCP Gateway Container

Verification Summary

All normative requirements from the ADR are satisfied by the implementation:

Requirement File Status
MUST inspect tools at compile time for auth.type: "github-oidc" pkg/workflow/mcp_environment.gohasGitHubOIDCAuthInTools()
MUST append -e ACTIONS_ID_TOKEN_REQUEST_URL and -e ACTIONS_ID_TOKEN_REQUEST_TOKEN if and only if OIDC auth found pkg/workflow/mcp_setup_generator.go:740–745
MUST NOT append those flags when no OIDC auth is present Controlled by hasOIDCAuth boolean; no other code path adds these flags
MUST register both vars in deduplication map when forwarded pkg/workflow/mcp_setup_generator.go:797–800
Detection helper MUST skip built-in tools (github, playwright, cache-memory, etc.) Hardcoded blocklist in hasGitHubOIDCAuthInTools()
SHOULD log when a tool with GitHub OIDC auth is found mcpEnvironmentLog.Printf(...) call present
MAY return early on first match Returns true immediately on first matching tool

Both unit tests (mcp_environment_test.go) and integration tests (mcp_setup_generator_test.go) cover the positive case (OIDC vars appear when expected) and negative case (OIDC vars absent when no OIDC auth is configured), consistent with the ADR's conformance clause.

The design decision has been recorded and the implementation follows it. 🏗️

🏗️ ADR gate enforced by Design Decision Gate 🏗️ · ● 84.8K ·

Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implementation verified: code aligns with the linked Architecture Decision Record (ADR-0001: Conditional OIDC Environment Variable Forwarding to MCP Gateway Container). All MUST and MUST NOT requirements are satisfied. See the inline comment for the full verification matrix.

@github-actions
Copy link
Copy Markdown
Contributor

🧪 Test Quality Sentinel Report

Test Quality Score: 90/100

Excellent test quality

Metric Value
New/modified tests analyzed 3
✅ Design tests (behavioral contracts) 3 (100%)
⚠️ Implementation tests (low value) 0 (0%)
Tests with error/edge cases 3 (100%)
Duplicate test clusters 0
Test inflation detected Yes (justified — see note)
🚨 Coding-guideline violations 0

Test Classification Details

Test File Classification Issues Detected
TestHasGitHubOIDCAuthInTools pkg/workflow/mcp_environment_test.go:11 ✅ Design None — 7-case table-driven test covers all variants
TestOIDCEnvVarsPassedToGatewayContainer pkg/workflow/mcp_setup_generator_test.go:604 ✅ Design None — end-to-end compilation + output assertion
TestOIDCEnvVarsNotPassedWithoutOIDCAuth pkg/workflow/mcp_setup_generator_test.go:662 ✅ Design None — verifies negative case (vars absent without OIDC auth)

Flagged Tests — Requires Review

No tests require review. All three tests are high-value behavioral contracts.


Test Inflation Note

Both test files exceed the 2:1 line-ratio threshold (test lines vs. production lines):

Test file Test lines added Production lines added Ratio
mcp_environment_test.go 109 36 3.0:1
mcp_setup_generator_test.go 106 12 8.8:1

This triggers a mechanical -10 point deduction per the rubric, but the inflation is structurally justified here:

  • mcp_environment_test.go: A 7-case table-driven test with detailed inline tool-configuration maps is naturally verbose — the ratio reflects thoroughness, not padding.
  • mcp_setup_generator_test.go: Integration tests that compile a full workflow and inspect the resulting YAML always produce more lines than the production code they cover. This is expected and healthy.

No remediation is needed.


Language Support

Tests analyzed:

  • 🐹 Go (*_test.go): 3 tests — 1 unit (//go:build !integration), 2 integration (//go:build integration)
  • 🟨 JavaScript (*.test.cjs, *.test.js): 0 tests

Verdict

Check passed. 0% of new tests are implementation tests (threshold: 30%). All three tests verify observable behavioral contracts: OIDC detection in tool configuration, presence of OIDC env var flags in the compiled docker command, and absence of those flags when OIDC auth is not configured.


📖 Understanding Test Classifications

Design Tests (High Value) verify what the system does:

  • Assert on observable outputs, return values, or state changes
  • Cover error paths and boundary conditions
  • Would catch a behavioral regression if deleted
  • Remain valid even after internal refactoring

Implementation Tests (Low Value) verify how the system does it:

  • Assert on internal function calls (mocking internals)
  • Only test the happy path with typical inputs
  • Break during legitimate refactoring even when behavior is correct
  • Give false assurance: they pass even when the system is wrong

Goal: Shift toward tests that describe the system's behavioral contract — the promises it makes to its users and collaborators.

🧪 Test quality analysis by Test Quality Sentinel · ● 715.7K ·

Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Test Quality Sentinel: 90/100. Test quality is excellent — 0% of new tests are implementation tests (threshold: 30%). All 3 new tests are behavioral contracts: a 7-case table-driven unit test for OIDC detection, and two integration tests verifying presence/absence of OIDC env var flags in compiled docker commands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants