-
Notifications
You must be signed in to change notification settings - Fork 79
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Message editing and deletion #425
base: master
Are you sure you want to change the base?
Conversation
For servers with CHATHISTORY implementations: any thoughts on exposing an "edited" tag in history playback (as with GitHub or Reddit comments)? Possibilities include:
|
Are you considering rewriting the original message in log storage or just indicating that the message has been edited but not what the new text is? I don't think the latter is very useful, but with the former you lose the ability to diff, which some people might find unacceptable for accountability reasons. |
I was planning to rewrite the original message in log storage, but now I'm thinking that it would be better to just record the edit as a separate event. |
In the typical case, where clients are loading chat history in reverse order (infinite scroll) they'll see the edit coming later in time and hold onto it until they see the message it points at. So by the time the original message is reached, the edit message should be known. This is not the case for clients loading history from an arbitrary point in time (e.g. a BETWEEN call to supplement a theoretical search result) It might be an idea for the log storage to index edits so they can be returned along with the original message, even if they happened in a much later history request window. |
Thoughts on adding a |
That does seem useful. I'm open to a suggested edit to the spec to cover that. |
Summarizing some discussion from #ircv3, we've identified two different models of how this should interact with Both have pros and cons. The "relay model" is more closely reflective of the actual security/privacy properties of editing and deleting messages: once a message has gone out, you have to assume that clients may retain the original text of the message. It's also harder to abuse. However, the "forum model" is a better fit for how users expect online services to treat their content; it also better addresses potential liability issues involved in the server "hosting" message content. (Hypothetically, in a later iteration of the ecosystem where most clients respect DELETE, the "forum model" would also provide a more powerful disincentive to spam.) I think it's preferable if we have a solution that is compatible with both approaches, leaving the choice up to the implementers and/or the server operators. Here's a rough proposal: create three new tags, In the relay model:
In the forum model, the above properties apply with the following modifications:
|
for access control expression, how about we mark messages with named access types and then tell each client what named access types they qualify for? very very rough example to convey the point:
then people going
and people opering up/down get
also, this allows you to give opered users different access types, so opers know they can't delete the messages of other oper users, or so. seems nicely flexible. we could even try to make the ACL of users public so other clients know who can edit their messages? |
I misunderstood the main point of this comment. Following discussion on IRC, here's clarification of @jesopo's suggestion. The idea of the tag is to communicate opaque, server-defined access list names like This avoids having to agree upon and define the meaning of specific access lists up front in a spec. Which seems sensible to me. Still an open question to me whether EDITACL should be replaced with something more generic like ACCESS. Might it be applicable to e.g. channel permission lists as well? e.g. (off the top of my head) a tag on channel JOIN/MODE commands like |
@slingamn the extra tags seem quite... complicated, and orthogonal (though related) to this spec itself. Is there anything about the editing spec itself that would need to change for these tags to be proposed as a separate extension that's more rooted in the chathistory spec itself? |
I think we can move forward on this spec without defining how to tell clients who/what is allowed to edit what. for the time being, servers can just reject attempts to do it from people that can't and we can spec out a more generalised access expression later, as it's own separate IRCv3 mechanism. if we need to do edit/delete-specific stuff for how it relates to a generalised access expression, we can do that in a further |
Ok, ignoring ACCESS for now, and scrolling back to the top of the page. Any queries about the notable changes in this PR and the question I asked?
|
Accepting that not many people will share this perspective: I see relay and playback as roughly equal in importance, so I think new specs are incomplete without a discussion of how they interact with history. They should have "history considerations" sections the way they currently have "security considerations" sections. If you think the question is better broken out into another spec, that's fine. But I, personally, can't really implement it without a picture of how it interacts with history. |
I think it would be helpful to discuss this in a separate issue. Collect all the aspects of other specs that touch on history and need special treatment in that context, see if a pattern emerges that we can turn that into a standard process for raising these issues within other specs or the chathistory spec, or both. Things like echo-message and label have a relationship that's mentioned in both specs. It could be we do something similar here. |
Alright, assuming no more comments, we can probably merge this next week. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I'm late to the party, but I couldn't find an answer to this: Why is the content of the EDIT message in a tag, instead of a param?
Is it to allow editing messages with draft/multiline
? Speaking of which, what about multiline messages over ~4096 bytes?
And how are clients supposed to send messages that don't go over the line size limit, if servers may add arbitrarily many bytes like in the "Fallback" example?
Originally this was because edits were in a tagmsg, with no fallback, and the batch version of multiline didn't exist. There could be an argument for reworking this as multiline EDIT batches maybe?
Up for debate whether that's more or less of a nightmare... |
The fallback example isn't normative, it's up to servers how or if they offer a fallback. |
Correction: no implementations yet, not ready for merge. |
Raised in IRC. The target could also be reworked to be an argument instead of a tag, the tag is mainly a holdover from this being a TAGMSG. |
C: PRIVMSG #channel :an example | ||
S: @msgid=123 :nick!u@h PRIVMSG #channel :an example | ||
C: @draft/target-msgid=123 DELETE #channel | ||
S: @msgid=124;draft/target-msgid=123 :nick!u@h DELETE #channel |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would this be possible?
S: @msgid=124;draft/target-msgid=123 :nick!u@h DELETE #channel | |
S: @msgid=124;draft/target-msgid=123 :nick!u@h DELETE #channel | |
Deleting a TAGMSG: | |
C: @draft/react=🤞TAGMSG #channel | |
S: @msgid=123;draft/react=🤞TAGMSG #channel | |
C: @draft/target-msgid=123 DELETE #channel | |
S: @msgid=124;draft/target-msgid=123 :nick@u@h DELETE #channel |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Possibly. I was also considering just proposing an unreact tag a while ago. Any other TAGMSGs you'd want to delete?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At this point no, there isn't really many other designed tags to consider. I think only typing
from https://ircv3.net/irc/ applys to TAGMSG
. typing
already designed its own mechanism for this (with additional semantics via paused
and done
), although perhaps it would have fit nicely into a single DELETE. Are there any other tag ideas in-flight that we might want to consider here?
I've definately see how useful unreact can be in other protocols, temporary reacting when something is in progress, in review and wanting to remove afterwards. There's also the element of removing an accidental tap or click such as if a client UI offers a single tap/click on top of an existing reaction. I agree it could be solved another way too.
You may also encode a reaction in a PRIVMSG, and then you'd be able to use DELETE
on it.
Regarding message validation, it seems this is settling on server-side which makes me fear servers may end up disabling it due to the high cost, plus the inability to authenticate non-authenticated users after reconnection, etc... I think client-side validation should be easy if we rely on simple crypto... How much space is available in the message tags? Considering we're no longer on 2600bps modems we could surely have a tag sufficiently long to hold a cryptographic key? DNSCurve has embedded crypto keys in SOA hostnames using 54 bytes (51+3 magic constant) using base32, and DNS is very strict on the charset and it may be possible to even use base58, base62 or base64 to sorten it further. Therefore if the tag can be long enough it could include the client's public key. The NaCl library behind DNSCurve has authentication functions with same key/signature size (32bytes/256bit), so the signature for edits could be included in the same tag format, replacing the public key with signature for edit verifiable with original message's public key (the client would match only the unique tag part, not the key/signature). This way clients would generate a random keypair on initial setup (or per day, per server or even channel if desired) and use it to sign updates. Clients who don't want to deal with crypto could just send an invalid/zeroed public key and lose edit capability, and in the hybrid approach suggested by @jwheare in #304 (comment) clients not implementing tags could just ignore the signature and print the update as a normal message without any validation. EDIT: Also, by extension this also means clients could implement signature validation using something like CTCP or even external methods if the server isn't trusted (TBH, I think we should try avoiding a situation where all client start generating a validation exchange for all clannel message, where a single post in a 1000-user channel ends up generating an additional 2000 CTCP messages, but it does open that possibility - the public key with a validation endpoint could also be shown elsewhere like in NickServ or on the user's website) |
strenuousness of proving ownership of a message is not a reasonable concern, and we shouldn't be allowing users to delete messages for previous connections
8192 bytes
absolutely not |
I don't see any benefit from asking clients to implement some overcomplicated crypto system that can't be solved already with server validation. |
Hummm ... Interesting way to close this for someone with her PGP fingerprint right up on her GH profile... I'd love to understand what are the reasons crypto solutions are being rejected like that.... That said since posting this I have read the new draft (as opposed to #304 which I found as a link from an ircd daemon/proxy on Gitlab) and I tend to agree on the benefits of server-validation... I would have probably read directly the latest spec if the title of #304 was modified to indicate it's been replaced by this one... |
Dealing with cryptography on the client side doesn't sound particularly fun unless it's for a really compelling use case. I think it's perfectly reasonable to only support modifying previous messages if the user was logged in when sending them originally, and is now authenticated as the same account.
The final comment links here. |
her*
Good point, I've updated the title of that issue and a couple others to point at their new PRs. If there's any other titles that should be updated similarly, give me a poke in the channel or with a pm on irc. |
* `FAIL DELETE DELETE_FORBIDDEN <target> <target-msgid> :You are not authorised to delete this message` | ||
* `FAIL EDIT INVALID_EDIT <target> :Invalid edit text` | ||
* `FAIL EDIT INVALID_EDIT <target> <target-msgid> :Invalid edit text` | ||
* `FAIL EDIT EDIT_WINDOW_EXPIRED <target> <target-msgid> <window> :You can no longer edit this message` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we define FAIL DELETE DELETE_WINDOW_EXPIRED
too?
I'm gonna give some more meat to the alter that is the carcass of any spec that makes it this far in the IRCv3 process ;) I don't love that It feels like that could go wrong in future as tags are changed, or be awkward/limiting. I would much prefer to read |
I'd like to spec out edit/delete reason field. e.g. |
Now that multiline is being implemented, I think keeping the edit text in a tag is going to be more annoying than putting it as the final parameter. That will also let clients make a normal message into a multiline message easily. Using the EDITMSG suggestion, something like this would be nicer:
And:
|
What fallback can we have when editing multiline message (which won't have Should we just make it mandatory for clients to request multiline (if advertised) before |
Here is a patch simplifying the spec a little, removing the need for extra tags, updating fail codes, etc diff --git a/extensions/message-edit.md b/extensions/message-edit.md
index ff700d7..e5ad3dd 100644
--- a/extensions/message-edit.md
+++ b/extensions/message-edit.md
@@ -42,24 +42,17 @@ Implementations that negotiate these capabilities indicate that they are capable
To edit a message, a client MUST negotiate the `draft/message-edit` capability and send an `EDIT` command to a target nickname or channel. The command is defined as follows:
- @<tags> EDIT <target>
+ EDITMSG <msgid> :<new content>
-And uses the following tags:
+If the client is authorised to edit the message, the server MUST forward this `EDITMSG` to the target recipients with an appropriate prefix, in the same way as PRIVMSG messages.
-* `draft/target-msgid=<msgid>` to indicate the [`msgid`] of the message to be edited
-* `draft/edit-text=<new-text>` to indicate the new text of the message
-
-If the client is authorised to edit the message, the server MUST forward this `EDIT` to the target recipients with an appropriate prefix, in the same way as PRIVMSG messages.
+As `msgid` is [required to be unique](../extensions/message-ids.html) across each network, it's unnecessary to specify within which channel a message is being edited.
### Deleting messages
-To delete a message, a client MUST negotiate the `draft/message-delete` capability and send a `DELETE` command to a target nickname or channel. The command is defined as follows:
-
- @<tags> DELETE <target>
-
-And uses the following:
+To delete a message, a client MUST negotiate the `draft/message-delete` capability and send a `DELETEMSG` command to a target nickname or channel. The command is defined as follows:
-* `draft/target-msgid=<msgid>` to indicate the [`msgid`] of the message to be deleted
+ DELETEMSG <msgid>
If the client is authorised to delete the message, the server MUST forward this `DELETE` to the target recipients with an appropriate prefix, in the same way as PRIVMSG messages.
@@ -67,38 +60,44 @@ If the client is authorised to delete the message, the server MUST forward this
This specification defines `FAIL` messages using the [standard replies][] framework for notifying clients of errors with message editing and deletion. The following codes are defined, with sample plain text descriptions.
-* `FAIL EDIT EDIT_FORBIDDEN <target> <target-msgid> :You are not authorised to edit this message`
-* `FAIL DELETE DELETE_FORBIDDEN <target> <target-msgid> :You are not authorised to delete this message`
-* `FAIL EDIT INVALID_EDIT <target> :Invalid edit text`
-* `FAIL EDIT INVALID_EDIT <target> <target-msgid> :Invalid edit text`
-* `FAIL EDIT EDIT_WINDOW_EXPIRED <target> <target-msgid> <window> :You can no longer edit this message`
+* `FAIL EDITMSG FORBIDDEN <msgid> :You are not authorised to edit this message`
+* `FAIL EDITMSG WINDOW_EXPIRED <msgid> :You can no longer edit this message`
+* `FAIL EDITMSG NOT_FOUND <msgid> :The network does not remember sending this message`
+
+* `FAIL DELETEMSG FORBIDDEN <msgid> :You are not authorised to delete this message`
+* `FAIL DELETEMSG WINDOW_EXPIRED <msgid> :You can no longer delete this message`
+* `FAIL DELETEMSG NOT_FOUND <msgid> :The network does not remember sending this message`
## Client implementation considerations
It is strongly RECOMMENDED that clients provide visible edit and deletion history to users. This helps ensure accountability, and mitigates abuse through malicious or surreptitious edits. This could be done via a tool tip, or a separate log. Edited messages SHOULD be clearly marked as such. Deleted messages MAY be hidden entirely from the primary message log, but a deletion log SHOULD be made available.
-For the purposes of user interface, clients MAY assume that their own messages are editable and deletable. However, this will not always be the case, and there could be other messages that they have permission to act on. Pending a mechanism for discovering editing permissions, clients SHOULD allow users to attempt to edit or delete any message via some mechanism.
+For the purposes of user interface, clients MAY assume that their own messages are editable and deletable, provided the respective capabilities have been acknowledged. However, this will not always be the case, and there could be other messages that they have permission to act on. Pending a mechanism for discovering editing permissions, clients SHOULD allow users to attempt to edit or delete any message via some mechanism.
## Server implementation considerations
This section is non-normative
-A key motivation for specifying this capability as a server tag, rather than a client-only message tag, is to enable more granular editing and deletion permissions. Clients might be able to determine which messages are their own, but other use cases would not be feasible without server validation.
+A key motivation for specifying these capabilities as commands, rather than client tags, is to enable more granular editing and deletion permissions. Clients might be able to determine which messages are their own, but validating other use cases would not be feasible without server mediation.
Such use cases might include:
* Allowing channel moderators or server admins to delete unwelcome messages from others
* Specifying a cut-off time after which message edits are no longer allowed
+Servers are encouraged to grant deletion and editing rights in ways which best serve their communities.
+
### Message validation
-To implement validation, servers require a mechanism for determining the permissions of a particular edit or delete action. The user requesting the action would need to be compared against properties of the message, given only the message ID and target.
+To implement validation, servers require a mechanism for determining the permissions of a particular edit or delete action. The user requesting the action would need to be compared against properties of the message, given only the message ID, and inferring the target.
+
+Servers with message history storage could look up the relevant properties based on the ID, but this might not be possible or desirable in all cases. Note that it's not necessary to store message _content_ to perform this kind of validation. At minimum, only the originating sender, and target, may be required.
-Servers with message history storage could look up the message properties from the ID, but this might not be possible or desirable in all cases. Another mechanism could involve encoding any required properties within the message ID itself, e.g. the account ID, timestamp, etc. Servers might choose to encrypt this information if it isn't usually public facing. Any information encoded in a message ID is still opaque and not intended to be parsed by clients.
+Another mechanism could involve encoding the required information within the message ID itself, e.g. the account ID, channel, and timestamp. Together with a cryptographic signature or encryption (see [JWTs](https://jwt.io)), this can provide tamper-proof validation without metadata storage. Any information encoded in a message ID is opaque and not intended to be parsed by clients.
-### Fallback
+### Fallbacks
-Server implementations might choose to inform clients that haven't negotiated the capability that an edit or deletion has taken place. The fallback method used (if any) is left up to server implementations, but could take the form of a standard NOTICE or PRIVMSG with information about the action. It might be preferable to use relative time descriptions if referring to messages in the past, for example:
+Server implementations might choose to inform clients that haven't negotiated the capability that an edit or deletion has taken place. The fallback methods used (if any) are left up to server implementations, but could take the form of a standard NOTICE or PRIVMSG with information about the action. It might be preferable to use relative time descriptions if referring to messages in the past, for example:
:irc.example.com NOTICE #channel :nickname edited a message from 5 seconds ago: an example
@@ -114,12 +113,12 @@ Editing a message:
C: PRIVMSG #channel :anex ample
S: @msgid=123 :nick!u@h PRIVMSG #channel :anex ample
- C: @draft/target-msgid=123;draft/edit-text=an\sexample EDIT #channel
- S: @msgid=124;draft/target-msgid=123;draft/edit-text=an\sexample :nick!u@h EDIT #channel
+ C: EDITMSG 123 :an example
+ S: :nick!u@h EDITMSG 123 :an example
Deleting a message:
C: PRIVMSG #channel :an example
S: @msgid=123 :nick!u@h PRIVMSG #channel :an example
- C: @draft/target-msgid=123 DELETE #channel
- S: @msgid=124;draft/target-msgid=123 :nick!u@h DELETE #channel
+ C: DELETEMSG 123
+ S: :nick!u@h DELETEMSG 123 |
I don't think you can drop recipient target. Sure, some implementations might be able to just go on the msgid alone, but others might need to know which channel or PM they're in first. |
Edit (2024-09-21): deletion was implemented as draft in #524
This replaces and builds on #304 and https://gist.github.com/jesopo/ef44e928aeb1c893e722909608960b1b
Notable changes:
Something missing is a mechanism for client facing discovery of editing permissions.
A few proposals have been informally discussed in IRC. ISUPPORT, cap-values, etc, but some notification command might be more appropriate to allow permission changes, either something generic and reusable with other specs (with a registry of permissions):
or something specific to this spec
Bit of a thorny one.
Also, do we need better/alternative fallback examples?