timvisee / prs

🔐 A secure, fast & convenient password manager CLI using GPG and git to sync.
https://gitlab.com/timvisee/prs
GNU General Public License v3.0
211 stars 8 forks source link

prs recipients add - feedback/bug #16

Open colemickens opened 1 year ago

colemickens commented 1 year ago

Okay, in writing out this comment I looked a bit more and figured out what's going on.

Context:

Scenario:

So I poked around a bit more, and then realized that something somewhat unexpected can happen -- my recipients and committed without me even realizing!

So, starting from a naive repo:

(back-context - I think I'd basically added my gpg key TWICE to public-keys, once named by the keygrip, once named by the fingerprint).

So, I guess net result is probably fine. It seems that prs edit <foo> and gopass edit <foo> work fine now.

However, this was definitely a bit unexpected. Even just a log message of "recipients refreshed and committed" at the end of the process would've helped me realize that even though I Ctrl+C'd out of the selection dialog, that prs still went ahead and updated my recipients.

Thought I'd mention this - I don't know if there's a way to use this that would result in valid public keys being dropped... I don't necessarily know what else there is to do here, feel free to close as you see fit, but I thought I'd mention it, it definitely confused me.

colemickens commented 1 year ago

(Unrelated, but I don't want to open another issue, a HUGE thank you for prs. Especially since I just realized there's tomb support too...)

timvisee commented 1 year ago

Interesting!

There must be a discrepancy in the way keys are configured/stored in the password store. My intention is to make prs compatible with the way gopass stores multiple keys.

prs tries to be smart in doing a lot of things for you in the background. One of those things is setting up your key configuration correctly, or syncing your keys. This isn't done on every invocation to limit the amount of work for each operation. Notably, this doesn't run on edit, but it does run on prs recip add (amongst other commands such as prs sync). You can also force this with prs housekeep sync-keys.

I'm assuming that prs had trouble understanding your key configuration, after which it fixed itself when starting the recipient add operation even though you didn't actually complete the operation. Does that make sense?

I must admit that I'm not familiar with 'keygrip' at all, and only know about fingerprints. Recipients are listed here and secrets are encrypted here. As you may be able to see it seems that prs enforces the use of fingerprints, even though a keygrip would probably be valid as well. Since I'm not familiar with keygrip's I must have missed that other implementations support this, and that these are valid recipient identifiers. I assume adding support for this internally would resolve this problem.

However, this was definitely a bit unexpected. Even just a log message of "recipients refreshed and committed" at the end of the process would've helped me realize that even though I Ctrl+C'd out of the selection dialog, that prs still went ahead and updated my recipients.

I agree. prs is as silent as possible to not-be-annoying. But I agree this is likely always significant enough to always notify the user. I think prs recip ls should also show a warning when an entry in .gpg-id is not recognized by prs.

Thought I'd mention this - I don't know if there's a way to use this that would result in valid public keys being dropped... I don't necessarily know what else there is to do here, feel free to close as you see fit, but I thought I'd mention it, it definitely confused me.

This may only remove keys from the .public-keys/ directory if parsing .gpg-id succeeds and a key isn't in there. It never removes a key from your system. And commits any changes (if sync is enabled). So this should never be super problematic.

a HUGE thank you for prs. Especially since I just realized there's tomb support too...

:innocent: :pray: Very happy to see you like it. If you have other suggestions or questions, please feel free to open another issue.

Thanks for your report.

Ps: if you didn't already, you can use prs git log