From aaa205b070722cdad97c5c95811fbe7c885900d5 Mon Sep 17 00:00:00 2001 From: Dave Pisek Date: Fri, 25 Oct 2024 09:02:31 +0200 Subject: [PATCH 1/3] Add disclaimer comment to AI-MR comment This commit adds a disclaimer text to the suggestion that gets generated when a security finding is resolved via AI on the MR-widget. --- .../create_from_vulnerability_data_service.rb | 14 ++++++++++++- ...te_from_vulnerability_data_service_spec.rb | 20 +++++++++++++++++++ lib/gitlab/diff/merge_request_suggestion.rb | 7 +++++-- .../diff/merge_request_suggestion_spec.rb | 15 ++++++++++++-- 4 files changed, 51 insertions(+), 5 deletions(-) diff --git a/ee/app/services/merge_requests/create_from_vulnerability_data_service.rb b/ee/app/services/merge_requests/create_from_vulnerability_data_service.rb index 63e25295529ae3..c73a4d5adb5f57 100644 --- a/ee/app/services/merge_requests/create_from_vulnerability_data_service.rb +++ b/ee/app/services/merge_requests/create_from_vulnerability_data_service.rb @@ -9,6 +9,18 @@ class CreateFromVulnerabilityDataService < ::BaseService START_NEW_CODE = '' END_NEW_CODE = '' + VULNERABILITY_RESOLUTION_DOCS_LINK = Gitlab::Routing.url_helpers.help_page_path('user/application_security/vulnerabilities/index.md', anchor: 'vulnerability-resolution') + + SUGGESTION_DISCLAIMER = <<~MARKDOWN.freeze + The suggested code changes were generated by GitLab Duo Vulnerability Resolution, an AI feature. + **Use this feature with caution.** Before you apply the code changes, carefully review and test them, to ensure that they + solve the vulnerability. + + The large language model that generated the suggested code changes was provided with the entire file that contains the + vulnerable lines of code. It is not aware of any functionality outside of this context. Please see our [documentation](#{VULNERABILITY_RESOLUTION_DOCS_LINK}) for more + information about this feature. + MARKDOWN + def initialize(project, vulnerability, user = nil, params = {}) super(project, user, params) @@ -86,7 +98,7 @@ def suggestion_merge_request def mr_suggestion_note_attributes_hash(diff) # convert a diff into a suggestion in the form of a note_attributes_hash - suggestion = Gitlab::Diff::MergeRequestSuggestion.new(diff, finding_presenter.file, suggestion_merge_request) + suggestion = Gitlab::Diff::MergeRequestSuggestion.new(diff, finding_presenter.file, suggestion_merge_request, SUGGESTION_DISCLAIMER) suggestion.note_attributes_hash end diff --git a/ee/spec/services/merge_requests/create_from_vulnerability_data_service_spec.rb b/ee/spec/services/merge_requests/create_from_vulnerability_data_service_spec.rb index 060c30b27ace8e..fefdf0af894ccc 100644 --- a/ee/spec/services/merge_requests/create_from_vulnerability_data_service_spec.rb +++ b/ee/spec/services/merge_requests/create_from_vulnerability_data_service_spec.rb @@ -16,6 +16,18 @@ SOURCE end + let_it_be(:suggestion_disclaimer) do + <<~MARKDOWN + The suggested code changes were generated by GitLab Duo Vulnerability Resolution, an AI feature. + **Use this feature with caution.** Before you apply the code changes, carefully review and test them, to ensure that they + solve the vulnerability. + + The large language model that generated the suggested code changes was provided with the entire file that contains the + vulnerable lines of code. It is not aware of any functionality outside of this context. Please see our [documentation](/help/user/application_security/vulnerabilities/index.md#vulnerability-resolution) for more + information about this feature. + MARKDOWN + end + let_it_be(:expected_source_branch) { 'remediate/authentication-bypass-via-incorrect-dom-traversal-and-canonical' } let(:service) { described_class.new(project, vulnerability, user, params) } @@ -588,6 +600,14 @@ end end + it 'requests that a suggestion be created with the correct disclaimer-text' do + result + + expect(Gitlab::Diff::MergeRequestSuggestion).to have_received(:new).with( + any_args, suggestion_disclaimer + ) + end + it 'requests that a note be created with the extracted patch' do result diff --git a/lib/gitlab/diff/merge_request_suggestion.rb b/lib/gitlab/diff/merge_request_suggestion.rb index afbe530febbf21..8a90af27e81adf 100644 --- a/lib/gitlab/diff/merge_request_suggestion.rb +++ b/lib/gitlab/diff/merge_request_suggestion.rb @@ -10,11 +10,12 @@ class MergeRequestSuggestion SUGGESTION_HEADER = "```suggestion:" SUGGESTION_FOOTER = "```" - def initialize(diff, path, merge_request) + def initialize(diff, path, merge_request, prepend_text = nil) @diff = diff @path = path @merge_request = merge_request @project = merge_request.project + @prepend_text = prepend_text end def note_attributes_hash @@ -100,7 +101,9 @@ def suggestion_meta end def suggestion - array = [SUGGESTION_HEADER + suggestion_meta] + array = [] + array << @prepend_text if @prepend_text + array << [SUGGESTION_HEADER + suggestion_meta] diff_lines.each do |line| array << line.text(prefix: false) if line.added? || line.unchanged? diff --git a/spec/lib/gitlab/diff/merge_request_suggestion_spec.rb b/spec/lib/gitlab/diff/merge_request_suggestion_spec.rb index f9f6a3dba8d416..38e38ac9d849d7 100644 --- a/spec/lib/gitlab/diff/merge_request_suggestion_spec.rb +++ b/spec/lib/gitlab/diff/merge_request_suggestion_spec.rb @@ -25,7 +25,8 @@ end let_it_be(:diff) { File.read(File.join(fixtures_folder, 'input.diff')) } - let(:mr_suggestion) { described_class.new(diff, filepath, merge_request) } + let_it_be(:prepend_text) { nil } + let(:mr_suggestion) { described_class.new(diff, filepath, merge_request, prepend_text) } subject(:attributes_hash) { mr_suggestion.note_attributes_hash } @@ -34,6 +35,8 @@ end context 'when a valid diff is supplied' do + let_it_be(:suggestion) { File.read(File.join(fixtures_folder, 'suggestion.md')) } + it 'returns a correctly formatted suggestion request payload' do position_payload = { position_type: 'text', @@ -51,7 +54,15 @@ expect(attributes_hash[:noteable_type]).to eq(MergeRequest) expect(attributes_hash[:noteable_id]).to eq(merge_request.id) expect(attributes_hash[:position]).to eq(position_payload) - expect(attributes_hash[:note]).to eq(File.read(File.join(fixtures_folder, 'suggestion.md'))) + expect(attributes_hash[:note]).to eq(suggestion) + end + + context 'when a prepend text is present' do + let_it_be(:prepend_text) { 'prepend text' } + + it 'returns a correctly formatted suggestion request payload' do + expect(attributes_hash[:note]).to eq("#{prepend_text}\n#{suggestion}") + end end end -- GitLab From 5793941c14be1347808185a1be73ab20090d0fbf Mon Sep 17 00:00:00 2001 From: Dave Pisek Date: Tue, 29 Oct 2024 08:31:01 +0100 Subject: [PATCH 2/3] Fix doc links and add feedback issue --- .../create_from_vulnerability_data_service.rb | 6 ++++-- .../create_from_vulnerability_data_service_spec.rb | 4 ++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/ee/app/services/merge_requests/create_from_vulnerability_data_service.rb b/ee/app/services/merge_requests/create_from_vulnerability_data_service.rb index c73a4d5adb5f57..0c438c6e3a2fee 100644 --- a/ee/app/services/merge_requests/create_from_vulnerability_data_service.rb +++ b/ee/app/services/merge_requests/create_from_vulnerability_data_service.rb @@ -9,7 +9,9 @@ class CreateFromVulnerabilityDataService < ::BaseService START_NEW_CODE = '' END_NEW_CODE = '' - VULNERABILITY_RESOLUTION_DOCS_LINK = Gitlab::Routing.url_helpers.help_page_path('user/application_security/vulnerabilities/index.md', anchor: 'vulnerability-resolution') + # VULNERABILITY_RESOLUTION_DOCS_LINK = ::Gitlab::Routing.url_helpers.help_page_path('user/application_security/vulnerabilities/index.md', anchor: 'vulnerability-resolution') + VULNERABILITY_RESOLUTION_DOCS_LINK = Rails.application.routes.url_helpers.help_page_url('user/application_security/vulnerabilities/index.md', anchor: 'vulnerability-resolution') + VULNERABILITY_RESOLUTION_FEEDBACK_ISSUE_LINK = 'https://gitlab.com/gitlab-org/gitlab/-/issues/476553' SUGGESTION_DISCLAIMER = <<~MARKDOWN.freeze The suggested code changes were generated by GitLab Duo Vulnerability Resolution, an AI feature. @@ -18,7 +20,7 @@ class CreateFromVulnerabilityDataService < ::BaseService The large language model that generated the suggested code changes was provided with the entire file that contains the vulnerable lines of code. It is not aware of any functionality outside of this context. Please see our [documentation](#{VULNERABILITY_RESOLUTION_DOCS_LINK}) for more - information about this feature. + information about this feature and leave feedback in this [issue](#{VULNERABILITY_RESOLUTION_FEEDBACK_ISSUE_LINK}). MARKDOWN def initialize(project, vulnerability, user = nil, params = {}) diff --git a/ee/spec/services/merge_requests/create_from_vulnerability_data_service_spec.rb b/ee/spec/services/merge_requests/create_from_vulnerability_data_service_spec.rb index fefdf0af894ccc..a05f93a8b169b9 100644 --- a/ee/spec/services/merge_requests/create_from_vulnerability_data_service_spec.rb +++ b/ee/spec/services/merge_requests/create_from_vulnerability_data_service_spec.rb @@ -23,8 +23,8 @@ solve the vulnerability. The large language model that generated the suggested code changes was provided with the entire file that contains the - vulnerable lines of code. It is not aware of any functionality outside of this context. Please see our [documentation](/help/user/application_security/vulnerabilities/index.md#vulnerability-resolution) for more - information about this feature. + vulnerable lines of code. It is not aware of any functionality outside of this context. Please see our [documentation](#{Rails.application.routes.url_helpers.help_page_url('user/application_security/vulnerabilities/index.md', anchor: 'vulnerability-resolution')}) for more + information about this feature and leave feedback in this [issue](https://gitlab.com/gitlab-org/gitlab/-/issues/476553). MARKDOWN end -- GitLab From 880c71daa71e42b8d83de399e207b0c9672775c7 Mon Sep 17 00:00:00 2001 From: Dave Pisek Date: Tue, 29 Oct 2024 14:54:37 +0100 Subject: [PATCH 3/3] Make disclaimer translatable --- .../create_from_vulnerability_data_service.rb | 15 ++++----------- ...create_from_vulnerability_data_service_spec.rb | 10 +--------- locale/gitlab.pot | 3 +++ 3 files changed, 8 insertions(+), 20 deletions(-) diff --git a/ee/app/services/merge_requests/create_from_vulnerability_data_service.rb b/ee/app/services/merge_requests/create_from_vulnerability_data_service.rb index 0c438c6e3a2fee..f54c85bdaf7f8c 100644 --- a/ee/app/services/merge_requests/create_from_vulnerability_data_service.rb +++ b/ee/app/services/merge_requests/create_from_vulnerability_data_service.rb @@ -13,16 +13,6 @@ class CreateFromVulnerabilityDataService < ::BaseService VULNERABILITY_RESOLUTION_DOCS_LINK = Rails.application.routes.url_helpers.help_page_url('user/application_security/vulnerabilities/index.md', anchor: 'vulnerability-resolution') VULNERABILITY_RESOLUTION_FEEDBACK_ISSUE_LINK = 'https://gitlab.com/gitlab-org/gitlab/-/issues/476553' - SUGGESTION_DISCLAIMER = <<~MARKDOWN.freeze - The suggested code changes were generated by GitLab Duo Vulnerability Resolution, an AI feature. - **Use this feature with caution.** Before you apply the code changes, carefully review and test them, to ensure that they - solve the vulnerability. - - The large language model that generated the suggested code changes was provided with the entire file that contains the - vulnerable lines of code. It is not aware of any functionality outside of this context. Please see our [documentation](#{VULNERABILITY_RESOLUTION_DOCS_LINK}) for more - information about this feature and leave feedback in this [issue](#{VULNERABILITY_RESOLUTION_FEEDBACK_ISSUE_LINK}). - MARKDOWN - def initialize(project, vulnerability, user = nil, params = {}) super(project, user, params) @@ -99,8 +89,11 @@ def suggestion_merge_request end def mr_suggestion_note_attributes_hash(diff) + suggestion_disclaimer = + format(_('The suggested code changes were generated by GitLab Duo Vulnerability Resolution, an AI feature. **Use this feature with caution.** Before you apply the code changes, carefully review and test them, to ensure that they solve the vulnerability.%{line_break}The large language model that generated the suggested code changes was provided with the entire file that contains the vulnerable lines of code. It is not aware of any functionality outside of this context. Please see our [documentation](%{docs_link}) for more information about this feature and leave feedback in this [issue](%{feedback_issue_link}).'), docs_link: VULNERABILITY_RESOLUTION_DOCS_LINK, feedback_issue_link: VULNERABILITY_RESOLUTION_FEEDBACK_ISSUE_LINK, line_break: '

') + # convert a diff into a suggestion in the form of a note_attributes_hash - suggestion = Gitlab::Diff::MergeRequestSuggestion.new(diff, finding_presenter.file, suggestion_merge_request, SUGGESTION_DISCLAIMER) + suggestion = Gitlab::Diff::MergeRequestSuggestion.new(diff, finding_presenter.file, suggestion_merge_request, suggestion_disclaimer) suggestion.note_attributes_hash end diff --git a/ee/spec/services/merge_requests/create_from_vulnerability_data_service_spec.rb b/ee/spec/services/merge_requests/create_from_vulnerability_data_service_spec.rb index a05f93a8b169b9..d68db0c658d7ce 100644 --- a/ee/spec/services/merge_requests/create_from_vulnerability_data_service_spec.rb +++ b/ee/spec/services/merge_requests/create_from_vulnerability_data_service_spec.rb @@ -17,15 +17,7 @@ end let_it_be(:suggestion_disclaimer) do - <<~MARKDOWN - The suggested code changes were generated by GitLab Duo Vulnerability Resolution, an AI feature. - **Use this feature with caution.** Before you apply the code changes, carefully review and test them, to ensure that they - solve the vulnerability. - - The large language model that generated the suggested code changes was provided with the entire file that contains the - vulnerable lines of code. It is not aware of any functionality outside of this context. Please see our [documentation](#{Rails.application.routes.url_helpers.help_page_url('user/application_security/vulnerabilities/index.md', anchor: 'vulnerability-resolution')}) for more - information about this feature and leave feedback in this [issue](https://gitlab.com/gitlab-org/gitlab/-/issues/476553). - MARKDOWN + "The suggested code changes were generated by GitLab Duo Vulnerability Resolution, an AI feature. **Use this feature with caution.** Before you apply the code changes, carefully review and test them, to ensure that they solve the vulnerability.

The large language model that generated the suggested code changes was provided with the entire file that contains the vulnerable lines of code. It is not aware of any functionality outside of this context. Please see our [documentation](http://localhost/help/user/application_security/vulnerabilities/index.md#vulnerability-resolution) for more information about this feature and leave feedback in this [issue](https://gitlab.com/gitlab-org/gitlab/-/issues/476553)." end let_it_be(:expected_source_branch) { 'remediate/authentication-bypass-via-incorrect-dom-traversal-and-canonical' } diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 0b272ccd198a79..2b7a792cbd95dc 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -55446,6 +55446,9 @@ msgstr "" msgid "The subject will be used as the title of the new issue, and the message will be the description. %{quickActionsLinkStart}Quick actions%{quickActionsLinkEnd} and styling with %{markdownLinkStart}Markdown%{markdownLinkEnd} are supported." msgstr "" +msgid "The suggested code changes were generated by GitLab Duo Vulnerability Resolution, an AI feature. **Use this feature with caution.** Before you apply the code changes, carefully review and test them, to ensure that they solve the vulnerability.%{line_break}The large language model that generated the suggested code changes was provided with the entire file that contains the vulnerable lines of code. It is not aware of any functionality outside of this context. Please see our [documentation](%{docs_link}) for more information about this feature and leave feedback in this [issue](%{feedback_issue_link})." +msgstr "" + msgid "The tag name can't be changed for an existing release." msgstr "" -- GitLab