Skip to content
Snippets Groups Projects
Commit 5b5fc97f authored by mo khan's avatar mo khan Committed by Jan Provaznik
Browse files

Ensure experimental settings are enabled

This change checks to see if the `third_party_ai_features_enabled`
and `experiment_features_enabled` namespace settings are enabled
before sending data to AI.

Changelog: changed
EE: true
MR: !119829
parent 1f5f81c4
No related branches found
No related tags found
1 merge request!119829Check if 3rd party AI features are enabled
......@@ -10,7 +10,6 @@ class VulnerabilitiesController < Projects::ApplicationController
push_frontend_feature_flag(:deprecate_vulnerabilities_feedback, @project)
push_frontend_feature_flag(:dismissal_reason, @project)
push_frontend_feature_flag(:openai_experimentation, @project)
push_frontend_feature_flag(:explain_vulnerability, @project)
end
before_action :vulnerability, except: [:index, :new]
......@@ -23,6 +22,10 @@ class VulnerabilitiesController < Projects::ApplicationController
urgency :low
def show
push_force_frontend_feature_flag(
:explain_vulnerability,
can?(current_user, :explain_vulnerability, vulnerability)
)
pipeline = vulnerability.finding.first_finding_pipeline
@pipeline = pipeline if Ability.allowed?(current_user, :read_pipeline, pipeline)
@gfm_form = true
......
......@@ -228,8 +228,7 @@ def latest_state_transition
end
def send_to_ai?
::Feature.enabled?(:explain_vulnerability, project) &&
finding.present?
finding.present?
end
private
......
......@@ -8,4 +8,13 @@ class VulnerabilityPolicy < BasePolicy
# since note permissions are shared, and this can have ripple effect on other parts.
rule { ~can?(:read_security_resource) }.prevent :create_note
rule { can?(:read_security_resource) }.enable :read_vulnerability
condition(:can_explain_vulnerability?, scope: :subject) do
Feature.enabled?(:openai_experimentation, @subject&.project) &&
Feature.enabled?(:explain_vulnerability, @subject&.project) &&
::Gitlab::Llm::StageCheck.available?(@subject&.project&.root_ancestor, :explain_vulnerability) &&
@subject&.send_to_ai?
end
rule { can_explain_vulnerability? & can?(:read_security_resource) }.enable(:explain_vulnerability)
end
......@@ -2,6 +2,10 @@
module Llm
class ExplainVulnerabilityService < BaseService
def valid?
super && Ability.allowed?(user, :explain_vulnerability, resource)
end
private
def perform
......
......@@ -9,7 +9,8 @@ class StageCheck
:explain_code,
:generate_description,
:generate_test_file,
:summarize_diff
:summarize_diff,
:explain_vulnerability
].freeze
BETA_FEATURES = [].freeze
THIRD_PARTY_FEATURES = EXPERIMENTAL_FEATURES + BETA_FEATURES
......
......@@ -930,60 +930,45 @@
end
describe '#send_to_ai?' do
context 'when the feature flag is disabled' do
let_it_be(:vulnerability) { create(:vulnerability) }
let_it_be_with_reload(:group) { create(:group) }
let_it_be_with_reload(:project) { create(:project, group: group) }
before do
stub_feature_flags(explain_vulnerability: false)
end
context 'when the vulnerabilty is from SAST and includes the required information' do
let_it_be(:vulnerability) { create(:vulnerability, :with_finding, project: project) }
it 'returns false' do
expect(vulnerability).not_to be_send_to_ai
end
it { expect(vulnerability).to be_send_to_ai }
end
context 'when the feature flag is enabled' do
before do
stub_feature_flags(explain_vulnerability: vulnerability.project)
end
context 'when the vulnerability is for a Secret Detection' do
let_it_be(:vulnerability) { create(:vulnerability, :secret_detection, :with_finding, project: project) }
context 'when the vulnerabilty is from SAST and includes the required information' do
let_it_be(:vulnerability) { create(:vulnerability, :with_finding) }
it { expect(vulnerability).to be_send_to_ai }
end
it { expect(vulnerability).to be_send_to_ai }
end
context 'when the vulnerability is for a Secret Detection' do
let_it_be(:vulnerability) { create(:vulnerability, :secret_detection, :with_finding) }
context 'when the vulnerability does not include a file' do
let_it_be(:vulnerability) { create(:vulnerability, :with_finding, project: project) }
it { expect(vulnerability).to be_send_to_ai }
before do
vulnerability.finding.location.delete('file')
end
context 'when the vulnerability does not include a file' do
let_it_be(:vulnerability) { create(:vulnerability, :with_finding) }
it { expect(vulnerability).to be_send_to_ai }
end
before do
vulnerability.finding.location.delete('file')
end
context 'when the vulnerability does not include a start line' do
let_it_be(:vulnerability) { create(:vulnerability, :with_finding, project: project) }
it { expect(vulnerability).to be_send_to_ai }
before do
vulnerability.finding.location.delete('start_line')
end
context 'when the vulnerability does not include a start line' do
let_it_be(:vulnerability) { create(:vulnerability, :with_finding) }
before do
vulnerability.finding.location.delete('start_line')
end
it { expect(vulnerability).to be_send_to_ai }
end
it { expect(vulnerability).to be_send_to_ai }
end
context 'when the vulnerability does not include a finding' do
let_it_be(:vulnerability) { create(:vulnerability) }
context 'when the vulnerability does not include a finding' do
let_it_be(:vulnerability) { create(:vulnerability, project: project) }
it { expect(vulnerability).not_to be_send_to_ai }
end
it { expect(vulnerability).not_to be_send_to_ai }
end
end
......
......@@ -3,10 +3,10 @@
require 'spec_helper'
RSpec.describe VulnerabilityPolicy, feature_category: :vulnerability_management do
describe 'read_security_resource' do
describe '#rules' do
let(:project) { create(:project) }
let(:user) { create(:user) }
let(:vulnerability) { create(:vulnerability, project: project) }
let(:vulnerability) { create(:vulnerability, :with_finding, project: project) }
subject { described_class.new(user, vulnerability) }
......@@ -19,6 +19,7 @@
it { is_expected.to be_disallowed(:read_security_resource) }
it { is_expected.to be_disallowed(:read_vulnerability) }
it { is_expected.to be_disallowed(:create_note) }
it { is_expected.to be_disallowed(:explain_vulnerability) }
end
context "when the current user has developer access to the vulnerability's project" do
......@@ -29,6 +30,23 @@
it { is_expected.to be_allowed(:read_security_resource) }
it { is_expected.to be_allowed(:read_vulnerability) }
it { is_expected.to be_allowed(:create_note) }
context 'when on gitlab.com', :saas do
let(:namespace) { create(:group_with_plan, plan: :ultimate_plan) }
let(:project) { create(:project, namespace: namespace) }
before do
stub_ee_application_setting(should_check_namespace_plan: true)
stub_licensed_features(ai_features: true, security_dashboard: true)
namespace.update!(
experiment_features_enabled: true,
third_party_ai_features_enabled: true
)
end
it { is_expected.to be_allowed(:explain_vulnerability) }
end
end
context 'when the user is project member but does not have proper access' do
......
......@@ -2,9 +2,10 @@
require 'spec_helper'
RSpec.describe Llm::ExplainVulnerabilityService, feature_category: :vulnerability_management do
RSpec.describe Llm::ExplainVulnerabilityService, :saas, feature_category: :vulnerability_management do
let_it_be(:user) { create(:user) }
let_it_be(:project) { create(:project, :private) }
let_it_be(:namespace) { create(:group_with_plan, plan: :ultimate_plan) }
let_it_be(:project) { create(:project, :public, namespace: namespace) }
let_it_be(:vulnerability) { create(:vulnerability, :with_finding, project: project) }
let_it_be(:options) { {} }
......@@ -12,11 +13,20 @@
before do
stub_feature_flags(openai_experimentation: true)
vulnerability.project = project
end
describe '#execute' do
before do
stub_feature_flags(explain_vulnerability: project)
stub_application_setting(check_namespace_plan: true)
stub_feature_flags(explain_vulnerability: project, openai_experimentation: true)
stub_licensed_features(ai_features: true, security_dashboard: true)
namespace.update!(
experiment_features_enabled: true,
third_party_ai_features_enabled: true
)
allow(Llm::CompletionWorker).to receive(:perform_async)
end
......@@ -61,5 +71,19 @@
expect(Llm::CompletionWorker).not_to have_received(:perform_async)
end
end
context 'when experimental features are disabled' do
before do
project.add_maintainer(user)
namespace.update!(experiment_features_enabled: false)
end
it 'returns an error' do
result = subject.execute
expect(result).to be_error
expect(Llm::CompletionWorker).not_to have_received(:perform_async)
end
end
end
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