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

Update remark-parse to newest version #5543

Closed
aaron-ngt opened this issue Jan 18, 2022 · 7 comments
Closed

Update remark-parse to newest version #5543

aaron-ngt opened this issue Jan 18, 2022 · 7 comments
Labels
dependencies Pull requests that update a dependency file stale-issue stale-issue-closed

Comments

@aaron-ngt
Copy link

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.

@chandlerprall
Copy link
Contributor

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

@aaron-ngt
Copy link
Author

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

@github-actions
Copy link

👋 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.

@tobio
Copy link
Member

tobio commented Jul 18, 2022

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

@cee-chen
Copy link
Contributor

cee-chen commented Dec 14, 2022

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:

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:

"**/trim": "0.0.3",

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

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.

@github-actions
Copy link

👋 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.

Copy link

❌ Per our previous message, this issue is auto-closing after having been open and inactive for a year. If you strongly feel this is still a high-priority issue, or are interested in contributing, please leave a comment or open a new issue linking to this one for context.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Apr 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file stale-issue stale-issue-closed
Projects
None yet
Development

No branches or pull requests

4 participants