boxdot / gurk-rs

Signal Messenger client for terminal
GNU Affero General Public License v3.0
458 stars 33 forks source link

Disappearing messages tracking PR #183

Closed Sup3Legacy closed 4 months ago

Sup3Legacy commented 1 year ago

Addresses #79 Non-exhaustive list of things to implement to fully support the disappearing messages feature:

My thoughts

Local deletion

Instead of deleting a message in-memory upon it expiring, I suggest to simply mark it as "expired", which will result in the UI not displaying it and the serializer not including it in the save file. This is already implemented, see the field Message.to_skip.

Keeping the message in-memory alleviates the need for costly and kinda ugly deletion of elements in the Vec holding the messages, while still ensuring that expired messages get deleted from the saved database.

UI

The UI should show a small indicator on every message giving a rough estimation of the remaining time, like in the official clients. Also showing the channel's timer policy can be a nice feature.

Policy synchronization

For now only done in a very simple way. There might be instances where a change is not handled by the current implementation.

a-wild-dev-appeared commented 1 year ago

Instead of deleting a message in-memory upon it expiring, I suggest to simply mark it as "expired"

This sounds horribly insecure.

Keeping the message in-memory alleviates the need for costly and kinda ugly deletion of elements in the Vec holding the messages, while still ensuring that expired messages get deleted from the saved database.

I'm sure users of this software would consider security a non-negotiable aspect and an obvious north star for guiding development.

Sup3Legacy commented 1 year ago

Thank you for this very constructive review, and for all the changes you kindly propose to address the shortcomings you mention!

Regarding your concerns about security, I would argue that any person whose threat-model includes the processes they run getting their private memory read using an exploit should not use this non-certified, non-verified, explicitly WIP piece of software anyway.

a-wild-dev-appeared commented 1 year ago

I'm happy to speak constructively about how to build more secure software for the benefit of the humans, heros, journalists, political dissidents and oppressed that may one day use the software being built here.

I'm even interested to help write some code to this end. I noticed that the database is not encrypted and there's been an open PR for years. I was thinking about tackling that and then saw this open PR. Sorry, I guess mine wasn't such a warm introduction. I do appreciate your efforts here ...

Keeping the message in-memory alleviates the need for costly and kinda ugly deletion of elements in the Vec holding the messages, while still ensuring that expired messages get deleted from the saved database.

Would you be open to my help to make this process less ugly and more secure?

boxdot commented 4 months ago

Closing since we would need to rework this impl from the beginning due to the stale status and merge conflicts.