From 5387c3e16388fff3bd10928a639c83d82dee26f2 Mon Sep 17 00:00:00 2001 From: badhope Date: Sat, 15 Aug 2026 07:05:59 +0800 Subject: [PATCH] fix(analyzer): make E2 whitespace-tolerant and detect all os.environ read forms The E2 regex fallback (used when Python source cannot be parsed by AST) was missing several common os.environ access patterns and was not whitespace-tolerant for the patterns it did cover. Add fallback patterns for: - os.environ['KEY'] / os.environ["KEY"] (whitespace-tolerant) - os.environ.get('KEY') (whitespace-tolerant) All existing patterns (items(), copy(), dict(), {**} spread) retain whitespace tolerance. The dict-spread regex explicitly requires braces ({**os.environ}) so bare exponentiation (2 ** os.environ) is not flagged as environment harvesting. AST-level detection (used when Python parses successfully) now also covers: - os.environ['KEY'] / os.environ["KEY"] via ast.Subscript handling - os.environ.get('KEY') by adding 'get' and 'setdefault' to the _ENVIRONMENT_MAPPING_METHOD_CONFIDENCE mapping This closes the gap where whitespace-obfuscated access (e.g. `os . environ [ 'API_KEY' ]`) was parsed by the AST but not emitted as a finding because Subscript nodes were not checked and the 'get' method was not in the confidence table. Add regression tests: - Whitespace-obfuscated environ access is detected (>= 2 findings) - 2 ** os.environ (exponentiation) is NOT flagged as E2 - {**os.environ} (dict spread) IS flagged as E2 Signed-off-by: badhope --- .../static_patterns_data_exfiltration.py | 31 ++++++++++++--- tests/nodes/analyzers/test_static_patterns.py | 38 +++++++++++++++++++ 2 files changed, 64 insertions(+), 5 deletions(-) diff --git a/src/skillspector/nodes/analyzers/static_patterns_data_exfiltration.py b/src/skillspector/nodes/analyzers/static_patterns_data_exfiltration.py index e49ff42a..b3fdf1f3 100644 --- a/src/skillspector/nodes/analyzers/static_patterns_data_exfiltration.py +++ b/src/skillspector/nodes/analyzers/static_patterns_data_exfiltration.py @@ -58,9 +58,21 @@ ), ] E2_PYTHON_FALLBACK_PATTERNS = [ + # Python: for k, v in os.environ.items() — whitespace-tolerant (r"for\s+\w+\s*,\s*\w+\s+in\s+os\s*\.\s*environ\s*\.\s*items\s*\(\s*\)", 0.7), + # Python: os.environ["KEY"] / os.environ['SECRET'] — whitespace-tolerant + ( + r"os\s*\.\s*environ\s*\[\s*['\"][^'\"]*(?:KEY|SECRET|TOKEN|PASSWORD|CREDENTIAL)[^'\"]*['\"]\s*\]", + 0.8, + ), + # Python: os.environ.get("KEY") — whitespace-tolerant + (r"os\s*\.\s*environ\s*\.\s*get\s*\([^)]*(?:KEY|SECRET|TOKEN|PASSWORD|CREDENTIAL)", 0.7), + # Python: os.environ.copy() — full environ read (r"os\s*\.\s*environ\s*\.\s*copy\s*\(\s*\)", 0.6), + # Python: dict(os.environ) — full environ read via dict() (r"dict\s*\(\s*os\s*\.\s*environ\s*\)", 0.6), + # Python: {**os.environ} — full environ read via dict-spread. + # Require braces so bare ``2 ** os.environ`` (exponentiation) is not flagged. (r"\{\s*\*\*\s*os\s*\.\s*environ\s*\}", 0.6), ] E2_OTHER_PATTERNS = [ @@ -79,6 +91,8 @@ "items": 0.7, "keys": 0.6, "values": 0.6, + "get": 0.7, + "setdefault": 0.6, } _ENVIRONMENT_COLLECTION_CALLS = frozenset({"dict", "list", "tuple", "set", "frozenset"}) _ENVIRONMENT_COPY_CALLS = frozenset({"copy.copy", "copy.deepcopy"}) @@ -182,11 +196,14 @@ def _analyze_python_environment_reads( ) -> list[AnalyzerFinding] | None: """Detect materializing or enumerating the complete ``os.environ`` mapping. - A full mapping copy or enumeration is an environment-harvesting signal, unlike a - targeted single-key lookup or passing ``os.environ`` through to a child process. - Credential flows to network and execution sinks remain covered by the behavioral - taint analyzer. AST parsing makes this check insensitive to formatting and lets it - resolve ``os`` / ``environ`` import aliases. + Detects full mapping copies/enumerations (``items()``, ``keys()``, + ``values()``, ``copy()``, ``dict(os.environ)``, ``{**os.environ}``), + single-key lookups (``os.environ['KEY']``, ``os.environ.get('SECRET')``), + and iteration over ``os.environ``. Single-key access is flagged at the same + severity because credential keys are the primary target of env harvesting. + Credential flows to network and execution sinks remain covered by the + behavioral taint analyzer. AST parsing makes this check insensitive to + formatting and lets it resolve ``os`` / ``environ`` import aliases. ``None`` means the source could not be parsed, so callers can retain the regex fallback for malformed Python files. Standalone callers parse through the @@ -263,6 +280,10 @@ def emit(node: ast.AST, confidence: float) -> None: if _is_os_environ_reference(ast_node.iter, aliases): emit(ast_node.iter, 0.7) + elif isinstance(ast_node, ast.Subscript): + if _is_os_environ_reference(ast_node.value, aliases): + emit(ast_node, 0.7) + return findings diff --git a/tests/nodes/analyzers/test_static_patterns.py b/tests/nodes/analyzers/test_static_patterns.py index 05fe19fc..72c02493 100644 --- a/tests/nodes/analyzers/test_static_patterns.py +++ b/tests/nodes/analyzers/test_static_patterns.py @@ -333,6 +333,44 @@ def test_e2_env_harvesting_produces_finding(self): e2 = next(f for f in findings if f.rule_id == "E2") assert e2.severity == "HIGH" + def test_e2_whitespace_tolerant_environ_access(self): + """Whitespace-obfuscated os.environ access is still detected.""" + state = { + "components": ["script.py"], + "file_cache": { + "script.py": "import os\nx = os . environ [ 'API_KEY' ]\ny = os.environ.get('SECRET')", + }, + } + findings = static_runner.run_static_patterns(state, [data_exfiltration_module]) + e2 = [f for f in findings if f.rule_id == "E2"] + assert len(e2) >= 2 + + def test_e2_exponentiation_not_flagged(self): + """Bare ``2 ** os.environ`` (exponentiation) must not be flagged as E2.""" + # Malformed Python (triggers regex fallback) with exponentiation + state = { + "components": ["script.py"], + "file_cache": { + "script.py": "import os\nresult = 2 ** os.environ\n def broken(", + }, + } + findings = static_runner.run_static_patterns(state, [data_exfiltration_module]) + e2 = [f for f in findings if f.rule_id == "E2"] + # Should NOT flag the exponentiation as env harvesting + assert not any("**" in f.matched_text for f in e2) + + def test_e2_dict_spread_environ_flagged(self): + """``{**os.environ}`` (dict spread) is flagged as full environ read.""" + state = { + "components": ["script.py"], + "file_cache": { + "script.py": "import os\nenv_copy = {**os.environ}", + }, + } + findings = static_runner.run_static_patterns(state, [data_exfiltration_module]) + e2 = [f for f in findings if f.rule_id == "E2"] + assert len(e2) >= 1 + def test_e5_boto3_put_object_produces_finding(self): """boto3 put_object yields E5, MEDIUM severity.""" state = {