Skip to content

fix(analyzer): resolve indirect subprocess shell values - #497

Open
chrisknvidia wants to merge 4 commits into
NVIDIA:mainfrom
chrisknvidia:feat/christopherk/issue-475-shell-truthiness
Open

fix(analyzer): resolve indirect subprocess shell values#497
chrisknvidia wants to merge 4 commits into
NVIDIA:mainfrom
chrisknvidia:feat/christopherk/issue-475-shell-truthiness

Conversation

@chrisknvidia

@chrisknvidia chrisknvidia commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Summary

  • fix shell=True detection can be bypassed through variable assignment (malicious skill example that bypasses this static detection model) #475 for direct one-level subprocess.* and bare Popen calls whose shell argument is a name bound to a definitely truthy immutable value, including both a = True and the reporter attachment's a = 'True'
  • preserve TM1's existing lexical classification semantics across raw, normalized, sliced, and continuity views by reconciling the bound form against its direct shell=True equivalent under the same artifact deadline
  • preserve direct-literal behavior, finding caps, locations, fingerprints, deduplication, ledger accounting, and public report output; reconciliation-only evidence is removed before findings leave the analyzer

Supported contract

The AST companion follows straight-line assignments at module scope and inside functions (including nested functions). It supports direct call expressions and assignment-RHS calls, a deliberately small passive immutable value subset, direct subprocess.* imports, and direct from subprocess import Popen bindings.

Unsupported expressions, compound control flow, class/method effects, import aliases, arbitrary dynamic effects, and return/yield positions are intentionally conservative boundaries: tracked facts are invalidated rather than guessed. Unresolved calls retain the existing AST4 finding without gaining TM1.

Verification

Candidate: 89ccd3681cf72e06691ff9ec0e4c9eddcf3f0770

  • regression-first history: the initial test commit reproduces the missing TM1 before the implementation commit
  • affected/shared/public-surface matrix: 889 passed
  • full non-integration/non-provider suite with coverage: 4,457 passed, 14 skipped, 38 deselected, 4 xfailed; 89% total coverage
  • installed-wheel CLI matrix: direct True, bound True, and bound 'True' produced equivalent HIGH/0.9 TM1 findings and identical risk results; false and control-flow-ambiguous controls produced no TM1
  • installed-wheel JSON, terminal, Markdown, and SARIF outputs; all complete, with correct source lines and no private reconciliation evidence
  • source distribution and wheel build; twine check, isolated install, and pip check
  • Ruff and format checks across src/ and tests/; mypy clean for all four touched production modules
  • no-cache Docker build on arm64; repository smoke passed version/git/local scan and a real octocat/Hello-World clone/scan; direct/bound/false matrix, all output formats, pip check, and compileall also passed
  • three independent defect-focused review passes covering scope invalidation, normalized/windowed parity, caps/deduplication, deadline/ledger behavior, public outputs, and regressions; no remaining actionable finding
  • normalization-heavy 900k-character probes completed within the artifact deadline with correct results and no private evidence leakage (slowest observed run: 26.3 seconds)
  • fresh GitHub CI on the exact candidate: changes, lint, test-unit, DCO, and Docker smoke all passed

Remaining verification limits

  • LLM-backed analyzers were not run because credentials were intentionally absent; this change is confined to the static TM1 path.
  • Local Docker verification was arm64-only; the pushed candidate also passed the hosted Ubuntu Docker job.
  • Repository-wide mypy is not currently a clean gate: it reports 139 existing errors in 15 untouched files. The four changed production modules pass independently.
  • Normalization-heavy near-limit inputs have limited runtime headroom. The analyzer fails closed through the existing partial/runtime-limit ledger contract when the shared deadline is exhausted.

Fixes #475

Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
@chrisknvidia
chrisknvidia marked this pull request as ready for review September 8, 2026 08:11
@chrisknvidia
chrisknvidia marked this pull request as draft September 8, 2026 08:28
Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
@chrisknvidia
chrisknvidia marked this pull request as ready for review September 8, 2026 23:30
@chrisknvidia
chrisknvidia marked this pull request as draft September 9, 2026 00:00
@chrisknvidia
chrisknvidia marked this pull request as ready for review September 9, 2026 06:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

shell=True detection can be bypassed through variable assignment (malicious skill example that bypasses this static detection model)

2 participants