Compare commits

...
Author SHA1 Message Date
aakashb95 e5688cc8f5 fix(email): reject Chatwoot continuity notifications 2026-06-03 14:38:40 +05:30
Sivin VargheseandGitHub d028cc1984 fix: prevent list marker overflow in messages (#14618) 2026-06-03 14:18:48 +05:30
Sivin VargheseandGitHub 0d59fb4459 fix: validate portal color format (#14632) 2026-06-03 14:18:19 +05:30
7acbe8b3ff fix(whatsapp): truncate location fallback_title to 255 chars to avoid silent message drop (#14517)
## Summary

`Whatsapp::IncomingMessageBaseService#attach_location` builds a
`fallback_title` by concatenating `location['name']` and
`location['address']` with no length cap, then stores it directly into
`Attachment#fallback_title`. `ApplicationRecord` enforces a generic
255-character limit on string columns, so any WhatsApp location whose
`"#{name}, #{address}"` exceeds 255 chars (a common case for Google
Places that include a long full address) raises
`ActiveRecord::RecordInvalid` deep inside the Sidekiq job. The message
and attachment INSERTs are part of the same transaction, so the whole
thing rolls back. Sidekiq retries once; the retry dedup-skips the wamid
silently and exits without an error. **Result: the message is
irrecoverably lost — no row in `messages`, no entry in the UI, no
outgoing webhook, no clue for the operator.**

Confirmed in `v4.13.0`, `v4.14.0`, and `develop` (commit `f33e469`,
2026-05-20). No upstream issue found before opening this PR.

## How to reproduce

1. From WhatsApp, share a Google Place whose `name + ", " + address` is
> 255 chars. The Spanish business address `Gremi de Fusters, 33,
Edificio VIP Asima, Piso 2, Local 2, Norte, 07009 Polígon industrial de
Son Castelló, Illes Balears, España` (132 chars) used as both `name` and
`address` is enough.
2. Sidekiq logs:
   ```
   ERROR ActiveRecord::RecordInvalid: Validation failed:
   Attachments fallback title is too long (maximum is 255 characters)
   ```
3. The `messages` table has no row. The conversation UI shows nothing
for that timestamp.
4. The first retry "Performed" successfully but creates nothing — the
dedup-by-source-id silently swallows the failure.

## Fix

Cap the existing concatenated title at 255 chars via `.first(255)`.
Minimal change, no behavioural difference for any message shorter than
the limit, prevents the silent data loss for any longer ones.

```diff
-    location_name = location['name'] ? "#{location['name']}, #{location['address']}" : ''
+    location_name = (location['name'] ? "#{location['name']}, #{location['address']}" : '').first(255)
```

## Alternatives considered

- **Increase the validation limit on `Attachment#fallback_title`**: more
invasive; would touch other inbound channels and possibly require a DB
column change.
- **Use `name` alone (no concat)**: cleaner semantically (in many real
payloads `name == address`), but changes user-visible behaviour. Left as
a follow-up if desired.
- **Truncate with ellipsis**: cosmetic only; deferred.

This PR is intentionally minimal so it can be merged on its own.

---------

Co-authored-by: Sony Mathew <sony@chatwoot.com>
Co-authored-by: Sony Mathew <2040199+sony-mathew@users.noreply.github.com>
2026-06-03 12:50:21 +05:30
Vinícius FitznerandGitHub b791d75b30 fix(microsoft): prevent OAuth admin consent loop (#13962)
Fixes #9775

## Description

This fixes a repeated admin consent loop in the Microsoft OAuth flow
when connecting a Microsoft email inbox.

Chatwoot was always sending `prompt=consent` in the Microsoft
authorization URL. In the current code path, this parameter is only used
when building the authorization URL and is not required by the callback,
token exchange, token persistence, or refresh flow.

By removing the forced consent prompt, the OAuth flow can proceed
normally without repeatedly sending users back through the admin consent
screen.

## What changed

- removed `prompt: 'consent'` from the Microsoft authorization URL
- added a regression assertion to ensure `prompt` is not included in the
generated URL

## Why this is safe

- `redirect_uri`, `scope`, and `state` remain unchanged
- callback and token exchange flow remain unchanged
- refresh token flow remains unchanged
- no other part of the current Microsoft inbox flow depends on forcing a
consent screen

## Testing

- updated controller spec to assert that the generated authorization URL
does not include `prompt`
2026-06-03 12:05:25 +05:30
9 changed files with 83 additions and 20 deletions
@@ -6,8 +6,7 @@ class Api::V1::Accounts::Microsoft::AuthorizationsController < Api::V1::Accounts
{
redirect_uri: "#{base_url}/microsoft/callback",
scope: scope,
state: state,
prompt: 'consent'
state: state
}
)
if redirect_url
+15 -4
View File
@@ -34,13 +34,24 @@ body {
.message-content {
ul {
list-style: disc;
@apply ltr:pl-3 rtl:pr-3;
@apply list-disc list-inside;
}
ol {
list-style: decimal;
@apply ltr:pl-4 rtl:pr-4;
@apply list-decimal list-inside;
}
li {
padding-inline-start: 1.5em;
text-indent: -1.5em;
> p:first-child {
@apply inline;
}
> * {
text-indent: 0;
}
}
}
+1
View File
@@ -42,6 +42,7 @@ class Portal < ApplicationRecord
validates :name, presence: true
validates :slug, presence: true, uniqueness: true
validates :custom_domain, uniqueness: true, allow_nil: true
validates :color, format: { with: /\A#(?:\h{3}|\h{6})\z/ }, allow_blank: true
validate :config_json_format
scope :active, -> { where(archived: false) }
+3 -1
View File
@@ -176,7 +176,9 @@ class MailPresenter < SimpleDelegator
def notification_email_from_chatwoot?
# notification emails are send via mailer sender email address. so it should match
configured_sender = Mail::Address.new(ENV.fetch('MAILER_SENDER_EMAIL', 'Chatwoot <accounts@chatwoot.com>')).address
original_sender.to_s.casecmp?(configured_sender)
actual_sender = parse_mail_address(@mail[:from]&.value)&.address
actual_sender.to_s.casecmp?(configured_sender)
end
private
@@ -147,7 +147,7 @@ class Whatsapp::IncomingMessageBaseService
def attach_location
location = messages_data.first['location']
location_name = location['name'] ? "#{location['name']}, #{location['address']}" : ''
location_name = (location['name'] ? "#{location['name']}, #{location['address']}" : '').first(255)
@message.attachments.new(
account_id: @message.account_id,
file_type: file_content_type(message_type),
@@ -43,6 +43,7 @@ RSpec.describe 'Microsoft Authorization API', type: :request do
]
expect(params['scope']).to eq(expected_scope)
expect(params['redirect_uri']).to eq(["#{ENV.fetch('FRONTEND_URL', 'http://localhost:3000')}/microsoft/callback"])
expect(url).not_to match(/(?:\?|&)prompt=/)
# Validate state parameter exists and can be decoded back to the account
expect(params['state']).to be_present
+17
View File
@@ -257,6 +257,23 @@ RSpec.describe MailPresenter do
expect(presenter.notification_email_from_chatwoot?).to be(true)
end
end
it 'detects Chatwoot notification emails even when reply_to points to the conversation reply address' do
notification_mail = Mail.new do
from 'Chatwoot <accounts@chatwoot.com>'
reply_to 'reply+5f85d369-8486-41c0-87ce-5a914a00b721@reply.chatwoot.com'
to 'Customer <customer@example.com>'
header['Subject'] = '[#81209] New messages on this conversation'
body 'Thank you for your understanding.'
end
with_modified_env MAILER_SENDER_EMAIL: 'Chatwoot <accounts@chatwoot.com>' do
presenter = described_class.new(notification_mail)
expect(presenter.original_sender).to eq('reply+5f85d369-8486-41c0-87ce-5a914a00b721@reply.chatwoot.com')
expect(presenter.notification_email_from_chatwoot?).to be(true)
end
end
end
end
end
@@ -381,6 +381,32 @@ describe Whatsapp::IncomingMessageService do
expect(location_attachment.coordinates_long).to eq(-122.3895553)
expect(location_attachment.external_url).to eq('http://location_url.test')
end
it 'truncates long fallback titles to avoid dropping location messages' do
long_place_name = [
'Gremi de Fusters, 33, Edificio VIP Asima, Piso 2, Local 2, Norte',
'07009 Poligon industrial de Son Castello, Illes Balears, Espana'
].join(', ')
source_id = 'wamid.long-location-fallback-title'
params = {
'contacts' => [{ 'profile' => { 'name' => 'Sojan Jose' }, 'wa_id' => '2423423243' }],
'messages' => [{ 'from' => '2423423243', 'id' => source_id,
'location' => { 'id' => 'b1c68f38-8734-4ad3-b4a1-ef0c10d683',
:address => long_place_name,
:latitude => 37.7893768,
:longitude => -122.3895553,
:name => long_place_name,
:url => 'http://location_url.test' },
'timestamp' => '1633034394', 'type' => 'location' }]
}.with_indifferent_access
expect { described_class.new(inbox: whatsapp_channel.inbox, params: params).perform }
.to change { Message.where(source_id: source_id).count }.from(0).to(1)
location_attachment = Message.find_by!(source_id: source_id).attachments.first
expect(location_attachment.fallback_title).to eq("#{long_place_name}, #{long_place_name}".first(255))
expect(location_attachment.fallback_title.length).to eq(255)
end
end
context 'when valid contact message params' do
+18 -12
View File
@@ -106,24 +106,30 @@ const tailwindConfig = {
textDecoration: 'underline',
},
ul: {
paddingInlineStart: '0.625em',
paddingInlineStart: '0',
listStylePosition: 'inside',
},
ol: {
paddingInlineStart: '0.625em',
paddingInlineStart: '0',
listStylePosition: 'inside',
},
'ul li': {
margin: '0 0 0.5em 1em',
'ul > li': {
marginBlockEnd: '0.5em',
listStyleType: 'disc',
'[dir="rtl"] &': {
margin: '0 1em 0.5em 0',
},
paddingInlineStart: '1.5em',
textIndent: '-1.5em',
},
'ol li': {
margin: '0 0 0.5em 1em',
'ol > li': {
marginBlockEnd: '0.5em',
listStyleType: 'decimal',
'[dir="rtl"] &': {
margin: '0 1em 0.5em 0',
},
paddingInlineStart: '1.5em',
textIndent: '-1.5em',
},
'li > p:first-child': {
display: 'inline',
},
'li > *': {
textIndent: '0',
},
blockquote: {
color: 'rgb(var(--slate-11))',