Avoid sending send-data headers in API response body

If processed incorrectly it may leak sensitive data
that is usually send via headers

Changelog: changed

lib/api/helpers.rb

@@ -681,20 +681,27 @@ module API
     def send_git_blob(repository, blob)
       env['api.format'] = :txt
       content_type 'text/plain'
+
       header['Content-Disposition'] = ActionDispatch::Http::ContentDisposition.format(disposition: 'inline', filename: blob.name)
 
       # Let Workhorse examine the content and determine the better content disposition
       header[Gitlab::Workhorse::DETECT_HEADER] = "true"
 
       header(*Gitlab::Workhorse.send_git_blob(repository, blob))
+
+      body ''
     end
 
     def send_git_archive(repository, **kwargs)
       header(*Gitlab::Workhorse.send_git_archive(repository, **kwargs))
+
+      body ''
     end
 
     def send_artifacts_entry(file, entry)
       header(*Gitlab::Workhorse.send_artifacts_entry(file, entry))
+
+      body ''
     end
 
     # The Grape Error Middleware only has access to `env` but not `params` nor

lib/api/helpers/project_snapshots_helpers.rb

@@ -11,6 +11,8 @@ module API
 
       def send_git_snapshot(repository)
         header(*Gitlab::Workhorse.send_git_snapshot(repository))
+
+        body ''
       end
 
       def snapshot_project

spec/lib/api/helpers_spec.rb

@@ -351,12 +351,14 @@ RSpec.describe API::Helpers do
 
     let(:send_git_blob) do
       subject.send(:send_git_blob, repository, blob)
+      subject.header
     end
 
     before do
       allow(subject).to receive(:env).and_return({})
       allow(subject).to receive(:content_type)
       allow(subject).to receive(:header).and_return({})
+      allow(subject).to receive(:body).and_return('')
       allow(Gitlab::Workhorse).to receive(:send_git_blob)
     end
 

spec/requests/api/ci/jobs_spec.rb

@@ -578,6 +578,7 @@ RSpec.describe API::Ci::Jobs do
           expect(response.headers.to_h)
             .to include('Content-Type' => 'application/json',
                         'Gitlab-Workhorse-Send-Data' => /artifacts-entry/)
+          expect(response.parsed_body).to be_empty
         end
 
         context 'when artifacts are locked' do
@@ -948,6 +949,7 @@ RSpec.describe API::Ci::Jobs do
           expect(response.headers.to_h)
             .to include('Content-Type' => 'application/json',
                         'Gitlab-Workhorse-Send-Data' => /artifacts-entry/)
+          expect(response.parsed_body).to be_empty
         end
       end
 

spec/requests/api/files_spec.rb

@@ -47,6 +47,15 @@ RSpec.describe API::Files do
     "/projects/#{project.id}/repository/files/#{file_path}"
   end
 
+  def expect_to_send_git_blob(url, params)
+    expect(Gitlab::Workhorse).to receive(:send_git_blob)
+
+    get url, params: params
+
+    expect(response).to have_gitlab_http_status(:ok)
+    expect(response.parsed_body).to be_empty
+  end
+
   context 'http headers' do
     it 'converts value into string' do
       helper.set_http_headers(test: 1)
@@ -257,11 +266,7 @@ RSpec.describe API::Files do
 
       it 'returns raw file info' do
         url = route(file_path) + "/raw"
-        expect(Gitlab::Workhorse).to receive(:send_git_blob)
-
-        get api(url, api_user, **options), params: params
-
-        expect(response).to have_gitlab_http_status(:ok)
+        expect_to_send_git_blob(api(url, api_user, **options), params)
         expect(headers[Gitlab::Workhorse::DETECT_HEADER]).to eq "true"
       end
 
@@ -523,11 +528,8 @@ RSpec.describe API::Files do
 
       it 'returns raw file info' do
         url = route(file_path) + "/raw"
-        expect(Gitlab::Workhorse).to receive(:send_git_blob)
 
-        get api(url, current_user), params: params
-
-        expect(response).to have_gitlab_http_status(:ok)
+        expect_to_send_git_blob(api(url, current_user), params)
       end
 
       context 'when ref is not provided' do
@@ -537,39 +539,29 @@ RSpec.describe API::Files do
 
         it 'returns response :ok', :aggregate_failures do
           url = route(file_path) + "/raw"
-          expect(Gitlab::Workhorse).to receive(:send_git_blob)
 
-          get api(url, current_user), params: {}
-
-          expect(response).to have_gitlab_http_status(:ok)
+          expect_to_send_git_blob(api(url, current_user), {})
         end
       end
 
       it 'returns raw file info for files with dots' do
         url = route('.gitignore') + "/raw"
-        expect(Gitlab::Workhorse).to receive(:send_git_blob)
 
-        get api(url, current_user), params: params
-
-        expect(response).to have_gitlab_http_status(:ok)
+        expect_to_send_git_blob(api(url, current_user), params)
       end
 
       it 'returns file by commit sha' do
         # This file is deleted on HEAD
         file_path = "files%2Fjs%2Fcommit%2Ejs%2Ecoffee"
         params[:ref] = "6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9"
-        expect(Gitlab::Workhorse).to receive(:send_git_blob)
 
-        get api(route(file_path) + "/raw", current_user), params: params
-
-        expect(response).to have_gitlab_http_status(:ok)
+        expect_to_send_git_blob(api(route(file_path) + "/raw", current_user), params)
       end
 
       it 'sets no-cache headers' do
         url = route('.gitignore') + "/raw"
-        expect(Gitlab::Workhorse).to receive(:send_git_blob)
 
-        get api(url, current_user), params: params
+        expect_to_send_git_blob(api(url, current_user), params)
 
         expect(response.headers["Cache-Control"]).to eq("max-age=0, private, must-revalidate, no-store, no-cache")
         expect(response.headers["Pragma"]).to eq("no-cache")
@@ -633,11 +625,9 @@ RSpec.describe API::Files do
         # This file is deleted on HEAD
         file_path = "files%2Fjs%2Fcommit%2Ejs%2Ecoffee"
         params[:ref] = "6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9"
-        expect(Gitlab::Workhorse).to receive(:send_git_blob)
+        url = api(route(file_path) + "/raw", personal_access_token: token)
 
-        get api(route(file_path) + "/raw", personal_access_token: token), params: params
-
-        expect(response).to have_gitlab_http_status(:ok)
+        expect_to_send_git_blob(url, params)
       end
     end
   end

spec/requests/api/project_snapshots_spec.rb

@@ -29,6 +29,7 @@ RSpec.describe API::ProjectSnapshots do
           repository: repository.gitaly_repository
         ).to_json
       )
+      expect(response.parsed_body).to be_empty
     end
 
     it 'returns authentication error as project owner' do

spec/requests/api/project_snippets_spec.rb

@@ -400,6 +400,7 @@ RSpec.describe API::ProjectSnippets do
 
       expect(response).to have_gitlab_http_status(:ok)
       expect(response.media_type).to eq 'text/plain'
+      expect(response.parsed_body).to be_empty
     end
 
     it 'returns 404 for invalid snippet id' do

spec/requests/api/repositories_spec.rb

@@ -197,6 +197,7 @@ RSpec.describe API::Repositories do
 
         expect(response).to have_gitlab_http_status(:ok)
         expect(headers[Gitlab::Workhorse::DETECT_HEADER]).to eq "true"
+        expect(response.parsed_body).to be_empty
       end
 
       it 'sets inline content disposition by default' do
@@ -274,6 +275,7 @@ RSpec.describe API::Repositories do
 
         expect(type).to eq('git-archive')
         expect(params['ArchivePath']).to match(/#{project.path}\-[^\.]+\.tar.gz/)
+        expect(response.parsed_body).to be_empty
       end
 
       it 'returns the repository archive archive.zip' do

spec/requests/api/snippets_spec.rb

@@ -113,6 +113,7 @@ RSpec.describe API::Snippets, factory_default: :keep do
       expect(response).to have_gitlab_http_status(:ok)
       expect(response.media_type).to eq 'text/plain'
       expect(headers['Content-Disposition']).to match(/^inline/)
+      expect(response.parsed_body).to be_empty
     end
 
     it 'returns 404 for invalid snippet id' do

spec/support/shared_examples/requests/snippet_shared_examples.rb

@@ -86,6 +86,7 @@ RSpec.shared_examples 'snippet blob content' do
 
     expect(response.header[Gitlab::Workhorse::DETECT_HEADER]).to eq 'true'
     expect(response.header[Gitlab::Workhorse::SEND_DATA_HEADER]).to start_with('git-blob:')
+    expect(response.parsed_body).to be_empty
   end
 
   context 'when snippet repository is empty' do