MSC3757: Restricting who can overwrite a state event

[andybalaam]

Rendered

Implementations:

• https://github.com/element-hq/synapse/pull/17513
• https://github.com/matrix-org/matrix-rust-sdk/pull/3836
• https://github.com/ruma/ruma/pull/1885

Written by @ara4n , with contributions from @Johennes and @andybalaam .

Shepherd: @AndrewFerr

FCP tickyboxes

[gleachkr]

Here's a small worry - I was hoping that this might resolve one of the potential issues listed for MSC3574, basically that anyone can overwrite an m.space.child event sent by someone else. But if the state_key is used semantically to indicate ownership, then this is going to be incompatible with any state event type where the state_key is used semantically to indicate something else (as in the case of m.space.child).

[ara4n]

Does anyone actually object to this MSC? Its absence is currently screwing over my attempt to track multiple beacons via MSC3489 from a given user :(

[toger5]

I would like to add another solution. State keys could be of type: string | string[] Where state_key: string = "hello" is a shortcut for state_key: string[] = ["hello"]. Our access rule would become:

If the events the state_key contains a user id that does not match the sender, reject.

• unless the sender's powerlevel is greater than the event type's required power level.

Treating state keys as a path also mirrors a nice mental model to split bigger state events into objects build up by multiple state events so we would not need a state change request. Also I am worried that a state subkey (like in MSC3760 could quickly get its own rules like: "only allow to chnage by the device with that deviceId" and we might need a state-sub-sub-key. Making the state key a path solves this. It would be trivial to add rules like [$userId, $deviceId, something] can only be edited by the defined device...

Here is a visualization of how the state path can be thought of as a path to a subsection of an object.

This array of stat events

[
{
 "state_key": "a",
 "content": {
  "my_property": "a"
 }
},

{
 "state_key": ["b", "a"],
 "content":{"test":1}
},

{
 "state_key": ["b","@myId:domain.com"],
 "content": {
   "data": "data",
   "note": "this object owned by the user: @myId:domain.com"
  }
},

{
 "state_key": ["b", "@myId:domain.com", "subElement"],
 "content": {
   "note": "also owned by: @myId:domain.com",
   "editProps": "but can be edited independently as a sub state object"
  }
}
]

Can be thought of/parsed as this room state object.

{
 "a": {
  "content":{"my_property": "a"}
 },
 "b": {
  "@myId:domain.com":{
   "content": {"other": "data"},
   "subElement"{
    "content":{
        "note": "also owned by: @myId:domain.com",
        "editProps": "but can be edited independently as a sub state object"
    }
   }
  },
  "a":{
   "content":{"test":1}
  }
 }
}

A concrete example for b could be: ["ephemeralLocations",$userId], ["presistentLocations",$userId] Or b could be the user Id and we inside each user we have a list of devices for the call member event example

One can think of the state event type as the first element in the array/path. state_property_path = [$state_event_type] + state_key It still makes sense however, that the type is its own field since we always need a type. Which is better then requireing the key array to be of min length one.

Additinally the state_key = "" would be equivalent to state_key=[] which makes much more sense thinking of it as a property path: path = [$state_event_type] + state_key = [$state_event_type]

[richvdh]

I'm not necessarily against this proposal per-se, but I remain very worried that we have no practical means of deleting state from a room, and it seems like most applications that would make use of this proposal would tend towards large amounts of state being added to a room which then has to hang around forever.

(https://github.com/matrix-org/matrix-spec-proposals/pull/3901 is a proposal to deal with deleting state, but it needs implementation work)

[AndrewFerr]

I remain very worried that we have no practical means of deleting state from a room, and it seems like most applications that would make use of this proposal would tend towards large amounts of state being added to a room which then has to hang around forever.

What could mitigate this somewhat is #4140 working together with #3901 to allow for state events that expire into obsoleted events. That would allow for applications of this MSC to set state that would eventually be marked for cleanup/deletion.

[andybalaam]

I would like to add another solution. State keys could be of type: string | string[] Where state_key: string = "hello" is a shortcut for state_key: string[] = ["hello"]. Our access rule would become:

I would buy into this if others felt it was better. I think it would make @turt2live happier?

I feel a little concerned about the ambiguity of whether "" = [""] or [] but that is a minor detail.

[erikjohnston]

I would like to add another solution. State keys could be of type: string | string[] Where state_key: string = "hello" is a shortcut for state_key: string[] = ["hello"]. Our access rule would become:

I would buy into this if others felt it was better. I think it would make @turt2live happier?

I feel a little concerned about the ambiguity of whether "" = [""] or [] but that is a minor detail.

I think changing the format of state_key is a pretty big change, and personally not something I would want to block this proposal on (e.g. we'd need to update DB tables in Synapse, update state resolution code, etc etc)

[erikjohnston]

It feels like this is a fairly useful thing to support, with no great alternative way of doing it in the short/medium term.

@mscbot fcp merge

[mscbot]

Team member @mscbot has proposed to merge this. The next step is review by the rest of the tagged people:

• [ ] @clokep
• [ ] @dbkr
• [ ] @uhoreg
• [ ] @turt2live
• [ ] @ara4n
• [ ] @anoadragon453
• [ ] @richvdh
• [ ] @tulir
• [x] @erikjohnston
• [ ] @KitsuneRal

Concerns:

• ~~Does not work with all MXIDs~~
• ~~changes to auth rules are unclear~~
• ~~issue of state event proliferation requires discussion~~
• ~~Alternatives section is missing some alternatives.~~
• ~~Specific alternative of top-level access control is not sufficiently discounted.~~
• Introduction does not sufficiently list benefiting MSCs/features.
• Insufficient implementation - Complement tests are required for this MSC.
• Alternatives insufficient explored.

Once at least 75% of reviewers approve (and there are no outstanding concerns), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for information about what commands tagged team members can give me.

[richvdh]

@mscbot concern changes to auth rules are unclear @mscbot concern issue of state event proliferation requires discussion

[clokep]

@mscbot concern Does not work with all MXIDs

Reposting due to matrix-org/mscbot-python#26 not yet being deployed.

[turt2live]

@mscbot concern Introduction does not sufficiently list benefiting MSCs/features. @mscbot concern Insufficient implementation - Complement tests are required for this MSC. @mscbot concern Alternatives section is missing some alternatives. @mscbot concern Specific alternative of top-level access control is not sufficiently discounted.

[AndrewFerr]

@mscbot concern Insufficient implementation - Complement tests are required for this MSC.

Complement tests are added by https://github.com/matrix-org/complement/pull/733.

[AndrewFerr]

There've been some issues/concerns raised about the state key string-packing:

• https://github.com/matrix-org/matrix-spec-proposals/pull/3757#discussion_r1103877363 (ownership semantics may benefit non-state events, too)
• https://github.com/matrix-org/matrix-spec-proposals/pull/3757#discussion_r1718989256 (danger of using an underscore as the string-packing delimiter)
• https://github.com/matrix-org/matrix-spec-proposals/pull/3757#pullrequestreview-2240327606 (danger of long post-packed state keys)

Adding a top-level event field to signify the "owner" of an event would help solve these issues, and is akin to the idea of state sub-keys proposed by MSC3760.

However, that approach was dismissed by https://github.com/matrix-org/matrix-spec-proposals/pull/3757/files#r848454293 & https://github.com/matrix-org/matrix-spec-proposals/pull/3757#issuecomment-2291031019 due to the friction of redefining state events & state resolution.

So it seems like either option (string-packed state keys vs a top-level owner field) has problems. Should the goal be to decide which option is less problematic, or to arrive at a compromise / a different option?

[mscbot]

That concern has already been raised.

[clokep]

@mscbot resolve Does not work with all MXIDs

@mscbot concern Alternatives insufficient explored.

[mscbot]

That concern has already been raised.

[AndrewFerr]

@mscbot concern Alternatives insufficient explored.

deba3b82..e16482ab re-examines the alternative of the m.peer_unwritable flag.

[turt2live]

This proposal requires re-review from several SCT members and adjacent folks. The priority is a bit unclear to me, but that is a different problem (see room).

[clokep]

On the Apache voting scale, I'm at a -1.0 for how the MSC is currently written. A stronger rationale for why this is needed now and strong discounting of a top-level ACL structure would change this to a -0.9 (sorry, I'm just not in favour of string packing here).

This is still the crux of this proposal and I'm in agreement with @turt2live that I'm a -1 for the string packing.

[AndrewFerr]

Maybe it's worth having a new MSC to explore one of the top-level field alternatives:

• Event ownership flag
• Owning user ID field

[clokep]

Maybe it's worth having a new MSC to explore one of the top-level field alternatives:

* [Event ownership flag](https://github.com/matrix-org/matrix-spec-proposals/blob/andybalaam/owner-state-events/proposals/3757-restricting-who-can-overwrite-a-state-event.md?rgh-link-date=2024-12-10T20%3A16%3A47Z#event-ownership-flag)

* [Owning user ID field](https://github.com/matrix-org/matrix-spec-proposals/blob/andybalaam/owner-state-events/proposals/3757-restricting-who-can-overwrite-a-state-event.md?rgh-link-date=2024-12-10T20%3A16%3A47Z#owning-user-id-field)

Maybe MSC3760 or MSC3779 was supposed to be that? (3760 appears to be very similar to this.)

[andybalaam]

Maybe MSC3760 or MSC3779 was supposed to be that? (3760 appears to be very similar to this.)

Yes, MSC3760 was an alternative without string-packing. MSC3779 is different: it is intended to allow low-power users to create these state events.

[richvdh]

@mscbot resolve concern changes to auth rules are unclear

[mscbot]

Unknown concern 'concern changes to auth rules are unclear'.

[richvdh]

@mscbot resolve changes to auth rules are unclear @mscbot resolve issue of state event proliferation requires discussion

[turt2live]

Concerns-I-have-raised update:

@mscbot resolve Alternatives section is missing some alternatives. @mscbot resolve Specific alternative of top-level access control is not sufficiently discounted.

Introduction does not sufficiently list benefiting MSCs/features.

The introduction currently relies on location sharing to drive it, which is a deprioritized feature at the spec level. I highly suggest adding the VoIP impact to the introduction to naturally drive this MSC up the list.

Insufficient implementation - Complement tests are required for this MSC.

This still appears to be the case.

[AndrewFerr]

Introduction does not sufficiently list benefiting MSCs/features.

The introduction currently relies on location sharing to drive it, which is a deprioritized feature at the spec level. I highly suggest adding the VoIP impact to the introduction to naturally drive this MSC up the list.

2a77266

Insufficient implementation - Complement tests are required for this MSC.

This still appears to be the case.

Is https://github.com/matrix-org/complement/pull/733 not sufficient?