Skip to content

Commit 04f7ac7

Browse files
committed
docs(stack,skills): correct why decrypt/bulkDecrypt skip Date reconstruction
`decryptModel(row, table)` returns a `Date` for a `types.Timestamp` column; `decrypt(payload)` / `bulkDecrypt(payloads)` return the string it was stored as. The split is intentional — the raw methods resolve to the FFI plaintext union, which excludes `Date`, so reconstructing without widening the return type would make the declared type wrong — but it was undocumented, and the JSDoc justified it with a reason that does not hold: that a lone ciphertext "carries no column identity". Every payload carries `i: { t, c }` (protect-ffi's `Identifier`, "shared by every payload"), so the identity is present and simply unused. The real constraint is static typing, not a missing runtime capability. No behaviour change. Every one of the five decrypt paths already agreed with its own declared type; what was wrong was the prose and the silence. - `decrypt` gets the boundary, its consequence, and the actual reason; `bulkDecrypt` gets its first JSDoc. Mirrored on the `wasm-inline` entry so the two don't drift. - The one-arg `decryptModel(row)` / `bulkDecryptModels(rows)` overloads carried the same false reason ("there is no `cast_as` to reconstruct from"). Corrected to name the real one: the `table` argument selects reconstruction, and `Decrypted<T>` types those fields `string` to match. Noted that a registered table's payload COULD resolve the cast — declining to is what keeps runtime and type in agreement. - `skills/stash-encryption` and the package README now state the Date-vs-string consequence and point at the model helpers. - `bulkDecrypt` was the only path untested either way; now pinned, with a payload whose `i: { t, c }` names a registered date-like column, so a future change of heart has to delete the test rather than drift into it. Incidental: the test file's client stubs were cast to `EncryptionClient` where `createEncryptionClient` takes the native client — a type error in all 12 call sites, invisible because `__tests__` is not typechecked in CI. Now derived via `Parameters<typeof createEncryptionClient>[0]`. Resolves #779.
1 parent c8b1325 commit 04f7ac7

6 files changed

Lines changed: 193 additions & 19 deletions

File tree

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
---
2+
'@cipherstash/stack': patch
3+
'stash': patch
4+
---
5+
6+
Document the `Date` reconstruction boundary on `decrypt` / `bulkDecrypt`, and correct the reason given for it.
7+
8+
A `types.Date` / `types.Timestamp` column comes back as a `Date` from `decryptModel(row, table)` and as the string it was stored as from `decrypt(payload)` / `bulkDecrypt(payloads)`. Reconstruction is driven by the table's `cast_as`, and only the model path is handed a table. That split is intentional — the raw methods resolve to the FFI plaintext union, which excludes `Date`, so reconstructing without widening the return type would make the declared type wrong — but it was undocumented, and the JSDoc explained it with a reason that does not hold: that a lone ciphertext "carries no column identity". Every stored payload carries `i: { t, c }`, so the identity is present and simply unused. The real constraint is static typing (TypeScript cannot know which column a runtime payload came from), not a missing capability at runtime.
9+
10+
No behaviour change. What changed:
11+
12+
- `decrypt` and the new `bulkDecrypt` JSDoc state the boundary, its consequence, and the actual reason, on both the native and `wasm-inline` entries.
13+
- The one-arg `decryptModel(row)` / `bulkDecryptModels(rows)` overloads had the same wrong justification ("there is no `cast_as` to reconstruct from"); corrected to name the real one — the `table` argument is what selects reconstruction, and `Decrypted<T>` types those fields `string` to match.
14+
- `skills/stash-encryption` and the `@cipherstash/stack` README now call out the `Date`-vs-string consequence and point at the model helpers.
15+
- `bulkDecrypt` was the one path with no test either way; it is now pinned, using a payload whose `i: { t, c }` names a registered date-like column, so a future change of heart is a deliberate decision rather than a silent drift.
16+
17+
Resolves #779.

‎packages/stack/README.md‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -736,7 +736,9 @@ Method signatures are derived from your schemas: plaintext arguments are pinned
736736
| `bulkDecrypt` | `(encryptedPayloads)` | `BulkDecryptOperation` (thenable) |
737737
| `getEncryptConfig` | `()` | The resolved encrypt config |
738738

739-
The thenable operations support `.withLockContext(lockContext)` for identity-aware encryption, and `decryptModel` / `bulkDecryptModels` also support `.audit({ metadata })`. Those two additionally accept the lock context as an optional third argument — use one form or the other. `decrypt` of a single value cannot be strongly typed (a lone ciphertext carries no column identity), and `encryptQuery` rejects storage-only columns at compile time.
739+
The thenable operations support `.withLockContext(lockContext)` for identity-aware encryption, and `decryptModel` / `bulkDecryptModels` also support `.audit({ metadata })`. Those two additionally accept the lock context as an optional third argument — use one form or the other. `decrypt` of a single value cannot be strongly typed (TypeScript cannot know which column a runtime payload came from), and `encryptQuery` rejects storage-only columns at compile time.
740+
741+
**`decrypt` / `bulkDecrypt` do not reconstruct `Date` values.** A `types.Date` / `types.Timestamp` column read through the raw path comes back as the string it was stored as; read through `decryptModel` / `bulkDecryptModels` with the table, it comes back as a `Date`. Reconstruction is driven by the table's `cast_as`, which only the model path is handed. Use the model helpers when you want the column's declared plaintext type, or rebuild at the call site with `new Date(value)`.
740742

741743
### `LockContext` (legacy)
742744

‎packages/stack/__tests__/typed-client-v3.test.ts‎

Lines changed: 103 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,19 @@
11
import { describe, expect, it } from 'vitest'
2-
import type { EncryptionClient } from '@/encryption'
32
import { createEncryptionClient } from '@/encryption/client-v3'
43
import { encryptedTable, types } from '@/eql/v3'
4+
import type { Encrypted } from '@/types'
5+
6+
/**
7+
* What `createEncryptionClient` wraps. Derived from the factory rather than
8+
* imported, because `UnderlyingNativeClient` is deliberately module-private —
9+
* naming it here is the only thing a test needs from it, and `Parameters` gets
10+
* that without widening the module's export surface.
11+
*
12+
* The stubs below are cast to this, not to `EncryptionClient`: the wrapper takes
13+
* the NATIVE client, and casting to the public client type instead was a type
14+
* error in every call (invisible, since `__tests__` is not typechecked in CI).
15+
*/
16+
type NativeClientStub = Parameters<typeof createEncryptionClient>[0]
517

618
const table = encryptedTable('t', {
719
when: types.Timestamp('when'),
@@ -34,11 +46,11 @@ function fakeOp<R>(result: R) {
3446
* A minimal client stub whose model-decrypt methods return an operation
3547
* resolving to a fixed `Result` payload.
3648
*/
37-
function fakeClient(data: Record<string, unknown>): EncryptionClient {
49+
function fakeClient(data: Record<string, unknown>): NativeClientStub {
3850
return {
3951
decryptModel: () => fakeOp({ data }),
4052
bulkDecryptModels: () => fakeOp({ data: [data] }),
41-
} as unknown as EncryptionClient
53+
} as unknown as NativeClientStub
4254
}
4355

4456
describe('createEncryptionClient — decrypt reconstruction', () => {
@@ -141,7 +153,7 @@ describe('createEncryptionClient — decrypt reconstruction', () => {
141153
fakeOp({
142154
failure: { type: 'DecryptionError', message: 'boom' },
143155
}),
144-
} as unknown as EncryptionClient
156+
} as unknown as NativeClientStub
145157

146158
const client = createEncryptionClient(failing, table)
147159
const result = await client.decryptModel({}, table)
@@ -232,3 +244,90 @@ describe('createEncryptionClient — decrypt reconstruction', () => {
232244
expect(rows[0].when).toBe('2021-06-01T00:00:00.000Z')
233245
})
234246
})
247+
248+
/**
249+
* The raw-path boundary, pinned rather than asserted away (#779).
250+
*
251+
* `decrypt` / `bulkDecrypt` are bare delegations — the typed wrapper adds no
252+
* `Date` reconstruction — so a `timestamp` column read this way is the string it
253+
* was stored as, where `decryptModel(row, table)` above turns the same value
254+
* into a `Date`. That split is deliberate: the raw methods resolve to the FFI
255+
* plaintext union, which excludes `Date`, and reconstructing without widening
256+
* it would make the declared type a lie.
257+
*
258+
* What it is NOT is a missing capability, which is the claim the JSDoc used to
259+
* make. The payloads below carry `i: { t, c }` naming a REGISTERED date-like
260+
* column — the identity needed to resolve `cast_as` is right there and
261+
* deliberately unused. Pinning it here means a future change of heart has to
262+
* delete this test, which is the point: the integration suite covers the
263+
* single-value half end-to-end (`integration/shared/v2-decrypt-compat`), but
264+
* nothing covered the bulk half, and that gap is what let the split read as an
265+
* accident.
266+
*/
267+
describe('createEncryptionClient — raw decrypt paths are unmapped', () => {
268+
const storedIso = '2020-01-02T03:04:05.000Z'
269+
// Shaped like a real storage payload: `i` names `t`'s `when` column, which
270+
// the table above declares `types.Timestamp` (`cast_as: 'timestamp'`). Typed
271+
// as `Encrypted` so both calls below go through the PUBLIC signature — the
272+
// arity a caller actually reaches, unlike the one-arg model tests above.
273+
const payload: Encrypted = {
274+
k: 'ct',
275+
v: 2,
276+
i: { t: 't', c: 'when' },
277+
c: 'ciphertext',
278+
}
279+
280+
/**
281+
* Stubs only the two raw methods. They are returned by the wrapper unchanged,
282+
* so — unlike the model paths, which get wrapped in a `MappedDecryptOperation`
283+
* that calls `.execute()` — the stub is awaited directly and can simply be a
284+
* promise of the `Result`.
285+
*/
286+
function rawFakeClient() {
287+
const forwarded: unknown[] = []
288+
const client = {
289+
decrypt: (encrypted: unknown) => {
290+
forwarded.push(encrypted)
291+
return Promise.resolve({ data: storedIso })
292+
},
293+
bulkDecrypt: (payloads: unknown) => {
294+
forwarded.push(payloads)
295+
return Promise.resolve({
296+
data: [{ id: 'u1', data: storedIso }],
297+
})
298+
},
299+
} as unknown as NativeClientStub
300+
return { client, forwarded }
301+
}
302+
303+
it('returns a date-like column as its stored string from decrypt', async () => {
304+
const { client: underlying, forwarded } = rawFakeClient()
305+
const client = createEncryptionClient(underlying, table)
306+
307+
const result = await client.decrypt(payload)
308+
expect(result.failure).toBeFalsy()
309+
if (result.failure) return
310+
311+
expect(result.data).toBe(storedIso)
312+
expect(result.data).not.toBeInstanceOf(Date)
313+
// Bare delegation: the payload reaches the underlying client untouched.
314+
expect(forwarded).toEqual([payload])
315+
})
316+
317+
it('returns date-like columns as stored strings from bulkDecrypt', async () => {
318+
const { client: underlying, forwarded } = rawFakeClient()
319+
const client = createEncryptionClient(underlying, table)
320+
321+
const result = await client.bulkDecrypt([{ id: 'u1', data: payload }])
322+
expect(result.failure).toBeFalsy()
323+
if (result.failure) return
324+
325+
expect(result.data).toHaveLength(1)
326+
const [first] = result.data
327+
expect(first.data).toBe(storedIso)
328+
expect(first.data).not.toBeInstanceOf(Date)
329+
// Position-stable identifiers survive the (absent) mapping too.
330+
expect(first.id).toBe('u1')
331+
expect(forwarded).toEqual([[{ id: 'u1', data: payload }]])
332+
})
333+
})

‎packages/stack/src/encryption/client-v3.ts‎

Lines changed: 46 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -190,8 +190,25 @@ export interface EncryptionClient<
190190
): BulkEncryptModelsOperation<V3EncryptedModel<Table, T>>
191191

192192
/**
193-
* Decrypt a single value. Cannot be strongly typed — a lone ciphertext carries
194-
* no column identity — so it resolves to the FFI plaintext union unchanged.
193+
* Decrypt a single stored payload, resolving to the FFI plaintext union
194+
* (`JsPlaintext`) unchanged.
195+
*
196+
* **A `date` / `timestamp` column comes back as the string it was stored as,
197+
* not a `Date`** — where `decryptModel(row, table)` reconstructs it. Same
198+
* value, two JavaScript types, depending on which method read it (#779).
199+
*
200+
* The scalar form cannot be strongly *typed*: TypeScript has no way to know
201+
* which column a runtime `Encrypted` came from, so the static type has to be
202+
* the whole union. That is a statement about the type layer only. At runtime
203+
* every payload carries its own `i: { t, c }` table/column identity, so the
204+
* `cast_as` is perfectly reachable from here — skipping reconstruction is a
205+
* deliberate contract line, not a missing capability. `JsPlaintext` excludes
206+
* `Date` by construction, and that is the type callers have annotated
207+
* against; reconstructing without widening it would make the type a lie.
208+
*
209+
* For `Date` values, read through {@link decryptModel} /
210+
* {@link bulkDecryptModels} with the table, or rebuild at the call site
211+
* (`new Date(value)`).
195212
*/
196213
decrypt(encrypted: Encrypted): DecryptOperation
197214

@@ -224,8 +241,12 @@ export interface EncryptionClient<
224241

225242
/**
226243
* Table-less form: decrypt whatever encrypted fields the model carries, with
227-
* no `Date` reconstruction (there is no `cast_as` to reconstruct from) and no
228-
* precise plaintext shape.
244+
* no `Date` reconstruction and no precise plaintext shape. The `table`
245+
* argument is what selects the reconstruction map, and `Decrypted<T>` types
246+
* every decrypted field as `string` to match. (For a table that IS
247+
* registered, the payload's own `i: { t, c }` would resolve the `cast_as` —
248+
* declining to use it is what keeps this arity's runtime agreeing with its
249+
* declared type. #779.)
229250
*
230251
* This is the read path for rows that predate this client's schemas — legacy
231252
* **EQL v2** models above all, whose table is not, and cannot be, a member of
@@ -258,6 +279,16 @@ export interface EncryptionClient<
258279
plaintexts: BulkEncryptPayloadFor<Col>,
259280
opts: { table: Table; column: Col },
260281
): BulkEncryptOperation
282+
283+
/**
284+
* Decrypt many stored payloads in one ZeroKMS round trip, position-stable
285+
* with a per-item `{ data } | { error }` entry.
286+
*
287+
* Draws the same boundary as {@link decrypt}, for the same reason: **no
288+
* `Date` reconstruction** — date-like columns arrive as their stored strings.
289+
* Prefer {@link bulkDecryptModels} with the table when you want the column's
290+
* declared plaintext type back (#779).
291+
*/
261292
bulkDecrypt(payloads: BulkDecryptPayload): BulkDecryptOperation
262293
getEncryptConfig(): ReturnType<UnderlyingNativeClient['getEncryptConfig']>
263294
}
@@ -342,12 +373,17 @@ export function createEncryptionClient<const S extends readonly AnyV3Table[]>(
342373
}
343374

344375
// Pass-through maps for the table-less one-arg decrypt call, where `table` is
345-
// absent: decrypt WITHOUT date reconstruction, because with no table there is
346-
// no `cast_as` to reconstruct from. This client is what `Encryption` returns
347-
// for every v3 schema set, so generic consumers — and the legacy EQL v2 read
348-
// path, whose table is not in `S` — can call `decryptModel(x)` /
349-
// `bulkDecryptModels(xs)` with no table. Degrade gracefully instead of
350-
// dereferencing `undefined.tableName`.
376+
// absent: decrypt WITHOUT date reconstruction. The `table` argument is what
377+
// selects a reconstructor, and the one-arg overload's `Decrypted<T>` return
378+
// type declares every decrypted field `string` to match. Note what is NOT the
379+
// reason: a payload from a registered table carries its own `i: { t, c }`, so
380+
// the `cast_as` could be resolved here (#779). Declining to is what keeps
381+
// this arity's runtime agreeing with its declared type — reconstructing under
382+
// a `string` type would trade a documented split for a lying one.
383+
// This client is what `Encryption` returns for every v3 schema set, so
384+
// generic consumers — and the legacy EQL v2 read path, whose table is not in
385+
// `S` — can call `decryptModel(x)` / `bulkDecryptModels(xs)` with no table.
386+
// Degrade gracefully instead of dereferencing `undefined.tableName`.
351387
const passthroughRow = (row: Record<string, unknown>) => row
352388
const passthroughRows = (rows: Array<Record<string, unknown>>) => rows
353389

‎packages/stack/src/wasm-inline.ts‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -819,6 +819,18 @@ export class WasmEncryptionClient {
819819
}, EncryptionErrorTypes.EncryptionError)
820820
}
821821

822+
/**
823+
* Decrypt a single stored payload, resolving to the plaintext union unchanged.
824+
*
825+
* **A `date` / `timestamp` column comes back as the string it was stored as,
826+
* not a `Date`** — where `decryptModel(row, table)` reconstructs it via
827+
* {@link datePropertyPaths}. Same boundary the native client draws, and for
828+
* the same reason: reconstruction is a property of the MODEL path, which is
829+
* handed a table to read `cast_as` from. Not because the identity is missing
830+
* — every payload carries `i: { t, c }` — but because this method's declared
831+
* plaintext union excludes `Date`, and returning one would make the type a
832+
* lie (#779). Read through the model helpers, or `new Date(value)` yourself.
833+
*/
822834
async decrypt(encrypted: Encrypted): Promise<WasmResult<WasmPlaintext>> {
823835
return wasmResult(
824836
async () =>
@@ -1074,6 +1086,12 @@ export class WasmEncryptionClient {
10741086
* worth considering if callers ask for it — but it is a different return
10751087
* type from every other method here, so it is not the default.)
10761088
*
1089+
* ## No date reconstruction
1090+
*
1091+
* Same boundary as {@link decrypt}: date-like columns arrive as their stored
1092+
* strings, not `Date` values. Prefer {@link bulkDecryptModels} with the table
1093+
* when you want the column's declared plaintext type back (#779).
1094+
*
10771095
* @example
10781096
* ```ts
10791097
* const rows = await sql`SELECT email FROM users LIMIT 50`

‎skills/stash-encryption/SKILL.md‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -364,7 +364,9 @@ if (!decrypted.failure) {
364364
}
365365
```
366366

367-
`decrypt` of a single value cannot be strongly typed — a lone ciphertext carries no column identity. All plaintext values passed to `encrypt` must be non-null; null handling is managed at the model level by `encryptModel` and `decryptModel`.
367+
`decrypt` of a single value cannot be strongly typed — TypeScript cannot know which column a runtime payload came from, so the result is the whole plaintext union. All plaintext values passed to `encrypt` must be non-null; null handling is managed at the model level by `encryptModel` and `decryptModel`.
368+
369+
> **`decrypt` / `bulkDecrypt` hand back date columns as strings, not `Date`.** Reconstruction is driven by the table's `cast_as`, and only the model path is given a table — so the same stored value comes back as a `Date` from `decryptModel(row, table)` and as its stored ISO string from `decrypt(payload)`. Nothing warns: the raw path's declared type is the plaintext union, which includes `string`. Comparing two ISO strings orders correctly, so this survives review and breaks later on `.getTime()` or date arithmetic. Use the model helpers when you want the column's declared plaintext type, or rebuild at the call site (`new Date(value)`). Same on the WASM entry, and same for the one-arg `decryptModel(row)` / `bulkDecryptModels(rows)` forms, which take no table.
368370
369371
## Model Operations
370372

@@ -414,7 +416,7 @@ const decrypted = await client.bulkDecryptModels(encrypted.data, users)
414416

415417
### Bulk Encrypt / Decrypt (Raw Values)
416418

417-
`bulkEncrypt` / `bulkDecrypt` are parity passthroughs (not v3-strengthened):
419+
`bulkEncrypt` / `bulkDecrypt` are parity passthroughs (not v3-strengthened), which includes **no `Date` reconstruction** — a `types.Timestamp` column read this way is the stored ISO string, where `bulkDecryptModels(rows, table)` gives you a `Date`:
418420

419421
```typescript
420422
const plaintexts = [
@@ -986,15 +988,15 @@ Useful when the backfill needs to run in a worker, on a schedule, or alongside a
986988
| Method | Signature | Returns |
987989
|---|---|---|
988990
| `encrypt` | `(plaintext, { table, column })` — plaintext pinned to the column's domain type | `EncryptOperation` |
989-
| `decrypt` | `(encryptedData)` — untyped (no column identity) | `DecryptOperation` |
991+
| `decrypt` | `(encryptedData)` — untyped (the column is not known statically); no `Date` reconstruction | `DecryptOperation` |
990992
| `encryptQuery` | `(plaintext, { table, column, queryType?, returnType? })` — queryable columns only; `queryType` constrained to the column's capabilities | `EncryptQueryOperation` |
991993
| `encryptQuery` | `(terms: readonly ScalarQueryTerm[])` — batch form | `BatchEncryptQueryOperation` |
992994
| `encryptModel` | `(model, table)` — schema fields validated against inferred plaintext types | `EncryptModelOperation<V3EncryptedModel<Table, T>>` |
993995
| `decryptModel` | `(model, table, lockContext?)` | `AuditableDecryptModelOperation<V3DecryptedModel<Table, T>>` |
994996
| `bulkEncryptModels` | `(models, table)` | `BulkEncryptModelsOperation<V3EncryptedModel<Table, T>>` |
995997
| `bulkDecryptModels` | `(models, table, lockContext?)` | `AuditableDecryptModelOperation<V3DecryptedModel<Table, T>[]>` |
996998
| `bulkEncrypt` | `(plaintexts, { column, table })` — parity passthrough | `BulkEncryptOperation` |
997-
| `bulkDecrypt` | `(encryptedPayloads)` — parity passthrough | `BulkDecryptOperation` |
999+
| `bulkDecrypt` | `(encryptedPayloads)` — parity passthrough; no `Date` reconstruction | `BulkDecryptOperation` |
9981000
| `getEncryptConfig` | `()` | The client's encrypt config |
9991001

10001002
All of these operations are thenable (awaitable) and support `.withLockContext()` and `.audit()` chaining — including `decryptModel`/`bulkDecryptModels`, which also accept the lock context as a third argument. Use one or the other: chaining `.withLockContext()` onto a decrypt that already took a positional lock context throws.

0 commit comments

Comments
 (0)