Fix SNYK-JS-ANSIREGEX-1583908 and move to ESM

[rafaelmaeuer]

Is your feature request related to a problem or a nice-to-have?? Please describe.

As of https://snyk.io/vuln/SNYK-JS-ANSIREGEX-1583908 my investigations lead to mocha referencing the vulnerability by peer dependencies of "wide-align": "v1.1.3" -> "string-width": "^1.0.2 || 2" -> "strip-ansi": "^4.0.0" -> "ansi-regex": "^3.0.0".

Describe the solution you'd like

Forcing @iarna to accept iarna/wide-align#57 from @jimmywarting will update "string-width": "^5.0.1" resulting in a fix of https://snyk.io/vuln/SNYK-JS-ANSIREGEX-1583908 with peer-dependencies "strip-ansi": "^7.0.1" -> "ansi-regex": "^6.0.1". It will require some changes towards ESM on mocha-side too:

mocha/lib/cli/one-and-dones.js

Line 11 in 27bfc74

const align = require('wide-align');

Describe alternatives you've considered

Alternatively the use of @jimmywarting branch https://github.com/jimmywarting/wide-align/tree/esm as dependency, or investigation of a replacement for string-width are thinkable.

[juergba]

We get security issues like this one on a regular basis.

• please add a PoC explaining how this ReDoS is supposed to affect Mocha. Is there a masked tester attacking himself? How will he/she accomplish this attack? Which server/services are endangered?
• [...] move to ESM: no, we won't move to ESM for now. Certainly not based on a security message like this.

[jimmywarting]

I have used a simpler variant in one other project of mine (note that it dose not take into consideration the characters width)
Feel free to take, copy use it if you like

export function alignLeft(str, width) {
    return String(str).slice(0, width).padEnd(width)
}
export function alignRight(str, width) {
    return String(str).slice(0, width).padStart(width)
}
export function alignCenter(str, width) {
    const internalString = String(str)
    const leftPadding = Math.floor((width - internalString.length) / 2) + internalString.length
    return internalString.padStart(leftPadding).padEnd(width).slice(0, width)
}

[jimmywarting]

honestly, now when i looked at what you used wide-align for in the one-and-done (which is the only place)
where it only use align left and dumps something out in the console.log

mocha/lib/cli/one-and-dones.js

Lines 32 to 36 in 27bfc74

	console.log(
	` ${align.left(key, maxKeyLength + 1)}${
	description ? `- ${description}` : ''
	}`
	);

then i don't think the wide-align package (and 4 other dependencies) is worth keeping around to solve one such tiny thing. it don't justify the only tiny thing you use it for.
there are other decent ways to present it into the log with 0 dependencies.

i say: remove the wide-align dependency (and don't replace it with something else)
find other ways to dump it nicely to the log, there are also console.table, console.group

[iarna]

I don't have an opinion here as to if you should use wide-align but I will note that it doesn't do the same thing as the code @jimmywarting posted above -- the whole point of wide-align is that it can do console alignment of wide/double-width/full-width characters (eg, characters that display using two cells instead of one), something that anything using padStart/padEnd (or string.length) is gonna fail at. If wide characters are not a concern, then there's no point in using the module. If they are important, then you need either wide-align or another module that implements the same logic.

[juergba]

@dhuang612 are you interested in removing dependency wide-align and replacing it with alignLeft() shown above?
If yes, go ahead and open a PR, please use npm v6 to rebuild package-lock.json.

[dhuang612]

Hi,
Was I tagged by mistake?

[juergba]

No

[dhuang612]

Sure, I can try to work on this

[juergba]

@dhuang612 I will publish a new release this weekend, probably.
I would take over this one, unless your PR is ready within the next days. Thanks.

[dhuang612]

I am really busy over this next week and wouldn't have time to get to it before October.