Fix leading slash in redirects and add cop

6 files changed, 92 insertions, 2 deletions

diff --git a/changelogs/unreleased/fix-leading-slash-in-redirects-plus-rubocop.yml b/changelogs/unreleased/fix-leading-slash-in-redirects-plus-rubocop.yml
new file mode 100644
index 00000000000..38b2486a475
--- /dev/null
+++ b/changelogs/unreleased/fix-leading-slash-in-redirects-plus-rubocop.yml
@@ -0,0 +1,5 @@
+---
+title: Fix leading slash in redirects and add rubocop cop
+merge_request: 21828
+author: Sanad Liaquat
+type: fixed
diff --git a/config/routes/instance_statistics.rb b/config/routes/instance_statistics.rb
index 824ef47cda3..1102ef6b017 100644
--- a/config/routes/instance_statistics.rb
+++ b/config/routes/instance_statistics.rb
@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 namespace :instance_statistics do
-  root to: redirect('/-/instance_statistics/conversational_development_index')
+  root to: redirect('-/instance_statistics/conversational_development_index')
 
   resources :cohorts, only: :index
   resources :conversational_development_index, only: :index
diff --git a/config/routes/wiki.rb b/config/routes/wiki.rb
index cd3828b743c..1a07b1c206b 100644
--- a/config/routes/wiki.rb
+++ b/config/routes/wiki.rb
@@ -2,7 +2,7 @@ scope(controller: :wikis) do
   scope(path: 'wikis', as: :wikis) do
     get :git_access
     get :pages
-    get '/', to: redirect('/%{namespace_id}/%{project_id}/wikis/home')
+    get '/', to: redirect('%{namespace_id}/%{project_id}/wikis/home')
     post '/', to: 'wikis#create'
   end
 
diff --git a/rubocop/cop/avoid_route_redirect_leading_slash.rb b/rubocop/cop/avoid_route_redirect_leading_slash.rb
new file mode 100644
index 00000000000..7ac1c881269
--- /dev/null
+++ b/rubocop/cop/avoid_route_redirect_leading_slash.rb
@@ -0,0 +1,52 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    # Checks for a leading '/' in route redirects
+    # For more information see: https://gitlab.com/gitlab-org/gitlab-ce/issues/50645
+    #
+    # @example
+    #   # bad
+    #   root to: redirect('/-/instance/statistics/conversational_development_index')
+    #
+    #   # good
+    #   root to: redirect('-/instance/statistics/conversational_development_index')
+    #
+
+    class AvoidRouteRedirectLeadingSlash < RuboCop::Cop::Cop
+      MSG = 'Do not use a leading "/" in route redirects'
+
+      def_node_matcher :leading_slash_in_redirect?, <<~PATTERN
+        (send nil? :redirect (str #has_leading_slash?))
+      PATTERN
+
+      def on_send(node)
+        return unless in_routes?(node)
+        return unless leading_slash_in_redirect?(node)
+
+        add_offense(node)
+      end
+
+      def has_leading_slash?(str)
+        str.start_with?("/")
+      end
+
+      def in_routes?(node)
+        path = node.location.expression.source_buffer.name
+        dirname = File.dirname(path)
+        filename = File.basename(path)
+        dirname.end_with?('config/routes') || filename.end_with?('routes.rb')
+      end
+
+      def autocorrect(node)
+        lambda do |corrector|
+          corrector.replace(node.loc.expression, remove_leading_slash(node))
+        end
+      end
+
+      def remove_leading_slash(node)
+        node.source.sub('/', '')
+      end
+    end
+  end
+end
diff --git a/rubocop/rubocop.rb b/rubocop/rubocop.rb
index 9d6cc73fc3b..ff929c7b6ce 100644
--- a/rubocop/rubocop.rb
+++ b/rubocop/rubocop.rb
@@ -7,6 +7,7 @@ require_relative 'cop/gitlab/union'
 require_relative 'cop/include_sidekiq_worker'
 require_relative 'cop/avoid_return_from_blocks'
 require_relative 'cop/avoid_break_from_strong_memoize'
+require_relative 'cop/avoid_route_redirect_leading_slash'
 require_relative 'cop/line_break_around_conditional_block'
 require_relative 'cop/prefer_class_methods_over_module'
 require_relative 'cop/migration/add_column'
diff --git a/spec/rubocop/cop/avoid_route_redirect_leading_slash_spec.rb b/spec/rubocop/cop/avoid_route_redirect_leading_slash_spec.rb
new file mode 100644
index 00000000000..c9eb61ccc72
--- /dev/null
+++ b/spec/rubocop/cop/avoid_route_redirect_leading_slash_spec.rb
@@ -0,0 +1,32 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+require 'rubocop'
+require_relative '../../../rubocop/cop/avoid_route_redirect_leading_slash'
+
+describe RuboCop::Cop::AvoidRouteRedirectLeadingSlash do
+  include CopHelper
+
+  subject(:cop) { described_class.new }
+
+  before do
+    allow(cop).to receive(:in_routes?).and_return(true)
+  end
+
+  it 'registers an offense when redirect has a leading slash' do
+    expect_offense(<<~PATTERN.strip_indent)
+      root to: redirect("/-/route")
+               ^^^^^^^^^^^^^^^^^^^^ Do not use a leading "/" in route redirects
+    PATTERN
+  end
+
+  it 'does not register an offense when redirect does not have a leading slash' do
+    expect_no_offenses(<<~PATTERN.strip_indent)
+      root to: redirect("-/route")
+    PATTERN
+  end
+
+  it 'autocorrect `/-/route` to `-/route`' do
+    expect(autocorrect_source('redirect("/-/route")')).to eq('redirect("-/route")')
+  end
+end