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

Remove unused gulp-css-url-adjuster dependency #1691

Merged
merged 2 commits into from
Oct 12, 2023

Conversation

whabanks
Copy link
Contributor

@whabanks whabanks commented Oct 11, 2023

Summary | Résumé

This PR is part 2 of 2 (part 1) to resolve a critical security vulnerability with versions of lodash that are < 4.5.0.

Why?

The dependency tree:

gulp-css-url-adjuster
|__ gulp-util
|____ lodash (< v4.5.0)

gulp-css-url-adjuster is dependent on gulp-util which has been deprecated. The authors recommend moving away from using it. Additionally the authors ask that if your project has a dependency which uses gulp-util to create an issue in the dep's repo recommending it's removal and offering replacements for certain API calls.

Initially this was my plan, however, there has not been a commit in the repo for over 8 years and there are issues that have been open for 6+ years. I expect that if we opened an issue it would receive the same treatment as maintenance of the dep seems to have halted.

Lastly, while we require the dependency in our gulp.js we do not appear to call or utilize it, and removing it has had no apparent affect on the building of our dev containers or within the running application.

An aside

It may be beneficial for relations to open a PR in GDS' Admin repo, or otherwise share this with them as they appear to be in the same boat.

Testing

  • Build the dev container, there should be no errors.
  • Run the app and poke around the site for anything CSS related that seems out of place or incorrect.

@github-actions
Copy link

Copy link
Member

@andrewleith andrewleith left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Good idea on letting GDS know, we have a shared slack channel with them as well. Could ask @jimleroyer his thoughts on the best way to share this.

As for this change, I compared the contents of the generated assets in this branch (all the files in app/static) with those in the main branch and they are identical, so I believe you are correct in that we are not using this anywhere and removing it is innocuous! 🎉

@whabanks whabanks merged commit 17d332e into main Oct 12, 2023
@whabanks whabanks deleted the security/remove-gulp-css-url-adjuster branch October 12, 2023 17:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants