[PATCH] RFC: hooks for clipboard manager integration
Dov Feldstern
dovdevel at gmail.com
Sun Jul 5 20:03:18 CEST 2020
Thanks for the feedback!
I'm not sure exactly what you mean by a "callback" here: a specific
implementation of how exactly to do the copy and paste (which the
current code seems to *almost* support, in the form of "copy_cmd" and
"paste_cmd", all that's missing is a means for changing them less
intrusively than by adding more if cases in the code itself)? Or
remiplementing the entire clip() function as a whole?
IIUC, the latter can already be achieved by providing a
platform-specific implementation of clip()?
The problem with reimplementation, though (and why I ultimately
preferred to use hooks), is that there is non-trivial logic going on
in the function, which might be lost by the reimplementations. For
example, the sample snippet you provide above doesn't deal with the
subtlety of avoiding a second call to 'pass -c' restoring -- after the
timeout -- the value copied there by the first 'pass -c' (it took me a
long time to realize that the current code is avoiding that using the
pkill...). Although, perhaps for a platform-specific implementation it
makes more sense to expect a high-quality implementation, than for a
clipboard-manager-specific implementation -- assuming that there is a
wider variety of clipboard managers than there are platforms...
I appreciate, though, that hooks do not solve the problem you are
trying to solve :/
On Sun, Jul 5, 2020 at 6:28 AM Allan Odgaard <lists+pass at simplit.com> wrote:
>
> On macOS the way for password managers to temporarily put sensitive data
> on the clipboard is by tagging the data.
>
> Therefore a pre/post hook will not be a good solution for macOS, instead
> it should be a callback that places the content on the clipboard.
>
> A simple interface would be a function that accepts $password and
> $duration, the default implementation on macOS (using `pbcopy` and
> `pbpaste`) would be something like this:
>
> function user_function {
> password="$1"
> duration="$2"
>
> old="$(pbpaste)"
> printf "%s" "$password" | pbcopy
>
> sleep "$duration"
>
> if [[ "$(pbpaste)" == "$password" ]]; then
> printf "%s" "$old" | pbcopy
> fi
> }
>
> The advantage of using pre/post hooks is that it does not become the job
> of the callback to sleep nor check if content has changed.
>
> However, for the latter, a callback might do better than `pass` itself,
> for example on macOS the clipboard has a change counter, so the callback
> can check that to learn about changes.
>
> Furthermore for the `sleep`: Having the callback perform the sleep, we
> avoid having to introduce state that is shared between the pre and post
> hook, if we also want to improve restoring content (e.g. on macOS `pass`
> will currently not restore non-text content or text with multiple
> representations).
>
> So all in all, I think the interface should be a single function that
> does all the work, instead of introducing hooks, as I don’t think the
> complexity of implementing a callback is higher than writing hooks, and
> a function is much less limited, e.g. on macOS we will be able both to
> tag the content (for clipboard managers) and we will be able to do a
> better job at restoring the old clipboard data, neither of which is
> possible with the hooks.
>
> For the records though, in order to improve things on macOS, we will
> need to write a compiled helper and use that instead of
> `pbcopy`/`pbpaste`.
>
>
> On 5 Jul 2020, at 8:20, Dov Feldstern wrote:
>
> > # HG changeset patch
> > # User Dov Feldstern <dovdevel at gmail.com>
> > # Date 1593908715 -10800
> > # Sun Jul 05 03:25:15 2020 +0300
> > # Node ID d4af3f470f3f04fb81bbcb079556ea56e8e43898
> > # Parent dcfa9a1c9c827206f899e7ba604f42366fbd60f3
> > RFC: hooks for clipboard manager integration
> >
> > I've been using password-store for a few months now, and am very happy
> > with it
> > -- thanks to all involved!
> >
> > One thing that I'm still grappling with is how to avoid having the
> > passwords
> > get stored by my clipboard manager (currently copyq). I see that
> > discussions of
> > this come up in the mailing list every once in a while, for various
> > clipboard
> > managers; and also from the clipboard manager side, I see that many
> > clipboard
> > managers discuss how to deal with the issue of passwords. But there
> > doesn't
> > seem to be any standard way, which obviously makes dealing with this
> > in a
> > generic way a bit difficult...
> >
> > In this patch, I propose to add hooks, which can then be implemented
> > as
> > appropriate for the various clipboard managers. I moved the handling
> > for
> > klipper into the "default" hook, and in contrib/ I added a reference
> > implementation for how I'd like passwords to be handled for copyq --
> > so this
> > demonstrates that the method works for at least two password managers
> > ;) .
> >
> > I am offering this here as an RFC: is this kind of approach acceptable
> > to the
> > project? If so, probably the hooking mechanism should be designed a
> > bit more
> > throughly than what I've done here (I am also not very proficient in
> > bash
> > programming, so there's probably a nicer way to add hooks than what
> > I've
> > done...). But once such a hooking mechanism is in place, I think it
> > offers a
> > more scalable solution for dealing with the variety of clipboard
> > managers, and
> > will probably also find a bunch of other use cases, as well...
> >
> > I'd be glad to hear any thoughts on this...
> >
> > Thanks!
> > Dov
> >
> > diff -r dcfa9a1c9c82 -r d4af3f470f3f contrib/hooks/copyq.sh
> > --- /dev/null Thu Jan 01 00:00:00 1970 +0000
> > +++ b/contrib/hooks/copyq.sh Sun Jul 05 03:25:15 2020 +0300
> > @@ -0,0 +1,11 @@
> > +hook_clip_pre_copy_pw() {
> > + copyq disable
> > +}
> > +
> > +hook_clip_post_copy_pw() {
> > + copyq enable
> > +}
> > +
> > +hook_clip_pre_restore() {
> > + return
> > +}
> > diff -r dcfa9a1c9c82 -r d4af3f470f3f src/password-store.sh
> > --- a/src/password-store.sh Thu Jun 25 23:41:11 2020 +0200
> > +++ b/src/password-store.sh Sun Jul 05 03:25:15 2020 +0300
> > @@ -14,6 +14,7 @@
> >
> > PREFIX="${PASSWORD_STORE_DIR:-$HOME/.password-store}"
> > EXTENSIONS="${PASSWORD_STORE_EXTENSIONS_DIR:-$PREFIX/.extensions}"
> > +HOOKS="${PASSWORD_STORE_HOOKS_DIR:-$PREFIX/.hooks}"
> > X_SELECTION="${PASSWORD_STORE_X_SELECTION:-clipboard}"
> > CLIP_TIME="${PASSWORD_STORE_CLIP_TIME:-45}"
> > GENERATED_LENGTH="${PASSWORD_STORE_GENERATED_LENGTH:-25}"
> > @@ -152,6 +153,25 @@
> > # BEGIN platform definable
> > #
> >
> > +hook_clip_pre_copy_pw() {
> > + return
> > +}
> > +
> > +hook_clip_post_copy_pw() {
> > + return
> > +}
> > +
> > +hook_clip_pre_restore() {
> > + # It might be nice to programatically check to see if klipper
> > exists,
> > + # as well as checking for other common clipboard managers. But for
> > now,
> > + # this works fine -- if qdbus isn't there or if klipper isn't
> > running,
> > + # this essentially becomes a no-op.
> > + #
> > + # Clipboard managers frequently write their history out in
> > plaintext,
> > + # so we axe it here:
> > + qdbus org.kde.klipper /klipper
> > org.kde.klipper.klipper.clearClipboardHistory &>/dev/null
> > +}
> > +
> > clip() {
> > if [[ -n $WAYLAND_DISPLAY ]]; then
> > local copy_cmd=( wl-copy )
> > @@ -175,21 +195,15 @@
> > # trailing new lines.
> > pkill -f "^$sleep_argv0" 2>/dev/null && sleep 0.5
> > local before="$("${paste_cmd[@]}" 2>/dev/null | $BASE64)"
> > + hook_clip_pre_copy_pw
> > echo -n "$1" | "${copy_cmd[@]}" || die "Error: Could not copy data
> > to the clipboard"
> > + hook_clip_post_copy_pw
> > (
> > ( exec -a "$sleep_argv0" bash <<<"trap 'kill %1' TERM; sleep
> > '$CLIP_TIME' & wait" )
> > local now="$("${paste_cmd[@]}" | $BASE64)"
> > [[ $now != $(echo -n "$1" | $BASE64) ]] && before="$now"
> >
> > - # It might be nice to programatically check to see if klipper
> > exists,
> > - # as well as checking for other common clipboard managers. But for
> > now,
> > - # this works fine -- if qdbus isn't there or if klipper isn't
> > running,
> > - # this essentially becomes a no-op.
> > - #
> > - # Clipboard managers frequently write their history out in
> > plaintext,
> > - # so we axe it here:
> > - qdbus org.kde.klipper /klipper
> > org.kde.klipper.klipper.clearClipboardHistory &>/dev/null
> > -
> > + hook_clip_pre_restore
> > echo "$before" | $BASE64 -d | "${copy_cmd[@]}"
> > ) >/dev/null 2>&1 & disown
> > echo "Copied $2 to clipboard. Will clear in $CLIP_TIME seconds."
> > @@ -246,6 +260,10 @@
> >
> > source "$(dirname "$0")/platform/$(uname | cut -d _ -f 1 | tr
> > '[:upper:]' '[:lower:]').sh" 2>/dev/null # PLATFORM_FUNCTION_FILE
> >
> > +for hook in $HOOKS/*.sh; do
> > + source $hook
> > +done
> > +
> > #
> > # END platform definable
> > #
More information about the Password-Store
mailing list