Skip to content

Conversation

ara4n
Copy link
Member

@ara4n ara4n commented May 20, 2025

Rendered

(Written by Matthew as Matrix project lead (while on the clock as Element CTO) and Kegan as Element staff eng)


SCT Stuff:

FCP tickyboxes

MSC checklist

@turt2live turt2live added proposal A matrix spec change proposal needs-implementation This MSC does not have a qualifying implementation for the SCT to review. The MSC cannot enter FCP. labels May 20, 2025
@turt2live turt2live marked this pull request as draft May 20, 2025 22:15
@turt2live

This comment was marked as resolved.

@turt2live turt2live added proposal-placeholder This label is removed and replaced with `proposal` once the placeholder status is cleared. action-required Just a bright label to differentiate arbitrary proposals. and removed proposal A matrix spec change proposal labels Jul 3, 2025
@github-project-automation github-project-automation bot moved this to Tracking for review in Spec Core Team Workflow Jul 8, 2025
@turt2live turt2live moved this from Tracking for review to Ready for general review in Spec Core Team Workflow Jul 8, 2025
Co-authored-by: Travis Ralston <[email protected]>
Co-authored-by: Alexey Rusakov <[email protected]>
Co-authored-by: Richard van der Hoff <[email protected]>
Co-authored-by: Matthias Ahouansou <[email protected]>
@tulir tulir changed the title [WIP] Placeholder MSC4291: Room IDs as hashes of the create event Jul 10, 2025
@tulir tulir marked this pull request as ready for review July 10, 2025 16:02
@tulir tulir added requires-room-version An idea which will require a bump in room version proposal A matrix spec change proposal s2s Server-to-Server API (federation) room-spec Something to do with the room version specifications unassigned-room-version Remove this label when things get versioned. kind:maintenance MSC which clarifies/updates existing spec and removed needs-implementation This MSC does not have a qualifying implementation for the SCT to review. The MSC cannot enter FCP. proposal-placeholder This label is removed and replaced with `proposal` once the placeholder status is cleared. action-required Just a bright label to differentiate arbitrary proposals. labels Jul 10, 2025
@turt2live turt2live added the maybe v12? candidate for room version 12 label Jul 10, 2025
Copy link
Member

@turt2live turt2live Jul 10, 2025

Choose a reason for hiding this comment

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

Implementation requirements:

Some clients to test/verify (incomplete list):

  • matrix.to web
  • Element Web (/Desktop)
  • Legacy Element Android
  • Legacy Element iOS
  • matrix-rust-sdk
    • Element X Android
    • Element X iOS
    • Fractal
  • Cinny
  • Nheko
  • Quotient/Quaternion
  • NeoChat
  • Rory&::LibMatrix
  • matrix-bot-sdk (both Element fork and upstream; covers most moderation tooling/bots)
  • Draupnir
  • Hydrogen

Copy link
Contributor

@Gnuxie Gnuxie Jul 11, 2025

Choose a reason for hiding this comment

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

Draupnir will crash, and please do not assume that ticking off the matrix-bot-sdk will cover us in future. We have our own infrastructure for parsing matrix events and matrix identifiers.

  • https://github.com/the-draupnir-project/matrix-basic-types includes parsing and type brands for matrix identifiers (potentially illegally in the case of room identifiers, but we do so because it is important for us to be able to identify the origin of rooms).
  • https://github.com/Gnuxie/matrix-protection-suite/ parses events, and will crash if room_id is removed from the top level of events. Not sure whether that specifically is illegal or not. We have a tolerant strategy for handling other fields that don't match the schema (including all content), but this isn't the case for room_id and event_id.

Copy link
Contributor

Choose a reason for hiding this comment

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

Fractal doesn't crash with the changes in this MSC.

Copy link
Member

Choose a reason for hiding this comment

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

matrix.to appears to overly validate the room ID:
image

Choose a reason for hiding this comment

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

@turt2live turt2live added the needs-implementation This MSC does not have a qualifying implementation for the SCT to review. The MSC cannot enter FCP. label Jul 10, 2025
@turt2live
Copy link
Member

turt2live commented Jul 10, 2025

MSCs proposed for Final Comment Period (FCP) should meet the requirements outlined in the checklist prior to being accepted into the spec. This checklist is a bit long, but aims to reduce the number of follow-on MSCs after a feature lands.

SCT members: please check off things you check for, and raise a concern against FCP if the checklist is incomplete. If an item doesn't apply, prefer to check it rather than remove it. Unchecking items is encouraged where applicable.

Checklist:

  • Are appropriate implementation(s) specified in the MSC’s PR description?
  • Are all MSCs that this MSC depends on already accepted?
  • For each new endpoint that is introduced:
    • Have authentication requirements been specified?
    • Have rate-limiting requirements been specified?
    • Have guest access requirements been specified?
    • Are error responses specified?
      • Does each error case have a specified errcode (e.g. M_FORBIDDEN) and HTTP status code?
        • If a new errcode is introduced, is it clear that it is new?
  • Will the MSC require a new room version, and if so, has that been made clear?
    • Is the reason for a new room version clearly stated? For example, modifying the set of redacted fields changes how event IDs are calculated, thus requiring a new room version.
  • Are backwards-compatibility concerns appropriately addressed?
  • Are the endpoint conventions honoured?
    • Do HTTP endpoints use_underscores_like_this?
    • Will the endpoint return unbounded data? If so, has pagination been considered?
    • If the endpoint utilises pagination, is it consistent with the appendices?
  • An introduction exists and clearly outlines the problem being solved. Ideally, the first paragraph should be understandable by a non-technical audience.
  • All outstanding threads are resolved
    • All feedback is incorporated into the proposal text itself, either as a fix or noted as an alternative
  • While the exact sections do not need to be present, the details implied by the proposal template are covered. Namely:
    • Introduction
    • Proposal text
    • Potential issues
    • Alternatives
    • Dependencies
  • Stable identifiers are used throughout the proposal, except for the unstable prefix section
    • Unstable prefixes consider the awkward accepted-but-not-merged state
    • Chosen unstable prefixes do not pollute any global namespace (use “org.matrix.mscXXXX”, not “org.matrix”).
  • Changes have applicable Sign Off from all authors/editors/contributors
  • There is a dedicated "Security Considerations" section which detail any possible attacks/vulnerabilities this proposal may introduce, even if this is "None.". See RFC3552 for things to think about, but in particular pay attention to the OWASP Top Ten.

Comment on lines +81 to +88
It is also desirable to know the creator's server name without having to join and get the full create event
for Trust & Safety purposes. The create event in invites can be spoofed, as can the room ID, but spoofing the
room ID makes the invite unusable whereas spoofing the create event does not. We could rely on the `sender`
instead, but for this to be effective it would be important for servers to use the `sender` domain for routing
purposes, such that if the `sender` was invalid it would cause the invite to be unusable. On the contrary, users
unfamiliar with Matrix may see a room ID like `!OGEhHVWSdvArJzumhm:matrix.org` and think that `matrix.org` hosts/moderates/
is somehow responsible for the room in some way. By dropping the domain, we clearly express that the creator domain
may not be responsible for the contents of the room.

This comment was marked as resolved.

@turt2live
Copy link
Member

(FCP is actually not finished due to outstanding comments)

@turt2live turt2live added final-comment-period This MSC has entered a final comment period in interest to approval, postpone, or delete in 5 days. and removed finished-final-comment-period labels Jul 21, 2025
@turt2live
Copy link
Member

Review feedback from the last several days, including from various rooms, has been addressed and as such the FCP is now finished.

@turt2live turt2live added finished-final-comment-period and removed final-comment-period This MSC has entered a final comment period in interest to approval, postpone, or delete in 5 days. labels Jul 22, 2025
@turt2live turt2live merged commit b1f4ea8 into main Jul 22, 2025
1 check passed
@turt2live turt2live added spec-pr-missing Proposal has been implemented and is being used in the wild but hasn't yet been added to the spec and removed finished-final-comment-period labels Jul 22, 2025
@turt2live turt2live moved this from In FCP to Requires spec writing in Spec Core Team Workflow Jul 22, 2025
@turt2live turt2live self-assigned this Jul 26, 2025
@turt2live
Copy link
Member

Spec PR is a WIP out of band by me.

@turt2live
Copy link
Member

Spec PR is now in review (and open) out of band.

@turt2live turt2live added spec-pr-in-review A proposal which has been PR'd against the spec and is in review and removed spec-pr-missing Proposal has been implemented and is being used in the wild but hasn't yet been added to the spec unassigned-room-version Remove this label when things get versioned. labels Jul 28, 2025
@turt2live turt2live moved this from Requires spec writing to Requires spec PR review in Spec Core Team Workflow Jul 28, 2025
@turt2live
Copy link
Member

For clarity: this MSC has been assigned to room version 12.

@turt2live turt2live added merged A proposal whose PR has merged into the spec! and removed spec-pr-in-review A proposal which has been PR'd against the spec and is in review labels Aug 14, 2025
@turt2live turt2live moved this from Requires spec PR review to Merged in Spec Core Team Workflow Aug 14, 2025
@turt2live
Copy link
Member

Merged 🎉

aichafellouss added a commit to aichafellouss/Draupnir that referenced this pull request Oct 19, 2025
- Updated to matrix-basic-types 1.4.0 which changes
  the regex validating room ids.

- Changed the package override so that all dependencies
  use matrix-basic-types 1.4.0, including the matrix-protection-suite.

- Removed code that tries to store details about discovered rooms in
  the room takdedown protection. These were unreliable for so many
  reasons and also are now broken given the room origin cannot be
  extracted from the room id. Details for why this is can be found in
  the reviews of
  matrix-org/matrix-spec-proposals#4291.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind:core MSC which is critical to the protocol's success maybe v12? candidate for room version 12 merged A proposal whose PR has merged into the spec! proposal A matrix spec change proposal requires-room-version An idea which will require a bump in room version room-spec Something to do with the room version specifications s2s Server-to-Server API (federation)

Projects

Status: Merged

Development

Successfully merging this pull request may close these issues.