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

Reviewed-on: #1077
Reviewed-by: floatingghost <hannah@coffee-and-dreams.uk>

changelog.d/database-config-whitelist.add

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

config/config.exs

@@ -566,6 +566,15 @@ config :pleroma, Pleroma.User,
   ],
   email_blacklist: []
 
+config :pleroma, :database_config_whitelist, [
+  {:pleroma},
+  {:cors_plug},
+  {:ex_aws, :s3},
+  {:mime},
+  {:web_push_encryption, :vapid_details},
+  {:logger}
+]
+
 config :pleroma, Oban,
   repo: Pleroma.Repo,
   log: false,

docs/docs/administration/CLI_tasks/config.md

@@ -205,3 +205,17 @@ Once you have modified the JSON file, you can load it back into the database.
     An instance reboot is needed for many changes to take effect,
     you may want to visit `/api/v1/pleroma/admin/restart` on your instance
     to soft-restart the instance.
+
+## 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/docs/configuration/cheatsheet.md

@@ -1090,8 +1090,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

@@ -244,6 +244,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_info("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_info("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
@@ -443,4 +498,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

@@ -402,5 +402,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

@@ -0,0 +1,64 @@
+# Pleroma: A lightweight social networking server
+# Copyright © 2017-2022 Pleroma Authors <https://pleroma.social/>
+# SPDX-License-Identifier: AGPL-3.0-only
+
+defmodule Pleroma.Web.AdminAPI.ConfigControllerTest do
+  use Pleroma.Web.ConnCase
+
+  import Pleroma.Factory
+
+  alias Pleroma.ConfigDB
+
+  setup do
+    admin = insert(:user, is_admin: true)
+    token = insert(:oauth_admin_token, user: admin)
+
+    conn =
+      build_conn()
+      |> assign(:user, admin)
+      |> assign(:token, token)
+
+    {:ok, %{admin: admin, token: token, conn: conn}}
+  end
+
+  describe "POST /api/v1/pleroma/admin/config" do
+    setup do: clear_config(:configurable_from_database, true)
+
+    test "doesn't allow updating the database_config_whitelist itself", %{conn: conn} do
+      clear_config(:database_config_whitelist, [{:pleroma}])
+
+      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/v1/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
+  end
+
+  describe "GET /api/v1/pleroma/admin/config/descriptions" do
+    test "all keys from description are whitelisted", %{conn: conn} do
+      conn = get(conn, "/api/v1/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