Pushrule tweaks, make `pattern` non-optional on `EventMatchCondition` (#2918)

This should fix https://github.com/matrix-org/dendrite/issues/2882 (Tested with FluffyChat 1.7.1) Also adds tests that the predefined push rules (as per the spec) is what we have in Dendrite.
2022-12-23 12:52:47 +01:00 · 2022-12-23 12:52:47 +01:00 · f47515e38b
parent 5eed31fea3
commit f47515e38b
12 changed files with 210 additions and 110 deletions
--- a/internal/pushrules/condition.go
+++ b/internal/pushrules/condition.go
@ -14,7 +14,7 @@ type Condition struct {

 	// Pattern indicates the value pattern that must match. Required
 	// for EventMatchCondition.
-	Pattern string `json:"pattern,omitempty"`
+	Pattern *string `json:"pattern,omitempty"`

 	// Is indicates the condition that must be fulfilled. Required for
 	// RoomMemberCountCondition.
--- a/internal/pushrules/default_content.go
+++ b/internal/pushrules/default_content.go
@ -15,13 +15,7 @@ func mRuleContainsUserNameDefinition(localpart string) *Rule {
 		RuleID:  MRuleContainsUserName,
 		Default: true,
 		Enabled: true,
-		Pattern: localpart,
-		Conditions: []*Condition{
-			{
-				Kind: EventMatchCondition,
-				Key:  "content.body",
-			},
-		},
+		Pattern: &localpart,
 		Actions: []*Action{
 			{Kind: NotifyAction},
 			{
@ -32,7 +26,6 @@ func mRuleContainsUserNameDefinition(localpart string) *Rule {
 			{
 				Kind:  SetTweakAction,
 				Tweak: HighlightTweak,
-				Value: true,
 			},
 		},
 	}
--- a/internal/pushrules/default_override.go
+++ b/internal/pushrules/default_override.go
@ -22,15 +22,15 @@ const (
 	MRuleTombstone           = ".m.rule.tombstone"
 	MRuleRoomNotif           = ".m.rule.roomnotif"
 	MRuleReaction            = ".m.rule.reaction"
+	MRuleRoomACLs            = ".m.rule.room.server_acl"
 )

 var (
 	mRuleMasterDefinition = Rule{
-		RuleID:     MRuleMaster,
-		Default:    true,
-		Enabled:    false,
-		Conditions: []*Condition{},
-		Actions:    []*Action{{Kind: DontNotifyAction}},
+		RuleID:  MRuleMaster,
+		Default: true,
+		Enabled: false,
+		Actions: []*Action{{Kind: DontNotifyAction}},
 	}
 	mRuleSuppressNoticesDefinition = Rule{
 		RuleID:  MRuleSuppressNotices,
@ -40,7 +40,7 @@ var (
 			{
 				Kind:    EventMatchCondition,
 				Key:     "content.msgtype",
-				Pattern: "m.notice",
+				Pattern: pointer("m.notice"),
 			},
 		},
 		Actions: []*Action{{Kind: DontNotifyAction}},
@ -53,7 +53,7 @@ var (
 			{
 				Kind:    EventMatchCondition,
 				Key:     "type",
-				Pattern: "m.room.member",
+				Pattern: pointer("m.room.member"),
 			},
 		},
 		Actions: []*Action{{Kind: DontNotifyAction}},
@ -73,7 +73,6 @@ var (
 			{
 				Kind:  SetTweakAction,
 				Tweak: HighlightTweak,
-				Value: true,
 			},
 		},
 	}
@ -85,12 +84,12 @@ var (
 			{
 				Kind:    EventMatchCondition,
 				Key:     "type",
-				Pattern: "m.room.tombstone",
+				Pattern: pointer("m.room.tombstone"),
 			},
 			{
 				Kind:    EventMatchCondition,
 				Key:     "state_key",
-				Pattern: "",
+				Pattern: pointer(""),
 			},
 		},
 		Actions: []*Action{
@ -98,10 +97,27 @@ var (
 			{
 				Kind:  SetTweakAction,
 				Tweak: HighlightTweak,
-				Value: true,
 			},
 		},
 	}
+	mRuleACLsDefinition = Rule{
+		RuleID:  MRuleRoomACLs,
+		Default: true,
+		Enabled: true,
+		Conditions: []*Condition{
+			{
+				Kind:    EventMatchCondition,
+				Key:     "type",
+				Pattern: pointer("m.room.server_acl"),
+			},
+			{
+				Kind:    EventMatchCondition,
+				Key:     "state_key",
+				Pattern: pointer(""),
+			},
+		},
+		Actions: []*Action{},
+	}
 	mRuleRoomNotifDefinition = Rule{
 		RuleID:  MRuleRoomNotif,
 		Default: true,
@ -110,7 +126,7 @@ var (
 			{
 				Kind:    EventMatchCondition,
 				Key:     "content.body",
-				Pattern: "@room",
+				Pattern: pointer("@room"),
 			},
 			{
 				Kind: SenderNotificationPermissionCondition,
@ -122,7 +138,6 @@ var (
 			{
 				Kind:  SetTweakAction,
 				Tweak: HighlightTweak,
-				Value: true,
 			},
 		},
 	}
@ -134,7 +149,7 @@ var (
 			{
 				Kind:    EventMatchCondition,
 				Key:     "type",
-				Pattern: "m.reaction",
+				Pattern: pointer("m.reaction"),
 			},
 		},
 		Actions: []*Action{
@ -152,17 +167,17 @@ func mRuleInviteForMeDefinition(userID string) *Rule {
 			{
 				Kind:    EventMatchCondition,
 				Key:     "type",
-				Pattern: "m.room.member",
+				Pattern: pointer("m.room.member"),
 			},
 			{
 				Kind:    EventMatchCondition,
 				Key:     "content.membership",
-				Pattern: "invite",
+				Pattern: pointer("invite"),
 			},
 			{
 				Kind:    EventMatchCondition,
 				Key:     "state_key",
-				Pattern: userID,
+				Pattern: pointer(userID),
 			},
 		},
 		Actions: []*Action{
@ -172,11 +187,6 @@ func mRuleInviteForMeDefinition(userID string) *Rule {
 				Tweak: SoundTweak,
 				Value: "default",
 			},
-			{
-				Kind:  SetTweakAction,
-				Tweak: HighlightTweak,
-				Value: false,
-			},
 		},
 	}
 }
--- a/internal/pushrules/default_pushrules_test.go
+++ b/internal/pushrules/default_pushrules_test.go
@ -0,0 +1,111 @@
+package pushrules
+
+import (
+	"encoding/json"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+// Tests that the pre-defined rules as of
+// https://spec.matrix.org/v1.4/client-server-api/#predefined-rules
+// are correct
+func TestDefaultRules(t *testing.T) {
+	type testCase struct {
+		name       string
+		inputBytes []byte
+		want       Rule
+	}
+
+	testCases := []testCase{
+		// Default override rules
+		{
+			name:       ".m.rule.master",
+			inputBytes: []byte(`{"rule_id":".m.rule.master","default":true,"enabled":false,"actions":["dont_notify"]}`),
+			want:       mRuleMasterDefinition,
+		},
+		{
+			name:       ".m.rule.suppress_notices",
+			inputBytes: []byte(`{"rule_id":".m.rule.suppress_notices","default":true,"enabled":true,"conditions":[{"kind":"event_match","key":"content.msgtype","pattern":"m.notice"}],"actions":["dont_notify"]}`),
+			want:       mRuleSuppressNoticesDefinition,
+		},
+		{
+			name:       ".m.rule.invite_for_me",
+			inputBytes: []byte(`{"rule_id":".m.rule.invite_for_me","default":true,"enabled":true,"conditions":[{"kind":"event_match","key":"type","pattern":"m.room.member"},{"kind":"event_match","key":"content.membership","pattern":"invite"},{"kind":"event_match","key":"state_key","pattern":"@test:localhost"}],"actions":["notify",{"set_tweak":"sound","value":"default"}]}`),
+			want:       *mRuleInviteForMeDefinition("@test:localhost"),
+		},
+		{
+			name:       ".m.rule.member_event",
+			inputBytes: []byte(`{"rule_id":".m.rule.member_event","default":true,"enabled":true,"conditions":[{"kind":"event_match","key":"type","pattern":"m.room.member"}],"actions":["dont_notify"]}`),
+			want:       mRuleMemberEventDefinition,
+		},
+		{
+			name:       ".m.rule.contains_display_name",
+			inputBytes: []byte(`{"rule_id":".m.rule.contains_display_name","default":true,"enabled":true,"conditions":[{"kind":"contains_display_name"}],"actions":["notify",{"set_tweak":"sound","value":"default"},{"set_tweak":"highlight"}]}`),
+			want:       mRuleContainsDisplayNameDefinition,
+		},
+		{
+			name:       ".m.rule.tombstone",
+			inputBytes: []byte(`{"rule_id":".m.rule.tombstone","default":true,"enabled":true,"conditions":[{"kind":"event_match","key":"type","pattern":"m.room.tombstone"},{"kind":"event_match","key":"state_key","pattern":""}],"actions":["notify",{"set_tweak":"highlight"}]}`),
+			want:       mRuleTombstoneDefinition,
+		},
+		{
+			name:       ".m.rule.room.server_acl",
+			inputBytes: []byte(`{"rule_id":".m.rule.room.server_acl","default":true,"enabled":true,"conditions":[{"kind":"event_match","key":"type","pattern":"m.room.server_acl"},{"kind":"event_match","key":"state_key","pattern":""}],"actions":[]}`),
+			want:       mRuleACLsDefinition,
+		},
+		{
+			name:       ".m.rule.roomnotif",
+			inputBytes: []byte(`{"rule_id":".m.rule.roomnotif","default":true,"enabled":true,"conditions":[{"kind":"event_match","key":"content.body","pattern":"@room"},{"kind":"sender_notification_permission","key":"room"}],"actions":["notify",{"set_tweak":"highlight"}]}`),
+			want:       mRuleRoomNotifDefinition,
+		},
+		// Default content rules
+		{
+			name:       ".m.rule.contains_user_name",
+			inputBytes: []byte(`{"rule_id":".m.rule.contains_user_name","default":true,"enabled":true,"actions":["notify",{"set_tweak":"sound","value":"default"},{"set_tweak":"highlight"}],"pattern":"myLocalUser"}`),
+			want:       *mRuleContainsUserNameDefinition("myLocalUser"),
+		},
+		// default underride rules
+		{
+			name:       ".m.rule.call",
+			inputBytes: []byte(`{"rule_id":".m.rule.call","default":true,"enabled":true,"conditions":[{"kind":"event_match","key":"type","pattern":"m.call.invite"}],"actions":["notify",{"set_tweak":"sound","value":"ring"}]}`),
+			want:       mRuleCallDefinition,
+		},
+		{
+			name:       ".m.rule.encrypted_room_one_to_one",
+			inputBytes: []byte(`{"rule_id":".m.rule.encrypted_room_one_to_one","default":true,"enabled":true,"conditions":[{"kind":"room_member_count","is":"2"},{"kind":"event_match","key":"type","pattern":"m.room.encrypted"}],"actions":["notify",{"set_tweak":"sound","value":"default"}]}`),
+			want:       mRuleEncryptedRoomOneToOneDefinition,
+		},
+		{
+			name:       ".m.rule.room_one_to_one",
+			inputBytes: []byte(`{"rule_id":".m.rule.room_one_to_one","default":true,"enabled":true,"conditions":[{"kind":"room_member_count","is":"2"},{"kind":"event_match","key":"type","pattern":"m.room.message"}],"actions":["notify",{"set_tweak":"sound","value":"default"}]}`),
+			want:       mRuleRoomOneToOneDefinition,
+		},
+		{
+			name:       ".m.rule.message",
+			inputBytes: []byte(`{"rule_id":".m.rule.message","default":true,"enabled":true,"conditions":[{"kind":"event_match","key":"type","pattern":"m.room.message"}],"actions":["notify"]}`),
+			want:       mRuleMessageDefinition,
+		},
+		{
+			name:       ".m.rule.encrypted",
+			inputBytes: []byte(`{"rule_id":".m.rule.encrypted","default":true,"enabled":true,"conditions":[{"kind":"event_match","key":"type","pattern":"m.room.encrypted"}],"actions":["notify"]}`),
+			want:       mRuleEncryptedDefinition,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			r := Rule{}
+			// unmarshal predefined push rules
+			err := json.Unmarshal(tc.inputBytes, &r)
+			assert.NoError(t, err)
+			assert.Equal(t, tc.want, r)
+
+			// and reverse it to check we get the expected result
+			got, err := json.Marshal(r)
+			assert.NoError(t, err)
+			assert.Equal(t, string(got), string(tc.inputBytes))
+		})
+
+	}
+}
--- a/internal/pushrules/default_underride.go
+++ b/internal/pushrules/default_underride.go
@ -25,7 +25,7 @@ var (
 			{
 				Kind:    EventMatchCondition,
 				Key:     "type",
-				Pattern: "m.call.invite",
+				Pattern: pointer("m.call.invite"),
 			},
 		},
 		Actions: []*Action{
@ -35,11 +35,6 @@ var (
 				Tweak: SoundTweak,
 				Value: "ring",
 			},
-			{
-				Kind:  SetTweakAction,
-				Tweak: HighlightTweak,
-				Value: false,
-			},
 		},
 	}
 	mRuleEncryptedRoomOneToOneDefinition = Rule{
@ -54,7 +49,7 @@ var (
 			{
 				Kind:    EventMatchCondition,
 				Key:     "type",
-				Pattern: "m.room.encrypted",
+				Pattern: pointer("m.room.encrypted"),
 			},
 		},
 		Actions: []*Action{
@ -64,11 +59,6 @@ var (
 				Tweak: SoundTweak,
 				Value: "default",
 			},
-			{
-				Kind:  SetTweakAction,
-				Tweak: HighlightTweak,
-				Value: false,
-			},
 		},
 	}
 	mRuleRoomOneToOneDefinition = Rule{
@ -83,20 +73,15 @@ var (
 			{
 				Kind:    EventMatchCondition,
 				Key:     "type",
-				Pattern: "m.room.message",
+				Pattern: pointer("m.room.message"),
 			},
 		},
 		Actions: []*Action{
 			{Kind: NotifyAction},
 			{
 				Kind:  SetTweakAction,
-				Tweak: HighlightTweak,
-				Value: false,
-			},
-			{
-				Kind:  SetTweakAction,
-				Tweak: HighlightTweak,
-				Value: false,
+				Tweak: SoundTweak,
+				Value: "default",
 			},
 		},
 	}
@ -108,16 +93,11 @@ var (
 			{
 				Kind:    EventMatchCondition,
 				Key:     "type",
-				Pattern: "m.room.message",
+				Pattern: pointer("m.room.message"),
 			},
 		},
 		Actions: []*Action{
 			{Kind: NotifyAction},
-			{
-				Kind:  SetTweakAction,
-				Tweak: HighlightTweak,
-				Value: false,
-			},
 		},
 	}
 	mRuleEncryptedDefinition = Rule{
@ -128,16 +108,11 @@ var (
 			{
 				Kind:    EventMatchCondition,
 				Key:     "type",
-				Pattern: "m.room.encrypted",
+				Pattern: pointer("m.room.encrypted"),
 			},
 		},
 		Actions: []*Action{
 			{Kind: NotifyAction},
-			{
-				Kind:  SetTweakAction,
-				Tweak: HighlightTweak,
-				Value: false,
-			},
 		},
 	}
 )
--- a/internal/pushrules/evaluate.go
+++ b/internal/pushrules/evaluate.go
@ -104,7 +104,10 @@ func ruleMatches(rule *Rule, kind Kind, event *gomatrixserverlib.Event, ec Evalu
 	case ContentKind:
 		// TODO: "These configure behaviour for (unencrypted) messages
 		// that match certain patterns." - Does that mean "content.body"?
-		return patternMatches("content.body", rule.Pattern, event)
+		if rule.Pattern == nil {
+			return false, nil
+		}
+		return patternMatches("content.body", *rule.Pattern, event)

 	case RoomKind:
 		return rule.RuleID == event.RoomID(), nil
@ -120,7 +123,10 @@ func ruleMatches(rule *Rule, kind Kind, event *gomatrixserverlib.Event, ec Evalu
 func conditionMatches(cond *Condition, event *gomatrixserverlib.Event, ec EvaluationContext) (bool, error) {
 	switch cond.Kind {
 	case EventMatchCondition:
-		return patternMatches(cond.Key, cond.Pattern, event)
+		if cond.Pattern == nil {
+			return false, fmt.Errorf("missing condition pattern")
+		}
+		return patternMatches(cond.Key, *cond.Pattern, event)

 	case ContainsDisplayNameCondition:
 		return patternMatches("content.body", ec.UserDisplayName(), event)
--- a/internal/pushrules/evaluate_test.go
+++ b/internal/pushrules/evaluate_test.go
@ -79,8 +79,8 @@ func TestRuleMatches(t *testing.T) {
 		{"underrideConditionMatch", UnderrideKind, Rule{Enabled: true}, `{}`, true},
 		{"underrideConditionNoMatch", UnderrideKind, Rule{Enabled: true, Conditions: []*Condition{{}}}, `{}`, false},

-		{"contentMatch", ContentKind, Rule{Enabled: true, Pattern: "b"}, `{"content":{"body":"abc"}}`, true},
-		{"contentNoMatch", ContentKind, Rule{Enabled: true, Pattern: "d"}, `{"content":{"body":"abc"}}`, false},
+		{"contentMatch", ContentKind, Rule{Enabled: true, Pattern: pointer("b")}, `{"content":{"body":"abc"}}`, true},
+		{"contentNoMatch", ContentKind, Rule{Enabled: true, Pattern: pointer("d")}, `{"content":{"body":"abc"}}`, false},

 		{"roomMatch", RoomKind, Rule{Enabled: true, RuleID: "!room@example.com"}, `{"room_id":"!room@example.com"}`, true},
 		{"roomNoMatch", RoomKind, Rule{Enabled: true, RuleID: "!room@example.com"}, `{"room_id":"!otherroom@example.com"}`, false},
@ -106,41 +106,44 @@ func TestConditionMatches(t *testing.T) {
 		Name      string
 		Cond      Condition
 		EventJSON string
-		Want      bool
+		WantMatch bool
+		WantErr   bool
 	}{
-		{"empty", Condition{}, `{}`, false},
-		{"empty", Condition{Kind: "unknownstring"}, `{}`, false},
+		{Name: "empty", Cond: Condition{}, EventJSON: `{}`, WantMatch: false, WantErr: false},
+		{Name: "empty", Cond: Condition{Kind: "unknownstring"}, EventJSON: `{}`, WantMatch: false, WantErr: false},

 		// Neither of these should match because `content` is not a full string match,
 		// and `content.body` is not a string value.
-		{"eventMatch", Condition{Kind: EventMatchCondition, Key: "content"}, `{"content":{}}`, false},
-		{"eventBodyMatch", Condition{Kind: EventMatchCondition, Key: "content.body", Is: "3"}, `{"content":{"body": 3}}`, false},
+		{Name: "eventMatch", Cond: Condition{Kind: EventMatchCondition, Key: "content", Pattern: pointer("")}, EventJSON: `{"content":{}}`, WantMatch: false, WantErr: false},
+		{Name: "eventBodyMatch", Cond: Condition{Kind: EventMatchCondition, Key: "content.body", Is: "3", Pattern: pointer("")}, EventJSON: `{"content":{"body": "3"}}`, WantMatch: false, WantErr: false},
+		{Name: "eventBodyMatch matches", Cond: Condition{Kind: EventMatchCondition, Key: "content.body", Pattern: pointer("world")}, EventJSON: `{"content":{"body": "hello world!"}}`, WantMatch: true, WantErr: false},
+		{Name: "EventMatch missing pattern", Cond: Condition{Kind: EventMatchCondition, Key: "content.body"}, EventJSON: `{"content":{"body": "hello world!"}}`, WantMatch: false, WantErr: true},

-		{"displayNameNoMatch", Condition{Kind: ContainsDisplayNameCondition}, `{"content":{"body":"something without displayname"}}`, false},
-		{"displayNameMatch", Condition{Kind: ContainsDisplayNameCondition}, `{"content":{"body":"hello Dear User, how are you?"}}`, true},
+		{Name: "displayNameNoMatch", Cond: Condition{Kind: ContainsDisplayNameCondition}, EventJSON: `{"content":{"body":"something without displayname"}}`, WantMatch: false, WantErr: false},
+		{Name: "displayNameMatch", Cond: Condition{Kind: ContainsDisplayNameCondition}, EventJSON: `{"content":{"body":"hello Dear User, how are you?"}}`, WantMatch: true, WantErr: false},

-		{"roomMemberCountLessNoMatch", Condition{Kind: RoomMemberCountCondition, Is: "<2"}, `{}`, false},
-		{"roomMemberCountLessMatch", Condition{Kind: RoomMemberCountCondition, Is: "<3"}, `{}`, true},
-		{"roomMemberCountLessEqualNoMatch", Condition{Kind: RoomMemberCountCondition, Is: "<=1"}, `{}`, false},
-		{"roomMemberCountLessEqualMatch", Condition{Kind: RoomMemberCountCondition, Is: "<=2"}, `{}`, true},
-		{"roomMemberCountEqualNoMatch", Condition{Kind: RoomMemberCountCondition, Is: "==1"}, `{}`, false},
-		{"roomMemberCountEqualMatch", Condition{Kind: RoomMemberCountCondition, Is: "==2"}, `{}`, true},
-		{"roomMemberCountGreaterEqualNoMatch", Condition{Kind: RoomMemberCountCondition, Is: ">=3"}, `{}`, false},
-		{"roomMemberCountGreaterEqualMatch", Condition{Kind: RoomMemberCountCondition, Is: ">=2"}, `{}`, true},
-		{"roomMemberCountGreaterNoMatch", Condition{Kind: RoomMemberCountCondition, Is: ">2"}, `{}`, false},
-		{"roomMemberCountGreaterMatch", Condition{Kind: RoomMemberCountCondition, Is: ">1"}, `{}`, true},
+		{Name: "roomMemberCountLessNoMatch", Cond: Condition{Kind: RoomMemberCountCondition, Is: "<2"}, EventJSON: `{}`, WantMatch: false, WantErr: false},
+		{Name: "roomMemberCountLessMatch", Cond: Condition{Kind: RoomMemberCountCondition, Is: "<3"}, EventJSON: `{}`, WantMatch: true, WantErr: false},
+		{Name: "roomMemberCountLessEqualNoMatch", Cond: Condition{Kind: RoomMemberCountCondition, Is: "<=1"}, EventJSON: `{}`, WantMatch: false, WantErr: false},
+		{Name: "roomMemberCountLessEqualMatch", Cond: Condition{Kind: RoomMemberCountCondition, Is: "<=2"}, EventJSON: `{}`, WantMatch: true, WantErr: false},
+		{Name: "roomMemberCountEqualNoMatch", Cond: Condition{Kind: RoomMemberCountCondition, Is: "==1"}, EventJSON: `{}`, WantMatch: false, WantErr: false},
+		{Name: "roomMemberCountEqualMatch", Cond: Condition{Kind: RoomMemberCountCondition, Is: "==2"}, EventJSON: `{}`, WantMatch: true, WantErr: false},
+		{Name: "roomMemberCountGreaterEqualNoMatch", Cond: Condition{Kind: RoomMemberCountCondition, Is: ">=3"}, EventJSON: `{}`, WantMatch: false, WantErr: false},
+		{Name: "roomMemberCountGreaterEqualMatch", Cond: Condition{Kind: RoomMemberCountCondition, Is: ">=2"}, EventJSON: `{}`, WantMatch: true, WantErr: false},
+		{Name: "roomMemberCountGreaterNoMatch", Cond: Condition{Kind: RoomMemberCountCondition, Is: ">2"}, EventJSON: `{}`, WantMatch: false, WantErr: false},
+		{Name: "roomMemberCountGreaterMatch", Cond: Condition{Kind: RoomMemberCountCondition, Is: ">1"}, EventJSON: `{}`, WantMatch: true, WantErr: false},

-		{"senderNotificationPermissionMatch", Condition{Kind: SenderNotificationPermissionCondition, Key: "powerlevel"}, `{"sender":"@poweruser:example.com"}`, true},
-		{"senderNotificationPermissionNoMatch", Condition{Kind: SenderNotificationPermissionCondition, Key: "powerlevel"}, `{"sender":"@nobody:example.com"}`, false},
+		{Name: "senderNotificationPermissionMatch", Cond: Condition{Kind: SenderNotificationPermissionCondition, Key: "powerlevel"}, EventJSON: `{"sender":"@poweruser:example.com"}`, WantMatch: true, WantErr: false},
+		{Name: "senderNotificationPermissionNoMatch", Cond: Condition{Kind: SenderNotificationPermissionCondition, Key: "powerlevel"}, EventJSON: `{"sender":"@nobody:example.com"}`, WantMatch: false, WantErr: false},
 	}
 	for _, tst := range tsts {
 		t.Run(tst.Name, func(t *testing.T) {
 			got, err := conditionMatches(&tst.Cond, mustEventFromJSON(t, tst.EventJSON), &fakeEvaluationContext{2})
-			if err != nil {
+			if err != nil && !tst.WantErr {
 				t.Fatalf("conditionMatches failed: %v", err)
 			}
-			if got != tst.Want {
-				t.Errorf("conditionMatches: got %v, want %v on %s", got, tst.Want, tst.Name)
+			if got != tst.WantMatch {
+				t.Errorf("conditionMatches: got %v, want %v on %s", got, tst.WantMatch, tst.Name)
 			}
 		})
 	}
--- a/internal/pushrules/pushrules.go
+++ b/internal/pushrules/pushrules.go
@ -36,18 +36,18 @@ type Rule struct {
 	// around. Required.
 	Enabled bool `json:"enabled"`

+	// Conditions provide the rule's conditions for OverrideKind and
+	// UnderrideKind. Not allowed for other kinds.
+	Conditions []*Condition `json:"conditions,omitempty"`
+
 	// Actions describe the desired outcome, should the rule
 	// match. Required.
 	Actions []*Action `json:"actions"`

-	// Conditions provide the rule's conditions for OverrideKind and
-	// UnderrideKind. Not allowed for other kinds.
-	Conditions []*Condition `json:"conditions"`
-
 	// Pattern is the body pattern to match for ContentKind. Required
 	// for that kind. The interpretation is the same as that of
 	// Condition.Pattern.
-	Pattern string `json:"pattern"`
+	Pattern *string `json:"pattern,omitempty"`
 }

 // Scope only has one valid value. See also AccountRuleSets.
--- a/internal/pushrules/util.go
+++ b/internal/pushrules/util.go
@ -128,3 +128,7 @@ func parseRoomMemberCountCondition(s string) (func(int) bool, error) {
 	b = int(v)
 	return cmp, nil
 }
+
+func pointer[t any](s t) *t {
+	return &s
+}
--- a/internal/pushrules/validate.go
+++ b/internal/pushrules/validate.go
@ -34,7 +34,10 @@ func ValidateRule(kind Kind, rule *Rule) []error {
 		}

 	case ContentKind:
-		if rule.Pattern == "" {
+		if rule.Pattern == nil {
+			errs = append(errs, fmt.Errorf("missing content rule pattern"))
+		}
+		if rule.Pattern != nil && *rule.Pattern == "" {
 			errs = append(errs, fmt.Errorf("missing content rule pattern"))
 		}

--- a/internal/pushrules/validate_test.go
+++ b/internal/pushrules/validate_test.go
@ -12,15 +12,16 @@ func TestValidateRuleNegatives(t *testing.T) {
 		Rule          Rule
 		WantErrString string
 	}{
-		{"emptyRuleID", OverrideKind, Rule{}, "invalid rule ID"},
-		{"invalidKind", Kind("something else"), Rule{}, "invalid rule kind"},
-		{"ruleIDBackslash", OverrideKind, Rule{RuleID: "#foo\\:example.com"}, "invalid rule ID"},
-		{"noActions", OverrideKind, Rule{}, "missing actions"},
-		{"invalidAction", OverrideKind, Rule{Actions: []*Action{{}}}, "invalid rule action kind"},
-		{"invalidCondition", OverrideKind, Rule{Conditions: []*Condition{{}}}, "invalid rule condition kind"},
-		{"overrideNoCondition", OverrideKind, Rule{}, "missing rule conditions"},
-		{"underrideNoCondition", UnderrideKind, Rule{}, "missing rule conditions"},
-		{"contentNoPattern", ContentKind, Rule{}, "missing content rule pattern"},
+		{Name: "emptyRuleID", Kind: OverrideKind, Rule: Rule{}, WantErrString: "invalid rule ID"},
+		{Name: "invalidKind", Kind: Kind("something else"), Rule: Rule{}, WantErrString: "invalid rule kind"},
+		{Name: "ruleIDBackslash", Kind: OverrideKind, Rule: Rule{RuleID: "#foo\\:example.com"}, WantErrString: "invalid rule ID"},
+		{Name: "noActions", Kind: OverrideKind, Rule: Rule{}, WantErrString: "missing actions"},
+		{Name: "invalidAction", Kind: OverrideKind, Rule: Rule{Actions: []*Action{{}}}, WantErrString: "invalid rule action kind"},
+		{Name: "invalidCondition", Kind: OverrideKind, Rule: Rule{Conditions: []*Condition{{}}}, WantErrString: "invalid rule condition kind"},
+		{Name: "overrideNoCondition", Kind: OverrideKind, Rule: Rule{}, WantErrString: "missing rule conditions"},
+		{Name: "underrideNoCondition", Kind: UnderrideKind, Rule: Rule{}, WantErrString: "missing rule conditions"},
+		{Name: "contentNoPattern", Kind: ContentKind, Rule: Rule{}, WantErrString: "missing content rule pattern"},
+		{Name: "contentEmptyPattern", Kind: ContentKind, Rule: Rule{Pattern: pointer("")}, WantErrString: "missing content rule pattern"},
 	}
 	for _, tst := range tsts {
 		t.Run(tst.Name, func(t *testing.T) {
--- a/userapi/consumers/roomserver_test.go
+++ b/userapi/consumers/roomserver_test.go
@ -81,11 +81,6 @@ func Test_evaluatePushRules(t *testing.T) {
 				wantAction:   pushrules.NotifyAction,
 				wantActions: []*pushrules.Action{
 					{Kind: pushrules.NotifyAction},
-					{
-						Kind:  pushrules.SetTweakAction,
-						Tweak: pushrules.HighlightTweak,
-						Value: false,
-					},
 				},
 			},
 			{
@ -103,7 +98,6 @@ func Test_evaluatePushRules(t *testing.T) {
 					{
 						Kind:  pushrules.SetTweakAction,
 						Tweak: pushrules.HighlightTweak,
-						Value: true,
 					},
 				},
 			},