From 896ba233da4fc7c13ae22cfce955a2be4a46bbb2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Wed, 21 Oct 2026 23:56:16 +0200 Subject: [PATCH] fix(import): refuse unknown query parameters on the CSV imports The lab, medication and measurement CSV routes ignored parameters they did not know, so a misspelt dryrun=1 fell back to a real import and wrote the file the caller meant only to check. Their query schemas are strict now: an unknown parameter answers 422 before anything is read or a rate bucket is spent. --- docs/api/openapi.yaml | 36 +++++++++++-------- .../__tests__/record-csv-routes.test.ts | 10 ++++++ .../api/import/csv/__tests__/route.test.ts | 17 +++++++++ src/app/api/import/csv/route.ts | 12 +++++-- src/lib/openapi/routes/import.ts | 16 +++------ src/lib/validations/record-import.ts | 14 ++++++++ 6 files changed, 75 insertions(+), 30 deletions(-) diff --git a/docs/api/openapi.yaml b/docs/api/openapi.yaml index 9effbc1d3..ecc92d47d 100644 --- a/docs/api/openapi.yaml +++ b/docs/api/openapi.yaml @@ -15308,17 +15308,21 @@ paths: reason `duplicate` and not written. Without it a row is deduplicated on `(type, measuredAt)` and a repeat is `skipped` / `duplicate`. Writes commit in chunks rather than as one transaction; a failure part-way leaves earlier chunks written, and re-uploading the same file completes the rest. `?dryRun=1` validates + previews - without writing. Shares the 5/hour `import:` rate bucket." + without writing. Shares the 5/hour `import:` rate bucket. An unknown query parameter is refused with 422, so a + misspelt `dryRun` never writes." parameters: - - name: dryRun - in: query - required: false + - in: query + name: dryRun schema: + description: "`1` validates every row and reports what would happen, writing nothing." + default: "0" type: string enum: + - "0" - "1" - "true" - description: When `1` / `true`, parse + validate + return the per-row envelope without writing. + - "false" + description: "`1` validates every row and reports what would happen, writing nothing." requestBody: required: true content: @@ -15357,11 +15361,12 @@ paths: refused). Re-uploading the same file writes nothing: every row is reported as a duplicate. `dryRun=1` checks the whole file without writing and returns the first rows as they would be stored; it spends a separate bucket of 30 checks an hour, the import itself the shared 5-an-hour `import:` bucket. Owner only; refused with - `module.disabled` when the module is switched off. Each row's name resolves to the account's marker the way a - scanned report's rows do (name, alias, catalog name); a new analyte mints one marker from its first row. A - numeric result needs a unit; one in another unit is converted when the conversion is known and keeps the printed - value beside it, otherwise the row is refused with `unit_mismatch`. Any other text in the value column is a - qualitative result. A date without a time is stored as that calendar date. At most 20 000 data rows." + `module.disabled` when the module is switched off. An unknown query parameter is refused with 422, so a misspelt + `dryRun` never writes. Each row's name resolves to the account's marker the way a scanned report's rows do + (name, alias, catalog name); a new analyte mints one marker from its first row. A numeric result needs a unit; + one in another unit is converted when the conversion is known and keeps the printed value beside it, otherwise + the row is refused with `unit_mismatch`. Any other text in the value column is a qualitative result. A date + without a time is stored as that calendar date. At most 20 000 data rows." parameters: - in: query name: dryRun @@ -15562,11 +15567,12 @@ paths: refused). Re-uploading the same file writes nothing: every row is reported as a duplicate. `dryRun=1` checks the whole file without writing and returns the first rows as they would be stored; it spends a separate bucket of 30 checks an hour, the import itself the shared 5-an-hour `import:` bucket. Owner only; refused with - `module.disabled` when the module is switched off. Maps name, dose (as written), daily intake times, an - as-needed mark and a start and end date; every other rhythm is set up afterwards in the app. A row needs times - unless it is marked as needed. Each row is checked against the create route's own schema; a dated window opens a - course. Reminders stay off unless `reminders=1`. A medication the record holds under the same name and dose is a - duplicate. At most 500 data rows." + `module.disabled` when the module is switched off. An unknown query parameter is refused with 422, so a misspelt + `dryRun` never writes. Maps name, dose (as written), daily intake times, an as-needed mark and a start and end + date; every other rhythm is set up afterwards in the app. A row needs times unless it is marked as needed. Each + row is checked against the create route's own schema; a dated window opens a course. Reminders stay off unless + `reminders=1`. A medication the record holds under the same name and dose is a duplicate. At most 500 data + rows." parameters: - in: query name: dryRun diff --git a/src/app/api/import/__tests__/record-csv-routes.test.ts b/src/app/api/import/__tests__/record-csv-routes.test.ts index 2c3ec0294..0c1ae9b70 100644 --- a/src/app/api/import/__tests__/record-csv-routes.test.ts +++ b/src/app/api/import/__tests__/record-csv-routes.test.ts @@ -100,6 +100,16 @@ describe("record CSV import routes", () => { expect(mocks.importLabsCsv).not.toHaveBeenCalled(); }); + it("refuses an unknown parameter, so a misspelt dry run never writes", async () => { + for (const typo of ["dryrun=1", "dry_run=1", "dryRun=1&preview=1"]) { + expect((await post(labsPost, `${LABS}&${typo}`)).status).toBe(422); + expect((await post(medsPost, `${MEDS}&${typo}`)).status).toBe(422); + } + expect(mocks.importLabsCsv).not.toHaveBeenCalled(); + expect(mocks.importMedicationsCsv).not.toHaveBeenCalled(); + expect(mocks.checkRateLimit).not.toHaveBeenCalled(); + }); + it("spends the preview bucket on a dry run and the import bucket otherwise", async () => { await post(labsPost, `${LABS}&dryRun=1`); expect(mocks.checkRateLimit).toHaveBeenLastCalledWith( diff --git a/src/app/api/import/csv/__tests__/route.test.ts b/src/app/api/import/csv/__tests__/route.test.ts index a6d8dcaa9..8ce841f24 100644 --- a/src/app/api/import/csv/__tests__/route.test.ts +++ b/src/app/api/import/csv/__tests__/route.test.ts @@ -164,6 +164,23 @@ const mMeasurement = () => updateMany: ReturnType; }; +describe("POST /api/import/csv — strict query", () => { + it("refuses a misspelt dry-run flag instead of importing", async () => { + for (const query of ["dryrun=1", "dry_run=1", "dryRun=yes"]) { + const res = await POST( + new NextRequest(`http://localhost/api/import/csv?${query}`, { + method: "POST", + body: [HEADER, "WEIGHT,80,kg,2026-05-01T08:00:00Z,,,"].join("\n"), + headers: { "content-type": "text/csv" }, + }), + ); + expect(res.status).toBe(422); + } + expect(mMeasurement().createManyAndReturn).not.toHaveBeenCalled(); + expect(checkRateLimit).not.toHaveBeenCalled(); + }); +}); + describe("POST /api/import/csv — fatal header error", () => { it("returns 422 when a required column is missing", async () => { const res = await POST(csvRequest("type,value,unit\nWEIGHT,80,kg")); diff --git a/src/app/api/import/csv/route.ts b/src/app/api/import/csv/route.ts index d39ccc3e4..97d1e4239 100644 --- a/src/app/api/import/csv/route.ts +++ b/src/app/api/import/csv/route.ts @@ -7,7 +7,9 @@ import { apiError, getClientIp, readBodyText, + returnAllZodIssues, } from "@/lib/api-response"; +import { csvImportQuerySchema } from "@/lib/validations/record-import"; import { encryptNote, readNote } from "@/lib/crypto/note-cipher"; import { checkRateLimit } from "@/lib/rate-limit"; import { NextRequest } from "next/server"; @@ -109,9 +111,13 @@ export const POST = apiHandler(async (request: NextRequest) => { const { user } = await requireAuth(); annotate({ action: { name: "import.csv.upload" } }); - const dryRun = - new URL(request.url).searchParams.get("dryRun") === "1" || - new URL(request.url).searchParams.get("dryRun") === "true"; + // Strict: an unknown parameter is refused, so a misspelt `dryrun=1` can + // never fall through to a real import. + const queryParse = csvImportQuerySchema.safeParse( + Object.fromEntries(new URL(request.url).searchParams), + ); + if (!queryParse.success) return returnAllZodIssues(queryParse.error, 422); + const dryRun = queryParse.data.dryRun; // Same blast-radius reasoning as the JSON import — share the bucket so a // CSV upload counts against the same 5/hour quota. diff --git a/src/lib/openapi/routes/import.ts b/src/lib/openapi/routes/import.ts index 6fee3fa7f..a9fc17f68 100644 --- a/src/lib/openapi/routes/import.ts +++ b/src/lib/openapi/routes/import.ts @@ -24,13 +24,14 @@ import { } from "./shared"; import { isoDateTime } from "@/lib/validations/datetime"; import { + csvImportQuerySchema, labsCsvQuerySchema, medicationsCsvQuerySchema, recordImportResultSchema, } from "@/lib/validations/record-import"; const RECORD_IMPORT_COMMON = - "Body is the raw CSV file (`text/csv`, ≤ 16 MB), read as a stream: a UTF-8 byte-order mark is dropped, the separator is `,`, `;` or a tab, quoted fields may hold separators and line breaks. Every row is checked on its own and a refused row is reported by line with a reason, never guessed (a number or date that reads two ways is refused). Re-uploading the same file writes nothing: every row is reported as a duplicate. `dryRun=1` checks the whole file without writing and returns the first rows as they would be stored; it spends a separate bucket of 30 checks an hour, the import itself the shared 5-an-hour `import:` bucket. Owner only; refused with `module.disabled` when the module is switched off."; + "Body is the raw CSV file (`text/csv`, ≤ 16 MB), read as a stream: a UTF-8 byte-order mark is dropped, the separator is `,`, `;` or a tab, quoted fields may hold separators and line breaks. Every row is checked on its own and a refused row is reported by line with a reason, never guessed (a number or date that reads two ways is refused). Re-uploading the same file writes nothing: every row is reported as a duplicate. `dryRun=1` checks the whole file without writing and returns the first rows as they would be stored; it spends a separate bucket of 30 checks an hour, the import itself the shared 5-an-hour `import:` bucket. Owner only; refused with `module.disabled` when the module is switched off. An unknown query parameter is refused with 422, so a misspelt `dryRun` never writes."; function recordImportResponses(id: string) { return { @@ -339,17 +340,8 @@ export const importPaths: NonNullable = { tags: ["Import"], summary: "Import measurements from CSV", description: - "Bulk-import measurements from a CSV file (web cold-start escape hatch). Body is raw `text/csv` (≤ 16 MB, ≤ 10 000 valid rows). Header (order-independent): `type,value,unit,measuredAt[,glucoseContext,notes,externalId]`. `measuredAt` must carry an explicit ISO-8601 offset and is bounded (no future beyond a 5-min skew, no instant before 1900). Glucose accepts `mmol/L` (converted to canonical `mg/dL`); weight accepts `lb` (converted to `kg`). `glucoseContext` is optional on a `BLOOD_GLUCOSE` row — a blank cell stores no context, which is the normal shape of a continuous-sensor export; a non-empty value is still checked against the enum, and the column is refused on any other type. Re-uploading a file is idempotent. With an `externalId` column a row upserts on `(userId, type, source=IMPORT, externalId)`: a changed row is `updated`, an unchanged one is `skipped` with reason `duplicate` and not written. Without it a row is deduplicated on `(type, measuredAt)` and a repeat is `skipped` / `duplicate`. Writes commit in chunks rather than as one transaction; a failure part-way leaves earlier chunks written, and re-uploading the same file completes the rest. `?dryRun=1` validates + previews without writing. Shares the 5/hour `import:` rate bucket.", - parameters: [ - { - name: "dryRun", - in: "query", - required: false, - schema: { type: "string", enum: ["1", "true"] }, - description: - "When `1` / `true`, parse + validate + return the per-row envelope without writing.", - }, - ], + "Bulk-import measurements from a CSV file (web cold-start escape hatch). Body is raw `text/csv` (≤ 16 MB, ≤ 10 000 valid rows). Header (order-independent): `type,value,unit,measuredAt[,glucoseContext,notes,externalId]`. `measuredAt` must carry an explicit ISO-8601 offset and is bounded (no future beyond a 5-min skew, no instant before 1900). Glucose accepts `mmol/L` (converted to canonical `mg/dL`); weight accepts `lb` (converted to `kg`). `glucoseContext` is optional on a `BLOOD_GLUCOSE` row — a blank cell stores no context, which is the normal shape of a continuous-sensor export; a non-empty value is still checked against the enum, and the column is refused on any other type. Re-uploading a file is idempotent. With an `externalId` column a row upserts on `(userId, type, source=IMPORT, externalId)`: a changed row is `updated`, an unchanged one is `skipped` with reason `duplicate` and not written. Without it a row is deduplicated on `(type, measuredAt)` and a repeat is `skipped` / `duplicate`. Writes commit in chunks rather than as one transaction; a failure part-way leaves earlier chunks written, and re-uploading the same file completes the rest. `?dryRun=1` validates + previews without writing. Shares the 5/hour `import:` rate bucket. An unknown query parameter is refused with 422, so a misspelt `dryRun` never writes.", + requestParams: { query: csvImportQuerySchema }, requestBody: { required: true, content: { diff --git a/src/lib/validations/record-import.ts b/src/lib/validations/record-import.ts index c6c572d1f..b65a5e2fd 100644 --- a/src/lib/validations/record-import.ts +++ b/src/lib/validations/record-import.ts @@ -8,6 +8,10 @@ * order and the zone a time without an offset is read in. The client works * these out with the person on the mapping screen; the server never guesses * a column. + * + * Both query schemas are strict: a parameter they do not know is a 422, not + * ignored. A misspelt `dryrun=1` or `dry_run=1` would otherwise fall back to + * the default `dryRun=0` and write the file the caller only meant to check. */ import { z } from "zod/v4"; @@ -60,6 +64,14 @@ const formatFields = { ), }; +/** + * Query of `POST /api/import/csv` (the measurement CSV). Strict for the same + * reason as the two record imports: a misspelt flag must never write. + */ +export const csvImportQuerySchema = z + .object({ dryRun: formatFields.dryRun }) + .strict(); + function distinctColumns( value: Record, keys: readonly string[], @@ -103,6 +115,7 @@ export const labsCsvQuerySchema = z panel: column.optional(), note: column.optional().describe("Column of a note; stored encrypted."), }) + .strict() .refine((v) => distinctColumns(v, LAB_COLUMN_KEYS), { message: "Each column can feed one field", path: ["analyte"], @@ -144,6 +157,7 @@ export const medicationsCsvQuerySchema = z "`1` turns reminders on for imported medications that have times. Off by default, so an import never starts reminding on its own.", ), }) + .strict() .refine((v) => distinctColumns(v, MEDICATION_COLUMN_KEYS), { message: "Each column can feed one field", path: ["name"],