-
Notifications
You must be signed in to change notification settings - Fork 1.9k
[Detector] - Restore and Refactor Detectors starting with *CA* #4315
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?
Changes from all commits
674ad9a
19fd5ee
a546d8d
e8f2a43
e3f4173
0a8b805
033ae77
3e0403a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,6 +3,7 @@ package calendlyapikey | |
import ( | ||
"context" | ||
"fmt" | ||
"io" | ||
"net/http" | ||
"strings" | ||
|
||
|
@@ -22,7 +23,7 @@ var ( | |
client = common.SaneHttpClient() | ||
|
||
// Make sure that your group is surrounded in boundary characters such as below to reduce false positives. | ||
keyPat = regexp.MustCompile(detectors.PrefixRegex([]string{"calendly"}) + `\b(eyJ[A-Za-z0-9-_]{100,300}\.eyJ[A-Za-z0-9-_]{100,300}\.[A-Za-z0-9-_]+)\b`) | ||
keyPat = regexp.MustCompile(detectors.PrefixRegex([]string{"calendly"}) + `\b(eyJ[A-Za-z0-9-_]{10,300}\.eyJ[A-Za-z0-9-_]{10,300}\.[A-Za-z0-9-_]+)\b`) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In my opinion, this change could lead to more false positives. We're essentially reducing our safeguards. Are we confident about this change? Also, have we verified whether Calendly actually generates keys of this shorter length? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I made this change cos I saw some valid keys that were less than 100 letters. If we keep it as is, we're potentially increasing false negatives. I'll confirm this again. |
||
) | ||
|
||
// Keywords are used for efficiently pre-filtering chunks. | ||
|
@@ -46,18 +47,9 @@ func (s Scanner) FromData(ctx context.Context, verify bool, data []byte) (result | |
} | ||
|
||
if verify { | ||
req, err := http.NewRequestWithContext(ctx, "GET", "https://api.calendly.com/users/me", nil) | ||
if err != nil { | ||
continue | ||
} | ||
req.Header.Add("Authorization", fmt.Sprintf("Bearer %s", resMatch)) | ||
res, err := client.Do(req) | ||
if err == nil { | ||
defer res.Body.Close() | ||
if res.StatusCode >= 200 && res.StatusCode < 300 { | ||
s1.Verified = true | ||
} | ||
} | ||
isVerified, verificationErr := verifyMatch(ctx, client, resMatch) | ||
s1.Verified = isVerified | ||
s1.SetVerificationError(verificationErr, resMatch) | ||
} | ||
|
||
results = append(results, s1) | ||
|
@@ -66,6 +58,33 @@ func (s Scanner) FromData(ctx context.Context, verify bool, data []byte) (result | |
return results, nil | ||
} | ||
|
||
func verifyMatch(ctx context.Context, client *http.Client, token string) (bool, error) { | ||
req, err := http.NewRequestWithContext(ctx, http.MethodGet, "https://api.calendly.com/users/me", http.NoBody) | ||
if err != nil { | ||
return false, err | ||
} | ||
req.Header.Add("Authorization", fmt.Sprintf("Bearer %s", token)) | ||
res, err := client.Do(req) | ||
|
||
if err != nil { | ||
return false, err | ||
} | ||
|
||
defer func() { | ||
_, _ = io.Copy(io.Discard, res.Body) | ||
_ = res.Body.Close() | ||
}() | ||
|
||
switch res.StatusCode { | ||
case http.StatusOK: | ||
return true, nil | ||
case http.StatusUnauthorized: | ||
return false, nil | ||
default: | ||
return false, fmt.Errorf("unexpected HTTP response status %d", res.StatusCode) | ||
} | ||
} | ||
|
||
func (s Scanner) Type() detectorspb.DetectorType { | ||
return detectorspb.DetectorType_CalendlyApiKey | ||
} | ||
|
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.
nit: Seems like a good opportunity to replace
nil
and hard coded strings withhttp.NoBody
andhttp.MethodGet
in all of the detectors