diff --git a/detect_secrets/core/gate.py b/detect_secrets/core/gate.py index ed482014d..30c588092 100644 --- a/detect_secrets/core/gate.py +++ b/detect_secrets/core/gate.py @@ -13,9 +13,7 @@ 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'[\'":=]' +_ENTROPY_VALUE_PATTERN = r'[\'":=]\s*(?:\S+\s+){0,2}\S{12,}' # `denylist` is the RegexBasedDetector contract; `multiline_deny_list` is checkov's # CustomRegexDetector-specific attribute for isMultiline policies with no prerun. @@ -118,7 +116,7 @@ def __init__(self) -> None: 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) + fragments.append(_ENTROPY_VALUE_PATTERN) standalone: List[Pattern[str]] = [] untriggerable: Set[str] = set() diff --git a/detect_secrets/util/inject.py b/detect_secrets/util/inject.py index a35d9dd5f..7397afbdb 100644 --- a/detect_secrets/util/inject.py +++ b/detect_secrets/util/inject.py @@ -1,4 +1,3 @@ -import inspect import os from types import MethodType from typing import Any @@ -49,11 +48,14 @@ def call_function_with_arguments( def _call_with_cache(func: Union[Callable, SelfAwareCallable], **kwargs: Any) -> Any: """Fast path: use cached plan to avoid repeated inspect calls.""" - is_bound = inspect.ismethod(func) + # isinstance() is ~10x faster than inspect.ismethod() — the latter is just + # ``isinstance(object, types.MethodType)`` with extra function-call overhead. + is_bound = isinstance(func, MethodType) # Use the underlying function's id for bound methods — stable across calls # (Python creates a new bound method object on each attribute access, but - # func.__func__ is the stable underlying function object) + # func.__func__ is the stable underlying function object). + # cast() is a no-op at runtime; it only satisfies mypy's union-attr check. cache_key = id(cast(MethodType, func).__func__) if is_bound else id(func) plan = _plan_cache.get(cache_key) @@ -93,7 +95,7 @@ def _call_without_cache(func: Union[Callable, SelfAwareCallable], **kwargs: Any) function = func if isinstance(func, SelfAwareCallable) else make_function_self_aware(func) # If `function` is derived from a method, we add the instance of the class by default. - if inspect.ismethod(func) and not inspect.ismethod(function): + if isinstance(func, MethodType) and not isinstance(function, MethodType): kwargs[get_injectable_variables(func)[0]] = func.__self__ variables_to_inject = set(kwargs.keys()) @@ -115,7 +117,7 @@ def make_function_self_aware(func: Callable) -> SelfAwareCallable: # We can't add arbitrary attributes to methods, but we can to functions. Therefore, # we need to reference the underlying function itself. - if inspect.ismethod(func): + if isinstance(func, MethodType): klass = func.__self__.__class__ function = getattr(klass, func.__name__) function.injectable_variables = set(get_injectable_variables(func)) diff --git a/tests/core/gate_test.py b/tests/core/gate_test.py index 90f179d15..4a02ba778 100644 --- a/tests/core/gate_test.py +++ b/tests/core/gate_test.py @@ -153,8 +153,9 @@ class _FakePlugin(RegexBasedDetector): class TestGateBuild: def test_empty_plugin_list_still_builds_a_valid_gate(self): gate = build_gate([]) - # keyword denylist + entropy delimiter are always present + # keyword denylist + entropy value pattern are always present assert gate.trigger_pattern_count > 0 + # 'password' is in the keyword denylist, so it passes regardless of value length assert gate.could_contain_secret('password = "x"') is True def test_plugin_with_no_denylist_attribute_is_skipped_safely(self): @@ -356,3 +357,97 @@ class _OnlyNewTrigger(RegexBasedDetector): finally: settings.plugins = original_plugins scan_module._bust_gate_cache() + + +class TestEntropyValuePattern: + """Tests for the smarter entropy value pattern that requires ≥8 non-whitespace + chars after a delimiter, replacing the old blanket delimiter pattern.""" + + def test_short_json_values_are_filtered(self): + """JSON lines with short values (< 12 chars) should be rejected by the + gate when no keyword is present.""" + gate = build_gate([]) + short_value_lines = [ + '"color": "#fff"', + '"name": "red"', + '"x": 42', + '"enabled": true', + '"items": []', + '"data": null', + '"id": "abc"', + '"status": "active"', + '"type": "button"', + ] + for line in short_value_lines: + assert not gate.could_contain_secret(line), ( + f'Gate passed a short-value JSON line that cannot contain a secret:\n {line!r}' + ) + + def test_long_json_values_pass_gate(self): + """JSON lines with values ≥ 12 contiguous non-whitespace chars + after a delimiter should pass.""" + gate = build_gate([]) + long_value_lines = [ + '"api_key": "sk-abc123def456ghi789"', + '"token": "AKIAIOSFODNN7EXAMPLE"', + '"secret": "wJalrXUtnFEMI/K7MDENG"', + 'value = "abcdefghijklmnop"', + "key: 'longvalue12345678'", + ] + for line in long_value_lines: + assert gate.could_contain_secret(line), ( + f'Gate rejected a line with a long value that could be a secret:\n {line!r}' + ) + + def test_authorization_bearer_header_passes_gate(self): + """Lines like 'Authorization: Bearer ' must pass because + the pattern allows up to 2 whitespace gaps before the 12+ char token.""" + gate = build_gate([]) + assert gate.could_contain_secret( + 'Authorization: Bearer eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9', + ), 'Gate rejected an Authorization Bearer header with a long JWT token' + + def test_keyword_lines_pass_regardless_of_value_length(self): + """Lines containing keyword denylist words should always pass, + even if the value after the delimiter is short.""" + gate = build_gate([]) + keyword_lines = [ + 'password = "x"', + 'secret: "ab"', + 'api_key = ""', + 'token: "hi"', + ] + for line in keyword_lines: + assert gate.could_contain_secret(line), ( + f'Gate rejected a keyword-bearing line:\n {line!r}' + ) + + def test_structural_json_lines_are_filtered(self): + """Pure structural JSON lines (braces, brackets, commas) should be filtered.""" + gate = build_gate([]) + structural_lines = [ + '{', + '}', + ' },', + ' ],', + ' [', + ] + for line in structural_lines: + assert not gate.could_contain_secret(line), ( + f'Gate passed a structural JSON line:\n {line!r}' + ) + + def test_value_pattern_boundary_at_12_chars(self): + r"""The pattern requires ≥12 contiguous non-whitespace chars after a + delimiter. Note: the closing quote counts as part of the \S run, so + a quoted value of N chars produces an N+1 char \S run (value + quote). + For unquoted values the boundary is exact.""" + gate = build_gate([]) + # 12-char unquoted value after "=" — should pass + assert gate.could_contain_secret('x = 123456789012'), ( + 'Gate rejected a line with exactly 12-char unquoted value' + ) + # 11-char unquoted value after "=" — should NOT pass + assert not gate.could_contain_secret('x = 12345678901'), ( + 'Gate passed a line with only 11-char unquoted value and no keyword' + ) diff --git a/tests/perf/inject_cache_test.py b/tests/perf/inject_cache_test.py index d12718d02..422563de5 100644 --- a/tests/perf/inject_cache_test.py +++ b/tests/perf/inject_cache_test.py @@ -3,9 +3,14 @@ The cache eliminates repeated inspect calls in the hot path by caching the parameter plan for each callable after the first call. + +The isinstance(func, MethodType) optimization replaces the slower +inspect.ismethod(func) call — both are semantically identical but +isinstance avoids the function-call overhead of the inspect module. """ from __future__ import annotations +import types from unittest.mock import patch import pytest @@ -137,3 +142,114 @@ def test_di_cache_disabled_by_flag(): assert mock_msfa.call_count >= 2, ( f'Expected ≥2 calls when cache disabled, got {mock_msfa.call_count}' ) + + +# --- isinstance optimization tests ------------------------------------------- + +class _PluginWithReturn: + """Plugin that returns identifiable values so we can assert on real results.""" + def analyze_line(self, filename: str, line: str, line_number: int = 0) -> list: + return [f'found:{line}'] + + def filter_check(self, line: str) -> bool: + return 'secret' in line + + +def _plain_function(filename: str, line: str) -> str: + return f'plain:{filename}:{line}' + + +def test_isinstance_bound_method_returns_correct_result(): + """Bound method dispatch via isinstance(func, MethodType) returns the + real result from the plugin method.""" + plugin = _PluginWithReturn() + inject_module._plan_cache.clear() + + result = call_function_with_arguments( + plugin.analyze_line, + filename='test.py', + line='hello', + line_number=1, + ) + assert result == ['found:hello'] + + +def test_isinstance_plain_function_returns_correct_result(): + """Plain (non-bound) function dispatch returns the real result.""" + inject_module._plan_cache.clear() + + result = call_function_with_arguments( + _plain_function, + filename='test.py', + line='world', + ) + assert result == 'plain:test.py:world' + + +def test_isinstance_bound_method_ignores_extra_kwargs(): + """Extra kwargs not in the method signature are silently ignored.""" + plugin = _PluginWithReturn() + inject_module._plan_cache.clear() + + result = call_function_with_arguments( + plugin.analyze_line, + filename='test.py', + line='data', + line_number=5, + extra_kwarg='should_be_ignored', + another_extra=42, + ) + assert result == ['found:data'] + + +def test_isinstance_no_inspect_module_imported(): + """The inject module should no longer import the inspect module at all.""" + import importlib + import detect_secrets.util.inject as fresh_module + # Check that 'inspect' is not in the module's namespace + assert not hasattr(fresh_module, 'inspect'), ( + 'inject.py still imports the inspect module — ' + 'isinstance(func, MethodType) should have replaced all inspect.ismethod() calls' + ) + + +def test_isinstance_method_type_detection_matches_inspect(): + """isinstance(func, MethodType) must agree with the old inspect.ismethod() + for both bound methods and plain functions.""" + import inspect + plugin = _PluginWithReturn() + + bound = plugin.analyze_line + assert isinstance(bound, types.MethodType) == inspect.ismethod(bound), ( + 'isinstance(func, MethodType) disagrees with inspect.ismethod() for a bound method' + ) + + assert isinstance(_plain_function, types.MethodType) == inspect.ismethod(_plain_function), ( + 'isinstance(func, MethodType) disagrees with inspect.ismethod() for a plain function' + ) + + +def test_isinstance_cached_and_uncached_paths_produce_same_result(): + """Both the cached (DI_CACHE_ENABLED=True) and uncached paths must + produce identical results for the same input.""" + plugin = _PluginWithReturn() + inject_module._plan_cache.clear() + + # Cached path + result_cached = call_function_with_arguments( + plugin.analyze_line, + filename='f.py', + line='test_line', + line_number=1, + ) + + # Uncached path + with patch.object(inject_module, '_DI_CACHE_ENABLED', False): + result_uncached = call_function_with_arguments( + plugin.analyze_line, + filename='f.py', + line='test_line', + line_number=1, + ) + + assert result_cached == result_uncached == ['found:test_line']