-
Notifications
You must be signed in to change notification settings - Fork 399
MSC3989: Redact origin
property on events
#3989
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
Merged
Merged
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
8173509
Redact `origin` field on events
turt2live 84051f2
Merge branch 'main' into travis/msc/redact-origin-field
turt2live 5ce2eec
Provide context and remove hunt for more useless fields
turt2live 056bdb1
spelling: actually
turt2live 6317497
Apply suggestions from code review
turt2live e47ec7f
Slight wording alterations
turt2live 1f44feb
Property
turt2live File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,61 @@ | ||
# MSC3989: Redact `origin` field on events | ||
turt2live marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
The [current redaction algorithm](https://spec.matrix.org/v1.6/rooms/v10/#redactions) *keeps* the | ||
top-level `origin` field on events during redaction, however the only use of it within the spec | ||
as of writing is a malformed example of event format. The field has no significant meaning in | ||
modern room versions. | ||
turt2live marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
Within the ecosystem, it's clear that we'd [prefer the field to disappear](https://github.com/matrix-org/matrix-spec/issues/374), | ||
and have [tried to do so](https://github.com/matrix-org/matrix-spec/pull/998) in the past. The | ||
malformed examples are even [known to us](https://github.com/matrix-org/matrix-spec/issues/1480). | ||
|
||
What's not clear, and mentioned in [a comment](https://github.com/matrix-org/matrix-spec/issues/1480#issuecomment-1495183789), | ||
is whether the `origin` field is *actually* used. There do not appear to be any auth rules or similar | ||
which would use the field, however it'd hardly be the first time that the spec was wrong about an | ||
ancient room version like v1. What is clear is that Synapse, where this question would be asked, | ||
wants to [drop support](https://github.com/matrix-org/synapse/issues/3816) for the field and has | ||
taken [steps](https://github.com/matrix-org/synapse/pull/8324) towards that mission by fixing bugs | ||
in the area. In a [quick audit](https://github.com/matrix-org/matrix-spec-proposals/pull/3989#issuecomment-1497659507) | ||
of the Synapse codebase during implementation of this MSC, the `origin` field appears unused. | ||
|
||
Given the above context, this proposal removes the `origin` field from the redaction algorithm in | ||
turt2live marked this conversation as resolved.
Show resolved
Hide resolved
|
||
future room versions, leaving it as-is for existing versions (not that an MSC can change the behaviour | ||
of an already-stable room version). | ||
|
||
Some other fields are additionally useless in modern room versions, however they are already adapted | ||
by [MSC2176](https://github.com/matrix-org/matrix-spec-proposals/pull/2176). | ||
|
||
## Proposal | ||
|
||
In a future room version, the `origin` field is *removed* from the list of *event* keys which are | ||
kept during redaction. Note that this requires a new room version because changing the redaction | ||
algorithm changes how [event IDs](https://spec.matrix.org/v1.6/rooms/v10/#event-ids) are calculated, | ||
as they are [reference hashes](https://spec.matrix.org/v1.6/server-server-api/#calculating-the-reference-hash-for-an-event) | ||
which redact the event during calculation. | ||
|
||
## Potential issues | ||
|
||
No major concerns. | ||
|
||
## Alternatives | ||
|
||
No significant alternatives. | ||
|
||
## Security considerations | ||
|
||
No major concerns. | ||
|
||
## Unstable prefix | ||
|
||
While this MSC is not considered stable, implementations should use `org.matrix.msc3989` as the room | ||
version identifier, using v10 as a base. | ||
|
||
## Dependencies | ||
|
||
No blocking dependencies. | ||
|
||
This MSC would partner well with the following MSCs, however: | ||
* [MSC2176](https://github.com/matrix-org/matrix-spec-proposals/pull/2176) | ||
* [MSC2175](https://github.com/matrix-org/matrix-spec-proposals/pull/2175) | ||
* [MSC2174](https://github.com/matrix-org/matrix-spec-proposals/pull/2174) | ||
* [MSC3821](https://github.com/matrix-org/matrix-spec-proposals/pull/3821) |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.