From 819c08cb21a4b7664523ce33d07f6e9b8004210c Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 14 Nov 2026 10:10:57 +0000 Subject: [PATCH 1/6] fix(code_safety): js_exec_sync reads spawn-family arguments, not a regex 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 Claude-Session: https://claude.ai/code/session_0136P7ZkGCWGEyedTtdEHAPT --- lib/rules/code_safety.ex | 91 +++++++++----- lib/rules/js_call_scan.ex | 246 ++++++++++++++++++++++++++++++++++++++ test/code_safety_test.exs | 41 +++++++ 3 files changed, 350 insertions(+), 28 deletions(-) create mode 100644 lib/rules/js_call_scan.ex diff --git a/lib/rules/code_safety.ex b/lib/rules/code_safety.ex index 6ac1888b..868b27d5 100644 --- a/lib/rules/code_safety.ex +++ b/lib/rules/code_safety.ex @@ -614,24 +614,25 @@ defmodule Hypatia.Rules.CodeSafety do # a local function named exec is not a shell; # * .exec( / .execSync( where is the name the file binds # node:child_process to (import * as ns / import ns / require / - # await import), and + # await import), the compiled TypeScript form `(0, ns.exec)(`, and # require('child_process').exec( -- any argument shape; # * spawn/spawnSync/execFile/execFileSync (or a name the file imports - # them as) called with a `shell` option (bare or quoted key) set to - # true or to a shell path. fork() ignores `shell`, so it is not here. + # them as, or `(0, ns.spawn)(`) whose options object has its own + # `shell` key (bare or quoted) set to true or to a string. This is + # read by Hypatia.Rules.JsCallScan, not a regex: strings, templates + # and comments are skipped, and a `shell` key inside a nested + # object (`{env: {shell: 'x'}}`) is not the call's option. + # fork() ignores `shell`, so it is not here. # Importing node:child_process is not a finding, nor is a shell-free # call with an argv array, RegExp.exec, or a `shell` key outside a - # process call. The calls are matched per file by - # pattern_for/2 (`bound_calls`), so every call counts, on its own line. + # process call. Bindings are read from the file before any inline + # `hypatia:ignore` blanks a line, so ignoring the import line does not + # hide the calls below it; every call counts, on its own line. # Not covered: a shell option passed through a variable, tainted argv to - # a shell-free call, and call arguments nested more than two - # parentheses deep -- a pattern cannot trace values (mathesis#90). - # Known false positive: a `shell` key nested inside another option of a - # process call (`spawn(c, a, {env: {shell: 'x'}})`) is reported; telling - # object depth apart needs a parser, and a rare false alarm is the safer - # error for a high-severity rule. + # a shell-free call, and the JsCallScan limits (a regex literal holding + # a quote, a call inside a template's ${...}) (mathesis#90). pattern: - ~r/\bexecSync\s*\(|require\(\s*['"](?:node:)?child_process['"]\s*\)\s*\.exec(?:Sync)?\s*\(|\b(?:spawn|spawnSync|execFile|execFileSync)\s*\((?:[^;()]|\((?:[^()]|\([^()]*\))*\))*?['"]?\bshell['"]?\s*:\s*(?:true\b|['"`])/, + ~r/\bexecSync\s*\(|require\(\s*['"](?:node:)?child_process['"]\s*\)\s*\.exec(?:Sync)?\s*\(/, bound_calls: :child_process, cwe: "CWE-78", description: "Shell execution -- validate input before passing to shell" @@ -922,7 +923,7 @@ defmodule Hypatia.Rules.CodeSafety do # line where the match starts) is not a finding; nor is `unsafe {` # inside a string literal. Matches are excused after the scan, so a # block whose brace is on the next line is judged as one block. - matches = Regex.scan(pattern_for(rule, subject), subject, return: :index) + matches = rule_matches(rule, subject, content) matches = if rule.id == :unsafe_block, @@ -1656,7 +1657,7 @@ defmodule Hypatia.Rules.CodeSafety do # * destructured exec or execSync (`import {exec, execSync as run} # from`, `const {exec: run} = require(...)`) adds `exec(` / `run(`; # * a renamed spawn/spawnSync/execFile/execFileSync (`{spawn as run}`) - # adds `run(` with a `shell` option, like the unrenamed names. + # is handed to JsCallScan, like the unrenamed names. # `require(...)` may also be `import(...)` or `await import(...)`. # Regex sources, compiled at call time: a compiled regex in a module # attribute does not survive OTP 28. @@ -1670,12 +1671,8 @@ defmodule Hypatia.Rules.CodeSafety do @cp_source <> ~S/|(?:const|let|var)\s*\{([^}]*)\}\s*=\s*/ <> @cp_load <> @cp_source <> ~S|\s*\))| - # The argument span of a call up to a `shell` option set to true or to a - # shell path; arguments may nest parentheses two deep. - @shell_option_tail ~S/\s*\((?:[^;()]|\((?:[^()]|\([^()]*\))*\))*?['"]?\bshell['"]?\s*:\s*(?:true\b|['"`])/ - @doc false - def pattern_for(%{bound_calls: :child_process} = rule, content) do + def child_process_bindings(content) do namespaces = captures(Regex.compile!(@cp_namespace), content) bindings = @@ -1695,17 +1692,36 @@ defmodule Hypatia.Rules.CodeSafety do end) |> Enum.uniq() - # exec/execSync run a shell whatever the arguments; the spawn family - # does only with a `shell` option (unrenamed names are in the pattern). - execs = for {name, local} <- bindings, name in ["exec", "execSync"], do: local + %{ + namespaces: namespaces, + # exec/execSync run a shell whatever the arguments. + execs: for({name, local} <- bindings, name in ["exec", "execSync"], do: local), + # The spawn family does only with a `shell` option; the plain names + # are always looked for, so only renamed ones are listed. + spawns: + for( + {name, local} <- bindings, + name not in ["exec", "execSync"], + local != name, + do: local + ) + } + end - spawns = - for {name, local} <- bindings, name not in ["exec", "execSync"], local != name, do: local + @doc false + def pattern_for(%{bound_calls: :child_process} = rule, content) do + %{namespaces: namespaces, execs: execs} = child_process_bindings(content) extra = - Enum.map(namespaces, &("(? Regex.escape(&1) <> "\\.exec(?:Sync)?\\s*\\(")) ++ - Enum.map(execs, &("(? Regex.escape(&1) <> "\\s*\\(")) ++ - Enum.map(spawns, &("(? Regex.escape(&1) <> @shell_option_tail)) + Enum.flat_map(namespaces, fn ns -> + e = Regex.escape(ns) + + [ + "(? e <> "\\.exec(?:Sync)?\\s*\\(", + "\\(\\s*0\\s*,\\s*" <> e <> "\\s*\\.\\s*exec(?:Sync)?\\s*\\)\\s*\\(" + ] + end) ++ + Enum.map(execs, &("(? Regex.escape(&1) <> "\\s*\\(")) case extra do [] -> rule.pattern @@ -1715,6 +1731,25 @@ defmodule Hypatia.Rules.CodeSafety do def pattern_for(rule, _content), do: rule.pattern + # Match offsets ([[{offset, length}]], as Regex.scan return: :index) of a + # rule in `subject`. `content` is the unsuppressed file: a binding is read + # there even when an inline directive blanked its line in `subject`. + @doc false + def rule_matches(%{bound_calls: :child_process} = rule, subject, content) do + regex_hits = Regex.scan(pattern_for(rule, content), subject, return: :index) + + shell_hits = + subject + |> Hypatia.Rules.JsCallScan.shell_option_calls(child_process_bindings(content).spawns) + |> Enum.map(&[{&1, 1}]) + + (regex_hits ++ shell_hits) + |> Enum.uniq_by(fn [{off, _} | _] -> off end) + |> Enum.sort_by(fn [{off, _} | _] -> off end) + end + + def rule_matches(rule, subject, _content), do: Regex.scan(rule.pattern, subject, return: :index) + defp captures(regex, content) do regex |> Regex.scan(content, capture: :all_but_first) @@ -1726,7 +1761,7 @@ defmodule Hypatia.Rules.CodeSafety do @doc "Scan JavaScript/TypeScript content for web security issues" def scan_web_security(content) do Enum.flat_map(@javascript_patterns, fn rule -> - case Regex.scan(pattern_for(rule, content), content) do + case rule_matches(rule, content, content) do [] -> [] diff --git a/lib/rules/js_call_scan.ex b/lib/rules/js_call_scan.ex new file mode 100644 index 00000000..419076a5 --- /dev/null +++ b/lib/rules/js_call_scan.ex @@ -0,0 +1,246 @@ +# SPDX-License-Identifier: MPL-2.0 + +defmodule Hypatia.Rules.JsCallScan do + @moduledoc """ + A small JavaScript-aware reader for one question: does a call to a + process API pass an options object whose own `shell` key is `true` or a + string? (`js_exec_sync`, hyperpolymath/mathesis#90.) + + A regex cannot answer it: `;` and parentheses inside string or template + arguments end a regex span early, and a `shell` key inside a nested + object (`{env: {shell: '/bin/bash'}}`) is not the call's option. This + reader walks the call's argument list skipping strings, template + literals (with `${...}` nesting), and `//` and `/* */` comments, tracks + `()[]{}` depth, and looks only at the top level of each object-literal + argument. + + Limits, stated rather than guessed at: a regex literal containing a quote + or bracket can desynchronise the walk; an options object passed through a + variable is not followed; a call written inside a template literal's + `${...}` is not looked for. + """ + + @spawn_family ~w(spawn spawnSync execFile execFileSync) + + @doc """ + Byte offsets of calls whose options set `shell`. `locals` are extra + callee names the file binds the spawn family to (`{spawn as run}`). + Calls are found as `spawn(`, `ns.spawn(`, a local name, or the compiled + form `(0, ns.spawn)(`, outside strings and comments. + """ + def shell_option_calls(src, locals \\ []) when is_binary(src) do + family = Enum.join(@spawn_family, "|") + + local_alt = + case locals do + [] -> [] + names -> ["(? Enum.map_join(names, "|", &Regex.escape/1) <> ")\\s*\\("] + end + + source = + Enum.join( + [ + "\\b(?:" <> family <> ")\\s*\\(", + "\\(\\s*0\\s*,\\s*[A-Za-z_$][\\w$]*\\s*\\.\\s*(?:" <> family <> ")\\s*\\)\\s*\\(" + | local_alt + ], + "|" + ) + + noncode = noncode_spans(src) + + source + |> Regex.compile!() + |> Regex.scan(src, return: :index) + |> Enum.flat_map(fn [{off, len} | _] -> + open = off + len - 1 + + if not inside?(noncode, off) and shell_option?(src, open), do: [off], else: [] + end) + end + + # ── the call's arguments ──────────────────────────────────────────────── + + # `open` is the byte of the call's `(`. + defp shell_option?(src, open) do + case split_top_level(src, open + 1, ?)) do + {:ok, args, _close} -> + Enum.any?(args, fn {from, to} -> + case first_code_byte(src, from, to) do + {:ok, brace} when brace < to -> + if :binary.at(src, brace) == ?{, do: object_has_shell?(src, brace), else: false + + _ -> + false + end + end) + + :error -> + false + end + end + + defp object_has_shell?(src, brace) do + case split_top_level(src, brace + 1, ?}) do + {:ok, props, _close} -> + Enum.any?(props, fn {from, to} -> + Regex.match?( + ~r/\A\s*(?:shell|'shell'|"shell")\s*:\s*(?:true\b|['"`])/, + binary_part(src, from, to - from) + ) + end) + + :error -> + false + end + end + + # Split the text from `i` up to the matching `closer` (at depth 0) into + # top-level comma-separated spans. Strings, templates and comments are + # skipped whole, so their commas and brackets do not count. + defp split_top_level(src, i, closer), do: split(src, i, closer, 0, i, []) + + defp split(src, i, _closer, _depth, _start, _acc) when i >= byte_size(src), do: :error + + defp split(src, i, closer, depth, start, acc) do + case :binary.at(src, i) do + c when c in [?', ?", ?`] -> + split(src, skip_string(src, i), closer, depth, start, acc) + + ?/ -> + case comment_end(src, i) do + nil -> split(src, i + 1, closer, depth, start, acc) + j -> split(src, j, closer, depth, start, acc) + end + + c when c in [?(, ?[, ?{] -> + split(src, i + 1, closer, depth + 1, start, acc) + + c when c in [?), ?], ?}] and depth > 0 -> + split(src, i + 1, closer, depth - 1, start, acc) + + ^closer when depth == 0 -> + {:ok, Enum.reverse([{start, i} | acc]), i} + + c when c in [?), ?], ?}] -> + :error + + ?, when depth == 0 -> + split(src, i + 1, closer, depth, i + 1, [{start, i} | acc]) + + _ -> + split(src, i + 1, closer, depth, start, acc) + end + end + + defp first_code_byte(src, i, to) when i < to do + case :binary.at(src, i) do + c when c in [?\s, ?\t, ?\r, ?\n] -> + first_code_byte(src, i + 1, to) + + ?/ -> + case comment_end(src, i) do + nil -> {:ok, i} + j -> first_code_byte(src, j, to) + end + + _ -> + {:ok, i} + end + end + + defp first_code_byte(_src, _i, _to), do: :none + + # ── strings, templates, comments ──────────────────────────────────────── + + # Index just past the string, template or comment that starts at `i`. + defp skip_string(src, i) do + case :binary.at(src, i) do + ?` -> skip_template(src, i + 1) + q -> skip_quoted(src, i + 1, q) + end + end + + defp skip_quoted(src, i, _q) when i >= byte_size(src), do: i + + defp skip_quoted(src, i, q) do + case :binary.at(src, i) do + ?\\ -> skip_quoted(src, i + 2, q) + ^q -> i + 1 + ?\n -> i + 1 + _ -> skip_quoted(src, i + 1, q) + end + end + + defp skip_template(src, i) when i >= byte_size(src), do: i + + defp skip_template(src, i) do + case :binary.at(src, i) do + ?\\ -> + skip_template(src, i + 2) + + ?` -> + i + 1 + + ?$ when i + 1 < byte_size(src) -> + if :binary.at(src, i + 1) == ?{ do + case split_top_level(src, i + 2, ?}) do + {:ok, _, close} -> skip_template(src, close + 1) + :error -> byte_size(src) + end + else + skip_template(src, i + 1) + end + + _ -> + skip_template(src, i + 1) + end + end + + # End of a `//` or `/* */` comment starting at `i`, or nil if `/` at `i` + # does not start one. + defp comment_end(src, i) do + if i + 1 < byte_size(src) do + case :binary.at(src, i + 1) do + ?/ -> + case :binary.match(src, "\n", scope: {i, byte_size(src) - i}) do + {nl, _} -> nl + 1 + :nomatch -> byte_size(src) + end + + ?* -> + case :binary.match(src, "*/", scope: {i + 2, byte_size(src) - i - 2}) do + {e, _} -> e + 2 + :nomatch -> byte_size(src) + end + + _ -> + nil + end + end + end + + # Spans of the file that are strings, templates or comments. + defp noncode_spans(src), do: noncode(src, 0, []) + + defp noncode(src, i, acc) when i >= byte_size(src), do: Enum.reverse(acc) + + defp noncode(src, i, acc) do + case :binary.at(src, i) do + c when c in [?', ?", ?`] -> + j = skip_string(src, i) + noncode(src, j, [{i, j} | acc]) + + ?/ -> + case comment_end(src, i) do + nil -> noncode(src, i + 1, acc) + j -> noncode(src, j, [{i, j} | acc]) + end + + _ -> + noncode(src, i + 1, acc) + end + end + + defp inside?(spans, off), do: Enum.any?(spans, fn {a, b} -> off >= a and off < b end) +end diff --git a/test/code_safety_test.exs b/test/code_safety_test.exs index f45d5426..826a0355 100644 --- a/test/code_safety_test.exs +++ b/test/code_safety_test.exs @@ -972,6 +972,47 @@ defmodule Hypatia.Rules.CodeSafetyTest do refute exec_sync?("import {spawn as run} from 'node:child_process';\nrun(cmd, a);\n") end + test "a shell key nested inside another option is not the call's option (A)" do + refute exec_sync?("spawn(c, a, {env: {shell: '/bin/bash'}});\n") + refute exec_sync?("spawnSync(c, a, {env: {...process.env, shell: true}, cwd});\n") + assert exec_sync?("spawn(c, a, {env: {shell: 'x'}, shell: true});\n") + end + + test "an ignore directive on the import line does not hide the calls below it (1)" do + code = + "import * as cp from 'node:child_process'; // hypatia:ignore js_exec_sync -- import only\n" <> + "\ncp.exec(command);\n" + + [f] = Enum.filter(CodeSafety.scan_content(code, "javascript"), &(&1.rule == :js_exec_sync)) + assert f.lines == [3] + end + + test "the compiled TypeScript call form is followed (2)" do + code = + "const child_process_1 = require(\"child_process\");\n" <> + "(0, child_process_1.exec)(cmd);\n(0, child_process_1.execSync)(cmd);\n" <> + "(0, child_process_1.spawn)(cmd, args, { shell: true });\n" + + [f] = Enum.filter(CodeSafety.scan_content(code, "javascript"), &(&1.rule == :js_exec_sync)) + assert f.lines == [2, 3, 4] + refute exec_sync?("const cp_1 = require('child_process');\n(0, cp_1.spawn)(cmd, args);\n") + end + + test "a ; or bracket inside a string or template argument does not end the call (3)" do + assert exec_sync?("spawn(`echo ${input}; date`, [], {shell: true});\n") + assert exec_sync?("spawnSync('a); b', ['(', ']'], {shell: true});\n") + assert exec_sync?("execFile(cmd, [x], /* ; ) */ {shell: '/bin/sh'});\n") + end + + test "a spawn call written inside a string or comment is not a call" do + refute exec_sync?("const doc = 'spawn(cmd, a, {shell: true})';\n") + refute exec_sync?("// spawn(cmd, a, {shell: true})\n") + end + + test "arguments nested deeper than two levels no longer hide the option" do + assert exec_sync?("spawn(a(b(c(d))), [e(f(g()))], {shell: true});\n") + end + test "a dynamic import binds child_process too" do assert exec_sync?("const cp = await import('node:child_process');\ncp.exec(command);\n") assert exec_sync?("const {exec: sh} = await import('child_process');\nsh(command);\n") From bab176b1d0615cabf1da4c733830e28377768837 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 14 Nov 2026 10:15:06 +0000 Subject: [PATCH 2/6] docs: count lib/rules/js_call_scan.ex (44 rule files, 158 under lib/) 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 Claude-Session: https://claude.ai/code/session_0136P7ZkGCWGEyedTtdEHAPT --- .claude/CLAUDE.md | 2 +- README.adoc | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.claude/CLAUDE.md b/.claude/CLAUDE.md index 20a593ed..01440beb 100644 --- a/.claude/CLAUDE.md +++ b/.claude/CLAUDE.md @@ -13,7 +13,7 @@ Hypatia is the neurosymbolic CI/CD intelligence layer for the hyperpolymath ecos ## Architecture ``` -Hypatia # 157 Elixir source files under lib/ (recursive tree count, 2026-11-13) +Hypatia # 158 Elixir source files under lib/ (recursive tree count, 2026-11-14) ├── Elixir pipeline # core: pattern analysis, dispatch, learning ├── Merge orchestration # 12 modules — leases, ledger, ticker (largest subsystem) ├── Kin subsystem # 6 modules — contingency, arbiter, gate, watchdog diff --git a/README.adoc b/README.adoc index 282681da..f018afd6 100644 --- a/README.adoc +++ b/README.adoc @@ -20,7 +20,7 @@ toc::[] It combines: * *Symbolic reasoning* — Elixir rules producing fast, deterministic findings, in - 43 source files under `lib/rules/` + 44 source files under `lib/rules/` (link:docs/rules/catalogue.adoc[full rule catalogue]) * *Neural learning* — 8 networks on a shared blackboard, learning which fixes actually stick * *Bayesian confidence* — every recipe carries a Beta-distributed confidence updated from real @@ -76,8 +76,8 @@ PatternRegistry ──> TriangleRouter ──> FleetDispatcher ──> DispatchM [cols="1,3"] |=== -| `lib/` | 157 Elixir source files — pipeline, rules, VCL, neural, safety, web, merge orchestration -| `lib/rules/` | 43 source files: rule modules and their helpers; see link:docs/rules/catalogue.adoc[the catalogue] +| `lib/` | 158 Elixir source files — pipeline, rules, VCL, neural, safety, web, merge orchestration +| `lib/rules/` | 44 source files: rule modules and their helpers; see link:docs/rules/catalogue.adoc[the catalogue] | `lib/neural/` | 8 networks + blackboard, coordinator, persistence, rebalancer | `lib/vcl/` | VCL query layer over verisim-data: parser, file executor, cache, federation | `lib/merge_orchestration/` | 12 modules — leases, ledger, ticker From 224a1ba7663b67170bbd6ba9ef22247225e5f679 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 14 Nov 2026 10:27:06 +0000 Subject: [PATCH 3/6] fix(code_safety): comments before the shell key; no exec hits in strings 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 Claude-Session: https://claude.ai/code/session_0136P7ZkGCWGEyedTtdEHAPT --- lib/rules/code_safety.ex | 13 ++++++++++++- lib/rules/js_call_scan.ex | 26 +++++++++++++++++++++----- test/code_safety_test.exs | 21 +++++++++++++++++++++ 3 files changed, 54 insertions(+), 6 deletions(-) diff --git a/lib/rules/code_safety.ex b/lib/rules/code_safety.ex index b1d082da..acb81835 100644 --- a/lib/rules/code_safety.ex +++ b/lib/rules/code_safety.ex @@ -1746,7 +1746,18 @@ defmodule Hypatia.Rules.CodeSafety do # there even when an inline directive blanked its line in `subject`. @doc false def rule_matches(%{bound_calls: :child_process} = rule, subject, content) do - regex_hits = Regex.scan(pattern_for(rule, content), subject, return: :index) + hits = Regex.scan(pattern_for(rule, content), subject, return: :index) + + # A call written in a string, template or comment is not a call. + code = + MapSet.new( + Hypatia.Rules.JsCallScan.code_offsets( + subject, + Enum.map(hits, fn [{off, _} | _] -> off end) + ) + ) + + regex_hits = Enum.filter(hits, fn [{off, _} | _] -> MapSet.member?(code, off) end) shell_hits = subject diff --git a/lib/rules/js_call_scan.ex b/lib/rules/js_call_scan.ex index 419076a5..2e6680e8 100644 --- a/lib/rules/js_call_scan.ex +++ b/lib/rules/js_call_scan.ex @@ -17,7 +17,7 @@ defmodule Hypatia.Rules.JsCallScan do Limits, stated rather than guessed at: a regex literal containing a quote or bracket can desynchronise the walk; an options object passed through a variable is not followed; a call written inside a template literal's - `${...}` is not looked for. + `${...}` is treated as text, not code, for every form of the rule. """ @spawn_family ~w(spawn spawnSync execFile execFileSync) @@ -84,10 +84,17 @@ defmodule Hypatia.Rules.JsCallScan do case split_top_level(src, brace + 1, ?}) do {:ok, props, _close} -> Enum.any?(props, fn {from, to} -> - Regex.match?( - ~r/\A\s*(?:shell|'shell'|"shell")\s*:\s*(?:true\b|['"`])/, - binary_part(src, from, to - from) - ) + # A comment may sit before the key: start at the first code byte. + case first_code_byte(src, from, to) do + {:ok, key} -> + Regex.match?( + ~r/\A(?:shell|'shell'|"shell")\s*:\s*(?:true\b|['"`])/, + binary_part(src, key, to - key) + ) + + :none -> + false + end end) :error -> @@ -220,6 +227,15 @@ defmodule Hypatia.Rules.JsCallScan do end end + @doc """ + The offsets in `offsets` that are code, not inside a string, template + literal or comment of `src`. + """ + def code_offsets(src, offsets) when is_binary(src) do + spans = noncode_spans(src) + Enum.reject(offsets, &inside?(spans, &1)) + end + # Spans of the file that are strings, templates or comments. defp noncode_spans(src), do: noncode(src, 0, []) diff --git a/test/code_safety_test.exs b/test/code_safety_test.exs index 3a9512d5..eed3d9f6 100644 --- a/test/code_safety_test.exs +++ b/test/code_safety_test.exs @@ -1034,6 +1034,27 @@ defmodule Hypatia.Rules.CodeSafetyTest do refute exec_sync?("// spawn(cmd, a, {shell: true})\n") end + test "a comment before the shell key does not hide it" do + assert exec_sync?("spawn(c, a, { /* use shell */ shell: true });\n") + assert exec_sync?("spawn(c, a, {\n // needs a shell\n shell: true\n});\n") + end + + test "exec calls written in a string or comment are not calls" do + bound = "const cp_1 = require('child_process');\n" + + for line <- [ + "const s = '(0, cp_1.exec)(cmd)';\n", + "// (0, cp_1.execSync)(cmd)\n", + "const t = `cp_1.exec(cmd)`;\n", + "/* cp_1.exec(cmd) */\n" + ] do + refute exec_sync?(bound <> line), line + end + + assert exec_sync?(bound <> "(0, cp_1.exec)(cmd);\n") + assert exec_sync?(bound <> "cp_1.exec(cmd);\n") + end + test "arguments nested deeper than two levels no longer hide the option" do assert exec_sync?("spawn(a(b(c(d))), [e(f(g()))], {shell: true});\n") end From b7b654d2921bb4773a05246888f004268c783f49 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 14 Nov 2026 10:34:34 +0000 Subject: [PATCH 4/6] fix(code_safety): JsCallScan reads ({...}) and ["shell"] keys; linear 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 Claude-Session: https://claude.ai/code/session_0136P7ZkGCWGEyedTtdEHAPT --- lib/rules/js_call_scan.ex | 62 ++++++++++++++++++++++++++------------- test/code_safety_test.exs | 23 +++++++++++++++ 2 files changed, 65 insertions(+), 20 deletions(-) diff --git a/lib/rules/js_call_scan.ex b/lib/rules/js_call_scan.ex index 2e6680e8..c4f28863 100644 --- a/lib/rules/js_call_scan.ex +++ b/lib/rules/js_call_scan.ex @@ -47,16 +47,15 @@ defmodule Hypatia.Rules.JsCallScan do "|" ) - noncode = noncode_spans(src) - - source - |> Regex.compile!() - |> Regex.scan(src, return: :index) - |> Enum.flat_map(fn [{off, len} | _] -> - open = off + len - 1 - - if not inside?(noncode, off) and shell_option?(src, open), do: [off], else: [] - end) + hits = + source + |> Regex.compile!() + |> Regex.scan(src, return: :index) + |> Map.new(fn [{off, len} | _] -> {off, off + len - 1} end) + + src + |> code_offsets(Map.keys(hits)) + |> Enum.filter(&shell_option?(src, Map.fetch!(hits, &1))) end # ── the call's arguments ──────────────────────────────────────────────── @@ -66,12 +65,9 @@ defmodule Hypatia.Rules.JsCallScan do case split_top_level(src, open + 1, ?)) do {:ok, args, _close} -> Enum.any?(args, fn {from, to} -> - case first_code_byte(src, from, to) do - {:ok, brace} when brace < to -> - if :binary.at(src, brace) == ?{, do: object_has_shell?(src, brace), else: false - - _ -> - false + case object_start(src, from, to) do + {:ok, brace} -> object_has_shell?(src, brace) + :none -> false end end) @@ -80,6 +76,22 @@ defmodule Hypatia.Rules.JsCallScan do end end + # The `{` an argument starts with, stepping over wrapping parentheses + # (`({shell: true})`) and the whitespace and comments around them. + defp object_start(src, from, to) do + case first_code_byte(src, from, to) do + {:ok, i} when i < to -> + case :binary.at(src, i) do + ?{ -> {:ok, i} + ?( -> object_start(src, i + 1, to) + _ -> :none + end + + _ -> + :none + end + end + defp object_has_shell?(src, brace) do case split_top_level(src, brace + 1, ?}) do {:ok, props, _close} -> @@ -88,7 +100,7 @@ defmodule Hypatia.Rules.JsCallScan do case first_code_byte(src, from, to) do {:ok, key} -> Regex.match?( - ~r/\A(?:shell|'shell'|"shell")\s*:\s*(?:true\b|['"`])/, + ~r/\A(?:shell|'shell'|"shell"|\[\s*(['"`])shell\1\s*\])\s*:\s*(?:true\b|['"`])/, binary_part(src, key, to - key) ) @@ -232,8 +244,9 @@ defmodule Hypatia.Rules.JsCallScan do literal or comment of `src`. """ def code_offsets(src, offsets) when is_binary(src) do - spans = noncode_spans(src) - Enum.reject(offsets, &inside?(spans, &1)) + # Spans come out sorted and disjoint: walk them with the sorted offsets + # in one pass. + keep_code(Enum.sort(offsets), noncode_spans(src), []) end # Spans of the file that are strings, templates or comments. @@ -258,5 +271,14 @@ defmodule Hypatia.Rules.JsCallScan do end end - defp inside?(spans, off), do: Enum.any?(spans, fn {a, b} -> off >= a and off < b end) + defp keep_code([], _spans, acc), do: Enum.reverse(acc) + defp keep_code(offs, [], acc), do: Enum.reverse(acc, offs) + + defp keep_code([off | rest] = offs, [{a, b} | spans_rest] = spans, acc) do + cond do + off >= b -> keep_code(offs, spans_rest, acc) + off >= a -> keep_code(rest, spans, acc) + true -> keep_code(rest, spans, [off | acc]) + end + end end diff --git a/test/code_safety_test.exs b/test/code_safety_test.exs index eed3d9f6..c8c23053 100644 --- a/test/code_safety_test.exs +++ b/test/code_safety_test.exs @@ -1055,6 +1055,29 @@ defmodule Hypatia.Rules.CodeSafetyTest do assert exec_sync?(bound <> "cp_1.exec(cmd);\n") end + test "a parenthesised options object is still the call's options" do + assert exec_sync?("spawn(c, a, ({shell: true}));\n") + assert exec_sync?("spawn(c, a, (\n /* x */ {shell: true}));\n") + refute exec_sync?("spawn(c, a, ({env: {shell: true}}));\n") + end + + test "a computed string key names the shell option too" do + for key <- ["['shell']", ~s(["shell"]), "[`shell`]", "[ 'shell' ]"] do + assert exec_sync?("spawn(c, a, {#{key}: true});\n"), key + end + + refute exec_sync?("spawn(c, a, {[shellKey]: true});\n") + end + + test "string and comment offsets are found in one pass over many calls" do + calls = Enum.map_join(1..400, "", fn i -> "spawn(c#{i}, a, {shell: true}); // x\n" end) + + [f] = + Enum.filter(CodeSafety.scan_content(calls, "javascript"), &(&1.rule == :js_exec_sync)) + + assert f.occurrences == 400 + end + test "arguments nested deeper than two levels no longer hide the option" do assert exec_sync?("spawn(a(b(c(d))), [e(f(g()))], {shell: true});\n") end From 9fe59fb899f7c2b9ad3109b28b11494893be3d44 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 14 Nov 2026 10:41:42 +0000 Subject: [PATCH 5/6] fix(code_safety): JsCallScan reads inline spreads; comma operands are 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 Claude-Session: https://claude.ai/code/session_0136P7ZkGCWGEyedTtdEHAPT --- lib/rules/js_call_scan.ex | 55 ++++++++++++++++++++++++++++++++------- test/code_safety_test.exs | 13 +++++++++ 2 files changed, 58 insertions(+), 10 deletions(-) diff --git a/lib/rules/js_call_scan.ex b/lib/rules/js_call_scan.ex index c4f28863..c97b0cb0 100644 --- a/lib/rules/js_call_scan.ex +++ b/lib/rules/js_call_scan.ex @@ -17,7 +17,11 @@ defmodule Hypatia.Rules.JsCallScan do Limits, stated rather than guessed at: a regex literal containing a quote or bracket can desynchronise the walk; an options object passed through a variable is not followed; a call written inside a template literal's - `${...}` is treated as text, not code, for every form of the rule. + `${...}` is treated as text, not code, for every form of the rule; each + spawn-family call site re-reads its own argument list, so the cost is + quadratic in how deeply such calls nest inside one another's arguments + (real code does not nest them deeply, and only those four callees are + candidates). """ @spawn_family ~w(spawn spawnSync execFile execFileSync) @@ -76,15 +80,32 @@ defmodule Hypatia.Rules.JsCallScan do end end - # The `{` an argument starts with, stepping over wrapping parentheses - # (`({shell: true})`) and the whitespace and comments around them. + # The `{` of an argument that is an object literal, stepping over wrapping + # parentheses (`({shell: true})`) and the whitespace and comments around + # them. The object (or the parenthesised one) must be the whole argument: + # in a comma expression `({shell: true}, {shell: false})` the value is the + # last operand, so it is not read as this object. defp object_start(src, from, to) do case first_code_byte(src, from, to) do {:ok, i} when i < to -> case :binary.at(src, i) do - ?{ -> {:ok, i} - ?( -> object_start(src, i + 1, to) - _ -> :none + ?{ -> + case split_top_level(src, i + 1, ?}) do + {:ok, _props, close} -> if trivia?(src, close + 1, to), do: {:ok, i}, else: :none + :error -> :none + end + + ?( -> + case split_top_level(src, i + 1, ?)) do + {:ok, [{a, b}], close} -> + if trivia?(src, close + 1, to), do: object_start(src, a, b), else: :none + + _ -> + :none + end + + _ -> + :none end _ -> @@ -92,6 +113,9 @@ defmodule Hypatia.Rules.JsCallScan do end end + # Only whitespace and comments from `from` up to `to`. + defp trivia?(src, from, to), do: first_code_byte(src, from, to) == :none + defp object_has_shell?(src, brace) do case split_top_level(src, brace + 1, ?}) do {:ok, props, _close} -> @@ -99,10 +123,21 @@ defmodule Hypatia.Rules.JsCallScan do # A comment may sit before the key: start at the first code byte. case first_code_byte(src, from, to) do {:ok, key} -> - Regex.match?( - ~r/\A(?:shell|'shell'|"shell"|\[\s*(['"`])shell\1\s*\])\s*:\s*(?:true\b|['"`])/, - binary_part(src, key, to - key) - ) + text = binary_part(src, key, to - key) + + if String.starts_with?(text, "...") do + # An inline spread `...{shell: true}` contributes its own + # top-level keys. + case object_start(src, key + 3, to) do + {:ok, inner} -> object_has_shell?(src, inner) + :none -> false + end + else + Regex.match?( + ~r/\A(?:shell|'shell'|"shell"|\[\s*(['"`])shell\1\s*\])\s*:\s*(?:true\b|['"`])/, + text + ) + end :none -> false diff --git a/test/code_safety_test.exs b/test/code_safety_test.exs index c8c23053..66140bab 100644 --- a/test/code_safety_test.exs +++ b/test/code_safety_test.exs @@ -1061,6 +1061,19 @@ defmodule Hypatia.Rules.CodeSafetyTest do refute exec_sync?("spawn(c, a, ({env: {shell: true}}));\n") end + test "an inline spread object contributes its own keys" do + assert exec_sync?("spawn(c, a, {...{shell: true}});\n") + assert exec_sync?("spawn(c, a, {cwd, ...({shell: '/bin/sh'})});\n") + refute exec_sync?("spawn(c, a, {...{env: {shell: true}}});\n") + end + + test "a comma expression's value is its last operand" do + refute exec_sync?("spawn(c, a, ({shell: true}, {shell: false}));\n") + refute exec_sync?("spawn(c, a, {shell: true} && opts);\n") + assert exec_sync?("spawn(c, a, (({shell: true})));\n") + assert exec_sync?("spawn(c, a, ({shell: true} /* why */));\n") + end + test "a computed string key names the shell option too" do for key <- ["['shell']", ~s(["shell"]), "[`shell`]", "[ 'shell' ]"] do assert exec_sync?("spawn(c, a, {#{key}: true});\n"), key From 014e69e75d3fb68fe8ac88a64b827f92ee1499e9 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 14 Nov 2026 10:49:08 +0000 Subject: [PATCH 6/6] fix(code_safety): JsCallScan reads every top-level operand object 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 Claude-Session: https://claude.ai/code/session_0136P7ZkGCWGEyedTtdEHAPT --- lib/rules/js_call_scan.ex | 101 +++++++++++++++++++++++--------------- test/code_safety_test.exs | 13 ++++- 2 files changed, 74 insertions(+), 40 deletions(-) diff --git a/lib/rules/js_call_scan.ex b/lib/rules/js_call_scan.ex index c97b0cb0..cad44bb3 100644 --- a/lib/rules/js_call_scan.ex +++ b/lib/rules/js_call_scan.ex @@ -21,7 +21,11 @@ defmodule Hypatia.Rules.JsCallScan do spawn-family call site re-reads its own argument list, so the cost is quadratic in how deeply such calls nest inside one another's arguments (real code does not nest them deeply, and only those four callees are - candidates). + candidates); a quote in JSX text (`Don't`) is read as + opening a string to the end of that line, so a call later on the same + line is missed; an object that is any top-level operand of an option + argument counts, so `{shell: true} && opts` is reported even when `opts` + wins -- a rare false alarm is the safer error for a high-severity rule. """ @spawn_family ~w(spawn spawnSync execFile execFileSync) @@ -69,10 +73,7 @@ defmodule Hypatia.Rules.JsCallScan do case split_top_level(src, open + 1, ?)) do {:ok, args, _close} -> Enum.any?(args, fn {from, to} -> - case object_start(src, from, to) do - {:ok, brace} -> object_has_shell?(src, brace) - :none -> false - end + src |> object_operands(from, to) |> Enum.any?(&object_has_shell?(src, &1)) end) :error -> @@ -80,41 +81,66 @@ defmodule Hypatia.Rules.JsCallScan do end end - # The `{` of an argument that is an object literal, stepping over wrapping - # parentheses (`({shell: true})`) and the whitespace and comments around - # them. The object (or the parenthesised one) must be the whole argument: - # in a comma expression `({shell: true}, {shell: false})` the value is the - # last operand, so it is not read as this object. - defp object_start(src, from, to) do - case first_code_byte(src, from, to) do - {:ok, i} when i < to -> - case :binary.at(src, i) do - ?{ -> - case split_top_level(src, i + 1, ?}) do - {:ok, _props, close} -> if trivia?(src, close + 1, to), do: {:ok, i}, else: :none - :error -> :none - end - - ?( -> - case split_top_level(src, i + 1, ?)) do - {:ok, [{a, b}], close} -> - if trivia?(src, close + 1, to), do: object_start(src, a, b), else: :none - - _ -> - :none - end - - _ -> - :none + # The `{` of every object literal that is a top-level operand of the + # expression in `from..to`: reached without entering any bracket except + # wrapping parentheses. So a ternary branch, an `as`/`satisfies` assertion + # and an `||`/`??`/`&&` operand all count; an object inside a call, an + # array or another object does not. In a parenthesised comma expression + # only the last element is the value. + defp object_operands(src, from, to), do: operands(src, from, to, nil, []) + + defp operands(_src, i, to, _prev, acc) when i >= to, do: Enum.reverse(acc) + + defp operands(src, i, to, prev, acc) do + case :binary.at(src, i) do + c when c in [?', ?", ?`] -> + operands(src, skip_string(src, i), to, ?", acc) + + ?/ -> + case comment_end(src, i) do + nil -> operands(src, i + 1, to, ?/, acc) + j -> operands(src, j, to, prev, acc) end - _ -> - :none + ?{ -> + case split_top_level(src, i + 1, ?}) do + {:ok, _props, close} -> operands(src, close + 1, to, ?}, [i | acc]) + :error -> Enum.reverse(acc) + end + + ?( -> + case split_top_level(src, i + 1, ?)) do + {:ok, elems, close} -> + # After a name, `)` or `]` this is a call; otherwise a group. + inner = + if call_paren?(prev), + do: [], + else: (fn {a, b} -> object_operands(src, a, b) end).(List.last(elems)) + + operands(src, close + 1, to, ?), Enum.reverse(inner, acc)) + + :error -> + Enum.reverse(acc) + end + + ?[ -> + case split_top_level(src, i + 1, ?]) do + {:ok, _, close} -> operands(src, close + 1, to, ?], acc) + :error -> Enum.reverse(acc) + end + + c when c in [?\s, ?\t, ?\r, ?\n] -> + operands(src, i + 1, to, prev, acc) + + c -> + operands(src, i + 1, to, c, acc) end end - # Only whitespace and comments from `from` up to `to`. - defp trivia?(src, from, to), do: first_code_byte(src, from, to) == :none + defp call_paren?(prev) when is_integer(prev), + do: prev in [?), ?], ?_, ?$] or prev in ?a..?z or prev in ?A..?Z or prev in ?0..?9 + + defp call_paren?(_), do: false defp object_has_shell?(src, brace) do case split_top_level(src, brace + 1, ?}) do @@ -128,10 +154,7 @@ defmodule Hypatia.Rules.JsCallScan do if String.starts_with?(text, "...") do # An inline spread `...{shell: true}` contributes its own # top-level keys. - case object_start(src, key + 3, to) do - {:ok, inner} -> object_has_shell?(src, inner) - :none -> false - end + src |> object_operands(key + 3, to) |> Enum.any?(&object_has_shell?(src, &1)) else Regex.match?( ~r/\A(?:shell|'shell'|"shell"|\[\s*(['"`])shell\1\s*\])\s*:\s*(?:true\b|['"`])/, diff --git a/test/code_safety_test.exs b/test/code_safety_test.exs index 66140bab..05e32962 100644 --- a/test/code_safety_test.exs +++ b/test/code_safety_test.exs @@ -1069,11 +1069,22 @@ defmodule Hypatia.Rules.CodeSafetyTest do test "a comma expression's value is its last operand" do refute exec_sync?("spawn(c, a, ({shell: true}, {shell: false}));\n") - refute exec_sync?("spawn(c, a, {shell: true} && opts);\n") + # Accepted over-report: any top-level operand object counts. + assert exec_sync?("spawn(c, a, {shell: true} && opts);\n") assert exec_sync?("spawn(c, a, (({shell: true})));\n") assert exec_sync?("spawn(c, a, ({shell: true} /* why */));\n") end + test "an options object that is any top-level operand counts" do + assert exec_sync?("spawn(c, a, useShell ? {shell: true} : {shell: false});\n") + assert exec_sync?("spawn(c, a, useShell ? {} : {shell: '/bin/sh'});\n") + assert exec_sync?("spawn(c, a, {shell: true} as SpawnOptions);\n") + assert exec_sync?("spawn(c, a, {shell: true} satisfies SpawnOptions);\n") + assert exec_sync?("spawn(c, a, opts || {shell: true});\n") + refute exec_sync?("spawn(c, a, cond ? f({shell: true}) : x);\n") + refute exec_sync?("spawn(c, a, [{shell: true}]);\n") + end + test "a computed string key names the shell option too" do for key <- ["['shell']", ~s(["shell"]), "[`shell`]", "[ 'shell' ]"] do assert exec_sync?("spawn(c, a, {#{key}: true});\n"), key