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

fix: some vulnerabilities #717

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

Florian-Schoenherr
Copy link

Purpose of this pull request?

  • Documentation update
  • Bug fix
  • Enhancement
  • Other, please explain:

What changes did you make?

Fixed some of the security vulnerabilities, the others depend on some of the PRs in sub repos.

Is there anything you'd like reviewers to focus on?

I removed the "pretest": "xo" script for now, as it doesn't work. I think temporarily removing it is better than keeping a high security vulnerability around. This is the error when keeping it:
image
the cut line says Cannot find package 'D:\...\yo\node_modules\xo\node_modules\eslint\' imported from D:\...\yo\node_modules\xo\index.js

Signed-off-by: Florian-Schoenherr <[email protected]>
Signed-off-by: Florian-Schoenherr <[email protected]>
Signed-off-by: Florian-Schoenherr <[email protected]>
Signed-off-by: Florian-Schoenherr <[email protected]>
@Florian-Schoenherr
Copy link
Author

oh and I replaced got with node-fetch, which wasn't really necessary here, could be reverted, too.
Although does the fetch right there even work? If I just go on that link it gives back a 404 page.

@@ -17,7 +17,6 @@
"scripts": {
"postinstall": "yodoctor",
"postupdate": "yodoctor",
"pretest": "xo",
Copy link
Member

Choose a reason for hiding this comment

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

We should keep pretest script.

@@ -84,7 +82,7 @@
"proxyquire": "^2.0.1",
"registry-url": "^5.1.0",
"sinon": "^6.1.3",
"xo": "0.38.0"
"xo": "^0.47.0"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"xo": "^0.47.0"
"xo": "0.38.0"

There are security problems, but this is dev only, won’t be a problem for npm install yo.
Version 0.40.0 is esm only. So 0.39.x should be to newest possible.

@crazy-tush
Copy link

@Florian-Schoenherr - could you please merge the changes related to "npm-keyword" & "latest-version".

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.

3 participants