From b96836fad1408805e02e84720b0183274bdb642e Mon Sep 17 00:00:00 2001 From: Brad Burch Date: Tue, 14 Jul 2026 14:24:19 -0700 Subject: [PATCH 1/3] Fix CSV parser dropping newlines inside quoted fields MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit parseCsvRows split the input into rows before handling quotes, so a newline embedded in a quoted field (e.g. a multi-line Notes value pasted from a spreadsheet) was treated as a row break — shredding one logical row into several and silently corrupting client/pet imports. Rewrite as a single-pass state machine that only breaks rows on newlines outside quotes (CRLF/CR/LF all handled). All prior behavior preserved; adds LF and CRLF embedded-newline tests. Co-Authored-By: Claude Opus 4.8 (1M context) --- server/__tests__/csv-parser.test.ts | 16 +++++++ server/lib/csv.ts | 70 +++++++++++++++++------------ 2 files changed, 58 insertions(+), 28 deletions(-) diff --git a/server/__tests__/csv-parser.test.ts b/server/__tests__/csv-parser.test.ts index d754eb4..aca200b 100644 --- a/server/__tests__/csv-parser.test.ts +++ b/server/__tests__/csv-parser.test.ts @@ -39,4 +39,20 @@ describe('parseCsvRows', () => { it('returns an empty array for an empty string', () => { expect(parseCsvRows('')).toEqual([]); }); + + it('keeps a newline inside a quoted field as one cell in one row', () => { + const text = 'Name,Notes\nJane,"line one\nline two"'; + expect(parseCsvRows(text)).toEqual([ + ['Name', 'Notes'], + ['Jane', 'line one\nline two'], + ]); + }); + + it('keeps a CRLF inside a quoted field as one cell in one row', () => { + const text = 'Name,Notes\r\nJane,"line one\r\nline two",done'; + expect(parseCsvRows(text)).toEqual([ + ['Name', 'Notes'], + ['Jane', 'line one\r\nline two', 'done'], + ]); + }); }); diff --git a/server/lib/csv.ts b/server/lib/csv.ts index 37ebb4f..9a7d738 100644 --- a/server/lib/csv.ts +++ b/server/lib/csv.ts @@ -1,42 +1,56 @@ /** - * Minimal RFC4180-ish CSV row parser: splits on commas outside quotes, un-escapes `""` to `"` - * inside a quoted field. Not a general CSV library — this codebase's only CSV use is the fixed - * 4-column client/pet import format (see - * docs/superpowers/specs/2026-07-10-csv-client-import-design.md), so a small hand-rolled parser - * avoids a dependency for a narrow, well-defined need. + * Minimal RFC4180-ish CSV parser tailored to this codebase's only CSV use: the fixed + * client/pet import format (see docs/superpowers/specs/2026-07-10-csv-client-import-design.md), + * whose free-text `Notes` column may contain newlines pasted from Excel/Google Sheets. + * + * It runs a single-pass state machine over the whole input string (not line-by-line), so a + * quoted field spanning multiple physical lines stays in one cell / one row: + * - `"` outside a quoted field opens one; inside a quoted field `""` is a literal `"` and a + * lone `"` closes the field. + * - `,` outside quotes ends a cell. + * - `\r\n`, lone `\r`, and lone `\n` outside quotes each end a row (a CRLF is one row break). + * - Any newline inside a quoted field is kept as a literal character in the cell. + * + * A hand-rolled parser avoids a dependency for this narrow, well-defined need. */ export function parseCsvRows(text: string): string[][] { if (text === '') return []; const rows: string[][] = []; - const lines = text.split(/\r\n|\r|\n/); - for (const line of lines) { - const cells: string[] = []; - let cell = ''; - let inQuotes = false; - for (let i = 0; i < line.length; i++) { - const ch = line[i]; - if (inQuotes) { - if (ch === '"') { - if (line[i + 1] === '"') { - cell += '"'; - i++; - } else { - inQuotes = false; - } + let cells: string[] = []; + let cell = ''; + let inQuotes = false; + + for (let i = 0; i < text.length; i++) { + const ch = text[i]; + if (inQuotes) { + if (ch === '"') { + if (text[i + 1] === '"') { + cell += '"'; + i++; } else { - cell += ch; + inQuotes = false; } - } else if (ch === '"') { - inQuotes = true; - } else if (ch === ',') { - cells.push(cell); - cell = ''; } else { cell += ch; } + } else if (ch === '"') { + inQuotes = true; + } else if (ch === ',') { + cells.push(cell); + cell = ''; + } else if (ch === '\r' || ch === '\n') { + // Row terminator: \r\n counts as a single break. + if (ch === '\r' && text[i + 1] === '\n') i++; + cells.push(cell); + cell = ''; + rows.push(cells); + cells = []; + } else { + cell += ch; } - cells.push(cell); - rows.push(cells); } + + cells.push(cell); + rows.push(cells); return rows; } From 884bc8ee71649dbe1d48b2292efff6f90daef72c Mon Sep 17 00:00:00 2001 From: Brad Burch Date: Tue, 14 Jul 2026 14:24:19 -0700 Subject: [PATCH 2/3] Validate auth route bodies with valibot Replace the untyped json().catch() + hand-rolled typeof guards in the /identify and /verify handlers with inline valibot schemas and a single safeParse. Email validation reuses the existing EMAIL_RE via v.regex, so every error response is byte-for-byte unchanged. Serves as the reference pattern for migrating the remaining route validation sites. Co-Authored-By: Claude Opus 4.8 (1M context) --- package-lock.json | 19 +++++++++++++++++-- package.json | 3 ++- server/routes/auth.ts | 38 ++++++++++++++++++++++++++++---------- 3 files changed, 47 insertions(+), 13 deletions(-) diff --git a/package-lock.json b/package-lock.json index 172819a..da9de9a 100644 --- a/package-lock.json +++ b/package-lock.json @@ -8,7 +8,8 @@ "dependencies": { "hono": "^4.12.25", "react": "^19.2.7", - "react-dom": "^19.2.7" + "react-dom": "^19.2.7", + "valibot": "^1.4.2" }, "devDependencies": { "@eslint/js": "^9.39.4", @@ -6484,7 +6485,7 @@ "version": "6.0.3", "resolved": "https://registry.npmjs.org/typescript/-/typescript-6.0.3.tgz", "integrity": "sha512-y2TvuxSZPDyQakkFRPZHKFm+KKVqIisdg9/CZwm9ftvKXLP8NRWj38/ODjNbr43SsoXqNuAisEf1GdCxqWcdBw==", - "dev": true, + "devOptional": true, "license": "Apache-2.0", "bin": { "tsc": "bin/tsc", @@ -6605,6 +6606,20 @@ "punycode": "^2.1.0" } }, + "node_modules/valibot": { + "version": "1.4.2", + "resolved": "https://registry.npmjs.org/valibot/-/valibot-1.4.2.tgz", + "integrity": "sha512-gjdCvJ6d3RyHAneqxMYMW9QMCwYMb3jpOO0IyHZV1bnRHFBHrX3VkIILt5XYR0WhwHiH7Mty8ovuPZ/O3gamrg==", + "license": "MIT", + "peerDependencies": { + "typescript": ">=5" + }, + "peerDependenciesMeta": { + "typescript": { + "optional": true + } + } + }, "node_modules/vite": { "version": "8.1.3", "resolved": "https://registry.npmjs.org/vite/-/vite-8.1.3.tgz", diff --git a/package.json b/package.json index d1fbb34..a0ec7a8 100644 --- a/package.json +++ b/package.json @@ -6,7 +6,8 @@ "dependencies": { "hono": "^4.12.25", "react": "^19.2.7", - "react-dom": "^19.2.7" + "react-dom": "^19.2.7", + "valibot": "^1.4.2" }, "devDependencies": { "@eslint/js": "^9.39.4", diff --git a/server/routes/auth.ts b/server/routes/auth.ts index 789f5cc..6d9d94b 100644 --- a/server/routes/auth.ts +++ b/server/routes/auth.ts @@ -1,4 +1,5 @@ import { Hono } from 'hono'; +import * as v from 'valibot'; import { consumeLoginCode, createLoginCode, @@ -12,6 +13,23 @@ import type { AppEnv } from '../types'; const CODE_TTL_MS = 10 * 60 * 1000; +// --- Reference valibot pattern --- +// This file is the reference for validating request bodies with valibot: declare a schema, then +// `safeParse` once to both validate and narrow types (replacing hand-rolled `typeof` guards + casts). +// Other routes should follow this shape. Keep schemas small and inline — no shared factory. +// +// The email schema reuses the repo's EMAIL_RE via a regex pipe (not valibot's own email heuristic) +// so validation stays byte-for-byte identical to the previous hand-check. It also trims + lowercases +// before the regex, matching the old `body.email.trim().toLowerCase()` normalization. +const IdentifyBody = v.object({ + email: v.pipe(v.string(), v.trim(), v.toLowerCase(), v.regex(EMAIL_RE)), +}); +// codeId is intentionally NOT trimmed (matches prior behavior); code is trimmed before consuming. +const VerifyBody = v.object({ + codeId: v.string(), + code: v.pipe(v.string(), v.trim()), +}); + function generateCode(): string { const n = crypto.getRandomValues(new Uint32Array(1))[0] % 1_000_000; return n.toString().padStart(6, '0'); @@ -20,9 +38,10 @@ function generateCode(): string { export const authRoutes = new Hono() .post('/:slug/identify', async (c) => { const tenant = c.get('tenant'); - const body = await c.req.json<{ email?: unknown }>().catch(() => ({}) as { email?: unknown }); - const email = typeof body.email === 'string' ? body.email.trim().toLowerCase() : ''; - if (!EMAIL_RE.test(email)) return c.json({ error: 'Enter a valid email.' }, 400); + const raw = await c.req.json().catch(() => ({})); + const parsed = v.safeParse(IdentifyBody, raw); + if (!parsed.success) return c.json({ error: 'Enter a valid email.' }, 400); + const { email } = parsed.output; // Invite-only: only customers the provider has added may receive a code. Do NOT auto-create. const user = await getEndUserByEmail(c.env.PAWBOOK_DB, tenant.Id, email); @@ -52,17 +71,16 @@ export const authRoutes = new Hono() .post('/:slug/verify', async (c) => { const tenant = c.get('tenant'); - const body = await c.req - .json<{ codeId?: unknown; code?: unknown }>() - .catch(() => ({}) as { codeId?: unknown; code?: unknown }); - if (typeof body.codeId !== 'string' || typeof body.code !== 'string') - return c.json({ error: 'Code required.' }, 400); + const raw = await c.req.json().catch(() => ({})); + const parsed = v.safeParse(VerifyBody, raw); + if (!parsed.success) return c.json({ error: 'Code required.' }, 400); + const { codeId, code } = parsed.output; const endUserId = await consumeLoginCode( c.env.PAWBOOK_DB, tenant.Id, - body.codeId, - body.code.trim(), + codeId, + code, new Date().toISOString(), ); if (!endUserId) return c.json({ error: 'That code is wrong or expired — try again.' }, 401); From 62f1eb19b5df24847975649e080a5545bafa5047 Mon Sep 17 00:00:00 2001 From: Brad Burch Date: Tue, 14 Jul 2026 14:32:19 -0700 Subject: [PATCH 3/3] Correct CSV docstring: import has no Notes column MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The import format is Client Email, Client Name, Pet Name, Pet Type — the newline-in-quoted-field fix is generically correct, but the rationale wrongly cited a Notes column. Point it at an actual quoted field instead. Co-Authored-By: Claude Opus 4.8 (1M context) --- server/lib/csv.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/lib/csv.ts b/server/lib/csv.ts index 9a7d738..5478ccb 100644 --- a/server/lib/csv.ts +++ b/server/lib/csv.ts @@ -1,7 +1,7 @@ /** * Minimal RFC4180-ish CSV parser tailored to this codebase's only CSV use: the fixed * client/pet import format (see docs/superpowers/specs/2026-07-10-csv-client-import-design.md), - * whose free-text `Notes` column may contain newlines pasted from Excel/Google Sheets. + * whose free-text fields (e.g. a quoted Client Name) may contain newlines pasted from a spreadsheet. * * It runs a single-pass state machine over the whole input string (not line-by-line), so a * quoted field spanning multiple physical lines stays in one cell / one row: