Merge pull request #1306 from MBombeck/fix/google-health-account-id-heal

Read the Google Health account id back; refuse a WHOOP profile without one

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");

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");
 

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: {}
 

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

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<WhoopProfile> = {};
+    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",

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({

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<GoogleHealthAccountIdHeal> {
+  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<void> {
+  if (googleUserId !== null) return;
+  const token = await runWithGoogleHealthSyncCycle(() => getValidToken(userId));
+  if (token.result) {
+    await healGoogleHealthAccountId(userId, token.result.accessToken);
+  }
+}

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

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();
+  });
+});

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.