Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

m.room.retention is not carried over during room upgrade #11279

Open
reivilibre opened this issue Nov 8, 2021 · 5 comments
Open

m.room.retention is not carried over during room upgrade #11279

reivilibre opened this issue Nov 8, 2021 · 5 comments
Labels
A-Message-Retention-Policies Issues related to Synapse's support of MSC1763 (message retention policies) S-Minor Blocks non-critical functionality, workarounds exist. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues.

Comments

@reivilibre
Copy link
Contributor

Discussed in https://github.com/matrix-org/synapse/discussions/11277

Originally posted by Mikaela November 8, 2021
I just upgraded a room in https://develop.element.io/ from version 6 to version 9. The version 6 had m.room.retention set, but after the upgrade, I couldn't find it from the room version 9.

(snip)

I think it should carry over as it may be a privacy or data hygiene problem to those rooms that enable it (while they are a minority) and carrying it over now would help not accidentally forgetting it later.

@reivilibre
Copy link
Contributor Author

Speaking personally, I would add that these people are already using experimental features, I don't see any harm in experimentally carrying them over on top of that. (It's not permanent is it?).

@clokep
Copy link
Member

clokep commented Nov 8, 2021

Note that the spec should allow this (relevant section: https://matrix.org/docs/spec/client_server/latest#id163), it specifies which events must be moved over, but then gives some allowance for server-specific behavior.

It would probably be good to update the room retention MSC to say whether these should be kept during room upgrades.

@babolivier
Copy link
Contributor

babolivier commented Nov 8, 2021

Continuing the conversation from the discussion and replying to @Mikaela's comment:

I think it should carry over as it may be a privacy or data hygiene problem to those rooms that enable it (while they are a minority) and carrying it over now would help not accidentally forgetting it later.

Another important point to take into account here is that the feature is currently disabled by default in Synapse (since it's an experimental feature), and afaik not implemented in other homeservers, so there are more severe privacy/data hygiene issues related to the current state of this feature in the ecosystem (namely that even once enabled in a federated room it's very unlikely that all servers will care about it), for which imo the fix is stabilising the feature in the spec (ie fixing the MSC and getting it through the spec process).

I do think that you make a good point here in that it should eventually be included in the event we carry over - and the MSC should also eventually update the spec's recommendations on which events to carry over during a room upgrade. However I worry that it might be understood as a sign that the feature is production-ready (since we'd already have taken care of details like this), which, to be clear, is not the case unless you're running very specific closed-federation setups. The current implementation is not complete, and will be subject to change whenever we have bandwidth to push the proposal to a point where we'd be happier to bring it into the spec.

TL;DR: I'm happy that we do it, but I think now is a bit early given the current state of the feature.

@clokep clokep added S-Minor Blocks non-critical functionality, workarounds exist. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues. labels Nov 8, 2021
@Mikaela
Copy link
Contributor

Mikaela commented Nov 8, 2021

Another important point to take into account here is that the feature is currently disabled by default in Synapse (since it's an experimental feature), and afaik not implemented in other homeservers, so there are more severe privacy/data hygiene issues related to the current state of this feature in the ecosystem (namely that even once enabled in a federated room it's very unlikely that all servers will care about it), for which imo the fix is stabilising the feature in the spec (ie fixing the MSC and getting it through the spec process).

Technically the rooms can use ACL and only allow trusted servers, that are known to have it enabled, to join.

Where is the MSC/spec trackable by the way?

@babolivier
Copy link
Contributor

babolivier commented Nov 9, 2021

Technically the rooms can use ACL and only allow trusted servers, that are known to have it enabled, to join.

Sure, but it doesn't change the fact that the current situation is temporary and the feature is still experimental, and things like client support etc is inexistant.

Where is the MSC/spec trackable by the way?

matrix-org/matrix-spec-proposals#1763

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Message-Retention-Policies Issues related to Synapse's support of MSC1763 (message retention policies) S-Minor Blocks non-critical functionality, workarounds exist. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues.
Projects
None yet
Development

No branches or pull requests

4 participants