Build deps have security issues

marcoscaceres commented 7 years ago

Snyk is reporting a bunch of medium to high security issues with dependencies with this version of highlightjs. It might just be a matter of updating some dependencies.

[ ] High severity vuln found in minimatch@0.3.0, introduced via highlight.js@9.8.0
[ ] Low severity vuln found in hawk@1.1.1, introduced via highlight.js@9.8.0
[ ] Medium severity vuln found in request@2.40.0, introduced via highlight.js@9.8.0
[ ] Low severity vuln found in cli@0.6.6, introduced via highlight.js@9.8.0
[ ] High severity vuln found in handlebars@2.0.0, introduced via highlight.js@9.8.0

isagalaev commented 7 years ago

Thanks, we'll look into it at some point!

Just a note though, these are all build dependency, we don't depend on anything at runtime, so users are safe :-)

marcoscaceres commented 7 years ago

@isagalaev, thanks! Updated the title so not to scare anyone.

isagalaev commented 7 years ago

Oh, that's thoughtful of you, thanks!

Sannis commented 7 years ago

@marcoscaceres, how are you get this report? I've tried to check hljs here and get "No known vulnerabilities found".

marcoscaceres commented 7 years ago

@Sannis, if I recall correctly, I got it from cloning:

https://github.com/w3c/respec/

And then running snyk wizard.

marcoscaceres commented 7 years ago

@Sannis you might need to "npm install" first, but not sure... might just work without.

sourrust commented 7 years ago

Going through all the dependencies listed, it looks like most are from the build library we are using gear.js -- which is pretty much a dead project.

minimatch@0.3.0 is from gear-lib -> glob
hawk@1.1.1 is from gear-lib -> less -> request
request@2.40.0 is from gear-lib -> less
cli@0.6.6 is from gear-lib -> jshint
handlebars@2.0.0 is from gear-lib

I've been thinking about rewriting the build system because I was pretty much the only one that knew how to modify it and the documentation for gear.js isn't that great. Now that we have security issues it might be the extra push to actually get started on this.

I don't want to bother with other build systems that have been coming out because our build process is complex enough that the style these build library have don't really compliment our build process and I will end up working against them in the long run.

I'll layout my ideas more cleanly in a seperate issue before actually starting the build script rewrite so @isagalaev, @Sannis, and myself can actually have a better comprehension of our new build script. I'll claim responsible of this issue, so thank you @marcoscaceres for reporting this.

isagalaev commented 7 years ago

Jeremy thanks for weighing in on that! One immediate note from me regarding this:

our build process is complex enough

It's also a good opportunity to re-think if we need it to be this complex these days. The Internet is different, and old assumption may not hold up anymore. I don't have anything concrete right now, but I want to signal that nothing is set in stone regarding it.

marcoscaceres commented 7 years ago

@sourrust @isagalaev, this is really exciting to hear! please cc me on that bug. I have a few suggestions to simplify things - and would like to show you our current set up (as it shows the issues with the current build system when used with another project).

logicplace commented 7 years ago

To comment on the build process being simplified, being able to use the system as-is in addition to building a package would be pretty ideal for debugging/development, I think. Right now, since I want to be able to debug things easily (also since it's GitHub pages...), I edited the lang files I needed to register inline and just include everything via script/src or jQuery loading. It works, it just feels unnecessary. I'm sure there's a good design that would allow for both models. :]

ydnar commented 5 years ago

This raised its head again today, with several upstream vulnerabilities.

slinkardbrandon commented 5 years ago

I have a project template I generate with (so I can’t have a package-lock file). This started killing my CI today.

marcoscaceres commented 5 years ago

We could really use some help with this. We need to get rid of gear, but it’s a fair amount of work.

saschanaz commented 5 years ago

How about forking gear to update its dependencies?

marcoscaceres commented 5 years ago

Worth a shot. The project is dead, so it’s not like it risks getting out of sync.

marcoscaceres commented 5 years ago

Seems that the project has literally been deleted from Github... it used to be hosted by Yahoo.

saschanaz commented 5 years ago

The main developer forked the project and it's now here: https://github.com/twobit/gear

marcoscaceres commented 5 years ago

Nice find @saschanaz! I've forked it here: https://github.com/highlightjs/gear

Do you think you have time to poke at it? Might just be a matter of updating some of the really bad ones.

zackdotcomputer commented 5 years ago

👋 just saying hi, one of the many people who followed their npm audit trail to this issue today.

A quick question, though... if the gear and gear-lib dependencies are only used for the build tools and not by anything in the actual "work product" of the library, is there a reason they couldn't just be moved to devDependencies while a longer-term fix is put in place? That way they would be able to stop flowing downstream to people depending on this package? I am new here, though, so totally would believe if the answer is a flat "no" because I'm missing something about how dependers are using the files in the tools/ folder.

marcoscaceres commented 5 years ago

@zackzachariah, it's a good suggestion but ./tools/build.js is actually a core part of the library as that's used to build bundles locally. The library doesn't ship with any bundles, and lots of people build bundles locally (remember this library predates webpack and friends).

zackdotcomputer commented 5 years ago

👍 Ok, makes complete sense. I'll let you focus on your work then and just keep lurking so I know when to ping the various packages in the dependency chain between us to update.

marcoscaceres commented 5 years ago

Thanks @zackzachariah. Appreciate any comments/suggestions.

saschanaz commented 5 years ago

Just cloning it... (||||-----------)

saschanaz commented 5 years ago

@marcoscaceres Would you also fork https://github.com/twobit/gear-lib ? The majority of security issues are from that package.

marcoscaceres commented 5 years ago

cloning ... (||||-----------)

marcoscaceres commented 5 years ago

https://github.com/highlightjs/gear-lib

isagalaev commented 5 years ago

@marcoscaceres

./tools/build.js is actually a core part of the library as that's used to build bundles locally.

Wait, that couldn't be right. When we build an npm build it doesn't include any javascript besides the core library and bundled languages (and those have literally no dependencies). So I think it's not only possible, but should be fairly trivial to move all of the node.js dependencies into devDependencies.

Building bundles locally definitely sounds to me as a developer activity, as it assumes a source checkout rather then npm install.

marcoscaceres commented 5 years ago

Building bundles locally definitely sounds to me as a developer activity, as it assumes a source checkout rather then npm install.

On reflection, I agree. I’ve sent a PR to move all the dependencies be dev dependencies. Waiting on review approval.

highlightjs / highlight.js

Build deps have security issues #1369