-
Notifications
You must be signed in to change notification settings - Fork 104
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
Add base64 masking #269
Add base64 masking #269
Conversation
src/main/java/org/jenkinsci/plugins/credentialsbinding/masking/Base64SecretPatternFactory.java
Outdated
Show resolved
Hide resolved
...st/java/org/jenkinsci/plugins/credentialsbinding/masking/Base64SecretPatternFactoryTest.java
Outdated
Show resolved
Hide resolved
src/main/java/org/jenkinsci/plugins/credentialsbinding/masking/Base64SecretPatternFactory.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems to work well in local testing
Base64.getUrlEncoder(), | ||
}; | ||
|
||
Collection<String> result = new ArrayList<>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps use Set over List?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should not matter I think; all the patterns will wind up merged into one big pattern sorted by length.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lines 61 to 65 in b19cfc6
.flatMap(input -> | |
secretPatternFactories.stream().flatMap(factory -> | |
factory.getEncodedForms(input).stream())) | |
.sorted(BY_LENGTH_DESCENDING) | |
.distinct() |
Ready to go I think; @daniel-beck or @Wadeck or anyone else want to take a look? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not manually tested this version, but looks very similar to what I expected 👍
There is no support for base64-encoded credentials, and it's not an encryption method and it can be easily decoded.
To prevent base64-encoded credentials from being exposed in build logs, I added a new SecretPatternFactory for it.
Testing done
I wrote a test to ensure it was done properly on Windows and Unix-based systems but I also tested it locally with the following pipeline:
Before:
After:
Submitter checklist