eui icon indicating copy to clipboard operation
eui copied to clipboard

Update remark-parse to newest version

Open aaron-ngt opened this issue 3 years ago • 4 comments
trafficstars

We are getting a security flag for remark-parse because of its use of [email protected].

All versions of package trim lower than 0.0.3 are vulnerable to Regular Expression Denial of Service (ReDoS) via trim().

The newest version, 10.x, eliminates the Trim dependency entirely. Making the switch would be most good, Newland. Most good.

aaron-ngt avatar Jan 18 '22 17:01 aaron-ngt

This will require a major version bump of unified (and remark too IIRC), but we should definitely bring all of these up to date.

chandlerprall avatar Jan 18 '22 17:01 chandlerprall

Excellent. Thanks. Relatively low risk for our specific project, but could be nasty for others.

aaron-ngt avatar Jan 18 '22 18:01 aaron-ngt

👋 Hey there. This issue hasn't had any activity for 180 days. We'll automatically close it if that trend continues for another week. If you feel this issue is still valid and needs attention please let us know with a comment.

github-actions[bot] avatar Jul 18 '22 00:07 github-actions[bot]

We're still getting security warnings related to this dependency, don't close this out github-actions :)

tobio avatar Jul 18 '22 00:07 tobio

After spending about a day investigating and spiking out this change request, here are my findings:

  • Unfortunately, there's no easy way to upgrade to "only" remove the trim dependency.
  • Anything above [email protected] (what EUI is at) would be a major breaking change due to the remark team completely changing their underlying parser in 9.x. This breaks EUI's own tooltip and checkbox plugins, and additionally will break any other custom plugins that consumers are currently using (and AFAICT there is no clear or easy-to-follow migration guide written by the team).
  • Note: upgrading remark-*, rehype-*, unified, and v-file etc. to their absolute newest versions, 10.x and above, is not a significantly higher lift than the above breaking change but is still a bit of a headache (mostly due to typing) as they switched to ESM in 10.x+ (link to release)

It's worth noting that several other plugins have struggled with upgrading and current usage of 8.0.3 is still almost double to quadruple 9.x and 10.x:

  • https://github.com/zestedesavoir/zmarkdown/issues/416
  • https://github.com/remarkjs/remark/issues/969

It's not that I don't think we should do this, but this is a significant lift for the EUI team which is currently very low on manpower. There is also something to be said about whether more complex application-like components like EuiMarkdownEditor and EuiDataGrid really belong in EUI, or if they belong at a higher application (e.g. Kibana) level, or if they're complex enough to spin off to their own team.

If the sole goal of this issue is to get remark and unified onto the latest majors, this is a large dev lift both for us and any consumers using custom plugins (larger than the React 17 or possibly even React 18 upgrades, which, IMO, would be significantly higher in priority).

So now what?

If the goal of this issue is primarily to stop the security warnings for [email protected]: I would suggest an interim solution of forking remark-parse at 8.0.3 and forcing it to use trim at 0.0.3 or higher, similar to how EUI handles it via resolutions:

https://github.com/elastic/eui/blob/8ce72124eca84e06182d90c9c8b51118e21135e9/package.json#L55

In fact... it looks like GitHub has done exactly that already:

  • https://github.com/docs/remark-parse-no-trim
  • https://www.npmjs.com/package/remark-parse-no-trim

I might spike/investigate a very basic swap of remark-parse for the above to see if that at least solves the security issues. If that is all folks care about, then great, we can close this issue. If not - I don't really have a timeline for a 9.x or 10.x upgrade, unfortunately.

cee-chen avatar Dec 14 '22 19:12 cee-chen

👋 Hi there - this issue hasn't had any activity in 6 months. If the EUI team has not explicitly expressed that this is something on our roadmap, it's unlikely that we'll pick this issue up. We would sincerely appreciate a PR/community contribution if this is something that matters to you! If not, and there is no further activity on this issue for another 6 months (i.e. it's stale for over a year), the issue will be auto-closed.

github-actions[bot] avatar Oct 22 '23 00:10 github-actions[bot]