mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-08 04:32:42 +00:00
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.
This commit is contained in:
1 parent
f8c17ee9bb
commit
896ba233da
6 files changed
+75
-30
No files matched your search
+21
-15
@@ -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
|
||||
|
||||
@@ -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(
|
||||
|
||||
@@ -164,6 +164,23 @@ const mMeasurement = () =>
|
||||
updateMany: ReturnType<typeof vi.fn>;
|
||||
};
|
||||
|
||||
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"));
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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<ZodOpenApiObject["paths"]> = {
|
||||
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: {
|
||||
|
||||
@@ -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<string, unknown>,
|
||||
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"],
|
||||
|
||||
Reference in new issue
Block a user