fix(03): IN-03 re-validate outbox payload before VEVENT build, hard-fail invalid rows
This commit is contained in:
@@ -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<typeof outboxPayloadSchema>
|
||||
|
||||
// ── Drain concurrency guard (CR-05) ──────────────────────────────────────────
|
||||
|
||||
/**
|
||||
@@ -190,12 +220,19 @@ async function dispatchRow(row: OutboxRow): Promise<DispatchResult> {
|
||||
}
|
||||
}
|
||||
// CR-02: parse the stored form JSON and build a real VCALENDAR string
|
||||
let fields: Record<string, unknown>
|
||||
let rawFields: Record<string, unknown>
|
||||
try {
|
||||
fields = JSON.parse(row.payload) as Record<string, unknown>
|
||||
rawFields = JSON.parse(row.payload) as Record<string, unknown>
|
||||
} 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<DispatchResult> {
|
||||
}
|
||||
}
|
||||
// CR-02: parse the stored form JSON and build a real VCALENDAR string
|
||||
let fields: Record<string, unknown>
|
||||
let rawFields: Record<string, unknown>
|
||||
try {
|
||||
fields = JSON.parse(row.payload) as Record<string, unknown>
|
||||
rawFields = JSON.parse(row.payload) as Record<string, unknown>
|
||||
} 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
|
||||
|
||||
@@ -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.
|
||||
|
||||
Reference in New Issue
Block a user