diff --git a/e2e/score-history.spec.ts b/e2e/score-history.spec.ts index 74cd68ddb..363720734 100644 --- a/e2e/score-history.spec.ts +++ b/e2e/score-history.spec.ts @@ -261,10 +261,17 @@ test.describe("score histories", () => { await expect( page.locator(`${chart} [data-slot="chart-overlay-controls-trigger"]`), ).toHaveCount(0); + // The choice is saved by a PUT; reloading before it answers cancels it. + const saved = page.waitForResponse( + (res) => + res.url().includes("/api/dashboard/chart-overlay-prefs") && + res.request().method() === "PUT", + ); await page .locator(`${chart} [data-slot="chart-range-tab"][data-value="7"]`) .click(); await expectSharedTabs(page, chart, "7"); + expect((await saved).status()).toBe(200); await page.reload(); await expectSharedTabs(page, chart, "7"); await resetRange(page, "scoreRecovery"); diff --git a/e2e/timeline.spec.ts b/e2e/timeline.spec.ts index 610a063d8..4ade3ce8b 100644 --- a/e2e/timeline.spec.ts +++ b/e2e/timeline.spec.ts @@ -865,9 +865,13 @@ test("a lab marker is picked, drawn as its readings, read on its own and kept", const target = page.locator( '[data-slot="timeline-point-target"][data-series="lab:bm-ferritin"][data-t="fe-1"]', ); - await target.focus(); const tip = page.locator('[data-slot="timeline-point-tip"]'); - await expect(tip).toHaveAttribute("data-t", "fe-1"); + // The chart redraws its targets once the marker's readings arrive, and a + // target focused just before that is gone; focus the one that stays. + await expect(async () => { + await target.focus(); + await expect(tip).toHaveAttribute("data-t", "fe-1", { timeout: 1_000 }); + }).toPass({ timeout: 10_000 }); await expect(tip).toContainText("22 ng/mL"); await expect(tip).toContainText("Laborbefund März"); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 81124e808..18b066b1a 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -36,6 +36,7 @@ overrides: brace-expansion@>=4.0.0 <5.0.12: ^5.0.12 undici@>=7.0.0 <7.30.1: ^7.30.1 vite@>=8.0.0 <8.1.6: ^8.1.6 + node-forge@<1.4.2: ^1.4.2 patchedDependencies: '@stryker-mutator/vitest-runner@10.0.0': 9ad3e9855fd9781fd36e872ed42c5f934f91b962d6830e6cc56d56bf16e1dabd @@ -5625,8 +5626,8 @@ packages: encoding: optional: true - node-forge@1.4.0: - resolution: {integrity: sha512-LarFH0+6VfriEhqMMcLX2F7SwSXeWwnEAJEsYm5QKWchiVYVvJyV9v7UDvUv+w5HO23ZpQTXDv/GxdDdMyOuoQ==} + node-forge@1.4.2: + resolution: {integrity: sha512-rRtAR5XLJ8gCKd9du4qrKAsgCG1ObGP4gLeNJMa4vYSeq4bmBMOJk5IHkrviBV0q7uoAJlVd8HEQYQSVUb4FGQ==} engines: {node: '>= 6.13.0'} node-releases@2.0.58: @@ -8711,7 +8712,7 @@ snapshots: dependencies: debug: 4.4.3(supports-color@7.2.0) jsonwebtoken: 9.0.3 - node-forge: 1.4.0 + node-forge: 1.4.2 verror: 1.10.1 transitivePeerDependencies: - supports-color @@ -12864,7 +12865,7 @@ snapshots: dependencies: whatwg-url: 5.0.0 - node-forge@1.4.0: {} + node-forge@1.4.2: {} node-releases@2.0.58: {} diff --git a/pnpm-workspace.yaml b/pnpm-workspace.yaml index fc7937736..f716b3daf 100644 --- a/pnpm-workspace.yaml +++ b/pnpm-workspace.yaml @@ -109,6 +109,11 @@ overrides: # under vitest, never in the image; vitest asks for ^8, so a floor on the # same major moves it and nothing else. "vite@>=8.0.0 <8.1.6": "^8.1.6" + # GHSA-86w9-cpqp-85rv, GHSA-7wch-p9px-956x, GHSA-jc66-7q78-782g, + # GHSA-8hrf-8g6f-7xwp, GHSA-hqq3-7jhf-c5jx, GHSA-5x38-2vff-x55p: signature, + # certificate-chain and TLS-record flaws in node-forge <= 1.4.0. It arrives + # only through @parse/node-apn; 1.4.2 is the release node-apn 8.1.1 pins. + "node-forge@<1.4.2": "^1.4.2" allowBuilds: "@prisma/engines": true @@ -129,15 +134,5 @@ minimumReleaseAgeExclude: - lucide-react@1.27.0 - undici@8.10.0 -auditConfig: - ignoreGhsas: - # node-forge <=1.4.0, RSA PKCS#1 v1.5 signature verification. It arrives - # only through @parse/node-apn, which uses it for certificate-based APNs - # credentials (p12 / PEM certificate). HealthLog authenticates to APNs - # with a token (.p8 key, APNS_KEY_ID / APNS_TEAM_ID), so that code path - # never runs. No patched release is published yet (latest is 1.4.0); - # remove this entry and raise the floor once 1.4.1 is out. - - GHSA-86w9-cpqp-85rv - patchedDependencies: "@stryker-mutator/vitest-runner@10.0.0": patches/@stryker-mutator__vitest-runner@10.0.0.patch diff --git a/src/app/api/whoop/callback/__tests__/route.test.ts b/src/app/api/whoop/callback/__tests__/route.test.ts index 58aed091c..d467bfccf 100644 --- a/src/app/api/whoop/callback/__tests__/route.test.ts +++ b/src/app/api/whoop/callback/__tests__/route.test.ts @@ -1,4 +1,5 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; +import type { WhoopProfile } from "@/lib/whoop/wire-types"; // Single-use claiming is covered against Postgres in // tests/integration/oauth-callback-single-use.test.ts. Here the state row is @@ -175,6 +176,26 @@ describe("GET /api/whoop/callback custom-scheme redirect", () => { expect(connUpsert).not.toHaveBeenCalled(); }); + it("refuses a profile without a user id instead of storing a shared placeholder", async () => { + stateDelete.mockResolvedValue({ + userId: "u1", + expiresAt: new Date(Date.now() + 60_000), + returnScheme: null, + }); + const { fetchProfile } = await import("@/lib/whoop/client"); + // An off-spec body: the wire type says user_id is always there. + const offSpec: Partial = {}; + vi.mocked(fetchProfile).mockResolvedValueOnce(offSpec as WhoopProfile); + + const res = await GET(makeReq("nonce-noid")); + + expect(res.headers.get("location")).toBe( + "https://app.example/settings/integrations?whoop=error&reason=token", + ); + expect(connFindUnique).not.toHaveBeenCalled(); + expect(connUpsert).not.toHaveBeenCalled(); + }); + it("maps a concurrent WHOOP identity P2002 to the safe owner-conflict redirect", async () => { stateDelete.mockResolvedValue({ userId: "local-user", diff --git a/src/app/api/whoop/callback/route.ts b/src/app/api/whoop/callback/route.ts index 36da98e8b..e1a511857 100644 --- a/src/app/api/whoop/callback/route.ts +++ b/src/app/api/whoop/callback/route.ts @@ -204,6 +204,15 @@ export const GET = apiHandler(async (request: NextRequest) => { const tokens = await whoopOAuth.exchange(code, creds); const profile = await fetchProfile(tokens.access_token); + // An off-spec profile without `user_id` would store the string + // "undefined" as the account id: every such connection would then look + // like one WHOOP account, and the check below would refuse the second. + if ( + typeof profile.user_id !== "number" || + !Number.isFinite(profile.user_id) + ) { + return outcomeRedirect(returnScheme, "error", "token"); + } const whoopUserId = String(profile.user_id); const expiresAt = new Date(Date.now() + tokens.expires_in * 1000); const owner = await prisma.whoopConnection.findUnique({ diff --git a/src/lib/google-health/account-id-heal.ts b/src/lib/google-health/account-id-heal.ts new file mode 100644 index 000000000..a596d1f6b --- /dev/null +++ b/src/lib/google-health/account-id-heal.ts @@ -0,0 +1,72 @@ +import { prisma } from "@/lib/db"; +import { annotate, getEvent } from "@/lib/logging/context"; +import { accountLinkedElsewhere } from "@/lib/integrations/oauth-kit/account-link"; +import { accountIdFor, fetchProfile } from "./client"; +import { getValidToken, runWithGoogleHealthSyncCycle } from "./sync-core"; + +/** How a cycle's look at a connection without an account id ended. */ +export type GoogleHealthAccountIdHeal = + "healed" | "unreadable" | "linked_elsewhere" | "failed"; + +/** + * Fill in the Google account id of a connection that has none. + * + * Until v1.46.4 every connection stored the constant `profile` as its id, and + * migration 0491 cleared it to null. Such a connection syncs normally, but the + * one-account-per-record check (`accountLinkedElsewhere`) cannot see it, so the + * same Google account could be connected to a second record. This reads the + * id with the connection's own token the way the callback does and stores it, + * which closes that gap without asking anyone to reconnect. + * + * Best-effort and never in the sync's way: a failed read leaves the id null + * for the next cycle, and an id another record already holds is not written + * (two records would then claim one account; the sync itself carries on, and + * the next connect on either record meets the check). The write is guarded on + * the id still being null, so a reconnect landing meanwhile wins. + */ +export async function healGoogleHealthAccountId( + userId: string, + accessToken: string, +): Promise { + let outcome: GoogleHealthAccountIdHeal; + try { + const profile = await fetchProfile(accessToken); + const googleUserId = await accountIdFor(profile, accessToken); + if (!googleUserId) { + outcome = "unreadable"; + } else if ( + await accountLinkedElsewhere("google-health", googleUserId, userId) + ) { + outcome = "linked_elsewhere"; + } else { + await prisma.googleHealthConnection.updateMany({ + where: { userId, googleUserId: null }, + data: { googleUserId }, + }); + outcome = "healed"; + } + } catch (err) { + getEvent()?.addWarning( + `google-health: account id read failed for ${userId}: ${err}`, + ); + outcome = "failed"; + } + annotate({ meta: { "googleHealth.accountId.heal": outcome } }); + return outcome; +} + +/** + * The sync cycle's hook: after a clean cycle, a connection that migration + * 0491 left without an account id reads it back. A connection that has one + * is left alone, so the profile is read once. + */ +export async function healUnknownGoogleHealthAccountId( + userId: string, + googleUserId: string | null, +): Promise { + if (googleUserId !== null) return; + const token = await runWithGoogleHealthSyncCycle(() => getValidToken(userId)); + if (token.result) { + await healGoogleHealthAccountId(userId, token.result.accessToken); + } +} diff --git a/src/lib/google-health/sync.ts b/src/lib/google-health/sync.ts index 02676d189..611f234a9 100644 --- a/src/lib/google-health/sync.ts +++ b/src/lib/google-health/sync.ts @@ -15,6 +15,7 @@ import { syncUserMetrics } from "./sync-metrics"; import { syncUserSleep } from "./sync-sleep"; import { syncUserWorkout } from "./sync-workout"; import { withGoogleHealthSyncLock } from "./sync-lock"; +import { healUnknownGoogleHealthAccountId } from "./account-id-heal"; import { GOOGLE_HEALTH_INTEGRATION_KEY, getValidToken, @@ -235,7 +236,7 @@ async function runGoogleHealthSyncCycle( const cycleStartedAt = new Date(); const connection = await prisma.googleHealthConnection.findUnique({ where: { userId }, - select: { lastSyncedAt: true }, + select: { lastSyncedAt: true, googleUserId: true }, }); if (!connection) { return { state: "failed", imported: 0, failed: true, resources: [] }; @@ -389,6 +390,7 @@ async function runGoogleHealthSyncCycle( if (!failed) { await markSynced(userId, cycleStartedAt); await recordSyncSuccess(userId, GOOGLE_HEALTH_INTEGRATION_KEY); + await healUnknownGoogleHealthAccountId(userId, connection.googleUserId); } // Background-sync posture (mirrors the Fitbit tail): mark the per-user diff --git a/tests/integration/google-health-account-id.test.ts b/tests/integration/google-health-account-id.test.ts index 5c0ec2cd1..07bfd9332 100644 --- a/tests/integration/google-health-account-id.test.ts +++ b/tests/integration/google-health-account-id.test.ts @@ -7,6 +7,10 @@ * stored, the constant `profile`, so every connection looked like the same * Google account and the one-account-per-record check refused the second * record on the instance with `already_linked`, also on a reconnect. + * + * Migration 0491 cleared those placeholders to null. A connection left with + * no id syncs, but the check cannot see it; a clean sync cycle reads the id + * back with the connection's own token and stores it. */ import { NextRequest } from "next/server"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; @@ -48,6 +52,22 @@ vi.mock("@/lib/jobs/boss-ready", () => ({ enqueueWhenBossReady: vi.fn().mockResolvedValue("skipped"), })); +// The sync cycle's four resources stand in as one written row each, so a +// cycle is clean and reaches the account-id read; their own wire is covered +// elsewhere. +vi.mock("@/lib/google-health/sync-workout", () => ({ + syncUserWorkout: vi.fn(async () => 1), +})); +vi.mock("@/lib/google-health/sync-sleep", () => ({ + syncUserSleep: vi.fn(async () => 1), +})); +vi.mock("@/lib/google-health/sync-activity", () => ({ + syncUserActivity: vi.fn(async () => 1), +})); +vi.mock("@/lib/google-health/sync-metrics", () => ({ + syncUserMetrics: vi.fn(async () => 1), +})); + const TOKEN_URL = "https://oauth2.googleapis.com/token"; const PROFILE_URL = "https://health.googleapis.com/v4/users/me/profile"; const IDENTITY_URL = "https://health.googleapis.com/v4/users/me/identity"; @@ -214,3 +234,106 @@ describe("Google Health account id", () => { expect(await storedId(SECOND)).toBeNull(); }); }); + +/** A connection as migration 0491 leaves it: synced, token valid, id unknown. */ +async function seedUnknownId( + userId: string, + googleUserId: string | null = null, +) { + const { encrypt } = await import("@/lib/crypto"); + await getPrismaClient().googleHealthConnection.create({ + data: { + userId, + googleUserId, + accessToken: encrypt(`access-${userId}`), + refreshToken: encrypt(`refresh-${userId}`), + tokenExpiresAt: new Date(Date.now() + 3_600_000), + needsReauth: false, + }, + }); +} + +function profileReads(): number { + return vi + .mocked(globalThis.fetch) + .mock.calls.filter(([input]) => urlOf(input) === PROFILE_URL).length; +} + +describe("Google Health account id after the placeholder repair", () => { + it("reads the id back on a clean sync, so the same account is refused elsewhere", async () => { + await seedUnknownId(FIRST); + googleAs("111"); + const { syncUserGoogleHealth } = await import("@/lib/google-health/sync"); + const result = await syncUserGoogleHealth(FIRST); + expect(result.failed).toBe(false); + expect(await storedId(FIRST)).toBe("111"); + + expect(await connect(SECOND)).toContain("reason=already_linked"); + expect(await storedId(SECOND)).toBeUndefined(); + }); + + it("does not read the profile again once the id is known", async () => { + await seedUnknownId(FIRST, "111"); + googleAs("111"); + const { syncUserGoogleHealth } = await import("@/lib/google-health/sync"); + await syncUserGoogleHealth(FIRST); + expect(profileReads()).toBe(0); + expect(await storedId(FIRST)).toBe("111"); + }); + + it("leaves the id unknown when another record already holds the account", async () => { + await seedUnknownId(FIRST); + await seedUnknownId(SECOND, "111"); + googleAs("111"); + const { healGoogleHealthAccountId } = + await import("@/lib/google-health/account-id-heal"); + expect(await healGoogleHealthAccountId(FIRST, "access-gh-first")).toBe( + "linked_elsewhere", + ); + expect(await storedId(FIRST)).toBeNull(); + expect(await storedId(SECOND)).toBe("111"); + }); + + it("keeps an id a reconnect stored while the read was under way", async () => { + await seedUnknownId(FIRST); + vi.mocked(globalThis.fetch).mockImplementation(async (input) => { + const url = urlOf(input); + if (url === PROFILE_URL) { + // The callback lands between the profile read and the write. + await getPrismaClient().googleHealthConnection.update({ + where: { userId: FIRST }, + data: { googleUserId: "999" }, + }); + return json({ name: "users/111/profile" }); + } + throw new Error(`Unexpected fetch: ${url}`); + }); + const { healGoogleHealthAccountId } = + await import("@/lib/google-health/account-id-heal"); + expect(await healGoogleHealthAccountId(FIRST, "access-gh-first")).toBe( + "healed", + ); + expect(await storedId(FIRST)).toBe("999"); + }); + + it("stays unknown, and the sync stays clean, when Google names no account", async () => { + await seedUnknownId(FIRST); + googleAs("444", { profileName: "users/me/profile", identityStatus: 403 }); + const { syncUserGoogleHealth } = await import("@/lib/google-health/sync"); + const result = await syncUserGoogleHealth(FIRST); + expect(result.failed).toBe(false); + expect(await storedId(FIRST)).toBeNull(); + }); + + it("keeps the sync clean when the profile read fails", async () => { + await seedUnknownId(FIRST); + vi.mocked(globalThis.fetch).mockImplementation(async (input) => { + if (urlOf(input) === PROFILE_URL) return json({ error: {} }, 503); + throw new Error(`Unexpected fetch: ${urlOf(input)}`); + }); + const { syncUserGoogleHealth } = await import("@/lib/google-health/sync"); + const result = await syncUserGoogleHealth(FIRST); + expect(result.failed).toBe(false); + expect(await storedId(FIRST)).toBeNull(); + }); +}); diff --git a/tests/integration/measurement-rollups.test.ts b/tests/integration/measurement-rollups.test.ts index c813fdebf..795b30c2a 100644 --- a/tests/integration/measurement-rollups.test.ts +++ b/tests/integration/measurement-rollups.test.ts @@ -25,6 +25,7 @@ */ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { settleBackgroundTasks } from "@/lib/logging/background-tasks"; import { getPrismaClient, truncateAllTables } from "./setup"; import { ROLLUP_FULL_BACKFILL_QUEUE, @@ -580,10 +581,14 @@ describe("measurement rollups — integration", () => { ], }); - // Warm-up read populates the rollups via `ensureUserRollupsFresh`. + // Warm-up read populates the rollups via `ensureUserRollupsFresh`. The + // read starts it detached, so wait for it: otherwise the third row's + // bucket can land first and the next read takes the one WEIGHT bucket + // for the whole history. const before = await buildComprehensiveAggregate(user.id); expect(before.summaries.WEIGHT.count).toBe(2); expect(before.summaries.WEIGHT.mean).toBe(80.5); + await settleBackgroundTasks(); // Insert a third row and synchronously refresh the DAY bucket the // way the write-path hooks do.