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

⚠️ Action required: remove polyfill.io in extra_javascript #7295

Closed
4 tasks done
FutureMatt opened this issue Jun 25, 2024 · 13 comments
Closed
4 tasks done

⚠️ Action required: remove polyfill.io in extra_javascript #7295

FutureMatt opened this issue Jun 25, 2024 · 13 comments
Assignees
Labels
announcement Issue announces news or new features

Comments

@FutureMatt
Copy link

FutureMatt commented Jun 25, 2024

Important

TL;DR: make sure to remove any script referenced in extra_javascript that points to polyfill.io:

  extra_javascript:
    - javascripts/mathjax.js
-   - https://polyfill.io/v3/polyfill.min.js?features=es6
    - https://unpkg.com/mathjax@3/es5/tex-mml-chtml.js

Added by @squidfunk


Context

No response

Description

Polyfill.io was bought by a Chinese company earlier this year and has since then gone on to inject malicious code into the polyfill code it delivers.

Polyfill.io should be removed where possible, if not Fastly and Cloudflare have set up mirrors of safe code.

Related links

Use Cases

This'll affect all users of the project.

Visuals

No response

Before submitting

@FutureMatt
Copy link
Author

I'm going to close this actually, as looking again, I think this is restricted to your examples in your documentation. However I'd recommend updating the documentation to not reference it.

@alexvoss alexvoss added the documentation Issue concerns the documentation label Jun 26, 2024
@alexvoss
Copy link
Sponsor Collaborator

Thanks for drawing attention to this. I do think we should address this, so am reopening it and will assign to myself. @squidfunk can override this if he wants. Am about to call it a day but can do a PR tomorrow first thing to address this.

IMHO, we should draw this to the attention of users, too. People using MathJAX might be affected if they have copied the suggested bit of configuration. Here, polyfill.io is needed only for IE11 compatibility, which I think few people will really need? We could conceivably just take out the mention of polyfill or refer to the MathJAX documentation so we don't have to figure out what the appropriate alternative is?

@alexvoss alexvoss reopened this Jun 26, 2024
@alexvoss alexvoss self-assigned this Jun 26, 2024
@squidfunk
Copy link
Owner

Thanks for reporting! Yes, we should definitely remove the references to poylfill.io. Note that we copied those from MathJax installation instructions some time ago. Nowadays, all browsers that we support should support the entirety of ES6 (or ES2015), so I'd consider it safe to remove it. I'll make the changes asap.

@squidfunk
Copy link
Owner

Fixed in 12a8e82. Since we don't have a channel to communicate this change, and we didn't include polyfill.io in our own sources (it's customization-only), I'm not sure where and how we can reach users to notify them about the recommended removal. Any ideas? Other than that, this issue can be considered resolved.

@alexvoss
Copy link
Sponsor Collaborator

alexvoss commented Jun 26, 2024

Candidates: mention in changelog, pin a discussion item. I will also let the good people at MathJax know and we can add the link to the issue there. I think it is less about effective communication, which we lack the channel for, than about doing our bit and spreading the word. Edit: mathjax/MathJax#3247

@kamilkrzyskow
Copy link
Collaborator

Since mkdocs-material has its own implementation of the search plugin, which is typically enabled for most users, I always thought it would be a good candidate to break the SOLID coding approach, and add a bunch of stuff there like validation of theme.features or in this case validation of extra_javascript. 😅

I'm just not sure if this issue is really that serious to warrant breaking convention, kind of doubt that tbh.

@squidfunk
Copy link
Owner

We currently have no impending release, as there are no changes yet, but I reopen this and pin it, so that users see it and we can add it to our release notes as an actionable item 🤟

Since mkdocs-material has its own implementation of the search plugin, which is typically enabled for most users, I always thought it would be a good candidate to break the SOLID coding approach, and add a bunch of stuff there like validation of theme.features or in this case validation of extra_javascript. 😅

Very valid idea. I've stopped counting how often I've hit the case where it would've been the easiest way for Material for MkDocs to add a few lines of Python to properly configure or validate or whatever do with MkDocs, but you know, it's "just a theme" with some optional plugins and MkDocs doesn't allow themes to execute logic. We might raise this to Tom, since he announced he'll be taking on maintainership to work on the next iteration of MkDocs, but I'm afraid he's heading in a different direction, so there's probably little hope of getting a theme entrypoint into the internals of MkDocs:

�� Templating over code.
Safe rendering, avoid Python knowledge as a dependancy.

Again, I'm taking this back to the drawing board. We need a solution for this. It's too important.

@squidfunk squidfunk reopened this Jun 26, 2024
@squidfunk squidfunk changed the title Polyfill.io is subject to a supply chain attack and should be removed Jun 26, 2024
@squidfunk squidfunk changed the title ⚠️ Action required: remove polyfill.io inextra_javascript Jun 26, 2024
@squidfunk squidfunk pinned this issue Jun 26, 2024
@squidfunk squidfunk added announcement Issue announces news or new features and removed documentation Issue concerns the documentation labels Jun 26, 2024
@vbd
Copy link

vbd commented Jun 27, 2024

In my site folder rg -l polyfill returned:

assets\javascripts\workers\search.b8dbb3d2.min.js.map
assets\javascripts\bundle.5cfa9459.min.js.map
assets\javascripts\bundle.5cfa9459.min.js

I checked my mkdocs.yml for any MathJax occurences but didn't find anything.
What should I do at the moment?

@squidfunk
Copy link
Owner

There's no problem with files that are bundled with the theme. You only have to check and make sure that you do not reference any file hosted on polyfill.io as part of extra_javascript.

@squidfunk
Copy link
Owner

I've updated the original post with a TL;DR.

@vbd
Copy link

vbd commented Jun 27, 2024

Thank you for the clarification.

@charliebritton
Copy link

The registrar have now taken the polyfill.io domain down, so there should be no immediate threats to sites for anyone who might be panicking about this, at least in the short term.

@squidfunk
Copy link
Owner

Perfect, thanks for noting! We can consider this issue resolved then.

@squidfunk squidfunk unpinned this issue Jun 28, 2024
ahouseholder added a commit to CERTCC/SSVC that referenced this issue Jul 9, 2024
Remove link to polyfill.io 
See squidfunk/mkdocs-material#7295
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
announcement Issue announces news or new features
6 participants