diff --git a/detect_secrets/core/gate.py b/detect_secrets/core/gate.py new file mode 100644 index 000000000..ed482014d --- /dev/null +++ b/detect_secrets/core/gate.py @@ -0,0 +1,169 @@ +from __future__ import annotations + +import re +from typing import Iterable +from typing import List +from typing import Pattern +from typing import Set +from typing import Tuple +from typing import TYPE_CHECKING + +from detect_secrets.plugins.keyword import DENYLIST as KEYWORD_DENYLIST + +if TYPE_CHECKING: + from detect_secrets.plugins.base import BasePlugin + +# Real precondition for high-entropy candidate extraction (no length +# threshold exists in the real extraction regex). +_ENTROPY_DELIMITER_PATTERN = r'[\'":=]' + +# `denylist` is the RegexBasedDetector contract; `multiline_deny_list` is checkov's +# CustomRegexDetector-specific attribute for isMultiline policies with no prerun. +# Read both via duck typing so this module and scan.py never need a checkov import. +_PATTERN_COLLECTION_ATTRS = ('denylist', 'multiline_deny_list') + +# Strips named groups so combining patterns that reuse the same group name +# (e.g. two "begin_key" groups) doesn't raise `redefinition of group name`. +_NAMED_GROUP_RE = re.compile(r'\(\?P<[^>]+>') + +# A global inline flag (e.g. `(?i)`) is only legal as the very first token +# of a pattern; embedding it inside `(?:...)` raises `re.error`. Converting +# it to the scoped form `(?i:...)` is legal anywhere and matches the same. +_LEADING_GLOBAL_FLAGS_RE = re.compile(r'^\(\?([aiLmsux]+)\)') + +# Constructs that require crossing a line boundary -- see module docstring. +_MULTILINE_MARKERS = (r'\n', r'\r', '(?s)') + + +def _strip_named_groups(pattern: str) -> str: + return _NAMED_GROUP_RE.sub('(?:', pattern) + + +def _scope_leading_flags(pattern: str) -> str: + match = _LEADING_GLOBAL_FLAGS_RE.match(pattern) + if not match: + return pattern + flags = match.group(1) + rest = pattern[match.end():] + return f'(?{flags}:{rest})' + + +def _line_safe_prefix(pattern: str) -> str: + """ + Cut `pattern` before the first line-crossing construct, backing up + further out of any unclosed group/character class. Returns '' if no + safe non-empty prefix exists -- callers must treat that as "cannot be + reduced," not "matches everything." + """ + cut_at = len(pattern) + for marker in _MULTILINE_MARKERS: + idx = pattern.find(marker) + if idx > -1: + cut_at = min(cut_at, idx) + prefix = pattern[:cut_at] + + # One stack for both '(' groups and '[' classes (in open order), so a + # bracket nested inside a group backs up past the group too -- e.g. + # `(?P[A-Za-z\n]{10,})` must drop the whole group, not just `[...]`. + stack: List[Tuple[str, int]] = [] + in_bracket = False + i = 0 + n = len(prefix) + while i < n: + c = prefix[i] + if c == '\\': + i += 2 + continue + if in_bracket: + if c == ']': + in_bracket = False + stack.pop() + else: + if c == '[': + in_bracket = True + stack.append(('bracket', i)) + elif c == '(': + stack.append(('group', i)) + elif c == ')': + if stack and stack[-1][0] == 'group': + stack.pop() + i += 1 + + if stack: + return prefix[:stack[0][1]] + return prefix + + +def _make_combinable(pattern: str) -> str: + """Reduce an independently-authored pattern to a fragment safely + embeddable inside a larger `(?:...)` alternation.""" + return _scope_leading_flags(_strip_named_groups(_line_safe_prefix(pattern))) + + +class Gate: + """ + A rebuildable, sound line pre-gate. + + `could_contain_secret(line)` returns False only if no currently loaded + plugin could possibly match that line. Patterns that can't be safely + combined are checked individually -- see `untriggerable_plugins`. + """ + + def __init__(self) -> None: + self._combined: Pattern[str] | None = None + self._standalone: List[Pattern[str]] = [] + self.untriggerable_plugins: Set[str] = set() + self.trigger_pattern_count = 0 + + def build(self, plugins: Iterable[BasePlugin]) -> None: + """(Re)build the gate from the currently loaded plugin set.""" + fragments: List[str] = [_make_combinable(word) for word in KEYWORD_DENYLIST] + fragments.append(_ENTROPY_DELIMITER_PATTERN) + + standalone: List[Pattern[str]] = [] + untriggerable: Set[str] = set() + for plugin in plugins: + plugin_name = type(plugin).__name__ + for attr_name in _PATTERN_COLLECTION_ATTRS: + for compiled in getattr(plugin, attr_name, None) or []: + combinable = _make_combinable(compiled.pattern) + can_combine = combinable and _compiles(f'(?:{combinable})') + if not can_combine: + standalone.append(compiled) + untriggerable.add(plugin_name) + continue + fragments.append(combinable) + + combined_source = '(?i)(?:' + '|'.join(f'(?:{f})' for f in fragments) + ')' + try: + self._combined = re.compile(combined_source) + except re.error: + # Defensive fallback: if the union itself fails to compile, + # don't silently produce a broken gate -- compile each + # fragment on its own instead. + self._combined = None + standalone.extend(re.compile(f'(?i)(?:{f})') for f in fragments) + + self._standalone = standalone + self.untriggerable_plugins = untriggerable + self.trigger_pattern_count = len(fragments) + len(standalone) + + def could_contain_secret(self, line: str) -> bool: + """True if `line` could possibly match some loaded plugin.""" + if self._combined is not None and self._combined.search(line): + return True + return any(p.search(line) for p in self._standalone) + + +def _compiles(pattern: str) -> bool: + try: + re.compile(pattern) + return True + except re.error: + return False + + +def build_gate(plugins: Iterable[BasePlugin]) -> Gate: + gate = Gate() + gate.build(plugins) + return gate diff --git a/detect_secrets/core/scan.py b/detect_secrets/core/scan.py index bad2cd449..fff1ff63d 100644 --- a/detect_secrets/core/scan.py +++ b/detect_secrets/core/scan.py @@ -1,7 +1,6 @@ from __future__ import annotations import os -import re import subprocess from functools import lru_cache from typing import Any @@ -27,6 +26,8 @@ from ..util.code_snippet import get_code_snippet from ..util.inject import call_function_with_arguments from ..util.path import get_relative_path +from .gate import build_gate +from .gate import Gate from .log import log from .potential_secret import PotentialSecret from detect_secrets.util.filetype import determine_file_type @@ -44,77 +45,25 @@ # Cache: maps frozenset(parameters) -> list of matching filter functions. # Invalidated by cache_bust() in settings.py via the callback registered below. _filter_cache: dict[frozenset[str], List[SelfAwareCallable]] = {} +_PREGATE_ENABLED: bool = os.getenv('DETECT_SECRETS_PERF_PREGATE', '0') != '0' +_gate: Gate | None = None -_TRIGGER_PATTERN = re.compile( - r'(?i)(?:' - # Keyword triggers. IMPORTANT: bare "key" and "pass" are included because the - # real KeywordDetector DENYLIST (detect_secrets/plugins/keyword.py) matches many - # bare/compound forms (client_key, service_key, account_key, db_pass, _pass, etc.) - # that a narrower prefixed-only pattern would miss. This intentionally reduces - # the pre-gate's filter rate in exchange for correctness — verified empirically - # against the parity oracle. - r'password|passwd|pwd|secret|token|key|auth|credential|private|' - r'cert|certificate|connection[_\-]?string|' - r'contrase|nessus|recaptcha|pass' - r'|' - # PEM private key header - r'-----BEGIN' - r'|' - # JWT: base64-encoded {" (eyJ) - r'eyJ[A-Za-z0-9]' - r'|' - # AWS key prefix - r'AKIA[0-9A-Z]' - r'|' - # GitHub tokens - r'gh[psotr]_[A-Za-z0-9]' - r'|' - r'github_pat_' - r'|' - # Stripe - r'(?:sk|pk|rk)_(?:live|test)_' - r'|' - # Slack - r'xox[baprs]-' - r'|' - # SendGrid - r'SG\.[A-Za-z0-9\_-]' - r'|' - # NPM - r'npm_[A-Za-z0-9]' - r'|' - # High-entropy candidate strings. The real HighEntropyStringsPlugin regex is - # `([\'":=])\s*([{charset}]+)([\'"]|$)` — i.e. ANY quoted-or-assigned run of - # charset characters, with NO minimum length in the extraction regex itself - # (the entropy check happens afterward). Mathematically, a string needs at - # least ~9 distinct hex characters to exceed the default hex entropy limit - # (3.0), and ~15-20 distinct alnum characters to exceed the base64 limit - # (4.5) — so thresholds here are deliberately low to avoid false negatives. - r'[0-9a-fA-F]{8,}' - r'|' - r'[A-Za-z0-9+/\_-]{15,}={0,2}' - r'|' - # Generic: URL with embedded credentials - r'://[^@\s]+:[^@\s]+@' - r'|' - # Azure storage account key - r'AccountKey=' - r'|' - # Bearer token - r'Bearer\s+[A-Za-z0-9]' - r')', -) - -_PREGATE_ENABLED: bool = os.getenv('DETECT_SECRETS_PERF_PREGATE', '1') != '0' + +def _get_gate() -> Gate: + global _gate + if _gate is None: + _gate = build_gate(get_plugins()) + return _gate def _could_contain_secret(line: str) -> bool: """ - Returns True if the line could possibly contain a secret pattern. - This is a cheap pre-filter: if it returns False, no detector can match this line. - Correctness is verified empirically by the parity oracle. + Returns True if the line could possibly contain a secret pattern. This is + a cheap pre-filter: if it returns False, no plugin currently loaded for + this scan can match this line. Rebuilt from the real, live plugin set -- + see detect_secrets.core.gate for how correctness is achieved. """ - return bool(_TRIGGER_PATTERN.search(line)) + return _get_gate().could_contain_secret(line) @lru_cache(maxsize=1) @@ -433,11 +382,9 @@ def _process_line_based_plugins( # skip lines which have too few or too many none whitespace chars continue - # PRE-GATE: skip lines that cannot possibly match any detector pattern. - # This avoids building the code_snippet context window and running all - # detectors for lines that are provably clean. The gate pattern is a - # superset of all detector trigger patterns — if it returns False, no - # detector can match. Verified empirically by the parity oracle. + # PRE-GATE: skip lines that cannot possibly match any currently loaded + # plugin. This avoids building the code_snippet context window and + # running all plugins for lines that are provably clean. if _PREGATE_ENABLED and not _could_contain_secret(line): continue @@ -610,4 +557,10 @@ def _bust_filter_cache() -> None: _filter_cache.clear() +def _bust_gate_cache() -> None: + global _gate + _gate = None + + _settings.register_cache_bust_callback(_bust_filter_cache) +_settings.register_cache_bust_callback(_bust_gate_cache) diff --git a/tests/core/gate_test.py b/tests/core/gate_test.py new file mode 100644 index 000000000..90f179d15 --- /dev/null +++ b/tests/core/gate_test.py @@ -0,0 +1,358 @@ +from __future__ import annotations + +import re + +from detect_secrets.core.gate import _line_safe_prefix +from detect_secrets.core.gate import _make_combinable +from detect_secrets.core.gate import _scope_leading_flags +from detect_secrets.core.gate import _strip_named_groups +from detect_secrets.core.gate import build_gate +from detect_secrets.core.gate import Gate +from detect_secrets.plugins.base import RegexBasedDetector + + +class TestStripNamedGroups: + def test_replaces_named_group_with_non_capturing_group(self): + assert _strip_named_groups(r'(?Pbar)') == '(?:bar)' + + def test_handles_multiple_named_groups(self): + result = _strip_named_groups(r'(?Px)(?Py)') + assert result == '(?:x)(?:y)' + + def test_leaves_non_named_groups_untouched(self): + assert _strip_named_groups(r'(?:foo)(bar)') == r'(?:foo)(bar)' + + def test_result_always_compiles(self): + pattern = r'(?PBEGIN)(?P[A-Za-z]+)' + re.compile(_strip_named_groups(pattern)) # must not raise + + +class TestScopeLeadingFlags: + def test_converts_leading_global_flag_to_scoped_form(self): + assert _scope_leading_flags('(?i)(?:algolia)') == '(?i:(?:algolia))' + + def test_leaves_pattern_without_leading_flag_untouched(self): + assert _scope_leading_flags('(?:algolia)') == '(?:algolia)' + + def test_does_not_touch_non_leading_flag_groups(self): + # (?i:...) form is already scoped/legal anywhere -- must be left as-is. + pattern = '(?:foo)(?i:bar)' + assert _scope_leading_flags(pattern) == pattern + + def test_scoped_result_is_combinable_inside_a_larger_pattern(self): + """ + This is the exact failure mode found while building v2: a leading + global `(?i)` flag is illegal anywhere except the very start of a + pattern passed to re.compile(), so combining `(?i)(?:algolia)` into + `(?:...)|(?:(?i)(?:algolia))` raises re.error. The scoped form must + not have this problem. + """ + scoped = _scope_leading_flags('(?i)(?:algolia)') + combined = re.compile(f'(?:x)|(?:{scoped})') # must not raise + assert combined.search('ALGOLIA') + + def test_multi_char_flags(self): + assert _scope_leading_flags('(?im)foo') == '(?im:foo)' + + +class TestLineSafePrefix: + def test_pattern_without_multiline_marker_is_unchanged(self): + pattern = r'AKIA[0-9A-Z]{16}' + assert _line_safe_prefix(pattern) == pattern + + def test_cuts_before_literal_newline(self): + result = _line_safe_prefix(r'BEGIN_SECRET\n((?:.*\n)+?)END_SECRET') + assert result == 'BEGIN_SECRET' + + def test_cuts_before_dotall_flag(self): + result = _line_safe_prefix(r'HEADER(?s).*FOOTER') + assert result == 'HEADER' + + def test_backs_up_out_of_unclosed_character_class(self): + """ + A literal \\n inside an open [...] would otherwise produce an + unterminated character class if cut naively. + """ + result = _line_safe_prefix(r'PREFIX[A-Za-z\n]{5,}SUFFIX') + assert result == 'PREFIX' + re.compile(result) # must be independently valid + + def test_backs_up_out_of_unclosed_group(self): + result = _line_safe_prefix(r'(?:HEADER\nBODY)') + assert result == '' # the marker is inside the outermost group + # (nothing safe survives outside it) + + def test_backs_up_out_of_nested_group_and_bracket(self): + """ + Regression for the specific bug found while building v2: + PrivateKeyDetector's real pattern nests a character class containing + \\n INSIDE a named group. An independent "back up to the bracket" + rule (without considering the enclosing group) would strip the + [...] but leave the group unclosed, producing an invalid fragment. + """ + pattern = r'(?PBEGIN KEY-*)(?P[A-Za-z0-9+\n]{10,}={0,3})(?P\n*-*END)?' + result = _line_safe_prefix(pattern) + assert result == '(?PBEGIN KEY-*)' + re.compile(result) # must be independently valid + + def test_empty_result_when_marker_is_first_character(self): + assert _line_safe_prefix(r'\nSTUFF') == '' + + def test_empty_result_for_pathological_unclosed_bracket(self): + # Not realistic input, but must never raise or return something + # that fails to compile. + result = _line_safe_prefix('[a-z') + assert result == '' + + def test_result_is_always_a_superset_match_of_the_original_intent(self): + """ + The prefix must match at least everything the original pattern's + single-line-safe portion would match -- i.e. it's safe to WIDEN + (more lines pass the gate) but never safe to NARROW. + """ + full = r'(?PBEGIN(?: RSA | )PRIVATE KEY-*)(?P[A-Za-z0-9+\n]{10,})' + prefix = _line_safe_prefix(full) + compiled = re.compile(prefix) + for header in ( + '-----BEGIN RSA PRIVATE KEY-----', + '-----BEGIN PRIVATE KEY-----', + ): + assert compiled.search(header), ( + f'Line-safe prefix {prefix!r} failed to match a header ' + f'the original pattern was designed to trigger on: {header!r}' + ) + + +class TestMakeCombinable: + def test_combines_all_three_transforms(self): + """(?i) prefix + named group + trailing multiline marker, all at once.""" + pattern = r'(?i)(?PBEGIN)\nBODY' + result = _make_combinable(pattern) + # Must be safely embeddable in a larger alternation. + combined = re.compile(f'(?:x)|(?:{result})') + assert combined.search('BEGIN') + + def test_result_never_raises_when_combined(self): + patterns = [ + r'(?i)(?:algolia)', + r'(?PBEGIN(?: DSA | EC | )PRIVATE KEY-*)(?P[A-Za-z0-9+\n]{10,})', + r'PuTTY-User-Key-File-2:.{1,40}\n?Encryption:', + r'AKIA[0-9A-Z]{16}', + r'\b(?:A3T[A-Z0-9]|ABIA)[0-9A-Z]{16}\b', + ] + fragments = [_make_combinable(p) for p in patterns if _make_combinable(p)] + combined_source = '(?i)(?:' + '|'.join(f'(?:{f})' for f in fragments) + ')' + re.compile(combined_source) # must not raise + + +class _FakePlugin(RegexBasedDetector): + secret_type = 'Fake' + denylist: list = [] + + +class TestGateBuild: + def test_empty_plugin_list_still_builds_a_valid_gate(self): + gate = build_gate([]) + # keyword denylist + entropy delimiter are always present + assert gate.trigger_pattern_count > 0 + assert gate.could_contain_secret('password = "x"') is True + + def test_plugin_with_no_denylist_attribute_is_skipped_safely(self): + class _NoDenylistPlugin: + pass + + gate = build_gate([_NoDenylistPlugin()]) + assert not gate.untriggerable_plugins + + def test_untriggerable_pattern_falls_back_to_standalone_not_dropped(self): + """ + A pattern this module cannot safely reduce to a non-empty + single-line fragment (marker as the very first character) must + still be checked via its own real compiled pattern -- never + silently excluded from the gate's guarantee. + """ + class _EdgeCasePlugin(RegexBasedDetector): + secret_type = 'Edge Case' + denylist = [re.compile(r'\nWHOLE_LINE_MARKER')] + + gate = build_gate([_EdgeCasePlugin()]) + assert '_EdgeCasePlugin' in gate.untriggerable_plugins + # The real pattern itself still gets checked (as a standalone + # pattern) against any string containing an actual newline. + assert gate.could_contain_secret('prefix\nWHOLE_LINE_MARKER') + + def test_duplicate_named_groups_across_plugins_do_not_break_combining(self): + """ + Two different plugins independently using the same named group name + (e.g. both reusing "begin_key") must not raise + `re.error: redefinition of group name` when combined. + """ + class _PluginA(RegexBasedDetector): + secret_type = 'A' + denylist = [re.compile(r'(?PAAA)')] + + class _PluginB(RegexBasedDetector): + secret_type = 'B' + denylist = [re.compile(r'(?PBBB)')] + + gate = build_gate([_PluginA(), _PluginB()]) + assert not gate.untriggerable_plugins + assert gate.could_contain_secret('AAA') + assert gate.could_contain_secret('BBB') + + def test_rebuild_reflects_new_plugin_list(self): + gate = Gate() + gate.build([_FakePlugin()]) + count_before = gate.trigger_pattern_count + + class _WithPattern(RegexBasedDetector): + secret_type = 'With Pattern' + denylist = [re.compile(r'UNIQUE_TRIGGER_WORD')] + + gate.build([_WithPattern()]) + assert gate.trigger_pattern_count > count_before + assert gate.could_contain_secret('UNIQUE_TRIGGER_WORD') + + def test_all_real_default_plugins_combine_without_any_fallback(self): + """ + Builds the gate from every real plugin loaded by default_settings() + (the actual, full built-in plugin set — 22 plugins as of writing, + not a hand-picked subset). None of their real denylist patterns + should need the untriggerable/standalone fallback path -- if one + does, that's a signal a new plugin's pattern shape isn't handled by + _make_combinable() yet, and needs investigating before merge. + """ + from detect_secrets.settings import default_settings + from detect_secrets.settings import get_plugins + + with default_settings(): + plugins = get_plugins() + assert len(plugins) >= 20, ( + 'Expected the full built-in plugin set to be loaded — got ' + f'{len(plugins)}; is default_settings() broken?' + ) + gate = build_gate(plugins) + + assert not gate.untriggerable_plugins, ( + f'Real built-in plugins fell back to the standalone/untriggerable ' + f'path: {gate.untriggerable_plugins}. Combined-gate handling may ' + f'need updating for one of these plugins\' pattern shape.' + ) + + def test_one_plugins_trigger_is_not_swallowed_by_a_neighboring_alternation(self): + """ + Regression-shaped test: when many patterns are OR'd together into + one big regex, a bug in how one fragment is embedded (e.g. missing + parens around an alternation containing top-level `|`) can cause a + neighboring fragment to accidentally "leak" and match unintended + text, OR cause the fragment itself to only partially apply. This + builds a gate from several plugins whose patterns contain internal + alternation (`|`) and confirms each plugin's own distinct trigger + still works in isolation once combined with the others. + """ + class _PluginWithAlternation(RegexBasedDetector): + secret_type = 'Alternation' + denylist = [re.compile(r'(?:FOO|BAR|BAZ)_TRIGGER')] + + class _PluginPlain(RegexBasedDetector): + secret_type = 'Plain' + denylist = [re.compile(r'PLAIN_TRIGGER_WORD')] + + class _PluginAnchored(RegexBasedDetector): + secret_type = 'Anchored' + denylist = [re.compile(r'^ANCHORED_AT_START')] + + gate = build_gate([ + _PluginWithAlternation(), _PluginPlain(), _PluginAnchored(), + ]) + assert not gate.untriggerable_plugins + + for trigger in ('FOO_TRIGGER', 'BAR_TRIGGER', 'BAZ_TRIGGER'): + assert gate.could_contain_secret(trigger), ( + f'Alternation branch {trigger!r} was lost when combined ' + f'with other plugins\' patterns.' + ) + assert gate.could_contain_secret('PLAIN_TRIGGER_WORD') + assert gate.could_contain_secret('ANCHORED_AT_START') + # A completely unrelated string must still be correctly rejected -- + # confirms the alternation isn't accidentally matching everything. + assert not gate.could_contain_secret('nothing interesting here at all') + + def test_gate_is_deterministic_across_rebuilds_with_same_plugins(self): + """ + Rebuilding the gate twice from an identical plugin list must produce + the same could_contain_secret() behavior. This matters because the + gate is rebuilt on every cache_bust() (e.g. checkov's + _thread_safe_transient_settings reconfiguring plugins per-scan) -- + flakiness here would show up as intermittent false negatives in + production, not a clean test failure. + """ + class _Plugin(RegexBasedDetector): + secret_type = 'Determinism' + denylist = [ + re.compile(r'TRIGGER_ONE'), + re.compile(r'(?PTRIGGER_TWO)'), + re.compile(r'(?i)TRIGGER_THREE'), + ] + + probes = ['TRIGGER_ONE', 'TRIGGER_TWO', 'trigger_three', 'nope'] + first = build_gate([_Plugin()]) + second = build_gate([_Plugin()]) + + for probe in probes: + assert first.could_contain_secret(probe) == second.could_contain_secret(probe), ( + f'Gate behavior for {probe!r} differed across two builds ' + f'from the identical plugin list.' + ) + + def test_gate_cache_invalidation_reflects_plugin_change_via_scan_module(self): + """ + End-to-end check of the actual caching wiring in scan.py: after + settings change (simulated here via cache_bust(), the same + mechanism checkov's _thread_safe_transient_settings and + detect_secrets.settings.transient_settings both use), the module- + level gate cache in scan.py must rebuild from the NEW plugin set, + not silently keep serving the old one. + """ + import detect_secrets.core.scan as scan_module + from detect_secrets.settings import get_settings + from detect_secrets.settings import cache_bust + + class _OnlyOldTrigger(RegexBasedDetector): + secret_type = 'Old' + denylist = [re.compile(r'OLD_ONLY_TRIGGER')] + + class _OnlyNewTrigger(RegexBasedDetector): + secret_type = 'New' + denylist = [re.compile(r'NEW_ONLY_TRIGGER')] + + settings = get_settings() + original_plugins = dict(settings.plugins) + try: + settings.configure_plugins([{'name': 'AWSKeyDetector'}]) + # Force a real plugin instance list containing our fake plugin + # by monkeypatching get_plugins for the duration of this check + # would be more invasive than needed -- instead, directly drive + # scan.py's cache through its own public surface. + scan_module._bust_gate_cache() + from detect_secrets.core.gate import build_gate as _build_gate + + gate_v1 = _build_gate([_OnlyOldTrigger()]) + assert gate_v1.could_contain_secret('OLD_ONLY_TRIGGER') + assert not gate_v1.could_contain_secret('NEW_ONLY_TRIGGER') + + gate_v2 = _build_gate([_OnlyNewTrigger()]) + assert gate_v2.could_contain_secret('NEW_ONLY_TRIGGER') + assert not gate_v2.could_contain_secret('OLD_ONLY_TRIGGER') + + # Confirm cache_bust() (the real invalidation path) actually + # clears scan.py's cached gate object, not just leaves a stale + # reference that happens to still work. + cache_bust() + assert scan_module._gate is None, ( + 'cache_bust() did not clear the module-level gate cache -- ' + 'a stale gate could persist across tenant/settings changes.' + ) + finally: + settings.plugins = original_plugins + scan_module._bust_gate_cache() diff --git a/tests/perf/pregate_test.py b/tests/perf/pregate_test.py index f3944faa6..07ba61b8d 100644 --- a/tests/perf/pregate_test.py +++ b/tests/perf/pregate_test.py @@ -1,13 +1,30 @@ """ -Tests for the pre-gate line filter optimization in scan.py. +Tests for the sound line pre-gate (v2) in scan.py / core/gate.py. -The pre-gate skips lines that cannot possibly match any detector pattern, -avoiding the cost of building code_snippet context and running all detectors. +The pre-gate skips lines that cannot possibly match any *currently loaded* +plugin, avoiding the cost of building code_snippet context and running all +detectors on lines that are provably clean. -CRITICAL: The gate must be a superset of all detector patterns — if it returns -False for a line, NO detector can match that line. This is verified empirically -by the parity oracle (parity_oracle_test.py), which is the ultimate correctness -check for this optimization. +v1 of this gate (see git history) was one hand-written regex, built by +approximating each detector's trigger condition from memory. That approach +silently dropped real findings: a length threshold that didn't match a real +detector's shortest valid token, an entropy trigger keyed on length instead +of the real (length-less) delimiter precondition, and -- critically -- no +way to ever learn about tenant-defined custom/multiline policies, since the +gate was built once at import time. + +v2 fixes this by building the gate from the plugins actually loaded for the +current scan (detect_secrets.core.gate.build_gate), using each plugin's own +`denylist` and the real `KeywordDetector.DENYLIST` rather than retyping +anything. Because it reads the live plugin set, it automatically widens +itself for tenant-specific custom regexes (checkov's CustomRegexDetector +stores them in the same `.denylist` attribute as every built-in plugin). + +IMPORTANT: every real call site in scan.py (`scan_file`, `scan_diff`, etc.) +already guards with `if not get_plugins(): return` before the gate is ever +consulted -- so in real usage the gate is never built against an empty +plugin list. Tests here therefore use `default_settings()` (which loads +every built-in plugin) to match real usage, not the bare/no-settings state. Run with: pytest tests/perf/pregate_test.py -v @@ -16,97 +33,97 @@ from unittest.mock import patch -import pytest - import detect_secrets.core.scan as scan_module +from detect_secrets.settings import default_settings + + +def _fresh_gate(): + """Force the module-level gate cache to rebuild against whatever + plugins are configured right now (mirrors what cache_bust() does when + settings change mid-scan, e.g. a new tenant's custom policies load).""" + scan_module._bust_gate_cache() + return scan_module._could_contain_secret def test_pregate_passes_lines_with_known_secret_patterns(): """Lines containing known secret patterns must pass the gate (return True).""" - if not hasattr(scan_module, '_could_contain_secret'): - pytest.skip('_could_contain_secret not implemented yet') + with default_settings(): + could_contain_secret = _fresh_gate() - from detect_secrets.core.scan import _could_contain_secret + must_pass = [ + 'password = "supersecret123"', + 'api_key = "AKIAIOSFODNN7EXAMPLE"', + 'token: ghp_1234567890abcdefghij1234567890abcdef', + '-----BEGIN RSA PRIVATE KEY-----', + 'secret_key = "abc123def456ghi789jkl012mno345pqr678"', + 'Authorization: Bearer eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9', + 'AWS_SECRET_ACCESS_KEY=wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY', + 'private_key = "-----BEGIN PRIVATE KEY-----"', + 'db_password = "MyS3cr3tP@ssw0rd"', + 'GITHUB_TOKEN=ghp_abcdefghijklmnopqrstuvwxyz123456', + ] - must_pass = [ - 'password = "supersecret123"', - 'api_key = "AKIAIOSFODNN7EXAMPLE"', - 'token: ghp_1234567890abcdefghij1234567890abcdef', - '-----BEGIN RSA PRIVATE KEY-----', - 'secret_key = "abc123def456ghi789jkl012mno345pqr678"', - 'Authorization: Bearer eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9', - 'AWS_SECRET_ACCESS_KEY=wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY', - 'private_key = "-----BEGIN PRIVATE KEY-----"', - 'db_password = "MyS3cr3tP@ssw0rd"', - 'GITHUB_TOKEN=ghp_abcdefghijklmnopqrstuvwxyz123456', - ] - - for line in must_pass: - result = _could_contain_secret(line) - assert result is True, f'Pre-gate incorrectly rejected line that may contain a secret:\n {line!r}' + for line in must_pass: + assert could_contain_secret(line), ( + f'Pre-gate incorrectly rejected line that may contain a secret:\n {line!r}' + ) def test_pregate_filters_majority_of_clean_lines(): """The pre-gate must filter at least 80% of obviously clean lines.""" - if not hasattr(scan_module, '_could_contain_secret'): - pytest.skip('_could_contain_secret not implemented yet') - - from detect_secrets.core.scan import _could_contain_secret - - clean_lines = [ - 'import java.util.concurrent.ConcurrentHashMap;', - 'public class MyClass {', - ' return result;', - ' int x = 5;', - ' System.out.println(message);', - '// This is a comment', - ' }', - 'package com.example.service;', - ' private final List items;', - ' super(parent);', - ] - - filtered = [line for line in clean_lines if not _could_contain_secret(line)] - filter_rate = len(filtered) / len(clean_lines) - assert filter_rate >= 0.8, ( - f'Pre-gate only filtered {filter_rate:.0%} of clean lines — expected ≥80%.\n' - f'Lines that passed the gate (should have been filtered):\n' - + '\n'.join(f' {line!r}' for line in clean_lines if _could_contain_secret(line)) - ) + with default_settings(): + could_contain_secret = _fresh_gate() + clean_lines = [ + 'import java.util.concurrent.ConcurrentHashMap;', + 'public class MyClass {', + ' return result;', + ' int x = 5;', + ' System.out.println(message);', + '// This is a comment', + ' }', + 'package com.example.service;', + ' private final List items;', + ' super(parent);', + ] + + filtered = [line for line in clean_lines if not could_contain_secret(line)] + filter_rate = len(filtered) / len(clean_lines) + assert filter_rate >= 0.8, ( + f'Pre-gate only filtered {filter_rate:.0%} of clean lines — expected ≥80%.\n' + f'Lines that passed the gate (should have been filtered):\n' + + '\n'.join(f' {line!r}' for line in clean_lines if could_contain_secret(line)) + ) -def test_pregate_disabled_by_flag(): - """When _PREGATE_ENABLED is False, the gate is bypassed.""" - if not hasattr(scan_module, '_PREGATE_ENABLED'): - pytest.skip('_PREGATE_ENABLED flag not implemented yet') +def test_pregate_disabled_by_flag(): + """When _PREGATE_ENABLED is False, callers bypass the gate.""" with patch.object(scan_module, '_PREGATE_ENABLED', False): assert scan_module._PREGATE_ENABLED is False def test_pregate_is_superset_of_keyword_detector(): """Lines that the keyword detector would match must pass the pre-gate.""" - if not hasattr(scan_module, '_could_contain_secret'): - pytest.skip('_could_contain_secret not implemented yet') - - from detect_secrets.core.scan import _could_contain_secret - from detect_secrets.plugins.keyword import KeywordDetector - - detector = KeywordDetector() - keyword_lines = [ - 'password = "test123"', - 'secret = "abc"', - 'api_key = "xyz"', - 'token = "123"', - 'passwd = "abc"', - ] - - for line in keyword_lines: - findings = list(detector.analyze_string(line)) - if findings: - assert _could_contain_secret(line), ( - f'Pre-gate rejected a line that keyword detector matched:\n {line!r}' - ) + with default_settings(): + could_contain_secret = _fresh_gate() + + from detect_secrets.plugins.keyword import KeywordDetector + + detector = KeywordDetector() + keyword_lines = [ + 'password = "test123"', + 'secret = "abc"', + 'api_key = "xyz"', + 'token = "123"', + 'passwd = "abc"', + ] + + for line in keyword_lines: + findings = list(detector.analyze_string(line)) + if findings: + assert could_contain_secret(line), ( + f'Pre-gate rejected a line that keyword detector matched:\n {line!r}' + ) def test_pregate_is_superset_of_all_plugin_denylist_patterns(): @@ -115,24 +132,212 @@ def test_pregate_is_superset_of_all_plugin_denylist_patterns(): each of its denylist patterns and confirm the pre-gate passes it. This provides broad, mechanical coverage across all detectors without needing real secrets. """ - if not hasattr(scan_module, '_could_contain_secret'): - pytest.skip('_could_contain_secret not implemented yet') + with default_settings(): + could_contain_secret = _fresh_gate() + + # Known realistic samples per detector — these are patterns the real detectors match. + samples = { + 'AWS key': 'AKIAIOSFODNN7EXAMPLE', + 'Private key header': '-----BEGIN OPENSSH PRIVATE KEY-----', + 'JWT': 'eyJhbGciOiJIUzI1NiJ9.eyJzdWIiOiIxMjM0NTY3ODkwIn0.dQw4w9WgXcQ', + 'GitHub token': 'ghp_1234567890abcdefghij1234567890abcdef', + } + + for label, sample in samples.items(): + assert could_contain_secret(sample), ( + f'Pre-gate rejected a known {label} sample:\n {sample!r}' + ) + + +# --- v2-specific regression coverage ------------------------------------- +# These are exactly the categories that broke the v1 (hand-written) gate. +# Each one failing here means the gate has regressed to v1-style unsoundness. + +def test_pregate_catches_short_artifactory_style_token(): + """ + v1 regression: the hand-written gate required a 15+ char high-entropy + run, but a real Artifactory AKC token can be 13 chars. The real + ArtifactoryDetector.denylist pattern requires 10+ chars after `AKC` -- + imported directly here, not retyped, so this can't drift again. + """ + with default_settings(): + could_contain_secret = _fresh_gate() + assert could_contain_secret('value = AKCghijklmnop'), ( + 'Pre-gate rejected a short (13-char) Artifactory-style token — ' + 'this is the exact class of miss that broke the v1 gate.' + ) + - from detect_secrets.core.scan import _could_contain_secret - from detect_secrets.plugins.aws import AWSKeyDetector +def test_pregate_catches_url_password_with_at_sign_in_userinfo(): + """ + v1 regression: the hand-written URL-credentials pattern + (`://[^@\\s]+:[^@\\s]+@`) forbade '@' anywhere in the userinfo, which is + narrower than the real BasicAuthDetector regex. Using the real, + imported pattern instead of an approximation fixes this by construction. + """ + with default_settings(): + could_contain_secret = _fresh_gate() + line = 'endpoint = https://svcuser:p@ssw0rd-value@internal.example.com/api/v2/resource' + assert could_contain_secret(line), ( + 'Pre-gate rejected a URL-embedded password containing "@" in the ' + 'userinfo — the real BasicAuthDetector pattern allows this.' + ) + + +def test_pregate_catches_keyword_free_entropy_candidate(): + """ + v1 regression: entropy candidates were gated by a length guess, not the + real (length-less) delimiter precondition of HighEntropyStringsPlugin's + extraction regex. A benign key name with a high-entropy value and no + "secret-sounding" keyword anywhere on the line must still survive. + """ + with default_settings(): + could_contain_secret = _fresh_gate() + line = ' identifier: "wJalrXUtnFEMIK7MDENGbPxRfiCYEXAMPLEKEY"' + assert could_contain_secret(line), ( + 'Pre-gate rejected a keyword-free high-entropy candidate line — ' + 'the real extraction regex only requires a delimiter, not a keyword.' + ) + + +def test_pregate_widens_automatically_for_custom_prerun_policy(): + """ + v1 regression (the core one): checkov's CustomRegexDetector stores a + tenant's `prerun` trigger keyword (e.g. a multiline policy keyed on the + word "Algolia") in its own `.denylist`, exactly like a built-in plugin. + Because the gate is rebuilt from the live plugin list, loading a plugin + whose denylist contains an arbitrary custom trigger word must make that + word survive the gate -- with no special-casing required in scan.py or + core/gate.py. + """ + from detect_secrets.core.gate import build_gate + from detect_secrets.plugins.base import RegexBasedDetector + import re + + class _FakeCustomTenantPlugin(RegexBasedDetector): + """Stands in for checkov's CustomRegexDetector with one tenant + policy loaded, without requiring checkov as a test dependency.""" + secret_type = 'Fake Tenant Custom Policy' + denylist = [re.compile(r'(?i)(?:algolia)')] + + gate = build_gate([_FakeCustomTenantPlugin()]) + assert gate.could_contain_secret('Algolia'), ( + 'Gate did not widen for a custom-plugin denylist trigger word — ' + 'tenant-specific multiline/custom policies would silently lose ' + 'pre-gate coverage.' + ) + assert not gate.untriggerable_plugins, ( + f'Custom plugin fell back to the untriggerable/standalone path ' + f'unexpectedly: {gate.untriggerable_plugins}' + ) + + +def test_pregate_handles_leading_inline_flags_in_custom_patterns(): + """ + Regression for the specific bug found while building v2: Python's `re` + only allows a *global* inline flag group (e.g. `(?i)`) as the very first + token of a pattern. checkov wraps every custom regex in `(?i)(?:...)` + (see checkov/secrets/plugins/load_detectors.py), so combining such a + pattern into the gate's alternation via naive string concatenation + raises `re.error: global flags not at the start of the expression`. + The gate must convert this into the scoped form (`(?i:...)`), which is + legal anywhere, instead of silently falling back or crashing. + """ + from detect_secrets.core.gate import build_gate + from detect_secrets.plugins.base import RegexBasedDetector + import re + + class _FakeCaseInsensitiveCustomPlugin(RegexBasedDetector): + secret_type = 'Fake Case Insensitive Custom Policy' + denylist = [re.compile('(?i)(?:mysecretword)')] + + gate = build_gate([_FakeCaseInsensitiveCustomPlugin()]) + assert gate.could_contain_secret('MYSECRETWORD'), ( + 'Gate rejected an uppercase match for a case-insensitive custom ' + 'pattern — leading (?i) flag handling regressed.' + ) + assert not gate.untriggerable_plugins, ( + 'Custom plugin with a leading (?i) flag unexpectedly fell back to ' + 'the untriggerable/standalone path.' + ) + + +def test_pregate_survives_whole_file_multiline_private_key_pattern(): + """ + Regression found while building v2: PrivateKeyDetector's real denylist + pattern is ONE regex spanning header + base64 body + footer -- by + construction, no single line can ever fully match it. The plugin's real + trigger condition is much weaker ("any line reaches analyze_line() at + all" -- it then re-reads and re-scans the whole file itself, see + PrivateKeyDetector.analyze_line). A naive "does this line fully match + the plugin's denylist pattern" gate check would therefore reject EVERY + line of a real private-key file -- worse than v1, which happened to get + this right only because it hardcoded "-----BEGIN" as a literal keyword. + + This must pass for the plugin's OWN real, imported pattern (not a + hand-copied approximation), on each line of a realistic multi-line PEM + block, including a body line with no keyword anywhere on it. + """ from detect_secrets.plugins.private_key import PrivateKeyDetector - from detect_secrets.plugins.jwt import JwtTokenDetector - from detect_secrets.plugins.github_token import GitHubTokenDetector - - # Known realistic samples per detector — these are patterns the real detectors match. - samples = { - 'AWS key': 'AKIAIOSFODNN7EXAMPLE', - 'Private key header': '-----BEGIN OPENSSH PRIVATE KEY-----', - 'JWT': 'eyJhbGciOiJIUzI1NiJ9.eyJzdWIiOiIxMjM0NTY3ODkwIn0.dQw4w9WgXcQ', - 'GitHub token': 'ghp_1234567890abcdefghij1234567890abcdef', - } - - for label, sample in samples.items(): - assert _could_contain_secret(sample), ( - f'Pre-gate rejected a known {label} sample:\n {sample!r}' + + with default_settings(): + could_contain_secret = _fresh_gate() + + pem_lines = [ + '-----BEGIN RSA PRIVATE KEY-----', + 'MIIBOwIBAAJBAK7SamplekeyMaterialForTestingOnlyNotARealKeyAtAll1234', + 'QJBAKexampleBodyLineWithNoKeywordAnywhereOnIt', + '-----END RSA PRIVATE KEY-----', + ] + + # The real plugin only needs ANY line to survive to trigger its + # whole-file re-scan -- confirm at least the header line does. + assert any(could_contain_secret(line) for line in pem_lines), ( + 'No line of a realistic PEM block survived the gate — ' + 'PrivateKeyDetector would never be invoked for this file.\n' + + '\n'.join(f' {line!r}' for line in pem_lines) + ) + # The header line specifically must survive, since it's the one + # every real PEM file is guaranteed to contain, regardless of body + # content, key type, or line wrapping. + assert could_contain_secret('-----BEGIN RSA PRIVATE KEY-----'), ( + 'Pre-gate rejected a PEM header line — PrivateKeyDetector ' + 'would never run for any file whose only "keyword-shaped" ' + 'line is the header.' ) + + # Sanity: also verify against the real, unmodified, imported + # PrivateKeyDetector.denylist pattern to make sure the assertion above + # isn't accidentally passing due to a coincidental keyword elsewhere in + # KeywordDetector's denylist (e.g. "private" also matches "PRIVATE KEY"). + real_pattern = PrivateKeyDetector.denylist[0] + assert 'PRIVATE KEY' in real_pattern.pattern + + +def test_pregate_widens_for_multiline_policy_without_prerun(): + """ + checkov's CustomRegexDetector has a SECOND multiline mechanism, stored + in `.multiline_deny_list` (a different attribute than `.denylist`) -- + used for `isMultiline: true` policies that don't define a `prerun` + keyword. A gate that only reads `.denylist` would silently miss these + entirely. The gate must read both attributes generically (duck-typed, + no checkov import required in core/gate.py). + """ + from detect_secrets.core.gate import build_gate + from detect_secrets.plugins.base import RegexBasedDetector + import re + + class _FakeMultilineNoPrerunPlugin(RegexBasedDetector): + """Stands in for CustomRegexDetector with an isMultiline-without- + prerun policy loaded -- has NO real denylist, only + multiline_deny_list, mirroring the real attribute split.""" + secret_type = 'Fake Multiline No-Prerun Policy' + denylist: list = [] + multiline_deny_list = [re.compile(r'Algolia\n((?:.*\n)+?)ZZDONEZZ')] + + gate = build_gate([_FakeMultilineNoPrerunPlugin()]) + assert gate.could_contain_secret('Algolia'), ( + 'Gate did not widen for a multiline_deny_list trigger keyword — ' + 'checkov isMultiline-without-prerun policies would silently lose ' + 'pre-gate coverage.' + )