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

Safer delete #5

Merged
merged 2 commits into from Dec 6, 2015

Conversation

Projects
None yet
3 participants
@quangv
Contributor

quangv commented Nov 4, 2015

Thanks for creating this plugin, it is useful.

I ran into a problem I had though. This plugin definitely very dangerous... this PR makes it a little less dangerous...

So I passed in

// WARNING: DON'T DO THIS AT HOME
var Clean = require('clean-webpack-plugin');
new Clean('');
// DO NOT DO THIS AT HOME.

and....... it deleted everything in my current project path. Bye bye, .git directory...
gladly it was a beginning of a small project, and the important files I still had tabs open...

but yeh, kinda nuts. I was trying to tell Clean to delete nothing, and I thought passing in an empty string, will cause it to not delete anything..... instead it deleted everything....

This PR prevents this... disallows deleting of the entire project library, also makes sure anything you do delete is a sub-folder of the current project....

// Also prevents these dangerous deletions...
// WARNING DO NOT TRY THIS!
Clean(['', '.', '../', '/'])
Clean(['', '.', '../', '/'], '/root/folder')
// WARNING DON'T DO THIS!

@quangv quangv force-pushed the quangv:saferDelete branch from a9f57dd to 140aa94 Nov 4, 2015

safer clear webpack
disallows deletion of the current project.
disallows deletion of directories outside current project.

@quangv quangv force-pushed the quangv:saferDelete branch from 140aa94 to d32e1b3 Nov 4, 2015

@Ben52

This comment has been minimized.

Ben52 commented Nov 27, 2015

👍 This is a nice plugin, but a little scary without some common sense safeguards in place.

@johnagan

This comment has been minimized.

Owner

johnagan commented Dec 1, 2015

Wow. completely missed this PR. Thank you @quangv

Give me a few days to test, but looks good at first glance 👍

johnagan pushed a commit that referenced this pull request Dec 6, 2015

@johnagan johnagan merged commit 9f27b08 into johnagan:master Dec 6, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment