From 7d6b62d2a1f8b664f7a9f6b3fa03f623c6cd6d91 Mon Sep 17 00:00:00 2001 From: Vishnu Narayanan Date: Mon, 6 Jul 2026 17:00:50 +0530 Subject: [PATCH] fix: do not merge into an identified contact without a matching identifier ContactIdentifyAction merges an incoming contact into an existing one that matches by identifier, email, or phone_number. merge_contacts? returned true whenever the request supplied no identifier, short-circuiting before the guard that prevents merging contacts with mismatched identifiers. As a result a request with no identifier could merge into (and overwrite) a contact that is already identified, purely by matching its email or phone_number. Drop the blank-identifier short-circuit so the identifier-mismatch guard runs in that case too: a contact that owns an identifier is only merged into when the request supplies the same identifier. Merges into non-identified contacts (anonymous dedup) are unchanged. --- app/actions/contact_identify_action.rb | 8 +++++--- spec/actions/contact_identify_action_spec.rb | 18 ++++++++++++++++++ 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/app/actions/contact_identify_action.rb b/app/actions/contact_identify_action.rb index 9afa5d019..0e8cf2f61 100644 --- a/app/actions/contact_identify_action.rb +++ b/app/actions/contact_identify_action.rb @@ -72,9 +72,11 @@ class ContactIdentifyAction def merge_contacts?(existing_contact, key) return if existing_contact.blank? - return true if params[:identifier].blank? - - # we want to prevent merging contacts with different identifiers + # Never merge into a contact that already owns an identifier unless the request + # supplies the matching one. This also covers the blank-identifier case: an + # unverified/anonymous request must not take over an identified contact just by + # matching its email or phone_number. Merges into non-identified contacts + # (anonymous dedup) are still allowed. if existing_contact.identifier.present? && existing_contact.identifier != params[:identifier] # we will remove attribute from update list @attributes_to_update.delete(key) diff --git a/spec/actions/contact_identify_action_spec.rb b/spec/actions/contact_identify_action_spec.rb index 76b370f35..70b977d69 100644 --- a/spec/actions/contact_identify_action_spec.rb +++ b/spec/actions/contact_identify_action_spec.rb @@ -107,6 +107,24 @@ describe ContactIdentifyAction do end end + context 'when a request without an identifier matches an already identified contact' do + it 'does not merge into the identified contact via a matching email' do + victim = create(:contact, account: account, identifier: 'victim_id', email: 'victim@test.com', name: 'Victim') + params = { email: 'victim@test.com', name: 'Attacker' } + result = described_class.new(contact: contact, params: params).perform + expect(result.id).not_to eq victim.id + expect(victim.reload.name).to eq 'Victim' + end + + it 'does not merge into the identified contact via a matching phone_number' do + victim = create(:contact, account: account, identifier: 'victim_id', phone_number: '+919999888877', name: 'Victim') + params = { phone_number: '+919999888877', name: 'Attacker' } + result = described_class.new(contact: contact, params: params).perform + expect(result.id).not_to eq victim.id + expect(victim.reload.name).to eq 'Victim' + end + end + context 'when contacts with blank identifiers exist and identify action is called with blank identifier' do it 'updates the attributes of contact passed in to identify action' do create(:contact, account: account, identifier: '')