Skip to content

Commit 509c40a

Browse files
fix(plugin-email): a non-string recipient display name answers the send door's 400, not 500 (#29955)
<!-- claude-projects-attribution: {"github_login":"os-support-ai"} --> _Requested by **Yibo Wang** · [project thread](https://claude.ai/code/project/chan_01BVUyyPie8gHwoPM18iABwB?thread=cmsg_01BVUyyPie8gHwoPM18iABwB7J3TmFTYMYQrfpye6GQNg9)_ Fixes #29891 Clause-②: no Metadata-protocol: none ## 维护者速读 - **改了什么**:`POST /api/v1/email/send` 里 `to` / `cc` / `bcc` / `replyTo`(含列表项)的显示名不是字符串时,现在返回 400 `VALIDATION_FAILED`("the display name of … must be a string"),不再是 500 `EMAIL_SEND_FAILED`;拒收记录照常写入 `sys_email`。 - **为什么改**:拒收判断本来就对(`formatCallerAddress`,#29250),但随后写拒收行的 `rawAddressText` 对调用方原始 `name` 调 `.trim()`,抛 TypeError,把 400 变成 500。 - **风险与回滚**:只改一个内部函数(未导出),非字符串显示名不写进拒收行(`error` 列已写明原因);字符串、缺省、`null` 显示名行为不变。回滚即 revert。 - **先看**:`packages/plugins/plugin-email/src/email-service.ts` 的 `rawAddressText`;新 pin `packages/rest/src/email-send-display-name.test.ts`。 - **两分钟验证**:`pnpm --filter @objectstack/rest exec vitest run --project local src/email-send-display-name.test.ts`,预期 19 passed。 - **需要你做的**:无,席位落地。 ## What changes - **`@objectstack/plugin-email` (patch).** `rawAddressText`, the writer of the rejection row in `EmailService.recordRejectedMessage`, reads each address part as unknown input: a string display name is trimmed and kept, any other type is left out of the row, and a non-object, non-string entry records nothing. It still judges nothing. `formatCallerAddress` stays the one place a caller's recipient shape is refused (it already refused non-string names, #29250), so no second validation was added. - **`@objectstack/rest` (tests only, no changeset).** A new pin drives the real route (`HonoHttpServer` + `RestServer`) over the real `EmailServicePlugin` (log provider) with `sys_email` persistence wired, which is the stock showcase shape. `email-send-door-bounds.test.ts` uses a mail double and never reaches the writer. Supporting changes: a devDependency on `@objectstack/plugin-email` (lockfile importer row), an anchored source alias in `vitest.config.ts` (`check:test-source-alias`), the double's `update()` routed through `assertEngineUpdateDispatch`, and the regenerated `scripts/engine-double-contract.pinned.json` row (`check:engine-double-contract`). ### Same-pattern sweep (what I checked) Every caller-supplied field the rejection row reads: `to`, `cc`, `bcc`, `replyTo` and `from` all go through the one `rawAddressText`, so the fix covers all five. `subject` is `String(...)`; `text` / `html` are `typeof`-guarded; `address` is `String(...)`; `relatedObject` / `relatedId` / `sentBy` / `organizationId` are stored, not trimmed. On the validation side, `formatCallerAddress` covers all four recipient fields. `formatAddress` (the sender's path) still calls `obj.name?.trim()`, but the REST door replaces a caller `from` with the configured sender, so a caller's non-string sender name never reaches it through this route. Left as is. ## Why Reproduced on `21510bba` (main) through the new pin before the fix: every non-string shape in every field answered `500` (see the ablation below). The card's own reproduction: `{"to":{"address":"ok@example.com","name":42},"subject":"x","text":"x"}`. ## Pins `packages/rest/src/email-send-display-name.test.ts`, 19 cases: - number / object / array / boolean display name × `to` / `cc` / `bcc` / `replyTo`: 400 `VALIDATION_FAILED` with the exact message naming the field, one rejection row with `status: 'failed'`, `error: 'rejected before delivery: VALIDATION_FAILED: …'` and the bare address in the field's column (16); - a `to` list entry with an object name: 400, and the row lists both entries (1); - `null` name sends (200), and a string name sends (200) (2). **Ablation, once.** Restored `const name = a.name?.trim()` in `rawAddressText` and rebuilt: 17 of 19 failed with `AssertionError: expected 500 to be 400` (all except the two 200 controls). Restored the fix: 19 passed. **Deviation from the order (null).** The order listed `null` among the shapes that answer 400. `formatCallerAddress` deliberately treats `null` as "no name" (`name !== undefined && name !== null`), and `null?.trim()` never threw, so `null` already answered 200 before this PR. Making it 400 would refuse input the door accepts today (a narrowing), so `null` is pinned at 200 instead. ## Verification - `pnpm --filter @objectstack/plugin-email test`, `pnpm --filter @objectstack/rest test`: exit 0. - `typecheck` and `check:test-typecheck` for both packages: exit 0. - `pnpm lint`: exit 0. - Full `turbo run build --concurrency=2`: exit 0. - `node scripts/pm/dispatch-gates.mjs --commands` on the final head: 82 derived, 82 run, all exit 0; `--ran`: "82 run, 0 NOT-MEASURED". In the first round (before commit 2), `check:engine-double-contract` and `check:test-source-alias` were red and are fixed by commit 2. `check:dual-build-cjs-loads` exited 3 until the full build ran. --- 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01WGufzj6ujX8a8Krdcs92Ye <!-- signed: domain:server#6 · os-support-ai · session_01WGufzj6ujX8a8Krdcs92Ye --> --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 699ebd8 commit 509c40a

7 files changed

Lines changed: 198 additions & 1 deletion

File tree

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
---
2+
'@objectstack/plugin-email': patch
3+
---
4+
5+
fix(plugin-email): a non-string recipient display name answers the send door's 400, not 500
6+
7+
Clause-②: no
8+
Metadata-protocol: none
9+
10+
`POST /api/v1/email/send` with a recipient such as `{ "address": "ok@example.com", "name": 42 }` in `to`, `cc`, `bcc` or `replyTo` (or as one entry of a `to` / `cc` / `bcc` list) now answers `400 VALIDATION_FAILED` with "invalid recipient address in `to`: the display name of ok@example.com must be a string" (`reply-to address` for `replyTo`). It used to answer `500 EMAIL_SEND_FAILED` with `a.name?.trim is not a function`.
11+
12+
The refusal itself was already right. The 500 came from the code that writes the `sys_email` rejection row after the refusal: it called `.trim()` on the caller's raw display name. That writer now reads the name as unknown input and leaves a non-string name out of the row (the row's `error` column names it), so the caller gets the 400 and the rejection is still recorded. A string name, an absent name and a `null` name behave as before.

‎packages/plugins/plugin-email/src/email-service.ts‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -405,14 +405,22 @@ const UNSTATED_COLUMN = '(none)';
405405
* because canonicalization threw, so the row records what the caller actually
406406
* passed, verbatim, rather than throwing a second time while trying to
407407
* describe the first throw.
408+
*
409+
* It judges nothing — {@link formatCallerAddress} is the one place a caller's
410+
* address shape is refused — so it reads every part as `unknown` (#29891): a
411+
* JSON body can carry a number, object, array or `null` as a display name,
412+
* and calling `.trim()` on one turned the caller's 400 into a 500. A
413+
* non-string name is left out of the row; the `error` column already names it.
408414
*/
409415
function rawAddressText(v: EmailAddress | EmailAddress[] | undefined): string {
410416
if (v == null) return '';
411417
const one = (a: EmailAddress): string => {
412418
if (a == null) return '';
413419
if (typeof a === 'string') return a.trim();
420+
if (typeof a !== 'object') return '';
414421
const address = String(a.address ?? '').trim();
415-
const name = a.name?.trim();
422+
const rawName: unknown = a.name;
423+
const name = typeof rawName === 'string' ? rawName.trim() : '';
416424
return name ? `${name} <${address}>` : address;
417425
};
418426
return (Array.isArray(v) ? v : [v]).map(one).filter(Boolean).join(', ');

‎packages/rest/package.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@
4444
"@objectstack/driver-sql": "workspace:*",
4545
"@objectstack/metadata-protocol": "workspace:*",
4646
"@objectstack/objectql": "workspace:*",
47+
"@objectstack/plugin-email": "workspace:*",
4748
"@objectstack/plugin-hono-server": "workspace:*",
4849
"@objectstack/plugin-security": "workspace:*",
4950
"@objectstack/service-analytics": "workspace:*",
Lines changed: 162 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,162 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* #29891 — `POST /api/v1/email/send` answers 400 `VALIDATION_FAILED`, never
5+
* 500 `EMAIL_SEND_FAILED`, when a caller-supplied address (`to` / `cc` /
6+
* `bcc` / `replyTo`) carries a display name that is not a string.
7+
*
8+
* The refusal itself was already right (`formatCallerAddress`, #29152 / PR
9+
* #29250); what turned it into a 500 was the REJECTION ROW's writer
10+
* (`rawAddressText` inside `EmailService.recordRejectedMessage`), which
11+
* called `name.trim()` on the caller's raw value after the 400 was raised.
12+
* `email-send-door-bounds.test.ts` drives this door over a mail double, so it
13+
* never reaches that writer — this file drives it over the REAL
14+
* `EmailServicePlugin` (log provider) with `sys_email` persistence wired, the
15+
* stock showcase boot's shape, through the real `HonoHttpServer` transport.
16+
*/
17+
18+
import { describe, it, expect, vi } from 'vitest';
19+
import { HonoHttpServer } from '@objectstack/plugin-hono-server';
20+
import { EmailServicePlugin } from '@objectstack/plugin-email';
21+
import { assertEngineUpdateDispatch } from '@objectstack/objectql';
22+
import type { IEmailService } from '@objectstack/spec/contracts';
23+
import { RestServer } from './rest-server';
24+
import { EMAIL_SEND_CAPABILITY } from './email-routes';
25+
26+
const ADMIN = { userId: 'usr_admin', systemPermissions: ['setup.access', EMAIL_SEND_CAPABILITY] };
27+
28+
/**
29+
* The slice of ObjectQL the plugin's `sys_email` persistence touches. `update`
30+
* routes through `assertEngineUpdateDispatch`, so the double cannot accept a
31+
* dispatch the real engine would refuse.
32+
*/
33+
function fakeEngine() {
34+
const rows: Array<{ object: string; row: any }> = [];
35+
return {
36+
rows,
37+
async insert(object: string, row: any) { rows.push({ object, row }); return { id: row.id }; },
38+
async update(object: string, data: any, options?: any) {
39+
const dispatch = assertEngineUpdateDispatch(data, options);
40+
if (dispatch.kind !== 'by-id') throw new Error(`unexpected update dispatch: ${dispatch.kind}`);
41+
const target = rows.find((r) => r.object === object && r.row.id === dispatch.id);
42+
if (target) Object.assign(target.row, data);
43+
return { affected: target ? 1 : 0 };
44+
},
45+
async find() { return []; },
46+
};
47+
}
48+
49+
async function bootEmail() {
50+
const engine = fakeEngine();
51+
const services: Record<string, unknown> = { manifest: { register: () => {} }, objectql: engine };
52+
const hooks: Record<string, Array<() => Promise<void> | void>> = {};
53+
const ctx = {
54+
logger: { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() },
55+
getService: <T,>(name: string): T => {
56+
if (!(name in services)) throw new Error(`service '${name}' not registered`);
57+
return services[name] as T;
58+
},
59+
registerService: (name: string, svc: unknown) => { services[name] = svc; },
60+
hook: (name: string, fn: () => Promise<void> | void) => { (hooks[name] ??= []).push(fn); },
61+
};
62+
const plugin = new EmailServicePlugin({ provider: 'log', defaultFrom: 'no-reply@acme.example' });
63+
await plugin.init(ctx as never);
64+
await plugin.start(ctx as never);
65+
for (const fn of hooks['kernel:ready'] ?? []) await fn();
66+
return { service: services.email as IEmailService, rows: engine.rows };
67+
}
68+
69+
async function boot() {
70+
const { service, rows } = await bootEmail();
71+
const server = new HonoHttpServer(0);
72+
const rest = new RestServer(
73+
server as any,
74+
{} as any,
75+
{ api: { requireAuth: false } } as any,
76+
undefined, // kernelManager
77+
undefined, // envRegistry
78+
undefined, // defaultEnvironmentIdProvider
79+
undefined, // authServiceProvider
80+
undefined, // objectQLProvider
81+
async () => service, // emailServiceProvider
82+
);
83+
(rest as any).resolveExecCtx = async () => ADMIN;
84+
rest.registerRoutes();
85+
server.installNotFoundSeam();
86+
const post = async (body: unknown) => {
87+
const res: Response = await server.getRawApp().fetch(new Request('http://local/api/v1/email/send', {
88+
method: 'POST',
89+
headers: { 'content-type': 'application/json' },
90+
body: JSON.stringify(body),
91+
}));
92+
return { status: res.status, body: (await res.json()) as any };
93+
};
94+
return { post, rows };
95+
}
96+
97+
const OK = 'ok@example.com';
98+
const BASE = { subject: 'x', text: 'x' };
99+
100+
/** Every non-string display-name shape a JSON body can carry. */
101+
const SHAPES: Array<[string, unknown]> = [
102+
['a number', 42],
103+
['an object', { x: 1 }],
104+
['an array', ['Ada']],
105+
['a boolean', true],
106+
];
107+
108+
describe('POST /api/v1/email/send — a non-string recipient display name is the caller\'s 400 (#29891)', () => {
109+
for (const [what, name] of SHAPES) {
110+
for (const field of ['to', 'cc', 'bcc', 'replyTo'] as const) {
111+
it(`\`${field}\` with ${what} as the display name: 400 VALIDATION_FAILED naming the field, and a rejection row`, async () => {
112+
const { post, rows } = await boot();
113+
const recipient = { address: OK, name };
114+
const body = field === 'to' ? { ...BASE, to: recipient } : { ...BASE, to: 'someone@example.org', [field]: recipient };
115+
const out = await post(body);
116+
117+
expect(out.status).toBe(400);
118+
expect(out.body.code).toBe('VALIDATION_FAILED');
119+
const what2 = field === 'replyTo' ? 'reply-to address' : 'recipient address';
120+
expect(out.body.error).toBe(`invalid ${what2} in \`${field}\`: the display name of ${OK} must be a string`);
121+
122+
// The rejection is still recorded — the writer no longer throws — and
123+
// records the bare address (the non-string name is left out; the
124+
// `error` column names it).
125+
expect(rows).toHaveLength(1);
126+
const row = rows[0].row;
127+
expect(row.status).toBe('failed');
128+
expect(row.error).toBe(`rejected before delivery: VALIDATION_FAILED: ${out.body.error}`);
129+
const column = { to: 'to_addresses', cc: 'cc_addresses', bcc: 'bcc_addresses', replyTo: 'reply_to' }[field];
130+
expect(row[column]).toBe(OK);
131+
});
132+
}
133+
}
134+
135+
it('a `to` LIST entry with an object display name: 400, the row lists both entries', async () => {
136+
const { post, rows } = await boot();
137+
const out = await post({ ...BASE, to: ['first@example.org', { address: OK, name: { x: 1 } }] });
138+
139+
expect(out.status).toBe(400);
140+
expect(out.body).toEqual({
141+
code: 'VALIDATION_FAILED',
142+
error: `invalid recipient address in \`to\`: the display name of ${OK} must be a string`,
143+
});
144+
expect(rows[0].row.to_addresses).toBe(`first@example.org, ${OK}`);
145+
});
146+
147+
it('a `null` display name is "no name": the send goes through (200), as an absent one does', async () => {
148+
const { post } = await boot();
149+
const out = await post({ ...BASE, to: { address: OK, name: null } });
150+
151+
expect(out.status).toBe(200);
152+
expect(out.body.status).toBe('sent');
153+
});
154+
155+
it('a string display name still sends (200) — the guard refuses only the non-string shapes', async () => {
156+
const { post } = await boot();
157+
const out = await post({ ...BASE, to: { address: OK, name: 'Ada' } });
158+
159+
expect(out.status).toBe(200);
160+
expect(out.body.status).toBe('sent');
161+
});
162+
});

‎packages/rest/vitest.config.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -204,6 +204,12 @@ export default defineConfig({
204204
find: /^@objectstack\/plugin-hono-server$/,
205205
replacement: path.resolve(__dirname, '../plugins/plugin-hono-server/src/index.ts'),
206206
},
207+
{
208+
// #29891: `email-send-display-name.test.ts` drives the send door over
209+
// the real `EmailServicePlugin`; the fix it pins lives in that source.
210+
find: /^@objectstack\/plugin-email$/,
211+
replacement: path.resolve(__dirname, '../plugins/plugin-email/src/index.ts'),
212+
},
207213
{
208214
find: /^@objectstack\/service-datasource$/,
209215
replacement: path.resolve(__dirname, '../services/service-datasource/src/index.ts'),

‎pnpm-lock.yaml‎

Lines changed: 3 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎scripts/engine-double-contract.pinned.json‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4476,6 +4476,11 @@
44764476
"verb": "findOne",
44774477
"pinned": 1
44784478
},
4479+
{
4480+
"file": "packages/rest/src/email-send-display-name.test.ts",
4481+
"verb": "update",
4482+
"pinned": 1
4483+
},
44794484
{
44804485
"file": "packages/rest/src/meta-compound-save-force-parity.test.ts",
44814486
"verb": "delete",

0 commit comments

Comments
 (0)