diff --git a/.changeset/patch-add-changeset-pr-5782.md b/.changeset/patch-add-changeset-pr-5782.md new file mode 100644 index 0000000000..3866c37988 --- /dev/null +++ b/.changeset/patch-add-changeset-pr-5782.md @@ -0,0 +1,5 @@ +--- +"gh-aw": patch +--- + +Add a changeset for pull request #5782. The PR description was empty, so this defaults to a patch-level changeset for internal/tooling/documentation changes. diff --git a/.github/workflows/daily-performance-summary.lock.yml b/.github/workflows/daily-performance-summary.lock.yml index e4513ae5e6..1cc5a8da6a 100644 --- a/.github/workflows/daily-performance-summary.lock.yml +++ b/.github/workflows/daily-performance-summary.lock.yml @@ -4152,11 +4152,10 @@ jobs: env_vars = ["GITHUB_PERSONAL_ACCESS_TOKEN"] [mcp_servers.safeinputs] - command = "node" - args = [ - "/tmp/gh-aw/safe-inputs/mcp-server.cjs", - ] - env_vars = ["GH_TOKEN"] + type = "http" + url = "http://localhost:$GH_AW_SAFE_INPUTS_PORT" + headers = { Authorization = "Bearer $GH_AW_SAFE_INPUTS_API_KEY" } + env_vars = ["GH_AW_SAFE_INPUTS_PORT", "GH_AW_SAFE_INPUTS_API_KEY", "GH_TOKEN"] [mcp_servers.safeoutputs] command = "node" diff --git a/.github/workflows/release.lock.yml b/.github/workflows/release.lock.yml index bfddcfddf9..0ba6e35b97 100644 --- a/.github/workflows/release.lock.yml +++ b/.github/workflows/release.lock.yml @@ -5973,19 +5973,19 @@ jobs: - name: Download Go modules run: go mod download - name: Generate SBOM (SPDX format) - uses: anchore/sbom-action@fbfd9c6c189226748411491745178e0c2017392d # v0 + uses: anchore/sbom-action@fbfd9c6c189226748411491745178e0c2017392d # v0.20.10 with: artifact-name: sbom.spdx.json format: spdx-json output-file: sbom.spdx.json - name: Generate SBOM (CycloneDX format) - uses: anchore/sbom-action@fbfd9c6c189226748411491745178e0c2017392d # v0 + uses: anchore/sbom-action@fbfd9c6c189226748411491745178e0c2017392d # v0.20.10 with: artifact-name: sbom.cdx.json format: cyclonedx-json output-file: sbom.cdx.json - name: Upload SBOM artifacts - uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4 + uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 # v5 with: name: sbom-artifacts path: | diff --git a/pkg/workflow/mcp_renderer.go b/pkg/workflow/mcp_renderer.go index 8548b671c8..f88ea71ba1 100644 --- a/pkg/workflow/mcp_renderer.go +++ b/pkg/workflow/mcp_renderer.go @@ -219,17 +219,20 @@ func (r *MCPConfigRendererUnified) RenderSafeInputsMCP(yaml *strings.Builder, sa } // renderSafeInputsTOML generates Safe Inputs MCP configuration in TOML format +// Uses HTTP transport for consistency with JSON format (Copilot/Claude) func (r *MCPConfigRendererUnified) renderSafeInputsTOML(yaml *strings.Builder, safeInputs *SafeInputsConfig) { + envVars := getSafeInputsEnvVars(safeInputs) + yaml.WriteString(" \n") yaml.WriteString(" [mcp_servers." + constants.SafeInputsMCPServerID + "]\n") - yaml.WriteString(" command = \"node\"\n") - yaml.WriteString(" args = [\n") - yaml.WriteString(" \"/tmp/gh-aw/safe-inputs/mcp-server.cjs\",\n") - yaml.WriteString(" ]\n") - // Add environment variables from safe-inputs config - envVars := getSafeInputsEnvVars(safeInputs) + yaml.WriteString(" type = \"http\"\n") + yaml.WriteString(" url = \"http://localhost:$GH_AW_SAFE_INPUTS_PORT\"\n") + yaml.WriteString(" headers = { Authorization = \"Bearer $GH_AW_SAFE_INPUTS_API_KEY\" }\n") + + // Add environment variables: server config + tool-specific vars + envVarsWithServerConfig := append([]string{"GH_AW_SAFE_INPUTS_PORT", "GH_AW_SAFE_INPUTS_API_KEY"}, envVars...) yaml.WriteString(" env_vars = [") - for i, envVar := range envVars { + for i, envVar := range envVarsWithServerConfig { if i > 0 { yaml.WriteString(", ") } diff --git a/pkg/workflow/safe_inputs_http_codex_test.go b/pkg/workflow/safe_inputs_http_codex_test.go new file mode 100644 index 0000000000..7d890bbb75 --- /dev/null +++ b/pkg/workflow/safe_inputs_http_codex_test.go @@ -0,0 +1,183 @@ +package workflow + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +// TestCodexSafeInputsHTTPTransport verifies that Codex engine uses HTTP transport for safe-inputs +// (not stdio transport) to be consistent with Copilot and Claude engines +func TestCodexSafeInputsHTTPTransport(t *testing.T) { + // Create a temporary workflow file + tempDir := t.TempDir() + workflowPath := filepath.Join(tempDir, "test-workflow.md") + + workflowContent := `--- +on: workflow_dispatch +engine: codex +safe-inputs: + test-tool: + description: Test tool + script: | + return { result: "test" }; +--- + +Test safe-inputs HTTP transport for Codex +` + + err := os.WriteFile(workflowPath, []byte(workflowContent), 0644) + if err != nil { + t.Fatalf("Failed to write workflow file: %v", err) + } + + // Compile the workflow + compiler := NewCompiler(false, "", "test") + err = compiler.CompileWorkflow(workflowPath) + if err != nil { + t.Fatalf("Failed to compile workflow: %v", err) + } + + // Read the generated lock file + lockPath := strings.TrimSuffix(workflowPath, ".md") + ".lock.yml" + lockContent, err := os.ReadFile(lockPath) + if err != nil { + t.Fatalf("Failed to read lock file: %v", err) + } + + yamlStr := string(lockContent) + + // Verify that the HTTP server configuration steps are generated + expectedSteps := []string{ + "Generate Safe Inputs MCP Server Config", + "Start Safe Inputs MCP HTTP Server", + "Setup MCPs", + } + + for _, stepName := range expectedSteps { + if !strings.Contains(yamlStr, stepName) { + t.Errorf("Expected step not found in workflow: %q", stepName) + } + } + + // Verify HTTP transport in TOML config (not stdio) + if !strings.Contains(yamlStr, "[mcp_servers.safeinputs]") { + t.Error("Safe-inputs MCP server config section not found") + } + + // Should have explicit type field + codexConfigSection := extractCodexConfigSection(yamlStr) + if !strings.Contains(codexConfigSection, `type = "http"`) { + t.Error("Expected type field set to 'http' in TOML format") + } + + // Should use HTTP transport (url + headers) + if !strings.Contains(yamlStr, `url = "http://localhost:$GH_AW_SAFE_INPUTS_PORT"`) { + t.Error("Expected HTTP URL config not found in TOML format") + } + + if !strings.Contains(yamlStr, `headers = { Authorization = "Bearer $GH_AW_SAFE_INPUTS_API_KEY" }`) { + t.Error("Expected HTTP headers config not found in TOML format") + } + + // Should NOT use stdio transport (command + args to node) + if strings.Contains(codexConfigSection, `command = "node"`) { + t.Error("Codex config should not use stdio transport (command = 'node'), should use HTTP") + } + + if strings.Contains(codexConfigSection, `args = [`) && strings.Contains(codexConfigSection, `/tmp/gh-aw/safe-inputs/mcp-server.cjs`) { + t.Error("Codex config should not use stdio transport with mcp-server.cjs args, should use HTTP") + } + + // Verify environment variables are included + if !strings.Contains(codexConfigSection, "GH_AW_SAFE_INPUTS_PORT") { + t.Error("Expected GH_AW_SAFE_INPUTS_PORT env var in config") + } + + if !strings.Contains(codexConfigSection, "GH_AW_SAFE_INPUTS_API_KEY") { + t.Error("Expected GH_AW_SAFE_INPUTS_API_KEY env var in config") + } + + t.Logf("✓ Codex engine correctly uses HTTP transport for safe-inputs") +} + +// extractCodexConfigSection extracts the Codex MCP config section from the workflow YAML +func extractCodexConfigSection(yamlContent string) string { + // Find the start of the safeinputs config + start := strings.Index(yamlContent, "[mcp_servers.safeinputs]") + if start == -1 { + return "" + } + + // Find the end (next section or EOF) + end := strings.Index(yamlContent[start:], "EOF") + if end == -1 { + return yamlContent[start:] + } + + return yamlContent[start : start+end] +} + +// TestCodexSafeInputsWithSecretsHTTPTransport verifies that environment variables +// from safe-inputs tools are properly passed through with HTTP transport +func TestCodexSafeInputsWithSecretsHTTPTransport(t *testing.T) { + tempDir := t.TempDir() + workflowPath := filepath.Join(tempDir, "test-workflow.md") + + workflowContent := `--- +on: workflow_dispatch +engine: codex +safe-inputs: + api-call: + description: Call an API + env: + API_KEY: ${{ secrets.API_KEY }} + GH_TOKEN: ${{ github.token }} + script: | + return { result: "test" }; +--- + +Test safe-inputs with secrets +` + + err := os.WriteFile(workflowPath, []byte(workflowContent), 0644) + if err != nil { + t.Fatalf("Failed to write workflow file: %v", err) + } + + compiler := NewCompiler(false, "", "test") + err = compiler.CompileWorkflow(workflowPath) + if err != nil { + t.Fatalf("Failed to compile workflow: %v", err) + } + + lockPath := strings.TrimSuffix(workflowPath, ".md") + ".lock.yml" + lockContent, err := os.ReadFile(lockPath) + if err != nil { + t.Fatalf("Failed to read lock file: %v", err) + } + + yamlStr := string(lockContent) + codexConfigSection := extractCodexConfigSection(yamlStr) + + // Verify tool-specific env vars are included in HTTP transport config + if !strings.Contains(codexConfigSection, "API_KEY") { + t.Error("Expected API_KEY env var in safe-inputs config") + } + + if !strings.Contains(codexConfigSection, "GH_TOKEN") { + t.Error("Expected GH_TOKEN env var in safe-inputs config") + } + + // Verify env vars are set in Setup MCPs step + if !strings.Contains(yamlStr, "API_KEY: ${{ secrets.API_KEY }}") { + t.Error("Expected API_KEY secret in Setup MCPs env section") + } + + if !strings.Contains(yamlStr, "GH_TOKEN: ${{ github.token }}") { + t.Error("Expected GH_TOKEN in Setup MCPs env section") + } + + t.Logf("✓ Codex engine correctly passes secrets through HTTP transport") +}