Prevent from removing whole password-store directory and hidden directories and files

[SergeyBear]

Clicking Delete button when there is no focus on any treeview item (when opening QtPass or clicking on already selected item, etc) will lead to complete removing of password-store directory including .git repository, so there will be no way to restore password-store if you don't have backups or remote origin.

QtPass version 1.2.3

[FiloSpaTeam]

Thank you! I was trying to reproduce but i didn't tried without focus! :+1: I'm sorry for your folder :(:(

[annejan]

I am sorry for your loss.

I'll try and fix this tomorrow (not around computer today) to prevent future incidents.

Weird thing is that I can remember this kind of thing (disabling delete button) used to work correctly in the past.

[SergeyBear]

Don't worry, I have backups - useful habit :-) Just don't want to someone shoot himself in a leg.

P.S Thank you for quick response and great QtPass!

[annejan]

Hi @SergeyBear I'm trying to reproduce this . . but the red X delete button gets disabled as soon as I don't have focus on any file or folder and the right click context menu doesn't show delete either in those cases for me . . on macOS at-least.

Can you help me reproduce this error?

[SergeyBear]

Hello @annejan !

To reproduce: click on any item in treeview twice with some delay (not doubleclick). After that focus disappears but Delete button stays enabled and offers to remove /

Found annother issue with focus: If I switch from one passwordstore to another without selecting item on treeview of openned passwordstore, focus stays on previous passwordstore and toolbar items perform actions also on previous passwordstore.

To reproduce: create two passwordstores Test1 and Test2. Select item on Test1 and switch to Test2 and press Create button on toolbar - it will offer to create item on Test1 onstead of Test2.

OS: Fedora 27/28 (all packages uptodate) PACKAGE: qtpass-1.2.3-1.fc27.x86_64/qtpass-1.2.3-1.fc28.x86_64

[annejan]

Thank you for this very complete report, will get on it when I find the time . .

The "funny" thing is that I didn't test this (probably) because it feels un-natural for me to click twice to deselect . . interesting . .

I can now reproduce and will fix ASAP.

[FiloSpaTeam]

With last pull #407 i fixed this too. I check for selectionModel when possible and rowCount of proxyModel to disable accordingly button and bypass Enter event :+1:

[rdoeffinger]

Maybe it would be good to not rely on the button being disabled, but also double-check in the actual delete function though? Or was that already done in some of those fixes?

[FiloSpaTeam]

The problem is that i cannot check in the delete function because in some way a valid path is passed but the tree source is on / or different path. At the moment I'm checking how the model is used with the proxy to solve definitively.

[rdoeffinger]

I was more thinking of things like refusing to delete files/directories starting with ".", refusing to delete non-empty directories (at least without extra confirmation), refusing anything that isn't actually below the password store repo or similar measures...

[FiloSpaTeam]

Yep this should be but in some way this is not working due to different parameters checking. I think i'll find a way checking the model or the proxyModel :+1: