Skip to content
Merged
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
6 changes: 4 additions & 2 deletions pkg/cli/mcp_add.go
Original file line number Diff line number Diff line change
Expand Up @@ -144,8 +144,10 @@ func AddMCPTool(workflowFile string, mcpServerID string, registryURL string, tra
mcpAddLog.Print("Compiling workflow after adding MCP tool")
compiler := workflow.NewCompiler(verbose, "", "")
if err := compiler.CompileWorkflow(workflowPath); err != nil {
mcpAddLog.Printf("Workflow compilation failed: %v", err)
fmt.Println(console.FormatWarningMessage(fmt.Sprintf("Workflow compilation failed: %v", err)))
// Security fix for CWE-312, CWE-315, CWE-359: Avoid logging detailed error messages
// that could contain sensitive information from secret references
mcpAddLog.Print("Workflow compilation failed")
fmt.Println(console.FormatWarningMessage("Workflow compilation failed. Please check your workflow configuration."))
Comment on lines +147 to +150
Copy link

Copilot AI Nov 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This security fix is overly broad and doesn't address the actual vulnerability. The fix suppresses all compilation error details, but the actual risk is specifically from logging secret values or references.

Analysis of the claim:
The PR description states that "sensitive data from secret configurations could flow through error messages to logging output." However:

  1. Secret references are safe to log: Expressions like ${{ secrets.MY_SECRET }} are placeholder references, not the actual secret values. GitHub Actions never exposes the actual secret values in error messages - it only shows the reference syntax.

  2. CompileWorkflow errors don't contain secret values: The compilation errors come from validation logic (schema validation, permission checks, expression safety, etc.) and contain structural information about the workflow configuration, not secret values.

  3. Actual secret values are never in the workflow file: The workflow markdown file only contains secret references (e.g., ${{ secrets.API_KEY }}), never actual secret values. Secret values are stored in GitHub's encrypted secrets store and are only injected at runtime by GitHub Actions.

Better solution:
Instead of suppressing all error details (which hurts debugging), implement targeted sanitization of the error message if specific sensitive patterns are detected. However, based on the code review, there's no evidence that CompileWorkflow errors actually contain sensitive information that needs redaction.

Impact on user experience:
Users lose valuable debugging information when compilation fails. The generic message "Workflow compilation failed. Please check your workflow configuration." doesn't tell them what's wrong, forcing them to run gh aw compile separately to see the actual error.

Suggested change
// Security fix for CWE-312, CWE-315, CWE-359: Avoid logging detailed error messages
// that could contain sensitive information from secret references
mcpAddLog.Print("Workflow compilation failed")
fmt.Println(console.FormatWarningMessage("Workflow compilation failed. Please check your workflow configuration."))
mcpAddLog.Print(fmt.Sprintf("Workflow compilation failed: %v", err))
fmt.Println(console.FormatErrorMessage(fmt.Sprintf("Workflow compilation failed: %v", err)))

Copilot uses AI. Check for mistakes.
fmt.Println(console.FormatInfoMessage("You can fix the issues and run 'gh aw compile' manually"))
} else {
mcpAddLog.Print("Workflow compiled successfully")
Expand Down
Loading