Skip to content

Potential fix for code scanning alert no. 115: Clear-text logging of sensitive information#3832

Open
fiftin wants to merge 1 commit into
developfrom
alert-autofix-115
Open

Potential fix for code scanning alert no. 115: Clear-text logging of sensitive information#3832
fiftin wants to merge 1 commit into
developfrom
alert-autofix-115

Conversation

@fiftin
Copy link
Copy Markdown
Collaborator

@fiftin fiftin commented May 5, 2026

Potential fix for https://github.com/semaphoreui/semaphore/security/code-scanning/115

General fix: ensure all log messages are sanitized before emission so secrets (especially credentials embedded in URLs like https://user:pass@host/...) are redacted.

Best single fix without changing functionality: update services/tasks/LocalJob.go so Log(msg string) passes a sanitized message to t.Logger.Log. Add a helper that redacts userinfo in http:// / https:// URLs from user[:password]@ to ***@. This preserves operational logging context while removing sensitive data. No behavior change outside log content redaction.

Changes needed:

  • File: services/tasks/LocalJob.go
    • Add import: regexp
    • Add helper method on LocalJob (or private function) to sanitize log message.
    • Update Log(msg string) to call sanitizer before logging.
  • No changes are required in db/Repository.go for this alert, since the sink-side sanitization addresses both variants centrally.

Suggested fixes powered by Copilot Autofix. Review carefully before merging.

…sensitive information

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>

func (t *LocalJob) Log(msg string) {
t.Logger.Log(msg)
t.Logger.Log(t.sanitizeLogMessage(msg))
@fiftin fiftin marked this pull request as ready for review May 5, 2026 10:50
Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Security review (automation)

Scope: services/tasks/LocalJob.gosanitizeLogMessage + wiring in Log.

Assessment: No medium / high / critical vulnerabilities introduced by this change.

  • What changed: Task log lines passed through LocalJob.Log now replace http(s)://…userinfo@ with http(s)://***@, which addresses clear-text logging of embedded HTTP(S) URL credentials for the common lowercase-scheme form.
  • Residual risk (low): The pattern is case-sensitive (https? only), so mixed-case schemes (e.g. HTTPS://user:pass@…) would not be redacted; many tools normalize to lowercase, so practical exposure is limited. Non-HTTP URL schemes (e.g. git@, ftp://) are unchanged — pre-existing breadth of logging, not expanded by this PR.
  • Out of scope for this diff: Call sites that invoke t.Logger.Log directly (e.g. git client helpers) bypass LocalJob.Log; that behavior predates this PR.

Slack-style summary

  • PR: Potential fix for code scanning alert 115 (clear-text logging of sensitive info in LocalJob.Log).
  • Outcome: No medium+ security findings on the added/modified code; change is a targeted redaction improvement with minor edge-case coverage limits noted above.
Open in Web View Automation 

Sent by Cursor Automation: Find vulnerabilities

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants