fix(analyzer): detect bound shell truthiness

[chrisknvidia]

Summary

• resolve definitely-truthy straight-line bindings used as shell= for ordinary Python subprocess calls
• preserve direct TM1 severity, confidence, source coordinates, contextual classification, and public evidence contracts
• invalidate facts conservatively across rebinding, imports, control flow, receiver mutation, protocol-consuming calls, and potentially observable stores
• keep unsupported aliases, generators/async functions, indirect calls, .pyw/shebang classification, recursive reporting, and platform parsing outside this focused change

Review context

This is the narrow issue #475 core split requested in the review of #497. It contains no scan-budget/CLI/reporting work and no Python execution-surface expansion.

Validation

• exact core contract: 15 expected failures before implementation, then 52 passes
• six independent review regressions verified red then green, covering finalizer/protocol mutation, receiver ordering, exact long-line identity, and global/nonlocal stores
• affected AST/security E2E suite: 157 passed
• full standard suite on the final head: 5,474 passed, 14 skipped, 39 deselected, 4 expected xfails
• built and installed wheel contains the new analyzer; installed-wheel probe emitted exactly one HIGH TM1 at the bound call
• Ruff lint/format, mypy, and diff checks passed

Fixes #475.
Split from #497 for focused architectural and security review.

[rng1995]

Reviewed exact head bd09251621e1edabcfb4615b04d420dbf83d02b6. Requesting changes because the issue #475 literal-versus-bound bypass remains when a later argument is effectful.

For enabled = True; subprocess.run(command, shell=enabled, env=build_env()), Python resolves the receiver and captures shell=True before evaluating the later env expression. The runtime shell value is therefore definitely true, but this head emits zero TM1 findings; the equivalent shell=True call emits one. Moving env=build_env() before shell=enabled correctly makes the value uncertain, so the two orders must not be treated identically.

_call_arguments_are_passive currently rejects the whole call when any argument is impure, including expressions evaluated after shell. Please make invalidation evaluation-order-aware and add before/after-shell parity regressions.

[rng1995]

[SkillSpector Review]

Reviewed exact head bd09251621e1edabcfb4615b04d420dbf83d02b6 and the complete bound-shell dataflow implementation.

The existing blocking review remains valid: _call_arguments_are_passive() rejects a call when any later argument is effectful, even though Python has already evaluated and captured an earlier shell=enabled value. This leaves the issue #475 bypass for shell=enabled, env=build_env() while the equivalent literal is detected. Make invalidation evaluation-order-aware and add before/after-shell parity regressions.

No duplicate inline comment was added.

[chrisknvidia]

@rng1995 I addressed the receiver-invalidation finding in 995d746: the current subprocess call is still reported, then effectful arguments clear receiver trust so later proxy calls are not misclassified. PTAL.

[rng1995]

[P2] Invalidate trusted receivers after effectful arguments (src/skillspector/nodes/analyzers/static_python_shell_truthiness.py:696-703, with analogous AnnAssign/Expr paths at 826-830 and 846-849). The original argument-order blocker is fixed, but an imported callback evaluated after shell= can mutate the caller module’s subprocess binding. The current call is correctly flagged because its receiver and shell value were captured first; however, trusted_names survives, so a subsequent Proxy.run call is falsely reported as subprocess. Exact runtime repro gives first=real, second=proxy, while the analyzer emits TM1 on both lines. Preserve the current finding, then clear trusted receiver names for later statements after any non-passive argument call, and add a regression.

[rng1995]

[SkillSpector Review]

Re-reviewed exact head 995d746cbc1c615a7bf17c04ed4943c917ce9beb. The original later-argument ordering defect is fixed, and the latest commit correctly preserves the current direct subprocess finding before clearing receiver trust after effectful arguments. All six hosted checks pass and the prior review thread is resolved.

One required receiver-invalidation case remains. An ordinary effectful call can rebind the caller's module-level subprocess name, but the non-direct call path clears value facts while retaining receiver trust unless the call AST itself contains a direct store/mutation. A subsequent proxy .run(..., shell=enabled) is therefore still misreported as subprocess TM1. The inline comment includes a concrete straight-line case. Invalidate trusted receivers after calls that are not proven receiver-safe, while preserving already-captured findings, and add a regression. This false positive blocks approval.