feat(python): classify executable source surfaces - #579
chrisknvidia wants to merge 2 commits into
Conversation
rng1995
left a comment
There was a problem hiding this comment.
Reviewed exact draft head 266d42e050493dad869fe50be420ace97914ea82. I found no additional blocker in the focused Python execution-surface range (d856d88..266d42e): 1,118 affected tests passed with 1 skip, 732 current-main merged-tree regressions passed, and lint/format/diff checks are clean.
I am requesting changes because the current combined tree still contains the confirmed #576/#577 blockers: a three-row ledger cap drops a second distinct fatal fact, and a definitely true bound shell= value evades TM1 when a later argument is effectful. This PR also inherits draft #578 and explicitly requires the dependency stack to land, rebase onto current main, fresh exact-head CI, and current-head review.
Please propagate the dependency fixes and rebase. If the focused range remains semantically unchanged and CI stays green, I found it otherwise suitable for approval.
rng1995
left a comment
There was a problem hiding this comment.
The focused #579 bare-uv and execution-type parse-limit changes look correct, but this stacked head inherits the exact #577 receiver-invalidation blocker: an effectful argument after shell= may replace the trusted subprocess receiver, yet later proxy calls are still classified as subprocess and receive false-positive TM1 findings. Please update the stack after #577 clears trusted_names on this effectful path and add the regression. The current head is otherwise conflict-free and its affected/integration suites are clean.
rng1995
left a comment
There was a problem hiding this comment.
[SkillSpector Review]
Changes requested on exact head 59a8d9fd60b150b27457254979c256bc80b82e37 for a confirmed remaining issue inherited from #577.
The earlier effectful-argument fix is present: direct subprocess calls now invalidate receiver trust after unsafe arguments, including expression, assignment, and annotated-assignment paths. However, an ordinary effectful function call can still replace the receiver without invalidating that trust; a later call through the replacement object is then reported as HIGH TM1. The inline comment provides the source-traced fixture and expected correction. Please propagate the #577 correction through this stack and cover this case.
Scope: I inspected the prior review history, current dependency fix, receiver-trust collector and both scan passes, its caller integration, tests for the direct-argument correction, and current checks. This is a focused blocking review, not approval or certification of the entire large combined stack. The remaining combined diff still needs complete current-head assessment after the dependency issue is addressed. Six hosted checks pass, but passing checks do not establish this untested receiver semantic.
No contributor-provided code or tests were executed locally.
c1fbda5 to
d38a3d4
Compare
Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
d38a3d4 to
3ee7334
Compare
…tion Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
Adds consistent Python execution-surface classification from raw bytes across
.py,.pyw, PEP 263 encodings, extensionless shebangs, and uv launchers. The classification flows through artifact inventory, nested and transitive analysis, static and behavioral analyzers, inspection ledgers, and provider-cache boundaries.Ambiguous or platform-dependent inputs remain explicitly incomplete. Analysis retains the decoded Python source while provider views remain bounded and respect local-only, hidden, and nested-content boundaries. This is the Python execution-surface split requested in #497.
The latest update at
59a8d9fincorporates #577’s receiver-invalidation fix through the updated #578 stack. Later calls through a potentially replaced receiver no longer produce the reported false-positive TM1 findings. Regression coverage includes expression, assignment, annotated assignment, and subsequent function-call paths. The previously reviewed bare-uv and execution-type-aware parser-limit fixes are preserved, and the merge conflicts are resolved.Dependency state: #576 is merged. #577 at
995d746and #578 at1c03702remain open; both fixes are included here. The Python-specific review range isgit diff 1c03702..59a8d9f.Validation at
59a8d9f: all six hosted checks pass, including the unit suite with coverage and Docker scans of a local directory and a GitHub URL. Independent review of the dependency update and merge resolution found no remaining blocker. Fresh static CLI checks in JSON, SARIF, and Markdown verified.pyw, encoded Python, extensionless Python, and uv script inputs.Ready for re-review of the updated head.