Skip to content

feat(python): classify executable source surfaces - #579

Open
chrisknvidia wants to merge 45 commits into
NVIDIA:mainfrom
chrisknvidia:feat/christopherk/python-execution-surface-classification
Open

chrisknvidia wants to merge 45 commits into
NVIDIA:mainfrom
chrisknvidia:feat/christopherk/python-execution-surface-classification

Conversation

@chrisknvidia

@chrisknvidia chrisknvidia commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

Summary

  • classify Python execution surfaces from raw bytes across .py, .pyw, PEP 263 encodings, extensionless shebangs, and ambiguous inputs
  • model platform-specific /usr/bin/env behavior for CPython, PyPy, and uv script launchers, failing closed when execution depends on platform, aliases, options, or invocation spelling
  • propagate classification through artifact inventory, nested/transitive analysis, static and behavioral analyzers, build context, inspection ledgers, and provider-cache boundaries
  • preserve exact decoded Python for analysis while keeping bounded provider views and preventing local-only, hidden, or nested content leakage

Review context

This is the Python decoding, shebang/platform-command parsing, and execution-surface split requested in the review of #497.

Open stacked dependency: #578 at 1404c29671c35e48e13f0da2c602d6cf16d4e5af, which contains the current #576 head fb86cf753c8e4a15cbaca12f39f106bc4a6cf8df and #577 head c3b0ff945013f2cc0eeaeb66515ba099d184478a. All three dependency PRs are ready for review but remain open and unmerged. This branch integrates them and current origin/main (d162d9b343e559be13df8ebba093df3bc9d58c90) with normal signed merge commits.

The current PR-specific review diff against the exact #578 dependency head is:

git diff 1404c29671c35e48e13f0da2c602d6cf16d4e5af..9e05e2987a8dd9d502c5ce6845d3e5ab275fbb12

That delta is 23 files (+5,380/-90). The current head is an empty DCO-signed CI retrigger commit whose tree is identical to reviewed commit 9cf3d3109091c93c7477a0a9204b0007bdc76e8b. The original Python-only range remains unchanged at d856d88c289c5ef3389060ef69651c9981aae38f..266d42e050493dad869fe50be420ace97914ea82 (23 files, +5,211/-89); the additional delta contains review fixes for selectable bare-uv sources and execution-type-aware parser-limit accounting.

Validation

  • focused classifier/cache/provider-boundary/ledger/analyzer/uv matrix: 1,468 passed, 9 skipped
  • affected static/parser regression matrix: 1,375 passed
  • full non-provider run: 6,644 passed, 14 skipped, 39 deselected, 4 expected xfails; the sole failure was an unchanged 5-second FIFO subprocess timing test under unrelated high host load, and that exact test then passed 3/3 in isolation
  • integration suite excluding live agent CLIs: 29 passed; live Claude no-model, injection-handling, and full graph paths: 3 passed
  • live Codex CLI: 2 environment failures because the isolated local runtime rejected its configured gpt-5.6-sol default; Gemini CLI absent; hosted OpenAI/Anthropic/NVIDIA provider tests skipped because their API keys are not configured
  • OpenCode TypeScript tests: 44 passed; Pi extension security tests: 21 passed
  • Ruff, formatting, diff integrity, and DCO audit clean; python_ast.py targeted mypy clean (the broader stack surfaces six pre-existing errors on older main-owned lines)
  • wheel built and installed into a fresh Python 3.13 environment; installed CLI JSON, Markdown, and SARIF scans all completed and preserved TM1, python_source_ambiguous, and static_parse_limit output
  • real uv runtime covered env/direct bare launchers, run and option-like source names, script selectors, global options, macOS case aliases, and option-like paths
  • two independent final code-review passes found the bare-uv and parser-exhaustion fail-closed gaps; both are fixed, regression-covered, and the final tree was re-reviewed with no findings

All six hosted checks pass at 9e05e2987a8dd9d502c5ce6845d3e5ab275fbb12, including the Linux unit and Docker smoke jobs. GitHub reports the branch conflict-free (MERGEABLE); the stacked dependencies remain open and unmerged.

Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
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 rng1995 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

…e-475-shell-truthiness-core

Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
…rsive-fail-closed-reporting

Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
Signed-off-by: Christopher Kevin <christopherk@nvidia.com>

# Conflicts:
#	src/skillspector/nodes/analyzers/static_patterns_tool_misuse.py
#	src/skillspector/nodes/analyzers/static_runner.py
Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
Signed-off-by: Christopher Kevin <christopherk@nvidia.com>

# Conflicts:
#	src/skillspector/nodes/analyzers/static_patterns_tool_misuse.py
#	src/skillspector/nodes/analyzers/static_python_shell_truthiness.py
#	tests/nodes/analyzers/test_tool_misuse_python_ast.py
…ssification

Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
…surface-classification

Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
…urface-classification

Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
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 21, 2026 05:23
@chrisknvidia

chrisknvidia commented Sep 21, 2026

Copy link
Copy Markdown
Contributor Author

@rng1995 The dependency fixes are included, the merge conflicts are resolved, and the bare-uv and parser-limit findings are fixed. This PR is ready for review. PTAL.

@rng1995 rng1995 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

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.

2 participants