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

Exit code 1 if unsanitised vulnerabilities found #156

Merged
merged 3 commits into from
Jul 27, 2018

Conversation

bcaller
Copy link
Collaborator

@bcaller bcaller commented Jul 27, 2018

Not sure if you want this, but having a non-zero exit code if there are vulnerabilities would help with using pyt in an automated system like CI.

bcaller added 3 commits July 27, 2018 18:03
assert_called_with does the assertion. The function has return value
None. So don't assert assert_called_with.

Fixes the "This with: makes no sense" comment.
To be useful as a linter process such as in a Continuous Integration
system, pyt should finish with pass or fail exit codes. Saves having to
grep the output.

If there are unsanitised vulnerabilities, sys.exit(1).

In the future we'll probably want a flag to not print sanitised vulns.
Can now use just `pyt` exactly like `python -m pyt`.

Before there was an error:
AttributeError: module 'pyt' has no attribute 'main'
@KevinHock KevinHock self-requested a review July 27, 2018 19:08
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.

Looks great to me!

mock_find_vulnerabilities.return_value,
mock_parse_args.return_value.output_file
)
mock_json.report.assert_called_with(
Copy link
Collaborator

@KevinHock KevinHock Jul 27, 2018

Choose a reason for hiding this comment

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

You're my hero! :D
What was the bug? re: This with: makes no sense

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@KevinHock no, the function mock_text.report.assert_called_with() does the assertion - it either raises AssertionError or returns None.

Therefore, if you do assert mock_text.report.assert_called_with() it's like assert None.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Wow 🤦‍♂️ I must have had tunnel vision at the time. Thanks so much for fixing that 😄

@KevinHock KevinHock merged commit 992fc61 into python-security:master Jul 27, 2018
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