Fix --audit Crash on Missing File #56
Conversation
detect_secrets/core/audit.py
Outdated
| @@ -173,6 +175,9 @@ def _secret_generator(baseline): | |||
| ) | |||
|
|
|||
| for filename, secrets in baseline['results'].items(): | |||
| if not os.path.exists(filename): | |||
| print('{} does not exist.'.format(filename)) | |||
There was a problem hiding this comment.
I'm debating the use of this case.
printstatements shouldn't be in the_secret_generator.- If the file is gone, there's nothing to audit (and we should probably just delete its entry in the updated baseline). After all, we already exclude it from the count (from your change above).
There was a problem hiding this comment.
I was ambivalent too, wasn't sure if we wanted audit to only have is_secret diffs, but yeah, removing all dead files in audit_baseline before calling _secret_generator etc. is cleaner, I'll change.
| @@ -160,7 +160,6 @@ def merge_baseline(old_baseline, new_baseline): | |||
| :type new_baseline: dict | |||
| :param new_baseline: most recent scan | |||
| """ | |||
| # TODO: merge_results does not take into account `is_secret` | |||
tests/core/audit_test.py
Outdated
| @@ -24,7 +24,12 @@ def test_no_baseline(self, mock_printer): | |||
| assert mock_printer.message == '' | |||
|
|
|||
| def test_quit_before_making_decision(self, mock_printer): | |||
| with self.mock_env(['q']) as m: | |||
| with mock.patch( | |||
There was a problem hiding this comment.
Let's throw this in mock_env. After all, we're already patching a bunch of things there.
…ne before auditing happens; [dry] moved the relevant mock in audit_test.py to the mock_env method
detect_secrets/core/audit.py
Outdated
| @@ -19,18 +20,30 @@ class SecretNotFoundOnSpecifiedLineError(Exception): | |||
| pass | |||
|
|
|||
|
|
|||
| def _clean_baseline_of_nonexistent_files(baseline): | |||
There was a problem hiding this comment.
_remove_nonexistent_files_from_baseline? And it should return True if modified -- just so it's less negatives, so we don't get a case where:
if not baseline_wasnt_modified:
do_something()
nit: let's throw this below audit_baseline function declaration, so it reads from top-down.
There was a problem hiding this comment.
Hmm, I think I originally made it that but I changed it to wasnt_modified was to avoid the and not baseline_was_modified, which I read as slightly more confusing. This may be a weird preference of mine though, I'll change it ;)
There was a problem hiding this comment.
Hmm and not files_removed: looks nice 👍
…rdered _functions to be in the order that they are called
| @@ -66,6 +70,45 @@ def audit_baseline(baseline_filename): | |||
| _save_baseline_to_file(baseline_filename, original_baseline) | |||
|
|
|||
|
|
|||
| def _get_baseline_from_file(filename): # pragma: no cover | |||
There was a problem hiding this comment.
Raised you one further and re-org'd them based on the order that they are called.
| @@ -66,6 +70,45 @@ def audit_baseline(baseline_filename): | |||
| _save_baseline_to_file(baseline_filename, original_baseline) | |||
|
|
|||
|
|
|||
| def _get_baseline_from_file(filename): # pragma: no cover | |||
No description provided.