-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Add Gitlab V3 Detector #4563
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
base: main
Are you sure you want to change the base?
Add Gitlab V3 Detector #4563
Conversation
|
Great work, @mustansir14. Let's add the v3 to engine defaults as well. |
pkg/detectors/gitlab/v3/gitlab_v3.go
Outdated
|
|
||
| var ( | ||
| defaultClient = common.SaneHttpClient() | ||
| keyPat = regexp.MustCompile(`\b(glpat-[a-zA-Z0-9\-=_]{27,300}.[0-9a-z]{2}.[a-z0-9]{9})\b`) |
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.
optional: It's probably worth adding a comment and a link to the gitlab commit that sets the regex, just so anyone curious about it can more easily pull up that info.
Though, it's also included as a part of the PR, so the information is available, which is great! A comment would just make it easier.
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.
Good point. Added.
shahzadhaider1
left a comment
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.
GW
Description:
This PR resolves #4551 by introducing a V3 detector for Gitlab. Newly generated Gitlab PATs have a different format than what we are looking for in v1 and v2.
Thanks to @trufflesteeeve, we found Gitlab's merged PR that introduced this change. The regex is now made flexible to match what they have.
Checklist:
make test-community)?make lintthis requires golangci-lint)?