pacote icon indicating copy to clipboard operation
pacote copied to clipboard

feature: "safe mode" for extraction

Open zkat opened this issue 9 years ago • 2 comments

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.

zkat avatar Apr 05 '17 07:04 zkat

Good if I take this one to get my feet wet?

aem avatar Apr 29 '17 14:04 aem

@aem totally! Go for it! :)

zkat avatar Apr 29 '17 18:04 zkat