From e113eff6634c9b398b0541ea6a0d1626f6732dcc Mon Sep 17 00:00:00 2001
From: Ted Johansson <ted@discourse.org>
Date: Fri, 6 Oct 2023 19:21:01 +0200
Subject: [PATCH] DEV: Sanitize integer site settings in front- and back-end
 (#23816)

Currently, if you set an integer site setting in the admin interface and include thousands separators, you will silently configure the wrong value.

This PR replaces TextField inputs for integer site settings with NumberField. It also cleans the numeric input of any non-digits in the backend in case any separators make it through.
---
 .../admin/addon/components/site-settings/integer.hbs       | 4 ++++
 .../admin/addon/components/site-settings/integer.js        | 3 +++
 .../javascripts/admin/addon/mixins/setting-component.js    | 1 +
 app/assets/stylesheets/common/admin/settings.scss          | 1 +
 app/controllers/admin/site_settings_controller.rb          | 5 ++++-
 spec/requests/admin/site_settings_controller_spec.rb       | 7 +++++++
 6 files changed, 20 insertions(+), 1 deletion(-)
 create mode 100644 app/assets/javascripts/admin/addon/components/site-settings/integer.hbs
 create mode 100644 app/assets/javascripts/admin/addon/components/site-settings/integer.js

diff --git a/app/assets/javascripts/admin/addon/components/site-settings/integer.hbs b/app/assets/javascripts/admin/addon/components/site-settings/integer.hbs
new file mode 100644
index 00000000000..a3a297a4976
--- /dev/null
+++ b/app/assets/javascripts/admin/addon/components/site-settings/integer.hbs
@@ -0,0 +1,4 @@
+<NumberField @value={{this.value}} @classNames="input-setting-integer" />
+
+<SettingValidationMessage @message={{this.validationMessage}} />
+<div class="desc">{{html-safe this.setting.description}}</div>
\ No newline at end of file
diff --git a/app/assets/javascripts/admin/addon/components/site-settings/integer.js b/app/assets/javascripts/admin/addon/components/site-settings/integer.js
new file mode 100644
index 00000000000..6df18068c19
--- /dev/null
+++ b/app/assets/javascripts/admin/addon/components/site-settings/integer.js
@@ -0,0 +1,3 @@
+import Component from "@ember/component";
+
+export default class Integer extends Component {}
diff --git a/app/assets/javascripts/admin/addon/mixins/setting-component.js b/app/assets/javascripts/admin/addon/mixins/setting-component.js
index 455991badf0..6ea8aeb9679 100644
--- a/app/assets/javascripts/admin/addon/mixins/setting-component.js
+++ b/app/assets/javascripts/admin/addon/mixins/setting-component.js
@@ -15,6 +15,7 @@ import SiteSettingDefaultCategoriesModal from "../components/modal/site-setting-
 
 const CUSTOM_TYPES = [
   "bool",
+  "integer",
   "enum",
   "list",
   "url_list",
diff --git a/app/assets/stylesheets/common/admin/settings.scss b/app/assets/stylesheets/common/admin/settings.scss
index 59e68a27ede..99a27fb033e 100644
--- a/app/assets/stylesheets/common/admin/settings.scss
+++ b/app/assets/stylesheets/common/admin/settings.scss
@@ -58,6 +58,7 @@
       float: left;
     }
     .input-setting-string,
+    .input-setting-integer,
     .input-setting-textarea {
       width: 100%;
       @media (max-width: $mobile-breakpoint) {
diff --git a/app/controllers/admin/site_settings_controller.rb b/app/controllers/admin/site_settings_controller.rb
index 55eaf550d8b..8ff483e388b 100644
--- a/app/controllers/admin/site_settings_controller.rb
+++ b/app/controllers/admin/site_settings_controller.rb
@@ -31,7 +31,10 @@ class Admin::SiteSettingsController < Admin::AdminController
 
     raise_access_hidden_setting(id)
 
-    if SiteSetting.type_supervisor.get_type(id) == :uploaded_image_list
+    case SiteSetting.type_supervisor.get_type(id)
+    when :integer
+      value = value.gsub(/\D/, "")
+    when :uploaded_image_list
       value = Upload.get_from_urls(value.split("|")).to_a
     end
 
diff --git a/spec/requests/admin/site_settings_controller_spec.rb b/spec/requests/admin/site_settings_controller_spec.rb
index e0f0a77ea7a..57fac97500e 100644
--- a/spec/requests/admin/site_settings_controller_spec.rb
+++ b/spec/requests/admin/site_settings_controller_spec.rb
@@ -269,6 +269,13 @@ RSpec.describe Admin::SiteSettingsController do
         expect(SiteSetting.title).to eq("")
       end
 
+      it "sanitizes integer values" do
+        put "/admin/site_settings/suggested_topics.json", params: { suggested_topics: "1,000" }
+
+        expect(response.status).to eq(200)
+        expect(SiteSetting.suggested_topics).to eq(1000)
+      end
+
       context "with default user options" do
         let!(:user1) { Fabricate(:user) }
         let!(:user2) { Fabricate(:user) }