Repository navigation
fix(code_safety): js_exec_sync reads spawn-family arguments, not a regex span - #1087
Conversation
…gex span Follow-up to #1083 (owner: fix everything first). - A shell key nested inside another option ({env: {shell: ...}}) is no longer the call's option (thread 4572752253). - An inline hypatia:ignore on the child_process import line no longer hides the calls below it: bindings are read from the unsuppressed file (4572814517). - The compiled TypeScript form (0, ns.exec)(cmd), .execSync and (0, ns.spawn)(...) are followed (4572814520). - A ; or bracket inside a string, template or comment argument no longer ends the call (4572814514), and the two-level nesting limit is gone. Hypatia.Rules.JsCallScan walks each spawn/spawnSync/execFile/execFileSync call's arguments, skipping strings, templates (with ${...}) and comments, and reports only an object-literal argument whose own shell key is true or a string. Calls written inside strings or comments are not calls. Limits are named in the module doc. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0136P7ZkGCWGEyedTtdEHAPT
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 16 minutes. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. |
The readme-claims gate re-measures the tree: the new module makes lib/rules 44 source files and lib/ 158. README.adoc and the .claude/CLAUDE.md architecture count now match (scripts/check-readme-claims.sh passes). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0136P7ZkGCWGEyedTtdEHAPT
|
This security review couldn’t start because the paying account has exhausted its Codex allowance and has insufficient available credits. The payer can check their balance and add credits in the Codex usage dashboard, then try again. If you do not manage the paying account, contact this repository's admins. |
Codex Review Summary: ❌ FailedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![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: 360682fd80
ℹ️ 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".
Codex review of #1087: - A comment before the shell key ({ /* x */ shell: true }, or a // line) hid it; each property now starts at its first code byte. - The exec forms ((0, cp.exec)(cmd), cp.exec( ...) matched inside a string, template or comment; regex hits are now kept only at code offsets (JsCallScan.code_offsets/2, one walk shared with the spawn scan). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0136P7ZkGCWGEyedTtdEHAPT
|
This security review couldn’t start because the paying account has exhausted its Codex allowance and has insufficient available credits. The payer can check their balance and add credits in the Codex usage dashboard, then try again. If you do not manage the paying account, contact this repository's admins. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 224a1ba766
ℹ️ 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".
… lookup Codex review of #1087: - A parenthesised options object (spawn(c, a, ({shell: true}))) is read: object_start/3 steps over wrapping parentheses and the whitespace and comments around them. - A computed string key (['shell'], ["shell"], [`shell`]) names the option too. Not a regression; the old regex missed it as well. - code_offsets/2 walks the sorted offsets against the sorted, disjoint non-code spans in one pass, and shell_option_calls/2 uses it, so there is one lookup and it is linear. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0136P7ZkGCWGEyedTtdEHAPT
|
This security review couldn’t start because the paying account has exhausted its Codex allowance and has insufficient available credits. The payer can check their balance and add credits in the Codex usage dashboard, then try again. If you do not manage the paying account, contact this repository's admins. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b7b654d292
ℹ️ 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".
… not the object Codex review of #1087: - {...{shell: true}} (and a parenthesised spread) contributes its own top-level keys; a shell key nested deeper in the spread does not. - object_start/3 accepts an object (or a parenthesised one) only when it is the whole argument, so ({shell: true}, {shell: false}) and {shell: true} && opts are not read as that object. - The module doc names the remaining cost limit: nested spawn-family calls re-read their own argument lists. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0136P7ZkGCWGEyedTtdEHAPT
|
This security review couldn’t start because the paying account has exhausted its Codex allowance and has insufficient available credits. The payer can check their balance and add credits in the Codex usage dashboard, then try again. If you do not manage the paying account, contact this repository's admins. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9fe59fb899
ℹ️ 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".
Codex review of #1087: a ternary branch (cond ? {shell: true} : {...}) and a TypeScript assertion ({shell: true} as / satisfies SpawnOptions) were missed, because an object counted only when it was the whole argument. One rule now: an object literal counts when it is a top-level operand of the argument expression, reached only through wrapping parentheses; in a parenthesised comma expression only the last element counts; an object inside a call, array or other object never counts. This reports {shell: true} && opts again, as an accepted over-report; the module doc says so, and also names the JSX-text apostrophe limit. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0136P7ZkGCWGEyedTtdEHAPT
|
This security review couldn’t start because the paying account has exhausted its Codex allowance and has insufficient available credits. The payer can check their balance and add credits in the Codex usage dashboard, then try again. If you do not manage the paying account, contact this repository's admins. |
|
Summary
This follows up #1083, which auto-merged before the owner's "Fix everything first" items were done (ruling in the Mathesis thread, 2026-11-14). It addresses the four remaining review threads on #1083:
spawn(c, a, {env: {shell: '/bin/bash'}})was reported (false positive)shellkey countshypatia:ignoreon the import line blanked the binding, so latercp.exec(...)calls went unreported(0, child_process_1.exec)(cmd)was missed(0, ns.exec)(,(0, ns.execSync)(and(0, ns.spawn)(...)are followed;or bracket inside a string or template argument ended the regex spanDesign. A new module,
Hypatia.Rules.JsCallScan, replaces the spawn-family regex. For eachspawn,spawnSync,execFileorexecFileSynccall (or a renamed local, or the compiled form), it:${...}nesting), and//and/* */comments;()[]{}depth and splits the top-level arguments;shellkey (bare or quoted) istrueor a string.This also removes the old two-level nesting limit. A call written inside a string or a comment is no longer counted.
scan_content/2andscan_web_security/1share onerule_matches/3, so lines and occurrence counts agree.Remaining limits, named in the module doc and the rule comment:
${...}is not looked for.Evidence
test/code_safety_test.exs: one each for (A), (1), (2) and (3); calls inside strings or comments; and arguments nested three or more levels deep. All 6 fail onmain(6693f9a) and pass on this branch. All 99 existing tests are kept: 105 code_safety tests pass.test/rules.hypatia_rules_praxis.deedis unchanged (--checkpasses), the format check passes, and there are no compile warnings in the changed files.The owner approved this PR in my session on 2026-11-14.
Generated by Claude Code