morispi / HG-CoLoR

Hybrid method based on a variable-order de bruijn Graph for the error Correction of Long Reads
GNU Affero General Public License v3.0
9 stars 2 forks source link

Serious safety problem, --tmpdir parameter is deleted with no checking #2

Closed RaverJay closed 6 years ago

RaverJay commented 6 years ago

Hey, your bash script has a major safety flaw in that it will delete the tmpdir without caring what it is. What if someone were to supply '.' or '~' as the --tmpdir parameter?

You really need to change it so it makes a new folder inside the supplied path and only deletes that after completion.

Another thing is that as per default bash behavior, your script will continue even if intermediate steps fail, even if that makes no sense at all. It thus always arrives at the dangerous rm -Rf command...

So I would advise to set -e in the beginning to change bash behavior to stop on a failed command.

Cheers

morispi commented 6 years ago

Hey,

I indeed assumed that the user would always supply "non critical" paths for the tmpdir, and thus didn't take this case into account. Thanks for warning me, and for the set -e tip.

Pushing the modified script and a few updates on the algorithm later today.

Cheers, P

morispi commented 6 years ago

A bit late, but just pushed the updates. Closing the issue.

Cheers, P