Add routes for unmatched url for not-get requests

Add changelog entry

Fix typo in routes

Fix git http spec

Fix uploads routing spec

Add matchers only to project facing paths

Add more specs for new routes

Add different shared examples

Fix rubocop offence

Add failure message to new matcher

Add cr remarks

Add cr remarks

changelogs/unreleased/secutity-404-difference.yml

+---
+title: Add routes for unmatched url for not-get requests
+merge_request:
+author:
+type: security

config/routes.rb

@@ -275,6 +275,7 @@ Rails.application.routes.draw do
   draw :dashboard
   draw :user
   draw :project
+  draw :unmatched_project
 
   # Issue https://gitlab.com/gitlab-org/gitlab/-/issues/210024
   scope as: 'deprecated' do

config/routes/unmatched_project.rb

+# frozen_string_literal: true
+
+scope(path: '*namespace_id',
+  as: :namespace,
+  namespace_id: Gitlab::PathRegex.full_namespace_route_regex) do
+  scope(path: ':project_id',
+    constraints: { project_id: Gitlab::PathRegex.project_route_regex },
+    as: :project) do
+    post '*all', to: 'application#route_not_found'
+    put '*all', to: 'application#route_not_found'
+    patch '*all', to: 'application#route_not_found'
+    delete '*all', to: 'application#route_not_found'
+    post '/', to: 'application#route_not_found'
+    put '/', to: 'application#route_not_found'
+    patch '/', to: 'application#route_not_found'
+    delete '/', to: 'application#route_not_found'
+  end
+end

ee/spec/routing/git_http_routing_spec.rb

@@ -9,5 +9,11 @@ RSpec.describe 'EE git_http routing' do
       let(:container_path) { '/gitlab-org/gitlab-test' }
       let(:params) { { geo_node_id: 'node', repository_path: 'gitlab-org/gitlab-test.git' } }
     end
+
+    it_behaves_like 'git repository routes with fallback for git-upload-pack' do
+      let(:path) { '/-/push_from_secondary/node/gitlab-org/gitlab-test.git' }
+      let(:container_path) { '/gitlab-org/gitlab-test' }
+      let(:params) { { geo_node_id: 'node', repository_path: 'gitlab-org/gitlab-test.git' } }
+    end
   end
 end

spec/requests/git_http_spec.rb

@@ -159,13 +159,17 @@ RSpec.describe 'Git HTTP requests' do
 
     context "POST git-upload-pack" do
       it "fails to find a route" do
-        expect { clone_post(repository_path) }.to raise_error(ActionController::RoutingError)
+        clone_post(repository_path) do |response|
+          expect(response).to have_gitlab_http_status(:not_found)
+        end
       end
     end
 
     context "POST git-receive-pack" do
       it "fails to find a route" do
-        expect { push_post(repository_path) }.to raise_error(ActionController::RoutingError)
+        push_post(repository_path) do |response|
+          expect(response).to have_gitlab_http_status(:not_found)
+        end
       end
     end
   end

spec/routing/git_http_routing_spec.rb

@@ -7,6 +7,10 @@ RSpec.describe 'git_http routing' do
     it_behaves_like 'git repository routes' do
       let(:path) { '/gitlab-org/gitlab-test.git' }
     end
+
+    it_behaves_like 'git repository routes with fallback for git-upload-pack' do
+      let(:path) { '/gitlab-org/gitlab-test.git' }
+    end
   end
 
   describe 'wiki repositories' do
@@ -14,6 +18,7 @@ RSpec.describe 'git_http routing' do
       let(:path) { '/gitlab-org/gitlab-test.wiki.git' }
 
       it_behaves_like 'git repository routes'
+      it_behaves_like 'git repository routes with fallback for git-upload-pack'
 
       describe 'redirects', type: :request do
         let(:web_path) { '/gitlab-org/gitlab-test/-/wikis' }
@@ -37,12 +42,20 @@ RSpec.describe 'git_http routing' do
       it_behaves_like 'git repository routes' do
         let(:path) { '/gitlab-org.wiki.git' }
       end
+
+      it_behaves_like 'git repository routes with fallback for git-upload-pack' do
+        let(:path) { '/gitlab-org.wiki.git' }
+      end
     end
 
     context 'in child group' do
       it_behaves_like 'git repository routes' do
         let(:path) { '/gitlab-org/child.wiki.git' }
       end
+
+      it_behaves_like 'git repository routes with fallback for git-upload-pack' do
+        let(:path) { '/gitlab-org/child.wiki.git' }
+      end
     end
   end
 
@@ -51,12 +64,20 @@ RSpec.describe 'git_http routing' do
       it_behaves_like 'git repository routes' do
         let(:path) { '/snippets/123.git' }
       end
+
+      it_behaves_like 'git repository routes without fallback' do
+        let(:path) { '/snippets/123.git' }
+      end
     end
 
     context 'project snippet' do
       it_behaves_like 'git repository routes' do
         let(:path) { '/gitlab-org/gitlab-test/snippets/123.git' }
       end
+
+      it_behaves_like 'git repository routes with fallback' do
+        let(:path) { '/gitlab-org/gitlab-test/snippets/123.git' }
+      end
     end
   end
 end

spec/routing/project_routing_spec.rb

@@ -876,4 +876,73 @@ RSpec.describe 'project routing' do
       )
     end
   end
+
+  context 'with a non-existent project' do
+    it 'routes to 404 with get request' do
+      expect(get: "/gitlab/not_exist").to route_to(
+        'application#route_not_found',
+        unmatched_route: 'gitlab/not_exist'
+      )
+    end
+
+    it 'routes to 404 with delete request' do
+      expect(delete: "/gitlab/not_exist").to route_to(
+        'application#route_not_found',
+        namespace_id: 'gitlab',
+        project_id: 'not_exist'
+      )
+    end
+
+    it 'routes to 404 with post request' do
+      expect(post: "/gitlab/not_exist").to route_to(
+        'application#route_not_found',
+        namespace_id: 'gitlab',
+        project_id: 'not_exist'
+      )
+    end
+
+    it 'routes to 404 with put request' do
+      expect(put: "/gitlab/not_exist").to route_to(
+        'application#route_not_found',
+        namespace_id: 'gitlab',
+        project_id: 'not_exist'
+      )
+    end
+
+    context 'with route to some action' do
+      it 'routes to 404 with get request to' do
+        expect(get: "/gitlab/not_exist/some_action").to route_to(
+          'application#route_not_found',
+          unmatched_route: 'gitlab/not_exist/some_action'
+        )
+      end
+
+      it 'routes to 404 with delete request' do
+        expect(delete: "/gitlab/not_exist/some_action").to route_to(
+          'application#route_not_found',
+          namespace_id: 'gitlab',
+          project_id: 'not_exist',
+          all: 'some_action'
+        )
+      end
+
+      it 'routes to 404 with post request' do
+        expect(post: "/gitlab/not_exist/some_action").to route_to(
+          'application#route_not_found',
+          namespace_id: 'gitlab',
+          project_id: 'not_exist',
+          all: 'some_action'
+        )
+      end
+
+      it 'routes to 404 with put request' do
+        expect(put: "/gitlab/not_exist/some_action").to route_to(
+          'application#route_not_found',
+          namespace_id: 'gitlab',
+          project_id: 'not_exist',
+          all: 'some_action'
+        )
+      end
+    end
+  end
 end

spec/support/matchers/route_to_route_not_found_matcher.rb

+# frozen_string_literal: true
+
+RSpec::Matchers.define :route_to_route_not_found do
+  match do |actual|
+    expect(actual).to route_to(controller: 'application', action: 'route_not_found')
+  rescue RSpec::Expectations::ExpectationNotMetError => e
+    # `route_to` matcher requires providing all params for exact match. As we use it in shared examples and we provide different paths,
+    # this matcher checks if provided route matches controller and action, without checking params.
+    expect(e.message).to include("-{\"controller\"=>\"application\", \"action\"=>\"route_not_found\"}\n+{\"controller\"=>\"application\", \"action\"=>\"route_not_found\",")
+  end
+
+  failure_message do |_|
+    "expected #{actual} to route to route_not_found"
+  end
+end

spec/support/shared_examples/routing/git_http_routing_shared_examples.rb

@@ -16,10 +16,6 @@ RSpec.shared_examples 'git repository routes' do
       expect(get("#{container_path}/info/refs?service=git-upload-pack")).to redirect_to("#{container_path}.git/info/refs?service=git-upload-pack")
       expect(get("#{container_path}/info/refs?service=git-receive-pack")).to redirect_to("#{container_path}.git/info/refs?service=git-receive-pack")
     end
-
-    it 'does not redirect other requests' do
-      expect(post("#{container_path}/git-upload-pack")).not_to be_routable
-    end
   end
 
   it 'routes LFS endpoints' do
@@ -35,6 +31,56 @@ RSpec.shared_examples 'git repository routes' do
     expect(get("#{path}/gitlab-lfs/objects/#{oid}")).to route_to('repositories/lfs_storage#download', oid: oid, **params)
     expect(put("#{path}/gitlab-lfs/objects/#{oid}/456/authorize")).to route_to('repositories/lfs_storage#upload_authorize', oid: oid, size: '456', **params)
     expect(put("#{path}/gitlab-lfs/objects/#{oid}/456")).to route_to('repositories/lfs_storage#upload_finalize', oid: oid, size: '456', **params)
+  end
+end
+
+RSpec.shared_examples 'git repository routes without fallback' do
+  let(:container_path) { path.delete_suffix('.git') }
+
+  context 'requests without .git format' do
+    it 'does not redirect other requests' do
+      expect(post("#{container_path}/git-upload-pack")).not_to be_routable
+    end
+  end
+
+  it 'routes LFS endpoints for unmatched routes' do
+    oid = generate(:oid)
+
+    expect(put("#{path}/gitlab-lfs/objects/foo")).not_to be_routable
+    expect(put("#{path}/gitlab-lfs/objects/#{oid}/foo")).not_to be_routable
+    expect(put("#{path}/gitlab-lfs/objects/#{oid}/foo/authorize")).not_to be_routable
+  end
+end
+
+RSpec.shared_examples 'git repository routes with fallback' do
+  let(:container_path) { path.delete_suffix('.git') }
+
+  context 'requests without .git format' do
+    it 'does not redirect other requests' do
+      expect(post("#{container_path}/git-upload-pack")).to route_to_route_not_found
+    end
+  end
+
+  it 'routes LFS endpoints' do
+    oid = generate(:oid)
+
+    expect(put("#{path}/gitlab-lfs/objects/foo")).to route_to_route_not_found
+    expect(put("#{path}/gitlab-lfs/objects/#{oid}/foo")).to route_to_route_not_found
+    expect(put("#{path}/gitlab-lfs/objects/#{oid}/foo/authorize")).to route_to_route_not_found
+  end
+end
+
+RSpec.shared_examples 'git repository routes with fallback for git-upload-pack' do
+  let(:container_path) { path.delete_suffix('.git') }
+
+  context 'requests without .git format' do
+    it 'does not redirect other requests' do
+      expect(post("#{container_path}/git-upload-pack")).to route_to_route_not_found
+    end
+  end
+
+  it 'routes LFS endpoints for unmatched routes' do
+    oid = generate(:oid)
 
     expect(put("#{path}/gitlab-lfs/objects/foo")).not_to be_routable
     expect(put("#{path}/gitlab-lfs/objects/#{oid}/foo")).not_to be_routable