Skip to content

feat(security): TypeScript/JavaScript sinks for the security lens (M4) - #71

Merged
Rinkia merged 1 commit into
masterfrom
feat/security-ts-sinks
Jul 23, 2026
Merged

Rinkia merged 1 commit into
masterfrom
feat/security-ts-sinks

Conversation

@Rinkia

@Rinkia Rinkia commented Jul 23, 2026

Copy link
Copy Markdown
Owner

What

Milestone 4 — the security lens covers a second language. modscan-audit --language typescript (or javascript) maps eval, Function/new Function, the vm module, the child_process family, dynamic require/import, and string-body timers, reusing the tree-sitter front-end the moddability side already ships.

Closes #62.

Modelled on eslint-plugin-security

The JS counterpart to Bandit. Two of its rules shaped the catalog directly:

  • detect-non-literal-require — a computed require(name) is a sink; a literal require('fs') is not. Deliberately stricter than the Python side (where import_module always fires) because the base rates differ by ecosystem. This validated a design call I'd made before finding the rule.
  • detect-child-process — split shell (exec/execSync, high) vs no-shell (spawn/execFile/fork, medium), the same split the Python catalog makes.

Where we go further: eslint-plugin-security has no rule for new Function or vm.*. Its out-of-scope rules (unsafe regex, object injection, timing attacks, fs filenames) are deliberately not mirrored — the lens doesn't claim them.

Scope addition beyond the plan: binding resolution

The plan assumed qualifier-strict matching would do. It won't. Idiomatic JS writes const {exec} = require('child_process') then exec(...) — matching only child_process.exec misses most real code, while matching the bare name exec fires on someRegex.exec(str), which is everywhere and harmless.

So each file's require/import bindings are resolved before matching, covering aliased, destructured, named and namespace forms. File-local by design — a binding passed through another module isn't followed. Stated as a limit, not hidden.

Verified discrimination

Fixture holds every catalogued sink in the forms real code uses, plus benign look-alikes. All 9 sinks detected at the right lines; zero from the benign half:

Fires ✅ Silent ✅
eval, new Function, Function JSON.parse (the json.loads analogue)
vm.runInThisContext (namespace import) re.exec (a regex, not child_process)
cp.exec (aliased), exec (destructured) require('fs') (literal → static import)
execFile (named import) setTimeout(() => …) (function, not string)
require(name) (computed), setTimeout('…') a property named eval

Honest limitation

There is no cross-check authority for TS here. Python was validated against Bandit at 27/27; TS validation is planted fixtures + decoys only. I am not claiming TS is validated to the Python standard. Node 24 is available, so an eslint-plugin-security cross-check is feasible as a follow-up — the rule mapping is already worked out above.

Test plan

  • tests/test_security_ts_sinks.py — catalog integrity, all sinks detected, benign half silent, shell/no-shell severity split, javascript alias, unknown-language ValueError. Skips cleanly without tree-sitter.
  • Suite 25/25, import modscan ok.
  • Benchmark 9/20; Python Bandit cross-check still 27/27 (no regression to the validated path).
  • Python remains the default — released CLI form and call signature unchanged.
  • CLI smoke: --language typescript renders a ranked report.

🤖 Generated with Claude Code

Milestone 4: the lens covers a second language. `modscan-audit --language
typescript` (or javascript) maps eval, Function/new Function, the vm module, the
child_process family, dynamic require/import, and string-body timers, reusing the
tree-sitter front-end the moddability side already ships.

Modelled on eslint-plugin-security, the JS counterpart to Bandit. Two of its
rules shaped the catalog: a computed require(name) is a sink while a literal
require('fs') is not (detect-non-literal-require), and the child_process family
is split shell vs no-shell the way the Python catalog splits os.system from the
exec family.

Resolves bindings per file before matching. Idiomatic JS writes
`const {exec} = require('child_process')` and then `exec(...)`, so matching only
the qualified form would miss most real code — while matching the bare name would
fire on `someRegex.exec(str)`, which is everywhere and harmless. Each file's
require/import bindings are resolved first, covering the aliased, destructured,
named and namespace forms. This is deliberately file-local: a binding passed
through another module is not followed, which is a stated limit rather than an
oversight.

Verified on a fixture holding every catalogued sink in the forms real code uses,
plus benign look-alikes: JSON.parse, a regex .exec, a literal require, ordinary
fs use, a function-body setTimeout and a property named eval all stay silent.

Python remains the default language, so the released call signature and CLI form
are unchanged. Suite 25/25; benchmark 9/20; the Python Bandit cross-check still
reports 27/27. The test skips cleanly when tree-sitter is absent.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown

MODScan attack-surface diff — modscan

No new execution sinks

New attack surface to review — NOT a vulnerability verdict. This compares enumerated execution sinks between two snapshots. It does not trace whether input reaches them (no taint analysis), match CVEs, or detect secrets. An empty diff means no new catalogued sinks — not that the change is safe.

The two snapshots enumerate the same execution sinks.

@github-actions

Copy link
Copy Markdown

MODScan extension-point diff

No breaking changes to extension points

Added (new extension points)

  • security.detect_ts:find_ts_risk_sinks
  • security.sinks_ts:all_ts_specs

@Rinkia
Rinkia merged commit d843f89 into master Jul 23, 2026
7 checks passed
@Rinkia
Rinkia deleted the feat/security-ts-sinks branch July 23, 2026 14:12
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.

Security sinks for TypeScript/JavaScript (reuse the existing TS front-end)

1 participant