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

Upgrade tar to address security vulnerability #713

Closed
wants to merge 3 commits into from

Conversation

iAmmar7
Copy link

@iAmmar7 iAmmar7 commented Apr 15, 2024

Upgrade the tar dependency to the latest v7.0.1 to address the following security issue:

GHSA-f5x3-32g6-xq36

@iAmmar7 iAmmar7 changed the title Update tar to address security vulnerability Upgrade tar to address security vulnerability Apr 15, 2024
@cclauss
Copy link
Collaborator

cclauss commented Apr 18, 2024

Please insert the line - npm run update-crosswalk after line 26 of the file appveyor.yml so we can see if your tests pass.

@@ -24,6 +24,7 @@ install:
- IF /I "%PLATFORM%" == "x64" CALL "C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\vcvarsall.bat" amd64
- IF /I "%PLATFORM%" == "x86" CALL "C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\vcvarsall.bat" x86
- npm ci
- npm run update-crosswalk
Copy link
Collaborator

@cclauss cclauss Apr 18, 2024

Choose a reason for hiding this comment

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

Great! Now set lines 5 thru 7 of appveyor.yml to

    - nodejs_version: 18
    - nodejs_version: 20
    - nodejs_version: 22

to match https://nodejs.org/en/about/previous-releases#release-schedule and your tests should be green. ✅

Copy link
Author

Choose a reason for hiding this comment

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

The tests would be green because of the newer Node versions, but are you sure I should remove the older versions from the tests?

Copy link
Author

Choose a reason for hiding this comment

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

Well, it did not pass for Node 20 and 21.

Copy link
Contributor

Choose a reason for hiding this comment

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

@bensquire
Copy link

I'm out of my depth here, but just to say we're seeing pressure to also update our packages because of the above tar dependancy.

@cclauss
Copy link
Collaborator

cclauss commented May 13, 2024

@bensquire I would recommend dropping node-pre-gyp because it is unmaintained.

@bensquire
Copy link

Thanks @cclauss. Will look at what I need to tear out in turn :)

- nodejs_version: 14
- nodejs_version: 18
- nodejs_version: 20
- nodejs_version: 21
Copy link
Contributor

Choose a reason for hiding this comment

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

node 22 is out now

Suggested change
- nodejs_version: 21
- nodejs_version: 22

@cclauss
Copy link
Collaborator

cclauss commented Jun 29, 2024

Git conflicts -- Please rebase.

@benmccann benmccann mentioned this pull request Jun 29, 2024
@cclauss cclauss closed this in #738 Jun 29, 2024
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.

4 participants