This PR:
- creates an OpenMetrics server that enables collecting performance data from this process by e.g. a Prometheus server;
- exposes as metrics the performance of http requests with MatrixBot.
Further metrics may of course be added.
This started out as just a way to find out why mjolnir was syncing with lists several times for each update to a policy list.
The main changes are
- Verbosity was irrelevant to the sync command but for some reason was an option.
Unfortunately all this did was suppress whether to tell you when it had finished, meaning it wouldn't
when verbose logging was disabled. Historically this was probably a parameter that got passed through
to applyServerAcl/applyUserBans, which can be horribly verbose, but they access the config directly.
- Stop emitting `'PolicyList.update'` when there are no changes.
- Include a revision ID for the `'PolicyList.update'`method and event.
- Use the revision ID in the `ProtectedRoomsSet` so that we don't unnecessarily resynchronize all rooms when the `'PolicyList.update'` event is received. Though not when the `sync` command is used. Since this is supposed to `sync` in the case when there is a state reset or otherwise or the user has changed some room settings.
- insert an await lock around the `PolicyList.update` method to avoid a race condition where a call can be started and finished within the extent of an existing call (via another task, this can happen if the server is slow with handling one request). `PolicyList.udpate` now has a helper that is synchronous to be called directly after requesting the room state. The reason for this is to enforce that no one `await`s while updating the policy list's cache of rules. Which is important because it is one of the biggest methods that I tolerate and visually checking for `await` is impossible.
- The revision ID uses a ULID, but this is unnecessary and could have just been a "dumb counter".
closes https://github.com/matrix-org/mjolnir/issues/447
* Refactor Matrix event listener in Mjolnir and ManagedMjolnir.
closes https://github.com/matrix-org/mjolnir/issues/411.
Issue #411 says that we have to be careful about room.join,
but this was before we figured how to make matrix-appservice-bridge
echo events sent by its own intents.
* Remove MatrixClientListener since it isn't actually needed.
* Protect which config values can be used for ManagedMjolnirs.
* Introduce MatrixSendClient
so listeners aren't accidentally added to a MatrixClient instead
of MatrixEmitter.
* doc
* Move provisioned mjolnir config to src/config.
This just aids maintance so whenever someone goes to change the config
of the bot they will see this and update it.
* doc for matrix intent listener.
The Sentry package is very useful for monitoring runtime errors. With this PR,
we simply add the necessary mechanism to:
- log to sentry any uncaught error that reaches the toplevel, including startup errors.
Mjolnir can now be run as an application service,
meaning it will host multiple independent mjolnirs that can be requested by users.
If the user is on the same homeserver as the appservice is deployed on,
then they can provision a mjolnir via a widget https://github.com/matrix-org/mjolnir-widget.
Otherwise they can invite the appservice bot to a room they want to protect.
This will create them a mjolnir, a management room and a policy list.
The appservice shares the same docker image as the bot,
but is started slightly differently by specifying "appservice"
as the first argument to docker run (this s managed by `mjolnir-entrypoint.sh`.
We could have used another Dockerfile for the appservice,
extending the existing one but we decided not to because there
would have been lots of fiddling around the entrypoint
and logistics involved around adding a tag for it via github actions.
Not to mention that this would be duplicating the image
just to run it with a different binary.
A list of followup issues can be found here https://github.com/issues?q=is%3Aopen+is%3Aissue+author%3AGnuxie+archived%3Afalse+label%3AA-Appservice.
Somewhat relevant and squashed commit messages(regrettably squashing because frankly these won't make sense in isolation):
* draft widget backend
* add `managementRoomId` to `provisionNewMjolnir`
* remove ratelimits from appservice mjolnirs
* add /join endpoint to api backend
* tighter guard around room type in PolicyList
matrix-bot-sdk imporved the types for this
* enable esModuleInterop
* launch and use postgres in a container whilst using mx-tester
* limited access control
policy list used for access control
* Redesign initialization API of many mjolnir.
It's much harder to forget to initialize the components now that you have to in order to construct them in the first place.
* Ammend config not to clash with existing CI
this means that the appsrvice bot is now called 'mjolnir-bot' by default
which was easier than going through old code base and renaming
* Change entrypoint in Dockerfile so that we can start the appservice.
We could have used another Dockerfile for the appservice,
extending the exising one but we decided not to because there
would have been lots of fiddling around the entrypoint
and logistics involved around adding a tag for it via github actions.
Not to mention that this would be duplicating the image
just to run it with a different binary.
This solution is much simpler, backwards compatible, and conscious about the future.
Co-authored-by: gnuxie <gnuxie@element.io>
The ACL unit allows you to combine an policy lists and conveniently test users and servers against them.
The main motivation for this work is provide access control on who can provision and continue to use mjolnir instances in the appservice component.
We include a new recommendation type org.matrix.mjolnir.allow which can be used with user and server entity types to create allow lists.
We have also replaced the destructing of policy lists in applyServerACL and applyMemberBans (in ProtectedRooms.ts) with calls to the AccessControlUnit.
Adding commands to add/remove allowed entities is not something i want to do at the moment.
In both our instructions and our tests, we use the r0 endpoint to intercept abuse reports. This endpoint is deprecated and not implemented by all clients. This PR updates the instructions and tests to the new endpoint.
* Attempt to factor out protected rooms from Mjolnir.
This is useful to the appservice because it means we don't
have to wrap a Mjolnir that is designed to sync.
It's also useful if we later on want to have specific
settings per space.
It's also just a nice seperation between Mjolnir's needs while
syncing via client-server and the behaviour of syncing policy rooms.
### Things that have changed
- `ErrorCache` no longer a static class (phew), gets used by `ProtectedRooms`.
- `ManagementRoomOutput` class gets created to handle logging back to the management room.
- Responsibilities for syncing member bans and server ACL are handled by `ProtectedRooms`.
- Responsibilities for watched lists should be moved to `ProtectedRooms` if they haven't been.
- `EventRedactionQueue` is moved to `ProtectedRooms` since this needs to happen after
member bans.
- ApplyServerAcls moved to `ProtectedRooms`
- ApplyMemberBans move to `ProtectedRooms`
- `logMessage` and `replaceRoomIdsWithPills` moved to `ManagementRoomOutput`.
- `resyncJoinedRooms` has been made a little more clear, though I am concerned about how often it does run because it does seem expensive.
* ProtectedRooms is not supposed to track joined rooms.
The reason is because it is supposed to represent a specific
set of rooms to protect, not do horrible logic
for working out what rooms mjolnir is supposed to protect.
* Rework the banning and unbanning of entities in PolicyLists.
1. We keep track of the event that created a list rule so that we
can remove the rule by having a way to determine the original state key for the rule.
This is because the state key of rules can be anything and should not be
relied on by Mjolnir to unban things (which it was doing).
2. The old scheme for producing a state key was causing for some entities to escape bans
https://github.com/matrix-org/mjolnir/issues/322.
We could have used a hash or something similar, but we know that
the reason for the `rule:${entity}` scheme existed was for ease of debugging
and finding rules in devtools. So instead we have followed a scheme simalar to
bridges where the first character of an mxid is replaced with an underscore.
Everything else just gets put into the state key. Since domains can't have '@'
and room ids, aliases can't either.
3. We have stopped the need for Mjolnir to wait for the next response from sync after banning,
unbanning an entity so that we can apply ACL's sooner.
* Use PolicyList's `banEntity` method to create imported rules.
If you add `"no-floating-promises": true` it's very easy
to find where this is done accidentally.
Not sure we can keep that on all the time yet though..
Replace acceptInvitesFromGroup with acceptInvitesFromSpace.
https://github.com/matrix-org/mjolnir/issues/125https://github.com/matrix-org/mjolnir/issues/99
acceptInvitesFromGroup was implemented with an experimental api
that was a precursor to spaces which was refereed to
as either communities or groups.
Support for communities/groups ended in Synapse 1.61.0
https://github.com/matrix-org/synapse/releases/tag/v1.61.0.
To test we just edit the config dynamically which changes how the join room listener functions
though idk, shouldn't we have just made a new mjolnir instance
for this test, or changed the config before the test started somehow?
Co-authored-by: jesopo <github@lolnerd.net>
The implementation is rubbish, as it doesn't avoid the exponential backoff
Remove default rate limit testing.
It doesn't work. No there really isn't more to say about it
you're welcome to dispute it if you're going to do the work investigating. I'm not.
We used to have a test here that tested whether Mjolnir was going to carry out a redact order the default limits in a reasonable time scale.
Now I think that's never going to happen without writing a new algorithm for respecting rate limiting.
Which is not something there is time for.
https://github.com/matrix-org/synapse/pull/13018
Synapse rate limits were broken and very permitting so that's why the current hack worked so well.
Now it is not broken, so our rate limit handling is.
b850e4554c
Honestly I don't think we can expect anyone to be able to use Mjolnir under default rate limits.
well, it's not quite simple as broken, but it is broken. With the default level in synapse (which is what matrix.org uses) it is struggling to redact 15 messages within 5 minutes. that means 5 messages over the burst count. This is ofc ontop mjolnir sending reactions / responding to replies (which isn't much but... enough to mess with the rate limiter since ofc, Synapse tells requests to wait x amount of time before trying again, but that doesn't help for concurrent requests since ofc there's only 1 slot available at that future time. This means Synapse just wacks everything with exponentially longer shit without many (or any?) events going through
it used to be fine
because rate limiting in synapse used to be a lot more liberal because it was "broken" or something, that's not me saying it's broken that's just what synapse devs say which is probably true.
if all requests went into a queue then yeah you could eliminate one problem
but that's a lot of work and i don't think we should be doing it
cos no one uses mjolnir like this anyways
* Stop the config being global (in almost all contexts).
* make sure unit test has a config
* Make failing word list more visible
* Only use Healthz from index.ts
Not really sure how useful it is anyways?
* banListTest would applyACL before rules appeared in `/state`.
Mjolnir will call applyServerAcls several times while a policy list is being updated, sometimes concurrently. This means a request to set a server ACL in a room which has old data can finish after a more recent recent request with the correct ACL. This means that the old ACL gets applied to the rooms (for a moment).
This is a follow up from 551065815e
* Only allow one invocation of applyServerAcls at a time as to not conflict with each other by using a promise chain.
We don't use the throttle queue because we don't want to be blocked by other background tasks.
We don't make another throttle queue because we don't want throttling and we don't want to delay the ACL application, which can happen even with throttle time of 0.
Towards opinions in PolicyLists.
This changeset is part of an ongoing effort to implement "opinions"
within policy lists, as per MSC3847.
For the time being:
- we rename BanList into PolicyList;
- we cleanup a little dead code;
- we replace a few `string`s with `enum`;
- `ListRule` becomes an abstract class with two concrete subclasses `ListRuleBan` and `ListRuleOpinion`.
* Remove debug leftovers from a test.
This is really terrible and has meant whenever anyone has run `yarn test:integration` they have only been running this test.
💀💀💀https://www.youtube.com/watch?v=jmX-tzSOFE0
* Set a default timeout for integration tests that is 5 minutes long.
Seriously, I don't think there is much to gain by making people guess
a reasnoble time for a test to complete in all the time, especially
with how much Synapse changes in response time and all of the machines
involved in running these tests.
* Warn when giving up on being throttled
* For some reason it takes longer for events to appear in /state
no i am not going to track down why yet.
* Rate limiting got a lot more aggresive.
https://github.com/matrix-org/synapse/pull/13018
Rate limiting in Synapse used to reset the burst count and remove
the backoff when you were spamming continuously, now it doesn't.
Ideally we'd rewrite the rate limiting logic to back off for longer
than suggested so we could get burst again, but for now
lets just unblock CI by reducing the number of events we send in these
tests.
So the test before sometimes sent the report *before*
the protection (that is used to check whether we have received
the report) was registered.
This of course meant that we didn't ever receive the report from
the perspectivee of the test.
This PR should now mean we always send the report after
registering the protection.
* standard protection consequences
* add integration test to make sure good users aren't banned
* the less far `event` propagates, the better
* better document consequence.ts
* improve innocent user integration test
* switch to room.event emit
* Test for batching ACL.
* Batch events from sync within BanList.
* Introduce the BanList.batch event to the BanList emitter to let Mjolnir sync after new events have been added from sync.
Fixes#203
