From b8c186491bca912e0007284f4366ea922ed6f386 Mon Sep 17 00:00:00 2001 From: Lucas Berger Date: Tue, 9 Jun 2026 11:05:50 -0400 Subject: [PATCH] fix(03): IN-03 re-validate outbox payload before VEVENT build, hard-fail invalid rows --- apps/api/src/broker/outboxWorker.ts | 51 ++++++++++++++++++++-- apps/api/tests/broker/outboxWorker.test.ts | 23 ++++++++++ 2 files changed, 70 insertions(+), 4 deletions(-) diff --git a/apps/api/src/broker/outboxWorker.ts b/apps/api/src/broker/outboxWorker.ts index 1d9a179..236e9cb 100644 --- a/apps/api/src/broker/outboxWorker.ts +++ b/apps/api/src/broker/outboxWorker.ts @@ -24,6 +24,7 @@ */ import { schedule } from 'node-cron' +import { z } from 'zod' import { and, eq, lte } from 'drizzle-orm' import { db } from '../db/client.js' import { calendarEvents, calendarOutbox, calendars, memberCredentials } from '../db/schema.js' @@ -53,6 +54,35 @@ const HARD_FAIL_STATUSES = new Set([400, 401, 403]) /** HTTP status code for CalDAV If-Match conflict — D-08 conflict flow. */ const CONFLICT_STATUS = 412 +// ── Outbox payload re-validation (IN-03) ───────────────────────────────────── + +/** + * IN-03: re-validate the JSON payload read back out of calendar_outbox before building + * a VEVENT from it. The payload was zod-validated at enqueue (routes/events.ts + * eventFieldsSchema), but a manually-inserted row or enqueue→drain schema drift could + * feed undefined/wrong-typed fields into buildVeventString, producing SUMMARY:undefined + * or an Invalid Date. Such a row can NEVER succeed, so on validation failure the caller + * hard-fails the row (no retry) instead of burning the backoff budget. + * + * Mirrors eventFieldsSchema in routes/events.ts. `_preservedRrule` (added by the edit-as- + * move route, CR-01) is allowed via .passthrough() so the move payload still validates. + */ +const outboxPayloadSchema = z + .object({ + title: z.string().min(1).max(255), + allDay: z.boolean(), + start: z.string().min(1).max(64), + end: z.string().min(1).max(64), + location: z.string().max(2000).optional(), + description: z.string().max(2000).optional(), + recurrence: z.enum(['none', 'daily', 'weekly', 'monthly', 'yearly']).optional(), + calendarUrl: z.string().url().max(1024).optional(), + _preservedRrule: z.string().max(1024).optional(), + }) + .passthrough() + +type OutboxPayloadFields = z.infer + // ── Drain concurrency guard (CR-05) ────────────────────────────────────────── /** @@ -190,12 +220,19 @@ async function dispatchRow(row: OutboxRow): Promise { } } // CR-02: parse the stored form JSON and build a real VCALENDAR string - let fields: Record + let rawFields: Record try { - fields = JSON.parse(row.payload) as Record + rawFields = JSON.parse(row.payload) as Record } catch { return { success: false, conflict: false, hardFail: true, transient: false, error: 'payload parse failed' } } + // IN-03: re-validate the parsed payload. A schema-invalid row can never succeed — + // hard-fail it (no retry) rather than feeding undefined/Invalid Date into the VEVENT. + const parsedFields = outboxPayloadSchema.safeParse(rawFields) + if (!parsedFields.success) { + return { success: false, conflict: false, hardFail: true, transient: false, error: `payload validation failed: ${parsedFields.error.message}` } + } + const fields: OutboxPayloadFields = parsedFields.data // WR-01: recurrence preservation. The PWA omits `recurrence` from an edit payload // (it cannot read the existing RRULE — not in the occurrence contract, D-03), so on // update we must NOT rebuild the VEVENT with no RRULE — that would silently convert a @@ -276,12 +313,18 @@ async function dispatchRow(row: OutboxRow): Promise { } } // CR-02: parse the stored form JSON and build a real VCALENDAR string - let fields: Record + let rawFields: Record try { - fields = JSON.parse(row.payload) as Record + rawFields = JSON.parse(row.payload) as Record } catch { return { success: false, conflict: false, hardFail: true, transient: false, error: 'payload parse failed' } } + // IN-03: re-validate the parsed payload — hard-fail a schema-invalid create row. + const parsedFields = outboxPayloadSchema.safeParse(rawFields) + if (!parsedFields.success) { + return { success: false, conflict: false, hardFail: true, transient: false, error: `payload validation failed: ${parsedFields.error.message}` } + } + const fields: OutboxPayloadFields = parsedFields.data // CR-01: edit-as-move RRULE preservation. The same-calendar `update` branch // preserves a recurring series' RRULE by reading rawVevent; the `create` branch // (used for the create half of an edit-as-move, D-04) has no source for the diff --git a/apps/api/tests/broker/outboxWorker.test.ts b/apps/api/tests/broker/outboxWorker.test.ts index d47d000..6d39ef5 100644 --- a/apps/api/tests/broker/outboxWorker.test.ts +++ b/apps/api/tests/broker/outboxWorker.test.ts @@ -282,6 +282,29 @@ describe('runOutboxDrain — ICS building from form JSON (CR-02)', () => { expect(setArg?.status).toBe('failed') }) + // IN-03 (iteration 2): a JSON-parseable but schema-INVALID payload (e.g. missing the + // required title) can never produce a valid VEVENT, so the row is hard-failed (no + // retry) rather than dispatched with SUMMARY:undefined. + it('IN-03: create row with schema-invalid payload (missing title) is hard-failed, never dispatched', async () => { + const { createCalendarEvent } = await import('../../src/broker/write.js') + vi.mocked(createCalendarEvent).mockResolvedValue(makeResponse(201)) + // Valid JSON, but title is missing → fails outboxPayloadSchema + const badPayload = JSON.stringify({ + allDay: false, + start: '2026-06-10T12:00:00', + end: '2026-06-10T13:00:00', + }) + mockPendingRows = [makeRow({ payload: badPayload })] + + await runOutboxDrain() + + // Must NOT have dispatched a CalDAV write with an invalid VEVENT + expect(createCalendarEvent).not.toHaveBeenCalled() + const setArg = mockUpdateSet.mock.calls[0]?.[0] as { status?: string; lastError?: string } + expect(setArg?.status).toBe('failed') + expect(setArg?.lastError).toMatch(/validation/i) + }) + // CR-01 (iteration 2): the edit-as-move create branch must re-apply the RRULE the // route stashed on the payload as `_preservedRrule`, so a moved recurring series keeps // its RRULE instead of collapsing into a single occurrence.