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

Only load json from stdin if it exists #69

Merged
merged 1 commit into from
Aug 9, 2018

Conversation

guykisel
Copy link
Contributor

@guykisel guykisel commented Aug 9, 2018

I was trying to warp detect-secrets in an automated code review tool I'm working on that shells out to various static analysis tools. However, when it runs detect-secrets, detect-secrets appears to expect JSON on stdin when run in non-interactive mode. This PR adds a small workaround to only load the JSON if stdin input actually exists.

An alternative approach might be a try/except block around the JSON load.

@KevinHock KevinHock self-requested a review August 9, 2018 21:04
Copy link
Collaborator

@KevinHock KevinHock left a comment

Choose a reason for hiding this comment

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

LGTM 🚢 ✨🍰✨
Thanks for making this!

@KevinHock
Copy link
Collaborator

Feel free to checkout pyt if you like static analysis 😉

@KevinHock KevinHock merged commit 49dc178 into Yelp:master Aug 9, 2018
@guykisel guykisel deleted the check-stdin branch August 10, 2018 04:04
@guykisel
Copy link
Contributor Author

Thanks! I will check it out. Currently for Python security linting I run Bandit :)

@KevinHock
Copy link
Collaborator

https://github.com/facebook/pyre-check Looks eventually promising as well, definitely great for type checking currently.

Bandit is very production ready but at the cost of lots of false positives, since it doesn't do dataflow like the other 2 do.

@guykisel
Copy link
Contributor Author

Nice! I like to keep an eye on https://github.com/mre/awesome-static-analysis and check recent commits occasionally to see if any new tools have showed up.

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