Avoid exposing unaccessible repo data upon GFM processing

When post-processing relative links to absolute links
RelativeLinkFilter didn't take into consideration that
internal repository data could be exposed for users
that do not have repository access to the project.

This commit solves that by checking whether the user
can `download_code` at this repository, avoiding any
processing of this filter if the user can't.

Additionally, if we're processing for a group (
no project was given), we check if the user can
read it in order to expand the href as an extra.
That doesn't seem necessarily a breach now,
but an extra check doesn't hurt as after all
the user needs to be able to `read_group`.

1 files changed, 70 insertions, 2 deletions

diff --git a/spec/lib/banzai/filter/relative_link_filter_spec.rb b/spec/lib/banzai/filter/relative_link_filter_spec.rb
index ecb83b6cb66..789530fbc56 100644
--- a/spec/lib/banzai/filter/relative_link_filter_spec.rb
+++ b/spec/lib/banzai/filter/relative_link_filter_spec.rb
@@ -7,6 +7,7 @@ describe Banzai::Filter::RelativeLinkFilter do
     contexts.reverse_merge!({
       commit:         commit,
       project:        project,
+      current_user:   user,
       group:          group,
       project_wiki:   project_wiki,
       ref:            ref,
@@ -33,7 +34,8 @@ describe Banzai::Filter::RelativeLinkFilter do
     %(<div>#{element}</div>)
   end
 
-  let(:project)        { create(:project, :repository) }
+  let(:project)        { create(:project, :repository, :public) }
+  let(:user)           { create(:user) }
   let(:group)          { nil }
   let(:project_path)   { project.full_path }
   let(:ref)            { 'markdown' }
@@ -75,6 +77,11 @@ describe Banzai::Filter::RelativeLinkFilter do
     include_examples :preserve_unchanged
   end
 
+  context 'without project repository access' do
+    let(:project) { create(:project, :repository, repository_access_level: ProjectFeature::PRIVATE) }
+    include_examples :preserve_unchanged
+  end
+
   it 'does not raise an exception on invalid URIs' do
     act = link("://foo")
     expect { filter(act) }.not_to raise_error
@@ -282,6 +289,37 @@ describe Banzai::Filter::RelativeLinkFilter do
     let(:relative_path) { "/#{project.full_path}#{upload_path}" }
 
     context 'to a project upload' do
+      context 'without project repository access' do
+        let(:project) { create(:project, :repository, repository_access_level: ProjectFeature::PRIVATE) }
+
+        it 'does not rebuild relative URL for a link' do
+          doc = filter(link(upload_path))
+          expect(doc.at_css('a')['href']).to eq(upload_path)
+
+          doc = filter(nested(link(upload_path)))
+          expect(doc.at_css('a')['href']).to eq(upload_path)
+        end
+
+        it 'does not rebuild relative URL for an image' do
+          doc = filter(image(upload_path))
+          expect(doc.at_css('img')['src']).to eq(upload_path)
+
+          doc = filter(nested(image(upload_path)))
+          expect(doc.at_css('img')['src']).to eq(upload_path)
+        end
+
+        context 'with an absolute URL' do
+          let(:absolute_path) { Gitlab.config.gitlab.url + relative_path }
+          let(:only_path) { false }
+
+          it 'does not rewrite the link' do
+            doc = filter(link(upload_path))
+
+            expect(doc.at_css('a')['href']).to eq(upload_path)
+          end
+        end
+      end
+
       context 'with an absolute URL' do
         let(:absolute_path) { Gitlab.config.gitlab.url + relative_path }
         let(:only_path) { false }
@@ -331,11 +369,41 @@ describe Banzai::Filter::RelativeLinkFilter do
     end
 
     context 'to a group upload' do
-      let(:upload_link) { link('/uploads/e90decf88d8f96fe9e1389afc2e4a91f/test.jpg') }
+      let(:upload_path) { '/uploads/e90decf88d8f96fe9e1389afc2e4a91f/test.jpg' }
+      let(:upload_link) { link(upload_path) }
       let(:group) { create(:group) }
       let(:project) { nil }
       let(:relative_path) { "/groups/#{group.full_path}/-/uploads/e90decf88d8f96fe9e1389afc2e4a91f/test.jpg" }
 
+      context 'without group read access' do
+        let(:group) { create(:group, :private) }
+
+        it 'does not rewrite the link' do
+          doc = filter(upload_link)
+
+          expect(doc.at_css('a')['href']).to eq(upload_path)
+        end
+
+        it 'does not rewrite the link for subgroup' do
+          group.update!(parent: create(:group))
+
+          doc = filter(upload_link)
+
+          expect(doc.at_css('a')['href']).to eq(upload_path)
+        end
+
+        context 'with an absolute URL' do
+          let(:absolute_path) { Gitlab.config.gitlab.url + relative_path }
+          let(:only_path) { false }
+
+          it 'does not rewrite the link' do
+            doc = filter(upload_link)
+
+            expect(doc.at_css('a')['href']).to eq(upload_path)
+          end
+        end
+      end
+
       context 'with an absolute URL' do
         let(:absolute_path) { Gitlab.config.gitlab.url + relative_path }
         let(:only_path) { false }