Specify basic validation for federation membership endpoints by tulir · Pull Request #2284 · matrix-org/matrix-spec

tulir · 2025-12-21T13:32:08Z

Preview: https://pr2284--matrix-spec-previews.netlify.app

Signed-off-by: Tulir Asokan <tulir@maunium.net>

tulir · 2025-12-21T13:43:48Z

                  }
        "400":
          description: |-
+            The `M_INVALID_PARAM` error code is used to indicate one or more of the following:


This error code is made up. Synapse seems to use M_UNKNOWN for most of the checks, but writing that into the spec wouldn't be very nice. It can be reworded to not use any specific error code if necessary

Wouldn't M_BAD_JSON make sense? Hammerhead uses this

Request contained valid JSON, but it was malformed in some way, e.g. missing required keys, invalid values for keys.

yeah, we do tend to use M_BAD_JSON for other things like this, for better or worse.

richvdh

Thanks very much for taking this on. It's certainly a valuable improvement.

I'm a bit concerned that putting the validation requirements way down in the "error codes" section buries the lede a bit, and that people aren't going to notice it. I'd be more inclined to put it in the endpoint description.

Similarly, I'd phrase it as "Servers MUST validate the following: ..." rather than "[error code] is used in the following situations: ...".

tulir · 2026-01-06T19:17:29Z

I'm a bit concerned that putting the validation requirements way down in the "error codes" section buries the lede a bit, and that people aren't going to notice it. I'd be more inclined to put it in the endpoint description.

Similarly, I'd phrase it as "Servers MUST validate the following: ..." rather than "[error code] is used in the following situations: ...".

The main reason for both points was that invites already had invite_room_state validation in the response spec rather than the endpoint description. Should I move that stuff as well?

richvdh · 2026-01-13T16:48:32Z

I'm a bit concerned that putting the validation requirements way down in the "error codes" section buries the lede a bit, and that people aren't going to notice it. I'd be more inclined to put it in the endpoint description.
Similarly, I'd phrase it as "Servers MUST validate the following: ..." rather than "[error code] is used in the following situations: ...".

The main reason for both points was that invites already had invite_room_state validation in the response spec rather than the endpoint description. Should I move that stuff as well?

Ideally, I think so, yes, though at least in the case of invite_room_state we have a note on the request param that there is validation that must be done.

tulir · 2026-02-07T18:57:59Z

Added the validation for GHSA-m5p2-vccg-8c9v, moved all of it from the response schemas to the request description and switched to use M_MISSING_PARAM for all errors

richvdh · 2026-02-17T20:46:17Z

+        Servers MUST apply certain validation to ensure they don't accidentally sign non-invite
+        events from a malicious server. The `M_MISSING_PARAM` error code is used to indicate one
+        or more of the following:
+
+        * The invite event fails a [signature check](/server-server-api/#validating-hashes-and-signatures-on-received-events).
+        * The event type is not `m.room.member`.
+        * The `membership` field inside the event content is not `invite`.
+        * The event sender is not a user ID on the origin server.
+        * The `state_key` is not a user ID on the receiving server.
+
+        The `M_MISSING_PARAM` error code is also used to indicate one or more of the following
+        problems in the `invite_room_state` field:
+
+        * The `m.room.create` event is missing from `invite_room_state`.
+        * One or more entries in `invite_room_state` are not formatted according
+          to the room's version.
+        * One or more events fails a [signature check](/server-server-api/#validating-hashes-and-signatures-on-received-events).
+        * One or more events does not reside in the same room as the invite.
+          Note: Some room versions may require calculating the room ID for an
+          event rather than relying on the presence of `room_id`.


I feel like we've gone in the wrong direction here in terms of the error code. M_MISSING_PARAM seems like a terrible choice for param that isn't missing.

Can we update both halves to use something more sensible like M_INVALID_PARAM? Or just leave the precise error code unspecified here?

If we want it to match implementations, probably just need to leave it unspecified. I guess it might be fine to specify something even if implementations don't match right now? Servers shouldn't be trying to catch and handle these errors anyway

Servers shouldn't be trying to catch and handle these errors anyway

Except that, in the section about response codes, the spec does mandate trying to catch these errors and handle them differently, at least in the case of /v2/invite/{roomId}/{eventId}: it says that errors should be passed through to the client, except for M_MISSING_PARAM which should be turned into a 500 error.

Ugh, I don't really know how best to proceed here. Certainly the best solution would be an MSC to clarify things, but that's a bunch of time and effort.

Maybe we can just get away with saying something like "errors which indicate a faulty request such as M_MISSING_PARAM and M_INVALID_PARAM should be turned into 500 errors", and then leaving the exact error code to be returned under specific circumstances unspecified for now?

(or, leave the "response codes" section just talking about M_MISSING_PARAM, and if we decide we need better error handling, MSC the response codes later?)

I made that case say both M_MISSING_PARAM and M_INVALID_PARAM should be hidden from clients, I guess the wording could be even more relaxed rather than specifying just 2 error codes

richvdh · 2026-02-17T20:51:59Z

            The error should be passed through to clients so that they
            may give better feedback to users.

+            If `M_MISSING_PARAM` is returned and the request is associated with a


The description for this endpoint specifies M_INVALID_PARAM for validation failures. Let's make sure they are consistent.

Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>

richvdh

lgtm. thanks.

Specify basic validation for federation membership endpoints

fe6c97f

Signed-off-by: Tulir Asokan <tulir@maunium.net>

tulir requested a review from a team as a code owner December 21, 2025 13:32

tulir commented Dec 21, 2025

View reviewed changes

richvdh requested changes Jan 6, 2026

View reviewed changes

Comment thread data/api/server-server/invites-v1.yaml Outdated

tulir added 4 commits February 7, 2026 20:53

Move validation to endpoint description

a7721b7

Specify more validation to cover GHSA-m5p2-vccg-8c9v

ec80512

Merge branch 'main' into tulir/federation-membership-validation

1da898f

Update changelog

aa9a203

tulir requested a review from richvdh February 10, 2026 11:22

richvdh requested changes Feb 17, 2026

View reviewed changes

tulir and others added 4 commits February 24, 2026 15:17

Don't mandate specific error code

521bf40

Adjust wording so the validation reads nicer

aa3abad

Make example responses match recommendations

8bb5138

Apply suggestion from @richvdh

325891f

Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>

richvdh approved these changes Feb 24, 2026

View reviewed changes

richvdh merged commit 6b6d351 into main Feb 24, 2026
13 checks passed

richvdh deleted the tulir/federation-membership-validation branch February 24, 2026 13:35

zecakeh mentioned this pull request Feb 25, 2026

Implement Matrix 1.18 changes ruma/ruma#2345

Closed

54 tasks

turt2live mentioned this pull request Jul 8, 2026

Support Matrix 1.18 element-hq/synapse#19932

Open

53 tasks

Uh oh!

Uh oh!

Conversation

tulir commented Dec 21, 2025 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

timedoutuk Dec 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

richvdh left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tulir commented Jan 6, 2026

Uh oh!

richvdh commented Jan 13, 2026

Uh oh!

tulir commented Feb 7, 2026

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

richvdh left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

tulir commented Dec 21, 2025 •

edited by github-actions Bot

Loading

timedoutuk Dec 24, 2025 •

edited

Loading