mirror of
https://github.com/element-hq/synapse.git
synced 2026-10-23 02:13:29 +00:00
c85b1fef9c0f9e49b3b5034391c4ea0d2fa3b3ce
4217
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Andrew Morgan
|
c9b61fc2a3 | Stop real threadpool for test DB pool before replacing it with threadless test implementation (#20272) | ||
|
Olivier 'reivilibreandEric Eastwood
|
d1874cdc37 |
Prevent MSC4354 Sticky Events being sticky when they were redacted prior to persistence. (#20232)
Part of: MSC4354, Part of: https://github.com/element-hq/synapse/issues/19641 This PR persists events in redacted form when they already have valid redactions. That's not directly a sticky events-specific change (and I feel it makes sense to redact immediately before persistence when we already have the redaction). This causes a nice effect which is that the event's stickiness is lost immediately before persistence, because the `msc4354_sticky` field is redacted by the redaction algorithm. --------- Signed-off-by: Olivier 'reivilibre <oliverw@matrix.org> Co-authored-by: Eric Eastwood <erice@element.io> |
||
|
Paul ChobertandOlivier 'reivilibre
|
db618e276d |
Fix appservice ephemeral read receipts being permanently skipped on large receipt bursts (#20108)
Fixes https://github.com/element-hq/synapse/issues/20096. Part of https://github.com/element-hq/synapse/issues/18118 The spec requires read receipts to be delivered to interested application services ([Pushing ephemeral data](https://spec.matrix.org/v1.19/application-service-api/#pushing-ephemeral-data)): > If the `receive_ephemeral` settings is enabled in the registration file, homeservers MUST send ephemeral data that is relevant to the application service via the transaction API, using the `ephemeral` property of the request's body. > > `m.receipt`: MUST be sent to the application service under the same rules as regular events, meaning that the application service must have registered interest in the room itself, or in a user that is in the room. Ephemeral data delivery to application services was added in Matrix v1.13 from [MSC2409](https://github.com/matrix-org/matrix-spec-proposals/pull/2409) (see the [v1.13 changelog](https://spec.matrix.org/v1.19/changelog/v1.13/#application-service-api)). ## Problem When more than 100 read receipts are covered by a single receipt stream update, only the last 100 are sent to appservices. The rest is permanently skipped. ## Proposed Fix - Looping over the receipts in batches of 100 items and updating the cursor on every iteration - Keep the fast-forward behavior for new appservices as intended in https://github.com/matrix-org/synapse/pull/8744 --------- Co-authored-by: Olivier 'reivilibre <oliverw@element.io> |
||
|
Ankit Jha
|
18c10b8075 |
Fix 500 error when user-interactive auth requests send a null or non-object auth (#20274)
Fixes #15871 ### What was broken Sending `{"auth": null}` to any endpoint that requires user-interactive auth (for example `POST /keys/device_signing/upload`, `DELETE /devices/{deviceId}` or `POST /register`) returned a 500 instead of the usual 401 with the available flows. A non-object `auth` on endpoints that do not validate the body with a model (such as `/register`) could also 500, e.g. `{"auth": ["session"]}`. ### Root cause `AuthHandler.check_ui_auth` did `clientdict.pop("auth", {})`, so the `{}` default only applies when the key is missing. An explicit `null` came through as `None` and the following `"session" in authdict` raised `TypeError`. `get_session_id` had the same assumption that `auth` is a dict. ### Pull Request Checklist * [x] Pull request is based on the develop branch * [x] Pull request includes a [changelog file](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#changelog). * [x] [Code style](https://element-hq.github.io/synapse/latest/code_style.html) is correct (run the [linters](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#run-the-linters)) --------- Signed-off-by: Ankit Jha <ankit.jha@tradomate.one> |
||
|
Ankit Jha
|
929e3524d1 |
fix: reject user creation via the admin API when delegating to MAS (#20241)
Signed-off-by: Ankit Jha <jhaankit373@gmail.com> Signed-off-by: Ankit Jha <ankit.jha@tradomate.one> |
||
|
Erik Johnston
|
7630492e90 |
Add metrics for wait_for_stream_token timeouts per lagging stream (#20095)
Add a metric for the number of times we see a stall due to being given a future token. This would have caught https://github.com/element-hq/synapse/issues/20080, where lots of sync streams got stuck due to a stream not getting replicated correctly. |
||
|
Johannes MarbachandAndrew Morgan
|
b8bd6f93a1 |
Add push rules for MSC4075: MatrixRTC invites and notifications (#20227)
Co-authored-by: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com> |
||
|
|
e863ce7fab |
Fire deferreds from Rust futures without taking the GIL (#20252)
Tokio tasks currently complete by taking the GIL and calling `reactor.callFromThread`. Taking the GIL may block for a period of time, and we do not want that to happen on the tokio reactor threads (as that can block other work from happening). To avoid this, we instead add a work queue that we can add to from Rust without taking the GIL, which is drained by the reactor. We signal to the Twisted reactor that it should wake up by using a unix socket pair, which is exactly how `reactor.callFromThread` works. --- The self-pipe trick is basically where you create a pair of unix sockets connected to each other. One end is added to the reactor so the reactor is woken up when there are bytes to read, and when another thread needs to wake up the reactor it just needs to write a byte into the other unix socket. --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com> Co-authored-by: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com> |
||
|
Jason Robinson
|
b0d44f9352 |
Fixes to the profile update stream when a user leaves a room (#20203)
This pull request fixes a few issues with the profile update stream, when a user leaves a room. This is basically just mimicking what we already had for when someone leaves a room, and they no longer share any rooms, but in reverse - ie when we leave a room, and no longer share rooms with some users. This was missed in the implementation when adding the profile update stream for legacy and sliding sync. [Fix missing profile update stream rows on user leaving room](https://github.com/element-hq/synapse/commit/e953322226731a99718ebe13139b9c5f05ae4b62) When a user leaves a room, profile update stream rows are generated with the `LEFT_ROOM` action for each user in the room that no longer shares a room with the user who left the room. This also needs to happen in reverse. The user who left the room needs to have a profile update stream row for each user they no longer share a room with. If we don't do this, clients may keep stale data around even after they don't share a room with a user, which may mean they don't know when to refetch profiles after re-joining a room with the stale profile data user. [Clear out old profile update stream rows when leaving a room](https://github.com/element-hq/synapse/commit/e7642bf141b098c1eefaf74655e5e7e18415500b) When we leave a room, ensure profile update stream rows are cleared out for every user we no longer share a room with. This is the same as what happens when someone else leaves a room, but in reverse. The only remaining profile update stream row should be the `LEFT_ROOM` action. ### Pull Request Checklist <!-- Please read https://element-hq.github.io/synapse/latest/development/contributing_guide.html before submitting your pull request --> * [x] Pull request is based on the develop branch * [x] Pull request includes a [changelog file](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#changelog). The entry should: - Be a short description of your change which makes sense to users. "Fixed a bug that prevented receiving messages from other servers." instead of "Moved X method from `EventStore` to `EventWorkerStore`.". - Use markdown where necessary, mostly for `code blocks`. - End with either a period (.) or an exclamation mark (!). - Start with a capital letter. - Feel free to credit yourself, by adding a sentence "Contributed by @github_username." or "Contributed by [Your Name]." to the end of the entry. * [x] [Code style](https://element-hq.github.io/synapse/latest/code_style.html) is correct (run the [linters](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#run-the-linters)) |
||
|
Will Hunt
|
b29c3c6764 |
Omit og:image entirely if the media is quarantined when handling /preview_url (#20211)
This is to prevent the case where URL previews return a MXC that immediately 404s because the media in question has been quarantined. This is mostly to help implementations which currently show an ugly empty preview due to the MXC being sent down, despite being invalid. ### Pull Request Checklist <!-- Please read https://element-hq.github.io/synapse/latest/development/contributing_guide.html before submitting your pull request --> * [x] Pull request is based on the develop branch * [x] Pull request includes a [changelog file](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#changelog). The entry should: - Be a short description of your change which makes sense to users. "Fixed a bug that prevented receiving messages from other servers." instead of "Moved X method from `EventStore` to `EventWorkerStore`.". - Use markdown where necessary, mostly for `code blocks`. - End with either a period (.) or an exclamation mark (!). - Start with a capital letter. - Feel free to credit yourself, by adding a sentence "Contributed by @github_username." or "Contributed by [Your Name]." to the end of the entry. * [ ] [Code style](https://element-hq.github.io/synapse/latest/code_style.html) is correct (run the [linters](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#run-the-linters)) |
||
|
Olivier 'reivilibre
|
62cb420a0e |
Send all of a room's MSC4354 Sticky Events down oldschool /sync when a user joins it. (#20233)
Part of: MSC4354 whose experimental feature tracking issue is https://github.com/element-hq/synapse/issues/19409 Part of: #19662 --------- Signed-off-by: Olivier 'reivilibre <oliverw@matrix.org> |
||
|
|
64f0590705 |
MSC4140: cancel a user's delayed events when their account is deactivated (#20247)
[MSC4140](https://github.com/matrix-org/matrix-spec-proposals/blob/main/proposals/4140-delayed-events-futures.md), "Account deactivation": > when an account is deactivated, the homeserver MUST cancel that account's delayed events which have not yet been added to a room's event DAG. These cancelled records MAY be removed immediately, including their stored event content, as an exception to the usual finalised-record retention policy. Deactivation currently leaves the user's scheduled delayed events, including their content, in place, and they are still attempted at their scheduled time: | Delayed event scheduled by the user | What happens after deactivation today | | --- | --- | | Message or state event, fires after deactivation has made the user leave the room | The send fails with a 403 (user not in room) and the record is dropped | | Message or state event, fires before deactivation has made the user leave the room (leaving happens room by room in the background) | The event is sent | | `m.room.member` join for themselves in a public room | The deactivated user re-joins the room | ### What changes - Deactivating an account (client or admin API) removes the user's unsent delayed events as its first step, and re-arms the send timer for whatever is scheduled next. - Deactivation can run on a worker, while the send timer lives on the main process, so the cancellation goes through a new replication request. - A delayed event whose send has already started is left alone, as with a normal cancel: it is already on its way into the DAG, and the send path removes its record itself. Suspension and locking are unchanged. ### After #19038 Today a cancelled delayed event is simply deleted, so this PR deletes the user's records too. #19038 changes cancellation to keep the record and mark it as cancelled ("finalised"), so that clients can look up what happened to a delayed event. Once it has landed, deactivation should probably finalise the user's records as cancelled the same way, rather than delete them. The MSC allows either: it permits removing the records immediately, content included, as an exception to the usual retention of finalised records, which is worth doing at least when the user asks to be erased. ### Pull Request Checklist <!-- Please read https://element-hq.github.io/synapse/latest/development/contributing_guide.html before submitting your pull request --> * [x] Pull request is based on the develop branch * [x] Pull request includes a [changelog file](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#changelog). The entry should: - Be a short description of your change which makes sense to users. "Fixed a bug that prevented receiving messages from other servers." instead of "Moved X method from `EventStore` to `EventWorkerStore`.". - Use markdown where necessary, mostly for `code blocks`. - End with either a period (.) or an exclamation mark (!). - Start with a capital letter. - Feel free to credit yourself, by adding a sentence "Contributed by @github_username." or "Contributed by [Your Name]." to the end of the entry. * [x] [Code style](https://element-hq.github.io/synapse/latest/code_style.html) is correct (run the [linters](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#run-the-linters)) --------- Co-authored-by: Andrew Ferrazzutti <af_0_af@hotmail.com> Co-authored-by: Andrew Ferrazzutti <andrewf@element.io> |
||
|
Paul ChobertandOlivier 'reivilibre'
|
c0b7224e85 |
Reject limit_profile_requests_to_users_who_share_rooms without require_auth_for_profile_requests (#20231)
As I was working on https://github.com/element-hq/synapse/pull/20218, I noticed what seemed to be an illegal configuration of synapse. - `require_auth_for_profile_requests`: blocks profile requests unless authenticated - `limit_profile_requests_to_users_who_share_rooms`: blocks profile requests unless authenticated user share a room with requested user This, I think, should be an illegal config: ``` require_auth_for_profile_requests = false limit_profile_requests_to_users_who_share_rooms = true ``` As of now, with such a config the shared-room check is never applied: a profile can be requested anonymously, and also by an authenticated user who doesn't share a room. ### Pull Request Checklist <!-- Please read https://element-hq.github.io/synapse/latest/development/contributing_guide.html before submitting your pull request --> * [x] Pull request is based on the develop branch * [x] Pull request includes a [changelog file](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#changelog). The entry should: - Be a short description of your change which makes sense to users. "Fixed a bug that prevented receiving messages from other servers." instead of "Moved X method from `EventStore` to `EventWorkerStore`.". - Use markdown where necessary, mostly for `code blocks`. - End with either a period (.) or an exclamation mark (!). - Start with a capital letter. - Feel free to credit yourself, by adding a sentence "Contributed by @github_username." or "Contributed by [Your Name]." to the end of the entry. * [x] [Code style](https://element-hq.github.io/synapse/latest/code_style.html) is correct (run the [linters](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#run-the-linters)) --------- Co-authored-by: Olivier 'reivilibre' <oliverw@element.io> |
||
|
|
fbfa7bcfd1 |
Fix push badge count ignoring a room's notifications when another room's summary is up to date (#20237)
The count of unread notifications is computed in two phases: 1. From the summaries (`event_push_summary`) when those are up to date with the user's last read receipt 2. Otherwise from counting the push actions (`event_push_actions`) after that receipt The problem in this process is that the threads whose summary is up to date are identified with only their `thread_id`. But all main timelines share the same `"main"` thread_id, so as soon as one room has an up-to-date summary, phase 2 skips the main timeline of every room. The fix identifies the up-to-date summaries with their `(room_id, thread_id)` pair. Authored by @sandhose. Found while investigating the `TestThreadedReceipts` Complement flake (#15517, #18537), but it is a bug on its own. ### Pull Request Checklist <!-- Please read https://element-hq.github.io/synapse/latest/development/contributing_guide.html before submitting your pull request --> * [x] Pull request is based on the develop branch * [x] Pull request includes a [changelog file](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#changelog). The entry should: - Be a short description of your change which makes sense to users. "Fixed a bug that prevented receiving messages from other servers." instead of "Moved X method from `EventStore` to `EventWorkerStore`.". - Use markdown where necessary, mostly for `code blocks`. - End with either a period (.) or an exclamation mark (!). - Start with a capital letter. - Feel free to credit yourself, by adding a sentence "Contributed by @github_username." or "Contributed by [Your Name]." to the end of the entry. * [x] [Code style](https://element-hq.github.io/synapse/latest/code_style.html) is correct (run the [linters](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#run-the-linters)) --------- Co-authored-by: Quentin Gliech <quentingliech@gmail.com> Co-authored-by: Devon Hudson <devonhudson@librem.one> |
||
|
Erik Johnston
|
818f2b25bb |
Fix error log when writing a large response. (#20246)
We sometimes see a lot of `ERROR` logs like the following: ``` Closing scope Scope<... master.write_bytes_to_request> which is not the currently-active one None ``` This error is generated by `opentracing.Scope` checking that it is the "active" one. Synapse tracks "active" spans via logcontexts, so this indirectly asserts that the scope is closed in the context it was opened in. However, the producer methods are often called from the reactor and therefore withing the sentinel logcontext, which produces the error above. This specifically happens when the producer tries to write large responses but gets paused, and then later resumes. The fix is to simply use `Span` directly, rather than scopes. `Span` does not perform the checks. I noticed this when deploying #19979, though it is unrelated. |
||
|
NEVIL ANSON DSOUZAandDevon Hudson
|
1d7880b9cb |
Make task scheduler concurrency configurable (#18308) (#20230)
### Summary Resolves #18308. Makes the `TaskScheduler`'s maximum concurrent running tasks configurable via `task_scheduler.max_concurrent_tasks` in the homeserver configuration and defaults it to `2` (reduced from the previous hardcoded limit of `5`). ### Motivation Twisted's `adbapi.ConnectionPool` defaults to 5 database connections (`cp_max=5`). When the `TaskScheduler` previously executed up to 5 tasks concurrently, it could exhaust the entire database connection pool, starving regular API and synchronization requests on smaller homeserver instances. Lowering the default to 2 preserves at least 3 connections for foreground requests while allowing concurrent tasks to progress, and gives administrators the ability to tune the limit according to their host and connection pool sizing. ### Changes - Added `TaskSchedulerConfig` (`task_scheduler.max_concurrent_tasks`) with validation (must be a positive integer) and registered it in `HomeServerConfig`. - Updated `TaskScheduler` to enforce the configured concurrency limit and updated class default to 2. - Updated config documentation and JSON schema. - Added config tests in `tests/config/test_task_scheduler_config.py` and updated unit tests in `tests/util/test_task_scheduler.py`. - Added newsfragment in `changelog.d/18308.feature`. --------- Signed-off-by: nevil06 <nevilansondsouza@gmail.com> Co-authored-by: Devon Hudson <devonhudson@librem.one> |
||
|
Eric Eastwood
|
625b331768 |
MSC4311: Use full PDU's in stripped state (like invite_room_state) over federation and always include m.room.create event (#19723)
### Background This PR was originally just trying to remove the flawed [MSC4311](https://github.com/matrix-org/matrix-spec-proposals/pull/4311) partial implementation as client side API's like `/sync` should still use stripped events. But it turns out we were just re-using the client logic for the federation side and things might break if we didn't include the full `m.room.create` event so this PR now introduces MSC4311 support to use full PDU's in the `invite_room_state`/`knock_room_state` in the federation API's. The flawed implementation was originally introduced in https://github.com/element-hq/synapse/commit/0eb7252a230811e59679cc7e55b92dac26532efc (no PR I assume because part of Hydra security fix) which was part of [Synapse v1.136.0](https://github.com/element-hq/synapse/blob/7530874a1250d6ad975b39582a784c594d29a505/CHANGES.md#synapse-11360-2025-08-12). Spawning from reviewing https://github.com/element-hq/synapse/pull/19722 and noticing that we have [`TestMSC4311FullCreateEventOnStrippedState`](https://github.com/matrix-org/complement/blob/1e2e12eebc1edb27bbf12108ec849a8254b6ddcd/tests/v12_test.go#L1341-L1376) in Complement which already passes even though that test looks [flawed](https://github.com/matrix-org/complement/pull/791#discussion_r3132468346): > I think this test is mixing up what [MSC4311](https://github.com/matrix-org/matrix-spec-proposals/pull/4311) proposes. Perhaps these were changes to the MSC that came after? > > For the client API's like `/sync`, it only proposes that `m.room.create` is a required *stripped* state event. > > For the federation API's, alongside requiring `m.room.create`, it also mandates using the full event PDU format for all events in the `invite_room_state`/`knock_room_state` on `m.room.member` events (in `unsigned`) ### What does this PR do? 1. Always use stripped state for client API's 1. Remove flawed [MSC4311](https://github.com/matrix-org/matrix-spec-proposals/pull/4311) partial implementation (as explained above) 1. Sanitize stripped state when we receive events over federation 1. Use full PDU's when sending `invite_room_state`/`knock_room_state` over federation 1. Validate PDU's and warn when receiving `invite_room_state`/`knock_room_state` over federation 1. In the future, we will strictly validate and reject Complement tests: https://github.com/matrix-org/complement/pull/796 --- Part of https://github.com/element-hq/synapse/issues/19414 |
||
|
Erik Johnston
|
f3ae564877 |
Report each ID generator's current position as a metric (#20097)
When a stream advances in the database but stops being replicated to a process, that process's view of the stream freezes. Requests that wait for it to catch up to a token issued by another worker then time out and return empty responses indefinitely (see #20080), and nothing exported said so. Report `get_current_token` from every ID generator, on every process. The value is comparable between processes, so a stream that has stopped reaching one of them shows up as divergence with no client traffic needed. It is also the position that `wait_for_stream_token` waits on, so its divergence is the failure itself rather than a proxy for it. Being a watermark over gapless runs of persisted IDs, it also catches a single writer of a sharded stream going quiet, which a maximum across writers would hide behind the writers still being replicated. |
||
|
Erik JohnstonandClaude Opus 5
|
9b5697d378 |
Move per-homeserver Rust state into a RustRuntime object on the HomeServer (#20011)
Previously the tokio runtime was stashed in a hidden attribute on the reactor object, installed lazily by whichever Rust code first needed it, and started via `callWhenRunning`. Instead, we create a `RustRuntime` (accessible via `HomeServer.get_rust_runtime()`) that holds any per-reactor Rust state, such as the tokio runtime. It is constructed lazily on use. Rust consumers (`HttpClient`, `VersionsHandler`, the Python DB pool wrapper) now receive the runtime or reactor handle explicitly, and the `reactor.run()` / manual-startup workarounds in tests are no longer needed. We also add helper wrappers in Rust for `Reactor` and `HomeServer` that exposes the needed functionality. The aim is to allow us to have a Rust-side clock (mainly to get the current time), that respects the unit test per-reactor time management. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Erik Johnston
|
218fc91210 |
Add a cache to state res to cache more cases (#20185)
This is an attempt to better cache the cases where there are a large number of extremities to resolve over, which keep slightly changing. This spawns from seeing issues on matrix.org. We already have a cache over the exact state groups being resolved. However, we can do better by caching the inputs into state res (i.e. the conflicted sets), which are more likely to be constant across repeated state res in a room. We key this cache based on a sha256 hash, on the assumption that this will never conflict. Also includes a commit that removes needless copying of the state. |
||
|
Erik Johnston
|
26e46786b3 |
Port LogContext to Rust (#19979)
Ports the logcontext classes to Rust, and gives tokio tasks a captured
logcontext so that work running in (or spawned from) Rust is attributed
to the request that caused it.
1. **Add characterization tests for logcontext error messages and the
filter** — pins the exact `logcontext_error` message shapes, the
abuse-detection code paths and `LoggingContextFilter`'s observable
behaviour, *against the existing Python implementation* (this commit is
green on its own). These are the behavioural contract the port has to
satisfy.
2. **Port `ContextResourceUsage` to a Rust pyclass** — self-contained:
the new `synapse_rust.logcontext` module, the class, its stub and the
re-export.
3. **Move the logcontext storage and `LoggingContext` to Rust** — the
core change; the commit message carries detailed design notes.
Highlights:
- The slot is typed `Option<Py<LoggingContext>>`, with `None`
representing the sentinel. `_Sentinel`/`SENTINEL_CONTEXT` stay pure
Python (unchanged); thin wrappers on
`current_context`/`set_current_context` convert at the boundary, and
pyo3's extraction enforces the type (`TypeError` otherwise).
- The accounting is native: one `getrusage(RUSAGE_THREAD)` read per
switch via libc, inline `stop`/`start` bookkeeping for base
`LoggingContext`s, Python dispatch only for subclasses
(`BackgroundProcessLoggingContext`) so their overrides run. The thread
id comes from `PyThread_get_thread_ident` (the exact
`threading.get_ident()`
value) without calling into Python.
- The hot paths avoid per-operation allocation: names are `Py<PyString>`
(the per-log-record `str(context)`/`server_name` reads are INCREF-only),
error branches materialise strings only when hit.
4. **Attribute Rust-spawned work to the caller's logcontext** —
`create_deferred` captures the caller's context and scopes it onto the
spawned task via a tokio task-local (`LogContextHandle`);
`current_context()` gives the task-local read precedence, so
`LoggingContextFilter`/`pyo3-log` resolve the right context on worker
threads with no per-record stamping. `run_python_awaitable` restores the
captured context (via a `with_logcontext` helper, the Rust
`PreserveLoggingContext`) around Python called back from Rust, so e.g.
`runInteraction` from the Rust `/versions` handler accounts its DB usage
against the right request. Integration tests exercise both guarantees
through real production code paths.
Follow-up work on top of this (separate PR): porting
`BackgroundProcessLoggingContext` natively and removing further `Py<_>`
indirections. The fact that `BackgroundProcessLoggingContext` is a
subclass is what forces some of the warts in this PR: e.g. having to use
`Py<LoggingContext>` everywhere, etc.
We don't try (yet) to make this pure Rust, instead we see this as simply
maintaining the Python logcontext machinery when crossing, rather than
trying to make a Rust equivalent that can be used by pure Rust
dependencies. We probably do want to do that in future, as well as wire
up e.g. CPU recording on Rust side, but that is unnecessary for now.
|
||
|
Erik Johnston
|
c595869fb7 |
Add cache to get_partial_filtered_current_state_ids (#20160)
For state filters that ask for concrete types. This allows us to cache the common case of asking for a specific type/state key. I noticed a bunch of queries in the jaeger traces that could be cached. |
||
|
Paul ChobertandErik Johnston
|
15624be279 |
Profile endpoint rate limit (#20218)
This provides configurable (via `rc_profile`) rate limits for profile
endpoints:
- `GET /profile/{username}`
- `GET /profile/{username}/{keyName}` including (`displayname` &
`avatar_url`)
### Pull Request Checklist
<!-- Please read
https://element-hq.github.io/synapse/latest/development/contributing_guide.html
before submitting your pull request -->
* [x] Pull request is based on the develop branch
* [x] Pull request includes a [changelog
file](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#changelog).
The entry should:
- Be a short description of your change which makes sense to users.
"Fixed a bug that prevented receiving messages from other servers."
instead of "Moved X method from `EventStore` to `EventWorkerStore`.".
- Use markdown where necessary, mostly for `code blocks`.
- End with either a period (.) or an exclamation mark (!).
- Start with a capital letter.
- Feel free to credit yourself, by adding a sentence "Contributed by
@github_username." or "Contributed by [Your Name]." to the end of the
entry.
* [x] [Code
style](https://element-hq.github.io/synapse/latest/code_style.html) is
correct (run the
[linters](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#run-the-linters))
---------
Co-authored-by: Erik Johnston <erik@matrix.org>
|
||
|
Jason LittleandPaul Chobert
|
7530874a12 |
Raise default room version to "12" (#20130)
Requires * #19768 * #19782 * https://github.com/matrix-org/complement/pull/915 To meet the requirements for bumping Synapse to support [Matrix spec to 1.16](https://github.com/element-hq/synapse/issues/19414) the default room version should be incremented to "12". Other than the two separate Synapse PRs above(which are included here but marked as "[diverted]" and should be removed prior to review) there is only [one other real change](https://github.com/element-hq/synapse/commit/c005e96c665b5a098a9b95d10f33faf224564be9) to the code base itself to fix a `KeyError` during logging for a `/sync` test against unknown room versions. Everything else should be on the unit tests themselves. Standard unit test running applies, should be nothing special to test this outright. `poetry run trial -jN tests` and similar for Postgresql. Probably ok to review commit-by-commit I took the liberty of writing a [room creating helper](https://github.com/element-hq/synapse/commit/5ba81ad5d4a0a6787b7626bdb3293f633ea4097a) for the two test series that try and test the sharding of the `event_persister` workers. I'm not certain it stands up to scrutiny, but at least does not do any funny mocking when producing room v12 appropriate room IDs. I also took the liberty of writing an [assertion helper](https://github.com/element-hq/synapse/pull/20130/commits/e408cef241d1b9c6a88f069eb5832de053790f9b) for comparing lists of dicts for a select subset of keys/values. This is used to compare stripped state selections while waiting on https://github.com/element-hq/synapse/pull/19723 to be completed. ### Pull Request Checklist <!-- Please read https://element-hq.github.io/synapse/latest/development/contributing_guide.html before submitting your pull request --> * [x] Pull request is based on the develop branch * [x] Pull request includes a [changelog file](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#changelog). The entry should: - Be a short description of your change which makes sense to users. "Fixed a bug that prevented receiving messages from other servers." instead of "Moved X method from `EventStore` to `EventWorkerStore`.". - Use markdown where necessary, mostly for `code blocks`. - End with either a period (.) or an exclamation mark (!). - Start with a capital letter. - Feel free to credit yourself, by adding a sentence "Contributed by @github_username." or "Contributed by [Your Name]." to the end of the entry. * [x] [Code style](https://element-hq.github.io/synapse/latest/code_style.html) is correct (run the [linters](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#run-the-linters)) --------- Co-authored-by: Paul Chobert <paul@chobert.fr> |
||
|
Paul Chobert
|
d553b05e45 |
Return allowed_room_ids in the client /hierarchy response (#20154)
Part of #18731 (Support Matrix 1.15). Since Matrix 1.15, the [spec](https://spec.matrix.org/v1.19/client-server-api/#get_matrixclientv1roomsroomidhierarchy) defines `allowed_room_ids` on the room summaries returned by `GET /_matrix/client/v1/rooms/{roomId}/hierarchy`, so that clients can tell whether a restricted room can be joined or only knocked at: > **`allowed_room_ids`** — `[Room ID]` — If the room is a [restricted room](https://spec.matrix.org/v1.19/client-server-api/#restricted-rooms), these are the room IDs which are specified by the join rules. Empty or omitted otherwise. Added in `v1.15` > > — [Matrix Spec](https://spec.matrix.org/v1.19/client-server-api/#get_matrixclientv1roomsroomidhierarchy) Synapse currently strips the field from the client `/hierarchy` response before returning it — a guard added in matrix-org/synapse#12175 (2022), correct at the time, when the spec defined the field for federation only and it was leaking into client responses. The other two surfaces that define the field (`/room_summary` and the federation `/hierarchy`) already return it. This PR removes the strip in `_RoomEntry.as_json` (and the now-unused `for_client` parameter), so client hierarchy entries include `allowed_room_ids` for restricted rooms, both local and received over federation. The first commit adds the failing test coverage, the second removes the strip. --- ### Pull Request Checklist <!-- Please read https://element-hq.github.io/synapse/latest/development/contributing_guide.html before submitting your pull request --> * [x] Pull request is based on the develop branch * [x] Pull request includes a [changelog file](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#changelog). The entry should: - Be a short description of your change which makes sense to users. "Fixed a bug that prevented receiving messages from other servers." instead of "Moved X method from `EventStore` to `EventWorkerStore`.". - Use markdown where necessary, mostly for `code blocks`. - End with either a period (.) or an exclamation mark (!). - Start with a capital letter. - Feel free to credit yourself, by adding a sentence "Contributed by @github_username." or "Contributed by [Your Name]." to the end of the entry. * [x] [Code style](https://element-hq.github.io/synapse/latest/code_style.html) is correct (run the [linters](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#run-the-linters)) |
||
|
Paul Chobert
|
84ac9ffe37 |
Drop federation device list updates from non-compliant user IDs (#20115)
This partially fixes the bug https://github.com/element-hq/synapse/issues/20116 Device list update EDUs from non-compliant (grandfathered historical) user IDs are currently accepted over federation, stored, and surfaced to clients in `/sync`'s `device_lists.changed` array. > For current room versions, servers must still accept events using such user IDs over federation; however they SHOULD NOT forward such user IDs to clients when referenced outside the context of an event. For example, device list updates from non-compliant user IDs would be dropped by the receiving server. > > -- [Matrix spec](https://spec.matrix.org/v1.14/appendices/#historical-user-ids), clarified in Matrix v1.14 by [matrix-spec#1506](https://github.com/matrix-org/matrix-spec/issues/1506) ### Problem Example A remote server sends an `m.device_list_update` EDU for `@héllo:remote.example` (non-ASCII localpart, outside the compliant U+0021–U+007E range). Synapse: - accepts and processes the update (resyncing the user's device list if needed) - stores it in the remote device list cache - forwards `@héllo:remote.example` to local clients via `device_lists.changed` in `/sync` (**the leak** — a non-compliant user ID referenced outside event context) --- ### Pull Request Checklist <!-- Please read https://element-hq.github.io/synapse/latest/development/contributing_guide.html before submitting your pull request --> * [x] Pull request is based on the develop branch * [x] Pull request includes a [changelog file](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#changelog). The entry should: - Be a short description of your change which makes sense to users. "Fixed a bug that prevented receiving messages from other servers." instead of "Moved X method from `EventStore` to `EventWorkerStore`.". - Use markdown where necessary, mostly for `code blocks`. - End with either a period (.) or an exclamation mark (!). - Start with a capital letter. - Feel free to credit yourself, by adding a sentence "Contributed by @github_username." or "Contributed by [Your Name]." to the end of the entry. * [x] [Code style](https://element-hq.github.io/synapse/latest/code_style.html) is correct (run the [linters](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#run-the-linters)) |
||
|
Paul Chobert
|
8c11b13f63 |
Fix state events missing from MSC4222 state_after when the since token falls inside a persist batch (#20171)
This PR fixes the issue described as comment here: https://github.com/element-hq/synapse/issues/18793#issuecomment-3502202379 In Element Call, this shows up as ghost participants: someone who left the call keeps being displayed until a later state change refreshes the room. The bug is not specific to Element Call: any state event can be affected, RTC membership just changes often enough to make it visible. ## What happens Alice has a client syncing against a homeserver where events are persisted by one worker (the event persister) and `/sync` is served by another (the sync worker). Her client is parked in a long-poll: `GET /sync?since=s99&timeout=30000`. Bob joins a call at the same moment Carol sends a message. Carol's message reaches the persister first; Bob's `m.call.member` arrives while that write is still in flight, so the per-room persist queue groups them into one transaction: ``` events (each gets its own stream ordering): stream_ordering 100: m.room.message Carol stream_ordering 101: m.call.member Bob (state) current_state_delta_stream (how state_after finds state changes): stream_id 100 ────► (m.call.member, @bob) -> $bob_join_call ▲ └─ stamped with the batch MINIMUM (100), not the event's own 101 (see `_update_current_state_txn`) ``` The transaction commits: both events and the delta row are now in the database, atomically. The persister then announces the new events over replication, one RDATA token per stream ordering — rows are only merged into one token when they share a position, and 100 and 101 don't. So the sync worker's events-stream position steps 99 → 100 → 101, and on reaching 100 it pokes the notifier. Alice's long-poll wakes at exactly that moment. Her response is built at the worker's *current* position — `end = 100` — with RDATA 101 still in the queue: ``` Sync A (since=99, end=100): timeline: events 99 < ordering ≤ 100 → [Carol's message] state_after: deltas 99 < stream_id ≤ 100 → [$bob_join_call] ← delivered EARLY next_batch: s100 ← mid-batch token ``` No race on the client's side is needed: the server *hands out* the mid-batch token as `next_batch`. Alice's client re-polls with it, as every sync client does. The worker has meanwhile processed RDATA 101: ``` Sync B (since=100, end=101): timeline: events 100 < ordering ≤ 101 → [Bob's m.call.member @101] ✓ state_after: deltas 100 < stream_id ≤ 101 → [] row is stamped 100 ✗ ``` A state event in the timeline with an empty `state_after`. An MSC4222 client trusts `state_after` over timeline state events, so Alice's copy of Bob's call membership never updates from this response. On a single process this cannot happen: the batch's stream IDs are released as a whole, so the position visible to `/sync` jumps 99 → 101 and `s100` is never handed out. Only a process that learns its position from replication — any sync worker — ticks through the middle of a batch. ### Pull Request Checklist <!-- Please read https://element-hq.github.io/synapse/latest/development/contributing_guide.html before submitting your pull request --> * [x] Pull request is based on the develop branch * [x] Pull request includes a [changelog file](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#changelog). The entry should: - Be a short description of your change which makes sense to users. "Fixed a bug that prevented receiving messages from other servers." instead of "Moved X method from `EventStore` to `EventWorkerStore`.". - Use markdown where necessary, mostly for `code blocks`. - End with either a period (.) or an exclamation mark (!). - Start with a capital letter. - Feel free to credit yourself, by adding a sentence "Contributed by @github_username." or "Contributed by [Your Name]." to the end of the entry. * [x] [Code style](https://element-hq.github.io/synapse/latest/code_style.html) is correct (run the [linters](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#run-the-linters)) |
||
|
Devon HudsonandClaude Sonnet 5
|
f7c16d0148 |
Support configuring a username for Redis connections (#20187)
Adds a `redis.username` config option. Details: A `username` without a `password` (or `password_path`) is refused at startup. Redis has no wire form for a username without a password, and txredisapi only sends `AUTH` when a password is set, so the username would otherwise be silently ignored. An explicitly empty password is accepted, since that is how a `nopass` ACL user is configured. This relies on txredisapi 1.4.12, the first release to accept a `username` kwarg. That upstream support was contributed by @karolyi specifically to unblock this. Fixes #19238. ### Pull Request Checklist <!-- Please read https://element-hq.github.io/synapse/latest/development/contributing_guide.html before submitting your pull request --> * [X] Pull request is based on the develop branch * [X] Pull request includes a [changelog file](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#changelog). The entry should: - Be a short description of your change which makes sense to users. "Fixed a bug that prevented receiving messages from other servers." instead of "Moved X method from `EventStore` to `EventWorkerStore`.". - Use markdown where necessary, mostly for `code blocks`. - End with either a period (.) or an exclamation mark (!). - Start with a capital letter. - Feel free to credit yourself, by adding a sentence "Contributed by @github_username." or "Contributed by [Your Name]." to the end of the entry. * [X] [Code style](https://element-hq.github.io/synapse/latest/code_style.html) is correct (run the [linters](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#run-the-linters)) --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> |
||
|
Olivier 'reivilibre
|
cd2c84b5a5 |
Stop treating unset display names and avatar URLs as profile fields with a null value. (#20145)
Instead, treat them as absent fields as they feel like they should be. The database implementation detail that these fields have a dedicated column with `NULL` when unset is kept to the storage layer. The goal here is to reduce the amount of special casing needed for these two original profile fields and treat them a little bit more like regular profile fields. Follows: #20003 Follows: #20147 (needed as a bugfix to continue sending them down oldschool sync when they get deleted. Without #20147, this PR would break that — which matches how custom profile fields were broken too.) --------- Signed-off-by: Olivier 'reivilibre <oliverw@matrix.org> |
||
|
Skye ElliotandAndrew Morgan
|
8c8138d572 |
Reject OTK uploads if clients exceed 500 server-side keys (#20162)
Co-authored-by: Andrew Morgan <andrew@amorgan.xyz> Signed-off-by: Skye Elliot <actuallyori@gmail.com> |
||
|
Olivier 'reivilibre
|
6825c98eb2 |
Add support for un-soft-failing MSC4354 Sticky Events when room state changes, making federation support more reliable. (#20204)
Part of: MSC4354 Experimental feature tracking issue: https://github.com/element-hq/synapse/issues/19409 Related Complement tests currently in https://github.com/matrix-org/complement/pull/806/files#diff-6c9d6d169485d0848c6b20dd9b43f6fe669a8a710e42f953d08fa25a99cc8f4cR509 --------- Signed-off-by: Olivier 'reivilibre <oliverw@matrix.org> |
||
|
Paul Chobert
|
3f17bd706a |
Return 200 {} again for an unset displayname/avatar_url (#20200)
Part of: #19414 When [MSC4133](https://github.com/matrix-org/matrix-spec-proposals/pull/4133) (custom profile fields) was implemented, the returned value for an unset display name changed from `200 {}` to `200 { displayname: null }`. This happened first on the unstable `uk.tcpip.msc4133` path in #17488 (1.123.0), then on the stable path when #18635 (1.135.0) unified the `displayname`, `avatar_url` and custom field servlets. Neither PR discussed the change in review, so it looks like an unintended side effect of the refactor rather than a deliberate decision. The v1.16 spec mandated to change from returning `200 {}` to `404` but change was not identified as breaking and was eventually not implemented in other clients and server. This PR has a sister MSC that proposes to return to the pre-1.16 error codes: [MSC4537](https://github.com/matrix-org/matrix-spec-proposals/pull/4537). Before: ``` GET /_matrix/client/v3/profile/@alice:test/displayname 200 {"displayname": null} ``` After: ``` GET /_matrix/client/v3/profile/@alice:test/displayname 200 {} ``` ### Pull Request Checklist <!-- Please read https://element-hq.github.io/synapse/latest/development/contributing_guide.html before submitting your pull request --> * [x] Pull request is based on the develop branch * [x] Pull request includes a [changelog file](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#changelog). The entry should: - Be a short description of your change which makes sense to users. "Fixed a bug that prevented receiving messages from other servers." instead of "Moved X method from `EventStore` to `EventWorkerStore`.". - Use markdown where necessary, mostly for `code blocks`. - End with either a period (.) or an exclamation mark (!). - Start with a capital letter. - Feel free to credit yourself, by adding a sentence "Contributed by @github_username." or "Contributed by [Your Name]." to the end of the entry. * [x] [Code style](https://element-hq.github.io/synapse/latest/code_style.html) is correct (run the [linters](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#run-the-linters)) |
||
|
Kegan DougalandEric Eastwood
|
54c0a78f76 |
MSC4242: State DAGs (serving) (#20133)
Adds the serving functions needed for MSC4242: State DAGs. This PR adds MSC4242 support to /make_join, /send_join and /get_missing_events, as well as calculates the destinations for /send events correctly using `prev_state_events`. Built on top of https://github.com/element-hq/synapse/pull/19718 for the storage functions it makes. Split out from https://github.com/element-hq/synapse/pull/19425 Part of a series of 5x PRs to land the federation part of [MSC4242](https://github.com/matrix-org/matrix-spec-proposals/pull/4242) ([storage](https://github.com/element-hq/synapse/pull/19718), [fedclient](https://github.com/element-hq/synapse/pull/20127), serving (this PR), inbound-joins, inbound-pulls). Whilst this is mostly a port of the code in #19425 there are a few changes: - `/get_missing_events` accepts message events when walking the state DAG, in which case it resolves the first hop to be that event's `prev_state_events`. The original PR made the client `/event` the message event and then set `latest=[prev_state_events]` on its own. This is not very efficient (extra round trip to fetch the event) and there's no reason why the server can't do the message->prev_state_events lookup, so we do so. This matches the MSC examples. - We cap the amount of events fetched via `/get_missing_events`. The MSC allows it, so it's a good safety check. - We sort the returned state DAG in `/send_join` by depth then event ID so it's "mostly" sorted. This is more a formality than anything else, the MSC does not mandate this, but it makes `/send_join` responses deterministic. - `notify_on_event_delivered_over_federation` is a new thing since #19425, so we include state DAG events in it like we do with state/auth_chain. This PR does remove the forced `m.federate: false` setting for MSC4242 rooms, so it makes it possible for federated MSC4242 rooms to be made. This is mostly so we can test via the endpoints. Given you must opt-in to MSC4242 via the experimental features config option, it seems reasonable to loosen this setting. The forced no-federation flag existed prior to review saying that the MSC4242 room version could itself be gated behind an experimental feature. Reviewable commit-by-commit. ### Pull Request Checklist <!-- Please read https://element-hq.github.io/synapse/latest/development/contributing_guide.html before submitting your pull request --> * [x] Pull request is based on the develop branch * [x] Pull request includes a [changelog file](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#changelog). The entry should: - Be a short description of your change which makes sense to users. "Fixed a bug that prevented receiving messages from other servers." instead of "Moved X method from `EventStore` to `EventWorkerStore`.". - Use markdown where necessary, mostly for `code blocks`. - End with either a period (.) or an exclamation mark (!). - Start with a capital letter. - Feel free to credit yourself, by adding a sentence "Contributed by @github_username." or "Contributed by [Your Name]." to the end of the entry. * [x] [Code style](https://element-hq.github.io/synapse/latest/code_style.html) is correct (run the [linters](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#run-the-linters)) --------- Co-authored-by: Eric Eastwood <erice@element.io> |
||
|
Olivier 'reivilibre
|
3a2fa70157 |
Improve the rendering of the set inequality errors produced by assertEqual in the tests. (#20193)
This also changes the rendering of our custom helper `assertIncludes`. Spawns from https://github.com/element-hq/synapse/pull/20019#discussion_r3676539092 This PR hijacks `assertEqual` in order to substitute in our own error rendering logic for set inequality. (The motivation to do this is that we should just render sets in our preferred style by default, without having to think about `assertIncludes` with the `exact` flag or risk forgetting it.) The error rendering for `assertIncludes` is adapted to make it reusable in our `assertEqual` and to make it clearer _to me_ (I found it a bit jarring that `+` was used more like a tick, when in other test frameworks I expect to see that as a diff marker). I have tried to make it as clear as I could without being cryptic. --------- Signed-off-by: Olivier 'reivilibre <oliverw@matrix.org> |
||
|
Johannes Marbach
|
f8f7738f5d |
Add experimental support for sending federation requests from app services as per MSC4512 (#19977)
This implements the outgoing part of [MSC4512](https://github.com/matrix-org/matrix-spec-proposals/pull/4512) and is another stopgap towards https://github.com/element-hq/voip-internal/issues/641. This adds the ability for proxying app services (https://github.com/element-hq/synapse/pull/19972) to trigger federation requests under their own proxy prefix. This depends on https://github.com/element-hq/synapse/pull/19972 and cannot make progress before it lands. |
||
|
Paul ChobertandQuentin Gliech
|
e35c8f0eff |
Fix client_secret param validator (#20104)
The validation of `client_secret` params accepts invalid values: any
value that includes a char in `[0-9a-zA-Z.=_-]` is accepted, for example
`"café"` is accepted.
Instead synapse should only accept if all chars are within
`[0-9a-zA-Z.=_-]`, not just one.
This is the current validator:
```python
ClientSecretStr = Annotated[
str,
StringConstraints(
pattern="[0-9a-zA-Z.=_-]",
min_length=1,
max_length=255,
strict=True,
),
]
```
Unfortunately, Pydantic only defines the `pattern` argument as:
> ### pattern
> A regex pattern that the string must match.
Which is extremely imprecise.
Here is a little script to verify the behavior:
```python
from typing import Annotated
from pydantic import BaseModel, StringConstraints, ValidationError
def check(pattern: str) -> None:
ClientSecretStr = Annotated[
str,
StringConstraints(pattern=pattern, min_length=1, max_length=255, strict=True),
]
class Body(BaseModel):
client_secret: ClientSecretStr
try:
Body.model_validate({"client_secret": "café"})
print(f"pattern = {pattern!r}: 'café' ACCEPTED <-- should have been rejected")
except ValidationError:
print(f"pattern = {pattern!r}: 'café' rejected")
check("[0-9a-zA-Z.=_-]") # before the fix (unanchored)
check("^[0-9a-zA-Z.=_-]+$") # after the fix
```
which would output
```
pattern = '[0-9a-zA-Z.=_-]': 'café' ACCEPTED <-- should have been rejected
pattern = '^[0-9a-zA-Z.=_-]+$': 'café' rejected
```
## History
This is a regression of a previously reported and fixed bug:
- matrix-org/synapse#6766 (2020) reported that Synapse did not enforce
the spec's `client_secret` regex at all — with real-world fallout:
FluffyChat had started sending secrets containing `:` because nothing
rejected them. Fixed by introducing `assert_valid_client_secret`
(matrix-org/synapse#6767).
- matrix-org/synapse#13188 (Synapse 1.66.0) ported the account endpoints
to Pydantic and transcribed the regex without anchors/quantifier;
Pydantic v1's `re.match` semantics meant only the *first* character was
validated.
- #19071 (Synapse 1.142.0) migrated to Pydantic v2, whose *search*
semantics weakened it further to "any one character anywhere".
---
### Pull Request Checklist
<!-- Please read
https://element-hq.github.io/synapse/latest/development/contributing_guide.html
before submitting your pull request -->
* [x] Pull request is based on the develop branch
* [x] Pull request includes a [changelog
file](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#changelog).
The entry should:
- Be a short description of your change which makes sense to users.
"Fixed a bug that prevented receiving messages from other servers."
instead of "Moved X method from `EventStore` to `EventWorkerStore`.".
- Use markdown where necessary, mostly for `code blocks`.
- End with either a period (.) or an exclamation mark (!).
- Start with a capital letter.
- Feel free to credit yourself, by adding a sentence "Contributed by
@github_username." or "Contributed by [Your Name]." to the end of the
entry.
* [x] [Code
style](https://element-hq.github.io/synapse/latest/code_style.html) is
correct (run the
[linters](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#run-the-linters))
---------
Co-authored-by: Quentin Gliech <quenting@element.io>
|
||
|
Paul Chobert
|
470b941895 |
Fix private read receipts of other users being leaked to application services (#20114)
Application services that opt in to receiving ephemeral events (`receive_ephemeral: true` in the [registration](https://spec.matrix.org/v1.19/application-service-api/#registration), added in Matrix v1.13 from [MSC2409](https://github.com/matrix-org/matrix-spec-proposals/pull/2409)) are sent the private read receipts (`m.read.private`) of **every** user in the rooms they are interested in — not just their own users. The [Application Service API](https://spec.matrix.org/v1.19/application-service-api/#pushing-ephemeral-data) is explicit about this (*Pushing ephemeral data*, `m.receipt`): > Private read receipts MUST only be sent for users matching one of the application service's namespaces. Normal read receipts and threaded read receipts are always sent. This matches the [Client-Server API](https://spec.matrix.org/v1.19/client-server-api/#private-read-receipts): "Servers MUST NOT send the `m.read.private` receipt to any other user than the one which originally sent it." The restriction to namespaced users is safe because the appservice could learn those receipts anyway by syncing as the user. For everyone else, `m.read.private` exists precisely so that nobody — including bridges and bots in the room — can observe it. For example, take an appservice registered with: ```yaml namespaces: users: - regex: "@_bridge_.*:example\\.org" exclusive: true ``` When `@alice:example.org` (a regular user, not one of the appservice's) and `@_bridge_bob:example.org` (a namespaced user) each send read receipts in a bridged room, the appservice receives: ```json { "type": "m.receipt", "room_id": "!room:example.org", "content": { "$event": { "m.read": { "@alice:example.org": { "ts": 1436451550453 } }, "m.read.private": { "@_bridge_bob:example.org": { "ts": 1436451550453 }, "@alice:example.org": { "ts": 1436451550453 } } } } } ``` - `m.read` from `@alice` — correct, public read receipts are always sent. - `m.read.private` from `@_bridge_bob` — correct, the user is within the appservice's namespaces. - `m.read.private` from `@alice` — **the leak**: their private read receipt must not be sent to the appservice. ## History - matrix-org/synapse#8437 (Oct 2020, Synapse 1.22.0) implemented MSC2409 ephemeral event delivery to appservices. No leak at that point: private read receipts did not exist yet. - matrix-org/synapse#10413 (Jul 2021, Synapse 1.40.0) added the initial, experimental-flag-gated implementation of MSC2285 ("hidden" read receipts) and filtered them out of the `/sync` path (`filter_out_hidden`) — but not out of the appservice path in the same file. This is where the leak originates, for servers with `msc2285_enabled`. - matrix-org/synapse#12168 (May 2022, Synapse 1.59.0) reworked this into the `m.read.private` receipt type and `filter_out_private_receipts`; the appservice path was again left unfiltered. - matrix-org/synapse#13273 (Aug 2022, Synapse 1.65.0) moved to the stable `m.read.private` identifier; the appservice path has leaked it ever since. --- ### Pull Request Checklist <!-- Please read https://element-hq.github.io/synapse/latest/development/contributing_guide.html before submitting your pull request --> * [x] Pull request is based on the develop branch * [x] Pull request includes a [changelog file](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#changelog). The entry should: - Be a short description of your change which makes sense to users. "Fixed a bug that prevented receiving messages from other servers." instead of "Moved X method from `EventStore` to `EventWorkerStore`.". - Use markdown where necessary, mostly for `code blocks`. - End with either a period (.) or an exclamation mark (!). - Start with a capital letter. - Feel free to credit yourself, by adding a sentence "Contributed by @github_username." or "Contributed by [Your Name]." to the end of the entry. * [x] [Code style](https://element-hq.github.io/synapse/latest/code_style.html) is correct (run the [linters](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#run-the-linters)) |
||
|
Paul Chobert
|
bd21823818 |
Fix 500 error when setting a custom profile field for a user with no profile row (#20172)
Follow-up to #20149, which fixed the 500 when *getting* a profile field for a user with no `profiles` row. The same crash was still reachable when *setting* one via `PUT /_matrix/client/v3/profile/{userId}/{field}` (as server admin). This PR splits that case in two: * **The user exists but has no `profiles` row** (e.g. profile erased upon deactivation): * Before: `500 M_UNKNOWN` (`TypeError: cannot unpack non-sequence NoneType` in the profile size check). * After: `200`, the profile row is recreated with the field set. * **The user does not exist at all**: * Before: `500 M_UNKNOWN` (same crash). * After: `404 M_NOT_FOUND`, without conjuring up an orphan profile row. Fixing the crash also surfaced a latent SQLite-only bug where a field set on a freshly created profile row was stored under the wrong key, making it 404 on `GET` right after a successful `PUT`. ### Pull Request Checklist <!-- Please read https://element-hq.github.io/synapse/latest/development/contributing_guide.html before submitting your pull request --> * [x] Pull request is based on the develop branch * [x] Pull request includes a [changelog file](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#changelog). The entry should: - Be a short description of your change which makes sense to users. "Fixed a bug that prevented receiving messages from other servers." instead of "Moved X method from `EventStore` to `EventWorkerStore`.". - Use markdown where necessary, mostly for `code blocks`. - End with either a period (.) or an exclamation mark (!). - Start with a capital letter. - Feel free to credit yourself, by adding a sentence "Contributed by @github_username." or "Contributed by [Your Name]." to the end of the entry. * [x] [Code style](https://element-hq.github.io/synapse/latest/code_style.html) is correct (run the [linters](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#run-the-linters)) |
||
|
Jason Little
|
a0b5a45c91 | fix MSC3912-relation-based-redaction for room versions > 10 (#19782) | ||
|
Kegan Dougal
|
0088e43da7 |
MSC4242: State DAGs (fed client) (#20127)
Split out from https://github.com/element-hq/synapse/pull/19425 - Add federation support for MSC4242 fields in `/send_join` and `/get_missing_events` - Move the gate preventing joining MSC4242 rooms higher in the call stack Part of a series of 5x PRs to land the federation part of MSC4242 (storage, fedclient, serving, inbound-joins, inbound-pulls). ### Pull Request Checklist <!-- Please read https://element-hq.github.io/synapse/latest/development/contributing_guide.html before submitting your pull request --> * [x] Pull request is based on the develop branch * [x] Pull request includes a [changelog file](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#changelog). The entry should: - Be a short description of your change which makes sense to users. "Fixed a bug that prevented receiving messages from other servers." instead of "Moved X method from `EventStore` to `EventWorkerStore`.". - Use markdown where necessary, mostly for `code blocks`. - End with either a period (.) or an exclamation mark (!). - Start with a capital letter. - Feel free to credit yourself, by adding a sentence "Contributed by @github_username." or "Contributed by [Your Name]." to the end of the entry. * [x] [Code style](https://element-hq.github.io/synapse/latest/code_style.html) is correct (run the [linters](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#run-the-linters)) |
||
|
Paul Chobert
|
207f6560ce |
Stabilize the M_APPSERVICE_LOGIN_UNSUPPORTED error code (#20180)
Part of: https://github.com/element-hq/synapse/issues/19415 Return `M_APPSERVICE_LOGIN_UNSUPPORTED` error code instead of the unstable `IO.ELEMENT.MSC4190.M_APPSERVICE_LOGIN_UNSUPPORTED` identifier. > Servers MUST still allow application services to use the `/register` endpoint with a login type of `m.login.application_service` even if they don't support the Legacy Authentication API. In that case application services MUST set the `"inhibit_login": true` parameter as they cannot use it to log in as users. If the `inhibit_login` parameter is not set to `true`, the server MUST return a 400 HTTP status code with an `M_APPSERVICE_LOGIN_UNSUPPORTED` error code. > > [...] > > Application services MUST NOT use the `/login` endpoint if the server doesn't support the Legacy authentication API. If `/login` is called with the `m.login.application_service` login type the server MUST return a 400 HTTP status code with an `M_APPSERVICE_LOGIN_UNSUPPORTED` error code. > > — [Matrix v1.19, Application Service API](https://spec.matrix.org/v1.19/application-service-api/#registration) Synapse returns the correct 400 on both endpoints, but with the unstable identifier. Before: ``` POST /_matrix/client/v3/login {"type": "m.login.application_service", ...} # appservice with MSC4190 device management POST /_matrix/client/v3/register {"type": "m.login.application_service", ...} # without "inhibit_login": true 400 {"errcode": "IO.ELEMENT.MSC4190.M_APPSERVICE_LOGIN_UNSUPPORTED"} ``` After: ``` POST /_matrix/client/v3/login {"type": "m.login.application_service", ...} # appservice with MSC4190 device management POST /_matrix/client/v3/register {"type": "m.login.application_service", ...} # without "inhibit_login": true 400 {"errcode": "M_APPSERVICE_LOGIN_UNSUPPORTED"} ``` A Sister PR exists in MAS: https://github.com/element-hq/matrix-authentication-service/pull/5961 ; when delegation is enabled, `/login` reaches MAS instead of Synapse, and MAS currently answers `m.login.application_service` with `M_UNKNOWN`. |
||
|
Paul Chobert
|
366a020f79 |
Fix MSC4178 error codes on /account/3pid/{email,msisdn}/requestToken (#20101)
Part of https://github.com/element-hq/synapse/issues/18118 ## What the spec says Since Matrix v1.13 (introduced by [MSC4178](https://github.com/matrix-org/matrix-spec-proposals/pull/4178)), the `400` response of [`POST /_matrix/client/v3/account/3pid/email/requestToken`](https://spec.matrix.org/v1.19/client-server-api/#post_matrixclientv3account3pidemailrequesttoken) lists, among the "Error codes that can be returned": > - `M_THREEPID_MEDIUM_NOT_SUPPORTED`: The homeserver does not support adding email addresses. > - `M_INVALID_PARAM`: The email address given was not valid. and [`POST /_matrix/client/v3/account/3pid/msisdn/requestToken`](https://spec.matrix.org/v1.19/client-server-api/#post_matrixclientv3account3pidmsisdnrequesttoken) likewise: > - `M_THREEPID_MEDIUM_NOT_SUPPORTED`: The homeserver does not support adding phone numbers. > - `M_INVALID_PARAM`: The phone number given was not valid. ## What was missing Synapse implemented the headline case (unsupported medium), but around it: - A malformed email address or country code was reported with the generic `M_BAD_JSON` instead of `M_INVALID_PARAM`. The email validator deliberately kept `M_BAD_JSON` "to ensure backward compatibility of HTTP error codes" (matrix-org/synapse#13687, 2022) — that predates Matrix v1.13, which now lists `M_INVALID_PARAM` for this case. - On the msisdn variant, the unsupported-medium check ran after the denied/in-use checks, so a request wrong in two ways reported the other fault; the email variant checks it first. ## What this PR changes - Malformed email addresses and country codes on `/account/3pid/{email,msisdn}/requestToken` are reported with `M_INVALID_PARAM`. Both flow through the existing errcode translation as `value_error`: the email validator raises a plain `ValueError`, and the country-code constraint (`ISO3166_1_Alpha_2`) declares its own error via pydantic-core's `custom_error_schema`. - On the msisdn variant the unsupported-medium check now runs before the denied/in-use checks, as on the email variant. - The country-code type is renamed from `ISO3116_1_Alpha_2` to `ISO3166_1_Alpha_2` (typo in the standard's number). [`/account/password/email/requestToken`](https://spec.matrix.org/v1.19/client-server-api/#post_matrixclientv3accountpasswordemailrequesttoken) (not covered by the v1.13 change) shares the email request body model, so a malformed email there is now also reported with `M_INVALID_PARAM` instead of `M_BAD_JSON`. Its `400` response is described as "the request was invalid" and only names `M_SERVER_NOT_TRUSTED` explicitly ("can be returned if…") rather than restricting the server to a fixed list, and `M_INVALID_PARAM` is the spec's generic code for "A parameter that was specified has the wrong value" ([other error codes](https://spec.matrix.org/v1.19/client-server-api/#other-error-codes)). |
||
|
Paul Chobert
|
1727ceee7b |
Ratelimit the room reporting endpoint (MSC4151) (#20036)
Apply the `rc_reports` rate limit to the room reporting endpoint, [`POST
/_matrix/client/v3/rooms/{roomId}/report`](https://spec.matrix.org/v1.19/client-server-api/#post_matrixclientv3roomsroomidreport)
(added in Matrix v1.13).
The spec marks this endpoint as **Rate-limited: Yes** (clients must
expect a `429 M_LIMIT_EXCEEDED`), and homeservers [SHOULD implement rate
limiting](https://spec.matrix.org/v1.19/client-server-api/#rate-limiting)
in general, but Synapse currently applies no limit here. The sibling
user reporting endpoint already uses `rc_reports`, so this reuses the
same limit instead of introducing a new config option.
Changes:
- Move the room report logic from `ReportRoomRestServlet` into a new
`ReportsHandler.report_room`, mirroring the existing `report_user`. The
rate limit is checked before the room existence lookup, so it bounds the
DB work a caller can trigger and cannot be used to tell existing rooms
from non-existing ones.
- The servlet keeps the existing behaviour of returning `200` regardless
of room existence when `msc4277_enabled` is set (the spec allows this
since v1.18).
- Add a regression test covering the `429` response and the per-user
rate limit override.
|
||
|
Paul Chobert
|
d4de2ae1c5 |
Fix state_after for left rooms including post-leave lazy-loaded memberships (#20169)
## The bug With the experimental [MSC4222](https://github.com/matrix-org/matrix-spec-proposals/pull/4222) implementation enabled (`use_state_after`) and lazy-loading of room members, an incremental `/sync` could disclose state from **after** the user's leave in a left room's `state_after`. 1. Alice syncs with `lazy_load_members: true` and `use_state_after=true`. 2. Bob sends a message in a room they share. 3. Alice leaves the room. 4. Bob updates his per-room displayname 5. Alice does an incremental sync covering steps 2–3. Alice's `state_after` contains Bob's post-leave membership event from step 4 Alice should not see the new per-room display name of Bob. ## The fix Copy what has been done for `_compute_state_delta_for_full_sync`: pass `joined` down and, for rooms the user is no longer joined to, fetch the memberships as of `end_token` via state groups (`get_state_ids_at`) instead of current state. |
||
|
guillermo
|
8f27ac006f |
Add unit tests for parse_stripped_state_event (#20136)
This PR adds a suite of tests for Synapse state events. It covers key scenarios around creation, updates and state consistency to prevent regressions in event processing and serialization. The primary goal is to increase test coverage. |
||
|
|
dea059a94d |
Restrict message events from being redacted by local users if past allowed period. (#20138)
Closes: #20065 --------- Co-authored-by: defaultdino <adve@adisve.net> Co-authored-by: Olivier 'reivilibre <oliverw@element.io> |
||
|
FrenchGithubUser
|
c40ab6e3b9 |
Fix sending duplicate m.room.encryption events on room creation (#20106)
On room creation when the client already supplies one in the initial state and `encryption_enabled_by_default_for_room_type` is enabled. |
||
|
Paul Chobert
|
f94abb6924 |
Return 403 instead of 400 when profile changes are disabled (#20173)
With `enable_set_displayname: false` (or `enable_set_avatar_url:
false`), refusing a profile change returned the right errcode with the
wrong status:
```
PUT /_matrix/client/v3/profile/@alice:example.com/displayname (displayname already set)
→ 400 {"errcode": "M_FORBIDDEN", "error": "Changing display name is disabled on this server"}
DELETE /_matrix/client/v3/profile/@alice:example.com/displayname
→ 400 {"errcode": "M_FORBIDDEN", "error": "Changing display name is disabled on this server"}
```
With this fix:
```
PUT /_matrix/client/v3/profile/@alice:example.com/displayname (displayname already set)
→ 403 {"errcode": "M_FORBIDDEN", "error": "Changing display name is disabled on this server"}
DELETE /_matrix/client/v3/profile/@alice:example.com/displayname
→ 403 {"errcode": "M_FORBIDDEN", "error": "Changing display name is disabled on this server"}
```
The spec defines the [403 response of `PUT
/_matrix/client/v3/profile/{userId}/{keyName}`](https://spec.matrix.org/v1.19/client-server-api/#put_matrixclientv3profileuseridkeyname)
as "The server is unwilling to perform the operation, either due to
insufficient permissions or **because profile modifications are
disabled**", while 400 is reserved for malformed input (`M_BAD_JSON`,
`M_MISSING_PARAM`, …).
Clients seem to rely on `errcode` field more than the HTTP Status Code,
that change seems safe.
|
||
|
Paul Chobert
|
b6798a1353 |
Fix event_search background reindex skipping m.room.topic events (#20119)
Room topics are dropped from the search index whenever the `event_search` background reindex runs (e.g. after a search index rebuild, or when the background update is re-run on an upgraded homeserver), making topics unsearchable even though the live write path indexes them correctly. The cause is a trailing comma in `_background_reindex_search`, which turns the topic `value` into a 1-tuple instead of a string: https://github.com/element-hq/synapse/blob/14c96c0f5444cbe28b6ac0361cf94216b8d352db/synapse/storage/databases/main/search.py#L211-L213 The downstream `if not isinstance(value, str): continue` guard then silently skips *every* `m.room.topic` event, so no topic ever reaches `event_search` during a reindex. The regression was introduced in #18195, which added rich-text topic support (MSC3765) to the reindex path. ### Problem Example 1. A room has topic "project roadmap". 2. An admin rebuilds the search index (or the `event_search` background update re-runs). 3. `m.room.message` and `m.room.name` events are reindexed fine, but every `m.room.topic` event is skipped. 4. Searching for "project roadmap" with key `content.topic` returns 0 results — the topic is permanently unsearchable until the event is sent again. --- ### Pull Request Checklist <!-- Please read https://element-hq.github.io/synapse/latest/development/contributing_guide.html before submitting your pull request --> * [x] Pull request is based on the develop branch * [x] Pull request includes a [changelog file](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#changelog). The entry should: - Be a short description of your change which makes sense to users. "Fixed a bug that prevented receiving messages from other servers." instead of "Moved X method from `EventStore` to `EventWorkerStore`.". - Use markdown where necessary, mostly for `code blocks`. - End with either a period (.) or an exclamation mark (!). - Start with a capital letter. - Feel free to credit yourself, by adding a sentence "Contributed by @github_username." or "Contributed by [Your Name]." to the end of the entry. * [x] [Code style](https://element-hq.github.io/synapse/latest/code_style.html) is correct (run the [linters](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#run-the-linters)) |
||
|
Kegan DougalandEric Eastwood
|
54bfa1a01b |
MSC4242: State DAGs (storage) (#19718)
Split out from https://github.com/element-hq/synapse/pull/19425 Add storage functions needed for federation support for MSC4242: - `get_missing_events_state_dag`: will be used to satisfy `/get_missing_events` requests. - `get_state_dag`: will be used to satisfy `/send_join` requests. Part of a series of 5x PRs to land the federation part of MSC4242 (storage, fedclient, serving, inbound-joins, inbound-pulls). ### Pull Request Checklist <!-- Please read https://element-hq.github.io/synapse/latest/development/contributing_guide.html before submitting your pull request --> * [x] Pull request is based on the develop branch * [x] Pull request includes a [changelog file](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#changelog). The entry should: - Be a short description of your change which makes sense to users. "Fixed a bug that prevented receiving messages from other servers." instead of "Moved X method from `EventStore` to `EventWorkerStore`.". - Use markdown where necessary, mostly for `code blocks`. - End with either a period (.) or an exclamation mark (!). - Start with a capital letter. - Feel free to credit yourself, by adding a sentence "Contributed by @github_username." or "Contributed by [Your Name]." to the end of the entry. * [x] [Code style](https://element-hq.github.io/synapse/latest/code_style.html) is correct (run the [linters](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#run-the-linters)) --------- Co-authored-by: Eric Eastwood <madlittlemods@gmail.com> |