feat: add subprocess wrapper detection (perl -e, python -c, node -e, ruby -e)#10
Conversation
…ruby -e) - Detect perl -e/-E, python -c, python3 -c, node -e/-E/--eval, ruby -e patterns - Pre-check in classifyBashCommand before LLM judge (deterministic, no runner call) - Return 'unsure' for these patterns (cannot statically analyze subprocess contents) - Update judge prompt taxonomy to include these patterns - Add 8 comprehensive test cases covering all variants (uppercase flags, long flags) - Tests verify that wrappers skip LLM call and return 'unsure' immediately - Tests verify non-wrapper commands still call the runner All 428 tests passing.
…ase tests Fixes from architect review: 1. **Regex improvements:** - Change quote requirement to optional argument: `perl -e $code` and `perl -e code` now detected - Use `(?:\s+\S|$)` pattern instead of `['"]` to match unquoted, variables, heredocs - Updated all patterns (perl, python, node, ruby) for consistency 2. **Edge case tests:** - Added test for unquoted form: `perl -e some_code` - Added test for variable expansion: `python -c $payload` - Added test for false-positive (acceptable fail-safe): `echo "perl -e foo"` marks unsure even though echo is harmless 3. **Documentation:** - Updated function comment to explain false-positive acceptance (fail-safe is better than false negatives) - Updated prompt taxonomy to note these patterns are caught by pre-check **All 431 tests passing** (no regressions, +3 edge case tests for coverage).
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0f12377527
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
|
|
||
| // node -e / -E / --eval with any argument | ||
| if (/\bnode\b.*(?:\s-[eE]|--eval)(?:\s+\S|$)/.test(command)) { |
There was a problem hiding this comment.
The new Node wrapper regex misses the canonical long-option form --eval=...: node --help documents -e, --eval=..., but this pattern only matches when --eval is followed by whitespace or end-of-command. As a result, commands like node --eval=require('fs').writeFileSync(...) bypass the deterministic unsure guard and fall through to the LLM path, which can incorrectly suppress review if it returns inspection_vcs_noop; at minimum, this defeats the intended hard fail-safe for opaque one-liners.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d87178de7e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
|
|
||
| // python / python3 -c with any argument | ||
| if (/\bpython\d?\b.*\s-c(?:\s+\S|$)/.test(command)) { |
There was a problem hiding this comment.
The wrapper regexes require whitespace after the flag via (?:\s+\S|$), so valid no-space forms like python -c'print(1)', ruby -e'puts 1', and perl -e'print 1' are missed by the deterministic guard. Those commands then fall through to the LLM classifier, which can return inspection_vcs_noop and incorrectly suppress review for code that may mutate files. Since the goal of this pre-check is to fail safe on opaque one-liners, these common shell forms should also be matched.
Useful? React with 👍 / 👎.
1 participant
Description
Deterministic pre-check in judge to detect opaque subprocess wrapper patterns that cannot be statically analyzed.
Changes
detectSubprocessWrapper()pre-check returning 'unsure' for perl -e/-E, python -c, node -e/-E/--eval, ruby -eclassifyBashCommand()to call pre-check before LLM judgeWhy
Subprocess one-liners are inherently opaque—their behavior cannot be determined from the command line alone. Conservative fail ('unsure') is better than LLM timeout or false negatives.
Architect Review Fixes (Commit 0f12377)
-e ') to optional argument pattern ((?:\s+\S|$)) to handle unquoted (perl -e code), variables (python -c $payload), and heredocsecho "perl -e foo"returns 'unsure' — acceptable fail-safe)Test Results
Related
Aligns with pi-branch-enforcer Tier 1.5 subprocess detection (same language patterns, different tool used at push-gate vs. code review)