fix pattern matching in fetch errors

This commit is contained in:
Floatingghost 2024-04-13 23:55:26 +01:00
parent 18442dcc7e
commit 2fc25980d1
7 changed files with 47 additions and 27 deletions

View file

@ -136,10 +136,13 @@ defmodule Pleroma.Object.Fetcher do
def fetch_object_from_id(id, options \\ []) do def fetch_object_from_id(id, options \\ []) do
with %URI{} = uri <- URI.parse(id), with %URI{} = uri <- URI.parse(id),
# let's check the URI is even vaguely valid first # let's check the URI is even vaguely valid first
{:valid_uri_scheme, true} <- {:valid_uri_scheme, uri.scheme == "http" or uri.scheme == "https"}, {:valid_uri_scheme, true} <-
{:valid_uri_scheme, uri.scheme == "http" or uri.scheme == "https"},
# If we have instance restrictions, apply them here to prevent fetching from unwanted instances # If we have instance restrictions, apply them here to prevent fetching from unwanted instances
{:ok, nil} <- Pleroma.Web.ActivityPub.MRF.SimplePolicy.check_reject(uri), {:mrf_reject_check, {:ok, nil}} <-
{:ok, _} <- Pleroma.Web.ActivityPub.MRF.SimplePolicy.check_accept(uri), {:mrf_reject_check, Pleroma.Web.ActivityPub.MRF.SimplePolicy.check_reject(uri)},
{:mrf_accept_check, {:ok, _}} <-
{:mrf_accept_check, Pleroma.Web.ActivityPub.MRF.SimplePolicy.check_accept(uri)},
{_, nil} <- {:fetch_object, Object.get_cached_by_ap_id(id)}, {_, nil} <- {:fetch_object, Object.get_cached_by_ap_id(id)},
{_, true} <- {:allowed_depth, Federator.allowed_thread_distance?(options[:depth])}, {_, true} <- {:allowed_depth, Federator.allowed_thread_distance?(options[:depth])},
{_, {:ok, data}} <- {:fetch, fetch_and_contain_remote_object_from_id(id)}, {_, {:ok, data}} <- {:fetch, fetch_and_contain_remote_object_from_id(id)},
@ -159,6 +162,14 @@ defmodule Pleroma.Object.Fetcher do
log_fetch_error(id, e) log_fetch_error(id, e)
{:error, :invalid_uri_scheme} {:error, :invalid_uri_scheme}
{:mrf_reject_check, _} = e ->
log_fetch_error(id, e)
{:reject, :mrf}
{:mrf_accept_check, _} = e ->
log_fetch_error(id, e)
{:reject, :mrf}
{:containment, reason} = e -> {:containment, reason} = e ->
log_fetch_error(id, e) log_fetch_error(id, e)
{:error, reason} {:error, reason}
@ -188,9 +199,6 @@ defmodule Pleroma.Object.Fetcher do
log_fetch_error(id, e) log_fetch_error(id, e)
{:error, reason} {:error, reason}
{:reject, reason} ->
{:reject, reason}
e -> e ->
log_fetch_error(id, e) log_fetch_error(id, e)
{:error, e} {:error, e}
@ -255,7 +263,12 @@ defmodule Pleroma.Object.Fetcher do
Logger.debug("Fetching object #{id} via AP") Logger.debug("Fetching object #{id} via AP")
with {:valid_uri_scheme, true} <- {:valid_uri_scheme, String.starts_with?(id, "http")}, with {:valid_uri_scheme, true} <- {:valid_uri_scheme, String.starts_with?(id, "http")},
{_, :ok} <- {:local_fetch, Containment.contain_local_fetch(id)}, %URI{} = uri <- URI.parse(id),
{:mrf_reject_check, {:ok, nil}} <-
{:mrf_reject_check, Pleroma.Web.ActivityPub.MRF.SimplePolicy.check_reject(uri)},
{:mrf_accept_check, {:ok, _}} <-
{:mrf_accept_check, Pleroma.Web.ActivityPub.MRF.SimplePolicy.check_accept(uri)},
{:local_fetch, :ok} <- {:local_fetch, Containment.contain_local_fetch(id)},
{:ok, final_id, body} <- get_object(id), {:ok, final_id, body} <- get_object(id),
{:ok, data} <- safe_json_decode(body), {:ok, data} <- safe_json_decode(body),
{_, :ok} <- {:strict_id, Containment.contain_id_to_fetch(final_id, data)}, {_, :ok} <- {:strict_id, Containment.contain_id_to_fetch(final_id, data)},
@ -266,10 +279,18 @@ defmodule Pleroma.Object.Fetcher do
{:ok, data} {:ok, data}
else else
{:strict_id, _} = e-> {:strict_id, _} = e ->
log_fetch_error(id, e) log_fetch_error(id, e)
{:error, :id_mismatch} {:error, :id_mismatch}
{:mrf_reject_check, _} = e ->
log_fetch_error(id, e)
{:reject, :mrf}
{:mrf_accept_check, _} = e ->
log_fetch_error(id, e)
{:reject, :mrf}
{:valid_uri_scheme, _} = e -> {:valid_uri_scheme, _} = e ->
log_fetch_error(id, e) log_fetch_error(id, e)
{:error, :invalid_uri_scheme} {:error, :invalid_uri_scheme}

View file

@ -1730,7 +1730,7 @@ defmodule Pleroma.Web.ActivityPub.ActivityPub do
Logger.debug("Could not decode user at fetch #{ap_id}, #{inspect(e)}") Logger.debug("Could not decode user at fetch #{ap_id}, #{inspect(e)}")
{:error, e} {:error, e}
{:error, {:reject, reason} = e} -> {:reject, reason} = e ->
Logger.debug("Rejected user #{ap_id}: #{inspect(reason)}") Logger.debug("Rejected user #{ap_id}: #{inspect(reason)}")
{:error, e} {:error, e}

View file

@ -162,7 +162,7 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do
object object
end end
else else
object object
end end
end end

View file

@ -22,6 +22,18 @@ defmodule Pleroma.Workers.RemoteFetcherWorker do
{:error, :allowed_depth} -> {:error, :allowed_depth} ->
{:discard, :allowed_depth} {:discard, :allowed_depth}
{:error, :invalid_uri_scheme} ->
{:discard, :invalid_uri_scheme}
{:error, :local_resource} ->
{:discard, :local_resource}
{:reject, _} ->
{:discard, :reject}
{:error, :id_mismatch} ->
{:discard, :id_mismatch}
_ -> _ ->
:error :error
end end

View file

@ -252,12 +252,12 @@ defmodule Pleroma.Object.FetcherTest do
end end
test "it does not fetch a spoofed object with id different from URL" do test "it does not fetch a spoofed object with id different from URL" do
assert {:error, "Object's ActivityPub id/url does not match final fetch URL"} = assert {:error, :id_mismatch} =
Fetcher.fetch_and_contain_remote_object_from_id( Fetcher.fetch_and_contain_remote_object_from_id(
"https://patch.cx/media/03ca3c8b4ac3ddd08bf0f84be7885f2f88de0f709112131a22d83650819e36c2.json" "https://patch.cx/media/03ca3c8b4ac3ddd08bf0f84be7885f2f88de0f709112131a22d83650819e36c2.json"
) )
assert {:error, "Object's ActivityPub id/url does not match final fetch URL"} = assert {:error, :id_mismatch} =
Fetcher.fetch_and_contain_remote_object_from_id( Fetcher.fetch_and_contain_remote_object_from_id(
"https://patch.cx/media/spoof_stage1.json" "https://patch.cx/media/spoof_stage1.json"
) )
@ -287,14 +287,14 @@ defmodule Pleroma.Object.FetcherTest do
end end
test "it does not fetch a spoofed object with a foreign actor" do test "it does not fetch a spoofed object with a foreign actor" do
assert {:error, "Object containment failed."} = assert {:error, _} =
Fetcher.fetch_and_contain_remote_object_from_id( Fetcher.fetch_and_contain_remote_object_from_id(
"https://patch.cx/objects/spoof_foreign_actor" "https://patch.cx/objects/spoof_foreign_actor"
) )
end end
test "it does not fetch from localhost" do test "it does not fetch from localhost" do
assert {:error, "Trying to fetch local resource"} = assert {:error, :local_resource} =
Fetcher.fetch_and_contain_remote_object_from_id( Fetcher.fetch_and_contain_remote_object_from_id(
Pleroma.Web.Endpoint.url() <> "/spoof_local" Pleroma.Web.Endpoint.url() <> "/spoof_local"
) )

View file

@ -125,18 +125,6 @@ defmodule Pleroma.Web.ActivityPub.TransmogrifierTest do
assert activity.data["context"] == object.data["context"] assert activity.data["context"] == object.data["context"]
end end
test "it keeps link tags" do
insert(:user, ap_id: "https://example.org/users/alice")
message = File.read!("test/fixtures/fep-e232.json") |> Jason.decode!()
assert capture_log(fn ->
assert {:ok, activity} = Transmogrifier.handle_incoming(message)
object = Object.normalize(activity)
assert [%{"type" => "Mention"}, %{"type" => "Link"}] = object.data["tag"]
end) =~ "Object rejected while fetching"
end
test "it accepts quote posts" do test "it accepts quote posts" do
insert(:user, ap_id: "https://misskey.io/users/7rkrarq81i") insert(:user, ap_id: "https://misskey.io/users/7rkrarq81i")

View file

@ -1340,7 +1340,6 @@ defmodule HttpRequestMock do
}} }}
end end
def get("https://example.org/emoji/firedfox.png", _, _, _) do def get("https://example.org/emoji/firedfox.png", _, _, _) do
{:ok, %Tesla.Env{status: 200, body: File.read!("test/fixtures/image.jpg")}} {:ok, %Tesla.Env{status: 200, body: File.read!("test/fixtures/image.jpg")}}
end end