ejs icon indicating copy to clipboard operation
ejs copied to clipboard

please remvove jake as not needed in production - fixes CVE-2021-43138

Open dev-trilobyte opened this issue 3 years ago • 10 comments

Due to the build only dependency "jake"" a multiple additional not needed dependencies are fetched into EJS. Now latest version of jake depends on insecure async package (CVE-2021-43138). Removing jake and restoring no-dep only state as old 2.x version of ejs will silence a lot of noise from different security scanner and people will not need to invest time checking if its really vulnerable or some whitelists needs to be updated.

OTOH whitelisting this vulnerability for ejs/jake will silence the alarm for other possible real threats/dependencies too and is not really an option...

Thanks in advance, S. Seide

dev-trilobyte avatar Apr 07 '22 18:04 dev-trilobyte

@mde I just made https://github.com/jakejs/jake/pull/406 to avoid the CVE in jake, if you want to go that route.

mceachen avatar Apr 07 '22 20:04 mceachen

@mceachen why is jake not a dev dependency? It seems that jake is used to built ejs and nothing else or is there some hidden built feature in ejs that I am not aware of?

dotnetCarpenter avatar Apr 09 '22 19:04 dotnetCarpenter

@mceachen why is jake not a dev dependency? It seems that jake is used to built ejs and nothing else or is there some hidden built feature in ejs that I am not aware of?

It's apparently used for argument parsing in the CLI, see the discussion in #645

ahoisl avatar Apr 13 '22 07:04 ahoisl

Please remove jake which will make ejs a "dependency free" module in prod mode. That will boost ejs weekly usage imo. Implement a direct way for cli argument parsing if you need that. I always prefer dep-free modules, because of exact this reason discussed here and ejs is very close to that :)

kareha avatar Apr 13 '22 13:04 kareha

@ahoisl thanks for the info. @mde I did a cursory read of the changes in #645 and they seem like a reasonable alternative to waiting for https://github.com/jakejs/jake/pull/406..

dotnetCarpenter avatar Apr 15 '22 19:04 dotnetCarpenter

Published https://www.npmjs.com/package/@nightwatch/ejs to circumvent this issue.

beatfactor avatar Apr 20 '22 08:04 beatfactor

jake has fixed their dependency https://github.com/jakejs/jake/pull/411

trazeris avatar Apr 20 '22 11:04 trazeris

Published https://www.npmjs.com/package/@nightwatch/ejs to circumvent this issue.

You can just run npm audit fix to circumvent the issue.

TobiasWehrum avatar Apr 20 '22 15:04 TobiasWehrum

Moving the CLI into another package, or at least using some lighter / more specialized dependency like commander or embedding the arg parser as suggested in the PR #645, could indeed be a good thing to do.

Using Jake as a production dependency has also a direct impact on the package installation size that increased from 117 kB (3.0.2) to 541 kB (3.1.2) and 1.31 MB (3.1.7). As ejs is downloaded more than 10M times a week, increasing the package size by more than 10x makes a lot of difference globally in network and file system usage (even if yes, it's still a small dependency)!

And it's even more unfortunate that more than 90% of the installation size is therefore linked to a secondary feature 😕.

packagephobia-ejs

GaelGirodon avatar Sep 18 '22 14:09 GaelGirodon

Published https://www.npmjs.com/package/@nightwatch/ejs to circumvent this issue.

FWIW, you didn't change the repository metadata in your package.json for your forked version--it's still pointing to this repo.

mceachen avatar Sep 18 '22 19:09 mceachen