Skip to content

Commit d74616f

Browse files
committed
fix: address review findings on attachment parsing edge cases
- reject stray ;key= tokens inside cid and filename values (type stays permissive for MIME parameters) - stop silently ignoring an explicit empty --attachments-file value, including in the --template conflict check - forward explicit empty --custom-return-path to the API - mention camelCase aliases in the unsupported-field error
1 parent 8ba38bd commit d74616f

6 files changed

Lines changed: 66 additions & 4 deletions

File tree

src/commands/domains/create.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ export const createDomainCommand = new Command('create')
7171
...(opts.trackingSubdomain && {
7272
trackingSubdomain: opts.trackingSubdomain,
7373
}),
74-
...(opts.customReturnPath && {
74+
...(opts.customReturnPath !== undefined && {
7575
customReturnPath: opts.customReturnPath,
7676
}),
7777
...((opts.sending || opts.receiving) && {

src/commands/emails/send.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,10 @@ export const sendCommand = new Command('send')
195195
);
196196
}
197197

198-
if (hasTemplate && (opts.attachment || opts.attachmentsFile)) {
198+
if (
199+
hasTemplate &&
200+
(opts.attachment || opts.attachmentsFile !== undefined)
201+
) {
199202
outputError(
200203
{
201204
message:
@@ -359,7 +362,7 @@ export const sendCommand = new Command('send')
359362
},
360363
);
361364

362-
if (opts.attachmentsFile) {
365+
if (opts.attachmentsFile !== undefined) {
363366
const raw = readFile(opts.attachmentsFile, globalOpts);
364367
try {
365368
attachments = [...(attachments ?? []), ...parseAttachmentsJson(raw)];

src/lib/attachments.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,13 @@ export function parseAttachmentSpec(value: string): AttachmentSpec {
4444
if (!paramValue) {
4545
throw new Error(`Empty ";${key}=" in attachment "${value}".`);
4646
}
47+
// MIME parameters like "type=text/plain;charset=utf-8" are legitimate,
48+
// but a ";key=" inside a cid or filename value is a typo.
49+
if (field !== 'contentType' && PARAM_LIKE.test(paramValue)) {
50+
throw new Error(
51+
`Unrecognized attachment parameter in "${value}". Supported: ;cid=, ;type=, ;filename= (use --attachments-file for paths containing ";key=").`,
52+
);
53+
}
4754
spec[field] = paramValue;
4855
}
4956
return spec;
@@ -83,7 +90,7 @@ export function parseAttachmentsJson(raw: string): Attachment[] {
8390
const key = FIELD_ALIASES[rawKey] ?? rawKey;
8491
if (!ALLOWED_FIELDS.has(key)) {
8592
throw new Error(
86-
`Attachment at index ${i} has unsupported field "${rawKey}". Supported: content, filename, path, content_type, content_id.`,
93+
`Attachment at index ${i} has unsupported field "${rawKey}". Supported: content, filename, path, content_type/contentType, content_id/contentId.`,
8794
);
8895
}
8996
if (key in attachment) {

tests/commands/domains/create.test.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,21 @@ describe('domains create command', () => {
130130
expect(args.customReturnPath).toBe('bounce');
131131
});
132132

133+
it('forwards an explicit empty --custom-return-path to the API', async () => {
134+
spies = setupOutputSpies();
135+
136+
const { createDomainCommand } = await import(
137+
'../../../src/commands/domains/create'
138+
);
139+
await createDomainCommand.parseAsync(
140+
['--name', 'example.com', '--custom-return-path', ''],
141+
{ from: 'user' },
142+
);
143+
144+
const args = mockCreate.mock.calls[0][0] as Record<string, unknown>;
145+
expect(args.customReturnPath).toBe('');
146+
});
147+
133148
it('omits customReturnPath when flag is absent', async () => {
134149
spies = setupOutputSpies();
135150

tests/commands/emails/send.test.ts

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1441,6 +1441,34 @@ describe('send command', () => {
14411441
expect(output).toContain('template_attachment_conflict');
14421442
});
14431443

1444+
it('does not silently ignore an empty --attachments-file value', async () => {
1445+
setNonInteractive();
1446+
errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {});
1447+
exitSpy = mockExitThrow();
1448+
1449+
const { sendCommand } = await import('../../../src/commands/emails/send');
1450+
await expectExit1(() =>
1451+
sendCommand.parseAsync(
1452+
[
1453+
'--from',
1454+
'a@test.com',
1455+
'--to',
1456+
'b@test.com',
1457+
'--subject',
1458+
'Test',
1459+
'--text',
1460+
'Hi',
1461+
'--attachments-file',
1462+
'',
1463+
],
1464+
{ from: 'user' },
1465+
),
1466+
);
1467+
1468+
const output = errorSpy.mock.calls.map((c) => c[0]).join(' ');
1469+
expect(output).toContain('file_read_error');
1470+
});
1471+
14441472
it('sends email with --react-email flag', async () => {
14451473
spies = setupOutputSpies();
14461474

tests/lib/attachments.test.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,15 @@ describe('parseAttachmentSpec', () => {
5555
);
5656
});
5757

58+
it('rejects unrecognized ;key= params inside cid and filename values', () => {
59+
expect(() => parseAttachmentSpec('./a.png;cid=logo;foo=bar')).toThrow(
60+
/Unrecognized attachment parameter/,
61+
);
62+
expect(() => parseAttachmentSpec('./a.png;filename=a;foo=bar')).toThrow(
63+
/Unrecognized attachment parameter/,
64+
);
65+
});
66+
5867
it('rejects duplicate params', () => {
5968
expect(() => parseAttachmentSpec('./a.png;cid=a;cid=b')).toThrow(
6069
/Duplicate ";cid="/,

0 commit comments

Comments
 (0)