From 672b98f17b1e66a581c967ae52d67bb39b05d53c Mon Sep 17 00:00:00 2001 From: "j.seto" Date: Fri, 8 Aug 2025 16:54:22 -0400 Subject: [PATCH 1/2] Refactor Gpg::Commit to delegate signature methods --- .../gpg_commit_delegate_to_signature.yml | 10 + lib/gitlab/gpg/commit.rb | 54 +- spec/lib/gitlab/gpg/commit_spec.rb | 564 +++++++++--------- 3 files changed, 350 insertions(+), 278 deletions(-) create mode 100644 config/feature_flags/gitlab_com_derisk/gpg_commit_delegate_to_signature.yml diff --git a/config/feature_flags/gitlab_com_derisk/gpg_commit_delegate_to_signature.yml b/config/feature_flags/gitlab_com_derisk/gpg_commit_delegate_to_signature.yml new file mode 100644 index 00000000000000..cd18f7b9984cd4 --- /dev/null +++ b/config/feature_flags/gitlab_com_derisk/gpg_commit_delegate_to_signature.yml @@ -0,0 +1,10 @@ +--- +name: gpg_commit_delegate_to_signature +description: +feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/19260 +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/200870 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/560641 +milestone: '18.4' +group: group::source code +type: gitlab_com_derisk +default_enabled: false diff --git a/lib/gitlab/gpg/commit.rb b/lib/gitlab/gpg/commit.rb index 60f62948b076d6..568b8d9836a20d 100644 --- a/lib/gitlab/gpg/commit.rb +++ b/lib/gitlab/gpg/commit.rb @@ -4,8 +4,12 @@ module Gitlab module Gpg class Commit < Gitlab::Repositories::BaseSignedCommit def update_signature!(cached_signature) - using_keychain do |gpg_key| - update_signature_with_keychain!(cached_signature, gpg_key) + if using_signature_class? + update_signature_with_keychain!(cached_signature, gpg_signature.gpg_key) + else + using_keychain do |gpg_key| + update_signature_with_keychain!(cached_signature, gpg_key) + end end end @@ -16,6 +20,10 @@ def update_signature_with_keychain!(cached_signature, gpg_key) private + def project + @commit.project + end + def signature_class CommitSignatures::GpgSignature end @@ -49,11 +57,20 @@ def verified_signature end def create_cached_signature! - using_keychain do |gpg_key| - attributes = attributes(gpg_key) - break CommitSignatures::GpgSignature.new(attributes) if Gitlab::Database.read_only? + if using_signature_class? + return unless gpg_signature.fingerprint + + attributes = attributes(gpg_signature.gpg_key) + return CommitSignatures::GpgSignature.new(attributes) if Gitlab::Database.read_only? CommitSignatures::GpgSignature.safe_create!(attributes) + else + using_keychain do |gpg_key| + attributes = attributes(gpg_key) + break CommitSignatures::GpgSignature.new(attributes) if Gitlab::Database.read_only? + + CommitSignatures::GpgSignature.safe_create!(attributes) + end end end @@ -81,13 +98,19 @@ def attributes(gpg_key) { commit_sha: @commit.sha, - project: @commit.project, + project: project, gpg_key: gpg_key, - gpg_key_primary_keyid: gpg_key&.keyid || verified_signature&.fingerprint, gpg_key_user_name: user_infos[:name], - gpg_key_user_email: gpg_key_user_email(user_infos, verification_status), - verification_status: verification_status - } + gpg_key_user_email: gpg_key_user_email(user_infos, verification_status) + }.tap do |attrs| + if using_signature_class? + attrs[:gpg_key_primary_keyid] = gpg_key&.keyid || gpg_signature.fingerprint + attrs[:verification_status] = gpg_signature.verification_status + else + attrs[:gpg_key_primary_keyid] = gpg_key&.keyid || verified_signature&.fingerprint + attrs[:verification_status] = verification_status + end + end end def verification_status(gpg_key) @@ -129,10 +152,19 @@ def find_gpg_key(fingerprint) def gpg_key_user_email(user_infos, verification_status) return user_infos[:email] unless Feature.enabled?(:check_for_mailmapped_commit_emails, - @commit.project) && verification_status == :verified_system + project) && verification_status == :verified_system user_infos[:email] || author_email end + + def gpg_signature + ::Gitlab::Gpg::Signature.new(signature_text, signed_text, signer, @commit.committer_email) + end + strong_memoize_attr :gpg_signature + + def using_signature_class? + Feature.enabled?(:gpg_commit_delegate_to_signature, project) + end end end end diff --git a/spec/lib/gitlab/gpg/commit_spec.rb b/spec/lib/gitlab/gpg/commit_spec.rb index a88f84afaedc10..bdf0e876ac37ed 100644 --- a/spec/lib/gitlab/gpg/commit_spec.rb +++ b/spec/lib/gitlab/gpg/commit_spec.rb @@ -26,6 +26,8 @@ } end + let(:use_signature_class) { true } + before do if mock_signature_data? allow(Gitlab::Git::Commit).to receive(:extract_signature_lazily) @@ -34,69 +36,114 @@ end end - describe '#signature' do - shared_examples 'returns the cached signature on second call' do - it 'returns the cached signature on second call' do - gpg_commit = described_class.new(commit) + shared_examples_for 'gpg commit' do |use_signature_class| + before do + stub_feature_flags(gpg_commit_delegate_to_signature: use_signature_class) + end - expect(gpg_commit).to receive(:using_keychain).and_call_original - gpg_commit.signature + describe '#signature' do + shared_examples 'returns the cached signature on second call' do |testing_signature_class| + if testing_signature_class + it 'returns the cached signature on second call' do + gpg_commit = described_class.new(commit) - # consecutive call - expect(gpg_commit).not_to receive(:using_keychain).and_call_original - gpg_commit.signature - end - end + expect_next_instance_of(Gitlab::Gpg::Signature) do |signature| + expect(signature).to receive(:using_keychain).once.and_call_original + end + + 2.times do + gpg_commit.signature + end + end + else + context 'when the render_gpg_signed_tags_verification_status feature flag is not enabled' do + it 'returns the cached signature on second call' do + gpg_commit = described_class.new(commit) - context 'unsigned commit' do - let(:signature_data) { nil } + expect(gpg_commit).to receive(:using_keychain).and_call_original + gpg_commit.signature - it 'returns nil' do - expect(described_class.new(commit).signature).to be_nil + # consecutive call + expect(gpg_commit).not_to receive(:using_keychain).and_call_original + gpg_commit.signature + end + end + end end - end - context 'invalid signature' do - let(:signature_data) do - { - # Corrupt the key - signature: GpgHelpers::User1.signed_commit_signature.tr('=', 'a'), - signed_text: GpgHelpers::User1.signed_commit_base_data, - signer: signer - } + context 'unsigned commit' do + let(:signature_data) { nil } + + it 'returns nil' do + expect(described_class.new(commit).signature).to be_nil + end end - it 'returns nil' do - expect(described_class.new(commit).signature).to be_nil + context 'invalid signature' do + let(:signature_data) do + { + # Corrupt the key + signature: GpgHelpers::User1.signed_commit_signature.tr('=', 'a'), + signed_text: GpgHelpers::User1.signed_commit_base_data, + signer: signer + } + end + + it 'returns nil' do + expect(described_class.new(commit).signature).to be_nil + end end - end - context 'known key' do - context 'user matches the key uid' do - context 'user email matches the email committer' do - it 'returns a valid signature' do - signature = described_class.new(commit).signature + context 'known key' do + context 'user matches the key uid' do + context 'user email matches the email committer' do + it 'returns a valid signature' do + signature = described_class.new(commit).signature - expect(signature).to have_attributes( - commit_sha: commit_sha, - project: project, - gpg_key: gpg_key, - gpg_key_primary_keyid: GpgHelpers::User1.primary_keyid, - gpg_key_user_name: GpgHelpers::User1.names.first, - gpg_key_user_email: GpgHelpers::User1.emails.first, - verification_status: 'verified' - ) - expect(signature.persisted?).to be_truthy - end + expect(signature).to have_attributes( + commit_sha: commit_sha, + project: project, + gpg_key: gpg_key, + gpg_key_primary_keyid: GpgHelpers::User1.primary_keyid, + gpg_key_user_name: GpgHelpers::User1.names.first, + gpg_key_user_email: GpgHelpers::User1.emails.first, + verification_status: 'verified' + ) + expect(signature.persisted?).to be_truthy + end - it_behaves_like 'returns the cached signature on second call' + it_behaves_like 'returns the cached signature on second call', use_signature_class + + context 'read-only mode' do + before do + allow(Gitlab::Database).to receive(:read_only?).and_return(true) + end + + it 'does not create a cached signature' do + signature = described_class.new(commit).signature + + expect(signature).to have_attributes( + commit_sha: commit_sha, + project: project, + gpg_key: gpg_key, + gpg_key_primary_keyid: GpgHelpers::User1.primary_keyid, + gpg_key_user_name: GpgHelpers::User1.names.first, + gpg_key_user_email: GpgHelpers::User1.emails.first, + verification_status: 'verified' + ) + expect(signature.persisted?).to be_falsey + end + end + end - context 'read-only mode' do + context 'valid key signed using recent version of Gnupg' do before do - allow(Gitlab::Database).to receive(:read_only?).and_return(true) + verified_signature = double('verified-signature', fingerprint: GpgHelpers::User1.fingerprint, valid?: true) + allow(GPGME::Crypto).to receive(:new).and_return(crypto) + allow(crypto).to receive(:verify).and_yield(verified_signature) end - it 'does not create a cached signature' do + it 'returns a valid signature' do signature = described_class.new(commit).signature expect(signature).to have_attributes( @@ -108,160 +155,156 @@ gpg_key_user_email: GpgHelpers::User1.emails.first, verification_status: 'verified' ) - expect(signature.persisted?).to be_falsey end end - end - context 'valid key signed using recent version of Gnupg' do - before do - verified_signature = double('verified-signature', fingerprint: GpgHelpers::User1.fingerprint, valid?: true) - allow(GPGME::Crypto).to receive(:new).and_return(crypto) - allow(crypto).to receive(:verify).and_yield(verified_signature) - end + context 'valid key signed using older version of Gnupg' do + before do + keyid = GpgHelpers::User1.fingerprint.last(16) + verified_signature = double('verified-signature', fingerprint: keyid, valid?: true) + allow(GPGME::Crypto).to receive(:new).and_return(crypto) + allow(crypto).to receive(:verify).and_yield(verified_signature) + end - it 'returns a valid signature' do - signature = described_class.new(commit).signature + it 'returns a valid signature' do + signature = described_class.new(commit).signature - expect(signature).to have_attributes( - commit_sha: commit_sha, - project: project, - gpg_key: gpg_key, - gpg_key_primary_keyid: GpgHelpers::User1.primary_keyid, - gpg_key_user_name: GpgHelpers::User1.names.first, - gpg_key_user_email: GpgHelpers::User1.emails.first, - verification_status: 'verified' - ) + expect(signature).to have_attributes( + commit_sha: commit_sha, + project: project, + gpg_key: gpg_key, + gpg_key_primary_keyid: GpgHelpers::User1.primary_keyid, + gpg_key_user_name: GpgHelpers::User1.names.first, + gpg_key_user_email: GpgHelpers::User1.emails.first, + verification_status: 'verified' + ) + end end - end - context 'valid key signed using older version of Gnupg' do - before do - keyid = GpgHelpers::User1.fingerprint.last(16) - verified_signature = double('verified-signature', fingerprint: keyid, valid?: true) - allow(GPGME::Crypto).to receive(:new).and_return(crypto) - allow(crypto).to receive(:verify).and_yield(verified_signature) - end + context 'commit with multiple signatures' do + before do + verified_signature = double('verified-signature', fingerprint: GpgHelpers::User1.fingerprint, valid?: true) + allow(GPGME::Crypto).to receive(:new).and_return(crypto) + allow(crypto).to receive(:verify).and_yield(verified_signature).and_yield(verified_signature) + end - it 'returns a valid signature' do - signature = described_class.new(commit).signature + it 'returns an invalid signatures error' do + signature = described_class.new(commit).signature - expect(signature).to have_attributes( - commit_sha: commit_sha, - project: project, - gpg_key: gpg_key, - gpg_key_primary_keyid: GpgHelpers::User1.primary_keyid, - gpg_key_user_name: GpgHelpers::User1.names.first, - gpg_key_user_email: GpgHelpers::User1.emails.first, - verification_status: 'verified' - ) + expect(signature).to have_attributes( + commit_sha: commit_sha, + project: project, + gpg_key: gpg_key, + gpg_key_primary_keyid: GpgHelpers::User1.primary_keyid, + gpg_key_user_name: GpgHelpers::User1.names.first, + gpg_key_user_email: GpgHelpers::User1.emails.first, + verification_status: 'multiple_signatures' + ) + end end - end - context 'commit with multiple signatures' do - before do - verified_signature = double('verified-signature', fingerprint: GpgHelpers::User1.fingerprint, valid?: true) - allow(GPGME::Crypto).to receive(:new).and_return(crypto) - allow(crypto).to receive(:verify).and_yield(verified_signature).and_yield(verified_signature) - end + context 'commit signed with a subkey' do + let(:committer_email) { GpgHelpers::User3.emails.first } + let(:public_key) { GpgHelpers::User3.public_key } - it 'returns an invalid signatures error' do - signature = described_class.new(commit).signature + let(:gpg_key_subkey) do + gpg_key.subkeys.find_by(fingerprint: GpgHelpers::User3.subkey_fingerprints.last) + end - expect(signature).to have_attributes( - commit_sha: commit_sha, - project: project, - gpg_key: gpg_key, - gpg_key_primary_keyid: GpgHelpers::User1.primary_keyid, - gpg_key_user_name: GpgHelpers::User1.names.first, - gpg_key_user_email: GpgHelpers::User1.emails.first, - verification_status: 'multiple_signatures' - ) - end - end + let(:signature_data) do + { + signature: GpgHelpers::User3.signed_commit_signature, + signed_text: GpgHelpers::User3.signed_commit_base_data, + signer: signer + } + end - context 'commit signed with a subkey' do - let(:committer_email) { GpgHelpers::User3.emails.first } - let(:public_key) { GpgHelpers::User3.public_key } + it 'returns a valid signature' do + expect(described_class.new(commit).signature).to have_attributes( + commit_sha: commit_sha, + project: project, + gpg_key: gpg_key_subkey, + gpg_key_primary_keyid: gpg_key_subkey.keyid, + gpg_key_user_name: GpgHelpers::User3.names.first, + gpg_key_user_email: GpgHelpers::User3.emails.first, + verification_status: 'verified' + ) + end - let(:gpg_key_subkey) do - gpg_key.subkeys.find_by(fingerprint: GpgHelpers::User3.subkey_fingerprints.last) + it_behaves_like 'returns the cached signature on second call', use_signature_class end - let(:signature_data) do - { - signature: GpgHelpers::User3.signed_commit_signature, - signed_text: GpgHelpers::User3.signed_commit_base_data, - signer: signer - } - end + context 'gpg key email does not match the committer_email but is the same user when the committer_email belongs to the user as a confirmed secondary email' do + let(:committer_email) { GpgHelpers::User2.emails.first } - it 'returns a valid signature' do - expect(described_class.new(commit).signature).to have_attributes( - commit_sha: commit_sha, - project: project, - gpg_key: gpg_key_subkey, - gpg_key_primary_keyid: gpg_key_subkey.keyid, - gpg_key_user_name: GpgHelpers::User3.names.first, - gpg_key_user_email: GpgHelpers::User3.emails.first, - verification_status: 'verified' - ) + let(:user) do + create(:user, email: GpgHelpers::User1.emails.first).tap do |user| + create :email, :confirmed, user: user, email: committer_email + end + end + + it 'returns an invalid signature' do + expect(described_class.new(commit).signature).to have_attributes( + commit_sha: commit_sha, + project: project, + gpg_key: gpg_key, + gpg_key_primary_keyid: GpgHelpers::User1.primary_keyid, + gpg_key_user_name: GpgHelpers::User1.names.first, + gpg_key_user_email: GpgHelpers::User1.emails.first, + verification_status: 'same_user_different_email' + ) + end + + it_behaves_like 'returns the cached signature on second call', use_signature_class end - it_behaves_like 'returns the cached signature on second call' - end + context 'gpg key email does not match the committer_email when the committer_email belongs to the user as a unconfirmed secondary email' do + let(:committer_email) { GpgHelpers::User2.emails.first } - context 'gpg key email does not match the committer_email but is the same user when the committer_email belongs to the user as a confirmed secondary email' do - let(:committer_email) { GpgHelpers::User2.emails.first } + let(:user) do + create(:user, email: GpgHelpers::User1.emails.first).tap do |user| + create :email, user: user, email: committer_email + end + end - let(:user) do - create(:user, email: GpgHelpers::User1.emails.first).tap do |user| - create :email, :confirmed, user: user, email: committer_email + it 'returns an invalid signature' do + expect(described_class.new(commit).signature).to have_attributes( + commit_sha: commit_sha, + project: project, + gpg_key: gpg_key, + gpg_key_primary_keyid: GpgHelpers::User1.primary_keyid, + gpg_key_user_name: GpgHelpers::User1.names.first, + gpg_key_user_email: GpgHelpers::User1.emails.first, + verification_status: 'other_user' + ) end - end - it 'returns an invalid signature' do - expect(described_class.new(commit).signature).to have_attributes( - commit_sha: commit_sha, - project: project, - gpg_key: gpg_key, - gpg_key_primary_keyid: GpgHelpers::User1.primary_keyid, - gpg_key_user_name: GpgHelpers::User1.names.first, - gpg_key_user_email: GpgHelpers::User1.emails.first, - verification_status: 'same_user_different_email' - ) + it_behaves_like 'returns the cached signature on second call', use_signature_class end - it_behaves_like 'returns the cached signature on second call' - end - - context 'gpg key email does not match the committer_email when the committer_email belongs to the user as a unconfirmed secondary email' do - let(:committer_email) { GpgHelpers::User2.emails.first } + context 'user email does not match the committer email' do + let(:committer_email) { GpgHelpers::User2.emails.first } + let(:user_email) { GpgHelpers::User1.emails.first } - let(:user) do - create(:user, email: GpgHelpers::User1.emails.first).tap do |user| - create :email, user: user, email: committer_email + it 'returns an invalid signature' do + expect(described_class.new(commit).signature).to have_attributes( + commit_sha: commit_sha, + project: project, + gpg_key: gpg_key, + gpg_key_primary_keyid: GpgHelpers::User1.primary_keyid, + gpg_key_user_name: GpgHelpers::User1.names.first, + gpg_key_user_email: GpgHelpers::User1.emails.first, + verification_status: 'other_user' + ) end - end - it 'returns an invalid signature' do - expect(described_class.new(commit).signature).to have_attributes( - commit_sha: commit_sha, - project: project, - gpg_key: gpg_key, - gpg_key_primary_keyid: GpgHelpers::User1.primary_keyid, - gpg_key_user_name: GpgHelpers::User1.names.first, - gpg_key_user_email: GpgHelpers::User1.emails.first, - verification_status: 'other_user' - ) + it_behaves_like 'returns the cached signature on second call', use_signature_class end - - it_behaves_like 'returns the cached signature on second call' end - context 'user email does not match the committer email' do - let(:committer_email) { GpgHelpers::User2.emails.first } - let(:user_email) { GpgHelpers::User1.emails.first } + context 'user does not match the key uid' do + let(:user_email) { GpgHelpers::User2.emails.first } + let(:public_key) { GpgHelpers::User1.public_key } it 'returns an invalid signature' do expect(described_class.new(commit).signature).to have_attributes( @@ -271,100 +314,63 @@ gpg_key_primary_keyid: GpgHelpers::User1.primary_keyid, gpg_key_user_name: GpgHelpers::User1.names.first, gpg_key_user_email: GpgHelpers::User1.emails.first, - verification_status: 'other_user' + verification_status: 'unverified_key' ) end - it_behaves_like 'returns the cached signature on second call' + it_behaves_like 'returns the cached signature on second call', use_signature_class end end - context 'user does not match the key uid' do - let(:user_email) { GpgHelpers::User2.emails.first } - let(:public_key) { GpgHelpers::User1.public_key } + context 'unknown key' do + let(:gpg_key) { nil } it 'returns an invalid signature' do expect(described_class.new(commit).signature).to have_attributes( commit_sha: commit_sha, project: project, - gpg_key: gpg_key, + gpg_key: nil, gpg_key_primary_keyid: GpgHelpers::User1.primary_keyid, - gpg_key_user_name: GpgHelpers::User1.names.first, - gpg_key_user_email: GpgHelpers::User1.emails.first, - verification_status: 'unverified_key' + gpg_key_user_name: nil, + gpg_key_user_email: nil, + verification_status: 'unknown_key' ) end - it_behaves_like 'returns the cached signature on second call' - end - end - - context 'unknown key' do - let(:gpg_key) { nil } - - it 'returns an invalid signature' do - expect(described_class.new(commit).signature).to have_attributes( - commit_sha: commit_sha, - project: project, - gpg_key: nil, - gpg_key_primary_keyid: GpgHelpers::User1.primary_keyid, - gpg_key_user_name: nil, - gpg_key_user_email: nil, - verification_status: 'unknown_key' - ) + it_behaves_like 'returns the cached signature on second call', use_signature_class end - it_behaves_like 'returns the cached signature on second call' - end - - context 'multiple commits with signatures' do - let(:mock_signature_data?) { false } + context 'multiple commits with signatures' do + let(:mock_signature_data?) { false } - let!(:first_signature) { create(:gpg_signature) } - let!(:gpg_key) { create(:gpg_key, key: GpgHelpers::User2.public_key) } - let!(:second_signature) { create(:gpg_signature, gpg_key: gpg_key) } - let!(:first_commit) { create(:commit, project: project, sha: first_signature.commit_sha) } - let!(:second_commit) { create(:commit, project: project, sha: second_signature.commit_sha) } + let!(:first_signature) { create(:gpg_signature) } + let!(:gpg_key) { create(:gpg_key, key: GpgHelpers::User2.public_key) } + let!(:second_signature) { create(:gpg_signature, gpg_key: gpg_key) } + let!(:first_commit) { create(:commit, project: project, sha: first_signature.commit_sha) } + let!(:second_commit) { create(:commit, project: project, sha: second_signature.commit_sha) } - let!(:commits) do - [first_commit, second_commit].map do |commit| - gpg_commit = described_class.new(commit) + let!(:commits) do + [first_commit, second_commit].map do |commit| + gpg_commit = described_class.new(commit) - allow(gpg_commit).to receive(:has_signature?).and_return(true) + allow(gpg_commit).to receive(:has_signature?).and_return(true) - gpg_commit - end - end - - it 'does an aggregated sql request instead of 2 separate ones' do - recorder = ActiveRecord::QueryRecorder.new do - commits.each(&:signature) + gpg_commit + end end - expect(recorder.count).to eq(1) - end - end + it 'does an aggregated sql request instead of 2 separate ones' do + recorder = ActiveRecord::QueryRecorder.new do + commits.each(&:signature) + end - context 'when signature created by GitLab' do - let(:signer) { :SIGNER_SYSTEM } - let(:gpg_key) { nil } - - it 'returns a valid signature' do - expect(described_class.new(commit).signature).to have_attributes( - commit_sha: commit_sha, - project: project, - gpg_key: nil, - gpg_key_primary_keyid: GpgHelpers::User1.primary_keyid, - gpg_key_user_name: nil, - gpg_key_user_email: user_email, - verification_status: 'verified_system' - ) + expect(recorder.count).to eq(1) + end end - context 'when check_for_mailmapped_commit_emails feature flag is disabled' do - before do - stub_feature_flags(check_for_mailmapped_commit_emails: false) - end + context 'when signature created by GitLab' do + let(:signer) { :SIGNER_SYSTEM } + let(:gpg_key) { nil } it 'returns a valid signature' do expect(described_class.new(commit).signature).to have_attributes( @@ -373,61 +379,85 @@ gpg_key: nil, gpg_key_primary_keyid: GpgHelpers::User1.primary_keyid, gpg_key_user_name: nil, - gpg_key_user_email: nil, + gpg_key_user_email: user_email, verification_status: 'verified_system' ) end - end - it_behaves_like 'returns the cached signature on second call' - end - end - - describe '#update_signature!' do - let!(:gpg_key) { nil } - - let(:signature) { described_class.new(commit).signature } - - it 'updates signature record' do - signature + context 'when check_for_mailmapped_commit_emails feature flag is disabled' do + before do + stub_feature_flags(check_for_mailmapped_commit_emails: false) + end - create(:gpg_key, key: public_key, user: user) + it 'returns a valid signature' do + expect(described_class.new(commit).signature).to have_attributes( + commit_sha: commit_sha, + project: project, + gpg_key: nil, + gpg_key_primary_keyid: GpgHelpers::User1.primary_keyid, + gpg_key_user_name: nil, + gpg_key_user_email: nil, + verification_status: 'verified_system' + ) + end + end - stored_signature = CommitSignatures::GpgSignature.find_by_commit_sha(commit_sha) - expect { described_class.new(commit).update_signature!(stored_signature) }.to( - change { signature.reload.verification_status }.from('unknown_key').to('verified') - ) + it_behaves_like 'returns the cached signature on second call', use_signature_class + end end - context 'when signature is system verified and gpg_key_user_email is nil' do - let(:signer) { :SIGNER_SYSTEM } + describe '#update_signature!' do + let!(:gpg_key) { nil } - it 'update gpg_key_user_email with signature_data author_email' do + let(:signature) { described_class.new(commit).signature } + + it 'updates signature record' do signature - stored_signature = CommitSignatures::GpgSignature.find_by_commit_sha(commit_sha) - stored_signature.update!(gpg_key_user_email: nil) + create(:gpg_key, key: public_key, user: user) + stored_signature = CommitSignatures::GpgSignature.find_by_commit_sha(commit_sha) expect { described_class.new(commit).update_signature!(stored_signature) }.to( - change { signature.reload.gpg_key_user_email }.from(nil).to(user_email) + change { signature.reload.verification_status }.from('unknown_key').to('verified') ) end - context 'when check_for_mailmapped_commit_emails feature flag is disabled' do - before do - stub_feature_flags(check_for_mailmapped_commit_emails: false) - end + context 'when signature is system verified and gpg_key_user_email is nil' do + let(:signer) { :SIGNER_SYSTEM } - it 'does not update gpg_key_user_email with signature_data author_email' do + it 'update gpg_key_user_email with signature_data author_email' do signature stored_signature = CommitSignatures::GpgSignature.find_by_commit_sha(commit_sha) stored_signature.update!(gpg_key_user_email: nil) expect { described_class.new(commit).update_signature!(stored_signature) }.to( - not_change { signature.reload.gpg_key_user_email }) + change { signature.reload.gpg_key_user_email }.from(nil).to(user_email) + ) + end + + context 'when check_for_mailmapped_commit_emails feature flag is disabled' do + before do + stub_feature_flags(check_for_mailmapped_commit_emails: false) + end + + it 'does not update gpg_key_user_email with signature_data author_email' do + signature + + stored_signature = CommitSignatures::GpgSignature.find_by_commit_sha(commit_sha) + stored_signature.update!(gpg_key_user_email: nil) + + expect { described_class.new(commit).update_signature!(stored_signature) }.to( + not_change { signature.reload.gpg_key_user_email }) + end end end end end + + context 'when the render_gpg_signed_tags_verification_status feature flag is not enabled' do + it_behaves_like 'gpg commit', false + end + + it_behaves_like 'gpg commit', true end -- GitLab From 5605a32c5c2ed5836c14f9923284df074a97bcad Mon Sep 17 00:00:00 2001 From: "j.seto" Date: Mon, 18 Aug 2025 09:30:01 -0400 Subject: [PATCH 2/2] correct ff name --- spec/lib/gitlab/gpg/commit_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/lib/gitlab/gpg/commit_spec.rb b/spec/lib/gitlab/gpg/commit_spec.rb index bdf0e876ac37ed..16b3cfe84b2dd4 100644 --- a/spec/lib/gitlab/gpg/commit_spec.rb +++ b/spec/lib/gitlab/gpg/commit_spec.rb @@ -56,7 +56,7 @@ end end else - context 'when the render_gpg_signed_tags_verification_status feature flag is not enabled' do + context 'when the gpg_commit_delegate_to_signature feature flag is not enabled' do it 'returns the cached signature on second call' do gpg_commit = described_class.new(commit) @@ -455,7 +455,7 @@ end end - context 'when the render_gpg_signed_tags_verification_status feature flag is not enabled' do + context 'when the gpg_commit_delegate_to_signature feature flag is not enabled' do it_behaves_like 'gpg commit', false end -- GitLab