New Artifactory / Slack Patterns #195
Conversation
detect_secrets/plugins/slack.py
Outdated
| @@ -10,8 +10,16 @@ | |||
|
|
|||
| class SlackDetector(RegexBasedDetector): | |||
|
|
|||
| secret_type = 'Slack Token' | |||
| secret_type = 'Slack Token / Webhook' | |||
There was a problem hiding this comment.
This is going to be a non-backwards compatible change.
Are you sure you want to do this?
There was a problem hiding this comment.
(Backwards compatibility aside)
I think we may want to make a different plugin, I say this because we should only have 1 verify method once #194 is merged, and verifying token's vs. webhooks is different.
There was a problem hiding this comment.
I'm fine with having the same plugin.
In the verify method, we can do:
def verify(self, token):
if token.startswith('https://hooks.slack.com'):
# do logic
There was a problem hiding this comment.
As far as backwards compatibility, I don't mind but there may be a user that does. Slack isn't one of the noisier plugins so I'd say this is okay :) What do you think @domanchi?
(To explain better, the reason this breaks backwards compatibility is because all of a user's old acknowledged Slack tokens will be re-alerted on, because the "hashed_secret" value will change.)
There was a problem hiding this comment.
I'm in favor of the slightly less descriptive, but more compatible decision to keep secret_type. IMO, adding the clarity that it also catches webhooks is less important.
There was a problem hiding this comment.
I would agree with @domanchi on more compatible approach.
There was a problem hiding this comment.
No objection here. I'll leave the type as it was.
| re.compile(r'(\s|=|:|"|^)AP6\w{10,}'), # password | ||
| re.compile(r'(\s|=|:|"|^)AKC[a-zA-Z0-9]{10,}'), # api token | ||
| # artifactory encrypted passwords begin with AP[A-Z] | ||
| re.compile(r'(\s|=|:|"|^)AP[\dABCDEF][a-zA-Z0-9]{8,}'), # password |
There was a problem hiding this comment.
cc @justineyster just as a heads up
The modifications in this PR are twofold: 1. Add ability to detect Slack Webhooks 2. Improved the artifactory password regex to catch passwords of different lengths and rotated passwords (Third char increments after user rotates password). Restore slack token secret type
adrianbn
force-pushed
the
add-artifactory-patterns
branch
from
90dd6d7
to
b34dda6
Compare
|
I've returned the secret type and rebased master. Let me know if there are further changes you want :) |
* Fix Artifactory validation failure case Closes git-defenders/detect-secrets-discuss#207 * update tests * Catch 401 and 403
* Fix Artifactory validation failure case Closes git-defenders/detect-secrets-discuss#207 * update tests * Catch 401 and 403
The modifications in this PR are twofold: