[pass] [PATCH v2] Team pass: enable multiple keys and per directory

[Matthew Cengia]

On 2014-03-19 18:09, Jason A. Donenfeld wrote:
[...]
> Thanks for doing this; I really appreciate it. If you'd like to read
> through the entire pass source
> <http://git.zx2c4.com/password-store/tree/src/password-store.sh> and
> critique the hell out of it, I'd be even more appreciative.

I have read through most of the code before, but only for the purposes
of writing my own patch (the one that supported multiple keys that I
submitted a month or so back). I haven't read through it with the
express purpose of critiquing the code though.

> >   while read -r gpg_id; do
> >     gpg_recipient_args+=( "-r" "$gpg_id" )
> >   done <<<"$PASSWORD_STORE_KEY"
> 
> I thought about this. GPG allows specifying keys by hex ID, though. I
> think this is easier to use than having the inconvenience of needing
> to use newlines in an environment variable. I could be persuaded
> otherwise though.

I don't object to that, as long as it's made clear that a key ID is
what's expected, and that using full names leads to undefined behaviour.

> >   find "$PREFIX/$id_path" -iname '*.gpg' -print0 | while IFS= read -rd '' passfile; do
> 
> Yea... I don't know, I guess. For whatever reason I'm sort of
> hesitant, because most shell situations break when filenames get
> newlines, and the overhead for fixing all of these seems quite
> cumbersome. But maybe I'm foolish here?

I don't think there are many places it needs changing; you've got a
reasonable coding style which for the most part seems to lend itself to
most of these sorts of cases being handled correctly. I'll try to find
some time to review the code myself to see how many places this could
bite you.

> >
> >   passfile_dir=${passfile%/*} passfile_dir=${passfile_dir#$PREFIX}
> >   set_gpg_recipients "$passfile_dir"
> 
> Thanks!

I should've made clear, the above is untested. It should work fine, and
has the down-side of being a bit more obfuscated.

> I use dirname a lot. I suppose I *could* get rid of these as you've
> suggested. Would appreciate a patch I could apply for this. Or I can
> just do it myself.

Not a big deal; as I said above, parameter (or brace) expansion is more
obfuscated, and for the most part you're not using huge loops that make
lots of calls to dirname, so it won't be a huge difference to
performance; my logic is basically that if you can do it in the shell
without it looking horrible and long-winded (both subjective), then do
it.

> > Um... reading a password with read, and echoing it to the terminal?
> > Shouldn't this use 'read -s'?
> 
> There are two modes for inserting a password. The default uses -s. If
> you explicitly ask, though, it takes this path.

Fair enough.

-- 
Regards,
Matthew Cengia
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 966 bytes
Desc: Digital signature
URL: <http://lists.zx2c4.com/pipermail/password-store/attachments/20140320/b52b7012/attachment.asc>