diff --git a/src/skillspector/nodes/analyzers/static_patterns_tool_misuse.py b/src/skillspector/nodes/analyzers/static_patterns_tool_misuse.py index 8d10c6553..b64592b93 100644 --- a/src/skillspector/nodes/analyzers/static_patterns_tool_misuse.py +++ b/src/skillspector/nodes/analyzers/static_patterns_tool_misuse.py @@ -27,13 +27,13 @@ import ast import re import sys -from bisect import bisect_right +from bisect import bisect_left, bisect_right from collections.abc import Callable, Iterator from dataclasses import dataclass from skillspector.logging_config import get_logger -from skillspector.models import AnalyzerFinding, Location, Severity -from skillspector.python_ast import parse_python_source +from skillspector.models import AnalyzerFinding, Finding, Location, Severity +from skillspector.python_ast import ParsedPythonFile, parse_python_source from skillspector.python_tokens import PythonLiteralSpans from skillspector.python_tokens import python_literal_spans as _python_literal_spans from skillspector.security_reconstruction import validated_json_string_spans @@ -53,6 +53,11 @@ logger = get_logger(__name__) ANALYZER_ID = "static_patterns_tool_misuse" +ANALYZE_USES_POSTPROCESS = True +POSTPROCESS_USES_PYTHON_AST = True +_VARIABLE_SHELL_FLAG_EVIDENCE = "_tm1_variable_shell_flag" +_TRUE_SHELL_DIRECT_EVIDENCE = "_tm1_true_shell_direct" +_TRUE_SHELL_ARGUMENT_RE = re.compile(r"\bshell\s*=\s*true", re.IGNORECASE) _SHELL_COMMAND_WORD_START_RE = re.compile(r"[rRdDeE$'\"`\\]") _SHELL_COMMAND_WORD_CHARS = 4096 @@ -190,11 +195,11 @@ # Match a literal True assigned to a local name shortly before it is passed as # shell= to a subprocess invocation. The bounded newline gap and the -# intervening-write guard keep this a local data-flow fact; Python scope +# AST binding-evidence guard keep this a local data-flow fact; Python scope # visibility is enforced separately in analyze() via _VARIABLE_SHELL_FLAG_RE. _VARIABLE_SHELL_FLAG_PATTERN = ( r"(?m)^\s*([A-Za-z_]\w*)\s*=\s*True\s*$\n" - r"(?:(?![^\n]*\b\1\s*=)[^\n]{0,240}\n){0,4}?[^\n]{0,240}" + r"(?:[^\n]{0,240}\n){0,4}?[^\n]{0,240}" r"(?:subprocess\.\w+|Popen)\s*\([^)]*\bshell\s*=\s*\1\b" ) _VARIABLE_SHELL_FLAG_RE = re.compile(_VARIABLE_SHELL_FLAG_PATTERN, re.IGNORECASE | re.MULTILINE) @@ -3246,167 +3251,916 @@ def _has_unsupported_brace_expansion(tokens: tuple[_ShellToken, ...]) -> bool: _SCOPE_NODE_TYPES = (ast.FunctionDef, ast.AsyncFunctionDef, ast.ClassDef, ast.Lambda) +_COMPREHENSION_SCOPE_TYPES = (ast.ListComp, ast.SetComp, ast.DictComp, ast.GeneratorExp) +_NATIVE_SHELL_RECEIVERS = frozenset({"subprocess", "Popen"}) -def _scope_chain(tree: ast.Module, target: ast.AST) -> tuple[ast.AST, ...] | None: - """Return the chain of enclosing scopes for *target*, outermost first. +def _paired_store_values(target: ast.AST, value: ast.AST) -> Iterator[tuple[ast.AST, ast.AST]]: + """Yield each store target with the RHS element it receives, when the shapes match.""" + pairs = [(target, value)] + while pairs: + destination, source = pairs.pop() + if ( + isinstance(destination, (ast.Tuple, ast.List)) + and isinstance(source, (ast.Tuple, ast.List)) + and len(destination.elts) == len(source.elts) + and not any(isinstance(item, ast.Starred) for item in (*destination.elts, *source.elts)) + ): + pairs.extend(zip(destination.elts, source.elts, strict=True)) + else: + yield destination, source - The module itself is the outermost scope. Returns None when *target* is - not part of *tree*. - """ - parents: dict[ast.AST, ast.AST] = {} - stack: list[ast.AST] = [tree] - found = False - while stack: - node = stack.pop() - if node is target: - found = True - break - for child in ast.iter_child_nodes(node): - parents[child] = node - stack.append(child) - if not found: - return None - chain: list[ast.AST] = [] - current: ast.AST | None = target - while current is not None: - if isinstance(current, _SCOPE_NODE_TYPES) or current is tree: - chain.append(current) - current = parents.get(current) - chain.reverse() - return tuple(chain) + +def _captures_native_receiver(value: ast.AST, receiver: str) -> bool: + """Return whether *value* reads the native *receiver* callable or module.""" + if isinstance(value, ast.Name): + return value.id == receiver + return ( + receiver == "Popen" + and isinstance(value, ast.Attribute) + and value.attr == "Popen" + and isinstance(value.value, ast.Name) + and value.value.id == "subprocess" + ) + + +def _native_receiver_aliases(tree: ast.Module) -> dict[str, str]: + """Map each name whose every binding captures one native receiver to it.""" + paired: dict[ast.AST, ast.AST] = {} + for node in ast.walk(tree): + if isinstance(node, ast.Assign): + for target in node.targets: + paired.update(_paired_store_values(target, node.value)) + elif isinstance(node, (ast.AnnAssign, ast.NamedExpr)) and node.value is not None: + paired[node.target] = node.value + captures: dict[str, set[str | None]] = {} + for node in ast.walk(tree): + if isinstance(node, (ast.Import, ast.ImportFrom)): + for alias in node.names: + if alias.name == "*": + continue + native = ( + "subprocess" + if isinstance(node, ast.Import) and alias.name == "subprocess" + else "Popen" + if isinstance(node, ast.ImportFrom) + and node.level == 0 + and node.module == "subprocess" + and alias.name == "Popen" + else None + ) + bound = alias.asname or alias.name.split(".", 1)[0] + captures.setdefault(bound, set()).add(native) + continue + name: str | None = None + native = None + if isinstance(node, ast.Name) and isinstance(node.ctx, (ast.Store, ast.Del)): + name = node.id + value = paired.get(node) if isinstance(node.ctx, ast.Store) else None + native = next( + ( + receiver + for receiver in sorted(_NATIVE_SHELL_RECEIVERS) + if value is not None and _captures_native_receiver(value, receiver) + ), + None, + ) + elif isinstance(node, ast.arg): + name = node.arg + elif isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef, ast.ClassDef)): + name = node.name + elif isinstance(node, (ast.ExceptHandler, ast.MatchAs, ast.MatchStar)): + name = node.name + elif isinstance(node, ast.MatchMapping): + name = node.rest + if name is not None: + captures.setdefault(name, set()).add(native) + return { + name: receiver + for name, values in captures.items() + if len(values) == 1 and (receiver := next(iter(values))) is not None + } -def _scope_binds_name(scope_node: ast.AST, name: str) -> bool: - """Return whether *name* is bound directly in *scope_node*. +def _store_keeps_shell_signal(name: str, value: ast.AST, aliases: dict[str, str]) -> bool | None: + """Classify one simple-name store for lexical TM1 reconciliation. - Nested function/class/lambda bodies are not descended into: their bindings - belong to those scopes, not this one. + ``None`` means an identity store that leaves the binding unchanged. ``True`` + means the store keeps the native receiver or a truthy shell flag. ``False`` + means the store may replace the binding. """ - if isinstance(scope_node, (ast.FunctionDef, ast.AsyncFunctionDef, ast.Lambda)): - args = scope_node.args - named = [arg.arg for arg in (*args.posonlyargs, *args.args, *args.kwonlyargs)] - if args.vararg is not None: - named.append(args.vararg.arg) - if args.kwarg is not None: - named.append(args.kwarg.arg) - if name in named: - return True - bodies: list[ast.AST] = ( - [scope_node.body] if isinstance(scope_node, ast.Lambda) else list(scope_node.body) + if isinstance(value, ast.Name) and value.id == name: + return None + if name in _NATIVE_SHELL_RECEIVERS: + return _captures_native_receiver(value, name) or ( + isinstance(value, ast.Name) and aliases.get(value.id) == name ) - elif isinstance(scope_node, (ast.ClassDef, ast.Module)): - bodies = list(scope_node.body) - else: + if name.casefold() in {"subprocess", "popen"}: return False - stack = list(bodies) - while stack: - node = stack.pop() - if isinstance(node, _SCOPE_NODE_TYPES): - continue - if isinstance(node, ast.Name) and isinstance(node.ctx, ast.Store) and node.id == name: - return True - stack.extend(ast.iter_child_nodes(node)) - return False + return isinstance(value, ast.Constant) and bool(value.value) -def _direct_global_nonlocal(scope_node: ast.AST, name: str) -> str | None: - """Return 'global'/'nonlocal' when *scope_node* declares *name* as such. +@dataclass(frozen=True) +class _ScopedShellNode: + """One relevant AST node plus its source and lexical-scope identity.""" - Only declarations directly in the scope are considered; nested scopes are - not descended into. - """ - if isinstance(scope_node, ast.Lambda): + node: ast.Assign | ast.Call + scope_chain: tuple[ast.AST, ...] + start: int + + +@dataclass(frozen=True) +class _VariableShellAstIndex: + """Linear-time index used to reconcile every bounded regex candidate.""" + + tree: ast.Module + line_character_starts: tuple[int, ...] + assignments: dict[tuple[int, str], tuple[_ScopedShellNode, ...]] + calls: dict[str, tuple[_ScopedShellNode, ...]] + call_starts: dict[str, tuple[int, ...]] + bindings: dict[ast.AST, frozenset[str]] + class_binding_starts: dict[ast.ClassDef, dict[str, tuple[int, ...]]] + declarations: dict[ast.AST, dict[str, str]] + binding_events: dict[tuple[ast.AST, ast.AST], dict[str, tuple[tuple[int, bool], ...]]] + eager_effect_starts: dict[ast.AST, tuple[int, ...]] + reference_starts: dict[str, tuple[int, ...]] + + +@dataclass(frozen=True) +class _VariableShellCandidate: + """One raw variable-shell regex candidate resolved against the shared AST.""" + + name: str + call: ast.Call + same_scope: bool + visible: bool + assignment_start: int + assignment_scope: ast.AST + call_scope_chain: tuple[ast.AST, ...] + + +def _resolved_name_scope( + index: _VariableShellAstIndex, + use_chain: tuple[ast.AST, ...], + name: str, + use_start: int, +) -> ast.AST: + """Return the Python scope that resolves *name* at one use site.""" + crossed_function = False + nonlocal_lookup = False + seen_class_scope = False + for scope in reversed(use_chain): + if scope is index.tree: + return index.tree + declaration = index.declarations.get(scope, {}).get(name) + if declaration == "global": + return index.tree + if declaration == "nonlocal": + nonlocal_lookup = True + crossed_function = True + continue + if isinstance(scope, (ast.FunctionDef, ast.AsyncFunctionDef, ast.Lambda)): + if name in index.bindings.get(scope, frozenset()): + return scope + crossed_function = True + continue + if isinstance(scope, _COMPREHENSION_SCOPE_TYPES): + if name in index.bindings.get(scope, frozenset()): + return scope + # Comprehensions use an implicit function scope. Free names skip a + # surrounding class namespace just like names in a method body. + crossed_function = True + continue + if isinstance(scope, ast.ClassDef): + # A method does not close over its class namespace. A call evaluated + # directly in the class body sees assignments already executed, but + # a later class assignment does not create a compile-time local. + starts = index.class_binding_starts.get(scope, {}).get(name, ()) + if ( + not crossed_function + and not nonlocal_lookup + and not seen_class_scope + and bisect_left(starts, use_start) + ): + return scope + seen_class_scope = True + return index.tree + + +def _node_character_span(parsed: ParsedPythonFile, node: ast.AST) -> tuple[int, int] | None: + """Return one AST node's absolute character span.""" + line = getattr(node, "lineno", None) + end_line = getattr(node, "end_lineno", None) + byte_column = getattr(node, "col_offset", None) + end_byte_column = getattr(node, "end_col_offset", None) + if not all(isinstance(value, int) for value in (line, end_line, byte_column, end_byte_column)): return None - bodies: list[ast.stmt] | None = None - if isinstance(scope_node, (ast.FunctionDef, ast.AsyncFunctionDef, ast.ClassDef, ast.Module)): - bodies = scope_node.body - if bodies is None: + assert isinstance(line, int) + assert isinstance(end_line, int) + assert isinstance(byte_column, int) + assert isinstance(end_byte_column, int) + start_column = parsed.character_column(line, byte_column) + end_column = parsed.character_column(end_line, end_byte_column) + if start_column is None or end_column is None: return None - stack: list[ast.AST] = list(bodies) - while stack: - node = stack.pop() - if isinstance(node, _SCOPE_NODE_TYPES): - continue - if isinstance(node, ast.Global) and name in node.names: - return "global" - if isinstance(node, ast.Nonlocal) and name in node.names: - return "nonlocal" - stack.extend(ast.iter_child_nodes(node)) - return None + start = parsed.line_character_starts[line - 1] + start_column + end = parsed.line_character_starts[end_line - 1] + end_column + return start, end -def _variable_shell_flag_same_scope(content: str, file_path: str, match: re.Match[str]) -> bool: - """Return whether a variable-shell-flag match is a same-scope data flow. +def _parameter_names(arguments: ast.arguments) -> tuple[str, ...]: + names = [ + argument.arg + for argument in (*arguments.posonlyargs, *arguments.args, *arguments.kwonlyargs) + ] + if arguments.vararg is not None: + names.append(arguments.vararg.arg) + if arguments.kwarg is not None: + names.append(arguments.kwarg.arg) + return tuple(names) + + +def _direct_shell_name(call: ast.Call) -> str | None: + function = call.func + direct = isinstance(function, ast.Name) and function.id == "Popen" + direct = direct or ( + isinstance(function, ast.Attribute) + and isinstance(function.value, ast.Name) + and function.value.id == "subprocess" + ) + if not direct: + return None + for keyword in call.keywords: + if keyword.arg == "shell" and isinstance(keyword.value, ast.Name): + return keyword.value.id + return None - The regex cannot see Python scopes, so ``use_shell = True`` in one - function followed by ``shell=use_shell`` in another still matches. Resolve - the matched assignment and the ``shell=`` use through the Python AST and - require the assignment to be visible from the use: identical scope chains, - a closure read from an enclosing scope, or a matching global/nonlocal - declaration. Unparseable content keeps the candidate so a syntax error - cannot silence the signal. - """ - var_name = match.group(1) - assign_line = content.count("\n", 0, match.start()) + 1 - use_line = content.count("\n", 0, match.end()) + 1 - tree = parse_python_source(content, file_path).tree + +def _build_variable_shell_ast_index(parsed: ParsedPythonFile) -> _VariableShellAstIndex | None: + """Index relevant nodes, bindings, and declarations in one AST traversal.""" + tree = parsed.tree if tree is None: - return True - assign_node: ast.Assign | None = None - call_node: ast.Call | None = None - for node in ast.walk(tree): + return None + assignments: dict[tuple[int, str], list[_ScopedShellNode]] = {} + calls: dict[str, list[_ScopedShellNode]] = {} + bindings: dict[ast.AST, set[str]] = {tree: set()} + class_binding_starts: dict[ast.ClassDef, dict[str, list[int]]] = {} + declarations: dict[ast.AST, dict[str, str]] = {} + binding_events: dict[ast.AST, dict[str, list[tuple[int, bool]]]] = {} + eager_effect_starts: dict[ast.AST, list[int]] = {} + reference_starts: dict[str, list[int]] = {} + parents = {child: parent for parent in ast.walk(tree) for child in ast.iter_child_nodes(parent)} + attribute_identity_stores: set[ast.Attribute] = set() + # Simple-name stores that keep the current binding, including unpacked + # self-stores. They neither replace nor restore a binding. + identity_name_stores: set[ast.Name] = set() + native_aliases = _native_receiver_aliases(tree) + + def mark_attribute_identity_stores(target: ast.AST, value: ast.AST) -> None: + for destination, source in _paired_store_values(target, value): + if ( + isinstance(destination, ast.Attribute) + and isinstance(destination.value, ast.Name) + and isinstance(source, ast.Attribute) + and isinstance(source.value, ast.Name) + and destination.attr == source.attr + and destination.value.id == source.value.id + ): + attribute_identity_stores.add(destination) + + def record_name_store_values(scope: ast.AST, target: ast.AST, value: ast.AST, end: int) -> None: + # Pair unpacked targets with their RHS elements so a native alias, a + # truthy flag, or a self-store is not mistaken for a replacement. + for destination, source in _paired_store_values(target, value): + if not isinstance(destination, ast.Name): + continue + keeps_signal = _store_keeps_shell_signal(destination.id, source, native_aliases) + if keeps_signal is None: + identity_name_stores.add(destination) + elif keeps_signal: + record_binding(scope, destination.id, end, True) + + def record_binding(scope: ast.AST, name: str, start: int, restores: bool = False) -> None: + # A conditional store is not affirmative replacement evidence. Only a + # literal branch whose execution is proven can revoke the lexical owner. + child = node + if child in identity_name_stores: + return + parent = parents.get(child) + if isinstance(parent, ast.AnnAssign) and parent.value is None and child is parent.target: + return + while parent is not None and parent is not scope: + if isinstance(parent, ast.If): + if not isinstance(parent.test, ast.Constant): + return + taken = parent.body if bool(parent.test.value) else parent.orelse + if child not in taken: + return + elif isinstance(parent, ast.BoolOp): + # Later operands run only after every earlier operand lets the + # short circuit continue. Only constant operands prove that. + continues = isinstance(parent.op, ast.And) + for operand in parent.values: + if operand is child: + break + if ( + not isinstance(operand, ast.Constant) + or bool(operand.value) is not continues + ): + return + elif isinstance(parent, ast.IfExp) and child is not parent.test: + if not isinstance(parent.test, ast.Constant): + return + if child is not (parent.body if bool(parent.test.value) else parent.orelse): + return + elif isinstance( + parent, + ( + ast.For, + ast.AsyncFor, + ast.While, + ast.With, + ast.AsyncWith, + ast.Try, + ast.TryStar, + ast.Match, + ast.comprehension, + *_COMPREHENSION_SCOPE_TYPES, + ), + ): + return + child, parent = parent, parents.get(parent) + binding_events.setdefault(scope, {}).setdefault(name, []).append((start, restores)) + + def bind(scope: ast.AST, name: str, start: int) -> None: + bindings.setdefault(scope, set()).add(name) + record_binding(scope, name, start) + if isinstance(scope, ast.ClassDef): + class_binding_starts.setdefault(scope, {}).setdefault(name, []).append(start) + + stack: list[tuple[ast.AST, tuple[ast.AST, ...], int | None]] = [(tree, (tree,), None)] + while stack: + node, scope_chain, binding_start = stack.pop() + scope = scope_chain[-1] + span = _node_character_span(parsed, node) + start = span[0] if span is not None else 0 + end = span[1] if span is not None else start + # A direct observed slot write is useful only before another eager + # effect. Cached-module lifetime across imports belongs to the companion. + if isinstance( + node, + ( + ast.Call, + ast.Attribute, + ast.Subscript, + ast.BinOp, + ast.BoolOp, + ast.Compare, + ast.UnaryOp, + ast.NamedExpr, + ast.JoinedStr, + ast.Import, + ast.ImportFrom, + ast.Delete, + ast.AugAssign, + ast.If, + ast.For, + ast.AsyncFor, + ast.While, + ast.With, + ast.AsyncWith, + ast.Try, + ast.TryStar, + ast.Match, + ast.FunctionDef, + ast.AsyncFunctionDef, + ast.ClassDef, + ast.Await, + ast.Yield, + ast.YieldFrom, + *_COMPREHENSION_SCOPE_TYPES, + ), + ): + execution = next( + ( + item + for item in reversed(scope_chain) + if isinstance(item, (ast.FunctionDef, ast.AsyncFunctionDef, ast.Lambda)) + ), + tree, + ) + eager_effect_starts.setdefault(execution, []).append(start) + # Any later reference may invoke a deferred body, so keep every load. + if isinstance(node, ast.Name) and isinstance(node.ctx, ast.Load): + reference_starts.setdefault(node.id, []).append(start) + elif isinstance(node, ast.Attribute) and isinstance(node.ctx, ast.Load): + reference_starts.setdefault(node.attr, []).append(start) + + if isinstance(node, ast.Assign): + for target in node.targets: + mark_attribute_identity_stores(target, node.value) + elif isinstance(node, ast.AnnAssign) and node.value is not None: + mark_attribute_identity_stores(node.target, node.value) + if isinstance(node, ast.Assign): + # The RHS is captured before any target is assigned. A final + # same-slot store restores that snapshot after earlier writes in + # this statement, including chained and repeated unpacking targets. + stores: dict[tuple[str, str], list[ast.Attribute]] = {} + targets = list(reversed(node.targets)) + while targets: + target = targets.pop() + if isinstance(target, ast.Attribute) and isinstance(target.value, ast.Name): + stores.setdefault((target.value.id, target.attr), []).append(target) + elif isinstance(target, (ast.Tuple, ast.List)): + targets.extend(reversed(target.elts)) + for slot_stores in stores.values(): + if slot_stores[-1] in attribute_identity_stores: + attribute_identity_stores.update(slot_stores) + + if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef)): + bind(scope, node.name, end) + nested_chain = (*scope_chain, node) + bindings.setdefault(node, set()).update(_parameter_names(node.args)) + header_nodes: list[ast.AST] = [*node.decorator_list, node.args] + if node.returns is not None: + header_nodes.append(node.returns) + header_nodes.extend(getattr(node, "type_params", [])) + stack.extend((child, scope_chain, None) for child in header_nodes) + stack.extend((child, nested_chain, None) for child in node.body) + continue + if isinstance(node, ast.Lambda): + nested_chain = (*scope_chain, node) + bindings.setdefault(node, set()).update(_parameter_names(node.args)) + stack.append((node.args, scope_chain, None)) + stack.append((node.body, nested_chain, None)) + continue + if isinstance(node, ast.ClassDef): + bind(scope, node.name, end) + nested_chain = (*scope_chain, node) + bindings.setdefault(node, set()) + header_nodes = [*node.decorator_list, *node.bases] + header_nodes.extend(keyword.value for keyword in node.keywords) + header_nodes.extend(getattr(node, "type_params", [])) + stack.extend((child, scope_chain, None) for child in header_nodes) + stack.extend((child, nested_chain, None) for child in node.body) + continue + if isinstance(node, _COMPREHENSION_SCOPE_TYPES): + nested_chain = (*scope_chain, node) + bindings.setdefault(node, set()) + generators = node.generators + if generators: + first, *remaining = generators + # The leftmost iterable is the only comprehension expression + # evaluated in the enclosing scope. + stack.append((first.iter, scope_chain, None)) + stack.append((first.target, nested_chain, None)) + stack.extend((condition, nested_chain, None) for condition in first.ifs) + for generator in remaining: + stack.append((generator.iter, nested_chain, None)) + stack.append((generator.target, nested_chain, None)) + stack.extend((condition, nested_chain, None) for condition in generator.ifs) + if isinstance(node, ast.DictComp): + stack.append((node.key, nested_chain, None)) + stack.append((node.value, nested_chain, None)) + else: + stack.append((node.elt, nested_chain, None)) + continue + if ( - assign_node is None - and isinstance(node, ast.Assign) - and node.lineno == assign_line + isinstance(node, ast.Attribute) + and isinstance(node.ctx, ast.Store) + and isinstance(node.value, ast.Name) + ): + assignment = parents.get(node) + if ( + node not in attribute_identity_stores + and isinstance(assignment, ast.Assign) + and len(assignment.targets) == 1 + and assignment.targets[0] is node + and tree.body + and assignment is tree.body[0] + and not any( + isinstance(value, ast.Name) + and isinstance(value.ctx, ast.Load) + and value.id in {node.value.id, "Popen"} + for value in ast.walk(assignment.value) + ) + ): + # Retain the legacy first-statement direct shadow control only. + # An RHS using the receiver or Popen may preserve a native callable. + # Prior stores/protocol effects need the companion's explicit proof. + record_binding( + scope, + node.value.id + "." + node.attr, + binding_start if binding_start is not None else end, + ) + + if isinstance(node, ast.Global): + for name in node.names: + declarations.setdefault(scope, {})[name] = "global" + elif isinstance(node, ast.Nonlocal): + for name in node.names: + declarations.setdefault(scope, {})[name] = "nonlocal" + elif isinstance(node, ast.Name) and isinstance(node.ctx, (ast.Store, ast.Del)): + bind(scope, node.id, binding_start if binding_start is not None else start) + elif isinstance(node, (ast.Import, ast.ImportFrom)): + for alias in node.names: + if alias.name != "*": + bound_name = alias.asname or alias.name.split(".", 1)[0] + bind(scope, bound_name, end) + restores = ( + isinstance(node, ast.Import) + and alias.name == "subprocess" + and bound_name == "subprocess" + ) or ( + isinstance(node, ast.ImportFrom) + and node.level == 0 + and node.module == "subprocess" + and alias.name == "Popen" + and bound_name == "Popen" + ) + if restores: + record_binding(scope, bound_name, end, True) + elif isinstance(node, ast.ExceptHandler) and node.name: + type_span = _node_character_span(parsed, node.type) if node.type is not None else None + bind(scope, node.name, type_span[1] if type_span is not None else start) + elif isinstance(node, (ast.MatchAs, ast.MatchStar)) and node.name: + bind(scope, node.name, start) + elif isinstance(node, ast.MatchMapping) and node.rest: + bind(scope, node.rest, start) + + if ( + isinstance(node, ast.Assign) + and span is not None and isinstance(node.value, ast.Constant) and node.value.value is True - and any( - isinstance(target, ast.Name) and target.id == var_name for target in node.targets - ) ): - assign_node = node - if ( - call_node is None - and isinstance(node, ast.Call) - and any( - keyword.arg == "shell" - and isinstance(keyword.value, ast.Name) - and keyword.value.id == var_name - and assign_line <= keyword.value.lineno <= use_line - for keyword in node.keywords + for target in node.targets: + if isinstance(target, ast.Name): + scoped = _ScopedShellNode(node=node, scope_chain=scope_chain, start=start) + assignments.setdefault((node.lineno, target.id), []).append(scoped) + elif isinstance(node, ast.Call) and span is not None: + shell_name = _direct_shell_name(node) + if shell_name is not None: + scoped = _ScopedShellNode(node=node, scope_chain=scope_chain, start=start) + calls.setdefault(shell_name, []).append(scoped) + + if isinstance(node, ast.Assign): + stack.append((node.value, scope_chain, None)) + stack.extend((target, scope_chain, end) for target in node.targets) + for target in node.targets: + record_name_store_values(scope, target, node.value, end) + elif isinstance(node, (ast.AnnAssign, ast.AugAssign)): + if node.value is not None: + stack.append((node.value, scope_chain, None)) + stack.append((node.target, scope_chain, end)) + if isinstance(node, ast.AnnAssign): + stack.append((node.annotation, scope_chain, None)) + if node.value is not None: + record_name_store_values(scope, node.target, node.value, end) + elif isinstance(node, ast.NamedExpr): + stack.append((node.value, scope_chain, None)) + # PEP 572 makes a walrus target inside one or more + # comprehensions local to the nearest containing real scope. + # Comprehension ``for`` targets remain local to their implicit + # scope, but the named-expression target skips those scopes. + target_chain = scope_chain + while len(target_chain) > 1 and isinstance( + target_chain[-1], _COMPREHENSION_SCOPE_TYPES + ): + target_chain = target_chain[:-1] + stack.append((node.target, target_chain, end)) + record_name_store_values(target_chain[-1], node.target, node.value, end) + elif isinstance(node, (ast.For, ast.AsyncFor)): + iteration_span = _node_character_span(parsed, node.iter) + iteration_end = iteration_span[1] if iteration_span is not None else end + stack.append((node.iter, scope_chain, None)) + stack.append((node.target, scope_chain, iteration_end)) + stack.extend((child, scope_chain, None) for child in (*node.body, *node.orelse)) + elif isinstance(node, ast.withitem): + context_span = _node_character_span(parsed, node.context_expr) + context_end = context_span[1] if context_span is not None else end + stack.append((node.context_expr, scope_chain, None)) + if node.optional_vars is not None: + stack.append((node.optional_vars, scope_chain, context_end)) + else: + stack.extend( + (child, scope_chain, binding_start) for child in ast.iter_child_nodes(node) ) + + def execution_scope(scope: ast.AST) -> ast.AST: + current: ast.AST | None = scope + while current is not None: + if isinstance(current, (ast.FunctionDef, ast.AsyncFunctionDef, ast.Lambda)): + return current + current = parents.get(current) + return tree + + # Keep the execution scope of every store. A global/nonlocal declaration + # changes the binding namespace; it never proves a deferred body was run. + scoped_events: dict[tuple[ast.AST, ast.AST], dict[str, list[tuple[int, bool]]]] = {} + sorted_class_starts = { + scope: {name: tuple(sorted(starts)) for name, starts in names.items()} + for scope, names in class_binding_starts.items() + } + for scope, names in binding_events.items(): + for name, events in names.items(): + receiver_name = name.partition(".")[0] + declaration = declarations.get(scope, {}).get(receiver_name) + for event in events: + target_scope = scope + if "." in name and declaration is None: + # A class body can change where its receiver resolves + # between stores. Resolve each slot store at its own offset. + current: ast.AST | None = scope + crossed_function = False + while current is not None: + if current is tree: + target_scope = tree + break + if isinstance(current, (ast.FunctionDef, ast.AsyncFunctionDef, ast.Lambda)): + if receiver_name in bindings.get(current, set()): + target_scope = current + break + crossed_function = True + elif isinstance(current, ast.ClassDef) and not crossed_function: + starts = sorted_class_starts.get(current, {}).get(receiver_name, ()) + if bisect_right(starts, event[0]): + target_scope = current + break + current = parents.get(current) + if declaration == "global": + target_scope = tree + elif declaration == "nonlocal": + outer = parents.get(scope) + while outer is not None: + if isinstance( + outer, (ast.FunctionDef, ast.AsyncFunctionDef, ast.Lambda) + ) and receiver_name in bindings.get(outer, set()): + target_scope = outer + break + outer = parents.get(outer) + key = (target_scope, execution_scope(scope)) + scoped_events.setdefault(key, {}).setdefault(name, []).append(event) + + frozen_calls = { + name: tuple(sorted(nodes, key=lambda item: item.start)) for name, nodes in calls.items() + } + return _VariableShellAstIndex( + tree=tree, + line_character_starts=parsed.line_character_starts, + assignments={key: tuple(nodes) for key, nodes in assignments.items()}, + calls=frozen_calls, + call_starts={ + name: tuple(item.start for item in nodes) for name, nodes in frozen_calls.items() + }, + bindings={scope: frozenset(names) for scope, names in bindings.items()}, + class_binding_starts=sorted_class_starts, + declarations=declarations, + eager_effect_starts={ + scope: tuple(sorted(set(starts))) for scope, starts in eager_effect_starts.items() + }, + reference_starts={name: tuple(sorted(starts)) for name, starts in reference_starts.items()}, + binding_events={ + scope: {name: tuple(sorted(set(events))) for name, events in names.items()} + for scope, names in scoped_events.items() + }, + ) + + +def _assignment_name_scope( + index: _VariableShellAstIndex, + scope_chain: tuple[ast.AST, ...], + name: str, +) -> ast.AST: + """Return the scope mutated by a simple-name assignment.""" + scope = scope_chain[-1] + declaration = index.declarations.get(scope, {}).get(name) + if declaration == "global": + return index.tree + if declaration == "nonlocal": + for outer in reversed(scope_chain[:-1]): + if isinstance(outer, (ast.FunctionDef, ast.AsyncFunctionDef, ast.Lambda)) and name in ( + index.bindings.get(outer, frozenset()) + ): + return outer + return scope + + +def _resolve_variable_shell_candidate( + index: _VariableShellAstIndex, + match: re.Match[str], + assignment_line: int, +) -> _VariableShellCandidate | None: + """Resolve a bounded regex match to its exact assignment, call, and binding.""" + name = match.group(1) + assignment_nodes = [ + item + for item in index.assignments.get((assignment_line, name), ()) + if match.start() <= item.start < match.end() + ] + call_nodes = index.calls.get(name, ()) + starts = index.call_starts.get(name, ()) + call_index = bisect_left(starts, match.start()) + if len(assignment_nodes) != 1 or call_index >= len(call_nodes): + return None + assignment = assignment_nodes[0] + call = call_nodes[call_index] + if call.start >= match.end(): + return None + assignment_scope = _assignment_name_scope(index, assignment.scope_chain, name) + resolved_scope = _resolved_name_scope(index, call.scope_chain, name, call.start) + return _VariableShellCandidate( + name=name, + call=call.node, + same_scope=assignment.scope_chain == call.scope_chain, + visible=resolved_scope is assignment_scope, + assignment_start=assignment.start, + assignment_scope=assignment_scope, + call_scope_chain=call.scope_chain, + ) + + +def _earliest_deferred_invocation( + index: _VariableShellAstIndex, + parsed: ParsedPythonFile, + call_scope_chain: tuple[ast.AST, ...], + owner_execution: ast.AST, +) -> float: + """Return the earliest owner-scope offset at which a deferred call may run. + + A function, method, or lambda can run once it exists and something refers to + it: a call in any statement (including ``if``/``try``/main guards), a callback + registration, a decorator, or another body. Without a visible reference it + runs only after the owner scope finishes. + """ + owner_position = next( + (position for position, scope in enumerate(call_scope_chain) if scope is owner_execution), + None, + ) + if owner_position is None or owner_position + 1 >= len(call_scope_chain): + return 0 + entries = call_scope_chain[owner_position + 1 :] + span = _node_character_span(parsed, entries[0]) + if span is None: + return 0 + defined = span[1] + if any( + isinstance(entry, (ast.Lambda, ast.GeneratorExp)) or getattr(entry, "decorator_list", None) + for entry in entries + ): + return defined + names = { + entry.name + for entry in entries + if isinstance(entry, (ast.FunctionDef, ast.AsyncFunctionDef, ast.ClassDef)) + } + references = [ + start + for name in names + for start in index.reference_starts.get(name, ()) + if not span[0] <= start < span[1] + ] + if not references: + return float("inf") + return max(defined, min(references)) + + +def _deferred_receiver_is_replaced( + index: _VariableShellAstIndex, + parsed: ParsedPythonFile, + candidate: _VariableShellCandidate, + receiver: str, + receiver_scope: ast.AST, + receiver_execution: ast.AST, + current_execution: ast.AST, + call_start: int, +) -> bool: + """Return whether every possible invocation of a deferred call sees a replacement.""" + local_events = index.binding_events.get((receiver_scope, current_execution), {}).get( + receiver, () + ) + local_index = bisect_right(local_events, (call_start, True)) + if local_index: + # A store earlier in the same invocation decides the receiver. + return not local_events[local_index - 1][1] + outer_events = index.binding_events.get((receiver_scope, receiver_execution), {}).get( + receiver, () + ) + first_invocation = _earliest_deferred_invocation( + index, parsed, candidate.call_scope_chain, receiver_execution + ) + keeps_receiver: dict[int, bool] = {} + for start, restores in outer_events: + keeps_receiver[start] = keeps_receiver.get(start, False) or restores + starts = sorted(keeps_receiver) + earlier = bisect_left(starts, first_invocation) + if not earlier: + return False + # The binding seen by the first possible invocation, then every later one. + return not any(keeps_receiver[start] for start in starts[earlier - 1 :]) + + +def _lexical_shell_has_counterevidence( + index: _VariableShellAstIndex, + parsed: ParsedPythonFile, + candidate: _VariableShellCandidate, + *, + receiver_trusted: bool | None, +) -> bool: + """Reject a lexical candidate only for an observed binding replacement. + + The companion's unknown receiver trust is an abstention. Generic calls, + protocol effects, and unsupported statements do not prove a replacement. + Stores/imports do; use their source order and Python binding scopes. + """ + call_span = _node_character_span(parsed, candidate.call) + if call_span is None: + return False + call_start = call_span[0] + shell = next((item.value for item in candidate.call.keywords if item.arg == "shell"), None) + shell_span = _node_character_span(parsed, shell) if shell is not None else None + shell_start = shell_span[0] if shell_span is not None else call_start + current_execution = next( + ( + scope + for scope in reversed(candidate.call_scope_chain) + if isinstance(scope, (ast.FunctionDef, ast.AsyncFunctionDef, ast.Lambda)) + ), + index.tree, + ) + + def binding_events(scope: ast.AST, name: str) -> tuple[tuple[int, bool], ...]: + owner_execution = ( + scope + if isinstance(scope, (ast.FunctionDef, ast.AsyncFunctionDef, ast.Lambda)) + else index.tree + ) + events = index.binding_events.get((scope, owner_execution), {}).get(name, ()) + if current_execution is not owner_execution: + local_events = index.binding_events.get((scope, current_execution), {}).get(name, ()) + return tuple(sorted((*events, *local_events))) + return events + + flag_events = binding_events(candidate.assignment_scope, candidate.name) + flag_index = bisect_right(flag_events, (shell_start, True)) + if flag_index: + last_flag = flag_events[flag_index - 1] + if last_flag[0] > candidate.assignment_start and not last_flag[1]: + return True + if receiver_trusted is True: + return False + function = candidate.call.func + receiver = function.id if isinstance(function, ast.Name) else function.value.id + receiver_scope = _resolved_name_scope(index, candidate.call_scope_chain, receiver, call_start) + receiver_execution = ( + receiver_scope + if isinstance(receiver_scope, (ast.FunctionDef, ast.AsyncFunctionDef, ast.Lambda)) + else index.tree + ) + # Called-module slot replacement is proved only by the companion cache + # state, which discards stale proof after an unknown eager effect. + receiver_events = binding_events(receiver_scope, receiver) + receiver_index = bisect_right(receiver_events, (call_start, True)) + if receiver_execution is not current_execution: + # Source order does not order a deferred body against outer stores. + # Require a replacement at every invocation the outer scope can reach. + if _deferred_receiver_is_replaced( + index, + parsed, + candidate, + receiver, + receiver_scope, + receiver_execution, + current_execution, + call_start, ): - call_node = node - if assign_node is None or call_node is None: - return True - assign_chain = _scope_chain(tree, assign_node) - use_chain = _scope_chain(tree, call_node) - if assign_chain is None or use_chain is None: - return True - if assign_chain == use_chain: - return True - if len(assign_chain) < len(use_chain) and use_chain[: len(assign_chain)] == assign_chain: - # Closure read: the use sits in a scope nested inside the assignment's - # scope, so the name resolves to the assigned value. - return True - use_scope = use_chain[-1] - if use_scope is not tree: - declaration = _direct_global_nonlocal(use_scope, var_name) - if declaration == "global": - return assign_chain == (tree,) - if declaration == "nonlocal": - binding = next( - ( - scope - for scope in use_chain[-2::-1] - if isinstance(scope, (ast.FunctionDef, ast.AsyncFunctionDef, ast.Lambda)) - and _scope_binds_name(scope, var_name) - ), - None, - ) - return binding is not None and assign_chain == _scope_chain(tree, binding) + return True + elif receiver_index: + if not receiver_events[receiver_index - 1][1]: + return True + elif isinstance(receiver_scope, (ast.FunctionDef, ast.AsyncFunctionDef, ast.Lambda)): + # Compile-time local bindings shadow a global receiver even before store. + if receiver in index.bindings.get(receiver_scope, frozenset()): + return True + if isinstance(function, ast.Attribute): + # Preserve a direct same-name replacement only in this execution scope, + # after the completed store and before another effect or receiver binding. + method_events = index.binding_events.get((receiver_scope, current_execution), {}).get( + receiver + "." + function.attr, () + ) + method_index = bisect_right(method_events, (call_start, True)) + if method_index: + method_start, restores = method_events[method_index - 1] + effects = index.eager_effect_starts.get(current_execution, ()) + effect_index = bisect_right(effects, method_start) + later_binding = receiver_index and receiver_events[receiver_index - 1][0] > method_start + if ( + not restores + and not later_binding + and (effect_index == len(effects) or effects[effect_index] >= call_start) + ): + return True return False @@ -3936,7 +4690,33 @@ def _line_containing(content: str, start: int, end: int) -> str: return content[line_start:line_end] -def analyze(content: str, file_path: str, file_type: str) -> list[AnalyzerFinding]: +def _classify_tm1( + context: str, + matched_text: str, + matched_line: str, + confidence: float, + file_type: str, +) -> tuple[Severity, float]: + """Apply the existing TM1 contextual classification to one candidate.""" + if ( + _is_safe_container_command(context) + or _is_safe_dockerfile_idiom(context, matched_text) + or _is_safe_cache_cleanup(matched_line) + ): + return Severity.LOW, min(confidence, 0.15) + adjusted = ( + min(1.0, confidence + 0.1) if file_type in ("python", "shell", "javascript") else confidence + ) + return Severity.HIGH, adjusted + + +def analyze( + content: str, + file_path: str, + file_type: str, + *, + defer_variable_reconciliation: bool = False, +) -> list[AnalyzerFinding]: """Analyze content for tool misuse patterns (TM1–TM3).""" findings: list[AnalyzerFinding] = [] @@ -3947,16 +4727,27 @@ def ctx(start: int) -> str: return get_context(content, start) tag = [PatternCategory.TOOL_MISUSE.value] - tm1_findings_by_key: dict[tuple[int, str], AnalyzerFinding] = {} + tm1_findings_by_key: dict[tuple[int, str, int], AnalyzerFinding] = {} - # The variable-shell-flag regex cannot see Python scopes, so an assignment - # in one function and a shell= use in another still match. Drop those - # cross-scope candidates for Python files. - cross_scope_starts: set[int] = set() - if file_type == "python": - for variable_match in _VARIABLE_SHELL_FLAG_RE.finditer(content): - if not _variable_shell_flag_same_scope(content, file_path, variable_match): - cross_scope_starts.add(variable_match.start()) + variable_matches = { + (match.start(), match.end()): (match.group(1), match) + for match in _VARIABLE_SHELL_FLAG_RE.finditer(content) + } + invisible_variable_matches: set[tuple[int, int]] = set() + if file_type == "python" and variable_matches and not defer_variable_reconciliation: + parsed = parse_python_source(content, file_path) + ast_index = _build_variable_shell_ast_index(parsed) + if ast_index is not None: + for span, (_, match) in variable_matches.items(): + assignment_line = bisect_right(ast_index.line_character_starts, match.start(1)) + candidate = _resolve_variable_shell_candidate(ast_index, match, assignment_line) + if candidate is not None and ( + not candidate.visible + or _lexical_shell_has_counterevidence( + ast_index, parsed, candidate, receiver_trusted=None + ) + ): + invisible_variable_matches.add(span) shell_content = ( _perl_literal_print_shell_text(content, lambda: None) if file_type == "perl" else None @@ -3964,34 +4755,37 @@ def ctx(start: int) -> str: for match_start, match_end, matched_text, confidence in _tm1_candidates( content, shell_content=shell_content ): - if match_start in cross_scope_starts: + variable_match = variable_matches.get((match_start, match_end)) + if variable_match is not None and variable_match[0].casefold().startswith("true"): + # The case-insensitive direct ``shell=True`` pattern already owns + # true-prefixed names at the exact call location. + continue + if variable_match is not None and (match_start, match_end) in invisible_variable_matches: continue line_num = get_line_number(content, match_start) context_text = ctx(match_start) matched = matched_text[:200] matched_line = _line_containing(content, match_start, match_end) - if ( - _is_safe_container_command(context_text) - or _is_safe_dockerfile_idiom(context_text, matched) - or _is_safe_cache_cleanup(matched_line) - ): - adj = min(confidence, 0.15) - sev = Severity.LOW - else: - adj = ( - min(1.0, confidence + 0.1) - if file_type in ("python", "shell", "javascript") - else confidence - ) - sev = Severity.HIGH - candidate_key = (line_num, " ".join(matched.strip().split())) + sev, adj = _classify_tm1( + context_text, + matched, + matched_line, + confidence, + file_type, + ) + candidate_key = (line_num, " ".join(matched.strip().split()), match_start) existing = tm1_findings_by_key.get(candidate_key) if existing is not None: if adj > existing.confidence: existing.confidence = adj existing.severity = sev continue + evidence: dict[str, object] = {static_runner._VIEW_START_EVIDENCE: match_start} + if variable_match is not None and defer_variable_reconciliation: + evidence[_VARIABLE_SHELL_FLAG_EVIDENCE] = variable_match[0] + elif defer_variable_reconciliation and _TRUE_SHELL_ARGUMENT_RE.search(matched_text): + evidence[_TRUE_SHELL_DIRECT_EVIDENCE] = True finding = AnalyzerFinding( rule_id="TM1", message="Tool Parameter Abuse", @@ -4002,7 +4796,7 @@ def ctx(start: int) -> str: context=context_text, matched_text=matched, complete_match=matched_text, - evidence={static_runner._VIEW_START_EVIDENCE: match_start}, + evidence=evidence, ) tm1_findings_by_key[candidate_key] = finding findings.append(finding) @@ -4082,8 +4876,171 @@ def ctx(start: int) -> str: return findings +def _bound_shell_call_key(call: ast.Call) -> tuple[int, int, int, int]: + """Return the call key shared with the flow-sensitive companion.""" + return ( + getattr(call, "lineno", 1), + getattr(call, "col_offset", 0), + getattr(call, "end_lineno", getattr(call, "lineno", 1)), + getattr(call, "end_col_offset", getattr(call, "col_offset", 0)), + ) + + +def cleanup_path_findings(findings: list[Finding]) -> list[Finding]: + """Remove private reconciliation evidence when postprocessing times out.""" + for finding in findings: + finding.evidence.pop(_VARIABLE_SHELL_FLAG_EVIDENCE, None) + finding.evidence.pop(_TRUE_SHELL_DIRECT_EVIDENCE, None) + return findings + + +def postprocess_path_findings( + content: str, + findings: list[Finding], + *, + python_ast: ParsedPythonFile | None, +) -> list[Finding]: + """Reconcile the legacy variable regex with the Python AST companion.""" + marked = [ + finding + for finding in findings + if isinstance(finding.evidence.get(_VARIABLE_SHELL_FLAG_EVIDENCE), str) + or finding.evidence.get(_TRUE_SHELL_DIRECT_EVIDENCE) is True + ] + if not marked: + return findings + + file_path = marked[0].file + file_type = static_runner._infer_file_type(file_path) + if file_type != "python": + reconciled: list[Finding] = [] + for finding in findings: + variable_name = finding.evidence.pop(_VARIABLE_SHELL_FLAG_EVIDENCE, None) + finding.evidence.pop(_TRUE_SHELL_DIRECT_EVIDENCE, None) + if not isinstance(variable_name, str): + reconciled.append(finding) + return reconciled + if python_ast is None or python_ast.tree is None: + return cleanup_path_findings(findings) + + from . import static_python_shell_truthiness + + ast_index = _build_variable_shell_ast_index(python_ast) + if ast_index is None: + return cleanup_path_findings(findings) + resolved: dict[tuple[int, str], list[_VariableShellCandidate]] = {} + for match in _VARIABLE_SHELL_FLAG_RE.finditer(content): + finding_line = bisect_right(ast_index.line_character_starts, match.start()) + assignment_line = bisect_right(ast_index.line_character_starts, match.start(1)) + candidate = _resolve_variable_shell_candidate(ast_index, match, assignment_line) + if candidate is None: + continue + key = (finding_line, candidate.name) + resolved.setdefault(key, []).append(candidate) + ownership, emitted, cached_replacements = ( + static_python_shell_truthiness.bound_shell_call_analysis(file_path, python_ast) + ) + retained_companion_locations = { + (finding.start_line, finding.start_column) + for finding in findings + if finding.evidence.get(static_python_shell_truthiness.BOUND_SHELL_EVIDENCE) is True + } + emitted = { + key + for key in emitted + if (key[0], python_ast.character_column(key[0], key[1])) in retained_companion_locations + } + cached_replacements_by_start = { + (line, column) + for line, byte_column, _, _ in cached_replacements + if (column := python_ast.character_column(line, byte_column)) is not None + } + ownership_by_start = { + (line, column): trusted + for (line, byte_column, _, _), trusted in ownership.items() + if (column := python_ast.character_column(line, byte_column)) is not None + } + + candidates_by_location: dict[tuple[int, int | None], list[_VariableShellCandidate]] = {} + for candidates in resolved.values(): + for candidate in candidates: + location = ( + candidate.call.lineno, + python_ast.character_column(candidate.call.lineno, candidate.call.col_offset), + ) + candidates_by_location.setdefault(location, []).append(candidate) + + reconciled: list[Finding] = [] + for finding in findings: + variable_name = finding.evidence.pop(_VARIABLE_SHELL_FLAG_EVIDENCE, None) + direct_true = finding.evidence.pop(_TRUE_SHELL_DIRECT_EVIDENCE, None) + if direct_true is True: + location = (finding.start_line, finding.start_column) + if location in cached_replacements_by_start: + continue + if finding.start_column is not None and ownership_by_start.get(location) is False: + matching = candidates_by_location.get(location, []) + if len(matching) == 1 and _lexical_shell_has_counterevidence( + ast_index, python_ast, matching[0], receiver_trusted=False + ): + continue + reconciled.append(finding) + continue + if not isinstance(variable_name, str): + reconciled.append(finding) + continue + candidates = resolved.get((finding.start_line, variable_name), []) + if len(candidates) != 1: + # Derived views and ambiguous recovery candidates retain the + # conservative lexical signal. + reconciled.append(finding) + continue + candidate = candidates[0] + if variable_name.casefold().startswith("true") or not candidate.visible: + continue + call_key = _bound_shell_call_key(candidate.call) + if ( + call_key in cached_replacements + or call_key in emitted + or _lexical_shell_has_counterevidence( + ast_index, python_ast, candidate, receiver_trusted=ownership.get(call_key) + ) + ): + continue + reconciled.append(finding) + return reconciled + + def node(state: SkillspectorState) -> AnalyzerNodeResponse: """Run tool_misuse patterns and return findings.""" - response = static_runner.run_static_patterns_with_ledger(state, [sys.modules[__name__]]) + from . import static_python_shell_truthiness + + response = static_runner.run_static_patterns_with_ledger( + state, + [sys.modules[__name__], static_python_shell_truthiness], + ) + file_cache = state.get("file_cache", {}) + for finding in response["findings"]: + if ( + finding.evidence.pop( + static_python_shell_truthiness.BOUND_SHELL_EVIDENCE, + None, + ) + is not True + ): + continue + content = file_cache.get(finding.file, "") + content_lines = content.splitlines() + line_index = max(0, finding.start_line - 1) + matched = (finding.matched_text or "")[:200] + matched_line = content_lines[line_index] if line_index < len(content_lines) else matched + severity, finding.confidence = _classify_tm1( + finding.context or "", + matched, + matched_line, + finding.confidence, + "python", + ) + finding.severity = severity.value logger.info("%s: %d findings", ANALYZER_ID, len(response["findings"])) return response diff --git a/src/skillspector/nodes/analyzers/static_python_shell_truthiness.py b/src/skillspector/nodes/analyzers/static_python_shell_truthiness.py new file mode 100644 index 000000000..e857679e2 --- /dev/null +++ b/src/skillspector/nodes/analyzers/static_python_shell_truthiness.py @@ -0,0 +1,1967 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""Find direct subprocess calls using a definitely truthy local name. + +This companion recognizes the straight-line ordinary-Python form reported in +issue #475. Arguments evaluated through ``shell=`` must be passive, and +unsupported expressions or compound statements discard facts rather than +guessing about Python execution. +""" + +from __future__ import annotations + +import ast + +from skillspector.models import AnalyzerFinding, Location, Severity +from skillspector.python_ast import ParsedPythonFile, parse_python_source + +from .common import get_complete_source_segment, get_context_from_lines +from .pattern_defaults import PatternCategory + +ANALYZER_ID = "static_patterns_tool_misuse" +USES_PYTHON_AST = True +BOUND_SHELL_EVIDENCE = "_tm1_bound_shell_value" +_DIRECT_CALL_NAMES = frozenset({"subprocess", "Popen"}) +_CACHED_SUBPROCESS_API_SLOTS = frozenset( + {"run", "Popen", "call", "check_call", "check_output", "getoutput", "getstatusoutput"} +) +BoundShellCallKey = tuple[int, int, int, int] + + +def _bound_shell_call_key(call: ast.Call) -> BoundShellCallKey: + """Return a stable source key for one call node.""" + return ( + getattr(call, "lineno", 1), + getattr(call, "col_offset", 0), + getattr(call, "end_lineno", getattr(call, "lineno", 1)), + getattr(call, "end_col_offset", getattr(call, "col_offset", 0)), + ) + + +def _truth_value( + expression: ast.expr | None, + facts: dict[str, bool], +) -> bool | None: + """Return truth for a small, immutable, side-effect-free expression subset.""" + if expression is None: + return None + + resolved: dict[ast.expr, bool] = {} + pending: list[tuple[ast.expr, bool]] = [(expression, False)] + while pending: + current, expanded = pending.pop() + if isinstance(current, ast.Constant): + resolved[current] = bool(current.value) + elif isinstance(current, ast.Name): + if current.id not in facts: + return None + resolved[current] = facts[current.id] + elif isinstance(current, ast.Tuple): + if not current.elts: + resolved[current] = False + elif any(isinstance(item, ast.Starred) for item in current.elts): + return None + elif all(_is_passive_argument(item) for item in current.elts): + resolved[current] = True + else: + return None + elif isinstance(current, ast.UnaryOp): + if not isinstance(current.op, ast.Not) and not ( + isinstance(current.op, (ast.UAdd, ast.USub)) + and isinstance(current.operand, ast.Constant) + and type(current.operand.value) in (bool, int, float, complex) + ): + return None + if expanded: + operand = resolved[current.operand] + resolved[current] = not operand if isinstance(current.op, ast.Not) else operand + else: + pending.append((current, True)) + pending.append((current.operand, False)) + else: + return None + return resolved[expression] + + +def _update_trusted_names_from_import( + statement: ast.Import | ast.ImportFrom, + trusted_names: set[str], +) -> None: + """Update only direct receiver names that the import actually binds.""" + if isinstance(statement, ast.Import): + for imported in statement.names: + bound = imported.asname or imported.name.partition(".")[0] + if imported.name == "subprocess" and bound == "subprocess": + trusted_names.add(bound) + elif bound in trusted_names: + trusted_names.discard(bound) + return + + if any(imported.name == "*" for imported in statement.names): + trusted_names.clear() + return + for imported in statement.names: + bound = imported.asname or imported.name + if ( + statement.level == 0 + and statement.module == "subprocess" + and imported.name == "Popen" + and bound == "Popen" + ): + trusted_names.add(bound) + elif bound in trusted_names: + trusted_names.discard(bound) + + +class _DirectBindingCollector: + """Collect direct receiver bindings without entering nested scopes.""" + + def __init__(self, tracked_names: set[str] | frozenset[str]) -> None: + self.tracked_names = tracked_names + self.bound: set[str] = set() + self.mutated: set[str] = set() + self.nonlocal_names: set[str] = set() + + @staticmethod + def _function_header_nodes( + node: ast.FunctionDef | ast.AsyncFunctionDef, + ) -> list[ast.AST]: + nodes: list[ast.AST] = [*node.decorator_list, *node.args.defaults] + nodes.extend(item for item in node.args.kw_defaults if item is not None) + arguments = (*node.args.posonlyargs, *node.args.args, *node.args.kwonlyargs) + nodes.extend( + argument.annotation for argument in arguments if argument.annotation is not None + ) + if node.args.vararg is not None and node.args.vararg.annotation is not None: + nodes.append(node.args.vararg.annotation) + if node.args.kwarg is not None and node.args.kwarg.annotation is not None: + nodes.append(node.args.kwarg.annotation) + if node.returns is not None: + nodes.append(node.returns) + nodes.extend(getattr(node, "type_params", ())) + return nodes + + def visit(self, node: ast.AST) -> None: + pending = [node] + while pending: + current = pending.pop() + if isinstance(current, ast.Name): + if ( + isinstance(current.ctx, (ast.Store, ast.Del)) + and current.id in self.tracked_names + ): + self.bound.add(current.id) + continue + if isinstance(current, (ast.Attribute, ast.Subscript)): + if isinstance(current.ctx, (ast.Store, ast.Del)): + root: ast.expr = current.value + while isinstance(root, (ast.Attribute, ast.Subscript)): + root = root.value + if isinstance(root, ast.Name) and root.id in self.tracked_names: + self.mutated.add(root.id) + pending.extend(ast.iter_child_nodes(current)) + continue + if isinstance(current, (ast.FunctionDef, ast.AsyncFunctionDef)): + if current.name in self.tracked_names: + self.bound.add(current.name) + pending.extend(self._function_header_nodes(current)) + continue + if isinstance(current, ast.ClassDef): + if current.name in self.tracked_names: + self.bound.add(current.name) + pending.extend(current.decorator_list) + pending.extend(current.bases) + pending.extend(keyword.value for keyword in current.keywords) + continue + if isinstance(current, ast.Lambda): + pending.extend(current.args.defaults) + pending.extend(item for item in current.args.kw_defaults if item is not None) + continue + if isinstance(current, (ast.ListComp, ast.SetComp, ast.GeneratorExp)): + pending.append(current.elt) + for generator in current.generators: + pending.append(generator.iter) + pending.extend(generator.ifs) + continue + if isinstance(current, ast.DictComp): + pending.extend((current.key, current.value)) + for generator in current.generators: + pending.append(generator.iter) + pending.extend(generator.ifs) + continue + if isinstance(current, ast.Import): + for imported in current.names: + bound = imported.asname or imported.name.partition(".")[0] + if bound in self.tracked_names: + self.bound.add(bound) + continue + if isinstance(current, ast.ImportFrom): + if any(imported.name == "*" for imported in current.names): + self.bound.update(self.tracked_names) + continue + for imported in current.names: + bound = imported.asname or imported.name + if bound in self.tracked_names: + self.bound.add(bound) + continue + if isinstance(current, ast.ExceptHandler): + if isinstance(current.name, str) and current.name in self.tracked_names: + self.bound.add(current.name) + pending.extend(ast.iter_child_nodes(current)) + continue + if isinstance(current, (ast.Global, ast.Nonlocal)): + self.nonlocal_names.update(current.names) + continue + if isinstance(current, ast.MatchAs): + if isinstance(current.name, str) and current.name in self.tracked_names: + self.bound.add(current.name) + if current.pattern is not None: + pending.append(current.pattern) + continue + if isinstance(current, ast.MatchStar): + if isinstance(current.name, str) and current.name in self.tracked_names: + self.bound.add(current.name) + continue + if isinstance(current, ast.MatchMapping): + if isinstance(current.rest, str) and current.rest in self.tracked_names: + self.bound.add(current.rest) + pending.extend(current.patterns) + continue + pending.extend(ast.iter_child_nodes(current)) + + +def _direct_bound_names(node: ast.AST) -> set[str]: + """Return names bound by *node* without entering deferred nested scopes.""" + candidates: set[str] = set() + for current in ast.walk(node): + if isinstance(current, ast.Name) and isinstance(current.ctx, (ast.Store, ast.Del)): + candidates.add(current.id) + elif isinstance(current, (ast.FunctionDef, ast.AsyncFunctionDef, ast.ClassDef)): + candidates.add(current.name) + elif isinstance(current, ast.Import): + candidates.update( + imported.asname or imported.name.partition(".")[0] for imported in current.names + ) + elif isinstance(current, ast.ImportFrom): + candidates.update( + imported.asname or imported.name + for imported in current.names + if imported.name != "*" + ) + elif isinstance(current, ast.ExceptHandler) and isinstance(current.name, str): + candidates.add(current.name) + elif isinstance(current, (ast.MatchAs, ast.MatchStar)) and isinstance( + current.name, + str, + ): + candidates.add(current.name) + elif isinstance(current, ast.MatchMapping) and isinstance(current.rest, str): + candidates.add(current.rest) + collector = _DirectBindingCollector(candidates) + collector.visit(node) + return collector.bound + + +def _function_parameter_names(statement: ast.FunctionDef | ast.AsyncFunctionDef) -> set[str]: + """Return names that may already hold unsafe values on body entry.""" + arguments = statement.args + names = { + argument.arg + for argument in (*arguments.posonlyargs, *arguments.args, *arguments.kwonlyargs) + } + if arguments.vararg is not None: + names.add(arguments.vararg.arg) + if arguments.kwarg is not None: + names.add(arguments.kwarg.arg) + declarations = _DirectBindingCollector(set()) + for child in statement.body: + declarations.visit(child) + names.update(declarations.nonlocal_names) + return names + + +def _function_bound_direct_names( + statement: ast.FunctionDef | ast.AsyncFunctionDef, + tracked_names: set[str], +) -> set[str]: + """Return compile-time local receiver names for one function scope.""" + arguments = statement.args + named = (*arguments.posonlyargs, *arguments.args, *arguments.kwonlyargs) + names = {argument.arg for argument in named} + if arguments.vararg is not None: + names.add(arguments.vararg.arg) + if arguments.kwarg is not None: + names.add(arguments.kwarg.arg) + collector = _DirectBindingCollector(tracked_names) + for child in statement.body: + collector.visit(child) + return names.intersection(tracked_names).union( + collector.bound.difference(collector.nonlocal_names) + ) + + +def _changed_direct_names(nodes: list[ast.AST], tracked_names: set[str]) -> set[str]: + """Return receiver names explicitly rebound or mutated by current-scope nodes.""" + collector = _DirectBindingCollector(tracked_names) + for node in nodes: + collector.visit(node) + return collector.bound.union(collector.mutated) + + +def _class_body_changed_direct_names( + statement: ast.ClassDef, + tracked_names: set[str], +) -> set[str]: + """Return explicit class-execution effects on outer receiver objects.""" + + def nested_classes(node: ast.AST) -> list[ast.ClassDef]: + classes: list[ast.ClassDef] = [] + pending = [node] + while pending: + current = pending.pop() + if isinstance(current, ast.ClassDef): + classes.append(current) + continue + if isinstance(current, (ast.FunctionDef, ast.AsyncFunctionDef, ast.Lambda)): + continue + pending.extend(ast.iter_child_nodes(current)) + return classes + + affected: set[str] = set() + pending_classes = [statement] + while pending_classes: + current_class = pending_classes.pop() + declaration_collector = _DirectBindingCollector(tracked_names) + for child in current_class.body: + declaration_collector.visit(child) + global_names = declaration_collector.nonlocal_names.intersection(tracked_names) + + local_direct: dict[str, bool] = {} + affected.update(global_names.intersection(declaration_collector.bound)) + for child in current_class.body: + collector = _DirectBindingCollector(tracked_names) + collector.visit(child) + affected.update( + name + for name in collector.mutated + if name in global_names or local_direct.get(name, True) + ) + affected.update(collector.bound.intersection(global_names)) + pending_classes.extend(nested_classes(child)) + + local_bound = collector.bound.difference(global_names) + if isinstance(child, ast.Import): + for imported in child.names: + bound = imported.asname or imported.name.partition(".")[0] + if bound in local_bound: + local_direct[bound] = ( + imported.name == "subprocess" and bound == "subprocess" + ) + elif isinstance(child, ast.ImportFrom): + for imported in child.names: + bound = imported.asname or imported.name + if bound in local_bound: + local_direct[bound] = ( + child.level == 0 + and child.module == "subprocess" + and imported.name == "Popen" + and bound == "Popen" + ) + elif isinstance(child, ast.Assign): + prior_local_direct = dict(local_direct) + for name in local_bound: + local_direct[name] = False + for target in child.targets: + if isinstance(target, ast.Name) and target.id in local_bound: + local_direct[target.id] = ( + isinstance(child.value, ast.Name) + and child.value.id == target.id + and prior_local_direct.get( + child.value.id, + child.value.id in tracked_names, + ) + ) + elif isinstance(child, ast.AnnAssign) and child.value is not None: + prior_local_direct = dict(local_direct) + for name in local_bound: + local_direct[name] = False + if isinstance(child.target, ast.Name) and child.target.id in local_bound: + local_direct[child.target.id] = ( + isinstance(child.value, ast.Name) + and child.value.id == child.target.id + and prior_local_direct.get( + child.value.id, + child.value.id in tracked_names, + ) + ) + elif isinstance(child, (ast.FunctionDef, ast.AsyncFunctionDef, ast.ClassDef)): + if child.name in tracked_names and child.name not in global_names: + local_direct[child.name] = False + elif isinstance(child, ast.Delete): + for name in collector.bound: + local_direct.pop(name, None) + elif local_bound: + for name in local_bound: + local_direct.pop(name, None) + return affected + + +def _nodes_contain_eager_call(nodes: list[ast.AST]) -> bool: + """Return whether eager evaluation reaches a call outside deferred bodies.""" + pending = list(nodes) + while pending: + current = pending.pop() + if isinstance(current, ast.Call): + return True + if isinstance(current, (ast.FunctionDef, ast.AsyncFunctionDef)): + if not _function_header_is_passive(current): + return True + continue + if isinstance(current, ast.ClassDef): + if ( + not _class_header_is_passive(current) + or _class_body_may_release_outer_value(current) + or _nodes_contain_eager_call(list(current.body)) + ): + return True + continue + if isinstance(current, ast.Lambda): + defaults = ( + *current.args.defaults, + *(item for item in current.args.kw_defaults if item is not None), + ) + if any(not _is_passive_argument(default) for default in defaults): + return True + continue + pending.extend(ast.iter_child_nodes(current)) + return False + + +def _class_body_may_release_outer_value(statement: ast.ClassDef) -> bool: + """Return whether class execution stores to a declared outer name.""" + declarations = _DirectBindingCollector(set()) + for child in statement.body: + declarations.visit(child) + outer_names = declarations.nonlocal_names + if outer_names: + bindings = _DirectBindingCollector(outer_names) + for child in statement.body: + bindings.visit(child) + if bindings.bound.intersection(outer_names): + return True + + pending: list[ast.AST] = list(statement.body) + while pending: + current = pending.pop() + if isinstance(current, ast.ClassDef): + if _class_body_may_release_outer_value(current): + return True + continue + if isinstance(current, (ast.FunctionDef, ast.AsyncFunctionDef, ast.Lambda)): + continue + pending.extend(ast.iter_child_nodes(current)) + return False + + +def _class_body_has_eager_effects(statement: ast.ClassDef) -> bool: + """Return whether class execution can run user code or unsafe finalizers.""" + declarations = _DirectBindingCollector(set()) + for child in statement.body: + declarations.visit(child) + outer_names = declarations.nonlocal_names + bound_names: set[str] = set() + finalizer_safe_names: set[str] = set() + + for child in statement.body: + if isinstance(child, (ast.Global, ast.Nonlocal, ast.Pass)) or ( + isinstance(child, ast.Expr) and isinstance(child.value, ast.Constant) + ): + continue + if isinstance(child, (ast.FunctionDef, ast.AsyncFunctionDef)): + releases_unsafe_value = ( + child.name in bound_names and child.name not in finalizer_safe_names + ) + if ( + child.name in outer_names + or not _function_header_is_passive(child) + or releases_unsafe_value + ): + return True + bound_names.add(child.name) + finalizer_safe_names.add(child.name) + continue + if isinstance(child, ast.ClassDef): + releases_unsafe_value = ( + child.name in bound_names and child.name not in finalizer_safe_names + ) + if ( + child.name in outer_names + or not _class_header_is_passive(child) + or releases_unsafe_value + or _class_body_has_eager_effects(child) + ): + return True + bound_names.add(child.name) + finalizer_safe_names.add(child.name) + continue + if isinstance(child, ast.Assign): + if not all(isinstance(target, ast.Name) for target in child.targets): + return True + target_names = {target.id for target in child.targets if isinstance(target, ast.Name)} + if target_names.intersection(outer_names): + return True + releases_unsafe_value = any( + name in bound_names + and name not in finalizer_safe_names + and not (isinstance(child.value, ast.Name) and child.value.id == name) + for name in target_names + ) + if releases_unsafe_value or not _is_finalizer_safe_value( + child.value, + finalizer_safe_names, + ): + return True + bound_names.update(target_names) + finalizer_safe_names.update(target_names) + continue + if isinstance(child, ast.AnnAssign): + if not isinstance(child.target, ast.Name) or not _annotation_is_passive( + child.annotation + ): + return True + if child.value is None: + continue + target_name = child.target.id + if target_name in outer_names: + return True + releases_unsafe_value = ( + target_name in bound_names + and target_name not in finalizer_safe_names + and not (isinstance(child.value, ast.Name) and child.value.id == target_name) + ) + if releases_unsafe_value or not _is_finalizer_safe_value( + child.value, + finalizer_safe_names, + ): + return True + bound_names.add(target_name) + finalizer_safe_names.add(target_name) + continue + if isinstance(child, ast.Assert) and _is_finalizer_safe_value( + child.test, + finalizer_safe_names, + ): + continue + return True + return False + + +def _class_deferred_receiver_trust( + statement: ast.ClassDef, + trusted_names: set[str], +) -> tuple[set[str], dict[int, set[str]]]: + """Return final and observed-call outer trust for deferred class methods.""" + deferred = set(trusted_names) + trusted_at_call_by_definition: dict[int, set[str]] = {} + active_functions: dict[str, int] = {} + declarations = _DirectBindingCollector(_DIRECT_CALL_NAMES) + for child in statement.body: + declarations.visit(child) + global_names = declarations.nonlocal_names.intersection(_DIRECT_CALL_NAMES) + + for child_index, child in enumerate(statement.body): + call = _passive_direct_call(child) + if call is not None: + assert isinstance(call.func, ast.Name) + owner = active_functions.get(call.func.id) + if owner is not None: + trusted_at_call_by_definition.setdefault(owner, set()).update(deferred) + + if _nodes_contain_eager_call([child]): + # Class-body expressions run before any method can be called and may + # mutate the surrounding module/function receiver binding. + deferred.clear() + + collector = _DirectBindingCollector(_DIRECT_CALL_NAMES) + collector.visit(child) + deferred.difference_update(collector.mutated) + globally_bound = collector.bound.intersection(global_names) + if isinstance(child, ast.Import): + for imported in child.names: + bound = imported.asname or imported.name.partition(".")[0] + if bound not in globally_bound: + continue + if imported.name == "subprocess" and bound == "subprocess": + deferred.add(bound) + else: + deferred.discard(bound) + elif isinstance(child, ast.ImportFrom): + if any(imported.name == "*" for imported in child.names) and global_names: + deferred.clear() + for imported in child.names: + bound = imported.asname or imported.name + if bound not in globally_bound: + continue + if ( + child.level == 0 + and child.module == "subprocess" + and imported.name == "Popen" + and bound == "Popen" + ): + deferred.add(bound) + else: + deferred.discard(bound) + else: + deferred.difference_update(globally_bound) + + changed_names = _direct_bound_names(child) + for name in changed_names: + active_functions.pop(name, None) + if ( + isinstance(child, (ast.FunctionDef, ast.AsyncFunctionDef)) + and _function_header_is_passive(child) + and _is_immediate_function(child) + ): + active_functions[child.name] = child_index + return deferred, trusted_at_call_by_definition + + +def _is_direct_subprocess_syntax(call: ast.Call) -> bool: + """Return whether a call uses one of the direct subprocess spellings.""" + function = call.func + if isinstance(function, ast.Name): + return function.id == "Popen" + return ( + isinstance(function, ast.Attribute) + and isinstance(function.value, ast.Name) + and function.value.id == "subprocess" + ) + + +def _is_direct_subprocess_call(call: ast.Call, trusted_names: set[str]) -> bool: + if not _is_direct_subprocess_syntax(call): + return False + function = call.func + receiver = function.id if isinstance(function, ast.Name) else function.value.id + return receiver in trusted_names + + +def _is_passive_argument(expression: ast.expr) -> bool: + """Return whether evaluation cannot invoke user-controlled Python code.""" + normal, hash_required, truth_required, numeric_required, integral_required = range(5) + pending: list[tuple[ast.expr, int]] = [(expression, normal)] + while pending: + current, requirement = pending.pop() + if isinstance(current, ast.Constant): + if requirement == numeric_required and type(current.value) not in ( + bool, + int, + float, + complex, + ): + return False + if requirement == integral_required and type(current.value) not in (bool, int): + return False + continue + if isinstance(current, ast.Name): + if requirement != normal: + return False + continue + if isinstance(current, ast.List): + if requirement in (hash_required, numeric_required, integral_required) or any( + isinstance(item, ast.Starred) for item in current.elts + ): + return False + pending.extend((item, normal) for item in current.elts) + continue + if isinstance(current, ast.Tuple): + if requirement in (numeric_required, integral_required): + return False + if any(isinstance(item, ast.Starred) for item in current.elts): + return False + nested_requirement = hash_required if requirement == hash_required else normal + pending.extend((item, nested_requirement) for item in current.elts) + continue + if isinstance(current, ast.Dict): + if requirement in (hash_required, numeric_required, integral_required) or any( + key is None for key in current.keys + ): + return False + pending.extend((key, hash_required) for key in current.keys if key is not None) + pending.extend((value, normal) for value in current.values) + continue + if isinstance(current, ast.Set): + if requirement in (hash_required, numeric_required, integral_required): + return False + pending.extend((item, hash_required) for item in current.elts) + continue + if isinstance(current, ast.UnaryOp): + if isinstance(current.op, ast.Not): + pending.append((current.operand, truth_required)) + elif isinstance(current.op, (ast.UAdd, ast.USub)): + operand_requirement = ( + integral_required if requirement == integral_required else numeric_required + ) + pending.append((current.operand, operand_requirement)) + elif isinstance(current.op, ast.Invert): + pending.append((current.operand, integral_required)) + else: + return False + continue + if isinstance(current, ast.JoinedStr) and all( + isinstance(item, ast.Constant) for item in current.values + ): + if requirement in (numeric_required, integral_required): + return False + continue + return False + return True + + +def _call_arguments_are_passive(call: ast.Call) -> bool: + return all(_is_passive_argument(argument) for argument in call.args) and all( + keyword.arg is not None and _is_passive_argument(keyword.value) for keyword in call.keywords + ) + + +def _value_preserves_receiver_trust( + expression: ast.expr, + trusted_names: set[str], + protocol_safe_names: set[str] | None = None, +) -> bool: + """Return whether eager evaluation cannot replace a direct receiver.""" + if _is_passive_argument(expression): + return True + if isinstance(expression, ast.Call): + return ( + _is_direct_subprocess_call(expression, trusted_names) + and _call_arguments_are_passive(expression) + and ( + protocol_safe_names is None + or _call_arguments_are_protocol_safe(expression, protocol_safe_names) + ) + ) + if isinstance(expression, (ast.Tuple, ast.List)): + return all( + not isinstance(item, ast.Starred) + and _value_preserves_receiver_trust( + item, + trusted_names, + protocol_safe_names, + ) + for item in expression.elts + ) + if isinstance(expression, (ast.ListComp, ast.SetComp, ast.GeneratorExp)): + values = [expression.elt] + elif isinstance(expression, ast.DictComp): + values = [expression.key, expression.value] + else: + return False + for generator in expression.generators: + if generator.is_async or not _is_simple_comprehension_target(generator.target): + return False + if protocol_safe_names is not None and ( + not _is_finalizer_safe_value(generator.iter, protocol_safe_names) + or any( + not _is_finalizer_safe_value(condition, protocol_safe_names) + for condition in generator.ifs + ) + ): + return False + values.extend((generator.iter, *generator.ifs)) + # Comprehensions have historically retained ownership for their direct + # subprocess calls. Preserve that behavior while still rejecting generic + # nested calls and receiver stores. + return all( + _value_preserves_receiver_trust( + value, + trusted_names, + protocol_safe_names, + ) + for value in values + ) + + +def _is_simple_comprehension_target(target: ast.expr) -> bool: + if isinstance(target, ast.Name): + return True + return isinstance(target, (ast.Tuple, ast.List)) and all( + _is_simple_comprehension_target(item) for item in target.elts + ) + + +def _shell_argument_is_captured_before_effects(call: ast.Call) -> bool: + """Return whether evaluation reaches ``shell=`` without user-code effects. + + Python evaluates every positional argument, including starred expansions, + before keyword arguments. Keyword values are then evaluated in their stored + order. Effects after ``shell=`` cannot change the already captured value. + """ + if any(not _is_passive_argument(argument) for argument in call.args): + return False + for keyword in call.keywords: + if keyword.arg == "shell": + return _is_passive_argument(keyword.value) + if keyword.arg is None or not _is_passive_argument(keyword.value): + return False + return False + + +def _is_finalizer_safe_value(expression: ast.expr, safe_names: set[str]) -> bool: + """Return whether releasing the resulting value cannot run user code.""" + if not _is_passive_argument(expression): + return False + return all( + not isinstance(node, ast.Name) or node.id in safe_names for node in ast.walk(expression) + ) + + +def _call_arguments_are_protocol_safe(call: ast.Call, safe_names: set[str]) -> bool: + """Return whether subprocess argument consumption cannot dispatch user code.""" + return all(_is_finalizer_safe_value(argument, safe_names) for argument in call.args) and all( + keyword.arg is not None and _is_finalizer_safe_value(keyword.value, safe_names) + for keyword in call.keywords + ) + + +def _annotation_is_passive(annotation: ast.expr) -> bool: + """Accept only annotation spellings whose evaluation cannot rebind a name.""" + return all( + isinstance(node, (ast.Name, ast.Constant, ast.Load)) for node in ast.walk(annotation) + ) + + +def _function_header_is_passive( + statement: ast.FunctionDef | ast.AsyncFunctionDef, +) -> bool: + """Reject definition-time expressions that could mutate tracked bindings.""" + if statement.decorator_list or getattr(statement, "type_params", []): + return False + defaults = (*statement.args.defaults, *(item for item in statement.args.kw_defaults if item)) + if any(not _is_passive_argument(default) for default in defaults): + return False + arguments = ( + *statement.args.posonlyargs, + *statement.args.args, + *statement.args.kwonlyargs, + ) + annotations = [argument.annotation for argument in arguments if argument.annotation is not None] + if statement.args.vararg is not None and statement.args.vararg.annotation is not None: + annotations.append(statement.args.vararg.annotation) + if statement.args.kwarg is not None and statement.args.kwarg.annotation is not None: + annotations.append(statement.args.kwarg.annotation) + if statement.returns is not None: + annotations.append(statement.returns) + return all(_annotation_is_passive(annotation) for annotation in annotations) + + +def _class_header_is_passive(statement: ast.ClassDef) -> bool: + """Return whether evaluating a class header cannot rebind a receiver.""" + return not ( + statement.decorator_list + or statement.bases + or statement.keywords + or getattr(statement, "type_params", []) + ) + + +def _is_immediate_function(statement: ast.FunctionDef | ast.AsyncFunctionDef) -> bool: + """Return whether a direct call begins executing this function body.""" + if isinstance(statement, ast.AsyncFunctionDef): + return False + pending: list[ast.AST] = list(statement.body) + while pending: + current = pending.pop() + if isinstance(current, (ast.Yield, ast.YieldFrom)): + return False + if isinstance(current, (ast.FunctionDef, ast.AsyncFunctionDef, ast.ClassDef, ast.Lambda)): + continue + pending.extend(ast.iter_child_nodes(current)) + return True + + +def _passive_direct_call(statement: ast.stmt) -> ast.Call | None: + """Return a directly evaluated simple-name call with passive arguments.""" + value: ast.expr | None = None + if isinstance(statement, (ast.Expr, ast.Assign)): + value = statement.value + elif isinstance(statement, ast.AnnAssign): + value = statement.value + if ( + isinstance(value, ast.Call) + and isinstance(value.func, ast.Name) + and _call_arguments_are_passive(value) + ): + return value + return None + + +def _advance_trusted_names( + statement: ast.stmt, + trusted_names: set[str], + bound_names: set[str], + finalizer_safe_names: set[str], + unknown_unsafe_bindings: list[bool], +) -> None: + """Apply one eager statement's receiver-trust and value-release effects.""" + if isinstance(statement, (ast.Import, ast.ImportFrom)): + imported_names = _direct_bound_names(statement) + releases_unsafe_value = any( + name in bound_names and name not in finalizer_safe_names for name in imported_names + ) + bound_names.update(imported_names) + finalizer_safe_names.difference_update(imported_names) + _update_trusted_names_from_import(statement, trusted_names) + imports_unknown_names = isinstance(statement, ast.ImportFrom) and any( + imported.name == "*" for imported in statement.names + ) + if releases_unsafe_value or imports_unknown_names: + unknown_unsafe_bindings[0] = True + if releases_unsafe_value or unknown_unsafe_bindings[0]: + trusted_names.clear() + return + if isinstance(statement, (ast.FunctionDef, ast.AsyncFunctionDef)): + releases_unsafe_value = ( + statement.name in bound_names and statement.name not in finalizer_safe_names + ) + if not _function_header_is_passive(statement) or releases_unsafe_value: + trusted_names.clear() + unknown_unsafe_bindings[0] = True + bound_names.add(statement.name) + finalizer_safe_names.discard(statement.name) + trusted_names.discard(statement.name) + return + if isinstance(statement, ast.Assign): + targets = list(statement.targets) + simple_targets = all(isinstance(target, ast.Name) for target in targets) + result_is_finalizer_safe = _is_finalizer_safe_value( + statement.value, + finalizer_safe_names, + ) + preserves_receiver_trust = simple_targets and _value_preserves_receiver_trust( + statement.value, + trusted_names, + finalizer_safe_names, + ) + releases_unsafe_value = simple_targets and any( + target.id in bound_names + and target.id not in finalizer_safe_names + and not (isinstance(statement.value, ast.Name) and statement.value.id == target.id) + for target in targets + if isinstance(target, ast.Name) + ) + if not preserves_receiver_trust or releases_unsafe_value: + trusted_names.clear() + finalizer_safe_names.clear() + unknown_unsafe_bindings[0] = True + if not simple_targets: + bound_names.update(_direct_bound_names(statement)) + return + changed = _changed_direct_names( + [statement.value, *statement.targets], + trusted_names, + ) + preserved = { + target.id + for target in statement.targets + if isinstance(target, ast.Name) + and isinstance(statement.value, ast.Name) + and statement.value.id == target.id + and target.id in trusted_names + } + trusted_names.difference_update(changed.difference(preserved)) + for target in targets: + assert isinstance(target, ast.Name) + bound_names.add(target.id) + if preserves_receiver_trust and not releases_unsafe_value and result_is_finalizer_safe: + finalizer_safe_names.add(target.id) + else: + finalizer_safe_names.discard(target.id) + return + if isinstance(statement, ast.AnnAssign): + value = statement.value + target = statement.target + simple_target = isinstance(target, ast.Name) + preserves_receiver_trust = ( + simple_target + and _annotation_is_passive(statement.annotation) + and ( + value is None + or _value_preserves_receiver_trust( + value, + trusted_names, + finalizer_safe_names, + ) + ) + ) + releases_unsafe_value = ( + value is not None + and simple_target + and target.id in bound_names + and target.id not in finalizer_safe_names + and not (isinstance(value, ast.Name) and value.id == target.id) + ) + result_is_finalizer_safe = value is not None and _is_finalizer_safe_value( + value, + finalizer_safe_names, + ) + if not preserves_receiver_trust or releases_unsafe_value: + trusted_names.clear() + finalizer_safe_names.clear() + unknown_unsafe_bindings[0] = True + if value is not None: + bound_names.update(_direct_bound_names(statement)) + if simple_target: + if ( + preserves_receiver_trust + and not releases_unsafe_value + and result_is_finalizer_safe + ): + finalizer_safe_names.add(target.id) + else: + finalizer_safe_names.discard(target.id) + trusted_names.difference_update(_changed_direct_names([statement], trusted_names)) + return + if isinstance(statement, ast.ClassDef): + releases_unsafe_value = ( + statement.name in bound_names and statement.name not in finalizer_safe_names + ) + unsafe_class_execution = ( + not _class_header_is_passive(statement) + or _class_body_has_eager_effects(statement) + or releases_unsafe_value + ) + if unsafe_class_execution: + trusted_names.clear() + unknown_unsafe_bindings[0] = True + finalizer_safe_names.clear() + else: + finalizer_safe_names.discard(statement.name) + trusted_names.difference_update(_changed_direct_names([statement], trusted_names)) + trusted_names.difference_update(_class_body_changed_direct_names(statement, trusted_names)) + bound_names.update(_direct_bound_names(statement)) + return + if isinstance(statement, ast.Expr): + value = statement.value + preserves_receiver_trust = _value_preserves_receiver_trust( + value, + trusted_names, + finalizer_safe_names, + ) + if not preserves_receiver_trust: + trusted_names.clear() + unknown_unsafe_bindings[0] = True + finalizer_safe_names.clear() + else: + trusted_names.difference_update(_changed_direct_names([statement], trusted_names)) + bound_names.update(_direct_bound_names(statement)) + return + if isinstance(statement, ast.Assert): + expressions = [statement.test] + if statement.msg is not None: + expressions.append(statement.msg) + if not ( + _is_finalizer_safe_value(statement.test, finalizer_safe_names) + and all( + _value_preserves_receiver_trust( + item, + trusted_names, + finalizer_safe_names, + ) + for item in expressions + ) + ): + trusted_names.clear() + unknown_unsafe_bindings[0] = True + finalizer_safe_names.clear() + return + if isinstance(statement, (ast.Global, ast.Nonlocal, ast.Pass)): + return + bound_names.update(_direct_bound_names(statement)) + finalizer_safe_names.clear() + trusted_names.clear() + unknown_unsafe_bindings[0] = True + + +class _CachedSubprocessState: + """Track explicit writes to cached module slots independently of imports.""" + + def __init__(self) -> None: + self.module_names = {"subprocess"} + self.changed_methods: set[str] = set() + self.changed_popen = False + self.effect_generation = 0 + self.protocol_unsafe = False + self.bound_names: set[str] = set() + self.safe_names: set[str] = set() + + def copy(self) -> _CachedSubprocessState: + copied = _CachedSubprocessState() + copied.module_names = set(self.module_names) + copied.changed_methods = set(self.changed_methods) + copied.changed_popen = self.changed_popen + copied.effect_generation = self.effect_generation + copied.protocol_unsafe = self.protocol_unsafe + copied.bound_names = set(self.bound_names) + copied.safe_names = set(self.safe_names) + return copied + + def _invalidate(self) -> None: + """An unmodeled eager effect can change receivers and restore old slots.""" + self.module_names.clear() + self.changed_methods.clear() + self.changed_popen = False + self.safe_names.clear() + self.effect_generation += 1 + + def _passive_value(self, value: ast.expr) -> bool: + pending = [value] + while pending: + current = pending.pop() + if _is_passive_argument(current): + continue + if ( + isinstance(current, ast.Attribute) + and isinstance(current.value, ast.Name) + and current.value.id in self.module_names + ): + continue + if isinstance(current, (ast.Tuple, ast.List)) and not any( + isinstance(item, ast.Starred) for item in current.elts + ): + pending.extend(current.elts) + continue + return False + return True + + def blocks(self, call: ast.Call) -> bool: + # A later import or slot write cannot establish that an unknown earlier + # effect left the cached module's protocol and finalizer bindings intact. + if self.protocol_unsafe or self.effect_generation > 0: + return False + function = call.func + if isinstance(function, ast.Name): + return function.id == "Popen" and self.changed_popen + return isinstance(function, ast.Attribute) and function.attr in self.changed_methods + + def advance(self, statement: ast.stmt) -> None: + """Apply only explicit eager bindings; deferred bodies do not execute here.""" + if isinstance(statement, (ast.Import, ast.ImportFrom)): + imported_names = _direct_bound_names(statement) + if any( + name in self.bound_names + and name not in self.safe_names + and name not in self.module_names + for name in imported_names + ): + self._invalidate() + ordinary_subprocess_import = ( + isinstance(statement, ast.Import) + and all(imported.name == "subprocess" for imported in statement.names) + ) or ( + isinstance(statement, ast.ImportFrom) + and statement.level == 0 + and statement.module == "subprocess" + and all( + imported.name == "Popen" and (imported.asname or imported.name) == "Popen" + for imported in statement.names + ) + ) + if not ordinary_subprocess_import: + self._invalidate() + self.module_names.difference_update(imported_names) + self.bound_names.update(imported_names) + self.safe_names.difference_update(imported_names) + if isinstance(statement, ast.Import): + self.module_names.update( + imported.asname or "subprocess" + for imported in statement.names + if imported.name == "subprocess" + ) + elif statement.level == 0 and statement.module == "subprocess": + for imported in statement.names: + if imported.name == "Popen" and (imported.asname or imported.name) == "Popen": + self.changed_popen = "Popen" in self.changed_methods + return + if isinstance(statement, ast.ClassDef): + if not _class_header_is_passive(statement): + self._invalidate() + class_state = self.copy() + declarations = _DirectBindingCollector(set()) + pending_classes = [statement] + while pending_classes: + for child in pending_classes.pop().body: + declarations.visit(child) + if isinstance(child, ast.ClassDef): + pending_classes.append(child) + local_bound = _direct_bound_names(statement).difference(declarations.nonlocal_names) + for child in statement.body: + local_bound.update( + _direct_bound_names(child).difference(declarations.nonlocal_names) + ) + class_state.bound_names.difference_update(local_bound) + class_state.safe_names.difference_update(local_bound) + for child in statement.body: + class_state.advance(child) + self.protocol_unsafe |= class_state.protocol_unsafe + if class_state.effect_generation != self.effect_generation: + self.module_names.clear() + self.changed_popen = False + self.safe_names.clear() + self.effect_generation = class_state.effect_generation + self.changed_methods = class_state.changed_methods + self.module_names.difference_update(declarations.nonlocal_names) + self.module_names.update( + class_state.module_names.intersection(declarations.nonlocal_names) + ) + if "Popen" in declarations.nonlocal_names: + self.changed_popen = class_state.changed_popen + self.bound_names.update( + class_state.bound_names.intersection(declarations.nonlocal_names) + ) + self.safe_names.difference_update(declarations.nonlocal_names) + self.safe_names.update(class_state.safe_names.intersection(declarations.nonlocal_names)) + if ( + statement.name in self.bound_names + and statement.name not in self.safe_names + and statement.name not in self.module_names + ): + self._invalidate() + self.bound_names.add(statement.name) + self.safe_names.discard(statement.name) + self.module_names.discard(statement.name) + return + if isinstance(statement, (ast.FunctionDef, ast.AsyncFunctionDef)): + if not _function_header_is_passive(statement) or ( + statement.name in self.bound_names + and statement.name not in self.safe_names + and statement.name not in self.module_names + ): + self._invalidate() + self.bound_names.add(statement.name) + self.safe_names.discard(statement.name) + self.module_names.discard(statement.name) + return + targets: list[ast.expr] = [] + value: ast.expr | None = None + if isinstance(statement, ast.Assign): + targets, value = list(statement.targets), statement.value + elif isinstance(statement, ast.AnnAssign): + if statement.value is None: + if not _annotation_is_passive(statement.annotation): + self._invalidate() + return + targets, value = [statement.target], statement.value + elif isinstance(statement, ast.AugAssign): + targets = [statement.target] + elif isinstance(statement, ast.Delete): + self._invalidate() + self.bound_names.difference_update(_direct_bound_names(statement)) + return + else: + if not ( + isinstance(statement, (ast.Global, ast.Nonlocal, ast.Pass)) + or isinstance(statement, (ast.Expr, ast.Return)) + and (statement.value is None or self._passive_value(statement.value)) + ): + self._invalidate() + return + + native_api_value = value is not None and any( + isinstance(node, ast.Attribute) + and isinstance(node.value, ast.Name) + and node.value.id in self.module_names + and node.attr in _CACHED_SUBPROCESS_API_SLOTS + and node.attr not in self.changed_methods + or isinstance(node, ast.Name) + and node.id == "Popen" + and not self.changed_popen + for node in ast.walk(value) + ) + same_slot_identity = ( + len(targets) == 1 + and isinstance(targets[0], ast.Attribute) + and isinstance(targets[0].value, ast.Name) + and targets[0].value.id in self.module_names + and isinstance(value, ast.Attribute) + and isinstance(value.value, ast.Name) + and value.value.id in self.module_names + and targets[0].attr == value.attr + ) + # A native callable capture or cross-slot store cannot establish a + # custom replacement. Abstain without tracking callable value aliases. + if native_api_value and not same_slot_identity: + self._invalidate() + if ( + isinstance(statement, ast.AugAssign) + or value is not None + and not self._passive_value(value) + ): + self._invalidate() + rhs_names = ( + {node.id for node in ast.walk(value) if isinstance(node, ast.Name)} + if value is not None + else set() + ) + before_names = rhs_names.intersection(self.module_names) + before_methods = set(self.changed_methods) + before_safe = rhs_names.intersection(self.safe_names) + pending = [(target, value) for target in reversed(targets)] + while pending: + target, source = pending.pop() + if isinstance(target, (ast.Tuple, ast.List)): + if not isinstance(statement, ast.Delete) and not ( + isinstance(source, (ast.Tuple, ast.List)) + and len(target.elts) == len(source.elts) + and not any(isinstance(item, ast.Starred) for item in source.elts) + and not any(isinstance(item, ast.Starred) for item in target.elts) + ): + self._invalidate() + sources = ( + list(source.elts) + if isinstance(source, (ast.Tuple, ast.List)) + and len(target.elts) == len(source.elts) + and not any(isinstance(item, ast.Starred) for item in source.elts) + else [None] * len(target.elts) + ) + pending.extend(reversed(list(zip(target.elts, sources, strict=True)))) + elif isinstance(target, ast.Starred): + pending.append((target.value, None)) + elif isinstance(target, ast.Name): + self_store = isinstance(source, ast.Name) and source.id == target.id + releases_unsafe = ( + target.id in self.bound_names + and target.id not in self.safe_names + and target.id not in self.module_names + and not self_store + ) + safe_value = source is not None and _is_finalizer_safe_value(source, before_safe) + self.module_names.discard(target.id) + if isinstance(source, ast.Name) and source.id in before_names: + self.module_names.add(target.id) + self.bound_names.add(target.id) + self.safe_names.discard(target.id) + if safe_value: + self.safe_names.add(target.id) + if releases_unsafe: + self._invalidate() + elif ( + isinstance(target, ast.Attribute) + and isinstance(target.value, ast.Name) + and target.value.id in self.module_names + ): + if target.attr not in _CACHED_SUBPROCESS_API_SLOTS or self.protocol_unsafe: + self.protocol_unsafe |= target.attr.startswith("__") + self._invalidate() + continue + preserves_slot = ( + isinstance(source, ast.Attribute) + and isinstance(source.value, ast.Name) + and source.value.id in before_names + and source.attr == target.attr + ) + if preserves_slot and target.attr not in before_methods: + self.changed_methods.discard(target.attr) + elif not preserves_slot and target.attr in before_methods: + # Releasing a previously replaced slot can run an arbitrary + # finalizer after the new store and restore its old value. + self._invalidate() + else: + self.changed_methods.add(target.attr) + else: + self._invalidate() + if isinstance(statement, ast.AnnAssign) and not _annotation_is_passive( + statement.annotation + ): + self._invalidate() + + +class _Analyzer: + def __init__(self, file_path: str, python_ast: ParsedPythonFile) -> None: + self.file_path = file_path + self.python_ast = python_ast + self.lines = python_ast.lines + self.findings: list[AnalyzerFinding] = [] + self.bound_shell_call_ownership: dict[BoundShellCallKey, bool] = {} + self.emitted_shell_calls: set[BoundShellCallKey] = set() + self.cached_replacement_by_call: dict[BoundShellCallKey, bool] = {} + self.cached_subprocess = _CachedSubprocessState() + + def _record_cached_replacement(self, call: ast.Call) -> None: + """Keep explicit cached-slot evidence distinct from receiver uncertainty.""" + shell = next((item.value for item in call.keywords if item.arg == "shell"), None) + if not _is_direct_subprocess_syntax(call) or not isinstance(shell, ast.Name): + return + key = _bound_shell_call_key(call) + blocked = self.cached_subprocess.blocks(call) + self.cached_replacement_by_call[key] = ( + self.cached_replacement_by_call.get(key, True) and blocked + ) + + def _record_eager_cached_replacements(self, expression: ast.expr) -> None: + """Visit eager expression order until an unmodeled effect or scope boundary.""" + pending: list[ast.expr | None] = [expression] + while pending: + current = pending.pop() + if current is None: + return + if isinstance(current, ast.Call): + function = current.func + passive_lookup = isinstance(function, ast.Name) or ( + isinstance(function, ast.Attribute) + and isinstance(function.value, ast.Name) + and function.value.id in self.cached_subprocess.module_names + ) + if passive_lookup: + self._record_cached_replacement(current) + children = [function, *current.args, *(item.value for item in current.keywords)] + pending.append(None) + pending.extend(reversed(children)) + elif isinstance(current, (ast.Name, ast.Constant)): + continue + elif isinstance(current, (ast.Tuple, ast.List)): + pending.extend(reversed(current.elts)) + elif isinstance(current, ast.Attribute): + if not ( + isinstance(current.value, ast.Name) + and current.value.id in self.cached_subprocess.module_names + ): + pending.append(None) + pending.append(current.value) + else: + return + + def _record_bound_shell_call(self, call: ast.Call, trusted_names: set[str]) -> None: + """Record whether the companion owns one supported bound-shell call.""" + shell = next((item.value for item in call.keywords if item.arg == "shell"), None) + if not _is_direct_subprocess_syntax(call) or not isinstance(shell, ast.Name): + return + self._record_cached_replacement(call) + key = _bound_shell_call_key(call) + blocked = self.cached_subprocess.blocks(call) + self.bound_shell_call_ownership[key] = bool( + _is_direct_subprocess_call(call, trusted_names) + and not blocked + and _shell_argument_is_captured_before_effects(call) + ) + + def _inspect_call(self, call: ast.Call, facts: dict[str, bool]) -> None: + shell = next((item.value for item in call.keywords if item.arg == "shell"), None) + if ( + not isinstance(shell, ast.Name) + or self.cached_subprocess.blocks(call) + or shell.id.casefold().startswith("true") + or facts.get(shell.id) is not True + ): + return + self.emitted_shell_calls.add(_bound_shell_call_key(call)) + line = getattr(call, "lineno", 1) + end_line = getattr(call, "end_lineno", None) + start_byte_column = getattr(call, "col_offset", 0) + end_byte_column = getattr(call, "end_col_offset", start_byte_column) + start_column = self.python_ast.character_column(line, start_byte_column) + end_column = self.python_ast.character_column(end_line or line, end_byte_column) + complete_match = self.python_ast.source_segment(call) + if complete_match is None: + complete_match = get_complete_source_segment(self.lines, line, end_line) + self.findings.append( + AnalyzerFinding( + rule_id="TM1", + message="Tool Parameter Abuse", + severity=Severity.HIGH, + location=Location( + file=self.file_path, + start_line=line, + end_line=end_line, + start_column=start_column, + end_column=end_column, + ), + confidence=0.8, + tags=[PatternCategory.TOOL_MISUSE.value], + context=get_context_from_lines( + self.lines, + line, + column=start_column if start_column is not None else 0, + ), + matched_text=complete_match[:200], + complete_match=complete_match, + evidence={BOUND_SHELL_EVIDENCE: True}, + ) + ) + + def _scan_assignment( + self, + targets: list[ast.expr], + value: ast.expr, + facts: dict[str, bool], + trusted_names: set[str], + bound_names: set[str], + finalizer_safe_names: set[str], + unknown_unsafe_bindings: list[bool], + ) -> None: + simple_targets = all(isinstance(target, ast.Name) for target in targets) + releases_unsafe_value = simple_targets and any( + target.id in bound_names + and target.id not in finalizer_safe_names + and not (isinstance(value, ast.Name) and value.id == target.id) + for target in targets + if isinstance(target, ast.Name) + ) + result_is_finalizer_safe = _is_finalizer_safe_value(value, finalizer_safe_names) + call_has_protocol_effects = False + preserves_receiver_trust = simple_targets and _value_preserves_receiver_trust( + value, + trusted_names, + finalizer_safe_names, + ) + if isinstance(value, ast.Call): + self._record_bound_shell_call(value, trusted_names) + if isinstance(value, ast.Call) and _is_direct_subprocess_call(value, trusted_names): + resolved = None + safe_value = _call_arguments_are_passive(value) + if _shell_argument_is_captured_before_effects(value): + self._inspect_call(value, facts) + if safe_value: + call_has_protocol_effects = not _call_arguments_are_protocol_safe( + value, + finalizer_safe_names, + ) + else: + resolved = _truth_value(value, facts) + safe_value = resolved is not None or _is_passive_argument(value) + + if not safe_value or not simple_targets: + facts.clear() + finalizer_safe_names.clear() + for target in targets: + if isinstance(target, ast.Name): + bound_names.add(target.id) + if not preserves_receiver_trust: + trusted_names.clear() + unknown_unsafe_bindings[0] = True + else: + trusted_names.difference_update( + _changed_direct_names([value, *targets], trusted_names) + ) + return + if releases_unsafe_value: + facts.clear() + finalizer_safe_names.clear() + trusted_names.clear() + unknown_unsafe_bindings[0] = True + if call_has_protocol_effects: + facts.clear() + finalizer_safe_names.clear() + trusted_names.clear() + unknown_unsafe_bindings[0] = True + for target in targets: + assert isinstance(target, ast.Name) + bound_names.add(target.id) + if releases_unsafe_value or call_has_protocol_effects or resolved is None: + facts.pop(target.id, None) + else: + facts[target.id] = resolved + if releases_unsafe_value or call_has_protocol_effects or not result_is_finalizer_safe: + finalizer_safe_names.discard(target.id) + else: + finalizer_safe_names.add(target.id) + preserves_binding = ( + isinstance(value, ast.Name) and value.id == target.id and value.id in trusted_names + ) + if not preserves_binding: + trusted_names.discard(target.id) + + def _scan_block( + self, + statements: list[ast.stmt], + *, + trusted_names: set[str] | None = None, + initial_bound_names: set[str] | None = None, + nested_function_trusted_names: set[str] | None = None, + nested_function_trusted_at_call: dict[int, set[str]] | None = None, + cached_subprocess: _CachedSubprocessState | None = None, + ) -> None: + previous_cached = self.cached_subprocess + self.cached_subprocess = (cached_subprocess or previous_cached).copy() + initial_cached = self.cached_subprocess.copy() + trusted_names = set(_DIRECT_CALL_NAMES if trusted_names is None else trusted_names) + facts: dict[str, bool] = {} + bound_names = set(initial_bound_names or ()) + finalizer_safe_names: set[str] = set() + unknown_unsafe_bindings = [False] + + last_invalidation_by_name: dict[str, int] = {} + receiver_trust = set(trusted_names) + receiver_bound_names = set(initial_bound_names or ()) + receiver_finalizer_safe_names: set[str] = set() + receiver_unknown_unsafe_bindings = [False] + for candidate_index, candidate in enumerate(statements): + before = set(receiver_trust) + _advance_trusted_names( + candidate, + receiver_trust, + receiver_bound_names, + receiver_finalizer_safe_names, + receiver_unknown_unsafe_bindings, + ) + for name in before.difference(receiver_trust): + last_invalidation_by_name[name] = candidate_index + + trusted_at_call_by_definition: dict[int, set[str]] = {} + receiver_trust = set(trusted_names) + receiver_bound_names = set(initial_bound_names or ()) + receiver_finalizer_safe_names = set() + receiver_unknown_unsafe_bindings = [False] + active_functions: dict[str, int] = {} + cached_at_call_by_definition: dict[int, _CachedSubprocessState] = {} + deferred_cached = initial_cached.copy() + for candidate_index, candidate in enumerate(statements): + call = _passive_direct_call(candidate) + if call is not None: + assert isinstance(call.func, ast.Name) + owner = active_functions.get(call.func.id) + if owner is not None: + trusted_at_call_by_definition.setdefault(owner, set()).update(receiver_trust) + prior_cached = cached_at_call_by_definition.get(owner) + if prior_cached is None: + cached_at_call_by_definition[owner] = deferred_cached.copy() + else: + prior_cached.changed_methods.intersection_update( + deferred_cached.changed_methods + ) + prior_cached.changed_popen &= deferred_cached.changed_popen + prior_cached.effect_generation = max( + prior_cached.effect_generation, deferred_cached.effect_generation + ) + + changed_names = _direct_bound_names(candidate) + for name in changed_names: + active_functions.pop(name, None) + if ( + isinstance(candidate, (ast.FunctionDef, ast.AsyncFunctionDef)) + and _function_header_is_passive(candidate) + and _is_immediate_function(candidate) + ): + active_functions[candidate.name] = candidate_index + _advance_trusted_names( + candidate, + receiver_trust, + receiver_bound_names, + receiver_finalizer_safe_names, + receiver_unknown_unsafe_bindings, + ) + deferred_cached.advance(candidate) + + for index, statement in enumerate(statements): + if isinstance(statement, (ast.Expr, ast.Return, ast.Assign, ast.AnnAssign)): + if statement.value is not None: + self._record_eager_cached_replacements(statement.value) + if isinstance(statement, (ast.FunctionDef, ast.AsyncFunctionDef)): + passive_header = _function_header_is_passive(statement) + if nested_function_trusted_names is None: + trusted_at_call = trusted_at_call_by_definition.get(index, set()) + nested_trusted_names = set(trusted_names).union(trusted_at_call) + nested_trusted_names = { + name + for name in nested_trusted_names + if last_invalidation_by_name.get(name, -1) <= index + or name in trusted_at_call + } + else: + nested_trusted_names = set(nested_function_trusted_names).union( + (nested_function_trusted_at_call or {}).get(index, set()) + ) + nested_trusted_names.difference_update( + _function_bound_direct_names(statement, nested_trusted_names) + ) + nested_trusted_names.discard(statement.name) + if not passive_header: + nested_trusted_names.clear() + nested_cached = cached_at_call_by_definition.get(index, deferred_cached).copy() + nested_cached.module_names.difference_update( + _function_bound_direct_names(statement, nested_cached.module_names) + ) + local_bound = _function_bound_direct_names(statement, nested_cached.bound_names) + nested_cached.bound_names.difference_update(local_bound) + nested_cached.safe_names.difference_update(local_bound) + parameters = _function_parameter_names(statement) + nested_cached.bound_names.update(parameters) + nested_cached.safe_names.difference_update(parameters) + self._scan_block( + statement.body, + trusted_names=nested_trusted_names, + initial_bound_names=_function_parameter_names(statement), + cached_subprocess=nested_cached, + ) + releases_unsafe_value = ( + statement.name in bound_names and statement.name not in finalizer_safe_names + ) + if passive_header and not releases_unsafe_value: + facts.pop(statement.name, None) + else: + facts.clear() + finalizer_safe_names.clear() + trusted_names.clear() + unknown_unsafe_bindings[0] = True + bound_names.add(statement.name) + finalizer_safe_names.discard(statement.name) + trusted_names.discard(statement.name) + elif isinstance(statement, (ast.Import, ast.ImportFrom)): + imported_names = _direct_bound_names(statement) + releases_unsafe_value = any( + name in bound_names and name not in finalizer_safe_names + for name in imported_names + ) + facts.clear() + finalizer_safe_names.difference_update(imported_names) + bound_names.update(imported_names) + _update_trusted_names_from_import(statement, trusted_names) + imports_unknown_names = isinstance(statement, ast.ImportFrom) and any( + imported.name == "*" for imported in statement.names + ) + if releases_unsafe_value or imports_unknown_names: + unknown_unsafe_bindings[0] = True + if releases_unsafe_value or unknown_unsafe_bindings[0]: + trusted_names.clear() + elif isinstance(statement, ast.Assign): + self._scan_assignment( + list(statement.targets), + statement.value, + facts, + trusted_names, + bound_names, + finalizer_safe_names, + unknown_unsafe_bindings, + ) + elif isinstance(statement, ast.AnnAssign): + value = statement.value + target = statement.target + simple_target = isinstance(target, ast.Name) + releases_unsafe_value = ( + value is not None + and simple_target + and target.id in bound_names + and target.id not in finalizer_safe_names + and not (isinstance(value, ast.Name) and value.id == target.id) + ) + result_is_finalizer_safe = value is not None and _is_finalizer_safe_value( + value, + finalizer_safe_names, + ) + preserves_receiver_trust = ( + simple_target + and _annotation_is_passive(statement.annotation) + and ( + value is None + or _value_preserves_receiver_trust( + value, + trusted_names, + finalizer_safe_names, + ) + ) + ) + if isinstance(value, ast.Call): + self._record_bound_shell_call(value, trusted_names) + if isinstance(value, ast.Call) and _is_direct_subprocess_call(value, trusted_names): + if _shell_argument_is_captured_before_effects(value): + self._inspect_call(value, facts) + facts.clear() + if not preserves_receiver_trust or releases_unsafe_value: + finalizer_safe_names.clear() + if value is not None: + bound_names.update(_direct_bound_names(statement)) + if ( + simple_target + and preserves_receiver_trust + and not releases_unsafe_value + and result_is_finalizer_safe + ): + finalizer_safe_names.add(target.id) + elif simple_target: + finalizer_safe_names.discard(target.id) + if not preserves_receiver_trust or releases_unsafe_value: + trusted_names.clear() + unknown_unsafe_bindings[0] = True + else: + trusted_names.difference_update( + _changed_direct_names([statement], trusted_names) + ) + elif isinstance(statement, (ast.AugAssign, ast.Delete)): + facts.clear() + finalizer_safe_names.clear() + bound_names.update(_direct_bound_names(statement)) + trusted_names.clear() + unknown_unsafe_bindings[0] = True + elif isinstance(statement, ast.Expr) and isinstance(statement.value, ast.Call): + call = statement.value + self._record_bound_shell_call(call, trusted_names) + direct_call = _is_direct_subprocess_call(call, trusted_names) + if direct_call and _shell_argument_is_captured_before_effects(call): + self._inspect_call(call, facts) + arguments_are_passive = _call_arguments_are_passive(call) + if direct_call and arguments_are_passive: + if not _call_arguments_are_protocol_safe(call, finalizer_safe_names): + facts.clear() + finalizer_safe_names.clear() + trusted_names.clear() + unknown_unsafe_bindings[0] = True + else: + facts.clear() + finalizer_safe_names.clear() + trusted_names.clear() + unknown_unsafe_bindings[0] = True + elif isinstance(statement, (ast.Global, ast.Nonlocal, ast.Pass)) or ( + isinstance(statement, ast.Expr) and isinstance(statement.value, ast.Constant) + ): + continue + elif isinstance(statement, ast.Expr): + preserves_receiver_trust = _value_preserves_receiver_trust( + statement.value, + trusted_names, + finalizer_safe_names, + ) + bound_names.update(_direct_bound_names(statement)) + if preserves_receiver_trust: + trusted_names.difference_update( + _changed_direct_names([statement], trusted_names) + ) + else: + facts.clear() + finalizer_safe_names.clear() + trusted_names.clear() + unknown_unsafe_bindings[0] = True + elif isinstance(statement, ast.Assert): + expressions = [statement.test] + if statement.msg is not None: + expressions.append(statement.msg) + preserves_receiver_trust = all( + _value_preserves_receiver_trust( + item, + trusted_names, + finalizer_safe_names, + ) + for item in expressions + ) and _is_finalizer_safe_value(statement.test, finalizer_safe_names) + facts.clear() + if not preserves_receiver_trust: + finalizer_safe_names.clear() + trusted_names.clear() + unknown_unsafe_bindings[0] = True + elif isinstance(statement, ast.ClassDef): + releases_unsafe_value = ( + statement.name in bound_names and statement.name not in finalizer_safe_names + ) + class_trusted_names = set(trusted_names) + passive_class_header = _class_header_is_passive(statement) + if not passive_class_header: + class_trusted_names.clear() + method_outer_trust = set( + trusted_names + if nested_function_trusted_names is None + else nested_function_trusted_names + ) + if nested_function_trusted_names is None: + method_outer_trust = { + name + for name in method_outer_trust + if last_invalidation_by_name.get(name, -1) <= index + } + if not passive_class_header: + method_outer_trust.clear() + method_trusted_names, method_trusted_at_call = _class_deferred_receiver_trust( + statement, + method_outer_trust, + ) + self._scan_block( + statement.body, + trusted_names=class_trusted_names, + nested_function_trusted_names=method_trusted_names, + nested_function_trusted_at_call=method_trusted_at_call, + cached_subprocess=self.cached_subprocess, + ) + facts.clear() + bound_names.update(_direct_bound_names(statement)) + unsafe_class_execution = ( + not passive_class_header + or releases_unsafe_value + or _class_body_has_eager_effects(statement) + ) + if unsafe_class_execution: + finalizer_safe_names.clear() + trusted_names.clear() + unknown_unsafe_bindings[0] = True + else: + finalizer_safe_names.discard(statement.name) + trusted_names.difference_update( + _changed_direct_names([statement], trusted_names) + ) + trusted_names.difference_update( + _class_body_changed_direct_names(statement, trusted_names) + ) + else: + facts.clear() + finalizer_safe_names.clear() + bound_names.update(_direct_bound_names(statement)) + trusted_names.clear() + unknown_unsafe_bindings[0] = True + self.cached_subprocess.advance(statement) + + self.cached_subprocess = previous_cached + + def run(self, tree: ast.Module) -> list[AnalyzerFinding]: + self._scan_block(tree.body) + return sorted(self.findings, key=lambda finding: finding.location.start_line) + + +def analyze( + content: str, + file_path: str, + file_type: str, + *, + python_ast: ParsedPythonFile | None = None, +) -> list[AnalyzerFinding]: + """Find straight-line truthy names passed to direct subprocess calls.""" + if file_type != "python": + return [] + parsed = python_ast or parse_python_source(content, file_path) + if parsed.tree is None: + return [] + return _Analyzer(file_path, parsed).run(parsed.tree) + + +def bound_shell_call_analysis( + file_path: str, + python_ast: ParsedPythonFile, +) -> tuple[dict[BoundShellCallKey, bool], set[BoundShellCallKey], set[BoundShellCallKey]]: + """Separate trust, detections, and affirmative cached-slot replacements.""" + if python_ast.tree is None: + return {}, set(), set() + analyzer = _Analyzer(file_path, python_ast) + analyzer.run(python_ast.tree) + return ( + dict(analyzer.bound_shell_call_ownership), + set(analyzer.emitted_shell_calls), + {key for key, replaced in analyzer.cached_replacement_by_call.items() if replaced}, + ) + + +def bound_shell_call_state( + file_path: str, + python_ast: ParsedPythonFile, +) -> tuple[dict[BoundShellCallKey, bool], set[BoundShellCallKey]]: + """Return receiver trust separately from affirmative companion detections.""" + ownership, emitted, _ = bound_shell_call_analysis(file_path, python_ast) + return ownership, emitted + + +def bound_shell_call_ownership( + file_path: str, + python_ast: ParsedPythonFile, +) -> dict[BoundShellCallKey, bool]: + """Return supported bound-shell calls and whether their receiver is trusted.""" + if python_ast.tree is None: + return {} + analyzer = _Analyzer(file_path, python_ast) + analyzer.run(python_ast.tree) + return dict(analyzer.bound_shell_call_ownership) diff --git a/src/skillspector/nodes/analyzers/static_runner.py b/src/skillspector/nodes/analyzers/static_runner.py index ccd82281b..ab554bf9c 100644 --- a/src/skillspector/nodes/analyzers/static_runner.py +++ b/src/skillspector/nodes/analyzers/static_runner.py @@ -916,6 +916,8 @@ def _scan_path( analyze_kwargs["python_ast"] = python_ast if _uses_runtime_check(module): analyze_kwargs["check_runtime"] = finding_budget.check_runtime + if _explicit_module_hook(module, "ANALYZE_USES_POSTPROCESS") is True: + analyze_kwargs["defer_variable_reconciliation"] = True raw = module.analyze(**analyze_kwargs) finding_budget.check_runtime() for af in raw: diff --git a/tests/nodes/analyzers/test_python_shell_cached_imports.py b/tests/nodes/analyzers/test_python_shell_cached_imports.py new file mode 100644 index 000000000..52f0d008c --- /dev/null +++ b/tests/nodes/analyzers/test_python_shell_cached_imports.py @@ -0,0 +1,255 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""Cached module slots retain their state across imports in deferred scopes.""" + +from __future__ import annotations + +import pytest + +from skillspector.nodes.analyzers import static_python_shell_truthiness as truthiness +from skillspector.python_ast import parse_python_source + + +@pytest.mark.parametrize( + "source", + [ + "import subprocess\nsubprocess.run = proxy\n" + "def run():\n import subprocess\n enabled = True\n" + " subprocess.run(command, shell=enabled)\nrun()\n", + "import subprocess\nsubprocess.Popen = proxy\n" + "def run():\n from subprocess import Popen\n enabled = True\n" + " Popen(command, shell=enabled)\nrun()\n", + "import subprocess\ndef outer():\n subprocess.run = proxy\n" + " def inner():\n import subprocess\n enabled = True\n" + " subprocess.run(command, shell=enabled)\n inner()\nouter()\n", + "import subprocess\nsubprocess.run = proxy\nclass Tool:\n" + " def run(self):\n import subprocess\n enabled = True\n" + " subprocess.run(command, shell=enabled)\n", + "import subprocess\nclass Tool:\n def alter(self):\n" + " global subprocess\n subprocess = proxy\n" + "subprocess.run = handler\ndef run():\n import subprocess\n enabled = 1\n" + " subprocess.run(command, shell=enabled)\n", + ], +) +def test_local_import_does_not_restore_changed_cached_module_slot(source: str) -> None: + assert truthiness.analyze(source, "run.py", "python") == [] + + +@pytest.mark.parametrize( + "source", + [ + "import subprocess\ndef run():\n import subprocess\n enabled = True\n" + " subprocess.run(command, shell=enabled)\nrun()\nsubprocess.run = proxy\n", + "import subprocess\ndef alter():\n subprocess.run = proxy\n" + "def run():\n import subprocess\n enabled = True\n" + " subprocess.run(command, shell=enabled)\nrun()\n", + "subprocess = proxy\nsubprocess.run = proxy.run\ndef run():\n" + " import subprocess\n enabled = True\n" + " subprocess.run(command, shell=enabled)\nrun()\n", + "import subprocess\nsubprocess.Popen = proxy\ndef run():\n" + " import subprocess\n enabled = True\n" + " subprocess.run(command, shell=enabled)\nrun()\n", + "import subprocess\ndef outer(subprocess):\n subprocess.run = proxy\n" + " def inner():\n import subprocess\n enabled = 1\n" + " subprocess.run(command, shell=enabled)\n inner()\n", + "import subprocess\nclass Tool:\n global subprocess\n subprocess = proxy\n" + "subprocess.run = handler\ndef run():\n import subprocess\n enabled = 1\n" + " subprocess.run(command, shell=enabled)\n", + "import subprocess\nclass Outer:\n class Inner:\n" + " global subprocess\n subprocess = proxy\n" + "subprocess.run = handler\ndef run():\n import subprocess\n enabled = 1\n" + " subprocess.run(command, shell=enabled)\n", + "import subprocess\nsubprocess.run = (subprocess := proxy)\nimport subprocess\n" + "def run():\n import subprocess\n enabled = 1\n" + " subprocess.run(command, shell=enabled)\nrun()\n", + "import subprocess\ndef replace_receiver():\n global subprocess\n" + " subprocess = proxy\n return handler\n" + "subprocess.run = replace_receiver()\nimport subprocess\ndef run():\n" + " import subprocess\n enabled = 1\n" + " subprocess.run(command, shell=enabled)\nrun()\n", + ], +) +def test_cached_module_tracking_preserves_observed_and_unaffected_calls(source: str) -> None: + findings = truthiness.analyze(source, "run.py", "python") + assert len(findings) == 1 + assert findings[0].severity == "HIGH" + + +@pytest.mark.parametrize( + ("source", "replacement"), + [ + ( + "import subprocess\nsubprocess.run = proxy\nenabled = True\n" + "subprocess.run(command, shell=enabled)\n", + True, + ), + ( + "import subprocess\nsubprocess.Popen = proxy\nfrom subprocess import Popen\n" + "enabled = True\nPopen(command, shell=enabled)\n", + True, + ), + ( + "from subprocess import Popen\nimport subprocess\nsubprocess.Popen = proxy\n" + "enabled = True\nPopen(command, shell=enabled)\n", + False, + ), + ( + "import subprocess\nhelper()\nenabled = True\nsubprocess.run(command, shell=enabled)\n", + False, + ), + ( + "import subprocess\ndef run(subprocess, command):\n" + " subprocess.run = proxy\n import subprocess\n enabled = 1\n" + " subprocess.run(command, shell=enabled)\n", + False, + ), + ( + "import subprocess\nsubprocess.run = proxy\ntrue_flag = True\n" + "subprocess.run(command, shell=true_flag)\n", + True, + ), + ( + "import subprocess\ndef run():\n import subprocess\n enabled = True\n" + " subprocess.run(command, shell=enabled)\nrun()\n" + "subprocess.run = proxy\nrun()\n", + False, + ), + ], +) +def test_cached_replacement_is_separate_from_unknown_receiver_trust( + source: str, replacement: bool +) -> None: + parsed = parse_python_source(source, "run.py") + ownership, emitted, replacements = truthiness.bound_shell_call_analysis("run.py", parsed) + assert len(ownership) == 1 + assert bool(replacements) is replacement + assert replacements.issubset(ownership) + assert truthiness.bound_shell_call_state("run.py", parsed) == (ownership, emitted) + + +def test_cached_replacement_analysis_keeps_invalid_python_empty() -> None: + parsed = parse_python_source("def incomplete(", "run.py") + assert truthiness.bound_shell_call_analysis("run.py", parsed) == ({}, set(), set()) + + +@pytest.mark.parametrize( + "prefix", + [ + "box[redirect()] = subprocess.run = handler\n", + "def unused(default=(subprocess := proxy)):\n pass\nsubprocess.run = handler\n", + "class Tool((subprocess := proxy)):\n pass\nsubprocess.run = handler\n", + "original = subprocess.run\ndef restore():\n subprocess.run = original\n" + "subprocess.run = handler\nrestore()\n", + "original = subprocess.run\ndef restore():\n subprocess.run = original\n" + "subprocess.run = handler\ndef unused(default=restore()):\n pass\n", + "subprocess.run = proxy\nsubprocess.__class__ = Restoring\n", + "subprocess.__class__ = Restoring\nimport subprocess\nsubprocess.run = proxy\n", + "restorer = Untrusted()\nimport subprocess\nsubprocess.run = proxy\ndel restorer\n", + "subprocess.run = proxy\ndel subprocess.run\n", + "subprocess.run = proxy\nsubprocess.run = handler\n", + "subprocess.run = proxy\nimport restore_module\n", + "subprocess.run = proxy\nfrom restore_module import restored\n", + "subprocess.run = proxy\nimport subprocess, restore_module\n", + "restorer = Untrusted()\nimport subprocess\nsubprocess.run = proxy\nrestorer = 0\n", + ], +) +def test_unknown_eager_effect_invalidates_prior_replacement_proof(prefix: str) -> None: + source = ( + "import subprocess\n" + prefix + "import subprocess\ndef work():\n" + " import subprocess\n enabled = 1\n" + " subprocess.run(command, shell=enabled)\nwork()\n" + ) + assert len(truthiness.analyze(source, "run.py", "python")) == 1 + _, _, replacements = truthiness.bound_shell_call_analysis( + "run.py", parse_python_source(source, "run.py") + ) + assert not replacements + + +@pytest.mark.parametrize( + ("expression", "replacement"), + [ + ("return Popen(command, shell=enabled)", True), + ("wrapper(Popen(command, shell=enabled))", True), + ("return [Popen(command, shell=enabled)]", True), + ("return wrapper(helper(), Popen(command, shell=enabled))", False), + ("return lambda: Popen(command, shell=enabled)", False), + ("return (Popen(command, shell=enabled) for item in items)", False), + ], +) +def test_cached_replacement_proof_respects_eager_expression_order( + expression: str, replacement: bool +) -> None: + source = ( + "import subprocess\nsubprocess.Popen = proxy\ndef run():\n" + " from subprocess import Popen\n enabled = True\n" + f" {expression}\n" + ) + _, emitted, replacements = truthiness.bound_shell_call_analysis( + "run.py", parse_python_source(source, "run.py") + ) + assert bool(replacements) is replacement + assert not emitted + + +@pytest.mark.parametrize( + "source", + [ + "restorer = 0\nimport subprocess\nclass Tool:\n" + " helper()\n import subprocess\n subprocess.run = proxy\n" + "restorer = 0\nenabled = True\nsubprocess.run(command, shell=enabled)\n", + "helper()\nimport subprocess\nsubprocess.run = proxy\nrestorer = 0\n" + "enabled = True\nsubprocess.run(command, shell=enabled)\n", + "import subprocess\nenabled = True\nhelper()\nimport subprocess\n" + "subprocess.run = proxy\nsubprocess.run(command, shell=enabled)\n", + "import subprocess\nsubprocess.run = proxy\ndef work():\n" + " import subprocess\n enabled = 1\n" + " subprocess.run(command, shell=enabled)\nwork()\nhelper()\n" + "import subprocess\nsubprocess.run = proxy\nwork()\n", + ], + ids=["class-outer-finalizer", "unknown-new-binding", "unknown-protocol", "later-observation"], +) +def test_unknown_effect_cannot_reestablish_cached_replacement_proof(source: str) -> None: + # Reimporting the same cached module does not establish that an unknown + # earlier effect left its lookup protocol or other finalizer bindings intact. + ownership, _, replacements = truthiness.bound_shell_call_analysis( + "run.py", parse_python_source(source, "run.py") + ) + assert len(ownership) == 1 + assert not replacements + + +@pytest.mark.parametrize( + ("prefix", "method"), + [ + ("original = subprocess.run\nsubprocess.run = original\n", "run"), + ("subprocess.run = subprocess.check_call\n", "run"), + ( + "native_module = subprocess\noriginal = native_module.run\nsubprocess.run = original\n", + "run", + ), + ( + "import subprocess as native_module\nsubprocess.run = native_module.check_output\n", + "run", + ), + ("original, = (subprocess.run,)\nsubprocess.run = original\n", "run"), + ("[original] = [subprocess.run]\nsubprocess.run = original\n", "run"), + ("from subprocess import Popen\nsubprocess.run = Popen\n", "run"), + ("from subprocess import Popen\noriginal = Popen\nsubprocess.run = original\n", "run"), + ("subprocess.Popen = subprocess.run\n", "Popen"), + ], +) +def test_native_callable_value_does_not_prove_a_custom_replacement( + prefix: str, method: str +) -> None: + source = ( + "import subprocess\n" + prefix + "def work():\n" + " import subprocess\n enabled = True\n" + f" subprocess.{method}(command, shell=enabled)\nwork()\n" + ) + assert len(truthiness.analyze(source, "run.py", "python")) == 1 + _, _, replacements = truthiness.bound_shell_call_analysis( + "run.py", parse_python_source(source, "run.py") + ) + assert not replacements diff --git a/tests/nodes/analyzers/test_python_string_marker_ownership.py b/tests/nodes/analyzers/test_python_string_marker_ownership.py index 237e28cc0..eb691d054 100644 --- a/tests/nodes/analyzers/test_python_string_marker_ownership.py +++ b/tests/nodes/analyzers/test_python_string_marker_ownership.py @@ -432,6 +432,15 @@ def test_marker_declared_inside_a_python_token_still_fails_closed(content: str, ], ) def test_glued_marker_outside_proven_python_still_fails_closed(path: str, content: str) -> None: + if path.endswith(".py"): + # Tool misuse parses Python for TM1 reconciliation, so the + # higher-precedence syntax error names its partial event. A lexical-only + # analyzer still reports the unproven marker reading. + _assert_partial_marker_text(_ledger(path, content, ar_module)) + event = _ledger(path, content)["inspection_ledger"][0] + assert event["outcome"] is LedgerOutcome.PARTIAL + assert event["reason_code"] is LedgerReason.SYNTAX_ERROR + return _assert_partial_marker_text(_ledger(path, content)) diff --git a/tests/nodes/analyzers/test_shared_python_ast.py b/tests/nodes/analyzers/test_shared_python_ast.py index 052a52cfe..68d71ce1d 100644 --- a/tests/nodes/analyzers/test_shared_python_ast.py +++ b/tests/nodes/analyzers/test_shared_python_ast.py @@ -16,6 +16,7 @@ behavioral_taint_tracking, static_patterns_data_exfiltration, static_patterns_output_handling, + static_patterns_tool_misuse, static_runner, ) from skillspector.nodes.build_context import build_context @@ -59,11 +60,12 @@ def code(tail: str) -> str: def test_preparsed_python_is_reused_by_all_ast_analyzers(tmp_path, monkeypatch) -> None: """One scan parses each eligible Python file once before analyzer fan-out.""" (tmp_path / "script.py").write_text( - "import os\n" "import subprocess\n" + "enabled = True\n" + "subprocess.run(output, shell=enabled)\n" + "import os\n" "payload = input()\n" "environment = os.environ.copy()\n" - "subprocess.run(output)\n" "exec(payload)\n", encoding="utf-8", ) @@ -91,11 +93,13 @@ def count_parse(*args, **kwargs): data_findings = static_patterns_data_exfiltration.node(state)["findings"] output_findings = static_patterns_output_handling.node(state)["findings"] + tool_misuse_findings = static_patterns_tool_misuse.node(state)["findings"] ast_findings = behavioral_ast.node(state)["findings"] taint_findings = behavioral_taint_tracking.node(state)["findings"] assert any(finding.rule_id == "E2" for finding in data_findings) assert any(finding.rule_id == "OH1" for finding in output_findings) + assert any(finding.rule_id == "TM1" for finding in tool_misuse_findings) assert any(finding.rule_id == "AST1" for finding in ast_findings) assert any(finding.rule_id == "TT5" for finding in taint_findings) assert parse_calls == 1 diff --git a/tests/nodes/analyzers/test_tm1_lexical_abstention.py b/tests/nodes/analyzers/test_tm1_lexical_abstention.py new file mode 100644 index 000000000..35b9fa268 --- /dev/null +++ b/tests/nodes/analyzers/test_tm1_lexical_abstention.py @@ -0,0 +1,633 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""Lexical TM1 remains a signal when conservative Python dataflow abstains.""" + +from __future__ import annotations + +import pytest + +from skillspector.nodes.analyzers import static_patterns_tool_misuse as tool_misuse +from skillspector.nodes.analyzers import static_python_shell_truthiness as truthiness +from skillspector.python_ast import parse_python_source + + +def _findings(source: str) -> list: + response = tool_misuse.node({"components": ["run.py"], "file_cache": {"run.py": source}}) + return [finding for finding in response["findings"] if finding.rule_id == "TM1"] + + +@pytest.mark.parametrize( + "source", + [ + pytest.param( + "import subprocess\nif True:\n enabled = True\n subprocess.run(command, shell=enabled)\n", + id="literal-compound", + ), + pytest.param( + 'import subprocess\ndef run(command):\n enabled = True\n subprocess.run(command, shell=enabled)\nif __name__ == "__main__":\n run("ls")\n', + id="ordinary-main-guard", + ), + pytest.param( + 'import subprocess\ncommand = f"python {sys.argv[1]}"\nenabled = True\nsubprocess.run(command, shell=enabled)\n', + id="formatted-command-before-binding", + ), + pytest.param( + "import subprocess\ndef run(command):\n enabled = True\n return subprocess.run(command, shell=enabled)\n", + id="return-call", + ), + pytest.param( + "import subprocess\nif False:\n enabled = False\nenabled = True\nsubprocess.run(command, shell=enabled)\n", + id="untaken-flag-store", + ), + pytest.param( + "import subprocess\nenabled = True\nif False:\n subprocess = proxy\nsubprocess.run(command, shell=enabled)\n", + id="untaken-receiver-store", + ), + pytest.param( + "import subprocess\nenabled = True\nfor subprocess in values:\n pass\nsubprocess.run(command, shell=enabled)\n", + id="possibly-empty-receiver-loop", + ), + pytest.param( + "import subprocess\nenabled = True\nunknown_effect()\nsubprocess = subprocess\nsubprocess.run(command, shell=enabled)\n", + id="self-store-retains-binding", + ), + pytest.param( + "import subprocess\nenabled = True\nunknown_effect()\nenabled: bool = True\nsubprocess.run(command, shell=enabled)\n", + id="annotated-true-after-unknown", + ), + ], +) +def test_companion_abstention_keeps_lexical_high(source: str) -> None: + findings = _findings(source) + assert len(findings) == 1 + assert findings[0].severity == "HIGH" + assert "shell=enabled" in (findings[0].matched_text or "") + assert not findings[0].evidence + + +@pytest.mark.parametrize( + "prefix", + ["payload = input()", "sys.path.insert(0, path)", "load_dotenv()", "ignored = (helper(),)"], +) +@pytest.mark.parametrize("placement", ["module", "function"]) +def test_fresh_import_after_unknown_effect_keeps_lexical_high(prefix: str, placement: str) -> None: + source = f"import os\n{prefix}\nimport subprocess\n" + if placement == "function": + source += ( + "def run(command):\n enabled = True\n subprocess.run(command, shell=enabled)\n" + ) + else: + source += "enabled = True\nsubprocess.run(command, shell=enabled)\n" + assert len(_findings(source)) == 1 + assert _findings(source)[0].severity == "HIGH" + + +@pytest.mark.parametrize( + "effect", + [ + "helper()", + "result = helper()", + "result = (helper(),)", + "result: object = [helper()]", + "assert (helper(),)", + "result = f'{value}'", + "[value for value in values]", + ], +) +@pytest.mark.parametrize("placement", ["module", "function", "class", "deferred"]) +def test_generic_eager_effect_does_not_prove_receiver_replacement( + effect: str, placement: str +) -> None: + body = f"{effect}\nenabled = True\nsubprocess.run(command, shell=enabled)\n" + if placement in {"function", "class"}: + header = "def run():\n" if placement == "function" else "class Run:\n" + source = ( + "import subprocess\n" + + header + + "".join(" " + line + "\n" for line in body.splitlines()) + ) + elif placement == "deferred": + source = ( + "import subprocess\ndef run():\n enabled = True\n subprocess.run(command, shell=enabled)\n" + + effect + + "\nrun()\n" + ) + else: + source = "import subprocess\n" + body + # Preserve conservative receiver invalidation in the AST companion itself. + assert not truthiness.analyze(source, "run.py", "python") + findings = _findings(source) + assert len(findings) == 1 + assert findings[0].severity == "HIGH" + + +@pytest.mark.parametrize( + "replacement", + [ + "enabled = False", + "enabled = dynamic", + "del enabled", + "import settings as enabled", + "from settings import enabled", + "enabled: bool = False", + ], +) +def test_observed_flag_replacement_removes_stale_lexical_binding(replacement: str) -> None: + source = f"import subprocess\nenabled = True\n{replacement}\nsubprocess.run(command, shell=enabled)\n" + assert not _findings(source) + + +@pytest.mark.parametrize( + "replacement", + [ + "subprocess = proxy", + "subprocess = True", + "subprocess.run = proxy", + "del subprocess", + "import settings as subprocess", + ], +) +def test_observed_receiver_replacement_removes_lexical_candidate(replacement: str) -> None: + source = f"import subprocess\n{replacement}\nenabled = True\nsubprocess.run(command, shell=enabled)\n" + assert not _findings(source) + + +def test_fresh_direct_import_supersedes_explicit_receiver_replacement() -> None: + source = "subprocess = proxy\nhelper()\nimport subprocess\nenabled = True\nsubprocess.run(command, shell=enabled)\n" + findings = _findings(source) + assert len(findings) == 1 + assert findings[0].severity == "HIGH" + + +def test_positive_companion_decision_has_one_owner() -> None: + source = "import subprocess\nenabled = True\nsubprocess.run(command, shell=enabled)\n" + parsed = parse_python_source(source, "run.py") + ownership, emitted = truthiness.bound_shell_call_state("run.py", parsed) + assert len(emitted) == 1 + assert next(iter(ownership.values())) is True + assert len(_findings(source)) == 1 + + +def test_unknown_companion_decision_is_distinct_from_emission() -> None: + source = "import subprocess\nhelper()\nenabled = True\nsubprocess.run(command, shell=enabled)\n" + ownership, emitted = truthiness.bound_shell_call_state( + "run.py", parse_python_source(source, "run.py") + ) + assert next(iter(ownership.values())) is False + assert not emitted + assert len(_findings(source)) == 1 + + +def test_positive_dataflow_without_retained_finding_keeps_lexical_owner( + monkeypatch: pytest.MonkeyPatch, +) -> None: + source = "import subprocess\nenabled = True\nsubprocess.run(command, shell=enabled)\n" + monkeypatch.setattr(truthiness, "analyze", lambda *args, **kwargs: []) + findings = _findings(source) + assert len(findings) == 1 + assert findings[0].severity == "HIGH" + + +@pytest.mark.parametrize("replacement", ["enabled = False", "subprocess = proxy"]) +def test_uncalled_deferred_global_store_is_not_counterevidence(replacement: str) -> None: + name = replacement.partition(" =")[0] + source = ( + "payload = input()\nimport subprocess\nenabled = True\n" + f"def disable():\n global {name}\n {replacement}\n" + "subprocess.run(command, shell=enabled)\n" + ) + findings = _findings(source) + assert len(findings) == 1 + assert findings[0].severity == "HIGH" + + +@pytest.mark.parametrize( + "mutation", ["subprocess.label = 'documentation'", "subprocess.check_call = proxy"] +) +def test_unrelated_receiver_attribute_is_not_counterevidence(mutation: str) -> None: + source = ( + f"import subprocess\n{mutation}\nenabled = True\nsubprocess.run(command, shell=enabled)\n" + ) + findings = _findings(source) + assert len(findings) == 1 + assert findings[0].severity == "HIGH" + + +def test_reimport_restores_method_after_mutating_a_replaced_receiver() -> None: + source = ( + "subprocess = proxy\nsubprocess.run = proxy.run\nimport subprocess\n" + "enabled = True\nsubprocess.run(command, shell=enabled)\n" + ) + findings = _findings(source) + assert len(findings) == 1 + assert findings[0].severity == "HIGH" + + +@pytest.mark.parametrize( + "assignment", + [ + "subprocess.run = subprocess.run", + "subprocess.run: object = subprocess.run", + "subprocess.run = saved = subprocess.run", + "saved = subprocess.run = subprocess.run", + "subprocess.run, saved = subprocess.run, 1", + "saved, [subprocess.run] = 1, [subprocess.run]", + "subprocess.run, subprocess.run = proxy, subprocess.run", + "[subprocess.run, subprocess.run] = [proxy, subprocess.run]", + "subprocess.run = [subprocess.run] = [subprocess.run]", + ], +) +@pytest.mark.parametrize("intervening", ["", "helper()\n"]) +def test_called_method_self_store_preserves_lexical_finding( + assignment: str, intervening: str +) -> None: + source = ( + f"import subprocess\nenabled = True\n{assignment}\n" + f"{intervening}subprocess.run(command, shell=enabled)\n" + ) + findings = _findings(source) + assert len(findings) == 1 + assert findings[0].severity == "HIGH" + + +@pytest.mark.parametrize( + ("intervening", "expected"), + [("import subprocess\n", False), ("helper()\nimport subprocess\n", True)], +) +def test_reimport_does_not_prove_slot_replacement_after_unknown_effect( + intervening: str, expected: bool +) -> None: + source = ( + "import subprocess\nsubprocess.run = proxy\n" + f"{intervening}enabled = True\nsubprocess.run(command, shell=enabled)\n" + ) + findings = _findings(source) + assert bool(findings) is expected + if expected: + assert findings[0].severity == "HIGH" + + +def test_later_fresh_import_restores_deferred_receiver_signal() -> None: + source = ( + "import subprocess\ndef run():\n enabled = True\n" + " subprocess.run(command, shell=enabled)\n" + "subprocess = proxy\nimport subprocess\nrun()\n" + ) + findings = _findings(source) + assert len(findings) == 1 + assert findings[0].severity == "HIGH" + + +@pytest.mark.parametrize( + "annotation", ["enabled: bool", "subprocess: object", "subprocess.run: object"] +) +def test_annotation_without_value_is_not_runtime_counterevidence(annotation: str) -> None: + source = ( + f"import subprocess\nenabled = True\n{annotation}\nsubprocess.run(command, shell=enabled)\n" + ) + findings = _findings(source) + assert len(findings) == 1 + assert findings[0].severity == "HIGH" + + +@pytest.mark.parametrize( + "replacement", + ["enabled = False\nenabled = enabled", "subprocess = proxy\nsubprocess = subprocess"], +) +def test_self_store_does_not_undo_observed_replacement(replacement: str) -> None: + source = f"import subprocess\nenabled = True\n{replacement}\nsubprocess.run(command, shell=enabled)\n" + assert not _findings(source) + + +def test_future_module_method_replacement_suppresses_deferred_call() -> None: + source = ( + "import subprocess\ndef run():\n enabled = True\n" + " subprocess.run(command, shell=enabled)\nsubprocess.run = proxy\nrun()\n" + ) + assert not _findings(source) + + +def test_later_method_store_in_same_function_preserves_earlier_call() -> None: + source = ( + "import subprocess\ndef run():\n helper()\n enabled = True\n" + " subprocess.run(command, shell=enabled)\n subprocess.run = proxy\n" + ) + findings = _findings(source) + assert len(findings) == 1 + assert findings[0].severity == "HIGH" + + +def test_local_import_restores_module_after_outer_proxy_slot_mutation() -> None: + source = ( + "subprocess = proxy\nsubprocess.run = handler\ndef run(command):\n" + " import subprocess\n enabled = True\n" + " subprocess.run(command, shell=enabled)\n" + ) + findings = _findings(source) + assert len(findings) == 1 + assert findings[0].severity == "HIGH" + + +def test_later_class_proxy_store_keeps_cached_replacement_unknown() -> None: + source = ( + "import subprocess\nclass Tool:\n subprocess.run = proxy\n" + " subprocess = other\n subprocess.run = other\n def run(self):\n" + " enabled = True\n subprocess.run(command, shell=enabled)\n" + ) + # The later write on an unknown proxy may have protocol side effects. + assert len(_findings(source)) == 1 + + +@pytest.mark.parametrize("local_import", [False, True]) +def test_bare_popen_import_captures_cached_module_slot(local_import: bool) -> None: + prefix = "import subprocess\nsubprocess.Popen = proxy\n" + body = "from subprocess import Popen\nenabled = True\nPopen(command, shell=enabled)\n" + source = prefix + ( + "def run():\n" + "".join(" " + line + "\n" for line in body.splitlines()) + if local_import + else body + ) + assert not _findings(source) + + +def test_bare_popen_import_before_module_mutation_keeps_captured_callable() -> None: + source = ( + "import subprocess\nfrom subprocess import Popen\nsubprocess.Popen = proxy\n" + "enabled = True\nPopen(command, shell=enabled)\n" + ) + findings = _findings(source) + assert len(findings) == 1 + assert findings[0].severity == "HIGH" + + +@pytest.mark.parametrize( + "assignment", + ["subprocess.run = subprocess = proxy", "subprocess.run, subprocess = proxy, other"], +) +def test_class_assignment_target_order_keeps_earlier_module_mutation(assignment: str) -> None: + source = ( + f"import subprocess\nclass Tool:\n {assignment}\n def run(self):\n" + " enabled = True\n subprocess.run(command, shell=enabled)\n" + ) + assert not _findings(source) + + +def test_class_assignment_target_order_ignores_later_proxy_slot_mutation() -> None: + source = ( + "import subprocess\nclass Tool:\n subprocess, subprocess.run = other, proxy\n" + " def run(self):\n enabled = True\n subprocess.run(command, shell=enabled)\n" + ) + findings = _findings(source) + assert len(findings) == 1 + assert findings[0].severity == "HIGH" + + +@pytest.mark.parametrize( + "statement", + [ + "return Popen(command, shell=enabled)", + "wrapper(Popen(command, shell=enabled))", + "return [Popen(command, shell=enabled)]", + ], +) +def test_local_popen_cached_replacement_in_eager_return_expression(statement: str) -> None: + source = ( + "import subprocess\nsubprocess.Popen = proxy\ndef run():\n" + " from subprocess import Popen\n enabled = True\n" + f" {statement}\nrun()\n" + ) + assert not _findings(source) + + +@pytest.mark.parametrize("intervening", ["", "restore()\n"]) +def test_unknown_helper_can_restore_a_changed_module_slot(intervening: str) -> None: + source = ( + "import subprocess\nsubprocess.run = proxy\n" + f"{intervening}enabled = True\nsubprocess.run(command, shell=enabled)\n" + ) + findings = _findings(source) + assert bool(findings) is bool(intervening) + if findings: + assert findings[0].severity == "HIGH" + + +@pytest.mark.parametrize("intervening", ["", "restore()\n", "import subprocess\n"]) +def test_direct_effectful_slot_store_has_only_local_lifetime(intervening: str) -> None: + source = ( + "subprocess.run = Proxy()\n" + + intervening + + "enabled = True\nsubprocess.run(command, shell=enabled)\n" + ) + assert bool(_findings(source)) is bool(intervening) + + +@pytest.mark.parametrize( + "statement", + [ + "subprocess.run: restore() = Proxy()", + "subprocess.run = other.slot = Proxy()", + "other.slot = subprocess.run = Proxy()", + ], +) +def test_composed_or_annotated_store_does_not_supply_local_slot_proof(statement: str) -> None: + source = ( + "import subprocess\n" + + statement + + "\nenabled = True\nsubprocess.run(command, shell=enabled)\n" + ) + assert len(_findings(source)) == 1 + + +@pytest.mark.parametrize( + "prefix", + [ + "subprocess.run = Proxy()\n", + "subprocess.__class__ = OtherModule\n", + ], +) +def test_later_single_store_cannot_override_companion_abstention(prefix: str) -> None: + source = ( + prefix + "subprocess.run = proxy\nenabled = True\nsubprocess.run(command, shell=enabled)\n" + ) + assert len(_findings(source)) == 1 + + +@pytest.mark.parametrize( + "store", + [ + pytest.param("False and (enabled := False)", id="and-flag"), + pytest.param("False and (subprocess := None)", id="and-receiver"), + pytest.param("True or (enabled := False)", id="or-flag"), + pytest.param("ready and (enabled := False)", id="unknown-and-flag"), + pytest.param("result = 1 if ready else (enabled := False)", id="ifexp-flag"), + pytest.param("result = (subprocess := None) if False else 1", id="ifexp-receiver"), + pytest.param("result = [(enabled := False) for _ in values]", id="listcomp-flag"), + pytest.param("result = ((subprocess := None) for _ in values)", id="genexp-receiver"), + ], +) +def test_untaken_expression_store_is_not_counterevidence(store: str) -> None: + source = ( + f'import subprocess\nenabled = True\n{store}\nsubprocess.run("echo ok", shell=enabled)\n' + ) + findings = _findings(source) + assert len(findings) == 1 + assert findings[0].severity == "HIGH" + + +@pytest.mark.parametrize( + "store", + [ + "True and (enabled := False)", + "False or (subprocess := None)", + "result = (enabled := False) if True else 1", + "result = 1 if False else (subprocess := None)", + ], +) +def test_executed_expression_store_removes_lexical_candidate(store: str) -> None: + source = ( + f'import subprocess\nenabled = True\n{store}\nsubprocess.run("echo ok", shell=enabled)\n' + ) + assert not _findings(source) + + +@pytest.mark.parametrize( + ("prefix", "call"), + [ + pytest.param( + "import subprocess\nsaved = subprocess\nsubprocess = saved\n", + 'subprocess.run("echo ok", shell=enabled)', + id="native-alias-round-trip", + ), + pytest.param( + "import subprocess as saved\nimport subprocess\nsubprocess = saved\n", + 'subprocess.run("echo ok", shell=enabled)', + id="native-import-alias", + ), + pytest.param( + "import subprocess\nsubprocess, saved = subprocess, 1\n", + 'subprocess.run("echo ok", shell=enabled)', + id="unpacked-receiver-self-store", + ), + pytest.param( + "import subprocess\n[subprocess, saved] = [subprocess, 1]\n", + 'subprocess.run("echo ok", shell=enabled)', + id="list-unpacked-receiver-self-store", + ), + pytest.param( + "from subprocess import Popen\nsaved = Popen\nPopen = saved\n", + 'Popen("echo ok", shell=enabled)', + id="native-popen-alias", + ), + pytest.param( + "import subprocess\nfrom subprocess import Popen\nPopen = subprocess.Popen\n", + 'Popen("echo ok", shell=enabled)', + id="native-popen-attribute", + ), + ], +) +def test_native_identity_store_keeps_lexical_high(prefix: str, call: str) -> None: + source = f"{prefix}enabled = True\n{call}\n" + findings = _findings(source) + assert len(findings) == 1 + assert findings[0].severity == "HIGH" + + +@pytest.mark.parametrize( + "store", + [ + "enabled, ignored = True, 1", + "[enabled, ignored] = [True, 1]", + "enabled, ignored = 'yes', 1", + "(enabled := True)", + "enabled = 1", + ], +) +def test_truthy_flag_store_keeps_lexical_high(store: str) -> None: + source = ( + f'import subprocess\nenabled = True\n{store}\nsubprocess.run("echo ok", shell=enabled)\n' + ) + findings = _findings(source) + assert len(findings) == 1 + assert findings[0].severity == "HIGH" + + +@pytest.mark.parametrize( + "store", + [ + "enabled, ignored = False, 1", + "enabled, ignored = dynamic, 1", + "subprocess, saved = proxy, 1", + "saved = proxy\nsubprocess = saved", + "saved = subprocess\nsaved = proxy\nsubprocess = saved", + ], +) +def test_unpacked_or_aliased_replacement_removes_lexical_candidate(store: str) -> None: + source = ( + f'import subprocess\nenabled = True\n{store}\nsubprocess.run("echo ok", shell=enabled)\n' + ) + assert not _findings(source) + + +_DEFERRED_RUN = ( + "import subprocess\ndef run(command):\n enabled = True\n" + " subprocess.run(command, shell=enabled)\n" +) + + +@pytest.mark.parametrize( + "invocation", + [ + pytest.param('if True:\n run("echo ok")\n', id="literal-branch"), + pytest.param('if __name__ == "__main__":\n run("echo ok")\n', id="main-guard"), + pytest.param('try:\n run("echo ok")\nfinally:\n pass\n', id="try"), + pytest.param('for _ in range(1):\n run("echo ok")\n', id="loop"), + pytest.param('with context:\n run("echo ok")\n', id="with"), + pytest.param( + 'threading.Thread(target=run, args=("echo ok",)).start()\n', id="thread-start" + ), + pytest.param('def main():\n run("echo ok")\nmain()\n', id="nested-caller"), + ], +) +@pytest.mark.parametrize("replacement", ["subprocess = None", "subprocess = proxy"]) +def test_invocation_before_future_store_keeps_lexical_high( + invocation: str, replacement: str +) -> None: + source = f"{_DEFERRED_RUN}{invocation}{replacement}\n" + findings = _findings(source) + assert len(findings) == 1 + assert findings[0].severity == "HIGH" + + +def test_method_invoked_before_future_store_keeps_lexical_high() -> None: + source = ( + "import subprocess\nclass Tool:\n def run(self, command):\n enabled = True\n" + ' subprocess.run(command, shell=enabled)\nif True:\n Tool().run("echo ok")\n' + "subprocess = None\n" + ) + findings = _findings(source) + assert len(findings) == 1 + assert findings[0].severity == "HIGH" + + +def test_reimport_before_invocation_restores_deferred_receiver() -> None: + source = ( + "import subprocess\nsubprocess = proxy\n" + f"{_DEFERRED_RUN.removeprefix('import subprocess')}" + 'import subprocess\nif True:\n run("echo ok")\n' + ) + findings = _findings(source) + assert len(findings) == 1 + assert findings[0].severity == "HIGH" + + +@pytest.mark.parametrize( + "tail", + [ + pytest.param('subprocess = None\nrun("echo ok")\n', id="store-before-call"), + pytest.param('subprocess = None\nif True:\n run("echo ok")\n', id="store-before-branch"), + pytest.param("subprocess = None\n", id="never-called-in-module"), + ], +) +def test_store_before_every_invocation_removes_deferred_candidate(tail: str) -> None: + assert not _findings(f"{_DEFERRED_RUN}{tail}") diff --git a/tests/nodes/analyzers/test_tool_misuse_python_ast.py b/tests/nodes/analyzers/test_tool_misuse_python_ast.py new file mode 100644 index 000000000..94761a709 --- /dev/null +++ b/tests/nodes/analyzers/test_tool_misuse_python_ast.py @@ -0,0 +1,1453 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""Focused coverage for issue #475's ordinary-Python binding form.""" + +from __future__ import annotations + +import pytest + +from skillspector.nodes.analyzers import static_patterns_tool_misuse as tm_module +from skillspector.nodes.analyzers import static_python_shell_truthiness as python_tm_module +from skillspector.nodes.deduplicate import deduplicate + + +def _run(content: str, path: str = "run.py") -> dict: + return tm_module.node({"components": [path], "file_cache": {path: content}}) + + +def _tm1(content: str, path: str = "run.py") -> list: + return [finding for finding in _run(content, path)["findings"] if finding.rule_id == "TM1"] + + +def _tm1_ast(content: str, path: str = "run.py") -> list: + return python_tm_module.analyze(content, path, "python") + + +@pytest.mark.parametrize( + "value", + [ + pytest.param("True", id="issue-body-boolean"), + pytest.param("'True'", id="reporter-attachment-string"), + ], +) +def test_issue_475_multiline_binding_matches_direct_tm1(value: str) -> None: + findings = _tm1( + "import subprocess\n" + "command = f'python a.py'\n" + f"enabled = {value}\n" + "result = subprocess.run(\n" + " command,\n" + " shell=enabled,\n" + " capture_output=True,\n" + " text=True,\n" + ")\n" + ) + + assert len(findings) == 1 + assert findings[0].start_line == 4 + assert findings[0].severity == "HIGH" + assert findings[0].confidence == pytest.approx(0.9) + assert "shell=enabled" in findings[0].matched_text + assert not findings[0].evidence + + +@pytest.mark.parametrize( + "value", + [ + pytest.param("1", id="integer"), + pytest.param("-1", id="negative-integer"), + pytest.param("(0,)", id="nonempty-tuple"), + pytest.param("not False", id="negation"), + ], +) +def test_simple_immutable_truthy_values_are_tracked(value: str) -> None: + assert ( + len(_tm1(f"import subprocess\nenabled = {value}\nsubprocess.run(cmd, shell=enabled)\n")) + == 1 + ) + + +@pytest.mark.parametrize( + "value", + [ + pytest.param("False", id="boolean"), + pytest.param("0", id="integer"), + pytest.param("''", id="string"), + pytest.param("()", id="tuple"), + pytest.param("None", id="none"), + ], +) +def test_definitely_false_values_are_not_tracked(value: str) -> None: + assert not _tm1(f"import subprocess\nenabled = {value}\nsubprocess.run(cmd, shell=enabled)\n") + + +def test_simple_alias_chain_and_bare_popen_are_tracked() -> None: + findings = _tm1( + "first = 'enabled'\nsecond = first\nthird = second\nPopen(command, shell=third)\n" + ) + + assert len(findings) == 1 + assert findings[0].start_line == 4 + + +@pytest.mark.parametrize( + "rebind", + [ + pytest.param("enabled = False", id="false-assignment"), + pytest.param("enabled = dynamic", id="unknown-assignment"), + pytest.param("import pathlib as enabled", id="import"), + pytest.param("from settings import enabled", id="from-import"), + ], +) +def test_rebinding_invalidates_truthy_fact(rebind: str) -> None: + assert not _tm1( + f"import subprocess\nenabled = True\n{rebind}\nsubprocess.run(command, shell=enabled)\n" + ) + + +def test_import_side_effect_boundary_clears_truth_facts() -> None: + assert not _tm1_ast( + "import subprocess\nenabled = True\nimport attacker\n" + "subprocess.run(command, shell=enabled)\n" + ) + + +@pytest.mark.parametrize( + "shadow", + [ + pytest.param("subprocess = Proxy()", id="assignment"), + pytest.param("import other as subprocess", id="import-alias"), + pytest.param("subprocess.run = Proxy()", id="attribute-mutation"), + pytest.param("subprocess, other = pair", id="unpacking"), + ], +) +def test_explicit_subprocess_shadow_rejects_bound_call(shadow: str) -> None: + assert not _tm1(f"{shadow}\nenabled = True\nsubprocess.run(cmd, shell=enabled)\n") + + +@pytest.mark.parametrize( + "native_rhs", + [ + pytest.param("subprocess.check_call", id="native-cross-api"), + pytest.param("Popen", id="native-popen"), + pytest.param("functools.partial(Popen, command)", id="wrapped-popen"), + pytest.param("functools.partial(subprocess.run, command)", id="wrapped-native"), + pytest.param("Proxy(subprocess)", id="receiver-passed-to-constructor"), + ], +) +def test_first_method_store_with_native_receiver_rhs_retains_bound_call(native_rhs: str) -> None: + findings = _tm1( + f"subprocess.run = {native_rhs}\nenabled = True\nsubprocess.run(command, shell=enabled)\n" + ) + + assert len(findings) == 1 + assert findings[0].severity == "HIGH" + assert findings[0].start_line == 2 + assert "shell=enabled" in findings[0].matched_text + + +def test_explicit_import_reestablishes_direct_receivers() -> None: + assert ( + len( + _tm1( + "subprocess = None\n" + "Popen = None\n" + "import subprocess\n" + "from subprocess import Popen\n" + "enabled = True\n" + "subprocess.run('/usr/bin/true', shell=enabled)\n" + "Popen('/usr/bin/true', shell=enabled)\n" + ) + ) + == 2 + ) + + +def test_relative_import_does_not_establish_bare_popen() -> None: + assert not _tm1( + "Popen = proxy\nfrom .subprocess import Popen\nenabled = True\n" + "Popen(command, shell=enabled)\n" + ) + + +def test_function_local_binding_and_outer_fact_are_independent() -> None: + findings = _tm1( + "outer = True\n" + "def execute(command):\n" + " enabled = 'True'\n" + " subprocess.run(command, shell=enabled)\n" + "subprocess.run('/usr/bin/true', shell=outer)\n" + ) + + assert [finding.start_line for finding in findings] == [4, 5] + + +def test_function_compile_time_receiver_shadow_rejects_earlier_lookup() -> None: + assert not _tm1( + "def execute(command):\n" + " enabled = True\n" + " subprocess.run(command, shell=enabled)\n" + " subprocess = Proxy()\n" + ) + + +def test_later_global_receiver_mutation_does_not_suppress_earlier_function_call() -> None: + findings = _tm1( + "def execute(command):\n" + " enabled = True\n" + " subprocess.run(command, shell=enabled)\n" + "execute(command)\n" + "subprocess = Proxy()\n" + ) + + assert [finding.start_line for finding in findings] == [3] + + +def test_later_global_receiver_mutation_suppresses_unobserved_function_body() -> None: + assert not _tm1( + "def execute(command):\n" + " enabled = True\n" + " subprocess.run(command, shell=enabled)\n" + "subprocess = Proxy()\n" + ) + + +def test_passive_function_definition_preserves_outer_fact() -> None: + assert ( + len( + _tm1( + "enabled = True\n" + "def helper(value=1):\n" + " pass\n" + "subprocess.run(command, shell=enabled)\n" + ) + ) + == 1 + ) + + +@pytest.mark.parametrize( + "compound", + [ + pytest.param("if condition:\n pass", id="if"), + pytest.param("for item in values:\n pass", id="for"), + pytest.param("with provider():\n pass", id="with"), + pytest.param("try:\n pass\nexcept Exception:\n pass", id="try"), + pytest.param("class Local:\n pass", id="class"), + ], +) +def test_compound_statement_conservatively_clears_truth_facts(compound: str) -> None: + assert not _tm1_ast(f"enabled = True\n{compound}\nsubprocess.run(command, shell=enabled)\n") + + +def test_calls_inside_compound_statements_are_out_of_scope() -> None: + assert not _tm1_ast( + "if condition:\n enabled = True\n subprocess.run(command, shell=enabled)\n" + ) + + +def test_class_body_same_scope_binding_is_tracked() -> None: + findings = _tm1( + "class Runner:\n enabled = True\n subprocess.run(command, shell=enabled)\n" + ) + + assert [finding.start_line for finding in findings] == [3] + + +def test_generic_call_invalidates_receiver_trust_in_class_body() -> None: + assert not _tm1_ast( + "class Runner:\n" + " replace_subprocess()\n" + " enabled = True\n" + " subprocess.run(command, shell=enabled)\n" + ) + + +def test_class_method_does_not_close_over_class_binding() -> None: + assert not _tm1( + "class Runner:\n" + " enabled = True\n" + " def run(self):\n" + " subprocess.run(command, shell=enabled)\n" + ) + + +def test_class_body_lookup_precedes_later_class_binding() -> None: + findings = _tm1( + "enabled = True\n" + "class Runner:\n" + " subprocess.run(command, shell=enabled)\n" + " enabled = False\n" + ) + + assert [finding.start_line for finding in findings] == [1] + + +@pytest.mark.parametrize( + "class_body", + [ + pytest.param( + " enabled = subprocess.run(command, shell=enabled)\n", + id="assignment-rhs-before-store", + ), + pytest.param( + " def enabled(value=subprocess.run(command, shell=enabled)):\n pass\n", + id="function-default-before-name-binding", + ), + pytest.param( + " with manager(subprocess.run(command, shell=enabled)) as enabled:\n pass\n", + id="with-target-after-context", + ), + pytest.param( + " try:\n" + " raise Error\n" + " except subprocess.run(command, shell=enabled) as enabled:\n" + " pass\n", + id="except-target-after-type", + ), + ], +) +def test_class_binding_is_installed_after_value_evaluation(class_body: str) -> None: + findings = _tm1("enabled = True\nclass Runner:\n" + class_body) + + assert [finding.start_line for finding in findings] == [1] + + +def test_class_local_import_does_not_establish_method_receiver_trust() -> None: + assert not _tm1( + "subprocess = proxy\n" + "class Runner:\n" + " import subprocess\n" + " def run(self):\n" + " enabled = True\n" + " subprocess.run(command, shell=enabled)\n" + ) + + +def test_class_local_shadow_does_not_hide_global_method_receiver() -> None: + findings = _tm1( + "import subprocess\n" + "class Runner:\n" + " subprocess = proxy\n" + " def run(self):\n" + " enabled = True\n" + " subprocess.run(command, shell=enabled)\n" + ) + + assert [finding.start_line for finding in findings] == [6] + + +def test_class_body_generic_call_invalidates_deferred_method_receiver() -> None: + assert not _tm1_ast( + "import subprocess\n" + "class Runner:\n" + " replace_subprocess()\n" + " def run(self):\n" + " enabled = True\n" + " subprocess.run(command, shell=enabled)\n" + ) + + +def test_called_class_method_preserves_receiver_before_later_invalidation() -> None: + findings = _tm1( + "import subprocess\n" + "class Runner:\n" + " def run():\n" + " enabled = True\n" + " subprocess.run(command, shell=enabled)\n" + " run()\n" + " replace_subprocess()\n" + ) + + assert [finding.start_line for finding in findings] == [5] + + +def test_class_method_call_after_invalidation_is_rejected() -> None: + assert not _tm1_ast( + "import subprocess\n" + "class Runner:\n" + " def run():\n" + " enabled = True\n" + " subprocess.run(command, shell=enabled)\n" + " replace_subprocess()\n" + " run()\n" + ) + + +@pytest.mark.parametrize( + "class_header", + [ + pytest.param("@replace_subprocess()\nclass Runner:", id="decorator"), + pytest.param("class Runner(replace_subprocess()):", id="base"), + ], +) +def test_effectful_class_header_invalidates_deferred_method_receiver(class_header: str) -> None: + assert not _tm1_ast( + "import subprocess\n" + f"{class_header}\n" + " def run(self):\n" + " enabled = True\n" + " subprocess.run(command, shell=enabled)\n" + ) + + +def test_class_for_target_is_bound_after_iterable_evaluation() -> None: + findings = _tm1( + "enabled = True\n" + "class Runner:\n" + " for enabled in subprocess.run(command, shell=enabled):\n" + " pass\n" + ) + + assert [finding.start_line for finding in findings] == [1] + + +def test_comprehension_target_shadows_outer_shell_flag() -> None: + assert not _tm1( + "enabled = True\n" + "class Runner:\n" + " values = [\n" + " subprocess.run(command, shell=enabled)\n" + " for enabled in items\n" + " ]\n" + ) + + +@pytest.mark.parametrize( + "argument", + [ + pytest.param("disable()", id="call"), + pytest.param("mutator.command", id="attribute"), + pytest.param("holder[0]", id="subscript"), + pytest.param("left + right", id="operator"), + pytest.param("f'{value}'", id="formatted-string"), + pytest.param("[item for item in items]", id="comprehension"), + pytest.param("*commands", id="starred-expansion"), + ], +) +def test_side_effect_capable_call_arguments_are_rejected(argument: str) -> None: + assert not _tm1_ast(f"enabled = True\nsubprocess.run({argument}, shell=enabled)\n") + + +@pytest.mark.parametrize( + "statement", + [ + pytest.param( + "subprocess.run(command, shell=enabled, env=build_env())", + id="expression", + ), + pytest.param( + "result = subprocess.run(command, shell=enabled, env=build_env())", + id="assignment", + ), + pytest.param( + "result: object = subprocess.run(command, shell=enabled, env=build_env())", + id="annotated-assignment", + ), + ], +) +def test_later_keyword_effect_preserves_captured_shell_value(statement: str) -> None: + findings = _tm1(f"enabled = True\n{statement}\n") + literal_findings = _tm1(statement.replace("shell=enabled", "shell=True")) + + assert len(findings) == len(literal_findings) == 1 + assert findings[0].start_line == 2 + assert findings[0].severity == literal_findings[0].severity + assert findings[0].confidence == literal_findings[0].confidence + + +@pytest.mark.parametrize( + "call", + [ + pytest.param( + "subprocess.run(command, env=build_env(), shell=enabled)", + id="earlier-keyword", + ), + pytest.param( + "subprocess.run(build_command(), shell=enabled)", + id="earlier-positional", + ), + pytest.param( + "subprocess.run(shell=enabled, *build_args())", + id="starred-positional-written-later", + ), + pytest.param( + "subprocess.run(command, **build_options(), shell=enabled)", + id="earlier-keyword-expansion", + ), + ], +) +def test_earlier_argument_effect_keeps_shell_value_uncertain(call: str) -> None: + assert not _tm1_ast(f"enabled = True\n{call}\n") + + +def test_later_keyword_expansion_preserves_captured_shell_value() -> None: + findings = _tm1("enabled = True\nsubprocess.run(command, shell=enabled, **build_options())\n") + + assert len(findings) == 1 + assert findings[0].start_line == 2 + + +def test_later_argument_effect_invalidates_fact_after_captured_call() -> None: + findings = _tm1( + "enabled = True\n" + "subprocess.run(command, shell=enabled, env=build_env())\n" + "subprocess.run(command, shell=enabled)\n" + ) + + assert [finding.start_line for finding in findings] == [2] + + +@pytest.mark.parametrize( + "statement", + [ + pytest.param( + "subprocess.run(command, shell=enabled, env=replace_subprocess())", + id="expression", + ), + pytest.param( + "result = subprocess.run(command, shell=enabled, env=replace_subprocess())", + id="assignment", + ), + pytest.param( + "result: object = subprocess.run(command, shell=enabled, env=replace_subprocess())", + id="annotated-assignment", + ), + ], +) +def test_later_argument_effect_invalidates_receiver_after_captured_call(statement: str) -> None: + findings = _tm1_ast( + "import subprocess\n" + "from helpers import replace_subprocess\n" + "enabled = True\n" + f"{statement}\n" + "later_enabled = True\n" + "subprocess.run(command, shell=later_enabled)\n" + ) + + assert [finding.location.start_line for finding in findings] == [4] + + +def test_later_argument_effect_invalidates_receiver_for_called_function() -> None: + findings = _tm1_ast( + "import subprocess\n" + "from helpers import replace_subprocess\n" + "enabled = True\n" + "subprocess.run(command, shell=enabled, env=replace_subprocess())\n" + "def execute():\n" + " later_enabled = True\n" + " subprocess.run(command, shell=later_enabled)\n" + "execute()\n" + ) + + assert [finding.location.start_line for finding in findings] == [4] + + +@pytest.mark.parametrize( + "statement", + [ + pytest.param("replace_subprocess()", id="expression"), + pytest.param("result = replace_subprocess()", id="assignment"), + pytest.param("result: object = replace_subprocess()", id="annotated-assignment"), + pytest.param("result = (replace_subprocess(),)", id="tuple-rhs"), + pytest.param("result = [replace_subprocess()]", id="list-rhs"), + pytest.param( + "result: object = (replace_subprocess(),)", + id="annotated-tuple-rhs", + ), + pytest.param( + "result: object = [replace_subprocess()]", + id="annotated-list-rhs", + ), + pytest.param("result = (mutator.value,)", id="tuple-protocol-rhs"), + pytest.param( + "result: object = [mutator[0]]", + id="annotated-list-protocol-rhs", + ), + ], +) +def test_generic_call_invalidates_receiver_trust(statement: str) -> None: + findings = _tm1_ast( + "import subprocess\n" + "from helpers import replace_subprocess\n" + f"{statement}\n" + "enabled = True\n" + "subprocess.run(command, shell=enabled)\n" + ) + + assert not findings + + +@pytest.mark.parametrize( + "statement", + [ + pytest.param("replace_subprocess()", id="expression"), + pytest.param("result = replace_subprocess()", id="assignment"), + pytest.param("result: object = replace_subprocess()", id="annotated-assignment"), + pytest.param("result = (replace_subprocess(),)", id="tuple-rhs"), + pytest.param("result = [replace_subprocess()]", id="list-rhs"), + pytest.param( + "result: object = (replace_subprocess(),)", + id="annotated-tuple-rhs", + ), + pytest.param( + "result: object = [replace_subprocess()]", + id="annotated-list-rhs", + ), + pytest.param("result = (mutator.value,)", id="tuple-protocol-rhs"), + pytest.param( + "result: object = [mutator[0]]", + id="annotated-list-protocol-rhs", + ), + ], +) +def test_generic_call_invalidates_receiver_trust_for_called_function(statement: str) -> None: + findings = _tm1_ast( + "import subprocess\n" + "from helpers import replace_subprocess\n" + "def execute():\n" + " enabled = True\n" + " subprocess.run(command, shell=enabled)\n" + f"{statement}\n" + "execute()\n" + ) + + assert not findings + + +@pytest.mark.parametrize( + "statement", + [ + pytest.param("assert (replace_subprocess(),)", id="assert"), + pytest.param("(replace_subprocess(),)", id="tuple-expression"), + pytest.param("[replace_subprocess()]", id="list-expression"), + ], +) +def test_unsupported_eager_statement_invalidates_receiver_trust(statement: str) -> None: + assert not _tm1_ast( + "import subprocess\n" + "from helpers import replace_subprocess\n" + f"{statement}\n" + "enabled = True\n" + "subprocess.run(command, shell=enabled)\n" + ) + + +@pytest.mark.parametrize( + "statement", + [ + pytest.param("assert (replace_subprocess(),)", id="assert"), + pytest.param("(replace_subprocess(),)", id="tuple-expression"), + pytest.param("[replace_subprocess()]", id="list-expression"), + ], +) +def test_unsupported_eager_statement_invalidates_receiver_for_called_function( + statement: str, +) -> None: + assert not _tm1_ast( + "import subprocess\n" + "from helpers import replace_subprocess\n" + "def execute():\n" + " enabled = True\n" + " subprocess.run(command, shell=enabled)\n" + f"{statement}\n" + "execute()\n" + ) + + +def test_generic_call_invalidates_receiver_trust_for_nested_closure() -> None: + assert not _tm1_ast( + "import subprocess\n" + "def outer():\n" + " enabled = True\n" + " def inner():\n" + " subprocess.run(command, shell=enabled)\n" + " replace_subprocess()\n" + " inner()\n" + "outer()\n" + ) + + +@pytest.mark.parametrize( + "statement", + [ + pytest.param( + "subprocess.run('/usr/bin/true', shell=enabled)", + id="expression", + ), + pytest.param( + "result = subprocess.run('/usr/bin/true', shell=enabled)", + id="assignment", + ), + pytest.param( + "result: object = subprocess.run('/usr/bin/true', shell=enabled)", + id="annotated-assignment", + ), + ], +) +def test_direct_subprocess_call_remains_detected(statement: str) -> None: + findings = _tm1_ast( + "import subprocess\n" + "def execute():\n" + " later_enabled = True\n" + " subprocess.run('/usr/bin/true', shell=later_enabled)\n" + "enabled = True\n" + f"{statement}\n" + "execute()\n" + ) + + assert [finding.location.start_line for finding in findings] == [4, 6] + + +def test_blank_line_before_assignment_has_one_tm1_owner() -> None: + findings = _tm1("import subprocess\n\nenabled = True\nsubprocess.run(command, shell=enabled)\n") + + assert len(findings) == 1 + assert findings[0].start_line == 4 + + +def test_unsupported_assignment_clears_existing_facts() -> None: + assert not _tm1_ast("enabled = True\nresult = factory()\nsubprocess.run(cmd, shell=enabled)\n") + + +def test_simple_name_store_with_unsafe_prior_binding_invalidates_truth_facts() -> None: + findings = _tm1_ast( + "import subprocess\n" + "class Trigger:\n" + " def __del__(self):\n" + " global enabled\n" + " enabled = False\n" + "trigger = Trigger()\n" + "enabled = True\n" + "trigger = 0\n" + "subprocess.run('/usr/bin/true', shell=enabled)\n" + ) + + assert not findings + + +def test_external_name_store_treats_prior_binding_as_finalizer_capable() -> None: + findings = _tm1_ast( + "import subprocess\n" + "class Trigger:\n" + " def __del__(self):\n" + " global enabled\n" + " enabled = False\n" + "trigger = Trigger()\n" + "enabled = False\n" + "def execute():\n" + " global enabled, trigger\n" + " enabled = True\n" + " trigger = 0\n" + " subprocess.run('/usr/bin/true', shell=enabled)\n" + "execute()\n" + ) + + assert not findings + + +def test_protocol_consuming_direct_call_invalidates_later_truth_fact() -> None: + findings = _tm1( + "from helpers import mutator\n" + "import subprocess\n" + "enabled = True\n" + "subprocess.run(mutator, shell=enabled)\n" + "subprocess.run('/usr/bin/true', shell=enabled)\n" + ) + + assert [finding.start_line for finding in findings] == [4] + + +def test_annotated_assignment_is_outside_side_effect_free_contract() -> None: + assert not _tm1("enabled: bool = True\nsubprocess.run(command, shell=enabled)\n") + + +def test_assignment_rhs_direct_call_is_inspected_before_invalidation() -> None: + findings = _tm1("enabled = True\nresult = subprocess.run(command, shell=enabled)\n") + + assert len(findings) == 1 + assert findings[0].start_line == 2 + + +def test_true_prefixed_identifier_has_one_lexical_owner() -> None: + findings = _tm1("true_value = True\nsubprocess.run(command, shell=true_value)\n") + + assert len(findings) == 1 + + +@pytest.mark.parametrize("name", ["true", "TRUE", "TrUe"]) +def test_case_variant_true_has_one_direct_owner(name: str) -> None: + findings = _tm1(f"{name} = True\nsubprocess.run(command, shell={name})\n") + + assert len(findings) == 1 + assert findings[0].start_line == 2 + assert not findings[0].evidence + + +def test_true_prefixed_closure_has_one_lexical_owner() -> None: + findings = _tm1( + "true_value = True\n" + "def inner():\n" + " # one\n" + " # two\n" + " # three\n" + " subprocess.run(command, shell=true_value)\n" + ) + + assert len(findings) == 1 + assert findings[0].start_line == 6 + snippet = str(findings[0].to_dict()["code_snippet"]) + assert "subprocess.run" in snippet + assert "true_value = True" not in snippet + assert findings[0].severity == "HIGH" + assert findings[0].confidence == pytest.approx(0.9) + + +def test_normalized_prefix_keeps_lexical_signal_after_unknown_receiver_effect() -> None: + assert ( + len( + _tm1( + "true_value = True\n" + "def inner():\n" + " ff; subprocess.run(command, shell=true_value)\n" + "replace_subprocess()\n" + "inner()\n" + ) + ) + == 1 + ) + + +def test_normalized_prefix_does_not_duplicate_true_direct_fallback() -> None: + findings = _tm1("true = True\nff; subprocess.run(command, shell=true)\n") + + assert len(findings) == 1 + assert (findings[0].start_line, findings[0].start_column) == (2, 3) + assert not findings[0].evidence + + +@pytest.mark.parametrize( + "call_line", + [ + "subprocess.run(command, shell=true); " + "[subprocess.run(command, shell=true) for item in items]", + "[subprocess.run(command, shell=true) for item in items]; " + "subprocess.run(command, shell=true)", + ], +) +def test_true_direct_calls_on_one_line_keep_distinct_locations(call_line: str) -> None: + findings = _tm1(f"items = [1]\ncommand = '/bin/true'\ntrue = True\n{call_line}\n") + expected_columns = [ + index for index in range(len(call_line)) if call_line.startswith("subprocess.run", index) + ] + + assert len(findings) == 2 + assert sorted(finding.start_column for finding in findings) == expected_columns + + +def test_unknown_comprehension_protocol_invalidates_receiver_trust() -> None: + assert not _tm1_ast( + "import subprocess\n" + "[subprocess.run('/bin/true', shell=False) for item in items]\n" + "enabled = True\n" + "subprocess.run(command, shell=enabled)\n" + ) + + +def test_unknown_comprehension_protocol_invalidates_called_function_trust() -> None: + assert not _tm1_ast( + "import subprocess\n" + "def execute():\n" + " enabled = True\n" + " subprocess.run(command, shell=enabled)\n" + "[subprocess.run('/bin/true', shell=False) for item in items]\n" + "execute()\n" + ) + + +def test_true_direct_lexical_signal_survives_unknown_receiver_effect() -> None: + findings = _tm1( + "true = True\n" + "subprocess.run(command, shell=true)\n" + "replace_subprocess()\n" + "subprocess.run(command, shell=true)\n" + ) + + assert [finding.start_line for finding in findings] == [2, 4] + + +@pytest.mark.parametrize( + ("path", "suffix"), + [ + pytest.param("run.py", "if:\n", id="malformed-python"), + pytest.param("run.js", "", id="non-python"), + ], +) +def test_true_direct_fallback_survives_without_ast_ownership(path: str, suffix: str) -> None: + findings = _tm1(f"true = True\nsubprocess.run(command, shell=true)\n{suffix}", path) + + assert len(findings) == 1 + assert not findings[0].evidence + + +@pytest.mark.parametrize( + "content", + [ + pytest.param( + "enabled = True\ndef run(enabled):\n subprocess.run(command, shell=enabled)\n", + id="parameter-shadow", + ), + pytest.param( + "enabled = True\ndef run():\n" + " subprocess.run(command, shell=enabled)\n enabled = False\n", + id="later-local-shadow", + ), + pytest.param( + "def outer():\n enabled = True\n def inner():\n" + " global enabled\n subprocess.run(command, shell=enabled)\n", + id="global-redirect", + ), + ], +) +def test_cross_scope_binding_must_resolve_to_literal_assignment(content: str) -> None: + assert not _tm1(content) + + +@pytest.mark.parametrize( + "content", + [ + pytest.param( + "import subprocess\n" + "enabled = True\n" + "values = [(subprocess.run(command, shell=enabled), (enabled := False)) " + "for item in items]\n", + id="module", + ), + pytest.param( + "import subprocess\n" + "def execute():\n" + " enabled = True\n" + " return [(subprocess.run(command, shell=enabled), " + "(enabled := False)) for item in items]\n", + id="function", + ), + pytest.param( + "import subprocess\n" + "enabled = True\n" + "values = [[(subprocess.run(command, shell=enabled), (enabled := False)) " + "for inner in inners] for outer in outers]\n", + id="nested-comprehension", + ), + ], +) +def test_comprehension_walrus_binds_in_containing_scope(content: str) -> None: + findings = _tm1(content) + + assert len(findings) == 1 + + +def test_malformed_python_keeps_bounded_lexical_fallback() -> None: + findings = _tm1("enabled = True\nsubprocess.run(command, shell=enabled)\nif:\n") + + assert len(findings) == 1 + assert "_tm1_variable_shell_flag" not in findings[0].evidence + + +@pytest.mark.parametrize("name", ["enabled", "true"]) +def test_oversized_python_keeps_bounded_lexical_fallback( + monkeypatch: pytest.MonkeyPatch, + name: str, +) -> None: + content = f"{name} = True\nsubprocess.run(command, shell={name})\n" + "# padding\n" * 20 + monkeypatch.setattr(tm_module.static_runner, "MAX_FILE_CHARS", 80) + + findings = _tm1(content) + + assert len(findings) == 1 + assert "_tm1_variable_shell_flag" not in findings[0].evidence + + +def test_long_same_line_calls_keep_exact_coordinates_and_distinct_identity() -> None: + payload = "x" * 240 + first_call = f'subprocess.run("{payload}A", shell=enabled)' + second_call = f'subprocess.run("{payload}B", shell=enabled)' + call_line = f"first = {first_call}; second = {second_call}" + + findings = _tm1(f"import subprocess\nenabled = True\n{call_line}\n") + + assert len(findings) == 2 + assert [(finding.start_column, finding.end_column) for finding in findings] == [ + ( + call_line.index(first_call), + call_line.index(first_call) + len(first_call), + ), + ( + call_line.index(second_call), + call_line.index(second_call) + len(second_call), + ), + ] + assert findings[0].fingerprint() != findings[1].fingerprint() + assert len(deduplicate(findings)) == 2 + + +@pytest.mark.parametrize("path", ["run.pyw", "run", "run.sh"]) +def test_non_py_surfaces_do_not_enable_ast_companion(path: str) -> None: + assert not _tm1("enabled = True\nsubprocess.run(command, shell=enabled)\n", path) + + +def test_unsupported_eager_expression_invalidates_receiver_trust() -> None: + findings = _tm1_ast( + "import subprocess\n" + "from helpers import replace_subprocess\n" + "assert (replace_subprocess(),)\n" + "enabled = True\n" + "subprocess.run(command, shell=enabled)\n" + ) + + assert not findings + + +def test_unsupported_eager_expression_invalidates_called_function_trust() -> None: + findings = _tm1_ast( + "import subprocess\n" + "from helpers import replace_subprocess\n" + "def execute():\n" + " enabled = True\n" + " subprocess.run(command, shell=enabled)\n" + "assert (replace_subprocess(),)\n" + "execute()\n" + ) + + assert not findings + + +@pytest.mark.parametrize( + "statement", + [ + pytest.param("(replace_subprocess(),)", id="tuple"), + pytest.param("[replace_subprocess()]", id="list"), + ], +) +def test_eager_container_expression_invalidates_receiver_trust(statement: str) -> None: + findings = _tm1_ast( + "import subprocess\n" + "from helpers import replace_subprocess\n" + f"{statement}\n" + "enabled = True\n" + "subprocess.run(command, shell=enabled)\n" + ) + + assert not findings + + +@pytest.mark.parametrize( + "statement", + [ + pytest.param("(replace_subprocess(),)", id="tuple"), + pytest.param("[replace_subprocess()]", id="list"), + ], +) +def test_eager_container_expression_invalidates_called_function_trust( + statement: str, +) -> None: + findings = _tm1_ast( + "import subprocess\n" + "from helpers import replace_subprocess\n" + "def execute():\n" + " enabled = True\n" + " subprocess.run(command, shell=enabled)\n" + f"{statement}\n" + "execute()\n" + ) + + assert not findings + + +def test_protocol_consuming_direct_call_invalidates_called_function_trust() -> None: + findings = _tm1_ast( + "import subprocess\n" + "from helpers import mutator\n" + "def execute():\n" + " enabled = True\n" + " subprocess.run('/usr/bin/true', shell=enabled)\n" + "subprocess.run(mutator, shell=False)\n" + "execute()\n" + ) + + assert not findings + + +def test_unsafe_value_release_invalidates_called_function_trust() -> None: + findings = _tm1_ast( + "class Trigger:\n" + " def __del__(self):\n" + " global subprocess\n" + " subprocess = replacement\n" + "trigger = Trigger()\n" + "import subprocess\n" + "def execute():\n" + " enabled = True\n" + " subprocess.run('/usr/bin/true', shell=enabled)\n" + "trigger = 0\n" + "execute()\n" + ) + + assert not findings + + +@pytest.mark.parametrize( + "statement", + [ + pytest.param("replace_subprocess()", id="direct-call"), + pytest.param("(replace_subprocess(),)", id="nested-call"), + ], +) +def test_class_body_eager_call_invalidates_enclosing_receiver_trust(statement: str) -> None: + findings = _tm1_ast( + "import subprocess\n" + "from helpers import replace_subprocess\n" + "class Runner:\n" + f" {statement}\n" + "enabled = True\n" + "subprocess.run(command, shell=enabled)\n" + ) + + assert not findings + + +def test_class_body_outer_store_release_invalidates_receiver_trust() -> None: + findings = _tm1_ast( + "from helpers import trigger\n" + "import subprocess\n" + "class Runner:\n" + " global trigger\n" + " trigger = 0\n" + "enabled = True\n" + "subprocess.run('/bin/true', shell=enabled)\n" + ) + + assert not findings + + +def test_class_body_outer_store_release_invalidates_called_function_trust() -> None: + findings = _tm1_ast( + "from helpers import trigger\n" + "import subprocess\n" + "def execute():\n" + " enabled = True\n" + " subprocess.run('/bin/true', shell=enabled)\n" + "class Runner:\n" + " global trigger\n" + " trigger = 0\n" + "execute()\n" + ) + + assert not findings + + +def test_nested_class_outer_store_release_invalidates_receiver_trust() -> None: + findings = _tm1_ast( + "from helpers import trigger\n" + "import subprocess\n" + "class Outer:\n" + " class Inner:\n" + " global trigger\n" + " trigger = 0\n" + "enabled = True\n" + "subprocess.run('/bin/true', shell=enabled)\n" + ) + + assert not findings + + +def test_nested_class_outer_store_release_invalidates_called_function_trust() -> None: + findings = _tm1_ast( + "from helpers import trigger\n" + "import subprocess\n" + "def execute():\n" + " enabled = True\n" + " subprocess.run('/bin/true', shell=enabled)\n" + "class Outer:\n" + " class Inner:\n" + " global trigger\n" + " trigger = 0\n" + "execute()\n" + ) + + assert not findings + + +@pytest.mark.parametrize( + "definition", + [ + pytest.param(" def trigger():\n pass\n", id="function"), + pytest.param(" class trigger:\n pass\n", id="class"), + ], +) +def test_class_body_outer_definition_release_invalidates_receiver_trust( + definition: str, +) -> None: + findings = _tm1_ast( + "from helpers import trigger\n" + "import subprocess\n" + "class Outer:\n" + " global trigger\n" + + definition + + "enabled = True\n" + + "subprocess.run('/bin/true', shell=enabled)\n" + ) + + assert not findings + + +@pytest.mark.parametrize( + "class_body", + [ + pytest.param( + " @replace_subprocess\n def run():\n pass\n", + id="method-decorator", + ), + pytest.param( + " class Inner(Base):\n pass\n", + id="nested-class-base", + ), + pytest.param( + " @replace_subprocess\n class Inner:\n pass\n", + id="nested-class-decorator", + ), + ], +) +def test_class_implicit_eager_effect_invalidates_enclosing_receiver_trust( + class_body: str, +) -> None: + findings = _tm1_ast( + "from helpers import Base, replace_subprocess\n" + "import subprocess\n" + "class Runner:\n" + + class_body + + "enabled = True\n" + + "subprocess.run('/bin/true', shell=enabled)\n" + ) + + assert not findings + + +@pytest.mark.parametrize( + "class_body", + [ + pytest.param(" assert mutator\n", id="assert-truth"), + pytest.param(" target.attr = 1\n", id="attribute-store"), + pytest.param(" target += 1\n", id="augmented-assignment"), + pytest.param( + " with manager:\n pass\n", + id="context-manager", + ), + ], +) +def test_class_protocol_effect_invalidates_enclosing_receiver_trust( + class_body: str, +) -> None: + findings = _tm1_ast( + "from helpers import manager, mutator, target\n" + "import subprocess\n" + "class Runner:\n" + + class_body + + "enabled = True\n" + + "subprocess.run('/bin/true', shell=enabled)\n" + ) + + assert not findings + + +def test_class_protocol_effect_invalidates_called_function_trust() -> None: + findings = _tm1_ast( + "from helpers import mutator\n" + "import subprocess\n" + "def execute():\n" + " enabled = True\n" + " subprocess.run('/bin/true', shell=enabled)\n" + "class Runner:\n" + " assert mutator\n" + "execute()\n" + ) + + assert not findings + + +@pytest.mark.parametrize( + "header", + [ + pytest.param("@replace_subprocess\nclass Runner:", id="decorator-name"), + pytest.param("class Runner(Base):", id="base-name"), + ], +) +def test_class_creation_invalidates_enclosing_receiver_trust(header: str) -> None: + findings = _tm1_ast( + "import subprocess\n" + "from helpers import Base, replace_subprocess\n" + f"{header}\n" + " pass\n" + "enabled = True\n" + "subprocess.run(command, shell=enabled)\n" + ) + + assert not findings + + +def test_import_binding_release_invalidates_receiver_trust() -> None: + findings = _tm1_ast( + "class Trigger:\n" + " def __del__(self):\n" + " global subprocess\n" + " subprocess = replacement\n" + "subprocess = Trigger()\n" + "import subprocess\n" + "enabled = True\n" + "subprocess.run('/bin/true', shell=enabled)\n" + ) + + assert not findings + + +def test_import_binding_release_invalidates_called_function_trust() -> None: + source = ( + "class Trigger:\n" + " def __del__(self):\n" + " global subprocess\n" + " subprocess = replacement\n" + "subprocess = Trigger()\n" + "def execute():\n" + " enabled = True\n" + " subprocess.run('/bin/true', shell=enabled)\n" + "import subprocess\n" + "execute()\n" + ) + + # The release is an unknown effect: the companion abstains, and like the + # module-level layout the lexical signal stays because no store proves a + # replacement before the call. + assert not _tm1_ast(source) + findings = _tm1(source) + assert len(findings) == 1 + assert findings[0].severity == "HIGH" + + +def test_class_binding_release_invalidates_receiver_trust() -> None: + findings = _tm1_ast( + "class Trigger:\n" + " def __del__(self):\n" + " global subprocess\n" + " subprocess = replacement\n" + "trigger = Trigger()\n" + "import subprocess\n" + "class trigger:\n" + " pass\n" + "enabled = True\n" + "subprocess.run('/bin/true', shell=enabled)\n" + ) + + assert not findings + + +def test_class_binding_release_invalidates_called_function_trust() -> None: + findings = _tm1_ast( + "class Trigger:\n" + " def __del__(self):\n" + " global subprocess\n" + " subprocess = replacement\n" + "trigger = Trigger()\n" + "import subprocess\n" + "def execute():\n" + " enabled = True\n" + " subprocess.run('/bin/true', shell=enabled)\n" + "class trigger:\n" + " pass\n" + "execute()\n" + ) + + assert not findings + + +def test_assert_truth_protocol_invalidates_receiver_trust() -> None: + findings = _tm1_ast( + "import subprocess\n" + "from helpers import mutator\n" + "assert mutator\n" + "enabled = True\n" + "subprocess.run(command, shell=enabled)\n" + ) + + assert not findings + + +def test_assert_truth_protocol_invalidates_called_function_trust() -> None: + findings = _tm1_ast( + "import subprocess\n" + "from helpers import mutator\n" + "def execute():\n" + " enabled = True\n" + " subprocess.run(command, shell=enabled)\n" + "assert mutator\n" + "execute()\n" + ) + + assert not findings + + +@pytest.mark.parametrize( + "unknown_binding", + [ + pytest.param( + "class Trigger:\n" + " def __del__(self):\n" + " global subprocess\n" + " subprocess = replacement\n" + "globals()['trigger'] = Trigger()\n", + id="dynamic-global", + ), + pytest.param("from helpers import *\n", id="star-import"), + ], +) +def test_unknown_unsafe_binding_blocks_receiver_reestablishment( + unknown_binding: str, +) -> None: + findings = _tm1_ast( + unknown_binding + + "import subprocess\n" + + "trigger = 0\n" + + "enabled = True\n" + + "subprocess.run('/bin/true', shell=enabled)\n" + ) + + assert not findings + + +@pytest.mark.parametrize( + "unknown_binding", + [ + pytest.param( + "class Trigger:\n" + " def __del__(self):\n" + " global subprocess\n" + " subprocess = replacement\n" + "globals()['trigger'] = Trigger()\n", + id="dynamic-global", + ), + pytest.param("from helpers import *\n", id="star-import"), + ], +) +def test_unknown_unsafe_binding_blocks_called_function_trust( + unknown_binding: str, +) -> None: + findings = _tm1_ast( + unknown_binding + + "def execute():\n" + + " enabled = True\n" + + " subprocess.run('/bin/true', shell=enabled)\n" + + "import subprocess\n" + + "trigger = 0\n" + + "execute()\n" + ) + + assert not findings diff --git a/tests/nodes/test_security_end_to_end.py b/tests/nodes/test_security_end_to_end.py index 46fbd34ff..b9d02d1b2 100644 --- a/tests/nodes/test_security_end_to_end.py +++ b/tests/nodes/test_security_end_to_end.py @@ -328,6 +328,119 @@ async def _assert_incomplete_across_public_surfaces( assert sc9["evidence"]["excluded_inspection_incomplete"] is True +@pytest.mark.parametrize( + "bound_value", + [ + pytest.param("True", id="boolean"), + pytest.param("'True'", id="reporter-truthy-string"), + ], +) +def test_tm1_bound_true_matches_literal_in_graph( + tmp_path: Path, + bound_value: str, +) -> None: + direct = tmp_path / "direct-shell" + bound = tmp_path / "bound-shell" + _write_bundle( + direct, + { + "SKILL.md": "# Shell helper", + "run.py": "import subprocess\nsubprocess.run(command, shell=True)\n", + }, + ) + _write_bundle( + bound, + { + "SKILL.md": "# Shell helper", + "run.py": ( + "import subprocess\n" + f"use_shell = {bound_value}\n" + "subprocess.run(command, shell=use_shell)\n" + ), + }, + ) + + direct_result = _scan(direct) + bound_result = _scan(bound) + direct_tm1 = _assert_rule(direct_result, "TM1", "run.py") + bound_tm1 = _assert_rule(bound_result, "TM1", "run.py") + + assert len(direct_tm1) == len(bound_tm1) == 1 + assert (bound_tm1[0].severity, bound_tm1[0].confidence) == ( + direct_tm1[0].severity, + direct_tm1[0].confidence, + ) + assert ( + bound_result["risk_score"], + bound_result["risk_severity"], + bound_result["risk_recommendation"], + ) == ( + direct_result["risk_score"], + direct_result["risk_severity"], + direct_result["risk_recommendation"], + ) + + +@pytest.mark.parametrize( + "source", + [ + pytest.param( + "import subprocess\nenabled = True\nFalse and (enabled := False)\n" + 'subprocess.run("echo ok", shell=enabled)\n', + id="untaken-short-circuit-store", + ), + pytest.param( + "import subprocess\nsubprocess, saved = subprocess, 1\nenabled = True\n" + 'subprocess.run("echo ok", shell=enabled)\n', + id="unpacked-receiver-self-store", + ), + pytest.param( + "import subprocess\nsaved = subprocess\nsubprocess = saved\nenabled = True\n" + 'subprocess.run("echo ok", shell=enabled)\n', + id="native-alias-round-trip", + ), + pytest.param( + "import subprocess\ndef run(command):\n enabled = True\n" + ' subprocess.run(command, shell=enabled)\nif True:\n run("echo ok")\n' + "subprocess = None\n", + id="invocation-before-future-store", + ), + ], +) +def test_tm1_bound_true_without_proven_replacement_in_graph(tmp_path: Path, source: str) -> None: + bundle = tmp_path / "bound-shell" + _write_bundle(bundle, {"SKILL.md": "# Shell helper", "run.py": source}) + + result = _scan(bundle) + tm1 = _assert_rule(result, "TM1", "run.py") + + assert len(tm1) == 1 + assert tm1[0].severity == "HIGH" + assert result["risk_recommendation"] != "SAFE" + + +@pytest.mark.asyncio +async def test_tm1_bound_true_across_public_surfaces(tmp_path: Path) -> None: + bound = tmp_path / "bound-shell-public" + _write_bundle( + bound, + { + "SKILL.md": "# Shell helper", + "run.py": ( + "import subprocess\nuse_shell = True\nsubprocess.run(command, shell=use_shell)\n" + ), + }, + ) + + result = _scan(bound) + _assert_rule(result, "TM1", "run.py") + await _assert_rules_across_public_surfaces( + bound, + expected_locations={"TM1": {"run.py"}}, + python_result=result, + ) + + @pytest.mark.parametrize( ("finding", "normal_files", "bypass_files", "rule_id", "normal_path", "bypass_path"), [