diff --git a/.changeset/fix-safe-outputs-dynamic-input-mcp-container.md b/.changeset/fix-safe-outputs-dynamic-input-mcp-container.md new file mode 100644 index 00000000000..1fc4c4b223b --- /dev/null +++ b/.changeset/fix-safe-outputs-dynamic-input-mcp-container.md @@ -0,0 +1,5 @@ +--- +"gh-aw": patch +--- + +Fixed `create-pull-request` (and other safe-outputs handlers) failing with "No remote refs available for merge-base calculation" when using dynamic values like `base-branch: ${{ inputs.base_branch }}`. The compiler now forwards all `GH_AW_INPUT_*` environment variables to the MCP gateway container's `-e` allowlist and to the Start MCP Gateway step env, so the containerised safe-outputs MCP server can resolve `${GH_AW_INPUT_*}` placeholders in `config.json` at runtime. diff --git a/.github/skills/agentic-workflows/SKILL.md b/.github/skills/agentic-workflows/SKILL.md index 3fc711d4035..6f24708a24e 100644 --- a/.github/skills/agentic-workflows/SKILL.md +++ b/.github/skills/agentic-workflows/SKILL.md @@ -37,6 +37,7 @@ Load these files from `github/gh-aw` (they are not available locally). - `.github/aw/github-agentic-workflows.md` - `.github/aw/github-mcp-server.md` - `.github/aw/instructions.md` +- `.github/aw/linter-workflows.md` - `.github/aw/llms.md` - `.github/aw/loop.md` - `.github/aw/lsp.md` diff --git a/actions/setup/js/safe_outputs_config.cjs b/actions/setup/js/safe_outputs_config.cjs index b6423999a54..1668d61f809 100644 --- a/actions/setup/js/safe_outputs_config.cjs +++ b/actions/setup/js/safe_outputs_config.cjs @@ -7,6 +7,26 @@ const { ERR_SYSTEM } = require("./error_codes.cjs"); const fs = require("fs"); const path = require("path"); +/** + * Collect the names of any ${GH_AW_INPUT_*} placeholders that are unresolved + * (i.e. the corresponding environment variable is absent from process.env). + * Returns an empty array when all placeholders are resolved. + * @param {string} rawJson - The raw JSON string before placeholder substitution + * @returns {string[]} + */ +function collectUnresolvedInputPlaceholders(rawJson) { + const unresolved = []; + const pattern = /\$\{(GH_AW_INPUT_[A-Z0-9_]+)\}/g; + let match; + while ((match = pattern.exec(rawJson)) !== null) { + const envName = match[1]; + if (process.env[envName] === undefined && !unresolved.includes(envName)) { + unresolved.push(envName); + } + } + return unresolved; +} + function resolveEnvPlaceholders(value) { if (Array.isArray(value)) { return value.map(resolveEnvPlaceholders); @@ -45,6 +65,19 @@ function loadConfig(server) { server.debug(`Config file content length: ${configFileContent.length} characters`); // Don't log raw content to avoid exposing sensitive configuration data server.debug(`Config file read successfully, attempting to parse JSON`); + // Warn about any GH_AW_INPUT_* placeholders that cannot be resolved before substitution. + // This should not happen when the workflow is compiled correctly (the compiler ensures these + // env vars are in the MCP gateway step env and the docker -e allowlist), but if it does the + // error will surface later as a cryptic "No remote refs available for merge-base calculation" + // rather than pointing at the root cause. + const unresolvedInputs = collectUnresolvedInputPlaceholders(configFileContent); + if (unresolvedInputs.length > 0) { + const varList = unresolvedInputs.join(", "); + const unresolvedMsg = `ERR_CONFIG: Unresolved workflow input placeholder(s) in safe-outputs config: ${varList}. The values were not passed to the MCP container. Verify that the workflow was compiled with a version that forwards GH_AW_INPUT_* to the container env.`; + server.error(unresolvedMsg); + console.error(`[safe_outputs_config] ERR_CONFIG: Unresolved workflow input placeholder(s): ${varList}`); + throw new Error(unresolvedMsg); + } safeOutputsConfigRaw = resolveEnvPlaceholders(JSON.parse(configFileContent)); server.debug(`Successfully parsed config from file with ${Object.keys(safeOutputsConfigRaw).length} configuration keys`); } else { diff --git a/actions/setup/js/safe_outputs_config.test.cjs b/actions/setup/js/safe_outputs_config.test.cjs index 7fe4d13504c..c8908fdc7c7 100644 --- a/actions/setup/js/safe_outputs_config.test.cjs +++ b/actions/setup/js/safe_outputs_config.test.cjs @@ -16,6 +16,7 @@ describe("safe_outputs_config", () => { // Create a mock server with debug function mockServer = { debug: vi.fn(), + error: vi.fn(), }; // Use unique paths for each test @@ -216,5 +217,65 @@ describe("safe_outputs_config", () => { expect(debugOutput).toContain("***REDACTED***"); expect(debugOutput).not.toContain("runtime-project-token"); }); + + it("should emit exactly one diagnostic when a GH_AW_INPUT_* placeholder is duplicated and unresolved", () => { + const configDir = path.dirname(testConfigPath); + fs.mkdirSync(configDir, { recursive: true }); + + // ${GH_AW_INPUT_FOO} appears twice but the env var is not set + fs.writeFileSync( + testConfigPath, + JSON.stringify({ + "create-pull-request": { + "base-branch": "${GH_AW_INPUT_FOO}", + "head-branch": "${GH_AW_INPUT_FOO}", + }, + }) + ); + + const consoleSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + try { + loadConfig(mockServer); + + // Deduplication: only one diagnostic per unique unresolved var + const unresolvedPlaceholderErrors = consoleSpy.mock.calls.filter(call => String(call[0]).includes("GH_AW_INPUT_FOO")); + expect(unresolvedPlaceholderErrors).toHaveLength(1); + + const errorOutput = mockServer.error.mock.calls.map(call => String(call[0])).join("\n"); + expect(errorOutput).toContain("GH_AW_INPUT_FOO"); + expect(errorOutput).toContain("Unresolved workflow input placeholder"); + } finally { + consoleSpy.mockRestore(); + } + }); + + it("should emit no diagnostic when all GH_AW_INPUT_* placeholders are resolved", () => { + const configDir = path.dirname(testConfigPath); + fs.mkdirSync(configDir, { recursive: true }); + process.env.GH_AW_INPUT_BASE_BRANCH = "develop"; + + fs.writeFileSync( + testConfigPath, + JSON.stringify({ + "create-pull-request": { + "base-branch": "${GH_AW_INPUT_BASE_BRANCH}", + }, + }) + ); + + const consoleSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + try { + loadConfig(mockServer); + + const inputPlaceholderErrors = consoleSpy.mock.calls.filter(call => String(call[0]).includes("GH_AW_INPUT_")); + expect(inputPlaceholderErrors).toHaveLength(0); + + const errorOutput = mockServer.error.mock.calls.map(call => String(call[0])).join("\n"); + expect(errorOutput).not.toContain("Unresolved workflow input placeholder"); + } finally { + consoleSpy.mockRestore(); + delete process.env.GH_AW_INPUT_BASE_BRANCH; + } + }); }); }); diff --git a/docs/adr/48099-forward-gh-aw-input-vars-to-mcp-gateway-container.md b/docs/adr/48099-forward-gh-aw-input-vars-to-mcp-gateway-container.md new file mode 100644 index 00000000000..76552d8bf41 --- /dev/null +++ b/docs/adr/48099-forward-gh-aw-input-vars-to-mcp-gateway-container.md @@ -0,0 +1,48 @@ +# ADR-48099: Forward GH_AW_INPUT_* Vars to MCP Gateway Container + +**Date**: 2026-07-26 +**Status**: Accepted +**Deciders**: pelikhan, copilot-swe-agent + +--- + +### Context + +Since v0.80.0, the safe-outputs MCP server runs inside a Docker container with a filtered `-e` allowlist. When a workflow author uses a dynamic safe-outputs field — for example `base-branch: ${{ inputs.base_branch }}` — the compiler writes a `${GH_AW_INPUT_BASE_BRANCH}` shell-style placeholder into the container's `config.json`. However, because `GH_AW_INPUT_*` variables were never included in the docker run `-e` allowlist, and were not emitted in the "Start MCP Gateway" step `env:` block, those placeholders remained unresolved at runtime inside the container. This caused `create_pull_request` (and other safe-outputs handlers) to fail with the cryptic error "No remote refs available for merge-base calculation" — with no indication that an unresolved placeholder was the root cause. + +### Decision + +We will extend the workflow compiler to extract all `GH_AW_INPUT_*` environment variable names referenced in the safe-outputs config, emit them in the "Start MCP Gateway" step `env:` block (so the runner process holds the resolved values), and append a `-e GH_AW_INPUT_*` flag for each one in the `docker run` command (so the container inherits those values). Additionally, we will add a `collectUnresolvedInputPlaceholders()` diagnostic in the MCP server's config loader to surface any remaining unresolved placeholders with a clear error message instead of a silent runtime failure. + +### Alternatives Considered + +#### Alternative 1: Pass All Runner Environment Variables to the Container + +Instead of selectively forwarding only `GH_AW_INPUT_*` vars, forward the entire runner environment into the container via `--env-file` or `--env-host`. This would have resolved the issue without any compiler changes. It was rejected because the filtered `-e` allowlist exists specifically to limit what the container can observe — passing the full runner env would expose secrets, tokens, and other runner-level vars to the containerized MCP server, defeating the security boundary that the allowlist enforces. + +#### Alternative 2: Resolve Placeholders at Compile Time + +Substitute `${{ inputs.* }}` values directly into `config.json` at compile time so no placeholder survives to runtime. This was rejected because workflow input values are not known at compile time — they are resolved by GitHub Actions at workflow-dispatch time. Embedding them statically would require a separate "late-compilation" step at workflow startup, and would force config.json to be regenerated per-run, complicating the current architecture where `config.json` is written once at compile time as a static artifact. + +#### Alternative 3: Redesign Config Using a Runtime-Resolved Mechanism + +Replace the `${GH_AW_INPUT_*}` placeholder pattern in `config.json` with a runtime mechanism (e.g., the container reads vars directly from a mounted secrets file or a well-known env var namespace). This would eliminate the coupling between safe-outputs config parsing and MCP gateway code generation. It was rejected because it would require a significant redesign of the safe-outputs config system and a coordinated breaking change to the MCP server's config loader — too high a cost for a targeted bug fix. + +### Consequences + +#### Positive +- Dynamic safe-outputs fields like `base-branch: ${{ inputs.base_branch }}` now work correctly in the containerized MCP server for all safe-outputs handlers, not just `create-pull-request`. +- Unresolved placeholder errors now surface with an explicit, actionable error message (`ERR_CONFIG: Unresolved workflow input placeholder(s): ...`) rather than a cryptic merge-base failure. +- The fix is compiler-driven and backward-compatible: existing workflows without dynamic safe-outputs fields are unaffected (the new code paths only activate when `GH_AW_INPUT_*` vars are detected in the safe-outputs config). + +#### Negative +- Each `${{ inputs.* }}` reference in a safe-outputs config now results in one additional env var forwarded to the MCP gateway container, incrementally widening the container's visible environment beyond what the original allowlist intended. +- The compiler now couples safe-outputs config parsing (`extractSafeOutputsInputEnvVars`) to MCP gateway step generation (`generateMCPGatewaySetup`), increasing the surface area that must be updated if either subsystem changes independently. + +#### Neutral +- Two new test cases validate the fix: an update to the existing dynamic-allowed-repos test (asserts `GH_AW_INPUT_*` appears in both step env blocks and the docker run command) and a new regression test `TestSafeOutputsDynamicBaseBranchPassedToMCPContainer` covering the exact failure scenario. +- The `appendMCPGatewaySafeOutputsInputEnvFlags` function is ordered after `appendMCPGatewayConditionalEnvFlags` in the docker run command assembly, which means `GH_AW_INPUT_*` flags appear after standard conditional flags — this ordering has no functional impact but may affect readability of generated workflow YAML. + +--- + +*ADR created by [adr-writer agent]. Review and finalize before changing status from Draft to Accepted.* diff --git a/pkg/cli/data/agentic_workflows_fallback_aw_files.json b/pkg/cli/data/agentic_workflows_fallback_aw_files.json index 2165df7dc76..ba497b7d5cc 100644 --- a/pkg/cli/data/agentic_workflows_fallback_aw_files.json +++ b/pkg/cli/data/agentic_workflows_fallback_aw_files.json @@ -21,6 +21,7 @@ "github-agentic-workflows.md", "github-mcp-server.md", "instructions.md", + "linter-workflows.md", "llms.md", "loop.md", "lsp.md", diff --git a/pkg/workflow/mcp_renderer_builtin.go b/pkg/workflow/mcp_renderer_builtin.go index 5080ca92264..bb417ca58b3 100644 --- a/pkg/workflow/mcp_renderer_builtin.go +++ b/pkg/workflow/mcp_renderer_builtin.go @@ -5,6 +5,7 @@ import ( "github.com/github/gh-aw/pkg/constants" "github.com/github/gh-aw/pkg/logger" + "github.com/github/gh-aw/pkg/sliceutil" ) var mcpRendererBuiltinLog = logger.New("workflow:mcp_renderer_builtin") @@ -98,7 +99,24 @@ func (r *MCPConfigRendererUnified) renderSafeOutputsTOML(yaml *strings.Builder, yaml.WriteString(" args = [\"-w\", \"$GITHUB_WORKSPACE\"]\n") yaml.WriteString(" entrypoint = \"sh\"\n") yaml.WriteString(" entrypointArgs = [\"-c\", \"sh ${RUNNER_TEMP}/gh-aw/safeoutputs/start_safe_outputs_mcp.sh\"]\n") - yaml.WriteString(" env_vars = [\"DEBUG\", \"DEFAULT_BRANCH\", \"GH_AW_ASSETS_ALLOWED_EXTS\", \"GH_AW_ASSETS_BRANCH\", \"GH_AW_ASSETS_MAX_SIZE_KB\", \"GH_AW_MCP_LOG_DIR\", \"GH_AW_SAFE_OUTPUTS\", \"GH_AW_SAFE_OUTPUTS_CONFIG_PATH\", \"GH_AW_SAFE_OUTPUTS_TOOLS_PATH\", \"GH_AW_POLICY_ALLOW_CREATE_PULL_REQUEST\", \"GITHUB_REPOSITORY\", \"GITHUB_SHA\", \"GITHUB_TOKEN\", \"GITHUB_WORKSPACE\", \"RUNNER_TEMP\"]\n") + + // Build the env_vars list: fixed vars + any GH_AW_INPUT_* vars referenced by the + // safe-outputs config so the nested container can resolve ${GH_AW_INPUT_…} placeholders. + safeOutputsEnvVars := []string{ + "DEBUG", "DEFAULT_BRANCH", + "GH_AW_ASSETS_ALLOWED_EXTS", "GH_AW_ASSETS_BRANCH", "GH_AW_ASSETS_MAX_SIZE_KB", + "GH_AW_MCP_LOG_DIR", "GH_AW_SAFE_OUTPUTS", "GH_AW_SAFE_OUTPUTS_CONFIG_PATH", + "GH_AW_SAFE_OUTPUTS_TOOLS_PATH", "GH_AW_POLICY_ALLOW_CREATE_PULL_REQUEST", + "GITHUB_REPOSITORY", "GITHUB_SHA", "GITHUB_TOKEN", "GITHUB_WORKSPACE", "RUNNER_TEMP", + } + if workflowData != nil { + safeOutputsEnvVars = append(safeOutputsEnvVars, sliceutil.SortedKeys(workflowData.SafeOutputsInputEnvVars)...) + } + quoted := make([]string, len(safeOutputsEnvVars)) + for i, v := range safeOutputsEnvVars { + quoted[i] = "\"" + v + "\"" + } + yaml.WriteString(" env_vars = [" + strings.Join(quoted, ", ") + "]\n") // Check if GitHub tool has guard-policies configured (or auto-lockdown will run) // If so, generate a linked write-sink guard-policy for safeoutputs @@ -274,6 +292,18 @@ func renderSafeOutputsMCPConfigWithOptions(yaml *strings.Builder, isLast bool, i {"RUNNER_TEMP", "RUNNER_TEMP", false}, } + // Append GH_AW_INPUT_* vars referenced by the safe-outputs config so the nested + // container can resolve ${GH_AW_INPUT_…} shell-style placeholders at runtime. + if workflowData != nil { + for _, name := range sliceutil.SortedKeys(workflowData.SafeOutputsInputEnvVars) { + envVars = append(envVars, struct { + name string + value string + isLiteral bool + }{name, name, false}) + } + } + for i, envVar := range envVars { comma := "," if i == len(envVars)-1 { diff --git a/pkg/workflow/mcp_setup_gateway.go b/pkg/workflow/mcp_setup_gateway.go index b203c99e017..2618760989b 100644 --- a/pkg/workflow/mcp_setup_gateway.go +++ b/pkg/workflow/mcp_setup_gateway.go @@ -14,11 +14,11 @@ import ( "github.com/github/gh-aw/pkg/workflow/compilerenv" ) -func generateMCPGatewaySetup(yaml *strings.Builder, tools map[string]any, mcpTools []string, engine CodingAgentEngine, workflowData *WorkflowData, hasAgenticWorkflows bool) error { +func generateMCPGatewaySetup(yaml *strings.Builder, tools map[string]any, mcpTools []string, engine CodingAgentEngine, workflowData *WorkflowData, hasAgenticWorkflows bool, safeOutputsInputEnvVars map[string]string) error { yaml.WriteString(" - name: Start MCP Gateway\n") yaml.WriteString(" id: start-mcp-gateway\n") mcpEnvVars := collectMCPEnvironmentVariables(tools, mcpTools, workflowData, hasAgenticWorkflows) - writeMCPGatewayStepEnv(yaml, mcpEnvVars) + writeMCPGatewayStepEnv(yaml, mcpEnvVars, safeOutputsInputEnvVars) yaml.WriteString(" run: |\n") yaml.WriteString(" set -eo pipefail\n") yaml.WriteString(" mkdir -p \"${RUNNER_TEMP}/gh-aw/mcp-config\"\n") @@ -44,15 +44,16 @@ func generateMCPGatewaySetup(yaml *strings.Builder, tools map[string]any, mcpToo payloadSizeThreshold: payloadSizeThreshold, }) containerCmd := buildMCPGatewayContainerCommand(buildMCPGatewayContainerCommandOptions{ - engine: engine, - workflowData: workflowData, - gatewayConfig: gatewayConfig, - mcpEnvVars: mcpEnvVars, - payloadDir: payloadDir, - payloadPathPrefix: payloadPathPrefix, - hasGitHub: hasGitHub, - githubTool: githubTool, - tools: tools, + engine: engine, + workflowData: workflowData, + gatewayConfig: gatewayConfig, + mcpEnvVars: mcpEnvVars, + payloadDir: payloadDir, + payloadPathPrefix: payloadPathPrefix, + hasGitHub: hasGitHub, + githubTool: githubTool, + tools: tools, + safeOutputsInputEnvVars: safeOutputsInputEnvVars, }) yaml.WriteString(" MCP_GATEWAY_UID=$(id -u 2>/dev/null || echo '0')\n") yaml.WriteString(" MCP_GATEWAY_GID=$(id -g 2>/dev/null || echo '0')\n") @@ -67,16 +68,23 @@ func generateMCPGatewaySetup(yaml *strings.Builder, tools map[string]any, mcpToo return engine.RenderMCPConfig(yaml, tools, mcpTools, workflowData) } -func writeMCPGatewayStepEnv(yaml *strings.Builder, mcpEnvVars map[string]string) { - if len(mcpEnvVars) == 0 { +func writeMCPGatewayStepEnv(yaml *strings.Builder, mcpEnvVars map[string]string, safeOutputsInputEnvVars map[string]string) { + if len(mcpEnvVars) == 0 && len(safeOutputsInputEnvVars) == 0 { return } yaml.WriteString(" env:\n") + // Write MCP env vars first (sorted) envVarNames := sliceutil.MapKeys(mcpEnvVars) sort.Strings(envVarNames) for _, envVarName := range envVarNames { fmt.Fprintf(yaml, " %s: %s\n", envVarName, mcpEnvVars[envVarName]) } + // Write safe-outputs input env vars (sorted); these must also be present in the + // runner step environment so the docker -e flag can forward them to the container. + inputVarNames := sliceutil.SortedKeys(safeOutputsInputEnvVars) + for _, envVarName := range inputVarNames { + fmt.Fprintf(yaml, " %s: %s\n", envVarName, safeOutputsInputEnvVars[envVarName]) + } } func resolveMCPGatewayValues(workflowData *WorkflowData, gatewayConfig *MCPGatewayRuntimeConfig) (int, string, string, string, int) { @@ -190,15 +198,16 @@ func writeMCPGatewayExports(yaml *strings.Builder, opts writeMCPGatewayExportsOp // buildMCPGatewayContainerCommandOptions holds configuration for buildMCPGatewayContainerCommand. type buildMCPGatewayContainerCommandOptions struct { - engine CodingAgentEngine - workflowData *WorkflowData - gatewayConfig *MCPGatewayRuntimeConfig - mcpEnvVars map[string]string - payloadDir string - payloadPathPrefix string - hasGitHub bool - githubTool map[string]any - tools map[string]any + engine CodingAgentEngine + workflowData *WorkflowData + gatewayConfig *MCPGatewayRuntimeConfig + mcpEnvVars map[string]string + payloadDir string + payloadPathPrefix string + hasGitHub bool + githubTool map[string]any + tools map[string]any + safeOutputsInputEnvVars map[string]string } func buildMCPGatewayContainerCommand(opts buildMCPGatewayContainerCommandOptions) string { @@ -211,6 +220,7 @@ func buildMCPGatewayContainerCommand(opts buildMCPGatewayContainerCommandOptions hasGitHub := opts.hasGitHub githubTool := opts.githubTool tools := opts.tools + safeOutputsInputEnvVars := opts.safeOutputsInputEnvVars containerImage := gatewayConfig.Container if gatewayConfig.Version != "" { containerImage += ":" + gatewayConfig.Version @@ -257,6 +267,7 @@ func buildMCPGatewayContainerCommand(opts buildMCPGatewayContainerCommandOptions containerCmd.WriteString(" -v ${DOCKER_SOCK_PATH}:/var/run/docker.sock") appendMCPGatewayBaseEnvFlags(&containerCmd, payloadPathPrefix) appendMCPGatewayConditionalEnvFlags(&containerCmd, workflowData, engine, hasGitHub, githubTool, tools) + appendMCPGatewaySafeOutputsInputEnvFlags(&containerCmd, safeOutputsInputEnvVars) appendMCPGatewayCustomAndHTTPEnvFlags(&containerCmd, workflowData, gatewayConfig, mcpEnvVars, hasGitHub, githubTool, tools, engine) if payloadDir != "" { containerCmd.WriteString(" -v " + payloadDir + ":" + payloadDir + ":rw") @@ -351,6 +362,20 @@ func appendMCPGatewayConditionalEnvFlags(containerCmd *strings.Builder, workflow } } +// appendMCPGatewaySafeOutputsInputEnvFlags adds -e flags for GH_AW_INPUT_* environment variables +// that are referenced by the safe-outputs config. These variables are written into config.json as +// ${GH_AW_INPUT_…} shell-style placeholders at compile time and must be resolvable inside the +// containerised safe-outputs MCP server at runtime. +func appendMCPGatewaySafeOutputsInputEnvFlags(containerCmd *strings.Builder, safeOutputsInputEnvVars map[string]string) { + if len(safeOutputsInputEnvVars) == 0 { + return + } + envVarNames := sliceutil.SortedKeys(safeOutputsInputEnvVars) + for _, envVarName := range envVarNames { + containerCmd.WriteString(" -e " + envVarName) + } +} + func appendMCPGatewayCustomAndHTTPEnvFlags(containerCmd *strings.Builder, workflowData *WorkflowData, gatewayConfig *MCPGatewayRuntimeConfig, mcpEnvVars map[string]string, hasGitHub bool, githubTool map[string]any, tools map[string]any, engine CodingAgentEngine) { if len(gatewayConfig.Env) > 0 { envVarNames := sliceutil.MapKeys(gatewayConfig.Env) diff --git a/pkg/workflow/mcp_setup_generator.go b/pkg/workflow/mcp_setup_generator.go index 98ccaf384c6..08f0e7faff9 100644 --- a/pkg/workflow/mcp_setup_generator.go +++ b/pkg/workflow/mcp_setup_generator.go @@ -125,7 +125,16 @@ func (c *Compiler) generateMCPSetup(yaml *strings.Builder, tools map[string]any, if err := generateMCPScriptsSetup(yaml, workflowData); err != nil { return fmt.Errorf("failed to generate mcp-scripts setup YAML: %w", err) } - return generateMCPGatewaySetup(yaml, tools, mcpTools, engine, workflowData, hasAgenticWorkflows) + // Extract GH_AW_INPUT_* env vars from the safe-outputs config so the MCP + // gateway container receives them in its -e allowlist and the nested + // safe-outputs container inherits them via its env_vars/env allowlist. + // Without this, any safe-outputs field that references ${{ inputs.* }} is + // written to config.json as a ${GH_AW_INPUT_…} placeholder that the + // containerised MCP server cannot resolve, causing failures such as + // "No remote refs available for merge-base calculation" when using a + // dynamic base-branch. + workflowData.SafeOutputsInputEnvVars = extractSafeOutputsInputEnvVars(safeOutputConfig) + return generateMCPGatewaySetup(yaml, tools, mcpTools, engine, workflowData, hasAgenticWorkflows, workflowData.SafeOutputsInputEnvVars) } func collectMCPTools(workflowData *WorkflowData) []string { @@ -172,3 +181,25 @@ func generateSafeOutputsConfigIfEnabled(workflowData *WorkflowData) (string, err } return safeOutputConfig, nil } + +// extractSafeOutputsInputEnvVars returns a map of GH_AW_INPUT_* environment variable names +// to their GitHub Actions expressions for all ${{ inputs.* }} references in the safe-outputs +// config. The map is used to populate the MCP gateway step env block AND the docker run -e +// allowlist, so the containerised safe-outputs MCP server can resolve the ${GH_AW_INPUT_…} +// shell-style placeholders written into config.json at compile time. +func extractSafeOutputsInputEnvVars(safeOutputConfig string) map[string]string { + if safeOutputConfig == "" { + return nil + } + envKeys, envValues := buildSafeOutputsConfigRuntimeEnvVars(safeOutputConfig) + result := make(map[string]string) + for _, key := range envKeys { + if strings.HasPrefix(key, "GH_AW_INPUT_") { + result[key] = envValues[key] + } + } + if len(result) == 0 { + return nil + } + return result +} diff --git a/pkg/workflow/safe_outputs_dynamic_allowed_repos_test.go b/pkg/workflow/safe_outputs_dynamic_allowed_repos_test.go index 6dc6ef4ae7d..c887bd82c85 100644 --- a/pkg/workflow/safe_outputs_dynamic_allowed_repos_test.go +++ b/pkg/workflow/safe_outputs_dynamic_allowed_repos_test.go @@ -58,10 +58,10 @@ Test workflow "Generate Safe Outputs Config step should map inputs.target_repo to an env var") assert.Contains(t, compiled, "GH_AW_INPUT_BASE_BRANCH: ${{ inputs.base_branch }}", "Generate Safe Outputs Config step should map inputs.base_branch to an env var") - assert.GreaterOrEqual(t, strings.Count(compiled, "GH_AW_INPUT_TARGET_REPO: ${{ inputs.target_repo }}"), 1, - "Dynamic input env vars should be exported anywhere the runtime still resolves placeholders in memory") - assert.GreaterOrEqual(t, strings.Count(compiled, "GH_AW_INPUT_BASE_BRANCH: ${{ inputs.base_branch }}"), 1, - "Dynamic input env vars should be exported anywhere the runtime still resolves placeholders in memory") + assert.GreaterOrEqual(t, strings.Count(compiled, "GH_AW_INPUT_TARGET_REPO: ${{ inputs.target_repo }}"), 2, + "GH_AW_INPUT_TARGET_REPO should appear in both the Generate Safe Outputs Config step and the Start MCP Gateway step") + assert.GreaterOrEqual(t, strings.Count(compiled, "GH_AW_INPUT_BASE_BRANCH: ${{ inputs.base_branch }}"), 2, + "GH_AW_INPUT_BASE_BRANCH should appear in both the Generate Safe Outputs Config step and the Start MCP Gateway step") assert.Contains(t, compiled, `"allowed_repos":"${GH_AW_INPUT_TARGET_REPO}"`, "config.json payload should preserve env placeholder for allowed_repos") assert.Contains(t, compiled, `"allowed_base_branches":"${GH_AW_INPUT_BASE_BRANCH}"`, @@ -74,6 +74,23 @@ Test workflow unquotedHeredocPattern := regexp.MustCompile(`cat > "\$\{RUNNER_TEMP\}/gh-aw/safeoutputs/config\.json" << GH_AW_SAFE_OUTPUTS_CONFIG_[0-9a-f]{16}_EOF`) assert.False(t, unquotedHeredocPattern.MatchString(compiled), "Safe outputs config heredoc should never be unquoted for dynamic config placeholders") + + // Verify GH_AW_INPUT_* env vars are forwarded to the MCP gateway container via -e flags. + // Without this the safe-outputs MCP server cannot resolve ${GH_AW_INPUT_…} placeholders in + // config.json, causing failures such as "No remote refs available for merge-base calculation". + assert.Contains(t, compiled, "-e GH_AW_INPUT_TARGET_REPO", + "MCP gateway docker run command should include -e GH_AW_INPUT_TARGET_REPO so the container can resolve the placeholder") + assert.Contains(t, compiled, "-e GH_AW_INPUT_BASE_BRANCH", + "MCP gateway docker run command should include -e GH_AW_INPUT_BASE_BRANCH so the container can resolve the placeholder") + + // Verify GH_AW_INPUT_* vars also appear in the nested safe-outputs MCP server container + // config (the JSON env block written into mcp-servers.json). The gateway forwards env vars + // to nested containers only if they appear in that config's env allowlist; without this the + // containerised safe-outputs server cannot resolve the ${GH_AW_INPUT_…} placeholders. + assert.Contains(t, compiled, `"GH_AW_INPUT_TARGET_REPO": "\${GH_AW_INPUT_TARGET_REPO}"`, + "safe-outputs MCP server JSON env block should include GH_AW_INPUT_TARGET_REPO so the nested container inherits the value") + assert.Contains(t, compiled, `"GH_AW_INPUT_BASE_BRANCH": "\${GH_AW_INPUT_BASE_BRANCH}"`, + "safe-outputs MCP server JSON env block should include GH_AW_INPUT_BASE_BRANCH so the nested container inherits the value") } func TestSafeOutputsConfigPreservesSecretPlaceholdersOnDisk(t *testing.T) { @@ -121,3 +138,65 @@ Test workflow assert.False(t, unquotedHeredocPattern.MatchString(compiled), "Safe outputs config heredoc should not be unquoted when secrets are present") } + +// TestSafeOutputsDynamicBaseBranchPassedToMCPContainer is a regression test for +// github/gh-aw#47795: when create-pull-request uses a dynamic base-branch such as +// ${{ inputs.base_branch }}, the compiled workflow must forward the resolved value to +// the MCP gateway container both in the step env block and in the docker run -e allowlist. +// Without this the safe-outputs MCP server cannot resolve the ${GH_AW_INPUT_BASE_BRANCH} +// placeholder in config.json and fails with "No remote refs available for merge-base calculation". +func TestSafeOutputsDynamicBaseBranchPassedToMCPContainer(t *testing.T) { + tmpDir := testutil.TempDir(t, "safe-outputs-dynamic-base-branch") + mdFile := filepath.Join(tmpDir, "dynamic-base-branch.md") + + content := `--- +name: Dynamic Base Branch +on: + workflow_dispatch: + inputs: + base_branch: + required: false + default: develop + type: string +engine: copilot +safe-outputs: + create-pull-request: + base-branch: ${{ inputs.base_branch }} +--- + +Test workflow +` + + err := os.WriteFile(mdFile, []byte(content), 0600) + require.NoError(t, err, "Failed to write test workflow markdown") + + compiler := NewCompiler() + err = compiler.CompileWorkflow(mdFile) + require.NoError(t, err, "Failed to compile workflow") + + lockFile := stringutil.MarkdownToLockFile(mdFile) + compiledBytes, err := os.ReadFile(lockFile) + require.NoError(t, err, "Failed to read compiled workflow") + compiled := string(compiledBytes) + + // The placeholder must appear in config.json on disk so no shell expansion happens at write-time. + assert.Contains(t, compiled, `"base_branch":"${GH_AW_INPUT_BASE_BRANCH}"`, + "config.json payload should preserve the ${GH_AW_INPUT_BASE_BRANCH} placeholder for runtime resolution") + + // The env var must be set in both the "Generate Safe Outputs Config" step and the + // "Start MCP Gateway" step so the MCP container process can resolve the placeholder. + assert.GreaterOrEqual(t, strings.Count(compiled, "GH_AW_INPUT_BASE_BRANCH: ${{ inputs.base_branch }}"), 2, + "GH_AW_INPUT_BASE_BRANCH must appear in at least two step env blocks: Generate Safe Outputs Config and Start MCP Gateway") + + // The docker run command must include -e GH_AW_INPUT_BASE_BRANCH so the container + // inherits the value from the runner step environment. + assert.Contains(t, compiled, "-e GH_AW_INPUT_BASE_BRANCH", + "MCP gateway docker run command must include -e GH_AW_INPUT_BASE_BRANCH so the containerised MCP server can resolve the placeholder") + + // The nested safe-outputs MCP server container config must include GH_AW_INPUT_BASE_BRANCH + // in its env allowlist. The gateway only forwards env vars to nested containers that are + // listed in the server config's env block; without this the placeholder remains unresolved + // inside the containerised safe-outputs server, causing the merge-base failure. + assert.Contains(t, compiled, `"GH_AW_INPUT_BASE_BRANCH": "\${GH_AW_INPUT_BASE_BRANCH}"`, + "safe-outputs MCP server JSON env block must include GH_AW_INPUT_BASE_BRANCH so the nested container inherits the runtime value") +} diff --git a/pkg/workflow/workflow_data.go b/pkg/workflow/workflow_data.go index 38e0442667f..4531c330282 100644 --- a/pkg/workflow/workflow_data.go +++ b/pkg/workflow/workflow_data.go @@ -130,6 +130,7 @@ type WorkflowData struct { SandboxConfig *SandboxConfig // parsed sandbox configuration (AWF or SRT) RunnerConfig *RunnerConfig // parsed runner topology configuration (e.g., arc-dind) SafeOutputs *SafeOutputsConfig // output configuration for automatic output routes + SafeOutputsInputEnvVars map[string]string // GH_AW_INPUT_* env vars referenced by safe-outputs config; populated during MCP setup generation so renderers can forward them to the nested container MCPScripts *MCPScriptsConfig // mcp-scripts configuration for custom MCP tools LabelNames []string // label names that must match for pull_request_target labeled events (on.labels) Roles []string // permission levels required to trigger workflow