Skip to content
This repository has been archived by the owner on Jan 17, 2023. It is now read-only.

[bug 1478028] replace nsp with npm audit #4704

Closed
g-k opened this issue Jul 24, 2018 · 4 comments
Closed

[bug 1478028] replace nsp with npm audit #4704

g-k opened this issue Jul 24, 2018 · 4 comments
Assignees
Labels
security Security issue: can be an active issue, or related to security hygene

Comments

@g-k
Copy link
Contributor

g-k commented Jul 24, 2018

nsp is deprecated and shutting down: https://blog.npmjs.org/post/175511531085/the-node-security-platform-service-is-shutting

Should allow us to close #4324 and with npm audit fix make updating transitive dependencies for #4617 too.

e.g. https://github.com/mozilla-services/ip-reputation-js-client/blob/78d76c26f412ffddb920a4a8250a1141c24861d7/.travis.yml#L13-L15

@g-k g-k added the security Security issue: can be an active issue, or related to security hygene label Jul 24, 2018
@jaredhirsch
Copy link
Member

Unfortunately, npm audit does not cleanly replace nsp, as there's no way for npm audit to conditionally ignore specific vulnerabilities in transitive dependencies; see npm/npm#20565.

It looks like FxA is currently hacking around this limitation via "npm audit || true" in their CI builds.

We've discussed this a bit, and current consensus would be to warn on npm audit errors in the dev build, but fail on such errors in the stage build, to prevent deploying insecure code to production.

@g-k
Copy link
Contributor Author

g-k commented Jul 24, 2018

That's frustrating. I was hoping npm audit fix would work for everything screenshots pulls in.

If we have github (and sentry if you're using it) vulnerability alerts enabled, swapping npm audit || true would be fine.

Also if you want automatic PRs, @hwine can enable dependabot https://dependabot.com/blog/automatically-respond-to-security-advisories for JS. It looks like it's pulling the github NVD and from the node security working group https://github.com/nodejs/security-wg

@ianb ianb self-assigned this Jul 25, 2018
@ianb ianb added this to the Sprint 18 (63-2) milestone Jul 25, 2018
@ghost ghost mentioned this issue Aug 2, 2018
@ghost ghost modified the milestones: Sprint 18 (63-2), Sprint 19 (63-3) Aug 6, 2018
@ianb
Copy link
Contributor

ianb commented Aug 22, 2018

I've created #4803 as a followup about npm audit || true. Otherwise this was fixed in #4617 (in a5935e1)

@ianb ianb closed this as completed Aug 22, 2018
@g-k
Copy link
Contributor Author

g-k commented Aug 22, 2018

Thanks! Let me know if you write the script in #4803, since we'll probably want to reuse it for other projects too.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
security Security issue: can be an active issue, or related to security hygene
Projects
None yet
Development

No branches or pull requests

4 participants