govuk-prototype-kit icon indicating copy to clipboard operation
govuk-prototype-kit copied to clipboard

Remove jQuery from the default kit installation

Open edwardhorsford opened this issue 7 years ago • 13 comments

I've seen a few messages today about jQuery vulnerabilities < v3. We're currently on 1.11.3.

Should we explore upgrading?

edwardhorsford avatar Jan 23 '18 13:01 edwardhorsford

It's definitely worth exploring updating to 1.12.4 which is the most recent 1.x version. However, this will not fix CVE-2015-9251 which I assume is what you're referring to.

jQuery 1.x is the only version of jQuery that supports IE8.

Unfortunately it is also no longer maintained, which means that the recent jQuery vulnerability is not going to be fixed in 1.x – in fact, by definition, the only way to fix it would require a breaking change, which is not possible without bumping the major version number.

However, as I understand it, CVE-2015-9251 relates only to jQuery's built in AJAX functionality and can be effectively mitigated by ensuring AJAX requests specify a suitable dataType. It's also worth noting that it's been an known issue since 2015, it's just that it's only recently been assigned a CVE number and thus is only now showing up in GitHub's vulnerability reports.

36degrees avatar Jan 24 '18 11:01 36degrees

I think now that we do not have a dependency on jQuery we could consider shipping 3.x for user's who want to use it, or remove it entirely.

NickColley avatar Aug 20 '18 11:08 NickColley

Changing this to bug fix as such an old version of jQuery is a security issue

joelanman avatar Sep 08 '21 14:09 joelanman

We'll probably have to deal with #1081 first, since the out-of-date step by step pattern requires jQuery.

It has been updated to remove the jQuery dependency (https://github.com/alphagov/govuk_publishing_components/pull/1645).

If we decided to bump our version of jQuery, worth noting that the older step by step has not necessarily been thoroughly tested (https://github.com/alphagov/govuk_publishing_components/pull/927).

domoscargin avatar Sep 23 '21 09:09 domoscargin

oh good catch thanks!

joelanman avatar Sep 23 '21 09:09 joelanman

I'm leaning to removing it - its not hard for our users to add it themselves, either in /app/assets or by referencing a CDN.

joelanman avatar Dec 02 '21 17:12 joelanman

Is it possible to find out how many users use it?

My guess is that it's probably quite common - so removing it would mean an extra step for users.

edwardhorsford avatar Dec 02 '21 18:12 edwardhorsford

2 thoughts:

  1. The current update process does not touch /app so nothing will change in existing prototypes. This is not great if we make this as a deliberate change to remove old code.

  2. If people need jQuery if we remove it, the steps to add would be:

add this into your prototype, in app/views/includes/scripts.html

<script src="https://code.jquery.com/jquery-3.6.0.min.js" integrity="sha256-/xUj+3OJU5yExlq6GSYGSHk7tPXikynS7ogEvDej/m4=" crossorigin="anonymous"></script>`

However we probably want to point them at https://code.jquery.com/ to get the latest version of this line

joelanman avatar Dec 03 '21 11:12 joelanman

we have had more reports of people using it, and MOJ Frontend relies on it (see above)

joelanman avatar Dec 03 '21 14:12 joelanman

Would updating jQuery be more or less onerous for users? I'm not entirely familiar with the major changes between v1 and v3 of jQuery, but presumably users would need to migrate - probably beyond our remit to help with this.

Alternatively, if they really needed v1, they could just replace the script src as needed.

domoscargin avatar Dec 03 '21 14:12 domoscargin

If we chose to update jQuery, we'd need to:

  • point out that users can downgrade manually at their own risk
  • point them to jQuery's migration plugin to get their prototypes working with v3+

domoscargin avatar Dec 07 '21 09:12 domoscargin

Decision is to remove it.

Izabela-16 avatar Dec 07 '21 11:12 Izabela-16

We're removing jquery bit by bit:

  • [x] #1418
  • [x] #1427
  • [x] #1448

lfdebrux avatar Jun 29 '22 13:06 lfdebrux

This will be fixed in v13.

lfdebrux avatar Oct 31 '22 12:10 lfdebrux