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

Snyk / npm install / audit alerts #933

Closed
DevRCRun opened this issue Sep 18, 2020 · 4 comments · Fixed by #938
Closed

Snyk / npm install / audit alerts #933

DevRCRun opened this issue Sep 18, 2020 · 4 comments · Fixed by #938
Assignees
Labels
🕔 Hours A well understood issue which we expect to take less than a day to resolve.

Comments

@DevRCRun
Copy link

Hi

I've not looked into exactly how it is used in the kit, but snyk is flagging the "marked" dependency as follows, any chance of a major version upgrade?

  Upgrade marked@0.8.2 to marked@1.1.1 to fix
  ✗ Regular Expression Denial of Service (ReDoS  ) [Medium Severity][https://snyk.io/vuln/SNYK-JS-MARKED-584281] in marked@0.8.2

Also are there any plans to swap out node-sass in favour of the javascript only sass? https://www.npmjs.com/package/sass

While this is just the prototype kit, and we'd catch any issues in any real apps in CI, I'm trying to train our team to always be mindful of npm / snyk warnings rather than just learning the behaviour of ignoring them! (i.e. read that line about request being deprecated and don't spend time implementing it only to have to rip it out later!)

many thanks

@kellylee-gds kellylee-gds added 🕔 Hours A well understood issue which we expect to take less than a day to resolve. ⚠️ high priority and removed awaiting triage labels Sep 21, 2020
@DevRCRun
Copy link
Author

DevRCRun commented Sep 23, 2020

Thanks for the high priority label. This one's just popped up as well, must be the weather for it!

https://snyk.io/vuln/SNYK-JS-UAPARSERJS-610226

Edit - whoops I think that one is only showing up as I don't have a lock file committed (it's a transitive dep from browser-sync)

@joelanman
Copy link
Contributor

thanks, are you getting gulp-sass come up too?

https://snyk.io/test/github/alphagov/govuk-prototype-kit

doesnt seem to be a fix for that one

@DevRCRun
Copy link
Author

DevRCRun commented Sep 23, 2020

Yes I am. In other projects I've been able to swap that out for the reference dart sass https://www.npmjs.com/package/sass so it's a pure javascript implementation (which has in the past been handy doing dev / builds on windows, though I believe node-sass have sorted those problems now)

I'd imagine a similar swap is non trivial for you here?

iirc we made the change when the docs got updated https://frontend.design-system.service.gov.uk/installing-with-npm/#install-with-node-js-package-manager-npm after keeping an eye on alphagov/govuk-frontend#1683

@joelanman
Copy link
Contributor

ugh sorry you mentioned sass in the original post! Have opened an issue for it: #939

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🕔 Hours A well understood issue which we expect to take less than a day to resolve.
Projects
Development

Successfully merging a pull request may close this issue.

3 participants