search

almende / vis

⚠️ This project is not maintained anymore! Please go to https://github.com/visjs
7.85k stars 1.48k forks source link

outdated dependencies and security vulnerabilities #4155

Open clayt85 opened 6 years ago

clayt85 commented 6 years ago

Install from source flags multiple security vulnerabilities:

[clone fresh copy of the repo]
> cd vis
> npm install
[...]
found 7 security vulnerabilities (2 low, 4 high, 1 critical)

An npm audit shows that mocha.js and gulp.js are the culprits. Unfortunately, the fix is a SEMVER breaking change.

This behavior occurs at both the master and develop branches. I have confirmed on both CentOS 6 and Fedora 27. In general, these are just outdated dependencies that need to be updated anyway.

I can confirm that updating the dependency versions in package.json (or using npm audit fix --force) will fix the vulnerability, but it causes multiple test failures. (It also causes the test suite to hang, rather than exiting gracefully).

clayt85 commented 6 years ago

Here is a fix that updates the gulp and mocha dependencies as well as updates the test suite: https://github.com/clayt85/vis/tree/update_dep

There was one bad function call that needed to be fixed. This was affecting multiple tests. One of the tests is for deprecated behavior. That deprecation has now escalated into some error text that gets printed to the log. I resolved the issue by skipping the test.

As noted when I opened the issue, the test suite hangs rather than exits now that these dependencies have been updated. In short, this is associated with a change from mocha v3 to v4. It means that, somewhere in the vis.js test library, there is an asynchronous call that is not being cleaned up properly. I have not been able to locate the issue. See here

clayt85 commented 6 years ago

OK, you can patch the issue by passing the --exit flag to mocha. See link above where I have pushed another commit containing this patch.

In yet another commit, I also replaced the deprecated mocha --compilers option.

If someone will review/comment, I suspect this is ready for (a couple of) pull requests.

clayt85 commented 6 years ago

Oh dear... There are many more issues to be worked out... (I hadn't noticed that npm run build had failed so I was not actually updating the user files.) Still, it is a start, and the issue remains open!

clayt85 commented 6 years ago

OK, one more note. I updated the gulpfile so that npm run build proceeds without error. Afterward, npm run test proceeds without error.

However, not all is well. In particular, this demo does not work for me.

tbnorth commented 5 years ago

@clayt85 in what way is the demo not working? Was working ok just now for me in Chrome. Just asking to clarify this issue.

clayt85 commented 5 years ago

@tbnorth It seems I had confused myself. I just pulled a fresh copy of the repo (https://github.com/clayt85/vis/tree/update_dep) and everything seems to be operating just fine (tested on Chrome and Firefox). Pull request?

mojoaxel commented 5 years ago

In general, these are just outdated dependencies that need to be updated anyway.

@clayt85 Thanks for reporting this. As I see it the audit is not useful in this case. The problematic dependencies are just dev-dependencies and don't get deployed to any server. This warnings can easily be ignored! If you want you can create a Pull-Request (to the develop branch) and update these dependencies. Please only update the mentioned packages and make sure that all tests and the build process still runs. Thanks!