diff --git a/config/feature_flags/gitlab_com_derisk/secret_checks_for_web_requests.yml b/config/feature_flags/gitlab_com_derisk/secret_checks_for_web_requests.yml new file mode 100644 index 0000000000000000000000000000000000000000..97a7566eb09ac79d4334b62f87817780b4d3a649 --- /dev/null +++ b/config/feature_flags/gitlab_com_derisk/secret_checks_for_web_requests.yml @@ -0,0 +1,9 @@ +--- +name: secret_checks_for_web_requests +feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/491282 +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/178726 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/521677 +milestone: '17.10' +group: group::secret detection +type: gitlab_com_derisk +default_enabled: false diff --git a/ee/lib/gitlab/checks/secrets_check.rb b/ee/lib/gitlab/checks/secrets_check.rb index 1d36cd42ce1c27dba7f280fe9862986e14793a3d..0f4d39c0eadbb4cda93e3050bde372779235c349 100644 --- a/ee/lib/gitlab/checks/secrets_check.rb +++ b/ee/lib/gitlab/checks/secrets_check.rb @@ -204,7 +204,14 @@ def http_or_ssh_protocol? end def use_diff_scan? - Feature.enabled?(:spp_scan_diffs, project) && http_or_ssh_protocol? + Feature.enabled?(:spp_scan_diffs, project) && (http_or_ssh_protocol? || secrets_check_enabled_for_web_requests?) + end + + def secrets_check_enabled_for_web_requests? + return false if Feature.disabled?(:secret_checks_for_web_requests, project) + return false if changes_access.gitaly_context.nil? + + changes_access.gitaly_context['enable_secrets_check'] == true end def includes_full_revision_history? diff --git a/ee/spec/lib/gitlab/checks/secrets_check_spec.rb b/ee/spec/lib/gitlab/checks/secrets_check_spec.rb index 9fcf993088039522e0f855f66778d1c5018c0fa3..e7a129f150e30a7333aa3d7002403fbf62ae1d98 100644 --- a/ee/spec/lib/gitlab/checks/secrets_check_spec.rb +++ b/ee/spec/lib/gitlab/checks/secrets_check_spec.rb @@ -111,6 +111,7 @@ it_behaves_like 'diff scan passed' it_behaves_like 'scan detected secrets in diffs' it_behaves_like 'detects secrets with special characters in diffs' + it_behaves_like 'processes hunk headers' it 'tracks and recovers errors when getting diff' do expect(repository).to receive(:diff_blobs).and_raise(::GRPC::InvalidArgument) @@ -126,32 +127,40 @@ context 'when the protocol is web' do subject(:secrets_check) { described_class.new(changes_access_web) } - it_behaves_like 'entire file scan passed' - it_behaves_like 'scan detected secrets' - it_behaves_like 'scan detected secrets but some errors occured' - it_behaves_like 'scan timed out' - it_behaves_like 'scan failed to initialize' - it_behaves_like 'scan failed with invalid input' - it_behaves_like 'scan skipped due to invalid status' - it_behaves_like 'scan skipped when a commit has special bypass flag' - it_behaves_like 'scan skipped when secret_push_protection.skip_all push option is passed' - it_behaves_like 'scan discarded secrets because they match exclusions' - it_behaves_like 'detects secrets with special characters in full files' - end - - context 'when the spp_scan_diffs flag is enabled' do - it_behaves_like 'diff scan passed' - it_behaves_like 'scan detected secrets in diffs' - it_behaves_like 'processes hunk headers' - it_behaves_like 'detects secrets with special characters in diffs' - - context 'when the protocol is web' do - subject(:secrets_check) { described_class.new(changes_access_web) } + context 'when the secret_checks_for_web_requests feature flag is disabled' do + before do + stub_feature_flags(secret_checks_for_web_requests: false) + end it_behaves_like 'entire file scan passed' it_behaves_like 'scan detected secrets' + it_behaves_like 'scan detected secrets but some errors occured' + it_behaves_like 'scan timed out' + it_behaves_like 'scan failed to initialize' + it_behaves_like 'scan failed with invalid input' + it_behaves_like 'scan skipped due to invalid status' + it_behaves_like 'scan skipped when a commit has special bypass flag' + it_behaves_like 'scan skipped when secret_push_protection.skip_all push option is passed' + it_behaves_like 'scan discarded secrets because they match exclusions' it_behaves_like 'detects secrets with special characters in full files' end + + context 'when the secret_checks_for_web_requests feature flag is enabled' do + context 'when changes_access.gitaly_context enable_secrets_check is false' do + it_behaves_like 'entire file scan passed' + it_behaves_like 'scan detected secrets' + it_behaves_like 'detects secrets with special characters in full files' + end + + # TODO: Remove this block when `spp_scan_diffs` is removed and cleaned up. + context 'when changes_access.gitaly_context enable_secrets_check is true' do + subject(:secrets_check) { described_class.new(changes_access_web_secrets_check_enabled) } + + it_behaves_like 'diff scan passed' + it_behaves_like 'scan detected secrets in diffs' + it_behaves_like 'detects secrets with special characters in diffs' + end + end end end end diff --git a/ee/spec/support/shared_contexts/secrets_check_shared_contexts.rb b/ee/spec/support/shared_contexts/secrets_check_shared_contexts.rb index 66486c55f776cf6513b0b9bc34534901d0e4c721..21d67e1495a93120077d7eea6c592ecfa1278af0 100644 --- a/ee/spec/support/shared_contexts/secrets_check_shared_contexts.rb +++ b/ee/spec/support/shared_contexts/secrets_check_shared_contexts.rb @@ -119,6 +119,18 @@ ) end + let(:changes_access_web_secrets_check_enabled) do + Gitlab::Checks::ChangesAccess.new( + changes, + project: project, + user_access: user_access, + protocol: 'web', + logger: logger, + push_options: push_options, + gitaly_context: { 'enable_secrets_check' => true } + ) + end + let(:delete_changes_access) do Gitlab::Checks::ChangesAccess.new( delete_changes, diff --git a/lib/gitlab/gitaly_client/conflicts_service.rb b/lib/gitlab/gitaly_client/conflicts_service.rb index 831c5ca1305a42af34641697682627140afdad25..35763960424b2ca5f704dcef7e4ea7f6d42ea256 100644 --- a/lib/gitlab/gitaly_client/conflicts_service.rb +++ b/lib/gitlab/gitaly_client/conflicts_service.rb @@ -55,7 +55,15 @@ def resolve_conflicts(target_repository, resolution, source_branch, target_branc end end - response = gitaly_client_call(@repository.storage, :conflicts_service, :resolve_conflicts, req_enum, remote_storage: target_repository.storage, timeout: GitalyClient.long_timeout) + response = gitaly_client_call( + @repository.storage, + :conflicts_service, + :resolve_conflicts, + req_enum, + remote_storage: target_repository.storage, + timeout: GitalyClient.long_timeout, + gitaly_context: { 'enable_secrets_check' => true } + ) if response.resolution_error.present? raise Gitlab::Git::Conflict::Resolver::ResolutionError, response.resolution_error diff --git a/lib/gitlab/gitaly_client/operation_service.rb b/lib/gitlab/gitaly_client/operation_service.rb index fe352ade7978cffd49603bb8b0c0cc3f601ace5a..c68191e6ec36f9c2b8a74005e3d3ccbf8f38f423 100644 --- a/lib/gitlab/gitaly_client/operation_service.rb +++ b/lib/gitlab/gitaly_client/operation_service.rb @@ -284,7 +284,8 @@ def user_cherry_pick( :user_cherry_pick, request, remote_storage: start_repository.storage, - timeout: GitalyClient.long_timeout + timeout: GitalyClient.long_timeout, + gitaly_context: { 'enable_secrets_check' => true } ) Gitlab::Git::OperationService::BranchUpdate.from_gitaly(response.branch_update) @@ -368,7 +369,8 @@ def rebase(user, rebase_id, branch:, branch_sha:, remote_repository:, remote_bra :user_rebase_confirmable, request_enum.each, timeout: GitalyClient.long_timeout, - remote_storage: remote_repository.storage + remote_storage: remote_repository.storage, + gitaly_context: { 'enable_secrets_check' => true } ) # First request @@ -535,8 +537,14 @@ def user_commit_files( end response = gitaly_client_call( - @repository.storage, :operation_service, :user_commit_files, req_enum, - timeout: GitalyClient.long_timeout, remote_storage: start_repository&.storage) + @repository.storage, + :operation_service, + :user_commit_files, + req_enum, + timeout: GitalyClient.long_timeout, + remote_storage: start_repository&.storage, + gitaly_context: { 'enable_secrets_check' => true } + ) if (pre_receive_error = response.pre_receive_error.presence) raise Gitlab::Git::PreReceiveError, pre_receive_error @@ -588,8 +596,14 @@ def user_commit_patches(user, branch_name:, patches:, target_sha: nil) end end - response = gitaly_client_call(@repository.storage, :operation_service, - :user_apply_patch, chunks, timeout: GitalyClient.long_timeout) + response = gitaly_client_call( + @repository.storage, + :operation_service, + :user_apply_patch, + chunks, + timeout: GitalyClient.long_timeout, + gitaly_context: { 'enable_secrets_check' => true } + ) Gitlab::Git::OperationService::BranchUpdate.from_gitaly(response.branch_update) end