Add file attachment support with validation and size limits#52
Open
jaballer wants to merge 4 commits into
Open
Add file attachment support with validation and size limits#52jaballer wants to merge 4 commits into
jaballer wants to merge 4 commits into
Conversation
A colleague reported that when asked to send a photo along with an email, the assistant sent the email, later decided the embedded image was corrupted, and silently sent a second "corrected" email without asking — risky behavior for anything time-sensitive (OTP codes, expiring links). Root cause: none of the four sending tools had a real attachment mechanism. Asked to attach a photo, the only option was hand-embedding a base64 data URI in htmlBody — a plain string with zero validation. LLMs can't transcribe arbitrary binary data byte-for-byte from having "seen" an image, so that embedding corrupted easily; nothing caught it, so the broken send reported success. Attachments: - sendEmail, sendEmailWithTemplate, sendBatch, sendBatchWithTemplate all accept an optional `attachments` array (name/content/contentType/ contentId), mapped to Postmark's native Attachments field. - Each attachment is validated before any Postmark request: well-formed base64 (charset, padding, round-trip re-encode — Buffer.from() silently drops invalid runs rather than throwing, so round-trip is what actually catches malformed data), a matching file signature for common image/PDF types, a forbidden-extension check, and Postmark's documented size limits. A failed check means nothing is sent, so it's always safe to retry with corrected data. - Limits enforced (sourced from Postmark's docs, linked in lib/attachments.js): 10 MB total per message (body + attachments, measured after base64 encoding), 5 MB per textBody/htmlBody section, 50 MB total per batch call, and Postmark's forbidden-extension list (exe, bat, vbs, and 28 others). Numbers are surfaced in each tool's description and in README.md so a calling model has them before attempting a send, not just after failing. Guardrail against the silent-resend behavior: - Every sending tool's description now states it sends immediately, cannot be recalled, and must not be called again to "correct" a previous send without user confirmation — called out specifically for OTP codes and expiring links. This is a prompt-level mitigation (the server can't force a client to comply) but reduces the risk in practice. Refactor: - Attachment validation moved to lib/attachments.js, mirroring the existing lib/log.js split, so it's directly unit-testable with multi-megabyte inputs at millisecond speed instead of only reachable by driving a real MCP/stdio round-trip (which took 27s for a single 50MB test before the move). Tests: 83 total (57 unit + 19 offline + 7 e2e), full suite ~4.5s. Verified live against a real Postmark account: valid attachment sent and rendered correctly; corrupted attachment rejected locally with nothing sent.
…ral validation Live-tested the previous commit's fix immediately: sent a real ~68 KB PNG through Claude Desktop and it arrived with the same filename, no image rendering, and a different size — silently, no error. The attachment validation added moments earlier should have caught this and didn't. Root cause: the file-signature check only compared the first few header bytes (8 for PNG) against the expected magic number. A model transcribing a long base64 string tends to get the short, position-stable header right while the body drifts — so the file still "matched the signature" while being corrupted everywhere else. Confirmed by reproduction: reading the real file, flipping one byte deep in its IDAT data, and re-running it through validateAttachment showed the old check passing it cleanly. Fix: full structural validation per type instead of a magic-byte check. - PNG: walk the entire chunk stream and verify every chunk's CRC32, and that the chunks exactly span the buffer ending in IEND. - JPEG/GIF: require both the start-of-file marker and the correct end-of-file marker/trailer (catches truncation and tail corruption; full segment parsing isn't needed for that). - WEBP: the RIFF header's declared size must equal the actual buffer length. - PDF: must end in %%EOF (allowing trailing whitespace). Verified against the real file that triggered this: the untouched original passes; a single corrupted byte anywhere in the body, or truncation, is now rejected before anything is sent. Also sent the real file through the live sendEmail tool end-to-end to confirm delivery. Tests: 11 new (real-fixture regression for the exact corruption pattern, plus synthetic framing tests for JPEG/GIF/WEBP/PDF). 94 total, ~4.5s.
…se64 Attachments now accept `path` as an alternative to `content`. The server reads the file itself, inferring name and contentType, so a caller emits a short path instead of tens of thousands of base64 characters. MCP has no mechanism for a client to hand a server file bytes: tool arguments are JSON only, image content blocks are result-side, and roots carry file:// URIs but never bytes. That left base64-in-arguments as the only channel, and it truncates in practice — a 70 KB PNG is ~93k characters, and the observed failures were "length is not a multiple of 4" plus one call that lost `contentType` (the field right after `content`) entirely, the signature of a call that ran past its output limit. - POSTMARK_ATTACHMENT_DIR optionally confines path reads to one directory, resolving both sides through realpath so a symlink cannot escape it - base64 failure messages now redirect to `path` rather than reading as "fix your encoding", which invited an endless retry loop - paths under agent-sandbox prefixes are diagnosed as a filesystem mismatch instead of a missing file, since the caller may run in a different container than the server process - fix: sanitizeArgs did not recognise path-only attachments and dropped the filename from the audit log; it now matches both input modes and logs the basename only, never the directory layout Oversized files are rejected from stat before being read. Structural integrity checks still run on path-read files, since a file on disk can itself be corrupt.
…se64 Attachments now accept `path` as an alternative to `content`. The server reads the file itself, inferring name and contentType, so a caller emits a short path instead of tens of thousands of base64 characters. MCP has no mechanism for a client to hand a server file bytes: tool arguments are JSON only, image content blocks are result-side, and roots carry file:// URIs but never bytes. That left base64-in-arguments as the only channel, and it truncates in practice — a 70 KB PNG is ~93k characters, and the observed failures were "length is not a multiple of 4" plus one call that lost `contentType` (the field right after `content`) entirely, the signature of a call that ran past its output limit. - POSTMARK_ATTACHMENT_DIR optionally confines path reads to one directory, resolving both sides through realpath so a symlink cannot escape it - base64 failure messages now redirect to `path` rather than reading as "fix your encoding", which invited an endless retry loop - paths under agent-sandbox prefixes are diagnosed as a filesystem mismatch instead of a missing file, since the caller may run in a different container than the server process - fix: sanitizeArgs did not recognise path-only attachments and dropped the filename from the audit log; it now matches both input modes and logs the basename only, never the directory layout Oversized files are rejected from stat before being read. Structural integrity checks still run on path-read files, since a file on disk can itself be corrupt.
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
1 participant
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
attachmentsfield:name/content/contentType/contentId) tosendEmail,sendEmailWithTemplate,sendBatch, andsendBatchWithTemplate, replacing the only prior workaround — hand-embedding base64 inhtmlBody, which had zero validation and was prone to silent corruption.%%EOF. This closes a real gap found via live testing (see second commit): a header-only signature check let a body-corrupted attachment through silently, since a model transcribing a long base64 string tends to get the short header right while the body drifts.lib/attachments.js, mirroring the existinglib/log.jssplit, so size-limit tests (which need multi-megabyte inputs) run in milliseconds instead of the tens of seconds a real MCP/stdio round-trip would take.Test plan
Notes
postmark-logo.png) was used locally to reproduce and verify the structural-validation fix but is intentionally not part of this PR (untracked, not committed).