Repository navigation
Mask camelCase secret names and URL userinfo credentials - #11
Open
Abhishek-B-R wants to merge 1 commit into
Open
Abhishek-B-R wants to merge 1 commit into
Abhishek-B-R wants to merge 1 commit into
Conversation
Author
|
@okwasniewski friendly bump: this masks camelCase secret names ( |
Author
|
@okwasniewski following up once more: the branch is up to date with main and cubic reported no issues, so the only thing outstanding is approving the workflow run so CI can execute. If you'd prefer a different approach to the redaction rules, I'm glad to rework it. |
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
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.
Problem
Two gaps in redaction, both of which end up in stored request records and surfaced errors unless the value also happens to be a resolved secret.
The
SECRETISH_*patterns insrc/redaction.tsandsrc/http-executor.tsonly match a keyword liketokenorsecretat the start of the name or after-or_, so camelCase names slip through.Observed: the URL comes back unchanged, all four values visible. Expected: all four values masked.
Credentials in URL userinfo are never removed.
Observed: both return
admin:hunter2@intact. Expected: the credentials are blanked.Fix
isSecretishName(name, pattern)inredaction.tstests the name as is and again with camelCase boundaries split (accessTokenis tested asaccess_Token). All lowercase names are unaffected, somonkey,monkeyIdandauthorIdstill pass through.redactUrl, for the sensitive header and query checks inexecuteCall, for response header redaction, and for the fuzz body secret collection.redactUrlandredactMessagenow blankurl.usernameandurl.password.isProbeCredentialHeader,isProbeCredentialQuery) is unchanged, so whatnoAuthandinvalidAuthmutate is the same as before. Body key redaction forcallis also unchanged.Tests
Added cases to
src/redaction.test.ts: camelCase query names are masked, benign camelCase names (monkeyId,authorId) are kept, and userinfo is stripped by bothredactUrlandredactMessage. The three new masking tests fail onmainand pass with the fix.pnpm test: 259 passed, 2 skippedpnpm typecheck: cleanpnpm lint: cleanWorth knowing
sessionId, for example).session_idwas already masked, so this is consistent.noAuthprobe may still send anaccessTokenquery param. Left out to keep this to redaction; happy to follow up separately.