fix(safety): plug 3 EMAIL_REDIRECT_TO leaks + 10 unit tests + live smoke
A pre-import audit caught three places where outbound comms could escape
even with EMAIL_REDIRECT_TO set. Plugged each, added unit tests so the
behavior can't silently regress, and shipped a live smoke script the
operator can run before any production data import.
Leak 1: email-compose.service.ts (per-account user composer)
Built its own nodemailer transporter and called sendMail() directly,
bypassing the centralized sendEmail()'s redirect. Now mirrors the same
redirect: when EMAIL_REDIRECT_TO is set, "to" is rewritten, "cc" is
dropped, and the subject is prefixed with "[redirected from <orig>]".
Leak 2: documenso-client.sendDocument()
Tells Documenso to actually email the document. Recipient emails were
rerouted at create-time (in pass-3) but a document created BEFORE the
redirect was turned on could still trigger a real-client email. Now
short-circuited when the redirect is set — returns the existing doc
shape so downstream code doesn't see an unexpected null.
Leak 3: documenso-client.sendReminder()
Same shape as sendDocument: emails a stored recipient address that may
predate the redirect. Now short-circuits with a warn-level log.
Tests (tests/unit/comms-safety.test.ts):
- createDocument rewrites recipients
- generateDocumentFromTemplate rewrites both v1.13 formValues.*Email
keys AND v2.x recipients[] arrays
- sendDocument is short-circuited (no /send call)
- sendReminder is short-circuited (no /remind call)
- createDocument passes through unchanged when redirect unset
- sendEmail rewrites to + subject for single recipient
- sendEmail handles array of recipients (joined into subject prefix)
- sendEmail passes through unchanged when redirect unset
- Webhook worker reads process.env.EMAIL_REDIRECT_TO at dispatch time
(no module-level caching that could miss a runtime flip)
Live smoke (scripts/smoke-test-redirect.ts):
Monkey-patches nodemailer.createTransport, calls the real sendEmail()
with a fake real-client address, verifies the captured outbound has
the right "to" + subject. Run: `pnpm tsx scripts/smoke-test-redirect.ts`.
Exits non-zero if the redirect failed for any reason — drop-in for a
pre-deploy check.
Verification:
pnpm exec tsc --noEmit — 0 errors
pnpm exec vitest run — 936/936 (was 926, +10 new safety tests)
pnpm tsx scripts/smoke-test-redirect.ts — PASS
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -180,7 +180,26 @@ export async function generateDocumentFromTemplate(
|
||||
).then(normalizeDocument);
|
||||
}
|
||||
|
||||
/**
|
||||
* Tell Documenso to actually email the document to its recipients. The
|
||||
* recipients themselves are set at create-time (and rerouted to
|
||||
* EMAIL_REDIRECT_TO when set), but this is a belt-and-braces guard for
|
||||
* documents that may have been created BEFORE the redirect was turned on
|
||||
* (i.e. real-recipient documents now triggered by an automation while
|
||||
* we're trying to hold comms). When the redirect is on we skip the API
|
||||
* call entirely and return a synthetic "still pending" response.
|
||||
*/
|
||||
export async function sendDocument(docId: string, portId?: string): Promise<DocumensoDocument> {
|
||||
if (env.EMAIL_REDIRECT_TO) {
|
||||
logger.warn(
|
||||
{ docId, portId, redirect: env.EMAIL_REDIRECT_TO },
|
||||
'sendDocument SKIPPED — EMAIL_REDIRECT_TO is set, outbound comms paused',
|
||||
);
|
||||
// Return the existing doc shape so downstream code doesn't see an
|
||||
// unexpected null. The document remains in DRAFT/PENDING from
|
||||
// Documenso's perspective.
|
||||
return getDocument(docId, portId);
|
||||
}
|
||||
return documensoFetch(
|
||||
`/api/v1/documents/${docId}/send`,
|
||||
{
|
||||
@@ -194,11 +213,23 @@ export async function getDocument(docId: string, portId?: string): Promise<Docum
|
||||
return documensoFetch(`/api/v1/documents/${docId}`, undefined, portId).then(normalizeDocument);
|
||||
}
|
||||
|
||||
/**
|
||||
* Email a signing reminder to one recipient. Skipped entirely when
|
||||
* EMAIL_REDIRECT_TO is set — the recipient's stored email may still be
|
||||
* a real client address from before the redirect was enabled.
|
||||
*/
|
||||
export async function sendReminder(
|
||||
docId: string,
|
||||
signerId: string,
|
||||
portId?: string,
|
||||
): Promise<void> {
|
||||
if (env.EMAIL_REDIRECT_TO) {
|
||||
logger.warn(
|
||||
{ docId, signerId, portId, redirect: env.EMAIL_REDIRECT_TO },
|
||||
'sendReminder SKIPPED — EMAIL_REDIRECT_TO is set, outbound comms paused',
|
||||
);
|
||||
return;
|
||||
}
|
||||
await documensoFetch(
|
||||
`/api/v1/documents/${docId}/recipients/${signerId}/remind`,
|
||||
{
|
||||
|
||||
@@ -5,7 +5,9 @@ import { db } from '@/lib/db';
|
||||
import { emailAccounts, emailMessages, emailThreads } from '@/lib/db/schema/email';
|
||||
import { documents, documentEvents, files } from '@/lib/db/schema/documents';
|
||||
import { createAuditLog, type AuditMeta } from '@/lib/audit';
|
||||
import { env } from '@/lib/env';
|
||||
import { NotFoundError, ForbiddenError } from '@/lib/errors';
|
||||
import { logger } from '@/lib/logger';
|
||||
import { getDecryptedCredentials } from '@/lib/services/email-accounts.service';
|
||||
import { getPortEmailConfig } from '@/lib/services/port-config';
|
||||
import { sendEmail as sendSystemEmail } from '@/lib/email';
|
||||
@@ -127,12 +129,38 @@ export async function sendEmail(
|
||||
)
|
||||
: undefined;
|
||||
|
||||
// Safety net: when EMAIL_REDIRECT_TO is set, every recipient is rerouted
|
||||
// to that address and the subject is prefixed so the operator can see
|
||||
// who would have received the message. This service builds its OWN
|
||||
// transporter (per-account SMTP) so it doesn't go through sendEmail's
|
||||
// redirect — we apply the same logic here.
|
||||
const requestedTo = data.to.join(', ');
|
||||
const requestedCc = data.cc?.join(', ');
|
||||
const effectiveTo = env.EMAIL_REDIRECT_TO ?? requestedTo;
|
||||
const effectiveCc = env.EMAIL_REDIRECT_TO ? undefined : requestedCc;
|
||||
const effectiveSubject = env.EMAIL_REDIRECT_TO
|
||||
? `[redirected from ${requestedTo}${requestedCc ? `, cc=${requestedCc}` : ''}] ${data.subject}`
|
||||
: data.subject;
|
||||
if (env.EMAIL_REDIRECT_TO) {
|
||||
logger.info(
|
||||
{
|
||||
userId,
|
||||
portId,
|
||||
accountId: data.accountId,
|
||||
originalTo: requestedTo,
|
||||
originalCc: requestedCc ?? null,
|
||||
redirectedTo: env.EMAIL_REDIRECT_TO,
|
||||
},
|
||||
'email-compose redirected to EMAIL_REDIRECT_TO',
|
||||
);
|
||||
}
|
||||
|
||||
// Send via the user's SMTP transporter
|
||||
const info = await transporter.sendMail({
|
||||
from: account.emailAddress,
|
||||
to: data.to.join(', '),
|
||||
cc: data.cc?.join(', '),
|
||||
subject: data.subject,
|
||||
to: effectiveTo,
|
||||
cc: effectiveCc,
|
||||
subject: effectiveSubject,
|
||||
html: data.bodyHtml,
|
||||
inReplyTo,
|
||||
references,
|
||||
|
||||
Reference in New Issue
Block a user