Closed failable closed 11 months ago
Same issue here. It basically makes this package unusable for me :(
How can I confirm if this is the case? Running Debian 11.
If you run KDE then there's a Clipboard entry on the System Tray that shows the contents of the system clipboard. Not sure about Gnome, maybe a Widget?
In any case, after the timeout to clear password in Emacs, you cannot paste/yank anymore in Emacs, but you can outside of Emacs (e.g. on any other text editor, your browser, etc) because while the Emacs kill-ring has been cleared the system clipboard has not.
BTW, this issue is on password-store
which pass
uses, not sure how to report it there.
In any case, after the timeout to clear password in Emacs, you cannot paste/yank anymore in Emacs, but you can outside of Emacs (e.g. on any other text editor, your browser, etc) because while the Emacs kill-ring has been cleared the system clipboard has not.
Ok, thank you. Yes, I can confirm too that this is the case. @calancha I believe you maintain password-store.el
right? This would appear to be a serious bug.
this issue is on password-store which pass uses, not sure how to report it there.
password-store.el
is maintained in the password-store
repository namely:
https://git.zx2c4.com/password-store/tree/contrib/emacs
and I believe they prefer to use a mailing list.
Edit: Add more information.
I see password-store-timeout-timer
is set to nil
. It's a defvar
rather than a defcustom
. I wonder is it just a matter of changing it to match password-store-time-before-clipboard-restore
which is a defcustom
and has a default value of 45s.
mmmm, don't know much elisp but I think that's a timer object of some kind and not just the time to timeout, it is set when saving to the kill-ring
(setq password-store-timeout-timer
(run-at-time password-store-time-before-clipboard-restore nil
(lambda () (funcall #'password-store-clear field))))
Yes, I think you are right.
cc: @DamienCassou
Hi. Sorry for the long delay. I'm not sure what we can do here if it is an upstream issue. If you believe there is anything we can do, please open a PR and make sure you ping me.
@jvillasante I've submitted the below patch (and others) to the mailing list. This seems to work for me. Can you confirm?
@DamienCassou I'd appreciate any suggestion you think could improve this.
From 98a2ccc4c8e3b41b8e54d84f33de318f587c5002 Mon Sep 17 00:00:00 2001
From: Niall Dooley <dooleyn@gmail.com>
Date: Mon, 27 Nov 2023 18:11:34 +0100
Subject: [PATCH] emacs: Fix bug in clipboard management
Prior to this the password/secret was cleared from the kill ring but
remained in the system clipboard after the timeout expired. This
ensures the system clipboard is cleared as well.
---
contrib/emacs/password-store.el | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/contrib/emacs/password-store.el b/contrib/emacs/password-store.el
index 280aee5..c28bc7d 100644
--- a/contrib/emacs/password-store.el
+++ b/contrib/emacs/password-store.el
@@ -262,7 +262,7 @@ the stored secret to clear; if nil, then set it to 'secret.
Note, FIELD does not affect the function logic; it is only used
to display the message:
-\(message \"Field %s cleared.\" field)."
+\(message \"Field %s cleared from kill ring and system clipboard.\" field)."
(interactive "i")
(unless field (setq field 'secret))
(when password-store-timeout-timer
@@ -270,14 +270,15 @@ to display the message:
(setq password-store-timeout-timer nil))
(when password-store-kill-ring-pointer
(setcar password-store-kill-ring-pointer "")
+ (kill-new "")
(setq password-store-kill-ring-pointer nil)
- (message "Field %s cleared." field)))
+ (message "Field %s cleared from kill ring and system clipboard." field)))
(defun password-store--save-field-in-kill-ring (entry secret field)
(password-store-clear field)
(kill-new secret)
(setq password-store-kill-ring-pointer kill-ring-yank-pointer)
- (message "Copied %s for %s to the kill ring. Will clear in %s seconds."
+ (message "Copied %s for %s to the kill ring and system clipboard. Will clear in %s seconds."
field entry password-store-time-before-clipboard-restore)
(setq password-store-timeout-timer
(run-at-time password-store-time-before-clipboard-restore nil
--
2.30.2
@doolio This works perfectly for me! Thanks for taking the time to figure and fix this out!
Glad to hear it. Hopefully, the maintainer of password-store.el
can review it and make it more robust if necessary.
@doolio Until then this is what I'm doing, the call to (kill-new "")
is enough and you can advice the function from the library itself. Hopefully I remember to remove the advice when your change gets merged!
(defun my--password-store-clear-system-clipboard (&rest args)
(kill-new ""))
(advice-add #'password-store-clear :after #'my--password-store-clear-system-clipboard)
Thanks. I use straight.el
so I'm able to rebuild the package with my patch and then later pull in from the repository if and when the changes get merged.
Thank you for the great discussion. The patch sent looks great to me and I hope it will get merged. I think we should close this issue. Anyone against that?
OK, for me. The bug is upstream as well. Hopefully, the patch is merged and/or improved upon.
@jvillasante I've submitted the below patch (and others) to the mailing list. This seems to work for me. Can you confirm?
@DamienCassou I'd appreciate any suggestion you think could improve this.
From 98a2ccc4c8e3b41b8e54d84f33de318f587c5002 Mon Sep 17 00:00:00 2001 From: Niall Dooley <dooleyn@gmail.com> Date: Mon, 27 Nov 2023 18:11:34 +0100 Subject: [PATCH] emacs: Fix bug in clipboard management Prior to this the password/secret was cleared from the kill ring but remained in the system clipboard after the timeout expired. This ensures the system clipboard is cleared as well. --- contrib/emacs/password-store.el | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/contrib/emacs/password-store.el b/contrib/emacs/password-store.el index 280aee5..c28bc7d 100644 --- a/contrib/emacs/password-store.el +++ b/contrib/emacs/password-store.el @@ -262,7 +262,7 @@ the stored secret to clear; if nil, then set it to 'secret. Note, FIELD does not affect the function logic; it is only used to display the message: -\(message \"Field %s cleared.\" field)." +\(message \"Field %s cleared from kill ring and system clipboard.\" field)." (interactive "i") (unless field (setq field 'secret)) (when password-store-timeout-timer @@ -270,14 +270,15 @@ to display the message: (setq password-store-timeout-timer nil)) (when password-store-kill-ring-pointer (setcar password-store-kill-ring-pointer "") + (kill-new "") (setq password-store-kill-ring-pointer nil) - (message "Field %s cleared." field))) + (message "Field %s cleared from kill ring and system clipboard." field))) (defun password-store--save-field-in-kill-ring (entry secret field) (password-store-clear field) (kill-new secret) (setq password-store-kill-ring-pointer kill-ring-yank-pointer) - (message "Copied %s for %s to the kill ring. Will clear in %s seconds." + (message "Copied %s for %s to the kill ring and system clipboard. Will clear in %s seconds." field entry password-store-time-before-clipboard-restore) (setq password-store-timeout-timer (run-at-time password-store-time-before-clipboard-restore nil -- 2.30.2
Niall, thank you so much for your patch! This patch effectively resolves a long-standing issue. I have pushed your fix upstream.
Happy to help.
Although the password in kill ring is removed after seconds, it is kept in system clipboard.