From f94d358ade024a8bbe3a97e703caacd248a30a8c Mon Sep 17 00:00:00 2001 From: Antiheld86 <4201585+Antiheld86@users.noreply.github.com> Date: Tue, 1 Sep 2026 08:16:53 +0000 Subject: [PATCH] feat(measurements): write EXTERNAL from both ingest write paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Under a `measurements:write` credential the row's source is now decided by the server on both routes, and the body may not name one at all. The 422 widens from "names APPLE_HEALTH" to "names any source". It was narrow only because the forced value was MANUAL, so a body saying MANUAL was never actually overridden. Now it would be — and overriding a client's explicit assertion quietly is the thing both routes' comments rule out, while honouring it would defeat the change: a bridge that kept sending MANUAL would stay indistinguishable from a typed reading. Status and errorCode are unchanged, and the documented Home Assistant payload sends no source at all. The single-POST predicate keeps reading the raw body: the create schema defaults `source` to MANUAL, so after parsing "said MANUAL" and "said nothing" are the same value. Both of that route's create sites resolve the source, the array arm included — that is the combined BP + Pulse path a bridge uses, and missing it would leave exactly those rows unbadged. Two consequences worth naming. Bridge rows no longer join the MANUAL/APPLE_HEALTH cross-source merge, so they can't be swallowed by a rule written for the iOS adopt-on-pair case. And a bridge posting at the same instant as a hand entry now lands both rows instead of a 409, which is honest and does not double-count: the rollup collapse is per (type, day, source) and MANUAL outranks the unlisted EXTERNAL. --- src/app/api/measurements/batch/route.ts | 52 ++++++++++------- src/app/api/measurements/route.ts | 76 ++++++++++++++++++------- 2 files changed, 89 insertions(+), 39 deletions(-) diff --git a/src/app/api/measurements/batch/route.ts b/src/app/api/measurements/batch/route.ts index 776e870b7..278e4959a 100644 --- a/src/app/api/measurements/batch/route.ts +++ b/src/app/api/measurements/batch/route.ts @@ -39,6 +39,7 @@ import { z } from "zod/v4"; import { prisma } from "@/lib/db"; import { apiHandler, isScopedCredential, requireAuth } from "@/lib/api-handler"; +import { EXTERNAL_SOURCE } from "@/lib/measurements/external-source"; import { MEASUREMENTS_WRITE_SCOPE } from "@/lib/measurements/scopes"; import { annotate } from "@/lib/logging/context"; import { auditLog } from "@/lib/auth/audit"; @@ -283,25 +284,34 @@ async function postBatch(request: NextRequest): Promise { ); } - // A scoped credential may attribute MANUAL and nothing else. Refused loudly - // rather than rewritten quietly: silently relabelling a client's explicit - // assertion hands it rows it did not ask for and no way to notice, while a - // 422 is fixable on the first call. + // A scoped credential may attribute nothing at all — the source of its rows + // is `EXTERNAL`, resolved from the credential below. Refused loudly rather + // than rewritten quietly: silently relabelling a client's explicit assertion + // hands it rows it did not ask for and no way to notice, while a 422 is + // fixable on the first call. // - // `APPLE_HEALTH` is the label that matters, and forging it is not merely - // mislabelling. It is half the `(userId, type, source, externalId)` dedup key, - // so a bridge's rows would land in the phone's namespace; it is a mergeable - // source, so a forged row joins the cross-source merge and can suppress a - // genuine one; and it is what `healthKitSyncSucceeded` reads to decide the - // Apple Health card may claim the phone synced. - if (scoped && parsed.data.entries.some((e) => e.source === "APPLE_HEALTH")) { + // `APPLE_HEALTH` is still the label that matters most, and forging it is not + // merely mislabelling. It is half the `(userId, type, source, externalId)` + // dedup key, so a bridge's rows would land in the phone's namespace; it is a + // mergeable source, so a forged row joins the cross-source merge and can + // suppress a genuine one; and it is what `healthKitSyncSucceeded` reads to + // decide the Apple Health card may claim the phone synced. + // + // v1.38.x — `MANUAL` joins it in being refused. It was permitted only + // because it was the value the route forced anyway, so nothing was ever + // overridden; now that bridge rows carry their own provenance, honouring it + // would defeat the point and overriding it would be the quiet relabelling + // the paragraph above rules out. Status and `errorCode` are unchanged. + if (scoped && parsed.data.entries.some((e) => e.source !== undefined)) { annotate({ action: { name: "measurement.batch.ingest" }, meta: { outcome: "source_not_permitted" }, }); - return apiError("This credential may only attribute MANUAL readings", 422, { - errorCode: "measurement.batch.source_not_permitted", - }); + return apiError( + "This credential resolves the source itself; omit the field", + 422, + { errorCode: "measurement.batch.source_not_permitted" }, + ); } const { entries, syncTrigger } = parsed.data; @@ -446,12 +456,14 @@ async function postBatch(request: NextRequest): Promise { // v1.8.6 W6 — honour the per-entry source tag, defaulting to // `APPLE_HEALTH` when absent so legacy callers are unchanged. // - // A scoped credential defaults to `MANUAL` instead, and can hold no - // other value: the guard above already refused an explicit - // `APPLE_HEALTH`, so this is the only remaining arm for it. The default - // has to move with the credential rather than with the route, because - // the route serves both the phone and a third-party bridge. - source: scoped ? "MANUAL" : (entry.source ?? "APPLE_HEALTH"), + // v1.38.x — a scoped credential gets `EXTERNAL`, and can hold no other + // value: the guard above refused any body that named one, so nothing + // asserted is being discarded here. The value has to move with the + // credential rather than with the route, because the route serves both + // the phone and a third-party bridge. It also moves the dedup key to + // `(userId, type, EXTERNAL, externalId)`, which is the isolated + // namespace this source exists to give. + source: scoped ? EXTERNAL_SOURCE : (entry.source ?? "APPLE_HEALTH"), measuredAt: mapped.takenAt, externalId: entry.externalId, externalSourceVersion: entry.externalSourceVersion ?? null, diff --git a/src/app/api/measurements/route.ts b/src/app/api/measurements/route.ts index a1eb337a1..6767b1ad7 100644 --- a/src/app/api/measurements/route.ts +++ b/src/app/api/measurements/route.ts @@ -4,6 +4,7 @@ import { isScopedCredential, requireRecordAuth, } from "@/lib/api-handler"; +import { EXTERNAL_SOURCE } from "@/lib/measurements/external-source"; import { MEASUREMENTS_WRITE_SCOPE } from "@/lib/measurements/scopes"; import { annotate } from "@/lib/logging/context"; import { fireAndForget } from "@/lib/logging/fire-and-forget"; @@ -678,19 +679,29 @@ async function sleepListResponse( } /** - * Does this body attribute any reading to `APPLE_HEALTH`? + * Does this body attribute a source to any reading — any source at all? * - * Read off the raw body rather than the parsed result because the answer has to - * be the same for the object arm and the array arm, and those parse against - * different schemas further down. Anything that is not a recognisable - * measurement shape answers false and falls through to the validation error it - * has coming — this function decides attribution, not validity. + * Read off the RAW body rather than the parsed result, and that is load-bearing + * twice over. It has to give the same answer for the object arm and the array + * arm, which parse against different schemas further down; and + * `createMeasurementSchema.source` carries `.default("MANUAL")`, so after + * parsing "said MANUAL" and "said nothing" are the same value and the + * distinction this predicate exists to draw is already gone. + * + * v1.38.x — widened from "names APPLE_HEALTH" to "names anything". Under a + * scoped credential the row's source is now resolved by the server, so an + * explicit `MANUAL` is an assertion the route would have to override — which + * is the silent relabelling the refusal below refuses to do. + * + * Anything that is not a recognisable measurement shape answers false and + * falls through to the validation error it has coming: this function decides + * attribution, not validity. */ -function namesAppleHealthSource(body: unknown): boolean { +function namesAnySource(body: unknown): boolean { const names = (entry: unknown): boolean => typeof entry === "object" && entry !== null && - (entry as { source?: unknown }).source === "APPLE_HEALTH"; + (entry as { source?: unknown }).source !== undefined; return Array.isArray(body) ? body.some(names) : names(body); } @@ -730,16 +741,28 @@ async function postMeasurement(request: NextRequest) { if (jsonError) return jsonError; - // A scoped credential may attribute MANUAL and nothing else — the schema's - // own default, so only an explicit `APPLE_HEALTH` needs refusing. Checked - // before validation branches so the object and array arms share one answer, - // and refused rather than rewritten for the reason the batch route gives: - // relabelling a client's explicit assertion hands it rows it cannot see. - if (scoped && namesAppleHealthSource(body)) { + // A scoped credential may attribute nothing: the source of its rows is + // `EXTERNAL`, decided here from the credential rather than taken from the + // body. Checked before the validation branches so the object and array arms + // share one answer, and refused rather than rewritten for the reason the + // batch route gives: relabelling a client's explicit assertion hands it rows + // it did not ask for and no way to notice. + // + // v1.38.x — this used to refuse only an explicit `APPLE_HEALTH`, because the + // forced value was `MANUAL` and so a body naming `MANUAL` was never actually + // overridden. Now that bridge rows carry their own provenance, honouring + // such a body would defeat the point (a bridge that kept sending `MANUAL` + // would stay indistinguishable from a typed reading) and overriding it + // silently is what the paragraph above forbids. Status and `errorCode` are + // unchanged, so client error handling is untouched; the documented Home + // Assistant payload sends no `source` at all. + if (scoped && namesAnySource(body)) { annotate({ action: { name: "measurements.create.source-not-permitted" } }); - return apiError("This credential may only attribute MANUAL readings", 422, { - errorCode: "measurement.create.source_not_permitted", - }); + return apiError( + "This credential resolves the source itself; omit the field", + 422, + { errorCode: "measurement.create.source_not_permitted" }, + ); } // Batch mode (array of measurements, e.g. combined BP + Pulse) @@ -791,7 +814,16 @@ async function postMeasurement(request: NextRequest) { type: m.type as MeasurementType, value: m.value, unit: getUnitForType(m.type), - source: (m.source ?? "MANUAL") as MeasurementSource, + // v1.38.x — the array arm too, not just the single create + // below. This is the combined BP + Pulse path a bridge uses for + // a two-value reading; missing it here would leave exactly those + // rows attributed MANUAL and invisible to the badge and the + // source filter. `scoped` already implies `m.source` is + // undefined — the refusal above returned otherwise — so this is + // the resolution, not an override. + source: (scoped + ? EXTERNAL_SOURCE + : (m.source ?? "MANUAL")) as MeasurementSource, measuredAt: m.measuredAt, // v1.23 — encrypt the note at rest; the legacy plaintext column is // written null for new rows. @@ -946,7 +978,13 @@ async function postMeasurement(request: NextRequest) { type: type as MeasurementType, value, unit: getUnitForType(type), - source: (source ?? "MANUAL") as MeasurementSource, + // v1.38.x — server-resolved under a scoped credential. `source` is + // necessarily the schema default here (the refusal above returned on + // any body that named one), so nothing a caller asserted is being + // discarded. + source: (scoped + ? EXTERNAL_SOURCE + : (source ?? "MANUAL")) as MeasurementSource, measuredAt, // v1.23 — encrypt the note at rest; legacy plaintext column nulled. notes: null,