Skip to content
Snippets Groups Projects
Verified Commit dde22646 authored by Alex Buijs's avatar Alex Buijs
Browse files

Remove Resolve Vulnerability Vertex prompt

This removes the deprecated Vertex implementation of
Resolve Vulnerability.

Changelog: removed
EE: true
parent 6f8ae5e8
No related branches found
No related tags found
2 merge requests!164749Enable parallel in test-on-omnibus,!162872Remove Resolve Vulnerability Vertex prompt
Showing
with 128 additions and 303 deletions
......@@ -100,11 +100,7 @@ export default {
}
if (this.glAbilities.resolveVulnerabilityWithAi) {
if (glFeatures.resolveVulnerabilityAiGateway) {
buttons.push(CREATE_MR_AI_ACTION_DEPRECATED);
} else {
buttons.push(CREATE_MR_AI_ACTION_DEPRECATED);
}
buttons.push(CREATE_MR_AI_ACTION_DEPRECATED);
}
if (this.glAbilities.explainVulnerabilityWithAi) {
......
......@@ -26,7 +26,6 @@ def show
push_frontend_feature_flag(:vulnerability_resolution_ga, vulnerability.project)
push_frontend_feature_flag(:explain_vulnerability_tool, vulnerability.project)
push_frontend_feature_flag(:resolve_vulnerability_ai_gateway, vulnerability.project)
pipeline = vulnerability.finding.first_finding_pipeline
@pipeline = pipeline if can?(current_user, :read_pipeline, pipeline)
@gfm_form = true
......
......@@ -178,7 +178,6 @@ def render_description(vulnerability)
vulnerability: vulnerability,
llm_patch: llm_patch,
finding: finding_presenter,
resolve_vulnerability_ai_gateway: Feature.enabled?(:resolve_vulnerability_ai_gateway, @project),
description_options: @params[:description_options]
}
)
......
<% if llm_patch.present? %>
<% if resolve_vulnerability_ai_gateway %>
## AI GENERATED PATCH
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.
......@@ -7,16 +6,6 @@ The large language model that generated the suggested code changes was provided
Please see [our documentation](https://docs.gitlab.com/ee/user/application_security/vulnerabilities/#vulnerability-resolution) for more information about this feature.
<% else %>
## AI GENERATED PATCH
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, don't harm the functional behavior of your application or introduce new vulnerabilities.
The large language model that generated the suggested code changes was only provided with the affected lines of code, and the vulnerability in that code. It is not aware of any functionality outside of this context.
Please see [our documentation](https://docs.gitlab.com/ee/user/application_security/vulnerabilities/#vulnerability-resolution) for more information about this feature. We'd love to hear [your feedback](https://gitlab.com/gitlab-org/gitlab/-/issues/435721) so we can improve on this feature as we work to bring it to general availability.
<% end %>
<% end %>
### Description:
......
---
name: resolve_vulnerability_ai_gateway
feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/457232
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/155774
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/464712
milestone: '17.1'
group: group::threat insights
type: beta
default_enabled: true
......@@ -11,10 +11,6 @@ class ResolveVulnerability < Gitlab::Llm::Completions::Base
'AI resolution to a merge request.'
MR_LINK_ERROR = 'An error occurred while attempting to link the MR to the vulnerability.'
RESPONSE_FAILURE_ERROR = 'Response from AI is unreadable. Please try again.'
# Extract the first triple ` quoted code from the llm response with a named capture group called "change".
# Important that the `.+` syntax be permitted to match over newlines.
LLM_DIFF_REGEX = /```\w*\n(?<change>.+?)\n```/ms
START_CODE = "<fixed_code>\n"
END_CODE = "</fixed_code>"
START_ANALYSIS = "<analysis>\n"
......@@ -52,14 +48,8 @@ def execute
def response_for(user, vulnerability, options)
Rails.cache.fetch(cache_key(user, vulnerability), expires_in: 10.minutes, skip_nil: true) do
if Feature.enabled?(:resolve_vulnerability_ai_gateway, vulnerability.project)
@ai_prompt_class = ::Gitlab::Llm::Templates::Vulnerabilities::ResolveVulnerabilityAnthropic
prompt = ai_prompt_class.new(vulnerability, options).to_prompt
ai_response = anthropic_request(user, prompt)
else
prompt = ai_prompt_class.new(vulnerability, options).to_prompt
ai_response = vertex_request(user, prompt, vulnerability)
end
prompt = ai_prompt_class.new(vulnerability, options).to_prompt
ai_response = request(user, prompt)
extract_llm_change(ai_response)
end
......@@ -70,23 +60,16 @@ def response_for(user, vulnerability, options)
def extract_llm_change(ai_response)
content = ai_response.dig('predictions', 0, 'content') || ai_response.dig('content', 0, 'text') || ''
description_options = {}
if Feature.enabled?(:resolve_vulnerability_ai_gateway, vulnerability.project)
regex = /(?<=#{Regexp.escape(START_CODE)}).*?(?=#{Regexp.escape(END_CODE)})/mo
regex = /(?<=#{Regexp.escape(START_CODE)}).*?(?=#{Regexp.escape(END_CODE)})/mo
match_data = content.match(regex)
return [ai_response, false, description_options] unless match_data
return [{ false_positive: true }, false, description_options] if match_data[0].blank?
match_data = content.match(regex)
return [ai_response, false, description_options] unless match_data
return [{ false_positive: true }, false, description_options] if match_data[0].blank?
description_options[:analysis_data] = get_analysis_data(content)
description_options[:summary_data] = get_summary_data(content)
description_options[:analysis_data] = get_analysis_data(content)
description_options[:summary_data] = get_summary_data(content)
[match_data[0], true, description_options]
else
match_data = content.to_s.match(LLM_DIFF_REGEX)
return [ai_response, false, description_options] unless match_data
[match_data[:change], true, description_options]
end
[match_data[0], true, description_options]
end
def create_merge_request(user, vulnerability, response, description_options = {})
......@@ -156,14 +139,7 @@ def modify_response(response, _vulnerability)
::Gitlab::Llm::ResponseModifiers::ResolveVulnerability.new(response)
end
def vertex_request(user, prompt, _vulnerability)
::Gitlab::Llm::VertexAi::Client.new(user,
unit_primitive: 'resolve_vulnerability',
tracking_context: tracking_context
).code(content: prompt)
end
def anthropic_request(user, prompt)
def request(user, prompt)
::Gitlab::Llm::ResolveVulnerability::Client.new(user,
unit_primitive: 'resolve_vulnerability',
tracking_context: tracking_context
......
......@@ -4,12 +4,86 @@ module Gitlab
module Llm
module Templates
module Vulnerabilities
# We use this exception class for errors where we want
# the exception message to appear verbatim in the
# errors property of the response.
PromptError = Class.new(StandardError)
class ResolveVulnerability < VulnerabilityTemplate
MAX_TOKENS = 4096
SYSTEM_MESSAGE = Gitlab::Llm::Chain::Utils::Prompt.as_system(
<<~PROMPT.chomp
You are a skilled security analyst and programmer tasked with resolving code security vulnerabilities.
You will be given a vulnerability report and the corresponding source code. Your job is to analyze the report, determine if it's a false positive, and if not, provide a fix for the vulnerability.
PROMPT
)
USER_MESSAGE = Gitlab::Llm::Chain::Utils::Prompt.as_user(
<<~PROMPT.chomp
First, carefully review the vulnerability report:
<vulnerability_report>
Name: %<name>s
Description: %<vulnerability_description>s
</vulnerability_report>
%<identifiers>s
Now, examine the source code from the file %<filename>s in question:
<source_code>
%<source_code>s
</source_code>
<vulnerable_code>
%<vulnerable_code>s
</vulnerable_code>
Analyze the vulnerability report and the source code. Consider the following:
1. The type of vulnerability reported (e.g., CWE, OWASP, linting rule)
2. The specific part of the code that is flagged as vulnerable in the vulnerable_code section
3. The potential security implications of the reported issue
Consider the context of the code and whether the reported issue truly represents a security concern.
<analysis>
Provide your analysis here, explaining your reasoning. Ensure that it is markdown formatted with all the code highlighting if needed.
</analysis>
Present your solution in the following format:
<fixed_code>
<old_code>
[Include the vulnerable code snippet, plus 2 lines above and 2 lines below for context]
</old_code>
<new_code>
[Include the fixed code snippet, plus 2 lines above and 2 lines below for context - that needs to be same lines as for <old_code>]
</new_code>
</fixed_code>
1. If there are multiple code locations that need to be addressed, repeat the <old_code>...</old_code> and <new_code>...</new_code> structure for each fix.
2. Ensure that your fixes address the security vulnerability without changing the overall functionality of the code.
3. Include all changes that are necessary to make the <new_code> parts work
4. If you need to add new lines of code as part of your fix, include them in the <new_code> section and adjust the context lines accordingly.
5. If you need to remove lines of code as part of your fix, omit them from the <new_code> section and adjust the context lines accordingly.
Remember to maintain the structure and formatting of the original code as much as possible, only making changes necessary to address the security vulnerability.
Provide your complete response, including all fixes, within a single set of <fixed_code> tags.
Finally, provide a summary of your findings and actions:
<summary>
1. Briefly restate the reported vulnerability
2. Summarize the fix you provided and explain how it addresses the security concern
3. Ensure that it is markdown formatted with all the code highlighting if needed.
</summary>
Remember to be thorough in your analysis and clear in your explanations. Your goal is to ensure the
security of the code while avoiding unnecessary changes. If you believe the vulnerability cannot be fixed
without additional information, explain why in your analysis.
PROMPT
)
def to_prompt
ensure_eligible_code!
prompt
......@@ -18,15 +92,25 @@ def to_prompt
private
def prompt
<<~PROMPT
The file "#{filename}" has this vulnerable code:
```
#{vulnerable_code}
```
{
messages: Gitlab::Llm::Chain::Utils::Prompt.role_conversation(
Gitlab::Llm::Chain::Utils::Prompt.format_conversation([USER_MESSAGE], variables)
),
system: Gitlab::Llm::Chain::Utils::Prompt.no_role_text([SYSTEM_MESSAGE], variables),
model: ::Gitlab::Llm::Anthropic::Client::CLAUDE_3_5_SONNET,
max_tokens: MAX_TOKENS
}
end
It has the security vulnerability "#{title} - (#{identifiers})". Write code that fixes the vulnerability.
PROMPT
def variables
{
name: title,
filename: filename,
identifiers: formatted_identifiers,
vulnerable_code: vulnerable_code.chomp,
source_code: source_code,
vulnerability_description: description
}
end
end
end
......
# frozen_string_literal: true
module Gitlab
module Llm
module Templates
module Vulnerabilities
class ResolveVulnerabilityAnthropic < ResolveVulnerability
include Gitlab::Llm::Chain::Concerns::AnthropicPrompt
MAX_TOKENS = 4096
SYSTEM_MESSAGE = Gitlab::Llm::Chain::Utils::Prompt.as_system(
<<~PROMPT.chomp
You are a skilled security analyst and programmer tasked with resolving code security vulnerabilities.
You will be given a vulnerability report and the corresponding source code. Your job is to analyze the report, determine if it's a false positive, and if not, provide a fix for the vulnerability.
PROMPT
)
USER_MESSAGE = Gitlab::Llm::Chain::Utils::Prompt.as_user(
<<~PROMPT.chomp
First, carefully review the vulnerability report:
<vulnerability_report>
Name: %<name>s
Description: %<vulnerability_description>s
</vulnerability_report>
%<identifiers>s
Now, examine the source code from the file %<filename>s in question:
<source_code>
%<source_code>s
</source_code>
<vulnerable_code>
%<vulnerable_code>s
</vulnerable_code>
Analyze the vulnerability report and the source code. Consider the following:
1. The type of vulnerability reported (e.g., CWE, OWASP, linting rule)
2. The specific part of the code that is flagged as vulnerable in the vulnerable_code section
3. The potential security implications of the reported issue
Consider the context of the code and whether the reported issue truly represents a security concern.
<analysis>
Provide your analysis here, explaining your reasoning. Ensure that it is markdown formatted with all the code highlighting if needed.
</analysis>
Present your solution in the following format:
<fixed_code>
<old_code>
[Include the vulnerable code snippet, plus 2 lines above and 2 lines below for context]
</old_code>
<new_code>
[Include the fixed code snippet, plus 2 lines above and 2 lines below for context - that needs to be same lines as for <old_code>]
</new_code>
</fixed_code>
1. If there are multiple code locations that need to be addressed, repeat the <old_code>...</old_code> and <new_code>...</new_code> structure for each fix.
2. Ensure that your fixes address the security vulnerability without changing the overall functionality of the code.
3. Include all changes that are necessary to make the <new_code> parts work
4. If you need to add new lines of code as part of your fix, include them in the <new_code> section and adjust the context lines accordingly.
5. If you need to remove lines of code as part of your fix, omit them from the <new_code> section and adjust the context lines accordingly.
Remember to maintain the structure and formatting of the original code as much as possible, only making changes necessary to address the security vulnerability.
Provide your complete response, including all fixes, within a single set of <fixed_code> tags.
Finally, provide a summary of your findings and actions:
<summary>
1. Briefly restate the reported vulnerability
2. Summarize the fix you provided and explain how it addresses the security concern
3. Ensure that it is markdown formatted with all the code highlighting if needed.
</summary>
Remember to be thorough in your analysis and clear in your explanations. Your goal is to ensure the
security of the code while avoiding unnecessary changes. If you believe the vulnerability cannot be fixed
without additional information, explain why in your analysis.
PROMPT
)
private
def max_code_length
MAX_CHARACTERS / 10
end
def prompt
{
messages: Gitlab::Llm::Chain::Utils::Prompt.role_conversation(
Gitlab::Llm::Chain::Utils::Prompt.format_conversation([USER_MESSAGE], variables)
),
system: Gitlab::Llm::Chain::Utils::Prompt.no_role_text([SYSTEM_MESSAGE], variables),
model: ::Gitlab::Llm::Anthropic::Client::CLAUDE_3_5_SONNET,
max_tokens: MAX_TOKENS
}
end
def variables
{
name: title,
filename: filename,
identifiers: formatted_identifiers,
vulnerable_code: vulnerable_code.chomp,
source_code: finding.source_code,
vulnerability_description: vulnerability.description
}
end
def formatted_identifiers
identifiers = finding.identifier_names
return '' if identifiers.empty?
names = identifiers.join("\n* ")
"<report_identifiers>\n * #{names}\n</report_identifiers>"
end
end
end
end
end
end
......@@ -4,11 +4,15 @@ module Gitlab
module Llm
module Templates
module Vulnerabilities
# We use this exception class for errors where we want
# the exception message to appear verbatim in the
# errors property of the response.
PromptError = Class.new(StandardError)
class VulnerabilityTemplate
include Gitlab::Llm::Chain::Concerns::AnthropicPrompt
include Gitlab::Utils::StrongMemoize
MAX_CODE_LENGTH = 1_024
def initialize(vulnerability, params = {})
@vulnerability = vulnerability
@params = params
......@@ -18,12 +22,8 @@ def initialize(vulnerability, params = {})
attr_reader :vulnerability
delegate :title, :description, :file, to: :vulnerability
delegate :source_code?, :vulnerable_code, to: :finding
def identifiers
finding.identifier_names
end
delegate :title, :file, :description, :secret_detection?, to: :vulnerability
delegate :source_code, :source_code?, :identifier_names, :vulnerable_code, to: :finding
def finding
vulnerability.finding
......@@ -34,8 +34,17 @@ def filename
File.basename(file)
end
def formatted_identifiers
identifiers = identifier_names
return '' if identifiers.empty?
names = identifiers.join("\n* ")
"<report_identifiers>\n * #{names}\n</report_identifiers>"
end
def max_code_length
MAX_CODE_LENGTH
MAX_CHARACTERS / 10
end
def ensure_eligible_code!
......@@ -45,7 +54,7 @@ def ensure_eligible_code!
raise PromptError, "Vulnerable code exceeds maximum length (#{max_code_length})"
end
raise PromptError, "Refusing to send possible secrets in AI prompt" if vulnerability.secret_detection?
raise PromptError, "Refusing to send possible secrets in AI prompt" if secret_detection?
end
end
end
......
......@@ -138,7 +138,6 @@ describe('Vulnerability Header', () => {
dismissalDescriptions,
glFeatures: {
explainVulnerabilityTool: true,
resolveVulnerabilityAiGateway: true,
vulnerabilityResolutionGa: true,
...glFeatures,
},
......@@ -379,7 +378,6 @@ describe('Vulnerability Header', () => {
canDownloadPatch: true,
}),
glFeatures: {
resolveVulnerabilityAiGateway: false,
resolveVulnerability: false,
vulnerabilityResolutionGa: false,
},
......@@ -403,7 +401,6 @@ describe('Vulnerability Header', () => {
createWrapper({
glFeatures: {
explainVulnerabilityTool: false,
resolveVulnerabilityAiGateway: false,
resolveVulnerability: true,
vulnerabilityResolutionGa: false,
},
......
......@@ -435,15 +435,5 @@ def execute_resolve(message_params = {}, options = {})
end
end
end
context 'when the `resolve_vulnerability_ai_gateway` feature flag is disabled' do
before do
stub_feature_flags(resolve_vulnerability_ai_gateway: false)
end
it_behaves_like 'resolve vulnerability completions', ::Gitlab::Llm::VertexAi::Client, :code do
let(:code_patch) { changed_code }
end
end
end
end
......@@ -2,7 +2,7 @@
require 'spec_helper'
RSpec.describe Gitlab::Llm::Templates::Vulnerabilities::ResolveVulnerabilityAnthropic,
RSpec.describe Gitlab::Llm::Templates::Vulnerabilities::ResolveVulnerability,
feature_category: :vulnerability_management do
let(:source_code) do
<<~SOURCE
......
......@@ -550,82 +550,6 @@
end
end
end
context 'when FF is disabled' do
before do
stub_feature_flags(resolve_vulnerability_ai_gateway: false)
end
let(:expected_description) do
<<~DESC.chomp
## AI GENERATED PATCH
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, don't harm the functional behavior of your application or introduce new vulnerabilities.
The large language model that generated the suggested code changes was only provided with the affected lines of code, and the vulnerability in that code. It is not aware of any functionality outside of this context.
Please see [our documentation](https://docs.gitlab.com/ee/user/application_security/vulnerabilities/#vulnerability-resolution) for more information about this feature. We'd love to hear [your feedback](https://gitlab.com/gitlab-org/gitlab/-/issues/435721) so we can improve on this feature as we work to bring it to general availability.
### Description:
Long numbers in C code are bad, use shorter number
* Severity: high
* Confidence: medium
* Location: [src/main.c:6](src/main.c:6)
### Analysis:
The vulnerability report lacks specific details about the type of vulnerability, making it challenging to directly correlate with the provided code. However, upon examining the source code, a clear buffer overflow vulnerability is present.
The vulnerable code section is:
char buf[8];
memcpy(&buf, "123456789");
This code creates a buffer buf of size 8 bytes, but then attempts to copy 9 bytes (the string "123456789") into it using memcpy(). This results in a buffer overflow, which can lead to memory corruption, crashes, or even arbitrary code execution in some cases.
The vulnerability falls under the category of CWE-119: Improper Restriction of Operations within the Bounds of a Memory Buffer.
This is a genuine security concern as buffer overflows can be exploited by attackers to manipulate program behavior or execute malicious code.
### Summary:
1. The reported vulnerability is a buffer overflow in the main() function, where a 9-byte string is copied into an 8-byte buffer.
2. The fix addresses this issue by:
- Increasing the buffer size to 10 bytes to accommodate the 9-byte string plus a null terminator.
- Changing memcpy(&buf, "123456789") to memcpy(buf, "123456789", 9).
3. These changes ensure that:
- The buffer is large enough to hold the string.
- The correct number of bytes (9) is copied.
- The & operator is removed from buf, as it's unnecessary and incorrect when used with memcpy().
This fix prevents buffer overflow by ensuring that the destination buffer is large enough to hold the source data. It maintains the original functionality while adhering to secure coding practices.
Additional recommendations:
- Consider using strncpy() or snprintf() for string operations, as they provide more safety features.
- Always check the return value of memory operations for error handling.
- If the string length is known at compile-time, consider using sizeof() for buffer allocation to prevent manual size miscalculations.
char buf[sizeof("123456789")];
strncpy(buf, "123456789", sizeof(buf));
buf[sizeof(buf) - 1] = '\\0' // Ensure null-termination
This approach would further improve the code's safety and maintainability.
### Solution:
GCM mode introduces an HMAC into the resulting encrypted data, providing integrity of the result.
### Links:
* [Cipher does not check for integrity first?](https://crypto.stackexchange.com/questions/31428/pbewithmd5anddes-cipher-does-not-check-for-integrity-first)
DESC
end
it_behaves_like 'a created merge_request'
end
end
# rubocop:enable RSpec/MultipleMemoizedHelpers
end
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment