Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NPM Audit Vulnerability #715

Closed
joepaquette opened this issue Mar 18, 2020 · 15 comments
Closed

NPM Audit Vulnerability #715

joepaquette opened this issue Mar 18, 2020 · 15 comments

Comments

@joepaquette
Copy link

Moderate Vulnerabilty: Prototype Pollution
Package: minimist
Patched in: >=1.2.3

See https://npmjs.com/advisories/1179

@duniul
Copy link

duniul commented Mar 18, 2020

There's a PR up for this, it just needs to be merged by someone: #713

@joepaquette
Copy link
Author

NOTE: PR #713 will not resolve this vulnerability. The minimum version for minimist is 1.2.3. The PR only bumps the version to 1.2.2.

@duniul
Copy link

duniul commented Mar 20, 2020

My bad, misread the version number! I wonder why Dependabot doesn't update it to the latest version?

@bajtos
Copy link

bajtos commented Apr 6, 2020

AFAICT, there are two bot-contributed PRs open to resolve this issue:

Both pull requests are failing on CI, I think the problem is Windows specific. I am not able to view logs from Azure Pipelines, the AppVeyor log is complaining about incorrect CI setup.

https://ci.appveyor.com/project/jimthedev/cz-cli/builds/31474679

Build started
git clone -q --branch=renovate/npm-minimist-vulnerability https://github.com/commitizen/cz-cli.git C:\projects\cz-cli
git checkout -qf a162b40f6e7373887ac577187306bf8b5b477cc4
The build phase is set to "MSBuild" mode (default), but no Visual Studio project or 
solution files were found in the root directory. If you are not building Visual Studio 
project switch build mode to "Script" and provide your custom build command.

@bajtos
Copy link

bajtos commented Apr 6, 2020

Ping @jimthedev, what's the status of this project? I see the last commit was merged in August 2019. I am an open-source maintainer myself, so I totally understand if you don't have time or energy to maintain this project going forward. Do you have any recommendations for us, commitizen users, how to deal with the current security warning and/or help this project to become actively maintained again?

@cjolif
Copy link

cjolif commented Apr 7, 2020

Note that I'm coming here because I have this very security warning with cz-conventional-changelog so once fixed it would be good to get cz-conventional-changelog use the new minimist version as well.

@jimthedev
Copy link
Member

jimthedev commented Apr 8, 2020

@bajtos https://dev.azure.com/commitizen/cbf3c633-a6bc-43ce-ae50-fe434d244768/_apis/build/builds/851/logs/50 is the current error I am working through. For some reason windows fails with a ShellJS internal error, something about permissions on package.json. Not sure why it is failing on windows.

2020-04-07T20:05:12.6606024Z ##[section]Starting: CmdLine
2020-04-07T20:05:12.6709426Z ==============================================================================
2020-04-07T20:05:12.6710031Z Task         : Command line
2020-04-07T20:05:12.6710497Z Description  : Run a command line script using Bash on Linux and macOS and cmd.exe on Windows
2020-04-07T20:05:12.6710951Z Version      : 2.164.0
2020-04-07T20:05:12.6711304Z Author       : Microsoft Corporation
2020-04-07T20:05:12.6711767Z Help         : https://docs.microsoft.com/azure/devops/pipelines/tasks/utility/command-line
2020-04-07T20:05:12.6712303Z ==============================================================================
2020-04-07T20:05:13.4903228Z Generating script.
2020-04-07T20:05:13.4998081Z Script contents:
2020-04-07T20:05:13.5004561Z npm test && npm run write-coverage
2020-04-07T20:05:13.5342796Z ========================== Starting Command Output ===========================
2020-04-07T20:05:13.5594528Z ##[command]"C:\windows\system32\cmd.exe" /D /E:ON /V:OFF /S /C "CALL "d:\a\_temp\199dae85-2615-4a37-b23e-df12c3bfbf06.cmd""
2020-04-07T20:05:14.1913196Z 
2020-04-07T20:05:14.1914622Z > commitizen@0.0.0-semantically-released test D:\a\1\s
2020-04-07T20:05:14.1915461Z > nyc --require @babel/register npm run test-actual
2020-04-07T20:05:14.1916016Z 
2020-04-07T20:05:15.3964071Z 
2020-04-07T20:05:15.3965137Z > commitizen@0.0.0-semantically-released test-actual D:\a\1\s
2020-04-07T20:05:15.3965759Z > mocha --reporter=mocha-multi-reporters --reporter-options configFile=./test/mochareporters.json test/tests/index.js
2020-04-07T20:05:15.3966205Z 
2020-04-07T20:05:20.3482640Z 
2020-04-07T20:05:20.3491540Z 
2020-04-07T20:05:20.3580345Z   adapter
2020-04-07T20:05:45.2251196Z     √ resolves adapter paths (20284ms)
2020-04-07T20:06:03.6470506Z     √ resolves scoped adapter paths (3240ms)
2020-04-07T20:06:40.1738702Z [BABEL] Note: The code generator has deoptimised the styling of D:\a\1\s\test\.tmp\enduser-app\node_modules\lodash\lodash.js as it exceeds the max of 500KB.
2020-04-07T20:06:43.9848780Z     √ gets adapter prompter functions (31718ms)
2020-04-07T20:06:50.5327645Z ShellJS: internal error
2020-04-07T20:06:50.5410420Z Error: EPERM: operation not permitted, lstat 'D:/a/1/s/test/.tmp/enduser-app/package.json'
2020-04-07T20:06:50.5412145Z     at Object.lstatSync (fs.js:923:3)
2020-04-07T20:06:50.5412996Z     at rmdirSyncRecursive (D:\a\1\s\node_modules\shelljs\src\rm.js:28:23)
2020-04-07T20:06:50.5414175Z     at D:\a\1\s\node_modules\shelljs\src\rm.js:136:9
2020-04-07T20:06:50.5414922Z     at Array.forEach (<anonymous>)
2020-04-07T20:06:50.5415566Z     at Object._rm (D:\a\1\s\node_modules\shelljs\src\rm.js:113:9)
2020-04-07T20:06:50.5416273Z     at Object.rm (D:\a\1\s\node_modules\shelljs\src\common.js:365:25)
2020-04-07T20:06:50.5416954Z     at rm (D:\a\1\s\test\tools/clean.js:119:6)
2020-04-07T20:06:50.5417990Z     at Object.cleanPath (D:\a\1\s\test\tools/clean.js:24:3)
2020-04-07T20:06:50.5418725Z     at Context.afterEach (D:\a\1\s\test\tests/commit.js:319:9)
2020-04-07T20:06:50.5420922Z     at callFn (D:\a\1\s\node_modules\mocha\lib\runnable.js:387:21)
2020-04-07T20:06:50.5421986Z     at Hook.Runnable.run (D:\a\1\s\node_modules\mocha\lib\runnable.js:379:7)
2020-04-07T20:06:50.5422674Z     at next (D:\a\1\s\node_modules\mocha\lib\runner.js:384:10)
2020-04-07T20:06:50.5423314Z     at D:\a\1\s\node_modules\mocha\lib\runner.js:420:7
2020-04-07T20:06:50.5425129Z     at done (D:\a\1\s\node_modules\mocha\lib\runnable.js:334:5)
2020-04-07T20:06:50.5426600Z     at callFn (D:\a\1\s\node_modules\mocha\lib\runnable.js:410:7)
2020-04-07T20:06:50.5427456Z     at Hook.Runnable.run (D:\a\1\s\node_modules\mocha\lib\runnable.js:379:7)
2020-04-07T20:06:50.5428281Z     at next (D:\a\1\s\node_modules\mocha\lib\runner.js:384:10)
2020-04-07T20:06:50.5429308Z     at Immediate._onImmediate (D:\a\1\s\node_modules\mocha\lib\runner.js:425:5)
2020-04-07T20:06:50.5430453Z     at processImmediate (internal/timers.js:456:21)
2020-04-07T20:06:50.5787488Z npm ERR! code ELIFECYCLE
2020-04-07T20:06:50.5789952Z npm ERR! errno 1
2020-04-07T20:06:50.5797278Z npm ERR! commitizen@0.0.0-semantically-released test-actual: `mocha --reporter=mocha-multi-reporters --reporter-options configFile=./test/mochareporters.json test/tests/index.js`
2020-04-07T20:06:50.5798260Z npm ERR! Exit status 1
2020-04-07T20:06:50.5798894Z npm ERR! 
2020-04-07T20:06:50.5799540Z npm ERR! Failed at the commitizen@0.0.0-semantically-released test-actual script.
2020-04-07T20:06:50.5800335Z npm ERR! This is probably not a problem with npm. There is likely additional logging output above.
2020-04-07T20:06:50.6132605Z 
2020-04-07T20:06:50.6133884Z npm ERR! A complete log of this run can be found in:
2020-04-07T20:06:50.6134754Z npm ERR!     C:\npm\cache\_logs\2020-04-07T20_06_50_573Z-debug.log
2020-04-07T20:06:50.6970361Z -------------------------|---------|----------|---------|---------|-----------------------
2020-04-07T20:06:50.6972484Z File                     | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s     
2020-04-07T20:06:50.6973616Z -------------------------|---------|----------|---------|---------|-----------------------
2020-04-07T20:06:50.6981488Z All files                |   43.05 |    42.36 |   47.06 |   43.44 |                       
2020-04-07T20:06:50.6983782Z  commitizen              |   52.04 |    47.37 |      50 |   52.04 |                       
2020-04-07T20:06:50.6985292Z   adapter.js             |   82.86 |    51.61 |      80 |   82.86 | 81-90,133             
2020-04-07T20:06:50.6986668Z   cache.js               |       0 |      100 |       0 |       0 | 14-40                 
2020-04-07T20:06:50.6987820Z   commit.js              |       0 |        0 |       0 |       0 | 15-62                 
2020-04-07T20:06:50.6989495Z   configLoader.js        |     100 |      100 |     100 |     100 |                       
2020-04-07T20:06:50.6990310Z   init.js                |   76.92 |    64.52 |     100 |   76.92 | 81,97,107,110,113,124 
2020-04-07T20:06:50.6991161Z   staging.js             |       0 |        0 |       0 |       0 | 9-17                  
2020-04-07T20:06:50.6991927Z  common                  |   29.17 |    26.67 |   42.86 |   29.17 |                       
2020-04-07T20:06:50.6992746Z   util.js                |   29.17 |    26.67 |   42.86 |   29.17 | 27-52,62,64,75-81     
2020-04-07T20:06:50.6993577Z  configLoader            |    71.7 |       60 |      90 |    71.7 |                       
2020-04-07T20:06:50.6994335Z   findup.js              |   83.33 |       75 |     100 |   83.33 | 31-32                 
2020-04-07T20:06:50.6995114Z   getContent.js          |      75 |       60 |      80 |      75 | 24,36-41,53,60,78     
2020-04-07T20:06:50.6997379Z   getNormalizedConfig.js |    37.5 |       50 |     100 |    37.5 | 13,17-25              
2020-04-07T20:06:50.7002693Z   loader.js              |   77.78 |    66.67 |     100 |   77.78 | 26,40                 
2020-04-07T20:06:50.7003533Z  git                     |       0 |        0 |       0 |       0 |                       
2020-04-07T20:06:50.7004348Z   add.js                 |       0 |      100 |       0 |       0 | 10-19                 
2020-04-07T20:06:50.7005123Z   commit.js              |       0 |        0 |       0 |       0 | 15-78                 
2020-04-07T20:06:50.7005868Z   init.js                |       0 |      100 |       0 |       0 | 7-8                   
2020-04-07T20:06:50.7006658Z   log.js                 |       0 |        0 |       0 |       0 | 9-16                  
2020-04-07T20:06:50.7009700Z   whatChanged.js         |       0 |        0 |       0 |       0 | 9-16                  
2020-04-07T20:06:50.7010952Z -------------------------|---------|----------|---------|---------|-----------------------
2020-04-07T20:06:50.7123398Z npm ERR! Test failed.  See above for more details.
2020-04-07T20:06:50.8345942Z ##[error]Cmd.exe exited with code '1'.
2020-04-07T20:06:50.8714689Z ##[section]Finishing: CmdLine

@tjapro
Copy link

tjapro commented Apr 13, 2020

The pakage core-js also needs to be updated ...

warning: commitizen > cz-conventional-changelog > @commitlint/load > babel-runtime > core-js@2.6.11: core-js@<3 is no longer maintained and not recommended for usage due to the number of issues. Please, upgrade your dependencies to the actual version of core-js@3.

From #716

@hdmr14
Copy link
Contributor

hdmr14 commented Apr 30, 2020

@bajtos https://dev.azure.com/commitizen/cbf3c633-a6bc-43ce-ae50-fe434d244768/_apis/build/builds/851/logs/50 is the current error I am working through. For some reason windows fails with a ShellJS internal error, something about permissions on package.json. Not sure why it is failing on windows.

Apply below changes fixes ShellJS internal error and some errors.
These changes passed all tests with Windows 10 for me.
hope this helps.

diff --git a/src/commitizen/adapter.js b/src/commitizen/adapter.js
index 6878946..6765618 100644
--- a/src/commitizen/adapter.js
+++ b/src/commitizen/adapter.js
@@ -78,7 +78,7 @@ function generateNpmInstallAdapterCommand (stringMappings, adapterNpmName) {
 function generateYarnAddAdapterCommand (stringMappings, adapterNpmName) {

   // Start with an initial yarn add command
-  let installAdapterCommand = `yarn add ${adapterNpmName}`;
+  let installAdapterCommand = `npx yarn add ${adapterNpmName}`;

   // Append the necessary arguments to it based on user preferences
   for (let value of stringMappings.values()) {
diff --git a/src/commitizen/init.js b/src/commitizen/init.js
index 624d5c4..396f0ae 100644
--- a/src/commitizen/init.js
+++ b/src/commitizen/init.js
@@ -96,6 +96,8 @@ function init (sh, repoPath, adapterNpmName, {
   } catch (e) {
     console.error(e);
   }
+
+  sh.cd(__dirname);
 }

 /**

sh.cd() is changing process.cwd(). due to this behavior, process grab directory access until test exit even if we want remove it.

@LinusU
Copy link
Contributor

LinusU commented Apr 30, 2020

A PR to remove ShellJS would be very welcome 😬

Since the CI is current red for master anyway, I made a commit that fixes the vulnerable packages, by bumping minimist and running npm audit fix.

There are a number of dev-dependencies that would still need to be bumped though, which probably includes dropping support for older versions of Node.js...

@LinusU
Copy link
Contributor

LinusU commented Apr 30, 2020

Fixed in commitizen@4.0.5, sorry for the delay everyone!

@LinusU LinusU closed this as completed Apr 30, 2020
@hdmr14
Copy link
Contributor

hdmr14 commented Apr 30, 2020

@LinusU thanks update packages!

Tests failed on Windows is not caused by ShellJS.
It's just a problem with changing working directory(even if not via ShellJS) for exec some command(git, npm) in temporally directory for tests.

Could you approve above fix(sh.cd(__dirname), npx yarn add(yarn is locally installed at test))?
If so, I'll create a PR and test non-Windows platforms as well.

@LinusU
Copy link
Contributor

LinusU commented May 1, 2020

@hdmr14 I have a branch that I made last night which I think solves it by not using childProcess.{spawn,exec} and explicitly passing the working directory. Just fell asleep before I got time to commit and push 😅

I'll push it later today, and then maybe you can review it for me?

@hdmr14
Copy link
Contributor

hdmr14 commented May 1, 2020

@LinusU what a nice work! off course, I can 😍

@LinusU
Copy link
Contributor

LinusU commented May 3, 2020

Posted a PR here #729 , but the test is still failing on Windows, although in a slightly different way...

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

No branches or pull requests

8 participants