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

Secret detection: handle surrounding quotes, expand AST parser usage #87

Merged
merged 2 commits into from
Sep 28, 2022

Conversation

jodyheavener
Copy link
Member

We've identified a couple ways secret detection can be improved. This PR adds them:

Allow language-specific parsers to be applied in more scenarios.

Currently we only check the language ID of a file to determine if an AST parser should be applied. Specifically in .env file cases, support for the specific language ID we were checking (dotenv) requires the installation of another extension, but a user might not have it installed. So now we're also going allow the file extension to be used as a qualifier for applying an AST parser. For example, a file ending in .env will now use the DotEnv parser.

Account for secrets wrapped in quotes.

When looking for secrets we check before and after the secret to make sure that it's not part of a larger string in order to avoid false positives. However, this meant that it excluded secrets if they were surrounded by quotation marks.

That means this wouldn't be detected:

const secret = "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";

But this would:

const secret = " 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 ";

We're now going to allow for secret detection if the matched secret is surrounded by single or double quotes.

Copy link
Member

@mrjones2014 mrjones2014 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All pretty straightforward. Looks good to me.

@jodyheavener jodyheavener merged commit afed799 into main Sep 28, 2022
@jodyheavener jodyheavener deleted the jh/detection-updates branch October 26, 2022 18:37
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.

2 participants