FIX: disallow invalid top_menu and post_menu and share_links

In the past any text could be entered there causing big potential issues

config/locales/server.en.yml

@@ -154,6 +154,9 @@ en:
     embed:
       load_from_remote: "There was an error loading that post."
     site_settings:
+      invalid_choice:
+        one: 'You specified the invalid choice %{name}'
+        other: 'You specified the invalid choices %{name}'
       min_username_length_exists: "You cannot set the minimum username length above the shortest username."
       min_username_length_range: "You cannot set the minimum above the maximum."
       max_username_length_exists: "You cannot set the maximum username length below the longest username."

config/site_settings.yml

@@ -7,6 +7,7 @@
 # max                - For a string setting, the maximum length. For an integer setting, the maximum value.
 # regex              - A regex that the value must match.
 # validator          - The name of the class that will be use to validate the value of the setting.
+# allow_any          - For choice settings allow items not specified in the choice list (default true)
 # enum               - The setting has a fixed set of allowed values, and only one can be chosen.
 #                      Set to the class name that defines the set.
 # shadowed_by_global - "Shadow" a site setting with a GlobalSetting. If the GlobalSetting
@@ -121,6 +122,7 @@ basic:
     regex: "latest"
     regex_error: "site_settings.errors.must_include_latest"
     validator: RegexPresenceValidator
+    allow_any: false
     choices:
      - latest
      - new
@@ -134,6 +136,7 @@ basic:
     client: true
     type: list
     default: 'like-count|like|share|flag|edit|bookmark|delete|admin|reply'
+    allow_any: false
     choices:
      - like-count
      - like
@@ -148,6 +151,7 @@ basic:
     client: true
     type: list
     default: 'flag|bookmark|edit|delete|admin'
+    allow_any: false
     choices:
      - like
      - edit
@@ -161,6 +165,7 @@ basic:
     client: true
     type: list
     default: 'twitter|facebook|email'
+    allow_any: false
     choices:
      - twitter
      - facebook

lib/site_settings/type_supervisor.rb

@@ -6,7 +6,7 @@ module SiteSettings; end
 class SiteSettings::TypeSupervisor
   include SiteSettings::Validations
 
-  CONSUMED_OPTS = %i[enum choices type validator min max regex hidden regex_error].freeze
+  CONSUMED_OPTS = %i[enum choices type validator min max regex hidden regex_error allow_any].freeze
   VALIDATOR_OPTS = %i[min max regex hidden regex_error].freeze
 
   # For plugins, so they can tell if a feature is supported
@@ -61,6 +61,7 @@ class SiteSettings::TypeSupervisor
     @choices = {}
     @validators = {}
     @types = {}
+    @allow_any = {}
   end
 
   def load_setting(name_arg, opts = {})
@@ -83,6 +84,10 @@ class SiteSettings::TypeSupervisor
 
     if (type = opts[:type])
       @static_types[name] = type.to_sym
+
+      if type.to_sym == :list
+        @allow_any[name] = opts[:allow_any] == false ? false : true
+      end
     end
     @types[name] = get_data_type(name, @defaults_provider[name])
 
@@ -168,6 +173,16 @@ class SiteSettings::TypeSupervisor
       end
     end
 
+    if type == self.class.types[:list] || type == self.class.types[:string]
+      if @allow_any.key?(name) && !@allow_any[name]
+        split = val.to_s.split("|")
+        diff = (split - @choices[name])
+        if diff.length > 0
+          raise Discourse::InvalidParameters.new(I18n.t('errors.site_settings.invalid_choice', name: diff.join(','), count: diff.length))
+        end
+      end
+    end
+
     if (v = @validators[name])
       validator = v[:class].new(v[:opts])
       unless validator.valid_value?(val)

spec/models/site_setting_spec.rb

@@ -52,7 +52,19 @@ describe SiteSetting do
   end
 
   describe "top_menu" do
-    before { SiteSetting.top_menu = 'one,-nope|two|three,-not|four,ignored|category/xyz|latest' }
+    describe "validations" do
+      it "always demands latest" do
+        expect do
+          SiteSetting.top_menu = 'categories'
+        end.to raise_error(Discourse::InvalidParameters)
+      end
+
+      it "does not allow random text" do
+        expect do
+          SiteSetting.top_menu = 'latest|random'
+        end.to raise_error(Discourse::InvalidParameters)
+      end
+    end
 
     describe "items" do
       let(:items) { SiteSetting.top_menu_items }
@@ -64,7 +76,8 @@ describe SiteSetting do
 
     describe "homepage" do
       it "has homepage" do
-        expect(SiteSetting.homepage).to eq('one')
+        SiteSetting.top_menu = "bookmarks|latest"
+        expect(SiteSetting.homepage).to eq('bookmarks')
       end
     end
   end