From 166a41c31c800da8b24b54f99548e5e77098cf25 Mon Sep 17 00:00:00 2001 From: Muhsin Keloth Date: Wed, 22 Jul 2026 12:40:28 +0400 Subject: [PATCH 1/6] fix(whatsapp): prevent invalid automation sends outside reply window (#15113) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit WhatsApp automations now fail locally when they attempt to send a free-form message after the 24-hour customer service window has closed. This avoids sending an invalid template request to Meta and gives users a clear, actionable error instead of “Template not found or invalid template name.” Template messages continue to be sent whenever template parameters are present. Free-form messages continue to be sent normally while the conversation is replyable. Fixes https://linear.app/chatwoot/issue/PLA-183/prevent-whatsapp-automations-outside-the-24-hour-window-from-producing ### How to reproduce 1. Create a WhatsApp automation that sends a message without template parameters. 2. Trigger it on a conversation whose 24-hour customer service window is closed. 3. Observe that the message previously reached the template send path and failed with a misleading provider error. ### How to test 1. Trigger an automation with template parameters and confirm it sends as a template message. 2. Trigger an automation without template parameters inside the 24-hour window and confirm it sends as a free-form message. 3. Trigger an automation without template parameters outside the 24-hour window and confirm it fails locally with a clear error and makes no request to Meta. ### Things to know This changes only the invalid closed-window, no-template path. Existing template and in-window message behavior remains unchanged. --------- Co-authored-by: Muhsin <12408980+muhsin-k@users.noreply.github.com> --- app/services/whatsapp/send_on_whatsapp_service.rb | 10 ++++------ config/locales/en.yml | 1 + .../whatsapp/send_on_whatsapp_service_spec.rb | 15 +++++++++++++++ 3 files changed, 20 insertions(+), 6 deletions(-) diff --git a/app/services/whatsapp/send_on_whatsapp_service.rb b/app/services/whatsapp/send_on_whatsapp_service.rb index 20419c0cd..b8de35d1c 100644 --- a/app/services/whatsapp/send_on_whatsapp_service.rb +++ b/app/services/whatsapp/send_on_whatsapp_service.rb @@ -6,12 +6,10 @@ class Whatsapp::SendOnWhatsappService < Base::SendOnChannelService end def perform_reply - should_send_template_message = template_params.present? || !message.conversation.can_reply? - if should_send_template_message - send_template_message - else - send_session_message - end + return send_template_message if template_params.present? + return send_session_message if message.conversation.can_reply? + + message.update!(status: :failed, external_error: I18n.t('errors.whatsapp.message_outside_messaging_window')) end def send_template_message diff --git a/config/locales/en.yml b/config/locales/en.yml index 735d52205..1efae55e1 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -154,6 +154,7 @@ en: invalid_token_permissions: 'The access token does not have the required permissions for WhatsApp.' phone_info_fetch_failed: 'Failed to fetch phone number information. Please try again.' phone_number_already_exists: 'Channel already exists for this phone number: %{phone_number}, please contact support if the error persists' + message_outside_messaging_window: 'Message not sent because the WhatsApp 24-hour customer service window is closed and no template parameters were provided. Send an approved template message instead.' reauthorization: generic: 'Failed to reauthorize WhatsApp. Please try again.' not_supported: 'Reauthorization is not supported for this type of WhatsApp channel.' diff --git a/spec/services/whatsapp/send_on_whatsapp_service_spec.rb b/spec/services/whatsapp/send_on_whatsapp_service_spec.rb index c27295b3e..850685718 100644 --- a/spec/services/whatsapp/send_on_whatsapp_service_spec.rb +++ b/spec/services/whatsapp/send_on_whatsapp_service_spec.rb @@ -72,6 +72,21 @@ describe Whatsapp::SendOnWhatsappService do expect(message.reload.source_id).to eq('123456789') end + it 'fails a free-form message without contacting the provider when outside the 24 hour limit' do + create(:message, message_type: :incoming, content: 'test', created_at: 25.hours.ago, + conversation: conversation, account: conversation.account) + message = create(:message, message_type: :outgoing, content: 'test', + conversation: conversation, account: conversation.account) + + expect(Whatsapp::TemplateProcessorService).not_to receive(:new) + + described_class.new(message: message).perform + + expect(message.reload.status).to eq('failed') + expect(message.external_error).to eq(I18n.t('errors.whatsapp.message_outside_messaging_window')) + expect(a_request(:post, 'https://waba.360dialog.io/v1/messages')).not_to have_been_made + end + it 'marks message as failed when template name is blank' do processor = instance_double(Whatsapp::TemplateProcessorService) allow(Whatsapp::TemplateProcessorService).to receive(:new).and_return(processor) From fbb3479263865c5227945539045d7ebb1f6cda20 Mon Sep 17 00:00:00 2001 From: Sivin Varghese <64252451+iamsivin@users.noreply.github.com> Date: Wed, 22 Jul 2026 15:23:19 +0530 Subject: [PATCH 2/6] fix: guard agent sort against null names in assignment dropdown (#15125) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Pull Request Template ## Description This PR fixes a crash where opening a conversation threw `TypeError: Cannot read properties of null (reading 'localeCompare')` and prevented the agent assignment dropdown from rendering. Since #14866, agent bots are included in the assignable agents list. `AgentBot#name` is not presence-validated, so system bots (account-less, global) can have a `null` name. Those nameless bots flowed into name-based operations that assumed a string, causing crashes and warnings across multiple surfaces: * **Assignment dropdown sort:** `getAgentsByAvailability` called `a.name.localeCompare(b.name)`, causing a `localeCompare` `TypeError`. * **Dropdown search:** `MultiselectDropdownItems` called `option.name.toLowerCase()`, causing a `toLowerCase` `TypeError`. * **Agent Bots settings:** `Avatar` received `name=null` for a `String` prop, triggering a Vue prop validation warning. ### What changed * Keep nameless agent bots in the assignment dropdown and render a `-` fallback label in `useAgentsList`. These are still valid, assignable-by-ID records: the assignable agents API includes accessible bots, and `Conversations::AssignmentService` assigns them by ID. Preserving them avoids hiding valid assignment targets. Bots are still included only when `includeAgentBots` is enabled. * Make the sort in `getAgentsByAvailability` null-safe by coercing missing names to an empty string (defense in depth). * Make the search filter in `MultiselectDropdownItems` null-safe (defense in depth). * Pass a null-safe `name` prop to `Avatar` in the Agent Bots settings list to eliminate the Vue prop validation warning. Fixes https://linear.app/chatwoot/issue/CW-7670/agent-assignment-dropdown-crashes-with-cannot-read-properties-of-null ## Type of change - [x] Bug fix (non-breaking change which fixes an issue) ## How Has This Been Tested? 1. Have a system agent bot (`name: null`) that is assignable to an inbox. 2. Open any conversation in that inbox. * The agent assignment dropdown renders without console errors. * The nameless bot is listed with a `-` label and can be assigned. 3. Go to **Settings → Agent Bots**. * The page renders without the `Avatar` prop validation warning. ## Checklist: - [x] My code follows the style guidelines of this project - [x] I have performed a self-review of my code - [x] I have commented on my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective or that my feature works - [x] New and existing unit tests pass locally with my changes - [ ] Any dependent changes have been merged and published in downstream modules --- .../composables/spec/useAgentsList.spec.js | 34 ++++++++++++++++--- .../dashboard/composables/useAgentsList.js | 10 ++++-- .../dashboard/helper/agentHelper.js | 2 +- .../helper/specs/agentHelper.spec.js | 12 +++++++ .../dashboard/settings/agentBots/Index.vue | 2 +- .../components/ui/MultiselectDropdown.vue | 10 ++++-- .../ui/MultiselectDropdownItems.vue | 4 ++- 7 files changed, 61 insertions(+), 13 deletions(-) diff --git a/app/javascript/dashboard/composables/spec/useAgentsList.spec.js b/app/javascript/dashboard/composables/spec/useAgentsList.spec.js index 3a39a6be9..33a8edc0d 100644 --- a/app/javascript/dashboard/composables/spec/useAgentsList.spec.js +++ b/app/javascript/dashboard/composables/spec/useAgentsList.spec.js @@ -1,9 +1,9 @@ -import { ref } from 'vue'; -import { describe, it, expect, vi, beforeEach } from 'vitest'; -import { useAgentsList } from '../useAgentsList'; import { useMapGetter } from 'dashboard/composables/store'; -import { allAgentsData, formattedAgentsData } from './fixtures/agentFixtures'; import * as agentHelper from 'dashboard/helper/agentHelper'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { ref } from 'vue'; +import { useAgentsList } from '../useAgentsList'; +import { allAgentsData, formattedAgentsData } from './fixtures/agentFixtures'; // Mock vue-i18n vi.mock('vue-i18n', () => ({ @@ -94,6 +94,32 @@ describe('useAgentsList', () => { expect(agentsList.value.length).toBe(formattedAgentsData.slice(1).length); }); + it('keeps nameless agent bots and applies a fallback label', () => { + const namelessBot = { + id: 91, + name: null, + assignee_type: 'AgentBot', + availability_status: 'offline', + }; + mockUseMapGetter({ + 'inboxAssignableAgents/getAssignableAgents': ref(() => [ + ...allAgentsData, + namelessBot, + ]), + }); + + const { agentsList } = useAgentsList(); + // access the computed to trigger evaluation + expect(agentsList.value).toBeDefined(); + + const passedAgents = + agentHelper.getAgentsByUpdatedPresence.mock.calls[0][0]; + expect(passedAgents).toContainEqual({ + ...namelessBot, + name: '-', + }); + }); + it('handles empty assignable agents', () => { mockUseMapGetter({ 'inboxAssignableAgents/getAssignableAgents': ref(() => []), diff --git a/app/javascript/dashboard/composables/useAgentsList.js b/app/javascript/dashboard/composables/useAgentsList.js index 8e8ee5568..d39b54c33 100644 --- a/app/javascript/dashboard/composables/useAgentsList.js +++ b/app/javascript/dashboard/composables/useAgentsList.js @@ -1,10 +1,10 @@ -import { computed } from 'vue'; import { useMapGetter } from 'dashboard/composables/store'; -import { useI18n } from 'vue-i18n'; import { getAgentsByUpdatedPresence, getSortedAgentsByAvailability, } from 'dashboard/helper/agentHelper'; +import { computed } from 'vue'; +import { useI18n } from 'vue-i18n'; /** * A composable function that provides a list of agents for assignment. @@ -53,7 +53,11 @@ export function useAgentsList( * @type {import('vue').ComputedRef} */ const agentsList = computed(() => { - const agents = assignableAgents.value || []; + const agents = (assignableAgents.value || []).map(agent => + !agent.name && agent.assignee_type === 'AgentBot' + ? { ...agent, name: '-' } + : agent + ); const agentsByUpdatedPresence = getAgentsByUpdatedPresence( agents, currentUser.value, diff --git a/app/javascript/dashboard/helper/agentHelper.js b/app/javascript/dashboard/helper/agentHelper.js index ff1123f66..6d592e551 100644 --- a/app/javascript/dashboard/helper/agentHelper.js +++ b/app/javascript/dashboard/helper/agentHelper.js @@ -7,7 +7,7 @@ export const getAgentsByAvailability = (agents, availability) => { return agents .filter(agent => agent.availability_status === availability) - .sort((a, b) => a.name.localeCompare(b.name)); + .sort((a, b) => (a.name || '').localeCompare(b.name || '')); }; /** diff --git a/app/javascript/dashboard/helper/specs/agentHelper.spec.js b/app/javascript/dashboard/helper/specs/agentHelper.spec.js index 273834a11..154d8faf4 100644 --- a/app/javascript/dashboard/helper/specs/agentHelper.spec.js +++ b/app/javascript/dashboard/helper/specs/agentHelper.spec.js @@ -26,6 +26,18 @@ describe('agentHelper', () => { offlineAgentsData ); }); + + it('does not throw when an agent has a null name', () => { + const agents = [ + { id: 1, name: null, availability_status: 'offline' }, + { id: 2, name: 'Zoe', availability_status: 'offline' }, + ]; + + expect(() => getAgentsByAvailability(agents, 'offline')).not.toThrow(); + expect( + getAgentsByAvailability(agents, 'offline').map(agent => agent.id) + ).toEqual([1, 2]); + }); }); describe('getSortedAgentsByAvailability', () => { diff --git a/app/javascript/dashboard/routes/dashboard/settings/agentBots/Index.vue b/app/javascript/dashboard/routes/dashboard/settings/agentBots/Index.vue index 88286bd88..5547d7a4a 100644 --- a/app/javascript/dashboard/routes/dashboard/settings/agentBots/Index.vue +++ b/app/javascript/dashboard/routes/dashboard/settings/agentBots/Index.vue @@ -135,7 +135,7 @@ onMounted(() => {
props.selectedItem?.assignee_type === 'AgentBot' ); +const selectedItemName = computed(() => + !props.selectedItem?.name && isAgentBot.value ? '-' : props.selectedItem?.name +); + const selectedThumbnail = computed( () => props.selectedItem?.thumbnail || props.selectedItem?.avatar_url ); @@ -95,16 +99,16 @@ const selectedThumbnail = computed(

- {{ selectedItem.name }} + {{ selectedItemName }}

{ - return option.name.toLowerCase().includes(this.search.toLowerCase()); + return (option.name || '') + .toLowerCase() + .includes(this.search.toLowerCase()); }); }, noResult() { From 1e52d23d7af123e3a28374b2fc05817178ab2f3f Mon Sep 17 00:00:00 2001 From: Sivin Varghese <64252451+iamsivin@users.noreply.github.com> Date: Wed, 22 Jul 2026 15:27:04 +0530 Subject: [PATCH 3/6] fix: guard agent sort against null names in assignment dropdown (#15125) From 5733b822e3a384d5e237446cb775acd04371b228 Mon Sep 17 00:00:00 2001 From: Vishnu Narayanan Date: Wed, 22 Jul 2026 17:36:36 +0530 Subject: [PATCH 4/6] fix: guard widget email-transcript button against repeat clicks (#15095) ## Description The widget email-transcript button (`ChatFooter.vue`) had no client-side guard. Its visibility depends only on whether the contact has an email, and the click handler fired a request on every click with no in-flight lock, no disabled state, and no post-send handling. A user could therefore trigger a large number of duplicate transcript emails from a single conversation just by clicking repeatedly. This adds a re-entry guard in the handler, disables the button while a send is in flight, and applies a short cooldown (15s) after a successful send. Normal use is unaffected: the button sends once, shows the success toast, then briefly disables and automatically re-enables so a genuine later re-request still works. On failure the button stays enabled so the user can retry immediately. The cooldown timer is cleared on unmount. Using a timed cooldown (rather than a permanent post-send lock) also avoids the button getting stuck disabled if a resolved conversation is reopened and later re-resolved. This is the client-side complement to the server-side rate limit added in #15085. Fixes https://linear.app/chatwoot/issue/CW-7640 --- .../widget/components/ChatFooter.vue | 49 ++++++++++++++----- 1 file changed, 37 insertions(+), 12 deletions(-) diff --git a/app/javascript/widget/components/ChatFooter.vue b/app/javascript/widget/components/ChatFooter.vue index c85727a2b..2cad4d177 100755 --- a/app/javascript/widget/components/ChatFooter.vue +++ b/app/javascript/widget/components/ChatFooter.vue @@ -11,6 +11,8 @@ import { IFrameHelper } from '../helpers/utils'; import { CHATWOOT_ON_START_CONVERSATION } from '../constants/sdkEvents'; import { emitter } from 'shared/helpers/mitt'; +const TRANSCRIPT_COOLDOWN_MS = 15000; + export default { components: { ChatInputWrap, @@ -24,6 +26,9 @@ export default { data() { return { inReplyTo: null, + isSendingTranscript: false, + transcriptCooldown: false, + transcriptCooldownTimer: null, }; }, computed: { @@ -57,6 +62,9 @@ export default { mounted() { emitter.on(BUS_EVENTS.TOGGLE_REPLY_TO_MESSAGE, this.toggleReplyTo); }, + beforeUnmount() { + clearTimeout(this.transcriptCooldownTimer); + }, methods: { ...mapActions('conversation', ['sendMessage', 'sendAttachment']), ...mapActions('conversationAttributes', ['getAttributes']), @@ -90,19 +98,35 @@ export default { toggleReplyTo(message) { this.inReplyTo = message; }, + startTranscriptCooldown() { + this.transcriptCooldown = true; + clearTimeout(this.transcriptCooldownTimer); + this.transcriptCooldownTimer = setTimeout(() => { + this.transcriptCooldown = false; + }, TRANSCRIPT_COOLDOWN_MS); + }, async sendTranscript() { - if (this.hasEmail) { - try { - await sendEmailTranscript(); - emitter.emit(BUS_EVENTS.SHOW_ALERT, { - message: this.$t('EMAIL_TRANSCRIPT.SEND_EMAIL_SUCCESS'), - type: 'success', - }); - } catch (error) { - emitter.$emit(BUS_EVENTS.SHOW_ALERT, { - message: this.$t('EMAIL_TRANSCRIPT.SEND_EMAIL_ERROR'), - }); - } + if ( + !this.hasEmail || + this.isSendingTranscript || + this.transcriptCooldown + ) { + return; + } + this.isSendingTranscript = true; + try { + await sendEmailTranscript(); + this.startTranscriptCooldown(); + emitter.emit(BUS_EVENTS.SHOW_ALERT, { + message: this.$t('EMAIL_TRANSCRIPT.SEND_EMAIL_SUCCESS'), + type: 'success', + }); + } catch (error) { + emitter.emit(BUS_EVENTS.SHOW_ALERT, { + message: this.$t('EMAIL_TRANSCRIPT.SEND_EMAIL_ERROR'), + }); + } finally { + this.isSendingTranscript = false; } }, }, @@ -144,6 +168,7 @@ export default { v-if="showEmailTranscriptButton" type="clear" class="font-normal" + :disabled="isSendingTranscript || transcriptCooldown" @click="sendTranscript" > {{ $t('EMAIL_TRANSCRIPT.BUTTON_TEXT') }} From 8aee5181492b4fb83071b283a9927d8a5f7e69ec Mon Sep 17 00:00:00 2001 From: Tanmay Deep Sharma <32020192+tds-1@users.noreply.github.com> Date: Wed, 22 Jul 2026 17:53:25 +0530 Subject: [PATCH 5/6] fix(integrations): restrict Linear/Notion/Shopify hook deletion to admins (#15126) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Non-admin agents could delete an account's Linear, Notion, or Shopify integration through the dedicated integration endpoints, which — unlike the generic hooks endpoint — never checked the caller's role. This restores the intended admin-only boundary for removing an integration. ## Closes - https://linear.app/chatwoot/issue/CW-7383 - https://linear.app/chatwoot/issue/CW-7384 - https://linear.app/chatwoot/issue/CW-7189 ## How to reproduce As a non-admin **agent**, `DELETE /api/v1/accounts/:id/integrations/{linear,notion,shopify}` returned `200` and removed the account-wide integration. After this change it returns `401` and the integration is preserved; administrators can still remove it. ## What changed - Route integration-hook deletion through `HookPolicy` (admin-only) via a shared `Integrations::BaseController`, matching the generic hooks controller. Co-authored-by: Vishnu Narayanan --- .../accounts/integrations/base_controller.rb | 9 +++++++++ .../accounts/integrations/hooks_controller.rb | 6 +----- .../accounts/integrations/linear_controller.rb | 3 ++- .../accounts/integrations/notion_controller.rb | 3 ++- .../integrations/shopify_controller.rb | 3 ++- .../integrations/linear_controller_spec.rb | 14 ++++++++++++-- .../integrations/shopify_controller_spec.rb | 18 ++++++++++++++++-- 7 files changed, 44 insertions(+), 12 deletions(-) create mode 100644 app/controllers/api/v1/accounts/integrations/base_controller.rb diff --git a/app/controllers/api/v1/accounts/integrations/base_controller.rb b/app/controllers/api/v1/accounts/integrations/base_controller.rb new file mode 100644 index 000000000..ef1ebb713 --- /dev/null +++ b/app/controllers/api/v1/accounts/integrations/base_controller.rb @@ -0,0 +1,9 @@ +class Api::V1::Accounts::Integrations::BaseController < Api::V1::Accounts::BaseController + private + + # Managing an integration hook (create/update/destroy) is admin-only, enforced via HookPolicy. + # Subclasses opt in per action with `before_action :check_authorization, only: [...]`. + def check_authorization + authorize(:hook) + end +end diff --git a/app/controllers/api/v1/accounts/integrations/hooks_controller.rb b/app/controllers/api/v1/accounts/integrations/hooks_controller.rb index 087a9b78d..aec105930 100644 --- a/app/controllers/api/v1/accounts/integrations/hooks_controller.rb +++ b/app/controllers/api/v1/accounts/integrations/hooks_controller.rb @@ -1,4 +1,4 @@ -class Api::V1::Accounts::Integrations::HooksController < Api::V1::Accounts::BaseController +class Api::V1::Accounts::Integrations::HooksController < Api::V1::Accounts::Integrations::BaseController before_action :fetch_hook, except: [:create] before_action :check_authorization @@ -35,10 +35,6 @@ class Api::V1::Accounts::Integrations::HooksController < Api::V1::Accounts::Base @hook = Current.account.hooks.find(params[:id]) end - def check_authorization - authorize(:hook) - end - def permitted_params params.require(:hook).permit(:app_id, :inbox_id, :status, settings: {}) end diff --git a/app/controllers/api/v1/accounts/integrations/linear_controller.rb b/app/controllers/api/v1/accounts/integrations/linear_controller.rb index 9ca0c72fd..8ae3109b9 100644 --- a/app/controllers/api/v1/accounts/integrations/linear_controller.rb +++ b/app/controllers/api/v1/accounts/integrations/linear_controller.rb @@ -1,6 +1,7 @@ -class Api::V1::Accounts::Integrations::LinearController < Api::V1::Accounts::BaseController +class Api::V1::Accounts::Integrations::LinearController < Api::V1::Accounts::Integrations::BaseController before_action :fetch_conversation, only: [:create_issue, :link_issue, :unlink_issue, :linked_issues] before_action :fetch_hook, only: [:destroy] + before_action :check_authorization, only: [:destroy] def destroy revoke_linear_token diff --git a/app/controllers/api/v1/accounts/integrations/notion_controller.rb b/app/controllers/api/v1/accounts/integrations/notion_controller.rb index ecf6bae6e..29343e4f5 100644 --- a/app/controllers/api/v1/accounts/integrations/notion_controller.rb +++ b/app/controllers/api/v1/accounts/integrations/notion_controller.rb @@ -1,5 +1,6 @@ -class Api::V1::Accounts::Integrations::NotionController < Api::V1::Accounts::BaseController +class Api::V1::Accounts::Integrations::NotionController < Api::V1::Accounts::Integrations::BaseController before_action :fetch_hook, only: [:destroy] + before_action :check_authorization, only: [:destroy] def destroy @hook.destroy! diff --git a/app/controllers/api/v1/accounts/integrations/shopify_controller.rb b/app/controllers/api/v1/accounts/integrations/shopify_controller.rb index 7fe31889b..c847a85df 100644 --- a/app/controllers/api/v1/accounts/integrations/shopify_controller.rb +++ b/app/controllers/api/v1/accounts/integrations/shopify_controller.rb @@ -1,7 +1,8 @@ -class Api::V1::Accounts::Integrations::ShopifyController < Api::V1::Accounts::BaseController +class Api::V1::Accounts::Integrations::ShopifyController < Api::V1::Accounts::Integrations::BaseController include Shopify::IntegrationHelper before_action :setup_shopify_context, only: [:orders] before_action :fetch_hook, except: [:auth] + before_action :check_authorization, only: [:destroy] before_action :validate_contact, only: [:orders] def auth diff --git a/spec/controllers/api/v1/accounts/integrations/linear_controller_spec.rb b/spec/controllers/api/v1/accounts/integrations/linear_controller_spec.rb index 5f512b2bd..8d28a2f2a 100644 --- a/spec/controllers/api/v1/accounts/integrations/linear_controller_spec.rb +++ b/spec/controllers/api/v1/accounts/integrations/linear_controller_spec.rb @@ -13,7 +13,9 @@ RSpec.describe 'Linear Integration API', type: :request do end describe 'DELETE /api/v1/accounts/:account_id/integrations/linear' do - it 'deletes the linear integration' do + let(:admin) { create(:user, account: account, role: :administrator) } + + it 'deletes the linear integration when the user is an administrator' do # Stub the HTTP call to Linear's revoke endpoint allow(HTTParty).to receive(:post).with( 'https://api.linear.app/oauth/revoke', @@ -21,11 +23,19 @@ RSpec.describe 'Linear Integration API', type: :request do ).and_return(instance_double(HTTParty::Response, success?: true)) delete "/api/v1/accounts/#{account.id}/integrations/linear", - headers: agent.create_new_auth_token, + headers: admin.create_new_auth_token, as: :json expect(response).to have_http_status(:ok) expect(account.hooks.count).to eq(0) end + + it 'returns unauthorized for an agent and keeps the integration' do + delete "/api/v1/accounts/#{account.id}/integrations/linear", + headers: agent.create_new_auth_token, + as: :json + expect(response).to have_http_status(:unauthorized) + expect(account.hooks.count).to eq(1) + end end describe 'GET /api/v1/accounts/:account_id/integrations/linear/teams' do diff --git a/spec/controllers/api/v1/accounts/integrations/shopify_controller_spec.rb b/spec/controllers/api/v1/accounts/integrations/shopify_controller_spec.rb index ef6c4d367..7f9a032ce 100644 --- a/spec/controllers/api/v1/accounts/integrations/shopify_controller_spec.rb +++ b/spec/controllers/api/v1/accounts/integrations/shopify_controller_spec.rb @@ -159,15 +159,17 @@ RSpec.describe 'Shopify Integration API', type: :request do end describe 'DELETE /api/v1/accounts/:account_id/integrations/shopify' do + let(:admin) { create(:user, account: account, role: :administrator) } + before do create(:integrations_hook, :shopify, account: account) end - context 'when it is an authenticated user' do + context 'when it is an administrator' do it 'deletes the shopify integration' do expect do delete "/api/v1/accounts/#{account.id}/integrations/shopify", - headers: agent.create_new_auth_token, + headers: admin.create_new_auth_token, as: :json end.to change { account.hooks.count }.by(-1) @@ -175,6 +177,18 @@ RSpec.describe 'Shopify Integration API', type: :request do end end + context 'when it is an agent' do + it 'returns unauthorized and keeps the integration' do + expect do + delete "/api/v1/accounts/#{account.id}/integrations/shopify", + headers: agent.create_new_auth_token, + as: :json + end.not_to(change { account.hooks.count }) + + expect(response).to have_http_status(:unauthorized) + end + end + context 'when it is an unauthenticated user' do it 'returns unauthorized' do delete "/api/v1/accounts/#{account.id}/integrations/shopify", From 887897ea986eca619e750e0651a54443d204c601 Mon Sep 17 00:00:00 2001 From: Sony Mathew Date: Wed, 22 Jul 2026 18:43:00 +0530 Subject: [PATCH 6/6] fix: lock agent quota checks (#15029) # Pull Request Template ## Description Locks the agent quota check to the account row while creating account users. This fixes a race where concurrent agent-create requests could all observe the same remaining seat before any `account_users` row was inserted. The API continues to return the existing `402 Account limit exceeded. Please purchase more licenses` response when the limit is reached. Bulk create now preflights the requested email count while holding the account lock, then creates each agent through the same locked builder path. The Enterprise custom-role hook now no-ops when create did not produce an agent. Fixes: [CW-7039](https://linear.app/chatwoot/issue/CW-7039/race-condition-in-agent-creation-bypasses-plan-agent-seat-limit) ## Type of change - [x] Bug fix (non-breaking change which fixes an issue) ## How Has This Been Tested? - `POSTGRES_DATABASE=chatwoot_test_c20f_agent_quota REDIS_DB=9 bundle exec rspec spec/builders/agent_builder_spec.rb spec/enterprise/builders/agent_builder_spec.rb spec/controllers/api/v1/accounts/agents_controller_spec.rb spec/enterprise/controllers/api/v1/accounts/agents_controller_spec.rb spec/enterprise/controllers/enterprise/api/v1/accounts/agents_controller_spec.rb` - `bundle exec rubocop app/builders/agent_builder.rb app/controllers/api/v1/accounts/agents_controller.rb enterprise/app/controllers/enterprise/api/v1/accounts/agents_controller.rb spec/builders/agent_builder_spec.rb spec/enterprise/controllers/api/v1/accounts/agents_controller_spec.rb` - `git diff --check` - One-off threaded Rails validation with 8 concurrent `AgentBuilder` calls against an account with one remaining seat: `created: 1`, `limited: 7`, final `count=2`, `limit=2`. ## Checklist: - [x] My code follows the style guidelines of this project - [x] I have performed a self-review of my code - [x] I have commented on my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective or that my feature works - [x] New and existing unit tests pass locally with my changes - [ ] Any dependent changes have been merged and published in downstream modules Co-authored-by: Muhsin Keloth --- app/builders/agent_builder.rb | 22 +++++++- .../api/v1/accounts/agents_controller.rb | 55 +++++++++---------- .../api/v1/accounts/agents_controller.rb | 2 + spec/builders/agent_builder_spec.rb | 18 ++++++ .../api/v1/accounts/agents_controller_spec.rb | 21 +++++++ 5 files changed, 87 insertions(+), 31 deletions(-) diff --git a/app/builders/agent_builder.rb b/app/builders/agent_builder.rb index d2715011c..af68eefc5 100644 --- a/app/builders/agent_builder.rb +++ b/app/builders/agent_builder.rb @@ -2,6 +2,14 @@ # It initializes with necessary attributes and provides a perform method # to create a user and account user in a transaction. class AgentBuilder + LIMIT_EXCEEDED_MESSAGE = 'Account limit exceeded. Please purchase more licenses'.freeze + + class LimitExceededError < StandardError + def initialize + super(AgentBuilder::LIMIT_EXCEEDED_MESSAGE) + end + end + # Initializes an AgentBuilder with necessary attributes. # @param email [String] the email of the user. # @param name [String] the name of the user. @@ -14,15 +22,23 @@ class AgentBuilder # Creates a user and account user in a transaction. # @return [User] the created user. def perform - ActiveRecord::Base.transaction do - @user = find_or_create_user - create_account_user + account.with_lock do + raise LimitExceededError unless can_add_agent? + + ActiveRecord::Base.transaction do + @user = find_or_create_user + create_account_user + end end @user end private + def can_add_agent? + account.usage_limits[:agents] > account.account_users.count + end + # Finds a user by email or creates a new one with a temporary password. # @return [User] the found or created user. def find_or_create_user diff --git a/app/controllers/api/v1/accounts/agents_controller.rb b/app/controllers/api/v1/accounts/agents_controller.rb index 438944f04..864c50bb4 100644 --- a/app/controllers/api/v1/accounts/agents_controller.rb +++ b/app/controllers/api/v1/accounts/agents_controller.rb @@ -1,8 +1,6 @@ class Api::V1::Accounts::AgentsController < Api::V1::Accounts::BaseController before_action :fetch_agent, except: [:create, :index, :bulk_create] before_action :check_authorization - before_action :validate_limit, only: [:create] - before_action :validate_limit_for_bulk_create, only: [:bulk_create] def index @agents = agents @@ -20,6 +18,8 @@ class Api::V1::Accounts::AgentsController < Api::V1::Accounts::BaseController ) @agent = builder.perform + rescue AgentBuilder::LimitExceededError => e + render_payment_required(e.message) end def update @@ -36,25 +36,13 @@ class Api::V1::Accounts::AgentsController < Api::V1::Accounts::BaseController def bulk_create emails = params[:emails] - emails.each do |email| - builder = AgentBuilder.new( - email: email, - name: email.split('@').first, - inviter: current_user, - account: Current.account - ) - begin - builder.perform - rescue ActiveRecord::RecordInvalid => e - Rails.logger.info "[Agent#bulk_create] ignoring email #{email}, errors: #{e.record.errors}" - end - end - + bulk_create_agents(emails) # This endpoint is used to bulk create agents during onboarding # onboarding_step key in present in Current account custom attributes, since this is a one time operation - Current.account.custom_attributes.delete('onboarding_step') - Current.account.save! + clear_onboarding_step head :ok + rescue AgentBuilder::LimitExceededError => e + render_payment_required(e.message) end private @@ -87,22 +75,33 @@ class Api::V1::Accounts::AgentsController < Api::V1::Accounts::BaseController @agents ||= Current.account.users.order_by_full_name.includes(:account_users, { avatar_attachment: [:blob] }) end - def validate_limit_for_bulk_create - limit_available = params[:emails].count <= available_agent_count + def bulk_create_agents(emails) + Current.account.with_lock do + raise AgentBuilder::LimitExceededError if emails.count > available_agent_count - render_payment_required('Account limit exceeded. Please purchase more licenses') unless limit_available + emails.each { |email| create_agent_from_email(email) } + end end - def validate_limit - render_payment_required('Account limit exceeded. Please purchase more licenses') unless can_add_agent? + def create_agent_from_email(email) + builder = AgentBuilder.new( + email: email, + name: email.split('@').first, + inviter: current_user, + account: Current.account + ) + builder.perform + rescue ActiveRecord::RecordInvalid => e + Rails.logger.info "[Agent#bulk_create] ignoring email #{email}, errors: #{e.record.errors}" + end + + def clear_onboarding_step + Current.account.custom_attributes.delete('onboarding_step') + Current.account.save! end def available_agent_count - Current.account.usage_limits[:agents] - agents.count - end - - def can_add_agent? - available_agent_count.positive? + Current.account.usage_limits[:agents] - Current.account.account_users.count end def delete_user_record(agent) diff --git a/enterprise/app/controllers/enterprise/api/v1/accounts/agents_controller.rb b/enterprise/app/controllers/enterprise/api/v1/accounts/agents_controller.rb index b3a27c4ad..02d027b58 100644 --- a/enterprise/app/controllers/enterprise/api/v1/accounts/agents_controller.rb +++ b/enterprise/app/controllers/enterprise/api/v1/accounts/agents_controller.rb @@ -1,6 +1,8 @@ module Enterprise::Api::V1::Accounts::AgentsController def create super + return if @agent.blank? + associate_agent_with_custom_role end diff --git a/spec/builders/agent_builder_spec.rb b/spec/builders/agent_builder_spec.rb index f140f2f29..69cacb22b 100644 --- a/spec/builders/agent_builder_spec.rb +++ b/spec/builders/agent_builder_spec.rb @@ -23,6 +23,12 @@ RSpec.describe AgentBuilder, type: :model do end describe '#perform' do + it 'locks the account while checking and creating the agent' do + expect(account).to receive(:with_lock).and_call_original + + agent_builder.perform + end + context 'when user does not exist' do it 'creates a new user' do expect { agent_builder.perform }.to change(User, :count).by(1) @@ -67,5 +73,17 @@ RSpec.describe AgentBuilder, type: :model do expect(user.encrypted_password).not_to be_empty end end + + context 'when the account has reached its agent limit' do + before do + allow(account).to receive(:usage_limits).and_return({ agents: account.account_users.count }) + end + + it 'raises a limit exceeded error without creating a user' do + expect { agent_builder.perform }.to raise_error(described_class::LimitExceededError, described_class::LIMIT_EXCEEDED_MESSAGE) + + expect(User.from_email(email)).to be_nil + end + end end end diff --git a/spec/enterprise/controllers/api/v1/accounts/agents_controller_spec.rb b/spec/enterprise/controllers/api/v1/accounts/agents_controller_spec.rb index e5a8a5b7d..270150979 100644 --- a/spec/enterprise/controllers/api/v1/accounts/agents_controller_spec.rb +++ b/spec/enterprise/controllers/api/v1/accounts/agents_controller_spec.rb @@ -21,6 +21,27 @@ RSpec.describe 'Agents API', type: :request do expect(response).to have_http_status(:payment_required) expect(response.body).to include('Account limit exceeded. Please purchase more licenses') end + + it 'prevents adding an agent if the last seat is consumed before creation' do + account.update!(limits: { agents: account.account_users.count + 1 }) + competing_agent_created = false + + allow(AgentBuilder).to receive(:new).and_wrap_original do |method, *args| + unless competing_agent_created + create(:user, account: account, role: :agent) + competing_agent_created = true + end + + method.call(*args) + end + + post "/api/v1/accounts/#{account.id}/agents", params: params, headers: admin.create_new_auth_token, as: :json + + expect(response).to have_http_status(:payment_required) + expect(response.body).to include('Account limit exceeded. Please purchase more licenses') + expect(User.from_email(params[:email])).to be_nil + expect(account.account_users.count).to eq(account.usage_limits[:agents]) + end end end