Explicitly allow configDB keys #628

Closed
floatingghost wants to merge 9 commits from config-db-keys into develop
5 changed files with 102 additions and 91 deletions
Showing only changes of commit ccaff7104d - Show all commits

View file

@ -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,
@ -792,17 +795,6 @@
config :pleroma, :modules, runtime_dir: "instance/modules"
config :pleroma, configurable_from_database: false
# Don't allow arbitrary module config here, you can only
# adjust our own config.
config :pleroma,
database_config_whitelist: [
{:pleroma},
{:logger}
]
config :pleroma, Pleroma.Repo,
parameters: [gin_fuzzy_search_limit: "500"],
prepare: :unnamed
config :pleroma, :majic_pool, size: 2

View file

@ -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",

View file

@ -0,0 +1,78 @@
defmodule Pleroma.Config.ConfigurableFromDatabase do
# 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},
Review

The first line of this comment makes sense in the scope of this PR, but not for someone looking at the code as-is. I would remove that line.

I'm not sure what's meant in the last line with "our stuff", so maybe that should also be removed, or clarified?

The first line of this comment makes sense in the scope of this PR, but not for someone looking at the code as-is. I would remove that line. I'm not sure what's meant in the last line with "our stuff", so maybe that should also be removed, or clarified?
{:pleroma, Pleroma.Captcha},
{:pleroma, Pleroma.Captcha.Kocaptcha},
{:pleroma, Pleroma.Upload},
{:pleroma, Pleroma.Uploaders.Local},
{:pleroma, Pleroma.Uploaders.S3},
{: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!
]
def allowed_groups, do: @allowed_groups
end

View file

@ -9,14 +9,6 @@ defmodule Pleroma.Web.AdminAPI.ConfigController do
alias Pleroma.ConfigDB
alias Pleroma.Web.Plugs.OAuthScopesPlug
@banned_in_db [
# this would make no sense if you could change it in the db
{:pleroma, :database_config_whitelist},
# called with System.cmd
{:pleroma, Pleroma.Web.MediaProxy.Invalidation.Script},
{:pleroma, :argos_translate}
]
plug(Pleroma.Web.ApiSpec.CastAndValidate)
plug(OAuthScopesPlug, %{scopes: ["admin:write"]} when action == :update)
@ -183,9 +175,7 @@ defp configurable_from_database do
end
defp whitelisted_config?(group, key) do
whitelisted =
:database_config_whitelist
|> Config.get([{:pleroma}])
Pleroma.Config.ConfigurableFromDatabase.allowed_groups()
|> Enum.any?(fn
{whitelisted_group} ->
group == inspect(whitelisted_group)
@ -193,8 +183,6 @@ defp whitelisted_config?(group, key) do
{whitelisted_group, whitelisted_key} ->
group == inspect(whitelisted_group) && key == inspect(whitelisted_key)
end)
whitelisted && !disallowed_config?(group, key)
end
defp whitelisted_config?(%{group: group, key: key}) do
@ -204,14 +192,4 @@ defp whitelisted_config?(%{group: group, key: key}) do
defp whitelisted_config?(%{group: group} = config) do
whitelisted_config?(group, config[:key])
end
defp disallowed_config?(group, key) do
Enum.any?(@banned_in_db, fn
{disallowed_group} ->
group == inspect(disallowed_group)
{disallowed_group, disallowed_key} ->
group == inspect(disallowed_group) && key == inspect(disallowed_key)
end)
end
end

View file

@ -38,58 +38,32 @@ test "Refuses to update non-whitelisted config options", %{conn: conn} do
%{tuple: [":path", "sh"]},
%{tuple: [":args", ["-c", "echo pwnd > /tmp/a"]]}
]
}
]
}
clear_config([:database_config_whitelist], [{:pleroma}])
resp_that_should_not_work =
conn
|> post(~p"/api/v1/pleroma/admin/config", banned_config)
|> json_response_and_validate_schema(200)
assert Enum.empty?(resp_that_should_not_work["configs"])
clear_config([:database_config_whitelist], [{:mogrify}])
resp_that_should_work =
conn
|> post(~p"/api/v1/pleroma/admin/config", banned_config)
|> json_response_and_validate_schema(200)
refute Enum.empty?(resp_that_should_work["configs"])
end
test "Refuses to update strictly disallowed options", %{conn: conn} do
banned_config = %{
configs: [
%{
group: ":pleroma",
key: ":database_config_whitelist",
value: [":pleroma"]
},
%{
group: ":pleroma",
key: ":argos_translate",
key: ":http",
value: [
%{tuple: [":command_argospm", "/opt/oepsiewoepsie"]}
%{tuple: ["wow", "nice"]}
]
},
%{
group: ":pleroma",
key: "Pleroma.Web.MediaProxy.Invalidation.Script",
value: "wowee"
}
]
}
resp_that_should_not_work =
resp =
conn
|> post(~p"/api/v1/pleroma/admin/config", banned_config)
|> json_response_and_validate_schema(200)
assert Enum.empty?(resp_that_should_not_work["configs"])
# It should basically just throw out the mogrify option
assert Enum.count(resp["configs"]) == 1
assert %{
"configs" => [
%{
"group" => ":pleroma"
}
]
} = resp
end
end
end