-
Notifications
You must be signed in to change notification settings - Fork 242
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
fix(loki.secretfilter): Fix partial masking for short secrets and support multiple allowlists per rule #2320
Conversation
💻 Deploy preview deleted. |
Regexes []string | ||
} | ||
Rules []struct { | ||
ID string | ||
Description string | ||
Description string // Not used |
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.
Are these unused variables still here so the format processes? What is the harm in removing them?
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.
Yes, indeed they could be removed. I initially added them to keep track of features we don't currently support but might in the future.
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.
In that case I would rather remote/comment out the whole line so they cant mistakenly be used.
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.
👍 I removed it
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.
LGTM!
Co-authored-by: Clayton Cornell <[email protected]>
@clayton-cornell @mattdurham |
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.
LGTM
Will merge once clayton approves. |
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.
LGTM @mattdurham it's good to go
PR Description
partial_mask
value.[[rules.allowlists]]
format added in Gitleaks v8.21.0Which issue(s) this PR fixes
Notes to the Reviewer
PR Checklist