Skip to content
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

Obfuscate Credentials in Logs #312

Merged
merged 5 commits into from
Feb 15, 2024
Merged

Conversation

bitgully
Copy link
Contributor

@bitgully bitgully commented Feb 2, 2024

Summary

When using HTTP basic authentication credentials in binding URIs, this change obfuscates the username and password in the log entries.

Use Cases

When adding a binding for an artifact hosted in a private registry, it can be necessary to provide HTTP basic authentication credentials with the URI. This already works fine for downloading the desired dependency. However, when doing so, the username and password that are part of the URI, get printed to stdout in plain text at the moment. This PR should solve this issue by obfuscating the credentials.
To make sure this change has no adverse side effects in case the provided URI is not well-formed, this implementation uses a regular expression instead of trying to parse the string to a "net/url" object.

Checklist

  • I have viewed, signed, and submitted the Contributor License Agreement.
  • I have linked issue(s) that this PR should close using keywords or the Github UI (See docs)
  • I have added an integration test, if necessary.
  • I have reviewed the styleguide for guidance on my code quality.
  • I'm happy with the commit history on this PR (I have rebased/squashed as needed).

@bitgully bitgully requested a review from a team as a code owner February 2, 2024 19:21
dependency_cache.go Outdated Show resolved Hide resolved
@dmikusa dmikusa added type:bug A general bug semver:patch A change requiring a patch version bump labels Feb 6, 2024
dependency_cache.go Outdated Show resolved Hide resolved
dependency_cache.go Outdated Show resolved Hide resolved
Copy link
Contributor

@dmikusa dmikusa left a comment

Choose a reason for hiding this comment

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

Two small changes. If those are OK, let me know and I'll merge everything. Thanks!

@bitgully
Copy link
Contributor Author

Sure, they are okay. Thanks for the review.

@dmikusa dmikusa merged commit 10a9afa into paketo-buildpacks:main Feb 15, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver:patch A change requiring a patch version bump type:bug A general bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants