Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
May 15, 2025

Conversation

vprusso
Copy link
Collaborator

@vprusso vprusso commented May 12, 2025

Potential fix for https://github.com/unitaryfoundation/metriq-api/security/code-scanning/1

To fix the issue, we need to ensure that sensitive information, such as the database password, is not logged. Instead of logging the full connection string, we can log a sanitized version that excludes sensitive details. This can be achieved by constructing a version of the connection string that omits the password before logging it.

The fix involves:

  1. Modifying the logging statement on line 138 in metriq-api/index.js to log a sanitized version of config.pgConnectionString.
  2. Adding a utility function to sanitize the connection string by removing the password.

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

…nsitive information

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
@vprusso vprusso marked this pull request as ready for review May 12, 2025 18:01
@vprusso vprusso requested a review from cosenal May 12, 2025 18:01
Copy link
Contributor

@cosenal cosenal left a comment

Choose a reason for hiding this comment

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

Can we just remove the log line?
(CodeQL is still failing anyway)

@vprusso
Copy link
Collaborator Author

vprusso commented May 14, 2025

Can we just remove the log line? (CodeQL is still failing anyway)

Yeah, let's just remove the line. Removed in e4dc69c.

@vprusso vprusso requested a review from cosenal May 14, 2025 23:50
@@ -1,4 +1,8 @@
process.env.METRIQ_MODE = undefined

function sanitizeConnectionString(connectionString) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Then we don't need this anymore

@cosenal cosenal merged commit 4940f32 into main May 15, 2025
4 checks passed
@cosenal cosenal deleted the alert-autofix-1 branch May 15, 2025 14:22
cosenal pushed a commit that referenced this pull request May 15, 2025
…nsitive information (#255)

* Potential fix for code scanning alert no. 1: Clear-text logging of sensitive information

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

* chore: remove console log statement.

* chore: remove sanitize function

---------

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