diff --git a/changelog.d/notification-filter-nil-content.fix b/changelog.d/notification-filter-nil-content.fix new file mode 100644 index 000000000..56b3ad6d1 --- /dev/null +++ b/changelog.d/notification-filter-nil-content.fix @@ -0,0 +1 @@ +Do not crash when creating notifications for posts without content while the user has filters diff --git a/changelog.d/object-age-non-public.security b/changelog.d/object-age-non-public.security new file mode 100644 index 000000000..aee358d38 --- /dev/null +++ b/changelog.d/object-age-non-public.security @@ -0,0 +1 @@ +ObjectAgePolicy no longer makes old DMs and followers-only posts public diff --git a/changelog.d/rate-limiter-test-flake.skip b/changelog.d/rate-limiter-test-flake.skip new file mode 100644 index 000000000..e69de29bb diff --git a/lib/pleroma/notification.ex b/lib/pleroma/notification.ex index afd750823..6e4a97a25 100644 --- a/lib/pleroma/notification.ex +++ b/lib/pleroma/notification.ex @@ -821,6 +821,9 @@ defmodule Pleroma.Notification do object.data["actor"] == user.ap_id -> false + not is_binary(object.data["content"]) -> + false + not is_nil(regex = Pleroma.Filter.compose_regex(user, :re)) -> Regex.match?(regex, object.data["content"]) diff --git a/lib/pleroma/web/activity_pub/mrf/object_age_policy.ex b/lib/pleroma/web/activity_pub/mrf/object_age_policy.ex index 00f4cf84b..25ae02f60 100644 --- a/lib/pleroma/web/activity_pub/mrf/object_age_policy.ex +++ b/lib/pleroma/web/activity_pub/mrf/object_age_policy.ex @@ -36,16 +36,34 @@ defmodule Pleroma.Web.ActivityPub.MRF.ObjectAgePolicy do end end + @spec delete_and_count(list(), term()) :: {integer(), list()} + defp delete_and_count(list, element), do: delete_and_count(list, element, {0, [], list}) + + defp delete_and_count([], _element, {0, _nlist, olist}), do: {0, olist} + defp delete_and_count([], _element, {count, nlist, _olist}), do: {count, Enum.reverse(nlist)} + + defp delete_and_count([h | r], h, {count, nlist, olist}), + do: delete_and_count(r, h, {count + 1, nlist, olist}) + + defp delete_and_count([h | r], element, {count, nlist, olist}), + do: delete_and_count(r, element, {count, [h | nlist], olist}) + + defp insert_if_needed(list, oldcount, element) do + if oldcount <= 0 || Enum.member?(list, element) do + list + else + [element | list] + end + end + defp check_delist(activity, actions) do if :delist in actions do with %User{} = user <- User.get_cached_by_ap_id(activity["actor"]) do - to = - List.delete(activity["to"] || [], Pleroma.Constants.as_public()) ++ - [user.follower_address] + {pubcnt, to} = delete_and_count(activity["to"] || [], Pleroma.Constants.as_public()) + {flwcnt, cc} = delete_and_count(activity["cc"] || [], user.follower_address) - cc = - List.delete(activity["cc"] || [], user.follower_address) ++ - [Pleroma.Constants.as_public()] + cc = insert_if_needed(cc, pubcnt, Pleroma.Constants.as_public()) + to = insert_if_needed(to, flwcnt, user.follower_address) activity = activity @@ -67,8 +85,8 @@ defmodule Pleroma.Web.ActivityPub.MRF.ObjectAgePolicy do defp check_strip_followers(activity, actions) do if :strip_followers in actions do with %User{} = user <- User.get_cached_by_ap_id(activity["actor"]) do - to = List.delete(activity["to"] || [], user.follower_address) - cc = List.delete(activity["cc"] || [], user.follower_address) + {_, to} = delete_and_count(activity["to"] || [], user.follower_address) + {_, cc} = delete_and_count(activity["cc"] || [], user.follower_address) activity = activity diff --git a/test/pleroma/notification_test.exs b/test/pleroma/notification_test.exs index 992b15f8b..1ff014a8d 100644 --- a/test/pleroma/notification_test.exs +++ b/test/pleroma/notification_test.exs @@ -9,6 +9,7 @@ defmodule Pleroma.NotificationTest do alias Pleroma.FollowingRelationship alias Pleroma.Notification + alias Pleroma.Object alias Pleroma.Repo alias Pleroma.Tests.ObanHelpers alias Pleroma.User @@ -276,6 +277,22 @@ defmodule Pleroma.NotificationTest do assert {:ok, []} == Notification.create_notifications(status) end + test "it creates notifications for objects without content when the user has filters" do + author = insert(:user) + user = insert(:user) + insert(:filter, user: user, phrase: "cofe", hide: true) + + {:ok, status} = CommonAPI.post(author, %{status: "hey @#{user.nickname}"}) + object = Object.normalize(status, fetch: false) + + {:ok, _} = + object + |> Object.change(%{data: Map.delete(object.data, "content")}) + |> Object.update_and_set_cache() + + assert {:ok, [_]} = Notification.create_notifications(Pleroma.Activity.get_by_id(status.id)) + end + test "it creates notifications if content matches with a not irreversible filter" do user = insert(:user) subscriber = insert(:user) diff --git a/test/pleroma/web/activity_pub/mrf/object_age_policy_test.exs b/test/pleroma/web/activity_pub/mrf/object_age_policy_test.exs index 7a67645aa..119a54717 100644 --- a/test/pleroma/web/activity_pub/mrf/object_age_policy_test.exs +++ b/test/pleroma/web/activity_pub/mrf/object_age_policy_test.exs @@ -8,6 +8,8 @@ defmodule Pleroma.Web.ActivityPub.MRF.ObjectAgePolicyTest do alias Pleroma.Web.ActivityPub.MRF.ObjectAgePolicy alias Pleroma.Web.ActivityPub.Visibility + require Pleroma.Constants + setup do: clear_config(:mrf_object_age, threshold: 172_800, @@ -79,7 +81,7 @@ defmodule Pleroma.Web.ActivityPub.MRF.ObjectAgePolicyTest do {:ok, data} = ObjectAgePolicy.filter(data) - assert Visibility.get_visibility(%{data: data}) == "unlisted" + assert Visibility.get_visibility(%{data: data}) == "direct" end test "it delists an old post" do @@ -94,6 +96,90 @@ defmodule Pleroma.Web.ActivityPub.MRF.ObjectAgePolicyTest do assert Visibility.get_visibility(%{data: data}) == "unlisted" end + test "it keeps an old followers-only post private" do + clear_config([:mrf_object_age, :actions], [:delist]) + + data = get_old_message() + {:ok, user} = User.get_or_fetch_by_ap_id(data["actor"]) + + data = + data + |> Map.put("to", [user.follower_address]) + |> Map.put("cc", []) + |> put_in(["object", "to"], [user.follower_address]) + |> put_in(["object", "cc"], []) + + {:ok, data} = ObjectAgePolicy.filter(data) + + assert data["to"] == [user.follower_address] + assert data["cc"] == [] + assert Visibility.get_visibility(%{data: data}) == "private" + assert data["object"]["to"] == data["to"] + assert data["object"]["cc"] == data["cc"] + end + + test "it keeps an old direct message direct" do + clear_config([:mrf_object_age, :actions], [:delist]) + + data = get_old_message() + {:ok, _user} = User.get_or_fetch_by_ap_id(data["actor"]) + recipient = "https://example.com/users/someone" + + data = + data + |> Map.put("to", [recipient]) + |> Map.put("cc", []) + |> put_in(["object", "to"], [recipient]) + |> put_in(["object", "cc"], []) + + {:ok, data} = ObjectAgePolicy.filter(data) + + assert data["to"] == [recipient] + assert data["cc"] == [] + assert Visibility.get_visibility(%{data: data}) == "direct" + end + + test "it removes every copy of the follower address" do + clear_config([:mrf_object_age, :actions], [:strip_followers]) + + data = get_old_message() + {:ok, user} = User.get_or_fetch_by_ap_id(data["actor"]) + public = Pleroma.Constants.as_public() + followers = [user.follower_address, user.follower_address] + + data = + data + |> Map.put("to", [public]) + |> Map.put("cc", followers) + |> put_in(["object", "to"], [public]) + |> put_in(["object", "cc"], followers) + + {:ok, data} = ObjectAgePolicy.filter(data) + + assert data["to"] == [public] + assert data["cc"] == [] + end + + test "it does not duplicate addresses of an old unlisted post" do + clear_config([:mrf_object_age, :actions], [:delist, :strip_followers]) + + data = get_old_message() + {:ok, user} = User.get_or_fetch_by_ap_id(data["actor"]) + public = Pleroma.Constants.as_public() + + data = + data + |> Map.put("to", [user.follower_address]) + |> Map.put("cc", [public]) + |> put_in(["object", "to"], [user.follower_address]) + |> put_in(["object", "cc"], [public]) + + {:ok, data} = ObjectAgePolicy.filter(data) + + assert data["to"] == [] + assert data["cc"] == [public] + end + test "it allows a new post" do clear_config([:mrf_object_age, :actions], [:delist]) diff --git a/test/pleroma/web/plugs/rate_limiter_test.exs b/test/pleroma/web/plugs/rate_limiter_test.exs index 10c93fa73..058c501f5 100644 --- a/test/pleroma/web/plugs/rate_limiter_test.exs +++ b/test/pleroma/web/plugs/rate_limiter_test.exs @@ -50,7 +50,9 @@ defmodule Pleroma.Web.Plugs.RateLimiterTest do test "it restricts based on config values" do limiter_name = :test_plug_opts - scale = 80 + # Long enough that the bucket cannot expire during the test on slow CI. + # The test expires it explicitly with expire_ttl/2. + scale = 60_000 limit = 5 clear_config([Pleroma.Web.Endpoint, :http, :ip], {127, 0, 0, 1}) @@ -184,7 +186,8 @@ defmodule Pleroma.Web.Plugs.RateLimiterTest do test "can have limits separate from unauthenticated connections" do limiter_name = :test_authenticated1 - scale = 50 + # Long enough that the bucket cannot expire during the test on slow CI. + scale = 60_000 limit = 5 clear_config([Pleroma.Web.Endpoint, :http, :ip], {8, 8, 8, 8}) clear_config([:rate_limit, limiter_name], [{1000, 1}, {scale, limit}])