Skip to content

Add SMTP attachment support - #124

Open
diesdasjunge wants to merge 1 commit into
opencoredev:mainfrom
diesdasjunge:add-attachments-to-smtp-adapter
Open

Add SMTP attachment support#124
diesdasjunge wants to merge 1 commit into
opencoredev:mainfrom
diesdasjunge:add-attachments-to-smtp-adapter

Conversation

@diesdasjunge

@diesdasjunge diesdasjunge commented Jul 5, 2026

Copy link
Copy Markdown

Why

The built-in SMTP adapter rejected attachments, so it could not serve as a fallback for messages that include files. The original implementation was written before the v1 adapter API and no longer merged cleanly with main.

What changed

  • port SMTP attachment support to the v1 adapter contract
  • build multipart/mixed messages with nested multipart/alternative bodies when both text and HTML are present
  • support raw, Base64, byte, Blob, ArrayBuffer, and file-backed attachment content through the shared attachment decoder
  • validate attachment filenames, content types, content IDs, and dispositions before opening an SMTP connection
  • update the v1 SMTP docs, README, field-support data, and generated snapshot
  • add a minor changeset for @opencoredev/email-sdk

Testing

  • bun run release:ci
  • bun test packages/email-sdk/src/smtp.test.ts packages/email-sdk/src/cli.test.ts
  • bunx oxlint on the touched TypeScript files
  • git diff --check

@tripwire-prod

tripwire-prod Bot commented Jul 5, 2026

Copy link
Copy Markdown

Tripwire: This PR was automatically closed.

Reason: @diesdasjunge does not have a profile README.

Think this was a mistake? Request a review as @diesdasjunge

@tripwire-prod tripwire-prod Bot closed this Jul 5, 2026
@vercel

vercel Bot commented Jul 5, 2026

Copy link
Copy Markdown

@diesdasjunge is attempting to deploy a commit to the OpenCore Team on Vercel.

A member of the Team first needs to authorize it.

@leoisadev1 leoisadev1 reopened this Jul 7, 2026
@leoisadev1
leoisadev1 force-pushed the add-attachments-to-smtp-adapter branch from 5c7d893 to a8f65e6 Compare July 30, 2026 16:00
@leoisadev1
leoisadev1 marked this pull request as ready for review July 30, 2026 16:00
Comment on lines +628 to +635
if (
typeof attachment.filename !== "string" ||
!/^[\x20-\x21\x23-\x5b\x5d-\x7e]+$/.test(attachment.filename)
) {
throw new EmailValidationError(
`SMTP attachment filename ${JSON.stringify(attachment.filename)} contains invalid characters.`,
);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Internationalized filenames are rejected

SMTP attachment validation only permits ASCII filenames, so valid names such as résumé.pdf and 請求書.pdf fail with EmailValidationError before SMTP connects. This prevents delivery of attachments with internationalized filenames. Preserve control-character and MIME-parameter injection protections, but support properly encoded MIME filename parameters with a safe ASCII fallback where appropriate.

Artifacts

SMTP Unicode filename reproduction script

  • Runs the actual SDK SMTP send path against an in-process SMTP server and compares an ASCII attachment filename with two Unicode filenames; it proves the Unicode inputs are rejected before connection.

SMTP Unicode filename reproduction output

  • Captured output of the focused Bun reproduction command, including its working directory and zero exit status; it shows ASCII send succeeds while both Unicode filenames are rejected before connection.

SMTP validation and MIME implementation output

  • Captured numbered source excerpts for the attachment validator and MIME part builder; it shows ASCII-only validation and no encoded filename parameter path.

View artifacts

T-Rex Ran code and verified through T-Rex

Fix in Codex

Comment on lines +56 to 57
const raw = await buildMimeMessage(message, options.defaults, context.idempotencyKey);
const client = new SmtpClient(options, port, context.idempotencyKey);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Attachment loading ignores cancellation

send() builds the entire MIME message without checking or passing context.signal. An aborted send can remain blocked on attachment loading and then transmit the completed message after cancellation. Propagate the signal through MIME construction and file reads, and check it before opening or continuing SMTP delivery.

Artifacts

Executable aborted SMTP attachment reproduction

  • This Bun script uses a local SMTP server and delayed FIFO-backed attachment read, aborting at 50ms before releasing the file at 350ms; takeaway: it reproduces SMTP delivery after cancellation.

Aborted SMTP reproduction output

  • Captured output for `ABORT_DELAY_MS=50 bun trex-artifacts/smtp-abort-attachment-repro.ts` shows `aborted: true`, `EmailAbortError`, `delivered: true`, and `attachmentEncoded: true`; takeaway: the message was sent after abort.

View artifacts

T-Rex Ran code and verified through T-Rex

Fix in Codex

@greptile-apps

greptile-apps Bot commented Jul 30, 2026

Copy link
Copy Markdown

Greptile Summary

Adds SMTP attachment delivery, multipart MIME construction, attachment validation, test coverage, and documentation updates.

Two attachment-delivery failures were reproduced:

  • SMTP rejects valid Unicode attachment filenames such as résumé.pdf and 請求書.pdf before connecting to the server.
  • Aborting a send while a file-backed attachment is loading does not stop MIME construction or SMTP delivery; the caller receives an abort error even though the message and attachment are transmitted.

Do not merge until SMTP supports safely encoded internationalized filenames and cancellation prevents post-abort delivery.

Confidence Score: 3/5

T-Rex T-Rex Logs

What T-Rex did

  • T-Rex reproduced the P1 finding by running the SMTP Unicode filename reproduction script and captured the output and validation results.
  • T-Rex produced a second P1 finding proof showing an aborted SMTP attachment reproduction and its output.
  • T-Rex produced a third P1 finding proof that had no artifacts recorded.
  • T-Rex performed a general contract validation by executing bun trex-artifacts/smtp-unicode-filename-repro.ts and documenting the exact command, observed ASCII vs Unicode behavior, the uploaded sources, and the command output.

View all artifacts

T-Rex Ran code and verified through T-Rex

Comments Outside Diff (1)

  1. General comment

    P1 SMTP attachments cannot use valid Unicode filenames

    • Bug
      • smtp().send() rejects résumé.pdf and 請求書.pdf with EmailValidationError before any SMTP connection, so users cannot send/build messages containing those attachment names.
    • Cause
      • packages/email-sdk/src/utils.ts:629-630 restricts attachment filenames to an ASCII character class. Further, packages/email-sdk/src/smtp.ts:343 and :345 only interpolate a raw quoted name=/filename= value, with no RFC 2231/RFC 5987 encoded filename parameter implementation.
    • Fix
      • Allow Unicode filenames only alongside safe MIME parameter encoding. Emit a compatible ASCII fallback plus RFC 2231/RFC 5987 extended parameters (for example filename*=UTF-8''... and corresponding name*=), while retaining CR/LF and parameter-injection protections.

    T-Rex Ran code and verified through T-Rex

Fix All in Codex

Reviews (1): Last reviewed commit: "feat(email-sdk): add SMTP attachment sup..." | Re-trigger Greptile

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants