JackGruber / joplin-plugin-hotfolder

A plugin to Monitor a locale folder and import the files as a new note.
46 stars 4 forks source link

Possible High Risk Issue with non-existing Hotfolder Paths #28

Closed docphees closed 1 year ago

docphees commented 1 year ago

I have Athena and Hotfolder installed and experienced a really serious issue. I don't know yet if Athena did that or Hotfolder, but it was quite devastating. Maybe someone with a safe test-environment can test with Hotfolder, I am not yet ready to do that:

I apparently did set a Hotfolder Path that did not exist in that form, possibly containing a space character. This lead to Joplin suddenly importing my home directory. I only found out when some programs where suddenly back to default settings. I found, that my /home/username/ directory had been emptied and Joplin suddenly had many many new notes, contained a couple of mp4 videos (!) and lots of note content which were originally dot-files. On the other hand the original files were all gone.

I hope this was Athena, as I rely on it a lot less and will probably kick it out again. But maybe it was Hotfolder. If so, I think it should simply be hardcoded to dismiss imports if they originate from certain paths: a user's home directory, .local/, .config,/ and similar. It should also warn the user if Hotfolder Path is set to a directory which has a huge amount of files in it or sub-directories. I don't know Joplin's API yet, but I hope it lets the plugin create an OK/Cancel dialog if an imported will result in pulling in certain red-flagged file types like dot-files or if it would import a large number of files etc.

JackGruber commented 1 year ago

I don't know what Athena is in this context.

Path that did not exist in that form, possibly containing a space character

If the hotfolder path is a empty string, it is not monitored, also paths with spaces work without problems.

Or sub-directories.

No data is imported recusive, only the root folder is mounted.

lets the plugin create an OK/Cancel dialog if an imported will result in pulling in ...

I will not include a confirmation dialog for something like this. File extensions can be excluded in the settings.

docphees commented 1 year ago

Athena is not important here (it is basically the same as hotfolder with better pdf import).

I partly agree and disagree with you:

My point is, that sanity and existence checks should be put in place to prohibit mass-imports and file deletions. An import directory of the home root or any other basic directory should at least be remarked on by the settings dialog (a red "please check" or similar).

And: No. The root folder is not "mounted", but the files had been imported and then deleted. Not recursively, but everything in the root folder. That can be very dangerous behavior, especially if you have to read the readme file to make sure you get the notation right.

There is no "simulate settings" button or anything and a program that deletes files has to put security checks in or has to be considered unfit.

I am not trying to bash this plugin, it is great btw., but this is absolutely necessary behavior of such a program. And the least I would expect from it is a 100% clear explanation of the settings fields. Saying "read the manual" is not the correct answer here.

That behavior really led to a very bad situation on my machine and I am usually not the overly anxious type. :D

Another option to deal with such things might be to create a "JOPLIN_HOTFOLDER_BACKUP" directory, where imported files could be copied to instead of deleted and either have them deleted after a certain number of days or even leave it to the user to clean out. Keeping proper file names, possibly with a suffix stating date and time of import would be lovely, as that was my main issue in the end: Matching all the files in Joplin with real file names and renaming them after.