Restrict nested group names to prevent ambiguous routes

Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>

app/validators/namespace_validator.rb

@@ -35,13 +35,22 @@ class NamespaceValidator < ActiveModel::EachValidator
     users
   ].freeze
 
+  WILDCARD_ROUTES = %w[tree commits wikis new edit create update logs_tree
+                       preview blob blame raw files create_dir find_file].freeze
+
+  STRICT_RESERVED = (RESERVED + WILDCARD_ROUTES).freeze
+
   def self.valid?(value)
     !reserved?(value) && follow_format?(value)
   end
 
-  def self.reserved?(value)
+  def self.reserved?(value, strict: false)
+    if strict
+      STRICT_RESERVED.include?(value)
+    else
       RESERVED.include?(value)
     end
+  end
 
   def self.follow_format?(value)
     value =~ Gitlab::Regex.namespace_regex
@@ -54,7 +63,9 @@ class NamespaceValidator < ActiveModel::EachValidator
       record.errors.add(attribute, Gitlab::Regex.namespace_regex_message)
     end
 
-    if reserved?(value)
+    strict = record.is_a?(Group) && record.parent_id
+
+    if reserved?(value, strict: strict)
       record.errors.add(attribute, "#{value} is a reserved name")
     end
   end

app/validators/project_path_validator.rb

@@ -14,10 +14,8 @@ class ProjectPathValidator < ActiveModel::EachValidator
   #  without tree as reserved name routing can match 'group/project' as group name,
   #  'tree' as project name and 'deploy_keys' as route.
   #
-  RESERVED = (NamespaceValidator::RESERVED -
-              %w[dashboard help ci admin search notes services assets profile public] +
-              %w[tree commits wikis new edit create update logs_tree
-                 preview blob blame raw files create_dir find_file]).freeze
+  RESERVED = (NamespaceValidator::STRICT_RESERVED -
+              %w[dashboard help ci admin search notes services assets profile public]).freeze
 
   def self.valid?(value)
     !reserved?(value)

changelogs/unreleased/dz-nested-groups-restrictions.yml

+---
+title: Restrict nested group names to prevent ambiguous routes
+merge_request: 9738
+author:

spec/models/namespace_spec.rb

@@ -28,6 +28,20 @@ describe Namespace, models: true do
       expect(nested).not_to be_valid
       expect(nested.errors[:parent_id].first).to eq('has too deep level of nesting')
     end
+
+    describe 'reserved path validation' do
+      context 'nested group' do
+        let(:group) { build(:group, :nested, path: 'tree') }
+
+        it { expect(group).not_to be_valid }
+      end
+
+      context 'top-level group' do
+        let(:group) { build(:group, path: 'tree') }
+
+        it { expect(group).to be_valid }
+      end
+    end
   end
 
   describe "Respond to" do