Merge branch 'vij-invalid-path-error' into 'master'

Handle Snippet file name errors in backfill

See merge request gitlab-org/gitlab!30981

changelogs/unreleased/vij-invalid-path-error.yml

+---
+title: Handle Snippet file name errors in backfill
+merge_request: 30981
+author:
+type: fixed

lib/gitlab/background_migration/backfill_snippet_repositories.rb

@@ -15,12 +15,15 @@ module Gitlab
           next if repository_present?(snippet)
 
           retry_index = 0
+          @invalid_path_error = false
 
           begin
             create_repository_and_files(snippet)
 
             logger.info(message: 'Snippet Migration: repository created and migrated', snippet: snippet.id)
           rescue => e
+            set_file_path_error(e)
+
             retry_index += 1
 
             retry if retry_index < MAX_RETRIES
@@ -73,7 +76,10 @@ module Gitlab
       end
 
       def filename(snippet)
-        snippet.file_name.presence || empty_file_name
+        file_name = snippet.file_name
+        file_name = file_name.parameterize if @invalid_path_error
+
+        file_name.presence || empty_file_name
       end
 
       def empty_file_name
@@ -104,6 +110,15 @@ module Gitlab
       def admin_user
         @admin_user ||= User.admins.active.first
       end
+
+      # We sometimes receive invalid path errors from Gitaly if the Snippet filename
+      # cannot be parsed into a valid git path.
+      # In this situation, we need to parameterize the file name of the Snippet so that
+      # the migration can succeed, to achieve that, we'll identify in migration retries
+      # that the path is invalid
+      def set_file_path_error(error)
+        @invalid_path_error = error.message.downcase.start_with?('invalid path', 'path cannot include directory traversal')
+      end
     end
   end
 end

spec/lib/gitlab/background_migration/backfill_snippet_repositories_spec.rb

@@ -177,6 +177,48 @@ describe Gitlab::BackgroundMigration::BackfillSnippetRepositories, :migration, s
         end
       end
     end
+
+    context 'with invalid file names' do
+      using RSpec::Parameterized::TableSyntax
+
+      where(:invalid_file_name, :converted_file_name) do
+        'filename.js // with comment'       | 'filename-js-with-comment'
+        '.git/hooks/pre-commit'             | 'git-hooks-pre-commit'
+        'https://gitlab.com'                | 'https-gitlab-com'
+        'html://web.title%mp4/mpg/mpeg.net' | 'html-web-title-mp4-mpg-mpeg-net'
+        '../../etc/passwd'                  | 'etc-passwd'
+        '.'                                 | 'snippetfile1.txt'
+      end
+
+      with_them do
+        let!(:snippet_with_invalid_path) { snippets.create(id: 4, type: 'PersonalSnippet', author_id: user.id, file_name: invalid_file_name, content: content) }
+        let!(:snippet_with_valid_path) { snippets.create(id: 5, type: 'PersonalSnippet', author_id: user.id, file_name: file_name, content: content) }
+        let(:ids) { [4, 5] }
+
+        after do
+          raw_repository(snippet_with_invalid_path).remove
+          raw_repository(snippet_with_valid_path).remove
+        end
+
+        it 'checks for file path errors when errors are raised' do
+          expect(service).to receive(:set_file_path_error).once.and_call_original
+
+          subject
+        end
+
+        it 'converts invalid filenames' do
+          subject
+
+          expect(blob_at(snippet_with_invalid_path, converted_file_name)).to be
+        end
+
+        it 'does not convert valid filenames on subsequent migrations' do
+          subject
+
+          expect(blob_at(snippet_with_valid_path, file_name)).to be
+        end
+      end
+    end
   end
 
   def blob_at(snippet, path)