Copilot/update entrypoint args lockdown - #17382
Conversation
… MCP server is in lockdown mode Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
|
@pelikhan was experimenting locally with github mcp lockdown mode and realized that an extra entrypoint arg is required for it to work with gateway. looks like it's not supported by the compiler yet but this PR should add it. |
There was a problem hiding this comment.
Pull request overview
This pull request adds the entrypointArgs field with --lockdown-mode flag to GitHub MCP server configurations when lockdown mode is explicitly enabled. This aligns the configuration with the MCP Gateway specification and ensures the GitHub MCP server receives explicit lockdown instructions via command-line arguments in addition to the environment variable.
Changes:
- Added
entrypointArgsrendering logic for both JSON/Docker and TOML configuration paths when explicit lockdown is enabled - Updated all test cases to verify
entrypointArgspresence in lockdown scenarios - Regenerated workflow lock files for all workflows with explicit lockdown configuration
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| pkg/workflow/mcp_renderer.go | Added entrypointArgs rendering for lockdown mode in both JSON (line 737-740) and TOML (line 560-563) paths |
| pkg/workflow/github_lockdown_test.go | Updated test expectations to verify entrypointArgs field is present when lockdown is enabled and absent when disabled |
| pkg/workflow/github_lockdown_integration_test.go | Enhanced integration tests to check for entrypointArgs in various engine configurations (Copilot, Claude, Codex) |
| .github/workflows/*.lock.yml | Regenerated 13 workflow lock files to include entrypointArgs in their MCP configurations for explicit lockdown scenarios |
Comments suppressed due to low confidence (2)
pkg/workflow/mcp_renderer.go:605
- The TOML rendering path in
renderGitHubTOML(used by Codex engine) doesn't support runtime-determined lockdown, creating a feature gap compared to the JSON rendering path.
When lockdown is not explicitly set in the workflow frontmatter, the JSON path correctly handles runtime lockdown determination by:
- Checking
LockdownFromStepat line 738:if options.Lockdown && !options.LockdownFromStep - Using dynamic environment variable at line 767:
envVars["GITHUB_LOCKDOWN_MODE"] = "$GITHUB_MCP_LOCKDOWN"
However, renderGitHubTOML doesn't have access to shouldUseStepOutput or LockdownFromStep, so it can't distinguish between explicit and runtime-determined lockdown. Currently, it defaults to lockdown = false when not explicitly set (line 485), which means Codex workflows don't benefit from automatic lockdown determination.
While this PR correctly adds entrypointArgs for explicit lockdown cases, the TOML path should be updated to support runtime-determined lockdown by:
- Passing
shouldUseStepOutputinformation torenderGitHubTOML - Conditionally adding
entrypointArgsonly when lockdown is explicit - Using
"$GITHUB_MCP_LOCKDOWN"environment variable when lockdown is runtime-determined
Note: This is a pre-existing limitation, not a regression from this PR.
func (r *MCPConfigRendererUnified) renderGitHubTOML(yaml *strings.Builder, githubTool any, workflowData *WorkflowData) {
githubType := getGitHubType(githubTool)
readOnly := getGitHubReadOnly(githubTool)
lockdown := getGitHubLockdown(githubTool)
toolsets := getGitHubToolsets(githubTool)
yaml.WriteString(" \n")
yaml.WriteString(" [mcp_servers.github]\n")
// Add user_agent field defaulting to workflow identifier
userAgent := "github-agentic-workflow"
if workflowData != nil {
// Check if user_agent is configured in engine config first
if workflowData.EngineConfig != nil && workflowData.EngineConfig.UserAgent != "" {
userAgent = workflowData.EngineConfig.UserAgent
} else if workflowData.Name != "" {
// Fall back to sanitizing workflow name to identifier
userAgent = SanitizeIdentifier(workflowData.Name)
}
}
yaml.WriteString(" user_agent = \"" + userAgent + "\"\n")
// Use tools.startup-timeout if specified, otherwise default to DefaultMCPStartupTimeout
startupTimeout := int(constants.DefaultMCPStartupTimeout / time.Second)
if workflowData != nil && workflowData.ToolsStartupTimeout > 0 {
startupTimeout = workflowData.ToolsStartupTimeout
}
fmt.Fprintf(yaml, " startup_timeout_sec = %d\n", startupTimeout)
// Use tools.timeout if specified, otherwise default to DefaultToolTimeout
toolTimeout := int(constants.DefaultToolTimeout / time.Second)
if workflowData != nil && workflowData.ToolsTimeout > 0 {
toolTimeout = workflowData.ToolsTimeout
}
fmt.Fprintf(yaml, " tool_timeout_sec = %d\n", toolTimeout)
// Check if remote mode is enabled
if githubType == "remote" {
// Remote mode - use hosted GitHub MCP server with streamable HTTP
// Use readonly endpoint if read-only mode is enabled
if readOnly {
yaml.WriteString(" url = \"https://api.githubcopilot.com/mcp-readonly/\"\n")
} else {
yaml.WriteString(" url = \"https://api.githubcopilot.com/mcp/\"\n")
}
// Use bearer_token_env_var for authentication
yaml.WriteString(" bearer_token_env_var = \"GH_AW_GITHUB_TOKEN\"\n")
} else {
// Local mode - use Docker-based GitHub MCP server with MCP Gateway spec format
githubDockerImageVersion := getGitHubDockerImageVersion(githubTool)
customArgs := getGitHubCustomArgs(githubTool)
mounts := getGitHubMounts(githubTool)
// MCP Gateway spec fields for containerized stdio servers
yaml.WriteString(" container = \"ghcr.io/github/github-mcp-server:" + githubDockerImageVersion + "\"\n")
// Append custom args if present (these are Docker runtime args, go before container image)
if len(customArgs) > 0 {
yaml.WriteString(" args = [\n")
for _, arg := range customArgs {
yaml.WriteString(" \"" + arg + "\",\n")
}
yaml.WriteString(" ]\n")
}
// Add volume mounts if present
if len(mounts) > 0 {
yaml.WriteString(" mounts = [")
for i, mount := range mounts {
if i > 0 {
yaml.WriteString(", ")
}
yaml.WriteString("\"" + mount + "\"")
}
yaml.WriteString("]\n")
}
// Add entrypointArgs for lockdown mode
if lockdown {
yaml.WriteString(" entrypointArgs = [\"stdio\", \"--lockdown-mode\"]\n")
}
// Build environment variables
envVars := make(map[string]string)
envVars["GITHUB_PERSONAL_ACCESS_TOKEN"] = "$GH_AW_GITHUB_TOKEN"
if readOnly {
envVars["GITHUB_READ_ONLY"] = "1"
}
if lockdown {
envVars["GITHUB_LOCKDOWN_MODE"] = "1"
}
envVars["GITHUB_TOOLSETS"] = toolsets
// Write environment variables in sorted order for deterministic output
envKeys := make([]string, 0, len(envVars))
for key := range envVars {
envKeys = append(envKeys, key)
}
sort.Strings(envKeys)
yaml.WriteString(" env = { ")
for i, key := range envKeys {
if i > 0 {
yaml.WriteString(", ")
}
fmt.Fprintf(yaml, "\"%s\" = \"%s\"", key, envVars[key])
}
yaml.WriteString(" }\n")
// Use env_vars array to reference environment variables
yaml.WriteString(" env_vars = [")
for i, key := range envKeys {
if i > 0 {
yaml.WriteString(", ")
}
fmt.Fprintf(yaml, "\"%s\"", key)
}
yaml.WriteString("]\n")
}
}
pkg/workflow/github_lockdown_test.go:194
- The test file lacks coverage for the
LockdownFromStepscenario where lockdown is determined at runtime rather than explicitly set. This is a critical code path that should be tested.
Consider adding a test case like:
{
name: "Docker mode with lockdown from step output",
options: GitHubMCPDockerOptions{
ReadOnly: false,
Lockdown: true,
LockdownFromStep: true,
Toolsets: "default",
DockerImageVersion: "latest",
IncludeTypeField: true,
},
expected: []string{
`"type": "stdio"`,
`"GITHUB_LOCKDOWN_MODE": "$GITHUB_MCP_LOCKDOWN"`,
`"container": "ghcr.io/github/github-mcp-server:latest"`,
},
notFound: []string{
`"entrypointArgs"`, // Should not be present when lockdown is from step
},
}
This ensures the implementation correctly handles runtime-determined lockdown where the entrypointArgs should not include --lockdown-mode since the mode is dynamically determined.
func TestRenderGitHubMCPDockerConfigWithLockdown(t *testing.T) {
tests := []struct {
name string
options GitHubMCPDockerOptions
expected []string
notFound []string
}{
{
name: "Docker mode with lockdown enabled",
options: GitHubMCPDockerOptions{
ReadOnly: false,
Lockdown: true,
Toolsets: "default",
DockerImageVersion: "latest",
IncludeTypeField: true,
AllowedTools: nil,
},
expected: []string{
`"type": "stdio"`,
`"entrypointArgs": ["stdio", "--lockdown-mode"]`,
`"GITHUB_LOCKDOWN_MODE": "1"`,
`"GITHUB_TOOLSETS": "default"`,
`"container": "ghcr.io/github/github-mcp-server:latest"`,
},
notFound: []string{},
},
{
name: "Docker mode with lockdown disabled",
options: GitHubMCPDockerOptions{
ReadOnly: false,
Lockdown: false,
Toolsets: "default",
DockerImageVersion: "latest",
IncludeTypeField: true,
AllowedTools: nil,
},
expected: []string{
`"type": "stdio"`,
`"GITHUB_TOOLSETS": "default"`,
`"container": "ghcr.io/github/github-mcp-server:latest"`,
},
notFound: []string{
`"GITHUB_LOCKDOWN_MODE"`,
`"entrypointArgs"`,
},
},
{
name: "Docker mode with lockdown and read-only both enabled",
options: GitHubMCPDockerOptions{
ReadOnly: true,
Lockdown: true,
Toolsets: "default",
DockerImageVersion: "v1.0.0",
IncludeTypeField: false,
AllowedTools: nil,
},
expected: []string{
`"GITHUB_READ_ONLY": "1"`,
`"entrypointArgs": ["stdio", "--lockdown-mode"]`,
`"GITHUB_LOCKDOWN_MODE": "1"`,
`"container": "ghcr.io/github/github-mcp-server:v1.0.0"`,
},
notFound: []string{},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
var yaml strings.Builder
RenderGitHubMCPDockerConfig(&yaml, tt.options)
output := yaml.String()
// Check expected strings
for _, expected := range tt.expected {
if !strings.Contains(output, expected) {
t.Errorf("Expected output to contain %q, but it doesn't.\nOutput: %s", expected, output)
}
}
// Check strings that should NOT be present
for _, notFound := range tt.notFound {
if strings.Contains(output, notFound) {
t.Errorf("Expected output NOT to contain %q, but it does.\nOutput: %s", notFound, output)
}
}
})
}
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
No description provided.