fix(crypto): one read order for a sealed column beside its readable one

Notes, workout tracks and the cached insights payload each read their
ciphertext and legacy column their own way: the cache read the readable
column first, and the two that read the ciphertext first disagreed on a
value that does not open. readSealedOrLegacy fixes the order (ciphertext
whenever present, the readable column only without one, never as a stand-in
for a failed open) and makes each caller state what a failed open means.

src/lib/crypto/__tests__/sealed-or-legacy.test.ts

@@ -0,0 +1,69 @@
+import { describe, expect, it, vi } from "vitest";
+
+const addWarning = vi.fn();
+vi.mock("@/lib/logging/context", () => ({ getEvent: () => ({ addWarning }) }));
+
+import { readSealedOrLegacy, warnAndDrop } from "../sealed-or-legacy";
+
+const sealed = new Uint8Array([1, 2, 3]);
+const open = (bytes: Uint8Array) => `opened:${bytes.length}`;
+const broken = () => {
+  throw new Error("bad tag");
+};
+
+describe("readSealedOrLegacy", () => {
+  it("reads the ciphertext first, even with a readable value beside it", () => {
+    expect(
+      readSealedOrLegacy({
+        sealed,
+        legacy: "readable",
+        open,
+        onUndecryptable: "throw",
+      }),
+    ).toBe("opened:3");
+  });
+
+  it("reads the readable column only when there is no ciphertext", () => {
+    for (const none of [null, undefined, new Uint8Array(0)]) {
+      expect(
+        readSealedOrLegacy({
+          sealed: none,
+          legacy: "readable",
+          open,
+          onUndecryptable: "throw",
+        }),
+      ).toBe("readable");
+    }
+    expect(
+      readSealedOrLegacy({
+        sealed: null,
+        legacy: null,
+        open,
+        onUndecryptable: "throw",
+      }),
+    ).toBeNull();
+  });
+
+  it("throws on a ciphertext that does not open when told to", () => {
+    expect(() =>
+      readSealedOrLegacy({
+        sealed,
+        legacy: "readable",
+        open: broken,
+        onUndecryptable: "throw",
+      }),
+    ).toThrow("bad tag");
+  });
+
+  it("drops with a warning, never falling back to the readable value", () => {
+    expect(
+      readSealedOrLegacy({
+        sealed,
+        legacy: "readable",
+        open: broken,
+        onUndecryptable: warnAndDrop("thing did not open"),
+      }),
+    ).toBeNull();
+    expect(addWarning).toHaveBeenCalledWith("thing did not open: bad tag");
+  });
+});

src/lib/crypto/note-cipher.ts

@@ -24,6 +24,7 @@
  * when there is NO ciphertext (a legacy row not yet covered by the backfill).
  */
 import { decryptFromBytes, encryptToBytes } from "@/lib/ai/coach/bytes-codec";
+import { readSealedOrLegacy } from "@/lib/crypto/sealed-or-legacy";
 
 /**
  * Encrypt a free-text note for the `Bytes` ciphertext column. Returns `null`
@@ -51,10 +52,12 @@ export function readNote(
   ciphertext: Uint8Array | null | undefined,
   plaintextFallback: string | null | undefined,
 ): string | null {
-  if (ciphertext && ciphertext.byteLength > 0) {
-    return decryptFromBytes(ciphertext);
-  }
-  return plaintextFallback ?? null;
+  return readSealedOrLegacy({
+    sealed: ciphertext,
+    legacy: plaintextFallback,
+    open: decryptFromBytes,
+    onUndecryptable: "throw",
+  });
 }
 
 /**

src/lib/crypto/sealed-or-legacy.ts

@@ -0,0 +1,53 @@
+/**
+ * The one read rule for a value stored sealed beside a legacy readable column.
+ *
+ * Several columns moved from plaintext to AES-256-GCM and keep the readable
+ * column until a backfill reaches every row: notes, workout tracks, the cached
+ * insights payload. Each reader once picked its own order and its own answer
+ * to a ciphertext that does not open, and they disagreed: two read the
+ * ciphertext first, one read the readable column first, and of the two that
+ * opened the ciphertext first one threw and one returned nothing.
+ *
+ * The order is fixed here and is not a parameter. A present ciphertext is
+ * always the answer: the readable column is read only when the row has no
+ * ciphertext, never as a stand-in for one that failed to open, because that
+ * would serve whatever an older writer left in the clear in place of the value
+ * the key was meant to protect. What a failure to open means is the caller's
+ * decision and must be stated: `"throw"` for a record whose loss must surface,
+ * or a handler for a value whose absence the surface can carry, typically
+ * {@link warnAndDrop}.
+ */
+import { getEvent } from "@/lib/logging/context";
+
+export type OnUndecryptable<T> = "throw" | ((error: unknown) => T | null);
+
+export function readSealedOrLegacy<T>(args: {
+  sealed: Uint8Array | null | undefined;
+  legacy: T | null | undefined;
+  open: (sealed: Uint8Array) => T;
+  onUndecryptable: OnUndecryptable<T>;
+}): T | null {
+  const { sealed, legacy, open, onUndecryptable } = args;
+  if (sealed && sealed.byteLength > 0) {
+    if (onUndecryptable === "throw") return open(sealed);
+    try {
+      return open(sealed);
+    } catch (error) {
+      return onUndecryptable(error);
+    }
+  }
+  return legacy ?? null;
+}
+
+/**
+ * A handler that records the failure as a warning on the request's event and
+ * reads the value as absent.
+ */
+export function warnAndDrop(label: string): (error: unknown) => null {
+  return (error) => {
+    getEvent()?.addWarning(
+      `${label}: ${error instanceof Error ? error.message : String(error)}`,
+    );
+    return null;
+  };
+}

src/lib/insights/__tests__/cached-text-cipher.test.ts

@@ -41,13 +41,25 @@ describe("cached insights payload at rest", () => {
     ).toBe(PAYLOAD);
   });
 
-  it("prefers a readable value over a sealed one, since only an older writer can have put it there", () => {
+  // This used to prefer the readable column, the one sealed column read
+  // that way. Every sealed column now reads the ciphertext first.
+  it("reads a sealed value before a readable one beside it", () => {
     expect(
       openInsightsCachedText({
-        insightsCachedText: "newer, from the previous release",
-        insightsCachedTextEncrypted: sealInsightsCachedText("older"),
+        insightsCachedText: "readable",
+        insightsCachedTextEncrypted: sealInsightsCachedText("sealed"),
       }),
-    ).toBe("newer, from the previous release");
+    ).toBe("sealed");
+  });
+
+  it("never serves the readable value in place of a sealed one that does not open", () => {
+    const foreign = encryptBytes(Buffer.from(PAYLOAD, "utf8"), "other/v1");
+    expect(
+      openInsightsCachedText({
+        insightsCachedText: "readable",
+        insightsCachedTextEncrypted: new Uint8Array(foreign),
+      }),
+    ).toBeNull();
   });
 
   it("does not open a value sealed for another purpose", () => {

src/lib/insights/cached-text-cipher.ts

@@ -19,7 +19,7 @@ import { Buffer } from "node:buffer";
 
 import { decryptBytes, encryptBytes } from "@/lib/crypto";
 import { INSIGHTS_CACHED_TEXT_AAD } from "@/lib/crypto/encrypted-columns";
-import { getEvent } from "@/lib/logging/context";
+import { readSealedOrLegacy, warnAndDrop } from "@/lib/crypto/sealed-or-legacy";
 
 /** The columns a reader selects to open the cache. */
 export const INSIGHTS_CACHE_SELECT = {
@@ -76,10 +76,14 @@ export const INSIGHTS_CACHE_PRESENT_WHERE = {
 /**
  * The cached payload as text, or null when there is none.
  *
- * A legacy readable value wins when one is present: from v1.43.7 on nothing
- * writes that column, so a row holding both can only come from a writer of the
- * previous release still running after the upgrade, and its value is the
- * newer one. The backfill seals it on its next pass.
+ * The ciphertext is read first, as every sealed column is read
+ * (`readSealedOrLegacy`); the readable column only when the row holds no
+ * ciphertext. A row holding both can come from a writer of the previous
+ * release still running after the upgrade; its readable value may be the
+ * newer one, and the free-text backfill seals it over the old ciphertext on
+ * its next pass. Until then the reader serves the sealed one: an older
+ * briefing for a while, never a readable value preferred over a protected
+ * one.
  *
  * A sealed value that does not open reads as an empty cache with a warning on
  * the event. It is a cache: the next warm overwrites it under the active key,
@@ -89,17 +93,13 @@ export function openInsightsCachedText(
   row: InsightsCacheColumns | null | undefined,
 ): string | null {
   if (!row) return null;
-  if (typeof row.insightsCachedText === "string") return row.insightsCachedText;
-  const sealed = row.insightsCachedTextEncrypted;
-  if (!sealed || sealed.byteLength === 0) return null;
-  try {
-    return decryptBytes(Buffer.from(sealed), INSIGHTS_CACHED_TEXT_AAD).toString(
-      "utf8",
-    );
-  } catch (err) {
-    getEvent()?.addWarning(
-      `insights cache did not open: ${err instanceof Error ? err.message : String(err)}`,
-    );
-    return null;
-  }
+  return readSealedOrLegacy({
+    sealed: row.insightsCachedTextEncrypted,
+    legacy: row.insightsCachedText,
+    open: (sealed) =>
+      decryptBytes(Buffer.from(sealed), INSIGHTS_CACHED_TEXT_AAD).toString(
+        "utf8",
+      ),
+    onUndecryptable: warnAndDrop("insights cache did not open"),
+  });
 }

src/lib/workouts/route-geometry-cipher.ts

@@ -23,7 +23,7 @@ import { Buffer } from "node:buffer";
 
 import { decryptBytes, encryptBytes } from "@/lib/crypto";
 import { WORKOUT_ROUTE_GEOMETRY_AAD } from "@/lib/crypto/encrypted-columns";
-import { getEvent } from "@/lib/logging/context";
+import { readSealedOrLegacy, warnAndDrop } from "@/lib/crypto/sealed-or-legacy";
 
 /** Seal a route's GeoJSON geometry for `WorkoutRoute.geometryEncrypted`. */
 export function encryptRouteGeometry(
@@ -53,17 +53,10 @@ export function readRouteGeometry(row: {
   geometry?: unknown;
   geometryEncrypted?: Uint8Array | null;
 }): unknown {
-  if (row.geometryEncrypted && row.geometryEncrypted.byteLength > 0) {
-    try {
-      return decryptRouteGeometry(row.geometryEncrypted);
-    } catch (err) {
-      getEvent()?.addWarning(
-        `workout route decrypt failed: ${
-          err instanceof Error ? err.message : String(err)
-        }`,
-      );
-      return null;
-    }
-  }
-  return row.geometry ?? null;
+  return readSealedOrLegacy<unknown>({
+    sealed: row.geometryEncrypted,
+    legacy: row.geometry,
+    open: decryptRouteGeometry,
+    onUndecryptable: warnAndDrop("workout route decrypt failed"),
+  });
 }