Merge branch 'sourgraph-csp-path' into 'master'

Use a more precise Sourcegraph URL in CSP

See merge request gitlab-org/gitlab!72552

app/controllers/concerns/sourcegraph_decorator.rb

@@ -11,7 +11,7 @@ module SourcegraphDecorator
       next unless Gitlab::CurrentSettings.sourcegraph_enabled
 
       default_connect_src = p.directives['connect-src'] || p.directives['default-src']
-      connect_src_values = Array.wrap(default_connect_src) | [Gitlab::CurrentSettings.sourcegraph_url]
+      connect_src_values = Array.wrap(default_connect_src) | [Gitlab::Utils.append_path(Gitlab::CurrentSettings.sourcegraph_url, '.api/')]
       p.connect_src(*connect_src_values)
     end
   end

lib/gitlab/content_security_policy/config_loader.rb

@@ -147,10 +147,7 @@ module Gitlab
       # Using 'self' in the CSP introduces several CSP bypass opportunities
       # for this reason we list the URLs where GitLab frames itself instead
       def self.allow_framed_gitlab_paths(directives)
-        # We need the version without trailing / for the sidekiq page itself
-        # and we also need the version with trailing / for "deeper" pages
-        # like /admin/sidekiq/busy
-        ['/admin/sidekiq', '/admin/sidekiq/', '/-/speedscope/index.html'].map do |path|
+        ['/admin/', '/assets/', '/-/speedscope/index.html'].map do |path|
           append_to_directive(directives, 'frame_src', Gitlab::Utils.append_path(Gitlab.config.gitlab.url, path))
         end
       end

spec/features/ide/clientside_preview_csp_spec.rb

@@ -12,7 +12,7 @@ RSpec.describe 'IDE Clientside Preview CSP' do
   end
 
   it_behaves_like 'setting CSP', 'frame-src' do
-    let(:whitelisted_url) { 'https://sandbox.gitlab-static.test' }
+    let(:allowlisted_url) { 'https://sandbox.gitlab-static.test' }
     let(:extended_controller_class) { IdeController }
 
     subject do
@@ -23,7 +23,7 @@ RSpec.describe 'IDE Clientside Preview CSP' do
 
     before do
       stub_application_setting(web_ide_clientside_preview_enabled: true)
-      stub_application_setting(web_ide_clientside_preview_bundler_url: whitelisted_url)
+      stub_application_setting(web_ide_clientside_preview_bundler_url: allowlisted_url)
 
       sign_in(user)
     end

spec/features/ide/static_object_external_storage_csp_spec.rb

@@ -12,7 +12,7 @@ RSpec.describe 'Static Object External Storage Content Security Policy' do
   end
 
   it_behaves_like 'setting CSP', 'connect-src' do
-    let_it_be(:whitelisted_url) { 'https://static-objects.test' }
+    let_it_be(:allowlisted_url) { 'https://static-objects.test' }
     let_it_be(:extended_controller_class) { IdeController }
 
     subject do
@@ -22,7 +22,7 @@ RSpec.describe 'Static Object External Storage Content Security Policy' do
     end
 
     before do
-      allow_any_instance_of(ApplicationSetting).to receive(:static_objects_external_storage_url).and_return(whitelisted_url)
+      allow_any_instance_of(ApplicationSetting).to receive(:static_objects_external_storage_url).and_return(allowlisted_url)
       allow_any_instance_of(ApplicationSetting).to receive(:static_objects_external_storage_auth_token).and_return('letmein')
 
       sign_in(user)

spec/features/projects/sourcegraph_csp_spec.rb

@@ -13,7 +13,8 @@ RSpec.describe 'Sourcegraph Content Security Policy' do
   end
 
   it_behaves_like 'setting CSP', 'connect-src' do
-    let_it_be(:whitelisted_url) { 'https://sourcegraph.test' }
+    let_it_be(:sourcegraph_url) { 'https://sourcegraph.test' }
+    let_it_be(:allowlisted_url) { "#{sourcegraph_url}/.api/" }
     let_it_be(:extended_controller_class) { Projects::BlobController }
 
     subject do
@@ -23,7 +24,7 @@ RSpec.describe 'Sourcegraph Content Security Policy' do
     end
 
     before do
-      allow(Gitlab::CurrentSettings).to receive(:sourcegraph_url).and_return(whitelisted_url)
+      allow(Gitlab::CurrentSettings).to receive(:sourcegraph_url).and_return(sourcegraph_url)
       allow(Gitlab::CurrentSettings).to receive(:sourcegraph_enabled).and_return(true)
 
       sign_in(user)

spec/lib/gitlab/content_security_policy/config_loader_spec.rb

@@ -85,7 +85,7 @@ RSpec.describe Gitlab::ContentSecurityPolicy::ConfigLoader do
         expect(directives['style_src']).to eq("'self' 'unsafe-inline' https://cdn.example.com")
         expect(directives['font_src']).to eq("'self' https://cdn.example.com")
         expect(directives['worker_src']).to eq('http://localhost/assets/ blob: data: https://cdn.example.com')
-        expect(directives['frame_src']).to eq(::Gitlab::ContentSecurityPolicy::Directives.frame_src + " https://cdn.example.com http://localhost/admin/sidekiq http://localhost/admin/sidekiq/ http://localhost/-/speedscope/index.html")
+        expect(directives['frame_src']).to eq(::Gitlab::ContentSecurityPolicy::Directives.frame_src + " https://cdn.example.com http://localhost/admin/ http://localhost/assets/ http://localhost/-/speedscope/index.html")
       end
     end
 
@@ -113,7 +113,7 @@ RSpec.describe Gitlab::ContentSecurityPolicy::ConfigLoader do
         end
 
         it 'does not add CUSTOMER_PORTAL_URL to CSP' do
-          expect(directives['frame_src']).to eq(::Gitlab::ContentSecurityPolicy::Directives.frame_src + " http://localhost/admin/sidekiq http://localhost/admin/sidekiq/ http://localhost/-/speedscope/index.html")
+          expect(directives['frame_src']).to eq(::Gitlab::ContentSecurityPolicy::Directives.frame_src + " http://localhost/admin/ http://localhost/assets/ http://localhost/-/speedscope/index.html")
         end
       end
 
@@ -123,7 +123,7 @@ RSpec.describe Gitlab::ContentSecurityPolicy::ConfigLoader do
         end
 
         it 'adds CUSTOMER_PORTAL_URL to CSP' do
-          expect(directives['frame_src']).to eq(::Gitlab::ContentSecurityPolicy::Directives.frame_src + " http://localhost/rails/letter_opener/ https://customers.example.com http://localhost/admin/sidekiq http://localhost/admin/sidekiq/ http://localhost/-/speedscope/index.html")
+          expect(directives['frame_src']).to eq(::Gitlab::ContentSecurityPolicy::Directives.frame_src + " http://localhost/rails/letter_opener/ https://customers.example.com http://localhost/admin/ http://localhost/assets/ http://localhost/-/speedscope/index.html")
         end
       end
     end

spec/support/shared_examples/csp.rb

@@ -28,7 +28,7 @@ RSpec.shared_examples 'setting CSP' do |rule_name|
 
     context 'when feature is enabled' do
       it "appends to #{rule_name}" do
-        is_expected.to eql("#{rule_name} #{default_csp_values} #{whitelisted_url}")
+        is_expected.to eql("#{rule_name} #{default_csp_values} #{allowlisted_url}")
       end
     end
 
@@ -46,7 +46,7 @@ RSpec.shared_examples 'setting CSP' do |rule_name|
 
     context 'when feature is enabled' do
       it "uses default-src values in #{rule_name}" do
-        is_expected.to eql("default-src #{default_csp_values}; #{rule_name} #{default_csp_values} #{whitelisted_url}")
+        is_expected.to eql("default-src #{default_csp_values}; #{rule_name} #{default_csp_values} #{allowlisted_url}")
       end
     end
 
@@ -64,7 +64,7 @@ RSpec.shared_examples 'setting CSP' do |rule_name|
 
     context 'when feature is enabled' do
       it "uses default-src values in #{rule_name}" do
-        is_expected.to eql("font-src #{default_csp_values}; #{rule_name} #{whitelisted_url}")
+        is_expected.to eql("font-src #{default_csp_values}; #{rule_name} #{allowlisted_url}")
       end
     end