Merge pull request 'Add sane defaults for :database_config_whitelist, add a task to remove non-whitelisted configs' (#7837) from pleroma-database-config-whitelist into develop

Reviewed-on: https://git.pleroma.social/pleroma/pleroma/pulls/7837

changelog.d/database-config-whitelist.add

@@ -0,0 +1 @@
+Add reasonable defaults for :database_config_whitelist

config/config.exs

@@ -960,6 +960,15 @@ config :pleroma, Pleroma.Search.QdrantSearch,
     vectors: %{size: 384, distance: "Cosine"}
   }
 
+config :pleroma, :database_config_whitelist, [
+  {:pleroma},
+  {:cors_plug},
+  {:ex_aws, :s3},
+  {:mime},
+  {:prometheus, Pleroma.Web.Endpoint.MetricsExporter},
+  {:web_push_encryption, :vapid_details}
+]
+
 # Import environment specific config. This must remain at the bottom
 # of this file so it overrides the configuration defined above.
 import_config "#{Mix.env()}.exs"

docs/administration/CLI_tasks/config.md

@@ -169,4 +169,18 @@ This forcibly removes any enabled MRF that does not exist and will fix the abili
 === "From Source"
     ```sh
     mix pleroma.config fix_mrf_policies
-    ```
+    ```
+
+## Remove non-whitelisted configs from the database
+
+This removes any configuration value that is not explicitly whitelisted by `:pleroma, :database_config_whitelist`. Might be useful after updating the whitelist.
+
+=== "OTP"
+    ```sh
+    ./bin/pleroma_ctl config filter_whitelisted
+    ```
+
+=== "From Source"
+    ```sh
+    mix pleroma.config filter_whitelisted
+    ```

docs/configuration/cheatsheet.md

@@ -1132,8 +1132,9 @@ Boolean, enables/disables in-database configuration. Read [Transferring the conf
 
 List of valid configuration sections which are allowed to be configured from the
 database. Settings stored in the database before the whitelist is configured are
-still applied, so it is suggested to only use the whitelist on instances that
-have not migrated the config to the database.
+still applied. Consider running the `mix pleroma.config filter_whitelisted` task
+after updating the whitelist. Read [Remove non-whitelisted configs from the database](../administration/CLI_tasks/config.md#remove-non-whitelisted-configs-from-the-database)
+for more information.
 
 Example:
 ```elixir

lib/mix/tasks/pleroma/config.ex

@@ -234,6 +234,61 @@ defmodule Mix.Tasks.Pleroma.Config do
     end)
   end
 
+  # Removes non-whitelisted configuration sections
+  def run(["filter_whitelisted" | rest]) do
+    {options, [], []} =
+      OptionParser.parse(
+        rest,
+        strict: [force: :boolean],
+        aliases: [f: :force]
+      )
+
+    force = Keyword.get(options, :force, false)
+
+    start_pleroma()
+
+    whitelisted_configs = Pleroma.Config.get(:database_config_whitelist)
+
+    if whitelisted_configs in [nil, false] do
+      shell_error("No unwanted settings in ConfigDB. No changes made.")
+    else
+      whitelisted_groups =
+        whitelisted_configs
+        |> Enum.filter(fn
+          {_group} -> true
+          _ -> false
+        end)
+        |> Enum.map(fn {group} -> group end)
+
+      whitelisted_keys =
+        whitelisted_configs
+        |> Enum.filter(fn
+          {_group, _key} -> true
+          _ -> false
+        end)
+
+      filtered =
+        from(c in ConfigDB)
+        |> Repo.all()
+        |> Enum.filter(&not_whitelisted?(&1, whitelisted_groups, whitelisted_keys))
+
+      if not Enum.empty?(filtered) do
+        shell_info("The following settings will be removed from ConfigDB:\n")
+        Enum.each(filtered, &dump(&1))
+
+        if force or shell_prompt("Are you sure you want to continue?", "n") in ~w(Yn Y y) do
+          filtered_ids = Enum.map(filtered, fn %{id: id} -> id end)
+
+          Repo.delete_all(from(c in ConfigDB, where: c.id in ^filtered_ids))
+        else
+          shell_error("No changes made.")
+        end
+      else
+        shell_error("No unwanted settings in ConfigDB. No changes made.")
+      end
+    end
+  end
+
   @spec migrate_to_db(Path.t() | nil) :: any()
   def migrate_to_db(file_path \\ nil) do
     with :ok <- Pleroma.Config.DeprecationWarnings.warn() do
@@ -434,4 +489,9 @@ defmodule Mix.Tasks.Pleroma.Config do
     Ecto.Adapters.SQL.query!(Repo, "TRUNCATE config;")
     Ecto.Adapters.SQL.query!(Repo, "ALTER SEQUENCE config_id_seq RESTART;")
   end
+
+  defp not_whitelisted?(%{group: group, key: key}, whitelisted_groups, whitelisted_keys) do
+    not Enum.member?(whitelisted_groups, group) and
+      not Enum.member?(whitelisted_keys, {group, key})
+  end
 end

lib/pleroma/web/admin_api/controllers/config_controller.ex

@@ -174,6 +174,8 @@ defmodule Pleroma.Web.AdminAPI.ConfigController do
     end
   end
 
+  defp whitelisted_config?(":pleroma", ":database_config_whitelist"), do: false
+
   defp whitelisted_config?(group, key) do
     if whitelisted_configs = Config.get(:database_config_whitelist) do
       Enum.any?(whitelisted_configs, fn

test/mix/tasks/pleroma/config_test.exs

@@ -329,5 +329,39 @@ defmodule Mix.Tasks.Pleroma.ConfigTest do
 
       assert config_records() == []
     end
+
+    test "filters non-whitelisted settings" do
+      clear_config(:database_config_whitelist, [
+        {:pleroma},
+        {:web_push_encryption, :vapid_details}
+      ])
+
+      insert_config_record(:web_push_encryption, :non_whitelisted_key, a: 1)
+      insert_config_record(:web_push_encryption, :vapid_details, b: 1)
+
+      MixTask.run(["filter_whitelisted", "--force"])
+
+      assert [
+               %ConfigDB{group: :pleroma, key: :instance},
+               %ConfigDB{group: :pleroma, key: Pleroma.Captcha},
+               %ConfigDB{group: :web_push_encryption, key: :vapid_details}
+             ] = config_records()
+    end
+
+    test "filter_whitelisted doesn't crash when whitelist is unset" do
+      clear_config(:database_config_whitelist, nil)
+
+      existing = config_records()
+      MixTask.run(["filter_whitelisted", "--force"])
+      assert config_records() == existing
+    end
+
+    test "filter_whitelisted doesn't crash when whitelist is disabled" do
+      clear_config(:database_config_whitelist, false)
+
+      existing = config_records()
+      MixTask.run(["filter_whitelisted", "--force"])
+      assert config_records() == existing
+    end
   end
 end

test/pleroma/web/admin_api/controllers/config_controller_test.exs

@@ -194,6 +194,16 @@ defmodule Pleroma.Web.AdminAPI.ConfigControllerTest do
 
     setup do: clear_config(:configurable_from_database, true)
 
+    setup do:
+            clear_config(:database_config_whitelist, [
+              {:pleroma},
+              {:http},
+              {:idna},
+              {:oban},
+              {:tesla},
+              {:ueberauth}
+            ])
+
     test "create new config setting in db", %{conn: conn} do
       ueberauth = Application.get_env(:ueberauth, Ueberauth)
       on_exit(fn -> Application.put_env(:ueberauth, Ueberauth, ueberauth) end)
@@ -807,7 +817,7 @@ defmodule Pleroma.Web.AdminAPI.ConfigControllerTest do
                                   %{
                                     "tuple" => [
                                       "/websocket",
-                                      "Phoenix.Endpoint.CowboyWebSocket",
+                                      ":sth",
                                       %{
                                         "tuple" => [
                                           "Phoenix.Transports.WebSocket",
@@ -871,7 +881,7 @@ defmodule Pleroma.Web.AdminAPI.ConfigControllerTest do
                                        %{
                                          "tuple" => [
                                            "/websocket",
-                                           "Phoenix.Endpoint.CowboyWebSocket",
+                                           ":sth",
                                            %{
                                              "tuple" => [
                                                "Phoenix.Transports.WebSocket",
@@ -1210,6 +1220,31 @@ defmodule Pleroma.Web.AdminAPI.ConfigControllerTest do
       assert Application.get_env(:not_real, :anything) == "value6"
     end
 
+    test "doesn't allow updating the database_config_whitelist itself", %{conn: conn} do
+      original_whitelist = Pleroma.Config.get(:database_config_whitelist)
+
+      refute ConfigDB.get_by_group_and_key(:pleroma, :database_config_whitelist)
+
+      conn =
+        conn
+        |> put_req_header("content-type", "application/json")
+        |> post("/api/pleroma/admin/config", %{
+          configs: [
+            %{
+              group: ":pleroma",
+              key: ":database_config_whitelist",
+              value: [%{"tuple" => [":pleroma", ":key1"]}]
+            }
+          ]
+        })
+
+      %{"configs" => configs} = json_response_and_validate_schema(conn, 200)
+
+      assert configs == []
+      assert Pleroma.Config.get(:database_config_whitelist) == original_whitelist
+      refute ConfigDB.get_by_group_and_key(:pleroma, :database_config_whitelist)
+    end
+
     test "args for Pleroma.Upload.Filter.Mogrify with custom tuples", %{conn: conn} do
       assert conn
              |> put_req_header("content-type", "application/json")
@@ -1472,5 +1507,13 @@ defmodule Pleroma.Web.AdminAPI.ConfigControllerTest do
       web_endpoint = Enum.find(children, fn c -> c["key"] == "Pleroma.Upload" end)
       assert web_endpoint["children"]
     end
+
+    test "all keys from description are whitelisted", %{conn: conn} do
+      conn = get(conn, "/api/pleroma/admin/config/descriptions")
+
+      assert response = json_response_and_validate_schema(conn, 200)
+
+      assert length(response) == length(Pleroma.Docs.JSON.compiled_descriptions())
+    end
   end
 end