fix: address PR #228 review feedback - strong params, guards, and safety fixes

Author: CayoPOliveira

app/controllers/api/v1/accounts/contacts/group_join_requests_controller.rb

@@ -9,14 +9,18 @@ def index
 
   def handle
     authorize @contact, :update?
-    channel.handle_group_join_requests(@contact.identifier, params[:participants], params[:request_action])
+    channel.handle_group_join_requests(@contact.identifier, handle_params[:participants], handle_params[:request_action])
     head :ok
   rescue Whatsapp::Providers::WhatsappBaileysService::ProviderUnavailableError => e
     render json: { error: e.message }, status: :unprocessable_entity
   end
 
   private
 
+  def handle_params
+    params.permit(:request_action, participants: [])
+  end
+
   def channel
     @channel ||= @contact.group_channel
   end

app/controllers/api/v1/accounts/contacts/group_members_controller.rb

@@ -11,8 +11,8 @@ def index
                             .includes(:contact)
 
     @total_count = base_query.count
-    @page = (params[:page] || 1).to_i
-    @per_page = (params[:per_page] || DEFAULT_PER_PAGE).to_i
+    @page = [(params[:page] || 1).to_i, 1].max
+    @per_page = (params[:per_page] || DEFAULT_PER_PAGE).to_i.clamp(1, 100)
     @inbox_phone_number = inbox_phone_number
 
     paginated = base_query.order(role: :desc, id: :asc)
@@ -24,21 +24,25 @@ def index
 
   def create
     authorize @contact, :update?
+    participants = create_params[:participants]
+    return render json: { error: 'participants_required' }, status: :unprocessable_entity if participants.blank?
 
-    channel.update_group_participants(@contact.identifier, format_participants(params[:participants]), 'add')
-    add_group_members(params[:participants])
+    channel.update_group_participants(@contact.identifier, format_participants(participants), 'add')
+    add_group_members(participants)
     head :ok
   rescue Whatsapp::Providers::WhatsappBaileysService::ProviderUnavailableError => e
     render json: { error: e.message }, status: :unprocessable_entity
   end
 
   def update
     authorize @contact, :update?
+    role = update_params[:role]
+    return render json: { error: 'invalid_role' }, status: :unprocessable_entity unless %w[admin member].include?(role)
 
     member = group_members.find(params[:member_id])
-    action = params[:role] == 'admin' ? 'promote' : 'demote'
+    action = role == 'admin' ? 'promote' : 'demote'
     channel.update_group_participants(@contact.identifier, [jid_for_member(member)], action)
-    member.update!(role: params[:role])
+    member.update!(role: role)
     head :ok
   rescue Whatsapp::Providers::WhatsappBaileysService::GroupParticipantNotAllowedError
     render json: { error: 'group_creator_not_modifiable' }, status: :unprocessable_entity
@@ -71,6 +75,14 @@ def group_members
     GroupMember.where(group_contact: @contact)
   end
 
+  def create_params
+    params.permit(participants: [])
+  end
+
+  def update_params
+    params.permit(:role)
+  end
+
   def channel
     @channel ||= @contact.group_channel
   end
@@ -112,7 +124,9 @@ def jid_for_member(member)
   def add_group_members(phone_numbers)
     inbox = @contact.contact_inboxes.first&.inbox
     Array(phone_numbers).each do |phone|
-      normalized = phone.start_with?('+') ? phone : "+#{phone}"
+      normalized = normalize_phone(phone)
+      next if normalized.blank?
+
       contact_inbox = ::ContactInboxWithContactBuilder.new(
         source_id: normalized.delete('+'),
         inbox: inbox,
@@ -124,4 +138,11 @@ def add_group_members(phone_numbers)
       member.update!(role: :member, is_active: true) unless member.persisted? && member.is_active?
     end
   end
+
+  def normalize_phone(phone)
+    cleaned = phone.to_s.strip
+    return nil if cleaned.blank?
+
+    cleaned.start_with?('+') ? cleaned : "+#{cleaned}"
+  end
 end

app/controllers/api/v1/accounts/contacts/group_metadata_controller.rb

@@ -1,23 +1,27 @@
 class Api::V1::Accounts::Contacts::GroupMetadataController < Api::V1::Accounts::Contacts::BaseController
   def update
     authorize @contact, :update?
-    update_subject if params[:subject].present?
-    update_description if params[:description].present?
+    update_subject if metadata_params[:subject].present?
+    update_description if metadata_params[:description].present?
     render json: { id: @contact.id, name: @contact.name, additional_attributes: @contact.additional_attributes }
   rescue Whatsapp::Providers::WhatsappBaileysService::ProviderUnavailableError => e
     render json: { error: e.message }, status: :unprocessable_entity
   end
 
   private
 
+  def metadata_params
+    params.permit(:subject, :description)
+  end
+
   def update_subject
-    channel.update_group_subject(@contact.identifier, params[:subject])
-    @contact.update!(name: params[:subject])
+    channel.update_group_subject(@contact.identifier, metadata_params[:subject])
+    @contact.update!(name: metadata_params[:subject])
   end
 
   def update_description
-    channel.update_group_description(@contact.identifier, params[:description])
-    attrs = @contact.additional_attributes.merge('description' => params[:description])
+    channel.update_group_description(@contact.identifier, metadata_params[:description])
+    attrs = @contact.additional_attributes.merge('description' => metadata_params[:description])
     @contact.update!(additional_attributes: attrs)
   end
 

app/controllers/api/v1/accounts/contacts/group_settings_controller.rb

@@ -10,24 +10,39 @@ def leave
 
   def update
     authorize @contact, :update?
-    channel.group_setting_update(@contact.identifier, params[:setting])
-    update_contact_setting(params[:setting])
+    setting = setting_params[:setting]
+    valid_settings = %w[announcement not_announcement locked unlocked]
+    return render json: { error: 'invalid_setting' }, status: :unprocessable_entity unless setting.in?(valid_settings)
+
+    channel.group_setting_update(@contact.identifier, setting)
+    update_contact_setting(setting)
     head :ok
   rescue Whatsapp::Providers::WhatsappBaileysService::ProviderUnavailableError => e
     render json: { error: e.message }, status: :unprocessable_entity
   end
 
   def toggle_join_approval
     authorize @contact, :update?
-    channel.group_join_approval_mode(@contact.identifier, params[:mode])
-    update_contact_attribute('join_approval_mode', params[:mode] == 'on')
+    mode = join_approval_params[:mode]
+    return render json: { error: 'invalid_mode' }, status: :unprocessable_entity unless mode.in?(%w[on off])
+
+    channel.group_join_approval_mode(@contact.identifier, mode)
+    update_contact_attribute('join_approval_mode', mode == 'on')
     head :ok
   rescue Whatsapp::Providers::WhatsappBaileysService::ProviderUnavailableError => e
     render json: { error: e.message }, status: :unprocessable_entity
   end
 
   private
 
+  def setting_params
+    params.permit(:setting)
+  end
+
+  def join_approval_params
+    params.permit(:mode)
+  end
+
   def channel
     @channel ||= @contact.group_channel
   end

app/controllers/api/v1/accounts/groups_controller.rb

@@ -1,12 +1,12 @@
 class Api::V1::Accounts::GroupsController < Api::V1::Accounts::BaseController
   def create
-    inbox = Current.account.inboxes.find_by(id: params[:inbox_id])
+    inbox = Current.account.inboxes.find_by(id: group_params[:inbox_id])
     return render json: { error: 'Access Denied' }, status: :forbidden unless inbox_accessible?(inbox)
 
     result = Groups::CreateService.new(
       inbox: inbox,
-      subject: params[:subject],
-      participants: Array(params[:participants])
+      subject: group_params[:subject],
+      participants: Array(group_params[:participants])
     ).perform
 
     render json: result
@@ -16,6 +16,10 @@ def create
 
   private
 
+  def group_params
+    params.permit(:inbox_id, :subject, participants: [])
+  end
+
   def inbox_accessible?(inbox)
     inbox.present? && Current.user.assigned_inboxes.exists?(id: inbox.id) && inbox.channel.try(:allow_group_creation?)
   end

app/javascript/dashboard/helper/actionCable.js

@@ -93,6 +93,8 @@ class ActionCableConnector extends BaseActionCableConnector {
     const pendingJid = pendingGroupNavigation.consume();
     if (pendingJid && data.meta?.sender?.identifier === pendingJid) {
       emitter.emit(BUS_EVENTS.NAVIGATE_TO_GROUP, { conversationId: data.id });
+    } else if (pendingJid) {
+      pendingGroupNavigation.set(pendingJid);
     }
   };
 

app/services/groups/create_service.rb

@@ -3,7 +3,7 @@ class Groups::CreateService
 
   def perform
     group_data = channel.create_group(subject, format_participants)
-    group_jid = group_data[:id]
+    group_jid = group_data&.dig(:id)
     raise Whatsapp::Providers::WhatsappBaileysService::ProviderUnavailableError, 'Group JID missing from response' if group_jid.blank?
 
     { group_jid: group_jid }

app/services/whatsapp/baileys_handlers/concerns/group_contact_message_handler.rb

@@ -188,7 +188,7 @@ def baileys_sender_phone
 
     jid_part = sender_jid.split('@').first
     parts = jid_part.split(':')
-    parts.first if parts.first.match?(/^\d+$/) && parts.length > 1
+    parts.first if parts.first.match?(/^\d+$/)
   end
 
   def baileys_sender_lid

app/services/whatsapp/baileys_handlers/concerns/group_stub_message_handler.rb

@@ -90,7 +90,7 @@ def resolve_membership_request_contact_name(stub_params)
     phone = extract_jid_user(participant_data['pn'])
 
     find_contact_display_name(lid, phone) || format_fallback_name(lid, phone)
-  rescue JSON::ParserError
+  rescue JSON::ParserError, TypeError
     extract_jid_user(@raw_message[:key][:participant])
   end
 

app/services/whatsapp/baileys_handlers/group_participants_update.rb

@@ -7,12 +7,10 @@ module Whatsapp::BaileysHandlers::GroupParticipantsUpdate
 
   def process_group_participants_update
     data = processed_params[:data]
-    group_jid = data[:id]
-    author = data[:author]
-    action = data[:action]
-    participants = data[:participants]
+    return if data.blank?
 
-    return if group_jid.blank? || action.blank? || participants.blank?
+    group_jid, author, action, participants = data.values_at(:id, :author, :action, :participants)
+    return unless valid_participant_update?(group_jid, action, participants)
 
     with_contact_lock(group_jid) do
       group_contact_inbox = find_or_create_group_contact_inbox_by_jid(group_jid)
@@ -29,6 +27,10 @@ def process_group_participants_update
     end
   end
 
+  def valid_participant_update?(group_jid, action, participants)
+    group_jid.present? && action.present? && participants.present? && action.in?(%w[add remove promote demote])
+  end
+
   def apply_participant_action(action, group_contact, contact)
     case action
     when 'add'