Skip to content

Commit 712d7fa

Browse files
committed
fix(stack): restore runtime null guards in encryption operations
Commit 5b7288b (feat(stack): remove null from Encrypted type) tightened the public types to disallow null in single-value encrypt/decrypt, and deleted the matching runtime `if (value === null) return null` guards across every operation in packages/stack/src/encryption/operations/. The type narrowing does not survive runtime. Callers that reach an operation through a cast (e.g. `null as any`), dynamic model field walking, or JS interop will silently have their null encrypted by protect-ffi into a real SteVec ciphertext (`{ k: 'sv', v: 2, ... }`), breaking symmetry with the model-helpers layer where null is treated as "absent" at the field level. The newly-ported searchable-json `round- trips null values` test in PR #328 exercises this path directly and surfaced the regression. Restored guards mirror the @cipherstash/protect pattern: - encrypt / EncryptOperationWithLockContext: early return null - bulkEncrypt / *WithLockContext: filter-null + position-preserving merge - decrypt / *WithLockContext: early return null - bulkDecrypt / *WithLockContext: filter-null + position-preserving merge - encryptQuery / *WithLockContext: return { data: null } for null/undefined - batchEncryptQuery / *WithLockContext: per-element filter, position-stable Internal operation field/return types widen to `T | null` so the restored guards compile. Public bulk types (`BulkEncryptPayload`, `BulkEncryptedData`, `BulkDecryptPayload`, `BulkDecryptedData`) and `EncryptedQueryResult` widen to admit null in element positions — these now honestly reflect runtime behavior and let callers process mixed nullable arrays without filtering ahead of time. `Encryption.decrypt()` accepts `Encrypted | null`. `Encryption.encrypt()`'s public signature stays narrow (`JsPlaintext`); the runtime guard is defense in depth.
1 parent a087730 commit 712d7fa

9 files changed

Lines changed: 248 additions & 83 deletions

File tree

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
---
2+
"@cipherstash/stack": patch
3+
---
4+
5+
Fix: restore runtime null short-circuits in the encryption operation classes.
6+
7+
A prior refactor (`feat(stack): remove null from Encrypted type`) tightened the type signatures to disallow `null` and, alongside that, deleted the `if (value === null) return null` guards from every operation in `packages/stack/src/encryption/operations/`. The type guard does not survive runtime: callers reaching the operation through a cast (e.g. `null as any`), dynamic model walking, or JS interop would then have their null silently encrypted by protect-ffi into a real SteVec ciphertext (`{ k: 'sv', v: 2, ... }`) — which is observable, surprising, and breaks symmetry with the model-helpers layer that does still treat null as "absent" at the field level.
8+
9+
Restored, mirroring the pattern in `@cipherstash/protect`:
10+
11+
- `encrypt` / `encryptWithLockContext`: `if (plaintext === null) return null`.
12+
- `bulkEncrypt` / `bulkEncryptWithLockContext`: per-element null filter; nulls are preserved in position in the output.
13+
- `decrypt` / `decryptWithLockContext`: `if (encryptedData === null) return null`.
14+
- `bulkDecrypt` / `bulkDecryptWithLockContext`: per-element null filter, position-preserving merge.
15+
- `encryptQuery` / `encryptQueryWithLockContext`: `if (plaintext === null || plaintext === undefined) return { data: null }`.
16+
- `batchEncryptQuery` / `batchEncryptQueryWithLockContext`: per-element null/undefined filter; null slots in the input array stay null in the result array.
17+
18+
Type adjustments to support the runtime behavior honestly:
19+
20+
- `BulkEncryptPayload['plaintext']`, `BulkEncryptedData['data']`, `BulkDecryptPayload['data']`, and the `T` of `BulkDecryptedData` all widen to `... | null`. Bulk APIs now accept and return mixed nullable arrays without filtering ahead of time.
21+
- `EncryptedQueryResult` widens to include `null` so the batch query path can return position-stable arrays with null slots.
22+
- `Encryption.decrypt()` accepts `Encrypted | null` (returns null for null input).
23+
- `Encryption.encrypt()`'s public signature is unchanged — still `JsPlaintext` (no null). The runtime guard is defense in depth for the cases the type system can't catch.

packages/stack/src/encryption/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -311,7 +311,7 @@ export class EncryptionClient {
311311
* @see {@link LockContext}
312312
* @see {@link DecryptOperation}
313313
*/
314-
decrypt(encryptedData: Encrypted): DecryptOperation {
314+
decrypt(encryptedData: Encrypted | null): DecryptOperation {
315315
return new DecryptOperation(this.client, encryptedData)
316316
}
317317

packages/stack/src/encryption/operations/batch-encrypt-query.ts

Lines changed: 49 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import type { Client, EncryptedQueryResult, ScalarQueryTerm } from '@/types'
66
import { createRequestLogger } from '@/utils/logger'
77
import { type Result, withResult } from '@byteslice/result'
88
import {
9+
type JsPlaintext,
910
type QueryPayload,
1011
encryptQueryBulk as ffiEncryptQueryBulk,
1112
} from '@cipherstash/protect-ffi'
@@ -21,6 +22,21 @@ import {
2122
import { noClientError } from '../index'
2223
import { EncryptionOperation } from './base-operation'
2324

25+
// Separates null/undefined values from non-null terms in the input array
26+
// so they bypass the FFI call. Original indices are tracked so the
27+
// reassembled result preserves position.
28+
function filterNullTerms(terms: readonly ScalarQueryTerm[]): {
29+
nonNullTerms: { term: ScalarQueryTerm; originalIndex: number }[]
30+
} {
31+
const nonNullTerms: { term: ScalarQueryTerm; originalIndex: number }[] = []
32+
terms.forEach((term, index) => {
33+
if (term.value !== null && term.value !== undefined) {
34+
nonNullTerms.push({ term, originalIndex: index })
35+
}
36+
})
37+
return { nonNullTerms }
38+
}
39+
2440
/**
2541
* Validates and transforms a single term into a QueryPayload.
2642
* Throws an error if the value is NaN or Infinity.
@@ -42,7 +58,7 @@ function buildQueryPayload(
4258
assertValueIndexCompatibility(term.value, indexType, term.column.getName())
4359

4460
const payload: QueryPayload = {
45-
plaintext: term.value,
61+
plaintext: term.value as JsPlaintext,
4662
column: term.column.getName(),
4763
table: term.table.tableName,
4864
indexType,
@@ -57,15 +73,22 @@ function buildQueryPayload(
5773
}
5874

5975
/**
60-
* Maps encrypted values to formatted results based on term.returnType.
76+
* Reassembles the result array, slotting null at the original indices of
77+
* filtered-out terms and formatting encrypted values for non-null terms.
6178
*/
6279
function assembleResults(
63-
terms: readonly ScalarQueryTerm[],
80+
totalLength: number,
6481
encryptedValues: (CipherStashEncrypted | CipherStashEncryptedQuery)[],
82+
nonNullTerms: { term: ScalarQueryTerm; originalIndex: number }[],
6583
): EncryptedQueryResult[] {
66-
return terms.map((term, i) =>
67-
formatEncryptedResult(encryptedValues[i], term.returnType),
68-
)
84+
const results: EncryptedQueryResult[] = new Array(totalLength).fill(null)
85+
nonNullTerms.forEach(({ term, originalIndex }, i) => {
86+
results[originalIndex] = formatEncryptedResult(
87+
encryptedValues[i],
88+
term.returnType,
89+
)
90+
})
91+
return results
6992
}
7093

7194
/**
@@ -107,13 +130,20 @@ export class BatchEncryptQueryOperation extends EncryptionOperation<
107130
return { data: [] }
108131
}
109132

133+
const { nonNullTerms } = filterNullTerms(this.terms)
134+
135+
if (nonNullTerms.length === 0) {
136+
log.emit()
137+
return { data: this.terms.map(() => null) }
138+
}
139+
110140
const result = await withResult(
111141
async () => {
112142
if (!this.client) throw noClientError()
113143

114144
const { metadata } = this.getAuditData()
115145

116-
const queries: QueryPayload[] = this.terms.map((term) =>
146+
const queries: QueryPayload[] = nonNullTerms.map(({ term }) =>
117147
buildQueryPayload(term),
118148
)
119149

@@ -122,7 +152,7 @@ export class BatchEncryptQueryOperation extends EncryptionOperation<
122152
unverifiedContext: metadata,
123153
})
124154

125-
return assembleResults(this.terms, encrypted)
155+
return assembleResults(this.terms.length, encrypted, nonNullTerms)
126156
},
127157
(error: unknown) => {
128158
log.set({ errorCode: getErrorCode(error) ?? 'unknown' })
@@ -169,6 +199,15 @@ export class BatchEncryptQueryOperationWithLockContext extends EncryptionOperati
169199
return { data: [] }
170200
}
171201

202+
// Check for all-null terms BEFORE fetching lockContext to avoid an
203+
// unnecessary network call.
204+
const { nonNullTerms } = filterNullTerms(this.terms)
205+
206+
if (nonNullTerms.length === 0) {
207+
log.emit()
208+
return { data: this.terms.map(() => null) }
209+
}
210+
172211
const lockContextResult = await this.lockContext.getLockContext()
173212
if (lockContextResult.failure) {
174213
log.emit()
@@ -183,7 +222,7 @@ export class BatchEncryptQueryOperationWithLockContext extends EncryptionOperati
183222

184223
const { metadata } = this.getAuditData()
185224

186-
const queries: QueryPayload[] = this.terms.map((term) =>
225+
const queries: QueryPayload[] = nonNullTerms.map(({ term }) =>
187226
buildQueryPayload(term, context),
188227
)
189228

@@ -193,7 +232,7 @@ export class BatchEncryptQueryOperationWithLockContext extends EncryptionOperati
193232
unverifiedContext: metadata,
194233
})
195234

196-
return assembleResults(this.terms, encrypted)
235+
return assembleResults(this.terms.length, encrypted, nonNullTerms)
197236
},
198237
(error: unknown) => {
199238
log.set({ errorCode: getErrorCode(error) ?? 'unknown' })

packages/stack/src/encryption/operations/bulk-decrypt.ts

Lines changed: 41 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -5,40 +5,53 @@ import type { BulkDecryptPayload, BulkDecryptedData, Client } from '@/types'
55
import { createRequestLogger } from '@/utils/logger'
66
import { type Result, withResult } from '@byteslice/result'
77
import {
8+
type Encrypted as CipherStashEncrypted,
89
type DecryptResult,
910
decryptBulkFallible,
1011
} from '@cipherstash/protect-ffi'
1112
import { noClientError } from '../index'
1213
import { EncryptionOperation } from './base-operation'
1314

14-
// Helper functions for better composability
15+
// Drops nulls so they don't reach protect-ffi's bulk decrypt. The
16+
// dropped positions are re-inserted as null in `mapDecryptedDataToResult`.
1517
const createDecryptPayloads = (
1618
encryptedPayloads: BulkDecryptPayload,
1719
lockContext?: Context,
1820
) => {
19-
return encryptedPayloads.map(({ id, data }) => ({
20-
id,
21-
ciphertext: data,
22-
...(lockContext && { lockContext }),
23-
}))
21+
return encryptedPayloads
22+
.filter(({ data }) => data !== null)
23+
.map(({ id, data }) => ({
24+
id,
25+
ciphertext: data as CipherStashEncrypted,
26+
...(lockContext && { lockContext }),
27+
}))
2428
}
2529

30+
const createNullResult = (
31+
encryptedPayloads: BulkDecryptPayload,
32+
): BulkDecryptedData =>
33+
encryptedPayloads.map(({ id }) => ({ id, data: null }))
34+
2635
const mapDecryptedDataToResult = (
2736
encryptedPayloads: BulkDecryptPayload,
2837
decryptedData: DecryptResult[],
2938
): BulkDecryptedData => {
30-
return decryptedData.map((decryptResult, i) => {
31-
if ('error' in decryptResult) {
32-
return {
33-
id: encryptedPayloads[i].id,
34-
error: decryptResult.error,
39+
const result: BulkDecryptedData = new Array(encryptedPayloads.length)
40+
let decryptedIndex = 0
41+
for (let i = 0; i < encryptedPayloads.length; i++) {
42+
if (encryptedPayloads[i].data === null) {
43+
result[i] = { id: encryptedPayloads[i].id, data: null }
44+
} else {
45+
const decryptResult = decryptedData[decryptedIndex]
46+
if ('error' in decryptResult) {
47+
result[i] = { id: encryptedPayloads[i].id, error: decryptResult.error }
48+
} else {
49+
result[i] = { id: encryptedPayloads[i].id, data: decryptResult.data }
3550
}
51+
decryptedIndex++
3652
}
37-
return {
38-
id: encryptedPayloads[i].id,
39-
data: decryptResult.data,
40-
}
41-
})
53+
}
54+
return result
4255
}
4356

4457
export class BulkDecryptOperation extends EncryptionOperation<BulkDecryptedData> {
@@ -71,12 +84,16 @@ export class BulkDecryptOperation extends EncryptionOperation<BulkDecryptedData>
7184
if (!this.encryptedPayloads || this.encryptedPayloads.length === 0)
7285
return []
7386

74-
const payloads = createDecryptPayloads(this.encryptedPayloads)
87+
const nonNullPayloads = createDecryptPayloads(this.encryptedPayloads)
88+
89+
if (nonNullPayloads.length === 0) {
90+
return createNullResult(this.encryptedPayloads)
91+
}
7592

7693
const { metadata } = this.getAuditData()
7794

7895
const decryptedData = await decryptBulkFallible(this.client, {
79-
ciphertexts: payloads,
96+
ciphertexts: nonNullPayloads,
8097
unverifiedContext: metadata,
8198
})
8299

@@ -140,15 +157,19 @@ export class BulkDecryptOperationWithLockContext extends EncryptionOperation<Bul
140157
throw new Error(`[encryption]: ${context.failure.message}`)
141158
}
142159

143-
const payloads = createDecryptPayloads(
160+
const nonNullPayloads = createDecryptPayloads(
144161
encryptedPayloads,
145162
context.data.context,
146163
)
147164

165+
if (nonNullPayloads.length === 0) {
166+
return createNullResult(encryptedPayloads)
167+
}
168+
148169
const { metadata } = this.getAuditData()
149170

150171
const decryptedData = await decryptBulkFallible(client, {
151-
ciphertexts: payloads,
172+
ciphertexts: nonNullPayloads,
152173
serviceToken: context.data.ctsToken,
153174
unverifiedContext: metadata,
154175
})

0 commit comments

Comments
 (0)