fix(dev): update remark-mdx-frontmatter

[MichaelDeBoey]

Fix for GHSA-f7f6-9jq7-3rqj

---

Closes #10576

CC/ @Evanion

[changeset-bot]

⚠️ No Changeset found

Latest commit: 837f725

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

[Copilot]

Copilot reviewed 2 out of 4 changed files in this pull request and generated 1 comment.

Files not reviewed (2)

• packages/remix-dev/package.json: Language not supported
• pnpm-lock.yaml: Language not supported

Comments suppressed due to low confidence (1)

packages/remix-dev/compiler/plugins/mdx.ts:4

• Verify that 'remark-mdx-frontmatter' now provides a default export. If the package version does not support default exports, this change may lead to runtime errors.

import remarkMdxFrontmatter from "remark-mdx-frontmatter";

[charIeszhao]

@brophdawg11 Hi Matt, could you please take a look at this PR and solve the vulnerability issue?

[brophdawg11]

Does anyone know why the tests are failing? Looks to be an ESM/CJS issue

[charIeszhao]

Does anyone know why the tests are failing? Looks to be an ESM/CJS issue

Seems like remix-run does not support ESM...

[crestwood204]

To solve the cjs/esm issue, you can use a dynamic import inside the processMDX function and get rid of the import at the top of the file.

let remarkMdxFrontmatter = await import("remark-mdx-frontmatter");

However, trying this with the tests is giving me other mdx specific errors.

Just need to do let remarkMdxFrontmatter = await import("remark-mdx-frontmatter").default;

I created a PR with my changes here: #10589

[crestwood204]

looks like the integration tests are only failing on msedge. Are those tests flaky? Can you re-trigger them and see if they pass? Would love to get this fix in sooner rather than later since it seems that it can't be solved with an override 🙏

[MichaelDeBoey]

@brophdawg11 I think this PR is good to be merged.
Or do you still have any concerns?

[brophdawg11]

Doesn't this include breaking changes and wouldn't that mean it can't be done in a Remix minor/patch release?

https://github.com/remcohaszing/remark-mdx-frontmatter/releases

From talking to @markdalgleish and @pcattori as well - this is also only an issue in the classic compiler and not an issue if you've moved to Vite (and by proxy RRv7) so maybe those are the proper non-breaking recommendations?

[MichaelDeBoey]

@brophdawg11 What are the breaking changes you are thinking about here?

[brophdawg11]

Generally speaking it's 4 major version bumps so I think it's very unlikely there are no breaking changes.

But more specifically, a few of them sound breaking so I'm not confident we want to just blindly ship it for the old/legacy compiler in a no-longer-maintained v2 line of Remix. From the release notes:

• Remove support for using object keys as export names. The default name is now frontmatter
• Replace js-yaml with yaml as default yaml parser
  • I know nothing about these packages and what might break moving from one to the other
• Update to unified 11 and MDX 3.
  • Same here - did these downstream packages have breaking changes causing a major bum in the mdx package?

Folks have a few ways to move past this vulnerability:

• Move to the vite compiler in Remix v2
• Upgrade to RR v7
• And if neither of those is an option, use patch-package to upgrade it internally until one of the above can be tackled

And finally, I'm not sure if the reported vulnerability is actually something that's exposed at the browser browser level since MDX is processed at build time on your secure server?

[crestwood204]

Move to the vite compiler in Remix v2

With the vite compiler, it suggests in the docs to use import { vitePlugin as remix } from "@remix-run/dev";, which means it's still a dependency even if you're using vite.

Upgrade to RR v7

How would upgrading to react router v7 solve this issue?

And if neither of those is an option, use patch-package to upgrade it internally until one of the above can be tackled

I suppose this is an option, but a pretty bad one imo if you have to patch remix manually in order to use it without vulnerabilities.

[MichaelDeBoey]

Generally speaking it's 4 major version bumps so I think it's very unlikely there are no breaking changes.

From what I see in the release notes, I don't see much that's really a breaking change for us tbh, but people like @Evanion & @crestwood204 can probably also give some more context here

• Remove support for using object keys as export names. The default name is now frontmatter

This only causes Remix to need to update some imports and has no impact on user-code afaik

• Replace js-yaml with yaml as default yaml parser
  • I know nothing about these packages and what might break moving from one to the other

From what I can read, their API is the same.
If parser errors would still occur, we could still point people towards upgrading to Vite I think?

• Update to unified 11 and MDX 3.
  • Same here - did these downstream packages have breaking changes causing a major bum in the mdx package?

From what I can make of the release notes, this won't impact user code either
@wooorm can probably confirm this

Folks have a few ways to move past this vulnerability:

• Move to the vite compiler in Remix v2

This should be the first thing people should do, but not everyone has the time to do so unfortunately, so imo we should not keep these people with a potential vulnerability

• Upgrade to RR v7

That would be the preferred way, but since the team decided we would keep fixing security bugs, I think this PR should be merged

• And if neither of those is an option, use patch-package to upgrade it internally until one of the above can be tackled

That's something I would want to keep as the absolute last result if we can't merge anything to help out.

And finally, I'm not sure if the reported vulnerability is actually something that's exposed at the browser browser level since MDX is processed at build time on your secure server?

I'm not 100% sure either, but since this is a non-breaking change for user-land, I think the best approach is to fix the security vulnerability to help out the community that's still using the old Remix compiler

[wooorm]

@wooorm can probably confirm this

Ehh, most of the releases was minimal, stuff like the breaking node change. But everything’s in the release notes!

[brophdawg11]

which means it's still a dependency even if you're using vite.

Technically yes, but an unused one - so we have to be a bit pragmatic here considering the fact that Remix v2 is in maintenance mode, breaking changes are not an option, and there are multiple other ways to get past this issue. https://overreacted.io/npm-audit-broken-by-design does a great job of touching on where npm audit is problematic here.

How would upgrading to react router v7 solve this issue?

This is no longer a dependency in react router 7 (link)

since this is a non-breaking change for user-land

I guess I'm just not convinced this is true. We're prefacing a lot in the above with "from what I can see/read" so it sounds like we hope) it's not breaking but we are not sure.

If parser errors would still occur, we could still point people towards upgrading to Vite I think?

This is exactly my hesitation - it might be a breaking change so we can't ship it in a v2 release.

Unfortunately, I don't know enough about the mdx story in Remix classic compiler to move this further though and it will have to be something @pcattori or @markdalgleish can move forward.

have to patch remix manually in order to use it without vulnerabilities.

Can someone provide a concrete example of how this is a vulnerability for production apps? This feels very much like it could be a false positive case like the ones outlined in https://overreacted.io/npm-audit-broken-by-design?

[Copilot]

Pull Request Overview

This PR updates the "remark-mdx-frontmatter" dependency to version ^5.1.0 to address a security advisory and ensures that its usage is consistent across the project.

• Updated the dependency version in package.json
• Modified the compiler plugin to use a dynamic default import instead of a static named import
• Aligned the documentation with ES module import syntax

Reviewed Changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated 1 comment.

File	Description
packages/remix-dev/package.json	Upgraded remark-mdx-frontmatter from ^1.0.1 to ^5.1.0 to address security advisories.
packages/remix-dev/compiler/plugins/mdx.ts	Replaced static import with a dynamic default import to match the updated package usage.
docs/guides/mdx.md	Updated the import syntax in documentation to reflect ES module usage.

Files not reviewed (1)

• pnpm-lock.yaml: Language not supported

[pcattori]

We only use remark-mdx-frontmatter to parse frontmatter for routes included in your Remix app. Since you own the routes in your application, there's no way for this to be a security vulnerability unless you deliberately create that vulnerability.

The only "problematic" scenario I can think of is that you may be blindly adding routes from untrusted sources into your app. But at that point all bets are off anyway.

Like @brophdawg11 mentioned, this seems like an all-too-common false positive from npm audit.

[brophdawg11]

Thanks for the details @pcattori - I think I'm convinced enough we can close this out for all the reasons discussed above.

• We've not seen any proof it's a user-facing vulnerability
• There is enough concern from the release notes that this might be a breaking change for existing Remix v2 apps
• There are multiple options for current users to get past this:
  • This is only an issue with the classic esbuild compiler, so upgrading to vite in Remix v2 makes this a non-issue
  • Upgrading to React Router v7 makes this a non-issue
  • And as a last resort folks can patch-package to update the package on their own