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

8 files changed, 111 insertions, 7 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 6a341ddbe86..67c57a07988 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)
 
     attr_accessor :url, :token
 
@@ -92,5 +93,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 d0e6bd783f3..18a686df4f2 100644
--- a/lib/error_tracking/sentry_client/issue.rb
+++ b/lib/error_tracking/sentry_client/issue.rb
@@ -120,8 +120,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),
@@ -130,7 +132,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'),
@@ -141,8 +143,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),
@@ -152,7 +156,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/spec/frontend/error_tracking/components/error_tracking_list_spec.js b/spec/frontend/error_tracking/components/error_tracking_list_spec.js
index b7dffbbec04..a3047752822 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 64e674f1e9b..9107920219c 100644
--- a/spec/lib/error_tracking/sentry_client/event_spec.rb
+++ b/spec/lib/error_tracking/sentry_client/event_spec.rb
@@ -71,5 +71,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 d7bb0ca5c9a..e2135dc12cd 100644
--- a/spec/lib/error_tracking/sentry_client/issue_spec.rb
+++ b/spec/lib/error_tracking/sentry_client/issue_spec.rb
@@ -209,6 +209,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
@@ -218,8 +227,9 @@ RSpec.describe ErrorTracking::SentryClient::Issue do
       )
     end
 
+    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: issue_sample_response) }
+    let!(:sentry_api_request) { stub_sentry_request(sentry_request_url, body: sentry_api_response) }
 
     subject { client.issue_details(issue_id: issue_id) }
 
@@ -304,6 +314,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/support/shared_examples/lib/sentry/client_shared_examples.rb b/spec/support/shared_examples/lib/sentry/client_shared_examples.rb
index d73c7b6848d..2712dce31e9 100644
--- a/spec/support/shared_examples/lib/sentry/client_shared_examples.rb
+++ b/spec/support/shared_examples/lib/sentry/client_shared_examples.rb
@@ -58,3 +58,19 @@ RSpec.shared_examples 'maps Sentry exceptions' do |http_method|
     end
   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