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

Feature: G101 match variable values and names #971

Merged
merged 11 commits into from
Jun 15, 2023

Conversation

morgenm
Copy link
Contributor

@morgenm morgenm commented Jun 13, 2023

I updated the HardcodedCredentials code to apply regex against the actual values themselves in assignment statements, valuespecs, and equalities. The intention is to search for secrets, such as API keys, by looking for the values. For example if we have:

key = "3490b39bc66f7e7730e8fa68041da2e4f7c0a6a8cb119466c3ddc5245bda1781"

this would previously not be caught by gosec, because gosec only looks at the variable names.

Now, with these changes, the code matches values too using a new regex pattern that will look for secrets such as this. I added a new config variable called "patternValue," with a default regex which searches for VirusTotal API keys (or other keys with the same format).

Copy link
Member

@ccojocar ccojocar left a comment

Choose a reason for hiding this comment

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

Thanks for this contribution. It looks good. Is this already covered by existing rule tests? If not, please could you add some test cases? Thanks again

@ccojocar
Copy link
Member

There are some lint issues. Please could you fix them?

Copy link
Contributor

@audunmo audunmo left a comment

Choose a reason for hiding this comment

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

I have some quick thoughts on the code here. As an aside, you could also install Talisman to solve this issue for yourself today.

Also, would probably be good to have a test case in this PR

rules/hardcoded_credentials.go Outdated Show resolved Hide resolved
rules/hardcoded_credentials.go Outdated Show resolved Hide resolved
rules/hardcoded_credentials.go Outdated Show resolved Hide resolved
rules/hardcoded_credentials.go Outdated Show resolved Hide resolved
rules/hardcoded_credentials.go Outdated Show resolved Hide resolved
rules/hardcoded_credentials.go Outdated Show resolved Hide resolved
rules/hardcoded_credentials.go Outdated Show resolved Hide resolved
rules/hardcoded_credentials.go Outdated Show resolved Hide resolved
@audunmo
Copy link
Contributor

audunmo commented Jun 14, 2023

Don't mean to butt in here, just really excited about gosec 😄

testutils/source.go Outdated Show resolved Hide resolved
@morgenm
Copy link
Contributor Author

morgenm commented Jun 14, 2023

Don't mean to butt in here, just really excited about gosec smile

Thanks for the suggestions on my code changes. Please me know if there are any further suggestions or anything.

There are some lint issues. Please could you fix them?

I think I fixed them, I do not see any more issues on when running golangci-lint locally. I have no further changes unless anything else comes up.

@ccojocar ccojocar merged commit abeab10 into securego:master Jun 15, 2023
5 of 6 checks passed
@ccojocar
Copy link
Member

@morgenm Thanks for addressing all review comments. The change looks good now.

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

4 participants