Skip to content
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

feat: add sendgrid key detection #128

Merged
merged 9 commits into from
Apr 27, 2020
Merged

feat: add sendgrid key detection #128

merged 9 commits into from
Apr 27, 2020

Conversation

eccentricexit
Copy link
Member

This rule allows detecting more generic api keys like the ones used by SendGrid.

I cannot run the tests locally or compile the project to continue development due to a typescript error which seems to be present for all packages:

TSError: ⨯ Unable to compile TypeScript:
test/index.test.ts:1:26 - error TS2307: Cannot find module '@secretlint/tester'.

1 import { snapshot } from "@secretlint/tester";
                           ~~~~~~~~~~~~~~~~~~~~
test/index.test.ts:18:17 - error TS7006: Parameter 'name' implicitly has an 'any' type.

18     }).forEach((name, test) => {
                   ~~~~
test/index.test.ts:18:23 - error TS7006: Parameter 'test' implicitly has an 'any' type.

18     }).forEach((name, test) => {

...

Do you have any tips on how I can fix this?

@azu
Copy link
Member

azu commented Apr 12, 2020

I cannot run the tests locally or compile the project to continue development due to a typescript error which seems to be present for all packages:

Fmm.
It is related with #119 changes(not yet released)

Can you run yarn install && yarn run build on root directory?
#119 changes all packages and need to rebuild all packages.
It will build new @secretlint/types pacakge in your local.

It is implicitly dependencies.
Project References will resolve implicitly dependencies and I've created an issue #129

generic key detection

Would we change this rule to more specific rule like @secretlint/secret-rule-sendgrid?
Or, We need to add more strict condition for avoiding false-positive.

We want to reduce false-positive. It is one of Secretlint's Philosophy.

I think that current implementation has a false-positive.

For example enctrypted key like SealedSecrets

api-key="<encrypted-apikey>"

I found similar problem in git-secrets https://github.com/awslabs/git-secrets/blob/5e28df337746db4f070c84f7069d365bfd0d72a8/git-secrets#L239 and I've changed this logic in secretlint-rule-aws

// git-secrets implementation match _KEY=XXX, but it is false-positive
// https://github.com/awslabs/git-secrets/blob/5e28df337746db4f070c84f7069d365bfd0d72a8/git-secrets#L239
// This Pattern match only `AWS?_SECRET_ACCESS_KEY=XXX`
const AWSSecretPatten = regx`${QUOTE}${AWS}(?:SECRET|secret|Secret)_?(?:ACCESS|access|Access)_?(?:KEY|key|Key)${QUOTE}${CONNECT}${QUOTE}([A-Za-z0-9/\+=]{40})${QUOTE}\b`;
const results = matchAll(source.content, AWSSecretPatten);

While, Detecting SG.<ID>.<VALUE> as Send Grid API Key is reasonable.

It is similar with slack api key.

// Based on https://docs.cribl.io/docs/regexesyml
// https://api.slack.com/docs/token-types
// Bot user token strings begin with xoxb-
// User token strings begin with xoxp-
// Workspace access token strings begin with xoxa-2
// Workspace refresh token strings begin with xoxr.
const PRIVATE_KEY_PATTERN = /\bxox[abposr]-(?:\d{1,40}-)+[a-zA-Z0-9]{1,40}\b/g;

@eccentricexit
Copy link
Member Author

eccentricexit commented Apr 22, 2020

Can you run yarn install && yarn run build on root directory?

I did that and it seems to be all good now.

Would we change this rule to more specific rule like @secretlint/secret-rule-sendgrid?
Or, We need to add more strict condition for avoiding false-positive.
We want to reduce false-positive. It is one of Secretlint's Philosophy.
I think that current implementation has a false-positive.

Oh I see. You are right, I'll update this to do @secretlint/secret-rule-sendgrid instead 👍

@eccentricexit eccentricexit changed the title feat: add generic key detection feat: add sendgrid key detection Apr 25, 2020
eccentricexit and others added 3 commits April 26, 2020 14:32
Co-Authored-By: azu <azu@users.noreply.github.com>
Co-Authored-By: azu <azu@users.noreply.github.com>
@azu azu merged commit 71d32cb into secretlint:master Apr 27, 2020
@azu
Copy link
Member

azu commented Apr 27, 2020

I'll publish this pacakge in next update(today ~ tommorow).
Also, I'll add secretlint-rule-sendgrid to recommended preset #131

Thanks for improving!
I've invited @mtsalenc as collaboration into secretlint organization 🎉

@azu azu mentioned this pull request Apr 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants