Narigo / keepass-diff

A CLI-tool to diff Keepass (.kdbx) files. Useful, if syncing with Dropbox or NextCloud and getting multiple files due to conflicts.
https://keepass-diff.narigo.dev/
MIT License
306 stars 28 forks source link

Use entry title as hashmap key for entries #44

Closed ohemelaar closed 2 years ago

ohemelaar commented 2 years ago

The command output seemed more logic for me this way.

Narigo commented 2 years ago

Hi @ohemelaar and thanks for reaching out! Could you please send an output of this, so I know what this change will really do?

Did you try doing this with a kdbx file that does not set a title in an entry? I think we may have had something like this previously and it could have caused issues

ohemelaar commented 2 years ago

Of course, here are two example files. I used Keeweb to create New.kdbx with three passwords, then copied the file and changed the second password and added a fourth one in New1.kdbx. example.zip

I ran cargo run New.kdbx New1.kdbx --passwords new.

This is the output before the PR:

- [New3, third, Password = bv345ncKEJqpfPZ1]
+ [New3, third, Password = VJPM6YZ2fXr258sQ]
- [New3, third, Title = third]
+ [New3, third, Title = fourth]

This is the output after:

+ [New3, fourth]
- [New3, second, Password = NXaCT4U9xCUNJM22]
+ [New3, second, Password = h2git247iJM36wW1]

In the second output I see the diff reflects the changes I actually made to the file. However for the first output I don't really understand what it tells me.

I haven't tested entries without titles though, maybe then it would be preferable to use the group name in that case to keep the current behaviour?

Edit: Changed confusing wording

Narigo commented 2 years ago

Of course, here are two example files. I used Keeweb to create New.kdbx with three passwords, then copied the file and changed the second password and added a fourth one in New1.kdbx. example.zip

I can't check them with my KeePassX client on Mac - it says "Unsupported KeePass database version." 🙁

I ran cargo run New.kdbx New1.kdbx --passwords new.

This is the output before the PR:

- [New3, third, Password = bv345ncKEJqpfPZ1]
+ [New3, third, Password = VJPM6YZ2fXr258sQ]
- [New3, third, Title = third]
+ [New3, third, Title = fourth]

This is the output after:

+ [New3, fourth]
- [New3, second, Password = NXaCT4U9xCUNJM22]
+ [New3, second, Password = h2git247iJM36wW1]

In the second output I see the diff reflects the changes I actually made to the file. However for the first output I don't really understand what it tells me.

Since the passwords do not match, it doesn't look like the outputs were generated with the same files and this could add to the confusion... 😬

Anyways, the first output should tell you there is a group New3 with an entry called third. There are multiple fields in there Title and Password with both of them having changed. You can use the --verbose flag to get a different output that should show it more like a tree structure with the paths and may be better see.

In the second output, is second a group or an item name now? What has happened with item third? Is there just a new item fourth but the third isn't there?

I haven't tested entries without titles though, maybe then it would be preferable to use the group name in that case to keep the current behaviour?

Please check this, I think it will break. I think we had this issue with earlier versions. If it does, it would be great to have a test case for that (I don't think there is one yet).

Edit: This is the output with --verbose:

keepass-diff test/example/New.kdbx test/example/New1.kdbx --passwords new --verbose
~ Group 'New3'
~   Entry 'third'
-     Field 'Password' = 'bv345ncKEJqpfPZ1'
+     Field 'Password' = 'VJPM6YZ2fXr258sQ'
-     Field 'Title' = 'third'
+     Field 'Title' = 'fourth'
ohemelaar commented 2 years ago

I can't check them with my KeePassX client on Mac - it says "Unsupported KeePass database version." slightly_frowning_face

The databases were in KDBX4 format which KeePassX doesn't seem to support. I created new examples in KDBX3 format. examples_kdbx3.zip

Here is the output with --verbose.

branch main
~ Group 'Root'
~   Entry 'third'
-     Field 'Password' = 'mtKfNND2fwyS6wAo'
+     Field 'Password' = 'hWACB2WvAi5rAWXW'
-     Field 'Title' = 'third'
+     Field 'Title' = 'fourth'
branch match-by-title-before-diff
~ Group 'Root'
+   Entry 'fourth'
~   Entry 'second'
-     Field 'Password' = 'jJoru8HHbN9Wg9Nr'
+     Field 'Password' = 'T9sjBcStHu2EE9JP'

Anyways, the first output should tell you there is a group New3 with an entry called third. There are multiple fields in there Title and Password with both of them having changed. You can use the --verbose flag to get a different output that should show it more like a tree structure with the paths and may be better see.

In the second output, is second a group or an item name now? What has happened with item third? Is there just a new item fourth but the third isn't there?

This is where I'm confused, because I didn't change third's Title or Password, I changed second's Password. As third wasn't changed I assume it shouldn't appear in the diff. And fourth appears because it's an entry I added.

Please check this, I think it will break. I think we had this issue with earlier versions. If it does, it would be great to have a test case for that (I don't think there is one yet).

I'd be glad to look into it! :)

Narigo commented 2 years ago

From the files, it looks a lot more like there is a bug lurking in the diffs 🤔

ohemelaar commented 2 years ago

I checked the behavior with databases containing no Title fields. examples_notitle.zip (password is demopass) The changes made in NoTitle1.kdbx are:

Even though the result is the same on the main branch as on this branch, I'm puzzled by the output:

cargo run NoTitle.kdbx NoTitle1.kdbx --passwords demopass --verbose
~ Group 'Root'
~   Entry ''
-     Field 'Password' = 'PsXy84QdGF9moRTg'
+     Field 'Password' = 'cY3XXHfkag7NGWiW'
-     Field 'URL' = 'https://example.com'
+     Field 'URL' = ''
-     Field 'UserName' = ''
+     Field 'UserName' = 'other user'
Narigo commented 2 years ago

There is definitely more wrong currently and I believe that's the root cause of why the output was puzzling in the first place 🙁 I'll add a few tests to check what the problems is and let me have a closer look into that

Narigo commented 2 years ago

I think I have identified the problem: It currently took the name of the group as a key for the entries. It took only one of those of the group and didn't really show the correct changes. I'm fixing this. In the end, you should see all the changes again and you should have the title. I just tested it with your (first) example files, the output would be:

- [New3, second, Password = NXaCT4U9xCUNJM22]
+ [New3, second, Password = h2git247iJM36wW1]
+ [New3, fourth]

and with --verbose:

~ Group 'New3'
~   Entry 'second'
-     Field 'Password' = 'NXaCT4U9xCUNJM22'
+     Field 'Password' = 'h2git247iJM36wW1'
+   Entry 'fourth'

That will sort it by entry index though and not alphabetically.

Narigo commented 2 years ago

Quick update: I've added a couple of test snapshots on a branch called fix-diff-output and created a PR (#46) for it. I think I've fixed it now. As far as I could see it, there was a bigger issue with groups and entries that had the same name, which resulted in very weird and very wrong results.

Would be great if you could have a look @ohemelaar and check whether it works for you as well and the output makes sense to you. Thanks again for pointing out the problem by showing it with your kdbx files!

ohemelaar commented 2 years ago

Indeed fix-diff-output fixes the problem I saw both with or without titles, so I'll close this PR. Thanks for the update!

Narigo commented 2 years ago

@ohemelaar version 1.1.3 is finally out after fixing another bug! Please let me know if anything doesn't work as expected. Thanks!