diff --git a/src/lib/crypto/__tests__/sealed-or-legacy.test.ts b/src/lib/crypto/__tests__/sealed-or-legacy.test.ts new file mode 100644 index 000000000..71e816a1e --- /dev/null +++ b/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"); + }); +}); diff --git a/src/lib/crypto/note-cipher.ts b/src/lib/crypto/note-cipher.ts index dc63001dd..04463d9e2 100644 --- a/src/lib/crypto/note-cipher.ts +++ b/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", + }); } /** diff --git a/src/lib/crypto/sealed-or-legacy.ts b/src/lib/crypto/sealed-or-legacy.ts new file mode 100644 index 000000000..4de4cfb22 --- /dev/null +++ b/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 = "throw" | ((error: unknown) => T | null); + +export function readSealedOrLegacy(args: { + sealed: Uint8Array | null | undefined; + legacy: T | null | undefined; + open: (sealed: Uint8Array) => T; + onUndecryptable: OnUndecryptable; +}): 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; + }; +} diff --git a/src/lib/insights/__tests__/cached-text-cipher.test.ts b/src/lib/insights/__tests__/cached-text-cipher.test.ts index 276347f58..8fcd71142 100644 --- a/src/lib/insights/__tests__/cached-text-cipher.test.ts +++ b/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", () => { diff --git a/src/lib/insights/cached-text-cipher.ts b/src/lib/insights/cached-text-cipher.ts index d51cb65a7..c74cbfcb0 100644 --- a/src/lib/insights/cached-text-cipher.ts +++ b/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"), + }); } diff --git a/src/lib/workouts/route-geometry-cipher.ts b/src/lib/workouts/route-geometry-cipher.ts index 80ed5f4fb..5387a663c 100644 --- a/src/lib/workouts/route-geometry-cipher.ts +++ b/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({ + sealed: row.geometryEncrypted, + legacy: row.geometry, + open: decryptRouteGeometry, + onUndecryptable: warnAndDrop("workout route decrypt failed"), + }); }