fix: redact full password when it contains @ sign#785
Open
Yanhu007 wants to merge 1 commit into
Open
Conversation
The redactLogString regex uses [^@]+ to match the password, which stops at the first @ character. If the password contains an @ (e.g. 'pass@word'), only the part before the first @ is redacted, leaking the rest of the password in error messages. Use .+ (greedy) instead to match everything up to the LAST @, ensuring the entire password is redacted regardless of content. Add test case for password containing @ sign. Fixes amacneil#784
Collaborator
|
Will this overmatch if there are |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #784
Problem
redactLogStringuses[^@]+to match the password portion, stopping at the first@. When a password contains@(e.g.pass@word), only the part before the first@is redacted, leaking the rest in error messages:Fix
Change
[^@]+to.+(greedy) so the regex matches everything up to the last@, redacting the entire password:Added test case for passwords containing
@.