Skip to content

decrypt / bulkDecrypt hand back date columns as ISO strings while the model helpers return Date — silently, and not for the stated reason #779

Description

@coderdan

Found while documenting the bulk surface for #754 (skills PR #777). Not a ciphertext or crypto issue — encryption and decryption are correct. This is JS-side type reconstruction.

The behaviour

The same stored value comes back as two different JavaScript types depending on which decrypt method you call:

const events = encryptedTable('events', { createdAt: types.TimestampOrd('created_at') })

// model path → Date
const a = await client.bulkDecryptModels(rows, events)
a.data[0].createdAt instanceof Date        // true

// raw path → ISO string
const b = await client.bulkDecrypt(payloads)
b.data[0].data instanceof Date             // false — it's a string

decrypt (singular) has the same gap.

Why it's easy to get bitten

  • Nothing warns. BulkDecryptedData is Array<DecryptionResult<JsPlaintext | null>> (packages/stack/src/types.ts:424), and JsPlaintext includes string, so the compiler is satisfied either way.
  • It half-works. Comparing two ISO-8601 strings lexicographically gives the right answer, so ordering code passes review and passes tests. It breaks later on arithmetic, .getTime(), or an offset-bearing serialization.
  • Undocumented. Nothing in skills/stash-encryption or the JSDoc says the raw path skips reconstruction.
  • Untested either way. Nothing in packages/stack/__tests__/ asserts a date column's type through bulkDecrypt, so the current behaviour isn't a pinned contract.

The stated reason doesn't hold at runtime

TypedEncryptionClient.decrypt's JSDoc (packages/stack/src/encryption/v3.ts:96-98):

Decrypt a single value. Cannot be strongly typed — a lone ciphertext carries no column identity — so it resolves to the FFI plaintext union unchanged.

That is correct about static types — TypeScript can't know at compile time which column a runtime Encrypted came from. But it has been carried over into the runtime behaviour, where it isn't true: every payload carries its column identity. protect-ffi's EncryptedScalar and EncryptedSteVec both declare (lib/index.d.cts:111-140):

i: { t: string; c: string }   // table, column

It's the same field the Supabase example surfaces as ciphertextIdentifier: { t: "users", c: "email" }. So decrypt / bulkDecrypt have everything needed to look up cast_as for (i.t, i.c) and rebuild the Date — they just don't.

Where the split actually lives

Reconstruction is implemented once per table and keyed by JS property name, which is a model-shaped key:

  • rowReconstructorpackages/stack/src/encryption/v3.ts:150-176
  • applied by decryptModel / bulkDecryptModels via MappedDecryptOperationv3.ts:270-309
  • the raw methods are one-line delegations with no wrapping — v3.ts:311-312, under the comment // Parity passthroughs — not v3-strengthened, delegated as-is. (v3.ts:125)

The WASM entry draws the identical boundary: datePropertyPaths (packages/stack/src/wasm-inline.ts:597-608) is also keyed by JS property name and also feeds the model path only.

So the model/raw split is consistent and evidently deliberate as a boundary. What seems wrong is (a) the justification given for it, which doesn't survive contact with the payload shape, and (b) that the consequence is silent and undocumented.

Suggested fix

Add a (table, column) → isDateLike map alongside the existing per-table one, built from the same DATE_LIKE_CASTS source, and apply it in decrypt / bulkDecrypt by reading each payload's i.t / i.c.

Details worth getting right:

  1. Position stability and the per-item error union. bulkDecrypt returns DecryptionResult<T> = { data } | { error } (types.ts:426-434). Map only the success arm, leave error entries and nulls untouched.
  2. Return type. JsPlaintext deliberately excludes Date (v3.ts:136-137), so the typed client's decrypt / bulkDecrypt signatures need widening to JsPlaintext | Date. That's the only API-surface change.
  3. Unregistered tables. A payload whose i.t isn't in the client's schemas should pass through unchanged rather than fail — a raw decrypt of a foreign payload is legitimate.
  4. Idempotence. new Date(aDate) is a no-op, which the existing reconstructor already relies on (v3.ts:138-139), so callers currently doing new Date(str) themselves keep working. That's what keeps breakage risk low.
  5. Both entries — mirror it in wasm-inline.ts so the two don't diverge.
  6. Round-trip tests asserting instanceof Date through decrypt, bulkDecrypt, decryptModel, and bulkDecryptModels, since none exist today.

Alternative, if the boundary is intended

If raw-path reconstruction is deliberately out of scope, then the fix is documentation rather than code: correct the decrypt JSDoc so it doesn't imply the payload lacks column identity, and state the Date-vs-string consequence in skills/stash-encryption's Bulk Operations section with a pointer to the model helpers. Happy either way — flagging it so it's a decision rather than an accident.

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions