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

Do not run npm audit on npm install #7144

Closed
web-padawan opened this issue Dec 12, 2019 · 3 comments · Fixed by #7355
Closed

Do not run npm audit on npm install #7144

web-padawan opened this issue Dec 12, 2019 · 3 comments · Fixed by #7355

Comments

@web-padawan
Copy link
Member

Currently there is a following report because of #7121

added 889 packages from 413 contributors and audited 16985 packages in 59.814s
found 2 moderate severity vulnerabilities
  run `npm audit fix` to fix them, or `npm audit` for details

This report is confusing and we don't want users to be alarmed with these warnings:

  1. Vaadin users don't have to do anything. Using npm audit fix is not a valid solution because Flow manages package.json on its own and the local changes would be overridden

  2. Typically the vulnerabilities only matter for the projects that have Node.js runtime. We only use Node for building with webpack so in most of cases we would be unaffected.

The solution would be to add --no-audit flag which turns the audit off.

@pleku
Copy link
Contributor

pleku commented Dec 20, 2019

I agree, this is confusing the users since they should not really do anything. Specially in its current form.

But I'm not sure if someone could add a @NpmPackage that eg. brings in a frontend library that expose vulnerabilities when loaded on the browser ? In case that is something in the application developers code or from an 3rd party add-on, as a user, I would want to know about this.

Thus could we somehow make it so that any vulnerabilities are reported in a way that by default there is a message:

There were <number> vulnerabilities discovered in the frontend dependencies used in this project.
Please update to the latest Vaadin version, or run with the build with log mode DEBUG or use command <npm audit> to get more detailed list of issues.

And then if we could provide more details when the logging level is DEBUG, by executing npm audit ? Or something better ?

@pleku
Copy link
Contributor

pleku commented Jan 8, 2020

Acceptance Criteria

  • add --no-audit flag for the only npm run we have for pnpm based 2.2, and to explicit npm mode

We might use pnpm audit to be able to notice the users about vulnerabilities in 3rd party addons, but that will be a separate issue.

@pleku pleku moved this from Product backlog to Ready To Go in OLD Vaadin Flow ongoing work (Vaadin 10+) Jan 8, 2020
@denis-anisimov denis-anisimov self-assigned this Jan 15, 2020
@project-bot project-bot bot moved this from Ready To Go to In progress in OLD Vaadin Flow ongoing work (Vaadin 10+) Jan 15, 2020
OLD Vaadin Flow ongoing work (Vaadin 10+) automation moved this from In progress to Done - pending release Jan 20, 2020
pleku pushed a commit that referenced this issue Jan 20, 2020
@pleku
Copy link
Contributor

pleku commented Jan 20, 2020

I can't figure out why we would not pick this for 2.1, it might be an improvement, but then again showing the audit log message to the users makes no sense as they can't react to it - so I'd claim that this is a bug that was fixed.

@pleku pleku added bug and removed enhancement labels Jan 20, 2020
@denis-anisimov denis-anisimov added this to the 2.1.7 milestone Apr 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
OLD Vaadin Flow ongoing work (Vaadin ...
  
Done - pending release
Development

Successfully merging a pull request may close this issue.

4 participants