From 59fcb5c96ebebad6973f294d47ec6f7ed75b38bd Mon Sep 17 00:00:00 2001 From: Oneric Date: Mon, 1 Dec 2025 00:06:13 +0100 Subject: [PATCH 01/29] api: ensure only visible posts are interactable Port of Akkoma PR 1014 with a few changes: - comments regarding akkomafe changed to Pleroma-FE when applicable - different error message for replying to/interacting with invisible post in Pleroma.Web.CommonAPI.ActivityDraft.in_reply_to/1 - split "doesn't do funny things to other users favs" test into three: - can't unfavourite post that isn't favourited - can't unfavourite other user's favs - can't unfavourite other user's favs using their activity - switched order of args for some CommonAPI function since Akkoma hasn't backported our old change for that Pleroma.Web.CommonAPI.ActivityDraft.in_reply_to/1 now refactored to use `with` statement as in Akkoma. Some defp in_reply_to/1 were therefore removed Original PR author: Oneric Original commit message: It doesn't make sense to like, react, reply, etc to something you cannot see and is unexpected for the author of the interacted with post and might make them believe the reacting user actually _can_ see the post. Wrt to fav, reblog, reaction indexes the missing visibility check was also leaking some (presumably/hopefully) low-severity data. Add full-API test for all modes of interactions with private posts. --- .../operations/emoji_reaction_operation.ex | 3 +- .../api_spec/operations/status_operation.ex | 2 + lib/pleroma/web/common_api.ex | 10 +- lib/pleroma/web/common_api/activity_draft.ex | 35 +-- .../controllers/status_controller.ex | 3 + .../controllers/emoji_reaction_controller.ex | 5 + .../conversation/participation_test.exs | 2 +- test/pleroma/conversation_test.exs | 10 +- .../notification_controller_test.exs | 3 + .../controllers/status_controller_test.exs | 237 ++++++++++++++++++ .../emoji_reaction_controller_test.exs | 108 ++++++++ 11 files changed, 398 insertions(+), 20 deletions(-) diff --git a/lib/pleroma/web/api_spec/operations/emoji_reaction_operation.ex b/lib/pleroma/web/api_spec/operations/emoji_reaction_operation.ex index 8d6be89a7..60fe8f011 100644 --- a/lib/pleroma/web/api_spec/operations/emoji_reaction_operation.ex +++ b/lib/pleroma/web/api_spec/operations/emoji_reaction_operation.ex @@ -35,7 +35,8 @@ defmodule Pleroma.Web.ApiSpec.EmojiReactionOperation do security: [%{"oAuth" => ["read:statuses"]}], operationId: "EmojiReactionController.index", responses: %{ - 200 => array_of_reactions_response() + 200 => array_of_reactions_response(), + 403 => Operation.response("Access denied", "application/json", ApiError) } } end diff --git a/lib/pleroma/web/api_spec/operations/status_operation.ex b/lib/pleroma/web/api_spec/operations/status_operation.ex index 500fbb64e..daf473fa0 100644 --- a/lib/pleroma/web/api_spec/operations/status_operation.ex +++ b/lib/pleroma/web/api_spec/operations/status_operation.ex @@ -162,6 +162,7 @@ defmodule Pleroma.Web.ApiSpec.StatusOperation do parameters: [id_param()], responses: %{ 200 => status_response(), + 403 => Operation.response("Access denied", "application/json", ApiError), 404 => Operation.response("Not Found", "application/json", ApiError) } } @@ -388,6 +389,7 @@ defmodule Pleroma.Web.ApiSpec.StatusOperation do "application/json", AccountOperation.array_of_accounts() ), + 403 => Operation.response("Access denied", "application/json", ApiError), 404 => Operation.response("Not Found", "application/json", ApiError) } } diff --git a/lib/pleroma/web/common_api.ex b/lib/pleroma/web/common_api.ex index ae554d0b9..c1719c8d4 100644 --- a/lib/pleroma/web/common_api.ex +++ b/lib/pleroma/web/common_api.ex @@ -258,7 +258,7 @@ defmodule Pleroma.Web.CommonAPI do {:ok, _} = res -> res - {:error, :not_found} = res -> + {:error, reason} = res when reason in [:not_found, :forbidden] -> res {:error, e} -> @@ -269,6 +269,7 @@ defmodule Pleroma.Web.CommonAPI do defp favorite_helper(user, id) do with {_, %Activity{object: object}} <- {:find_object, Activity.get_by_id_with_object(id)}, + {_, true} <- {:visible, Visibility.visible_for_user?(object, user)}, {_, {:ok, like_object, meta}} <- {:build_object, Builder.like(user, object)}, {_, {:ok, %Activity{} = activity, _meta}} <- {:common_pipeline, @@ -278,6 +279,9 @@ defmodule Pleroma.Web.CommonAPI do {:find_object, _} -> {:error, :not_found} + {:visible, _} -> + {:error, :forbidden} + {:common_pipeline, {:error, {:validate, {:error, changeset}}}} = e -> if {:object, {"already liked by this actor", []}} in changeset.errors do {:ok, :already_liked} @@ -311,11 +315,15 @@ defmodule Pleroma.Web.CommonAPI do {:ok, Activity.t()} | {:error, String.t()} def react_with_emoji(id, user, emoji) do with %Activity{} = activity <- Activity.get_by_id(id), + {_, true} <- {:visible, Visibility.visible_for_user?(activity, user)}, object <- Object.normalize(activity, fetch: false), {:ok, emoji_react, _} <- Builder.emoji_react(user, object, emoji), {:ok, activity, _} <- Pipeline.common_pipeline(emoji_react, local: true) do {:ok, activity} else + {:visible, _} -> + {:error, dgettext("errors", "Must be able to access post to interact with it")} + _ -> {:error, dgettext("errors", "Could not add reaction emoji")} end diff --git a/lib/pleroma/web/common_api/activity_draft.ex b/lib/pleroma/web/common_api/activity_draft.ex index f60ed8b02..2d212a443 100644 --- a/lib/pleroma/web/common_api/activity_draft.ex +++ b/lib/pleroma/web/common_api/activity_draft.ex @@ -136,22 +136,29 @@ defmodule Pleroma.Web.CommonAPI.ActivityDraft do defp in_reply_to(%{params: %{in_reply_to_status_id: ""}} = draft), do: draft - defp in_reply_to(%{params: %{in_reply_to_status_id: :deleted}} = draft) do - add_error(draft, dgettext("errors", "Cannot reply to a deleted status")) - end + defp in_reply_to(%{params: %{in_reply_to_status_id: id}} = draft) when is_binary(id) do + # If a post was deleted all its activities (except the newly added Delete) are purged too, + # thus lookup by Create db ID will yield nil just as if it never existed in the first place. + # + # We allow replying to Announce here, due to a Pleroma-FE quirk where if presented with an Announce id + # it will render it as if it was just the normal referenced post, but use the Announce id for replies + # in the in_reply_to_id key of a POST request to /api/v1/statuses, or as an :id in /api/v1/statuses/:id/*. + # TODO: Fix this quirk in FE and remove here and other affected places + with %Activity{} = activity <- Activity.get_by_id(id), + true <- Visibility.visible_for_user?(activity, draft.user), + {:type, type} when type in ["Create", "Announce"] <- {:type, activity.data["type"]} do + %__MODULE__{draft | in_reply_to: activity} + else + nil -> + add_error(draft, dgettext("errors", "Cannot reply to a deleted status")) - defp in_reply_to(%{params: %{in_reply_to_status_id: id} = params} = draft) when is_binary(id) do - activity = Activity.get_by_id(id) + false -> + add_error(draft, dgettext("errors", "Replying to a status that is not visibile to user")) - params = - if is_nil(activity) do - # Deleted activities are returned as nil - Map.put(params, :in_reply_to_status_id, :deleted) - else - Map.put(params, :in_reply_to_status_id, activity) - end - - in_reply_to(%{draft | params: params}) + {:type, type} -> + add_error(draft, dgettext("errors", "Can only reply to posts, not %{type} activities", + type: inspect(type))) + end end defp in_reply_to(%{params: %{in_reply_to_status_id: %Activity{} = in_reply_to}} = draft) do diff --git a/lib/pleroma/web/mastodon_api/controllers/status_controller.ex b/lib/pleroma/web/mastodon_api/controllers/status_controller.ex index 218ee7e28..62cc6a670 100644 --- a/lib/pleroma/web/mastodon_api/controllers/status_controller.ex +++ b/lib/pleroma/web/mastodon_api/controllers/status_controller.ex @@ -319,6 +319,7 @@ defmodule Pleroma.Web.MastodonAPI.StatusController do @doc "DELETE /api/v1/statuses/:id" def delete(%{assigns: %{user: user}, private: %{open_api_spex: %{params: %{id: id}}}} = conn, _) do with %Activity{} = activity <- Activity.get_by_id_with_object(id), + # CommonAPI already checks whether user is allowed to delete {:ok, %Activity{}} <- CommonAPI.delete(id, user) do try_render(conn, "show.json", activity: activity, @@ -340,6 +341,7 @@ defmodule Pleroma.Web.MastodonAPI.StatusController do _ ) do with {:ok, announce} <- CommonAPI.repeat(ap_id_or_id, user, params), + # CommonAPI already checks whether user is allowed to reblog %Activity{} = announce <- Activity.normalize(announce.data) do try_render(conn, "show.json", %{activity: announce, for: user, as: :activity}) end @@ -364,6 +366,7 @@ defmodule Pleroma.Web.MastodonAPI.StatusController do _ ) do with {:ok, _fav} <- CommonAPI.favorite(activity_id, user), + # CommonAPI already checks whether user is allowed to reblog %Activity{} = activity <- Activity.get_by_id(activity_id) do try_render(conn, "show.json", activity: activity, for: user, as: :activity) end diff --git a/lib/pleroma/web/pleroma_api/controllers/emoji_reaction_controller.ex b/lib/pleroma/web/pleroma_api/controllers/emoji_reaction_controller.ex index 662cc15d6..472c3742a 100644 --- a/lib/pleroma/web/pleroma_api/controllers/emoji_reaction_controller.ex +++ b/lib/pleroma/web/pleroma_api/controllers/emoji_reaction_controller.ex @@ -8,6 +8,7 @@ defmodule Pleroma.Web.PleromaAPI.EmojiReactionController do alias Pleroma.Activity alias Pleroma.Object alias Pleroma.User + alias Pleroma.Web.ActivityPub.Visibility alias Pleroma.Web.CommonAPI alias Pleroma.Web.MastodonAPI.StatusView alias Pleroma.Web.Plugs.OAuthScopesPlug @@ -28,6 +29,7 @@ defmodule Pleroma.Web.PleromaAPI.EmojiReactionController do def index(%{assigns: %{user: user}} = conn, %{id: activity_id} = params) do with true <- Pleroma.Config.get([:instance, :show_reactions]), %Activity{} = activity <- Activity.get_by_id_with_object(activity_id), + {_, true} <- {:visible, Visibility.visible_for_user?(activity, user)}, %Object{} = object <- Object.normalize(activity, fetch: false), reactions <- Object.get_emoji_reactions(object) do reactions = @@ -37,6 +39,7 @@ defmodule Pleroma.Web.PleromaAPI.EmojiReactionController do render(conn, "index.json", emoji_reactions: reactions, user: user) else + {:visible, _} -> {:error, :forbidden} _e -> json(conn, []) end end @@ -76,6 +79,7 @@ defmodule Pleroma.Web.PleromaAPI.EmojiReactionController do |> Pleroma.Emoji.fully_qualify_emoji() |> Pleroma.Emoji.maybe_quote() + # CommonAPI checks if allowed to react with {:ok, _activity} <- CommonAPI.react_with_emoji(activity_id, user, emoji) do activity = Activity.get_by_id(activity_id) @@ -91,6 +95,7 @@ defmodule Pleroma.Web.PleromaAPI.EmojiReactionController do |> Pleroma.Emoji.fully_qualify_emoji() |> Pleroma.Emoji.maybe_quote() + # CommonAPI checks only author can revoke reactions with {:ok, _activity} <- CommonAPI.unreact_with_emoji(activity_id, user, emoji) do activity = Activity.get_by_id(activity_id) diff --git a/test/pleroma/conversation/participation_test.exs b/test/pleroma/conversation/participation_test.exs index 697bdb7f9..f6fde1875 100644 --- a/test/pleroma/conversation/participation_test.exs +++ b/test/pleroma/conversation/participation_test.exs @@ -332,7 +332,7 @@ defmodule Pleroma.Conversation.ParticipationTest do # When it's a reply from the blocked user {:ok, _direct2} = CommonAPI.post(blocked, %{ - status: "reply", + status: "@#{third_user.nickname}, #{blocker.nickname} reply", visibility: "direct", in_reply_to_conversation_id: blocked_participation.id }) diff --git a/test/pleroma/conversation_test.exs b/test/pleroma/conversation_test.exs index 02b5de615..bc41165ed 100644 --- a/test/pleroma/conversation_test.exs +++ b/test/pleroma/conversation_test.exs @@ -66,8 +66,10 @@ defmodule Pleroma.ConversationTest do jafnhar = insert(:user, local: false) tridi = insert(:user) + to = [har.nickname, jafnhar.nickname, tridi.nickname] + {:ok, activity} = - CommonAPI.post(har, %{status: "Hey @#{jafnhar.nickname}", visibility: "direct"}) + CommonAPI.post(har, %{status: "Hey @#{jafnhar.nickname}", visibility: "direct", to: to}) object = Pleroma.Object.normalize(activity, fetch: false) context = object.data["context"] @@ -88,7 +90,8 @@ defmodule Pleroma.ConversationTest do CommonAPI.post(jafnhar, %{ status: "Hey @#{har.nickname}", visibility: "direct", - in_reply_to_status_id: activity.id + in_reply_to_status_id: activity.id, + to: to }) object = Pleroma.Object.normalize(activity, fetch: false) @@ -112,7 +115,8 @@ defmodule Pleroma.ConversationTest do CommonAPI.post(tridi, %{ status: "Hey @#{har.nickname}", visibility: "direct", - in_reply_to_status_id: activity.id + in_reply_to_status_id: activity.id, + to: to }) object = Pleroma.Object.normalize(activity, fetch: false) diff --git a/test/pleroma/web/mastodon_api/controllers/notification_controller_test.exs b/test/pleroma/web/mastodon_api/controllers/notification_controller_test.exs index 88f2fb7af..dd4b7389e 100644 --- a/test/pleroma/web/mastodon_api/controllers/notification_controller_test.exs +++ b/test/pleroma/web/mastodon_api/controllers/notification_controller_test.exs @@ -316,6 +316,9 @@ defmodule Pleroma.Web.MastodonAPI.NotificationControllerTest do user = insert(:user) %{user: other_user, conn: conn} = oauth_access(["read:notifications"]) + {:ok, _, _, %{data: %{"state" => "accept"}}} = CommonAPI.follow(other_user, user) + {:ok, _, _, %{data: %{"state" => "accept"}}} = CommonAPI.follow(user, other_user) + {:ok, public_activity} = CommonAPI.post(other_user, %{status: ".", visibility: "public"}) {:ok, direct_activity} = diff --git a/test/pleroma/web/mastodon_api/controllers/status_controller_test.exs b/test/pleroma/web/mastodon_api/controllers/status_controller_test.exs index c19a6ca4e..d8558465a 100644 --- a/test/pleroma/web/mastodon_api/controllers/status_controller_test.exs +++ b/test/pleroma/web/mastodon_api/controllers/status_controller_test.exs @@ -17,6 +17,7 @@ defmodule Pleroma.Web.MastodonAPI.StatusControllerTest do alias Pleroma.User alias Pleroma.Web.ActivityPub.ActivityPub alias Pleroma.Web.ActivityPub.Utils + alias Pleroma.Web.ActivityPub.Visibility alias Pleroma.Web.CommonAPI alias Pleroma.Workers.ScheduledActivityWorker @@ -267,6 +268,72 @@ defmodule Pleroma.Web.MastodonAPI.StatusControllerTest do end) end + test "replying to a post the current user can't access fails", %{user: user, conn: conn} do + stranger = insert(:user) + + {:ok, priv_post_act} = + CommonAPI.post(stranger, %{status: "forbidden knowledge", visibility: "private"}) + + assert Visibility.visible_for_user?(priv_post_act, stranger) + refute Visibility.visible_for_user?(priv_post_act, user) + + resp = + conn + |> put_req_header("content-type", "application/json") + |> post("/api/v1/statuses", %{ + "status" => "@#{stranger.nickname} :peek:", + "in_reply_to_id" => priv_post_act.id, + "visibility" => "private" + }) + |> json_response_and_validate_schema(422) + + assert match?(%{"error" => _}, resp) + end + + test "replying to own DM succeeds", %{user: user, conn: conn} do + # this is an "edge" case for visibility: replying user is not part of addressed users (but is the author) + stranger = insert(:user) + + {:ok, %{id: dm_id} = dm_post_act} = + CommonAPI.post(user, %{ + status: "@#{stranger.nickname} wanna lose your mind to forbidden knowledge?", + visibility: "direct" + }) + + assert Visibility.visible_for_user?(dm_post_act, stranger) + assert Visibility.visible_for_user?(dm_post_act, user) + + resp = + conn + |> put_req_header("content-type", "application/json") + |> post("/api/v1/statuses", %{ + "status" => "@#{stranger.nickname} :peek:", + "in_reply_to_id" => dm_id, + "visibility" => "direct" + }) + |> json_response_and_validate_schema(200) + + assert match?(%{"in_reply_to_id" => ^dm_id}, resp) + end + + test "replying to a non-post activity fails", %{conn: conn, user: user} do + other_user = insert(:user) + + {:ok, _, _, follow_activity} = CommonAPI.follow(other_user, user) + assert Visibility.visible_for_user?(follow_activity, user) + + conn = + conn + |> put_req_header("content-type", "application/json") + |> post("/api/v1/statuses", %{ + "status" => "hiiii!", + "in_reply_to_id" => to_string(follow_activity.id) + }) + + assert %{"error" => "Can only reply to posts, not \"Follow\" activities"} = + json_response_and_validate_schema(conn, 422) + end + test "posting a status with an invalid in_reply_to_id", %{conn: conn} do conn = conn @@ -1416,6 +1483,24 @@ defmodule Pleroma.Web.MastodonAPI.StatusControllerTest do assert to_string(activity.id) == id end + + test "cannot reblog private status of others (even if visible)", %{conn: conn, user: user} do + followed = insert(:user, local: true) + + {:ok, _, _, %{data: %{"state" => "accept"}}} = CommonAPI.follow(followed, user) + + {:ok, activity} = CommonAPI.post(followed, %{status: "cofe", visibility: "private"}) + + assert Visibility.visible_for_user?(activity, user) + + resp = + conn + |> put_req_header("content-type", "application/json") + |> post("/api/v1/statuses/#{activity.id}/reblog") + |> json_response_and_validate_schema(404) + + assert match?(%{"error" => _}, resp) + end end describe "unreblogging" do @@ -1445,6 +1530,33 @@ defmodule Pleroma.Web.MastodonAPI.StatusControllerTest do assert json_response_and_validate_schema(conn, 404) == %{"error" => "Record not found"} end + + test "can't unreblog someone else's reblog", %{user: user, conn: conn} do + activity = insert(:note_activity) + other_user = insert(:user) + + {:ok, %{id: reblog_id}} = CommonAPI.repeat(activity.id, other_user) + + # unreblog by base post + resp1 = + conn + |> put_req_header("content-type", "application/json") + |> post("/api/v1/statuses/#{activity.id}/unreblog") + |> json_response(400) + + assert match?(%{"error" => _}, resp1) + + # unreblog by reblog ID (reblog IDs are accepted by some APIs; ensure it fails here one way or another) + resp2 = + build_conn() + |> assign(:user, user) + |> assign(:token, insert(:oauth_token, user: user, scopes: ["write", "read"])) + |> put_req_header("content-type", "application/json") + |> post("/api/v1/statuses/#{reblog_id}/unreblog") + |> json_response_and_validate_schema(404) + + assert match?(%{"error" => _}, resp2) + end end describe "favoriting" do @@ -1477,6 +1589,21 @@ defmodule Pleroma.Web.MastodonAPI.StatusControllerTest do |> json_response_and_validate_schema(200) end + test "a status you cannot see fails", %{conn: conn} do + stranger = insert(:user) + + {:ok, activity} = + CommonAPI.post(stranger, %{status: "it can eternal lie", visibility: "private"}) + + resp = + conn + |> put_req_header("content-type", "application/json") + |> post("/api/v1/statuses/#{activity.id}/favourite") + |> json_response_and_validate_schema(403) + + assert match?(%{"error" => _}, resp) + end + test "returns 404 error for a wrong id", %{conn: conn} do conn = conn @@ -1506,6 +1633,50 @@ defmodule Pleroma.Web.MastodonAPI.StatusControllerTest do assert to_string(activity.id) == id end + test "can't unfavourite post that isn't favourited", %{conn: conn} do + activity = insert(:note_activity) + + # using base post ID + resp = + conn + |> put_req_header("content-type", "application/json") + |> post("/api/v1/statuses/#{activity.id}/unfavourite") + |> json_response(400) + + assert match?(%{"error" => "Could not unfavorite"}, resp) + end + + test "can't unfavourite other user's favs", %{conn: conn} do + activity = insert(:note_activity) + + other = insert(:user) + {:ok, _} = CommonAPI.favorite(activity.id, other) + + # using base post ID + resp = + conn + |> put_req_header("content-type", "application/json") + |> post("/api/v1/statuses/#{activity.id}/unfavourite") + |> json_response(400) + + assert match?(%{"error" => "Could not unfavorite"}, resp) + end + + test "can't unfavourite other user's favs using their activity", %{conn: conn} do + activity = insert(:note_activity) + + other = insert(:user) + {:ok, fav_activity} = CommonAPI.favorite(activity.id, other) + # some APIs (used to) take IDs of any activity type, make sure this fails one way or another + resp = + conn + |> put_req_header("content-type", "application/json") + |> post("/api/v1/statuses/#{fav_activity.id}/unfavourite") + |> json_response_and_validate_schema(404) + + assert match?(%{"error" => _}, resp) + end + test "returns 404 error for a wrong id", %{conn: conn} do conn = conn @@ -1959,6 +2130,25 @@ defmodule Pleroma.Web.MastodonAPI.StatusControllerTest do assert id == other_user.id end + test "fails when base post not visible to current user", %{user: user} do + other_user = insert(:user, local: true) + + {:ok, activity} = + CommonAPI.post(user, %{ + status: "craving tea and mochi rn", + visibility: "private" + }) + + resp = + build_conn() + |> assign(:user, other_user) + |> assign(:token, insert(:oauth_token, user: other_user, scopes: ["read:accounts"])) + |> get("/api/v1/statuses/#{activity.id}/favourited_by") + |> json_response_and_validate_schema(404) + + assert match?(%{"error" => _}, resp) + end + test "returns empty array when :show_reactions is disabled", %{conn: conn, activity: activity} do clear_config([:instance, :show_reactions], false) @@ -2077,6 +2267,25 @@ defmodule Pleroma.Web.MastodonAPI.StatusControllerTest do assert [] == response end + + test "does fail when requesting for a non-visible status", %{user: user} do + other_user = insert(:user, local: true) + + {:ok, activity} = + CommonAPI.post(user, %{ + status: "deep below it sleeps and mustn't wake", + visibility: "private" + }) + + response = + build_conn() + |> assign(:user, other_user) + |> assign(:token, insert(:oauth_token, user: other_user, scopes: ["read"])) + |> get("/api/v1/statuses/#{activity.id}/reblogged_by") + |> json_response_and_validate_schema(404) + + assert match?(%{"error" => _}, response) + end end test "context" do @@ -2099,6 +2308,34 @@ defmodule Pleroma.Web.MastodonAPI.StatusControllerTest do } = response end + test "context doesn't leak priv posts" do + %{user: user, conn: conn} = oauth_access(["read:statuses"]) + stranger = insert(:user) + + {:ok, %{id: id1}} = CommonAPI.post(stranger, %{status: "1", visibility: "public"}) + + {:ok, %{id: id2}} = + CommonAPI.post(stranger, %{status: "2", visibility: "unlisted", in_reply_to_status_id: id1}) + + {:ok, %{id: _id_boo} = act_boo} = + CommonAPI.post(stranger, %{status: "boo", visibility: "private", in_reply_to_status_id: id1}) + + refute Visibility.visible_for_user?(act_boo, user) + + response = + conn + |> get("/api/v1/statuses/#{id1}/context") + |> json_response_and_validate_schema(:ok) + + assert match?( + %{ + "ancestors" => [], + "descendants" => [%{"id" => ^id2}] + }, + response + ) + end + test "favorites paginate correctly" do %{user: user, conn: conn} = oauth_access(["read:favourites"]) other_user = insert(:user) diff --git a/test/pleroma/web/pleroma_api/controllers/emoji_reaction_controller_test.exs b/test/pleroma/web/pleroma_api/controllers/emoji_reaction_controller_test.exs index c1e452a1e..f74f5ebc0 100644 --- a/test/pleroma/web/pleroma_api/controllers/emoji_reaction_controller_test.exs +++ b/test/pleroma/web/pleroma_api/controllers/emoji_reaction_controller_test.exs @@ -9,10 +9,38 @@ defmodule Pleroma.Web.PleromaAPI.EmojiReactionControllerTest do alias Pleroma.Object alias Pleroma.Tests.ObanHelpers alias Pleroma.User + alias Pleroma.Web.ActivityPub.Visibility alias Pleroma.Web.CommonAPI import Pleroma.Factory + defp prepare_reacted_post(visibility \\ "private") do + unrelated_user = insert(:user, local: true) + poster = insert(:user, local: true) + follower = insert(:user, local: true) + {:ok, _, _, %{data: %{"state" => "accept"}}} = CommonAPI.follow(poster, follower) + + {:ok, post_activity} = CommonAPI.post(poster, %{status: "miaow!", visibility: visibility}) + + if visibility != "direct" do + assert Visibility.visible_for_user?(post_activity, follower) + end + + if visibility in ["direct", "private"] do + refute Visibility.visible_for_user?(post_activity, unrelated_user) + end + + {:ok, _react_activity} = CommonAPI.react_with_emoji(post_activity.id, follower, "🐾") + + {post_activity, poster, follower, unrelated_user} + end + + defp prepare_conn_of_user(conn, user) do + conn + |> assign(:user, user) + |> assign(:token, insert(:oauth_token, user: user, scopes: ["write", "read"])) + end + setup do Mox.stub_with(Pleroma.UnstubbedConfigMock, Pleroma.Test.StaticConfig) :ok @@ -137,6 +165,28 @@ defmodule Pleroma.Web.PleromaAPI.EmojiReactionControllerTest do |> json_response_and_validate_schema(400) end + test "PUT /api/v1/pleroma/statuses/:id/reactions/:emoji not allowed for non-visible posts", %{ + conn: conn + } do + {%{id: activity_id} = _activity, _author, follower, stranger} = prepare_reacted_post() + + # Works for follower + resp = + prepare_conn_of_user(conn, follower) + |> put("/api/v1/pleroma/statuses/#{activity_id}/reactions/🐈") + |> json_response_and_validate_schema(200) + + assert match?(%{"id" => ^activity_id}, resp) + + # Fails for stranger + resp = + prepare_conn_of_user(conn, stranger) + |> put("/api/v1/pleroma/statuses/#{activity_id}/reactions/🐈") + |> json_response_and_validate_schema(400) + + assert match?(%{"error" => _}, resp) + end + test "DELETE /api/v1/pleroma/statuses/:id/reactions/:emoji", %{conn: conn} do user = insert(:user) other_user = insert(:user) @@ -211,6 +261,26 @@ defmodule Pleroma.Web.PleromaAPI.EmojiReactionControllerTest do |> json_response(400) end + test "DELETE /api/v1/pleroma/statuses/:id/reactions/:emoji only allows original reacter to revoke", + %{conn: conn} do + {%{id: activity_id} = _activity, author, follower, unrelated} = prepare_reacted_post("public") + + # Works for original reacter + prepare_conn_of_user(conn, follower) + |> delete("/api/v1/pleroma/statuses/#{activity_id}/reactions/🐾") + |> json_response_and_validate_schema(200) + + # Fails for anyone else + for u <- [author, unrelated] do + resp = + prepare_conn_of_user(conn, u) + |> delete("/api/v1/pleroma/statuses/#{activity_id}/reactions/🐾") + |> json_response(400) + + assert match?(%{"error" => _}, resp) + end + end + test "GET /api/v1/pleroma/statuses/:id/reactions", %{conn: conn} do user = insert(:user) other_user = insert(:user) @@ -324,6 +394,28 @@ defmodule Pleroma.Web.PleromaAPI.EmojiReactionControllerTest do assert [%{"name" => "πŸŽ…", "count" => 2}] = result end + test "GET /api/v1/pleroma/statuses/:id/reactions not allowed for non-visible posts", %{ + conn: conn + } do + {%{id: activity_id} = _activity, _author, follower, stranger} = prepare_reacted_post() + + # Works for follower + resp = + prepare_conn_of_user(conn, follower) + |> get("/api/v1/pleroma/statuses/#{activity_id}/reactions") + |> json_response_and_validate_schema(200) + + assert match?([%{"name" => _, "count" => _} | _], resp) + + # Fails for stranger + resp = + prepare_conn_of_user(conn, stranger) + |> get("/api/v1/pleroma/statuses/#{activity_id}/reactions") + |> json_response_and_validate_schema(403) + + assert match?(%{"error" => _}, resp) + end + test "GET /api/v1/pleroma/statuses/:id/reactions with :show_reactions disabled", %{conn: conn} do clear_config([:instance, :show_reactions], false) @@ -372,4 +464,20 @@ defmodule Pleroma.Web.PleromaAPI.EmojiReactionControllerTest do assert represented_user["id"] == other_user.id end + + test "GET /api/v1/pleroma/statuses/:id/reactions/:emoji not allowed for non-visible posts", %{ + conn: conn + } do + {%{id: activity_id} = _activity, _author, follower, stranger} = prepare_reacted_post() + + # Works for follower + prepare_conn_of_user(conn, follower) + |> get("/api/v1/pleroma/statuses/#{activity_id}/reactions/🐈") + |> json_response_and_validate_schema(200) + + # Fails for stranger + prepare_conn_of_user(conn, stranger) + |> get("/api/v1/pleroma/statuses/#{activity_id}/reactions/🐈") + |> json_response_and_validate_schema(403) + end end From f8db412af42dad0eb5f3510030fc089dba77d822 Mon Sep 17 00:00:00 2001 From: Oneric Date: Sun, 23 Nov 2025 00:00:00 +0000 Subject: [PATCH 02/29] fed/fetch: don't serve unsanitised object data for some activities MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the object associated with the activity was preloaded (which happens automatically with Activity.normalize used in the controller) Object.normalize’s "id_only" option did not actually work. This option and it’s usage were introduced to fix display of Undo activities in e88f36f72b5317debafcc4209b91eb35ad8f0691. For "Undo"s (and "Delete"s) there is no object preloaded (since it is already gone from the database) thus this appeared to work and for the particular case considered there in fact did. Create activities use different rendering logic and thus remained unaffected too. However, for all other types of Activities (yes, including Update which really _should_ include a properly sanitised, full object) this new attempt at including "just the id", lead to it instead including the full, unsanitised data of the referenced object. This is obviously bad and can get worse due to access restrictions on the activity being solely performed based on the addressing of the activity itself, not of the (unintentionally) embedded object. Starting with the obvious, this leaks all "internal" fields but as already mentioned in 8243fc0ef482a28daf2bcae2c64a9510bdb76489 all current "internal" fields from Constants.object_internal_fields are already publicised via MastoAPI etc anyway. Assuming matching addressing of the referenced object and activity this isn't problematic with regard to confidentiality. Except, the internal "voters" field recording who voted for a poll is currently just omitted from Constants.object_internal_fields and indeed confidential information (fix in subsequent commit). Fortunately this list is for the poll as a whole and there are no inlined lists for individual choices. While this thus leaks _who_ voted for a poll, it at least doesn't directly expose _what_ each voter chose if there are multiple voters. As alluded to before, the access restriction not being aware of the misplaced object data into account makes the issue worse. If the activity addressing is not a subset of the referenced object’s addressing, this will leak private objects to unauthorised users. This begs the question whether such mismatched addressing can occur. For remote activities the answer is ofc a resounding YES, but we only serve local ActivityPub objects and for the latter it currently(!) seems like a "no". For all intended interactions, the user interacting must already have access to the object of interest and our ActivityPub Builder already uses a subset of the original posts addressing for posts not publicly accessible. This addressing creation logic was last touched six years ago predating the introduction of this exposure blunder. The rather big caveat her being, until it was fixed just yesterday in dff532ac723310903e58c5d28f897cc2d116594f it was indeed possible to interact with posts one is not allowed to actually see. Combined, this allowed unauthorised access to private posts. (The API ID of such private posts can be obtained e.g. from replies one _is_ allowed to see) During the time when ActivityPub C2S was supported there might have been more ways to create activities with mismatched addressing and sneak a peek on private posts. (The AP id can be obtained in an analogous way) Replaces and fixes e88f36f72b5317debafcc4209b91eb35ad8f0691. Since there never were any users of the bugged "id_only" option it is removed. This was reported by silverpill as an ActivityPub interop issue, since this blunder of course also leads to invalid AP documents by adding an additional layer in form of the "data" key and directly exposing the internal Pleroma representation which is not always identical to valid AP. Fixes: https://akkoma.dev/AkkomaGang/akkoma/issues/1017 --- lib/pleroma/object.ex | 5 +---- lib/pleroma/web/activity_pub/views/object_view.ex | 6 +++++- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/lib/pleroma/object.ex b/lib/pleroma/object.ex index 77dfda851..20e5bfb5c 100644 --- a/lib/pleroma/object.ex +++ b/lib/pleroma/object.ex @@ -126,7 +126,7 @@ defmodule Pleroma.Object do Logger.debug("Backtrace: #{inspect(Process.info(:erlang.self(), :current_stacktrace))}") end - def normalize(_, options \\ [fetch: false, id_only: false]) + def normalize(_, options \\ [fetch: false]) # If we pass an Activity to Object.normalize(), we can try to use the preloaded object. # Use this whenever possible, especially when walking graphs in an O(N) loop! @@ -155,9 +155,6 @@ defmodule Pleroma.Object do def normalize(ap_id, options) when is_binary(ap_id) do cond do - Keyword.get(options, :id_only) -> - ap_id - Keyword.get(options, :fetch) -> case Fetcher.fetch_object_from_id(ap_id, options) do {:ok, object} -> object diff --git a/lib/pleroma/web/activity_pub/views/object_view.ex b/lib/pleroma/web/activity_pub/views/object_view.ex index 13b5b2542..4a70c5d47 100644 --- a/lib/pleroma/web/activity_pub/views/object_view.ex +++ b/lib/pleroma/web/activity_pub/views/object_view.ex @@ -7,6 +7,7 @@ defmodule Pleroma.Web.ActivityPub.ObjectView do alias Pleroma.Activity alias Pleroma.Object alias Pleroma.Web.ActivityPub.Transmogrifier + alias Pleroma.Web.ActivityPub.Utils def render("object.json", %{object: %Object{} = object}) do base = Pleroma.Web.ActivityPub.Utils.make_json_ld_header(object.data) @@ -29,7 +30,7 @@ defmodule Pleroma.Web.ActivityPub.ObjectView do def render("object.json", %{object: %Activity{} = activity}) do base = Pleroma.Web.ActivityPub.Utils.make_json_ld_header(activity.data) - object_id = Object.normalize(activity, id_only: true) + object_id = object_id_from_activity(activity) additional = Transmogrifier.prepare_object(activity.data) @@ -37,4 +38,7 @@ defmodule Pleroma.Web.ActivityPub.ObjectView do Map.merge(base, additional) end + + defp object_id_from_activity(%Activity{object: %Object{data: %{"id" => obj_id}}}), do: obj_id + defp object_id_from_activity(%Activity{data: %{"object" => ap_object_ref}}), do: Utils.get_ap_id(ap_object_ref) end From 409698ca632f7a4884a8a8b809614c481e77a254 Mon Sep 17 00:00:00 2001 From: Oneric Date: Sun, 23 Nov 2025 00:00:00 +0000 Subject: [PATCH 03/29] fed/out: ensure we never serve Updates for objects we deem static --- lib/pleroma/web/activity_pub/transmogrifier.ex | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/pleroma/web/activity_pub/transmogrifier.ex b/lib/pleroma/web/activity_pub/transmogrifier.ex index 00339fad9..c9cd91e9e 100644 --- a/lib/pleroma/web/activity_pub/transmogrifier.ex +++ b/lib/pleroma/web/activity_pub/transmogrifier.ex @@ -851,6 +851,10 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do {:ok, data} end + def prepare_outgoing(%{"type" => "Update", "object" => %{}} = data) do + raise "Requested to serve an Update for non-updateable object type: #{inspect(data)}" + end + def prepare_outgoing(%{"type" => "Announce", "actor" => ap_id, "object" => object_id} = data) do object = object_id From a1662f05e06a2c87af51a16e60e2672457d5ff93 Mon Sep 17 00:00:00 2001 From: Oneric Date: Mon, 1 Dec 2025 18:14:38 +0100 Subject: [PATCH 04/29] fed/fetch: use same sanitisation logic as when delivering to inboxes Port of commit 85171750f17725b71dcda098a5085b7f402cb061 from Akkoma PR 1018. Modifications from Akkoma patch: - Pleroma.Web.ActivityPub.Utils.make_json_ld_header() calls had activity.data as argument. - render() had Listen activities in activity_type, Akkoma only has Create activities there. Needs testing whether transmogrifier can handle this. Original commit author: Oneric Original commit message: Duped code just means double the chance to mess up. This would have prevented the leak of confidential info more minimally fixed in 6a8b8a14999f3ed82fdaedf6a53f9a391280df2f and now furthermore fixes the representation of Update activites which _need_ to have their object inlined, as well as better interop for follow Accept and Reject activities and all other special cases already handled in Transmogrifier. It also means we get more thorough tests for free. This also already adds JSON-LD context and does not add bogus Note-only fields as happened before due to this views misuse of prepare_object for activities. The doc of prepare_object clearly states it is only intended for creatable objects, i.e. (for us) Notes and Questions. --- .../web/activity_pub/views/object_view.ex | 26 ++----------------- 1 file changed, 2 insertions(+), 24 deletions(-) diff --git a/lib/pleroma/web/activity_pub/views/object_view.ex b/lib/pleroma/web/activity_pub/views/object_view.ex index 4a70c5d47..857410e43 100644 --- a/lib/pleroma/web/activity_pub/views/object_view.ex +++ b/lib/pleroma/web/activity_pub/views/object_view.ex @@ -7,7 +7,6 @@ defmodule Pleroma.Web.ActivityPub.ObjectView do alias Pleroma.Activity alias Pleroma.Object alias Pleroma.Web.ActivityPub.Transmogrifier - alias Pleroma.Web.ActivityPub.Utils def render("object.json", %{object: %Object{} = object}) do base = Pleroma.Web.ActivityPub.Utils.make_json_ld_header(object.data) @@ -16,29 +15,8 @@ defmodule Pleroma.Web.ActivityPub.ObjectView do Map.merge(base, additional) end - def render("object.json", %{object: %Activity{data: %{"type" => activity_type}} = activity}) - when activity_type in ["Create", "Listen"] do - base = Pleroma.Web.ActivityPub.Utils.make_json_ld_header(activity.data) - object = Object.normalize(activity, fetch: false) - - additional = - Transmogrifier.prepare_object(activity.data) - |> Map.put("object", Transmogrifier.prepare_object(object.data)) - - Map.merge(base, additional) - end - def render("object.json", %{object: %Activity{} = activity}) do - base = Pleroma.Web.ActivityPub.Utils.make_json_ld_header(activity.data) - object_id = object_id_from_activity(activity) - - additional = - Transmogrifier.prepare_object(activity.data) - |> Map.put("object", object_id) - - Map.merge(base, additional) + {:ok, ap_data} = Transmogrifier.prepare_outgoing(activity.data) + ap_data end - - defp object_id_from_activity(%Activity{object: %Object{data: %{"id" => obj_id}}}), do: obj_id - defp object_id_from_activity(%Activity{data: %{"object" => ap_object_ref}}), do: Utils.get_ap_id(ap_object_ref) end From 885ba3a46f8653f74f998245f0acf983672ecacc Mon Sep 17 00:00:00 2001 From: Oneric Date: Mon, 1 Dec 2025 18:25:58 +0100 Subject: [PATCH 05/29] test: add more representation tests for perpare_outgoing Port of commit 272799da6242dbf7387d2d42dfc98512cd7efd7e from Akkoma PR 1018. Changes from Akkoma commit: - changed order of arguments in CommonAPI.(un)block, because Akkoma hasn't backported our change for the unified arg order yet In particular this covers the case e88f36f72b5317debafcc4209b91eb35ad8f0691 was meant to fix and --- .../web/activity_pub/transmogrifier_test.exs | 64 +++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/test/pleroma/web/activity_pub/transmogrifier_test.exs b/test/pleroma/web/activity_pub/transmogrifier_test.exs index e34a101b2..108f17caf 100644 --- a/test/pleroma/web/activity_pub/transmogrifier_test.exs +++ b/test/pleroma/web/activity_pub/transmogrifier_test.exs @@ -611,6 +611,70 @@ defmodule Pleroma.Web.ActivityPub.TransmogrifierTest do } = prepared["object"] end + test "Correctly handles Undo activities" do + blocked = insert(:user) + blocker = insert(:user, local: true) + + blocked_ap_id = blocked.ap_id + blocker_ap_id = blocker.ap_id + + {:ok, %Activity{} = block_activity} = CommonAPI.block(blocked, blocker) + {:ok, %Activity{} = undo_activity} = CommonAPI.unblock(blocked, blocker) + {:ok, data} = Transmogrifier.prepare_outgoing(undo_activity.data) + + block_ap_id = block_activity.data["id"] + assert is_binary(block_ap_id) + + assert match?( + %{ + "@context" => [_ | _], + "type" => "Undo", + "id" => "http://localhost" <> _, + "actor" => ^blocker_ap_id, + "object" => ^block_ap_id, + "to" => [^blocked_ap_id], + "cc" => [], + "bto" => [], + "bcc" => [] + }, + data + ) + end + + test "Correctly handles EmojiReact activities" do + user = insert(:user, local: true) + note_activity = insert(:note_activity) + + user_ap_id = user.ap_id + user_followers = user.follower_address + note_author = note_activity.data["actor"] + note_ap_id = note_activity.data["object"] + + assert is_binary(note_author) + assert is_binary(note_ap_id) + + {:ok, react_activity} = CommonAPI.react_with_emoji(note_activity.id, user, "🐈") + {:ok, data} = Transmogrifier.prepare_outgoing(react_activity.data) + + assert match?( + %{ + "@context" => [_ | _], + "type" => "EmojiReact", + "actor" => ^user_ap_id, + "to" => [^user_followers, ^note_author], + "cc" => ["https://www.w3.org/ns/activitystreams#Public"], + "bto" => [], + "bcc" => [], + "content" => "🐈", + "context" => "2hu", + "id" => "http://localhost" <> _, + "object" => ^note_ap_id, + "tag" => [] + }, + data + ) + end + test "it prepares a quote post" do user = insert(:user) From 18d762c01b83900baeb6752f551ddeaf6abda78c Mon Sep 17 00:00:00 2001 From: Oneric Date: Mon, 1 Dec 2025 18:51:39 +0100 Subject: [PATCH 06/29] Add voters key to internal object fields It is inlined and used to keep track of who already voted for a poll. This is expected to be confidential information and must no be exposed --- lib/pleroma/constants.ex | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/pleroma/constants.ex b/lib/pleroma/constants.ex index f02607273..c0411edbf 100644 --- a/lib/pleroma/constants.ex +++ b/lib/pleroma/constants.ex @@ -21,7 +21,8 @@ defmodule Pleroma.Constants do "pleroma_internal", "generator", "rules", - "language" + "language", + "voters" ] ) From 75353282ee140e535e551b02e6f54d3f8a8666ca Mon Sep 17 00:00:00 2001 From: Phantasm Date: Tue, 2 Dec 2025 21:44:21 +0100 Subject: [PATCH 07/29] AP ObjectView: add test for Listen activities --- .../activity_pub/views/object_view_test.exs | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/test/pleroma/web/activity_pub/views/object_view_test.exs b/test/pleroma/web/activity_pub/views/object_view_test.exs index 14258795f..cc276b4b7 100644 --- a/test/pleroma/web/activity_pub/views/object_view_test.exs +++ b/test/pleroma/web/activity_pub/views/object_view_test.exs @@ -95,4 +95,23 @@ defmodule Pleroma.Web.ActivityPub.ObjectViewTest do assert result["object"] == announce.data["id"] assert result["type"] == "Undo" end + + test "renders a listen activity" do + audio = insert(:audio) + user = insert(:user) + + {:ok, listen_activity} = CommonAPI.listen(user, audio.data) + + result = ObjectView.render("object.json", %{object: listen_activity}) + + assert result["id"] == listen_activity.data["id"] + assert result["to"] == listen_activity.data["to"] + assert result["type"] == "Listen" + assert result["object"]["album"] == listen_activity.data["album"] + assert result["object"]["artist"] == listen_activity.data["artist"] + assert result["object"]["length"] == listen_activity.data["length"] + assert result["object"]["title"] == listen_activity.data["title"] + assert result["object"]["type"] == "Audio" + assert result["@context"] + end end From b3887a6fa775bda1f51efab4774cba838ad0db91 Mon Sep 17 00:00:00 2001 From: Phantasm Date: Tue, 2 Dec 2025 23:25:42 +0100 Subject: [PATCH 08/29] AP C2S: Validate visibility for C2S requests to /users/:nickname/outbox A local user could previously send Announce/EmojiReact/Like activities to their outbox referencing objects that aren't visible to them and they would get processed as if can see them. Only requirement is knowing the URI of the object and the users instance having C2S enabled (currently disabled by default). --- .../activity_pub/activity_pub_controller.ex | 21 ++++- .../activity_pub_controller_test.exs | 76 +++++++++++++++++++ 2 files changed, 96 insertions(+), 1 deletion(-) diff --git a/lib/pleroma/web/activity_pub/activity_pub_controller.ex b/lib/pleroma/web/activity_pub/activity_pub_controller.ex index daf0d38e6..3d126e7d3 100644 --- a/lib/pleroma/web/activity_pub/activity_pub_controller.ex +++ b/lib/pleroma/web/activity_pub/activity_pub_controller.ex @@ -482,6 +482,24 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubController do {:ok, activity} end + defp validate_visibility(%User{} = user, %{"type" => type, "object" => object} = activity) do + with {_, %Object{} = normalized_object} <- {:normalize, Object.normalize(object, fetch: false)}, + {_, true} <- {:visibility, Visibility.visible_for_user?(normalized_object, user)} do + {:ok, activity} + else + {:normalize, _} -> + if user.local and type == "Create" do + # Creating new object via C2S + {:ok, activity} + else + {:error, "No such object found"} + end + + {:visibility, _} -> + {:forbidden, "You can't interact with this object"} + end + end + def update_outbox( %{assigns: %{user: %User{nickname: nickname, ap_id: actor} = user}} = conn, %{"nickname" => nickname} = params @@ -493,7 +511,8 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubController do |> Map.put("actor", actor) with {:ok, params} <- fix_user_message(user, params), - {:ok, activity, _} <- Pipeline.common_pipeline(params, local: true), + {:ok, activity} <- validate_visibility(user, params), + {:ok, activity, _} <- Pipeline.common_pipeline(activity, local: true), %Activity{data: activity_data} <- Activity.normalize(activity) do conn |> put_status(:created) diff --git a/test/pleroma/web/activity_pub/activity_pub_controller_test.exs b/test/pleroma/web/activity_pub/activity_pub_controller_test.exs index 2b8418dcd..adb65431c 100644 --- a/test/pleroma/web/activity_pub/activity_pub_controller_test.exs +++ b/test/pleroma/web/activity_pub/activity_pub_controller_test.exs @@ -1706,6 +1706,82 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubControllerTest do assert note_object == Object.normalize(note_activity, fetch: false) end + test "it rejects like activity to object invisible to actor", %{conn: conn} do + user = insert(:user) + stranger = insert(:user, local: true) + {:ok, post} = CommonAPI.post(user, %{status: "cofe", visibility: "private"}) + + assert Pleroma.Web.ActivityPub.Visibility.private?(post) + + post_object = Object.normalize(post, fetch: false) + + data = %{ + type: "Like", + object: %{ + id: post_object.data["id"] + } + } + + conn = + conn + |> assign(:user, stranger) + |> put_req_header("content-type", "application/activity+json") + |> post("/users/#{stranger.nickname}/outbox", data) + + assert json_response(conn, 403) + end + + test "it rejects announce activity to object invisible to actor", %{conn: conn} do + user = insert(:user) + stranger = insert(:user, local: true) + {:ok, post} = CommonAPI.post(user, %{status: "cofe", visibility: "private"}) + + assert Pleroma.Web.ActivityPub.Visibility.private?(post) + + post_object = Object.normalize(post, fetch: false) + + data = %{ + type: "Announce", + object: %{ + id: post_object.data["id"] + } + } + + conn = + conn + |> assign(:user, stranger) + |> put_req_header("content-type", "application/activity+json") + |> post("/users/#{stranger.nickname}/outbox", data) + + assert json_response(conn, 403) + end + + test "it rejects emojireact activity to object invisible to actor", %{conn: conn} do + user = insert(:user) + stranger = insert(:user, local: true) + {:ok, post} = CommonAPI.post(user, %{status: "cofe", visibility: "private"}) + + assert Pleroma.Web.ActivityPub.Visibility.private?(post) + + post_object = Object.normalize(post, fetch: false) + + data = %{ + type: "EmojiReact", + object: %{ + id: post_object.data["id"] + }, + content: "πŸ˜€" + } + + conn = + conn + |> assign(:user, stranger) + |> put_req_header("content-type", "application/activity+json") + |> post("/users/#{stranger.nickname}/outbox", data) + + assert json_response(conn, 403) + end + test "it increases like count when receiving a like action", %{conn: conn} do note_activity = insert(:note_activity) note_object = Object.normalize(note_activity, fetch: false) From a4e480a6368e20e26293fa88205b63d8e3bb837e Mon Sep 17 00:00:00 2001 From: Phantasm Date: Wed, 3 Dec 2025 00:06:32 +0100 Subject: [PATCH 09/29] lint and credo --- .../web/activity_pub/activity_pub_controller.ex | 3 ++- lib/pleroma/web/common_api/activity_draft.ex | 15 ++++++++++----- .../controllers/status_controller_test.exs | 6 ++++-- 3 files changed, 16 insertions(+), 8 deletions(-) diff --git a/lib/pleroma/web/activity_pub/activity_pub_controller.ex b/lib/pleroma/web/activity_pub/activity_pub_controller.ex index 3d126e7d3..ddc836e16 100644 --- a/lib/pleroma/web/activity_pub/activity_pub_controller.ex +++ b/lib/pleroma/web/activity_pub/activity_pub_controller.ex @@ -483,7 +483,8 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubController do end defp validate_visibility(%User{} = user, %{"type" => type, "object" => object} = activity) do - with {_, %Object{} = normalized_object} <- {:normalize, Object.normalize(object, fetch: false)}, + with {_, %Object{} = normalized_object} <- + {:normalize, Object.normalize(object, fetch: false)}, {_, true} <- {:visibility, Visibility.visible_for_user?(normalized_object, user)} do {:ok, activity} else diff --git a/lib/pleroma/web/common_api/activity_draft.ex b/lib/pleroma/web/common_api/activity_draft.ex index 2d212a443..f7bdba2f5 100644 --- a/lib/pleroma/web/common_api/activity_draft.ex +++ b/lib/pleroma/web/common_api/activity_draft.ex @@ -140,9 +140,10 @@ defmodule Pleroma.Web.CommonAPI.ActivityDraft do # If a post was deleted all its activities (except the newly added Delete) are purged too, # thus lookup by Create db ID will yield nil just as if it never existed in the first place. # - # We allow replying to Announce here, due to a Pleroma-FE quirk where if presented with an Announce id - # it will render it as if it was just the normal referenced post, but use the Announce id for replies - # in the in_reply_to_id key of a POST request to /api/v1/statuses, or as an :id in /api/v1/statuses/:id/*. + # We allow replying to Announce here, due to a Pleroma-FE quirk where if presented with + # an Announce id it will render it as if it was just the normal referenced post, but + # use the Announce id for replies in the in_reply_to_id key of a POST request to + # /api/v1/statuses, or as an :id in /api/v1/statuses/:id/*. # TODO: Fix this quirk in FE and remove here and other affected places with %Activity{} = activity <- Activity.get_by_id(id), true <- Visibility.visible_for_user?(activity, draft.user), @@ -156,8 +157,12 @@ defmodule Pleroma.Web.CommonAPI.ActivityDraft do add_error(draft, dgettext("errors", "Replying to a status that is not visibile to user")) {:type, type} -> - add_error(draft, dgettext("errors", "Can only reply to posts, not %{type} activities", - type: inspect(type))) + add_error( + draft, + dgettext("errors", "Can only reply to posts, not %{type} activities", + type: inspect(type) + ) + ) end end diff --git a/test/pleroma/web/mastodon_api/controllers/status_controller_test.exs b/test/pleroma/web/mastodon_api/controllers/status_controller_test.exs index d8558465a..7a16343f1 100644 --- a/test/pleroma/web/mastodon_api/controllers/status_controller_test.exs +++ b/test/pleroma/web/mastodon_api/controllers/status_controller_test.exs @@ -291,7 +291,8 @@ defmodule Pleroma.Web.MastodonAPI.StatusControllerTest do end test "replying to own DM succeeds", %{user: user, conn: conn} do - # this is an "edge" case for visibility: replying user is not part of addressed users (but is the author) + # this is an "edge" case for visibility: replying user is not + # part of addressed users (but is the author) stranger = insert(:user) {:ok, %{id: dm_id} = dm_post_act} = @@ -1546,7 +1547,8 @@ defmodule Pleroma.Web.MastodonAPI.StatusControllerTest do assert match?(%{"error" => _}, resp1) - # unreblog by reblog ID (reblog IDs are accepted by some APIs; ensure it fails here one way or another) + # unreblog by reblog ID (reblog IDs are accepted by some APIs; + # ensure it fails here one way or another) resp2 = build_conn() |> assign(:user, user) From 2b76243ec82f2c246264f2a1f655b975f12e7290 Mon Sep 17 00:00:00 2001 From: Phantasm Date: Wed, 3 Dec 2025 23:34:39 +0100 Subject: [PATCH 10/29] CommonAPI: Fail when user sends report with posts not visible to them --- .../api_spec/operations/report_operation.ex | 12 ++++- lib/pleroma/web/common_api.ex | 19 ++++++++ lib/pleroma/web/common_api/activity_draft.ex | 2 +- .../controllers/report_controller.ex | 6 +++ test/pleroma/web/common_api_test.exs | 41 ++++++++++++++++ .../controllers/report_controller_test.exs | 47 ++++++++++++++++++- 6 files changed, 124 insertions(+), 3 deletions(-) diff --git a/lib/pleroma/web/api_spec/operations/report_operation.ex b/lib/pleroma/web/api_spec/operations/report_operation.ex index f5f88974c..c28e3eda1 100644 --- a/lib/pleroma/web/api_spec/operations/report_operation.ex +++ b/lib/pleroma/web/api_spec/operations/report_operation.ex @@ -24,7 +24,17 @@ defmodule Pleroma.Web.ApiSpec.ReportOperation do requestBody: Helpers.request_body("Parameters", create_request(), required: true), responses: %{ 200 => Operation.response("Report", "application/json", create_response()), - 400 => Operation.response("Report", "application/json", ApiError) + 400 => Operation.response("Report", "application/json", ApiError), + 404 => + Operation.response( + "Report", + "application/json", + %Schema{ + allOf: [ApiError], + title: "Report", + example: %{"error" => "Record not found"} + } + ) } } end diff --git a/lib/pleroma/web/common_api.ex b/lib/pleroma/web/common_api.ex index c1719c8d4..ca7eb8c41 100644 --- a/lib/pleroma/web/common_api.ex +++ b/lib/pleroma/web/common_api.ex @@ -620,6 +620,7 @@ defmodule Pleroma.Web.CommonAPI do with {:ok, account} <- get_reported_account(data.account_id), {:ok, {content_html, _, _}} <- make_report_content_html(data[:comment]), {:ok, statuses} <- get_report_statuses(account, data), + true <- check_statuses_visibility(user, statuses), rules <- get_report_rules(Map.get(data, :rule_ids, nil)) do ActivityPub.flag(%{ context: Utils.generate_context_id(), @@ -630,9 +631,27 @@ defmodule Pleroma.Web.CommonAPI do forward: Map.get(data, :forward, false), rules: rules }) + else + false -> + {:error, :visibility} + + error -> + error end end + defp check_statuses_visibility(user, statuses) when is_list(statuses) do + visibility = for status <- statuses, do: Visibility.visible_for_user?(status, user) + + case Enum.all?(visibility) do + true -> true + _ -> false + end + end + + # There are no statuses associated with the report, pass! + defp check_statuses_visibility(_, status) when status == nil, do: true + defp get_reported_account(account_id) do case User.get_cached_by_id(account_id) do %User{} = account -> {:ok, account} diff --git a/lib/pleroma/web/common_api/activity_draft.ex b/lib/pleroma/web/common_api/activity_draft.ex index f7bdba2f5..759fa302c 100644 --- a/lib/pleroma/web/common_api/activity_draft.ex +++ b/lib/pleroma/web/common_api/activity_draft.ex @@ -147,7 +147,7 @@ defmodule Pleroma.Web.CommonAPI.ActivityDraft do # TODO: Fix this quirk in FE and remove here and other affected places with %Activity{} = activity <- Activity.get_by_id(id), true <- Visibility.visible_for_user?(activity, draft.user), - {:type, type} when type in ["Create", "Announce"] <- {:type, activity.data["type"]} do + {_, type} when type in ["Create", "Announce"] <- {:type, activity.data["type"]} do %__MODULE__{draft | in_reply_to: activity} else nil -> diff --git a/lib/pleroma/web/mastodon_api/controllers/report_controller.ex b/lib/pleroma/web/mastodon_api/controllers/report_controller.ex index 3db80d728..a2f777326 100644 --- a/lib/pleroma/web/mastodon_api/controllers/report_controller.ex +++ b/lib/pleroma/web/mastodon_api/controllers/report_controller.ex @@ -16,6 +16,12 @@ defmodule Pleroma.Web.MastodonAPI.ReportController do def create(%{assigns: %{user: user}, body_params: params} = conn, _) do with {:ok, activity} <- Pleroma.Web.CommonAPI.report(user, params) do render(conn, "show.json", activity: activity) + else + {:error, :visibility} -> + {:error, :not_found, "Record not found"} + + error -> + error end end end diff --git a/test/pleroma/web/common_api_test.exs b/test/pleroma/web/common_api_test.exs index 6b5d31537..eb837b11c 100644 --- a/test/pleroma/web/common_api_test.exs +++ b/test/pleroma/web/common_api_test.exs @@ -1286,6 +1286,47 @@ defmodule Pleroma.Web.CommonAPITest do } = flag_activity end + test "doesn't create a report when post is not visible to user" do + reporter = insert(:user) + target_user = insert(:user) + {:ok, post} = CommonAPI.post(target_user, %{status: "Eric", visibility: "private"}) + + assert Pleroma.Web.ActivityPub.Visibility.private?(post) + refute Pleroma.Web.ActivityPub.Visibility.visible_for_user?(post, reporter) + + # Fails when all status are invisible + report_data = %{ + account_id: target_user.id, + comment: "foobar", + status_ids: [post.id] + } + + assert {:error, :visibility} = CommonAPI.report(reporter, report_data) + end + + test "doesn't create a report when some posts are not visible to user" do + reporter = insert(:user) + target_user = insert(:user) + + {:ok, visible_activity} = CommonAPI.post(target_user, %{status: "cofe"}) + + {:ok, invisibile_activity} = + CommonAPI.post(target_user, %{status: "cawfee", visibility: "private"}) + + assert Pleroma.Web.ActivityPub.Visibility.private?(invisibile_activity) + assert Pleroma.Web.ActivityPub.Visibility.public?(visible_activity) + refute Pleroma.Web.ActivityPub.Visibility.visible_for_user?(invisibile_activity, reporter) + + # Fails when some statuses are invisible + report_data_partial = %{ + account_id: target_user.id, + comment: "foobar", + status_ids: [visible_activity.id, invisibile_activity.id] + } + + assert {:error, :visibility} = CommonAPI.report(reporter, report_data_partial) + end + test "updates report state" do [reporter, target_user] = insert_pair(:user) activity = insert(:note_activity, user: target_user) diff --git a/test/pleroma/web/mastodon_api/controllers/report_controller_test.exs b/test/pleroma/web/mastodon_api/controllers/report_controller_test.exs index 4ab5d0771..50d38ddaf 100644 --- a/test/pleroma/web/mastodon_api/controllers/report_controller_test.exs +++ b/test/pleroma/web/mastodon_api/controllers/report_controller_test.exs @@ -147,7 +147,7 @@ defmodule Pleroma.Web.MastodonAPI.ReportControllerTest do |> json_response_and_validate_schema(400) end - test "returns error when account is not exist", %{ + test "returns error when account does not exist", %{ conn: conn, activity: activity } do @@ -159,6 +159,51 @@ defmodule Pleroma.Web.MastodonAPI.ReportControllerTest do assert json_response_and_validate_schema(conn, 400) == %{"error" => "Account not found"} end + test "returns not found when post isn't visible to reporter", %{user: target_user} do + %{conn: conn, user: reporter} = oauth_access(["write:reports"]) + + {:ok, invisible_activity} = + CommonAPI.post(target_user, %{status: "Invisible!", visibility: "private"}) + + assert Pleroma.Web.ActivityPub.Visibility.private?(invisible_activity) + refute Pleroma.Web.ActivityPub.Visibility.visible_for_user?(invisible_activity, reporter) + + assert %{"error" => "Record not found"} = + conn + |> put_req_header("content-type", "application/json") + |> post( + "/api/v1/reports", + %{"account_id" => target_user.id, "status_ids" => [invisible_activity.id]} + ) + |> json_response_and_validate_schema(404) + end + + test "returns not found when some post aren't visible to reporter", %{ + activity: activity, + user: target_user + } do + %{conn: conn, user: reporter} = oauth_access(["write:reports"]) + + {:ok, invisible_activity} = + CommonAPI.post(target_user, %{status: "Invisible!", visibility: "private"}) + + assert Pleroma.Web.ActivityPub.Visibility.private?(invisible_activity) + assert Pleroma.Web.ActivityPub.Visibility.visible_for_user?(activity, reporter) + refute Pleroma.Web.ActivityPub.Visibility.visible_for_user?(invisible_activity, reporter) + + assert %{"error" => "Record not found"} = + conn + |> put_req_header("content-type", "application/json") + |> post( + "/api/v1/reports", + %{ + "account_id" => target_user.id, + "status_ids" => [activity.id, invisible_activity.id] + } + ) + |> json_response_and_validate_schema(404) + end + test "doesn't fail if an admin has no email", %{conn: conn, target_user: target_user} do insert(:user, %{is_admin: true, email: nil}) From 7f3b3c249137dd0dc23a97c5f6a08a868a4ad294 Mon Sep 17 00:00:00 2001 From: Phantasm Date: Wed, 3 Dec 2025 23:37:46 +0100 Subject: [PATCH 11/29] AP C2S: remove check for local user since user is already authenticated Before a request arrives to update_outbox, it already passed through out Plug authentication (:authenticate), so at this point all users should be local. Also adds Listen Activities to the list of allowed Activities that don't need an existing normalized object referenced in them. --- lib/pleroma/web/activity_pub/activity_pub_controller.ex | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/pleroma/web/activity_pub/activity_pub_controller.ex b/lib/pleroma/web/activity_pub/activity_pub_controller.ex index ddc836e16..5b8c47ccd 100644 --- a/lib/pleroma/web/activity_pub/activity_pub_controller.ex +++ b/lib/pleroma/web/activity_pub/activity_pub_controller.ex @@ -489,8 +489,9 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubController do {:ok, activity} else {:normalize, _} -> - if user.local and type == "Create" do - # Creating new object via C2S + if type in ["Create", "Listen"] do + # Creating new object via C2S; user is local and authenticated + # via the :authenticate Plug pipeline. {:ok, activity} else {:error, "No such object found"} From 21b2fd1e05a059bb7a9cae7b94221e2ac3d6b5f3 Mon Sep 17 00:00:00 2001 From: Phantasm Date: Thu, 4 Dec 2025 23:58:44 +0100 Subject: [PATCH 12/29] AP C2S: reject Flag activities, add visibility refutes to some tests --- lib/pleroma/web/activity_pub/activity_pub_controller.ex | 8 ++++++++ .../web/activity_pub/activity_pub_controller_test.exs | 3 +++ 2 files changed, 11 insertions(+) diff --git a/lib/pleroma/web/activity_pub/activity_pub_controller.ex b/lib/pleroma/web/activity_pub/activity_pub_controller.ex index 5b8c47ccd..5a6ffa156 100644 --- a/lib/pleroma/web/activity_pub/activity_pub_controller.ex +++ b/lib/pleroma/web/activity_pub/activity_pub_controller.ex @@ -482,6 +482,14 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubController do {:ok, activity} end + # We currently lack a Flag ObjectValidator since both CommonAPI and Transmogrifier + # both send it straight to ActivityPub.flag and C2S currently has to go through + # the normal pipeline which requires an ObjectValidator. + # TODO: Add a Flag Activity ObjectValidator + defp validate_visibility(_, %{"type" => "Flag"}) do + {:error, "Flag activities aren't currently supported in C2S"} + end + defp validate_visibility(%User{} = user, %{"type" => type, "object" => object} = activity) do with {_, %Object{} = normalized_object} <- {:normalize, Object.normalize(object, fetch: false)}, diff --git a/test/pleroma/web/activity_pub/activity_pub_controller_test.exs b/test/pleroma/web/activity_pub/activity_pub_controller_test.exs index adb65431c..0f7b199fb 100644 --- a/test/pleroma/web/activity_pub/activity_pub_controller_test.exs +++ b/test/pleroma/web/activity_pub/activity_pub_controller_test.exs @@ -1712,6 +1712,7 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubControllerTest do {:ok, post} = CommonAPI.post(user, %{status: "cofe", visibility: "private"}) assert Pleroma.Web.ActivityPub.Visibility.private?(post) + refute Pleroma.Web.ActivityPub.Visibility.visible_for_user?(post, stranger) post_object = Object.normalize(post, fetch: false) @@ -1737,6 +1738,7 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubControllerTest do {:ok, post} = CommonAPI.post(user, %{status: "cofe", visibility: "private"}) assert Pleroma.Web.ActivityPub.Visibility.private?(post) + refute Pleroma.Web.ActivityPub.Visibility.visible_for_user?(post, stranger) post_object = Object.normalize(post, fetch: false) @@ -1762,6 +1764,7 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubControllerTest do {:ok, post} = CommonAPI.post(user, %{status: "cofe", visibility: "private"}) assert Pleroma.Web.ActivityPub.Visibility.private?(post) + refute Pleroma.Web.ActivityPub.Visibility.visible_for_user?(post, stranger) post_object = Object.normalize(post, fetch: false) From 3f16965178ef3751800ba4bd5e046be956adaeb2 Mon Sep 17 00:00:00 2001 From: Phantasm Date: Fri, 5 Dec 2025 15:58:50 +0100 Subject: [PATCH 13/29] Transmogrifier: update internal fields list according to constant --- test/pleroma/web/activity_pub/transmogrifier_test.exs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/pleroma/web/activity_pub/transmogrifier_test.exs b/test/pleroma/web/activity_pub/transmogrifier_test.exs index 108f17caf..dd71ad64f 100644 --- a/test/pleroma/web/activity_pub/transmogrifier_test.exs +++ b/test/pleroma/web/activity_pub/transmogrifier_test.exs @@ -506,6 +506,9 @@ defmodule Pleroma.Web.ActivityPub.TransmogrifierTest do assert is_nil(modified["object"]["announcements"]) assert is_nil(modified["object"]["announcement_count"]) assert is_nil(modified["object"]["generator"]) + assert is_nil(modified["object"]["rules"]) + assert is_nil(modified["object"]["language"]) + assert is_nil(modified["object"]["voters"]) end test "it strips internal fields of article" do From f91474851050e5d9b67e301d3558f236d253f002 Mon Sep 17 00:00:00 2001 From: Phantasm Date: Fri, 5 Dec 2025 15:59:25 +0100 Subject: [PATCH 14/29] Transmogrifier: make Listen Activity test more strict --- .../web/activity_pub/transmogrifier_test.exs | 32 ++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/test/pleroma/web/activity_pub/transmogrifier_test.exs b/test/pleroma/web/activity_pub/transmogrifier_test.exs index dd71ad64f..18d1a699d 100644 --- a/test/pleroma/web/activity_pub/transmogrifier_test.exs +++ b/test/pleroma/web/activity_pub/transmogrifier_test.exs @@ -566,6 +566,7 @@ defmodule Pleroma.Web.ActivityPub.TransmogrifierTest do test "it can handle Listen activities" do listen_activity = insert(:listen) + # This has an inlined object as in ObjectView {:ok, modified} = Transmogrifier.prepare_outgoing(listen_activity.data) assert modified["type"] == "Listen" @@ -574,7 +575,36 @@ defmodule Pleroma.Web.ActivityPub.TransmogrifierTest do {:ok, activity} = CommonAPI.listen(user, %{"title" => "lain radio episode 1"}) - {:ok, _modified} = Transmogrifier.prepare_outgoing(activity.data) + user_ap_id = user.ap_id + activity_ap_id = activity.data["id"] + activity_to = activity.data["to"] + activity_cc = activity.data["cc"] + object_ap_id = activity.data["object"] + object_type = activity.object.data["type"] + + # This does not have an inlined object + {:ok, modified2} = Transmogrifier.prepare_outgoing(activity.data) + + assert match?( + %{ + "@context" => [_ | _], + "type" => "Listen", + "actor" => ^user_ap_id, + "to" => ^activity_to, + "cc" => ^activity_cc, + "context" => "http://localhost" <> _, + "id" => ^activity_ap_id, + "object" => %{ + "actor" => ^user_ap_id, + "attributedTo" => ^user_ap_id, + "id" => ^object_ap_id, + "type" => ^object_type, + "to" => ^activity_to, + "cc" => ^activity_cc + } + }, + modified2 + ) end test "custom emoji urls are URI encoded" do From 426535bc38330cff207cea4a0ba113b68ecbaee3 Mon Sep 17 00:00:00 2001 From: Phantasm Date: Sat, 6 Dec 2025 23:59:44 +0100 Subject: [PATCH 15/29] CommonAPI: Forbid disallowed status (un)muting and unpinning When a user tried to unpin a status not belonging to them, a full MastoAPI response was sent back even if status was not visible to them. Ditto with (un)mutting except ownership. --- .../api_spec/operations/status_operation.ex | 38 +++++++++++++++++-- lib/pleroma/web/common_api.ex | 13 +++++-- .../controllers/status_controller.ex | 21 ++++++++++ .../controllers/status_controller_test.exs | 35 +++++++++++++++++ 4 files changed, 101 insertions(+), 6 deletions(-) diff --git a/lib/pleroma/web/api_spec/operations/status_operation.ex b/lib/pleroma/web/api_spec/operations/status_operation.ex index daf473fa0..e7dd5d9f5 100644 --- a/lib/pleroma/web/api_spec/operations/status_operation.ex +++ b/lib/pleroma/web/api_spec/operations/status_operation.ex @@ -250,7 +250,19 @@ defmodule Pleroma.Web.ApiSpec.StatusOperation do example: %{ "error" => "Record not found" } - }) + }), + 422 => + Operation.response( + "Unprocessable Entity", + "application/json", + %Schema{ + allOf: [ApiError], + title: "Unprocessable Entity", + example: %{ + "error" => "Someone else's status cannot be unpinned" + } + } + ) } } end @@ -325,7 +337,17 @@ defmodule Pleroma.Web.ApiSpec.StatusOperation do ], responses: %{ 200 => status_response(), - 400 => Operation.response("Error", "application/json", ApiError) + 400 => Operation.response("Error", "application/json", ApiError), + 404 => + Operation.response( + "Unprocessable Entity", + "application/json", + %Schema{ + allOf: [ApiError], + title: "Error", + example: %{"error" => "Record not found"} + } + ) } } end @@ -341,7 +363,17 @@ defmodule Pleroma.Web.ApiSpec.StatusOperation do parameters: [id_param()], responses: %{ 200 => status_response(), - 400 => Operation.response("Error", "application/json", ApiError) + 400 => Operation.response("Error", "application/json", ApiError), + 404 => + Operation.response( + "Error", + "application/json", + %Schema{ + allOf: [ApiError], + title: "Error", + example: %{"error" => "Record not found"} + } + ) } } end diff --git a/lib/pleroma/web/common_api.ex b/lib/pleroma/web/common_api.ex index ca7eb8c41..fc2b0a180 100644 --- a/lib/pleroma/web/common_api.ex +++ b/lib/pleroma/web/common_api.ex @@ -554,6 +554,7 @@ defmodule Pleroma.Web.CommonAPI do @spec unpin(String.t(), User.t()) :: {:ok, Activity.t()} | Pipeline.errors() def unpin(id, user) do with %Activity{} = activity <- create_activity_by_id(id), + true <- activity_belongs_to_actor(activity, user.ap_id), {:ok, unpin_data, _} <- Builder.unpin(user, activity.object), {:ok, _unpin, _} <- Pipeline.common_pipeline(unpin_data, @@ -570,7 +571,8 @@ defmodule Pleroma.Web.CommonAPI do def add_mute(activity, user, params \\ %{}) do expires_in = Map.get(params, :expires_in, 0) - with {:ok, _} <- ThreadMute.add_mute(user.id, activity.data["context"]), + with true <- Visibility.visible_for_user?(activity, user), + {:ok, _} <- ThreadMute.add_mute(user.id, activity.data["context"]), _ <- Pleroma.Notification.mark_context_as_read(user, activity.data["context"]) do if expires_in > 0 do Pleroma.Workers.MuteExpireWorker.new( @@ -583,13 +585,18 @@ defmodule Pleroma.Web.CommonAPI do {:ok, activity} else {:error, _} -> {:error, dgettext("errors", "conversation is already muted")} + false -> {:error, :visibility_error} end end @spec remove_mute(Activity.t(), User.t()) :: {:ok, Activity.t()} | {:error, any()} def remove_mute(%Activity{} = activity, %User{} = user) do - ThreadMute.remove_mute(user.id, activity.data["context"]) - {:ok, activity} + if Visibility.visible_for_user?(activity, user) do + ThreadMute.remove_mute(user.id, activity.data["context"]) + {:ok, activity} + else + {:error, :visibility_error} + end end @spec remove_mute(String.t(), String.t()) :: {:ok, Activity.t()} | {:error, any()} diff --git a/lib/pleroma/web/mastodon_api/controllers/status_controller.ex b/lib/pleroma/web/mastodon_api/controllers/status_controller.ex index 62cc6a670..b8207158d 100644 --- a/lib/pleroma/web/mastodon_api/controllers/status_controller.ex +++ b/lib/pleroma/web/mastodon_api/controllers/status_controller.ex @@ -413,8 +413,15 @@ defmodule Pleroma.Web.MastodonAPI.StatusController do conn, _ ) do + # CommonAPI already checks whether user can unpin with {:ok, activity} <- CommonAPI.unpin(ap_id_or_id, user) do try_render(conn, "show.json", activity: activity, for: user, as: :activity) + else + {:error, :ownership_error} -> + {:error, :unprocessable_entity, "Someone else's status cannot be unpinned"} + + error -> + error end end @@ -462,8 +469,15 @@ defmodule Pleroma.Web.MastodonAPI.StatusController do _ ) do with %Activity{} = activity <- Activity.get_by_id(id), + # CommonAPI already checks whether user is allowed to mute {:ok, activity} <- CommonAPI.add_mute(activity, user, params) do try_render(conn, "show.json", activity: activity, for: user, as: :activity) + else + {:error, :visibility_error} -> + {:error, :not_found, "Record not found"} + + error -> + error end end @@ -476,8 +490,15 @@ defmodule Pleroma.Web.MastodonAPI.StatusController do _ ) do with %Activity{} = activity <- Activity.get_by_id(id), + # CommonAPI already checks whether user is allowed to unmute {:ok, activity} <- CommonAPI.remove_mute(activity, user) do try_render(conn, "show.json", activity: activity, for: user, as: :activity) + else + {:error, :visibility_error} -> + {:error, :not_found, "Record not found"} + + error -> + error end end diff --git a/test/pleroma/web/mastodon_api/controllers/status_controller_test.exs b/test/pleroma/web/mastodon_api/controllers/status_controller_test.exs index 7a16343f1..d90029bdb 100644 --- a/test/pleroma/web/mastodon_api/controllers/status_controller_test.exs +++ b/test/pleroma/web/mastodon_api/controllers/status_controller_test.exs @@ -1769,6 +1769,17 @@ defmodule Pleroma.Web.MastodonAPI.StatusControllerTest do |> json_response_and_validate_schema(404) == %{"error" => "Record not found"} end + test "/unpin: returns 422 error when activity not owned by user", %{activity: activity} do + %{conn: conn} = oauth_access(["write:accounts"]) + + assert conn + |> put_req_header("content-type", "application/json") + |> post("/api/v1/statuses/#{activity.id}/unpin") + |> json_response_and_validate_schema(422) == %{ + "error" => "Someone else's status cannot be unpinned" + } + end + test "max pinned statuses", %{conn: conn, user: user, activity: activity_one} do {:ok, activity_two} = CommonAPI.post(user, %{status: "HI!!!"}) @@ -1977,6 +1988,30 @@ defmodule Pleroma.Web.MastodonAPI.StatusControllerTest do |> post("/api/v1/statuses/#{activity.id}/unmute") |> json_response_and_validate_schema(200) end + + test "cannot mute not visible conversation", %{user: user} do + {:ok, activity} = CommonAPI.post(user, %{status: "Invisible!", visibility: "private"}) + %{conn: conn} = oauth_access(["write:mutes"]) + + assert conn + |> put_req_header("content-type", "application/json") + |> post("/api/v1/statuses/#{activity.id}/mute") + |> json_response_and_validate_schema(404) == %{ + "error" => "Record not found" + } + end + + test "cannot unmute not visible conversation", %{user: user} do + {:ok, activity} = CommonAPI.post(user, %{status: "Invisible!", visibility: "private"}) + %{conn: conn} = oauth_access(["write:mutes"]) + + assert conn + |> put_req_header("content-type", "application/json") + |> post("/api/v1/statuses/#{activity.id}/unmute") + |> json_response_and_validate_schema(404) == %{ + "error" => "Record not found" + } + end end test "Repeated posts that are replies incorrectly have in_reply_to_id null", %{conn: conn} do From 63bdf4dc2bd5258d0f306eeed71e49ed1e4b17a7 Mon Sep 17 00:00:00 2001 From: Phantasm Date: Wed, 10 Dec 2025 01:11:16 +0100 Subject: [PATCH 16/29] C2S: New Add/Remove and Actor creation tests Creating Actors via C2S doesn't make sense, thus it should fail. Tests creating Actors with type: Application/Person/Service. All Create Activities for new Actors currently fail with `validator not set` in the pipeline. --- .../activity_pub_controller_test.exs | 103 ++++++++++++++++++ 1 file changed, 103 insertions(+) diff --git a/test/pleroma/web/activity_pub/activity_pub_controller_test.exs b/test/pleroma/web/activity_pub/activity_pub_controller_test.exs index 0f7b199fb..812a45e7c 100644 --- a/test/pleroma/web/activity_pub/activity_pub_controller_test.exs +++ b/test/pleroma/web/activity_pub/activity_pub_controller_test.exs @@ -1706,6 +1706,109 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubControllerTest do assert note_object == Object.normalize(note_activity, fetch: false) end + test "it rejects Add to other user's collection", %{conn: conn} do + user = insert(:user) + target_user = insert(:user) + + {:ok, activity} = CommonAPI.post(user, %{status: "Post"}) + object = Object.normalize(activity, fetch: false) + object_id = object.data["id"] + + data = %{ + type: "Add", + target: "#{Pleroma.Web.Endpoint.url()}/users/#{target_user.nickname}/collections/featured", + object: object_id + } + + conn = + conn + |> assign(:user, user) + |> put_req_header("content-type", "application/activity+json") + |> post("/users/#{user.nickname}/outbox", data) + + assert json_response(conn, 400) + end + + test "it rejects Remove to other user's collection", %{conn: conn} do + user = insert(:user) + target_user = insert(:user) + + {:ok, activity} = CommonAPI.post(user, %{status: "Post"}) + object = Object.normalize(activity, fetch: false) + object_id = object.data["id"] + + data = %{ + type: "Remove", + target: "#{Pleroma.Web.Endpoint.url()}/users/#{target_user.nickname}/collections/featured", + object: object_id + } + + conn = + conn + |> assign(:user, user) + |> put_req_header("content-type", "application/activity+json") + |> post("/users/#{user.nickname}/outbox", data) + + assert json_response(conn, 400) + end + + test "it rejects creating Actors of type Application", %{conn: conn} do + user = insert(:user, local: true) + + data = %{ + type: "Create", + object: %{ + type: "Application" + } + } + + conn = + conn + |> assign(:user, user) + |> put_req_header("content-type", "application/json") + |> post("/users/#{user.nickname}/outbox", data) + + assert json_response(conn, 400) + end + + test "it rejects creating Actors of type Person", %{conn: conn} do + user = insert(:user, local: true) + + data = %{ + type: "Create", + object: %{ + type: "Person" + } + } + + conn = + conn + |> assign(:user, user) + |> put_req_header("content-type", "application/json") + |> post("/users/#{user.nickname}/outbox", data) + + assert json_response(conn, 400) + end + + test "it rejects creating Actors of type Service", %{conn: conn} do + user = insert(:user, local: true) + + data = %{ + type: "Create", + object: %{ + type: "Service" + } + } + + conn = + conn + |> assign(:user, user) + |> put_req_header("content-type", "application/json") + |> post("/users/#{user.nickname}/outbox", data) + + assert json_response(conn, 400) + end + test "it rejects like activity to object invisible to actor", %{conn: conn} do user = insert(:user) stranger = insert(:user, local: true) From 9d89156b84b8d17c7b228957a142da96e74e7218 Mon Sep 17 00:00:00 2001 From: Phantasm Date: Wed, 10 Dec 2025 11:49:01 +0100 Subject: [PATCH 17/29] AP C2S: Explicitly reject Updates to Actors that failed silently --- .../activity_pub/activity_pub_controller.ex | 15 ++- .../activity_pub_controller_test.exs | 105 +++++++++++++++++- 2 files changed, 115 insertions(+), 5 deletions(-) diff --git a/lib/pleroma/web/activity_pub/activity_pub_controller.ex b/lib/pleroma/web/activity_pub/activity_pub_controller.ex index 5a6ffa156..df5fcf9f6 100644 --- a/lib/pleroma/web/activity_pub/activity_pub_controller.ex +++ b/lib/pleroma/web/activity_pub/activity_pub_controller.ex @@ -486,10 +486,18 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubController do # both send it straight to ActivityPub.flag and C2S currently has to go through # the normal pipeline which requires an ObjectValidator. # TODO: Add a Flag Activity ObjectValidator - defp validate_visibility(_, %{"type" => "Flag"}) do + defp check_allowed_action(_, %{"type" => "Flag"}) do {:error, "Flag activities aren't currently supported in C2S"} end + # It would respond with 201 and silently fail with: + # Could not decode featured collection at fetch #{user.ap_id} \ + # {:error, "Trying to fetch local resource"} + defp check_allowed_action(%{ap_id: ap_id}, %{"type" => "Update", "object" => %{"id" => ap_id}}), + do: {:error, "Updating profile is not currently supported in C2S"} + + defp check_allowed_action(_, activity), do: {:ok, activity} + defp validate_visibility(%User{} = user, %{"type" => type, "object" => object} = activity) do with {_, %Object{} = normalized_object} <- {:normalize, Object.normalize(object, fetch: false)}, @@ -521,8 +529,9 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubController do |> Map.put("actor", actor) with {:ok, params} <- fix_user_message(user, params), - {:ok, activity} <- validate_visibility(user, params), - {:ok, activity, _} <- Pipeline.common_pipeline(activity, local: true), + {:ok, params} <- check_allowed_action(user, params), + {:ok, params} <- validate_visibility(user, params), + {:ok, activity, _} <- Pipeline.common_pipeline(params, local: true), %Activity{data: activity_data} <- Activity.normalize(activity) do conn |> put_status(:created) diff --git a/test/pleroma/web/activity_pub/activity_pub_controller_test.exs b/test/pleroma/web/activity_pub/activity_pub_controller_test.exs index 812a45e7c..8599cf516 100644 --- a/test/pleroma/web/activity_pub/activity_pub_controller_test.exs +++ b/test/pleroma/web/activity_pub/activity_pub_controller_test.exs @@ -1716,7 +1716,8 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubControllerTest do data = %{ type: "Add", - target: "#{Pleroma.Web.Endpoint.url()}/users/#{target_user.nickname}/collections/featured", + target: + "#{Pleroma.Web.Endpoint.url()}/users/#{target_user.nickname}/collections/featured", object: object_id } @@ -1739,7 +1740,8 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubControllerTest do data = %{ type: "Remove", - target: "#{Pleroma.Web.Endpoint.url()}/users/#{target_user.nickname}/collections/featured", + target: + "#{Pleroma.Web.Endpoint.url()}/users/#{target_user.nickname}/collections/featured", object: object_id } @@ -1752,6 +1754,105 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubControllerTest do assert json_response(conn, 400) end + test "it rejects updating Actor's profile", %{conn: conn} do + user = insert(:user, local: true) + + user_object = Pleroma.Web.ActivityPub.UserView.render("user.json", %{user: user}) + user_object_new = Map.put(user_object, "name", "lain") + + data = %{ + type: "Update", + object: user_object_new + } + + conn = + conn + |> assign(:user, user) + |> put_req_header("content-type", "application/json") + |> post("/users/#{user.nickname}/outbox", data) + + updated_user_object = Pleroma.Web.ActivityPub.UserView.render("user.json", %{user: user}) + + assert updated_user_object == user_object + assert json_response(conn, 400) + end + + # Actor publicKey tests are redundant with above test, + # left here for the case that Updating Actors is ever supported + test "it rejects updating Actor's publicKey", %{conn: conn} do + user = insert(:user, local: true) + + {:ok, pem} = Pleroma.Keys.generate_rsa_pem() + {:ok, _, public_key} = Pleroma.Keys.keys_from_pem(pem) + # Taken from UserView + public_key = :public_key.pem_entry_encode(:SubjectPublicKeyInfo, public_key) + public_key = :public_key.pem_encode([public_key]) + + user_object = Pleroma.Web.ActivityPub.UserView.render("user.json", %{user: user}) + user_object_public_key = Map.fetch!(user_object, "publicKey") + user_object_public_key = Map.put(user_object_public_key, "publicKeyPem", public_key) + user_object_new = Map.put(user_object, "publicKey", user_object_public_key) + + refute user_object == user_object_new + + data = %{ + type: "Update", + object: user_object_new + } + + conn = + conn + |> assign(:user, user) + |> put_req_header("content-type", "application/json") + |> post("/users/#{user.nickname}/outbox", data) + + new_user_object = Pleroma.Web.ActivityPub.UserView.render("user.json", %{user: user}) + + assert user_object == new_user_object + assert json_response(conn, 400) + end + + test "it rejects updating Actor's publicKey of another user", %{conn: conn} do + user = insert(:user) + target_user = insert(:user, local: true) + + {:ok, pem} = Pleroma.Keys.generate_rsa_pem() + {:ok, _, public_key} = Pleroma.Keys.keys_from_pem(pem) + # Taken from UserView + public_key = :public_key.pem_entry_encode(:SubjectPublicKeyInfo, public_key) + public_key = :public_key.pem_encode([public_key]) + + target_user_object = + Pleroma.Web.ActivityPub.UserView.render("user.json", %{user: target_user}) + + target_user_object_public_key = Map.fetch!(target_user_object, "publicKey") + + target_user_object_public_key = + Map.put(target_user_object_public_key, "publicKeyPem", public_key) + + target_user_object_new = + Map.put(target_user_object, "publicKey", target_user_object_public_key) + + refute target_user_object == target_user_object_new + + data = %{ + type: "Update", + object: target_user_object_new + } + + conn = + conn + |> assign(:user, user) + |> put_req_header("content-type", "application/json") + |> post("/users/#{target_user.nickname}/outbox", data) + + new_target_user_object = + Pleroma.Web.ActivityPub.UserView.render("user.json", %{user: target_user}) + + assert target_user_object == new_target_user_object + assert json_response(conn, 403) + end + test "it rejects creating Actors of type Application", %{conn: conn} do user = insert(:user, local: true) From 293628fb241e765f33bcbf20b876f19537af7fd4 Mon Sep 17 00:00:00 2001 From: Phantasm Date: Wed, 10 Dec 2025 14:31:22 +0100 Subject: [PATCH 18/29] MastoAPI/CommonAPI: Return 404 when post not visible to user Akkoma patches returned 403 and some of my previous commits returned 422. This unifies the errors returned to 404 "Record not found", gaslighting user just like we do for other endpoints and how Mastodon does it. --- .../api_spec/operations/status_operation.ex | 7 +- lib/pleroma/web/common_api.ex | 16 +++- lib/pleroma/web/common_api/activity_draft.ex | 2 +- .../controllers/status_controller.ex | 5 ++ .../controllers/status_controller_test.exs | 86 +++++++++---------- 5 files changed, 66 insertions(+), 50 deletions(-) diff --git a/lib/pleroma/web/api_spec/operations/status_operation.ex b/lib/pleroma/web/api_spec/operations/status_operation.ex index e7dd5d9f5..b894a2787 100644 --- a/lib/pleroma/web/api_spec/operations/status_operation.ex +++ b/lib/pleroma/web/api_spec/operations/status_operation.ex @@ -178,6 +178,7 @@ defmodule Pleroma.Web.ApiSpec.StatusOperation do parameters: [id_param()], responses: %{ 200 => status_response(), + 400 => Operation.response("Error", "application/json", ApiError), 404 => Operation.response("Not Found", "application/json", ApiError) } } @@ -246,7 +247,7 @@ defmodule Pleroma.Web.ApiSpec.StatusOperation do 404 => Operation.response("Not found", "application/json", %Schema{ allOf: [ApiError], - title: "Unprocessable Entity", + title: "Not Found", example: %{ "error" => "Record not found" } @@ -340,7 +341,7 @@ defmodule Pleroma.Web.ApiSpec.StatusOperation do 400 => Operation.response("Error", "application/json", ApiError), 404 => Operation.response( - "Unprocessable Entity", + "Not Found", "application/json", %Schema{ allOf: [ApiError], @@ -366,7 +367,7 @@ defmodule Pleroma.Web.ApiSpec.StatusOperation do 400 => Operation.response("Error", "application/json", ApiError), 404 => Operation.response( - "Error", + "Not Found", "application/json", %Schema{ allOf: [ApiError], diff --git a/lib/pleroma/web/common_api.ex b/lib/pleroma/web/common_api.ex index fc2b0a180..b53fee6f6 100644 --- a/lib/pleroma/web/common_api.ex +++ b/lib/pleroma/web/common_api.ex @@ -258,7 +258,7 @@ defmodule Pleroma.Web.CommonAPI do {:ok, _} = res -> res - {:error, reason} = res when reason in [:not_found, :forbidden] -> + {:error, :not_found} = res -> res {:error, e} -> @@ -280,7 +280,7 @@ defmodule Pleroma.Web.CommonAPI do {:error, :not_found} {:visible, _} -> - {:error, :forbidden} + {:error, :not_found} {:common_pipeline, {:error, {:validate, {:error, changeset}}}} = e -> if {:object, {"already liked by this actor", []}} in changeset.errors do @@ -539,6 +539,14 @@ defmodule Pleroma.Web.CommonAPI do defp activity_belongs_to_actor(%{actor: actor}, actor), do: true defp activity_belongs_to_actor(_, _), do: {:error, :ownership_error} + defp activity_visible_to_actor(activity, %User{} = user) do + if Visibility.visible_for_user?(activity, user) do + true + else + {:error, :visibility_error} + end + end + defp object_type_is_allowed_for_pin(%{data: %{"type" => type}}) do with false <- type in ["Note", "Article", "Question"] do {:error, :not_allowed} @@ -553,7 +561,11 @@ defmodule Pleroma.Web.CommonAPI do @spec unpin(String.t(), User.t()) :: {:ok, Activity.t()} | Pipeline.errors() def unpin(id, user) do + # Order of visibility/belonging matters for MastoAPI responses. + # post not visible -> 404 + # post visible, not owned -> 422 with %Activity{} = activity <- create_activity_by_id(id), + true <- activity_visible_to_actor(activity, user), true <- activity_belongs_to_actor(activity, user.ap_id), {:ok, unpin_data, _} <- Builder.unpin(user, activity.object), {:ok, _unpin, _} <- diff --git a/lib/pleroma/web/common_api/activity_draft.ex b/lib/pleroma/web/common_api/activity_draft.ex index 759fa302c..f520d5775 100644 --- a/lib/pleroma/web/common_api/activity_draft.ex +++ b/lib/pleroma/web/common_api/activity_draft.ex @@ -154,7 +154,7 @@ defmodule Pleroma.Web.CommonAPI.ActivityDraft do add_error(draft, dgettext("errors", "Cannot reply to a deleted status")) false -> - add_error(draft, dgettext("errors", "Replying to a status that is not visibile to user")) + add_error(draft, dgettext("errors", "Record not found")) {:type, type} -> add_error( diff --git a/lib/pleroma/web/mastodon_api/controllers/status_controller.ex b/lib/pleroma/web/mastodon_api/controllers/status_controller.ex index b8207158d..ea20c0ded 100644 --- a/lib/pleroma/web/mastodon_api/controllers/status_controller.ex +++ b/lib/pleroma/web/mastodon_api/controllers/status_controller.ex @@ -417,6 +417,11 @@ defmodule Pleroma.Web.MastodonAPI.StatusController do with {:ok, activity} <- CommonAPI.unpin(ap_id_or_id, user) do try_render(conn, "show.json", activity: activity, for: user, as: :activity) else + # Order matters, if status is not owned by user and is not visible to user + # return 404 just like other endpoints + {:error, :visibility_error} -> + {:error, :not_found, "Record not found"} + {:error, :ownership_error} -> {:error, :unprocessable_entity, "Someone else's status cannot be unpinned"} diff --git a/test/pleroma/web/mastodon_api/controllers/status_controller_test.exs b/test/pleroma/web/mastodon_api/controllers/status_controller_test.exs index d90029bdb..ad012cfa0 100644 --- a/test/pleroma/web/mastodon_api/controllers/status_controller_test.exs +++ b/test/pleroma/web/mastodon_api/controllers/status_controller_test.exs @@ -1597,22 +1597,17 @@ defmodule Pleroma.Web.MastodonAPI.StatusControllerTest do {:ok, activity} = CommonAPI.post(stranger, %{status: "it can eternal lie", visibility: "private"}) - resp = - conn - |> put_req_header("content-type", "application/json") - |> post("/api/v1/statuses/#{activity.id}/favourite") - |> json_response_and_validate_schema(403) - - assert match?(%{"error" => _}, resp) + assert conn + |> put_req_header("content-type", "application/json") + |> post("/api/v1/statuses/#{activity.id}/favourite") + |> json_response_and_validate_schema(404) == %{"error" => "Record not found"} end test "returns 404 error for a wrong id", %{conn: conn} do - conn = - conn - |> put_req_header("content-type", "application/json") - |> post("/api/v1/statuses/1/favourite") - - assert json_response_and_validate_schema(conn, 404) == %{"error" => "Record not found"} + assert conn + |> put_req_header("content-type", "application/json") + |> post("/api/v1/statuses/1/favourite") + |> json_response_and_validate_schema(404) == %{"error" => "Record not found"} end end @@ -1639,13 +1634,10 @@ defmodule Pleroma.Web.MastodonAPI.StatusControllerTest do activity = insert(:note_activity) # using base post ID - resp = - conn - |> put_req_header("content-type", "application/json") - |> post("/api/v1/statuses/#{activity.id}/unfavourite") - |> json_response(400) - - assert match?(%{"error" => "Could not unfavorite"}, resp) + assert conn + |> put_req_header("content-type", "application/json") + |> post("/api/v1/statuses/#{activity.id}/unfavourite") + |> json_response_and_validate_schema(400) == %{"error" => "Could not unfavorite"} end test "can't unfavourite other user's favs", %{conn: conn} do @@ -1655,13 +1647,10 @@ defmodule Pleroma.Web.MastodonAPI.StatusControllerTest do {:ok, _} = CommonAPI.favorite(activity.id, other) # using base post ID - resp = - conn - |> put_req_header("content-type", "application/json") - |> post("/api/v1/statuses/#{activity.id}/unfavourite") - |> json_response(400) - - assert match?(%{"error" => "Could not unfavorite"}, resp) + assert conn + |> put_req_header("content-type", "application/json") + |> post("/api/v1/statuses/#{activity.id}/unfavourite") + |> json_response_and_validate_schema(400) == %{"error" => "Could not unfavorite"} end test "can't unfavourite other user's favs using their activity", %{conn: conn} do @@ -1670,13 +1659,10 @@ defmodule Pleroma.Web.MastodonAPI.StatusControllerTest do other = insert(:user) {:ok, fav_activity} = CommonAPI.favorite(activity.id, other) # some APIs (used to) take IDs of any activity type, make sure this fails one way or another - resp = - conn - |> put_req_header("content-type", "application/json") - |> post("/api/v1/statuses/#{fav_activity.id}/unfavourite") - |> json_response_and_validate_schema(404) - - assert match?(%{"error" => _}, resp) + assert conn + |> put_req_header("content-type", "application/json") + |> post("/api/v1/statuses/#{fav_activity.id}/unfavourite") + |> json_response_and_validate_schema(404) == %{"error" => "Record not found"} end test "returns 404 error for a wrong id", %{conn: conn} do @@ -1722,7 +1708,7 @@ defmodule Pleroma.Web.MastodonAPI.StatusControllerTest do |> json_response(403) == %{"error" => "Invalid credentials."} end - test "/pin: returns 400 error when activity is not public", %{conn: conn, user: user} do + test "/pin: returns 422 error when activity is not public", %{conn: conn, user: user} do {:ok, dm} = CommonAPI.post(user, %{status: "test", visibility: "direct"}) conn = @@ -1769,8 +1755,23 @@ defmodule Pleroma.Web.MastodonAPI.StatusControllerTest do |> json_response_and_validate_schema(404) == %{"error" => "Record not found"} end + test "/unpin: returns 404 error when activity not visible to user", %{user: user} do + %{conn: conn, user: stranger} = oauth_access(["write:accounts"]) + {:ok, activity} = CommonAPI.post(user, %{status: "yumi", visibility: "private"}) + + refute Pleroma.Web.ActivityPub.Visibility.visible_for_user?(activity, stranger) + + assert conn + |> assign(:user, stranger) + |> put_req_header("content-type", "application/json") + |> post("/api/v1/statuses/#{activity.id}/unpin") + |> json_response_and_validate_schema(404) == %{"error" => "Record not found"} + end + test "/unpin: returns 422 error when activity not owned by user", %{activity: activity} do - %{conn: conn} = oauth_access(["write:accounts"]) + %{conn: conn, user: user} = oauth_access(["write:accounts"]) + + assert Pleroma.Web.ActivityPub.Visibility.visible_for_user?(activity, user) assert conn |> put_req_header("content-type", "application/json") @@ -2169,6 +2170,7 @@ defmodule Pleroma.Web.MastodonAPI.StatusControllerTest do test "fails when base post not visible to current user", %{user: user} do other_user = insert(:user, local: true) + %{conn: conn} = oauth_access(["read:accounts"]) {:ok, activity} = CommonAPI.post(user, %{ @@ -2176,14 +2178,10 @@ defmodule Pleroma.Web.MastodonAPI.StatusControllerTest do visibility: "private" }) - resp = - build_conn() - |> assign(:user, other_user) - |> assign(:token, insert(:oauth_token, user: other_user, scopes: ["read:accounts"])) - |> get("/api/v1/statuses/#{activity.id}/favourited_by") - |> json_response_and_validate_schema(404) - - assert match?(%{"error" => _}, resp) + assert conn + |> assign(:user, other_user) + |> get("/api/v1/statuses/#{activity.id}/favourited_by") + |> json_response_and_validate_schema(404) == %{"error" => "Record not found"} end test "returns empty array when :show_reactions is disabled", %{conn: conn, activity: activity} do From 73a3f06f7169b4de7d9237c7d258e3fd21aeaacf Mon Sep 17 00:00:00 2001 From: Phantasm Date: Thu, 11 Dec 2025 22:31:30 +0100 Subject: [PATCH 19/29] PleromaAPI: Change EmojiReact to invisible post response from 400 to 404 --- .../operations/emoji_reaction_operation.ex | 6 +++-- .../api_spec/schemas/api_not_found_error.ex | 19 ++++++++++++++ lib/pleroma/web/common_api.ex | 2 +- .../controllers/emoji_reaction_controller.ex | 2 +- .../emoji_reaction_controller_test.exs | 25 ++++++++----------- 5 files changed, 36 insertions(+), 18 deletions(-) create mode 100644 lib/pleroma/web/api_spec/schemas/api_not_found_error.ex diff --git a/lib/pleroma/web/api_spec/operations/emoji_reaction_operation.ex b/lib/pleroma/web/api_spec/operations/emoji_reaction_operation.ex index 60fe8f011..505749d85 100644 --- a/lib/pleroma/web/api_spec/operations/emoji_reaction_operation.ex +++ b/lib/pleroma/web/api_spec/operations/emoji_reaction_operation.ex @@ -7,6 +7,7 @@ defmodule Pleroma.Web.ApiSpec.EmojiReactionOperation do alias OpenApiSpex.Schema alias Pleroma.Web.ApiSpec.Schemas.Account alias Pleroma.Web.ApiSpec.Schemas.ApiError + alias Pleroma.Web.ApiSpec.Schemas.ApiNotFoundError alias Pleroma.Web.ApiSpec.Schemas.FlakeID alias Pleroma.Web.ApiSpec.Schemas.Status @@ -36,7 +37,7 @@ defmodule Pleroma.Web.ApiSpec.EmojiReactionOperation do operationId: "EmojiReactionController.index", responses: %{ 200 => array_of_reactions_response(), - 403 => Operation.response("Access denied", "application/json", ApiError) + 404 => Operation.response("Access denied", "application/json", ApiError) } } end @@ -55,7 +56,8 @@ defmodule Pleroma.Web.ApiSpec.EmojiReactionOperation do operationId: "EmojiReactionController.create", responses: %{ 200 => Operation.response("Status", "application/json", Status), - 400 => Operation.response("Bad Request", "application/json", ApiError) + 400 => Operation.response("Bad Request", "application/json", ApiError), + 404 => Operation.response("Not Found", "application/json", ApiNotFoundError) } } end diff --git a/lib/pleroma/web/api_spec/schemas/api_not_found_error.ex b/lib/pleroma/web/api_spec/schemas/api_not_found_error.ex new file mode 100644 index 000000000..48e7abdf8 --- /dev/null +++ b/lib/pleroma/web/api_spec/schemas/api_not_found_error.ex @@ -0,0 +1,19 @@ +# Pleroma: A lightweight social networking server +# Copyright Β© 2017-2022 Pleroma Authors +# SPDX-License-Identifier: AGPL-3.0-only + +defmodule Pleroma.Web.ApiSpec.Schemas.ApiNotFoundError do + alias OpenApiSpex.Schema + + require OpenApiSpex + + OpenApiSpex.schema(%{ + title: "Not Found", + description: "Response schema for 404 API errors", + type: :object, + properties: %{error: %Schema{type: :string}}, + example: %{ + "error" => "Record not found" + } + }) +end diff --git a/lib/pleroma/web/common_api.ex b/lib/pleroma/web/common_api.ex index b53fee6f6..8e073cf41 100644 --- a/lib/pleroma/web/common_api.ex +++ b/lib/pleroma/web/common_api.ex @@ -322,7 +322,7 @@ defmodule Pleroma.Web.CommonAPI do {:ok, activity} else {:visible, _} -> - {:error, dgettext("errors", "Must be able to access post to interact with it")} + {:error, :not_found} _ -> {:error, dgettext("errors", "Could not add reaction emoji")} diff --git a/lib/pleroma/web/pleroma_api/controllers/emoji_reaction_controller.ex b/lib/pleroma/web/pleroma_api/controllers/emoji_reaction_controller.ex index 472c3742a..82abfce7f 100644 --- a/lib/pleroma/web/pleroma_api/controllers/emoji_reaction_controller.ex +++ b/lib/pleroma/web/pleroma_api/controllers/emoji_reaction_controller.ex @@ -39,7 +39,7 @@ defmodule Pleroma.Web.PleromaAPI.EmojiReactionController do render(conn, "index.json", emoji_reactions: reactions, user: user) else - {:visible, _} -> {:error, :forbidden} + {:visible, _} -> {:error, :not_found} _e -> json(conn, []) end end diff --git a/test/pleroma/web/pleroma_api/controllers/emoji_reaction_controller_test.exs b/test/pleroma/web/pleroma_api/controllers/emoji_reaction_controller_test.exs index f74f5ebc0..79b805aca 100644 --- a/test/pleroma/web/pleroma_api/controllers/emoji_reaction_controller_test.exs +++ b/test/pleroma/web/pleroma_api/controllers/emoji_reaction_controller_test.exs @@ -182,9 +182,9 @@ defmodule Pleroma.Web.PleromaAPI.EmojiReactionControllerTest do resp = prepare_conn_of_user(conn, stranger) |> put("/api/v1/pleroma/statuses/#{activity_id}/reactions/🐈") - |> json_response_and_validate_schema(400) + |> json_response_and_validate_schema(404) - assert match?(%{"error" => _}, resp) + assert match?(%{"error" => "Record not found"}, resp) end test "DELETE /api/v1/pleroma/statuses/:id/reactions/:emoji", %{conn: conn} do @@ -408,12 +408,9 @@ defmodule Pleroma.Web.PleromaAPI.EmojiReactionControllerTest do assert match?([%{"name" => _, "count" => _} | _], resp) # Fails for stranger - resp = - prepare_conn_of_user(conn, stranger) - |> get("/api/v1/pleroma/statuses/#{activity_id}/reactions") - |> json_response_and_validate_schema(403) - - assert match?(%{"error" => _}, resp) + assert prepare_conn_of_user(conn, stranger) + |> get("/api/v1/pleroma/statuses/#{activity_id}/reactions") + |> json_response_and_validate_schema(404) == %{"error" => "Record not found"} end test "GET /api/v1/pleroma/statuses/:id/reactions with :show_reactions disabled", %{conn: conn} do @@ -471,13 +468,13 @@ defmodule Pleroma.Web.PleromaAPI.EmojiReactionControllerTest do {%{id: activity_id} = _activity, _author, follower, stranger} = prepare_reacted_post() # Works for follower - prepare_conn_of_user(conn, follower) - |> get("/api/v1/pleroma/statuses/#{activity_id}/reactions/🐈") - |> json_response_and_validate_schema(200) + assert prepare_conn_of_user(conn, follower) + |> get("/api/v1/pleroma/statuses/#{activity_id}/reactions/🐈") + |> json_response_and_validate_schema(200) # Fails for stranger - prepare_conn_of_user(conn, stranger) - |> get("/api/v1/pleroma/statuses/#{activity_id}/reactions/🐈") - |> json_response_and_validate_schema(403) + assert prepare_conn_of_user(conn, stranger) + |> get("/api/v1/pleroma/statuses/#{activity_id}/reactions/🐈") + |> json_response_and_validate_schema(404) == %{"error" => "Record not found"} end end From fe7108cbc28581cf7242c73867a06a68a96ac14b Mon Sep 17 00:00:00 2001 From: Phantasm Date: Thu, 11 Dec 2025 22:37:51 +0100 Subject: [PATCH 20/29] MastoAPI: Unify pin/bookmark/mute/fav not visible responses to 404 Also adds more tests for these interactions. --- .../api_spec/operations/status_operation.ex | 18 ++---- lib/pleroma/web/common_api.ex | 5 +- .../controllers/status_controller.ex | 17 ++++++ test/pleroma/web/common_api_test.exs | 2 +- .../controllers/status_controller_test.exs | 60 +++++++++++++++++++ 5 files changed, 88 insertions(+), 14 deletions(-) diff --git a/lib/pleroma/web/api_spec/operations/status_operation.ex b/lib/pleroma/web/api_spec/operations/status_operation.ex index b894a2787..97f14ea03 100644 --- a/lib/pleroma/web/api_spec/operations/status_operation.ex +++ b/lib/pleroma/web/api_spec/operations/status_operation.ex @@ -8,6 +8,7 @@ defmodule Pleroma.Web.ApiSpec.StatusOperation do alias Pleroma.Web.ApiSpec.AccountOperation alias Pleroma.Web.ApiSpec.Schemas.Account alias Pleroma.Web.ApiSpec.Schemas.ApiError + alias Pleroma.Web.ApiSpec.Schemas.ApiNotFoundError alias Pleroma.Web.ApiSpec.Schemas.Attachment alias Pleroma.Web.ApiSpec.Schemas.BooleanLike alias Pleroma.Web.ApiSpec.Schemas.Emoji @@ -289,7 +290,8 @@ defmodule Pleroma.Web.ApiSpec.StatusOperation do } }), responses: %{ - 200 => status_response() + 200 => status_response(), + 404 => Operation.response("Not found", "application/json", ApiNotFoundError) } } end @@ -303,7 +305,8 @@ defmodule Pleroma.Web.ApiSpec.StatusOperation do operationId: "StatusController.unbookmark", parameters: [id_param()], responses: %{ - 200 => status_response() + 200 => status_response(), + 404 => Operation.response("Not found", "application/json", ApiNotFoundError) } } end @@ -339,16 +342,7 @@ defmodule Pleroma.Web.ApiSpec.StatusOperation do responses: %{ 200 => status_response(), 400 => Operation.response("Error", "application/json", ApiError), - 404 => - Operation.response( - "Not Found", - "application/json", - %Schema{ - allOf: [ApiError], - title: "Error", - example: %{"error" => "Record not found"} - } - ) + 404 => Operation.response("Not found", "application/json", ApiError) } } end diff --git a/lib/pleroma/web/common_api.ex b/lib/pleroma/web/common_api.ex index 8e073cf41..d3c600b97 100644 --- a/lib/pleroma/web/common_api.ex +++ b/lib/pleroma/web/common_api.ex @@ -300,6 +300,7 @@ defmodule Pleroma.Web.CommonAPI do with {_, %Activity{data: %{"type" => "Create"}} = activity} <- {:find_activity, Activity.get_by_id(id)}, %Object{} = note <- Object.normalize(activity, fetch: false), + {_, true} <- {:visibility, activity_visible_to_actor(note, user)}, %Activity{} = like <- Utils.get_existing_like(user.ap_id, note), {_, {:ok, _}} <- {:cancel_jobs, maybe_cancel_jobs(like)}, {:ok, undo, _} <- Builder.undo(user, like), @@ -307,6 +308,7 @@ defmodule Pleroma.Web.CommonAPI do {:ok, activity} else {:find_activity, _} -> {:error, :not_found} + {:visibility, _} -> {:error, :not_found} _ -> {:error, dgettext("errors", "Could not unfavorite")} end end @@ -514,6 +516,7 @@ defmodule Pleroma.Web.CommonAPI do @spec pin(String.t(), User.t()) :: {:ok, Activity.t()} | Pipeline.errors() def pin(id, %User{} = user) do with %Activity{} = activity <- create_activity_by_id(id), + true <- activity_visible_to_actor(activity, user), true <- activity_belongs_to_actor(activity, user.ap_id), true <- object_type_is_allowed_for_pin(activity.object), true <- activity_is_public(activity), @@ -555,7 +558,7 @@ defmodule Pleroma.Web.CommonAPI do defp activity_is_public(activity) do with false <- Visibility.public?(activity) do - {:error, :visibility_error} + {:error, :non_public_error} end end diff --git a/lib/pleroma/web/mastodon_api/controllers/status_controller.ex b/lib/pleroma/web/mastodon_api/controllers/status_controller.ex index ea20c0ded..71fa5a584 100644 --- a/lib/pleroma/web/mastodon_api/controllers/status_controller.ex +++ b/lib/pleroma/web/mastodon_api/controllers/status_controller.ex @@ -393,6 +393,8 @@ defmodule Pleroma.Web.MastodonAPI.StatusController do with {:ok, activity} <- CommonAPI.pin(ap_id_or_id, user) do try_render(conn, "show.json", activity: activity, for: user, as: :activity) else + # Order matters, if status is not owned by user and is not visible to user + # return 404 just like other endpoints {:error, :pinned_statuses_limit_reached} -> {:error, "You have already pinned the maximum number of statuses"} @@ -400,6 +402,9 @@ defmodule Pleroma.Web.MastodonAPI.StatusController do {:error, :unprocessable_entity, "Someone else's status cannot be pinned"} {:error, :visibility_error} -> + {:error, :not_found, "Record not found"} + + {:error, :non_public_error} -> {:error, :unprocessable_entity, "Non-public status cannot be pinned"} error -> @@ -449,6 +454,12 @@ defmodule Pleroma.Web.MastodonAPI.StatusController do ), {:ok, _bookmark} <- Bookmark.create(user.id, activity.id, folder_id) do try_render(conn, "show.json", activity: activity, for: user, as: :activity) + else + false -> + {:error, :not_found, "Record not found"} + + error -> + error end end @@ -462,6 +473,12 @@ defmodule Pleroma.Web.MastodonAPI.StatusController do true <- Visibility.visible_for_user?(activity, user), {:ok, _bookmark} <- Bookmark.destroy(user.id, activity.id) do try_render(conn, "show.json", activity: activity, for: user, as: :activity) + else + false -> + {:error, :not_found, "Record not found"} + + error -> + error end end diff --git a/test/pleroma/web/common_api_test.exs b/test/pleroma/web/common_api_test.exs index eb837b11c..a403fe7aa 100644 --- a/test/pleroma/web/common_api_test.exs +++ b/test/pleroma/web/common_api_test.exs @@ -1072,7 +1072,7 @@ defmodule Pleroma.Web.CommonAPITest do test "only public can be pinned", %{user: user} do {:ok, activity} = CommonAPI.post(user, %{status: "private status", visibility: "private"}) - {:error, :visibility_error} = CommonAPI.pin(activity.id, user) + {:error, :non_public_error} = CommonAPI.pin(activity.id, user) end test "unpin status", %{user: user, activity: activity} do diff --git a/test/pleroma/web/mastodon_api/controllers/status_controller_test.exs b/test/pleroma/web/mastodon_api/controllers/status_controller_test.exs index ad012cfa0..cb1d04f21 100644 --- a/test/pleroma/web/mastodon_api/controllers/status_controller_test.exs +++ b/test/pleroma/web/mastodon_api/controllers/status_controller_test.exs @@ -1630,6 +1630,19 @@ defmodule Pleroma.Web.MastodonAPI.StatusControllerTest do assert to_string(activity.id) == id end + test "can't unfavourite post that isn't visible to user" do + user = insert(:user) + %{conn: conn, user: stranger} = oauth_access(["write:favourites"]) + {:ok, activity} = CommonAPI.post(user, %{status: "invisible", visibility: "private"}) + + refute Pleroma.Web.ActivityPub.Visibility.visible_for_user?(activity, stranger) + + assert conn + |> put_req_header("content-type", "application/json") + |> post("/api/v1/statuses/#{activity.id}/unfavourite") + |> json_response_and_validate_schema(404) == %{"error" => "Record not found"} + end + test "can't unfavourite post that isn't favourited", %{conn: conn} do activity = insert(:note_activity) @@ -1675,6 +1688,19 @@ defmodule Pleroma.Web.MastodonAPI.StatusControllerTest do end end + test "can't favourite post that isn't visible to user" do + user = insert(:user) + %{conn: conn, user: stranger} = oauth_access(["write:favourites"]) + {:ok, activity} = CommonAPI.post(user, %{status: "invisible", visibility: "private"}) + + refute Pleroma.Web.ActivityPub.Visibility.visible_for_user?(activity, stranger) + + assert conn + |> put_req_header("content-type", "application/json") + |> post("/api/v1/statuses/#{activity.id}/favourite") + |> json_response_and_validate_schema(404) == %{"error" => "Record not found"} + end + describe "pinned statuses" do setup do: oauth_access(["write:accounts"]) @@ -1721,6 +1747,18 @@ defmodule Pleroma.Web.MastodonAPI.StatusControllerTest do } end + test "/pin: returns 404 error when activity not visible to user", %{user: user} do + %{conn: conn, user: stranger} = oauth_access(["write:accounts"]) + {:ok, activity} = CommonAPI.post(user, %{status: "invisible", visibility: "private"}) + + refute Pleroma.Web.ActivityPub.Visibility.visible_for_user?(activity, stranger) + + assert conn + |> put_req_header("content-type", "application/json") + |> post("/api/v1/statuses/#{activity.id}/pin") + |> json_response_and_validate_schema(404) == %{"error" => "Record not found"} + end + test "pin by another user", %{activity: activity} do %{conn: conn} = oauth_access(["write:accounts"]) @@ -1892,6 +1930,28 @@ defmodule Pleroma.Web.MastodonAPI.StatusControllerTest do json_response_and_validate_schema(bookmarks, 200) end + test "cannot bookmark invisible post" do + user = insert(:user) + %{conn: conn, user: stranger} = oauth_access(["write:bookmarks"]) + {:ok, activity} = CommonAPI.post(user, %{status: "mocha", visibility: "private"}) + + refute Pleroma.Web.ActivityPub.Visibility.visible_for_user?(activity, stranger) + + resp1 = + conn + |> put_req_header("content-type", "application/json") + |> post("/api/v1/statuses/#{activity.id}/bookmark") + + assert json_response_and_validate_schema(resp1, 404) == %{"error" => "Record not found"} + + resp2 = + conn + |> put_req_header("content-type", "application/json") + |> post("/api/v1/statuses/#{activity.id}/unbookmark") + + assert json_response_and_validate_schema(resp2, 404) == %{"error" => "Record not found"} + end + test "bookmark folders" do %{conn: conn, user: user} = oauth_access(["write:bookmarks", "read:bookmarks"]) From 374305d5fe8313bf2d53833cb6073b2639adcaf0 Mon Sep 17 00:00:00 2001 From: Phantasm Date: Thu, 11 Dec 2025 23:28:17 +0100 Subject: [PATCH 21/29] AP C2S: Add reply test --- .../activity_pub_controller_test.exs | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/test/pleroma/web/activity_pub/activity_pub_controller_test.exs b/test/pleroma/web/activity_pub/activity_pub_controller_test.exs index 8599cf516..bd84e0001 100644 --- a/test/pleroma/web/activity_pub/activity_pub_controller_test.exs +++ b/test/pleroma/web/activity_pub/activity_pub_controller_test.exs @@ -1580,6 +1580,41 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubControllerTest do assert object["content"] == activity["object"]["content"] end + test "it inserts an incoming reply create activity into the database", %{conn: conn} do + user = insert(:user) + replying_user = insert(:user) + + {:ok, activity} = CommonAPI.post(user, %{status: "cofe"}) + + data = %{ + type: "Create", + object: %{ + to: [Pleroma.Constants.as_public(), user.ap_id], + cc: [replying_user.follower_address], + inReplyTo: activity.object.data["id"], + content: "green tea", + type: "Note" + } + } + + result = + conn + |> assign(:user, replying_user) + |> put_req_header("content-type", "application/activity+json") + |> post("/users/#{replying_user.nickname}/outbox", data) + |> json_response(201) + + updated_object = Object.normalize(activity.object.data["id"], fetch: false) + + assert Activity.get_by_ap_id(result["id"]) + assert result["object"] + assert %Object{data: object} = Object.normalize(result["object"], fetch: false) + assert object["content"] == data.object.content + assert Pleroma.Web.ActivityPub.Visibility.public?(object) + assert object["inReplyTo"] == activity.object.data["id"] + assert updated_object.data["repliesCount"] == 1 + end + test "it rejects anything beyond 'Note' creations", %{conn: conn, activity: activity} do user = insert(:user) From 53f23dd2596c1a9b7676f886c7b93c5bd2701392 Mon Sep 17 00:00:00 2001 From: Phantasm Date: Thu, 11 Dec 2025 23:50:03 +0100 Subject: [PATCH 22/29] MastoAPI docs: Remove unused 403 respones --- lib/pleroma/web/api_spec/operations/status_operation.ex | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/pleroma/web/api_spec/operations/status_operation.ex b/lib/pleroma/web/api_spec/operations/status_operation.ex index 97f14ea03..a4cf0a396 100644 --- a/lib/pleroma/web/api_spec/operations/status_operation.ex +++ b/lib/pleroma/web/api_spec/operations/status_operation.ex @@ -163,7 +163,6 @@ defmodule Pleroma.Web.ApiSpec.StatusOperation do parameters: [id_param()], responses: %{ 200 => status_response(), - 403 => Operation.response("Access denied", "application/json", ApiError), 404 => Operation.response("Not Found", "application/json", ApiError) } } @@ -416,7 +415,6 @@ defmodule Pleroma.Web.ApiSpec.StatusOperation do "application/json", AccountOperation.array_of_accounts() ), - 403 => Operation.response("Access denied", "application/json", ApiError), 404 => Operation.response("Not Found", "application/json", ApiError) } } From 6f55763db44a4fb7ef821884d0136428f12c2d09 Mon Sep 17 00:00:00 2001 From: Phantasm Date: Fri, 12 Dec 2025 00:11:11 +0100 Subject: [PATCH 23/29] add changelogs --- changelog.d/ap-c2s-interaction-perms.fix | 1 + changelog.d/mastoapi-interaction-perms.fix | 1 + 2 files changed, 2 insertions(+) create mode 100644 changelog.d/ap-c2s-interaction-perms.fix create mode 100644 changelog.d/mastoapi-interaction-perms.fix diff --git a/changelog.d/ap-c2s-interaction-perms.fix b/changelog.d/ap-c2s-interaction-perms.fix new file mode 100644 index 000000000..18caf9b2f --- /dev/null +++ b/changelog.d/ap-c2s-interaction-perms.fix @@ -0,0 +1 @@ +AP C2S: Reject interactions with statuses not visible to Actor diff --git a/changelog.d/mastoapi-interaction-perms.fix b/changelog.d/mastoapi-interaction-perms.fix new file mode 100644 index 000000000..857d59400 --- /dev/null +++ b/changelog.d/mastoapi-interaction-perms.fix @@ -0,0 +1 @@ +MastodonAPI: Reject interactions with statuses not visible to user From 49a5630c75b4eb9f9ec764dab0acc5e66c370609 Mon Sep 17 00:00:00 2001 From: Phantasm Date: Fri, 12 Dec 2025 18:05:58 +0100 Subject: [PATCH 24/29] CommonAPI: Standardize visibility error, use helper function if possible --- lib/pleroma/web/common_api.ex | 29 ++++++++++--------- .../controllers/report_controller.ex | 2 +- test/pleroma/web/common_api_test.exs | 4 +-- 3 files changed, 18 insertions(+), 17 deletions(-) diff --git a/lib/pleroma/web/common_api.ex b/lib/pleroma/web/common_api.ex index d3c600b97..64530ef68 100644 --- a/lib/pleroma/web/common_api.ex +++ b/lib/pleroma/web/common_api.ex @@ -269,7 +269,7 @@ defmodule Pleroma.Web.CommonAPI do defp favorite_helper(user, id) do with {_, %Activity{object: object}} <- {:find_object, Activity.get_by_id_with_object(id)}, - {_, true} <- {:visible, Visibility.visible_for_user?(object, user)}, + {_, true} <- {:visibility_error, activity_visible_to_actor(object, user)}, {_, {:ok, like_object, meta}} <- {:build_object, Builder.like(user, object)}, {_, {:ok, %Activity{} = activity, _meta}} <- {:common_pipeline, @@ -279,7 +279,7 @@ defmodule Pleroma.Web.CommonAPI do {:find_object, _} -> {:error, :not_found} - {:visible, _} -> + {:visibility_error, _} -> {:error, :not_found} {:common_pipeline, {:error, {:validate, {:error, changeset}}}} = e -> @@ -300,7 +300,7 @@ defmodule Pleroma.Web.CommonAPI do with {_, %Activity{data: %{"type" => "Create"}} = activity} <- {:find_activity, Activity.get_by_id(id)}, %Object{} = note <- Object.normalize(activity, fetch: false), - {_, true} <- {:visibility, activity_visible_to_actor(note, user)}, + {_, true} <- {:visibility_error, activity_visible_to_actor(note, user)}, %Activity{} = like <- Utils.get_existing_like(user.ap_id, note), {_, {:ok, _}} <- {:cancel_jobs, maybe_cancel_jobs(like)}, {:ok, undo, _} <- Builder.undo(user, like), @@ -308,7 +308,7 @@ defmodule Pleroma.Web.CommonAPI do {:ok, activity} else {:find_activity, _} -> {:error, :not_found} - {:visibility, _} -> {:error, :not_found} + {:visibility_error, _} -> {:error, :not_found} _ -> {:error, dgettext("errors", "Could not unfavorite")} end end @@ -317,13 +317,13 @@ defmodule Pleroma.Web.CommonAPI do {:ok, Activity.t()} | {:error, String.t()} def react_with_emoji(id, user, emoji) do with %Activity{} = activity <- Activity.get_by_id(id), - {_, true} <- {:visible, Visibility.visible_for_user?(activity, user)}, + {_, true} <- {:visibility_error, activity_visible_to_actor(activity, user)}, object <- Object.normalize(activity, fetch: false), {:ok, emoji_react, _} <- Builder.emoji_react(user, object, emoji), {:ok, activity, _} <- Pipeline.common_pipeline(emoji_react, local: true) do {:ok, activity} else - {:visible, _} -> + {:visibility_error, _} -> {:error, :not_found} _ -> @@ -586,7 +586,7 @@ defmodule Pleroma.Web.CommonAPI do def add_mute(activity, user, params \\ %{}) do expires_in = Map.get(params, :expires_in, 0) - with true <- Visibility.visible_for_user?(activity, user), + with true <- activity_visible_to_actor(activity, user), {:ok, _} <- ThreadMute.add_mute(user.id, activity.data["context"]), _ <- Pleroma.Notification.mark_context_as_read(user, activity.data["context"]) do if expires_in > 0 do @@ -599,18 +599,19 @@ defmodule Pleroma.Web.CommonAPI do {:ok, activity} else + {:error, :visibility_error} -> {:error, :visibility_error} {:error, _} -> {:error, dgettext("errors", "conversation is already muted")} - false -> {:error, :visibility_error} end end @spec remove_mute(Activity.t(), User.t()) :: {:ok, Activity.t()} | {:error, any()} def remove_mute(%Activity{} = activity, %User{} = user) do - if Visibility.visible_for_user?(activity, user) do - ThreadMute.remove_mute(user.id, activity.data["context"]) - {:ok, activity} - else - {:error, :visibility_error} + case activity_visible_to_actor(activity, user) do + true -> + ThreadMute.remove_mute(user.id, activity.data["context"]) + {:ok, activity} + error -> + error end end @@ -655,7 +656,7 @@ defmodule Pleroma.Web.CommonAPI do }) else false -> - {:error, :visibility} + {:error, :visibility_error} error -> error diff --git a/lib/pleroma/web/mastodon_api/controllers/report_controller.ex b/lib/pleroma/web/mastodon_api/controllers/report_controller.ex index a2f777326..e3496cac0 100644 --- a/lib/pleroma/web/mastodon_api/controllers/report_controller.ex +++ b/lib/pleroma/web/mastodon_api/controllers/report_controller.ex @@ -17,7 +17,7 @@ defmodule Pleroma.Web.MastodonAPI.ReportController do with {:ok, activity} <- Pleroma.Web.CommonAPI.report(user, params) do render(conn, "show.json", activity: activity) else - {:error, :visibility} -> + {:error, :visibility_error} -> {:error, :not_found, "Record not found"} error -> diff --git a/test/pleroma/web/common_api_test.exs b/test/pleroma/web/common_api_test.exs index a403fe7aa..da39f59ad 100644 --- a/test/pleroma/web/common_api_test.exs +++ b/test/pleroma/web/common_api_test.exs @@ -1301,7 +1301,7 @@ defmodule Pleroma.Web.CommonAPITest do status_ids: [post.id] } - assert {:error, :visibility} = CommonAPI.report(reporter, report_data) + assert {:error, :visibility_error} = CommonAPI.report(reporter, report_data) end test "doesn't create a report when some posts are not visible to user" do @@ -1324,7 +1324,7 @@ defmodule Pleroma.Web.CommonAPITest do status_ids: [visible_activity.id, invisibile_activity.id] } - assert {:error, :visibility} = CommonAPI.report(reporter, report_data_partial) + assert {:error, :visibility_error} = CommonAPI.report(reporter, report_data_partial) end test "updates report state" do From d36d0abd276e0ec26d96f99e30ee0bbf331d204f Mon Sep 17 00:00:00 2001 From: Phantasm Date: Fri, 12 Dec 2025 21:17:58 +0100 Subject: [PATCH 25/29] API Docs: Switch some added 404 API response to ApiNotFoundError schema --- .../operations/emoji_reaction_operation.ex | 2 +- .../api_spec/operations/report_operation.ex | 12 ++-------- .../api_spec/operations/status_operation.ex | 22 +++---------------- 3 files changed, 6 insertions(+), 30 deletions(-) diff --git a/lib/pleroma/web/api_spec/operations/emoji_reaction_operation.ex b/lib/pleroma/web/api_spec/operations/emoji_reaction_operation.ex index 505749d85..f1ae0b261 100644 --- a/lib/pleroma/web/api_spec/operations/emoji_reaction_operation.ex +++ b/lib/pleroma/web/api_spec/operations/emoji_reaction_operation.ex @@ -37,7 +37,7 @@ defmodule Pleroma.Web.ApiSpec.EmojiReactionOperation do operationId: "EmojiReactionController.index", responses: %{ 200 => array_of_reactions_response(), - 404 => Operation.response("Access denied", "application/json", ApiError) + 404 => Operation.response("Access denied", "application/json", ApiNotFoundError) } } end diff --git a/lib/pleroma/web/api_spec/operations/report_operation.ex b/lib/pleroma/web/api_spec/operations/report_operation.ex index c28e3eda1..c29598b3f 100644 --- a/lib/pleroma/web/api_spec/operations/report_operation.ex +++ b/lib/pleroma/web/api_spec/operations/report_operation.ex @@ -7,6 +7,7 @@ defmodule Pleroma.Web.ApiSpec.ReportOperation do alias OpenApiSpex.Schema alias Pleroma.Web.ApiSpec.Helpers alias Pleroma.Web.ApiSpec.Schemas.ApiError + alias Pleroma.Web.ApiSpec.Schemas.ApiNotFoundError alias Pleroma.Web.ApiSpec.Schemas.BooleanLike def open_api_operation(action) do @@ -25,16 +26,7 @@ defmodule Pleroma.Web.ApiSpec.ReportOperation do responses: %{ 200 => Operation.response("Report", "application/json", create_response()), 400 => Operation.response("Report", "application/json", ApiError), - 404 => - Operation.response( - "Report", - "application/json", - %Schema{ - allOf: [ApiError], - title: "Report", - example: %{"error" => "Record not found"} - } - ) + 404 => Operation.response("Report", "application/json", ApiNotFoundError) } } end diff --git a/lib/pleroma/web/api_spec/operations/status_operation.ex b/lib/pleroma/web/api_spec/operations/status_operation.ex index a4cf0a396..687c64844 100644 --- a/lib/pleroma/web/api_spec/operations/status_operation.ex +++ b/lib/pleroma/web/api_spec/operations/status_operation.ex @@ -244,14 +244,7 @@ defmodule Pleroma.Web.ApiSpec.StatusOperation do "error" => "You have already pinned the maximum number of statuses" } }), - 404 => - Operation.response("Not found", "application/json", %Schema{ - allOf: [ApiError], - title: "Not Found", - example: %{ - "error" => "Record not found" - } - }), + 404 => Operation.response("Not found", "application/json", ApiNotFoundError), 422 => Operation.response( "Unprocessable Entity", @@ -341,7 +334,7 @@ defmodule Pleroma.Web.ApiSpec.StatusOperation do responses: %{ 200 => status_response(), 400 => Operation.response("Error", "application/json", ApiError), - 404 => Operation.response("Not found", "application/json", ApiError) + 404 => Operation.response("Not found", "application/json", ApiNotFoundError) } } end @@ -358,16 +351,7 @@ defmodule Pleroma.Web.ApiSpec.StatusOperation do responses: %{ 200 => status_response(), 400 => Operation.response("Error", "application/json", ApiError), - 404 => - Operation.response( - "Not Found", - "application/json", - %Schema{ - allOf: [ApiError], - title: "Error", - example: %{"error" => "Record not found"} - } - ) + 404 => Operation.response("Not Found", "application/json", ApiNotFoundError) } } end From 3466b626d6a4a92c873c1a402e1c955e647b54cc Mon Sep 17 00:00:00 2001 From: Phantasm Date: Sun, 14 Dec 2025 14:06:38 +0100 Subject: [PATCH 26/29] lint --- lib/pleroma/web/common_api.ex | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/pleroma/web/common_api.ex b/lib/pleroma/web/common_api.ex index 64530ef68..8e96ef5b6 100644 --- a/lib/pleroma/web/common_api.ex +++ b/lib/pleroma/web/common_api.ex @@ -610,6 +610,7 @@ defmodule Pleroma.Web.CommonAPI do true -> ThreadMute.remove_mute(user.id, activity.data["context"]) {:ok, activity} + error -> error end From 4b168691fefd2dc168ed900c2521f51b6c0e602d Mon Sep 17 00:00:00 2001 From: Phantasm Date: Tue, 16 Dec 2025 20:40:55 +0100 Subject: [PATCH 27/29] add missing changelog --- changelog.d/view-internals-leaks.fix | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/view-internals-leaks.fix diff --git a/changelog.d/view-internals-leaks.fix b/changelog.d/view-internals-leaks.fix new file mode 100644 index 000000000..a1a09afe1 --- /dev/null +++ b/changelog.d/view-internals-leaks.fix @@ -0,0 +1 @@ +ObjectView: Do not leak unsanitized internal representation of non-Create/non-Undo Activities on fetches From ed538603fb5f2941bbdce61e11e9e3ca6adec395 Mon Sep 17 00:00:00 2001 From: Lain Soykaf Date: Sun, 21 Dec 2025 14:04:19 +0400 Subject: [PATCH 28/29] TransmogrifierTest: Add failing test for Update. --- .../web/activity_pub/transmogrifier_test.exs | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/test/pleroma/web/activity_pub/transmogrifier_test.exs b/test/pleroma/web/activity_pub/transmogrifier_test.exs index 18d1a699d..7934d3c34 100644 --- a/test/pleroma/web/activity_pub/transmogrifier_test.exs +++ b/test/pleroma/web/activity_pub/transmogrifier_test.exs @@ -9,7 +9,10 @@ defmodule Pleroma.Web.ActivityPub.TransmogrifierTest do alias Pleroma.Activity alias Pleroma.Object alias Pleroma.User + alias Pleroma.Web.ActivityPub.Builder + alias Pleroma.Web.ActivityPub.Pipeline alias Pleroma.Web.ActivityPub.Transmogrifier + alias Pleroma.Web.ActivityPub.UserView alias Pleroma.Web.ActivityPub.Utils alias Pleroma.Web.AdminAPI.AccountView alias Pleroma.Web.CommonAPI @@ -644,6 +647,30 @@ defmodule Pleroma.Web.ActivityPub.TransmogrifierTest do } = prepared["object"] end + test "Updates of Actors are handled" do + user = insert(:user, local: true) + + changeset = User.update_changeset(user, %{name: "new name"}) + {:ok, unpersisted_user} = Ecto.Changeset.apply_action(changeset, :update) + + updated_object = + UserView.render("user.json", user: unpersisted_user) + |> Map.delete("@context") + + {:ok, update_data, []} = Builder.update(user, updated_object) + + {:ok, activity, _} = + Pipeline.common_pipeline(update_data, + local: true, + user_update_changeset: changeset + ) + + assert {:ok, prepared} = Transmogrifier.prepare_outgoing(activity.data) + assert prepared["type"] == "Update" + assert prepared["@context"] + assert prepared["object"]["type"] == user.actor_type + end + test "Correctly handles Undo activities" do blocked = insert(:user) blocker = insert(:user, local: true) From 98f300c5ae98c2057ff91a03537b17bd0bf8aa78 Mon Sep 17 00:00:00 2001 From: Lain Soykaf Date: Sun, 21 Dec 2025 14:16:57 +0400 Subject: [PATCH 29/29] Transmogrifier: Handle user updates. --- lib/pleroma/web/activity_pub/transmogrifier.ex | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/lib/pleroma/web/activity_pub/transmogrifier.ex b/lib/pleroma/web/activity_pub/transmogrifier.ex index c9cd91e9e..74d259084 100644 --- a/lib/pleroma/web/activity_pub/transmogrifier.ex +++ b/lib/pleroma/web/activity_pub/transmogrifier.ex @@ -851,6 +851,23 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do {:ok, data} end + def prepare_outgoing(%{"type" => "Update", "object" => %{"type" => objtype} = object} = data) + when objtype in Pleroma.Constants.actor_types() do + object = + object + |> maybe_fix_user_object() + |> strip_internal_fields() + + data = + data + |> Map.put("object", object) + |> strip_internal_fields() + |> Map.merge(Utils.make_json_ld_header(object)) + |> Map.delete("bcc") + + {:ok, data} + end + def prepare_outgoing(%{"type" => "Update", "object" => %{}} = data) do raise "Requested to serve an Update for non-updateable object type: #{inspect(data)}" end