tree-sitter / node-tree-sitter

Node.js bindings for tree-sitter
https://www.npmjs.com/package/tree-sitter
MIT License
625 stars 107 forks source link

`prebuild-install` should be in package.json's dev dependencies #135

Closed aedryan closed 1 year ago

aedryan commented 1 year ago

Since prebuild-install is only used during the package install phase, it should be moved to the package.json's dev dependencies. Presently the version of prebuild-install set in this package winds up using an outdated version of ansi-regex which has a DOS vulnerability. Security scanners will pick this up as a vulnerability that would otherwise be ignored if it were properly tagged as a dev dependency instead of a dependency.

Path to vulnerability:

tree-sitter@0.20.1 > prebuild-install@6.1.4 > npmlog@4.1.2 > gauge@2.7.4 > strip-ansi@3.0.1 > ansi-regex@2.1.1

https://security.snyk.io/vuln/SNYK-JS-ANSIREGEX-1583908

verhovsky commented 1 year ago

https://nodejs.github.io/node-addon-examples/build-tools/prebuild/#the-code-classlanguage-textdependenciescode-property

If you put something in devDependencies it won't be installed when someone just does npm install tree-sitter.

The way prebuild works is you publish an empty package to npm and upload a bunch of binary files to GitHub Releases associated with that same release. prebuilt-install is needed to download the correct binary from GitHub Releases, it essentially replaces npm install tree-sitter with "download this binary file from GitHub".

aedryan commented 1 year ago

Got it, I did not know that about prebuild-install, it's a shame that it can't be moved to dev deps in order to avoid raising false positives in vulnerability scanners. I see that tree sitter is a major version behind prebuild-install where at least the vulnerability path described above are not relevant. Is an upgrade of prebuild-install on the roadmap for this package?

verhovsky commented 1 year ago

https://github.com/tree-sitter/node-tree-sitter/pull/127#issuecomment-1471942554