From 351b47a86ac300f38832ff99feae393d24e3045d Mon Sep 17 00:00:00 2001 From: Mark Mishaev Date: Thu, 10 Jul 2025 18:35:27 +0300 Subject: [PATCH 01/12] Audit API access to group and project variables Adds an audit event when a group or project variable is accessed via their respective API endpoints. Changelog: added --- .../types/variable_viewed_api.yml | 10 ++++++ doc/user/compliance/audit_event_types.md | 6 ++++ lib/api/ci/variables.rb | 12 +++++++ lib/api/group_variables.rb | 12 +++++++ spec/requests/api/ci/variables_spec.rb | 34 +++++++++++++++++++ spec/requests/api/group_variables_spec.rb | 31 +++++++++++++++++ 6 files changed, 105 insertions(+) create mode 100644 config/audit_events/types/variable_viewed_api.yml diff --git a/config/audit_events/types/variable_viewed_api.yml b/config/audit_events/types/variable_viewed_api.yml new file mode 100644 index 00000000000000..7e0382f807d181 --- /dev/null +++ b/config/audit_events/types/variable_viewed_api.yml @@ -0,0 +1,10 @@ +--- +name: variable_viewed_api +description: A CI/CD variable is accessed via API +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/555960 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/197385 +feature_category: ci_variables +milestone: "18.3" +saved_to_database: true +streamed: true +scope: [Project, Group] diff --git a/doc/user/compliance/audit_event_types.md b/doc/user/compliance/audit_event_types.md index 78db3be8cffe18..ec4c8365467194 100644 --- a/doc/user/compliance/audit_event_types.md +++ b/doc/user/compliance/audit_event_types.md @@ -113,6 +113,12 @@ Audit event types belong to the following product categories. |:----------|:---------------------|:------------------|:--------------|:------| | [`job_artifact_downloaded`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/129608) | A user downloads a job artifact from a project | {{< icon name="dotted-circle" >}} No | GitLab [16.8](https://gitlab.com/gitlab-org/gitlab/-/issues/250663) | Project | +### Ci variables + +| Type name | Event triggered when | Saved to database | Introduced in | Scope | +|:----------|:---------------------|:------------------|:--------------|:------| +| [`variable_viewed_api`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/197385) | A CI/CD variable is accessed via API | {{< icon name="check-circle" >}} Yes | GitLab [18.3](https://gitlab.com/gitlab-org/gitlab/-/issues/555960) | Project, Group | + ### Code review | Type name | Event triggered when | Saved to database | Introduced in | Scope | diff --git a/lib/api/ci/variables.rb b/lib/api/ci/variables.rb index 7b4fede99cf05f..3d78c23eb2c6f0 100644 --- a/lib/api/ci/variables.rb +++ b/lib/api/ci/variables.rb @@ -44,6 +44,18 @@ class Variables < ::API::Base variable = find_variable(user_project, params) not_found!('Variable') unless variable + message = "CI/CD variable '#{variable.key}' accessed via API" + message += " (hidden variable, no value shown)" if variable.hidden? + + audit_context = { + name: 'variable_viewed_api', + author: current_user, + scope: user_project, + target: variable, + message: message + } + ::Gitlab::Audit::Auditor.audit(audit_context) + present variable, with: Entities::Ci::Variable end diff --git a/lib/api/group_variables.rb b/lib/api/group_variables.rb index 3297f2ca868f36..1fbafcc40c7335 100644 --- a/lib/api/group_variables.rb +++ b/lib/api/group_variables.rb @@ -41,6 +41,18 @@ class GroupVariables < ::API::Base break not_found!('GroupVariable') unless variable + message = "CI/CD variable '#{variable.key}' accessed via API" + message += " (hidden variable, no value shown)" if variable.hidden? + + audit_context = { + name: 'variable_viewed_api', + author: current_user, + scope: user_group, + target: variable, + message: message + } + ::Gitlab::Audit::Auditor.audit(audit_context) + present variable, with: Entities::Ci::Variable end diff --git a/spec/requests/api/ci/variables_spec.rb b/spec/requests/api/ci/variables_spec.rb index a080ffe4b5cec2..6b73324704796c 100644 --- a/spec/requests/api/ci/variables_spec.rb +++ b/spec/requests/api/ci/variables_spec.rb @@ -12,6 +12,10 @@ let(:is_masked_variable) { true } let!(:variable) { create(:ci_variable, project: project, hidden: is_hidden_variable, masked: is_masked_variable) } + before do + stub_licensed_features(audit_events: true) + end + describe 'GET /projects/:id/variables' do context 'authorized user with proper permissions' do it 'returns project variables' do @@ -56,6 +60,21 @@ expect(json_response['description']).to be_nil expect(json_response['hidden']).to eq(true) end + + it 'creates an audit event when variable value is hidden' do + expect do + get api("/projects/#{project.id}/variables/#{variable.key}", user) + end.to change { AuditEvent.count }.by(1) + + audit_event = AuditEvent.order(:id).last + expect(audit_event.details[:custom_message]).to eq( + "CI/CD variable '#{variable.key}' accessed via API (hidden variable, no value shown)" + ) + expect(audit_event.details[:event_name]).to eq('variable_viewed_api') + expect(audit_event.details[:target_details]).to eq(variable.key) + expect(audit_event.author_id).to eq(user.id) + expect(audit_event.entity_id).to eq(project.id) + end end context 'when variable is not hidden' do @@ -74,6 +93,21 @@ expect(json_response['description']).to be_nil expect(json_response['hidden']).to eq(false) end + + it 'creates an audit event when variable value is accessed' do + expect do + get api("/projects/#{project.id}/variables/#{variable.key}", user) + end.to change { AuditEvent.count }.by(1) + + audit_event = AuditEvent.order(:id).last + expect(audit_event.details[:custom_message]).to eq( + "CI/CD variable '#{variable.key}' accessed via API" + ) + expect(audit_event.details[:event_name]).to eq('variable_viewed_api') + expect(audit_event.details[:target_details]).to eq(variable.key) + expect(audit_event.author_id).to eq(user.id) + expect(audit_event.entity_id).to eq(project.id) + end end it 'responds with 404 Not Found if requesting non-existing variable' do diff --git a/spec/requests/api/group_variables_spec.rb b/spec/requests/api/group_variables_spec.rb index 8e48bc7b19e2a7..624a52d34001c6 100644 --- a/spec/requests/api/group_variables_spec.rb +++ b/spec/requests/api/group_variables_spec.rb @@ -10,6 +10,7 @@ before do group.add_member(user, access_level) if access_level + stub_licensed_features(audit_events: true) end describe 'GET /groups/:id/variables' do @@ -61,6 +62,21 @@ expect(json_response['environment_scope']).to eq(variable.environment_scope) expect(json_response['description']).to be_nil end + + it 'creates an audit event when variable value is hidden' do + expect do + get api("/groups/#{group.id}/variables/#{variable.key}", user) + end.to change { AuditEvent.count }.by(1) + + audit_event = AuditEvent.order(:id).last + expect(audit_event.details[:custom_message]).to eq( + "CI/CD variable '#{variable.key}' accessed via API (hidden variable, no value shown)" + ) + expect(audit_event.details[:event_name]).to eq('variable_viewed_api') + expect(audit_event.details[:target_details]).to eq(variable.key) + expect(audit_event.author_id).to eq(user.id) + expect(audit_event.entity_id).to eq(group.id) + end end context 'when variable is not hidden' do @@ -77,6 +93,21 @@ expect(json_response['environment_scope']).to eq(variable.environment_scope) expect(json_response['description']).to be_nil end + + it 'creates an audit event when variable value is accessed' do + expect do + get api("/groups/#{group.id}/variables/#{variable.key}", user) + end.to change { AuditEvent.count }.by(1) + + audit_event = AuditEvent.order(:id).last + expect(audit_event.details[:custom_message]).to eq( + "CI/CD variable '#{variable.key}' accessed via API" + ) + expect(audit_event.details[:event_name]).to eq('variable_viewed_api') + expect(audit_event.details[:target_details]).to eq(variable.key) + expect(audit_event.author_id).to eq(user.id) + expect(audit_event.entity_id).to eq(group.id) + end end it 'responds with 404 Not Found if requesting non-existing variable' do -- GitLab From 01309ee2899955e176ac78f1fd64e7f2bb74c074 Mon Sep 17 00:00:00 2001 From: Mark Mishaev Date: Fri, 18 Jul 2025 06:21:16 +0300 Subject: [PATCH 02/12] Apply 1 suggestion(s) to 1 file(s) Co-authored-by: Marcel Amirault --- lib/api/group_variables.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/api/group_variables.rb b/lib/api/group_variables.rb index 1fbafcc40c7335..55db7c9ef4dbac 100644 --- a/lib/api/group_variables.rb +++ b/lib/api/group_variables.rb @@ -41,7 +41,7 @@ class GroupVariables < ::API::Base break not_found!('GroupVariable') unless variable - message = "CI/CD variable '#{variable.key}' accessed via API" + message = "CI/CD variable '#{variable.key}' accessed with the API" message += " (hidden variable, no value shown)" if variable.hidden? audit_context = { -- GitLab From 3c75b94a3044c5b3b3fc11b587ae0137afc01aec Mon Sep 17 00:00:00 2001 From: Mark Mishaev Date: Fri, 18 Jul 2025 06:21:23 +0300 Subject: [PATCH 03/12] Apply 1 suggestion(s) to 1 file(s) Co-authored-by: Marcel Amirault --- config/audit_events/types/variable_viewed_api.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/audit_events/types/variable_viewed_api.yml b/config/audit_events/types/variable_viewed_api.yml index 7e0382f807d181..cdac7e4f955cb8 100644 --- a/config/audit_events/types/variable_viewed_api.yml +++ b/config/audit_events/types/variable_viewed_api.yml @@ -1,6 +1,6 @@ --- name: variable_viewed_api -description: A CI/CD variable is accessed via API +description: A CI/CD variable is accessed with the API introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/555960 introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/197385 feature_category: ci_variables -- GitLab From 963dd79b13cd424b5299d41b413cf5931b1f61d2 Mon Sep 17 00:00:00 2001 From: Mark Mishaev Date: Fri, 18 Jul 2025 06:21:31 +0300 Subject: [PATCH 04/12] Apply 1 suggestion(s) to 1 file(s) Co-authored-by: Marcel Amirault --- lib/api/ci/variables.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/api/ci/variables.rb b/lib/api/ci/variables.rb index 3d78c23eb2c6f0..5c4b611f53e0a5 100644 --- a/lib/api/ci/variables.rb +++ b/lib/api/ci/variables.rb @@ -44,7 +44,7 @@ class Variables < ::API::Base variable = find_variable(user_project, params) not_found!('Variable') unless variable - message = "CI/CD variable '#{variable.key}' accessed via API" + message = "CI/CD variable '#{variable.key}' accessed with the API" message += " (hidden variable, no value shown)" if variable.hidden? audit_context = { -- GitLab From aa995bbf7d623514897ef29425d9f79a580ebbaa Mon Sep 17 00:00:00 2001 From: Mark Mishaev Date: Fri, 18 Jul 2025 07:05:40 +0300 Subject: [PATCH 05/12] Update audit event types documentation --- doc/user/compliance/audit_event_types.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/user/compliance/audit_event_types.md b/doc/user/compliance/audit_event_types.md index ec4c8365467194..416b1cbfd214c2 100644 --- a/doc/user/compliance/audit_event_types.md +++ b/doc/user/compliance/audit_event_types.md @@ -117,7 +117,7 @@ Audit event types belong to the following product categories. | Type name | Event triggered when | Saved to database | Introduced in | Scope | |:----------|:---------------------|:------------------|:--------------|:------| -| [`variable_viewed_api`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/197385) | A CI/CD variable is accessed via API | {{< icon name="check-circle" >}} Yes | GitLab [18.3](https://gitlab.com/gitlab-org/gitlab/-/issues/555960) | Project, Group | +| [`variable_viewed_api`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/197385) | A CI/CD variable is accessed with the API | {{< icon name="check-circle" >}} Yes | GitLab [18.3](https://gitlab.com/gitlab-org/gitlab/-/issues/555960) | Project, Group | ### Code review -- GitLab From 6f377117359e7f1247bebfecca89e7dca20fa0ec Mon Sep 17 00:00:00 2001 From: Mark Mishaev Date: Fri, 18 Jul 2025 07:28:32 +0300 Subject: [PATCH 06/12] Refactor audit event code to eliminate duplication - Add audit_variable_access helper method to VariablesHelpers - Create shared examples for audit event tests - Update all variable API endpoints to use helper method - Replace duplicated test code with shared examples - Add audit event tests to admin instance variables API Addresses MR review suggestions from @nav-j --- lib/api/ci/variables.rb | 12 +---- lib/api/group_variables.rb | 12 +---- lib/api/helpers/variables_helpers.rb | 14 +++++ spec/requests/api/ci/variables_spec.rb | 32 +++--------- spec/requests/api/group_variables_spec.rb | 32 +++--------- .../requests/api/variables_shared_examples.rb | 52 +++++++++++++++++++ 6 files changed, 80 insertions(+), 74 deletions(-) create mode 100644 spec/requests/api/variables_shared_examples.rb diff --git a/lib/api/ci/variables.rb b/lib/api/ci/variables.rb index 5c4b611f53e0a5..6e3f35f300a438 100644 --- a/lib/api/ci/variables.rb +++ b/lib/api/ci/variables.rb @@ -44,17 +44,7 @@ class Variables < ::API::Base variable = find_variable(user_project, params) not_found!('Variable') unless variable - message = "CI/CD variable '#{variable.key}' accessed with the API" - message += " (hidden variable, no value shown)" if variable.hidden? - - audit_context = { - name: 'variable_viewed_api', - author: current_user, - scope: user_project, - target: variable, - message: message - } - ::Gitlab::Audit::Auditor.audit(audit_context) + audit_variable_access(variable, user_project) present variable, with: Entities::Ci::Variable end diff --git a/lib/api/group_variables.rb b/lib/api/group_variables.rb index 55db7c9ef4dbac..648ce1747787ec 100644 --- a/lib/api/group_variables.rb +++ b/lib/api/group_variables.rb @@ -41,17 +41,7 @@ class GroupVariables < ::API::Base break not_found!('GroupVariable') unless variable - message = "CI/CD variable '#{variable.key}' accessed with the API" - message += " (hidden variable, no value shown)" if variable.hidden? - - audit_context = { - name: 'variable_viewed_api', - author: current_user, - scope: user_group, - target: variable, - message: message - } - ::Gitlab::Audit::Auditor.audit(audit_context) + audit_variable_access(variable, user_group) present variable, with: Entities::Ci::Variable end diff --git a/lib/api/helpers/variables_helpers.rb b/lib/api/helpers/variables_helpers.rb index edbdcb257e72e7..1cc0f35d04a8b5 100644 --- a/lib/api/helpers/variables_helpers.rb +++ b/lib/api/helpers/variables_helpers.rb @@ -20,6 +20,20 @@ def find_variable(owner, params) conflict!("There are multiple variables with provided parameters. Please use 'filter[environment_scope]'") end + + def audit_variable_access(variable, scope) + message = "CI/CD variable '#{variable.key}' accessed via API" + message += " (hidden variable, no value shown)" if variable.hidden? + + audit_context = { + name: 'variable_viewed_api', + author: current_user, + scope: scope, + target: variable, + message: message + } + ::Gitlab::Audit::Auditor.audit(audit_context) + end end end end diff --git a/spec/requests/api/ci/variables_spec.rb b/spec/requests/api/ci/variables_spec.rb index 6b73324704796c..f812283beb8233 100644 --- a/spec/requests/api/ci/variables_spec.rb +++ b/spec/requests/api/ci/variables_spec.rb @@ -61,19 +61,9 @@ expect(json_response['hidden']).to eq(true) end - it 'creates an audit event when variable value is hidden' do - expect do - get api("/projects/#{project.id}/variables/#{variable.key}", user) - end.to change { AuditEvent.count }.by(1) - - audit_event = AuditEvent.order(:id).last - expect(audit_event.details[:custom_message]).to eq( - "CI/CD variable '#{variable.key}' accessed via API (hidden variable, no value shown)" - ) - expect(audit_event.details[:event_name]).to eq('variable_viewed_api') - expect(audit_event.details[:target_details]).to eq(variable.key) - expect(audit_event.author_id).to eq(user.id) - expect(audit_event.entity_id).to eq(project.id) + include_examples 'audit event for variable access', :ci_variable do + let(:make_request) { get api("/projects/#{project.id}/variables/#{variable.key}", user) } + let(:expected_entity_id) { project.id } end end @@ -94,19 +84,9 @@ expect(json_response['hidden']).to eq(false) end - it 'creates an audit event when variable value is accessed' do - expect do - get api("/projects/#{project.id}/variables/#{variable.key}", user) - end.to change { AuditEvent.count }.by(1) - - audit_event = AuditEvent.order(:id).last - expect(audit_event.details[:custom_message]).to eq( - "CI/CD variable '#{variable.key}' accessed via API" - ) - expect(audit_event.details[:event_name]).to eq('variable_viewed_api') - expect(audit_event.details[:target_details]).to eq(variable.key) - expect(audit_event.author_id).to eq(user.id) - expect(audit_event.entity_id).to eq(project.id) + include_examples 'audit event for variable access', :ci_variable do + let(:make_request) { get api("/projects/#{project.id}/variables/#{variable.key}", user) } + let(:expected_entity_id) { project.id } end end diff --git a/spec/requests/api/group_variables_spec.rb b/spec/requests/api/group_variables_spec.rb index 624a52d34001c6..14d3a577445a5c 100644 --- a/spec/requests/api/group_variables_spec.rb +++ b/spec/requests/api/group_variables_spec.rb @@ -63,19 +63,9 @@ expect(json_response['description']).to be_nil end - it 'creates an audit event when variable value is hidden' do - expect do - get api("/groups/#{group.id}/variables/#{variable.key}", user) - end.to change { AuditEvent.count }.by(1) - - audit_event = AuditEvent.order(:id).last - expect(audit_event.details[:custom_message]).to eq( - "CI/CD variable '#{variable.key}' accessed via API (hidden variable, no value shown)" - ) - expect(audit_event.details[:event_name]).to eq('variable_viewed_api') - expect(audit_event.details[:target_details]).to eq(variable.key) - expect(audit_event.author_id).to eq(user.id) - expect(audit_event.entity_id).to eq(group.id) + include_examples 'audit event for variable access', :ci_group_variable do + let(:make_request) { get api("/groups/#{group.id}/variables/#{variable.key}", user) } + let(:expected_entity_id) { group.id } end end @@ -94,19 +84,9 @@ expect(json_response['description']).to be_nil end - it 'creates an audit event when variable value is accessed' do - expect do - get api("/groups/#{group.id}/variables/#{variable.key}", user) - end.to change { AuditEvent.count }.by(1) - - audit_event = AuditEvent.order(:id).last - expect(audit_event.details[:custom_message]).to eq( - "CI/CD variable '#{variable.key}' accessed via API" - ) - expect(audit_event.details[:event_name]).to eq('variable_viewed_api') - expect(audit_event.details[:target_details]).to eq(variable.key) - expect(audit_event.author_id).to eq(user.id) - expect(audit_event.entity_id).to eq(group.id) + include_examples 'audit event for variable access', :ci_group_variable do + let(:make_request) { get api("/groups/#{group.id}/variables/#{variable.key}", user) } + let(:expected_entity_id) { group.id } end end diff --git a/spec/requests/api/variables_shared_examples.rb b/spec/requests/api/variables_shared_examples.rb new file mode 100644 index 00000000000000..9395f87d7c19e0 --- /dev/null +++ b/spec/requests/api/variables_shared_examples.rb @@ -0,0 +1,52 @@ +# frozen_string_literal: true + +RSpec.shared_examples 'audit event for variable access' do |variable_type| + let(:variable) { create(variable_type, **variable_attributes) } + let(:variable_attributes) { { hidden: is_hidden_variable, masked: is_masked_variable } } + let(:is_hidden_variable) { false } + let(:is_masked_variable) { false } + + before do + stub_licensed_features(audit_events: true) + end + + context 'when variable is not hidden' do + let(:is_hidden_variable) { false } + let(:is_masked_variable) { false } + + it 'creates an audit event when variable value is accessed' do + expect do + make_request + end.to change { AuditEvent.count }.by(1) + + audit_event = AuditEvent.order(:id).last + expect(audit_event.details[:custom_message]).to eq( + "CI/CD variable '#{variable.key}' accessed via API" + ) + expect(audit_event.details[:event_name]).to eq('variable_viewed_api') + expect(audit_event.details[:target_details]).to eq(variable.key) + expect(audit_event.author_id).to eq(user.id) + expect(audit_event.entity_id).to eq(expected_entity_id) + end + end + + context 'when variable is hidden' do + let(:is_hidden_variable) { true } + let(:is_masked_variable) { true } + + it 'creates an audit event when variable value is hidden' do + expect do + make_request + end.to change { AuditEvent.count }.by(1) + + audit_event = AuditEvent.order(:id).last + expect(audit_event.details[:custom_message]).to eq( + "CI/CD variable '#{variable.key}' accessed via API (hidden variable, no value shown)" + ) + expect(audit_event.details[:event_name]).to eq('variable_viewed_api') + expect(audit_event.details[:target_details]).to eq(variable.key) + expect(audit_event.author_id).to eq(user.id) + expect(audit_event.entity_id).to eq(expected_entity_id) + end + end +end -- GitLab From 5b5dc78a6dccfdcadd5a4939ea5dc97a35af01ae Mon Sep 17 00:00:00 2001 From: Manuel Grabowski Date: Fri, 18 Jul 2025 12:50:01 +0200 Subject: [PATCH 07/12] Include examples only once, avoid side effects --- spec/requests/api/ci/variables_spec.rb | 19 ++++++------------- spec/requests/api/group_variables_spec.rb | 16 ++++++---------- .../requests/api/variables_shared_examples.rb | 13 +++++-------- 3 files changed, 17 insertions(+), 31 deletions(-) diff --git a/spec/requests/api/ci/variables_spec.rb b/spec/requests/api/ci/variables_spec.rb index f812283beb8233..1a4ee12f1de305 100644 --- a/spec/requests/api/ci/variables_spec.rb +++ b/spec/requests/api/ci/variables_spec.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require 'spec_helper' +require_relative '../variables_shared_examples' RSpec.describe API::Ci::Variables, feature_category: :ci_variables do let(:user) { create(:user) } @@ -12,10 +13,6 @@ let(:is_masked_variable) { true } let!(:variable) { create(:ci_variable, project: project, hidden: is_hidden_variable, masked: is_masked_variable) } - before do - stub_licensed_features(audit_events: true) - end - describe 'GET /projects/:id/variables' do context 'authorized user with proper permissions' do it 'returns project variables' do @@ -60,11 +57,6 @@ expect(json_response['description']).to be_nil expect(json_response['hidden']).to eq(true) end - - include_examples 'audit event for variable access', :ci_variable do - let(:make_request) { get api("/projects/#{project.id}/variables/#{variable.key}", user) } - let(:expected_entity_id) { project.id } - end end context 'when variable is not hidden' do @@ -83,11 +75,12 @@ expect(json_response['description']).to be_nil expect(json_response['hidden']).to eq(false) end + end - include_examples 'audit event for variable access', :ci_variable do - let(:make_request) { get api("/projects/#{project.id}/variables/#{variable.key}", user) } - let(:expected_entity_id) { project.id } - end + include_examples 'audit event for variable access', :ci_variable do + let(:make_request) { get api("/projects/#{project.id}/variables/#{audited_variable.key}", user) } + let(:expected_entity_id) { project.id } + let(:variable_attributes) { { project: project, hidden: is_hidden_variable, masked: is_masked_variable } } end it 'responds with 404 Not Found if requesting non-existing variable' do diff --git a/spec/requests/api/group_variables_spec.rb b/spec/requests/api/group_variables_spec.rb index 14d3a577445a5c..a4e39402911b9b 100644 --- a/spec/requests/api/group_variables_spec.rb +++ b/spec/requests/api/group_variables_spec.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require 'spec_helper' +require_relative './variables_shared_examples' RSpec.describe API::GroupVariables, feature_category: :ci_variables do let_it_be(:group) { create(:group) } @@ -10,7 +11,6 @@ before do group.add_member(user, access_level) if access_level - stub_licensed_features(audit_events: true) end describe 'GET /groups/:id/variables' do @@ -62,11 +62,6 @@ expect(json_response['environment_scope']).to eq(variable.environment_scope) expect(json_response['description']).to be_nil end - - include_examples 'audit event for variable access', :ci_group_variable do - let(:make_request) { get api("/groups/#{group.id}/variables/#{variable.key}", user) } - let(:expected_entity_id) { group.id } - end end context 'when variable is not hidden' do @@ -83,11 +78,12 @@ expect(json_response['environment_scope']).to eq(variable.environment_scope) expect(json_response['description']).to be_nil end + end - include_examples 'audit event for variable access', :ci_group_variable do - let(:make_request) { get api("/groups/#{group.id}/variables/#{variable.key}", user) } - let(:expected_entity_id) { group.id } - end + include_examples 'audit event for variable access', :ci_group_variable do + let(:make_request) { get api("/groups/#{group.id}/variables/#{audited_variable.key}", user) } + let(:expected_entity_id) { group.id } + let(:variable_attributes) { { group: group, hidden: is_hidden_variable, masked: is_masked_variable } } end it 'responds with 404 Not Found if requesting non-existing variable' do diff --git a/spec/requests/api/variables_shared_examples.rb b/spec/requests/api/variables_shared_examples.rb index 9395f87d7c19e0..183cccf4f2158a 100644 --- a/spec/requests/api/variables_shared_examples.rb +++ b/spec/requests/api/variables_shared_examples.rb @@ -1,10 +1,7 @@ # frozen_string_literal: true RSpec.shared_examples 'audit event for variable access' do |variable_type| - let(:variable) { create(variable_type, **variable_attributes) } - let(:variable_attributes) { { hidden: is_hidden_variable, masked: is_masked_variable } } - let(:is_hidden_variable) { false } - let(:is_masked_variable) { false } + let(:audited_variable) { create(variable_type, **variable_attributes) } before do stub_licensed_features(audit_events: true) @@ -21,10 +18,10 @@ audit_event = AuditEvent.order(:id).last expect(audit_event.details[:custom_message]).to eq( - "CI/CD variable '#{variable.key}' accessed via API" + "CI/CD variable '#{audited_variable.key}' accessed via API" ) expect(audit_event.details[:event_name]).to eq('variable_viewed_api') - expect(audit_event.details[:target_details]).to eq(variable.key) + expect(audit_event.details[:target_details]).to eq(audited_variable.key) expect(audit_event.author_id).to eq(user.id) expect(audit_event.entity_id).to eq(expected_entity_id) end @@ -41,10 +38,10 @@ audit_event = AuditEvent.order(:id).last expect(audit_event.details[:custom_message]).to eq( - "CI/CD variable '#{variable.key}' accessed via API (hidden variable, no value shown)" + "CI/CD variable '#{audited_variable.key}' accessed via API (hidden variable, no value shown)" ) expect(audit_event.details[:event_name]).to eq('variable_viewed_api') - expect(audit_event.details[:target_details]).to eq(variable.key) + expect(audit_event.details[:target_details]).to eq(audited_variable.key) expect(audit_event.author_id).to eq(user.id) expect(audit_event.entity_id).to eq(expected_entity_id) end -- GitLab From 1d6249d52adc6a95d2885dd8f4948418a7e79b33 Mon Sep 17 00:00:00 2001 From: Manuel Grabowski Date: Fri, 18 Jul 2025 13:41:11 +0200 Subject: [PATCH 08/12] Avoid usage of "via" --- lib/api/helpers/variables_helpers.rb | 2 +- spec/requests/api/variables_shared_examples.rb | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/api/helpers/variables_helpers.rb b/lib/api/helpers/variables_helpers.rb index 1cc0f35d04a8b5..f8e9202baebac6 100644 --- a/lib/api/helpers/variables_helpers.rb +++ b/lib/api/helpers/variables_helpers.rb @@ -22,7 +22,7 @@ def find_variable(owner, params) end def audit_variable_access(variable, scope) - message = "CI/CD variable '#{variable.key}' accessed via API" + message = "CI/CD variable '#{variable.key}' accessed with the API" message += " (hidden variable, no value shown)" if variable.hidden? audit_context = { diff --git a/spec/requests/api/variables_shared_examples.rb b/spec/requests/api/variables_shared_examples.rb index 183cccf4f2158a..3fb1960e438311 100644 --- a/spec/requests/api/variables_shared_examples.rb +++ b/spec/requests/api/variables_shared_examples.rb @@ -18,7 +18,7 @@ audit_event = AuditEvent.order(:id).last expect(audit_event.details[:custom_message]).to eq( - "CI/CD variable '#{audited_variable.key}' accessed via API" + "CI/CD variable '#{audited_variable.key}' accessed with the API" ) expect(audit_event.details[:event_name]).to eq('variable_viewed_api') expect(audit_event.details[:target_details]).to eq(audited_variable.key) @@ -38,7 +38,7 @@ audit_event = AuditEvent.order(:id).last expect(audit_event.details[:custom_message]).to eq( - "CI/CD variable '#{audited_variable.key}' accessed via API (hidden variable, no value shown)" + "CI/CD variable '#{audited_variable.key}' accessed with the API (hidden variable, no value shown)" ) expect(audit_event.details[:event_name]).to eq('variable_viewed_api') expect(audit_event.details[:target_details]).to eq(audited_variable.key) -- GitLab From c2f153eb55d4edec835090f449b039abb57f203f Mon Sep 17 00:00:00 2001 From: Manuel Grabowski Date: Tue, 22 Jul 2025 17:24:13 +0200 Subject: [PATCH 09/12] Move shared examples into proper place --- spec/requests/api/ci/variables_spec.rb | 1 - spec/requests/api/group_variables_spec.rb | 1 - .../shared_examples/lib}/api/variables_shared_examples.rb | 0 3 files changed, 2 deletions(-) rename spec/{requests => support/shared_examples/lib}/api/variables_shared_examples.rb (100%) diff --git a/spec/requests/api/ci/variables_spec.rb b/spec/requests/api/ci/variables_spec.rb index 1a4ee12f1de305..689e95a540f032 100644 --- a/spec/requests/api/ci/variables_spec.rb +++ b/spec/requests/api/ci/variables_spec.rb @@ -1,7 +1,6 @@ # frozen_string_literal: true require 'spec_helper' -require_relative '../variables_shared_examples' RSpec.describe API::Ci::Variables, feature_category: :ci_variables do let(:user) { create(:user) } diff --git a/spec/requests/api/group_variables_spec.rb b/spec/requests/api/group_variables_spec.rb index a4e39402911b9b..b066a9c4a628d8 100644 --- a/spec/requests/api/group_variables_spec.rb +++ b/spec/requests/api/group_variables_spec.rb @@ -1,7 +1,6 @@ # frozen_string_literal: true require 'spec_helper' -require_relative './variables_shared_examples' RSpec.describe API::GroupVariables, feature_category: :ci_variables do let_it_be(:group) { create(:group) } diff --git a/spec/requests/api/variables_shared_examples.rb b/spec/support/shared_examples/lib/api/variables_shared_examples.rb similarity index 100% rename from spec/requests/api/variables_shared_examples.rb rename to spec/support/shared_examples/lib/api/variables_shared_examples.rb -- GitLab From 00a779816002bea13ba5e59a04068be68db2c4a0 Mon Sep 17 00:00:00 2001 From: Manuel Grabowski Date: Wed, 23 Jul 2025 17:35:14 +0200 Subject: [PATCH 10/12] Apply 2 suggestion(s) to 1 file(s) Co-authored-by: Pedro Pombeiro --- .../shared_examples/lib/api/variables_shared_examples.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/support/shared_examples/lib/api/variables_shared_examples.rb b/spec/support/shared_examples/lib/api/variables_shared_examples.rb index 3fb1960e438311..17ab5e3c17f2a4 100644 --- a/spec/support/shared_examples/lib/api/variables_shared_examples.rb +++ b/spec/support/shared_examples/lib/api/variables_shared_examples.rb @@ -11,7 +11,7 @@ let(:is_hidden_variable) { false } let(:is_masked_variable) { false } - it 'creates an audit event when variable value is accessed' do + it 'creates an audit event' do expect do make_request end.to change { AuditEvent.count }.by(1) @@ -31,7 +31,7 @@ let(:is_hidden_variable) { true } let(:is_masked_variable) { true } - it 'creates an audit event when variable value is hidden' do + it 'creates an audit event with mention to hidden variable' do expect do make_request end.to change { AuditEvent.count }.by(1) -- GitLab From d1e00f0af42be6b8ca4c78d9f940179d73bb263a Mon Sep 17 00:00:00 2001 From: Manuel Grabowski Date: Thu, 24 Jul 2025 20:49:09 +0200 Subject: [PATCH 11/12] Ensure proper EE/CE compatibility Project and Group Audit Events are only available with a subscription, so the relevant code and specs must be in the ee/ directory structure. --- .../audit_events/types/variable_viewed_api.yml | 0 ee/lib/ee/api/helpers/variables_helpers.rb | 15 +++++++++++++++ ee/spec/requests/api/ci/variables_spec.rb | 8 ++++++++ ee/spec/requests/api/group_variables_spec.rb | 6 ++++++ .../lib/api/variables_shared_examples.rb | 0 lib/api/ci/variables.rb | 2 +- lib/api/group_variables.rb | 2 +- lib/api/helpers/variables_helpers.rb | 12 +----------- spec/requests/api/ci/variables_spec.rb | 6 ------ spec/requests/api/group_variables_spec.rb | 6 ------ 10 files changed, 32 insertions(+), 25 deletions(-) rename {config => ee/config}/audit_events/types/variable_viewed_api.yml (100%) rename {spec => ee/spec}/support/shared_examples/lib/api/variables_shared_examples.rb (100%) diff --git a/config/audit_events/types/variable_viewed_api.yml b/ee/config/audit_events/types/variable_viewed_api.yml similarity index 100% rename from config/audit_events/types/variable_viewed_api.yml rename to ee/config/audit_events/types/variable_viewed_api.yml diff --git a/ee/lib/ee/api/helpers/variables_helpers.rb b/ee/lib/ee/api/helpers/variables_helpers.rb index 52280b7d4265d2..e761a94938d1a6 100644 --- a/ee/lib/ee/api/helpers/variables_helpers.rb +++ b/ee/lib/ee/api/helpers/variables_helpers.rb @@ -21,6 +21,21 @@ def filter_variable_parameters(owner, params) params end + + override :audit_variable_access + def audit_variable_access(variable, scope) + message = "CI/CD variable '#{variable.key}' accessed with the API" + message += " (hidden variable, no value shown)" if variable.hidden? + + audit_context = { + name: 'variable_viewed_api', + author: current_user, + scope: scope, + target: variable, + message: message + } + ::Gitlab::Audit::Auditor.audit(audit_context) + end end end end diff --git a/ee/spec/requests/api/ci/variables_spec.rb b/ee/spec/requests/api/ci/variables_spec.rb index d80fa460afab41..13619095a68346 100644 --- a/ee/spec/requests/api/ci/variables_spec.rb +++ b/ee/spec/requests/api/ci/variables_spec.rb @@ -12,6 +12,14 @@ stub_licensed_features(audit_events: true) end + describe 'GET /projects/:id/variables/:key' do + include_examples 'audit event for variable access', :ci_variable do + let(:make_request) { get api("/projects/#{project.id}/variables/#{audited_variable.key}", user) } + let(:expected_entity_id) { project.id } + let(:variable_attributes) { { project: project, hidden: is_hidden_variable, masked: is_masked_variable } } + end + end + describe 'POST /projects/:id/variables' do subject(:post_create) do post api("/projects/#{project.id}/variables", user), params: { key: 'new_variable', value: 'secret_value', protected: true } diff --git a/ee/spec/requests/api/group_variables_spec.rb b/ee/spec/requests/api/group_variables_spec.rb index 02a7c32a96e3f1..8548cd63e44a39 100644 --- a/ee/spec/requests/api/group_variables_spec.rb +++ b/ee/spec/requests/api/group_variables_spec.rb @@ -50,6 +50,12 @@ end end end + + include_examples 'audit event for variable access', :ci_group_variable do + let(:make_request) { get api("/groups/#{group.id}/variables/#{audited_variable.key}", user) } + let(:expected_entity_id) { group.id } + let(:variable_attributes) { { group: group, hidden: is_hidden_variable, masked: is_masked_variable } } + end end describe 'POST /groups/:id/variables' do diff --git a/spec/support/shared_examples/lib/api/variables_shared_examples.rb b/ee/spec/support/shared_examples/lib/api/variables_shared_examples.rb similarity index 100% rename from spec/support/shared_examples/lib/api/variables_shared_examples.rb rename to ee/spec/support/shared_examples/lib/api/variables_shared_examples.rb diff --git a/lib/api/ci/variables.rb b/lib/api/ci/variables.rb index 6e3f35f300a438..7afa16652438e4 100644 --- a/lib/api/ci/variables.rb +++ b/lib/api/ci/variables.rb @@ -44,7 +44,7 @@ class Variables < ::API::Base variable = find_variable(user_project, params) not_found!('Variable') unless variable - audit_variable_access(variable, user_project) + audit_variable_access(variable, user_project) if user_project.licensed_feature_available?(:audit_events) present variable, with: Entities::Ci::Variable end diff --git a/lib/api/group_variables.rb b/lib/api/group_variables.rb index 648ce1747787ec..0e2bc09801cbe5 100644 --- a/lib/api/group_variables.rb +++ b/lib/api/group_variables.rb @@ -41,7 +41,7 @@ class GroupVariables < ::API::Base break not_found!('GroupVariable') unless variable - audit_variable_access(variable, user_group) + audit_variable_access(variable, user_group) if user_group.licensed_feature_available?(:audit_events) present variable, with: Entities::Ci::Variable end diff --git a/lib/api/helpers/variables_helpers.rb b/lib/api/helpers/variables_helpers.rb index f8e9202baebac6..abe8f4ea3e75c4 100644 --- a/lib/api/helpers/variables_helpers.rb +++ b/lib/api/helpers/variables_helpers.rb @@ -22,17 +22,7 @@ def find_variable(owner, params) end def audit_variable_access(variable, scope) - message = "CI/CD variable '#{variable.key}' accessed with the API" - message += " (hidden variable, no value shown)" if variable.hidden? - - audit_context = { - name: 'variable_viewed_api', - author: current_user, - scope: scope, - target: variable, - message: message - } - ::Gitlab::Audit::Auditor.audit(audit_context) + # overridden in EE end end end diff --git a/spec/requests/api/ci/variables_spec.rb b/spec/requests/api/ci/variables_spec.rb index 689e95a540f032..a080ffe4b5cec2 100644 --- a/spec/requests/api/ci/variables_spec.rb +++ b/spec/requests/api/ci/variables_spec.rb @@ -76,12 +76,6 @@ end end - include_examples 'audit event for variable access', :ci_variable do - let(:make_request) { get api("/projects/#{project.id}/variables/#{audited_variable.key}", user) } - let(:expected_entity_id) { project.id } - let(:variable_attributes) { { project: project, hidden: is_hidden_variable, masked: is_masked_variable } } - end - it 'responds with 404 Not Found if requesting non-existing variable' do get api("/projects/#{project.id}/variables/non_existing_variable", user) diff --git a/spec/requests/api/group_variables_spec.rb b/spec/requests/api/group_variables_spec.rb index b066a9c4a628d8..8e48bc7b19e2a7 100644 --- a/spec/requests/api/group_variables_spec.rb +++ b/spec/requests/api/group_variables_spec.rb @@ -79,12 +79,6 @@ end end - include_examples 'audit event for variable access', :ci_group_variable do - let(:make_request) { get api("/groups/#{group.id}/variables/#{audited_variable.key}", user) } - let(:expected_entity_id) { group.id } - let(:variable_attributes) { { group: group, hidden: is_hidden_variable, masked: is_masked_variable } } - end - it 'responds with 404 Not Found if requesting non-existing variable' do get api("/groups/#{group.id}/variables/non_existing_variable", user) -- GitLab From ce989c02da81b43cecdb5876fa0cd9113360e80d Mon Sep 17 00:00:00 2001 From: Manuel Grabowski Date: Mon, 28 Jul 2025 09:52:48 +0200 Subject: [PATCH 12/12] Always create audit events --- lib/api/ci/variables.rb | 2 +- lib/api/group_variables.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/api/ci/variables.rb b/lib/api/ci/variables.rb index 7afa16652438e4..6e3f35f300a438 100644 --- a/lib/api/ci/variables.rb +++ b/lib/api/ci/variables.rb @@ -44,7 +44,7 @@ class Variables < ::API::Base variable = find_variable(user_project, params) not_found!('Variable') unless variable - audit_variable_access(variable, user_project) if user_project.licensed_feature_available?(:audit_events) + audit_variable_access(variable, user_project) present variable, with: Entities::Ci::Variable end diff --git a/lib/api/group_variables.rb b/lib/api/group_variables.rb index 0e2bc09801cbe5..648ce1747787ec 100644 --- a/lib/api/group_variables.rb +++ b/lib/api/group_variables.rb @@ -41,7 +41,7 @@ class GroupVariables < ::API::Base break not_found!('GroupVariable') unless variable - audit_variable_access(variable, user_group) if user_group.licensed_feature_available?(:audit_events) + audit_variable_access(variable, user_group) present variable, with: Entities::Ci::Variable end -- GitLab