Fix silent attachment dropping; cap inline images; strip raw email attachments server-side - #91
Merged
Merged
Conversation
Oversized attachments (>10MB) and attachments whose S3 upload failed were silently discarded in the content sanitizer with no log line and no trace in the response. Now they're collected as droppedAttachments (filename, mimeType, sizeBytes, reason), returned in the sanitizer response, logged as a TRACK from the sanitizer, and surfaced as a WARN from the processor when present. Also add a TRACK log when MIME parsing itself takes longer than 10s, as an earlier signal than the existing 50s near-timeout alert. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V85rptvcLi67TUmaqU1s53
…em limit Inline CID images were embedded as unbounded base64 data URIs directly in htmlBody, which is a stored DynamoDB attribute subject to the table's 400KB item cap. Now only small images (<=100KB, first 3 per message) are inlined; anything past either cap is uploaded to S3 like a regular attachment via a new InboundEmailSignalData.inlineImages list (s3Key only, never the bytes), and its cid: reference is left unresolved in htmlBody until API read time. Along the way, discovered and fixed two pre-existing issues this surfaced: - mailparser's default keepCidLinks:false silently base64-embeds every CID image into parsed.html before our own code ever runs, which is why the existing size-aware logic could never have worked without keepCidLinks: true — now set explicitly so the sanitizer's own budget decides. - The sanitizer's cid resolution keyed off attachment.contentId (the raw, angle-bracketed Content-ID header value, e.g. "<logo>") instead of attachment.cid (the bracket-stripped form actually referenced by bare `cid:` links in the HTML) — a latent mismatch that never surfaced because mailparser's own auto-embed ran first and masked it. signalsApi.ts and threadsApi.ts each had an identical local withAttachmentUrls() helper; consolidated into one shared withResolvedContentUrls() in signal-transforms.ts that both computes Attachment.url from s3Key (as before) and now also resolves any leftover cid: reference in htmlBody using inlineImages — computed lazily at read time, never persisted as a baked-in URL, matching the existing convention. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V85rptvcLi67TUmaqU1s53
…ntend The "view original email" feature previously served the fully original raw MIME (including base64 attachment bodies) and had the frontend strip attachments for display only — Copy/Download still shipped the full raw bytes to the client. Move this to the backend instead: the content sanitizer now builds a display-safe copy of the raw MIME at ingestion time (attachments fully stripped; small inline images kept intact so the .eml still renders in a real mail client if downloaded, capped at 100KB per image and 300KB cumulative per message — additional images beyond either cap are truncated), uploads it alongside extracted attachments, and the processor persists its s3Key as InboundEmailSignalData.displayRawS3Key. The GET .../signals/:id/raw endpoint now redirects to this display-safe copy when available, falling back to the true original for signals processed before this feature existed. The true original is never served through that path anymore — it stays available server-side only (e.g. for reprocessing). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V85rptvcLi67TUmaqU1s53
wparad
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
MAX_SINGLE_ATTACHMENT_SIZE, 10MB) and attachments whose S3 upload failed were silently discarded in the content sanitizer — no log line, no trace in the response. They're now collected asdroppedAttachments(filename,mimeType,sizeBytes,reason: "too_large" | "upload_failed"), returned in the sanitizer response, logged as aTRACK(content_sanitizer.attachments_dropped) from the sanitizer, and surfaced as aWARN(processor.attachments_dropped) from the processor.TRACKlog (content_sanitizer.slow_parse) when MIME parsing itself takes longer than 10s — an earlier signal than the existing 50s near-timeout alert.htmlBody— a large inline logo could blow past DynamoDB's 400KB item cap. Fixed a latent bug where mailparser's default auto-embedding bypassed our own size checks entirely (keepCidLinks: truenow disables that), and a key mismatch (attachment.contentIdvs the bracket-strippedattachment.cidmailparser actually matchescid:refs against). Now only images ≤100KB, first 3 per message, are inlined as data URIs; the rest are uploaded to S3 like regular attachments (inlineImages) and resolved to a CDN url at API read time (new sharedwithResolvedContentUrls()insignal-transforms.ts, replacing two duplicatedwithAttachmentUrls()helpers).raw-email-display.ts. Persisted asInboundEmailSignalData.displayRawS3Key; the.../signals/:id/rawendpoint redirects to it, falling back to the true original for signals processed before this existed. The true original is never served through that endpoint anymore.Test plan
npx tsc -p tsconfig.check.json --noEmitnpx eslinton all changedsrc/filescontent-sanitizer-attachments.spec.ts,content-sanitizer-inline-images.spec.ts,raw-email-display.spec.ts,content-sanitizer-display-raw.spec.ts, pluswithResolvedContentUrlscases insignal-transforms.spec.ts🤖 Generated with Claude Code
https://claude.ai/code/session_01V85rptvcLi67TUmaqU1s53
Generated by Claude Code