[security-fix] Security Fix: Prevent logging of secret key names in validation errors (Alert #71)

[github-actions]

Security Fix: Prevent Clear-Text Logging of Secret Key Names

Alert Number: #71
Severity: High (Error)
Rule: go/clear-text-logging
CWE: CWE-312, CWE-315, CWE-359

Vulnerability Description

CodeQL detected a security vulnerability where sensitive data from the secretKeys variable flows through error messages into JSON output logs at pkg/cli/compile_orchestrator.go:586.

The data flow path:

1. Secret key names (e.g., "DATABASE_PASSWORD", "API_TOKEN", "DEPLOY_KEY") from the secretKeys variable in pkg/workflow/jobs.go:328
2. Pass through validateSecretsExpression() in pkg/workflow/compiler_jobs.go:389
3. When validation fails, error messages include the key name: "jobs.secrets.%s must be a GitHub Actions expression..."
4. These errors are added to ValidationError.Message in pkg/cli/compile_orchestrator.go
5. validationResults is marshaled to JSON and printed to stdout at line 586
6. Secret key names are exposed in logs

While actual secret VALUES were never logged, exposing secret KEY NAMES is a security issue as it reveals information about an organization's security infrastructure, authentication mechanisms, and system architecture.

Previous Fix Attempts

PR #7224 removed key names from log statements but NOT from error messages. This PR addresses the root cause by removing key names from the error messages themselves, preventing them from flowing to JSON output.

Fix Applied

Modified pkg/workflow/secrets_validation.go:

• Removed the %s format specifier and key parameter from the error message returned by validateSecretsExpression()
• Error message changed from: "jobs.secrets.%s must be a GitHub Actions expression..." to: "invalid secrets expression: must be a GitHub Actions expression..."
• Added explanatory comment documenting why key names are intentionally excluded

Before:

return fmt.Errorf("jobs.secrets.%s must be a GitHub Actions expression with secrets reference (e.g., '${{ secrets.MY_SECRET }}' or '${{ secrets.SECRET1 || secrets.SECRET2 }}')", key)

After:

// Note: We intentionally do NOT include the key name in the error message to avoid
// logging sensitive information (secret key names) that could expose details about
// the organization's security infrastructure. The key name is available in the
// calling context for debugging purposes if needed.
return fmt.Errorf("invalid secrets expression: must be a GitHub Actions expression with secrets reference (e.g., '${{ secrets.MY_SECRET }}' or '${{ secrets.SECRET1 || secrets.SECRET2 }}')")

Updated pkg/workflow/secrets_validation_test.go:

• Updated test expectations to verify that key names are NOT in error messages
• Added explicit test cases to verify the security fix
• Changed test assertions from expecting key names to expecting they are absent
• All tests verify that error messages do NOT contain sensitive key names

Security Best Practices Applied

1. Minimal Information Disclosure: Error messages provide guidance without exposing sensitive infrastructure details
2. Defense in Depth: Key names are removed at the source (error creation) before they can flow to any logging mechanism
3. Secure by Default: Error messages are helpful for developers but never expose organizational security details
4. Testable Security: Comprehensive tests explicitly verify that sensitive data is not leaked

Testing Considerations

✅ All tests pass:

• TestValidateSecretsExpression - Pattern matching and validation logic unchanged
• TestValidateSecretsExpressionErrorMessages - Verifies key names are NOT in error messages
• TestValidateSecretsExpressionWithDifferentKeys - Verifies security fix across various key names
• TestSecretsValidationEdgeCases - Edge cases handled correctly
• TestJobsSecretsValidation - End-to-end compilation with secrets validation

✅ Error messages remain informative: Users still see helpful guidance about the correct format

✅ No breaking changes: The fix only affects error message content, not functionality or API

Run tests with:

go test -v ./pkg/workflow -run TestValidateSecretsExpression
go test -v ./pkg/workflow -run TestJobsSecretsValidation

Impact

This is a security-only fix with no functional changes:

• ✅ Validation logic is unchanged
• ✅ Error handling flow is unchanged
• ✅ API/interface is unchanged
• ✅ Error messages remain helpful for debugging
• ⚠️ Key names no longer appear in error messages (intentional security improvement)

The fix prevents exposure of organizational security infrastructure details while maintaining all existing functionality.

Why This Approach is Correct

Unlike previous attempts that tried to sanitize error messages after they were created, this fix removes the sensitive data at the source - the error message format string itself. This ensures that no matter how the error propagates through the system, the key name will never be logged.

References

• CodeQL Rule: go/clear-text-logging
• 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
• OWASP: [Password Plaintext Storage]((redacted)

---

🤖 Generated with [Claude Code]((redacted)

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

AI generated by Security Fix PR