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

updated tar package version to 4.4.8 #1713

Merged
merged 2 commits into from
Apr 11, 2019

Conversation

MaksPob
Copy link
Contributor

@MaksPob MaksPob commented Apr 8, 2019

Checklist
Description of change

I updated tar package version in which there were vulnerabilities:
About vulnerability: https://app.snyk.io/vuln/SNYK-JS-TAR-174125

Reviewers

@TooTallNate

@stof
Copy link

stof commented Apr 11, 2019

should package-lock.json actually be committed ? It is not until now.

@stof
Copy link

stof commented Apr 11, 2019

AFAICT, the only BC break in v4 is isaacs/node-tar@a22932a

mkaraula added a commit to mkaraula/node-sass that referenced this pull request Apr 11, 2019
Updated node-gyp to 3.8.1 (nodejs/node-gyp#1713) which got updated because of a security Issue in tar (https://www.npmjs.com/advisories/803)
@stof
Copy link

stof commented Apr 11, 2019

shouldn't this be patched in the 3.x to be able to have it in a 3.8.1 release ?

@jimmybrawn
Copy link

pls merge

@refack
Copy link
Contributor

refack commented Apr 11, 2019

CI: https://ci.nodejs.org/job/nodegyp-test-pull-request/118/ ✔️

@refack refack merged commit 1456ef2 into nodejs:master Apr 11, 2019
@iansltx
Copy link

iansltx commented Apr 11, 2019

/me follows thread so he'll know when 3.8.1 is tagged, to unblock builds 'n' stuff

@stevendarby
Copy link

When might this be released?

refack pushed a commit that referenced this pull request Apr 11, 2019
PR-URL: #1713
Reviewed-By: Refael Ackermann <refack@gmail.com>
@refack
Copy link
Contributor

refack commented Apr 11, 2019

Running CI for v3.x branch - https://ci.nodejs.org/job/nodegyp-test-pull-request/119/

@refack
Copy link
Contributor

refack commented Apr 11, 2019

CI for v3.x fails. Could someone backport this? I'll try to publish as soon as we get tests to pass.

/home/iojs/build/workspace/nodegyp-test-commit/nodes/ubuntu1604-64/lib/install.js:152
        , extracter = tar.Extract({ path: devDir, strip: 1, filter: isValid })
                          ^

TypeError: tar.Extract is not a function
    at /home/iojs/build/workspace/nodegyp-test-commit/nodes/ubuntu1604-64/lib/install.js:152:27
    at /home/iojs/build/workspace/nodegyp-test-commit/nodes/ubuntu1604-64/node_modules/mkdirp/index.js:30:20
    at FSReqWrap.oncomplete (fs.js:135:15)

@jaredbeck
Copy link

jaredbeck commented Apr 11, 2019

Gotta love it when you apply a security patch to your dependency (in this case, tar) and it includes breaking changes (TypeError: tar.Extract is not a function). </sarcasm>

-  "tar": "^3.1.3",
+  "tar": "^4.4.8",

tar isn't going to backport the security patch to the 3-series, huh?

@richardlau
Copy link
Member

Gotta love it when you apply a security patch to your dependency (in this case, tar) and it includes breaking changes (TypeError: tar.Extract is not a function). </sarcasm>

-  "tar": "^3.1.3",
+  "tar": "^4.4.8",

tar isn't going to backport the security patch to the 3-series, huh?

tar.Extract was removed in tar@3. node-gyp@3.8.0 is still using tar@2

"tar": "^2.0.0",

When we bumped tar to 3 (#1212) it was deemed a breaking change so it was made on master but not the v3.x branch. It was breaking for Node.js v0.10 and Node.js v0.12 which are long out of support, so maybe we can reconsider that decision? cc @nodejs/node-gyp

@mstubna
Copy link

mstubna commented Apr 12, 2019

@richardlau confirmed that porting the changes from #1212 into the v3.x branch makes all the tests pass

bsclifton added a commit to brave-experiments/ad-block that referenced this pull request Apr 12, 2019
Once `node-gyp` issues a release, we can back this out and update. The version number for that will likely be `3.8.1`

More info at: nodejs/node-gyp#1713
bsclifton added a commit to brave-experiments/ad-block that referenced this pull request Apr 12, 2019
Once `node-gyp` issues a release, we can back this out and update. The version number for that will likely be `3.8.1`

More info at: nodejs/node-gyp#1713
refack pushed a commit to refack/node-gyp that referenced this pull request Apr 12, 2019
PR-URL: nodejs#1713
Reviewed-By: Refael Ackermann <refack@gmail.com>
@refack refack mentioned this pull request Apr 12, 2019
3 tasks
refack pushed a commit to refack/node-gyp that referenced this pull request Apr 12, 2019
PR-URL: nodejs#1713
Reviewed-By: Refael Ackermann <refack@gmail.com>
@ebmeierj
Copy link

Any idea when this will be released? All of our CI builds are complaining about the vulnerability.

@stof
Copy link

stof commented Apr 15, 2019

@ebmeierj see #1718 for the work on bring that in the 3.x codebase (currently, it is merged only in master, which is the WIP of 4.0)

harkylton pushed a commit to bisondev/node-gyp that referenced this pull request Apr 16, 2019
PR-URL: nodejs#1713
Reviewed-By: Refael Ackermann <refack@gmail.com>
(cherry picked from commit 1456ef2)
rvagg pushed a commit that referenced this pull request Apr 24, 2019
PR-URL: #1713
Reviewed-By: Refael Ackermann <refack@gmail.com>
@smity81435
Copy link

smity81435 commented Apr 24, 2019

Seems that a lot of people are having this issue (myself included)... I suppose you could say they are stuck on the tar?

@samabp
Copy link

samabp commented Apr 24, 2019

@smity81435 Yes that is where I am stuck.

@iwaduarte
Copy link

iwaduarte commented Apr 26, 2019

Is there anything that we could in order to (at least) temporarily fix this ?

@jimmybrawn
Copy link

jimmybrawn commented Apr 26, 2019 via email

@jaysunwalter123
Copy link

Change your package-lock and use npm ci to install your deps

On Fri, 26 Apr 2019, 13:21 iwaduarte, @.***> wrote: Is there anything that we could in order to temporarily fix this ? — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#1713 (comment)>, or mute the thread https://github.com/notifications/unsubscribe-auth/AA4MKIWDTZLONOIWN6LQGVDPSLQSTANCNFSM4HEGUDDQ .

Change it how?

@tsvetomir
Copy link

@KamilPacanek
Copy link

Is it safe/recommended to modify npm manged file 'package-lock.json'? I've always thought it's not and such manual edits are discouraged.

Change your package-lock and use npm ci to install your deps

devo253 added a commit to devo253/actionsrepro that referenced this pull request May 4, 2019
@bitcoinvsalts
Copy link

I am still getting this issue:

Error: node-gyp rebuild failed with: Error: Command failed: node-gyp rebuild
gyp info it worked if it ends with ok
gyp info using node-gyp@5.0.3
gyp info using node@10.16.0 | darwin | x64
gyp info find Python using Python version 2.7.15 found at "/usr/local/Cellar/python@2/2.7.15_1/Frameworks/Python.framework/Versions/2.7/Resources/Python.app/Contents/MacOS/Python"
gyp http GET https://nodejs.org/download/release/v10.16.0/node-v10.16.0-headers.tar.gz
gyp http 200 https://nodejs.org/download/release/v10.16.0/node-v10.16.0-headers.tar.gz
gyp ERR! UNCAUGHT EXCEPTION 
gyp ERR! stack TypeError: tar.extract is not a function

any idea how to resolve this?

HF

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.