Improve Bitfinex Detector - Implemented API-Based Secret Validation #4368
Conversation
| detectors.DefaultMultiPartCredentialProvider | ||
| } | ||
|
|
||
| // Ensure the Scanner satisfies the interface at compile time. | ||
| var _ detectors.Detector = (*Scanner)(nil) | ||
|
|
||
| var ( | ||
| client = common.SaneHttpClient() | ||
| defaultClient = common.SaneHttpClient() | ||
|
|
||
| // related resource https://medium.com/@Bitfinex/api-development-update-april-65fe52f84124 | ||
| apiKeyPat = regexp.MustCompile(detectors.PrefixRegex([]string{"bitfinex"}) + `\b([A-Za-z0-9_-]{43})\b`) | ||
| apiSecretPat = regexp.MustCompile(detectors.PrefixRegex([]string{"bitfinex"}) + `\b([A-Za-z0-9_-]{43})\b`) |
There was a problem hiding this comment.
Both patterns are identical. Can we optimize this further by using a single regex pattern and scanning the data only once? We're already testing all the combinations found anyway. Since this is an improvement PR, I thought we could take a look at this as well.
There was a problem hiding this comment.
Thanks for pointing that out. You're absolutely right, both regex patterns are currently identical, and optimizing this does make sense. This is something that I've also observed in many detectors. For example:
Since this PR is focused specifically on improving secret validation and removing the Bitfinex Go SDK dependency as mentioned in the ticket, I’ve kept the existing detection logic unchanged to keep the scope limited and focused. But this is definitely a good follow-up improvement, and I’d be happy to open a separate PR to clean this up afterward.
| @@ -30,14 +30,12 @@ require ( | |||
| github.com/aws/smithy-go v1.22.5 | |||
| github.com/aymanbagabas/go-osc52 v1.2.1 | |||
| github.com/bill-rich/go-syslog v0.0.0-20220413021637-49edb52a574c | |||
| github.com/bitfinexcom/bitfinex-api-go v0.0.0-20210608095005-9e0b26f200fb | |||
Description:
This PR improves the Bitfinex detector by replacing the Go SDK usage with a custom implementation using the standard
net/httppackage to validate detected credentials against the Bitfinex v2 API/auth/r/wallets.Key Changes
Testing
Checklist:
make test-community)?make lintthis requires golangci-lint)?