Skip to content

updated tar package version to 4.4.8#1713

Merged
refack merged 2 commits into
nodejs:masterfrom
MaksPob:update-tar-package-to-4-4-8
Apr 11, 2019
Merged

updated tar package version to 4.4.8#1713
refack merged 2 commits into
nodejs:masterfrom
MaksPob:update-tar-package-to-4-4-8

Conversation

@MaksPob

@MaksPob MaksPob commented Apr 8, 2019

Copy link
Copy Markdown
Contributor
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

stof commented Apr 11, 2019

Copy link
Copy Markdown

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

@stof

stof commented Apr 11, 2019

Copy link
Copy Markdown

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

stof commented Apr 11, 2019

Copy link
Copy Markdown

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

@jimmybrawn

Copy link
Copy Markdown

pls merge

@refack

refack commented Apr 11, 2019

Copy link
Copy Markdown
Contributor

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

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

iansltx commented Apr 11, 2019

Copy link
Copy Markdown

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

@stevendarby

Copy link
Copy Markdown

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

refack commented Apr 11, 2019

Copy link
Copy Markdown
Contributor

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

@refack

refack commented Apr 11, 2019

Copy link
Copy Markdown
Contributor

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

jaredbeck commented Apr 11, 2019

Copy link
Copy Markdown

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
Copy Markdown
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

mstubna commented Apr 12, 2019

Copy link
Copy Markdown

@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
Copy Markdown

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

@stof

stof commented Apr 15, 2019

Copy link
Copy Markdown

@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

smity81435 commented Apr 24, 2019

Copy link
Copy Markdown

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

@samabp

samabp commented Apr 24, 2019

Copy link
Copy Markdown

@smity81435 Yes that is where I am stuck.

@iwaduarte

iwaduarte commented Apr 26, 2019

Copy link
Copy Markdown

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

@jimmybrawn

jimmybrawn commented Apr 26, 2019 via email

Copy link
Copy Markdown

@jaysunwalter123

Copy link
Copy Markdown

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
Copy Markdown

@KamilPacanek

Copy link
Copy Markdown

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
Copy Markdown

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.