Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
50 changes: 30 additions & 20 deletions pkg/workflow/codex_mcp.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,16 +36,17 @@ func (e *CodexEngine) RenderMCPConfig(yaml *strings.Builder, tools map[string]an
})
}

delimiter := GenerateHeredocDelimiterFromSeed("MCP_CONFIG", workflowData.FrontmatterHash)
yaml.WriteString(" cat > \"${RUNNER_TEMP}/gh-aw/mcp-config/config.toml\" << " + delimiter + "\n")
// Build the heredoc content into a temporary buffer so we can derive the
// delimiter from a SHA-256 hash of the content before writing it to the YAML output.
var mcpConfigContent strings.Builder

// Add history configuration to disable persistence
yaml.WriteString(" [history]\n")
yaml.WriteString(" persistence = \"none\"\n")
mcpConfigContent.WriteString(" [history]\n")
mcpConfigContent.WriteString(" persistence = \"none\"\n")

// Add shell environment policy to control which environment variables are passed through
// This is a security feature to prevent accidental exposure of secrets
e.renderShellEnvironmentPolicy(yaml, tools, mcpTools)
e.renderShellEnvironmentPolicy(&mcpConfigContent, tools, mcpTools)

// Expand neutral tools (like playwright: null) to include the copilot agent tools
expandedTools := e.expandNeutralToolsToCodexToolsFromMap(tools)
Expand All @@ -56,47 +57,52 @@ func (e *CodexEngine) RenderMCPConfig(yaml *strings.Builder, tools map[string]an
switch toolName {
case "github":
githubTool := expandedTools["github"]
renderer.RenderGitHubMCP(yaml, githubTool, workflowData)
renderer.RenderGitHubMCP(&mcpConfigContent, githubTool, workflowData)
case "playwright":
playwrightTool := expandedTools["playwright"]
renderer.RenderPlaywrightMCP(yaml, playwrightTool)
renderer.RenderPlaywrightMCP(&mcpConfigContent, playwrightTool)
case "agentic-workflows":
renderer.RenderAgenticWorkflowsMCP(yaml)
renderer.RenderAgenticWorkflowsMCP(&mcpConfigContent)
case "safe-outputs":
// Add safe-outputs MCP server if safe-outputs are configured
hasSafeOutputs := workflowData != nil && workflowData.SafeOutputs != nil && HasSafeOutputsEnabled(workflowData.SafeOutputs)
if hasSafeOutputs {
renderer.RenderSafeOutputsMCP(yaml, workflowData)
renderer.RenderSafeOutputsMCP(&mcpConfigContent, workflowData)
}
case "mcp-scripts":
// Add mcp-scripts MCP server if mcp-scripts are configured and feature flag is enabled
hasMCPScripts := workflowData != nil && IsMCPScriptsEnabled(workflowData.MCPScripts)
if hasMCPScripts {
renderer.RenderMCPScriptsMCP(yaml, workflowData.MCPScripts, workflowData)
renderer.RenderMCPScriptsMCP(&mcpConfigContent, workflowData.MCPScripts, workflowData)
}
default:
// Handle custom MCP tools using shared helper (with adapter for isLast parameter)
HandleCustomMCPToolInSwitch(yaml, toolName, expandedTools, false, func(yaml *strings.Builder, toolName string, toolConfig map[string]any, isLast bool) error {
HandleCustomMCPToolInSwitch(&mcpConfigContent, toolName, expandedTools, false, func(yaml *strings.Builder, toolName string, toolConfig map[string]any, isLast bool) error {
return e.renderCodexMCPConfigWithContext(yaml, toolName, toolConfig, workflowData)
})
}
}

// Append custom config if provided
if workflowData.EngineConfig != nil && workflowData.EngineConfig.Config != "" {
yaml.WriteString(" \n")
yaml.WriteString(" # Custom configuration\n")
mcpConfigContent.WriteString(" \n")
mcpConfigContent.WriteString(" # Custom configuration\n")
// Write the custom config line by line with proper indentation
configLines := strings.SplitSeq(workflowData.EngineConfig.Config, "\n")
for line := range configLines {
if strings.TrimSpace(line) != "" {
yaml.WriteString(" " + line + "\n")
mcpConfigContent.WriteString(" " + line + "\n")
} else {
yaml.WriteString(" \n")
mcpConfigContent.WriteString(" \n")
}
}
}

// Derive the delimiter from the content so it is stable across builds.
delimiter := GenerateHeredocDelimiterFromContent("MCP_CONFIG", mcpConfigContent.String())
yaml.WriteString(" cat > \"${RUNNER_TEMP}/gh-aw/mcp-config/config.toml\" << " + delimiter + "\n")
yaml.WriteString(mcpConfigContent.String())

// End the heredoc for config.toml
yaml.WriteString(" " + delimiter + "\n")

Expand Down Expand Up @@ -127,20 +133,24 @@ func (e *CodexEngine) RenderMCPConfig(yaml *strings.Builder, tools map[string]an
yaml.WriteString(" # Sync converter output to writable CODEX_HOME for Codex\n")
yaml.WriteString(" mkdir -p /tmp/gh-aw/mcp-config\n")

shellPolicyDelimiter := GenerateHeredocDelimiterFromSeed("CODEX_SHELL_POLICY", workflowData.FrontmatterHash)
yaml.WriteString(" cat > \"/tmp/gh-aw/mcp-config/config.toml\" << " + shellPolicyDelimiter + "\n")
// Build the shell-policy heredoc content into a temp buffer so the delimiter
// can be derived from a SHA-256 hash of the content for build stability.
var shellPolicyContent strings.Builder
if isFirewallEnabled(workflowData) {
e.renderOpenAIProxyProviderToml(yaml, " ")
e.renderOpenAIProxyProviderToml(&shellPolicyContent, " ")
}
e.renderShellEnvironmentPolicyToml(yaml, tools, mcpTools, " ")
e.renderShellEnvironmentPolicyToml(&shellPolicyContent, tools, mcpTools, " ")
shellPolicyDelimiter := GenerateHeredocDelimiterFromContent("CODEX_SHELL_POLICY", shellPolicyContent.String())
yaml.WriteString(" cat > \"/tmp/gh-aw/mcp-config/config.toml\" << " + shellPolicyDelimiter + "\n")
yaml.WriteString(shellPolicyContent.String())
yaml.WriteString(" " + shellPolicyDelimiter + "\n")
if isFirewallEnabled(workflowData) {
e.renderAppendConvertedConfigWithoutOpenAIProxy(yaml)
} else {
yaml.WriteString(" cat \"${RUNNER_TEMP}/gh-aw/mcp-config/config.toml\" >> \"/tmp/gh-aw/mcp-config/config.toml\"\n")
}
if workflowData.EngineConfig != nil && strings.TrimSpace(workflowData.EngineConfig.Config) != "" {
customConfigDelimiter := GenerateHeredocDelimiterFromSeed("CODEX_CUSTOM_CONFIG", workflowData.FrontmatterHash)
customConfigDelimiter := GenerateHeredocDelimiterFromContent("CODEX_CUSTOM_CONFIG", workflowData.EngineConfig.Config)
yaml.WriteString(" \n")
yaml.WriteString(" # Append engine-level custom Codex config\n")
yaml.WriteString(" cat >> \"/tmp/gh-aw/mcp-config/config.toml\" << " + customConfigDelimiter + "\n")
Expand Down
6 changes: 3 additions & 3 deletions pkg/workflow/compiler_safe_outputs_job.go
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ func (c *Compiler) buildSafeOutputsHandlerOutputsAndActionSteps(data *WorkflowDa
// This must run before the handler manager step so the files are available for require()
if len(data.SafeOutputs.Scripts) > 0 {
consolidatedSafeOutputsJobLog.Printf("Adding setup step for %d custom safe-output script(s)", len(data.SafeOutputs.Scripts))
scriptSetupSteps, err := buildCustomScriptFilesStep(data.SafeOutputs.Scripts, data.FrontmatterHash)
scriptSetupSteps, err := buildCustomScriptFilesStep(data.SafeOutputs.Scripts)
if err != nil {
return nil, nil, nil, fmt.Errorf("failed to build custom script files step: %w", err)
}
Expand Down Expand Up @@ -874,7 +874,7 @@ func generateSafeOutputScriptContent(scriptName string, scriptConfig *SafeScript
// Users write only the handler body; the compiler wraps it with config destructuring,
// the handler function, and module.exports boilerplate.
// Each script is written using a heredoc to avoid shell quoting issues.
func buildCustomScriptFilesStep(scripts map[string]*SafeScriptConfig, frontmatterHash string) ([]string, error) {
func buildCustomScriptFilesStep(scripts map[string]*SafeScriptConfig) ([]string, error) {
if len(scripts) == 0 {
return nil, nil
}
Expand All @@ -895,8 +895,8 @@ func buildCustomScriptFilesStep(scripts map[string]*SafeScriptConfig, frontmatte
normalizedName := stringutil.NormalizeSafeOutputIdentifier(scriptName)
filename := safeOutputScriptFilename(normalizedName)
filePath := SetupActionDestinationShell + "/" + filename
delimiter := GenerateHeredocDelimiterFromSeed("SAFE_OUTPUT_SCRIPT_"+strings.ToUpper(normalizedName), frontmatterHash)
scriptContent := generateSafeOutputScriptContent(scriptName, scriptConfig)
delimiter := GenerateHeredocDelimiterFromContent("SAFE_OUTPUT_SCRIPT_"+strings.ToUpper(normalizedName), scriptContent)

if err := ValidateHeredocContent(scriptContent, delimiter); err != nil {
return nil, fmt.Errorf("safe-output script %q: %w", scriptName, err)
Expand Down
2 changes: 1 addition & 1 deletion pkg/workflow/mcp_renderer.go
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ func RenderJSONMCPConfig(
// Get the generated configuration
generatedConfig := configBuilder.String()

delimiter := GenerateHeredocDelimiterFromSeed("MCP_CONFIG", workflowData.FrontmatterHash)
delimiter := GenerateHeredocDelimiterFromContent("MCP_CONFIG", generatedConfig)
// Resolve the node binary to its absolute path so the command is robust
// against PATH modifications that may occur later in the workflow.
yaml.WriteString(" GH_AW_NODE=$(which node 2>/dev/null || command -v node 2>/dev/null || echo node)\n")
Expand Down
14 changes: 7 additions & 7 deletions pkg/workflow/mcp_setup_generator.go
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@ func generateSafeOutputsSetup(c *Compiler, yaml *strings.Builder, safeOutputConf
yaml.WriteString(" mkdir -p \"${RUNNER_TEMP}/gh-aw/safeoutputs/upload-artifacts\"\n")
}

delimiter := GenerateHeredocDelimiterFromSeed("SAFE_OUTPUTS_CONFIG", workflowData.FrontmatterHash)
delimiter := GenerateHeredocDelimiterFromContent("SAFE_OUTPUTS_CONFIG", sanitizedConfig)
if safeOutputConfig != "" {
yaml.WriteString(" cat > \"${RUNNER_TEMP}/gh-aw/safeoutputs/config.json\" << '" + delimiter + "'\n")
yaml.WriteString(" " + sanitizedConfig + "\n")
Expand Down Expand Up @@ -424,7 +424,7 @@ func generateMCPScriptsSetup(yaml *strings.Builder, workflowData *WorkflowData)
yaml.WriteString(" mkdir -p \"${RUNNER_TEMP}/gh-aw/mcp-scripts/logs\"\n")

toolsJSON := GenerateMCPScriptsToolsConfig(workflowData.MCPScripts)
toolsDelimiter := GenerateHeredocDelimiterFromSeed("MCP_SCRIPTS_TOOLS", workflowData.FrontmatterHash)
toolsDelimiter := GenerateHeredocDelimiterFromContent("MCP_SCRIPTS_TOOLS", toolsJSON)
if err := ValidateHeredocContent(toolsJSON, toolsDelimiter); err != nil {
return fmt.Errorf("mcp-scripts tools.json: %w", err)
}
Expand All @@ -435,7 +435,7 @@ func generateMCPScriptsSetup(yaml *strings.Builder, workflowData *WorkflowData)
yaml.WriteString(" " + toolsDelimiter + "\n")

mcpScriptsMCPServer := GenerateMCPScriptsMCPServerScript(workflowData.MCPScripts)
serverDelimiter := GenerateHeredocDelimiterFromSeed("MCP_SCRIPTS_SERVER", workflowData.FrontmatterHash)
serverDelimiter := GenerateHeredocDelimiterFromContent("MCP_SCRIPTS_SERVER", mcpScriptsMCPServer)
if err := ValidateHeredocContent(mcpScriptsMCPServer, serverDelimiter); err != nil {
return fmt.Errorf("mcp-scripts mcp-server.cjs: %w", err)
}
Expand Down Expand Up @@ -505,7 +505,7 @@ func generateMCPScriptsSetup(yaml *strings.Builder, workflowData *WorkflowData)
func appendMCPScriptToolFile(yaml *strings.Builder, workflowData *WorkflowData, toolName string, toolConfig *MCPScriptToolConfig) error {
if toolConfig.Script != "" {
toolScript := GenerateMCPScriptJavaScriptToolScript(toolConfig)
jsDelimiter := GenerateHeredocDelimiterFromSeed("MCP_SCRIPTS_JS_"+strings.ToUpper(toolName), workflowData.FrontmatterHash)
jsDelimiter := GenerateHeredocDelimiterFromContent("MCP_SCRIPTS_JS_"+strings.ToUpper(toolName), toolScript)
if err := ValidateHeredocContent(toolScript, jsDelimiter); err != nil {
return fmt.Errorf("mcp-scripts tool %q (js): %w", toolName, err)
}
Expand All @@ -518,7 +518,7 @@ func appendMCPScriptToolFile(yaml *strings.Builder, workflowData *WorkflowData,
}
if toolConfig.Run != "" {
toolScript := GenerateMCPScriptShellToolScript(toolConfig)
shDelimiter := GenerateHeredocDelimiterFromSeed("MCP_SCRIPTS_SH_"+strings.ToUpper(toolName), workflowData.FrontmatterHash)
shDelimiter := GenerateHeredocDelimiterFromContent("MCP_SCRIPTS_SH_"+strings.ToUpper(toolName), toolScript)
if err := ValidateHeredocContent(toolScript, shDelimiter); err != nil {
return fmt.Errorf("mcp-scripts tool %q (sh): %w", toolName, err)
}
Expand All @@ -532,7 +532,7 @@ func appendMCPScriptToolFile(yaml *strings.Builder, workflowData *WorkflowData,
}
if toolConfig.Py != "" {
toolScript := GenerateMCPScriptPythonToolScript(toolConfig)
pyDelimiter := GenerateHeredocDelimiterFromSeed("MCP_SCRIPTS_PY_"+strings.ToUpper(toolName), workflowData.FrontmatterHash)
pyDelimiter := GenerateHeredocDelimiterFromContent("MCP_SCRIPTS_PY_"+strings.ToUpper(toolName), toolScript)
if err := ValidateHeredocContent(toolScript, pyDelimiter); err != nil {
return fmt.Errorf("mcp-scripts tool %q (py): %w", toolName, err)
}
Expand All @@ -546,7 +546,7 @@ func appendMCPScriptToolFile(yaml *strings.Builder, workflowData *WorkflowData,
}
if toolConfig.Go != "" {
toolScript := GenerateMCPScriptGoToolScript(toolConfig)
goDelimiter := GenerateHeredocDelimiterFromSeed("MCP_SCRIPTS_GO_"+strings.ToUpper(toolName), workflowData.FrontmatterHash)
goDelimiter := GenerateHeredocDelimiterFromContent("MCP_SCRIPTS_GO_"+strings.ToUpper(toolName), toolScript)
if err := ValidateHeredocContent(toolScript, goDelimiter); err != nil {
return fmt.Errorf("mcp-scripts tool %q (go): %w", toolName, err)
}
Expand Down
6 changes: 3 additions & 3 deletions pkg/workflow/safe_scripts_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ func TestBuildCustomScriptFilesStep(t *testing.T) {
},
}

steps, err := buildCustomScriptFilesStep(scripts, "")
steps, err := buildCustomScriptFilesStep(scripts)

require.NoError(t, err, "Should not return error for valid scripts")
require.NotEmpty(t, steps, "Should produce steps")
Expand All @@ -282,11 +282,11 @@ func TestBuildCustomScriptFilesStep(t *testing.T) {

// TestBuildCustomScriptFilesStepEmpty verifies nil return for empty scripts
func TestBuildCustomScriptFilesStepEmpty(t *testing.T) {
steps, err := buildCustomScriptFilesStep(nil, "")
steps, err := buildCustomScriptFilesStep(nil)
require.NoError(t, err, "Should not return error for nil scripts")
assert.Nil(t, steps, "Should return nil for empty scripts")

stepsEmpty, err := buildCustomScriptFilesStep(map[string]*SafeScriptConfig{}, "")
stepsEmpty, err := buildCustomScriptFilesStep(map[string]*SafeScriptConfig{})
require.NoError(t, err, "Should not return error for empty map")
assert.Nil(t, stepsEmpty, "Should return nil for empty map")
}
Expand Down
19 changes: 19 additions & 0 deletions pkg/workflow/strings.go
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,25 @@ func GenerateHeredocDelimiterFromSeed(name string, seed string) string {
return "GH_AW_" + upperName + "_" + tag + "_EOF"
}

// GenerateHeredocDelimiterFromContent creates a stable heredoc delimiter derived from the
// content it wraps. The 16-character hex tag is the first 8 bytes of SHA-256 of the content,
// so the delimiter is identical across builds whenever the content is unchanged and changes
// only when the content changes — reducing unnecessary diff noise and merge conflicts.
//
// The name prefix (e.g. "PROMPT", "MCP_CONFIG") is included for readability and to ensure
// that two different heredocs wrapping identical content still produce distinct delimiters.
Comment on lines +193 to +199
func GenerateHeredocDelimiterFromContent(name string, content string) string {
h := sha256.New()
h.Write([]byte(strings.ToUpper(name)))
h.Write([]byte(content))
tag := hex.EncodeToString(h.Sum(nil)[:8])
upperName := strings.ToUpper(name)
if name == "" {
return "GH_AW_" + tag + "_EOF"
}
return "GH_AW_" + upperName + "_" + tag + "_EOF"
}

// heredocDelimiterRE matches randomized heredoc delimiters of the form GH_AW_<NAME>_<16hexchars>_EOF.
// Used to normalize delimiters when comparing compiled output to skip unnecessary writes.
var heredocDelimiterRE = regexp.MustCompile(`GH_AW_([A-Z0-9_]+)_[0-9a-f]{16}_EOF`)
Expand Down
62 changes: 62 additions & 0 deletions pkg/workflow/strings_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -577,6 +577,68 @@ func TestGenerateHeredocDelimiterFromSeed_EmptyName(t *testing.T) {
assert.True(t, pattern.MatchString(result), "Empty-name seeded delimiter should match GH_AW_<hex>_EOF, got %q", result)
}

func TestGenerateHeredocDelimiterFromContent_Stability(t *testing.T) {
content := `{"key":"value","items":[1,2,3]}`

// Same name and content must always produce the same delimiter.
result1 := GenerateHeredocDelimiterFromContent("MCP_CONFIG", content)
result2 := GenerateHeredocDelimiterFromContent("MCP_CONFIG", content)
assert.Equal(t, result1, result2, "Same name+content should produce identical delimiters")

// Format should match GH_AW_<NAME>_<16hex>_EOF.
pattern := regexp.MustCompile(`^GH_AW_MCP_CONFIG_[0-9a-f]{16}_EOF$`)
assert.True(t, pattern.MatchString(result1), "Content-based delimiter should match expected format, got %q", result1)
}

func TestGenerateHeredocDelimiterFromContent_ContentSensitive(t *testing.T) {
// Different content should produce different delimiters.
delim1 := GenerateHeredocDelimiterFromContent("PROMPT", "hello world")
delim2 := GenerateHeredocDelimiterFromContent("PROMPT", "hello world!")
assert.NotEqual(t, delim1, delim2, "Different content should produce different delimiters")
}

func TestGenerateHeredocDelimiterFromContent_DifferentNames(t *testing.T) {
content := "shared content"

// Same content, different names → different delimiters.
promptDelim := GenerateHeredocDelimiterFromContent("PROMPT", content)
mcpDelim := GenerateHeredocDelimiterFromContent("MCP_CONFIG", content)
safeDelim := GenerateHeredocDelimiterFromContent("SAFE_OUTPUTS_CONFIG", content)

assert.NotEqual(t, promptDelim, mcpDelim, "Different names should produce different delimiters")
assert.NotEqual(t, mcpDelim, safeDelim, "Different names should produce different delimiters")
assert.NotEqual(t, promptDelim, safeDelim, "Different names should produce different delimiters")

assert.Contains(t, promptDelim, "GH_AW_PROMPT_", "Delimiter should contain the name")
assert.Contains(t, mcpDelim, "GH_AW_MCP_CONFIG_", "Delimiter should contain the name")
assert.Contains(t, safeDelim, "GH_AW_SAFE_OUTPUTS_CONFIG_", "Delimiter should contain the name")
}

func TestGenerateHeredocDelimiterFromContent_EmptyName(t *testing.T) {
// Empty name should produce GH_AW_<16hex>_EOF (no name segment).
result := GenerateHeredocDelimiterFromContent("", "some content")
pattern := regexp.MustCompile(`^GH_AW_[0-9a-f]{16}_EOF$`)
assert.True(t, pattern.MatchString(result), "Empty-name content delimiter should match GH_AW_<hex>_EOF, got %q", result)
}

func TestGenerateHeredocDelimiterFromContent_EmptyContent(t *testing.T) {
// Empty content should still produce a valid, stable delimiter.
result1 := GenerateHeredocDelimiterFromContent("PROMPT", "")
result2 := GenerateHeredocDelimiterFromContent("PROMPT", "")
assert.Equal(t, result1, result2, "Empty content should produce a stable delimiter")

pattern := regexp.MustCompile(`^GH_AW_PROMPT_[0-9a-f]{16}_EOF$`)
assert.True(t, pattern.MatchString(result1), "Empty-content delimiter should match expected format, got %q", result1)
}

func TestGenerateHeredocDelimiterFromContent_NameCaseNormalized(t *testing.T) {
// Name should be uppercased, so mixed and upper case produce the same delimiter.
lower := GenerateHeredocDelimiterFromContent("prompt", "content")
upper := GenerateHeredocDelimiterFromContent("PROMPT", "content")
assert.Equal(t, lower, upper, "Name casing should be normalised to uppercase")
}


func TestValidateHeredocContent(t *testing.T) {
tests := []struct {
name string
Expand Down
Loading
Loading