MCP Gateway: Add upstream OIDC authentication to schema, spec, and compiler#23611
MCP Gateway: Add upstream OIDC authentication to schema, spec, and compiler#23611
Conversation
… and compiler - Add MCPAuthConfig struct and Auth field to BaseMCPServerConfig in pkg/types/mcp.go - Implement GetAny method on MapToolConfig in pkg/workflow/mcp_config_types.go - Update mcpServerConfigToMap to include auth field in pkg/workflow/tools_types.go - Add auth field parsing and JSON rendering in pkg/workflow/mcp_config_custom.go - Add auth property to httpServerConfig in both schema files - Add auth row to Section 4.1.2 and new Section 7.6 in mcp-gateway.md Closes #23566 Agent-Logs-Url: https://github.com/github/gh-aw/sessions/fe03ae40-ff9a-43ab-b882-da184a4adbcd Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds end-to-end support for configuring upstream GitHub Actions OIDC authentication (auth) on HTTP MCP servers across schema validation, workflow compiler codegen, and reference documentation.
Changes:
- Extend MCP gateway JSON schemas (compiler + published docs) to allow
httpServerConfig.authwithtype: github-oidcand optionalaudience. - Add
MCPAuthConfigto shared MCP server types and render/parseauthin the workflow compiler output. - Document the new
authfield and expected OIDC behavior in the MCP gateway reference docs.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/tools_types.go | Includes auth when converting MCPServerConfig to a backward-compat map representation. |
| pkg/workflow/schemas/mcp-gateway-config.schema.json | Adds auth object schema under httpServerConfig. |
| pkg/workflow/mcp_config_types.go | Adds GetAny accessor to support extracting non-stringly typed fields like auth. |
| pkg/workflow/mcp_config_custom.go | Adds auth to JSON property ordering, rendering, and HTTP-branch parsing; updates known-properties allowlist. |
| pkg/types/mcp.go | Adds Auth *MCPAuthConfig to BaseMCPServerConfig and defines MCPAuthConfig. |
| docs/src/content/docs/reference/mcp-gateway.md | Documents auth field and new OIDC upstream authentication section. |
| docs/public/schemas/mcp-gateway-config.schema.json | Updates published schema to match the compiler schema changes. |
Comments suppressed due to low confidence (2)
pkg/workflow/mcp_config_custom.go:593
authis added to the globalknownPropertiesset, but there’s no subsequent validation to ensure it’s only used on HTTP MCP servers. As a result, a stdio server config that includesauthwill pass the unknown-field check and then haveauthsilently ignored in thestdiobranch, which contradicts the spec/docs (“stdio servers withauthMUST be rejected”). Consider adding an explicit check after inferring/readingresult.Typethat returns a validation error whenauthis present andresult.Type != "http".
// Validate known properties - fail if unknown properties are found
knownProperties := map[string]bool{
"type": true,
"mode": true, // Added for MCPServerConfig struct
"command": true,
"container": true,
"version": true,
"args": true,
"entrypoint": true,
"entrypointArgs": true,
"mounts": true,
"env": true,
"proxy-args": true,
"url": true,
"headers": true,
"auth": true,
"registry": true,
"allowed": true,
"toolsets": true, // Added for MCPServerConfig struct
}
pkg/workflow/mcp_config_custom.go:722
- The HTTP
authparsing is currently permissive: it silently dropsauthiftypeis missing/empty, accepts anytypestring (no enum check), and ignores unknown fields inside theauthobject. This can lead to workflows compiling successfully but producing gateway configs that fail schema validation or behave unexpectedly. Recommend treating presence ofauthas a strict validation surface: requireauthto be an object, require non-emptyauth.typeand reject unsupported values (e.g., anything other thangithub-oidc), validate/parseaudienceif present, and error on unknown keys in theauthmap.
if authVal, hasAuth := config.GetAny("auth"); hasAuth {
if authMap, ok := authVal.(map[string]any); ok {
authConfig := &types.MCPAuthConfig{}
if authType, ok := authMap["type"].(string); ok {
authConfig.Type = authType
}
if audience, ok := authMap["audience"].(string); ok {
authConfig.Audience = audience
}
if authConfig.Type != "" {
result.Auth = authConfig
}
} else if authCfg, ok := authVal.(*types.MCPAuthConfig); ok {
result.Auth = authCfg
}
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // JSON format - include tools field for MCP gateway tool filtering (all engines) | ||
| // For HTTP MCP with secrets in headers, env passthrough is needed | ||
| if len(headerSecrets) > 0 { | ||
| propertyOrder = []string{"type", "url", "headers", "tools", "env"} | ||
| propertyOrder = []string{"type", "url", "headers", "auth", "tools", "env"} | ||
| } else { | ||
| propertyOrder = []string{"type", "url", "headers", "tools"} | ||
| propertyOrder = []string{"type", "url", "headers", "auth", "tools"} | ||
| } |
There was a problem hiding this comment.
There are existing unit tests covering getMCPConfig behavior, but no tests exercise the new auth handling (parsing, type restrictions, and rendering order). Adding focused tests would help prevent regressions: (1) auth is accepted and round-trips for HTTP servers, (2) auth on stdio is rejected, (3) missing/unsupported auth.type and unknown auth fields produce clear errors, and (4) JSON rendering includes auth in the intended property order after headers.
This issue also appears in the following locations of the same file:
- line 574
- line 707
| // Add HTTP-specific fields | ||
| if config.URL != "" { | ||
| result["url"] = config.URL | ||
| } | ||
| if len(config.Headers) > 0 { | ||
| result["headers"] = config.Headers | ||
| } | ||
| if config.Auth != nil { | ||
| result["auth"] = config.Auth | ||
| } |
There was a problem hiding this comment.
mcpServerConfigToMap now includes the new auth field, but the existing unit tests for this helper don’t cover it. Please add/extend a test case to assert that when config.Auth is set, the resulting map includes an auth entry with the expected serialized shape (and that it’s omitted when nil).
|
Hey Add tests — the project has extensive MCP test coverage (
If you'd like a hand, you can assign this prompt to your coding agent:
|
|
@copilot review comments |
- Add auth field validation in mcp_config_validation.go: - Add 'auth' to knownToolFields (prevents spurious unknown-property errors) - HTTP servers: validate auth.type is present, non-empty, and 'github-oidc' - Stdio servers: reject auth with a clear error message - Add TestGetMCPConfigWithAuth: round-trip parsing of auth for HTTP servers - Add auth cases to TestValidateMCPConfigs: valid, stdio-rejection, missing type, unsupported type, empty type - Add auth cases to TestMCPServerConfigToMap: set and nil auth - Add TestRenderSharedMCPConfig_WithAuth: JSON rendering with/without auth, property order Agent-Logs-Url: https://github.com/github/gh-aw/sessions/aab4c252-327f-4964-b814-cd5bb33643f1 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Added tests and validation in commit f28f999: Tests added:
Validation added (
Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
The MCP Gateway supports GitHub Actions OIDC token injection for HTTP servers, but the schema rejected the
authfield and the compiler had no support for it. This adds end-to-end support: schema validation, compiler codegen, spec documentation, and runtime validation.Schema (
mcp-gateway-config.schema.json)authtohttpServerConfig.properties(notstdioServerConfig)auth.type: required, enum["github-oidc"]auth.audience: optional URI, defaults to serverurlif omittedCompiler (
pkg/workflow/mcp_config_custom.go+pkg/types/mcp.go)MCPAuthConfigstruct added toBaseMCPServerConfig(shared by parser and workflow packages)getMCPConfigparsesauthfrommap[string]anyin the HTTP branchauthrendered in JSON property order afterheaders, with nil guard"auth"added toknownPropertiesso unknown-field validation doesn't reject itValidation (
pkg/workflow/mcp_config_validation.go)"auth"added toknownToolFieldsso HTTP server configs withauthdon't trigger the unknown-property errorauthare rejected with a clear error messageauth.typeis validated: required, non-empty string, and must be"github-oidc"Documentation (
docs/.../mcp-gateway.md)Tests
TestGetMCPConfigWithAuth: round-trip parsing ofauthfor HTTP servers (type+audience, type-only, absent)TestRenderSharedMCPConfig_WithAuth: JSON rendering with/withoutauth, property order (type → url → headers → auth → tools)TestValidateMCPConfigs: valid http auth, stdio rejection, missingauth.type, unsupported type, empty typeTestMCPServerConfigToMap:authincluded when set, omitted when nilExample frontmatter:
Generated config: