Removing workhorse_set_content_type feature flag

Removing workhorse_set_content_type feature flag introduced in
https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/22667
parent ffef28cc
...@@ -140,36 +140,6 @@ module BlobHelper ...@@ -140,36 +140,6 @@ module BlobHelper
Gitlab::Sanitizers::SVG.clean(data) Gitlab::Sanitizers::SVG.clean(data)
end end
# Remove once https://gitlab.com/gitlab-org/gitlab-ce/issues/36103 is closed
# and :workhorse_set_content_type flag is removed
# If we blindly set the 'real' content type when serving a Git blob we
# are enabling XSS attacks. An attacker could upload e.g. a Javascript
# file to a Git repository, trick the browser of a victim into
# downloading the blob, and then the 'application/javascript' content
# type would tell the browser to execute the attacker's Javascript. By
# overriding the content type and setting it to 'text/plain' (in the
# example of Javascript) we tell the browser of the victim not to
# execute untrusted data.
def safe_content_type(blob)
if blob.extension == 'svg'
blob.mime_type
elsif blob.text?
'text/plain; charset=utf-8'
elsif blob.image?
blob.content_type
else
'application/octet-stream'
end
end
def content_disposition(blob, inline)
# Remove the following line when https://gitlab.com/gitlab-org/gitlab-ce/issues/36103
# is closed and :workhorse_set_content_type flag is removed
return 'attachment' if blob.extension == 'svg'
inline ? 'inline' : 'attachment'
end
def ref_project def ref_project
@ref_project ||= @target_project || @project @ref_project ||= @target_project || @project
end end
......
...@@ -7,8 +7,7 @@ module WorkhorseHelper ...@@ -7,8 +7,7 @@ module WorkhorseHelper
def send_git_blob(repository, blob, inline: true) def send_git_blob(repository, blob, inline: true)
headers.store(*Gitlab::Workhorse.send_git_blob(repository, blob)) headers.store(*Gitlab::Workhorse.send_git_blob(repository, blob))
headers['Content-Disposition'] = content_disposition(blob, inline) headers['Content-Disposition'] = inline ? 'inline' : 'attachment'
headers['Content-Type'] = safe_content_type(blob)
# If enabled, this will override the values set above # If enabled, this will override the values set above
workhorse_set_content_type! workhorse_set_content_type!
...@@ -47,6 +46,6 @@ module WorkhorseHelper ...@@ -47,6 +46,6 @@ module WorkhorseHelper
end end
def workhorse_set_content_type! def workhorse_set_content_type!
headers[Gitlab::Workhorse::DETECT_HEADER] = "true" if Feature.enabled?(:workhorse_set_content_type) headers[Gitlab::Workhorse::DETECT_HEADER] = "true"
end end
end end
...@@ -26,40 +26,16 @@ describe Projects::AvatarsController do ...@@ -26,40 +26,16 @@ describe Projects::AvatarsController do
context 'when the avatar is stored in the repository' do context 'when the avatar is stored in the repository' do
let(:filepath) { 'files/images/logo-white.png' } let(:filepath) { 'files/images/logo-white.png' }
context 'when feature flag workhorse_set_content_type is' do
before do
stub_feature_flags(workhorse_set_content_type: flag_value)
end
context 'enabled' do
let(:flag_value) { true }
it 'sends the avatar' do it 'sends the avatar' do
subject subject
expect(response).to have_gitlab_http_status(200) expect(response).to have_gitlab_http_status(200)
expect(response.header['Content-Disposition']).to eq('inline') expect(response.header['Content-Disposition']).to eq('inline')
expect(response.header['Content-Type']).to eq 'image/png'
expect(response.header[Gitlab::Workhorse::SEND_DATA_HEADER]).to start_with('git-blob:') expect(response.header[Gitlab::Workhorse::SEND_DATA_HEADER]).to start_with('git-blob:')
expect(response.header[Gitlab::Workhorse::DETECT_HEADER]).to eq "true" expect(response.header[Gitlab::Workhorse::DETECT_HEADER]).to eq "true"
end end
end end
context 'disabled' do
let(:flag_value) { false }
it 'sends the avatar' do
subject
expect(response).to have_gitlab_http_status(200)
expect(response.header['Content-Type']).to eq('image/png')
expect(response.header[Gitlab::Workhorse::SEND_DATA_HEADER]).to start_with('git-blob:')
expect(response.header[Gitlab::Workhorse::DETECT_HEADER]).to eq nil
end
end
end
end
context 'when the avatar is stored in lfs' do context 'when the avatar is stored in lfs' do
it_behaves_like 'repository lfs file load' do it_behaves_like 'repository lfs file load' do
let(:filename) { 'lfs_object.iso' } let(:filename) { 'lfs_object.iso' }
......
...@@ -892,14 +892,6 @@ describe Projects::JobsController, :clean_gitlab_redis_shared_state do ...@@ -892,14 +892,6 @@ describe Projects::JobsController, :clean_gitlab_redis_shared_state do
context "when job has a trace artifact" do context "when job has a trace artifact" do
let(:job) { create(:ci_build, :trace_artifact, pipeline: pipeline) } let(:job) { create(:ci_build, :trace_artifact, pipeline: pipeline) }
context 'when feature flag workhorse_set_content_type is' do
before do
stub_feature_flags(workhorse_set_content_type: flag_value)
end
context 'enabled' do
let(:flag_value) { true }
it "sets #{Gitlab::Workhorse::DETECT_HEADER} header" do it "sets #{Gitlab::Workhorse::DETECT_HEADER} header" do
response = subject response = subject
...@@ -910,21 +902,6 @@ describe Projects::JobsController, :clean_gitlab_redis_shared_state do ...@@ -910,21 +902,6 @@ describe Projects::JobsController, :clean_gitlab_redis_shared_state do
end end
end end
context 'disabled' do
let(:flag_value) { false }
it 'returns a trace' do
response = subject
expect(response).to have_gitlab_http_status(:ok)
expect(response.headers["Content-Type"]).to eq("text/plain; charset=utf-8")
expect(response.body).to eq(job.job_artifacts_trace.open.read)
expect(response.header[Gitlab::Workhorse::DETECT_HEADER]).to be nil
end
end
end
end
context "when job has a trace file" do context "when job has a trace file" do
let(:job) { create(:ci_build, :trace_live, pipeline: pipeline) } let(:job) { create(:ci_build, :trace_live, pipeline: pipeline) }
......
...@@ -16,17 +16,9 @@ describe Projects::RawController do ...@@ -16,17 +16,9 @@ describe Projects::RawController do
context 'regular filename' do context 'regular filename' do
let(:filepath) { 'master/README.md' } let(:filepath) { 'master/README.md' }
context 'when feature flag workhorse_set_content_type is' do it 'delivers ASCII file' do
before do
stub_feature_flags(workhorse_set_content_type: flag_value)
subject subject
end
context 'enabled' do
let(:flag_value) { true }
it 'delivers ASCII file' do
expect(response).to have_gitlab_http_status(200) expect(response).to have_gitlab_http_status(200)
expect(response.header['Content-Type']).to eq('text/plain; charset=utf-8') expect(response.header['Content-Type']).to eq('text/plain; charset=utf-8')
expect(response.header['Content-Disposition']).to eq('inline') expect(response.header['Content-Disposition']).to eq('inline')
...@@ -35,58 +27,19 @@ describe Projects::RawController do ...@@ -35,58 +27,19 @@ describe Projects::RawController do
end end
end end
context 'disabled' do
let(:flag_value) { false }
it 'delivers ASCII file' do
expect(response).to have_gitlab_http_status(200)
expect(response.header['Content-Type']).to eq('text/plain; charset=utf-8')
expect(response.header['Content-Disposition']).to eq('inline')
expect(response.header[Gitlab::Workhorse::DETECT_HEADER]).to eq nil
expect(response.header[Gitlab::Workhorse::SEND_DATA_HEADER]).to start_with('git-blob:')
end
end
end
end
context 'image header' do context 'image header' do
let(:filepath) { 'master/files/images/6049019_460s.jpg' } let(:filepath) { 'master/files/images/6049019_460s.jpg' }
context 'when feature flag workhorse_set_content_type is' do
before do
stub_feature_flags(workhorse_set_content_type: flag_value)
end
context 'enabled' do
let(:flag_value) { true }
it 'leaves image content disposition' do it 'leaves image content disposition' do
subject subject
expect(response).to have_gitlab_http_status(200) expect(response).to have_gitlab_http_status(200)
expect(response.header['Content-Type']).to eq('image/jpeg')
expect(response.header['Content-Disposition']).to eq('inline') expect(response.header['Content-Disposition']).to eq('inline')
expect(response.header[Gitlab::Workhorse::DETECT_HEADER]).to eq "true" 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.header[Gitlab::Workhorse::SEND_DATA_HEADER]).to start_with('git-blob:')
end end
end end
context 'disabled' do
let(:flag_value) { false }
it 'sets image content type header' do
subject
expect(response).to have_gitlab_http_status(200)
expect(response.header['Content-Type']).to eq('image/jpeg')
expect(response.header['Content-Disposition']).to eq('inline')
expect(response.header[Gitlab::Workhorse::DETECT_HEADER]).to eq nil
expect(response.header[Gitlab::Workhorse::SEND_DATA_HEADER]).to start_with('git-blob:')
end
end
end
end
it_behaves_like 'repository lfs file load' do it_behaves_like 'repository lfs file load' do
let(:filename) { 'lfs_object.iso' } let(:filename) { 'lfs_object.iso' }
let(:filepath) { "be93687/files/lfs/#{filename}" } let(:filepath) { "be93687/files/lfs/#{filename}" }
......
...@@ -52,23 +52,14 @@ describe Projects::WikisController do ...@@ -52,23 +52,14 @@ describe Projects::WikisController do
let(:path) { upload_file_to_wiki(project, user, file_name) } let(:path) { upload_file_to_wiki(project, user, file_name) }
subject { get :show, params: { namespace_id: project.namespace, project_id: project, id: path } }
context 'when file is an image' do
let(:file_name) { 'dk.png' }
context 'when feature flag workhorse_set_content_type is' do
before do before do
stub_feature_flags(workhorse_set_content_type: flag_value) get :show, params: { namespace_id: project.namespace, project_id: project, id: path }
subject
end end
context 'enabled' do context 'when file is an image' do
let(:flag_value) { true } let(:file_name) { 'dk.png' }
it 'delivers the image' do it 'delivers the image' do
expect(response.headers['Content-Type']).to eq('image/png')
expect(response.headers['Content-Disposition']).to match(/^inline/) expect(response.headers['Content-Disposition']).to match(/^inline/)
expect(response.headers[Gitlab::Workhorse::DETECT_HEADER]).to eq "true" expect(response.headers[Gitlab::Workhorse::DETECT_HEADER]).to eq "true"
end end
...@@ -77,31 +68,8 @@ describe Projects::WikisController do ...@@ -77,31 +68,8 @@ describe Projects::WikisController do
let(:file_name) { 'unsanitized.svg' } let(:file_name) { 'unsanitized.svg' }
it 'delivers the image' do it 'delivers the image' do
expect(response.headers['Content-Type']).to eq('image/svg+xml')
expect(response.headers['Content-Disposition']).to match(/^attachment/)
expect(response.headers[Gitlab::Workhorse::DETECT_HEADER]).to eq "true"
end
end
end
context 'disabled' do
let(:flag_value) { false }
it 'renders the content inline' do
expect(response.headers['Content-Type']).to eq('image/png')
expect(response.headers['Content-Disposition']).to match(/^inline/) expect(response.headers['Content-Disposition']).to match(/^inline/)
expect(response.headers[Gitlab::Workhorse::DETECT_HEADER]).to eq nil expect(response.headers[Gitlab::Workhorse::DETECT_HEADER]).to eq "true"
end
context 'when file is a svg' do
let(:file_name) { 'unsanitized.svg' }
it 'renders the content as an attachment' do
expect(response.headers['Content-Type']).to eq('image/svg+xml')
expect(response.headers['Content-Disposition']).to match(/^attachment/)
expect(response.headers[Gitlab::Workhorse::DETECT_HEADER]).to eq nil
end
end
end end
end end
end end
...@@ -109,34 +77,11 @@ describe Projects::WikisController do ...@@ -109,34 +77,11 @@ describe Projects::WikisController do
context 'when file is a pdf' do context 'when file is a pdf' do
let(:file_name) { 'git-cheat-sheet.pdf' } let(:file_name) { 'git-cheat-sheet.pdf' }
context 'when feature flag workhorse_set_content_type is' do
before do
stub_feature_flags(workhorse_set_content_type: flag_value)
subject
end
context 'enabled' do
let(:flag_value) { true }
it 'sets the content type to sets the content response headers' do it 'sets the content type to sets the content response headers' do
expect(response.headers['Content-Type']).to eq 'application/octet-stream'
expect(response.headers['Content-Disposition']).to match(/^inline/) expect(response.headers['Content-Disposition']).to match(/^inline/)
expect(response.headers[Gitlab::Workhorse::DETECT_HEADER]).to eq "true" expect(response.headers[Gitlab::Workhorse::DETECT_HEADER]).to eq "true"
end end
end end
context 'disabled' do
let(:flag_value) { false }
it 'sets the content response headers' do
expect(response.headers['Content-Type']).to eq 'application/octet-stream'
expect(response.headers['Content-Disposition']).to match(/^inline/)
expect(response.headers[Gitlab::Workhorse::DETECT_HEADER]).to eq nil
end
end
end
end
end end
end end
......
...@@ -437,10 +437,7 @@ describe SnippetsController do ...@@ -437,10 +437,7 @@ describe SnippetsController do
end end
context 'when signed in user is the author' do context 'when signed in user is the author' do
let(:flag_value) { false }
before do before do
stub_feature_flags(workhorse_set_content_type: flag_value)
get :raw, params: { id: personal_snippet.to_param } get :raw, params: { id: personal_snippet.to_param }
end end
...@@ -455,24 +452,11 @@ describe SnippetsController do ...@@ -455,24 +452,11 @@ describe SnippetsController do
expect(response.header['Content-Disposition']).to match(/inline/) expect(response.header['Content-Disposition']).to match(/inline/)
end end
context 'when feature flag workhorse_set_content_type is' do
context 'enabled' do
let(:flag_value) { true }
it "sets #{Gitlab::Workhorse::DETECT_HEADER} header" do it "sets #{Gitlab::Workhorse::DETECT_HEADER} header" do
expect(response).to have_gitlab_http_status(200) expect(response).to have_gitlab_http_status(200)
expect(response.header[Gitlab::Workhorse::DETECT_HEADER]).to eq "true" expect(response.header[Gitlab::Workhorse::DETECT_HEADER]).to eq "true"
end end
end end
context 'disabled' do
it "does not set #{Gitlab::Workhorse::DETECT_HEADER} header" do
expect(response).to have_gitlab_http_status(200)
expect(response.header[Gitlab::Workhorse::DETECT_HEADER]).to be nil
end
end
end
end
end end
context 'when not signed in' do context 'when not signed in' do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment