we should validate server ACLs against a schema to stop people uploading mangled ones and locking themselves out

[ara4n]

No description provided.

[richvdh]

The problem with this is that there's not much to validate. As per the spec, all the fields are optional.

Of course, a server could reject attempts to set an ACL which would lock itself out. But that's different from validating against a schema.

(Incidentally, event validation in general is the topic of MSC1646, which has stalled somewhat)

[turt2live]

in this case the user doubly-nested their ACL:

{
  "content": {
     "content": {
        "allow": ["*"]
     }
  }
}

[richvdh]

yup, so it was valid, but missing the allow field.

(in general, I feel like we missed a few tricks when we specced m.room.server_acl. The fact that subdomains aren't included implicitly is probably more annoying than the fact the default value for allow is <empty> rather than *.)

[turt2live]

we could probably do some simple sanity validation on the event for the common cases: If the object is empty or has at least one of the spec fields, allow it. Would prevent accidental setting of ACLs with nested content (content is not a valid field, the object is non-empty, and there are no other fields from the spec in the object) and similar accidents. Empty objects and objects with at least one of the fields would be valid enough to send through though.

[richvdh]

I feel like just checking that you're not banning yourself (as per #4042) would cover 95% of the problem here.