fix(encryption): remove aad parameter from decrypt methods

decryptText/decryptObject now read the aad off the stored blob instead
of requiring the caller to reconstruct it, since only 1 of the 22
production decrypt call sites derived one and duplicating that string
between writer and reader was a correctness footgun with no real
payoff. encryptText/encryptObject keep their aad parameter to stamp
the value at write time.

The one place that read .aad directly (the appointment-token purpose
check) keeps its own parameter and comparison, since that guards token
purpose separation rather than crypto authentication and now does so
alone. Also reverts platform-credential's required-aad schema and its
now-inaccurate rationale, since decrypt no longer needs a caller-
derived aad to bind against.

Author: realcodesiman

packages/business/__tests__/platform-credential-service.test.ts

@@ -9,13 +9,24 @@ vi.mock("@chatbotx.io/database/client", () => ({
   eq: vi.fn(),
   isNull: vi.fn(),
 }))
+const credentialSchemas = {
+  instagram: { name: "instagram-schema" },
+  messenger: { name: "messenger-schema" },
+}
+const credentialPublicSchemas = {
+  messenger: { parse: vi.fn((value: unknown) => value) },
+}
+const credentialEncryptedSchema = {
+  parse: vi.fn((value: unknown) => value),
+}
 vi.mock("@chatbotx.io/database/partials", () => ({
-  credentialEncryptedSchema: {},
-  credentialPublicSchemas: {},
-  credentialSchemas: {},
+  credentialEncryptedSchema,
+  credentialPublicSchemas,
+  credentialSchemas,
 }))
 vi.mock("@chatbotx.io/database/schema", () => ({ platformCredentialModel: {} }))
-vi.mock("@chatbotx.io/encryption", () => ({ encryptUtils: {} }))
+const encryptUtils = { decryptObject: vi.fn(), encryptObject: vi.fn() }
+vi.mock("@chatbotx.io/encryption", () => ({ encryptUtils }))
 vi.mock("@chatbotx.io/redis", () => ({
   invalidateCacheByTags: vi.fn(async () => undefined),
   withCache: vi.fn(async (_key: string, fn: () => unknown) => fn()),
@@ -39,6 +50,9 @@ beforeEach(() => {
 
 afterEach(() => {
   vi.restoreAllMocks()
+  encryptUtils.decryptObject.mockReset()
+  encryptUtils.encryptObject.mockReset()
+  credentialEncryptedSchema.parse.mockClear()
 })
 
 describe("resolveForOwner", () => {
@@ -229,3 +243,140 @@ describe("resolvePlatformAppAccessToken", () => {
     ).resolves.toBeUndefined()
   })
 })
+
+describe("_decrypt", () => {
+  // decryptObject takes no aad argument: it reads the aad off the blob
+  // itself (see packages/encryption/src/encryption.ts decryptText), so
+  // `_decrypt` doesn't need to re-derive the row-scoped aad the writers
+  // stamped — it just passes the blob and schema through.
+  test("decrypts a platform-scoped row", async () => {
+    encryptUtils.decryptObject.mockResolvedValue({ clientId: "client-1" })
+
+    await expect(
+      (platformCredentialService as never)._decrypt({
+        id: "platform-1",
+        userId: null,
+        type: "instagram",
+        livemode: false,
+        value: { encrypted: true },
+        publicConfig: { clientId: "client-1" },
+        createdAt: new Date("2026-08-13T00:00:00.000Z"),
+        updatedAt: new Date("2026-08-13T00:00:00.000Z"),
+      }),
+    ).resolves.toEqual(
+      expect.objectContaining({
+        id: "platform-1",
+        userId: null,
+        type: "instagram",
+        config: { clientId: "client-1" },
+      }),
+    )
+
+    expect(encryptUtils.decryptObject).toHaveBeenCalledTimes(1)
+    expect(encryptUtils.decryptObject).toHaveBeenCalledWith(
+      { encrypted: true },
+      credentialSchemas.instagram,
+    )
+  })
+
+  test("decrypts a user-scoped row", async () => {
+    encryptUtils.decryptObject.mockResolvedValue({ clientId: "client-2" })
+
+    await expect(
+      (platformCredentialService as never)._decrypt({
+        id: "user-1",
+        userId: "owner-1",
+        type: "messenger",
+        livemode: true,
+        value: { encrypted: true },
+        publicConfig: { clientId: "client-2" },
+        createdAt: new Date("2026-08-13T00:00:00.000Z"),
+        updatedAt: new Date("2026-08-13T00:00:00.000Z"),
+      }),
+    ).resolves.toEqual(
+      expect.objectContaining({
+        id: "user-1",
+        userId: "owner-1",
+        type: "messenger",
+        config: { clientId: "client-2" },
+      }),
+    )
+
+    expect(encryptUtils.decryptObject).toHaveBeenCalledTimes(1)
+    expect(encryptUtils.decryptObject).toHaveBeenCalledWith(
+      { encrypted: true },
+      credentialSchemas.messenger,
+    )
+  })
+})
+
+// Fakes a minimal chainable drizzle-style tx sufficient for
+// upsertForUser/upsertPlatform's `.insert().values().onConflictDoUpdate()`
+// call shape, without needing a real DB client.
+const fakeTx = () => {
+  const tx = {
+    insert: vi.fn(() => tx),
+    values: vi.fn(() => tx),
+    onConflictDoUpdate: vi.fn(() => Promise.resolve()),
+  }
+  return tx as unknown as Parameters<
+    typeof platformCredentialService.upsertForUser
+  >[0]["tx"]
+}
+
+describe("upsertForUser / upsertPlatform write path", () => {
+  // Writers still stamp a row-derived aad at encrypt time — only decrypt
+  // stopped taking one. These assertions lock in that the derivation
+  // (user:<id>:<type>:<livemode> / platform:<type>:<livemode>) still reaches
+  // encryptObject unchanged.
+  test("upsertForUser encrypts with a row-derived aad", async () => {
+    encryptUtils.encryptObject.mockResolvedValue({
+      v: 1,
+      iv: "iv",
+      text: "text",
+      tag: "tag",
+      aad: "user:owner-1:messenger:false",
+    })
+    vi.spyOn(
+      platformCredentialService,
+      "invalidateCacheTags",
+    ).mockResolvedValue(undefined)
+
+    await platformCredentialService.upsertForUser({
+      userId: "owner-1",
+      type: "messenger",
+      config: { clientId: "c", clientSecret: "s" } as never,
+      tx: fakeTx(),
+    })
+
+    expect(encryptUtils.encryptObject).toHaveBeenCalledWith(
+      { clientId: "c", clientSecret: "s" },
+      "user:owner-1:messenger:false",
+    )
+  })
+
+  test("upsertPlatform encrypts with a platform-scoped aad", async () => {
+    encryptUtils.encryptObject.mockResolvedValue({
+      v: 1,
+      iv: "iv",
+      text: "text",
+      tag: "tag",
+      aad: "platform:messenger:false",
+    })
+    vi.spyOn(
+      platformCredentialService,
+      "invalidateCacheTags",
+    ).mockResolvedValue(undefined)
+
+    await platformCredentialService.upsertPlatform({
+      type: "messenger",
+      config: { clientId: "c", clientSecret: "s" } as never,
+      tx: fakeTx(),
+    })
+
+    expect(encryptUtils.encryptObject).toHaveBeenCalledWith(
+      { clientId: "c", clientSecret: "s" },
+      "platform:messenger:false",
+    )
+  })
+})

packages/database/__tests__/credential.test.ts

@@ -1,5 +1,6 @@
 import { describe, expect, test } from "vitest"
 import {
+  credentialEncryptedSchema,
   giphyCredentialUpdateSchema,
   googleCredentialUpdateSchema,
   instagramCredentialUpdateSchema,
@@ -141,3 +142,64 @@ describe("credential update schemas", () => {
     })
   })
 })
+
+describe("credentialEncryptedSchema", () => {
+  const validBlob = {
+    v: 1 as const,
+    iv: "a".repeat(24),
+    text: "ciphertext-hex",
+    tag: "b".repeat(32),
+    aad: "user:1:messenger:false",
+  }
+
+  test("accepts a blob with a non-empty aad", () => {
+    const result = credentialEncryptedSchema.safeParse(validBlob)
+    expect(result.success).toBe(true)
+  })
+
+  // aad is optional here, matching the transport schema: the writers stamp
+  // it and decrypt reads it back off the blob, so no reader needs to
+  // reconstruct or pass one, and a blob with no aad is a legitimate case
+  // rather than an error.
+  test("accepts a blob missing aad entirely", () => {
+    const { aad: _aad, ...blobWithoutAad } = validBlob
+    const result = credentialEncryptedSchema.safeParse(blobWithoutAad)
+    expect(result.success).toBe(true)
+  })
+
+  test("kid remains optional for legacy blobs predating key versioning", () => {
+    const { kid: _kid, ...blobWithoutKid } = { ...validBlob, kid: "k1" }
+    const result = credentialEncryptedSchema.safeParse(blobWithoutKid)
+    expect(result.success).toBe(true)
+  })
+
+  // These mirror the length/non-empty checks encryptedDataSchema enforces on
+  // the same fields (packages/encryption/src/encryption.ts). credentialEncryptedSchema
+  // is intentionally standalone rather than derived from that schema, so
+  // without these checks a malformed iv/text/tag would pass this parse and
+  // only fail later inside hexToBytes/WebCrypto with an opaque error instead
+  // of a clear validation failure at the storage boundary.
+  test("rejects a blob with a wrong-length iv", () => {
+    const result = credentialEncryptedSchema.safeParse({
+      ...validBlob,
+      iv: "a".repeat(10),
+    })
+    expect(result.success).toBe(false)
+  })
+
+  test("rejects a blob with an empty-string text", () => {
+    const result = credentialEncryptedSchema.safeParse({
+      ...validBlob,
+      text: "",
+    })
+    expect(result.success).toBe(false)
+  })
+
+  test("rejects a blob with a wrong-length tag", () => {
+    const result = credentialEncryptedSchema.safeParse({
+      ...validBlob,
+      tag: "b".repeat(10),
+    })
+    expect(result.success).toBe(false)
+  })
+})

packages/database/scripts/rotate-encryption-key.ts

@@ -41,15 +41,40 @@ const main = async (): Promise<void> => {
 
   const rows = await db.select().from(platformCredentialModel)
 
+  const unparseable: (typeof rows)[number][] = []
   const toRotate = rows.filter((row) => {
     const result = credentialEncryptedSchema.safeParse(row.value)
-    return result.success && result.data.kid !== activeKid
+    if (!result.success) {
+      unparseable.push(row)
+      return false
+    }
+    return result.data.kid !== activeKid
   })
 
   console.log(
     `Found ${toRotate.length} of ${rows.length} rows to rotate → kid="${activeKid}".`,
   )
 
+  if (unparseable.length > 0) {
+    // These rows fail schema validation (e.g. an unexpected `v`, or a
+    // malformed iv/text/tag) and are silently excluded from `toRotate`
+    // above. Left unrotated, they will become
+    // permanently undecryptable once ENCRYPTION_KEY_PREV is removed, with no
+    // other signal that they were ever a problem. Surface them loudly here
+    // instead of letting the exclusion pass unnoticed.
+    console.error(
+      `Warning: ${unparseable.length} row(s) failed schema validation and ` +
+        "will NOT be rotated. They will become undecryptable once " +
+        "ENCRYPTION_KEY_PREV is removed. Investigate before completing " +
+        "rotation:",
+    )
+    for (const row of unparseable) {
+      console.error(
+        `  [id:${row.id} userId:${row.userId ?? "platform"} type:${row.type}]`,
+      )
+    }
+  }
+
   if (isDryRun) {
     console.log("Dry run — no changes written.")
     return
@@ -69,7 +94,7 @@ const main = async (): Promise<void> => {
         CredentialByType[CredentialType]
       >
 
-      const config = await encryptUtils.decryptObject(blob, schema, aad)
+      const config = await encryptUtils.decryptObject(blob, schema)
       const newValue = await encryptUtils.encryptObject(config, aad)
 
       await db

packages/database/src/partials/credential.ts

@@ -351,12 +351,24 @@ export type MakeCredentialUpdate = z.infer<typeof makeCredentialUpdateSchema>
 
 // ─── Encrypted blob shape stored in Credential.value ─────────────────────────
 
+// Storage guard for Credential.value. Deliberately standalone rather than
+// derived from @chatbotx.io/encryption's encryptedDataSchema (even though
+// packages/database already depends on that package for other things), so
+// the field constraints below are mirrored by hand and must be kept in sync
+// with encryptedDataSchema if that shape ever changes.
+//
+// `iv`/`text`/`tag` mirror the length/non-empty checks encryptedDataSchema
+// already enforces, so a malformed row fails loudly here instead of only
+// surfacing later as an opaque hexToBytes/WebCrypto error. `aad` stays
+// optional, matching the transport schema: it is stamped by the writers
+// (upsertForUser, upsertPlatform, rotate-encryption-key.ts) and read back
+// off the blob by decrypt — no caller needs to reconstruct or pass it.
 export const credentialEncryptedSchema = z.object({
   v: z.literal(1),
   kid: z.string().optional(),
-  iv: z.string(),
-  text: z.string(),
-  tag: z.string(),
+  iv: z.string().length(24),
+  text: z.string().min(1),
+  tag: z.string().length(32),
   aad: z.string().optional(),
 })
 export type CredentialEncrypted = z.infer<typeof credentialEncryptedSchema>