From eb99b686eb6d557c7448acf48c891373a937e3d4 Mon Sep 17 00:00:00 2001 From: Saar Ettinger Date: Thu, 6 Aug 2026 12:06:40 +0300 Subject: [PATCH] perf(private_key): look up file size once per file instead of per line `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. --- detect_secrets/plugins/private_key.py | 19 +++++++-- tests/plugins/private_key_test.py | 60 +++++++++++++++++++++++++++ 2 files changed, 76 insertions(+), 3 deletions(-) diff --git a/detect_secrets/plugins/private_key.py b/detect_secrets/plugins/private_key.py index 9cec82857..744435ccd 100644 --- a/detect_secrets/plugins/private_key.py +++ b/detect_secrets/plugins/private_key.py @@ -68,6 +68,12 @@ class PrivateKeyDetector(RegexBasedDetector): def __init__(self) -> None: self._analyzed_files: Set[str] = set() + # Tracks files whose on-disk size has already been checked, regardless of + # whether that size fell within the scannable range. Without this, files + # outside the size range (e.g. larger than MAX_FILE_SIZE) would never be + # recorded in ``_analyzed_files``, causing ``get_file_size`` to re-run for + # every single line of the file -- a per-file operation executed per-line. + self._sized_files: Set[str] = set() self._commit_hashes: Set[Tuple[str, str]] = set() def analyze_line( @@ -111,9 +117,16 @@ def analyze_line( self._commit_hashes.add((filename, commit_hash)) return output - if filename not in self._analyzed_files \ - and 0 < self.get_file_size(filename) < PrivateKeyDetector.MAX_FILE_SIZE: - self._analyzed_files.add(filename) + # Determine the file size at most once per file. Files whose size falls + # outside the scannable range are still recorded (via ``_sized_files``) so + # that we never re-run this filesystem lookup on subsequent lines. + if filename not in self._sized_files: + self._sized_files.add(filename) + if 0 < self.get_file_size(filename) < PrivateKeyDetector.MAX_FILE_SIZE: + self._analyzed_files.add(filename) + + if filename in self._analyzed_files: + self._analyzed_files.discard(filename) file_content = self.read_file(filename) if file_content: found_secrets = super().analyze_line( diff --git a/tests/plugins/private_key_test.py b/tests/plugins/private_key_test.py index 198c57cfa..b3115d86e 100644 --- a/tests/plugins/private_key_test.py +++ b/tests/plugins/private_key_test.py @@ -1,4 +1,5 @@ import json +from unittest import mock import pytest @@ -150,6 +151,65 @@ def test_private_key_line_number_2(): ) +def test_get_file_size_is_called_at_most_once_per_file(): + """Regression test for a performance bug. + + ``PrivateKeyDetector.analyze_line`` runs once per line. It must not perform + a filesystem ``getsize`` lookup on every line: for a file larger than + ``MAX_FILE_SIZE`` (which is never added to the ``_analyzed_files`` cache), + the size lookup used to fire once per line, making the cost scale with + (files x lines) instead of (files). This test proves the file-size lookup + happens at most once for the whole file. + """ + # Build a file that is well over MAX_FILE_SIZE (8 KiB) and has many lines, + # none of which contain a private key. + line = 'this is a perfectly ordinary line with no secrets in it at all' + file_content = '\n'.join(line for _ in range(500)) + assert len(file_content.encode()) > (8 * 1024) + + with mock_named_temporary_file() as f: + f.write(file_content.encode()) + f.seek(0) + + with mock.patch( + 'detect_secrets.plugins.private_key.os.path.getsize', + wraps=__import__('os').path.getsize, + ) as mock_getsize: + secrets = SecretsCollection() + secrets.scan_file(f.name) + + assert mock_getsize.call_count <= 1, ( + f'Expected the private-key file-size lookup to run at most once per ' + f'file, but it ran {mock_getsize.call_count} times.' + ) + + +def test_multiline_private_key_in_small_file_is_still_detected(): + """Guards the file-size fix. + + The size lookup was hoisted to run once per file, and the per-file content + read now happens exactly once. This confirms the whole-file read path still + fires for a small file, so a private key split across multiple lines (which + only matches when the full file content is scanned) is still detected. + """ + file_content = '\n'.join([ + 'Irrelevant line', + '-----BEGIN RSA PRIVATE KEY-----', + 'MIIBVwIBADANBgkqhkiG9w0BAQEFAASC', + '-----END RSA PRIVATE KEY-----', + ]) + assert len(file_content.encode()) < (8 * 1024) + + with mock_named_temporary_file() as f: + f.write(file_content.encode()) + f.seek(0) + + secrets = SecretsCollection() + secrets.scan_file(f.name) + + assert len(list(secrets)) == 1 + + @pytest.fixture(autouse=True) def configure_plugins(): with transient_settings({