From 21149d7467f066cc34017fe8dfcd9257e3d8a692 Mon Sep 17 00:00:00 2001 From: "Aaron K. Clark" Date: Tue, 19 May 2026 01:49:56 -0500 Subject: [PATCH] feat(invoice): reject inverted invDate/invDueDate at the schema layer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `createInvoiceBody` and `updateInvoiceBody` accepted any combination of `invDate` and `invDueDate` strings. There was no check that the due date was on or after the issue date, so an operator could persist an invoice that was "due" before it was issued — bookkeeping nonsense the controller had no recourse to reject. Same shape as the timeentry `teEndedAt >= teStartedAt` refinement from #130: add a zod `.refine()` that fires on both the single- and bulk-create paths, and on PATCH when both bounds appear in the same body. Equality stays allowed — `Due on Receipt` is a real billing term. String comparison is safe here because `isoDate` is the strict `^\d{4}-\d{2}-\d{2}$` regex above; lexicographic order on that shape matches chronological order for every valid input. No need to parse to Date objects (timeentry uses `new Date()` because `isoDatetime` has timezone offsets in play; isoDate does not). The bulk-create path inherits the refinement automatically because `bulkInvoiceBody` wraps `createInvoiceBody` in `z.array(...)` and zod runs each element's refinements during array validation — so an attacker can't bypass the check by wrapping the bad entry in a bulk envelope. Single-bound PATCH (only invDueDate or only invDate) is intentionally NOT rejected — the schema doesn't see the existing row's other half. That's a controller-layer follow-up. Five new tests cover: inverted single CREATE → 400, equality CREATE → schema-pass, inverted both-bound PATCH → 400, single- bound PATCH → schema-pass, inverted bulk entry → 400 (with the `invoices.0.invDueDate` issue path). Co-Authored-By: Claude Opus 4.7 (1M context) --- app/schemas/invoice.schema.js | 23 +++++++++++-- tests/api/invoice.test.js | 63 +++++++++++++++++++++++++++++++++++ 2 files changed, 84 insertions(+), 2 deletions(-) diff --git a/app/schemas/invoice.schema.js b/app/schemas/invoice.schema.js index 1e05d13..450d643 100644 --- a/app/schemas/invoice.schema.js +++ b/app/schemas/invoice.schema.js @@ -12,6 +12,25 @@ const isoDate = z.string().regex(/^\d{4}-\d{2}-\d{2}$/, { message: 'Must be an ISO 8601 date (YYYY-MM-DD).', }); +// Cross-field refinement: an invoice's due date must be on or +// after its issue date. Mirrors the teEndedAt >= teStartedAt +// constraint from #130 — a due-before-issue invoice is operator +// error worth surfacing at the boundary instead of papering over. +// Equality (due same day as issue) is allowed: zero-day-net is a +// real billing term ("Due on Receipt"). +// +// String comparison is safe here because `isoDate` is the strict +// `YYYY-MM-DD` regex above; lexicographic order on that shape +// matches chronological order for any valid input. +function refineDueDateAfterIssue(data) { + if (!data.invDate || !data.invDueDate) return true; + return data.invDueDate >= data.invDate; +} +const DUE_BEFORE_ISSUE = { + message: 'invDueDate must be on or after invDate.', + path: ['invDueDate'], +}; + const createInvoiceBody = z.object({ invCustId: z.coerce.number().int().positive(), invDate: isoDate, @@ -19,7 +38,7 @@ const createInvoiceBody = z.object({ invPaid: z.boolean().optional(), }).strict({ message: 'Unexpected field in body. Whitelist: invCustId, invDate, invDueDate, invPaid.', -}); +}).refine(refineDueDateAfterIssue, DUE_BEFORE_ISSUE); const updateInvoiceBody = z.object({ invDate: isoDate.optional(), @@ -27,7 +46,7 @@ const updateInvoiceBody = z.object({ invPaid: z.boolean().optional(), }).strict({ message: 'Unexpected field in body. Whitelist: invDate, invDueDate, invPaid.', -}); +}).refine(refineDueDateAfterIssue, DUE_BEFORE_ISSUE); const listByCustomerQuery = z.object({ limit: z.coerce.number().int().positive().max(500).optional(), diff --git a/tests/api/invoice.test.js b/tests/api/invoice.test.js index d682dc5..6043c00 100644 --- a/tests/api/invoice.test.js +++ b/tests/api/invoice.test.js @@ -56,4 +56,67 @@ describe('Invoice body validation', () => { const res = await request(app).post('/v1/invoice').set('authKey', 'any').send({ invCustId: 1, invDate: 'tomorrow', invDueDate: '2026-02-01' }); expect(res.status).toBe(400); }); + + test('POST rejects invDueDate strictly before invDate', async () => { + // Inverted range — due date *before* issue date is nonsense. Pin + // the refinement so a future schema refactor can't accidentally + // drop the check. + const res = await request(app).post('/v1/invoice').set('authKey', 'any').send({ + invCustId: 1, + invDate: '2026-05-15', + invDueDate: '2026-05-01', + }); + expect(res.status).toBe(400); + const issue = res.body.issues && res.body.issues.find((i) => i.path === 'invDueDate'); + expect(issue).toBeDefined(); + expect(issue.message).toMatch(/on or after invDate/i); + }); + + test('POST accepts invDueDate equal to invDate (zero-day-net / "Due on Receipt")', async () => { + // Equality is a legitimate billing term, not a bug. Schema must + // not 400 — auth/controller decides the final status from there. + const res = await request(app).post('/v1/invoice').set('authKey', 'any').send({ + invCustId: 1, + invDate: '2026-05-15', + invDueDate: '2026-05-15', + }); + expect(res.status).not.toBe(400); + }); + + test('PATCH rejects inverted range when both bounds are sent', async () => { + const res = await request(app).patch('/v1/invoice/1').set('authKey', 'any').send({ + invDate: '2026-05-15', + invDueDate: '2026-05-01', + }); + expect(res.status).toBe(400); + const issue = res.body.issues && res.body.issues.find((i) => i.path === 'invDueDate'); + expect(issue).toBeDefined(); + }); + + test('PATCH with only invDueDate is not blocked by the schema', async () => { + // The cross-field refinement can't validate a single-bound PATCH + // without seeing the existing row; the schema must not reject + // it. Controller-layer enforcement against the existing invDate + // is a separate follow-up. + const res = await request(app).patch('/v1/invoice/1').set('authKey', 'any').send({ + invDueDate: '2026-05-01', + }); + expect(res.status).not.toBe(400); + }); + + test('bulk POST rejects an inverted-range entry inside the batch', async () => { + // The bulk path validates each element through createInvoiceBody, + // so the refinement must fire there too — anything else would + // let an attacker bypass the check by wrapping the bad entry in + // a bulk envelope. + const res = await request(app).post('/v1/invoice/bulk').set('authKey', 'any').send({ + invoices: [ + { invCustId: 1, invDate: '2026-05-15', invDueDate: '2026-05-01' }, + ], + }); + expect(res.status).toBe(400); + // Path on a bulk entry's issue: `invoices.0.invDueDate`. + const issue = res.body.issues && res.body.issues.find((i) => i.path.endsWith('invDueDate')); + expect(issue).toBeDefined(); + }); });