From ad7dcf38a854ac762c812eae1ea5f8ba6b707cd6 Mon Sep 17 00:00:00 2001
From: FloatingGhost <hannah@coffee-and-dreams.uk>
Date: Fri, 15 Dec 2023 17:12:45 +0000
Subject: [PATCH 01/15] Add HTTP backoff cache to respect 429s

---
 lib/pleroma/application.ex    |  3 +-
 lib/pleroma/http/backoff.ex   | 57 +++++++++++++++++++++++++++++++++++
 lib/pleroma/object/fetcher.ex |  2 +-
 lib/pleroma/web/web_finger.ex |  5 +--
 4 files changed, 63 insertions(+), 4 deletions(-)
 create mode 100644 lib/pleroma/http/backoff.ex

diff --git a/lib/pleroma/application.ex b/lib/pleroma/application.ex
index 28a86d0aa..25fb11660 100644
--- a/lib/pleroma/application.ex
+++ b/lib/pleroma/application.ex
@@ -179,7 +179,8 @@ defp cachex_children do
       build_cachex("translations", default_ttl: :timer.hours(24 * 30), limit: 2500),
       build_cachex("instances", default_ttl: :timer.hours(24), ttl_interval: 1000, limit: 2500),
       build_cachex("request_signatures", default_ttl: :timer.hours(24 * 30), limit: 3000),
-      build_cachex("rel_me", default_ttl: :timer.hours(24 * 30), limit: 300)
+      build_cachex("rel_me", default_ttl: :timer.hours(24 * 30), limit: 300),
+      build_cachex("http_backoff", default_ttl: :timer.hours(24 * 30), limit: 10000)
     ]
   end
 
diff --git a/lib/pleroma/http/backoff.ex b/lib/pleroma/http/backoff.ex
new file mode 100644
index 000000000..d51c0547a
--- /dev/null
+++ b/lib/pleroma/http/backoff.ex
@@ -0,0 +1,57 @@
+defmodule Pleroma.HTTP.Backoff do
+  alias Pleroma.HTTP
+  require Logger
+
+  @cachex Pleroma.Config.get([:cachex, :provider], Cachex)
+  @backoff_cache :http_backoff_cache
+
+  defp next_backoff_timestamp(%{headers: headers}) when is_list(headers) do
+    # figure out from the 429 response when we can make the next request
+    # mastodon uses the x-ratelimit-reset header, so we will use that!
+    # other servers may not, so we'll default to 5 minutes from now if we can't find it
+    case Enum.find_value(headers, fn {"x-ratelimit-reset", value} -> value end) do
+      nil ->
+        DateTime.utc_now()
+        |> Timex.shift(seconds: 5 * 60)
+
+      value ->
+        {:ok, stamp} = DateTime.from_iso8601(value)
+        stamp
+    end
+  end
+
+  defp next_backoff_timestamp(_), do: DateTime.utc_now() |> Timex.shift(seconds: 5 * 60)
+
+  def get(url, headers \\ [], options \\ []) do
+    # this acts as a single throughput for all GET requests
+    # we will check if the host is in the cache, and if it is, we will automatically fail the request
+    # this ensures that we don't hammer the server with requests, and instead wait for the backoff to expire
+    # this is a very simple implementation, and can be improved upon!
+    %{host: host} = URI.parse(url)
+
+    case @cachex.get(@backoff_cache, host) do
+      {:ok, nil} ->
+        case HTTP.get(url, headers, options) do
+          {:ok, env} ->
+            case env.status do
+              429 ->
+                Logger.error("Rate limited on #{host}! Backing off...")
+                timestamp = next_backoff_timestamp(env)
+                ttl = Timex.diff(timestamp, DateTime.utc_now(), :seconds)
+                # we will cache the host for 5 minutes
+                @cachex.put(@backoff_cache, host, true, ttl)
+                {:error, :ratelimit}
+
+              _ ->
+                {:ok, env}
+            end
+
+          {:error, env} ->
+            {:error, env}
+        end
+
+      _ ->
+        {:error, :ratelimit}
+    end
+  end
+end
diff --git a/lib/pleroma/object/fetcher.ex b/lib/pleroma/object/fetcher.ex
index b9d8dbaaa..937026e04 100644
--- a/lib/pleroma/object/fetcher.ex
+++ b/lib/pleroma/object/fetcher.ex
@@ -354,7 +354,7 @@ def get_object(id) do
 
     with {:ok, %{body: body, status: code, headers: headers, url: final_url}}
          when code in 200..299 <-
-           HTTP.get(id, headers),
+           HTTP.Backoff.get(id, headers),
          remote_host <-
            URI.parse(final_url).host,
          {:cross_domain_redirect, false} <-
diff --git a/lib/pleroma/web/web_finger.ex b/lib/pleroma/web/web_finger.ex
index 9d5efbb3e..280ed236e 100644
--- a/lib/pleroma/web/web_finger.ex
+++ b/lib/pleroma/web/web_finger.ex
@@ -160,7 +160,8 @@ def find_lrdd_template(domain) do
     # WebFinger is restricted to HTTPS - https://tools.ietf.org/html/rfc7033#section-9.1
     meta_url = "https://#{domain}/.well-known/host-meta"
 
-    with {:ok, %{status: status, body: body}} when status in 200..299 <- HTTP.get(meta_url) do
+    with {:ok, %{status: status, body: body}} when status in 200..299 <-
+           HTTP.Backoff.get(meta_url) do
       get_template_from_xml(body)
     else
       error ->
@@ -197,7 +198,7 @@ def finger(account) do
 
     with address when is_binary(address) <- get_address_from_domain(domain, encoded_account),
          {:ok, %{status: status, body: body, headers: headers}} when status in 200..299 <-
-           HTTP.get(
+           HTTP.Backoff.get(
              address,
              [{"accept", "application/xrd+xml,application/jrd+json"}]
            ) do

From 2437a3e9ba191c20ef5231fa5a97bab99a8955a0 Mon Sep 17 00:00:00 2001
From: FloatingGhost <hannah@coffee-and-dreams.uk>
Date: Fri, 15 Dec 2023 17:29:02 +0000
Subject: [PATCH 02/15] add test for backoff

---
 lib/pleroma/http/backoff.ex        |  3 +--
 test/pleroma/http/backoff_test.exs | 34 ++++++++++++++++++++++++++++++
 2 files changed, 35 insertions(+), 2 deletions(-)
 create mode 100644 test/pleroma/http/backoff_test.exs

diff --git a/lib/pleroma/http/backoff.ex b/lib/pleroma/http/backoff.ex
index d51c0547a..d47d2ea6b 100644
--- a/lib/pleroma/http/backoff.ex
+++ b/lib/pleroma/http/backoff.ex
@@ -28,8 +28,7 @@ def get(url, headers \\ [], options \\ []) do
     # this ensures that we don't hammer the server with requests, and instead wait for the backoff to expire
     # this is a very simple implementation, and can be improved upon!
     %{host: host} = URI.parse(url)
-
-    case @cachex.get(@backoff_cache, host) do
+    case @cachex.get(@backoff_cache, host)  do
       {:ok, nil} ->
         case HTTP.get(url, headers, options) do
           {:ok, env} ->
diff --git a/test/pleroma/http/backoff_test.exs b/test/pleroma/http/backoff_test.exs
new file mode 100644
index 000000000..e8a571e87
--- /dev/null
+++ b/test/pleroma/http/backoff_test.exs
@@ -0,0 +1,34 @@
+defmodule Pleroma.HTTP.BackoffTest do
+  @backoff_cache :http_backoff_cache
+  use Pleroma.DataCase, async: false
+  alias Pleroma.HTTP.Backoff
+
+  describe "get/3" do
+    test "should return {:ok, env} when not rate limited" do
+      Tesla.Mock.mock_global(fn
+        %Tesla.Env{url: "https://akkoma.dev/api/v1/instance"} ->
+          {:ok, %Tesla.Env{status: 200, body: "ok"}}
+      end)
+      assert {:ok, env} = Backoff.get("https://akkoma.dev/api/v1/instance")
+      assert env.status == 200
+    end
+
+    test "should return {:error, env} when rate limited" do
+      # Shove a value into the cache to simulate a rate limit
+      Cachex.put(@backoff_cache, "akkoma.dev", true)
+      assert {:error, env} = Backoff.get("https://akkoma.dev/api/v1/instance")
+      assert env.status == 429
+    end
+
+    test "should insert a value into the cache when rate limited" do
+      Tesla.Mock.mock_global(fn
+        %Tesla.Env{url: "https://ratelimited.dev/api/v1/instance"} ->
+          {:ok, %Tesla.Env{status: 429, body: "Rate limited"}}
+      end)
+
+      assert {:error, env} = Backoff.get("https://ratelimited.dev/api/v1/instance")
+      assert env.status == 429
+      assert {:ok, true} = Cachex.get(@backoff_cache, "ratelimited.dev")
+  end
+  end
+end

From 3c384c1b7617f00a5222733e22e7b6cf7550c7fa Mon Sep 17 00:00:00 2001
From: FloatingGhost <hannah@coffee-and-dreams.uk>
Date: Wed, 20 Dec 2023 16:45:35 +0000
Subject: [PATCH 03/15] Add ratelimit backoff to HTTP get

---
 lib/pleroma/http/backoff.ex        | 21 ++++++++++++++++-----
 test/pleroma/http/backoff_test.exs | 22 +++++++++++++++++++++-
 2 files changed, 37 insertions(+), 6 deletions(-)

diff --git a/lib/pleroma/http/backoff.ex b/lib/pleroma/http/backoff.ex
index d47d2ea6b..cba6c0c17 100644
--- a/lib/pleroma/http/backoff.ex
+++ b/lib/pleroma/http/backoff.ex
@@ -9,14 +9,24 @@ defp next_backoff_timestamp(%{headers: headers}) when is_list(headers) do
     # figure out from the 429 response when we can make the next request
     # mastodon uses the x-ratelimit-reset header, so we will use that!
     # other servers may not, so we'll default to 5 minutes from now if we can't find it
+    default_5_minute_backoff =
+      DateTime.utc_now()
+      |> Timex.shift(seconds: 5 * 60)
+
     case Enum.find_value(headers, fn {"x-ratelimit-reset", value} -> value end) do
       nil ->
-        DateTime.utc_now()
-        |> Timex.shift(seconds: 5 * 60)
+        Logger.error("Rate limited, but couldn't find timestamp! Using default 5 minute backoff until #{default_5_minute_backoff}")
+        default_5_minute_backoff
 
       value ->
-        {:ok, stamp} = DateTime.from_iso8601(value)
-        stamp
+        with {:ok, stamp, _} <- DateTime.from_iso8601(value) do
+          Logger.error("Rate limited until #{stamp}")
+          stamp
+        else
+          _ ->
+            Logger.error("Rate limited, but couldn't parse timestamp! Using default 5 minute backoff until #{default_5_minute_backoff}")
+            default_5_minute_backoff
+        end
     end
   end
 
@@ -28,7 +38,8 @@ def get(url, headers \\ [], options \\ []) do
     # this ensures that we don't hammer the server with requests, and instead wait for the backoff to expire
     # this is a very simple implementation, and can be improved upon!
     %{host: host} = URI.parse(url)
-    case @cachex.get(@backoff_cache, host)  do
+
+    case @cachex.get(@backoff_cache, host) do
       {:ok, nil} ->
         case HTTP.get(url, headers, options) do
           {:ok, env} ->
diff --git a/test/pleroma/http/backoff_test.exs b/test/pleroma/http/backoff_test.exs
index e8a571e87..b50a4c458 100644
--- a/test/pleroma/http/backoff_test.exs
+++ b/test/pleroma/http/backoff_test.exs
@@ -9,6 +9,7 @@ test "should return {:ok, env} when not rate limited" do
         %Tesla.Env{url: "https://akkoma.dev/api/v1/instance"} ->
           {:ok, %Tesla.Env{status: 200, body: "ok"}}
       end)
+
       assert {:ok, env} = Backoff.get("https://akkoma.dev/api/v1/instance")
       assert env.status == 200
     end
@@ -29,6 +30,25 @@ test "should insert a value into the cache when rate limited" do
       assert {:error, env} = Backoff.get("https://ratelimited.dev/api/v1/instance")
       assert env.status == 429
       assert {:ok, true} = Cachex.get(@backoff_cache, "ratelimited.dev")
-  end
+    end
+
+    test "should parse the value of x-ratelimit-reset, if present" do
+      ten_minutes_from_now =
+        DateTime.utc_now() |> Timex.shift(minutes: 10) |> DateTime.to_iso8601()
+
+      Tesla.Mock.mock_global(fn
+        %Tesla.Env{url: "https://ratelimited.dev/api/v1/instance"} ->
+          {:ok,
+           %Tesla.Env{
+             status: 429,
+             body: "Rate limited",
+             headers: [{"x-ratelimit-reset", ten_minutes_from_now}]
+           }}
+      end)
+
+      assert {:error, env} = Backoff.get("https://ratelimited.dev/api/v1/instance")
+      assert env.status == 429
+      assert {:ok, true} = Cachex.get(@backoff_cache, "ratelimited.dev")
+    end
   end
 end

From ec7e9da734590622d180cb72dfc34ba3ab6bcfea Mon Sep 17 00:00:00 2001
From: Floatingghost <hannah@coffee-and-dreams.uk>
Date: Fri, 26 Apr 2024 19:05:12 +0100
Subject: [PATCH 04/15] Correct ttl syntax for new cachex

---
 lib/pleroma/http/backoff.ex        | 2 +-
 test/pleroma/http/backoff_test.exs | 9 +++------
 2 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/lib/pleroma/http/backoff.ex b/lib/pleroma/http/backoff.ex
index cba6c0c17..5fb37205e 100644
--- a/lib/pleroma/http/backoff.ex
+++ b/lib/pleroma/http/backoff.ex
@@ -49,7 +49,7 @@ def get(url, headers \\ [], options \\ []) do
                 timestamp = next_backoff_timestamp(env)
                 ttl = Timex.diff(timestamp, DateTime.utc_now(), :seconds)
                 # we will cache the host for 5 minutes
-                @cachex.put(@backoff_cache, host, true, ttl)
+                @cachex.put(@backoff_cache, host, true, ttl: ttl)
                 {:error, :ratelimit}
 
               _ ->
diff --git a/test/pleroma/http/backoff_test.exs b/test/pleroma/http/backoff_test.exs
index b50a4c458..62419eb5c 100644
--- a/test/pleroma/http/backoff_test.exs
+++ b/test/pleroma/http/backoff_test.exs
@@ -17,8 +17,7 @@ test "should return {:ok, env} when not rate limited" do
     test "should return {:error, env} when rate limited" do
       # Shove a value into the cache to simulate a rate limit
       Cachex.put(@backoff_cache, "akkoma.dev", true)
-      assert {:error, env} = Backoff.get("https://akkoma.dev/api/v1/instance")
-      assert env.status == 429
+      assert {:error, :ratelimit} = Backoff.get("https://akkoma.dev/api/v1/instance")
     end
 
     test "should insert a value into the cache when rate limited" do
@@ -27,8 +26,7 @@ test "should insert a value into the cache when rate limited" do
           {:ok, %Tesla.Env{status: 429, body: "Rate limited"}}
       end)
 
-      assert {:error, env} = Backoff.get("https://ratelimited.dev/api/v1/instance")
-      assert env.status == 429
+      assert {:error, :ratelimit} = Backoff.get("https://ratelimited.dev/api/v1/instance")
       assert {:ok, true} = Cachex.get(@backoff_cache, "ratelimited.dev")
     end
 
@@ -46,8 +44,7 @@ test "should parse the value of x-ratelimit-reset, if present" do
            }}
       end)
 
-      assert {:error, env} = Backoff.get("https://ratelimited.dev/api/v1/instance")
-      assert env.status == 429
+      assert {:error, :ratelimit} = Backoff.get("https://ratelimited.dev/api/v1/instance")
       assert {:ok, true} = Cachex.get(@backoff_cache, "ratelimited.dev")
     end
   end

From 9671cdecdf9a8bf968a7b1a87091b57c9490924a Mon Sep 17 00:00:00 2001
From: Floatingghost <hannah@coffee-and-dreams.uk>
Date: Fri, 26 Apr 2024 19:10:17 +0100
Subject: [PATCH 05/15] changelog entry

---
 CHANGELOG.md | 1 +
 1 file changed, 1 insertion(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 2facbd84d..257dc4adb 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -35,6 +35,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 - Issue leading to Mastodon bot accounts being rejected
 - Scope misdetection of remote posts resulting from not recognising
   JSON-LD-compacted forms of public scope; affected e.g. federation with bovine
+- Ratelimits encountered when fetching objects are now respected; 429 responses will cause a backoff when we get one.
 
 ## Removed
 - ActivityPub Client-To-Server write API endpoints have been disabled;

From 010e8c7bb26d537acf2b1d969adcc6ba42abe9a9 Mon Sep 17 00:00:00 2001
From: Floatingghost <hannah@coffee-and-dreams.uk>
Date: Fri, 26 Apr 2024 19:28:01 +0100
Subject: [PATCH 06/15] where were you when lint fail

---
 lib/pleroma/http/backoff.ex | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/lib/pleroma/http/backoff.ex b/lib/pleroma/http/backoff.ex
index 5fb37205e..dac05c971 100644
--- a/lib/pleroma/http/backoff.ex
+++ b/lib/pleroma/http/backoff.ex
@@ -15,7 +15,10 @@ defp next_backoff_timestamp(%{headers: headers}) when is_list(headers) do
 
     case Enum.find_value(headers, fn {"x-ratelimit-reset", value} -> value end) do
       nil ->
-        Logger.error("Rate limited, but couldn't find timestamp! Using default 5 minute backoff until #{default_5_minute_backoff}")
+        Logger.error(
+          "Rate limited, but couldn't find timestamp! Using default 5 minute backoff until #{default_5_minute_backoff}"
+        )
+
         default_5_minute_backoff
 
       value ->
@@ -24,7 +27,10 @@ defp next_backoff_timestamp(%{headers: headers}) when is_list(headers) do
           stamp
         else
           _ ->
-            Logger.error("Rate limited, but couldn't parse timestamp! Using default 5 minute backoff until #{default_5_minute_backoff}")
+            Logger.error(
+              "Rate limited, but couldn't parse timestamp! Using default 5 minute backoff until #{default_5_minute_backoff}"
+            )
+
             default_5_minute_backoff
         end
     end

From bd74693db63925a4469c618989dcc7c05aa81591 Mon Sep 17 00:00:00 2001
From: Floatingghost <hannah@coffee-and-dreams.uk>
Date: Mon, 6 May 2024 23:34:48 +0100
Subject: [PATCH 07/15] additionally support retry-after values

---
 lib/pleroma/http/backoff.ex        | 136 +++++++++++++++++++----------
 test/pleroma/http/backoff_test.exs |  44 ++++++++++
 2 files changed, 136 insertions(+), 44 deletions(-)

diff --git a/lib/pleroma/http/backoff.ex b/lib/pleroma/http/backoff.ex
index dac05c971..b3f734a92 100644
--- a/lib/pleroma/http/backoff.ex
+++ b/lib/pleroma/http/backoff.ex
@@ -5,66 +5,114 @@ defmodule Pleroma.HTTP.Backoff do
   @cachex Pleroma.Config.get([:cachex, :provider], Cachex)
   @backoff_cache :http_backoff_cache
 
+  # attempt to parse a timestamp from a header
+  # returns nil if it can't parse the timestamp
+  @spec timestamp_or_nil(binary) :: DateTime.t() | nil
+  defp timestamp_or_nil(header) do
+    case DateTime.from_iso8601(header) do
+      {:ok, stamp, _} ->
+        stamp
+
+      _ ->
+        nil
+    end
+  end
+
+  # attempt to parse the x-ratelimit-reset header from the headers
+  @spec x_ratelimit_reset(headers :: list) :: DateTime.t() | nil
+  defp x_ratelimit_reset(headers) do
+    with {_header, value} <- List.keyfind(headers, "x-ratelimit-reset", 0),
+         true <- is_binary(value) do
+      timestamp_or_nil(value)
+    else
+      _ ->
+        nil
+    end
+  end
+
+  # attempt to parse the Retry-After header from the headers
+  # this can be either a timestamp _or_ a number of seconds to wait!
+  # we'll return a datetime if we can parse it, or nil if we can't
+  @spec retry_after(headers :: list) :: DateTime.t() | nil
+  defp retry_after(headers) do
+    with {_header, value} <- List.keyfind(headers, "retry-after", 0),
+         true <- is_binary(value) do
+      # first, see if it's an integer
+      case Integer.parse(value) do
+        {seconds, ""} ->
+          Logger.debug("Parsed Retry-After header: #{seconds} seconds")
+          DateTime.utc_now() |> Timex.shift(seconds: seconds)
+
+        _ ->
+          # if it's not an integer, try to parse it as a timestamp
+          timestamp_or_nil(value)
+      end
+    else
+      _ ->
+        nil
+    end
+  end
+
+  # given a set of headers, will attempt to find the next backoff timestamp
+  # if it can't find one, it will default to 5 minutes from now
+  @spec next_backoff_timestamp(%{headers: list}) :: DateTime.t()
   defp next_backoff_timestamp(%{headers: headers}) when is_list(headers) do
-    # figure out from the 429 response when we can make the next request
-    # mastodon uses the x-ratelimit-reset header, so we will use that!
-    # other servers may not, so we'll default to 5 minutes from now if we can't find it
     default_5_minute_backoff =
       DateTime.utc_now()
       |> Timex.shift(seconds: 5 * 60)
 
-    case Enum.find_value(headers, fn {"x-ratelimit-reset", value} -> value end) do
-      nil ->
-        Logger.error(
-          "Rate limited, but couldn't find timestamp! Using default 5 minute backoff until #{default_5_minute_backoff}"
-        )
+    backoff =
+      [&x_ratelimit_reset/1, &retry_after/1]
+      |> Enum.map(& &1.(headers))
+      |> Enum.find(&(&1 != nil))
 
-        default_5_minute_backoff
-
-      value ->
-        with {:ok, stamp, _} <- DateTime.from_iso8601(value) do
-          Logger.error("Rate limited until #{stamp}")
-          stamp
-        else
-          _ ->
-            Logger.error(
-              "Rate limited, but couldn't parse timestamp! Using default 5 minute backoff until #{default_5_minute_backoff}"
-            )
-
-            default_5_minute_backoff
-        end
+    if is_nil(backoff) do
+      Logger.debug("No backoff headers found, defaulting to 5 minutes from now")
+      default_5_minute_backoff
+    else
+      Logger.debug("Found backoff header, will back off until: #{backoff}")
+      backoff
     end
   end
 
   defp next_backoff_timestamp(_), do: DateTime.utc_now() |> Timex.shift(seconds: 5 * 60)
 
+  # utility function to check the HTTP response for potential backoff headers
+  # will check if we get a 429 or 503 response, and if we do, will back off for a bit
+  @spec check_backoff({:ok | :error, HTTP.Env.t()}, binary()) ::
+          {:ok | :error, HTTP.Env.t()} | {:error, :ratelimit}
+  defp check_backoff({:ok, env}, host) do
+    case env.status do
+      status when status in [429, 503] ->
+        Logger.error("Rate limited on #{host}! Backing off...")
+        timestamp = next_backoff_timestamp(env)
+        ttl = Timex.diff(timestamp, DateTime.utc_now(), :seconds)
+        # we will cache the host for 5 minutes
+        @cachex.put(@backoff_cache, host, true, ttl: ttl)
+        {:error, :ratelimit}
+
+      _ ->
+        {:ok, env}
+    end
+  end
+
+  defp check_backoff(env, _), do: env
+
+  @doc """
+  this acts as a single throughput for all GET requests
+  we will check if the host is in the cache, and if it is, we will automatically fail the request
+  this ensures that we don't hammer the server with requests, and instead wait for the backoff to expire
+  this is a very simple implementation, and can be improved upon!
+  """
+  @spec get(binary, list, list) :: {:ok | :error, HTTP.Env.t()} | {:error, :ratelimit}
   def get(url, headers \\ [], options \\ []) do
-    # this acts as a single throughput for all GET requests
-    # we will check if the host is in the cache, and if it is, we will automatically fail the request
-    # this ensures that we don't hammer the server with requests, and instead wait for the backoff to expire
-    # this is a very simple implementation, and can be improved upon!
     %{host: host} = URI.parse(url)
 
     case @cachex.get(@backoff_cache, host) do
       {:ok, nil} ->
-        case HTTP.get(url, headers, options) do
-          {:ok, env} ->
-            case env.status do
-              429 ->
-                Logger.error("Rate limited on #{host}! Backing off...")
-                timestamp = next_backoff_timestamp(env)
-                ttl = Timex.diff(timestamp, DateTime.utc_now(), :seconds)
-                # we will cache the host for 5 minutes
-                @cachex.put(@backoff_cache, host, true, ttl: ttl)
-                {:error, :ratelimit}
-
-              _ ->
-                {:ok, env}
-            end
-
-          {:error, env} ->
-            {:error, env}
-        end
+        url
+        |> HTTP.get(headers, options)
+        |> check_backoff(host)
 
       _ ->
         {:error, :ratelimit}
diff --git a/test/pleroma/http/backoff_test.exs b/test/pleroma/http/backoff_test.exs
index 62419eb5c..33a4fd22f 100644
--- a/test/pleroma/http/backoff_test.exs
+++ b/test/pleroma/http/backoff_test.exs
@@ -3,6 +3,10 @@ defmodule Pleroma.HTTP.BackoffTest do
   use Pleroma.DataCase, async: false
   alias Pleroma.HTTP.Backoff
 
+  defp within_tolerance?(ttl, expected) do
+    ttl > expected - 10 and ttl < expected + 10
+  end
+
   describe "get/3" do
     test "should return {:ok, env} when not rate limited" do
       Tesla.Mock.mock_global(fn
@@ -46,6 +50,46 @@ test "should parse the value of x-ratelimit-reset, if present" do
 
       assert {:error, :ratelimit} = Backoff.get("https://ratelimited.dev/api/v1/instance")
       assert {:ok, true} = Cachex.get(@backoff_cache, "ratelimited.dev")
+      {:ok, ttl} = Cachex.ttl(@backoff_cache, "ratelimited.dev")
+      assert within_tolerance?(ttl, 600)
+    end
+
+    test "should parse the value of retry-after when it's a timestamp" do
+      ten_minutes_from_now =
+        DateTime.utc_now() |> Timex.shift(minutes: 10) |> DateTime.to_iso8601()
+
+      Tesla.Mock.mock_global(fn
+        %Tesla.Env{url: "https://ratelimited.dev/api/v1/instance"} ->
+          {:ok,
+           %Tesla.Env{
+             status: 429,
+             body: "Rate limited",
+             headers: [{"retry-after", ten_minutes_from_now}]
+           }}
+      end)
+
+      assert {:error, :ratelimit} = Backoff.get("https://ratelimited.dev/api/v1/instance")
+      assert {:ok, true} = Cachex.get(@backoff_cache, "ratelimited.dev")
+      {:ok, ttl} = Cachex.ttl(@backoff_cache, "ratelimited.dev")
+      assert within_tolerance?(ttl, 600)
+    end
+
+    test "should parse the value of retry-after when it's a number of seconds" do
+      Tesla.Mock.mock_global(fn
+        %Tesla.Env{url: "https://ratelimited.dev/api/v1/instance"} ->
+          {:ok,
+           %Tesla.Env{
+             status: 429,
+             body: "Rate limited",
+             headers: [{"retry-after", "600"}]
+           }}
+      end)
+
+      assert {:error, :ratelimit} = Backoff.get("https://ratelimited.dev/api/v1/instance")
+      assert {:ok, true} = Cachex.get(@backoff_cache, "ratelimited.dev")
+      # assert that the value is 10 minutes from now
+      {:ok, ttl} = Cachex.ttl(@backoff_cache, "ratelimited.dev")
+      assert within_tolerance?(ttl, 600)
     end
   end
 end

From ea6bc8a7c587c636793875c4d8d7ed534288336e Mon Sep 17 00:00:00 2001
From: Floatingghost <hannah@coffee-and-dreams.uk>
Date: Mon, 6 May 2024 23:36:00 +0100
Subject: [PATCH 08/15] add a test for 503-rate-limiting

---
 test/pleroma/http/backoff_test.exs | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/test/pleroma/http/backoff_test.exs b/test/pleroma/http/backoff_test.exs
index 33a4fd22f..f1b27f5b5 100644
--- a/test/pleroma/http/backoff_test.exs
+++ b/test/pleroma/http/backoff_test.exs
@@ -34,6 +34,16 @@ test "should insert a value into the cache when rate limited" do
       assert {:ok, true} = Cachex.get(@backoff_cache, "ratelimited.dev")
     end
 
+    test "should insert a value into the cache when rate limited with a 503 response" do
+      Tesla.Mock.mock_global(fn
+        %Tesla.Env{url: "https://ratelimited.dev/api/v1/instance"} ->
+          {:ok, %Tesla.Env{status: 503, body: "Rate limited"}}
+      end)
+
+      assert {:error, :ratelimit} = Backoff.get("https://ratelimited.dev/api/v1/instance")
+      assert {:ok, true} = Cachex.get(@backoff_cache, "ratelimited.dev")
+    end
+
     test "should parse the value of x-ratelimit-reset, if present" do
       ten_minutes_from_now =
         DateTime.utc_now() |> Timex.shift(minutes: 10) |> DateTime.to_iso8601()

From 34a48cb87f263b246f060b7f7a6178a9194af64d Mon Sep 17 00:00:00 2001
From: Oneric <oneric@oneric.stub>
Date: Tue, 23 Apr 2024 23:09:41 +0200
Subject: [PATCH 09/15] scheduled_activity: mark private functions as private

And remove unused due_activities/1
---
 lib/pleroma/scheduled_activity.ex | 24 +++++++-----------------
 1 file changed, 7 insertions(+), 17 deletions(-)

diff --git a/lib/pleroma/scheduled_activity.ex b/lib/pleroma/scheduled_activity.ex
index 2b156341f..3e1c51abb 100644
--- a/lib/pleroma/scheduled_activity.ex
+++ b/lib/pleroma/scheduled_activity.ex
@@ -28,7 +28,7 @@ defmodule Pleroma.ScheduledActivity do
     timestamps()
   end
 
-  def changeset(%ScheduledActivity{} = scheduled_activity, attrs) do
+  defp changeset(%ScheduledActivity{} = scheduled_activity, attrs) do
     scheduled_activity
     |> cast(attrs, [:scheduled_at, :params])
     |> validate_required([:scheduled_at, :params])
@@ -52,14 +52,14 @@ defp with_media_attachments(
 
   defp with_media_attachments(changeset), do: changeset
 
-  def update_changeset(%ScheduledActivity{} = scheduled_activity, attrs) do
+  defp update_changeset(%ScheduledActivity{} = scheduled_activity, attrs) do
     scheduled_activity
     |> cast(attrs, [:scheduled_at])
     |> validate_required([:scheduled_at])
     |> validate_scheduled_at()
   end
 
-  def validate_scheduled_at(changeset) do
+  defp validate_scheduled_at(changeset) do
     validate_change(changeset, :scheduled_at, fn _, scheduled_at ->
       cond do
         not far_enough?(scheduled_at) ->
@@ -77,7 +77,7 @@ def validate_scheduled_at(changeset) do
     end)
   end
 
-  def exceeds_daily_user_limit?(user_id, scheduled_at) do
+  defp exceeds_daily_user_limit?(user_id, scheduled_at) do
     ScheduledActivity
     |> where(user_id: ^user_id)
     |> where([sa], type(sa.scheduled_at, :date) == type(^scheduled_at, :date))
@@ -86,7 +86,7 @@ def exceeds_daily_user_limit?(user_id, scheduled_at) do
     |> Kernel.>=(Config.get([ScheduledActivity, :daily_user_limit]))
   end
 
-  def exceeds_total_user_limit?(user_id) do
+  defp exceeds_total_user_limit?(user_id) do
     ScheduledActivity
     |> where(user_id: ^user_id)
     |> select([sa], count(sa.id))
@@ -108,7 +108,7 @@ def far_enough?(scheduled_at) do
     diff > @min_offset
   end
 
-  def new(%User{} = user, attrs) do
+  defp new(%User{} = user, attrs) do
     changeset(%ScheduledActivity{user_id: user.id}, attrs)
   end
 
@@ -187,17 +187,7 @@ def for_user_query(%User{} = user) do
     |> where(user_id: ^user.id)
   end
 
-  def due_activities(offset \\ 0) do
-    naive_datetime =
-      NaiveDateTime.utc_now()
-      |> NaiveDateTime.add(offset, :millisecond)
-
-    ScheduledActivity
-    |> where([sa], sa.scheduled_at < ^naive_datetime)
-    |> Repo.all()
-  end
-
-  def job_query(scheduled_activity_id) do
+  defp job_query(scheduled_activity_id) do
     from(j in Oban.Job,
       where: j.queue == "scheduled_activities",
       where: fragment("args ->> 'activity_id' = ?::text", ^to_string(scheduled_activity_id))

From 873aa9da1cbb26eb25bac93e7ed8e95b2d47c4ad Mon Sep 17 00:00:00 2001
From: Oneric <oneric@oneric.stub>
Date: Tue, 23 Apr 2024 23:12:39 +0200
Subject: [PATCH 10/15] activity_draft: mark new/2 as private

---
 lib/pleroma/web/common_api/activity_draft.ex | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/pleroma/web/common_api/activity_draft.ex b/lib/pleroma/web/common_api/activity_draft.ex
index ced6371d6..4555efd61 100644
--- a/lib/pleroma/web/common_api/activity_draft.ex
+++ b/lib/pleroma/web/common_api/activity_draft.ex
@@ -41,7 +41,7 @@ defmodule Pleroma.Web.CommonAPI.ActivityDraft do
             preview?: false,
             changes: %{}
 
-  def new(user, params) do
+  defp new(user, params) do
     %__MODULE__{user: user}
     |> put_params(params)
   end

From 94e9c8f48a20e5cdd77854d40106acc7af10e16f Mon Sep 17 00:00:00 2001
From: Oneric <oneric@oneric.stub>
Date: Tue, 23 Apr 2024 23:59:42 +0200
Subject: [PATCH 11/15] Purge unused media description update on post
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

In MastoAPI media descriptions are updated via the
media update API not upon post creation or post update.

This functionality was originally added about 6 years ago in
ba93396649f65a1f32eeedfd9ccd32cf308e7210 which was part of
https://git.pleroma.social/pleroma/pleroma/-/merge_requests/626 and
https://git.pleroma.social/pleroma/pleroma-fe/-/merge_requests/450.
They introduced image descriptions to the front- and backend,
but predate adoption of Mastodon API.

For a while adding an `descriptions` array on post creation might have
continued to work as an undocumented Pleroma extension to Masto API, but
at latest when OpenAPI specs were added for those endpoints four years
ago in 7803a85d2ced092fbd8e0f1bde0944bd27f8d649, these codepaths ceased
to be used. The API specs don’t list a `descriptions` parameter and
any unknown parameters are stripped out.

The attachments_from_ids function is only called from
ScheduledActivity and ActivityDraft.create with the latter
only being called by CommonAPI.{post,update} whihc in turn
are only called from ScheduledActivity again, MastoAPI controller
and without any attachment or description parameter WelcomeMessage.
Therefore no codepath can contain a descriptions parameter.
---
 lib/pleroma/web/common_api/utils.ex        | 25 ++++----------------
 test/pleroma/web/common_api/utils_test.exs | 27 ----------------------
 2 files changed, 4 insertions(+), 48 deletions(-)

diff --git a/lib/pleroma/web/common_api/utils.ex b/lib/pleroma/web/common_api/utils.ex
index e79b12fc9..d80109a98 100644
--- a/lib/pleroma/web/common_api/utils.ex
+++ b/lib/pleroma/web/common_api/utils.ex
@@ -22,19 +22,13 @@ defmodule Pleroma.Web.CommonAPI.Utils do
   require Logger
   require Pleroma.Constants
 
-  def attachments_from_ids(%{media_ids: ids, descriptions: desc}) do
-    attachments_from_ids_descs(ids, desc)
-  end
-
   def attachments_from_ids(%{media_ids: ids}) do
-    attachments_from_ids_no_descs(ids)
+    attachments_from_ids(ids)
   end
 
-  def attachments_from_ids(_), do: []
+  def attachments_from_ids([]), do: []
 
-  def attachments_from_ids_no_descs([]), do: []
-
-  def attachments_from_ids_no_descs(ids) do
+  def attachments_from_ids(ids) when is_list(ids) do
     Enum.map(ids, fn media_id ->
       case get_attachment(media_id) do
         %Object{data: data} -> data
@@ -44,18 +38,7 @@ def attachments_from_ids_no_descs(ids) do
     |> Enum.reject(&is_nil/1)
   end
 
-  def attachments_from_ids_descs([], _), do: []
-
-  def attachments_from_ids_descs(ids, descs_str) do
-    {_, descs} = Jason.decode(descs_str)
-
-    Enum.map(ids, fn media_id ->
-      with %Object{data: data} <- get_attachment(media_id) do
-        Map.put(data, "name", descs[media_id])
-      end
-    end)
-    |> Enum.reject(&is_nil/1)
-  end
+  def attachments_from_ids(_), do: []
 
   defp get_attachment(media_id) do
     Repo.get(Object, media_id)
diff --git a/test/pleroma/web/common_api/utils_test.exs b/test/pleroma/web/common_api/utils_test.exs
index f56d21c70..d98eb76ad 100644
--- a/test/pleroma/web/common_api/utils_test.exs
+++ b/test/pleroma/web/common_api/utils_test.exs
@@ -590,34 +590,7 @@ test "returns recipients when object not found" do
     end
   end
 
-  describe "attachments_from_ids_descs/2" do
-    test "returns [] when attachment ids is empty" do
-      assert Utils.attachments_from_ids_descs([], "{}") == []
-    end
-
-    test "returns list attachments with desc" do
-      object = insert(:note)
-      desc = Jason.encode!(%{object.id => "test-desc"})
-
-      assert Utils.attachments_from_ids_descs(["#{object.id}", "34"], desc) == [
-               Map.merge(object.data, %{"name" => "test-desc"})
-             ]
-    end
-  end
-
   describe "attachments_from_ids/1" do
-    test "returns attachments with descs" do
-      object = insert(:note)
-      desc = Jason.encode!(%{object.id => "test-desc"})
-
-      assert Utils.attachments_from_ids(%{
-               media_ids: ["#{object.id}"],
-               descriptions: desc
-             }) == [
-               Map.merge(object.data, %{"name" => "test-desc"})
-             ]
-    end
-
     test "returns attachments without descs" do
       object = insert(:note)
       assert Utils.attachments_from_ids(%{media_ids: ["#{object.id}"]}) == [object.data]

From 6ef6b2a289f4165dee4e8ba800d7cb2e3b9285a1 Mon Sep 17 00:00:00 2001
From: Oneric <oneric@oneric.stub>
Date: Wed, 24 Apr 2024 20:03:30 +0200
Subject: [PATCH 12/15] Apply rate limits to status updates

---
 lib/pleroma/web/mastodon_api/controllers/status_controller.ex | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/pleroma/web/mastodon_api/controllers/status_controller.ex b/lib/pleroma/web/mastodon_api/controllers/status_controller.ex
index 338a35052..acb5f15a0 100644
--- a/lib/pleroma/web/mastodon_api/controllers/status_controller.ex
+++ b/lib/pleroma/web/mastodon_api/controllers/status_controller.ex
@@ -87,7 +87,7 @@ defmodule Pleroma.Web.MastodonAPI.StatusController do
     %{scopes: ["write:bookmarks"]} when action in [:bookmark, :unbookmark]
   )
 
-  @rate_limited_status_actions ~w(reblog unreblog favourite unfavourite create delete)a
+  @rate_limited_status_actions ~w(reblog unreblog favourite unfavourite create delete update)a
 
   plug(
     RateLimiter,

From 0c2b33458d1cccabfc4b5f0db53f963ea673ecb9 Mon Sep 17 00:00:00 2001
From: Oneric <oneric@oneric.stub>
Date: Wed, 24 Apr 2024 17:46:18 +0200
Subject: [PATCH 13/15] Restrict media usage to owners
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

In Mastodon media can only be used by owners and only be associated with
a single post. We currently allow media to be associated with several
posts and until now did not limit their usage in posts to media owners.
However, media update and GET lookup was already limited to owners.
(In accordance with allowing media reuse, we also still allow GET
lookups of media already used in a post unlike Mastodon)

Allowing reuse isn’t problematic per se, but allowing use by non-owners
can be problematic if media ids of private-scoped posts can be guessed
since creating a new post with this media id will reveal the uploaded
file content and alt text.
Given media ids are currently just part of a sequentieal series shared
with some other objects, guessing media ids is with some persistence
indeed feasible.

E.g. sampline some public media ids from a real-world
instance with 112 total and 61 monthly-active users:

  17.465.096  at  t0
  17.472.673  at  t1 = t0 + 4h
  17.473.248  at  t2 = t1 + 20min

This gives about 30 new ids per minute of which most won't be
local media but remote and local posts, poll answers etc.
Assuming the default ratelimit of 15 post actions per 10s, scraping all
media for the 4h interval takes about 84 minutes and scraping the 20min
range mere 6.3 minutes. (Until the preceding commit, post updates were
not rate limited at all, allowing even faster scraping.)
If an attacker can infer (e.g. via reply to a follower-only post not
accessbile to the attacker) some sensitive information was uploaded
during a specific time interval and has some pointers regarding the
nature of the information, identifying the specific upload out of all
scraped media for this timerange is not impossible.

Thus restrict media usage to owners.

Checking ownership just in ActivitDraft would already be sufficient,
since when a scheduled status actually gets posted it goes through
ActivityDraft again, but would erroneously return a success status
when scheduling an illegal post.

Independently discovered and fixed by mint in Pleroma
https://git.pleroma.social/pleroma/pleroma/-/commit/1afde067b12ad0062c1820091ea9b0a680819281
---
 CHANGELOG.md                                  |  3 +
 lib/pleroma/scheduled_activity.ex             | 43 +++++++++----
 lib/pleroma/web/common_api/activity_draft.ex  | 11 +++-
 lib/pleroma/web/common_api/utils.ex           | 26 ++++----
 test/pleroma/web/common_api/utils_test.exs    |  8 ++-
 .../controllers/status_controller_test.exs    | 64 +++++++++++++++++++
 .../views/scheduled_activity_view_test.exs    |  3 +-
 7 files changed, 125 insertions(+), 33 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 2284f5c8d..48e487bf1 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -6,6 +6,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 
 ## Unreleased
 
+## Fixed
+- Issue allowing non-owners to use media objects in posts
+
 ## 2024.04
 
 ## Added
diff --git a/lib/pleroma/scheduled_activity.ex b/lib/pleroma/scheduled_activity.ex
index 3e1c51abb..5292b1491 100644
--- a/lib/pleroma/scheduled_activity.ex
+++ b/lib/pleroma/scheduled_activity.ex
@@ -40,19 +40,29 @@ defp with_media_attachments(
          %{changes: %{params: %{"media_ids" => media_ids} = params}} = changeset
        )
        when is_list(media_ids) do
-    media_attachments = Utils.attachments_from_ids(%{media_ids: media_ids})
+    user = User.get_by_id(changeset.data.user_id)
 
-    params =
-      params
-      |> Map.put("media_attachments", media_attachments)
-      |> Map.put("media_ids", media_ids)
+    case Utils.attachments_from_ids(user, %{media_ids: media_ids}) do
+      media_attachments when is_list(media_attachments) ->
+        params =
+          params
+          |> Map.put("media_attachments", media_attachments)
+          |> Map.put("media_ids", media_ids)
 
-    put_change(changeset, :params, params)
+        put_change(changeset, :params, params)
+
+      {:error, _} = e ->
+        e
+
+      e ->
+        {:error, e}
+    end
   end
 
   defp with_media_attachments(changeset), do: changeset
 
   defp update_changeset(%ScheduledActivity{} = scheduled_activity, attrs) do
+    # note: should this ever allow swapping media attachments, make sure ownership is checked
     scheduled_activity
     |> cast(attrs, [:scheduled_at])
     |> validate_required([:scheduled_at])
@@ -115,13 +125,22 @@ defp new(%User{} = user, attrs) do
   @doc """
   Creates ScheduledActivity and add to queue to perform at scheduled_at date
   """
-  @spec create(User.t(), map()) :: {:ok, ScheduledActivity.t()} | {:error, Ecto.Changeset.t()}
+  @spec create(User.t(), map()) :: {:ok, ScheduledActivity.t()} | {:error, any()}
   def create(%User{} = user, attrs) do
-    Multi.new()
-    |> Multi.insert(:scheduled_activity, new(user, attrs))
-    |> maybe_add_jobs(Config.get([ScheduledActivity, :enabled]))
-    |> Repo.transaction()
-    |> transaction_response
+    case new(user, attrs) do
+      %Ecto.Changeset{} = sched_data ->
+        Multi.new()
+        |> Multi.insert(:scheduled_activity, sched_data)
+        |> maybe_add_jobs(Config.get([ScheduledActivity, :enabled]))
+        |> Repo.transaction()
+        |> transaction_response
+
+      {:error, _} = e ->
+        e
+
+      e ->
+        {:error, e}
+    end
   end
 
   defp maybe_add_jobs(multi, true) do
diff --git a/lib/pleroma/web/common_api/activity_draft.ex b/lib/pleroma/web/common_api/activity_draft.ex
index 4555efd61..2d24edec4 100644
--- a/lib/pleroma/web/common_api/activity_draft.ex
+++ b/lib/pleroma/web/common_api/activity_draft.ex
@@ -92,9 +92,14 @@ defp full_payload(%{status: status, summary: summary} = draft) do
     end
   end
 
-  defp attachments(%{params: params} = draft) do
-    attachments = Utils.attachments_from_ids(params)
-    %__MODULE__{draft | attachments: attachments}
+  defp attachments(%{params: params, user: user} = draft) do
+    case Utils.attachments_from_ids(user, params) do
+      attachments when is_list(attachments) ->
+        %__MODULE__{draft | attachments: attachments}
+
+      {:error, reason} ->
+        add_error(draft, reason)
+    end
   end
 
   defp in_reply_to(%{params: %{in_reply_to_status_id: ""}} = draft), do: draft
diff --git a/lib/pleroma/web/common_api/utils.ex b/lib/pleroma/web/common_api/utils.ex
index d80109a98..6d2113100 100644
--- a/lib/pleroma/web/common_api/utils.ex
+++ b/lib/pleroma/web/common_api/utils.ex
@@ -22,24 +22,24 @@ defmodule Pleroma.Web.CommonAPI.Utils do
   require Logger
   require Pleroma.Constants
 
-  def attachments_from_ids(%{media_ids: ids}) do
-    attachments_from_ids(ids)
+  def attachments_from_ids(user, %{media_ids: ids}) do
+    attachments_from_ids(user, ids, [])
   end
 
-  def attachments_from_ids([]), do: []
+  def attachments_from_ids(_, _), do: []
 
-  def attachments_from_ids(ids) when is_list(ids) do
-    Enum.map(ids, fn media_id ->
-      case get_attachment(media_id) do
-        %Object{data: data} -> data
-        _ -> nil
-      end
-    end)
-    |> Enum.reject(&is_nil/1)
+  defp attachments_from_ids(_user, [], acc), do: Enum.reverse(acc)
+
+  defp attachments_from_ids(user, [media_id | ids], acc) do
+    with {_, %Object{} = object} <- {:get, get_attachment(media_id)},
+         :ok <- Object.authorize_access(object, user) do
+      attachments_from_ids(user, ids, [object.data | acc])
+    else
+      {:get, _} -> attachments_from_ids(user, ids, acc)
+      {:error, reason} -> {:error, reason}
+    end
   end
 
-  def attachments_from_ids(_), do: []
-
   defp get_attachment(media_id) do
     Repo.get(Object, media_id)
   end
diff --git a/test/pleroma/web/common_api/utils_test.exs b/test/pleroma/web/common_api/utils_test.exs
index d98eb76ad..10abb89e5 100644
--- a/test/pleroma/web/common_api/utils_test.exs
+++ b/test/pleroma/web/common_api/utils_test.exs
@@ -592,12 +592,14 @@ test "returns recipients when object not found" do
 
   describe "attachments_from_ids/1" do
     test "returns attachments without descs" do
-      object = insert(:note)
-      assert Utils.attachments_from_ids(%{media_ids: ["#{object.id}"]}) == [object.data]
+      user = insert(:user)
+      object = insert(:note, user: user)
+      assert Utils.attachments_from_ids(user, %{media_ids: ["#{object.id}"]}) == [object.data]
     end
 
     test "returns [] when not pass media_ids" do
-      assert Utils.attachments_from_ids(%{}) == []
+      user = insert(:user)
+      assert Utils.attachments_from_ids(user, %{}) == []
     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 f58045640..a15fd42fc 100644
--- a/test/pleroma/web/mastodon_api/controllers/status_controller_test.exs
+++ b/test/pleroma/web/mastodon_api/controllers/status_controller_test.exs
@@ -220,6 +220,28 @@ test "posting an undefined status with an attachment", %{user: user, conn: conn}
       assert json_response_and_validate_schema(conn, 200)
     end
 
+    test "refuses to post non-owned media", %{conn: conn} do
+      other_user = insert(:user)
+
+      file = %Plug.Upload{
+        content_type: "image/jpeg",
+        path: Path.absname("test/fixtures/image.jpg"),
+        filename: "an_image.jpg"
+      }
+
+      {:ok, upload} = ActivityPub.upload(file, actor: other_user.ap_id)
+
+      conn =
+        conn
+        |> put_req_header("content-type", "application/json")
+        |> post("/api/v1/statuses", %{
+          "status" => "mew",
+          "media_ids" => [to_string(upload.id)]
+        })
+
+      assert json_response(conn, 422) == %{"error" => "forbidden"}
+    end
+
     test "posting a status with an invalid language", %{conn: conn} do
       conn =
         conn
@@ -569,6 +591,29 @@ test "creates a scheduled activity with a media attachment", %{user: user, conn:
       assert %{"type" => "image"} = media_attachment
     end
 
+    test "refuses to schedule post with non-owned media", %{conn: conn} do
+      other_user = insert(:user)
+
+      file = %Plug.Upload{
+        content_type: "image/jpeg",
+        path: Path.absname("test/fixtures/image.jpg"),
+        filename: "an_image.jpg"
+      }
+
+      {:ok, upload} = ActivityPub.upload(file, actor: other_user.ap_id)
+
+      conn =
+        conn
+        |> put_req_header("content-type", "application/json")
+        |> post("/api/v1/statuses", %{
+          "status" => "mew",
+          "scheduled_at" => DateTime.add(DateTime.utc_now(), 6, :minute),
+          "media_ids" => [to_string(upload.id)]
+        })
+
+      assert json_response(conn, 403) == %{"error" => "Access denied"}
+    end
+
     test "skips the scheduling and creates the activity if scheduled_at is earlier than 5 minutes from now",
          %{conn: conn} do
       scheduled_at =
@@ -2406,6 +2451,25 @@ test "it updates the attachments", %{conn: conn, user: user} do
       assert [%{"id" => ^attachment_id}] = response["media_attachments"]
     end
 
+    test "it does not update to non-owned attachments", %{conn: conn, user: user} do
+      other_user = insert(:user)
+      attachment = insert(:attachment, user: other_user)
+      attachment_id = to_string(attachment.id)
+
+      {:ok, activity} = CommonAPI.post(user, %{status: "mew mew #abc", spoiler_text: "#def"})
+
+      conn =
+        conn
+        |> put_req_header("content-type", "application/json")
+        |> put("/api/v1/statuses/#{activity.id}", %{
+          "status" => "mew mew #abc",
+          "spoiler_text" => "#def",
+          "media_ids" => [attachment_id]
+        })
+
+      assert json_response(conn, 400) == %{"error" => "internal_server_error"}
+    end
+
     test "it does not update visibility", %{conn: conn, user: user} do
       {:ok, activity} =
         CommonAPI.post(user, %{
diff --git a/test/pleroma/web/mastodon_api/views/scheduled_activity_view_test.exs b/test/pleroma/web/mastodon_api/views/scheduled_activity_view_test.exs
index e323f3a1f..cc893f94e 100644
--- a/test/pleroma/web/mastodon_api/views/scheduled_activity_view_test.exs
+++ b/test/pleroma/web/mastodon_api/views/scheduled_activity_view_test.exs
@@ -47,8 +47,7 @@ test "A scheduled activity with a media attachment" do
     expected = %{
       id: to_string(scheduled_activity.id),
       media_attachments:
-        %{media_ids: [upload.id]}
-        |> Utils.attachments_from_ids()
+        Utils.attachments_from_ids(user, %{media_ids: [upload.id]})
         |> Enum.map(&StatusView.render("attachment.json", %{attachment: &1})),
       params: %{
         in_reply_to_id: to_string(activity.id),

From fbd961c7474e5176fc26f850f8e6d3334e0aac27 Mon Sep 17 00:00:00 2001
From: Oneric <oneric@oneric.stub>
Date: Thu, 25 Apr 2024 18:10:34 +0200
Subject: [PATCH 14/15] Drop activity_type override for uploads

Afaict this was never used, but keeping this (in theory) possible
hinders detecting which objects are actually media uploads and
which proper ActivityPub objects.

It was originally added as part of upload support itself in
02d3dc6869f388388ea744ea4ee3b54279d55e86 without being used
and `git log -S:activity_type` and `git log -Sactivity_type:`
don't find any other commits using this.
---
 lib/pleroma/upload.ex | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/lib/pleroma/upload.ex b/lib/pleroma/upload.ex
index ad5c2c655..426fe9f1b 100644
--- a/lib/pleroma/upload.ex
+++ b/lib/pleroma/upload.ex
@@ -13,7 +13,6 @@ defmodule Pleroma.Upload do
   * `:uploader`: override uploader
   * `:filters`: override filters
   * `:size_limit`: override size limit
-  * `:activity_type`: override activity type
 
   The `%Pleroma.Upload{}` struct: all documented fields are meant to be overwritten in filters:
 
@@ -48,7 +47,6 @@ defmodule Pleroma.Upload do
   @type option ::
           {:type, :avatar | :banner | :background}
           | {:description, String.t()}
-          | {:activity_type, String.t()}
           | {:size_limit, nil | non_neg_integer()}
           | {:uploader, module()}
           | {:filters, [module()]}
@@ -143,7 +141,7 @@ defp get_opts(opts) do
       end
 
     %{
-      activity_type: Keyword.get(opts, :activity_type, activity_type),
+      activity_type: activity_type,
       size_limit: Keyword.get(opts, :size_limit, size_limit),
       uploader: Keyword.get(opts, :uploader, Pleroma.Config.get([__MODULE__, :uploader])),
       filters:

From 9a91299f96c4c405cfdb8b8f78d0906b1cf98001 Mon Sep 17 00:00:00 2001
From: Oneric <oneric@oneric.stub>
Date: Thu, 25 Apr 2024 18:16:21 +0200
Subject: [PATCH 15/15] Don't try to handle non-media objects as media

Trying to display non-media as media crashed the renderer,
but when posting a status with a valid, non-media object id
the post was still created, but then crashed e.g. timeline rendering.
It also crashed C2S inbox reads, so this could not be used to leak
private posts.
---
 CHANGELOG.md                                  |  1 +
 lib/pleroma/constants.ex                      |  3 +++
 lib/pleroma/web/common_api/utils.ex           |  9 +++++--
 .../controllers/media_controller.ex           | 11 +++++++--
 test/pleroma/web/common_api/utils_test.exs    |  8 ++++++-
 .../controllers/media_controller_test.exs     | 24 +++++++++++++++++++
 6 files changed, 51 insertions(+), 5 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 48e487bf1..edd51b74b 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -8,6 +8,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 
 ## Fixed
 - Issue allowing non-owners to use media objects in posts
+- Issue allowing use of non-media objects as attachments and crashing timeline rendering
 
 ## 2024.04
 
diff --git a/lib/pleroma/constants.ex b/lib/pleroma/constants.ex
index 94608a99b..2681d7671 100644
--- a/lib/pleroma/constants.ex
+++ b/lib/pleroma/constants.ex
@@ -64,4 +64,7 @@ defmodule Pleroma.Constants do
       "Service"
     ]
   )
+
+  # Internally used as top-level types for media attachments and user images
+  const(attachment_types, do: ["Document", "Image"])
 end
diff --git a/lib/pleroma/web/common_api/utils.ex b/lib/pleroma/web/common_api/utils.ex
index 6d2113100..635143621 100644
--- a/lib/pleroma/web/common_api/utils.ex
+++ b/lib/pleroma/web/common_api/utils.ex
@@ -40,8 +40,13 @@ defp attachments_from_ids(user, [media_id | ids], acc) do
     end
   end
 
-  defp get_attachment(media_id) do
-    Repo.get(Object, media_id)
+  def get_attachment(media_id) do
+    with %Object{} = object <- Repo.get(Object, media_id),
+         true <- object.data["type"] in Pleroma.Constants.attachment_types() do
+      object
+    else
+      _ -> nil
+    end
   end
 
   @spec get_to_and_cc(ActivityDraft.t()) :: {list(String.t()), list(String.t())}
diff --git a/lib/pleroma/web/mastodon_api/controllers/media_controller.ex b/lib/pleroma/web/mastodon_api/controllers/media_controller.ex
index 5918b288d..3e5a64b51 100644
--- a/lib/pleroma/web/mastodon_api/controllers/media_controller.ex
+++ b/lib/pleroma/web/mastodon_api/controllers/media_controller.ex
@@ -8,6 +8,7 @@ defmodule Pleroma.Web.MastodonAPI.MediaController do
   alias Pleroma.Object
   alias Pleroma.User
   alias Pleroma.Web.ActivityPub.ActivityPub
+  alias Pleroma.Web.CommonAPI.Utils
   alias Pleroma.Web.Plugs.OAuthScopesPlug
 
   action_fallback(Pleroma.Web.MastodonAPI.FallbackController)
@@ -55,12 +56,15 @@ def create2(_conn, _data), do: {:error, :bad_request}
 
   @doc "PUT /api/v1/media/:id"
   def update(%{assigns: %{user: user}, body_params: %{description: description}} = conn, %{id: id}) do
-    with %Object{} = object <- Object.get_by_id(id),
+    with {_, %Object{} = object} <- {:get, Utils.get_attachment(id)},
          :ok <- Object.authorize_access(object, user),
          {:ok, %Object{data: data}} <- Object.update_data(object, %{"name" => description}) do
       attachment_data = Map.put(data, "id", object.id)
 
       render(conn, "attachment.json", %{attachment: attachment_data})
+    else
+      {:get, _} -> {:error, :not_found}
+      e -> e
     end
   end
 
@@ -68,11 +72,14 @@ def update(conn, data), do: show(conn, data)
 
   @doc "GET /api/v1/media/:id"
   def show(%{assigns: %{user: user}} = conn, %{id: id}) do
-    with %Object{data: data, id: object_id} = object <- Object.get_by_id(id),
+    with {_, %Object{data: data, id: object_id} = object} <- {:get, Utils.get_attachment(id)},
          :ok <- Object.authorize_access(object, user) do
       attachment_data = Map.put(data, "id", object_id)
 
       render(conn, "attachment.json", %{attachment: attachment_data})
+    else
+      {:get, _} -> {:error, :not_found}
+      e -> e
     end
   end
 
diff --git a/test/pleroma/web/common_api/utils_test.exs b/test/pleroma/web/common_api/utils_test.exs
index 10abb89e5..3d639d389 100644
--- a/test/pleroma/web/common_api/utils_test.exs
+++ b/test/pleroma/web/common_api/utils_test.exs
@@ -593,10 +593,16 @@ test "returns recipients when object not found" do
   describe "attachments_from_ids/1" do
     test "returns attachments without descs" do
       user = insert(:user)
-      object = insert(:note, user: user)
+      object = insert(:attachment, user: user)
       assert Utils.attachments_from_ids(user, %{media_ids: ["#{object.id}"]}) == [object.data]
     end
 
+    test "returns [] when passed non-media object ids" do
+      user = insert(:user)
+      object = insert(:note, user: user)
+      assert Utils.attachments_from_ids(user, %{media_ids: ["#{object.id}"]}) == []
+    end
+
     test "returns [] when not pass media_ids" do
       user = insert(:user)
       assert Utils.attachments_from_ids(user, %{}) == []
diff --git a/test/pleroma/web/mastodon_api/controllers/media_controller_test.exs b/test/pleroma/web/mastodon_api/controllers/media_controller_test.exs
index 7b5f5850d..a224f063b 100644
--- a/test/pleroma/web/mastodon_api/controllers/media_controller_test.exs
+++ b/test/pleroma/web/mastodon_api/controllers/media_controller_test.exs
@@ -6,6 +6,7 @@ defmodule Pleroma.Web.MastodonAPI.MediaControllerTest do
   use Pleroma.Web.ConnCase, async: false
 
   import ExUnit.CaptureLog
+  import Pleroma.Factory
 
   alias Pleroma.Object
   alias Pleroma.User
@@ -174,6 +175,18 @@ test "/api/v1/media/:id good request", %{conn: conn, object: object} do
       assert media["description"] == "test-media"
       assert refresh_record(object).data["name"] == "test-media"
     end
+
+    test "won't update non-media", %{conn: conn, user: user} do
+      object = insert(:note, user: user)
+
+      response =
+        conn
+        |> put_req_header("content-type", "multipart/form-data")
+        |> put("/api/v1/media/#{object.id}", %{"description" => "test-media"})
+        |> json_response(404)
+
+      assert response == %{"error" => "Record not found"}
+    end
   end
 
   describe "Get media by id (/api/v1/media/:id)" do
@@ -207,6 +220,17 @@ test "it returns media object when requested by owner", %{conn: conn, object: ob
       assert media["id"]
     end
 
+    test "it returns 404 when requesting non-media object", %{conn: conn, user: user} do
+      object = insert(:note, user: user)
+
+      response =
+        conn
+        |> get("/api/v1/media/#{object.id}")
+        |> json_response(404)
+
+      assert response == %{"error" => "Record not found"}
+    end
+
     test "it returns 403 if media object requested by non-owner", %{object: object, user: user} do
       %{conn: conn, user: other_user} = oauth_access(["read:media"])