You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
This repository has been archived by the owner on Jul 3, 2019. It is now read-only.
While the CLI probably doesn't need to worry about this much except in case of catastrophe, there's some user tooling that could really benefit from pacote's default mode refusing to overwrite directory contents on extract.
opts.extractOverwrite should be required for anyone who targets a directory that: A. exists, B. has any contents in it.
It's ok to be racy about this. If two processes shove things in one dir at the same time, so be it. This feature is primarily to protect about what is bound to be a common footgun for users using straight-up pacote (it literally just bit me and I don't wanna even).
If you think this is an interesting bug to pick up, this is what I think is generally the direction to go in:
add the extractOverwrite option to lib/util/opt-check -- you won't be able to read it otherwise.
add a conditional readdirAsync() call early on in extract.js, before most other work is done. (note: readdirAsync is basically const readdirAsync = BB.promisify(fs.readdir), by convention).
If the resulting listing has any items in it, throw an error with a useful code and an explanation of what the user tried to do -- include a mention of opts.extractOverwrite in the error message so it's discoverable.
If opts.extractOverwrite is true, bypass the fs.readdirAsync call entirely with a BB.resolve().
Feel free to do it your way, too, if you find a better alternative. The goal is to prevent users from accidentally writing into things they didn't intend to write to.
The text was updated successfully, but these errors were encountered:
While the CLI probably doesn't need to worry about this much except in case of catastrophe, there's some user tooling that could really benefit from pacote's default mode refusing to overwrite directory contents on extract.
opts.extractOverwrite
should be required for anyone who targets a directory that: A. exists, B. has any contents in it.It's ok to be racy about this. If two processes shove things in one dir at the same time, so be it. This feature is primarily to protect about what is bound to be a common footgun for users using straight-up pacote (it literally just bit me and I don't wanna even).
If you think this is an interesting bug to pick up, this is what I think is generally the direction to go in:
add the
extractOverwrite
option tolib/util/opt-check
-- you won't be able to read it otherwise.add a conditional
readdirAsync()
call early on inextract.js
, before most other work is done. (note:readdirAsync
is basicallyconst readdirAsync = BB.promisify(fs.readdir)
, by convention).If the resulting listing has any items in it, throw an error with a useful code and an explanation of what the user tried to do -- include a mention of
opts.extractOverwrite
in the error message so it's discoverable.If
opts.extractOverwrite
is true, bypass thefs.readdirAsync
call entirely with aBB.resolve()
.Feel free to do it your way, too, if you find a better alternative. The goal is to prevent users from accidentally writing into things they didn't intend to write to.
The text was updated successfully, but these errors were encountered: