Security fix for semver vulnerability

[romainmenke]

Which issue, if any, is this issue related to?

Closes #5042
Closes #7040

Is there anything in the PR that needs further explanation?

Not sure how to describe this change within the conventions of the project.
Suggestions welcome.

---

How do we verify that #5042 is fixed?

Comments within that issue describe that updating meow is the solution, but it would be nice to verify this and to have a test that prevents regressions.

---

#7040 :

npm audit --production
found 0 vulnerabilities

[changeset-bot]

🦋 Changeset detected

Latest commit: 685c37f

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
stylelint	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[jeddy3]

Not sure how to describe this change within the conventions of the project.

Let's lead with the security fix. We can manually add a changeset entry for the windows 10 regression.

[jeddy3]

How do we verify that #5042 is fixed?

Not sure. In the past, for intermittent platform-specific issues we've just waited for users to verify it's working after release. Particularly when the issue was patched in one of our dependencies.

[ybiquitous]

@romainmenke I don't realize this solution! Thanks!

I still have two concerns:

1. meow requires Node.js >=14.16. Is this a breaking change? That is, should we update engines.node to ^14.18.0 || >= 16.0.0?

$ npm view meow@11.0.0 engines
{ node: '>=14.16' }

$ npx ls-engines@latest
...
┌───────────────────────────────────┬───────────────────────────┐
│ package engines:                  │ dependency graph engines: │
├───────────────────────────────────┼───────────────────────────┤
│ "engines": {                      │ "engines": {              │
│   "node": "^14.13.1 || >= 16.0.0" │   "node": ">= 14.18"      │
│ }                                 │ }                         │
└───────────────────────────────────┴───────────────────────────┘
...

2. People will not be able to require "stylelint/lib/cli.js". Should this be acceptable? I guess there may be few people who need such a CJS way... 🤔

[Mouvedia]

That is, should we update engines.node to ^14.18.0 || >= 16.0.0?

If we revert #7020, it should be "^14.13.1 || >=16.0.0".

[jeddy3]

meow requires Node.js >=14.16. Is this a breaking change? That is, should we update engines.node to ^14.18.0 || >= 16.0.0?

That's a shame. It is breaking (although probably for only a handful of users).

No easier answer here. Major releases are painful for plugin authors, so we only do it once a year.

Shall we move this change to v16? It's a security fix, but as we're a dev tool the impact should be low.

People will not be able to require "stylelint/lib/cli.js". Should this be acceptable?

I believe so, as it's not part of our public API.

[romainmenke]

meow requires Node.js >=14.16. Is this a breaking change? That is, should we update engines.node to ^14.18.0 || >= 16.0.0?

meow version 11 requires these node versions, but version 10 should be fine.
I did get a typescript warning on 10, but that might be possible to solve.

Version 10 also resolves the security issue.

I will look into this shortly :)

[Mouvedia]

meow version 11 requires these node versions, but version 10 should be fine.

I agree but it needs to be at least version 10.1.0.
cf #5042 (comment)

[romainmenke]

The types were incorrect at this version, but that doesn't affect the runtime of the cli.

[ybiquitous]

I also agree with updating to meow@10 instead of meow@11. 👍🏼

For your information:

$ npm view meow@^10.1.0 engines
meow@10.1.0 { node: '>=12.17' }
meow@10.1.1 { node: '>=12.17' }
meow@10.1.2 { node: '^12.20.0 || ^14.13.1 || >=16.0.0' }
meow@10.1.3 { node: '^12.20.0 || ^14.13.1 || >=16.0.0' }
meow@10.1.4 { node: '^12.20.0 || ^14.13.1 || >=16.0.0' }
meow@10.1.5 { node: '^12.20.0 || ^14.13.1 || >=16.0.0' }

Perhaps, it may be better to update the latest version of meow@^10.1.0 like this:

- "meow": "^10.1.0",
+ "meow": "^10.1.5",

[ybiquitous]

In addition, here's a result of ls-engines:

$ npx ls-engines@latest
...
┌───────────────────────────────────┬───────────────────────────┐
│ package engines:                  │ dependency graph engines: │
├───────────────────────────────────┼───────────────────────────┤
│ "engines": {                      │ "engines": {              │
│   "node": "^14.13.1 || >= 16.0.0" │   "node": ">= 14.18"      │
│ }                                 │ }                         │
└───────────────────────────────────┴───────────────────────────┘
...
Your “engines” field does not exactly match your dependency graph‘s requirements!
...

ls-engines still complains as above, but I think meow@^10.1.0 is acceptable because meow itself requires node: '^12.20.0 || ^14.13.1 || >=16.0.0'. 👍🏼

[ybiquitous]

Thank you for the excellent fix. LGTM 👍🏼

[ybiquitous]

[note] Our maintainer guide now doesn't mention the Security: prefix. This doesn't block this PR, but I'm a bit curious. 😅

stylelint/docs/maintainer-guide/pull-requests.md

Lines 22 to 23 in a42f955

	2. If applicable, add a [changeset](https://github.com/changesets/changesets) using the GitHub interface:
	- prefix the entry with either: "Removed", "Changed", "Deprecated", "Added", or "Fixed"

[jeddy3]

It's an oversight. It should list all the ones from https://keepachangelog.com/en/1.0.0/. I'll open a PR.

[ybiquitous]

I think this PR needs one more approvement.

[romainmenke]

ls-engines still complains as above

Seems to be because of supports-hyperlinks, which is a direct dependency.

"node_modules/supports-hyperlinks": {
	"version": "3.0.0",
	// ...
	"engines": {
		"node": ">=14.18"
	}
},

Luckily not a sub dependency of meow v10.x :)

[jeddy3]

LGTM, thank you very much.