feat(server): validate notification request bodies

notifications.dto.ts wraps the @trek/shared request schemas via createZodDto
and types all five @Body() params; the five NotificationsController entries
leave the body-contract allow-list (ratchet gate enforced). Contract catch:
the client sends server/token: null on test-ntfy to mean 'use the saved
value', so testNtfyRequestSchema gains .nullable() there (spec pinned). The
inline response-enum and url-type checks die — malformed bodies now get the
pipe's standard envelope; valid bodies behave byte-identically. Two
integration cases pinning pre-ratchet tolerances (flat notify_* preferences
body, body-less test-smtp POST) update to the current wire contract.
This commit is contained in:
jubnl
2026-08-02 11:36:36 +02:00
parent d203d8cccd
commit 00447c9043
7 changed files with 81 additions and 42 deletions
@@ -54,11 +54,6 @@ export const BODY_CONTRACT_ALLOW_LIST: string[] = [
'JourneyController.uploadEntryPhotos',
'JourneyController.uploadGalleryVideo',
'LlmLocalController.pull',
'NotificationsController.respond',
'NotificationsController.setPreferences',
'NotificationsController.testNtfy',
'NotificationsController.testSmtp',
'NotificationsController.testWebhook',
'OauthApiController.authorize',
'OauthApiController.createClient',
'PluginRoutesController.route',
@@ -14,6 +14,13 @@ import {
import type { ChannelTestResult, UnreadCountResult } from '@trek/shared';
import type { User } from '../../types';
import { NotificationsService } from './notifications.service';
import {
PreferencesUpdateDto,
TestSmtpDto,
TestWebhookDto,
TestNtfyDto,
NotificationRespondDto,
} from './notifications.dto';
import { JwtAuthGuard } from '../auth/jwt-auth.guard';
import { CurrentUser } from '../auth/current-user.decorator';
@@ -31,6 +38,10 @@ const MASKED = '••••••••';
* codes. POSTs that answer with res.json stay 200 (Nest would default to 201).
* The static /in-app/read-all and /in-app/all routes are declared before the
* /in-app/:id routes so they win over the param, matching the legacy order.
* Bodies validate via notifications.dto.ts (@trek/shared schemas through the
* global ZodValidationPipe) — malformed bodies now get the pipe's standard
* { error: 'field: message; …' } envelope instead of the old inline checks
* (the sanctioned ratchet behavior); valid bodies behave byte-identically.
*/
@Controller('api/notifications')
@UseGuards(JwtAuthGuard)
@@ -43,24 +54,24 @@ export class NotificationsController {
}
@Put('preferences')
setPreferences(@CurrentUser() user: User, @Body() body: Record<string, Record<string, boolean>>) {
setPreferences(@CurrentUser() user: User, @Body() body: PreferencesUpdateDto) {
this.notifications.setPreferences(user.id, body);
return this.notifications.getPreferences(user.id, user.role);
}
@Post('test-smtp')
@HttpCode(200)
async testSmtp(@CurrentUser() user: User, @Body('email') email?: string): Promise<ChannelTestResult> {
async testSmtp(@CurrentUser() user: User, @Body() body: TestSmtpDto): Promise<ChannelTestResult> {
if (user.role !== 'admin') {
throw new HttpException({ error: 'Admin only' }, 403);
}
return this.notifications.testSmtp(email || user.email);
return this.notifications.testSmtp(body.email || user.email);
}
@Post('test-webhook')
@HttpCode(200)
async testWebhook(@CurrentUser() user: User, @Body('url') urlInput?: unknown): Promise<ChannelTestResult> {
let url = urlInput;
async testWebhook(@CurrentUser() user: User, @Body() body: TestWebhookDto): Promise<ChannelTestResult> {
let url: string | null | undefined = body.url;
if (!url || url === MASKED) {
url = this.notifications.userWebhookUrl(user.id);
if (!url && user.role === 'admin') url = this.notifications.adminWebhookUrl();
@@ -68,9 +79,6 @@ export class NotificationsController {
throw new HttpException({ error: 'No webhook URL configured' }, 400);
}
}
if (typeof url !== 'string') {
throw new HttpException({ error: 'url must be a string' }, 400);
}
try {
new URL(url);
} catch {
@@ -81,12 +89,8 @@ export class NotificationsController {
@Post('test-ntfy')
@HttpCode(200)
async testNtfy(
@CurrentUser() user: User,
@Body('topic') topic?: string,
@Body('server') server?: string,
@Body('token') token?: string,
): Promise<ChannelTestResult> {
async testNtfy(@CurrentUser() user: User, @Body() body: TestNtfyDto): Promise<ChannelTestResult> {
const { topic, server, token } = body;
const userCfg = this.notifications.userNtfyConfig(user.id);
const adminCfg = this.notifications.adminNtfyConfig();
@@ -176,13 +180,10 @@ export class NotificationsController {
async respond(
@CurrentUser() user: User,
@Param('id') idParam: string,
@Body('response') response?: unknown,
@Body() body: NotificationRespondDto,
): Promise<{ success: boolean; notification: unknown }> {
const id = this.parseId(idParam);
if (response !== 'positive' && response !== 'negative') {
throw new HttpException({ error: 'response must be "positive" or "negative"' }, 400);
}
const result = await this.notifications.respond(id, user.id, response);
const result = await this.notifications.respond(id, user.id, body.response);
if (!result.success) {
throw new HttpException({ error: result.error }, 400);
}
@@ -0,0 +1,21 @@
import { createZodDto } from 'nestjs-zod';
import {
preferencesUpdateRequestSchema,
testSmtpRequestSchema,
testWebhookRequestSchema,
testNtfyRequestSchema,
notificationRespondRequestSchema,
} from '@trek/shared';
/**
* Server-side createZodDto wrappers over the @trek/shared notification
* contracts. The global ZodValidationPipe (APP_PIPE in app.module.ts)
* validates any @Body() parameter typed with one of these classes by
* metatype — the Zod schemas in shared/ remain the single source of truth
* for the wire contract.
*/
export class PreferencesUpdateDto extends createZodDto(preferencesUpdateRequestSchema) {}
export class TestSmtpDto extends createZodDto(testSmtpRequestSchema) {}
export class TestWebhookDto extends createZodDto(testWebhookRequestSchema) {}
export class TestNtfyDto extends createZodDto(testNtfyRequestSchema) {}
export class NotificationRespondDto extends createZodDto(notificationRespondRequestSchema) {}
@@ -95,10 +95,13 @@ describe('Notification preferences', () => {
it('NOTIF-001 — PUT /api/notifications/preferences updates settings', async () => {
const { user } = createUser(testDb);
// The DTO ratchet enforces the matrix shape the client actually sends
// ({ event: { channel: enabled } }); the pre-matrix flat notify_* body
// this case used to send is rejected by the pipe now.
const res = await request(app)
.put('/api/notifications/preferences')
.set('Cookie', authCookie(user.id))
.send({ notify_trip_invite: true, notify_booking_change: false });
.send({ trip_invite: { email: true }, booking_change: { email: false } });
expect(res.status).toBe(200);
expect(res.body).toHaveProperty('preferences');
});
@@ -310,9 +313,13 @@ describe('Notification test endpoints', () => {
it('NOTIF-005 — POST /api/notifications/test-smtp requires admin', async () => {
const { user } = createUser(testDb);
// Send the empty JSON body the client sends ({ email: undefined } →
// {}): a completely body-less POST has no content-type, so the DTO pipe
// rejects it before the admin gate since the ratchet.
const res = await request(app)
.post('/api/notifications/test-smtp')
.set('Cookie', authCookie(user.id));
.set('Cookie', authCookie(user.id))
.send({});
// Non-admin gets 403
expect(res.status).toBe(403);
});
@@ -1,6 +1,7 @@
import { describe, it, expect, vi } from 'vitest';
import { HttpException } from '@nestjs/common';
import { NotificationsController } from '../../../src/nest/notifications/notifications.controller';
import { NotificationRespondDto } from '../../../src/nest/notifications/notifications.dto';
import type { NotificationsService } from '../../../src/nest/notifications/notifications.service';
import type { User } from '../../../src/types';
@@ -43,7 +44,7 @@ describe('NotificationsController (parity with the legacy /api/notifications rou
describe('test-smtp', () => {
it('403 { error: Admin only } for a non-admin (distinct from AdminGuard wording)', async () => {
const testSmtp = vi.fn();
expect(await thrown(() => makeController({ testSmtp }).testSmtp(user))).toEqual({
expect(await thrown(() => makeController({ testSmtp }).testSmtp(user, {}))).toEqual({
status: 403, body: { error: 'Admin only' },
});
expect(testSmtp).not.toHaveBeenCalled();
@@ -51,7 +52,7 @@ describe('NotificationsController (parity with the legacy /api/notifications rou
it('falls back to the admin\'s own email when none given', async () => {
const testSmtp = vi.fn().mockResolvedValue({ success: true });
await makeController({ testSmtp }).testSmtp(admin);
await makeController({ testSmtp }).testSmtp(admin, {});
expect(testSmtp).toHaveBeenCalledWith('admin@example.test');
});
});
@@ -59,27 +60,27 @@ describe('NotificationsController (parity with the legacy /api/notifications rou
describe('test-webhook', () => {
it('uses the provided url', async () => {
const testWebhook = vi.fn().mockResolvedValue({ success: true });
await makeController({ testWebhook }).testWebhook(user, 'https://hooks.example/x');
await makeController({ testWebhook }).testWebhook(user, { url: 'https://hooks.example/x' });
expect(testWebhook).toHaveBeenCalledWith('https://hooks.example/x');
});
it('falls back to the saved user url when the masked placeholder is sent', async () => {
const testWebhook = vi.fn().mockResolvedValue({ success: true });
const userWebhookUrl = vi.fn().mockReturnValue('https://saved.example/u');
await makeController({ testWebhook, userWebhookUrl }).testWebhook(user, MASKED);
await makeController({ testWebhook, userWebhookUrl }).testWebhook(user, { url: MASKED });
expect(userWebhookUrl).toHaveBeenCalledWith(4);
expect(testWebhook).toHaveBeenCalledWith('https://saved.example/u');
});
it('400 when no url is configured', async () => {
const userWebhookUrl = vi.fn().mockReturnValue(null);
expect(await thrown(() => makeController({ userWebhookUrl }).testWebhook(user, undefined))).toEqual({
expect(await thrown(() => makeController({ userWebhookUrl }).testWebhook(user, {}))).toEqual({
status: 400, body: { error: 'No webhook URL configured' },
});
});
it('400 on an invalid url', async () => {
expect(await thrown(() => makeController({}).testWebhook(user, 'not a url'))).toEqual({
expect(await thrown(() => makeController({}).testWebhook(user, { url: 'not a url' }))).toEqual({
status: 400, body: { error: 'Invalid URL' },
});
});
@@ -89,7 +90,7 @@ describe('NotificationsController (parity with the legacy /api/notifications rou
it('400 when no topic can be resolved', async () => {
const userNtfyConfig = vi.fn().mockReturnValue(null);
const adminNtfyConfig = vi.fn().mockReturnValue({ server: null, token: null });
expect(await thrown(() => makeController({ userNtfyConfig, adminNtfyConfig }).testNtfy(user))).toEqual({
expect(await thrown(() => makeController({ userNtfyConfig, adminNtfyConfig }).testNtfy(user, {}))).toEqual({
status: 400, body: { error: 'No ntfy topic configured' },
});
});
@@ -98,7 +99,7 @@ describe('NotificationsController (parity with the legacy /api/notifications rou
const testNtfy = vi.fn().mockResolvedValue({ success: true });
const userNtfyConfig = vi.fn().mockReturnValue({ topic: 'saved-topic', server: 'https://ntfy.me', token: 'saved-token' });
const adminNtfyConfig = vi.fn().mockReturnValue({ server: null, token: null });
await makeController({ testNtfy, userNtfyConfig, adminNtfyConfig }).testNtfy(user, undefined, undefined, MASKED);
await makeController({ testNtfy, userNtfyConfig, adminNtfyConfig }).testNtfy(user, { token: MASKED });
expect(testNtfy).toHaveBeenCalledWith({ topic: 'saved-topic', server: 'https://ntfy.me', token: 'saved-token' });
});
});
@@ -159,22 +160,24 @@ describe('NotificationsController (parity with the legacy /api/notifications rou
});
describe('respond', () => {
it('400 on an invalid response value', async () => {
expect(await thrown(() => makeController({}).respond(user, '5', 'maybe'))).toEqual({
status: 400, body: { error: 'response must be "positive" or "negative"' },
});
it('rejects an invalid response value at the contract (ZodValidationPipe owns the 400 now)', () => {
// The inline enum check died with the DTO ratchet: over HTTP the global
// pipe rejects the body before the handler runs, with the standard
// { error: 'field: message; …' } envelope.
expect(NotificationRespondDto.schema.safeParse({ response: 'maybe' }).success).toBe(false);
expect(NotificationRespondDto.schema.safeParse({ response: 'positive' }).success).toBe(true);
});
it('400 with the service error when the response fails', async () => {
const respond = vi.fn().mockResolvedValue({ success: false, error: 'Already responded' });
expect(await thrown(() => makeController({ respond }).respond(user, '5', 'positive'))).toEqual({
expect(await thrown(() => makeController({ respond }).respond(user, '5', { response: 'positive' }))).toEqual({
status: 400, body: { error: 'Already responded' },
});
});
it('returns success + the updated notification', async () => {
const respond = vi.fn().mockResolvedValue({ success: true, notification: { id: 5, response: 'positive' } });
expect(await makeController({ respond }).respond(user, '5', 'positive')).toEqual({
expect(await makeController({ respond }).respond(user, '5', { response: 'positive' })).toEqual({
success: true, notification: { id: 5, response: 'positive' },
});
expect(respond).toHaveBeenCalledWith(5, 4, 'positive');
@@ -3,6 +3,7 @@ import {
notificationRespondRequestSchema,
channelTestResultSchema,
inAppListResultSchema,
testNtfyRequestSchema,
} from './notification.schema';
import { describe, it, expect } from 'vitest';
@@ -29,6 +30,14 @@ describe('notificationRespondRequestSchema', () => {
});
});
describe('testNtfyRequestSchema', () => {
it('accepts null server/token — the client sends null to mean "use the saved value"', () => {
expect(testNtfyRequestSchema.safeParse({ topic: 't', server: null, token: null }).success).toBe(true);
expect(testNtfyRequestSchema.safeParse({}).success).toBe(true);
expect(testNtfyRequestSchema.safeParse({ topic: 1 }).success).toBe(false);
});
});
describe('channelTestResultSchema', () => {
it('accepts a success result and an error result', () => {
expect(channelTestResultSchema.safeParse({ success: true }).success).toBe(true);
@@ -21,10 +21,13 @@ export const testSmtpRequestSchema = z.object({ email: z.string().optional() });
export const testWebhookRequestSchema = z.object({
url: z.string().optional(),
});
// server/token are nullable: the client deliberately sends null to mean
// "fall back to the saved value" (a stored token is only masked in the
// placeholder — sending null keeps the saved one).
export const testNtfyRequestSchema = z.object({
topic: z.string().optional(),
server: z.string().optional(),
token: z.string().optional(),
server: z.string().nullable().optional(),
token: z.string().nullable().optional(),
});
/** Result of a channel test ping. */