Do not index system notes for issue update

Since `system` notes have different permissions to normal notes on
issues we do not want to include them in the ElasticSearch index. The
reason being that the authorization logic we have in place for searching
for notes is only accounting for the non-system notes.

We have been under the impression that `system` notes have not been
getting indexed already because we were filtering them out in 2
different places already:

1. When new notes are created
https://gitlab.com/gitlab-org/gitlab/blob/2b607f0916c880a6c947ebfe4f4ee32f43fba551/ee/app/models/concerns/elastic/application_versioned_search.rb#L37
2. When indexing a whole project https://gitlab.com/gitlab-org/gitlab/blob/2b607f0916c880a6c947ebfe4f4ee32f43fba551/ee/app/models/concerns/elastic/projects_search.rb#L29

But there is a 3rd way in which notes get updated in the index which was
added in
https://gitlab.com/gitlab-org/gitlab/commit/db6a45ec17b2d165e85979dd7357e967d4346faa
. This is triggered when some auth related settings of an Issue are
updated as this changes who will be able to read notes in the issue so
it is updating all the notes for that issue. This third piece of logic
was now inserting all the system notes and should have been skipping
them like all other places where notes are added.

Also ignore Rubocop since this was like this before and we're fixing a
security vulnerability.

ee/app/services/elastic/index_record_service.rb

@@ -34,11 +34,13 @@ module Elastic
 
     private
 
+    # rubocop: disable CodeReuse/ActiveRecord
     def update_issue_notes(record, changed_fields)
       if changed_fields && (changed_fields & ISSUE_TRACKED_FIELDS).any?
-        import_association(Note, query: -> { where(noteable: record) })
+        import_association(Note, query: -> { searchable.where(noteable: record) })
       end
     end
+    # rubocop: enable CodeReuse/ActiveRecord
 
     def initial_index_project(project)
       # Enqueue the repository indexing jobs immediately so they run in parallel

ee/changelogs/unreleased/security-33660-comment-search-should-not-show-system-notes.yml

+---
+title: Do not index system notes for issue update
+merge_request:
+author:
+type: security

ee/spec/services/elastic/index_record_service_spec.rb

@@ -296,4 +296,50 @@ describe Elastic::IndexRecordService, :elastic do
 
     expect(Project.elastic_search('project_1').present?).to eq(false)
   end
+
+  context 'when updating an Issue' do
+    context 'when changing the confidential value' do
+      it 'updates issue notes excluding system notes' do
+        issue = nil
+        Sidekiq::Testing.disable! do
+          issue = create(:issue, confidential: false)
+          subject.execute(issue.project, true)
+          subject.execute(issue, false)
+          create(:note, note: 'the_normal_note', noteable: issue, project: issue.project)
+          create(:note, note: 'the_system_note', system: true, noteable: issue, project: issue.project)
+        end
+
+        options = { project_ids: [issue.project.id] }
+
+        Sidekiq::Testing.inline! do
+          expect(subject.execute(issue, false, 'changed_fields' => ['confidential'])).to eq(true)
+          Gitlab::Elastic::Helper.refresh_index
+        end
+
+        expect(Note.elastic_search('the_normal_note', options: options).present?).to eq(true)
+        expect(Note.elastic_search('the_system_note', options: options).present?).to eq(false)
+      end
+    end
+
+    context 'when changing the title' do
+      it 'does not update issue notes' do
+        issue = nil
+        Sidekiq::Testing.disable! do
+          issue = create(:issue, confidential: false)
+          subject.execute(issue.project, true)
+          subject.execute(issue, false)
+          create(:note, note: 'the_normal_note', noteable: issue, project: issue.project)
+        end
+
+        options = { project_ids: [issue.project.id] }
+
+        Sidekiq::Testing.inline! do
+          expect(subject.execute(issue, false, 'changed_fields' => ['title'])).to eq(true)
+          Gitlab::Elastic::Helper.refresh_index
+        end
+
+        expect(Note.elastic_search('the_normal_note', options: options).present?).to eq(false)
+      end
+    end
+  end
 end