feat(scan): Node.js SAST coverage — ecosystem rules, template engines… - #137
Merged
Conversation
…, missing controls Closes a coverage gap against njsscan, an established Node scanner: of 78 existing rules only ~20 were JavaScript, and template files were not read at all. Adds three pieces, plus a guard fix that was silently suppressing rules. packages/scan/src/node-rules.ts — 25 Node-ecosystem rules, each of which has to know which package it is looking at before it can claim anything: vm/vm2 execution, function-reconstructing deserialisers, server-side template injection, shelljs, $where and XPath assembly, XML entity resolution, the JWT `none` algorithm, broken/ECB/legacy ciphers, autoescape and serialize-javascript opt-outs, reflected CORS origins, headless-browser navigation, zip-slip, Electron renderer settings, disabled security headers, buffer bounds and uninitialised allocation, oversized body limits, and returned stack traces. packages/scan/src/template-rules.ts — a second engine keyed on file extension rather than language, because the extension is what names the template engine and the engine is what decides which syntax skips escaping. Covers Handlebars/ Mustache, Vue, Pug/Jade, EJS/ECT, Dust, Nunjucks/Twig/Jinja and Haml. Runs alongside the code rules, so a .vue file is still checked as JavaScript. packages/scan/src/controls.ts — reports controls the tree has no evidence of (headers, CSRF, rate limiting, body size limits). Whole-project, so it accumulates across the walk; opt-in behind `threatcrush scan --missing-controls` because an absence is a weaker claim than a present defect; and silent unless the tree actually serves HTTP. Guard fix in code-rules.ts. GENERIC_GUARD is case-insensitive, so the "Allow" in `Access-Control-Allow-Origin` matched its allow-list heuristic. As the guard is tested against an 8-line window, one CORS header line disabled every guardable rule around it — in the end-to-end fixture it suppressed both the `none`-algorithm finding and a returned stack trace. The failure mode was the dangerous kind: not fewer findings, none, indistinguishable from clean code. Scoped to the header prefix so real allow-lists, including `allow-list:` keys in scanned config files, keep guarding. Covered by a regression test. Written clean-room. njsscan is LGPL-3.0 and this package is MIT, so its rule definitions could not be reused even in translation; only the list of vulnerability classes was taken, which is a fact about the ecosystem rather than an expression of it. Every pattern, guard and message is original and written against this engine's confidence model, which njsscan does not share. Provenance and the licence reasoning are recorded in the module headers, and classes needing analysis this engine does not do are listed in KNOWN_GAPS rather than approximated. Tests: 252 pass in packages/scan (58 new), 2 in apps/cli; both packages typecheck; CLI builds; verified end to end against a fixture app. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| id: 'js-uninitialized-buffer', | ||
| title: 'buffer allocated without zeroing', | ||
| consequence: | ||
| '`allocUnsafe` and the old `new Buffer(size)` hand back whatever was previously in that heap memory — keys, session tokens, other users’ request bodies. Anything not overwritten before the buffer is sent leaks it.', |
ThreatCrush Security Scan69 finding(s) HIGH/CRITICAL: 12 | MEDIUM: 56 | LOW: 1
…and 19 more. Full results in the Security tab. Snippets are redacted; ThreatCrush never prints matched credential material. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
…, missing controls
Closes a coverage gap against njsscan, an established Node scanner: of 78 existing rules only ~20 were JavaScript, and template files were not read at all. Adds three pieces, plus a guard fix that was silently suppressing rules.
packages/scan/src/node-rules.ts — 25 Node-ecosystem rules, each of which has to know which package it is looking at before it can claim anything: vm/vm2 execution, function-reconstructing deserialisers, server-side template injection, shelljs, $where and XPath assembly, XML entity resolution, the JWT
nonealgorithm, broken/ECB/legacy ciphers, autoescape and serialize-javascript opt-outs, reflected CORS origins, headless-browser navigation, zip-slip, Electron renderer settings, disabled security headers, buffer bounds and uninitialised allocation, oversized body limits, and returned stack traces.packages/scan/src/template-rules.ts — a second engine keyed on file extension rather than language, because the extension is what names the template engine and the engine is what decides which syntax skips escaping. Covers Handlebars/ Mustache, Vue, Pug/Jade, EJS/ECT, Dust, Nunjucks/Twig/Jinja and Haml. Runs alongside the code rules, so a .vue file is still checked as JavaScript.
packages/scan/src/controls.ts — reports controls the tree has no evidence of (headers, CSRF, rate limiting, body size limits). Whole-project, so it accumulates across the walk; opt-in behind
threatcrush scan --missing-controlsbecause an absence is a weaker claim than a present defect; and silent unless the tree actually serves HTTP.Guard fix in code-rules.ts. GENERIC_GUARD is case-insensitive, so the "Allow" in
Access-Control-Allow-Originmatched its allow-list heuristic. As the guard is tested against an 8-line window, one CORS header line disabled every guardable rule around it — in the end-to-end fixture it suppressed both thenone-algorithm finding and a returned stack trace. The failure mode was the dangerous kind: not fewer findings, none, indistinguishable from clean code. Scoped to the header prefix so real allow-lists, includingallow-list:keys in scanned config files, keep guarding. Covered by a regression test.Written clean-room. njsscan is LGPL-3.0 and this package is MIT, so its rule definitions could not be reused even in translation; only the list of vulnerability classes was taken, which is a fact about the ecosystem rather than an expression of it. Every pattern, guard and message is original and written against this engine's confidence model, which njsscan does not share. Provenance and the licence reasoning are recorded in the module headers, and classes needing analysis this engine does not do are listed in KNOWN_GAPS rather than approximated.
Tests: 252 pass in packages/scan (58 new), 2 in apps/cli; both packages typecheck; CLI builds; verified end to end against a fixture app.