[security-fix] Security Fix: Prevent clear-text logging of sensitive information (Alert #34)

[github-actions]

Security Fix: Prevent Clear-text Logging of Sensitive Information

Alert Number: #34
Severity: High
Rule: go/clear-text-logging

Vulnerability Description

CodeQL detected that sensitive data from secret configurations could flow through error messages to logging output. When workflow compilation fails in the mcp add command, the error message (which may contain secret references or values) was being logged in clear text to both debug logs and console output.

The vulnerability was identified at pkg/cli/mcp_add.go:148 where workflow compilation errors were logged with full error details using:

• Line 147: mcpAddLog.Printf("Workflow compilation failed: %v", err)
• Line 148: fmt.Println(console.FormatWarningMessage(fmt.Sprintf("Workflow compilation failed: %v", err)))

This creates a data flow path where sensitive information from secretKeys and related secret processing could be exposed in logs.

Fix Applied

Modified the error handling in pkg/cli/mcp_add.go (lines 147-150) to:

• Remove detailed error information from both console and debug log output
• Use generic error messages that don't expose sensitive data
• Maintain user experience by providing clear guidance on next steps

Before:

mcpAddLog.Printf("Workflow compilation failed: %v", err)
fmt.Println(console.FormatWarningMessage(fmt.Sprintf("Workflow compilation failed: %v", err)))

After:

// 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."))

Security Best Practices

This fix follows OWASP recommendations for handling sensitive information:

1. Avoid logging sensitive data: Error messages no longer include details that could contain secret references
2. Minimal information disclosure: Generic error messages provide necessary feedback without exposing internals
3. Defense in depth: Even if secrets are properly referenced as ${{ secrets.NAME }}, we avoid any potential leakage through error messages

Testing Considerations

• ✅ Existing functionality preserved - users still know when compilation fails
• ✅ Generic error message provides sufficient actionable information
• ✅ Users can still use gh aw compile directly for detailed debugging if needed
• ✅ No breaking changes to the API or user workflow
• ✅ Debug logging still indicates failure without exposing sensitive details

References

• OWASP: Password Plaintext Storage
• CWE-312: Cleartext Storage of Sensitive Information
• CWE-315: Cleartext Storage of Sensitive Information in a Cookie
• CWE-359: Exposure of Private Personal Information to an Unauthorized Actor

---

🤖 Generated with Claude Code

Co-Authored-By: Claude (noreply@anthropic.com)

AI generated by Security Fix PR

[Copilot]

Pull request overview

This PR attempts to fix a CodeQL-reported security vulnerability (Alert #34) by removing detailed error information from workflow compilation failures in the mcp add command. The fix replaces specific error messages with generic ones to prevent potential exposure of sensitive information through logs.

Key Changes:

• Modified error logging in mcp_add.go to use generic messages instead of detailed error output
• Added security-focused comments explaining the CWE references

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

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)))