Add a final few DB exclusion reasons

CHANGELOG.md

@@ -9,6 +9,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 ## Added
 - Full compatibility with Erlang OTP26
 - handling of GET /api/v1/preferences
+- Explicit listing of config keys that are allowed to be set by the database
+  - Previously set config keys will still be loaded, but you will get a warning
+    that they probably should not be dynamically configured.
 
 ## Changed
 - OTP builds are now built on erlang OTP26

config/config.exs

@@ -49,7 +49,9 @@
 config :pleroma, Pleroma.Repo,
   telemetry_event: [Pleroma.Repo.Instrumenter],
   queue_target: 20_000,
-  migration_lock: nil
+  migration_lock: nil,
+  parameters: [gin_fuzzy_search_limit: "500"],
+  prepare: :unnamed
 
 config :pleroma, Pleroma.Captcha,
   enabled: true,
@@ -75,6 +77,7 @@
   truncated_namespace: nil,
   streaming_enabled: true
 
+# This cannot be configured in the database!
 config :ex_aws, :s3,
   # host: "s3.wasabisys.com", # required if not Amazon AWS
   access_key_id: nil,
@@ -453,10 +456,6 @@
   image_quality: 85,
   min_content_length: 100 * 1024
 
-config :pleroma, :shout,
-  enabled: true,
-  limit: 5_000
-
 config :phoenix, :format_encoders, json: Jason, "activity+json": Jason
 
 config :phoenix, :json_library, Jason
@@ -797,10 +796,6 @@
 
 config :pleroma, configurable_from_database: false
 
-config :pleroma, Pleroma.Repo,
-  parameters: [gin_fuzzy_search_limit: "500"],
-  prepare: :unnamed
-
 config :pleroma, :majic_pool, size: 2
 
 private_instance? = :if_instance_is_private

config/description.exs

@@ -1,16 +1,5 @@
 import Config
 
-websocket_config = [
-  path: "/websocket",
-  serializer: [
-    {Phoenix.Socket.V1.JSONSerializer, "~> 1.0.0"},
-    {Phoenix.Socket.V2.JSONSerializer, "~> 2.0.0"}
-  ],
-  timeout: 60_000,
-  transport_log: false,
-  compress: false
-]
-
 installed_frontend_options = [
   %{
     key: "name",
@@ -434,6 +423,7 @@
     label: "URI Schemes",
     type: :group,
     description: "URI schemes related settings",
+    db_exclusion_reason: "Does not make sense to configure dynamically",
     children: [
       %{
         key: :valid_schemes,
@@ -464,6 +454,7 @@
     key: :features,
     type: :group,
     description: "Customizable features",
+    db_exclusion_reason: "Should be provided at boot-time",
     children: [
       %{
         key: :improved_hashtag_timeline,
@@ -479,6 +470,7 @@
     key: :populate_hashtags_table,
     type: :group,
     description: "`populate_hashtags_table` background migration settings",
+    db_exclusion_reason: "Should be provided at boot-time",
     children: [
       %{
         key: :fault_rate_allowance,
@@ -1649,6 +1641,7 @@
     key: Pleroma.Web.MediaProxy.Invalidation.Script,
     type: :group,
     description: "Invalidation script settings",
+    db_exclusion_reason: "Provides an arbitrary execution path",
     children: [
       %{
         key: :script_path,
@@ -1762,6 +1755,7 @@
   %{
     group: :web_push_encryption,
     key: :vapid_details,
+    db_exclusion_reason: "Webserver secret keys",
     label: "Vapid Details",
     type: :group,
     description:
@@ -1833,19 +1827,13 @@
   %{
     group: :pleroma,
     label: "Pleroma Admin Token",
-    type: :group,
     description:
       "Allows setting a token that can be used to authenticate requests with admin privileges without a normal user account token. Append the `admin_token` parameter to requests to utilize it. (Please reconsider using HTTP Basic Auth or OAuth-based authentication if possible)",
-    children: [
+    type: :string,
-      %{
+    suggestions: [
-        key: :admin_token,
+      "Please use a high entropy string or UUID"
-        type: :string,
+    ],
-        description: "Admin token",
+    db_exclusion_reason: "Can provide passwordless admin access"
-        suggestions: [
-          "Please use a high entropy string or UUID"
-        ]
-      }
-    ]
   },
   %{
     group: :pleroma,
@@ -2189,6 +2177,7 @@
     label: "Pleroma Authenticator",
     type: :group,
     description: "Authenticator",
+    db_exclusion_reason: "Should be provided at boot-time",
     children: [
       %{
         key: Pleroma.Web.Auth.Authenticator,
@@ -2202,6 +2191,7 @@
     key: :ldap,
     label: "LDAP",
     type: :group,
+    db_exclusion_reason: "Provides access to another service",
     description:
       "Use LDAP for user authentication. When a user logs in to the Pleroma instance, the name and password" <>
         " will be verified by trying to authenticate (bind) to a LDAP server." <>
@@ -2601,6 +2591,7 @@
     label: "Mime Types",
     type: :group,
     description: "Mime Types settings",
+    db_exclusion_reason: "Should be provided at compile-time",
     children: [
       %{
         key: :types,
@@ -2807,6 +2798,7 @@
     group: :cors_plug,
     label: "CORS plug config",
     type: :group,
+    db_exclusion_reason: "Should be provided at compile-time",
     children: [
       %{
         key: :max_age,
@@ -2964,6 +2956,7 @@
     key: :modules,
     type: :group,
     description: "Custom Runtime Modules",
+    db_exclusion_reason: "Allows for custom elixir execution",
     children: [
       %{
         key: :runtime_dir,
@@ -3100,6 +3093,7 @@
     group: :ex_aws,
     key: :s3,
     type: :group,
+    db_exclusion_reason: "Provides access to another service",
     descriptions: "S3 service related settings",
     children: [
       %{
@@ -3479,6 +3473,7 @@
     key: :argos_translate,
     type: :group,
     description: "ArgosTranslate Settings.",
+    db_exclusion_reason: "Excluded for being able to set arbitrary paths to executables",
     children: [
       %{
         key: :command_argos_translate,

docs/docs/configuration/cheatsheet.md

@@ -375,6 +375,11 @@ This section describe PWA manifest instance-specific values. Currently this opti
 
 #### Pleroma.Web.MediaProxy.Invalidation.Script
 
+!!! warning
+    Invalidation script options cannot be set in the database due to the ability to
+    set the command options to arbitrary paths. The following options **MUST** be
+    set in your `.exs` file instead.
+
 This strategy allow perform external shell script to purge cache.
 Urls of attachments are passed to the script as arguments.
 
@@ -1011,21 +1016,6 @@ git clone <MY MODULE>
 
 Boolean, enables/disables in-database configuration. Read [Transfering the config to/from the database](../administration/CLI_tasks/config.md) for more information.
 
-## :database_config_whitelist
-
-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.
-
-Example:
-```elixir
-config :pleroma, :database_config_whitelist, [
-  {:pleroma, :instance},
-  {:pleroma, Pleroma.Web.Metadata},
-  {:auto_linker}
-]
-```
 
 ### Multi-factor authentication -  :two_factor_authentication
 * `totp` - a list containing TOTP configuration
@@ -1148,6 +1138,11 @@ Translations are available at `/api/v1/statuses/:id/translations/:language`, whe
 
 ### `:argos_translate`
 
+!!! warning
+    Argos Translate options cannot be set in the database due to the ability to
+    set the command options to arbitrary paths. The following options **MUST** be
+    set in your `.exs` file instead.
+
 - `:command_argos_translate` - command for `argos-translate`. Can be the command if it's in your PATH, or the full path to the file  (default: `argos-translate`).
 - `:command_argospm` - command for `argospm`. Can be the command if it's in your PATH, or the full path to the file (default: `argospm`).
 - `:strip_html` - Strip html from the post before translating it (default: `true`).

lib/mix/tasks/pleroma/config.ex

@@ -10,6 +10,7 @@ defmodule Mix.Tasks.Pleroma.Config do
 
   alias Pleroma.ConfigDB
   alias Pleroma.Repo
+  alias Pleroma.Config.ConfigurableFromDatabase
 
   @shortdoc "Manages the location of the config"
   @moduledoc File.read!("docs/docs/administration/CLI_tasks/config.md")
@@ -244,6 +245,38 @@ def run(["delete", group]) do
     end
   end
 
+  # Primarily a developer tool to check nothing was missed from
+  # db configwhitelist
+  def run(["check-allowed"]) do
+    start_pleroma()
+    Pleroma.Docs.JSON.compile()
+    raw = Pleroma.Docs.JSON.compiled_descriptions()
+    whitelisted = Enum.filter(raw, &ConfigurableFromDatabase.whitelisted_config?/1)
+    raw_map = MapSet.new(raw)
+    whitelisted_map = MapSet.new(whitelisted)
+
+    IO.puts(
+      "Config keys defined in description.exs but not listed as explicitly allowed in the db"
+    )
+
+    IO.puts(
+      "  Please check that standard admins should not need to touch the listed settings whilst the server is live."
+    )
+
+    IO.puts(
+      "  !! Please remember that admins are not neccesarily sysadmins nor are they immune to oauth/password leakage."
+    )
+
+    IO.puts("-------------")
+
+    MapSet.difference(raw_map, whitelisted_map)
+    |> Enum.each(fn map ->
+      IO.puts("#{map[:group]}, #{map[:key]} (#{map[:label]})")
+      IO.puts(map[:db_exclusion_reason] || "No exclusion reason set")
+      IO.puts("++")
+    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
@@ -286,6 +319,9 @@ defp do_migrate_to_db(config_file) do
   defp create(group, settings) do
     group
     |> Pleroma.Config.Loader.filter_group(settings)
+    |> Enum.filter(fn {key, _value} ->
+      Pleroma.Config.ConfigurableFromDatabase.whitelisted_config?(group, key)
+    end)
     |> Enum.each(fn {key, value} ->
       {:ok, _} = ConfigDB.update_or_create(%{group: group, key: key, value: value})
 

lib/pleroma/config/configurable_from_database.ex

@@ -0,0 +1,112 @@
+defmodule Pleroma.Config.ConfigurableFromDatabase do
+  alias Pleroma.Config
+
+  # Basically it's silly to let this be configurable
+  # set a list of things that we can set in the database
+  # this is mostly our stuff, with some extra in there
+  @allowed_groups [
+    {:logger},
+    {:pleroma, Pleroma.Captcha},
+    {:pleroma, Pleroma.Captcha.Kocaptcha},
+    {:pleroma, Pleroma.Upload},
+    {:pleroma, Pleroma.Uploaders.Local},
+    {:pleroma, Pleroma.Uploaders.S3},
+    {:pleroma, :auth},
+    {:pleroma, :emoji},
+    {:pleroma, :http},
+    {:pleroma, :instance},
+    {:pleroma, :welcome},
+    {:pleroma, :feed},
+    {:pleroma, :markup},
+    {:pleroma, :frontend_configurations},
+    {:pleroma, :assets},
+    {:pleroma, :manifest},
+    {:pleroma, :activitypub},
+    {:pleroma, :streamer},
+    {:pleroma, :user},
+    {:pleroma, :mrf_normalize_markup},
+    {:pleroma, :mrf_rejectnonpublic},
+    {:pleroma, :mrf_hellthread},
+    {:pleroma, :mrf_simple},
+    {:pleroma, :mrf_keyword},
+    {:pleroma, :mrf_hashtag},
+    {:pleroma, :mrf_subchain},
+    {:pleroma, :mrf_activity_expiration},
+    {:pleroma, :mrf_vocabulary},
+    {:pleroma, :mrf_inline_quote},
+    {:pleroma, :mrf_object_age},
+    {:pleroma, :mrf_follow_bot},
+    {:pleroma, :mrf_reject_newly_created_account_notes},
+    {:pleroma, :rich_media},
+    {:pleroma, :media_proxy},
+    {:pleroma, Pleroma.Web.MediaProxy.Invalidation.Http},
+    {:pleroma, :media_preview_proxy},
+    {:pleroma, Pleroma.Web.Metadata},
+    {:pleroma, Pleroma.Web.Metadata.Providers.Theme},
+    {:pleroma, Pleroma.Web.Preload},
+    {:pleroma, :http_security},
+    {:pleroma, Pleroma.User},
+    {:pleroma, Oban},
+    {:pleroma, :workers},
+    {:pleroma, Pleroma.Formatter},
+    {:pleroma, Pleroma.Emails.Mailer},
+    {:pleroma, Pleroma.Emails.UserEmail},
+    {:pleroma, Pleroma.Emails.NewUsersDigestEmail},
+    {:pleroma, Pleroma.ScheduledActivity},
+    {:pleroma, :email_notifications},
+    {:pleroma, :oauth2},
+    {:pleroma, Pleroma.Web.Plugs.RemoteIp},
+    {:pleroma, :static_fe},
+    {:pleroma, :frontends},
+    {:pleroma, :web_cache_ttl},
+    {:pleroma, :majic_pool},
+    {:pleroma, :restrict_unauthenticated},
+    {:pleroma, Pleroma.Web.ApiSpec.CastAndValidate},
+    {:pleroma, :mrf},
+    {:pleroma, :instances_favicons},
+    {:pleroma, :instances_nodeinfo},
+    {:pleroma, Pleroma.User.Backup},
+    {:pleroma, ConcurrentLimiter},
+    {:pleroma, Pleroma.Web.WebFinger},
+    {:pleroma, Pleroma.Search},
+    {:pleroma, Pleroma.Search.Meilisearch},
+    {:pleroma, Pleroma.Search.Elasticsearch.Cluster},
+    {:pleroma, :translator},
+    {:pleroma, :deepl},
+    {:pleroma, :libre_translate},
+    # But not argostranslate, because executables!
+    {:pleroma, Pleroma.Upload.Filter.AnonymizeFilename},
+    {:pleroma, Pleroma.Upload.Filter.Mogrify},
+    {:pleroma, Pleroma.Workers.PurgeExpiredActivity},
+    {:pleroma, :rate_limit}
+  ]
+
+  def allowed_groups, do: @allowed_groups
+
+  def enabled, do: Config.get(:configurable_from_database)
+
+  # the whitelist check can be called from either the loader or the
+  # doc generator, which is spitting out strings
+  defp maybe_stringified_atom_equal(a, b) do
+    a == inspect(b) || a == b
+  end
+
+  def whitelisted_config?(group, key) do
+    allowed_groups()
+    |> Enum.any?(fn
+      {whitelisted_group} ->
+        maybe_stringified_atom_equal(group, whitelisted_group)
+
+      {whitelisted_group, whitelisted_key} ->
+        maybe_stringified_atom_equal(group, whitelisted_group) && maybe_stringified_atom_equal(key, whitelisted_key)
+    end)
+  end
+
+  def whitelisted_config?(%{group: group, key: key}) do
+    whitelisted_config?(group, key)
+  end
+
+  def whitelisted_config?(%{group: group} = config) do
+    whitelisted_config?(group, config[:key])
+  end
+end

lib/pleroma/config/transfer_task.ex

@@ -8,6 +8,7 @@ defmodule Pleroma.Config.TransferTask do
   alias Pleroma.Config
   alias Pleroma.ConfigDB
   alias Pleroma.Repo
+  alias Pleroma.Config.ConfigurableFromDatabase
 
   require Logger
 
@@ -91,6 +92,18 @@ defp invalid_key_or_group(%ConfigDB{group: :invalid_atom}), do: true
   defp invalid_key_or_group(_), do: false
 
   defp merge_with_default(%{group: group, key: key, value: value} = setting) do
+    if !ConfigurableFromDatabase.whitelisted_config?(setting) do
+      Logger.warning(~s[
+        config #{inspect(group)}, #{inspect(key)} is set in the database,
+        but it is not explicitly allowed to be there. Consider removing it
+        with
+          MIX: mix pleroma.config delete #{group} #{key}
+          OTP: ./bin/pleroma_ctl config delete #{group} #{key}
+        and setting it in your .exs file instead
+          config #{inspect(group)}, #{inspect(key)}, #{inspect(value)}
+      ])
+    end
+
     default =
       if group == :pleroma do
         Config.get([key], Config.Holder.default_config(group, key))

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

@@ -8,6 +8,7 @@ defmodule Pleroma.Web.AdminAPI.ConfigController do
   alias Pleroma.Config
   alias Pleroma.ConfigDB
   alias Pleroma.Web.Plugs.OAuthScopesPlug
+  alias Pleroma.Config.ConfigurableFromDatabase
 
   plug(Pleroma.Web.ApiSpec.CastAndValidate)
   plug(OAuthScopesPlug, %{scopes: ["admin:write"]} when action == :update)
@@ -71,7 +72,11 @@ defp translate_children(item, _path) do
   end
 
   def descriptions(conn, _params) do
-    descriptions = Enum.filter(Pleroma.Docs.JSON.compiled_descriptions(), &whitelisted_config?/1)
+    descriptions =
+      Enum.filter(
+        Pleroma.Docs.JSON.compiled_descriptions(),
+        &ConfigurableFromDatabase.whitelisted_config?/1
+      )
 
     json(conn, translate_descriptions(descriptions))
   end
@@ -132,7 +137,7 @@ def update(%{body_params: %{configs: configs}} = conn, _) do
     with :ok <- configurable_from_database() do
       results =
         configs
-        |> Enum.filter(&whitelisted_config?/1)
+        |> Enum.filter(&ConfigurableFromDatabase.whitelisted_config?/1)
         |> Enum.map(fn
           %{group: group, key: key, delete: true} = params ->
             ConfigDB.delete(%{group: group, key: key, subkeys: params[:subkeys]})
@@ -167,32 +172,10 @@ def update(%{body_params: %{configs: configs}} = conn, _) do
   end
 
   defp configurable_from_database do
-    if Config.get(:configurable_from_database) do
+    if ConfigurableFromDatabase.enabled() do
       :ok
     else
       {:error, "You must enable configurable_from_database in your config file."}
     end
   end
-
-  defp whitelisted_config?(group, key) do
-    if whitelisted_configs = Config.get(:database_config_whitelist) do
-      Enum.any?(whitelisted_configs, fn
-        {whitelisted_group} ->
-          group == inspect(whitelisted_group)
-
-        {whitelisted_group, whitelisted_key} ->
-          group == inspect(whitelisted_group) && key == inspect(whitelisted_key)
-      end)
-    else
-      true
-    end
-  end
-
-  defp whitelisted_config?(%{group: group, key: key}) do
-    whitelisted_config?(group, key)
-  end
-
-  defp whitelisted_config?(%{group: group} = config) do
-    whitelisted_config?(group, config[:key])
-  end
 end

test/pleroma/config/transfer_task_test.exs

@@ -93,6 +93,31 @@ test "transfer config values with full subkey update" do
     assert assets_env[:mascots] == [a: 1, b: 2]
   end
 
+  test "transfer config values that are not explicitly whitelisted" do
+    insert(:config, key: :whoops, value: [not_allowed: true])
+
+    log =
+      capture_log(fn ->
+        TransferTask.start_link([])
+      end)
+
+    assert log =~ "config :pleroma, :whoops is set in the database"
+    assert log =~ "Consider removing it"
+    assert log =~ "mix pleroma.config delete pleroma whoops"
+  end
+
+  test "transferring whitelisted values should not warn" do
+    insert(:config, key: :emoji, value: [allowed: true])
+    insert(:config, key: Pleroma.Workers.PurgeExpiredActivity, value: [allowed: true])
+
+    log =
+      capture_log(fn ->
+        TransferTask.start_link([])
+      end)
+
+    refute log =~ "Consider removing it"
+  end
+
   describe "pleroma restart" do
     setup do
       on_exit(fn ->

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

@@ -0,0 +1,69 @@
+# Pleroma: A lightweight social networking server
+# Copyright © 2017-2021 Pleroma Authors <https://pleroma.social/>
+# SPDX-License-Identifier: AGPL-3.0-only
+
+defmodule Pleroma.Web.AdminAPI.ConfigControllerTest do
+  use Pleroma.Web.ConnCase, async: false
+
+  import Pleroma.Factory
+
+  setup_all do
+    Tesla.Mock.mock_global(fn env -> apply(HttpRequestMock, :request, [env]) end)
+
+    :ok
+  end
+
+  setup do
+    clear_config(:configurable_from_database, true)
+    admin = insert(:user, is_admin: true)
+    token = insert(:oauth_admin_token, user: admin)
+
+    conn =
+      build_conn()
+      |> assign(:user, admin)
+      |> assign(:token, token)
+      |> put_req_header("content-type", "application/json")
+
+    {:ok, %{admin: admin, token: token, conn: conn}}
+  end
+
+  describe "POST /api/v1/pleroma/admin/config" do
+    test "Refuses to update non-whitelisted config options", %{conn: conn} do
+      banned_config = %{
+        configs: [
+          %{
+            group: ":mogrify",
+            key: ":mogrify_command",
+            value: [
+              %{tuple: [":path", "sh"]},
+              %{tuple: [":args", ["-c", "echo pwnd > /tmp/a"]]}
+            ]
+          },
+          %{
+            group: ":pleroma",
+            key: ":http",
+            value: [
+              %{tuple: ["wow", "nice"]}
+            ]
+          }
+        ]
+      }
+
+      resp =
+        conn
+        |> post(~p"/api/v1/pleroma/admin/config", banned_config)
+        |> json_response_and_validate_schema(200)
+
+      # It should basically just throw out the mogrify option
+      assert Enum.count(resp["configs"]) == 1
+
+      assert %{
+               "configs" => [
+                 %{
+                   "group" => ":pleroma"
+                 }
+               ]
+             } = resp
+    end
+  end
+end