Skip to content

[security-fix] Fix hardcoded credentials false positive in safe_outputs_steps.go (Alert #433)#8888

Merged
pelikhan merged 1 commit intomainfrom
security-fix-alert-433-hardcoded-credentials-false-positive-c0769f63221431db
Jan 4, 2026
Merged

[security-fix] Fix hardcoded credentials false positive in safe_outputs_steps.go (Alert #433)#8888
pelikhan merged 1 commit intomainfrom
security-fix-alert-433-hardcoded-credentials-false-positive-c0769f63221431db

Conversation

@github-actions
Copy link
Contributor

@github-actions github-actions bot commented Jan 4, 2026

Security Fix: Hardcoded Credentials False Positive in Safe Output Steps

Alert Number: #433
Severity: High (but LOW confidence - false positive)
Rule: G101 - Potential hardcoded credentials
Tool: gosec (Golang security checks)
Location: pkg/workflow/safe_outputs_steps.go:87

Vulnerability Description

Gosec detected a potential hardcoded credential at line 87 (now line 88 after changes) where the code sets:

token = "${{ secrets.COPILOT_TOKEN || secrets.GITHUB_TOKEN }}"

However, this is a FALSE POSITIVE. The G101 rule is flagging a GitHub Actions expression template as a hardcoded credential, when in reality:

  1. NO actual credentials are hardcoded - The string ${{ secrets.COPILOT_TOKEN || secrets.GITHUB_TOKEN }} is a placeholder
  2. GitHub Actions runtime replaces it - At workflow execution time, GitHub Actions substitutes this template with the actual secret value
  3. Standard GitHub Actions pattern - This is the idiomatic way to reference secrets in GitHub Actions workflows
  4. Low confidence detection - Gosec itself marked this as "Confidence: LOW" indicating uncertainty

Fix Applied

Enhanced the #nosec G101 comment and added an inline #nosec directive to properly suppress the false positive alert:

Before:

if useCopilotToken {
    // #nosec G101 -- This is not a hardcoded credential, it's a GitHub Actions expression template
    // that will be rendered in the workflow YAML to reference secrets at runtime
    token = "${{ secrets.COPILOT_TOKEN || secrets.GITHUB_TOKEN }}"

After:

if useCopilotToken {
    // #nosec G101 -- This is NOT a hardcoded credential. It's a GitHub Actions expression template
    // "${{ secrets.COPILOT_TOKEN || secrets.GITHUB_TOKEN }}" that GitHub Actions runtime substitutes
    // with the actual secret value at workflow execution time. The string is a placeholder, not a credential.
    token = "${{ secrets.COPILOT_TOKEN || secrets.GITHUB_TOKEN }}" // #nosec G101

This approach:

  • Enhances the explanatory comment with more detail about the false positive
  • Adds an inline #nosec G101 directive on the same line as the assignment
  • Maintains full functionality - the GitHub Actions expression continues to work correctly
  • Follows best practices for handling false positives in static analysis

Why This Is a False Positive

  1. Template, Not Value: The string is a GitHub Actions expression template, not an actual credential
  2. Runtime Substitution: GitHub Actions engine substitutes the placeholder at runtime
  3. No Information Disclosure: The source code contains no secret values
  4. Industry Standard: All GitHub Actions workflows use this pattern for secret references
  5. Low Confidence: Gosec marked this as LOW confidence, acknowledging uncertainty
  6. Similar to Alert [Custom Engine Test] Test Pull Request - Custom Engine Safe Output #440: This is the same false positive pattern that was fixed in PR [security-fix] Fix hardcoded credentials false positive in copilot execution (Alert #440) #8840

Security Best Practices

No hardcoded credentials: Source code contains only template placeholders
Proper secret management: Actual secrets stored securely in GitHub Secrets
Runtime substitution: Secrets injected at execution time, never in source
Clear documentation: Enhanced comment explains why this is safe
Minimal change: Only adds clarifying comment and inline directive, no functional changes

Testing

Build succeeded: go build ./pkg/workflow/... passes without errors
No breaking changes: GitHub Actions expression continues to work correctly
No functional changes: Only adds explanatory comment and inline directive
Maintains security: No actual credentials exposed

Impact Assessment

Risk: None
Breaking Changes: None
Backwards Compatibility: Full
Performance: No impact

The fix only enhances a clarifying comment and adds an inline #nosec directive to suppress a false positive alert. All functionality remains unchanged, and no actual security risk exists.

Why This Approach Is Correct

  1. False Positive Handling: Using #nosec with detailed justification is the recommended approach for false positives
  2. Inline Directive: Placing #nosec on the same line as the flagged code is the most reliable way to suppress gosec warnings
  3. Maintains Code Quality: Prevents alert fatigue from false positives
  4. Educational Value: Enhanced comment explains GitHub Actions secret patterns for future maintainers
  5. Follows Go Best Practices: Documented use of linter suppressions with clear justification

Files Modified

  • pkg/workflow/safe_outputs_steps.go:
    • Lines 85-88: Enhanced #nosec G101 comment with detailed false positive explanation
    • Line 88: Added inline #nosec G101 directive on the assignment line

References


🤖 Generated by Security Fix Agent in workflow run 20694378469

AI generated by Security Fix PR

…ert #433)

Enhanced #nosec comment for GitHub Actions expression template at line 88
to properly suppress gosec G101 false positive warning.

🤖 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 January 4, 2026 14:46
@pelikhan pelikhan merged commit d4f83cf into main Jan 4, 2026
4 checks passed
@pelikhan pelikhan deleted the security-fix-alert-433-hardcoded-credentials-false-positive-c0769f63221431db branch January 4, 2026 14:47
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