From 4f9558f636eb0c7fb603ef52242e96dc36d7ffed Mon Sep 17 00:00:00 2001 From: israel Date: Thu, 3 Sep 2026 10:33:18 +0100 Subject: [PATCH] fix(platform): say when an email attachment is no longer available MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A message that advertises an attachment whose bytes are gone offered a download that failed with a browser error and no explanation. Presigning does not prove the object is there — it signs a path. So the read path already re-derived each URL and dropped it when the presign itself failed, but a presign for a missing object succeeds, and the chip went on showing a file size and a download button. The probe rides the presign loop that already walks each attachment, using the existing statOrgBlob rather than a second pass over the same metadata. A probe that THROWS is not evidence of absence — an unreachable store, an unresolved org — so it fails open and the attachment is offered as before. Only a definite miss marks it: url withheld, unavailable stamped, and the chip reads 'No longer available' where the size was. Closes #3017. Supersedes #3104, three of whose files #3125 deleted. --- .../conversations/components/message.test.tsx | 47 ++++++++++++++++++ .../conversations/components/message.tsx | 7 ++- .../backend/domains/conversations/service.ts | 26 +++++++++- .../platform/backend/integration-check.ts | 48 +++++++++++++++---- .../shared/conversations/conversation-item.ts | 4 ++ services/platform/messages/de.yml | 1 + services/platform/messages/en.yml | 1 + services/platform/messages/fr.yml | 1 + 8 files changed, 124 insertions(+), 11 deletions(-) diff --git a/services/platform/app/features/conversations/components/message.test.tsx b/services/platform/app/features/conversations/components/message.test.tsx index c9934e082f..70e707ce5f 100644 --- a/services/platform/app/features/conversations/components/message.test.tsx +++ b/services/platform/app/features/conversations/components/message.test.tsx @@ -270,4 +270,51 @@ describe('Message — attachment list vs inline images', () => { ); expect(screen.queryByText('logo.png')).not.toBeInTheDocument(); }); + + it('says the file is no longer available instead of its size', () => { + // The size reads as a promise the message cannot keep. + render( + My CV is attached.

', + attachments: [attachment({ unavailable: true, url: undefined })], + })} + />, + ); + expect(screen.getByText('attachment.unavailable')).toBeInTheDocument(); + expect(screen.queryByText(/23,359|22\.8/)).not.toBeInTheDocument(); + }); + + it('offers no download for an attachment whose bytes are gone', () => { + render( + My CV is attached.

', + attachments: [attachment({ unavailable: true, url: undefined })], + })} + />, + ); + expect( + screen.queryByRole('button', { name: /attachment\.download CV\.pdf/ }), + ).not.toBeInTheDocument(); + }); + + it('still shows the size and a download for an attachment that is there', () => { + // The other half of the pair: a flag that hid every attachment would + // pass the two cases above on its own. + render( + My CV is attached.

', + attachments: [attachment()], + })} + />, + ); + expect( + screen.queryByText('attachment.unavailable'), + ).not.toBeInTheDocument(); + expect( + screen.getByRole('button', { name: /attachment\.download CV\.pdf/ }), + ).toBeInTheDocument(); + }); }); diff --git a/services/platform/app/features/conversations/components/message.tsx b/services/platform/app/features/conversations/components/message.tsx index b446335d6d..579afbd53b 100644 --- a/services/platform/app/features/conversations/components/message.tsx +++ b/services/platform/app/features/conversations/components/message.tsx @@ -118,6 +118,7 @@ interface AttachmentCardProps { size: number; storageId?: string; url?: string; + unavailable?: boolean; }; } @@ -149,7 +150,11 @@ function AttachmentCard({ attachment }: AttachmentCardProps) { ); diff --git a/services/platform/backend/domains/conversations/service.ts b/services/platform/backend/domains/conversations/service.ts index daf1124ffb..cad997f897 100644 --- a/services/platform/backend/domains/conversations/service.ts +++ b/services/platform/backend/domains/conversations/service.ts @@ -13,7 +13,7 @@ import { notifyConversationAssignedTeam, } from '../collab/service.ts'; import { emitEvent } from '../events/emit.ts'; -import { getFileUrl } from '../files/service.ts'; +import { getFileUrl, statOrgBlob } from '../files/service.ts'; import { assertNotHeld } from '../legal_holds/service.ts'; /** @@ -986,6 +986,30 @@ export async function presignMessageAttachments( { organizationId }, raw.storageId, ); + // Presigning does NOT prove the object is there — it signs a + // path. Without this the message keeps offering a download that + // fails with a browser error and no explanation, which is what + // #3017 saw after a recovery re-imported the database without + // file storage. + // + // A probe that THROWS is not evidence of absence (unreachable + // store, unresolved org), so it fails open and the attachment is + // offered as before. Only a definite `null` marks it. + let gone = false; + try { + gone = + (await statOrgBlob(sql, organizationId, raw.storageId)) === + null; + } catch (error) { + console.warn( + `[conversations] attachment presence probe failed for ${raw.storageId}, offering it anyway:`, + error, + ); + } + if (gone) { + const { url: _unreachable, ...rest } = raw; + return { ...rest, unavailable: true }; + } return { ...raw, url }; } catch (error) { console.warn( diff --git a/services/platform/backend/integration-check.ts b/services/platform/backend/integration-check.ts index 22f4d9cbef..42e9a80902 100644 --- a/services/platform/backend/integration-check.ts +++ b/services/platform/backend/integration-check.ts @@ -14873,6 +14873,10 @@ async function checkConversations( // ── A bytesless attachment (Gmail/Outlook chips the connector never fetched) // must offer NO download affordance — the detail projection presigns a URL // only from a stored storageId, so a metadata-only chip carries none. + const attSlugRows = await sql<{ slug: string }[]>` + SELECT "slug" FROM "organization" WHERE "id" = ${orgId} LIMIT 1 + `; + const attOrgSlug = attSlugRows[0]?.slug ?? ''; await sql` INSERT INTO app.conversation_messages ( org_id, conversation_id, channel, direction, external_message_id, @@ -14891,10 +14895,23 @@ async function checkConversations( contentType: 'image/jpeg', size: 0, }, + // A ref whose blob is NOT in the store. Presigning signs a path and + // succeeds anyway, so without a presence probe the message keeps + // offering a download that 404s (#3017). + { + id: 'att-gone', + filename: 'gone.pdf', + contentType: 'application/pdf', + size: 23_359, + storageId: `s3:${attOrgSlug}/att-gone-never-uploaded.pdf`, + }, ], })}, ${Date.now()} ) `; + // The ref has to be org-scoped or `requireOrgScopedKey` refuses it before + // any presence probe runs — which is how the first draft of this check + // passed for the wrong reason. const attDetail = z .object({ item: z @@ -14913,18 +14930,31 @@ async function checkConversations( }) .loose() .safeParse(await (await api(`/${conversationId}`)).json()); - const bytelessChip = attDetail.success - ? attDetail.data.item.messages - .flatMap((message) => message.attachments ?? []) - .find((attachment) => attachment.id === 'att-nobytes') - : undefined; - record( - 'conversations: connectorName filters the Inbox, and a bytesless attachment offers no download', + const attChips = attDetail.success + ? attDetail.data.item.messages.flatMap( + (message) => message.attachments ?? [], + ) + : []; + const bytelessChip = attChips.find( + (attachment) => attachment.id === 'att-nobytes', + ); + // Only assertable with a store to probe: `statOrgBlob` throws without one, + // and that failure deliberately fails OPEN — a store it cannot reach is not + // evidence the blob is gone. + const goneChip = attChips.find((attachment) => attachment.id === 'att-gone'); + const goneMarked = + !process.env.ITEST_S3_ENDPOINT || + (goneChip !== undefined && + goneChip.unavailable === true && + !('url' in goneChip)); + record( + 'conversations: connectorName filters the Inbox; a bytesless and a vanished attachment both offer no download', matchedFilter.includes(conversationId) && !wrongFilter.includes(conversationId) && bytelessChip !== undefined && - !('url' in bytelessChip), - `imapFilter=${matchedFilter.includes(conversationId)} gmailExcluded=${!wrongFilter.includes(conversationId)} bytelessChipNoUrl=${bytelessChip !== undefined && !('url' in bytelessChip)}`, + !('url' in bytelessChip) && + goneMarked, + `imapFilter=${matchedFilter.includes(conversationId)} gmailExcluded=${!wrongFilter.includes(conversationId)} bytelessChipNoUrl=${bytelessChip !== undefined && !('url' in bytelessChip)} goneMarked=${goneMarked}${process.env.ITEST_S3_ENDPOINT ? ` (unavailableFlag=${goneChip?.unavailable === true} url=${goneChip !== undefined && 'url' in goneChip})` : ' (SKIPPED, no ITEST_S3_ENDPOINT)'}`, ); const closed = z diff --git a/services/platform/lib/shared/conversations/conversation-item.ts b/services/platform/lib/shared/conversations/conversation-item.ts index 9e7368ed80..ccdf60d7c9 100644 --- a/services/platform/lib/shared/conversations/conversation-item.ts +++ b/services/platform/lib/shared/conversations/conversation-item.ts @@ -75,6 +75,9 @@ export interface ProjectedMessage { storageId?: string; url?: string; contentId?: string; + /** The bytes are gone, so this cannot be offered for download. Stamped at + * read time from live storage; `url` is absent alongside it. */ + unavailable?: boolean; }[]; } @@ -117,6 +120,7 @@ function projectConversationMessage( ...(typeof a.contentId === 'string' ? { contentId: a.contentId } : {}), + ...(a.unavailable === true ? { unavailable: true } : {}), })) : undefined; const deliveryState = message.deliveryState || 'sent'; diff --git a/services/platform/messages/de.yml b/services/platform/messages/de.yml index e6809b5083..e35c9a4bc4 100644 --- a/services/platform/messages/de.yml +++ b/services/platform/messages/de.yml @@ -1835,6 +1835,7 @@ conversations: send: Nachricht senden attachment: download: Herunterladen + unavailable: Nicht mehr verfügbar attachments: '{count, plural, one {# Anhang} other {# Anhänge}}' panel: uploadFailed: Anhänge konnten nicht hochgeladen werden. Versuch es erneut. diff --git a/services/platform/messages/en.yml b/services/platform/messages/en.yml index 8aeaeadc2c..738d16875c 100644 --- a/services/platform/messages/en.yml +++ b/services/platform/messages/en.yml @@ -1778,6 +1778,7 @@ conversations: send: Send message attachment: download: Download + unavailable: No longer available attachments: '{count, plural, one {# attachment} other {# attachments}}' panel: uploadFailed: Couldn't upload attachments. Try again. diff --git a/services/platform/messages/fr.yml b/services/platform/messages/fr.yml index c69a7e20b5..50343eb8b1 100644 --- a/services/platform/messages/fr.yml +++ b/services/platform/messages/fr.yml @@ -1851,6 +1851,7 @@ conversations: send: Envoyer le message attachment: download: Télécharger + unavailable: Plus disponible attachments: '{count, plural, one {# pièce jointe} other {# pièces jointes}}' panel: uploadFailed: Échec du téléversement des pièces jointes. Merci de réessayer.