search

SUPERCILEX / gnome-clipboard-history

Gnome Clipboard History is a clipboard manager Gnome extension that saves what you've copied into an easily accessible, searchable history panel.
https://extensions.gnome.org/extension/4839/clipboard-history/
MIT License
470 stars 46 forks source link

Add option for ignoring specific mime type #170

Closed MrSmoer closed 3 months ago

MrSmoer commented 6 months ago

Addresses #92

Some Password-managers like KeePassXC just pin on an additional MIME-Type (the type "x-kde-passwordManagerHint") to mark the current clipboard-content as sensitive. I quickly created a small mockup of a setting that allows the user to specify such a type, which is to be ignored. In short it just calls get_mimetypes on the clipboard and checks if the returned array contains the user-specified type.

It works for me in this state. It is still very unpolished yet, and the hard work (like translations, testing) is still missing. I would like to add a button to overwrite the decision to ignore the clipboard to the notification as well. What do you think about the approach of filtering MIME-Types in general?

I am also a little bit unsure about the repo structure and the way the pre-45-branch is handled in contrast to master, please tell me if I did something wrong ^^

SUPERCILEX commented 6 months ago

What do you think about the approach of filtering MIME-Types in general?

Gut reaction is me no likey, but taking a step back this actually seems pretty reasonable. I guess my main concern is that this isn't applicable to other password managers. Would you mind either figuring out if something like 1Password does the mime type hint or asking folks on the issue to try copying a saved password and then check xclip -o -target TARGETS -selection clipboard to see if there's a password hint?

The settings should probably be a comma separated list. Or maybe we don't need a setting and can just hardcode ignoring entries with known password manager mime types?

I'm also not convinced by the notification... why do we need it?

I am also a little bit unsure about the repo structure and the way the pre-45-branch is handled in contrast to master, please tell me if I did something wrong ^^

Nah, you're good. I made a big ol mess, so I'll figure out how to patch this into the various gnome versions.

MrSmoer commented 6 months ago

I'm also not convinced by the notification... why do we need it?

We surely don't need it, it was nice for testing, and I thought it wouldn't hurt, but it doesn't really matter.

I did some further digging into the adoption of the x-kde-passwordManagerHint and it is not widely adopted. From what I can tell on the password-manager side, just KeePassXC uses it. I tested the way you described ('wl-paste -l' from wl-clipboard seems to work as well):

This seems to have its origin at klipper (the kde clipboard history tool) original commit here and the issue on phabricator-kde introducing this change.

Many other clipboard-managers have open issues about this:

The settings should probably be a comma separated list. Or maybe we don't need a setting and can just hardcode ignoring entries with known password manager mime types?

Actually as i notice now, they all set the value to "secret". This change just checks if the MIME-Type is present at all right now. I believe it should be hardcoded and also exposed as a switch, so the User can decide. From the discussion over at kde-connect, exlicitely used the history feature to transmit passwords. I don't see the usecase here, but there is always someone.

I am no sure whether this should be just a toggle switch and not support any other MIME-Types, because this seems to be the standard way (even though the name is a little bit obscure containing "kde").

SUPERCILEX commented 6 months ago

This is phenomenal research, thank you!

To me, it sounds like we should just hardcode the kde mime and have a "Try to avoid password managers" option with a more detailed explanation of what it does and why "try" is in there.

As for checking for the value secret, if we can't do that I think it's fine? Or at least it sounds like the mime hint will always have the value secret when present.

SUPERCILEX commented 6 months ago

We surely don't need it, it was nice for testing, and I thought it wouldn't hurt, but it doesn't really matter.

I'd say cut for the production release.

MrSmoer commented 5 months ago

I added a check for the content of the x-kde-passwordManagerHint. I was not sure how to do this properly, because the content of the callback has to influence the control flow. I am not sure if Promises are the right thing/we want something asynchronus there, but the call to check for the contents of x-kde-passwordManagerHint is only executed if the mime-type is even there.

I found another problem: For example copying text from inside gnome's search bar doesn't seem add a mime-type at all (at least on the layer the st module has acces to) and somehow it still keeps the mime-type from the last entry that added a mime-type.

To reproduce do this fast(-er than the keepassxc-countdown): Copy password from KeepassXC, hit Super-Key, type something in the search-field, copy text from search-field. This will also be blocked.

At some point, something adds a text/plain;charset=utf-8 at least to the xWayland-clipboard, maybe earlier (displayed with xclip -o -target TARGETS -selection clipboard ), when copying from the search field With unrelated testing using another library for writing content to the clipboard, there were also some inconsistencies with gnomes St-methods for interacting with the clipboard. To me it seems that the calls to the St methods have (too early) lower-level access to the clipboard.

SUPERCILEX commented 5 months ago

For example copying text from inside gnome's search bar doesn't seem add a mime-type at all (at least on the layer the st module has acces to) and somehow it still keeps the mime-type from the last entry that added a mime-type.

Ah, that's unfortunate. I was going to suggest saving the password to a variable and checking to see if the new clipboard entry matches, but that breaks if you copy two passwords in a row. I think we might just have to ignore that problem and say that applications which don't update TARGETS are broken.

A more complicated solution that could be interesting is to activate and deactivate private mode when we see x-kde-passwordManagerHint. So if you copy something with the hint, we immediately enable private mode and don't disable it until we see an entry without the hint. Then if the user disables private mode while the flag is still there we have a "manual override" flag that remembers private mode being disabled until the hint is gone. Thoughts?

MrSmoer commented 5 months ago

Ah, that's unfortunate. I was going to suggest saving the password to a variable and checking to see if the new clipboard entry matches, but that breaks if you copy two passwords in a row. I think we might just have to ignore that problem and say that applications which don't update TARGETS are broken.

I believe so too, but I am not sure how this agrees with the conventions (where and why the text/plain;charset=utf-8 gets tacked on in the end) and whether applications are expected to be able to handle clipboard content without a mime at all.

A more complicated solution that could be interesting is to activate and deactivate private mode when we see x-kde-passwordManagerHint. So if you copy something with the hint, we immediately enable private mode and don't disable it until we see an entry without the hint. Then if the user disables private mode while the flag is still there we have a "manual override" flag that remembers private mode being disabled until the hint is gone. Thoughts?

Yes, I think this would be a very good solution, because manual overrides are always nice. It also communicates to the user that the password wasn't copied by greying the icon, without generating a disturbing notification.

I see a tiny issue with that approach: I would also like to add the first entry without that MIME (the one that deactivates private mode) to the history too, to not disrupt the workflow.

But just looking at it isolated, if private mode was enabled (because of copying somethin with the MIME), the user could believe to be in proper private-mode and have something saved to the history, despite that private mode was enabled before.

I would suggest to add a third state (a temporary private mode), that displays something like a little clock. Or we could just discard the deactivating entry as well, I can't really judge which one would be harder to understand/maintain/frustrating to use.

MrSmoer commented 5 months ago

I also just realized that the asynchronus stuff doesnt seem to work at all with the promise. I got something wrong there.

Edit: It works again :)

SUPERCILEX commented 5 months ago

Hey, just wanted to let you know I'm still taking a look at this but will be busy until next week. Thanks for being patient. :)

schlagmichdoch commented 4 months ago

I like the idea of the mime recognition a lot (probably as I'm using KeepassXC as well).

I am not keen of the idea to activate/deactivate private mode automatically though as I think that would be rather confusing to the user and adds complexity. Often, If I have copied a password I still want to be able to open the clipboard history to select another entry. Alternatively, if I have activated private mode before hand and copy a password then, the manager should not switch to any other mode.

Maybe we could simply add a special entry to top of the history entry menu with type "redacted" as a placeholder that hints to the user that the current clipboard content is not saved to history. When selecting another clipboard entry or when copying something new (without the special mime) the redacted entry is just dropped. When another password is copied nothing changes.

To make this persistent as well, the entry could be saved to the database with empty content and is only shown if it's the last entry in the list. Alternatively, the entry is just not added to the database at all which would result in the last entry before the redacted one to be copied after restart.

I'd like this behavior:

1 Normal:     ° Entry B     Entry A

2 After copy of password:     ° Redacted     Entry B     Entry A

3A After copy of a new string:     ° Entry C     Entry B     Entry A

3B Or after selection of Entry A:     ° Entry A     Entry B

SnoutBug commented 4 months ago

I am not keen of the idea to activate/deactivate private mode automatically though as I think that would be rather confusing to the user and adds complexity.

I second this motion, this seems unnecessarily complicated from my perspective as a user, plus I don't see an issue which could be solved by a "manual override" or toggling private mode automatically. I think it would suffice to just not add an entry to the list when it does not belong on the list.

Windows uses ExcludeClipboardContentFromMonitorProcessing for excluding passwords in your history, do some of the password managers use this flag instead of x-kde-passwordManagerHint or do they just apply nothing at all?

MrSmoer commented 4 months ago

Windows uses ExcludeClipboardContentFromMonitorProcessing for excluding passwords in your history, do some of the password managers use this flag instead of x-kde-passwordManagerHint or do they just apply nothing at all?

TLDR; nothing at all

I am not sure if I get what you mean correctly, but interacting with the clipboard is very OS dependent. Windows uses their Registered Clipboard Formats for that and on linux the Xserver/Compositor implements some kind of other protocol that uses MIME-Types (like in Emails) to advertise data formats. On windows Microsoft made it part of the default win32 api that you can set ExcludeClipboardContentFromMonitorProcessing. On linux on the other hand someone just decided to use this concept using MIMEs on kde's clipboard history tool, and as some applications implement it, this seems to have become some kind of convention somehow. So it's just a different approach and I don't really see why any password manager would try to use something from the windows api on linux .. and there wouldn't be any value in making up a new MIME type called ExcludeClipboardContentFromMonitorProcessing and using that if there is already another one used for that purpose. I have not found any of the Password managers above using any other MIME type, most of them don't do anything, because they are built on Electron and Electron doesn't give that granular control of the clipboard to the application yet afaik.

SUPERCILEX commented 3 months ago

Sorry for dropping the ball here. Some context: I've been working on Ringboard, which is a replacement for this extension. As such, the extension is going to enter maintenance mode, meaning I won't be using it anymore and won't work on any of its issues. That said, I'll still be accepting PRs with small improvements (like this one!) or bug fixes.

Back to this PR: since I'm winding this extension down, I don't want to add any code that could break stuff which means we need to keep things as simple as possible. Here's what I'd like to see:

To all the people in this thread: would this simplification still meet people's needs?

SUPERCILEX commented 3 months ago

Can one of you confirm that my commit works?

MrSmoer commented 3 months ago

I tested your commit and it seems to work fine. I believe not checking the bytes is fine, but maybe add a comment that states that you should actually check for the text 'secret' in order to not mislead people inspired by this application ^^.

SUPERCILEX commented 3 months ago

Good point, done.