Mjolnir storing aliases for watched_lists account data (and possibly also protected rooms)

[Gnuxie]

It shouldn't do this because resolving aliases is unreliable, and it should already be joined to the room for it to end up in the account data for watched_lists (the same doesn't really apply for protected rooms, since they can be removed silently and the admin of mjolnir will need to know about that). It should only store the room id for watched_lists (in a matrix.to url though i don't really know the context for why it does that in the first place since these urls don't have any via params, making them useless)

{
  "type": "org.matrix.mjolnir.watched_lists",
  "content": {
    "references": [
      "https://matrix.to/#/#banlist-spam:systemtest.tk",
      "https://matrix.to/#/!zgbOLhInZBVQYLdPGk:tchncs.de",
      "https://matrix.to/#/!fTjMjIzNKEsFlUIiru:neko.dev",
      "https://matrix.to/#/#tchncs-ban-list:tchncs.de",
      "https://matrix.to/#/#public-servers-bans:aria-net.org",
      "https://matrix.to/#/#envs-ban-list:envs.net"
    ]
  }
}

The example here is that @mahdi1234:matrix.org couldn't resolve #public-servers-bans:aria-net.org (presumably the server died) and so mjolnir couldn't start.

[Yoric]

#462 and #463 should alleviate the issue by:

• making startup failures to resolve aliases non-fatal;
• actually adding useful error messages in case we cannot resolve.

In the longer run, we should probably simply store room ids.

[Gnuxie]

Hello, I shouldn't really have to come back here but

making startup failures to resolve aliases non-fatal;

This is intentionally fatal. If you start mjolnir without the complete set of watched lists, you will undo any entries to m.room.server_acl in the protected rooms that were a result of policies contained in the now missing lists.

[Yoric]

That sounds problematic. What do you mean "undo"? Is this something that would be resolved next time we start Mjölnir (if it manages to resolve the aliases)?

Note that the alternative is not starting Mjölnir at all, which is pretty bad, since it also blocks users from fixing the issue in the first place.

[Gnuxie]

Some background to this issue. When you use !mjolnir watch #somelist:example.com Mjolnir will not resolve the alias, and store it directly into watched_lists. Mjolnir of course resolves the alias when it is time to fetch the rules for the list, and this will work most of the time. However what is happening and the reason people are complaining about this issue, is that the server that the alias is registered on is experiencing a temporary issue and is throwing out 500's or other errors at the time Mjolnir is starting up. And this is fatal because the server is required to resolve the alias.

That sounds problematic. What do you mean "undo"? Is this something that would be resolved next time we start Mjölnir (if it manages to resolve the aliases)?

Each policy list has rules that mjolnir fetches when it starts. Some of these rules will be of the type m.policy.rule.server. When Mjolnir fails to fetch the rules from a list within watched_list at startup (when it was "fine" before, but the server is having temporary outage), then Mjolnir will have an incomplete set of rules from the perspective of the user. Now the set of m.policy.rule.server rules Mjolnir has will no longer construct the same m.room.server_acl as the last time mjolnir was running, as it is now missing some of the server rules. If Mjolnir is then allowed to silently ignore the failure to resolve the list, it will apply this incomplete set of rules to every m.room.server_acl event in all of Mjolnir's protected rooms and effectively unban the servers (that were listed in the now missing m.policy.rule.server rules) from the protected rooms.

since it also blocks users from fixing the issue in the first place.

It allows the user to be acutely aware that there is an issue. The real way to prevent the issue is to ensure that aliases are not stored in watched_lists and resolved as soon as possible (ie as soon as they are parsed within the context of a command). Code should also be added to migrate any existing aliases stored in watched_lists by resolving and then overwriting them. But it should still be fatal if it is not possible to resolve and then overwrite the alias, and the user will then have the option to wait it out or manually edit the account data to remove the alias and replace it with the room id.

[Yoric]

All your points are valid and generally, I agree that crashing on startup is better than hobbling. However, if Mjölnir won't start at all, it's also not possible to migrate.

If Mjolnir is then allowed to silently ignore the failure to resolve the list

Note that this is not silent, there is now an error message during startup, it just won't block startup.