Conversation
`PrivateKeyDetector.analyze_line` runs for every line of every scanned
file. The file-size guard was written as a single `and` expression:
if filename not in self._analyzed_files \
and 0 < self.get_file_size(filename) < MAX_FILE_SIZE:
self._analyzed_files.add(filename)
`_analyzed_files` was only populated when the size fell inside the
scannable range. For any file at or above MAX_FILE_SIZE (8 KiB) the
membership check kept failing, so `get_file_size` -> `os.path.getsize`
fired again on every single line: a per-file operation executed
per-line. Cost scaled with (files x lines) rather than (files).
Track the files whose size has already been measured in a separate
`_sized_files` set, so the lookup happens at most once per file
regardless of the result. The subsequent whole-file read is unchanged
and still happens exactly once per file, so multi-line private keys are
detected exactly as before.
Measured on a 21,359-file repository via
`checkov --framework secrets --enable-secret-scan-all-files`:
before: real 2120s user 2014s sys 230s
after: real 1679s user 1638s sys 60s
Wall time -21%; `sys` time (the syscall fingerprint of the redundant
getsize calls) down 3.9x. Findings are identical before and after
(14 findings, same files/lines/checks).
Adds two regression tests:
- `test_get_file_size_is_called_at_most_once_per_file` - asserts the
size lookup runs at most once for a 500-line, >8 KiB file
(previously 500 times).
- `test_multiline_private_key_in_small_file_is_still_detected` -
guards the whole-file read path that multi-line key detection
depends on.
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PrivateKeyDetector.analyze_lineruns for every line of every scanned file. The file-size guard was written as a singleandexpression:_analyzed_fileswas only populated when the size fell inside the scannable range. For any file at or above MAX_FILE_SIZE (8 KiB) the membership check kept failing, soget_file_size->os.path.getsizefired again on every single line: a per-file operation executed per-line. Cost scaled with (files x lines) rather than (files).Track the files whose size has already been measured in a separate
_sized_filesset, so the lookup happens at most once per file regardless of the result. The subsequent whole-file read is unchanged and still happens exactly once per file, so multi-line private keys are detected exactly as before.Measured on a 21,359-file repository via
checkov --framework secrets --enable-secret-scan-all-files:Wall time -21%;
systime (the syscall fingerprint of the redundant getsize calls) down 3.9x. Findings are identical before and after (14 findings, same files/lines/checks).Adds two regression tests:
test_get_file_size_is_called_at_most_once_per_file- asserts the size lookup runs at most once for a 500-line, >8 KiB file (previously 500 times).test_multiline_private_key_in_small_file_is_still_detected- guards the whole-file read path that multi-line key detection depends on.What is the new behavior (if this is a feature change)?
Does this PR introduce a breaking change?