-
Notifications
You must be signed in to change notification settings - Fork 401
MSC3952: Intentional Mentions #3952
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
Changes from 1 commit
ac0e70c
5d44598
a2896b2
99ab781
0ce2f60
521a5fd
4de7848
5dfc0bc
e809231
c4516ce
3deebe7
8a3c3c8
415c70b
3ef87c7
f2f4b7a
0ccdff2
4d68314
5bfebcf
bd215e0
e11847d
6269d0d
425d1d6
ced04e1
dc91cc1
86bf972
f0a1f6a
e346fbb
d619f21
8f2805d
7de34e4
040cc31
6b57cae
a2ae719
3461ef8
f5dee49
c79d36f
e869492
04ca142
6ac9392
ea1561c
ec29452
c151958
b975643
4147590
59b5fa9
725772f
16251bd
1d1c8f3
71a9e99
147ff78
9c6ae32
69e99c4
811c0df
63592e7
e0ebbf8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,268 @@ | ||
# MSC3952: Intentional Mentions | ||
|
||
Mentioning other users on Matrix is difficult -- it is not possible to know if | ||
[mentioning a user by display name or Matrix ID](https://github.com/matrix-org/matrix-spec/issues/353) | ||
will count as a mention, but is also too easy to mistakenly mention a user. | ||
|
||
(Note that throughout this proposal "mention" is considered equivalent to a "ping" | ||
or highlight notification.) | ||
|
||
Some situations that result in unintentional mentions include: | ||
|
||
* Replying to a message will re-issue pings from the initial message due to | ||
[fallback replies](https://spec.matrix.org/v1.5/client-server-api/#fallbacks-for-rich-replies). [^1] | ||
* Each time a message is edited the new version will be re-evaluated for mentions. | ||
clokep marked this conversation as resolved.
Show resolved
Hide resolved
|
||
* Mentions occurring [in spoiler contents](https://github.com/matrix-org/matrix-spec/issues/16) | ||
or [code blocks](https://github.com/matrix-org/matrix-spec/issues/15) are | ||
evaluated. | ||
* If the [localpart of your Matrix ID is a common word](https://github.com/matrix-org/matrix-spec-proposals/issues/3011) | ||
then the push rule matching usernames (`.m.rule.contains_user_name`) matches | ||
too often (e.g. Travis CI matching if your Matrix ID is `@travis:example.com`). | ||
* If the [localpart or display name of your Matrix ID matches the hostname](https://github.com/matrix-org/matrix-spec-proposals/issues/2735) | ||
(e.g. `@example:example.com` receives notifications whenever `@foo:example.com` | ||
is replied to). | ||
|
||
As a sender you do not know if including the user's display name or Matrix ID would | ||
even be interpreting as a mention (see [issue 353](https://github.com/matrix-org/matrix-spec/issues/353)). | ||
|
||
This also results in some unexpected behavior and bugs: | ||
|
||
* Matrix users use "leetspeak" when sending messages to avoid mentions (e.g. | ||
referring to M4tthew instead of Matthew). | ||
* Matrix users will append emoji or other unique text in their display names to | ||
avoid unintentional pings. | ||
* It is impossible to ping one out of multiple people with the same localpart | ||
(or display name). | ||
* Since the relation between `body` and `formatted_body` is ill-defined and | ||
["pills" are converted to display names](https://github.com/matrix-org/matrix-spec/issues/714), | ||
this can result in missed messages. | ||
* Bridges [must use display names](https://github.com/matrix-org/matrix-spec/issues/353#issuecomment-1055809364) | ||
as a trick to get pings to work. | ||
* If a user changes their display name in a room than whether or not they are | ||
mentioned changes unless you use historical display names to process push rules. | ||
(TODO I think there's an issue about this?) | ||
|
||
## Background | ||
|
||
Mentions are powered by two of the default push rules that search an event's | ||
`content.body` field for the current user's display name | ||
([`.m.rule.contains_display_name`](https://spec.matrix.org/v1.5/client-server-api/#default-override-rules)) | ||
or the localpart of their Matrix ID ([`.m.rule.contains_user_name`](https://spec.matrix.org/v1.5/client-server-api/#default-content-rules)). | ||
|
||
There's also a [section about "user and room mentions"](https://spec.matrix.org/v1.5/client-server-api/#user-and-room-mentions) | ||
which defines that messages which mention the current user in the `formatted_body` | ||
of the message should be colored differently: | ||
|
||
> If the current user is mentioned in a message (either by a mention as defined | ||
> in this module or by a push rule), the client should show that mention differently | ||
> from other mentions, such as by using a red background color to signify to the | ||
> user that they were mentioned. | ||
|
||
## Proposal | ||
|
||
Instead of searching a message's `body` for the user's display name or the localpart | ||
clokep marked this conversation as resolved.
Show resolved
Hide resolved
|
||
of their Matrix ID, it is proposed to use a field specific to mentions,[^2] and | ||
augment the push rules to search for the current user's Matrix ID. | ||
|
||
### New event field | ||
|
||
A new `mentions` field is added to the event content, it is an array of strings | ||
consistent of Matrix IDs. | ||
|
||
To limit the potential for abuse, only the first 10 items in the array should be | ||
clokep marked this conversation as resolved.
Show resolved
Hide resolved
|
||
considered. It is recommended that homeservers reject locally created events with | ||
more than 10 mentions with an error with a status code of `400` and an errcode of | ||
`M_INVALID_PARAM`. | ||
|
||
Clients should add a Matrix ID to this array whenever composing a message which | ||
includes an intentional [user mention](https://spec.matrix.org/v1.5/client-server-api/#user-and-room-mentions) | ||
(often called a "pill"). Clients may also add them at other times when it is | ||
obvious the user intends to explicitly mention a user. | ||
clokep marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
The `mentions` field is part of the plaintext event body and should be encrypted | ||
into the ciphertext for encrypted events. | ||
|
||
### New push rules | ||
|
||
A new push rule condition and a new default push rule will be added: | ||
clokep marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
```json | ||
{ | ||
"rule_id": ".m.rule.user_is_mentioned", | ||
"default": true, | ||
"enabled": true, | ||
"conditions": [ | ||
{ | ||
"kind": "is_mention" | ||
} | ||
], | ||
"actions": [ | ||
"notify", | ||
{ | ||
"set_tweak": "sound", | ||
"value": "default" | ||
}, | ||
{ | ||
"set_tweak": "highlight" | ||
} | ||
] | ||
} | ||
``` | ||
|
||
The `is_mention` push condition is defined as[^3]: | ||
|
||
> This matches messages where the `content.mentions` is an array containing the | ||
> owner’s Matrix ID in the first 10 entries. This condition has no parameters. | ||
> If the `mentions` key is absent or not an array then the rule does not match; | ||
> any array entries which are not strings are ignored, but count against the limit. | ||
|
||
An example matching event is provided below: | ||
|
||
```json | ||
{ | ||
"content": { | ||
"body": "This is an example mention @alice:example.org", | ||
"format": "org.matrix.custom.html", | ||
"formatted_body": "<b>This is an example mention</b> <a href='https://matrix.to/#/@alice:example.org'>Alice</a>", | ||
"msgtype": "m.text", | ||
"mentions": ["@alice:example.org"] | ||
}, | ||
"event_id": "$143273582443PhrSn:example.org", | ||
"origin_server_ts": 1432735824653, | ||
"room_id": "!somewhere:over.the.rainbow", | ||
"sender": "@example:example.org", | ||
"type": "m.room.message", | ||
"unsigned": { | ||
"age": 1234 | ||
} | ||
} | ||
``` | ||
|
||
### Backwards compatibility | ||
clokep marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
The the [`.m.rule.contains_display_name`](https://spec.matrix.org/v1.5/client-server-api/#default-override-rules) | ||
clokep marked this conversation as resolved.
Show resolved
Hide resolved
|
||
and [`.m.rule.contains_user_name`](https://spec.matrix.org/v1.5/client-server-api/#default-content-rules) | ||
push rules are both deprecated. To avoid the unintentional mentions they are both | ||
modified to only apply when the `mentions` field is missing. As this is for | ||
backwards-compatibility it is not implemented using a generic mechanism, but | ||
behavior specific to those push rules with those IDs. | ||
clokep marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
While this is being deployed there will be a mismatch for legacy clients which | ||
implement the deprecated `.m.rule.contains_display_name` and `.m.rule.contains_user_name` | ||
push rules locally while the `.m.rule.user_is_mentioned` push rule is used on | ||
the homeserver; as messages which | ||
[mention the user should also include the user's localpart](https://spec.matrix.org/v1.5/client-server-api/#user-and-room-mentions) | ||
in the message `body` it is likely that the `.m.rule.contains_user_name` | ||
will also match. It is postulated that this will not cause issues in most cases. | ||
clokep marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
clokep marked this conversation as resolved.
Show resolved
Hide resolved
|
||
## Potential issues | ||
turt2live marked this conversation as resolved.
Show resolved
Hide resolved
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. An outstanding question that feels implied as an undertone in discussions with the SCT is that this MSC might not have had enough time in the wild to really prove itself. We're never going to iron out all the issues, but we're aiming to measure implementation effort and possible ways the system could be abused here: some duration of time with it existing, being used in anger, and generally being available to see how it "feels" day to day might be needed? I'm somewhat indifferent on this personally (more time would be great, but we can also correct course post-FCP if needed), but will leave this here for other SCT/community members to pitch up at. Note: I would assume silence/lack of response means people generally agree with my position: would be nice to have more time, but not a blocker to do so. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @matrix-org/spec-core-team - please review this thread. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think with a little tweaking I can get an Element Web preview up that will work for testing this (it should work completely fine in encrypted rooms, even without server support). I have updated the matrix-js-sdk & matrix-react-sdk PRs to match the MSC though, so it seems quite doable implementation wise. Will get back when there's a thing to test. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. https://pr9983--matrix-react-sdk.netlify.app (built from matrix-org/matrix-react-sdk#9983) is the code that I've been playing with. It implements this MSC and seems to work fairly well. It likely misses some edge-cases (I'm in the process of writing tests) and might be a bit presumptuous that event content is of a reasonable form, but it does seem to illustrate the improvements decently. There's a few interesting things to test:
For testing without a custom Synapse server you can use an encrypted room (since those depend on client behavior anyway to properly handle mentions). Note that I am actively working on that PR so I'll try not to break it, but it might explode. 😄 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I think I've gotten most of the bugs out of this PR now (and wrote some tests for it), so it should now be reasonable to test against. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (leaving this thread open, but moving ahead with FCP anyways - SCT members who prefer to have more implementation first should |
||
|
||
clokep marked this conversation as resolved.
Show resolved
Hide resolved
|
||
### Abuse potential | ||
clokep marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
This proposal makes it trivial to "hide" mentions since it does not require the | ||
mentioned Matrix IDs to be part of the displayed text. This is not seen as | ||
worse than what is possible today. | ||
|
||
From discussions and research while writing this MSC there are some benefits to | ||
using a separate field for mentions: | ||
|
||
* The number of mentions is trivially limited. | ||
* Various forms of "mention bombing" are no longer possible. | ||
* It is simpler to collect data on how many users are being mentioned (without | ||
having to process the textual `body` for every user's display name and local | ||
part). | ||
|
||
Nonetheless, the conversations did result in some ideas to combat the potential | ||
for abuse, many of which apply regardless of whether this proposal is implemented. | ||
|
||
Clients could expose *why* an event has caused a notification and give users inline | ||
tools to combat potential abuse. For example, a client might show a tooltip: | ||
|
||
> The sender of the message (`@alice:example.org`) mentioned you in this event. | ||
> | ||
> Block `@alice:example.org` from sending you messages? `[Yes]` `[No]` | ||
|
||
It could also be worth exposing user interface for moderators to see messages | ||
which mention many users. | ||
|
||
A future MSC might wish to explore features for trusted contacts or soft-ignores | ||
to give users more control over who can generate notifications. | ||
clokep marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
Overall the proposal does not seem to increase the potential for malicious behavior. | ||
|
||
## Alternatives | ||
|
||
### Prior proposals | ||
clokep marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
There's a few prior proposals which tackle subsets of the above problem: | ||
clokep marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
* [MSC2463](https://github.com/matrix-org/matrix-spec-proposals/pull/2463): | ||
excludes searching inside a Matrix ID for localparts / display names of other | ||
users. | ||
* [MSC3517](https://github.com/matrix-org/matrix-spec-proposals/pull/3517): | ||
searches for Matrix IDs (instead of display name / localpart) and only searches | ||
specific event types & fields. | ||
* [Custom push rules](https://o.librepush.net/aux/matrix_reitools/pill_mention_rules.html) | ||
to search for matrix.to links instead of display name / localpart. | ||
|
||
<summary> | ||
|
||
This generates a new push rule to replace `.m.rule.contains_display_name` and | ||
`.m.rule.contains_user_name`: | ||
|
||
```json | ||
{ | ||
"conditions": [ | ||
{ | ||
"kind": "event_match", | ||
"key": "content.formatted_body", | ||
"pattern": "*https://matrix.to/#/@alice:example.org*" | ||
} | ||
], | ||
"actions" : [ | ||
"notify", | ||
{ | ||
"set_tweak": "sound", | ||
"value": "default" | ||
}, | ||
{ | ||
"set_tweak": "highlight" | ||
} | ||
] | ||
} | ||
``` | ||
|
||
</summary> | ||
|
||
### Room version for backwards compatibility | ||
clokep marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
Alternative backwards compatibility suggestions included using a new room version, | ||
similar to [MSC3932](https://github.com/matrix-org/matrix-spec-proposals/pull/3932) | ||
for extensible events. This does not seem like a good fit since room versions are | ||
not usually interested in non-state events. It would additionally require a stable | ||
room version before use, which would unnecessarily delay usage. | ||
clokep marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
## Security considerations | ||
|
||
None foreseen. | ||
clokep marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
## Unstable prefix | ||
|
||
During development the `.org.matrix.msc3952.is_user_mentioned` push rule will be | ||
used. If a client sees this rule available it should apply the custom logic discussed | ||
in the [backwards compatibility](#backwards-compatibility) section. | ||
|
||
## Dependencies | ||
|
||
N/A | ||
|
||
[^1]: This MSC does not attempt to solve this problem, but [MSC2781](https://github.com/matrix-org/matrix-spec-proposals/pull/2781) | ||
proposes removing reply fallbacks which would solve it. Although as noted in | ||
[MSC3676](https://github.com/matrix-org/matrix-spec-proposals/pull/3676) this | ||
may require landing [MSC3664](https://github.com/matrix-org/matrix-doc/pull/3664) | ||
in order to receive notifications of replies. | ||
clokep marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
[^2]: As proposed in [issue 353](https://github.com/matrix-org/matrix-spec/issues/353). | ||
|
||
[^3]: A new push condition is necessary since none of the current push conditions | ||
(e.g. `event_match`) can process arrays. |
Uh oh!
There was an error while loading. Please reload this page.