Commit 77909a88 authored by Douwe Maan's avatar Douwe Maan

Merge branch 'fj-remove-workhorse-set-content-type-feature-flag' into 'master'

Removing workhorse_set_content_type feature flag

See merge request gitlab-org/gitlab-ce!23930
parents d0848c08 33e59556
......@@ -140,36 +140,6 @@ module BlobHelper
Gitlab::Sanitizers::SVG.clean(data)
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
@ref_project ||= @target_project || @project
end
......
......@@ -7,8 +7,7 @@ module WorkhorseHelper
def send_git_blob(repository, blob, inline: true)
headers.store(*Gitlab::Workhorse.send_git_blob(repository, blob))
headers['Content-Disposition'] = content_disposition(blob, inline)
headers['Content-Type'] = safe_content_type(blob)
headers['Content-Disposition'] = inline ? 'inline' : 'attachment'
# If enabled, this will override the values set above
workhorse_set_content_type!
......@@ -47,6 +46,6 @@ module WorkhorseHelper
end
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
......@@ -26,37 +26,13 @@ describe Projects::AvatarsController do
context 'when the avatar is stored in the repository' do
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
it 'sends the avatar' do
subject
context 'enabled' do
let(:flag_value) { true }
it 'sends the avatar' do
subject
expect(response).to have_gitlab_http_status(200)
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::DETECT_HEADER]).to eq "true"
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
expect(response).to have_gitlab_http_status(200)
expect(response.header['Content-Disposition']).to eq('inline')
expect(response.header[Gitlab::Workhorse::SEND_DATA_HEADER]).to start_with('git-blob:')
expect(response.header[Gitlab::Workhorse::DETECT_HEADER]).to eq "true"
end
end
......
......@@ -892,36 +892,13 @@ describe Projects::JobsController, :clean_gitlab_redis_shared_state do
context "when job has a trace artifact" do
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
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 eq "true"
end
end
context 'disabled' do
let(:flag_value) { false }
it 'returns a trace' do
response = subject
it "sets #{Gitlab::Workhorse::DETECT_HEADER} header" 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
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 eq "true"
end
end
......
......@@ -16,74 +16,27 @@ describe Projects::RawController do
context 'regular filename' do
let(:filepath) { 'master/README.md' }
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 '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 "true"
expect(response.header[Gitlab::Workhorse::SEND_DATA_HEADER]).to start_with('git-blob:')
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
it 'delivers ASCII file' do
subject
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 "true"
expect(response.header[Gitlab::Workhorse::SEND_DATA_HEADER]).to start_with('git-blob:')
end
end
context 'image header' do
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
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 "true"
expect(response.header[Gitlab::Workhorse::SEND_DATA_HEADER]).to start_with('git-blob:')
end
end
context 'disabled' do
let(:flag_value) { false }
it 'sets image content type header' do
subject
it 'leaves image content disposition' 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
expect(response).to have_gitlab_http_status(200)
expect(response.header['Content-Disposition']).to eq('inline')
expect(response.header[Gitlab::Workhorse::DETECT_HEADER]).to eq "true"
expect(response.header[Gitlab::Workhorse::SEND_DATA_HEADER]).to start_with('git-blob:')
end
end
......
......@@ -52,56 +52,24 @@ describe Projects::WikisController do
let(:path) { upload_file_to_wiki(project, user, file_name) }
subject { get :show, params: { namespace_id: project.namespace, project_id: project, id: path } }
before do
get :show, params: { namespace_id: project.namespace, project_id: project, id: path }
end
context 'when file is an image' do
let(:file_name) { 'dk.png' }
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 'delivers the image' do
expect(response.headers['Content-Type']).to eq('image/png')
expect(response.headers['Content-Disposition']).to match(/^inline/)
expect(response.headers[Gitlab::Workhorse::DETECT_HEADER]).to eq "true"
end
context 'when file is a svg' do
let(:file_name) { 'unsanitized.svg' }
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[Gitlab::Workhorse::DETECT_HEADER]).to eq nil
end
it 'delivers the image' do
expect(response.headers['Content-Disposition']).to match(/^inline/)
expect(response.headers[Gitlab::Workhorse::DETECT_HEADER]).to eq "true"
end
context 'when file is a svg' do
let(:file_name) { 'unsanitized.svg' }
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
it 'delivers the image' do
expect(response.headers['Content-Disposition']).to match(/^inline/)
expect(response.headers[Gitlab::Workhorse::DETECT_HEADER]).to eq "true"
end
end
end
......@@ -109,32 +77,9 @@ describe Projects::WikisController do
context 'when file is a pdf' do
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
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 "true"
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
it 'sets the content type to sets the content response headers' do
expect(response.headers['Content-Disposition']).to match(/^inline/)
expect(response.headers[Gitlab::Workhorse::DETECT_HEADER]).to eq "true"
end
end
end
......
......@@ -437,10 +437,7 @@ describe SnippetsController do
end
context 'when signed in user is the author' do
let(:flag_value) { false }
before do
stub_feature_flags(workhorse_set_content_type: flag_value)
get :raw, params: { id: personal_snippet.to_param }
end
......@@ -455,22 +452,9 @@ describe SnippetsController do
expect(response.header['Content-Disposition']).to match(/inline/)
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
expect(response).to have_gitlab_http_status(200)
expect(response.header[Gitlab::Workhorse::DETECT_HEADER]).to eq "true"
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
it "sets #{Gitlab::Workhorse::DETECT_HEADER} header" do
expect(response).to have_gitlab_http_status(200)
expect(response.header[Gitlab::Workhorse::DETECT_HEADER]).to eq "true"
end
end
end
......
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