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

use custom glob; closes #2318 #2325

Closed
wants to merge 1 commit into from
Closed

use custom glob; closes #2318 #2325

wants to merge 1 commit into from

Conversation

boneskull
Copy link
Contributor

This addresses the security advisory in a non-breaking manner. It upgrades minimatch to 1.0.0, basically--but from what I can tell, the only meaningful change from 0.3.0 was a feature to support number padding in brace expansion.

Generally speaking, what we're doing here should be avoided (forking old versions of modules and re-publishing them under new names). Doing so could potentially expose us to security issues down the road--if an issue is detected in minimatch or glob and subsequently fixed, we will have to manually backport the fix--assuming we even know about any issue!

I could go either way here; I can't conceive of a manner in which it would affect users of Mocha via usage of mocha itself. This issue does not affect Mocha in the browser.

See the forked/renamed modules glob-mocha and minimatch-mocha, the latter of which contains a backport for the advisory fix.

cc @ScottFreeCode

@boneskull
Copy link
Contributor Author

Without this, I believe nsp will complain about any package depending upon mocha, even if that's a devDependency.

@boneskull
Copy link
Contributor Author

Would appreciate another set of eyes on this to ensure I'm not totally out-of-bounds.

cc @mochajs/core

@boneskull boneskull added High priority status: waiting for author waiting on response from OP - more information needed type: chore generally involving deps, tooling, configuration, etc. labels Jun 26, 2016
@ScottFreeCode
Copy link
Contributor

I haven't reviewed the change itself, but I want to raise a question: Would it be a backwards-incompatible change to Mocha (e.g. due to relevant changes to the behavior of globbing as used by/with Mocha) to fork the latest version of Glob/Minimatch/anything-else-we-have-to and just change the carets? (On the other hand, this might not be scalable -- if for instance the earliest version of library X required by the latest Minimatch has carets in its own dependencies then we also have to fork X to take out the carets... and there may be multiple "X"s in this case?) If it worked, the advantage would be that we can try to keep the forks tracking the current version and getting other fixes relatively easily that way.

In any case, we should start keeping a list of things we want to change in 3.x-not-a-total-rewrite if we hit the point where we can drop Node 0.8 before we hit the point where the rewrite is a viable replacement; updating to a new, non-forked Glob/Minimatch along with following the Pug renaming can both go on it for starters...

@boneskull
Copy link
Contributor Author

I think I want to scrap this idea and just release a major.

@boneskull boneskull closed this Jul 2, 2016
@boneskull boneskull deleted the issue/2318 branch July 2, 2016 05:20
@rugk rugk mentioned this pull request Jul 31, 2016
@boneskull boneskull removed the status: waiting for author waiting on response from OP - more information needed label Dec 12, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: chore generally involving deps, tooling, configuration, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants