Skip to content

[security-fix] Security Fix: Remove sensitive key names from secrets validation log messages (Alert #71)#7224

Merged
pelikhan merged 1 commit intomainfrom
main-ecc3f1100ebf4061
Dec 22, 2025
Merged

[security-fix] Security Fix: Remove sensitive key names from secrets validation log messages (Alert #71)#7224
pelikhan merged 1 commit intomainfrom
main-ecc3f1100ebf4061

Conversation

@github-actions
Copy link
Contributor

Security Fix: Clear-Text Logging of Sensitive Information

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

Vulnerability Description

CodeQL detected a security vulnerability where sensitive data (secret key names) from the secretKeys variable flows through to logging calls in the secrets validation function. While actual secret VALUES were never logged, secret key NAMES (such as "api_token", "deploy_key", "database_password") were being included in log messages, which can expose information about an organization's security infrastructure.

Data Flow Path:

  1. secretKeys variable in pkg/workflow/jobs.go:328 contains secret key names
  2. These keys are validated in pkg/workflow/compiler_jobs.go:389 via validateSecretsExpression(key, value)
  3. The key parameter was logged at pkg/workflow/secrets_validation.go:21 and :24
  4. Validation errors flow to pkg/cli/compile_orchestrator.go:586 where they're printed as JSON

Fix Applied

Removed secret key names from log messages in the validateSecretsExpression() function:

Before:

secretsValidationLog.Printf("Invalid secret expression for key %s", key)
secretsValidationLog.Printf("Valid secret expression for key %s", key)

After:

secretsValidationLog.Printf("Invalid secret expression detected")
secretsValidationLog.Printf("Valid secret expression validated")

Security Best Practices Applied

  1. Minimal Information Disclosure: Log messages now contain only the minimum necessary information
  2. User Feedback Preserved: Error messages returned to users still include the key name (via fmt.Errorf) for debugging, but logs do not
  3. Defense in Depth: Even though key names aren't secret values, avoiding their exposure in logs prevents potential information leakage

Testing Considerations

All existing tests pass, including:

  • TestValidateSecretsExpression - Core validation logic
  • TestValidateSecretsExpressionErrorMessages - Verifies sensitive data NOT in errors
  • TestJobsSecretsValidation - End-to-end workflow compilation with secrets
  • All other secrets-related tests

Error messages still functional: Users still see helpful error messages with key names via the returned error, just not in logs

No breaking changes: This is a surgical fix that only affects internal logging, not API or user-facing behavior

Impact

This fix eliminates the clear-text logging vulnerability while maintaining all existing functionality. The change is minimal and focused, affecting only two log statements in the secrets validation function.

AI generated by Security Fix PR

Fixed clear-text logging vulnerability (CodeQL alert #71) by removing
secret key names from log messages in secrets validation.

**Alert Details:**
- Alert Number: #71
- Severity: High
- Rule: go/clear-text-logging
- Location: pkg/workflow/secrets_validation.go

**Changes Made:**
- Removed key parameter from log messages in validateSecretsExpression()
- Changed "Invalid secret expression for key %s" to "Invalid secret expression detected"
- Changed "Valid secret expression for key %s" to "Valid secret expression validated"

**Security Rationale:**
While the actual secret VALUES were never logged, CodeQL detected that
secret key NAMES (e.g., "api_token", "deploy_key") from the secretKeys
variable flow through to logging calls. Even key names can be sensitive
as they reveal what secrets an organization uses.

**Testing:**
- All existing tests pass
- Error messages still include the key name for user feedback (via fmt.Errorf)
- Log messages now contain no sensitive information

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@pelikhan pelikhan marked this pull request as ready for review December 22, 2025 12:04
@pelikhan pelikhan merged commit 2140031 into main Dec 22, 2025
4 checks passed
@pelikhan pelikhan deleted the main-ecc3f1100ebf4061 branch December 22, 2025 12:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant