Skip to content

Commit f0ccd7b

Browse files
committed
fix(stack): address Copilot review on null guard restoration
- Keep public encrypt() / decrypt() return types narrow. The runtime guards now cast `null as unknown as Encrypted` / `JsPlaintext` at the short-circuit site so callers who respect the input contract get a non-nullable `data` field as before. The cast acknowledges that the null branch is unreachable by the public type — defense in depth only trips on direct construction with a cast plaintext / ciphertext. - Public `Encryption.decrypt()` reverts to `decrypt(encryptedData: Encrypted)` (was widened to `Encrypted | null`). JSDoc gets a `@remarks` block documenting the runtime null short-circuit so the defense-in-depth behavior is discoverable. - encrypt-query operations no longer carry a redundant `| null` in their generic / execute return — `EncryptedQueryResult` already includes null, so the union was double-nullable. - encrypt-query: capture a local `const plaintext: JsPlaintext` after the null/undefined guard so the three `as JsPlaintext` casts inside withResult drop. Same in the WithLockContext variant. - bulk-encrypt: consolidate the two `@/types` import statements. - Add `__tests__/null-guards.test.ts` covering encrypt(null), decrypt(null), bulkEncrypt all-null, bulkDecrypt all-null, encryptQuery(null|undefined), and batchEncryptQuery all-null/undefined. Constructs operation classes directly so the test runs without credentials — the short-circuits return before any FFI call.
1 parent 712d7fa commit f0ccd7b

7 files changed

Lines changed: 157 additions & 36 deletions

File tree

.changeset/restore-null-guards-in-encryption-ops.md

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,5 +19,4 @@ Type adjustments to support the runtime behavior honestly:
1919

2020
- `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.
2121
- `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.
22+
- `Encryption.encrypt()` and `Encryption.decrypt()` public signatures are unchanged — still narrow (`JsPlaintext` / `Encrypted` input, `Encrypted` / `JsPlaintext` non-nullable output). The runtime null short-circuit in `EncryptOperation` / `DecryptOperation` is defense in depth for callers reaching the operation classes through casts, dynamic field walking, or JS interop. The narrow-return contract holds for any caller that respects the input contract.
Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
// Defense-in-depth tests for the runtime null short-circuits restored
2+
// across the encryption operation classes. These hit the operation
3+
// constructors directly rather than going through `Encryption()` so they
4+
// don't need credentials or a network — the guard short-circuits before
5+
// any FFI call. See `fix(stack): restore runtime null guards in
6+
// encryption operations` for context.
7+
8+
import { BatchEncryptQueryOperation } from '@/encryption/operations/batch-encrypt-query'
9+
import { BulkDecryptOperation } from '@/encryption/operations/bulk-decrypt'
10+
import { BulkEncryptOperation } from '@/encryption/operations/bulk-encrypt'
11+
import { DecryptOperation } from '@/encryption/operations/decrypt'
12+
import { EncryptQueryOperation } from '@/encryption/operations/encrypt-query'
13+
import { EncryptOperation } from '@/encryption/operations/encrypt'
14+
import { encryptedColumn, encryptedTable } from '@/schema'
15+
import { describe, expect, it } from 'vitest'
16+
17+
const table = encryptedTable('null-guards-test', {
18+
metadata: encryptedColumn('metadata').searchableJson(),
19+
})
20+
21+
// Any truthy stand-in — the guard returns before the client is touched.
22+
const stubClient = {} as any
23+
24+
describe('runtime null guards (defense in depth)', () => {
25+
it('encrypt(null) short-circuits without an FFI call', async () => {
26+
const op = new EncryptOperation(stubClient, null, {
27+
column: table.metadata,
28+
table,
29+
})
30+
const result = await op.execute()
31+
if (result.failure) throw new Error(result.failure.message)
32+
expect(result.data).toBeNull()
33+
})
34+
35+
it('decrypt(null) short-circuits without an FFI call', async () => {
36+
const op = new DecryptOperation(stubClient, null)
37+
const result = await op.execute()
38+
if (result.failure) throw new Error(result.failure.message)
39+
expect(result.data).toBeNull()
40+
})
41+
42+
it('bulkEncrypt preserves null positions in mixed arrays', async () => {
43+
// Single null-only input — no FFI call should happen, the all-null
44+
// fast path returns a {data: null} placeholder per element.
45+
const op = new BulkEncryptOperation(
46+
stubClient,
47+
[{ id: 'a', plaintext: null }],
48+
{ column: table.metadata, table },
49+
)
50+
const result = await op.execute()
51+
if (result.failure) throw new Error(result.failure.message)
52+
expect(result.data).toEqual([{ id: 'a', data: null }])
53+
})
54+
55+
it('bulkDecrypt preserves null positions in mixed arrays', async () => {
56+
const op = new BulkDecryptOperation(stubClient, [{ id: 'a', data: null }])
57+
const result = await op.execute()
58+
if (result.failure) throw new Error(result.failure.message)
59+
expect(result.data).toEqual([{ id: 'a', data: null }])
60+
})
61+
62+
it('encryptQuery(null) short-circuits to { data: null }', async () => {
63+
const op = new EncryptQueryOperation(stubClient, null, {
64+
column: table.metadata,
65+
table,
66+
queryType: 'steVecSelector',
67+
returnType: 'composite-literal',
68+
} as any)
69+
const result = await op.execute()
70+
if (result.failure) throw new Error(result.failure.message)
71+
expect(result.data).toBeNull()
72+
})
73+
74+
it('encryptQuery(undefined) short-circuits to { data: null }', async () => {
75+
const op = new EncryptQueryOperation(stubClient, undefined, {
76+
column: table.metadata,
77+
table,
78+
queryType: 'steVecSelector',
79+
returnType: 'composite-literal',
80+
} as any)
81+
const result = await op.execute()
82+
if (result.failure) throw new Error(result.failure.message)
83+
expect(result.data).toBeNull()
84+
})
85+
86+
it('batchEncryptQuery preserves null/undefined slots position-stably', async () => {
87+
// All-null/undefined batch — no FFI call needed.
88+
const op = new BatchEncryptQueryOperation(stubClient, [
89+
{
90+
column: table.metadata,
91+
table,
92+
queryType: 'steVecSelector',
93+
returnType: 'composite-literal',
94+
value: null,
95+
} as any,
96+
{
97+
column: table.metadata,
98+
table,
99+
queryType: 'steVecSelector',
100+
returnType: 'composite-literal',
101+
value: undefined,
102+
} as any,
103+
])
104+
const result = await op.execute()
105+
if (result.failure) throw new Error(result.failure.message)
106+
expect(result.data).toEqual([null, null])
107+
})
108+
})

packages/stack/src/encryption/index.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -308,10 +308,17 @@ export class EncryptionClient {
308308
* .withLockContext(lockContext)
309309
* ```
310310
*
311+
* @remarks
312+
* The public input type rejects null, but at runtime `decrypt` will
313+
* short-circuit and return null when given a null ciphertext
314+
* (defense in depth for legacy / manually-NULLed DB rows reached via
315+
* casts or dynamic field walking). The narrow return type holds for
316+
* any caller that respects the input contract.
317+
*
311318
* @see {@link LockContext}
312319
* @see {@link DecryptOperation}
313320
*/
314-
decrypt(encryptedData: Encrypted | null): DecryptOperation {
321+
decrypt(encryptedData: Encrypted): DecryptOperation {
315322
return new DecryptOperation(this.client, encryptedData)
316323
}
317324

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,10 @@ import type {
1111
BulkEncryptPayload,
1212
BulkEncryptedData,
1313
Client,
14+
Encrypted,
1415
EncryptOptions,
1516
} from '@/types'
1617
import { createRequestLogger } from '@/utils/logger'
17-
import type { Encrypted } from '@/types'
1818
import { type Result, withResult } from '@byteslice/result'
1919
import { type JsPlaintext, encryptBulk } from '@cipherstash/protect-ffi'
2020
import { noClientError } from '../index'

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

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,12 @@ import { EncryptionOperation } from './base-operation'
1515
* Decrypts an encrypted payload using the provided client.
1616
* This is the type returned by the {@link EncryptionClient.decrypt | decrypt} method of the {@link EncryptionClient}.
1717
*/
18-
export class DecryptOperation extends EncryptionOperation<JsPlaintext | null> {
18+
export class DecryptOperation extends EncryptionOperation<JsPlaintext> {
1919
private client: Client
20-
// Widened to allow null so legacy / manually-NULLed rows can be
21-
// round-tripped through decrypt without misbehaving in protect-ffi.
20+
// Internally widened to allow null so the runtime guard below can
21+
// short-circuit on legacy / manually-NULLed rows. The public
22+
// `Encryption.decrypt()` signature still rejects null at the type
23+
// layer; this is defense in depth for direct construction.
2224
private encryptedData: Encrypted | null
2325

2426
constructor(client: Client, encryptedData: Encrypted | null) {
@@ -33,7 +35,7 @@ export class DecryptOperation extends EncryptionOperation<JsPlaintext | null> {
3335
return new DecryptOperationWithLockContext(this, lockContext)
3436
}
3537

36-
public async execute(): Promise<Result<JsPlaintext | null, EncryptionError>> {
38+
public async execute(): Promise<Result<JsPlaintext, EncryptionError>> {
3739
const log = createRequestLogger()
3840
log.set({
3941
op: 'decrypt',
@@ -47,7 +49,8 @@ export class DecryptOperation extends EncryptionOperation<JsPlaintext | null> {
4749
}
4850

4951
if (this.encryptedData === null) {
50-
return null
52+
// See encrypt.ts for the same defense-in-depth pattern.
53+
return null as unknown as JsPlaintext
5154
}
5255

5356
const { metadata } = this.getAuditData()
@@ -83,9 +86,7 @@ export class DecryptOperation extends EncryptionOperation<JsPlaintext | null> {
8386
}
8487
}
8588

86-
export class DecryptOperationWithLockContext extends EncryptionOperation<
87-
JsPlaintext | null
88-
> {
89+
export class DecryptOperationWithLockContext extends EncryptionOperation<JsPlaintext> {
8990
private operation: DecryptOperation
9091
private lockContext: LockContext
9192

@@ -99,7 +100,7 @@ export class DecryptOperationWithLockContext extends EncryptionOperation<
99100
}
100101
}
101102

102-
public async execute(): Promise<Result<JsPlaintext | null, EncryptionError>> {
103+
public async execute(): Promise<Result<JsPlaintext, EncryptionError>> {
103104
const log = createRequestLogger()
104105
log.set({
105106
op: 'decrypt',
@@ -115,7 +116,7 @@ export class DecryptOperationWithLockContext extends EncryptionOperation<
115116
}
116117

117118
if (encryptedData === null) {
118-
return null
119+
return null as unknown as JsPlaintext
119120
}
120121

121122
const { metadata } = this.getAuditData()

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

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,7 @@ import { EncryptionOperation } from './base-operation'
2020
/**
2121
* @internal Use {@link EncryptionClient.encryptQuery} instead.
2222
*/
23-
export class EncryptQueryOperation extends EncryptionOperation<
24-
EncryptedQueryResult | null
25-
> {
23+
export class EncryptQueryOperation extends EncryptionOperation<EncryptedQueryResult> {
2624
constructor(
2725
private client: Client,
2826
private plaintext: JsPlaintext | null | undefined,
@@ -44,7 +42,7 @@ export class EncryptQueryOperation extends EncryptionOperation<
4442
}
4543

4644
public async execute(): Promise<
47-
Result<EncryptedQueryResult | null, EncryptionError>
45+
Result<EncryptedQueryResult, EncryptionError>
4846
> {
4947
const log = createRequestLogger()
5048
log.set({
@@ -60,7 +58,9 @@ export class EncryptQueryOperation extends EncryptionOperation<
6058
return { data: null }
6159
}
6260

63-
const validationError = validateNumericValue(this.plaintext)
61+
const plaintext: JsPlaintext = this.plaintext
62+
63+
const validationError = validateNumericValue(plaintext)
6464
if (validationError?.failure) {
6565
log.emit()
6666
return { failure: validationError.failure }
@@ -75,18 +75,18 @@ export class EncryptQueryOperation extends EncryptionOperation<
7575
const { indexType, queryOp } = resolveIndexType(
7676
this.opts.column,
7777
this.opts.queryType,
78-
this.plaintext as JsPlaintext,
78+
plaintext,
7979
)
8080

8181
// Validate value/index compatibility
8282
assertValueIndexCompatibility(
83-
this.plaintext as JsPlaintext,
83+
plaintext,
8484
indexType,
8585
this.opts.column.getName(),
8686
)
8787

8888
const encrypted = await ffiEncryptQuery(this.client, {
89-
plaintext: this.plaintext as JsPlaintext,
89+
plaintext,
9090
column: this.opts.column.getName(),
9191
table: this.opts.table.tableName,
9292
indexType,
@@ -117,9 +117,7 @@ export class EncryptQueryOperation extends EncryptionOperation<
117117
/**
118118
* @internal Use {@link EncryptionClient.encryptQuery} with `.withLockContext()` instead.
119119
*/
120-
export class EncryptQueryOperationWithLockContext extends EncryptionOperation<
121-
EncryptedQueryResult | null
122-
> {
120+
export class EncryptQueryOperationWithLockContext extends EncryptionOperation<EncryptedQueryResult> {
123121
constructor(
124122
private client: Client,
125123
private plaintext: JsPlaintext | null | undefined,
@@ -132,7 +130,7 @@ export class EncryptQueryOperationWithLockContext extends EncryptionOperation<
132130
}
133131

134132
public async execute(): Promise<
135-
Result<EncryptedQueryResult | null, EncryptionError>
133+
Result<EncryptedQueryResult, EncryptionError>
136134
> {
137135
const log = createRequestLogger()
138136
log.set({
@@ -148,7 +146,9 @@ export class EncryptQueryOperationWithLockContext extends EncryptionOperation<
148146
return { data: null }
149147
}
150148

151-
const validationError = validateNumericValue(this.plaintext)
149+
const plaintext: JsPlaintext = this.plaintext
150+
151+
const validationError = validateNumericValue(plaintext)
152152
if (validationError?.failure) {
153153
log.emit()
154154
return { failure: validationError.failure }
@@ -171,18 +171,18 @@ export class EncryptQueryOperationWithLockContext extends EncryptionOperation<
171171
const { indexType, queryOp } = resolveIndexType(
172172
this.opts.column,
173173
this.opts.queryType,
174-
this.plaintext as JsPlaintext,
174+
plaintext,
175175
)
176176

177177
// Validate value/index compatibility
178178
assertValueIndexCompatibility(
179-
this.plaintext as JsPlaintext,
179+
plaintext,
180180
indexType,
181181
this.opts.column.getName(),
182182
)
183183

184184
const encrypted = await ffiEncryptQuery(this.client, {
185-
plaintext: this.plaintext as JsPlaintext,
185+
plaintext,
186186
column: this.opts.column.getName(),
187187
table: this.opts.table.tableName,
188188
indexType,

packages/stack/src/encryption/operations/encrypt.ts

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ import {
1717
import { noClientError } from '../index'
1818
import { EncryptionOperation } from './base-operation'
1919

20-
export class EncryptOperation extends EncryptionOperation<Encrypted | null> {
20+
export class EncryptOperation extends EncryptionOperation<Encrypted> {
2121
private client: Client
2222
// Internally widened to allow null so the runtime guard below can
2323
// short-circuit. The public `Encryption.encrypt()` signature still
@@ -45,7 +45,7 @@ export class EncryptOperation extends EncryptionOperation<Encrypted | null> {
4545
return new EncryptOperationWithLockContext(this, lockContext)
4646
}
4747

48-
public async execute(): Promise<Result<Encrypted | null, EncryptionError>> {
48+
public async execute(): Promise<Result<Encrypted, EncryptionError>> {
4949
const log = createRequestLogger()
5050
log.set({
5151
op: 'encrypt',
@@ -61,7 +61,13 @@ export class EncryptOperation extends EncryptionOperation<Encrypted | null> {
6161
}
6262

6363
if (this.plaintext === null) {
64-
return null
64+
// Defense in depth: the public `Encryption.encrypt()` signature
65+
// rejects null, but null can still arrive here via casts or
66+
// dynamic field walking. Return null directly so the result
67+
// matches DB NULL semantics rather than encrypting JSON null
68+
// into a SteVec. The cast acknowledges the type-narrow
69+
// contract at the public boundary.
70+
return null as unknown as Encrypted
6571
}
6672

6773
if (
@@ -115,7 +121,7 @@ export class EncryptOperation extends EncryptionOperation<Encrypted | null> {
115121
}
116122
}
117123

118-
export class EncryptOperationWithLockContext extends EncryptionOperation<Encrypted | null> {
124+
export class EncryptOperationWithLockContext extends EncryptionOperation<Encrypted> {
119125
private operation: EncryptOperation
120126
private lockContext: LockContext
121127

@@ -129,7 +135,7 @@ export class EncryptOperationWithLockContext extends EncryptionOperation<Encrypt
129135
}
130136
}
131137

132-
public async execute(): Promise<Result<Encrypted | null, EncryptionError>> {
138+
public async execute(): Promise<Result<Encrypted, EncryptionError>> {
133139
const { client, plaintext, column, table } = this.operation.getOperation()
134140

135141
const log = createRequestLogger()
@@ -147,7 +153,7 @@ export class EncryptOperationWithLockContext extends EncryptionOperation<Encrypt
147153
}
148154

149155
if (plaintext === null) {
150-
return null
156+
return null as unknown as Encrypted
151157
}
152158

153159
const { metadata } = this.getAuditData()

0 commit comments

Comments
 (0)