MRF Policies: Return a {:reject, reason} instead of {:reject, nil}

lib/pleroma/web/activity_pub/mrf/anti_followbot_policy.ex

@@ -60,7 +60,7 @@ def filter(%{"type" => "Follow", "actor" => actor_id} = message) do
     if score < 0.8 do
       {:ok, message}
     else
-      {:reject, nil}
+      {:reject, "[AntiFollowbotPolicy] Scored #{actor_id} as #{score}"}
     end
   end
 

lib/pleroma/web/activity_pub/mrf/anti_link_spam_policy.ex

@@ -39,14 +39,13 @@ def filter(%{"type" => "Create", "actor" => actor, "object" => object} = message
         {:ok, message}
 
       {:old_user, false} ->
-        {:reject, nil}
+        {:reject, "[AntiLinkSpamPolicy] User has no posts nor followers"}
 
       {:error, _} ->
-        {:reject, nil}
+        {:reject, "[AntiLinkSpamPolicy] Failed to get or fetch user by ap_id"}
 
       e ->
-        Logger.warn("[MRF anti-link-spam] WTF: unhandled error #{inspect(e)}")
+        {:reject, "[AntiLinkSpamPolicy] Unhandled error #{inspect(e)}"}
-        {:reject, nil}
     end
   end
 

lib/pleroma/web/activity_pub/mrf/hellthread_policy.ex

@@ -43,7 +43,7 @@ defp delist_message(message, _threshold), do: {:ok, message}
   defp reject_message(message, threshold) when threshold > 0 do
     with {_, recipients} <- get_recipient_count(message) do
       if recipients > threshold do
-        {:reject, nil}
+        {:reject, "[HellthreadPolicy] #{recipients} recipients is over the limit of #{threshold}"}
       else
         {:ok, message}
       end
@@ -87,7 +87,7 @@ def filter(%{"type" => "Create", "object" => %{"type" => object_type}} = message
          {:ok, message} <- delist_message(message, delist_threshold) do
       {:ok, message}
     else
-      _e -> {:reject, nil}
+      e -> e
     end
   end
 

lib/pleroma/web/activity_pub/mrf/keyword_policy.ex

@@ -24,7 +24,7 @@ defp check_reject(%{"object" => %{"content" => content, "summary" => summary}} =
     if Enum.any?(Pleroma.Config.get([:mrf_keyword, :reject]), fn pattern ->
          string_matches?(content, pattern) or string_matches?(summary, pattern)
        end) do
-      {:reject, nil}
+      {:reject, "[KeywordPolicy] Matches with rejected keyword"}
     else
       {:ok, message}
     end
@@ -89,8 +89,9 @@ def filter(%{"type" => "Create", "object" => %{"content" => _content}} = message
          {:ok, message} <- check_replace(message) do
       {:ok, message}
     else
-      _e ->
+      {:reject, nil} -> {:reject, "[KeywordPolicy] "}
-        {:reject, nil}
+      {:reject, _} = e -> e
+      _e -> {:reject, "[KeywordPolicy] "}
     end
   end
 

lib/pleroma/web/activity_pub/mrf/mention_policy.ex

@@ -12,8 +12,9 @@ def filter(%{"type" => "Create"} = message) do
     reject_actors = Pleroma.Config.get([:mrf_mention, :actors], [])
     recipients = (message["to"] || []) ++ (message["cc"] || [])
 
-    if Enum.any?(recipients, fn recipient -> Enum.member?(reject_actors, recipient) end) do
+    if rejected_mention =
-      {:reject, nil}
+         Enum.find(recipients, fn recipient -> Enum.member?(reject_actors, recipient) end) do
+      {:reject, "[MentionPolicy] Rejected for mention of #{rejected_mention}"}
     else
       {:ok, message}
     end

lib/pleroma/web/activity_pub/mrf/object_age_policy.ex

@@ -28,7 +28,7 @@ defp check_date(%{"object" => %{"published" => published}} = message) do
 
   defp check_reject(message, actions) do
     if :reject in actions do
-      {:reject, nil}
+      {:reject, "[ObjectAgePolicy]"}
     else
       {:ok, message}
     end
@@ -47,9 +47,8 @@ defp check_delist(message, actions) do
 
         {:ok, message}
       else
-        # Unhandleable error: somebody is messing around, just drop the message.
         _e ->
-          {:reject, nil}
+          {:reject, "[ObjectAgePolicy] Unhandled error"}
       end
     else
       {:ok, message}
@@ -69,9 +68,8 @@ defp check_strip_followers(message, actions) do
 
         {:ok, message}
       else
-        # Unhandleable error: somebody is messing around, just drop the message.
         _e ->
-          {:reject, nil}
+          {:reject, "[ObjectAgePolicy] Unhandled error"}
       end
     else
       {:ok, message}

lib/pleroma/web/activity_pub/mrf/reject_non_public.ex

@@ -38,7 +38,7 @@ def filter(%{"type" => "Create"} = object) do
         {:ok, object}
 
       true ->
-        {:reject, nil}
+        {:reject, "[RejectNonPublic] visibility: #{visibility}"}
     end
   end
 

lib/pleroma/web/activity_pub/mrf/simple_policy.ex

@@ -21,7 +21,7 @@ defp check_accept(%{host: actor_host} = _actor_info, object) do
       accepts == [] -> {:ok, object}
       actor_host == Config.get([Pleroma.Web.Endpoint, :url, :host]) -> {:ok, object}
       MRF.subdomain_match?(accepts, actor_host) -> {:ok, object}
-      true -> {:reject, nil}
+      true -> {:reject, "[SimplePolicy] host not in accept list"}
     end
   end
 
@@ -31,7 +31,7 @@ defp check_reject(%{host: actor_host} = _actor_info, object) do
       |> MRF.subdomains_regex()
 
     if MRF.subdomain_match?(rejects, actor_host) do
-      {:reject, nil}
+      {:reject, "[SimplePolicy] host in reject list"}
     else
       {:ok, object}
     end
@@ -114,7 +114,7 @@ defp check_report_removal(%{host: actor_host} = _actor_info, %{"type" => "Flag"}
       |> MRF.subdomains_regex()
 
     if MRF.subdomain_match?(report_removal, actor_host) do
-      {:reject, nil}
+      {:reject, "[SimplePolicy] host in report_removal list"}
     else
       {:ok, object}
     end
@@ -159,7 +159,7 @@ def filter(%{"type" => "Delete", "actor" => actor} = object) do
       |> MRF.subdomains_regex()
 
     if MRF.subdomain_match?(reject_deletes, actor_host) do
-      {:reject, nil}
+      {:reject, "[SimplePolicy] host in reject_deletes list"}
     else
       {:ok, object}
     end
@@ -177,7 +177,9 @@ def filter(%{"actor" => actor} = object) do
          {:ok, object} <- check_report_removal(actor_info, object) do
       {:ok, object}
     else
-      _e -> {:reject, nil}
+      {:reject, nil} -> {:reject, "[SimplePolicy]"}
+      {:reject, _} = e -> e
+      _ -> {:reject, "[SimplePolicy]"}
     end
   end
 
@@ -191,7 +193,9 @@ def filter(%{"id" => actor, "type" => obj_type} = object)
          {:ok, object} <- check_banner_removal(actor_info, object) do
       {:ok, object}
     else
-      _e -> {:reject, nil}
+      {:reject, nil} -> {:reject, "[SimplePolicy]"}
+      {:reject, _} = e -> e
+      _ -> {:reject, "[SimplePolicy]"}
     end
   end
 

lib/pleroma/web/activity_pub/mrf/tag_policy.ex

@@ -134,12 +134,13 @@ defp process_tag(
     if user.local == true do
       {:ok, message}
     else
-      {:reject, nil}
+      {:reject,
+       "[TagPolicy] Follow from #{actor} tagged with mrf_tag:disable-remote-subscription"}
     end
   end
 
-  defp process_tag("mrf_tag:disable-any-subscription", %{"type" => "Follow"}),
+  defp process_tag("mrf_tag:disable-any-subscription", %{"type" => "Follow", "actor" => actor}),
-    do: {:reject, nil}
+    do: {:reject, "[TagPolicy] Follow from #{actor} tagged with mrf_tag:disable-any-subscription"}
 
   defp process_tag(_, message), do: {:ok, message}
 

lib/pleroma/web/activity_pub/mrf/user_allow_list_policy.ex

@@ -14,7 +14,7 @@ defp filter_by_list(%{"actor" => actor} = object, allow_list) do
     if actor in allow_list do
       {:ok, object}
     else
-      {:reject, nil}
+      {:reject, "[UserAllowListPolicy] #{actor} not in the list"}
     end
   end
 

lib/pleroma/web/activity_pub/mrf/vocabulary_policy.ex

@@ -11,22 +11,26 @@ def filter(%{"type" => "Undo", "object" => child_message} = message) do
     with {:ok, _} <- filter(child_message) do
       {:ok, message}
     else
-      {:reject, nil} ->
+      {:reject, _} = e -> e
-        {:reject, nil}
     end
   end
 
   def filter(%{"type" => message_type} = message) do
     with accepted_vocabulary <- Pleroma.Config.get([:mrf_vocabulary, :accept]),
          rejected_vocabulary <- Pleroma.Config.get([:mrf_vocabulary, :reject]),
-         true <-
+         {_, true} <-
-           Enum.empty?(accepted_vocabulary) || Enum.member?(accepted_vocabulary, message_type),
+           {:accepted,
-         false <-
+            Enum.empty?(accepted_vocabulary) || Enum.member?(accepted_vocabulary, message_type)},
-           length(rejected_vocabulary) > 0 && Enum.member?(rejected_vocabulary, message_type),
+         {_, false} <-
+           {:rejected,
+            length(rejected_vocabulary) > 0 && Enum.member?(rejected_vocabulary, message_type)},
          {:ok, _} <- filter(message["object"]) do
       {:ok, message}
     else
-      _ -> {:reject, nil}
+      {:reject, _} = e -> e
+      {:accepted, _} -> {:reject, "[VocabularyPolicy] #{message_type} not in accept list"}
+      {:rejected, _} -> {:reject, "[VocabularyPolicy] #{message_type} in reject list"}
+      _ -> {:reject, "[VocabularyPolicy]"}
     end
   end
 

test/web/activity_pub/mrf/anti_followbot_policy_test.exs

@@ -21,7 +21,7 @@ test "matches followbots by nickname" do
         "id" => "https://example.com/activities/1234"
       }
 
-      {:reject, nil} = AntiFollowbotPolicy.filter(message)
+      assert {:reject, "[AntiFollowbotPolicy]" <> _} = AntiFollowbotPolicy.filter(message)
     end
 
     test "matches followbots by display name" do
@@ -36,7 +36,7 @@ test "matches followbots by display name" do
         "id" => "https://example.com/activities/1234"
       }
 
-      {:reject, nil} = AntiFollowbotPolicy.filter(message)
+      assert {:reject, "[AntiFollowbotPolicy]" <> _} = AntiFollowbotPolicy.filter(message)
     end
   end
 

test/web/activity_pub/mrf/hellthread_policy_test.exs

@@ -50,7 +50,8 @@ test "rejects the message if the recipient count is above reject_threshold", %{
     } do
       Pleroma.Config.put([:mrf_hellthread], %{delist_threshold: 0, reject_threshold: 2})
 
-      {:reject, nil} = filter(message)
+      assert {:reject, "[HellthreadPolicy] 3 recipients is over the limit of 2"} ==
+               filter(message)
     end
 
     test "does not reject the message if the recipient count is below reject_threshold", %{

test/web/activity_pub/mrf/keyword_policy_test.exs

@@ -25,7 +25,8 @@ test "rejects if string matches in content" do
         }
       }
 
-      assert {:reject, nil} == KeywordPolicy.filter(message)
+      assert {:reject, "[KeywordPolicy] Matches with rejected keyword"} =
+               KeywordPolicy.filter(message)
     end
 
     test "rejects if string matches in summary" do
@@ -39,7 +40,8 @@ test "rejects if string matches in summary" do
         }
       }
 
-      assert {:reject, nil} == KeywordPolicy.filter(message)
+      assert {:reject, "[KeywordPolicy] Matches with rejected keyword"} =
+               KeywordPolicy.filter(message)
     end
 
     test "rejects if regex matches in content" do
@@ -55,7 +57,8 @@ test "rejects if regex matches in content" do
                    }
                  }
 
-                 {:reject, nil} == KeywordPolicy.filter(message)
+                 {:reject, "[KeywordPolicy] Matches with rejected keyword"} ==
+                   KeywordPolicy.filter(message)
                end)
     end
 
@@ -72,7 +75,8 @@ test "rejects if regex matches in summary" do
                    }
                  }
 
-                 {:reject, nil} == KeywordPolicy.filter(message)
+                 {:reject, "[KeywordPolicy] Matches with rejected keyword"} ==
+                   KeywordPolicy.filter(message)
                end)
     end
   end

test/web/activity_pub/mrf/mention_policy_test.exs

@@ -76,7 +76,8 @@ test "to" do
         "to" => ["https://example.com/blocked"]
       }
 
-      assert MentionPolicy.filter(message) == {:reject, nil}
+      assert MentionPolicy.filter(message) ==
+               {:reject, "[MentionPolicy] Rejected for mention of https://example.com/blocked"}
     end
 
     test "cc" do
@@ -88,7 +89,8 @@ test "cc" do
         "cc" => ["https://example.com/blocked"]
       }
 
-      assert MentionPolicy.filter(message) == {:reject, nil}
+      assert MentionPolicy.filter(message) ==
+               {:reject, "[MentionPolicy] Rejected for mention of https://example.com/blocked"}
     end
   end
 end

test/web/activity_pub/mrf/reject_non_public_test.exs

@@ -64,7 +64,7 @@ test "it's rejected when addrer of message in the follower addresses of user and
       }
 
       Pleroma.Config.put([:mrf_rejectnonpublic, :allow_followersonly], false)
-      assert {:reject, nil} = RejectNonPublic.filter(message)
+      assert {:reject, _} = RejectNonPublic.filter(message)
     end
   end
 
@@ -94,7 +94,7 @@ test "it's reject when direct messages aren't allow" do
       }
 
       Pleroma.Config.put([:mrf_rejectnonpublic, :allow_direct], false)
-      assert {:reject, nil} = RejectNonPublic.filter(message)
+      assert {:reject, _} = RejectNonPublic.filter(message)
     end
   end
 end

test/web/activity_pub/mrf/simple_policy_test.exs

@@ -124,7 +124,7 @@ test "has a matching host" do
       report_message = build_report_message()
       local_message = build_local_message()
 
-      assert SimplePolicy.filter(report_message) == {:reject, nil}
+      assert {:reject, _} = SimplePolicy.filter(report_message)
       assert SimplePolicy.filter(local_message) == {:ok, local_message}
     end
 
@@ -133,7 +133,7 @@ test "match with wildcard domain" do
       report_message = build_report_message()
       local_message = build_local_message()
 
-      assert SimplePolicy.filter(report_message) == {:reject, nil}
+      assert {:reject, _} = SimplePolicy.filter(report_message)
       assert SimplePolicy.filter(local_message) == {:ok, local_message}
     end
   end
@@ -241,7 +241,7 @@ test "activity has a matching host" do
 
       remote_message = build_remote_message()
 
-      assert SimplePolicy.filter(remote_message) == {:reject, nil}
+      assert {:reject, _} = SimplePolicy.filter(remote_message)
     end
 
     test "activity matches with wildcard domain" do
@@ -249,7 +249,7 @@ test "activity matches with wildcard domain" do
 
       remote_message = build_remote_message()
 
-      assert SimplePolicy.filter(remote_message) == {:reject, nil}
+      assert {:reject, _} = SimplePolicy.filter(remote_message)
     end
 
     test "actor has a matching host" do
@@ -257,7 +257,7 @@ test "actor has a matching host" do
 
       remote_user = build_remote_user()
 
-      assert SimplePolicy.filter(remote_user) == {:reject, nil}
+      assert {:reject, _} = SimplePolicy.filter(remote_user)
     end
   end
 
@@ -279,7 +279,7 @@ test "is not empty but activity doesn't have a matching host" do
       remote_message = build_remote_message()
 
       assert SimplePolicy.filter(local_message) == {:ok, local_message}
-      assert SimplePolicy.filter(remote_message) == {:reject, nil}
+      assert {:reject, _} = SimplePolicy.filter(remote_message)
     end
 
     test "activity has a matching host" do
@@ -429,7 +429,7 @@ test "it accepts deletions even from non-whitelisted servers" do
     test "it rejects the deletion" do
       deletion_message = build_remote_deletion_message()
 
-      assert SimplePolicy.filter(deletion_message) == {:reject, nil}
+      assert {:reject, _} = SimplePolicy.filter(deletion_message)
     end
   end
 
@@ -439,7 +439,7 @@ test "it rejects the deletion" do
     test "it rejects the deletion" do
       deletion_message = build_remote_deletion_message()
 
-      assert SimplePolicy.filter(deletion_message) == {:reject, nil}
+      assert {:reject, _} = SimplePolicy.filter(deletion_message)
     end
   end
 

test/web/activity_pub/mrf/tag_policy_test.exs

@@ -12,8 +12,8 @@ defmodule Pleroma.Web.ActivityPub.MRF.TagPolicyTest do
   describe "mrf_tag:disable-any-subscription" do
     test "rejects message" do
       actor = insert(:user, tags: ["mrf_tag:disable-any-subscription"])
-      message = %{"object" => actor.ap_id, "type" => "Follow"}
+      message = %{"object" => actor.ap_id, "type" => "Follow", "actor" => actor.ap_id}
-      assert {:reject, nil} = TagPolicy.filter(message)
+      assert {:reject, _} = TagPolicy.filter(message)
     end
   end
 
@@ -22,7 +22,7 @@ test "rejects non-local follow requests" do
       actor = insert(:user, tags: ["mrf_tag:disable-remote-subscription"])
       follower = insert(:user, tags: ["mrf_tag:disable-remote-subscription"], local: false)
       message = %{"object" => actor.ap_id, "type" => "Follow", "actor" => follower.ap_id}
-      assert {:reject, nil} = TagPolicy.filter(message)
+      assert {:reject, _} = TagPolicy.filter(message)
     end
 
     test "allows non-local follow requests" do

test/web/activity_pub/mrf/user_allowlist_policy_test.exs

@@ -26,6 +26,6 @@ test "rejected if allow list isn't empty and user not in allow list" do
     actor = insert(:user)
     Pleroma.Config.put([:mrf_user_allowlist], %{"localhost" => ["test-ap-id"]})
     message = %{"actor" => actor.ap_id}
-    assert UserAllowListPolicy.filter(message) == {:reject, nil}
+    assert {:reject, _} = UserAllowListPolicy.filter(message)
   end
 end

test/web/activity_pub/mrf/vocabulary_policy_test.exs

@@ -46,7 +46,7 @@ test "it does not accept disallowed child objects" do
         }
       }
 
-      {:reject, nil} = VocabularyPolicy.filter(message)
+      {:reject, _} = VocabularyPolicy.filter(message)
     end
 
     test "it does not accept disallowed parent types" do
@@ -60,7 +60,7 @@ test "it does not accept disallowed parent types" do
         }
       }
 
-      {:reject, nil} = VocabularyPolicy.filter(message)
+      {:reject, _} = VocabularyPolicy.filter(message)
     end
   end
 
@@ -75,7 +75,7 @@ test "it rejects based on parent activity type" do
         "object" => "whatever"
       }
 
-      {:reject, nil} = VocabularyPolicy.filter(message)
+      {:reject, _} = VocabularyPolicy.filter(message)
     end
 
     test "it rejects based on child object type" do
@@ -89,7 +89,7 @@ test "it rejects based on child object type" do
         }
       }
 
-      {:reject, nil} = VocabularyPolicy.filter(message)
+      {:reject, _} = VocabularyPolicy.filter(message)
     end
 
     test "it passes through objects that aren't disallowed" do