Skip to content

Conversation

zecakeh
Copy link
Collaborator

@zecakeh zecakeh commented Jul 17, 2025

It is going away in MSC4291, which should be accepted next week. Removing it now makes sure that no one uses it.

It is going away in MSC4291, which should be accepted next week.
Removing it now makes sure that no one uses it.

Signed-off-by: Kévin Commaille <[email protected]>
@zecakeh zecakeh requested a review from a team as a code owner July 17, 2025 08:03
@zecakeh zecakeh requested review from Hywan and removed request for a team July 17, 2025 08:03
Signed-off-by: Kévin Commaille <[email protected]>
@codecov
Copy link

codecov bot commented Jul 17, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.85%. Comparing base (39cf8b3) to head (96e20ba).
Report is 3 commits behind head on main.

✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5419      +/-   ##
==========================================
+ Coverage   88.83%   88.85%   +0.02%     
==========================================
  Files         334      334              
  Lines       91072    91070       -2     
  Branches    91072    91070       -2     
==========================================
+ Hits        80907    80924      +17     
+ Misses       6349     6332      -17     
+ Partials     3816     3814       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Signed-off-by: Kévin Commaille <[email protected]>
@zecakeh zecakeh requested a review from poljar July 17, 2025 10:30
@poljar poljar enabled auto-merge (rebase) July 17, 2025 10:35
Copy link
Contributor

@poljar poljar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks.

@poljar poljar merged commit 1a9e5b9 into matrix-org:main Jul 17, 2025
44 checks passed
@zecakeh zecakeh deleted the rm-predecessor-event-id branch July 17, 2025 11:25
@martinetd
Copy link
Contributor

Hello

https://matrix.org/blog/2025/07/security-predisclosure/ was published a while ago with this statement:

Client developers should review MSC4291: “Room IDs as hashes of the create event” to ensure their clients can accept the new proposed format of room IDs, and no longer expects content.predecessor.event_id in m.room.create events.

Does that mean we need this PR in, or is not using the field explicitly in the client code enough?
(I'm asking because it's not part of 0.13, and I'd have expected this to be backported or a new release tagged if it was required, so I assume I don't actually have anything to do...)

Details should be published on Monday so happy to wait a bit longer if you don't want to discuss this yet

@poljar
Copy link
Contributor

poljar commented Aug 7, 2025

(I'm asking because it's not part of 0.13, and I'd have expected this to be backported or a new release tagged if it was required, so I assume I don't actually have anything to do...)

A release which includes this PR will happen sooner or later. I don't know the details of the vulnerability myself either, nor how important the removed event_id is.

@zecakeh
Copy link
Collaborator Author

zecakeh commented Aug 7, 2025

The predecessor will fail to deserialize in matrix-sdk 0.13 if the event_id is missing. So the impact will just be that you don't know that the room was upgraded from another room.

@zecakeh
Copy link
Collaborator Author

zecakeh commented Aug 7, 2025

Note that the removal of the event_id has no direct link to a vulnerability. It's just a consequence of changing the format of the room ID (as explained in MSC4291).

@martinetd
Copy link
Contributor

Note that the removal of the event_id has no direct link to a vulnerability. It's just a consequence of changing the format of the room ID (as explained in MSC4291).

Thanks for clarifying this isn't needed right away.

I (very roughly) understand the crux of the room confusion problem described in that MSC, but I honestly do not understand the communications around it...
More time was given for client developers to fix their clients (I think?), and I suppose I won't be able to join any room upgraded to v12 with my old client (unless it doesn't care as long as my homeserver is upgraded and handles it? But in this case what's with the delay, for the room upgrading procedure itself?)
So, I just don't see what I'm supposed to do with all of this as a sdk user as this is the only commit I've found referencing the MSC.

Anyway, it's getting late, and my client is pretty much just a wrapper around the sdk so I guess I'll figure it out after upgrading synapse on Monday and my two users will probably forgive me...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants