Add latest changes from gitlab-org/security/gitlab@15-4-stable-ee

10 files changed, 130 insertions, 8 deletions

diff --git a/app/assets/javascripts/error_tracking/components/error_tracking_list.vue b/app/assets/javascripts/error_tracking/components/error_tracking_list.vue
index a07428dafea..de4b11699fc 100644
--- a/app/assets/javascripts/error_tracking/components/error_tracking_list.vue
+++ b/app/assets/javascripts/error_tracking/components/error_tracking_list.vue
@@ -22,12 +22,16 @@ import AccessorUtils from '~/lib/utils/accessor';
 import { __ } from '~/locale';
 import Tracking from '~/tracking';
 import TimeAgo from '~/vue_shared/components/time_ago_tooltip.vue';
+import { sanitizeUrl } from '~/lib/utils/url_utility';
 import { trackErrorListViewsOptions, trackErrorStatusUpdateOptions } from '../utils';
 import { I18N_ERROR_TRACKING_LIST } from '../constants';
 import ErrorTrackingActions from './error_tracking_actions.vue';
 
 export const tableDataClass = 'table-col d-flex d-md-table-cell align-items-center';
 
+const isValidErrorId = (errorId) => {
+  return /^[0-9]+$/.test(errorId);
+};
 export default {
   FIRST_PAGE: 1,
   PREV_PAGE: 1,
@@ -202,6 +206,9 @@ export default {
       this.searchByQuery(text);
     },
     getDetailsLink(errorId) {
+      if (!isValidErrorId(errorId)) {
+        return 'about:blank';
+      }
       return `error_tracking/${errorId}/details`;
     },
     goToNextPage() {
@@ -222,7 +229,10 @@ export default {
       return filter === this.statusFilter;
     },
     getIssueUpdatePath(errorId) {
-      return `/${this.projectPath}/-/error_tracking/${errorId}.json`;
+      if (!isValidErrorId(errorId)) {
+        return 'about:blank';
+      }
+      return sanitizeUrl(`/${this.projectPath}/-/error_tracking/${errorId}.json`);
     },
     filterErrors(status, label) {
       this.filterValue = label;
diff --git a/lib/error_tracking/sentry_client.rb b/lib/error_tracking/sentry_client.rb
index 029389ab5d6..713cec7a7d6 100644
--- a/lib/error_tracking/sentry_client.rb
+++ b/lib/error_tracking/sentry_client.rb
@@ -10,6 +10,7 @@ module ErrorTracking
 
     Error = Class.new(StandardError)
     MissingKeysError = Class.new(StandardError)
+    InvalidFieldValueError = Class.new(StandardError)
     ResponseInvalidSizeError = Class.new(StandardError)
 
     RESPONSE_SIZE_LIMIT = 1.megabyte
@@ -110,5 +111,15 @@ module ErrorTracking
     def raise_error(message)
       raise SentryClient::Error, message
     end
+
+    def ensure_numeric!(field, value)
+      return value if /\A\d+\z/.match?(value)
+
+      raise_invalid_field_value!(field, "#{value.inspect} is not numeric")
+    end
+
+    def raise_invalid_field_value!(field, message)
+      raise InvalidFieldValueError, %(Sentry API response contains invalid value for field "#{field}": #{message})
+    end
   end
 end
diff --git a/lib/error_tracking/sentry_client/event.rb b/lib/error_tracking/sentry_client/event.rb
index 1db31abeeb2..d8ae81f5411 100644
--- a/lib/error_tracking/sentry_client/event.rb
+++ b/lib/error_tracking/sentry_client/event.rb
@@ -16,7 +16,7 @@ module ErrorTracking
 
         Gitlab::ErrorTracking::ErrorEvent.new(
           project_id: event['projectID'],
-          issue_id: event['groupID'],
+          issue_id: ensure_numeric!('issue_id', event['groupID']),
           date_received: event['dateReceived'],
           stack_trace_entries: stack_trace
         )
diff --git a/lib/error_tracking/sentry_client/issue.rb b/lib/error_tracking/sentry_client/issue.rb
index 3c846eb0635..5e2e0787a3f 100644
--- a/lib/error_tracking/sentry_client/issue.rb
+++ b/lib/error_tracking/sentry_client/issue.rb
@@ -114,8 +114,10 @@ module ErrorTracking
       end
 
       def map_to_error(issue)
+        id = ensure_numeric!('id', issue.fetch('id'))
+
         Gitlab::ErrorTracking::Error.new(
-          id: issue.fetch('id'),
+          id: id,
           first_seen: issue.fetch('firstSeen', nil),
           last_seen: issue.fetch('lastSeen', nil),
           title: issue.fetch('title', nil),
@@ -124,7 +126,7 @@ module ErrorTracking
           count: issue.fetch('count', nil),
           message: issue.dig('metadata', 'value'),
           culprit: issue.fetch('culprit', nil),
-          external_url: issue_url(issue.fetch('id')),
+          external_url: issue_url(id),
           short_id: issue.fetch('shortId', nil),
           status: issue.fetch('status', nil),
           frequency: issue.dig('stats', '24h'),
@@ -135,8 +137,10 @@ module ErrorTracking
       end
 
       def map_to_detailed_error(issue)
+        id = ensure_numeric!('id', issue.fetch('id'))
+
         Gitlab::ErrorTracking::DetailedError.new(
-          id: issue.fetch('id'),
+          id: id,
           first_seen: issue.fetch('firstSeen', nil),
           last_seen: issue.fetch('lastSeen', nil),
           tags: extract_tags(issue),
@@ -146,7 +150,7 @@ module ErrorTracking
           count: issue.fetch('count', nil),
           message: issue.dig('metadata', 'value'),
           culprit: issue.fetch('culprit', nil),
-          external_url: issue_url(issue.fetch('id')),
+          external_url: issue_url(id),
           external_base_url: project_url,
           short_id: issue.fetch('shortId', nil),
           status: issue.fetch('status', nil),
diff --git a/lib/gitlab/checks/tag_check.rb b/lib/gitlab/checks/tag_check.rb
index 5dd7720b67d..007a775eaf5 100644
--- a/lib/gitlab/checks/tag_check.rb
+++ b/lib/gitlab/checks/tag_check.rb
@@ -9,11 +9,13 @@ module Gitlab
         delete_protected_tag: 'You are not allowed to delete protected tags from this project. '\
           'Only a project maintainer or owner can delete a protected tag.',
         delete_protected_tag_non_web: 'You can only delete protected tags using the web interface.',
-        create_protected_tag: 'You are not allowed to create this tag as it is protected.'
+        create_protected_tag: 'You are not allowed to create this tag as it is protected.',
+        default_branch_collision: 'You cannot use default branch name to create a tag'
       }.freeze
 
       LOG_MESSAGES = {
         tag_checks: "Checking if you are allowed to change existing tags...",
+        default_branch_collision_check: "Checking if you are providing a valid tag name...",
         protected_tag_checks: "Checking if you are creating, updating or deleting a protected tag..."
       }.freeze
 
@@ -26,6 +28,7 @@ module Gitlab
           end
         end
 
+        default_branch_collision_check
         protected_tag_checks
       end
 
@@ -52,6 +55,14 @@ module Gitlab
           end
         end
       end
+
+      def default_branch_collision_check
+        logger.log_timed(LOG_MESSAGES[:default_branch_collision_check]) do
+          if creation? && tag_name == project.default_branch
+            raise GitAccess::ForbiddenError, ERROR_MESSAGES[:default_branch_collision]
+          end
+        end
+      end
     end
   end
 end
diff --git a/spec/frontend/error_tracking/components/error_tracking_list_spec.js b/spec/frontend/error_tracking/components/error_tracking_list_spec.js
index 805ada54509..adb2eaaf04e 100644
--- a/spec/frontend/error_tracking/components/error_tracking_list_spec.js
+++ b/spec/frontend/error_tracking/components/error_tracking_list_spec.js
@@ -314,6 +314,43 @@ describe('ErrorTrackingList', () => {
     });
   });
 
+  describe('when the resolve button is clicked with non numberic error id', () => {
+    beforeEach(() => {
+      store.state.list.loading = false;
+      store.state.list.errors = [
+        {
+          id: 'abc',
+          title: 'PG::ConnectionBad: FATAL',
+          type: 'error',
+          userCount: 0,
+          count: '53',
+          firstSeen: '2019-05-30T07:21:46Z',
+          lastSeen: '2019-11-06T03:21:39Z',
+          status: 'unresolved',
+        },
+      ];
+
+      mountComponent({
+        stubs: {
+          GlTable: false,
+          GlLink: false,
+        },
+      });
+    });
+
+    it('should show about:blank link', () => {
+      findErrorActions().vm.$emit('update-issue-status', {
+        errorId: 'abc',
+        status: 'resolved',
+      });
+
+      expect(actions.updateStatus).toHaveBeenCalledWith(expect.anything(), {
+        endpoint: 'about:blank',
+        status: 'resolved',
+      });
+    });
+  });
+
   describe('When error tracking is disabled and user is not allowed to enable it', () => {
     beforeEach(() => {
       mountComponent({
diff --git a/spec/lib/error_tracking/sentry_client/event_spec.rb b/spec/lib/error_tracking/sentry_client/event_spec.rb
index d65bfa31018..e7a9db771b6 100644
--- a/spec/lib/error_tracking/sentry_client/event_spec.rb
+++ b/spec/lib/error_tracking/sentry_client/event_spec.rb
@@ -72,5 +72,13 @@ RSpec.describe ErrorTracking::SentryClient do
         end
       end
     end
+
+    it_behaves_like 'non-numeric input handling in Sentry response', 'issue_id' do
+      let(:sentry_api_response) do
+        sample_response.tap do |event|
+          event[:groupID] = id_input
+        end
+      end
+    end
   end
 end
diff --git a/spec/lib/error_tracking/sentry_client/issue_spec.rb b/spec/lib/error_tracking/sentry_client/issue_spec.rb
index 1468a1ff7eb..ac6a4b9e8cd 100644
--- a/spec/lib/error_tracking/sentry_client/issue_spec.rb
+++ b/spec/lib/error_tracking/sentry_client/issue_spec.rb
@@ -199,6 +199,15 @@ RSpec.describe ErrorTracking::SentryClient::Issue do
       it_behaves_like 'issues have correct return type', Gitlab::ErrorTracking::Error
       it_behaves_like 'issues have correct length', 3
     end
+
+    it_behaves_like 'non-numeric input handling in Sentry response', 'id' do
+      let(:sentry_api_response) do
+        issues_sample_response.first(1).map do |issue|
+          issue[:id] = id_input
+          issue
+        end
+      end
+    end
   end
 
   describe '#issue_details' do
@@ -208,8 +217,8 @@ RSpec.describe ErrorTracking::SentryClient::Issue do
       )
     end
 
-    let(:sentry_request_url) { "#{sentry_url}/issues/#{issue_id}/" }
     let(:sentry_api_response) { issue_sample_response }
+    let(:sentry_request_url) { "#{sentry_url}/issues/#{issue_id}/" }
     let!(:sentry_api_request) { stub_sentry_request(sentry_request_url, body: sentry_api_response) }
 
     subject { client.issue_details(issue_id: issue_id) }
@@ -298,6 +307,14 @@ RSpec.describe ErrorTracking::SentryClient::Issue do
         expect(subject.tags).to eq({ level: issue_sample_response['level'], logger: issue_sample_response['logger'] })
       end
     end
+
+    it_behaves_like 'non-numeric input handling in Sentry response', 'id' do
+      let(:sentry_api_response) do
+        issue_sample_response.tap do |issue|
+          issue[:id] = id_input
+        end
+      end
+    end
   end
 
   describe '#update_issue' do
diff --git a/spec/lib/gitlab/checks/tag_check_spec.rb b/spec/lib/gitlab/checks/tag_check_spec.rb
index 6cd3a2d1c07..50ffa5fad10 100644
--- a/spec/lib/gitlab/checks/tag_check_spec.rb
+++ b/spec/lib/gitlab/checks/tag_check_spec.rb
@@ -81,6 +81,14 @@ RSpec.describe Gitlab::Checks::TagCheck do
           it 'allows tag creation' do
             expect { subject.validate! }.not_to raise_error
           end
+
+          context 'when tag name is the same as default branch' do
+            let(:ref) { "refs/tags/#{project.default_branch}" }
+
+            it 'is prevented' do
+              expect { subject.validate! }.to raise_error(Gitlab::GitAccess::ForbiddenError, /cannot use default branch name to create a tag/)
+            end
+          end
         end
       end
     end
diff --git a/spec/support/shared_examples/lib/sentry/client_shared_examples.rb b/spec/support/shared_examples/lib/sentry/client_shared_examples.rb
index 1c0e0061385..71b32005c55 100644
--- a/spec/support/shared_examples/lib/sentry/client_shared_examples.rb
+++ b/spec/support/shared_examples/lib/sentry/client_shared_examples.rb
@@ -59,6 +59,22 @@ RSpec.shared_examples 'maps Sentry exceptions' do |http_method|
   end
 end
 
+RSpec.shared_examples 'non-numeric input handling in Sentry response' do |field|
+  context 'with non-numeric error id' do
+    where(:id_input) do
+      ['string', '-1', '1\n2']
+    end
+
+    with_them do
+      it 'raises exception' do
+        message = %(Sentry API response contains invalid value for field "#{field}": #{id_input.inspect} is not numeric)
+
+        expect { subject }.to raise_error(ErrorTracking::SentryClient::InvalidFieldValueError, message)
+      end
+    end
+  end
+end
+
 # Expects to following variables:
 #   - subject
 #   - sentry_api_response