fix(aws vault): Fix default sanitizer hashcode append logic #148
Conversation
ghost
changed the title
ghost
changed the title
ghost
changed the title
ghost
changed the title
Codecov ReportAll modified and coverable lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #148 +/- ##
==========================================
+ Coverage 64.54% 64.71% +0.16%
==========================================
Files 26 26
Lines 629 632 +3
Branches 28 28
==========================================
+ Hits 406 409 +3
Misses 218 218
Partials 5 5 ☔ View full report in Codecov by Sentry. |
| if (originalKey.length() > 500) { | ||
| boolean originalKeyReplaced = false; | ||
|
|
||
| if (originalKey.length() > 512) { | ||
| //Substring to 500, to add the 12 char hashcode | ||
| key = originalKey.substring(0, 500); |
There was a problem hiding this comment.
I didn't get this 500/512 thing, looking at the tests seems like it's a sort of size limit for the key size, eventually it could be extracted as a constant called KEY_SIZE_LIMIT or something like that.
The comment is outdated (please adapt the code to being able to talk for itself)
| for (int i = 0; i < key.length(); i++) { | ||
| var c = key.charAt(i); | ||
| if (!Character.isLetterOrDigit(c) && c != '/' && c != '_' && c != '+' && c != '.' && c != '@' && c != '-') { | ||
| replacedIllegalCharacters = true; | ||
| originalKeyReplaced = true; | ||
| sb.append('-'); |
There was a problem hiding this comment.
the documentation says _ but the code does -. Is this an error?
There was a problem hiding this comment.
This logic is for replacing invalidad characters in the original key to "-". (ex: example#key, replaced to example-key)
The hashcode is appended afterwards.
There was a problem hiding this comment.
Ah, i see the question now. Yes, the documentation was off.
ghost
requested a review
from 
What this PR changes/adds
This changes the logic on how an hashcode is appended to a key after it is sanitized, during secrets manager vault store/retrieve/delete methods.
Why it does that
The current implementation of the default sanitizer strategy appends an hashcode to the secret key every time the store/retrieve/delete methods are used. While this has no impact for new secrets created by the vault, the vault fails to find existing secrets that were created previously either by AWS Console, CLI, another SDK, etc...
In such cases where the secret already exists, the sanization should have no effect in the given key.
The key should have an hashcode appended only when:
Further notes
The tests where also adapted to be aligned with the changed logic.
Linked Issue(s)
Closes #133