Merge branch 'fj-forbid-snippet-route' into 'master'

Forbidding the sitemap route path

See merge request gitlab-org/gitlab!45359

changelogs/unreleased/fj-forbid-snippet-route.yml

+---
+title: Forbid top-level route sitemap.xml
+merge_request: 45359
+author:
+type: changed

db/post_migrate/20201019094741_rename_sitemap_root_namespaces.rb

+# frozen_string_literal: true
+
+class RenameSitemapRootNamespaces < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+  include Gitlab::Database::RenameReservedPathsMigration::V1
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  # We're taking over the /sitemap.xml and /sitemap.xml.gz namespaces
+  # since they're necessary for the default behavior of Sitemaps
+  def up
+    disable_statement_timeout do
+      rename_root_paths(['sitemap.xml', 'sitemap.xml.gz'])
+    end
+  end
+
+  def down
+    disable_statement_timeout do
+      revert_renames
+    end
+  end
+end

db/schema_migrations/20201019094741

+16ef5ba153f1145dcd2578bb8c860d4e1a975d5df3a1b1d9315946e632a95999
\ No newline at end of file

doc/user/reserved_names.md

@@ -76,6 +76,8 @@ Currently the following names are reserved as top level groups:
 - `s`
 - `search`
 - `sent_notifications`
+- `sitemap.xml`
+- `sitemap.xml.gz`
 - `slash-command-logo.png`
 - `snippets`
 - `unsubscribes`

lib/gitlab/path_regex.rb

@@ -49,6 +49,8 @@ module Gitlab
       s
       search
       sent_notifications
+      sitemap.xml
+      sitemap.xml.gz
       slash-command-logo.png
       snippets
       unsubscribes

spec/lib/gitlab/path_regex_spec.rb

@@ -101,10 +101,15 @@ RSpec.describe Gitlab::PathRegex do
       .concat(ee_top_level_words)
       .concat(files_in_public)
       .concat(Array(API::API.prefix.to_s))
+      .concat(sitemap_words)
       .compact
       .uniq
   end
 
+  let(:sitemap_words) do
+    %w(sitemap.xml sitemap.xml.gz)
+  end
+
   let(:ee_top_level_words) do
     %w(unsubscribes v2)
   end
@@ -172,7 +177,7 @@ RSpec.describe Gitlab::PathRegex do
 
     # We ban new items in this list, see https://gitlab.com/gitlab-org/gitlab/-/issues/215362
     it 'does not allow expansion' do
-      expect(described_class::TOP_LEVEL_ROUTES.size).to eq(41)
+      expect(described_class::TOP_LEVEL_ROUTES.size).to eq(43)
     end
   end
 
@@ -218,6 +223,8 @@ RSpec.describe Gitlab::PathRegex do
       expect(subject).not_to match('admin/')
       expect(subject).not_to match('api/')
       expect(subject).not_to match('.well-known/')
+      expect(subject).not_to match('sitemap.xml/')
+      expect(subject).not_to match('sitemap.xml.gz/')
     end
 
     it 'accepts project wildcard routes' do

spec/migrations/rename_sitemap_root_namespaces_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+require Rails.root.join('db', 'post_migrate', '20201019094741_rename_sitemap_root_namespaces.rb')
+
+RSpec.describe RenameSitemapRootNamespaces do
+  let(:namespaces) { table(:namespaces) }
+  let(:routes) { table(:routes) }
+  let(:sitemap_path) { 'sitemap.xml' }
+  let(:sitemap_gz_path) { 'sitemap.xml.gz' }
+  let(:other_path1) { 'sitemap.xmlfoo' }
+  let(:other_path2) { 'foositemap.xml' }
+
+  it 'correctly run #up and #down' do
+    create_namespace(sitemap_path)
+    create_namespace(sitemap_gz_path)
+    create_namespace(other_path1)
+    create_namespace(other_path2)
+
+    reversible_migration do |migration|
+      migration.before -> {
+        expect(namespaces.pluck(:path)).to contain_exactly(sitemap_path, sitemap_gz_path, other_path1, other_path2)
+      }
+
+      migration.after -> {
+        expect(namespaces.pluck(:path)).to contain_exactly(sitemap_path + '0', sitemap_gz_path + '0', other_path1, other_path2)
+      }
+    end
+  end
+
+  def create_namespace(path)
+    namespaces.create!(name: path, path: path).tap do |namespace|
+      routes.create!(path: namespace.path, name: namespace.name, source_id: namespace.id, source_type: 'Namespace')
+    end
+  end
+end