fix(analyzer): detect bound shell truthiness - #577
chrisknvidia wants to merge 5 commits into
Conversation
Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
rng1995
left a comment
There was a problem hiding this comment.
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
left a comment
There was a problem hiding this comment.
[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.
…e-475-shell-truthiness-core Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
Summary
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
Fixes #475.
Split from #497 for focused architectural and security review.