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 <[email protected]>
# 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
#
